fix(cli): cdk diff falsely reports resource replacements on trivial template changes

packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts

@@ -1,10 +1,12 @@
+import { CloudFormation } from 'aws-sdk';
 import * as impl from './diff';
 import * as types from './diff/types';
 import { deepEqual, diffKeyedEntities, unionOf } from './diff/util';
+import { ChangeSetReplacement, ResourceReplacements } from './format';
 
 export * from './diff/types';
 
-type DiffHandler = (diff: types.ITemplateDiff, oldValue: any, newValue: any) => void;
+type DiffHandler = (diff: types.ITemplateDiff, oldValue: any, newValue: any, replacement?: ResourceReplacements) => void;
 type HandlerRegistry = { [section: string]: DiffHandler };
 
 const DIFF_HANDLERS: HandlerRegistry = {
@@ -22,8 +24,8 @@ const DIFF_HANDLERS: HandlerRegistry = {
     diff.conditions = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffCondition)),
   Transform: (diff, oldValue, newValue) =>
     diff.transform = impl.diffAttribute(oldValue, newValue),
-  Resources: (diff, oldValue, newValue) =>
-    diff.resources = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffResource)),
+  Resources: (diff, oldValue, newValue, replacements?: ResourceReplacements) =>
+    diff.resources = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffResource, replacements)),
   Outputs: (diff, oldValue, newValue) =>
     diff.outputs = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffOutput)),
 };
