Skip to content

Commit

Permalink
fix(core): use default account/region when environment is not specifi…
Browse files Browse the repository at this point in the history 
…ed (#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).
  • Loading branch information
@shivlaks
Elad Ben-Israel authored and shivlaks committed Jun 13, 2019
1 parent a691900 commit e9a4a79
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 8 deletions.
23 changes: 16 additions & 7 deletions packages/@aws-cdk/cdk/lib/stack.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};
}

Expand Down Expand Up @@ -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
Expand Down
139 changes: 138 additions & 1 deletion packages/@aws-cdk/cdk/test/test.environment.ts
View file
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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();
}
},
};

0 comments on commit e9a4a79

Please sign in to comment.