Skip to content

Commit

Permalink
refactor(core): revisit the Stack API (#2818)
Browse files Browse the repository at this point in the history
Fixes #2728 

BREAKING CHANGE: multiple changes to the `Stack` API
* **core:** `stack.name` renamed to `stack.stackName`
* **core:** `stack.stackName` will return the concrete stack name. Use `Aws.stackName` to indicate { Ref: "AWS::StackName" }.
* **core:** `stack.account` and `stack.region` will return the concrete account/region only if they are explicitly specified when the stack is defined (under the `env` prop). Otherwise, they will return a token that resolves to the AWS::AccountId and AWS::Region intrinsic references. Use `Context.getDefaultAccount()` and `Context.getDefaultRegion()` to obtain the defaults passed through the toolkit in case those are needed. Use `Token.isUnresolved(v)` to check if you have a concrete or intrinsic.
* **core:** `stack.logicalId` has been removed. Use `stack.getLogicalId()`
* **core:** `stack.env` has been removed, use `stack.account`, `stack.region` and `stack.environment` instead
* **core:** `stack.accountId` renamed to `stack.account` (to allow treating account more abstractly)
* **core:** `AvailabilityZoneProvider` can now be accessed through `Context.getAvailabilityZones()`
* **core:** `SSMParameterProvider` can now be accessed through `Context.getSsmParameter()`
* **core:** `parseArn` is now `Arn.parse`
* **core:** `arnFromComponents` is now `arn.format`
* **core:** `node.lock` and `node.unlock` are now private
* **core:** `stack.requireRegion` and `requireAccountId` have been removed. Use `Token.unresolved(stack.region)` instead
* **core:** `stack.parentApp` have been removed. Use `App.isApp(stack.node.root)` instead.
* **core:** `stack.missingContext` is now private
* **core:** `stack.renameLogical` have been renamed to `stack.renameLogicalId`
* **core:** `IAddressingScheme`, `HashedAddressingScheme` and `LogicalIDs` are now internal. Override `Stack.allocateLogicalId` to customize how logical IDs are allocated to resources.
  • Loading branch information
Elad Ben-Israel committed Jun 12, 2019
1 parent 0f30e39 commit 47afdc2
Show file tree
Hide file tree
Showing 47 changed files with 771 additions and 798 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class PipelineDeployStackAction extends cdk.Construct {
constructor(scope: cdk.Construct, id: string, props: PipelineDeployStackActionProps) {
super(scope, id);

if (!cdk.environmentEquals(props.stack.env, cdk.Stack.of(this).env)) {
if (props.stack.environment !== cdk.Stack.of(this).environment) {
// FIXME: Add the necessary to extend to stacks in a different account
throw new Error(`Cross-environment deployment is not supported`);
}
Expand All @@ -125,8 +125,8 @@ export class PipelineDeployStackAction extends cdk.Construct {
actionName: 'ChangeSet',
changeSetName,
runOrder: createChangeSetRunOrder,
stackName: props.stack.name,
templatePath: props.input.atPath(`${props.stack.name}.template.yaml`),
stackName: props.stack.stackName,
templatePath: props.input.atPath(`${props.stack.stackName}.template.yaml`),
adminPermissions: props.adminPermissions,
deploymentRole: props.role,
capabilities,
Expand All @@ -136,7 +136,7 @@ export class PipelineDeployStackAction extends cdk.Construct {
actionName: 'Execute',
changeSetName,
runOrder: executeChangeSetRunOrder,
stackName: props.stack.name,
stackName: props.stack.stackName,
}));

this.deploymentRole = changeSetAction.deploymentRole;
Expand All @@ -160,7 +160,7 @@ export class PipelineDeployStackAction extends cdk.Construct {
const assets = this.stack.node.metadata.filter(md => md.type === cxapi.ASSET_METADATA);
if (assets.length > 0) {
// FIXME: Implement the necessary actions to publish assets
result.push(`Cannot deploy the stack ${this.stack.name} because it references ${assets.length} asset(s)`);
result.push(`Cannot deploy the stack ${this.stack.stackName} because it references ${assets.length} asset(s)`);
}
return result;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/assert/lib/synth-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export class SynthUtils {
// always synthesize against the root (be it an App or whatever) so all artifacts will be included
const root = stack.node.root;
const assembly = ConstructNode.synth(root.node, options);
return assembly.getStack(stack.name);
return assembly.getStack(stack.stackName);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/assert/test/test.assertions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ function synthesizedStack(fn: (stack: cdk.Stack) => void): cx.CloudFormationStac
fn(stack);

const assembly = app.synth();
return assembly.getStack(stack.name);
return assembly.getStack(stack.stackName);
}

interface TestResourceProps extends cdk.CfnResourceProps {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assets/test/test.asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export = {
path: dirPath
});

const synth = app.synth().getStack(stack.name);
const synth = app.synth().getStack(stack.stackName);
const meta = synth.manifest.metadata || {};
test.ok(meta['/my-stack/MyAsset']);
test.ok(meta['/my-stack/MyAsset'][0]);
Expand Down Expand Up @@ -344,7 +344,7 @@ export = {

// WHEN
const session = app.synth();
const artifact = session.getStack(stack.name);
const artifact = session.getStack(stack.stackName);
const metadata = artifact.manifest.metadata || {};
const md = Object.values(metadata)[0]![0]!.data;
test.deepEqual(md.path, 'asset.6b84b87243a4a01c592d78e1fd3855c4bfef39328cd0a450cc97e81717fea2a2');
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/lib/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class LatestDeploymentResource extends CfnDeployment {
constructor(scope: Construct, id: string, props: CfnDeploymentProps) {
super(scope, id, props);

this.originalLogicalId = Stack.of(this).logicalIds.getLogicalId(this);
this.originalLogicalId = Stack.of(this).getLogicalId(this);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudtrail/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export class Trail extends Resource {
.addServicePrincipal(cloudTrailPrincipal));

s3bucket.addToResourcePolicy(new iam.PolicyStatement()
.addResource(s3bucket.arnForObjects(`AWSLogs/${Stack.of(this).accountId}/*`))
.addResource(s3bucket.arnForObjects(`AWSLogs/${Stack.of(this).account}/*`))
.addActions("s3:PutObject")
.addServicePrincipal(cloudTrailPrincipal)
.setCondition("StringEquals", {'s3:x-amz-acl': "bucket-owner-full-control"}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import iam = require('@aws-cdk/aws-iam');
import lambda = require('@aws-cdk/aws-lambda');
import s3 = require('@aws-cdk/aws-s3');
import sns = require('@aws-cdk/aws-sns');
import { App, CfnParameter, ConstructNode, PhysicalName, SecretValue, Stack } from '@aws-cdk/cdk';
import { App, Aws, CfnParameter, ConstructNode, PhysicalName, SecretValue, Stack } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import cpactions = require('../lib');

Expand Down Expand Up @@ -56,7 +56,7 @@ export = {
const stack = new Stack(undefined, 'StackName');

new codepipeline.Pipeline(stack, 'Pipeline', {
pipelineName: stack.stackName,
pipelineName: Aws.stackName,
});

expect(stack, true).to(haveResourceLike('AWS::CodePipeline::Pipeline', {
Expand Down Expand Up @@ -695,8 +695,8 @@ export = {

const usEast1ScaffoldStack = pipeline.crossRegionScaffolding['us-east-1'];
test.notEqual(usEast1ScaffoldStack, undefined);
test.equal(usEast1ScaffoldStack.env.region, 'us-east-1');
test.equal(usEast1ScaffoldStack.env.account, pipelineAccount);
test.equal(usEast1ScaffoldStack.region, 'us-east-1');
test.equal(usEast1ScaffoldStack.account, pipelineAccount);
test.ok(usEast1ScaffoldStack.node.id.indexOf('us-east-1') !== -1,
`expected '${usEast1ScaffoldStack.node.id}' to contain 'us-east-1'`);

Expand Down
34 changes: 22 additions & 12 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import events = require('@aws-cdk/aws-events');
import iam = require('@aws-cdk/aws-iam');
import kms = require('@aws-cdk/aws-kms');
import s3 = require('@aws-cdk/aws-s3');
import { Construct, Lazy, PhysicalName, RemovalPolicy, Resource, Stack } from '@aws-cdk/cdk';
import { App, Construct, Lazy, PhysicalName, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/cdk';
import { Action, IPipeline, IStage } from "./action";
import { CfnPipeline } from './codepipeline.generated';
import { Stage } from './stage';
Expand Down Expand Up @@ -364,14 +364,21 @@ export class Pipeline extends PipelineBase {
});
}

private requireRegion() {
const region = Stack.of(this).region;
if (Token.isUnresolved(region)) {
throw new Error(`You need to specify an explicit region when using CodePipeline's cross-region support`);
}
return region;
}

private ensureReplicationBucketExistsFor(region?: string) {
if (!region) {
return;
}

// get the region the Pipeline itself is in
const pipelineRegion = Stack.of(this).requireRegion(
"You need to specify an explicit region when using CodePipeline's cross-region support");
const pipelineRegion = this.requireRegion();

// if we already have an ArtifactStore generated for this region, or it's the Pipeline's region, nothing to do
if (this.artifactStores[region] || region === pipelineRegion) {
Expand All @@ -380,11 +387,14 @@ export class Pipeline extends PipelineBase {

let replicationBucketName = this.crossRegionReplicationBuckets[region];
if (!replicationBucketName) {
const pipelineAccount = Stack.of(this).requireAccountId(
"You need to specify an explicit account when using CodePipeline's cross-region support");
const app = Stack.of(this).parentApp();
if (!app) {
throw new Error(`Pipeline stack which uses cross region actions must be part of an application`);
const pipelineAccount = Stack.of(this).account;
if (Token.isUnresolved(pipelineAccount)) {
throw new Error("You need to specify an explicit account when using CodePipeline's cross-region support");
}

const app = this.node.root;
if (!app || !App.isApp(app)) {
throw new Error(`Pipeline stack which uses cross region actions must be part of a CDK app`);
}
const crossRegionScaffoldStack = new CrossRegionScaffoldStack(this, `cross-region-stack-${pipelineAccount}:${region}`, {
region,
Expand Down Expand Up @@ -421,8 +431,7 @@ export class Pipeline extends PipelineBase {
const pipelineStack = Stack.of(this);
const resourceStack = Stack.of(action.resource);
// check if resource is from a different account
if (pipelineStack.env.account && resourceStack.env.account &&
pipelineStack.env.account !== resourceStack.env.account) {
if (pipelineStack.environment !== resourceStack.environment) {
// if it is, the pipeline's bucket must have a KMS key
if (!this.artifactBucket.encryptionKey) {
throw new Error('The Pipeline is being used in a cross-account manner, ' +
Expand All @@ -434,7 +443,7 @@ export class Pipeline extends PipelineBase {
// generate a role in the other stack, that the Pipeline will assume for executing this action
actionRole = new iam.Role(resourceStack,
`${this.node.uniqueId}-${stage.stageName}-${action.actionName}-ActionRole`, {
assumedBy: new iam.AccountPrincipal(pipelineStack.env.account),
assumedBy: new iam.AccountPrincipal(pipelineStack.account),
roleName: PhysicalName.auto({ crossEnvironment: true }),
});

Expand Down Expand Up @@ -555,7 +564,8 @@ export class Pipeline extends PipelineBase {

// add the Pipeline's artifact store
const primaryStore = this.renderPrimaryArtifactStore();
this.artifactStores[Stack.of(this).requireRegion()] = {
const primaryRegion = this.requireRegion();
this.artifactStores[primaryRegion] = {
location: primaryStore.location,
type: primaryStore.type,
encryptionKey: primaryStore.encryptionKey,
Expand Down
20 changes: 8 additions & 12 deletions packages/@aws-cdk/aws-ec2/lib/machine-image.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, SSMParameterProvider, Stack } from '@aws-cdk/cdk';
import { Construct, Context, Stack, Token } from '@aws-cdk/cdk';

/**
* Interface for classes that can select an appropriate machine image to use
Expand All @@ -25,11 +25,7 @@ export class WindowsImage implements IMachineImageSource {
* Return the image to use in the given context
*/
public getImage(scope: Construct): MachineImage {
const ssmProvider = new SSMParameterProvider(scope, {
parameterName: this.imageParameterName(this.version),
});

const ami = ssmProvider.parameterValue();
const ami = Context.getSsmParameter(scope, this.imageParameterName(this.version));
return new MachineImage(ami, new WindowsOS());
}

Expand Down Expand Up @@ -106,11 +102,7 @@ export class AmazonLinuxImage implements IMachineImageSource {
].filter(x => x !== undefined); // Get rid of undefineds

const parameterName = '/aws/service/ami-amazon-linux-latest/' + parts.join('-');

const ssmProvider = new SSMParameterProvider(scope, {
parameterName,
});
const ami = ssmProvider.parameterValue();
const ami = Context.getSsmParameter(scope, parameterName);
return new MachineImage(ami, new LinuxOS());
}
}
Expand Down Expand Up @@ -188,7 +180,11 @@ export class GenericLinuxImage implements IMachineImageSource {
}

public getImage(scope: Construct): MachineImage {
const region = Stack.of(scope).requireRegion('AMI cannot be determined');
let region = Stack.of(scope).region;
if (Token.isUnresolved(region)) {
region = Context.getDefaultRegion(scope);
}

const ami = region !== 'test-region' ? this.amiMap[region] : 'ami-12345';
if (!ami) {
throw new Error(`Unable to find AMI in AMI map: no AMI specified for region '${region}'`);
Expand Down
7 changes: 1 addition & 6 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -789,8 +789,7 @@ export class Vpc extends VpcBase {

this.node.applyAspect(new cdk.Tag(NAME_TAG, this.node.path));

this.availabilityZones = new cdk.AvailabilityZoneProvider(this).availabilityZones;
this.availabilityZones.sort();
this.availabilityZones = cdk.Context.getAvailabilityZones(this);

const maxAZs = props.maxAZs !== undefined ? props.maxAZs : 3;
this.availabilityZones = this.availabilityZones.slice(0, maxAZs);
Expand Down Expand Up @@ -946,10 +945,6 @@ export class Vpc extends VpcBase {
private createSubnets() {
const remainingSpaceSubnets: SubnetConfiguration[] = [];

// Calculate number of public/private subnets based on number of AZs
const zones = new cdk.AvailabilityZoneProvider(this).availabilityZones;
zones.sort();

for (const subnet of this.subnetConfiguration) {
if (subnet.cidrMask === undefined) {
remainingSpaceSubnets.push(subnet);
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-ec2/test/test.vpc.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { countResources, expect, haveResource, haveResourceLike, isSuperObject } from '@aws-cdk/assert';
import { AvailabilityZoneProvider, Construct, Stack, Tag } from '@aws-cdk/cdk';
import { Construct, Context, Stack, Tag } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { CfnVPC, DefaultInstanceTenancy, IVpc, SubnetType, Vpc } from '../lib';
import { exportVpc } from './export-helper';
Expand Down Expand Up @@ -63,7 +63,7 @@ export = {
"contains the correct number of subnets"(test: Test) {
const stack = getTestStack();
const vpc = new Vpc(stack, 'TheVPC');
const zones = new AvailabilityZoneProvider(stack).availabilityZones.length;
const zones = Context.getAvailabilityZones(stack).length;
test.equal(vpc.publicSubnets.length, zones);
test.equal(vpc.privateSubnets.length, zones);
test.deepEqual(stack.resolve(vpc.vpcId), { Ref: 'TheVPC92636AB0' });
Expand Down Expand Up @@ -109,7 +109,7 @@ export = {

"with no subnets defined, the VPC should have an IGW, and a NAT Gateway per AZ"(test: Test) {
const stack = getTestStack();
const zones = new AvailabilityZoneProvider(stack).availabilityZones.length;
const zones = Context.getAvailabilityZones(stack).length;
new Vpc(stack, 'TheVPC', { });
expect(stack).to(countResources("AWS::EC2::InternetGateway", 1));
expect(stack).to(countResources("AWS::EC2::NatGateway", zones));
Expand Down Expand Up @@ -186,7 +186,7 @@ export = {
},
"with custom subnets, the VPC should have the right number of subnets, an IGW, and a NAT Gateway per AZ"(test: Test) {
const stack = getTestStack();
const zones = new AvailabilityZoneProvider(stack).availabilityZones.length;
const zones = Context.getAvailabilityZones(stack).length;
new Vpc(stack, 'TheVPC', {
cidr: '10.0.0.0/21',
subnetConfiguration: [
Expand Down
9 changes: 2 additions & 7 deletions packages/@aws-cdk/aws-ecs/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import cloudwatch = require ('@aws-cdk/aws-cloudwatch');
import ec2 = require('@aws-cdk/aws-ec2');
import iam = require('@aws-cdk/aws-iam');
import cloudmap = require('@aws-cdk/aws-servicediscovery');
import { Construct, IResource, Resource, SSMParameterProvider, Stack } from '@aws-cdk/cdk';
import { Construct, Context, IResource, Resource, Stack } from '@aws-cdk/cdk';
import { InstanceDrainHook } from './drain-hook/instance-drain-hook';
import { CfnCluster } from './ecs.generated';

Expand Down Expand Up @@ -268,13 +268,8 @@ export class EcsOptimizedAmi implements ec2.IMachineImageSource {
* Return the correct image
*/
public getImage(scope: Construct): ec2.MachineImage {
const ssmProvider = new SSMParameterProvider(scope, {
parameterName: this.amiParameterName
});

const json = ssmProvider.parameterValue("{\"image_id\": \"\"}");
const json = Context.getSsmParameter(scope, this.amiParameterName, { defaultValue: "{\"image_id\": \"\"}" });
const ami = JSON.parse(json).image_id;

return new ec2.MachineImage(ami, new ec2.LinuxOS());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import cloudwatch = require('@aws-cdk/aws-cloudwatch');
import ec2 = require('@aws-cdk/aws-ec2');
import iam = require('@aws-cdk/aws-iam');
import s3 = require('@aws-cdk/aws-s3');
import { Construct, Lazy, Resource, Stack } from '@aws-cdk/cdk';
import { Construct, Lazy, Resource, Stack, Token } from '@aws-cdk/cdk';
import { BaseLoadBalancer, BaseLoadBalancerProps, ILoadBalancerV2 } from '../shared/base-load-balancer';
import { IpAddressType } from '../shared/enums';
import { ApplicationListener, BaseApplicationListenerProps } from './application-listener';
Expand Down Expand Up @@ -86,7 +86,11 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic
this.setAttribute('access_logs.s3.bucket', bucket.bucketName.toString());
this.setAttribute('access_logs.s3.prefix', prefix);

const region = Stack.of(this).requireRegion('Enable ELBv2 access logging');
const region = Stack.of(this).region;
if (Token.isUnresolved(region)) {
throw new Error(`Region is required to enable ELBv2 access logging`);
}

const account = ELBV2_ACCOUNTS[region];
if (!account) {
throw new Error(`Cannot enable access logging; don't know ELBv2 account for region ${region}`);
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-glue/lib/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class Database extends Resource implements IDatabase {
public databaseArn = databaseArn;
public databaseName = stack.parseArn(databaseArn).resourceName!;
public catalogArn = stack.formatArn({ service: 'glue', resource: 'catalog' });
public catalogId = stack.accountId;
public catalogId = stack.account;
}

return new Import(scope, id);
Expand Down Expand Up @@ -95,7 +95,7 @@ export class Database extends Resource implements IDatabase {
this.locationUri = `s3://${bucket.bucketName}/${props.databaseName}`;
}

this.catalogId = Stack.of(this).accountId;
this.catalogId = Stack.of(this).account;
const resource = new CfnDatabase(this, 'Resource', {
catalogId: this.catalogId,
databaseInput: {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/lib/principals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export class FederatedPrincipal extends PrincipalBase {

export class AccountRootPrincipal extends AccountPrincipal {
constructor() {
super(new StackDependentToken(stack => stack.accountId).toString());
super(new StackDependentToken(stack => stack.account).toString());
}

public toString() {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-kms/test/integ.key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const key = new Key(stack, 'MyKey', { retain: false });
key.addToResourcePolicy(new PolicyStatement()
.addAllResources()
.addAction('kms:encrypt')
.addAwsPrincipal(stack.accountId));
.addAwsPrincipal(stack.account));

key.addAlias('alias/bar');

Expand Down
Loading

0 comments on commit 47afdc2

Please sign in to comment.