Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): overrideLogicalId: override IDs of CFN elements #1670

Merged
merged 22 commits into from
Feb 6, 2019
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions packages/@aws-cdk/assets/test/test.asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ export = {
// the correct information
const entry = asset.node.metadata.find(m => m.type === 'aws:cdk:asset');
test.ok(entry, 'found metadata entry');
test.deepEqual(entry!.data, {

// console.error(JSON.stringify(stack.node.resolve(entry!.data)));

test.deepEqual(stack.node.resolve(entry!.data), {
path: dirPath,
id: 'MyAsset',
packaging: 'zip',
Expand All @@ -34,13 +37,35 @@ export = {
test.done();
},

'verify that the app resolves tokens in metadata'(test: Test) {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'my-stack');
const dirPath = path.resolve(__dirname, 'sample-asset-directory');

new ZipDirectoryAsset(stack, 'MyAsset', {
path: dirPath
});

const synth = app.synthesizeStack(stack.name);

test.deepEqual(synth.metadata['/my-stack/MyAsset'][0].data, {
path: dirPath,
id: "mystackMyAssetD6B1B593",
packaging: "zip",
s3BucketParameter: "MyAssetS3Bucket68C9B344",
s3KeyParameter: "MyAssetS3VersionKey68E1A45D"
});

test.done();
},

'"file" assets'(test: Test) {
const stack = new cdk.Stack();
const filePath = path.join(__dirname, 'file-asset.txt');
const asset = new FileAsset(stack, 'MyAsset', { path: filePath });
const entry = asset.node.metadata.find(m => m.type === 'aws:cdk:asset');
test.ok(entry, 'found metadata entry');
test.deepEqual(entry!.data, {
test.deepEqual(stack.node.resolve(entry!.data), {
path: filePath,
packaging: 'file',
id: 'MyAsset',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,12 +526,12 @@
},
"DependsOn": [
"booksapiANYF4F0CDEB",
"booksapibooks97D84727",
"booksapibooksbookidDELETE214F4059",
"booksapibooksbookidGETCCE21986",
"booksapibooksbookid5264BCA2",
"booksapibooksGETA776447A",
"booksapibooksPOSTF6C6559D",
"booksapibooksbookid5264BCA2",
"booksapibooksbookidDELETE214F4059",
"booksapibooksbookidGETCCE21986"
"booksapibooks97D84727"
]
},
"booksapiDeploymentStageprod55D8E03E": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@
"Description": "Automatically created by the RestApi construct"
},
"DependsOn": [
"myapiv113487378",
"myapiv1appliances507FEFF4",
"myapiv1appliancesGET8FE872EC",
"myapiv1books1D4BE6C1",
"myapiv1appliances507FEFF4",
"myapiv1booksGETC6B996D0",
"myapiv1booksPOST53E2832E",
"myapiv1toysA55FCBC4",
"myapiv1books1D4BE6C1",
"myapiv113487378",
"myapiv1toysGET7348114D",
"myapiv1toysPOST55128058",
"myapiv1toysPUT59AFBBC2"
"myapiv1toysPUT59AFBBC2",
"myapiv1toysA55FCBC4"
],
"DeletionPolicy": "Retain"
},
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-apigateway/test/test.restapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -526,12 +526,12 @@ export = {
// THEN
expect(stack).to(haveResource('My::Resource', {
DependsOn: [
"myapiDeploymentB7EF8EB75c091a668064a3f3a1f6d68a3fb22cf9",
"myapi162F20B8",
"myapiAccountC3A4750C",
"myapiCloudWatchRoleEB425128",
"myapiDeploymentB7EF8EB75c091a668064a3f3a1f6d68a3fb22cf9",
"myapiDeploymentStageprod329F21FF",
"myapiGET9B7CD29E"
"myapiGET9B7CD29E",
"myapi162F20B8"
]
}, ResourcePart.CompleteDefinition));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export = {
// Lifecycle Hook has a dependency on the policy object
expect(stack).to(haveResource('AWS::AutoScaling::LifecycleHook', {
DependsOn: [
"ASGLifecycleHookTransitionRoleDefaultPolicy2E50C7DB",
"ASGLifecycleHookTransitionRole3AAA6BB7",
"ASGLifecycleHookTransitionRoleDefaultPolicy2E50C7DB"
]
}, ResourcePart.CompleteDefinition));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,8 @@
"Runtime": "nodejs8.10"
},
"DependsOn": [
"PreHookServiceRoleC724B9BA",
"PreHookServiceRoleDefaultPolicy65358F76"
"PreHookServiceRoleDefaultPolicy65358F76",
"PreHookServiceRoleC724B9BA"
]
},
"PostHookServiceRoleE8A6AAC2": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ export = {
MyResource: {
Type: "Test::Resource",
DependsOn: [
"LBListener49E825B4",
// 2nd dependency is there because of the structure of the construct tree.
// It does not harm.
"LBListenerGroupGroup79B304FF"
"LBListenerGroupGroup79B304FF",
"LBListener49E825B4",
]
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@
"Runtime": "nodejs8.10"
},
"DependsOn": [
"FServiceRole3AC82EE1",
"FServiceRoleDefaultPolicy17A19BFA"
"FServiceRoleDefaultPolicy17A19BFA",
"FServiceRole3AC82EE1"
]
},
"FDynamoDBEventSourcelambdaeventsourcedynamodbT7967476AE652DA48": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@
"Runtime": "nodejs8.10"
},
"DependsOn": [
"FServiceRole3AC82EE1",
"FServiceRoleDefaultPolicy17A19BFA"
"FServiceRoleDefaultPolicy17A19BFA",
"FServiceRole3AC82EE1"
]
},
"FKinesisEventSourcelambdaeventsourcekinesisQ645CE7DB2D6BCCF5": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@
"Runtime": "nodejs8.10"
},
"DependsOn": [
"FServiceRole3AC82EE1",
"FServiceRoleDefaultPolicy17A19BFA"
"FServiceRoleDefaultPolicy17A19BFA",
"FServiceRole3AC82EE1"
]
},
"FSqsEventSourcelambdaeventsourcesqsQ67DE9201754EC819": {
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-lambda/test/integ.lambda.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@
"Runtime": "nodejs6.10"
},
"DependsOn": [
"MyLambdaServiceRole4539ECB6",
"MyLambdaServiceRoleDefaultPolicy5BBC6F68"
"MyLambdaServiceRoleDefaultPolicy5BBC6F68",
"MyLambdaServiceRole4539ECB6"
]
},
"MyLambdaVersion16CDE3C40": {
Expand All @@ -96,4 +96,4 @@
}
}
}
}
}
22 changes: 11 additions & 11 deletions packages/@aws-cdk/aws-lambda/test/test.lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export = {
Handler: 'index.handler',
Role: { 'Fn::GetAtt': [ 'MyLambdaServiceRole4539ECB6', 'Arn' ] },
Runtime: 'nodejs6.10' },
DependsOn: [ 'MyLambdaServiceRole4539ECB6', 'MyLambdaServiceRoleDefaultPolicy5BBC6F68' ] } } } );
DependsOn: [ 'MyLambdaServiceRoleDefaultPolicy5BBC6F68', 'MyLambdaServiceRole4539ECB6' ] } } } );
test.done();

},
Expand Down Expand Up @@ -429,8 +429,8 @@ export = {
"FunctionName": "OneFunctionToRuleThemAll"
},
"DependsOn": [
"MyLambdaServiceRole4539ECB6",
"MyLambdaServiceRoleDefaultPolicy5BBC6F68"
"MyLambdaServiceRoleDefaultPolicy5BBC6F68",
"MyLambdaServiceRole4539ECB6"
]
}
}
Expand Down Expand Up @@ -539,8 +539,8 @@ export = {
}
},
"DependsOn": [
"MyLambdaServiceRole4539ECB6",
"MyLambdaServiceRoleDefaultPolicy5BBC6F68"
"MyLambdaServiceRoleDefaultPolicy5BBC6F68",
"MyLambdaServiceRole4539ECB6"
]
}
}
Expand Down Expand Up @@ -717,8 +717,8 @@ export = {
}
},
"DependsOn": [
"MyLambdaServiceRole4539ECB6",
"MyLambdaServiceRoleDefaultPolicy5BBC6F68"
"MyLambdaServiceRoleDefaultPolicy5BBC6F68",
"MyLambdaServiceRole4539ECB6"
]
}
}
Expand Down Expand Up @@ -827,8 +827,8 @@ export = {
}
},
"DependsOn": [
"MyLambdaServiceRole4539ECB6",
"MyLambdaServiceRoleDefaultPolicy5BBC6F68"
"MyLambdaServiceRoleDefaultPolicy5BBC6F68",
"MyLambdaServiceRole4539ECB6",
]
}
}
Expand Down Expand Up @@ -906,8 +906,8 @@ export = {
}
},
"DependsOn": [
"MyLambdaServiceRoleDefaultPolicy5BBC6F68",
"MyLambdaServiceRole4539ECB6",
"MyLambdaServiceRoleDefaultPolicy5BBC6F68"
]
}, ResourcePart.CompleteDefinition));

