From ca5ee3568fc19e1aa2f2fa9623c7010303c34e9e Mon Sep 17 00:00:00 2001
From: Elad Ben-Israel <benisrae@amazon.com>
Date: Thu, 10 Jan 2019 16:17:20 +0200
Subject: [PATCH] fix(core): automatic cross-stack refs for CFN resources
 (#1510)

The "toCloudFormation" method of generated CFN resources invoke
"resolve" so that any lazy tokens are evaluated. This escaped the
global settings set by `findTokens` which collect tokens so they
can be reported as references by `StackElement.prepare`.

To solve this, findTokens now accepts a function instead of an object
and basically just wraps it's invocation with settings that will collect
all tokens resolved within that scope. Not an ideal solution but
simple enough.

This was not discovered because we didn't have any tests that validated
the behavior of the generated CFN resources (they are not accessible
from the @aws-cdk/cdk library). We add a unit test in the IAM library to cover this use case.
---
 .../test/test.auto-cross-stack-refs.ts        | 52 +++++++++++++++++++
 .../cdk/lib/cloudformation/stack-element.ts   |  5 +-
 .../@aws-cdk/cdk/lib/core/tokens/resolve.ts   |  9 ++--
 3 files changed, 59 insertions(+), 7 deletions(-)
 create mode 100644 packages/@aws-cdk/aws-iam/test/test.auto-cross-stack-refs.ts

diff --git a/packages/@aws-cdk/aws-iam/test/test.auto-cross-stack-refs.ts b/packages/@aws-cdk/aws-iam/test/test.auto-cross-stack-refs.ts
new file mode 100644
index 0000000000000..ffd1ea94c6d7c
--- /dev/null
+++ b/packages/@aws-cdk/aws-iam/test/test.auto-cross-stack-refs.ts
@@ -0,0 +1,52 @@
+import { expect } from '@aws-cdk/assert';
+import cdk = require('@aws-cdk/cdk');
+import { Test } from 'nodeunit';
+import iam = require('../lib');
+
+export = {
+  'automatic exports are created when attributes are referneced across stacks'(test: Test) {
+    // GIVEN
+    const stackWithUser = new cdk.Stack();
+    const stackWithGroup = new cdk.Stack();
+    const user = new iam.User(stackWithUser, 'User');
+    const group = new iam.Group(stackWithGroup, 'Group');
+
+    // WHEN
+    group.addUser(user);
+
+    //
+    // `group.addUser` adds the group to the user resource definition, so we expect
+    // that an automatic export will be created for the group and the user's stack
+    // to use ImportValue to import it.
+    // note that order of "expect"s matters. we first need to synthesize the user's
+    // stack so that the cross stack reference will be reported and only then the
+    // group's stack. in the real world, App will take care of this.
+    //
+
+    // THEN
+    expect(stackWithUser).toMatch({
+      Resources: {
+        User00B015A1: {
+          Type: "AWS::IAM::User",
+          Properties: {
+            Groups: [ { "Fn::ImportValue": "ExportsOutputRefGroupC77FDACD8CF7DD5B" } ]
+          }
+        }
+      }
+    });
+    expect(stackWithGroup).toMatch({
+      Outputs: {
+        ExportsOutputRefGroupC77FDACD8CF7DD5B: {
+          Value: { Ref: "GroupC77FDACD" },
+          Export: { Name: "ExportsOutputRefGroupC77FDACD8CF7DD5B" }
+        }
+      },
+      Resources: {
+        GroupC77FDACD: {
+          Type: "AWS::IAM::Group"
+        }
+      }
+    });
+    test.done();
+  }
+};
\ No newline at end of file
diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/stack-element.ts b/packages/@aws-cdk/cdk/lib/cloudformation/stack-element.ts
index e07cdae7a0825..175c564aa8bfe 100644
--- a/packages/@aws-cdk/cdk/lib/cloudformation/stack-element.ts
+++ b/packages/@aws-cdk/cdk/lib/cloudformation/stack-element.ts
@@ -128,10 +128,7 @@ export abstract class StackElement extends Construct implements IDependable {
       // This does make the assumption that the error will not be rectified,
       // but the error will be thrown later on anyway. If the error doesn't
       // get thrown down the line, we may miss references.
-      this.node.recordReference(...findTokens(this.toCloudFormation(), {
-        scope: this,
-        prefix: []
-      }));
+      this.node.recordReference(...findTokens(this, () => this.toCloudFormation()));
     } catch (e) {
       if (e.type !== 'CfnSynthesisError') { throw e; }
     }
diff --git a/packages/@aws-cdk/cdk/lib/core/tokens/resolve.ts b/packages/@aws-cdk/cdk/lib/core/tokens/resolve.ts
index 9fec0fc3c12ac..3d3e91e3e627d 100644
--- a/packages/@aws-cdk/cdk/lib/core/tokens/resolve.ts
+++ b/packages/@aws-cdk/cdk/lib/core/tokens/resolve.ts
@@ -1,3 +1,4 @@
+import { IConstruct } from '../construct';
 import { containsListToken, TOKEN_MAP } from "./encoding";
 import { RESOLVE_OPTIONS } from "./options";
 import { RESOLVE_METHOD, ResolveContext, Token } from "./token";
@@ -120,13 +121,15 @@ export function resolve(obj: any, context: ResolveContext): any {
 /**
  * Find all Tokens that are used in the given structure
  */
-export function findTokens(obj: any, context: ResolveContext): Token[] {
+export function findTokens(scope: IConstruct, fn: () => any): Token[] {
   const ret = new Array<Token>();
 
   const options = RESOLVE_OPTIONS.push({ collect: ret.push.bind(ret) });
   try {
-    // resolve() for side effect of calling 'preProcess', which adds to the
-    resolve(obj, context);
+    resolve(fn(), {
+      scope,
+      prefix: []
+    });
   } finally {
     options.pop();
   }