fix(cli): prevent changeset diff for non-deployed stacks (#29394)

### Reason for this change

When a stack does not exist in CloudFormation, creating a changeset makes an empty `REVIEW_IN_PROGRESS` stack. We then call `delete-stack` to clean up the empty stack. However, this can cause a race condition with a deploy call. 

### Description of changes

This change prevents changeset diffs for stacks that do not yet exist in CloudFormation. This overrides the changeset diff flag. This change also adds logic for migrate stacks in the old diff logic to represent resource imports without needing the changeset present.

### Description of how you validated changes

Testing with new stacks only uses changeset diffs once the stack is deployed. Testing with new migrate stacks only uses changeset diffs once deployed. Pre-deployment the resources correctly show as imports.  

Note: the deleted test assumes the diff will be calculated using the mocked changeset. The new logic avoids the changeset, so the test is no longer relevant.

Closes #29265.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: scanlonp

Diff for: 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 for: packages/aws-cdk/lib/cdk-toolkit.ts

@@ -136,7 +136,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,
@@ -168,13 +175,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;
@@ -183,10 +200,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 for: 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 for: 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({