fix(cli): fast "no-op" deploys do not consider tags

packages/aws-cdk/README.md

@@ -128,9 +128,9 @@ bootstrapped (using `cdk bootstrap`), only stacks that are not using assets and
 $ cdk deploy --app='node bin/main.js' MyStackName
 ```
 
-Before creating a change set, `cdk deploy` will compare the template of the
-currently deployed stack to the template that is about to be deployed and will
-skip deployment if they are identical. Use `--force` to override this behavior
+Before creating a change set, `cdk deploy` will compare the template and tags of the
+currently deployed stack to the template and tags that are about to be deployed and
+will skip deployment if they are identical. Use `--force` to override this behavior
 and always deploy the stack.
 
 #### `cdk destroy`

packages/aws-cdk/lib/api/deploy-stack.ts

@@ -74,10 +74,11 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
   const deployName = options.deployName || options.stack.stackName;
 
   if (!options.force) {
-    debug(`checking if we can skip this stack based on the currently deployed template (use --force to override)`);
-    const deployed = await getDeployedTemplate(cfn, deployName);
-    if (deployed && JSON.stringify(options.stack.template) === JSON.stringify(deployed.template)) {
-      debug(`${deployName}: no change in template, skipping (use --force to override)`);
+    debug(`checking if we can skip this stack based on the currently deployed template and tags (use --force to override)`);
+    const deployed = await getDeployedStack(cfn, deployName);
+    const tagsIdentical = compareTags(deployed?.tags ?? [], options.tags ?? []);
+    if (deployed && JSON.stringify(options.stack.template) === JSON.stringify(deployed.template) && tagsIdentical) {
+      debug(`${deployName}: no change in template and tags, skipping (use --force to override)`);
       return {
         noOp: true,
         outputs: await getStackOutputs(cfn, deployName),
@@ -239,14 +240,22 @@ export async function destroyStack(options: DestroyStackOptions) {
   return;
 }
 
-async function getDeployedTemplate(cfn: aws.CloudFormation, stackName: string): Promise<{ template: any, stackId: string } | undefined> {
-  const stackId = await getStackId(cfn, stackName);
-  if (!stackId) {
+async function getDeployedStack(cfn: aws.CloudFormation, stackName: string): Promise<{ stackId: string, template: any, tags: Tag[] } | undefined> {
+  const stack = await getStack(cfn, stackName);
+  if (!stack) {
+    return undefined;
+  }
+
+  if (!stack.StackId) {
     return undefined;
   }
 
   const template = await readCurrentTemplate(cfn, stackName);
-  return { stackId, template };
+  return {
+    stackId: stack.StackId,
+    tags: stack.Tags ?? [],
+    template
+  };
 }
 
 export async function readCurrentTemplate(cfn: aws.CloudFormation, stackName: string) {
@@ -262,7 +271,7 @@ export async function readCurrentTemplate(cfn: aws.CloudFormation, stackName: st
   }
 }
 
-async function getStackId(cfn: aws.CloudFormation, stackName: string): Promise<string | undefined> {
+async function getStack(cfn: aws.CloudFormation, stackName: string): Promise<aws.CloudFormation.Stack | undefined> {
   try {
     const stacks = await cfn.describeStacks({ StackName: stackName }).promise();
     if (!stacks.Stacks) {
@@ -272,12 +281,28 @@ async function getStackId(cfn: aws.CloudFormation, stackName: string): Promise<s
       return undefined;
     }
 
-    return stacks.Stacks[0].StackId!;
+    return stacks.Stacks[0];
 
   } catch (e) {
     if (e.message.includes('does not exist')) {
       return undefined;
     }
     throw e;
   }
-}
+}
+
+function compareTags(a: Tag[], b: Tag[]): boolean {
+  if (a.length !== b.length) {
+    return false;
+  }
+
+  for (const aTag of a) {
+    const bTag = b.find(tag => tag.Key === aTag.Key);
+
+    if (!bTag || bTag.Value !== aTag.Value) {
+      return false;
+    }
+  }
+
+  return true;
+}

packages/aws-cdk/test/api/deploy-stack.test.ts

@@ -145,6 +145,216 @@ test('deploy is skipped if template did not change', async () => {
   expect(getTemplateInput!).toStrictEqual({ StackName: 'withouterrors', TemplateStage: 'Original' });
 });
 
+test('deploy is skipped if template and tags did not change', async () => {
+  const sdk = new MockSDK();
+  let describeStacksInput: AWS.CloudFormation.DescribeStacksInput;
+  let getTemplateInput: AWS.CloudFormation.GetTemplateInput;
+  let createChangeSetCalled = false;
+  let executeChangeSetCalled = false;
+
+  sdk.stubCloudFormation({
+    getTemplate(input) {
+      getTemplateInput = input;
+      return {
+        TemplateBody: JSON.stringify(FAKE_TEMPLATE)
+      };
+    },
+    describeStacks(input) {
+      describeStacksInput = input;
+      return {
+        Stacks: [
+          {
+            StackName: 'mock-stack-name',
+            StackId: 'mock-stack-id',
+            CreationTime: new Date(),
+            StackStatus: 'CREATE_COMPLETE',
+            Tags: [
+              {
+                Key: 'Key1',
+                Value: 'Value1'
+              },
+              {
+                Key: 'Key2',
+                Value: 'Value2'
+              }
+            ]
+          }
+        ]
+      };
+    },
+    createChangeSet() {
+      createChangeSetCalled = true;
+      return { };
+    },
+    executeChangeSet() {
+      executeChangeSetCalled = true;
+      return { };
+    }
+  });
+
+  await deployStack({
+    stack: FAKE_STACK,
+    tags: [
+      {
+        Key: 'Key1',
+        Value: 'Value1'
+      },
+      {
+        Key: 'Key2',
+        Value: 'Value2'
+      }
+    ],
+    sdk
+  });
+
+  expect(createChangeSetCalled).toBeFalsy();
+  expect(executeChangeSetCalled).toBeFalsy();
+  expect(describeStacksInput!).toStrictEqual({ StackName: 'withouterrors' });
+  expect(getTemplateInput!).toStrictEqual({ StackName: 'withouterrors', TemplateStage: 'Original' });
+});
+
+test('deploy not skipped if template did not change but tags changed', async () => {
+  const sdk = new MockSDK();
+  let describeStacksInput: AWS.CloudFormation.DescribeStacksInput;
+  let getTemplateInput: AWS.CloudFormation.GetTemplateInput;
+  let createChangeSetCalled = false;
+  let executeChangeSetCalled = false;
+  let describeChangeSetCalled = false;
+
+  sdk.stubCloudFormation({
+    getTemplate(input) {
+      getTemplateInput = input;
+      return {
+        TemplateBody: JSON.stringify(FAKE_TEMPLATE)
+      };
+    },
+    describeStacks(input) {
+      describeStacksInput = input;
+      return {
+        Stacks: [
+          {
+            StackName: 'mock-stack-name',
+            StackId: 'mock-stack-id',
+            CreationTime: new Date(),
+            StackStatus: 'CREATE_COMPLETE',
+            Tags: [
+              {
+                Key: 'Key',
+                Value: 'Value'
+              },
+            ]
+          }
+        ]
+      };
+    },
+    createChangeSet() {
+      createChangeSetCalled = true;
+      return { };
+    },
+    executeChangeSet() {
+      executeChangeSetCalled = true;
+      return { };
+    },
+    describeChangeSet() {
+      describeChangeSetCalled = true;
+      return {
+        Status: 'CREATE_COMPLETE',
+        Changes: [],
+      };
+    }
+  });
+
+  await deployStack({
+    stack: FAKE_STACK,
+    sdk,
+    tags: [
+      {
+        Key: 'Key',
+        Value: 'NewValue'
+      }
+    ]
+  });
+
+  expect(createChangeSetCalled).toBeTruthy();
+  expect(executeChangeSetCalled).toBeTruthy();
+  expect(describeChangeSetCalled).toBeTruthy();
+  expect(describeStacksInput!).toStrictEqual({ StackName: "withouterrors" });
+  expect(getTemplateInput!).toStrictEqual({ StackName: 'withouterrors', TemplateStage: 'Original' });
+});
+
+test('deploy not skipped if template did not change but one tag removed', async () => {
+  const sdk = new MockSDK();
+  let describeStacksInput: AWS.CloudFormation.DescribeStacksInput;
+  let getTemplateInput: AWS.CloudFormation.GetTemplateInput;
+  let createChangeSetCalled = false;
+  let executeChangeSetCalled = false;
+  let describeChangeSetCalled = false;
+
+  sdk.stubCloudFormation({
+    getTemplate(input) {
+      getTemplateInput = input;
+      return {
+        TemplateBody: JSON.stringify(FAKE_TEMPLATE)
+      };
+    },
+    describeStacks(input) {
+      describeStacksInput = input;
+      return {
+        Stacks: [
+          {
+            StackName: 'mock-stack-name',
+            StackId: 'mock-stack-id',
+            CreationTime: new Date(),
+            StackStatus: 'CREATE_COMPLETE',
+            Tags: [
+              {
+                Key: 'Key1',
+                Value: 'Value1'
+              },
+              {
+                Key: 'Key2',
+                Value: 'Value2'
+              },
+            ]
+          }
+        ]
+      };
+    },
+    createChangeSet() {
+      createChangeSetCalled = true;
+      return { };
+    },
+    executeChangeSet() {
+      executeChangeSetCalled = true;
+      return { };
+    },
+    describeChangeSet() {
+      describeChangeSetCalled = true;
+      return {
+        Status: 'CREATE_COMPLETE',
+        Changes: [],
+      };
+    }
+  });
+
+  await deployStack({
+    stack: FAKE_STACK,
+    sdk,
+    tags: [
+      {
+        Key: 'Key1',
+        Value: 'Value1'
+      }
+    ]
+  });
+
+  expect(createChangeSetCalled).toBeTruthy();
+  expect(executeChangeSetCalled).toBeTruthy();
+  expect(describeChangeSetCalled).toBeTruthy();
+  expect(describeStacksInput!).toStrictEqual({ StackName: "withouterrors" });
+  expect(getTemplateInput!).toStrictEqual({ StackName: 'withouterrors', TemplateStage: 'Original' });
+});
+
 test('deploy not skipped if template did not change and --force is applied', async () => {
   const sdk = new MockSDK();
   let describeStacksInput: AWS.CloudFormation.DescribeStacksInput;
@@ -250,13 +460,12 @@ test('deploy not skipped if template changed', async () => {
 
   await deployStack({
     stack: FAKE_STACK,
-    sdk,
-    force: true
+    sdk
   });
 
   expect(createChangeSetCalled).toBeTruthy();
   expect(executeChangeSetCalled).toBeTruthy();
   expect(describeChangeSetCalled).toBeTruthy();
-  expect(getTemplateInput!).toBeUndefined();
   expect(describeStacksInput!).toStrictEqual({ StackName: "withouterrors" });
-});
+  expect(getTemplateInput!).toStrictEqual({ StackName: 'withouterrors', TemplateStage: 'Original' });
+});

packages/aws-cdk/test/integ/cli/common.bash

@@ -66,20 +66,20 @@ function prepare_fixture() {
     cp -R app/* $integ_test_dir
     cd $integ_test_dir
 
-    # if this directory is missing, but exists in any of the 
+    # if this directory is missing, but exists in any of the
     # parent directories, npm will install these packages there. lets make sure
     # we install locally.
     mkdir -p node_modules
 
     npm install \
-        @aws-cdk/core \
-        @aws-cdk/aws-sns \
-        @aws-cdk/aws-iam \
-        @aws-cdk/aws-lambda \
-        @aws-cdk/aws-ssm \
-        @aws-cdk/aws-ecr-assets \
-        @aws-cdk/aws-cloudformation \
-        @aws-cdk/aws-ec2
+        @aws-cdk/core@^1 \
+        @aws-cdk/aws-sns@^1 \
+        @aws-cdk/aws-iam@^1 \
+        @aws-cdk/aws-lambda@^1 \
+        @aws-cdk/aws-ssm@^1 \
+        @aws-cdk/aws-ecr-assets@^1 \
+        @aws-cdk/aws-cloudformation@^1 \
+        @aws-cdk/aws-ec2@^1
 
     echo "| setup complete at: $PWD"
     echo "| 'cdk' is: $(type -p cdk)"

packages/aws-cdk/test/integ/cli/test-cdk-fast-deploy.sh

@@ -45,6 +45,16 @@ if [ "${changeset3}" == "${changeset1}" ]; then
     exit 1
 fi
 
+echo "============================================================"
+echo " deploying the same stack again with different tags"
+echo "============================================================"
+cdk deploy -v ${stack_name} --tags key=value
+changeset4=$(get_last_changeset)
+if [ "${changeset4}" == "${changeset1}" ]; then
+    echo "TEST FAILED: expected tags to create a new changeset"
+    exit 1
+fi
+
 # destroy
 cdk destroy -f ${stack_name}