feat(cli): gc can ignore user-verified unauthorized non-cdk stacks #850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open

dgandhi62 wants to merge 7 commits into aws:main from dgandhi62:feat-ignore-skip-stacks

Contributor

dgandhi62 commented Sep 17, 2025 •

edited

Loading

Fixes #640

The gc command currently fails when encountering AccessDenied errors on non-CDK stacks during garbage collection. This PR adds a new option --unauth-native-cfn-stacks-to-skip to handle this situation.

Users can specify multiple stack names or glob patterns to skip when unauthorized access errors occur. When stacks matching the provided patterns encounter access denied errors, the command shows a confirmation prompt asking whether to skip these stacks and continue with garbage collection. The default response is 'no' to ensure users explicitly confirm the action. It remains the user's responsibility to ensure that the stacks entered are non-cdk stacks.

The 'no' default was chosen over 'yes' to prevent unintended behavior until a future --yes flag is implemented for the cdk-cli. While a shorter alias was considered for the option name, it was rejected to avoid user confusion.

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

dgandhi62 had a problem deploying to integ-approval

September 17, 2025 18:23

— with

GitHub Actions Failure

aws-cdk-automation enabled auto-merge

September 17, 2025 18:24

github-actions bot added effort/small feature-request p2 labels

aws-cdk-automation requested a review from a team

September 17, 2025 18:24

codecov-commenter commented Sep 17, 2025 •

edited

Loading

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.74%. Comparing base (75b8256) to head (87054d9).

Files with missing lines	Patch %	Lines
packages/aws-cdk/lib/cli/cdk-toolkit.ts	88.88%	1 Missing ⚠️
packages/aws-cdk/lib/cli/cli.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #850      +/-   ##
==========================================
+ Coverage   83.23%   83.74%   +0.50%     
==========================================
  Files          71       71              
  Lines       10396    10407      +11     
  Branches     1306     1317      +11     
==========================================
+ Hits         8653     8715      +62     
+ Misses       1705     1651      -54     
- Partials       38       41       +3

Flag	Coverage Δ
suite.unit	`83.74% <81.81%> (+0.50%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dgandhi62 changed the title ~~feat: add stack filtering options to cdk gc command~~ feat: add stack filtering options to gc command

kaizencc requested changes

View reviewed changes

Contributor

kaizencc left a comment

also missing a readme entry. this definitely needs it because it is a very tricky property to add and without the proper documentation, users will use it incorrectly and accidentally delete important assets.

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/garbage-collector.ts Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts Outdated Show resolved Hide resolved

packages/aws-cdk/lib/cli/cdk-toolkit.ts Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/garbage-collector.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/garbage-collector.ts Outdated Show resolved Hide resolved

packages/aws-cdk/lib/cli/cli.ts Outdated

    
                        createdBufferDays: args['created-buffer-days'],

                        bootstrapStackName: args.toolkitStackName ?? args.bootstrapStackName,

                        confirm: args.confirm,

                        ignoreStacks: args['ignore-stacks'],

Contributor

kaizencc Sep 18, 2025

missing a cli integ test for this to ensure that this is actually exposed via the cli. integ test should deploy 2 stacks with an asset each, one stack with a prefix, and then verify that gc with ignore stacks actually does delete the asset of the stack with prefix.

kaizencc changed the title ~~feat: add stack filtering options to gc command~~ feat(cli): gc can ignore user-verified non-cdk stacks

dgandhi62 force-pushed the feat-ignore-skip-stacks branch from f00d0f1 to 65b3bf1 Compare

September 22, 2025 01:33

dgandhi62 had a problem deploying to integ-approval

September 22, 2025 01:33

— with

GitHub Actions Failure

mrgrain reviewed

View reviewed changes

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/garbage-collector.ts Outdated Show resolved Hide resolved

mrgrain previously requested changes

View reviewed changes

Contributor

mrgrain left a comment

option names

dgandhi62 had a problem deploying to integ-approval

September 22, 2025 14:04

— with

GitHub Actions Failure


          feat(cli): add skip-unauthorized-stacks-when-noncdk option for garbag…

248ad08

…e collection

- Removed old functionality on ignore-stacks & skip-unauthorized-stacks
- Add --skip-unauthorized-stacks-when-noncdk CLI option to handle AccessDenied errors for non-cdk stack names entered by user
- Update interfaces and constructors with new parameter

dgandhi62 force-pushed the feat-ignore-skip-stacks branch from bf76d06 to 248ad08 Compare

September 23, 2025 21:26

dgandhi62 had a problem deploying to integ-approval

September 23, 2025 21:26

— with

GitHub Actions Failure

dgandhi62 changed the title ~~feat(cli): gc can ignore user-verified non-cdk stacks~~ feat(cli): gc can ignore user-verified unauthorized non-cdk stacks

kaizencc requested changes

View reviewed changes

Contributor

kaizencc left a comment

also definitely needs a readme update :)

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/garbage-collector.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/garbage-collector.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/garbage-collector.ts Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts Outdated Show resolved Hide resolved


          feat(gc): improve unauthorized stack handling with batch prompts and …

140fcd9

…CI support

- Replace individual warnings with single batch prompt for unauthorized stacks
- Add CDK_GC_AUTO_APPROVE_UNAUTHORIZED env var for CI/CD automation
- Add CI detection to fail fast instead of hanging on user prompts
- Add performance optimization with early return for empty assets

dgandhi62 had a problem deploying to integ-approval

September 26, 2025 15:56

— with

GitHub Actions Failure


          Add environment variable to test for ci environments

7aed817

dgandhi62 had a problem deploying to integ-approval

September 26, 2025 16:05

— with

GitHub Actions Failure


          fix(test): add CI detection for garbage collection tests

698f26d

- Remove global environment variable from test setup and add it locally to test
- Add CI detection in failing tests

dgandhi62 had a problem deploying to integ-approval

September 26, 2025 17:04

— with

GitHub Actions Failure

kaizencc requested changes

View reviewed changes

Contributor

kaizencc left a comment

few more comments. this one is tricky!

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/garbage-collector.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts Outdated Show resolved Hide resolved

mrgrain dismissed their stale review

October 2, 2025 08:15

outdated

mrgrain reviewed

View reviewed changes

packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts Outdated Show resolved Hide resolved


          Merge branch 'aws:main' into feat-ignore-skip-stacks

dd4cf80

dgandhi62 requested a deployment to integ-approval

October 10, 2025 19:52

— with

GitHub Actions Waiting


          feat(gc): change default to 'no' for unauthorized stacks and rename o…

eb74966

…ption to --unauth-native-cfn-stacks-to-skip

- Change default response from 'yes' to 'no' when prompting to skip unauthorized stacks
- Rename parameter from skipUnauthorizedStacksWhenNonCdk to unauthNativeCfnStacksToSkip
- Remove wrong ci/cd detection logic

dgandhi62 requested a deployment to integ-approval

October 10, 2025 21:20

— with

GitHub Actions Waiting


          fix: add option to cli-config and remove it from parse-command-line-a…

87054d9

…rguments

dgandhi62 requested a deployment to integ-approval

October 21, 2025 05:20

— with

GitHub Actions Waiting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/small feature-request p2