Skip to content

Commit

Permalink
feat(core): implicit app for root stacks
Browse files Browse the repository at this point in the history
When a stack is created as the root of the construct tree, we now implicitly create an `App` that serves as its parent scope.

The root stack is created with the ID `Default`, which ensures that `node.uniqueId` of constructs within that stack is preserved.

BREAKING CHANGE: in unit tests, the `node.path` of constructs within stacks created the root of the tree via `new Stack()` will now have a prefix `Default/` which represents an implicit `App` root.

Related: aws/aws-cdk-rfcs#192
  • Loading branch information
Elad Ben-Israel committed Jul 29, 2020
1 parent 3cad6a3 commit a9b059f
Show file tree
Hide file tree
Showing 34 changed files with 186 additions and 124 deletions.
32 changes: 23 additions & 9 deletions packages/@aws-cdk/assert/lib/synth-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@ import * as core from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';

export class SynthUtils {
/**
* Returns the cloud assembly template artifact for a stack.
*/
public static synthesize(stack: core.Stack, options: core.SynthesisOptions = { }): cxapi.CloudFormationStackArtifact {
// always synthesize against the root (be it an App or whatever) so all artifacts will be included
const root = stack.node.root;

// if the root is an app, invoke "synth" to avoid double synthesis
const assembly = root instanceof core.App ? root.synth() : core.ConstructNode.synth(root.node, options);

const assembly = synthesizeApp(stack, options);
return assembly.getStackArtifact(stack.artifactId);
}

Expand Down Expand Up @@ -51,10 +50,7 @@ export class SynthUtils {
*/
public static _synthesizeWithNested(stack: core.Stack, options: core.SynthesisOptions = { }): cxapi.CloudFormationStackArtifact | object {
// always synthesize against the root (be it an App or whatever) so all artifacts will be included
const root = stack.node.root;

// if the root is an app, invoke "synth" to avoid double synthesis
const assembly = root instanceof core.App ? root.synth() : core.ConstructNode.synth(root.node, options);
const assembly = synthesizeApp(stack, options);

// if this is a nested stack (it has a parent), then just read the template as a string
if (stack.nestedStackParent) {
Expand All @@ -65,6 +61,24 @@ export class SynthUtils {
}
}

/**
* Synthesizes the app in which a stack resides and returns the cloud assembly object.
*/
function synthesizeApp(stack: core.Stack, options: core.SynthesisOptions) {
const root = stack.node.root;
if (!core.Stage.isStage(root)) {
throw new Error('unexpected: all stacks must be part of a Stage or an App');
}

// to support incremental assertions (i.e. "expect(stack).toNotContainSomething(); doSomething(); expect(stack).toContainSomthing()")
const force = true;

return root.synth({
force,
...options,
});
}

export interface SubsetOptions {
/**
* Match all resources of the given type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
"BooksApi60AC975F"
]
},
"BooksApiDeployment86CA39AFc1570c78b1ea90526c0309cd74b7b8d0": {
"BooksApiDeployment86CA39AF4ff82f86c127f53c9de94d266b1906be": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"RestApiId": {
Expand All @@ -141,7 +141,7 @@
"Ref": "BooksApi60AC975F"
},
"DeploymentId": {
"Ref": "BooksApiDeployment86CA39AFc1570c78b1ea90526c0309cd74b7b8d0"
"Ref": "BooksApiDeployment86CA39AF4ff82f86c127f53c9de94d266b1906be"
},
"StageName": "prod"
}
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-apigateway/test/test.deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,16 @@ export = {
// the logical ID changed
const template = synthesize();
test.ok(!template.Resources.deployment33381975bba46c5132329b81e7befcbbba5a0e75, 'old resource id is not deleted');
test.ok(template.Resources.deployment33381975075f46a4503208d69fcffed2f263c48c,
`new resource deployment33381975075f46a4503208d69fcffed2f263c48c is not created, instead found ${Object.keys(template.Resources)}`);
test.ok(template.Resources.deployment333819758aa4cdb9d204502b959c4903f4d5d29f,
`new resource deployment333819758aa4cdb9d204502b959c4903f4d5d29f is not created, instead found ${Object.keys(template.Resources)}`);

// tokens supported, and are resolved upon synthesis
const value = 'hello hello';
deployment.addToLogicalId({ foo: Lazy.stringValue({ produce: () => value }) });

const template2 = synthesize();
test.ok(template2.Resources.deployment33381975b6d7672e4c9afd0b741e41d07739786b,
`resource deployment33381975b6d7672e4c9afd0b741e41d07739786b not found, instead found ${Object.keys(template2.Resources)}`);
test.ok(template2.Resources.deployment333819758d91bed959c6bd6268ba84f6d33e888e,
`resource deployment333819758d91bed959c6bd6268ba84f6d33e888e not found, instead found ${Object.keys(template2.Resources)}`);

test.done();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ nodeunitShim({
'MyFleetInstanceSecurityGroup774E8234': {
'Type': 'AWS::EC2::SecurityGroup',
'Properties': {
'GroupDescription': 'MyFleet/InstanceSecurityGroup',
'GroupDescription': 'TestStack/MyFleet/InstanceSecurityGroup',
'SecurityGroupEgress': [
{
'CidrIp': '0.0.0.0/0',
Expand All @@ -43,7 +43,7 @@ nodeunitShim({
'Tags': [
{
'Key': 'Name',
'Value': 'MyFleet',
'Value': 'TestStack/MyFleet',
},
],

Expand All @@ -68,7 +68,7 @@ nodeunitShim({
'Tags': [
{
'Key': 'Name',
'Value': 'MyFleet',
'Value': 'TestStack/MyFleet',
},
],
},
Expand Down Expand Up @@ -122,7 +122,7 @@ nodeunitShim({
{
'Key': 'Name',
'PropagateAtLaunch': true,
'Value': 'MyFleet',
'Value': 'TestStack/MyFleet',
},
],

Expand Down Expand Up @@ -520,7 +520,7 @@ nodeunitShim({
{
Key: 'Name',
PropagateAtLaunch: true,
Value: 'MyFleet',
Value: 'TestStack/MyFleet',
},
{
Key: 'notsuper',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ nodeunitShim({
{
Key: 'Name',
PropagateAtLaunch: true,
Value: 'ASG',
Value: 'Default/ASG',
},
],
VPCZoneIdentifier: [
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codecommit/test/test.codecommit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export = {
'all',
],
DestinationArn: 'arn:aws:sns:*:123456789012:my_topic',
Name: 'MyRepository/arn:aws:sns:*:123456789012:my_topic',
Name: 'Default/MyRepository/arn:aws:sns:*:123456789012:my_topic',
},
],
},
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ test('if an encryption key is included, encrypt/decrypt permissions are also add
],
'Version': '2012-10-17',
},
'Description': 'Customer-managed key auto-created for encrypting DynamoDB table at Table A',
'Description': 'Customer-managed key auto-created for encrypting DynamoDB table at Default/Table A',
'EnableKeyRotation': true,
},
'UpdateReplacePolicy': 'Retain',
Expand Down Expand Up @@ -1755,7 +1755,7 @@ describe('grants', () => {
const user = new iam.User(stack, 'user');

// WHEN
expect(() => table.grantTableListStreams(user)).toThrow(/DynamoDB Streams must be enabled on the table my-table/);
expect(() => table.grantTableListStreams(user)).toThrow(/DynamoDB Streams must be enabled on the table Default\/my-table/);
});

test('"grantTableListStreams" allows principal to list all streams for this table', () => {
Expand Down Expand Up @@ -1801,7 +1801,7 @@ describe('grants', () => {
const user = new iam.User(stack, 'user');

// WHEN
expect(() => table.grantStreamRead(user)).toThrow(/DynamoDB Streams must be enabled on the table my-table/);
expect(() => table.grantStreamRead(user)).toThrow(/DynamoDB Streams must be enabled on the table Default\/my-table/);
});

test('"grantStreamRead" allows principal to read and describe the table stream"', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-ec2/test/connections.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ nodeunitShim({

// THEN
expect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
GroupDescription: 'SecurityGroup1',
GroupDescription: 'Default/SecurityGroup1',
SecurityGroupIngress: [
{
Description: 'from 0.0.0.0/0:88',
Expand All @@ -91,7 +91,7 @@ nodeunitShim({
}));

expect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
GroupDescription: 'SecurityGroup2',
GroupDescription: 'Default/SecurityGroup2',
SecurityGroupIngress: [
{
Description: 'from 0.0.0.0/0:88',
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-ec2/test/userdata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ nodeunitShim({

test.equals(rendered, '<powershell>trap {\n' +
'$success=($PSItem.Exception.Message -eq "Success")\n' +
'cfn-signal --stack Stack --resource RESOURCE1989552F --region ${Token[AWS::Region.4]} --success ($success.ToString().ToLower())\n' +
'cfn-signal --stack Default --resource RESOURCE1989552F --region ${Token[AWS::Region.4]} --success ($success.ToString().ToLower())\n' +
'break\n' +
'}\n' +
'command1\n' +
Expand Down Expand Up @@ -157,7 +157,7 @@ nodeunitShim({
test.equals(rendered, '#!/bin/bash\n' +
'function exitTrap(){\n' +
'exitCode=$?\n' +
'/opt/aws/bin/cfn-signal --stack Stack --resource RESOURCE1989552F --region ${Token[AWS::Region.4]} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n' +
'/opt/aws/bin/cfn-signal --stack Default --resource RESOURCE1989552F --region ${Token[AWS::Region.4]} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n' +
'}\n' +
'trap exitTrap EXIT\n' +
'command1');
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ec2/test/vpc-endpoint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ nodeunitShim({
}));

expect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
GroupDescription: 'VpcNetwork/EcrDocker/SecurityGroup',
GroupDescription: 'Default/VpcNetwork/EcrDocker/SecurityGroup',
VpcId: {
Ref: 'VpcNetworkB258E83A',
},
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-ec2/test/vpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ nodeunitShim({
new Vpc(stack, 'TheVPC');
expect(stack).to(
haveResource('AWS::EC2::VPC',
hasTags( [ {Key: 'Name', Value: 'TheVPC'} ])),
hasTags( [ {Key: 'Name', Value: 'TestStack/TheVPC'} ])),
);
expect(stack).to(
haveResource('AWS::EC2::InternetGateway',
hasTags( [ {Key: 'Name', Value: 'TheVPC'} ])),
hasTags( [ {Key: 'Name', Value: 'TestStack/TheVPC'} ])),
);
test.done();
},
Expand Down Expand Up @@ -458,7 +458,7 @@ nodeunitShim({
for (let i = 1; i < 4; i++) {
expect(stack).to(haveResource('AWS::EC2::Subnet', hasTags([{
Key: 'Name',
Value: `VPC/egressSubnet${i}`,
Value: `TestStack/VPC/egressSubnet${i}`,
}, {
Key: 'aws-cdk:subnet-name',
Value: 'egress',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export = {
}));

expect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
GroupDescription: 'FargateService/SecurityGroup',
GroupDescription: 'Default/FargateService/SecurityGroup',
SecurityGroupEgress: [
{
CidrIp: '0.0.0.0/0',
Expand Down Expand Up @@ -897,7 +897,7 @@ export = {
protocol: ecs.Protocol.TCP,
})],
});
}, /Container 'FargateTaskDef\/MainContainer' has no mapping for port 8001 and protocol tcp. Did you call "container.addPortMappings\(\)"\?/);
}, /Container 'Default\/FargateTaskDef\/MainContainer' has no mapping for port 8001 and protocol tcp. Did you call "container.addPortMappings\(\)"\?/);

test.done();
},
Expand Down Expand Up @@ -932,7 +932,7 @@ export = {
containerPort: 8002,
})],
});
}, /Container 'FargateTaskDef\/MainContainer' has no mapping for port 8002 and protocol tcp. Did you call "container.addPortMappings\(\)"\?/);
}, /Container 'Default\/FargateTaskDef\/MainContainer' has no mapping for port 8002 and protocol tcp. Did you call "container.addPortMappings\(\)"\?/);

test.done();
},
Expand Down
Loading

0 comments on commit a9b059f

Please sign in to comment.