fix(core): child stack assembly metadata is duplidated on parent (#4540)

* fix(core): child stack assembly metadata is duplidated on parent

When synthesizes the metadata section of a stack in the cloud assembly
manifest, we traverse the tree below the stack and collect the metadata
associated with all nodes in this tree. If this tree has a child stack,
we don't need to collect the metadata from it's node, because they would
appear in the metadata of that child stack.

Metadata annotated on nested stack nodes will be recorded by the
first non-nested stack parent.

Specifically this bug caused an issue when the child stack used assets.
Currently assets are reported through assembly metadata on the stack,
and they would be reported both on the child and on the parent (although
the CloudFormation parameters of the assets would not appear on the parent).
This resulted in a CloudFormation error that reads:

    ValidationError: Parameter values specified for a template which does not require them.

Therefore this fixes #2900

* remove unused import

packages/@aws-cdk/aws-cloudformation/test/test.nested-stack.ts

@@ -802,6 +802,31 @@ export = {
       }
     }));
 
+    test.done();
+  },
+
+  'metadata defined in nested stacks is reported at the parent stack level in the cloud assembly'(test: Test) {
+    // GIVEN
+    const app = new App({ stackTraces: false });
+    const parent = new Stack(app, 'parent');
+    const child = new Stack(parent, 'child');
+    const nested = new NestedStack(child, 'nested');
+    const resource = new CfnResource(nested, 'resource', { type: 'foo' });
+
+    // WHEN
+    resource.node.addMetadata('foo', 'bar');
+
+    // THEN: the first non-nested stack records the assembly metadata
+    const asm = app.synth();
+    test.deepEqual(asm.stacks.length, 2); // only one stack is defined as an artifact
+    test.deepEqual(asm.getStack(parent.stackName).findMetadataByType('foo'), []);
+    test.deepEqual(asm.getStack(child.stackName).findMetadataByType('foo'), [
+      {
+        path: '/parent/child/nested/resource',
+        type: 'foo',
+        data: 'bar'
+      }
+    ]);
     test.done();
   }
 };

packages/@aws-cdk/core/lib/stack.ts

@@ -860,6 +860,11 @@ export class Stack extends Construct implements ITaggable {
     return output;
 
     function visit(node: IConstruct) {
+      // break off if we reached a node that is not a child of this stack
+      const parent = findParentStack(node);
+      if (parent !== stack) {
+        return;
+      }
 
       if (node.node.metadata.length > 0) {
         // Make the path absolute
@@ -870,6 +875,18 @@ export class Stack extends Construct implements ITaggable {
         visit(child);
       }
     }
+
+    function findParentStack(node: IConstruct): Stack | undefined {
+      if (node instanceof Stack && node.parentStack === undefined) {
+        return node;
+      }
+
+      if (!node.node.scope) {
+        return undefined;
+      }
+
+      return findParentStack(node.node.scope);
+    }
   }
 
   /**

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

@@ -1,3 +1,4 @@
+import cxapi = require('@aws-cdk/cx-api');
 import { Test } from 'nodeunit';
 import { App, CfnCondition, CfnInclude, CfnOutput, CfnParameter, CfnResource, Construct, ConstructNode, Lazy, ScopedAws, Stack } from '../lib';
 import { validateString } from '../lib';
@@ -652,6 +653,28 @@ export = {
     test.deepEqual(stack1.templateFile, 'MyStack1.template.json');
     test.deepEqual(stack2.templateFile, 'MyRealStack2.template.json');
     test.done();
+  },
+
+  'metadata is collected at the stack boundary'(test: Test) {
+    // GIVEN
+    const app = new App({
+      context: {
+        [cxapi.DISABLE_METADATA_STACK_TRACE]: 'true'
+      }
+    });
+    const parent = new Stack(app, 'parent');
+    const child = new Stack(parent, 'child');
+
+    // WHEN
+    child.node.addMetadata('foo', 'bar');
+
+    // THEN
+    const asm = app.synth();
+    test.deepEqual(asm.getStack(parent.stackName).findMetadataByType('foo'), []);
+    test.deepEqual(asm.getStack(child.stackName).findMetadataByType('foo'), [
+      { path: '/parent/child', type: 'foo', data: 'bar' }
+    ]);
+    test.done();
   }
 };