From e9a4a7967a7f681d3757876fa90a22ffdad14723 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 13 Jun 2019 22:16:42 +0300 Subject: [PATCH] fix(core): use default account/region when environment is not specified (#2867) 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). --- packages/@aws-cdk/cdk/lib/stack.ts | 23 ++- .../@aws-cdk/cdk/test/test.environment.ts | 139 +++++++++++++++++- 2 files changed, 154 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index 7d88e7e84f31e..bde986884950e 100644 --- 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 index eaa0b8443b75f..47fac41072d0f 100644 --- 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(); + } + }, };