From 00cdd2a2188d146af8b8df998e97da91c77dc270 Mon Sep 17 00:00:00 2001
From: Steven Swartz <stevenswartz@live.ca>
Date: Tue, 9 Feb 2021 10:39:50 -0500
Subject: [PATCH] feat(cli): change set name is now a constant, and
 --no-execute will always produce one (even if empty) (#12683)

closes #11075

Adds two commands to the `deploy` CLI command to make it easier to externally execute change sets when using the `--no-execute` flag:

`--change-set-name`: Optional name of the CloudFormation change set to create, instead of using a random one. An external script or the CodePipeline CloudFormation action can use this name to later deploy the changes.

`--retain-empty-change-set`:  Optionally retain empty change sets instead of deleting them. This is useful for the (requested) CodePipeline use case, since the CodePipeline CloudFormation action always requires a change set, even if it's empty.

Questions for reviewer:
- Is `--retain-empty-change-set` needed? One workaround for the CodePipeline use case could be for users to write a lambda that generates the required empty change set whenever CDK doesn't generate one. Another idea would be to automatically retain change sets when using `--no-execute` to avoid this extra CLI flag, but this would be a small change in behavior.
- Are the new integration tests overkill? Also should unit tests be added or in-place of the integration tests?
---
 packages/aws-cdk/README.md                    | 12 +++++
 packages/aws-cdk/lib/api/deploy-stack.ts      | 34 ++++++++++----
 packages/aws-cdk/test/api/bootstrap.test.ts   |  1 +
 .../aws-cdk/test/api/deploy-stack.test.ts     | 47 +++++++++++++++++++
 4 files changed, 85 insertions(+), 9 deletions(-)

diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md
index d17a38e62f923..78e2177b186cd 100644
--- a/packages/aws-cdk/README.md
+++ b/packages/aws-cdk/README.md
@@ -285,6 +285,18 @@ When `cdk deploy` is executed, deployment events will include the complete histo
 
 The `progress` key can also be specified as a user setting (`~/.cdk.json`)
 
+#### Externally Executable CloudFormation Change Sets
+
+For more control over when stack changes are deployed, the CDK can generate a
+CloudFormation change set but not execute it. The name of the generated
+change set is *cdk-deploy-change-set*, and a previous change set with that
+name will be overwritten. The change set will always be created, even if it
+is empty.
+
+```console
+$ cdk deploy --no-execute
+```
+
 ### `cdk destroy`
 
 Deletes a stack from it's environment. This will cause the resources in the stack to be destroyed (unless they were
diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts
index 1b52fb736e946..2dfb2f5119c71 100644
--- a/packages/aws-cdk/lib/api/deploy-stack.ts
+++ b/packages/aws-cdk/lib/api/deploy-stack.ts
@@ -174,6 +174,7 @@ export interface DeployStackOptions {
 }
 
 const LARGE_TEMPLATE_SIZE_KB = 50;
+const CDK_CHANGE_SET_NAME = 'cdk-deploy-change-set';
 
 /** @experimental */
 export async function deployStack(options: DeployStackOptions): Promise<DeployStackResult> {
@@ -228,14 +229,20 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
 
   await publishAssets(legacyAssets.toManifest(stackArtifact.assembly.directory), options.sdkProvider, stackEnv);
 
-  const changeSetName = `CDK-${executionId}`;
+  if (cloudFormationStack.exists) {
+    //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 ${CDK_CHANGE_SET_NAME} if it exists`);
+    await cfn.deleteChangeSet({ StackName: deployName, ChangeSetName: CDK_CHANGE_SET_NAME }).promise();
+  }
+
   const update = cloudFormationStack.exists && cloudFormationStack.stackStatus.name !== 'REVIEW_IN_PROGRESS';
 
-  debug(`Attempting to create ChangeSet ${changeSetName} to ${update ? 'update' : 'create'} stack ${deployName}`);
+  debug(`Attempting to create ChangeSet ${CDK_CHANGE_SET_NAME} to ${update ? 'update' : 'create'} stack ${deployName}`);
   print('%s: creating CloudFormation changeset...', colors.bold(deployName));
   const changeSet = await cfn.createChangeSet({
     StackName: deployName,
-    ChangeSetName: changeSetName,
+    ChangeSetName: CDK_CHANGE_SET_NAME,
     ChangeSetType: update ? 'UPDATE' : 'CREATE',
     Description: `CDK Changeset for execution ${executionId}`,
     TemplateBody: bodyParameter.TemplateBody,
@@ -247,7 +254,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
     Tags: options.tags,
   }).promise();
   debug('Initiated creation of changeset: %s; waiting for it to finish creating...', changeSet.Id);
-  const changeSetDescription = await waitForChangeSet(cfn, deployName, changeSetName);
+  const changeSetDescription = await waitForChangeSet(cfn, deployName, CDK_CHANGE_SET_NAME);
 
   // Update termination protection only if it has changed.
   const terminationProtection = stackArtifact.terminationProtection ?? false;
@@ -262,21 +269,24 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
 
   if (changeSetHasNoChanges(changeSetDescription)) {
     debug('No changes are to be performed on %s.', deployName);
-    await cfn.deleteChangeSet({ StackName: deployName, ChangeSetName: changeSetName }).promise();
+    if (options.execute) {
+      debug('Deleting empty change set %s', changeSet.Id);
+      await cfn.deleteChangeSet({ StackName: deployName, ChangeSetName: CDK_CHANGE_SET_NAME }).promise();
+    }
     return { noOp: true, outputs: cloudFormationStack.outputs, stackArn: changeSet.StackId!, stackArtifact };
   }
 
   const execute = options.execute === undefined ? true : options.execute;
   if (execute) {
-    debug('Initiating execution of changeset %s on stack %s', changeSetName, deployName);
-    await cfn.executeChangeSet({ StackName: deployName, ChangeSetName: changeSetName }).promise();
+    debug('Initiating execution of changeset %s on stack %s', CDK_CHANGE_SET_NAME, deployName);
+    await cfn.executeChangeSet({ StackName: deployName, ChangeSetName: CDK_CHANGE_SET_NAME }).promise();
     // eslint-disable-next-line max-len
     const monitor = options.quiet ? undefined : StackActivityMonitor.withDefaultPrinter(cfn, deployName, stackArtifact, {
       resourcesTotal: (changeSetDescription.Changes ?? []).length,
       progress: options.progress,
       changeSetCreationTime: changeSetDescription.CreationTime,
     }).start();
-    debug('Execution of changeset %s on stack %s has started; waiting for the update to complete...', changeSetName, deployName);
+    debug('Execution of changeset %s on stack %s has started; waiting for the update to complete...', CDK_CHANGE_SET_NAME, deployName);
     try {
       const finalStack = await waitForStackDeploy(cfn, deployName);
 
@@ -288,7 +298,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
     }
     debug('Stack %s has completed updating', deployName);
   } else {
-    print('Changeset %s created and waiting in review for manual execution (--no-execute)', changeSetName);
+    print('Changeset %s created and waiting in review for manual execution (--no-execute)', CDK_CHANGE_SET_NAME);
   }
 
   return { noOp: false, outputs: cloudFormationStack.outputs, stackArn: changeSet.StackId!, stackArtifact };
@@ -408,6 +418,12 @@ async function canSkipDeploy(
     return false;
   }
 
+  // Creating changeset only (default true), never skip
+  if (deployStackOptions.execute === false) {
+    debug(`${deployName}: --no-execute, always creating change set`);
+    return false;
+  }
+
   // No existing stack
   if (!cloudFormationStack.exists) {
     debug(`${deployName}: no existing stack`);
diff --git a/packages/aws-cdk/test/api/bootstrap.test.ts b/packages/aws-cdk/test/api/bootstrap.test.ts
index d04a49ec11472..888a649cc735e 100644
--- a/packages/aws-cdk/test/api/bootstrap.test.ts
+++ b/packages/aws-cdk/test/api/bootstrap.test.ts
@@ -49,6 +49,7 @@ beforeEach(() => {
       executed = true;
       return {};
     }),
+    deleteChangeSet: jest.fn(),
     getTemplate: jest.fn(() => {
       executed = true;
       return {};
diff --git a/packages/aws-cdk/test/api/deploy-stack.test.ts b/packages/aws-cdk/test/api/deploy-stack.test.ts
index 1908b4716aac0..8fffb321ef995 100644
--- a/packages/aws-cdk/test/api/deploy-stack.test.ts
+++ b/packages/aws-cdk/test/api/deploy-stack.test.ts
@@ -459,6 +459,53 @@ test('not executed and no error if --no-execute is given', async () => {
   expect(cfnMocks.executeChangeSet).not.toHaveBeenCalled();
 });
 
+test('empty change set is deleted if --execute is given', async () => {
+  cfnMocks.describeChangeSet?.mockImplementation(() => ({
+    Status: 'FAILED',
+    StatusReason: 'No updates are to be performed.',
+  }));
+
+  // GIVEN
+  givenStackExists();
+
+  // WHEN
+  await deployStack({
+    ...standardDeployStackArguments(),
+    execute: true,
+    force: true, // Necessary to bypass "skip deploy"
+  });
+
+  // THEN
+  expect(cfnMocks.createChangeSet).toHaveBeenCalled();
+  expect(cfnMocks.executeChangeSet).not.toHaveBeenCalled();
+
+  //the first deletion is for any existing cdk change sets, the second is for the deleting the new empty change set
+  expect(cfnMocks.deleteChangeSet).toHaveBeenCalledTimes(2);
+});
+
+test('empty change set is not deleted if --no-execute is given', async () => {
+  cfnMocks.describeChangeSet?.mockImplementation(() => ({
+    Status: 'FAILED',
+    StatusReason: 'No updates are to be performed.',
+  }));
+
+  // GIVEN
+  givenStackExists();
+
+  // WHEN
+  await deployStack({
+    ...standardDeployStackArguments(),
+    execute: false,
+  });
+
+  // THEN
+  expect(cfnMocks.createChangeSet).toHaveBeenCalled();
+  expect(cfnMocks.executeChangeSet).not.toHaveBeenCalled();
+
+  //the first deletion is for any existing cdk change sets
+  expect(cfnMocks.deleteChangeSet).toHaveBeenCalledTimes(1);
+});
+
 test('use S3 url for stack deployment if present in Stack Artifact', async () => {
   // WHEN
   await deployStack({