fix(core): property overrides sometimes don't work with intrinsics

packages/@aws-cdk/core/lib/cfn-resource.ts

@@ -496,6 +496,33 @@ export interface ICfnResourceOptions {
   metadata?: { [key: string]: any };
 }
 
+/**
+ * Object keys that deepMerge should not consider. Currently these include
+ * CloudFormation intrinsics
+ *
+ * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html
+ */
+
+const MERGE_EXCLUDE_KEYS: string[] = [
+  'Ref',
+  'Fn::Base64',
+  'Fn::Cidr',
+  'Fn::FindInMap',
+  'Fn::GetAtt',
+  'Fn::GetAZs',
+  'Fn::ImportValue',
+  'Fn::Join',
+  'Fn::Select',
+  'Fn::Split',
+  'Fn::Sub',
+  'Fn::Transform',
+  'Fn::And',
+  'Fn::Equals',
+  'Fn::If',
+  'Fn::Not',
+  'Fn::Or',
+];
+
 /**
  * Merges `source` into `target`, overriding any existing values.
  * `null`s will cause a value to be deleted.
@@ -513,6 +540,32 @@ function deepMerge(target: any, ...sources: any[]) {
         // object so we can continue the recursion
         if (typeof(target[key]) !== 'object') {
           target[key] = {};
+
+          /**
+           * If we have something that looks like:
+           *
+           *   target: { Type: 'MyResourceType', Properties: { prop1: { Ref: 'Param' } } }
+           *   sources: [ { Properties: { prop1: [ 'Fn::Join': ['-', 'hello', 'world'] ] } } ]
+           *
+           * Eventually we will get to the point where we have
+           *
+           *   target: { prop1: { Ref: 'Param' } }
+           *   sources: [ { prop1: { 'Fn::Join': ['-', 'hello', 'world'] } } ]
+           *
+           * We need to recurse 1 more time, but if we do we will end up with
+           *   { prop1: { Ref: 'Param', 'Fn::Join': ['-', 'hello', 'world'] } }
+           * which is not what we want.
+           *
+           * Instead we check to see whether the `target` value (i.e. target.prop1)
+           * is an object that contains a key that we don't want to recurse on. If it does
+           * then we essentially drop it and end up with:
+           *
+           *   { prop1: { 'Fn::Join': ['-', 'hello', 'world'] } }
+           */
+        } else if (Object.keys(target[key]).length === 1) {
+          if (MERGE_EXCLUDE_KEYS.includes(Object.keys(target[key])[0])) {
+            target[key] = {};
+          }
         }
 
         deepMerge(target[key], value);

packages/@aws-cdk/core/test/resource.test.ts

@@ -734,6 +734,76 @@ describe('resource', () => {
 
     });
 
+    test('overrides allow overriding one intrinsic with another', () => {
+      // GIVEN
+      const stack = new Stack();
+
+      const resource = new CfnResource(stack, 'MyResource', {
+        type: 'MyResourceType',
+        properties: {
+          prop1: Fn.ref('Param'),
+        },
+      });
+
+      // WHEN
+      resource.addPropertyOverride('prop1', Fn.join('-', ['hello', Fn.ref('Param')]));
+      const cfn = toCloudFormation(stack);
+
+      // THEN
+      expect(cfn.Resources.MyResource).toEqual({
+        Type: 'MyResourceType',
+        Properties: {
+          prop1: {
+            'Fn::Join': [
+              '-',
+              [
+                'hello',
+                {
+                  Ref: 'Param',
+                },
+              ],
+            ],
+          },
+        },
+      });
+    });
+
+    test('overrides allow overriding a nested intrinsic', () => {
+      // GIVEN
+      const stack = new Stack();
+
+      const resource = new CfnResource(stack, 'MyResource', {
+        type: 'MyResourceType',
+        properties: {
+          prop1: Fn.importValue(Fn.sub('${Sub}', { Sub: 'Value' })),
+        },
+      });
+
+      // WHEN
+      resource.addPropertyOverride('prop1', Fn.importValue(Fn.join('-', ['abc', Fn.sub('${Sub}', { Sub: 'Value' })])));
+      const cfn = toCloudFormation(stack);
+
+      // THEN
+      expect(cfn.Resources.MyResource).toEqual({
+        Type: 'MyResourceType',
+        Properties: {
+          prop1: {
+            'Fn::ImportValue': {
+              'Fn::Join': [
+                '-',
+                [
+                  'abc',
+                  {
+                    'Fn::Sub': ['${Sub}', { Sub: 'Value' }],
+                  },
+                ],
+              ],
+            },
+          },
+        },
+      });
+    });
+
     describe('using mutable properties', () => {
 
       test('can be used by derived classes to specify overrides before render()', () => {