Skip to content

feat(aws-cdk): allow disabling deployment CloudTrail lookups - #2089

Open
proton0210 wants to merge 1 commit into
aws:mainfrom
proton0210:feature/cloudtrail-opt-out-2084
Open

proton0210 wants to merge 1 commit into
aws:mainfrom
proton0210:feature/cloudtrail-opt-out-2084

Conversation

@proton0210

Copy link
Copy Markdown
Contributor

Related to #2084; implements the opt-out discussed there while retaining the existing default.

Failed deployments automatically look up CloudTrail events, which can add unrelated or not-yet-delivered context. Add cdk deploy --no-cloudtrail to skip those lookups while keeping CloudFormation failure messages and other resource diagnostics. The equivalent "cloudtrail": false setting in cdk.json persists the preference; an explicit CLI flag overrides it. Programmatic deployments accept cloudTrail: false.

The option reaches change-set preparation, classic deployment, delegated change-set execution, and the failure diagnoser. CloudTrail remains enabled by default, including for explicit cdk diagnose. CLI parser, input types, conversion helpers, and type registry were regenerated from their source configuration.

Validation:

  • 232 focused tests passed across the CLI and toolkit packages. Coverage includes default behavior, flag syntax without consuming stack arguments, configuration precedence, deployment option forwarding, delegated execution, and retaining the original failure message with zero CloudTrail lookup requests.
  • Both affected packages compile; lint passes for changed TypeScript files. API Extractor and toolkit resource bundling pass.
  • All four generated CLI outputs reproduce byte-for-byte. Built CLI help exposes the option. Deployment I/O tests pass with and without color support.
  • The broader toolkit sweep passed 132 suites; eight suites affected by missing generated resources, the VM-module runtime flag, or fixture paths containing spaces passed on setup-corrected reruns (165 tests). All 140 toolkit suites have been validated.
  • The full CLI sweep was also attempted. Local init/migrate tests require a .NET SDK that is not installed; unquoted fixture paths and the existing color-sensitive I/O snapshot issue in fix(toolkit-lib): whitespace is not trimmed from colored messages #2047 also affected that initial run. Follow-up checks used a path without spaces and color support. Live AWS deployment tests were not run; this change is covered at the mocked SDK boundary.

Checklist

  • This change contains a major version upgrade for a dependency and I confirm all breaking changes are addressed
  • Unit tests and documentation added/updated
  • Generated files updated through the generator
  • Existing behavior retained when no opt-out is provided

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@maintainer-for-aws

Copy link
Copy Markdown

Automated review

A maintainer will still review this — treat the notes below as a starting point.

📌 Commit 84604b4

This PR (feat(aws-cdk): allow disabling deployment CloudTrail lookups) adds a new opt-out boolean preference — CLI flag --cloudtrail/--no-cloudtrail, config key cloudtrail, programmatic option cloudTrail — that threads from the CLI and configuration layers through cdk-toolkit.ts, toolkit.ts, and deployments.ts down to CloudFormationStackDiagnoser, where addCloudTrailContext() early-returns when the value is explicitly false. CloudTrail diagnostics remain on by default.

The change is clean, additive, and backwards-compatible. The guard keys specifically off === false, so undefined (the default for existing users and the explicit cdk diagnose path) preserves today's behavior, and default settings seed cloudtrail: true. The full wiring chain is consistent, including the lowercase-to-camelCase hand-off in cli.ts, and the generated CLI files track the single cli-config.ts source edit. Negative tests back each layer, covering forwarding, the skipped lookup, and config-vs-CLI precedence.

No issues found on this change. One product decision a maintainer may wish to confirm sits outside engineering review: the linked request asked for CloudTrail off by default, while this ships it as an on-by-default opt-out.

🔴 0 blocking · 🟡 0 recommended · ⚪ 0 optional


Generated automatically. React 👍 or 👎 to tell us whether this review helped, so we can improve these reviews.

@rittneje

Copy link
Copy Markdown

@proton0210 I really think this feature ought to be disabled by default for cdk deploy. Or frankly, always disabled for cdk deploy. As I mentioned in the issue report, there is basically zero chance it will ever grab the correct logs right after the deployment, as this is way too soon.

Instead, it should just tell people to run cdk diagnose in a few minutes. (By the way, that command is apparently undocumented?)

This branch is waiting to be deployed

1 failed and 1 waiting deployments
integ-approval — 84604b48 Waiting Oct 25, 2026 by proton0210 via prepare #7456
automation — 84604b48 Deployed Oct 25, 2026 by proton0210 via Triage Pull Requests #2246
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants