From 5a485e59b6ac908be436a8ae2cea77266ca546ef Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 14 Mar 2024 10:26:05 -0700 Subject: [PATCH 01/13] revert --- .../cloudformation-diff/lib/diff-template.ts | 29 ++++--------------- packages/aws-cdk/lib/cdk-toolkit.ts | 26 +++-------------- packages/aws-cdk/lib/diff.ts | 5 ++-- 3 files changed, 12 insertions(+), 48 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 284bb3a8d5f46..4bf1d29f57570 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -37,7 +37,6 @@ const DIFF_HANDLERS: HandlerRegistry = { * @param currentTemplate the current state of the stack. * @param newTemplate the target state of the stack. * @param changeSet the change set for this stack. - * @param isImport if the stack is importing resources (a migrate stack). * * @returns a +types.TemplateDiff+ object that represents the changes that will happen if * a stack which current state is described by +currentTemplate+ is updated with @@ -47,7 +46,6 @@ export function fullDiff( currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }, changeSet?: CloudFormation.DescribeChangeSetOutput, - isImport?: boolean, ): types.TemplateDiff { normalize(currentTemplate); @@ -57,9 +55,6 @@ export function fullDiff( filterFalsePositives(theDiff, changeSet); addImportInformation(theDiff, changeSet); } - if (isImport) { - addImportInformation(theDiff); - } return theDiff; } @@ -214,25 +209,13 @@ function deepCopy(x: any): any { return x; } -/** - * Sets import flag to true for resource imports. - * When the changeset parameter is not set, the stack is a new migrate stack, - * so all resource changes are imports. - */ -function addImportInformation(diff: types.TemplateDiff, changeSet?: CloudFormation.DescribeChangeSetOutput) { - if (changeSet) { - const imports = findResourceImports(changeSet); - diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => { - if (imports.includes(logicalId)) { - change.isImport = true; - } - }); - } else { - diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => { - logicalId; // dont know how to get past warning that this variable is not used. +function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) { + const imports = findResourceImports(changeSet); + diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => { + if (imports.includes(logicalId)) { change.isImport = true; - }); - } + } + }); } function filterFalsePositives(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 05aeee002c068..91ef83426d587 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -136,14 +136,7 @@ export class CdkToolkit { throw new Error(`There is no file at ${options.templatePath}`); } - const stackExistsOptions = { - stack: stacks.firstStack, - deployName: stacks.firstStack.stackName, - }; - - const stackExists = await this.props.deployments.stackExists(stackExistsOptions); - - const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({ + const changeSet = options.changeSet ? await createDiffChangeSet({ stack: stacks.firstStack, uuid: uuid.v4(), willExecute: false, @@ -175,23 +168,13 @@ export class CdkToolkit { removeNonImportResources(stack); } - const stackExistsOptions = { - stack, - deployName: stack.stackName, - }; - - const stackExists = await this.props.deployments.stackExists(stackExistsOptions); - - // if the stack does not already exist, do not do a changeset - // this prevents race conditions between deleting the dummy changeset stack and deploying the real changeset stack - // migrate stacks that import resources will not previously exist and default to old diff logic - const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({ + const changeSet = options.changeSet ? await createDiffChangeSet({ stack, uuid: uuid.v4(), deployments: this.props.deployments, willExecute: false, sdkProvider: this.props.sdkProvider, - parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), // should this be stack? + parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), resourcesToImport, stream, }) : undefined; @@ -200,11 +183,10 @@ export class CdkToolkit { stream.write('Parameters and rules created during migration do not affect resource configuration.\n'); } - // pass a boolean to print if the stack is a migrate stack in order to set all resource diffs to import const stackCount = options.securityOnly ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet))) - : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, nestedStacks, !!resourcesToImport)); + : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, nestedStacks)); diffs += stackCount; } diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts index b49f288fc2534..ce9c64eb4b443 100644 --- a/packages/aws-cdk/lib/diff.ts +++ b/packages/aws-cdk/lib/diff.ts @@ -26,10 +26,9 @@ export function printStackDiff( quiet: boolean, changeSet?: CloudFormation.DescribeChangeSetOutput, stream: cfnDiff.FormatStream = process.stderr, - nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }, - isImport?: boolean): number { + nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number { - let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet, isImport); + let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet); // detect and filter out mangled characters from the diff let filteredChangesCount = 0; From c8d5d9fb2ffaff7cea406259349894be50e746a0 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 14 Mar 2024 14:17:48 -0700 Subject: [PATCH 02/13] diff does not create a changeset for a non-existant stack --- .../aws-cdk/lib/api/util/cloudformation.ts | 21 +++++--------- packages/aws-cdk/lib/cdk-toolkit.ts | 28 ++++++++++++------- packages/aws-cdk/test/diff.test.ts | 19 +++++++++++++ 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts index b61991bf51699..3176b88ab621b 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation.ts @@ -364,7 +364,7 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp } async function createChangeSet(options: CreateChangeSetOptions): Promise { - await cleanupOldChangeset(options.exists, options.changeSetName, options.stack.stackName, options.cfn); + await cleanupOldChangeset(options.changeSetName, options.stack.stackName, options.cfn); debug(`Attempting to create ChangeSet with name ${options.changeSetName} for stack ${options.stack.stackName}`); @@ -388,23 +388,16 @@ async function createChangeSet(options: CreateChangeSetOptions): Promise { }); cloudFormation = instanceMockFrom(Deployments); + cloudFormation.stackExists = jest.fn().mockReturnValue(Promise.resolve(true)); toolkit = new CdkToolkit({ cloudExecutable, @@ -306,6 +307,24 @@ describe('non-nested stacks', () => { expect(buffer.data.trim()).not.toContain('There were no differences'); expect(exitCode).toBe(0); }); + + test('diff does not check for stack existance when --no-changeset is passed', async () => { + // GIVEN + const buffer = new StringWritable(); + + // WHEN + const exitCode = await toolkit.diff({ + stackNames: ['A', 'A'], + stream: buffer, + fail: false, + quiet: true, + changeSet: false, + }); + + // THEN + expect(exitCode).toBe(0); + expect(cloudFormation.stackExists).not.toHaveBeenCalled(); + }); }); describe('nested stacks', () => { From c49b58949d5968183661f56fd6d577a84197c2cf Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 14 Mar 2024 14:24:02 -0700 Subject: [PATCH 03/13] fixed template case --- packages/aws-cdk/lib/cdk-toolkit.ts | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index b08be9f5cb352..7a67eaf2e23f9 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -136,15 +136,23 @@ export class CdkToolkit { throw new Error(`There is no file at ${options.templatePath}`); } - const changeSet = options.changeSet ? await createDiffChangeSet({ - stack: stacks.firstStack, - uuid: uuid.v4(), - willExecute: false, - deployments: this.props.deployments, - sdkProvider: this.props.sdkProvider, - parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), - stream, - }) : undefined; + let changeSet = undefined; + + if (options.changeSet) { + const stackExists = await this.props.deployments.stackExists({ + stack: stacks.firstStack, + deployName: stacks.firstStack.stackName, + }); + changeSet = stackExists ? await createDiffChangeSet({ + stack: stacks.firstStack, + uuid: uuid.v4(), + willExecute: false, + deployments: this.props.deployments, + sdkProvider: this.props.sdkProvider, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), + stream, + }) : undefined; + } const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' })); diffs = options.securityOnly From f1177006005b84ca32b97a48e7206de6edbb6489 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 14 Mar 2024 14:27:55 -0700 Subject: [PATCH 04/13] refactor --- packages/aws-cdk/lib/cdk-toolkit.ts | 46 +++++++++++++++++------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 7a67eaf2e23f9..a2c44e1767921 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -143,15 +143,19 @@ export class CdkToolkit { stack: stacks.firstStack, deployName: stacks.firstStack.stackName, }); - changeSet = stackExists ? await createDiffChangeSet({ - stack: stacks.firstStack, - uuid: uuid.v4(), - willExecute: false, - deployments: this.props.deployments, - sdkProvider: this.props.sdkProvider, - parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), - stream, - }) : undefined; + if (stackExists) { + changeSet = await createDiffChangeSet({ + stack: stacks.firstStack, + uuid: uuid.v4(), + deployments: this.props.deployments, + willExecute: false, + sdkProvider: this.props.sdkProvider, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), + stream, + }); + } else { + debug(`the stack '${stacks.firstStack.stackName}' does not exist, skipping changeset creation.`); + } } const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' })); @@ -183,16 +187,20 @@ export class CdkToolkit { stack: stack, deployName: stack.stackName, }); - changeSet = stackExists ? await createDiffChangeSet({ - stack, - uuid: uuid.v4(), - deployments: this.props.deployments, - willExecute: false, - sdkProvider: this.props.sdkProvider, - parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), - resourcesToImport, - stream, - }) : undefined; + if (stackExists) { + changeSet = await createDiffChangeSet({ + stack, + uuid: uuid.v4(), + deployments: this.props.deployments, + willExecute: false, + sdkProvider: this.props.sdkProvider, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), + resourcesToImport, + stream, + }); + } else { + debug(`the stack '${stack.stackName}' does not exist, skipping changeset creation.`); + } } if (resourcesToImport) { From 0e22c53d13fc6f17fe172a876616d63de9c437fc Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 15 Mar 2024 09:16:22 -0700 Subject: [PATCH 05/13] works for migrate stacks --- packages/aws-cdk/lib/api/util/cloudformation.ts | 6 +++++- packages/aws-cdk/lib/cdk-toolkit.ts | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts index 3176b88ab621b..41d170c3ba386 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation.ts @@ -397,7 +397,11 @@ export async function cleanupOldChangeset(changeSetName: string, stackName: stri // Delete any existing change sets generated by CDK since change set names must be unique. // The delete request is successful as long as the stack exists (even if the change set does not exist). debug(`Removing existing change set with name ${changeSetName} if it exists`); - await cfn.deleteChangeSet({ StackName: stackName, ChangeSetName: changeSetName }).promise(); + try { + await cfn.deleteChangeSet({ StackName: stackName, ChangeSetName: changeSetName }).promise(); + } catch (e) { + debug(`no changeset to clean up for stack ${stackName}`); + } } /** diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index a2c44e1767921..bab08de98dc91 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -187,19 +187,19 @@ export class CdkToolkit { stack: stack, deployName: stack.stackName, }); - if (stackExists) { + if (stackExists || resourcesToImport) { changeSet = await createDiffChangeSet({ stack, uuid: uuid.v4(), deployments: this.props.deployments, willExecute: false, sdkProvider: this.props.sdkProvider, - parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), + parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), resourcesToImport, stream, }); } else { - debug(`the stack '${stack.stackName}' does not exist, skipping changeset creation.`); + debug(`the stack '${stack.stackName}' does not exist and we are not importing resources, skipping changeset creation.`); } } From 3cf8c60499c17a6dfb4cc69ad2f01931ba490d6d Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 15 Mar 2024 09:58:48 -0700 Subject: [PATCH 06/13] tests --- packages/aws-cdk/test/diff.test.ts | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index 956efb4d76c3a..c56102e9ed7a8 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -67,7 +67,6 @@ describe('imports', () => { }); cloudFormation = instanceMockFrom(Deployments); - cloudFormation.stackExists = jest.fn().mockReturnValue(Promise.resolve(true)); toolkit = new CdkToolkit({ cloudExecutable, @@ -95,9 +94,36 @@ describe('imports', () => { fs.rmSync('migrate.json'); }); - test('imports', async () => { + test('imports render correctly for a nonexistant stack', async () => { + // GIVEN + const buffer = new StringWritable(); + cloudFormation.stackExists = jest.fn().mockReturnValue(Promise.resolve(false)); + + // WHEN + const exitCode = await toolkit.diff({ + stackNames: ['A'], + stream: buffer, + changeSet: true, + }); + + // THEN + const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ''); + expect(plainTextOutput).toContain(`Stack A +Parameters and rules created during migration do not affect resource configuration. +Resources +[←] AWS::SQS::Queue Queue import +[←] AWS::SQS::Queue Queue2 import +[←] AWS::S3::Bucket Bucket import +`); + + expect(buffer.data.trim()).toContain('✨ Number of stacks with differences: 1'); + expect(exitCode).toBe(0); + }); + + test('imports render correctly for an existing stack', async () => { // GIVEN const buffer = new StringWritable(); + cloudFormation.stackExists = jest.fn().mockReturnValue(Promise.resolve(true)); // WHEN const exitCode = await toolkit.diff({ From be42b4ec61a51acd82093d59c0a96a2799719329 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 15 Mar 2024 10:09:36 -0700 Subject: [PATCH 07/13] cosmetic --- packages/aws-cdk/lib/cdk-toolkit.ts | 1 + packages/aws-cdk/test/diff.test.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index bab08de98dc91..745fdf90ca333 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -183,6 +183,7 @@ export class CdkToolkit { let changeSet = undefined; if (options.changeSet) { + // only perform this check if we're going to make a changeset. This check requires permissions that --no-changeset users might not have. const stackExists = await this.props.deployments.stackExists({ stack: stack, deployName: stack.stackName, diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index c56102e9ed7a8..79429830cbf6e 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -334,7 +334,7 @@ describe('non-nested stacks', () => { expect(exitCode).toBe(0); }); - test('diff does not check for stack existance when --no-changeset is passed', async () => { + test('diff does not check for stack existence when --no-changeset is passed', async () => { // GIVEN const buffer = new StringWritable(); From 1b8f8ef621794524ddd39f2030bda23235fd7f51 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 15 Mar 2024 12:10:50 -0700 Subject: [PATCH 08/13] don't create a changeset if the stack doesn't exist... --- .../@aws-cdk/cloudformation-diff/lib/diff-template.ts | 10 ++++++++++ packages/aws-cdk/lib/cdk-toolkit.ts | 6 +++--- packages/aws-cdk/lib/diff.ts | 4 +++- packages/aws-cdk/test/diff.test.ts | 6 ++++-- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 4bf1d29f57570..5252087f1f215 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -46,6 +46,7 @@ export function fullDiff( currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }, changeSet?: CloudFormation.DescribeChangeSetOutput, + isImport?: boolean, ): types.TemplateDiff { normalize(currentTemplate); @@ -55,6 +56,9 @@ export function fullDiff( filterFalsePositives(theDiff, changeSet); addImportInformation(theDiff, changeSet); } + if (isImport) { + makeAllResourceChangesImports(theDiff); + } return theDiff; } @@ -218,6 +222,12 @@ function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormatio }); } +function makeAllResourceChangesImports(diff: types.TemplateDiff) { + diff.resources.forEachDifference((_logicalId: string, change: types.ResourceDifference) => { + change.isImport = true; + }); +} + function filterFalsePositives(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) { const replacements = findResourceReplacements(changeSet); diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 745fdf90ca333..b06ac98d42211 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -161,7 +161,7 @@ export class CdkToolkit { const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' })); diffs = options.securityOnly ? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, changeSet)) - : printStackDiff(template, stacks.firstStack.template, strict, contextLines, quiet, changeSet, stream); + : printStackDiff(template, stacks.firstStack.template, strict, contextLines, quiet, changeSet, false, stream); } else { // Compare N stacks against deployed templates for (const stack of stacks.stackArtifacts) { @@ -188,7 +188,7 @@ export class CdkToolkit { stack: stack, deployName: stack.stackName, }); - if (stackExists || resourcesToImport) { + if (stackExists) { changeSet = await createDiffChangeSet({ stack, uuid: uuid.v4(), @@ -211,7 +211,7 @@ export class CdkToolkit { const stackCount = options.securityOnly ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet))) - : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, nestedStacks)); + : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, !!resourcesToImport, stream, nestedStacks)); diffs += stackCount; } diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts index ce9c64eb4b443..cc5cb3d3567fa 100644 --- a/packages/aws-cdk/lib/diff.ts +++ b/packages/aws-cdk/lib/diff.ts @@ -25,10 +25,11 @@ export function printStackDiff( context: number, quiet: boolean, changeSet?: CloudFormation.DescribeChangeSetOutput, + isImport?: boolean, stream: cfnDiff.FormatStream = process.stderr, nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number { - let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet); + let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet, isImport); // detect and filter out mangled characters from the diff let filteredChangesCount = 0; @@ -82,6 +83,7 @@ export function printStackDiff( context, quiet, undefined, + isImport, stream, nestedStack.nestedStackTemplates, ); diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index 79429830cbf6e..e120103390087 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -94,7 +94,7 @@ describe('imports', () => { fs.rmSync('migrate.json'); }); - test('imports render correctly for a nonexistant stack', async () => { + test('imports render correctly for a nonexistant stack without creating a changeset', async () => { // GIVEN const buffer = new StringWritable(); cloudFormation.stackExists = jest.fn().mockReturnValue(Promise.resolve(false)); @@ -108,6 +108,7 @@ describe('imports', () => { // THEN const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ''); + expect(cfn.createDiffChangeSet).not.toHaveBeenCalled(); expect(plainTextOutput).toContain(`Stack A Parameters and rules created during migration do not affect resource configuration. Resources @@ -120,7 +121,7 @@ Resources expect(exitCode).toBe(0); }); - test('imports render correctly for an existing stack', async () => { + test('imports render correctly for an existing stack and diff creates a changeset', async () => { // GIVEN const buffer = new StringWritable(); cloudFormation.stackExists = jest.fn().mockReturnValue(Promise.resolve(true)); @@ -134,6 +135,7 @@ Resources // THEN const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ''); + expect(cfn.createDiffChangeSet).toHaveBeenCalled(); expect(plainTextOutput).toContain(`Stack A Parameters and rules created during migration do not affect resource configuration. Resources From fc9f74eb2c9c76e7dca153a912eb2a188f7afedf Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 15 Mar 2024 12:17:56 -0700 Subject: [PATCH 09/13] unneeded --- packages/aws-cdk/lib/api/util/cloudformation.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts index 41d170c3ba386..3176b88ab621b 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation.ts @@ -397,11 +397,7 @@ export async function cleanupOldChangeset(changeSetName: string, stackName: stri // Delete any existing change sets generated by CDK since change set names must be unique. // The delete request is successful as long as the stack exists (even if the change set does not exist). debug(`Removing existing change set with name ${changeSetName} if it exists`); - try { - await cfn.deleteChangeSet({ StackName: stackName, ChangeSetName: changeSetName }).promise(); - } catch (e) { - debug(`no changeset to clean up for stack ${stackName}`); - } + await cfn.deleteChangeSet({ StackName: stackName, ChangeSetName: changeSetName }).promise(); } /** From 2a4819bd46d5e0e35543ff04902eb4c80c8ab514 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 15 Mar 2024 12:21:21 -0700 Subject: [PATCH 10/13] logging --- 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 b06ac98d42211..fb075a8d3a3de 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -200,7 +200,7 @@ export class CdkToolkit { stream, }); } else { - debug(`the stack '${stack.stackName}' does not exist and we are not importing resources, skipping changeset creation.`); + debug(`the stack '${stack.stackName}' does not exist, skipping changeset creation.`); } } From 0edd606287141270f9d8bbd378f72ab2bfec54f2 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 15 Mar 2024 12:22:32 -0700 Subject: [PATCH 11/13] branching was wrong --- packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 5252087f1f215..76f7f161f892d 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -55,8 +55,7 @@ export function fullDiff( if (changeSet) { filterFalsePositives(theDiff, changeSet); addImportInformation(theDiff, changeSet); - } - if (isImport) { + } else if (isImport) { makeAllResourceChangesImports(theDiff); } From 898e429cfed2ed71bcddc6d8a29fca60d3ac1e7b Mon Sep 17 00:00:00 2001 From: Calvin Combs <66279577+comcalvi@users.noreply.github.com> Date: Fri, 15 Mar 2024 13:32:52 -0700 Subject: [PATCH 12/13] logging Co-authored-by: Parker Scanlon <69879391+scanlonp@users.noreply.github.com> --- 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 fb075a8d3a3de..f0d350ec08839 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -200,7 +200,7 @@ export class CdkToolkit { stream, }); } else { - debug(`the stack '${stack.stackName}' does not exist, skipping changeset creation.`); + debug(`the stack '${stack.stackName}' has not been deployed to CloudFormation, skipping changeset creation.`); } } From 9b2178c1ddd2098cc1945384fe956d387477987e Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 15 Mar 2024 13:35:09 -0700 Subject: [PATCH 13/13] logging --- 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 fb075a8d3a3de..627328fcd35f0 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -154,7 +154,7 @@ export class CdkToolkit { stream, }); } else { - debug(`the stack '${stacks.firstStack.stackName}' does not exist, skipping changeset creation.`); + debug(`the stack '${stacks.firstStack.stackName}' has not been deployed to CloudFormation, skipping changeset creation.`); } }