TON-710/711/712: structured instrumentation policy statements with overlap filtering and size-based chunking#336
Open
fanny-jiang wants to merge 6 commits into
Conversation
…hunks
Fetch full API attributes, render ${AccountId}/${Partition} placeholders,
and attach rendered statement chunks directly, falling back to the legacy
per-chunk action lists when policy_statements isn't present.
This comment has been minimized.
This comment has been minimized.
…d permissions Only drop an instrumentation action when an existing standard-integration statement grants an equivalent-or-broader permission across Action, Resource, and Condition. The role-creation path filters against the permissions it just attached; the add-on path can't verify what's on the role, so it passes no standard statements and filtering is a no-op.
…nstrumentation policies Chunk structured policy_statements by managed-policy size limit, validate the full candidate chunk set (size + attachment quota) against the role's existing non-instrumentation policies before cleaning up and re-attaching.
fanny-jiang
changed the title
- use boto3 paginator for list_attached_role_policies, matching the existing pattern in datadog_agentless_api_call.py - inline the single-caller _create_and_attach_policy wrapper - chunk_statements tracks running document size incrementally instead of re-serializing the whole accumulated chunk each iteration - filter_instrumentation_statements precomputes standard Action sets once instead of scanning a list per action per standard statement
Contributor
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
…card matching Validate structured policy_statements (Effect/Action/Resource/Condition, including blank-string checks) before they can trigger cleanup of previously-attached instrumentation policies, and replace fnmatch with an IAM-correct action-glob matcher to avoid over-matching on bracket-class syntax fnmatch grants but IAM does not. Also trims duplicate validation/policy-JSON logic and a couple of overly-verbose comments.
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
1 participant
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.
Note: Please remember to review the contribution guidelines
if you have not yet done so.
What does this PR do?
Reworks how
attach_integration_permissions.py(and the inlined copy indatadog_integration_permissions.yaml) builds the instrumentation IAM policies attached to the Datadog integration role:policy_statementsfrom the Datadog API over the legacy flatpermissionslist, rendering${AccountId}/${Partition}placeholders in them. Falls back to the legacypermissionschunks (wrapped asAllow/*-resource statements) whenpolicy_statementsis absent.filter_instrumentation_statements), comparing Action, Resource, and Condition so an action is only dropped when an existing standard-integration statement grants an equivalent-or-broader permission — not just because the action name happens to also appear, unconditioned, elsewhere.chunk_statements, 6144-char AWS managed-policy limit) without splitting a statement's Action away from its Resource/Condition, and preflight-validate the resulting chunk count against the account's managed-policy-attachment quota (validate_statement_chunks) before touching anything currently attached.Motivation
The Datadog API can now return fully-formed IAM policy statements (with real
Resourcescoping) instead of just flat permission lists, letting generated policies be scoped tighter than the previousResource: "*"fallback. Once statements carry real scoping, they need to be filtered against what's already granted by the standard permissions (to avoid redundant/oversized policies) and safely chunked/validated against AWS's per-policy size and per-role attachment limits, since the unchunked structured statements are no longer pre-sized by the server the way the legacypermissionsshape was.Testing Guidelines
Added unit tests in
attach_integration_permissions_test.pycovering:render_placeholders(string/list/dict recursion, non-placeholder passthrough)resolve_instrumentation_statement_chunks(legacy fallback, explicit-emptypolicy_statementsnot treated as absent, preference for structured statements over legacy permissions)filter_instrumentation_statements(Action/Resource/Condition-aware overlap detection)chunk_statements/validate_statement_chunks(size-based chunking boundaries, oversize-chunk and attachment-quota preflight failures)Additional Notes
None.