fix(core): use default account/region when environment is not specified (#2867)

Elad Ben-Israel · shivlaks · commit e9a4a7967a7f · 2019-06-13T12:16:42.000-07:00
The PR #2728 caused a regression on how the framework behaves when a stack's environment (account and/or region) are not specified. The behavior is: if account and/or region are not specified at the stack level (or set to Aws.accountId and Aws.region, respectively), the cloud assembly manifest of this stack will have an environment specification based on the defaults passed in from the CLI. This is a temporary solution which fixes #2853 but doesn't fully implement the behavior we eventually want (which is documented in #2866).
diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts
@@ -505,17 +505,24 @@ export class Stack extends Construct implements ITaggable {
    */
   private parseEnvironment(env: Environment = {}) {
     // if an environment property is explicitly specified when the stack is
-    // created, it will be used as concrete values for all intents.
-    const region = env.region;
-    const account = env.account;
+    // created, it will be used as concrete values for all intents. if not, use
+    // tokens for account and region but they do not need to be scoped, the only
+    // situation in which export/fn::importvalue would work if { Ref:
+    // "AWS::AccountId" } is the same for provider and consumer anyway.
+    const region = env.region || Aws.region;
+    const account = env.account || Aws.accountId;
+
+    // temporary fix for #2853, eventually behavior will be based on #2866.
+    // set the cloud assembly manifest environment spec of this stack to use the
+    // default account/region from the toolkit in case account/region are undefined or
+    // unresolved (i.e. tokens).
+    const envAccount = !Token.isUnresolved(account) ? account : Context.getDefaultAccount(this) || 'unknown-account';
+    const envRegion  = !Token.isUnresolved(region)  ? region  : Context.getDefaultRegion(this)  || 'unknown-region';
 
-    // account and region do not need to be scoped, the only situation in which
-    // export/fn::importvalue would work if { Ref: "AWS::AccountId" } is the
-    // same for provider and consumer anyway.
     return {
       account: account || Aws.accountId,
       region: region || Aws.region,
-      environment: EnvironmentUtils.format(account || 'unknown-account', region || 'unknown-region')
+      environment: EnvironmentUtils.format(envAccount, envRegion)
     };
   }
 
@@ -651,9 +658,11 @@ function cfnElements(node: IConstruct, into: CfnElement[] = []): CfnElement[] {
 import { Arn, ArnComponents } from './arn';
 import { CfnElement } from './cfn-element';
 import { CfnResource, TagType } from './cfn-resource';
+import { Context } from './context';
 import { CfnReference } from './private/cfn-reference';
 import { Aws, ScopedAws } from './pseudo';
 import { ITaggable, TagManager } from './tag-manager';
+import { Token } from './token';
 
 /**
  * Find all resources in a set of constructs
diff --git a/packages/@aws-cdk/cdk/test/test.environment.ts b/packages/@aws-cdk/cdk/test/test.environment.ts
@@ -1,6 +1,7 @@
 import { DEFAULT_ACCOUNT_CONTEXT_KEY, DEFAULT_REGION_CONTEXT_KEY } from '@aws-cdk/cx-api';
+import cxapi = require('@aws-cdk/cx-api');
 import { Test } from 'nodeunit';
-import { App, Stack, Token } from '../lib';
+import { App, Aws, Stack, Token } from '../lib';
 
 export = {
   'By default, environment region and account are not defined and resolve to intrinsics'(test: Test) {
@@ -40,4 +41,140 @@ export = {
 
     test.done();
   },
+
+  'environment defaults': {
+    'default-account-unknown-region'(test: Test) {
+      // GIVEN
+      const app = new App();
+
+      // WHEN
+      app.node.setContext(cxapi.DEFAULT_ACCOUNT_CONTEXT_KEY, 'my-default-account');
+      const stack = new Stack(app, 'stack');
+
+      // THEN
+      test.deepEqual(stack.resolve(stack.account), { Ref: 'AWS::AccountId' }); // TODO: after we implement #2866 this should be 'my-default-account'
+      test.deepEqual(stack.resolve(stack.region), { Ref: 'AWS::Region' });
+      test.deepEqual(app.synth().getStack(stack.stackName).environment, {
+        account: 'my-default-account',
+        region: 'unknown-region',
+        name: 'aws://my-default-account/unknown-region'
+      });
+
+      test.done();
+    },
+
+    'default-account-explicit-region'(test: Test) {
+      // GIVEN
+      const app = new App();
+
+      // WHEN
+      app.node.setContext(cxapi.DEFAULT_ACCOUNT_CONTEXT_KEY, 'my-default-account');
+      const stack = new Stack(app, 'stack', { env: { region: 'explicit-region' }});
+
+      // THEN
+      test.deepEqual(stack.resolve(stack.account), { Ref: 'AWS::AccountId' }); // TODO: after we implement #2866 this should be 'my-default-account'
+      test.deepEqual(stack.resolve(stack.region), 'explicit-region');
+      test.deepEqual(app.synth().getStack(stack.stackName).environment, {
+        account: 'my-default-account',
+        region: 'explicit-region',
+        name: 'aws://my-default-account/explicit-region'
+      });
+
+      test.done();
+    },
+
+    'explicit-account-explicit-region'(test: Test) {
+      // GIVEN
+      const app = new App();
+
+      // WHEN
+      app.node.setContext(cxapi.DEFAULT_ACCOUNT_CONTEXT_KEY, 'my-default-account');
+      const stack = new Stack(app, 'stack', { env: {
+        account: 'explicit-account',
+        region: 'explicit-region'
+      }});
+
+      // THEN
+      test.deepEqual(stack.resolve(stack.account), 'explicit-account');
+      test.deepEqual(stack.resolve(stack.region), 'explicit-region');
+      test.deepEqual(app.synth().getStack(stack.stackName).environment, {
+        account: 'explicit-account',
+        region: 'explicit-region',
+        name: 'aws://explicit-account/explicit-region'
+      });
+
+      test.done();
+    },
+
+    'default-account-default-region'(test: Test) {
+      // GIVEN
+      const app = new App();
+
+      // WHEN
+      app.node.setContext(cxapi.DEFAULT_ACCOUNT_CONTEXT_KEY, 'my-default-account');
+      app.node.setContext(cxapi.DEFAULT_REGION_CONTEXT_KEY, 'my-default-region');
+      const stack = new Stack(app, 'stack');
+
+      // THEN
+      test.deepEqual(stack.resolve(stack.account), { Ref: 'AWS::AccountId' }); // TODO: after we implement #2866 this should be 'my-default-account'
+      test.deepEqual(stack.resolve(stack.region), { Ref: 'AWS::Region' });     // TODO: after we implement #2866 this should be 'my-default-region'
+      test.deepEqual(app.synth().getStack(stack.stackName).environment, {
+        account: 'my-default-account',
+        region: 'my-default-region',
+        name: 'aws://my-default-account/my-default-region'
+      });
+
+      test.done();
+    },
+
+    'token-account-token-region-no-defaults'(test: Test) {
+      // GIVEN
+      const app = new App();
+
+      // WHEN
+      app.node.setContext(cxapi.DEFAULT_ACCOUNT_CONTEXT_KEY, 'my-default-account');
+      app.node.setContext(cxapi.DEFAULT_REGION_CONTEXT_KEY, 'my-default-region');
+      const stack = new Stack(app, 'stack', {
+        env: {
+          account: Aws.accountId,
+          region: Aws.region
+        }
+      });
+
+      // THEN
+      test.deepEqual(stack.resolve(stack.account), { Ref: 'AWS::AccountId' });
+      test.deepEqual(stack.resolve(stack.region), { Ref: 'AWS::Region' });
+      test.deepEqual(app.synth().getStack(stack.stackName).environment, {
+        account: 'my-default-account',
+        region: 'my-default-region',
+        name: 'aws://my-default-account/my-default-region'
+      });
+
+      test.done();
+    },
+
+    'token-account-token-region-with-defaults'(test: Test) {
+      // GIVEN
+      const app = new App();
+
+      // WHEN
+      const stack = new Stack(app, 'stack', {
+        env: {
+          account: Aws.accountId,
+          region: Aws.region
+        }
+      });
+
+      // THEN
+      test.deepEqual(stack.resolve(stack.account), { Ref: 'AWS::AccountId' });
+      test.deepEqual(stack.resolve(stack.region), { Ref: 'AWS::Region' });
+      test.deepEqual(app.synth().getStack(stack.stackName).environment, {
+        account: 'unknown-account',
+        region: 'unknown-region',
+        name: 'aws://unknown-account/unknown-region'
+      });
+
+      test.done();
+    }
+  },
 };