@@ -38,17 +40,22 @@ const DIFF_HANDLERS: HandlerRegistry = {
  *      a stack which current state is described by +currentTemplate+ is updated with
  *      the template +newTemplate+.
  */
-export function diffTemplate(currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }): types.TemplateDiff {
+export function diffTemplate(
+  currentTemplate: { [key: string]: any },
+  newTemplate: { [key: string]: any },
+  changeSet?: CloudFormation.DescribeChangeSetOutput
+): types.TemplateDiff {
+  const replacements = changeSet ? findResourceReplacements(changeSet): undefined;
   // Base diff
-  const theDiff = calculateTemplateDiff(currentTemplate, newTemplate);
+  const theDiff = calculateTemplateDiff(currentTemplate, newTemplate, replacements);
 
   // We're going to modify this in-place
   const newTemplateCopy = deepCopy(newTemplate);
 
   let didPropagateReferenceChanges;
   let diffWithReplacements;
   do {
-    diffWithReplacements = calculateTemplateDiff(currentTemplate, newTemplateCopy);
+    diffWithReplacements = calculateTemplateDiff(currentTemplate, newTemplateCopy, replacements);
 
     // Propagate replacements for replaced resources
     didPropagateReferenceChanges = false;
@@ -93,7 +100,11 @@ function propagatePropertyReplacement(source: types.ResourceDifference, dest: ty
   }
 }
 
-function calculateTemplateDiff(currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }): types.TemplateDiff {
+function calculateTemplateDiff(
+  currentTemplate: { [key: string]: any },
+  newTemplate: { [key: string]: any },
+  replacements?: ResourceReplacements,
+): types.TemplateDiff {
   const differences: types.ITemplateDiff = {};
   const unknown: { [key: string]: types.Difference<any> } = {};
   for (const key of unionOf(Object.keys(currentTemplate), Object.keys(newTemplate)).sort()) {
@@ -104,8 +115,7 @@ function calculateTemplateDiff(currentTemplate: { [key: string]: any }, newTempl
     }
     const handler: DiffHandler = DIFF_HANDLERS[key]
                   || ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV));
-    handler(differences, oldValue, newValue);
-
+    handler(differences, oldValue, newValue, replacements);
   }
   if (Object.keys(unknown).length > 0) {
     differences.unknown = new types.DifferenceCollection(unknown);
@@ -114,13 +124,6 @@ function calculateTemplateDiff(currentTemplate: { [key: string]: any }, newTempl
   return new types.TemplateDiff(differences);
 }
 
-/**
- * Compare two CloudFormation resources and return semantic differences between them
- */
-export function diffResource(oldValue: types.Resource, newValue: types.Resource): types.ResourceDifference {
-  return impl.diffResource(oldValue, newValue);
-}
-
 /**
  * Replace all references to the given logicalID on the given template, in-place
  *
@@ -184,3 +187,30 @@ function deepCopy(x: any): any {
 
   return x;
 }
+
+function findResourceReplacements(changeSet: CloudFormation.DescribeChangeSetOutput): ResourceReplacements {
+  const replacements: ResourceReplacements = {};
+  for (const resourceChange of changeSet.Changes ?? []) {
+    const propertiesReplaced: { [propName: string]: ChangeSetReplacement } = {};
+    for (const propertyChange of resourceChange.ResourceChange?.Details ?? []) {
+      if (propertyChange.Target?.Attribute === 'Properties') {
+        const requiresReplacement = propertyChange.Target.RequiresRecreation === 'Always';
+        if (requiresReplacement && propertyChange.Evaluation === 'Static') {
+          propertiesReplaced[propertyChange.Target.Name!] = 'Always';
+        } else if (requiresReplacement && propertyChange.Evaluation === 'Dynamic') {
+          // If Evaluation is 'Dynamic', then this may cause replacement, or it may not.
+          // see 'Replacement': https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_ResourceChange.html
+          propertiesReplaced[propertyChange.Target.Name!] = 'Conditionally';
+        } else {
+          propertiesReplaced[propertyChange.Target.Name!] = propertyChange.Target.RequiresRecreation as ChangeSetReplacement;
+        }
+      }
+    }
+    replacements[resourceChange.ResourceChange?.LogicalResourceId ?? ''] = {
+      resourceReplaced: resourceChange.ResourceChange?.Replacement === 'True',
+      propertiesReplaced,
+    };
+  }
+
+  return replacements;
+}

packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts

@@ -1,6 +1,7 @@
 import { Resource } from '@aws-cdk/service-spec-types';
 import * as types from './types';
 import { deepEqual, diffKeyedEntities, loadResourceModel } from './util';
+import { ResourceReplacement } from '../format';
 
 export function diffAttribute(oldValue: any, newValue: any): types.Difference<string> {
   return new types.Difference<string>(_asString(oldValue), _asString(newValue));
@@ -26,7 +27,12 @@ export function diffParameter(oldValue: types.Parameter, newValue: types.Paramet
   return new types.ParameterDifference(oldValue, newValue);
 }
 
-export function diffResource(oldValue?: types.Resource, newValue?: types.Resource): types.ResourceDifference {
+export function diffResource(
+  oldValue?: types.Resource,
+  newValue?: types.Resource,
+  _key?: string,
+  replacement?: ResourceReplacement,
+): types.ResourceDifference {
   const resourceType = {
     oldType: oldValue && oldValue.Type,
     newType: newValue && newValue.Type,
@@ -39,35 +45,59 @@ export function diffResource(oldValue?: types.Resource, newValue?: types.Resourc
     const impl = loadResourceModel(resourceType.oldType);
     propertyDiffs = diffKeyedEntities(oldValue!.Properties,
       newValue!.Properties,
-      (oldVal, newVal, key) => _diffProperty(oldVal, newVal, key, impl));
+      (oldVal, newVal, key) => _diffProperty(oldVal, newVal, key, impl, replacement));
 
     otherDiffs = diffKeyedEntities(oldValue, newValue, _diffOther);
     delete otherDiffs.Properties;
+    // TODO: should only happen with the right flag set
+    delete otherDiffs.Metadata;
   }
 
   return new types.ResourceDifference(oldValue, newValue, {
     resourceType, propertyDiffs, otherDiffs,
   });
 
-  function _diffProperty(oldV: any, newV: any, key: string, resourceSpec?: Resource) {
-    let changeImpact = types.ResourceImpact.NO_CHANGE;
+  function _diffProperty(oldV: any, newV: any, key: string, resourceSpec?: Resource, changeSetReplacement?: ResourceReplacement) {
+    let changeImpact: types.ResourceImpact = types.ResourceImpact.NO_CHANGE;
+    if (changeSetReplacement === undefined) {
+      changeImpact = _resourceSpecImpact(oldV, newV, key, resourceSpec);
+    } else {
+      if (!(key in changeSetReplacement.propertiesReplaced)) {
+        // The changeset does not contain this property, which means it is not going to be updated. Hide this cosmetic template-only change from the diff
+        return new types.PropertyDifference(1, 1, { changeImpact });
+      }
+
+      switch (changeSetReplacement.propertiesReplaced[key]) {
+        case 'Always':
+          changeImpact = types.ResourceImpact.WILL_REPLACE;
+          break;
+        case 'Conditionally':
+          changeImpact = types.ResourceImpact.MAY_REPLACE;
+          break;
+        case 'Never':
+          changeImpact = types.ResourceImpact.WILL_UPDATE;
+          break;
+      }
+    }
 
+    return new types.PropertyDifference(oldV, newV, { changeImpact });
+  }
+
+  function _resourceSpecImpact(oldV: any, newV: any, key: string, resourceSpec?: Resource) {
     const spec = resourceSpec?.properties?.[key];
     if (spec && !deepEqual(oldV, newV)) {
       switch (spec.causesReplacement) {
         case 'yes':
-          changeImpact = types.ResourceImpact.WILL_REPLACE;
-          break;
+          return types.ResourceImpact.WILL_REPLACE;
         case 'maybe':
-          changeImpact = types.ResourceImpact.MAY_REPLACE;
-          break;
+          return types.ResourceImpact.MAY_REPLACE;
         default:
           // In those cases, whatever is the current value is what we should keep
-          changeImpact = types.ResourceImpact.WILL_UPDATE;
+          return types.ResourceImpact.WILL_UPDATE;
       }
     }
 
-    return new types.PropertyDifference(oldV, newV, { changeImpact });
+    return types.ResourceImpact.NO_CHANGE;
   }
 
   function _diffOther(oldV: any, newV: any) {

packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts

@@ -1,5 +1,6 @@
 import { loadAwsServiceSpecSync } from '@aws-cdk/aws-service-spec';
 import { Resource, SpecDatabase } from '@aws-cdk/service-spec-types';
+import { ResourceReplacement, ResourceReplacements } from '../format';
 
 /**
  * Compares two objects for equality, deeply. The function handles arguments that are
@@ -24,6 +25,12 @@ export function deepEqual(lvalue: any, rvalue: any): boolean {
       lvalue.toString() === rvalue.toString()) {
     return true;
   }
+
+  if (containsIntrinsic(lvalue) && containsIntrinsic(rvalue)) {
+    if (yamlIntrinsicEqual(lvalue, rvalue)) {
+      return true;
+    }
+  }
   // allows a numeric 10 and a literal "10" to be equivalent;
   // this is consistent with CloudFormation.
   if ((typeof lvalue === 'string' || typeof rvalue === 'string') &&
@@ -99,6 +106,65 @@ function dependsOnEqual(lvalue: any, rvalue: any): boolean {
   return false;
 }
 
+function containsIntrinsic(value: any): boolean {
+  return Object.keys(value ?? {}).filter((key => key ? (key.includes('Fn::') ? true : false) : false)).length > 0;
+}
+
+/**
+ * Checks if a value has changed intrinsic form.
+ * Only applicable to CDK Migrate.
+ * For example, if a user has created a yaml template that uses
+ *
+ * !Fn::GetAtt 'foo.bar', CDK Migrate converts this to
+ * Fn::GetAtt: ['foo', 'bar']
+ *
+ * Both forms are equivalent
+ */
+function yamlIntrinsicEqual(lvalue: any, rvalue: any): boolean {
+  for (const lintrinsic of Object.keys(lvalue)) {
+    for (const rintrinsic of Object.keys(rvalue)) {
+      if (lintrinsic !== rintrinsic) {
+        return false;
+      }
+      const intrinsic = lintrinsic;
+      switch (intrinsic) {
+        // checks for equivalency in both yaml forms of Fn::GetAtt.
+        // !Fn::GetAtt 'foo.bar' is equivalent to
+        // Fn::GetAtt: ['foo', 'bar']
+        case 'Fn::GetAtt':
+          let array = undefined;
+          let str = undefined;
+          if (Array.isArray(lvalue[intrinsic]) && !Array.isArray(rvalue[intrinsic]) && typeof rvalue[intrinsic] === 'string') {
+            array = lvalue[intrinsic];
+            str = rvalue[intrinsic];
+          } else if (!Array.isArray(lvalue[intrinsic]) && Array.isArray(rvalue[intrinsic]) && typeof lvalue[intrinsic] === 'string') {
+            array = rvalue[intrinsic];
+            str = lvalue[intrinsic];
+          }
+
+          // if one value is an array, and the other is a string, check for form equivalency
+          if (array && str) {
+            // check if array is a string[]
+            let strArr: boolean = true;
+            array.forEach((item: any) => {
+              if (typeof item !== 'string') {
+                strArr = false;
+              }
+            });
+
+            if (strArr && array.join('.') === str) {
+              return true;
+            }
+          }
+
+          return deepEqual(lvalue[intrinsic], rvalue[intrinsic]);
+      }
+    }
+  }
+
+  return false;
+}
+
 /**
  * Produce the differences between two maps, as a map, using a specified diff function.
  *
@@ -111,7 +177,8 @@ function dependsOnEqual(lvalue: any, rvalue: any): boolean {
 export function diffKeyedEntities<T>(
   oldValue: { [key: string]: any } | undefined,
   newValue: { [key: string]: any } | undefined,
-  elementDiff: (oldElement: any, newElement: any, key: string) => T): { [name: string]: T } {
+  elementDiff: (oldElement: any, newElement: any, key: string, replacements?: ResourceReplacement) => T,
+  replacements?: ResourceReplacements): { [name: string]: T } {
   const result: { [name: string]: T } = {};
   for (const logicalId of unionOf(Object.keys(oldValue || {}), Object.keys(newValue || {}))) {
     const oldElement = oldValue && oldValue[logicalId];
@@ -122,7 +189,7 @@ export function diffKeyedEntities<T>(
       continue;
     }
 
-    result[logicalId] = elementDiff(oldElement, newElement, logicalId);
+    result[logicalId] = elementDiff(oldElement, newElement, logicalId, replacements ? replacements[logicalId]: undefined);
   }
   return result;
 }

packages/@aws-cdk/cloudformation-diff/lib/format.ts

@@ -18,6 +18,18 @@ export interface FormatStream extends NodeJS.WritableStream {
   columns?: number;
 }
 
+/**
+ * maps logical IDs to their need to be replaced
+ */
+export type ResourceReplacements = { [logicalId: string]: ResourceReplacement };
+
+export interface ResourceReplacement {
+  resourceReplaced: boolean,
+  propertiesReplaced: { [propertyName: string]: ChangeSetReplacement };
+}
+
+export type ChangeSetReplacement = 'Always' | 'Never' | 'Conditionally';
+
 /**
  * Renders template differences to the process' console.
  *

packages/@aws-cdk/cloudformation-diff/package.json

@@ -29,7 +29,8 @@
     "diff": "^5.1.0",
     "fast-deep-equal": "^3.1.3",
     "string-width": "^4.2.3",
-    "table": "^6.8.1"
+    "table": "^6.8.1",
+    "aws-sdk": "2.1516.0"
   },
   "devDependencies": {
     "@aws-cdk/cdk-build-tools": "0.0.0",