feat: --drop-unresolvable-principals flag + drop-aware connection scaffolding (1/4)#629
Draft
michael-richey wants to merge 1 commit into
Draft
feat: --drop-unresolvable-principals flag + drop-aware connection scaffolding (1/4)#629michael-richey wants to merge 1 commit into
michael-richey wants to merge 1 commit into
Conversation
…on 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>
7db6366 to
a7514ad
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Stacked PR 1 of 4. Base:
main.In cross-datacenter managed-sync, some orgs reference roles/principals deleted from the source org before the org's first-ever import — so they have no source-state file and can never resolve. Today one such dead reference makes
connect_resources()raiseResourceConnectionErrorand skip the entire resource's access-control sync (confirmed via a dashboard whose whole restriction policy silently never applied because of one deleted role).This series adds an opt-in
--drop-unresolvable-principalsflag: drop a principal absent from both destination and source (permanently gone), keep syncing the rest, but still hard-fail loudly if dropping would empty a binding/list (access-elevation guard). Default-off = byte-for-byte unchanged behavior.This PR (scaffolding — inert when the flag is off)
--drop-unresolvable-principalsCLI option (in_diffs_options→ sync/diffs/migrate), threaded throughConfiguration.BaseResource._resolve_or_drop: three-way resolver (destination-present / source-present "not yet synced" → hard-fail as today / absent-from-both → droppable only under the flag)._filter_stale_binding_principals,_filter_stale_flat_roles,_raise_connection_error_if_anyhelpers. Lists are rebuilt (never index-mutated) to avoid the enumerate/index-shift skip bug.ResourceConnectionError.empty_binding_risk; newCounterbuckets + reset;config.counterback-reference; end-of-run summary lines;risk:empty_restriction_policymetric tag in_apply_resource_cb.No model opts into this yet — pure no-op at runtime until PRs 2–4.
Testing
New unit tests for the helper (all three branches + exception propagation), Counter fields/reset, the exception attribute, summary emission, the metric tag, and CLI flag round-trip. Full unit suite green;
ruffclean.🤖 Generated with Claude Code