From 834f12ecb8fa2c2614c50b6f4b7f472d51d0102e Mon Sep 17 00:00:00 2001 From: "Kenta Goto (k.goto)" <24818752+go-to-k@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:10:19 +0900 Subject: [PATCH] feat(cli): warn of non-existent stacks in `cdk destroy` (#32636) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Issue # (if applicable) Closes #32545. Fixes #27179. ### Reason for this change Once this [PR](https://github.com/aws/aws-cdk/pull/27921) was reverted by other cli-integ test regression. - revert PR: https://github.com/aws/aws-cdk/pull/29577 - the test for regression: https://github.com/aws/aws-cdk/blob/07ce8ecc42782475d099b89944571375341c28d3/packages/%40aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts#L190 But the regression was apparently due to a cause unrelated to that PR. That has been corrected in [this PR](https://github.com/aws/aws-cdk/pull/31107) (see: https://github.com/aws/aws-cdk/blob/v2.173.1/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts#L286-L291). Therefore, I submit the same PR again. ### Description of changes This PR for cli is to warn if stacks with wrong cases (=not exist) specified in `cdk destroy`. \* It does not display the message `Are you sure you want to delete:` if there is no matching stack. \* Even if the stack does not exist, `cdk destroy` will not fail, it will just print a warning. Actual Outputs: ![cdk-destroy-warn](https://github.com/user-attachments/assets/c0d70037-c863-4c78-bc22-8b51264393ac) ### Describe any new or updated permissions being added Nothing. ### Description of how you validated changes Both of unit tests and cli-integ tests. The changes were already approved in the last PR. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../tests/cli-integ-tests/cli.integtest.ts | 14 ++++ packages/aws-cdk/lib/cdk-toolkit.ts | 54 ++++++++++++++++ packages/aws-cdk/test/cdk-toolkit.test.ts | 64 +++++++++++++++++++ 3 files changed, 132 insertions(+) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index f4736584a065f..34526ece88265 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -2634,6 +2634,20 @@ integTest('hotswap ECS deployment respects properties override', withDefaultFixt expect(describeServicesResponse.services?.[0].deploymentConfiguration?.maximumPercent).toEqual(ecsMaximumHealthyPercent); })); +integTest('cdk destroy does not fail even if the stacks do not exist', withDefaultFixture(async (fixture) => { + const nonExistingStackName1 = 'non-existing-stack-1'; + const nonExistingStackName2 = 'non-existing-stack-2'; + + await expect(fixture.cdkDestroy([nonExistingStackName1, nonExistingStackName2])).resolves.not.toThrow(); +})); + +integTest('cdk destroy with no force option exits without prompt if the stacks do not exist', withDefaultFixture(async (fixture) => { + const nonExistingStackName1 = 'non-existing-stack-1'; + const nonExistingStackName2 = 'non-existing-stack-2'; + + await expect(fixture.cdk(['destroy', ...fixture.fullStackName([nonExistingStackName1, nonExistingStackName2])])).resolves.not.toThrow(); +})); + async function listChildren(parent: string, pred: (x: string) => Promise) { const ret = new Array(); for (const child of await fs.readdir(parent, { encoding: 'utf-8' })) { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 32f6616e5cdb6..64fa955912e4d 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -4,6 +4,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import * as chalk from 'chalk'; import * as chokidar from 'chokidar'; import * as fs from 'fs-extra'; +import { minimatch } from 'minimatch'; import * as promptly from 'promptly'; import * as uuid from 'uuid'; import { DeploymentMethod, SuccessfulDeployStackResult } from './api'; @@ -799,6 +800,16 @@ export class CdkToolkit { public async destroy(options: DestroyOptions) { let stacks = await this.selectStacksForDestroy(options.selector, options.exclusively); + await this.suggestStacks({ + selector: options.selector, + stacks, + exclusively: options.exclusively, + }); + if (stacks.stackArtifacts.length === 0) { + warning(`No stacks match the name(s): ${chalk.red(options.selector.patterns.join(', '))}`); + return; + } + // The stacks will have been ordered for deployment, so reverse them for deletion. stacks = stacks.reversed(); @@ -1162,6 +1173,49 @@ export class CdkToolkit { return stacks; } + private async suggestStacks(props: { + selector: StackSelector; + stacks: StackCollection; + exclusively?: boolean; + }) { + const assembly = await this.assembly(); + const selectorWithoutPatterns: StackSelector = { + ...props.selector, + allTopLevel: true, + patterns: [], + }; + const stacksWithoutPatterns = await assembly.selectStacks(selectorWithoutPatterns, { + extend: props.exclusively ? ExtendedStackSelection.None : ExtendedStackSelection.Downstream, + defaultBehavior: DefaultSelection.OnlySingle, + }); + + const patterns = props.selector.patterns.map(pattern => { + const notExist = !props.stacks.stackArtifacts.find(stack => + minimatch(stack.hierarchicalId, pattern), + ); + + const closelyMatched = notExist ? stacksWithoutPatterns.stackArtifacts.map(stack => { + if (minimatch(stack.hierarchicalId.toLowerCase(), pattern.toLowerCase())) { + return stack.hierarchicalId; + } + return; + }).filter((stack): stack is string => stack !== undefined) : []; + + return { + pattern, + notExist, + closelyMatched, + }; + }); + + for (const pattern of patterns) { + if (pattern.notExist) { + const closelyMatched = pattern.closelyMatched.length > 0 ? ` Do you mean ${chalk.blue(pattern.closelyMatched.join(', '))}?` : ''; + warning(`${chalk.red(pattern.pattern)} does not exist.${closelyMatched}`); + } + }; + } + /** * Validate the stacks for errors and warnings according to the CLI's current settings */ diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index 8dad7142baea7..5ae3a2f9a45ae 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -953,6 +953,70 @@ describe('destroy', () => { }); }).resolves; }); + + test('does not throw and warns if there are only non-existent stacks', async () => { + const toolkit = defaultToolkitSetup(); + + await toolkit.destroy({ + selector: { patterns: ['Test-Stack-X', 'Test-Stack-Y'] }, + exclusively: true, + force: true, + fromDeploy: true, + }); + + expect(flatten(stderrMock.mock.calls)).toEqual( + expect.arrayContaining([ + expect.stringMatching(/Test-Stack-X does not exist./), + expect.stringMatching(/Test-Stack-Y does not exist./), + expect.stringMatching(/No stacks match the name\(s\): Test-Stack-X, Test-Stack-Y/), + ]), + ); + }); + + test('does not throw and warns if there is a non-existent stack and the other exists', async () => { + const toolkit = defaultToolkitSetup(); + + await toolkit.destroy({ + selector: { patterns: ['Test-Stack-X', 'Test-Stack-B'] }, + exclusively: true, + force: true, + fromDeploy: true, + }); + + expect(flatten(stderrMock.mock.calls)).toEqual( + expect.arrayContaining([ + expect.stringMatching(/Test-Stack-X does not exist./), + ]), + ); + expect(flatten(stderrMock.mock.calls)).not.toEqual( + expect.arrayContaining([ + expect.stringMatching(/Test-Stack-B does not exist./), + ]), + ); + expect(flatten(stderrMock.mock.calls)).not.toEqual( + expect.arrayContaining([ + expect.stringMatching(/No stacks match the name\(s\)/), + ]), + ); + }); + + test('does not throw and suggests valid names if there is a non-existent but closely matching stack', async () => { + const toolkit = defaultToolkitSetup(); + + await toolkit.destroy({ + selector: { patterns: ['test-stack-b'] }, + exclusively: true, + force: true, + fromDeploy: true, + }); + + expect(flatten(stderrMock.mock.calls)).toEqual( + expect.arrayContaining([ + expect.stringMatching(/test-stack-b does not exist. Do you mean Test-Stack-B?/), + expect.stringMatching(/No stacks match the name\(s\): test-stack-b/), + ]), + ); + }); }); describe('watch', () => {