Skip to content

fix(dashboards,synthetics_private_locations): drop stale restricted_roles under new flag (4/4)#632

Merged
michael-richey merged 1 commit into
drop-unresolvable-principals-monitors-syntheticsfrom
drop-unresolvable-principals-dashboards-spl
Jul 15, 2026
Merged

fix(dashboards,synthetics_private_locations): drop stale restricted_roles under new flag (4/4)#632
michael-richey merged 1 commit into
drop-unresolvable-principals-monitors-syntheticsfrom
drop-unresolvable-principals-dashboards-spl

Conversation

@michael-richey

@michael-richey michael-richey commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Context

Stacked PR 4 of 4. Base: drop-unresolvable-principals-monitors-synthetics (PR #631). Completes the rollout.

This PR

Applies the flat-list drop-aware filter to the two remaining resource types with a restricted_roles list:

  • dashboards: restricted_roles
  • synthetics_private_locations: metadata.restricted_roles

Both now filter stale roles under --drop-unresolvable-principals while keeping widget/other connections on the generic path. An emptied list is a normal resource-connection failure: it skips by default, while --skip-failed-resource-connections=true intentionally continues with the explicit unrestricted-destination ERROR, risk metric, and end-of-run ERROR summary introduced in PR #629.

Testing

The dashboard and synthetics-private-location suites cover drop, default empty-risk failure, suppressed empty-risk propagation, middle-element regression, flag-off, and no-op-when-absent behavior. Focused tests green; ruff clean.

@michael-richey
michael-richey force-pushed the drop-unresolvable-principals-monitors-synthetics branch from 518113c to 27fd78f Compare July 14, 2026 21:32
@michael-richey
michael-richey force-pushed the drop-unresolvable-principals-dashboards-spl branch 2 times, most recently from a16b6d4 to 6630c1e Compare July 15, 2026 12:40
@michael-richey
michael-richey force-pushed the drop-unresolvable-principals-monitors-synthetics branch 2 times, most recently from de6e34d to bd82dbe Compare July 15, 2026 13:37
@michael-richey
michael-richey force-pushed the drop-unresolvable-principals-dashboards-spl branch from 6630c1e to 095704b Compare July 15, 2026 13:37
@michael-richey
michael-richey marked this pull request as ready for review July 15, 2026 13:40
@michael-richey
michael-richey requested a review from a team as a code owner July 15, 2026 13:40
@michael-richey
michael-richey requested a review from Copilot July 15, 2026 13:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends the --drop-unresolvable-principals rollout to the remaining resource types that carry a flat restricted_roles list, ensuring stale role references can be dropped (when enabled) while preserving the existing “empty restriction = access-elevation risk” failure/escalation behavior.

Changes:

  • Adds drop-aware connect_resources() overrides for Dashboards (restricted_roles) and Synthetics Private Locations (metadata.restricted_roles), while keeping non-access-control connections on the generic find_attr/connect_id path.
  • Introduces focused unit tests covering flag-off hard-fail, flag-on drop/keep behavior, empty-list risk signaling, suppressed connection-failure escalation, and middle-element drop regression cases for both resource types.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
datadog_sync/model/dashboards.py Adds a drop-aware connect_resources() override that filters stale restricted_roles while preserving other widget dependency connections via find_attr.
datadog_sync/model/synthetics_private_locations.py Adds a drop-aware connect_resources() override that filters stale roles inside metadata.restricted_roles while keeping the generic connection path for other future connections.
tests/unit/test_dashboards.py Adds a dedicated test suite validating dashboards’ restricted_roles drop/empty-risk behavior under the new flag and skip behavior.
tests/unit/test_synthetics_private_locations.py Adds new unit tests validating SPL metadata.restricted_roles drop/empty-risk behavior and regression coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@michael-richey
michael-richey force-pushed the drop-unresolvable-principals-dashboards-spl branch from 095704b to e971f5e Compare July 15, 2026 14:09
@michael-richey
michael-richey force-pushed the drop-unresolvable-principals-monitors-synthetics branch from bd82dbe to 3699f48 Compare July 15, 2026 14:09
…oles under new flag

Completes the rollout by applying the flat-list drop-aware filter to the two remaining
resource types with a `restricted_roles` list:

- dashboards: `restricted_roles`
- synthetics_private_locations: `metadata.restricted_roles`

Both previously had trivial pass-through `connect_id` overrides; they now get a
`connect_resources` override that filters stale roles under `--drop-unresolvable-principals`
while keeping widget/other connections on the generic path and preserving the empty-list
access-elevation hard-fail. Adds a new unit test module for synthetics_private_locations.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@michael-richey
michael-richey force-pushed the drop-unresolvable-principals-dashboards-spl branch from e971f5e to 11880bd Compare July 15, 2026 14:11
@michael-richey
michael-richey force-pushed the drop-unresolvable-principals-monitors-synthetics branch from 3699f48 to 63c0e2a Compare July 15, 2026 14:11
@michael-richey
michael-richey marked this pull request as draft July 15, 2026 14:28
@michael-richey
michael-richey marked this pull request as ready for review July 15, 2026 14:29
@michael-richey
michael-richey merged commit 39a27da into drop-unresolvable-principals-monitors-synthetics Jul 15, 2026
3 checks passed
@michael-richey
michael-richey deleted the drop-unresolvable-principals-dashboards-spl branch July 15, 2026 17:44
michael-richey added a commit that referenced this pull request Jul 15, 2026
…s under new flag (3/4) (#631)

* fix(monitors,synthetics_tests): drop stale restricted_roles/principals under new flag

Applies the same drop-aware pattern (PR1 helpers) to the two resource types that carry
both a flat `restricted_roles` list and a `restriction_policy.bindings.principals` composite:

- monitors: `restricted_roles` + `restriction_policy.bindings.principals`
- synthetics_tests: `options.restricted_roles` + `restriction_policy.bindings.principals`

Each `connect_resources` override keeps all non-access-control connections on the generic
path, filters access-control references through the shared helpers, and preserves the
empty-binding/empty-list access-elevation hard-fail. `extract_source_ids` left unaffected
(documented). Behavior is unchanged when `--drop-unresolvable-principals` is off.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(dashboards,synthetics_private_locations): drop stale restricted_roles under new flag (#632)

Completes the rollout by applying the flat-list drop-aware filter to the two remaining
resource types with a `restricted_roles` list:

- dashboards: `restricted_roles`
- synthetics_private_locations: `metadata.restricted_roles`

Both previously had trivial pass-through `connect_id` overrides; they now get a
`connect_resources` override that filters stale roles under `--drop-unresolvable-principals`
while keeping widget/other connections on the generic path and preserving the empty-list
access-elevation hard-fail. Adds a new unit test module for synthetics_private_locations.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
michael-richey added a commit that referenced this pull request Jul 15, 2026
…the whole policy (2/4) (#630)

* fix(restriction_policies): drop stale principals instead of skipping the whole policy

Overrides `connect_resources` so that, under `--drop-unresolvable-principals`, a principal
absent from both destination and source state (permanently gone — e.g. a role deleted
before the org's first-ever import) is dropped from its binding and the policy still syncs,
instead of the entire restriction policy being skipped on a single dead reference.

- Principals present in source but not destination keep today's hard-fail/retry behavior.
- If dropping empties a binding whose source list was non-empty, the resource is still
  skipped and flagged as an access-elevation risk (ERROR + metric), governed by
  `--skip-failed-resource-connections` like any other connection failure.
- The "id" (dashboard/slo/notebook) connections keep the generic path unchanged.
- `extract_source_ids` is intentionally left unaffected (documented) so minimize-reads
  lazy-loading can still resolve which principals to check.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(monitors,synthetics_tests): drop stale restricted_roles/principals under new flag (3/4) (#631)

* fix(monitors,synthetics_tests): drop stale restricted_roles/principals under new flag

Applies the same drop-aware pattern (PR1 helpers) to the two resource types that carry
both a flat `restricted_roles` list and a `restriction_policy.bindings.principals` composite:

- monitors: `restricted_roles` + `restriction_policy.bindings.principals`
- synthetics_tests: `options.restricted_roles` + `restriction_policy.bindings.principals`

Each `connect_resources` override keeps all non-access-control connections on the generic
path, filters access-control references through the shared helpers, and preserves the
empty-binding/empty-list access-elevation hard-fail. `extract_source_ids` left unaffected
(documented). Behavior is unchanged when `--drop-unresolvable-principals` is off.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(dashboards,synthetics_private_locations): drop stale restricted_roles under new flag (#632)

Completes the rollout by applying the flat-list drop-aware filter to the two remaining
resource types with a `restricted_roles` list:

- dashboards: `restricted_roles`
- synthetics_private_locations: `metadata.restricted_roles`

Both previously had trivial pass-through `connect_id` overrides; they now get a
`connect_resources` override that filters stale roles under `--drop-unresolvable-principals`
while keeping widget/other connections on the generic path and preserving the empty-list
access-elevation hard-fail. Adds a new unit test module for synthetics_private_locations.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
michael-richey added a commit that referenced this pull request Jul 15, 2026
…ffolding (1/4) (#629)

* feat: add --drop-unresolvable-principals flag and drop-aware connection scaffolding

Adds the opt-in `--drop-unresolvable-principals` flag plus the shared, inert-when-off
machinery that later PRs wire into individual resource models. No behavior changes when
the flag is absent (the default).

- New `--drop-unresolvable-principals` CLI option (in `_diffs_options`, so it applies to
  sync/diffs/migrate) threaded through the `Configuration` dataclass.
- `BaseResource._resolve_or_drop`: three-way resolver distinguishing destination-present,
  source-present ("not yet synced", hard-fail as today), and permanently-gone
  (absent from both — droppable only under the flag).
- Shared `_filter_stale_binding_principals` / `_filter_stale_flat_roles` /
  `_raise_connection_error_if_any` helpers (rebuild lists rather than mutating in place,
  avoiding the enumerate/index-shift bug).
- `ResourceConnectionError.empty_binding_risk` flag for the access-elevation case.
- New `Counter` buckets (stale drops, empty-binding risks) + reset, surfaced by
  `_emit_apply_summary`; `config.counter` back-reference so models can record drops.
- `_apply_resource_cb` adds a `risk:empty_restriction_policy` metric tag when a binding
  emptied out.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: report suppressed empty-binding connection risks

* fix: recheck destination after lazy state load

* refactor: name resource connection results

* fix(restriction_policies): drop stale principals instead of skipping the whole policy (2/4) (#630)

* fix(restriction_policies): drop stale principals instead of skipping the whole policy

Overrides `connect_resources` so that, under `--drop-unresolvable-principals`, a principal
absent from both destination and source state (permanently gone — e.g. a role deleted
before the org's first-ever import) is dropped from its binding and the policy still syncs,
instead of the entire restriction policy being skipped on a single dead reference.

- Principals present in source but not destination keep today's hard-fail/retry behavior.
- If dropping empties a binding whose source list was non-empty, the resource is still
  skipped and flagged as an access-elevation risk (ERROR + metric), governed by
  `--skip-failed-resource-connections` like any other connection failure.
- The "id" (dashboard/slo/notebook) connections keep the generic path unchanged.
- `extract_source_ids` is intentionally left unaffected (documented) so minimize-reads
  lazy-loading can still resolve which principals to check.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(monitors,synthetics_tests): drop stale restricted_roles/principals under new flag (3/4) (#631)

* fix(monitors,synthetics_tests): drop stale restricted_roles/principals under new flag

Applies the same drop-aware pattern (PR1 helpers) to the two resource types that carry
both a flat `restricted_roles` list and a `restriction_policy.bindings.principals` composite:

- monitors: `restricted_roles` + `restriction_policy.bindings.principals`
- synthetics_tests: `options.restricted_roles` + `restriction_policy.bindings.principals`

Each `connect_resources` override keeps all non-access-control connections on the generic
path, filters access-control references through the shared helpers, and preserves the
empty-binding/empty-list access-elevation hard-fail. `extract_source_ids` left unaffected
(documented). Behavior is unchanged when `--drop-unresolvable-principals` is off.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(dashboards,synthetics_private_locations): drop stale restricted_roles under new flag (#632)

Completes the rollout by applying the flat-list drop-aware filter to the two remaining
resource types with a `restricted_roles` list:

- dashboards: `restricted_roles`
- synthetics_private_locations: `metadata.restricted_roles`

Both previously had trivial pass-through `connect_id` overrides; they now get a
`connect_resources` override that filters stale roles under `--drop-unresolvable-principals`
while keeping widget/other connections on the generic path and preserving the empty-list
access-elevation hard-fail. Adds a new unit test module for synthetics_private_locations.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants