fix(cloudformation-diff): track replacements

lerna.json

@@ -6,6 +6,11 @@
     "packages/@aws-cdk/*",
     "tools/*"
   ],
+  "command": {
+    "bootstrap": {
+      "npmClientArgs": ["--no-package-lock"]
+    }
+  },
   "rejectCycles": "true",
   "version": "0.13.0"
 }

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

@@ -39,18 +39,40 @@ const DIFF_HANDLERS: HandlerRegistry = {
  *      the template +newTemplate+.
  */
 export function diffTemplate(currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }): 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()) {
-    const oldValue = currentTemplate[key];
-    const newValue = newTemplate[key];
-    if (deepEqual(oldValue, newValue)) { continue; }
-    const handler: DiffHandler = DIFF_HANDLERS[key]
-                   || ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV));
-    handler(differences, oldValue, newValue);
+  // We're going to modify this in-place
+  newTemplate = deepCopy(newTemplate);
+
+  while (true) {
+    const differences: types.ITemplateDiff = {};
+    const unknown: { [key: string]: types.Difference<any> } = {};
+    for (const key of unionOf(Object.keys(currentTemplate), Object.keys(newTemplate)).sort()) {
+      const oldValue = currentTemplate[key];
+      const newValue = newTemplate[key];
+      if (deepEqual(oldValue, newValue)) { continue; }
+      const handler: DiffHandler = DIFF_HANDLERS[key]
+                    || ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV));
+      handler(differences, oldValue, newValue);
+
+    }
+    if (Object.keys(unknown).length > 0) { differences.unknown = new types.DifferenceCollection(unknown); }
+
+    // Propagate replacements for replaced resources
+    let didPropagateReferenceChanges = false;
+    if (differences.resources) {
+      differences.resources.forEach((logicalId, change) => {
+        if (change.changeImpact === types.ResourceImpact.WILL_REPLACE) {
+          if (propagateReplacedReferences(newTemplate, logicalId)) {
+            didPropagateReferenceChanges = true;
+          }
+        }
+      });
+    }
+
+    // We're done only if we didn't have to propagate any more replacements.
+    if (!didPropagateReferenceChanges) {
+      return new types.TemplateDiff(differences);
+    }
   }
-  if (Object.keys(unknown).length > 0) { differences.unknown = new types.DifferenceCollection(unknown); }
-  return new types.TemplateDiff(differences);
 }
 
 /**
@@ -59,3 +81,67 @@ export function diffTemplate(currentTemplate: { [key: string]: any }, newTemplat
 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
+ *
+ * Returns true iff any references were replaced.
+ */
+function propagateReplacedReferences(template: object, logicalId: string): boolean {
+  let ret = false;
+
+  function recurse(obj: any) {
+    if (Array.isArray(obj)) {
+      obj.forEach(recurse);
+    }
+
+    if (typeof obj === 'object' && obj !== null) {
+      if (!replaceReference(obj)) {
+        Object.values(obj).forEach(recurse);
+      }
+    }
+  }
+
+  function replaceReference(obj: any) {
+    const keys = Object.keys(obj);
+    if (keys.length !== 1) { return false; }
+    const key = keys[0];
+
+    if (key === 'Ref') {
+      if (obj.Ref === logicalId) {
+        obj.Ref = logicalId + '(replaced)';
+        ret = true;
+      }
+      return true;
+    }
+
+    if (key.startsWith('Fn::')) {
+      if (Array.isArray(obj[key]) && obj[key].length > 0 && obj[key][0] === logicalId) {
+        obj[key][0] = logicalId + '(replaced)';
+        ret = true;
+      }
+      return true;
+    }
+
+    return false;
+  }
+
+  recurse(template);
+  return ret;
+}
+
+function deepCopy(x: any): any {
+  if (Array.isArray(x)) {
+    return x.map(deepCopy);
+  }
+
+  if (typeof x === 'object' && x !== null) {
+    const ret: any = {};
+    for (const key of Object.keys(x)) {
+      ret[key] = deepCopy(x[key]);
+    }
+    return ret;
+  }
+
+  return x;
+}

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

