From fac4a9c23f8e9090b3dc7e26a8306d3a8034b4c9 Mon Sep 17 00:00:00 2001 From: Calvin Combs <66279577+comcalvi@users.noreply.github.com> Date: Thu, 14 Mar 2024 12:36:28 -0700 Subject: [PATCH] revert: prevent changeset diff for non-deployed stacks (#29485) reverts #29394, which prevented changeset creation during `cdk diff` if a stack did not exist. The lookup of the stack to check its existence is failing for customers that have CI/CD that won't assume the deploy role when running CDK diff. Long-term fix: delete the stack if it didn't exist before we created the changeset, but wait for its state to reach `DELETE_COMPLETE` to avoid problems with subsequent commands. Preserves changes from #29172 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../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;