prevent changeset diff for non-deployed stacks

aws · Mar 7, 2024 · 8338c2a · 8338c2a
1 parent 4af0dfc
commit 8338c2a
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 106 deletions.
diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts
@@ -37,6 +37,7 @@ 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
@@ -46,6 +47,7 @@ export function fullDiff(
   currentTemplate: { [key: string]: any },
   newTemplate: { [key: string]: any },
   changeSet?: CloudFormation.DescribeChangeSetOutput,
+  isImport?: boolean,
 ): types.TemplateDiff {
 
   normalize(currentTemplate);
@@ -55,6 +57,9 @@ export function fullDiff(
     filterFalsePositivies(theDiff, changeSet);
     addImportInformation(theDiff, changeSet);
   }
+  if (isImport) {
+    addImportInformation(theDiff);
+  }
 
   return theDiff;
 }
@@ -209,13 +214,25 @@ function deepCopy(x: any): any {
   return x;
 }
 
-function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
-  const imports = findResourceImports(changeSet);
-  diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
-    if (imports.includes(logicalId)) {
+/**
+ * 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.
       change.isImport = true;
-    }
-  });
+    });
+  }
 }
 
 function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {

diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts
@@ -135,7 +135,14 @@ export class CdkToolkit {
         throw new Error(`There is no file at ${options.templatePath}`);
       }
 
-      const changeSet = options.changeSet ? await createDiffChangeSet({
+      const stackExistsOptions = {
+        stack: stacks.firstStack,
+        deployName: stacks.firstStack.stackName,
+      };
+
+      const stackExists = await this.props.deployments.stackExists(stackExistsOptions);
+
+      const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({
         stack: stacks.firstStack,
         uuid: uuid.v4(),
         willExecute: false,
@@ -167,13 +174,23 @@ export class CdkToolkit {
           removeNonImportResources(stack);
         }
 
-        const changeSet = options.changeSet ? await createDiffChangeSet({
+        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({
           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[stacks.firstStack.stackName]), // should this be stack?
           resourcesToImport,
           stream,
         }) : undefined;
@@ -182,10 +199,11 @@ 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)) > 0 ? 1 : 0)
-          : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream) > 0 ? 1 : 0);
+          : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, !!resourcesToImport) > 0 ? 1 : 0);
 
         diffs += stackCount + nestedStackCount;
       }

diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts
@@ -23,9 +23,10 @@ export function printStackDiff(
   context: number,
   quiet: boolean,
   changeSet?: CloudFormation.DescribeChangeSetOutput,
-  stream?: cfnDiff.FormatStream): number {
+  stream?: cfnDiff.FormatStream,
+  isImport?: boolean): 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;

diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts
@@ -12,100 +12,6 @@ let cloudExecutable: MockCloudExecutable;
 let cloudFormation: jest.Mocked<Deployments>;
 let toolkit: CdkToolkit;
 
-describe('imports', () => {
-  beforeEach(() => {
-    jest.spyOn(cfn, 'createDiffChangeSet').mockImplementation(async () => {
-      return {
-        Changes: [
-          {
-            ResourceChange: {
-              Action: 'Import',
-              LogicalResourceId: 'Queue',
-            },
-          },
-          {
-            ResourceChange: {
-              Action: 'Import',
-              LogicalResourceId: 'Bucket',
-            },
-          },
-          {
-            ResourceChange: {
-              Action: 'Import',
-              LogicalResourceId: 'Queue2',
-            },
-          },
-        ],
-      };
-    });
-    cloudExecutable = new MockCloudExecutable({
-      stacks: [{
-        stackName: 'A',
-        template: {
-          Resources: {
-            Queue: {
-              Type: 'AWS::SQS::Queue',
-            },
-            Queue2: {
-              Type: 'AWS::SQS::Queue',
-            },
-            Bucket: {
-              Type: 'AWS::S3::Bucket',
-            },
-          },
-        },
-      }],
-    });
-
-    cloudFormation = instanceMockFrom(Deployments);
-
-    toolkit = new CdkToolkit({
-      cloudExecutable,
-      deployments: cloudFormation,
-      configuration: cloudExecutable.configuration,
-      sdkProvider: cloudExecutable.sdkProvider,
-    });
-
-    // Default implementations
-    cloudFormation.readCurrentTemplateWithNestedStacks.mockImplementation((_stackArtifact: CloudFormationStackArtifact) => {
-      return Promise.resolve({
-        deployedTemplate: {},
-        nestedStackCount: 0,
-      });
-    });
-    cloudFormation.deployStack.mockImplementation((options) => Promise.resolve({
-      noOp: true,
-      outputs: {},
-      stackArn: '',
-      stackArtifact: options.stack,
-    }));
-  });
-
-  test('imports', async () => {
-    // GIVEN
-    const buffer = new StringWritable();
-
-    // 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
-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);
-  });
-});
-
 describe('non-nested stacks', () => {
   beforeEach(() => {
     cloudExecutable = new MockCloudExecutable({