Skip to content

[TON-513] Fix workflow instrumentation permissions#314

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
fanny/TON-513/workflow-instrumentation-permissions
May 27, 2026
Merged

[TON-513] Fix workflow instrumentation permissions#314
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
fanny/TON-513/workflow-instrumentation-permissions

Conversation

@fanny-jiang
Copy link
Copy Markdown
Contributor

What does this PR do?

Forwards InstrumentationResourceTypes and DatadogSite from the Datadog UI workflow templates into the nested integration role stack. This lets workflow-launched stacks attach the Agent instrumentation IAM permissions selected during onboarding, matching the existing main_v2.yaml behavior.

Also bumps aws_quickstart to v4.11.1 and documents the change.

Testing

  • cd aws_quickstart && python -B -S -m unittest attach_integration_permissions_test.py -v
  • cd aws_quickstart && python -B -S -m unittest datadog_agentless_api_call_test.py -v
  • git diff --check
  • cfn-lint aws_quickstart/main_workflow.yaml aws_quickstart/main_extended_workflow.yaml aws_quickstart/main_v2.yaml aws_quickstart/datadog_integration_role.yaml

Note: cfn-lint still reports pre-existing warnings for CloudSecurityPostureManagementPermissions, unused DisableMetricCollection, and redundant ForwarderStack dependencies.

@fanny-jiang fanny-jiang marked this pull request as ready for review May 27, 2026 20:39
@fanny-jiang fanny-jiang requested a review from a team as a code owner May 27, 2026 20:39
Copy link
Copy Markdown
Contributor

@gpalmz gpalmz left a comment

Choose a reason for hiding this comment

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

LGTM. Left one note about parity with main_extended.yaml.

ExternalId: !Ref ExternalId
IAMRoleName: !Ref IAMRoleName
ResourceCollectionPermissions: !If [ResourceCollectionPermissions, true, false]
InstrumentationResourceTypes: !Join [",", !Ref InstrumentationResourceTypes]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

main_extended.yaml looks like it has the same gap. Its DatadogIntegrationRoleStack invocation doesn't forward InstrumentationResourceTypes or DatadogSite to the nested stack either, so users launching that template manually (instead of via the Datadog UI) would hit the same missing-perms behavior this PR fixes for the workflow path. Worth folding in here, or filing a follow-up?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I'll fold the fix in with this PR

@fanny-jiang
Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351
Copy link
Copy Markdown

gh-worker-devflow-routing-ef8351 Bot commented May 27, 2026

View all feedbacks in Devflow UI.

2026-05-27 20:53:32 UTC ℹ️ Start processing command /merge


2026-05-27 20:53:38 UTC ℹ️ MergeQueue: waiting for PR to be ready

This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
It will be added to the queue as soon as checks pass and/or get approvals. View in MergeQueue UI.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.


2026-05-27 21:30:09 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in master is approximately 4m (p90).


2026-05-27 21:34:29 UTC ℹ️ MergeQueue: This merge request was merged

@fanny-jiang
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

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.

3 participants