From 56293ef10e1cde7390f31b79f98022b8d1cf4b9e Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 24 Sep 2023 01:42:35 +0900 Subject: [PATCH 1/9] fix(cli): No exception when stack with wrong cases is deployed --- packages/aws-cdk/lib/cdk-toolkit.ts | 10 ++++++++-- packages/aws-cdk/test/cdk-toolkit.test.ts | 13 +++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 52b458f692ffa..85079f6a0c0dd 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -558,16 +558,22 @@ export class CdkToolkit { // The stacks will have been ordered for deployment, so reverse them for deletion. stacks = stacks.reversed(); + const artifacts = stacks.stackArtifacts; + if (!options.force) { // eslint-disable-next-line max-len - const confirmed = await promptly.confirm(`Are you sure you want to delete: ${chalk.blue(stacks.stackArtifacts.map(s => s.hierarchicalId).join(', '))} (y/n)?`); + const confirmed = await promptly.confirm(`Are you sure you want to delete: ${chalk.blue(artifacts.map(s => s.hierarchicalId).join(', '))} (y/n)?`); if (!confirmed) { return; } } + if (artifacts.length === 0) { + throw new Error('Stack does not exist'); + } + const action = options.fromDeploy ? 'deploy' : 'destroy'; - for (const [index, stack] of stacks.stackArtifacts.entries()) { + for (const [index, stack] of artifacts.entries()) { success('%s: destroying... [%s/%s]', chalk.blue(stack.displayName), index+1, stacks.stackCount); try { await this.props.deployments.destroyStack({ diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index bb84e0fd6b4a7..f661d2b579d3c 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -602,6 +602,19 @@ describe('destroy', () => { }); }).resolves; }); + + test('fail on non-existent stack', async () => { + const toolkit = defaultToolkitSetup(); + + await expect(() => { + return toolkit.destroy({ + selector: { patterns: ['Test-Stack-X'] }, + exclusively: true, + force: true, + fromDeploy: true, + }); + }).rejects.toThrowError('Stack does not exist'); + }); }); describe('watch', () => { From bec1d0329d8165489922a2d54fa9586016d41e42 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 24 Sep 2023 01:56:41 +0900 Subject: [PATCH 2/9] Revert "fix(cli): No exception when stack with wrong cases is deployed" This reverts commit 56293ef10e1cde7390f31b79f98022b8d1cf4b9e. --- packages/aws-cdk/lib/cdk-toolkit.ts | 10 ++-------- packages/aws-cdk/test/cdk-toolkit.test.ts | 13 ------------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 85079f6a0c0dd..52b458f692ffa 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -558,22 +558,16 @@ export class CdkToolkit { // The stacks will have been ordered for deployment, so reverse them for deletion. stacks = stacks.reversed(); - const artifacts = stacks.stackArtifacts; - if (!options.force) { // eslint-disable-next-line max-len - const confirmed = await promptly.confirm(`Are you sure you want to delete: ${chalk.blue(artifacts.map(s => s.hierarchicalId).join(', '))} (y/n)?`); + const confirmed = await promptly.confirm(`Are you sure you want to delete: ${chalk.blue(stacks.stackArtifacts.map(s => s.hierarchicalId).join(', '))} (y/n)?`); if (!confirmed) { return; } } - if (artifacts.length === 0) { - throw new Error('Stack does not exist'); - } - const action = options.fromDeploy ? 'deploy' : 'destroy'; - for (const [index, stack] of artifacts.entries()) { + for (const [index, stack] of stacks.stackArtifacts.entries()) { success('%s: destroying... [%s/%s]', chalk.blue(stack.displayName), index+1, stacks.stackCount); try { await this.props.deployments.destroyStack({ diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index f661d2b579d3c..bb84e0fd6b4a7 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -602,19 +602,6 @@ describe('destroy', () => { }); }).resolves; }); - - test('fail on non-existent stack', async () => { - const toolkit = defaultToolkitSetup(); - - await expect(() => { - return toolkit.destroy({ - selector: { patterns: ['Test-Stack-X'] }, - exclusively: true, - force: true, - fromDeploy: true, - }); - }).rejects.toThrowError('Stack does not exist'); - }); }); describe('watch', () => { From 16b0babade9fd766c5f56b381c7da6f6f48a9770 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 24 Sep 2023 01:57:05 +0900 Subject: [PATCH 3/9] add a test --- packages/aws-cdk/test/cdk-toolkit.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index bb84e0fd6b4a7..f661d2b579d3c 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -602,6 +602,19 @@ describe('destroy', () => { }); }).resolves; }); + + test('fail on non-existent stack', async () => { + const toolkit = defaultToolkitSetup(); + + await expect(() => { + return toolkit.destroy({ + selector: { patterns: ['Test-Stack-X'] }, + exclusively: true, + force: true, + fromDeploy: true, + }); + }).rejects.toThrowError('Stack does not exist'); + }); }); describe('watch', () => { From c3963b10bcc520f920d003a3856f8668e7818da3 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 24 Sep 2023 03:18:55 +0900 Subject: [PATCH 4/9] using matchingPattern --- packages/aws-cdk/lib/cdk-toolkit.ts | 21 ++++++++++++++++++++- packages/aws-cdk/test/cdk-toolkit.test.ts | 4 ++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 52b458f692ffa..0aa72f4c87215 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -5,6 +5,7 @@ import * as chalk from 'chalk'; import * as chokidar from 'chokidar'; import * as fs from 'fs-extra'; import * as promptly from 'promptly'; +import * as semver from 'semver'; import { DeploymentMethod } from './api'; import { SdkProvider } from './api/aws-auth'; import { Bootstrapper, BootstrapEnvironmentOptions } from './api/bootstrap'; @@ -27,6 +28,8 @@ import { Concurrency, WorkGraph } from './util/work-graph'; import { WorkGraphBuilder } from './util/work-graph-builder'; import { AssetBuildNode, AssetPublishNode, StackNode } from './util/work-graph-types'; import { environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob } from '../lib/api/cxapp/environments'; +import { minimatch } from 'minimatch'; +import { versionNumber } from './version'; export interface CdkToolkitProps { @@ -769,7 +772,23 @@ export class CdkToolkit { defaultBehavior: DefaultSelection.OnlySingle, }); - // No validation + // cli tests use this to ensure tests do not depend on legacy behavior + // (otherwise they will fail in v2) + const disableLegacy = process.env.CXAPI_DISABLE_SELECT_BY_ID === '1'; + + const matchingPattern = (pattern: string, stack: cxapi.CloudFormationStackArtifact) => { + if (minimatch(stack.hierarchicalId, pattern)) { + return true; + } else if (!disableLegacy && stack.id === pattern && semver.major(versionNumber()) < 2) { + return true; + } + return false; + }; + + const notExistPatterns = selector.patterns.filter(p => !stacks.stackArtifacts.find(s => matchingPattern(p, s))); + if (notExistPatterns.length > 0) { + throw new Error(`Stacks not exist: ${notExistPatterns.join(', ')}`); + } return stacks; } diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index f661d2b579d3c..c15d42bbdf87b 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -608,12 +608,12 @@ describe('destroy', () => { await expect(() => { return toolkit.destroy({ - selector: { patterns: ['Test-Stack-X'] }, + selector: { patterns: ['Test-Stack-A/Test-Stack-C', 'Test-Stack-X', 'Test-Stack-Y'] }, exclusively: true, force: true, fromDeploy: true, }); - }).rejects.toThrowError('Stack does not exist'); + }).rejects.toThrowError('Stacks not exist: Test-Stack-X, Test-Stack-Y'); }); }); From 85284b2bc9990f6e39121d536cdc9118d0eba603 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Tue, 26 Sep 2023 21:06:23 +0900 Subject: [PATCH 5/9] update for selectStacksForDestroy --- packages/aws-cdk/lib/cdk-toolkit.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 0aa72f4c87215..c21ff3372546c 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -5,7 +5,6 @@ import * as chalk from 'chalk'; import * as chokidar from 'chokidar'; import * as fs from 'fs-extra'; import * as promptly from 'promptly'; -import * as semver from 'semver'; import { DeploymentMethod } from './api'; import { SdkProvider } from './api/aws-auth'; import { Bootstrapper, BootstrapEnvironmentOptions } from './api/bootstrap'; @@ -29,7 +28,6 @@ import { WorkGraphBuilder } from './util/work-graph-builder'; import { AssetBuildNode, AssetPublishNode, StackNode } from './util/work-graph-types'; import { environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob } from '../lib/api/cxapp/environments'; import { minimatch } from 'minimatch'; -import { versionNumber } from './version'; export interface CdkToolkitProps { @@ -772,20 +770,7 @@ export class CdkToolkit { defaultBehavior: DefaultSelection.OnlySingle, }); - // cli tests use this to ensure tests do not depend on legacy behavior - // (otherwise they will fail in v2) - const disableLegacy = process.env.CXAPI_DISABLE_SELECT_BY_ID === '1'; - - const matchingPattern = (pattern: string, stack: cxapi.CloudFormationStackArtifact) => { - if (minimatch(stack.hierarchicalId, pattern)) { - return true; - } else if (!disableLegacy && stack.id === pattern && semver.major(versionNumber()) < 2) { - return true; - } - return false; - }; - - const notExistPatterns = selector.patterns.filter(p => !stacks.stackArtifacts.find(s => matchingPattern(p, s))); + const notExistPatterns = selector.patterns.filter(p => !stacks.stackArtifacts.find(s => minimatch(s.hierarchicalId, p))); if (notExistPatterns.length > 0) { throw new Error(`Stacks not exist: ${notExistPatterns.join(', ')}`); } From 6563a5fc6744b080ae33535b914ae2c423d96683 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Tue, 26 Sep 2023 21:45:01 +0900 Subject: [PATCH 6/9] fix order of import --- packages/aws-cdk/lib/cdk-toolkit.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index c21ff3372546c..a6bbf29e335fc 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 { DeploymentMethod } from './api'; import { SdkProvider } from './api/aws-auth'; @@ -27,7 +28,6 @@ import { Concurrency, WorkGraph } from './util/work-graph'; import { WorkGraphBuilder } from './util/work-graph-builder'; import { AssetBuildNode, AssetPublishNode, StackNode } from './util/work-graph-types'; import { environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob } from '../lib/api/cxapp/environments'; -import { minimatch } from 'minimatch'; export interface CdkToolkitProps { From 27a39fcb63dbdf9da8fd22b11879fc9ca530454c Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:30:36 +0900 Subject: [PATCH 7/9] search for v1 --- packages/aws-cdk/lib/cdk-toolkit.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index a6bbf29e335fc..872850036a61d 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -6,6 +6,7 @@ import * as chokidar from 'chokidar'; import * as fs from 'fs-extra'; import { minimatch } from 'minimatch'; import * as promptly from 'promptly'; +import * as semver from 'semver'; import { DeploymentMethod } from './api'; import { SdkProvider } from './api/aws-auth'; import { Bootstrapper, BootstrapEnvironmentOptions } from './api/bootstrap'; @@ -28,6 +29,7 @@ import { Concurrency, WorkGraph } from './util/work-graph'; import { WorkGraphBuilder } from './util/work-graph-builder'; import { AssetBuildNode, AssetPublishNode, StackNode } from './util/work-graph-types'; import { environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob } from '../lib/api/cxapp/environments'; +import { versionNumber } from './version'; export interface CdkToolkitProps { @@ -770,7 +772,9 @@ export class CdkToolkit { defaultBehavior: DefaultSelection.OnlySingle, }); - const notExistPatterns = selector.patterns.filter(p => !stacks.stackArtifacts.find(s => minimatch(s.hierarchicalId, p))); + const notExistPatterns = selector.patterns.filter(pattern => !stacks.stackArtifacts.find(stack => + minimatch(stack.hierarchicalId, pattern) || (stack.id === pattern && semver.major(versionNumber()) < 2), + )); if (notExistPatterns.length > 0) { throw new Error(`Stacks not exist: ${notExistPatterns.join(', ')}`); } From 6b4dbe2104bc5f7c5be979b1a971934c81bcf2a0 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:38:16 +0900 Subject: [PATCH 8/9] change import order --- packages/aws-cdk/lib/cdk-toolkit.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 872850036a61d..49dda39aba448 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -28,8 +28,8 @@ import { validateSnsTopicArn } from './util/validate-notification-arn'; import { Concurrency, WorkGraph } from './util/work-graph'; import { WorkGraphBuilder } from './util/work-graph-builder'; import { AssetBuildNode, AssetPublishNode, StackNode } from './util/work-graph-types'; -import { environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob } from '../lib/api/cxapp/environments'; import { versionNumber } from './version'; +import { environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob } from '../lib/api/cxapp/environments'; export interface CdkToolkitProps { From 3dbc266fc070e095c306f5a7ed22db41bac5ca6b Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:22:23 +0900 Subject: [PATCH 9/9] cli-integ-tests --- .../cli-integ/tests/cli-integ-tests/cli.integtest.ts | 7 +++++++ packages/aws-cdk/lib/cdk-toolkit.ts | 2 +- packages/aws-cdk/test/cdk-toolkit.test.ts | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) 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 b5b55ba5a6a75..f6dd4809de60f 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 @@ -1209,6 +1209,13 @@ integTest('hotswap deployment supports Lambda function\'s description and enviro expect(deployOutput).toContain(`Lambda Function '${functionName}' hotswapped!`); })); +integTest('cdk destroy fails when 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])).rejects.toThrow('exited with error'); +})); + 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 49dda39aba448..5437ba95cdf6d 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -776,7 +776,7 @@ export class CdkToolkit { minimatch(stack.hierarchicalId, pattern) || (stack.id === pattern && semver.major(versionNumber()) < 2), )); if (notExistPatterns.length > 0) { - throw new Error(`Stacks not exist: ${notExistPatterns.join(', ')}`); + throw new Error(`Cannot run cdk destroy on stack(s) ${selector.patterns.join(', ')}. ${notExistPatterns.join(', ')} not exist.`); } return stacks; diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index c15d42bbdf87b..d19ad84ab7693 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -613,7 +613,7 @@ describe('destroy', () => { force: true, fromDeploy: true, }); - }).rejects.toThrowError('Stacks not exist: Test-Stack-X, Test-Stack-Y'); + }).rejects.toThrowError('Cannot run cdk destroy on stack(s) Test-Stack-A/Test-Stack-C, Test-Stack-X, Test-Stack-Y. Test-Stack-X, Test-Stack-Y not exist.'); }); });