Expand Down Expand Up @@ -964,8 +964,8 @@ export = {
}
},
"DependsOn": [
"MyLambdaServiceRoleDefaultPolicy5BBC6F68",
"MyLambdaServiceRole4539ECB6",
"MyLambdaServiceRoleDefaultPolicy5BBC6F68"
]
}, ResourcePart.CompleteDefinition));

Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-sns/test/test.sns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,8 @@ export = {
test.deepEqual(dest1.type, s3n.BucketNotificationDestinationType.Topic);

const dep: cdk.Construct = dest1.dependencies![0] as any;
test.deepEqual((dep.node.children[0] as any).logicalId, 'MyTopicPolicy12A5EC17', 'verify topic policy is added as dependency');
test.deepEqual(stack.node.resolve((dep.node.children[0] as any).logicalId),
'MyTopicPolicy12A5EC17', 'verify topic policy is added as dependency');

// calling again on the same bucket yields is idempotent
const dest2 = topic.asBucketNotificationDestination(bucketArn, bucketId);
Expand Down
4 changes: 4 additions & 0 deletions packages/@aws-cdk/cdk/lib/cloudformation/cfn-tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ export class CfnReference extends Token {
if (typeof(value) === 'function') {
throw new Error('CfnReference can only hold CloudFormation intrinsics (not a function)');
}
// prepend scope path to display name
if (displayName && scope) {
displayName = `${scope.node.path}.${displayName}`;
}
super(value, displayName);
this.replacementTokens = new Map<Stack, Token>();
this.isReference = true;
Expand Down
26 changes: 17 additions & 9 deletions packages/@aws-cdk/cdk/lib/cloudformation/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class Resource extends Referenceable {
*
* Is filled during prepare().
*/
private readonly dependsOn = new Set<string>();
private readonly dependsOn = new Set<Resource>();

/**
* Creates a resource construct.
Expand Down Expand Up @@ -117,7 +117,7 @@ export class Resource extends Referenceable {
* @param attributeName The name of the attribute.
*/
public getAtt(attributeName: string) {
return new CfnReference({ 'Fn::GetAtt': [this.logicalId, attributeName] }, `${this.logicalId}.${attributeName}`, this);
return new CfnReference({ 'Fn::GetAtt': [this.logicalId, attributeName] }, attributeName, this);
}

/**
Expand Down Expand Up @@ -179,8 +179,12 @@ export class Resource extends Referenceable {
this.addPropertyOverride(propertyPath, undefined);
}

/**
* Indicates that this resource depends on another resource and cannot be provisioned
* unless the other resource has been successfully provisioned.
*/
public addDependsOn(resource: Resource) {
this.dependsOn.add(resource.logicalId);
this.dependsOn.add(resource);
}

/**
Expand All @@ -197,7 +201,7 @@ export class Resource extends Referenceable {
Type: this.resourceType,
Properties: ignoreEmpty(this, properties),
// Return a sorted set of dependencies to be consistent across tests
DependsOn: ignoreEmpty(this, sortedSet(this.dependsOn)),
DependsOn: ignoreEmpty(this, renderDependsOn(this.dependsOn)),
CreationPolicy: capitalizePropertyNames(this, this.options.creationPolicy),
UpdatePolicy: capitalizePropertyNames(this, this.options.updatePolicy),
UpdateReplacePolicy: capitalizePropertyNames(this, this.options.updateReplacePolicy),
Expand All @@ -217,6 +221,15 @@ export class Resource extends Referenceable {
// Re-throw
throw e;
}

// returns the set of logical ID (tokens) this resource depends on
// sorted by construct paths to ensure test determinism
function renderDependsOn(dependsOn: Set<Resource>) {
return Array
.from(dependsOn)
.sort((x, y) => x.node.path.localeCompare(y.node.path))
.map(r => r.logicalId);
}
}

protected renderProperties(properties: any): { [key: string]: any } {
Expand Down Expand Up @@ -305,8 +318,3 @@ export function deepMerge(target: any, source: any) {

return target;
}
function sortedSet<T>(xs: Set<T>): T[] {
const ret = Array.from(xs);
ret.sort();
return ret;
}
Loading