@@ -109,7 +109,9 @@ exports.diffTemplate = {
     const bucketName = 'ShineyBucketName';
     const currentTemplate = {
       Resources: {
-        BucketPolicyResource: BUCKET_POLICY_RESOURCE,
+        QueueResource: {
+          Type: 'AWS::SQS::Queue'
+        },
         BucketResource: {
           Type: 'AWS::S3::Bucket',
           Properties: {
@@ -122,7 +124,9 @@ exports.diffTemplate = {
     const newBucketName = `${bucketName}-v2`;
     const newTemplate = {
       Resources: {
-        BucketPolicyResource: BUCKET_POLICY_RESOURCE,
+        QueueResource: {
+          Type: 'AWS::SQS::Queue'
+        },
         BucketResource: {
           Type: 'AWS::S3::Bucket',
           Properties: {
@@ -148,7 +152,9 @@ exports.diffTemplate = {
     const bucketName = 'ShineyBucketName';
     const currentTemplate = {
       Resources: {
-        BucketPolicyResource: BUCKET_POLICY_RESOURCE,
+        QueueResource: {
+          Type: 'AWS::SQS::Queue'
+        },
         BucketResource: {
           Type: 'AWS::S3::Bucket',
           Properties: {
@@ -160,7 +166,9 @@ exports.diffTemplate = {
 
     const newTemplate = {
       Resources: {
-        BucketPolicyResource: BUCKET_POLICY_RESOURCE,
+        QueueResource: {
+          Type: 'AWS::SQS::Queue'
+        },
         BucketResource: {
           Type: 'AWS::S3::Bucket'
         }
@@ -183,7 +191,9 @@ exports.diffTemplate = {
     const bucketName = 'ShineyBucketName';
     const currentTemplate = {
       Resources: {
-        BucketPolicyResource: BUCKET_POLICY_RESOURCE,
+        QueueResource: {
+          Type: 'AWS::SQS::Queue'
+        },
         BucketResource: {
           Type: 'AWS::S3::Bucket'
         }
@@ -192,7 +202,9 @@ exports.diffTemplate = {
 
     const newTemplate = {
       Resources: {
-        BucketPolicyResource: BUCKET_POLICY_RESOURCE,
+        QueueResource: {
+          Type: 'AWS::SQS::Queue'
+        },
         BucketResource: {
           Type: 'AWS::S3::Bucket',
           Properties: {
@@ -247,5 +259,52 @@ exports.diffTemplate = {
              'the difference reflects the type change');
     test.equal(difference && difference.changeImpact, ResourceImpact.WILL_REPLACE, 'the difference reflects that the resource will be replaced');
     test.done();
-  }
+  },
+
+  'resource replacement is tracked through references': (test: Test) => {
+    // If a resource is replaced, then that change shows that references are
+    // going to change. This may lead to replacement of downstream resources
+    // if the reference is used in an immutable property, and so on.
+
+    // GIVEN
+    const currentTemplate = {
+      Resources: {
+        Bucket: {
+          Type: 'AWS::S3::Bucket',
+          Properties: { BucketName: 'Name1', }, // Immutable prop
+        },
+        Queue: {
+          Type: 'AWS::SQS::Queue',
+          Properties: { QueueName: { Ref: 'Bucket' }}, // Immutable prop
+        },
+        Topic: {
+          Type: 'AWS::SNS::Topic',
+          Properties: { TopicName: { Ref: 'Queue' }}, // Immutable prop
+        }
+      }
+    };
+
+    // WHEN
+    const newTemplate = {
+      Resources: {
+        Bucket: {
+          Type: 'AWS::S3::Bucket',
+          Properties: { BucketName: 'Name2', },
+        },
+        Queue: {
+          Type: 'AWS::SQS::Queue',
+          Properties: { QueueName: { Ref: 'Bucket' }},
+        },
+        Topic: {
+          Type: 'AWS::SNS::Topic',
+          Properties: { TopicName: { Ref: 'Queue' }},
+        }
+      }
+    };
+    const differences = diffTemplate(currentTemplate, newTemplate);
+
+    // THEN
+    test.equal(differences.resources.count, 3, 'all resources are replaced');
+    test.done();
+  },
 };