diff --git a/packages/@aws-cdk/aws-apigateway/lib/deployment.ts b/packages/@aws-cdk/aws-apigateway/lib/deployment.ts index c72dba724f878..cf2f159025288 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/deployment.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/deployment.ts @@ -123,7 +123,7 @@ interface LatestDeploymentResourceProps { class LatestDeploymentResource extends CfnDeployment { private hashComponents = new Array(); - private originalLogicalId: string; + private api: IRestApi; constructor(scope: Construct, id: string, props: LatestDeploymentResourceProps) { @@ -133,7 +133,31 @@ class LatestDeploymentResource extends CfnDeployment { }); this.api = props.restApi; - this.originalLogicalId = Stack.of(this).getLogicalId(this); + + const originalLogicalId = Stack.of(this).getLogicalId(this); + + this.overrideLogicalId(Lazy.stringValue({ produce: ctx => { + const hash = [ ...this.hashComponents ]; + + if (this.api instanceof RestApi || this.api instanceof SpecRestApi) { // Ignore IRestApi that are imported + + // Add CfnRestApi to the logical id so a new deployment is triggered when any of its properties change. + const cfnRestApiCF = (this.api.node.defaultChild as any)._toCloudFormation(); + hash.push(ctx.resolve(cfnRestApiCF)); + } + + let lid = originalLogicalId; + + // if hash components were added to the deployment, we use them to calculate + // a logical ID for the deployment resource. + if (hash.length > 0) { + const md5 = crypto.createHash('md5'); + hash.map(x => ctx.resolve(x)).forEach(c => md5.update(JSON.stringify(c))); + lid += md5.digest('hex'); + } + + return lid; + }})); } /** @@ -149,28 +173,4 @@ class LatestDeploymentResource extends CfnDeployment { this.hashComponents.push(data); } - - /** - * Hooks into synthesis to calculate a logical ID that hashes all the components - * add via `addToLogicalId`. - */ - protected prepare() { - if (this.api instanceof RestApi || this.api instanceof SpecRestApi) { // Ignore IRestApi that are imported - - // Add CfnRestApi to the logical id so a new deployment is triggered when any of its properties change. - const cfnRestApiCF = (this.api.node.defaultChild as any)._toCloudFormation(); - this.addToLogicalId(Stack.of(this).resolve(cfnRestApiCF)); - } - - const stack = Stack.of(this); - - // if hash components were added to the deployment, we use them to calculate - // a logical ID for the deployment resource. - if (this.hashComponents.length > 0) { - const md5 = crypto.createHash('md5'); - this.hashComponents.map(x => stack.resolve(x)).forEach(c => md5.update(JSON.stringify(c))); - this.overrideLogicalId(this.originalLogicalId + md5.digest('hex')); - } - super.prepare(); - } } diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.multistack.expected.json b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.multistack.expected.json index 92ca8ad632bfc..5aa57732955bc 100644 --- a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.multistack.expected.json +++ b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.multistack.expected.json @@ -120,7 +120,7 @@ "BooksApi60AC975F" ] }, - "BooksApiDeployment86CA39AFc1570c78b1ea90526c0309cd74b7b8d0": { + "BooksApiDeployment86CA39AF4ff82f86c127f53c9de94d266b1906be": { "Type": "AWS::ApiGateway::Deployment", "Properties": { "RestApiId": { @@ -141,7 +141,7 @@ "Ref": "BooksApi60AC975F" }, "DeploymentId": { - "Ref": "BooksApiDeployment86CA39AFc1570c78b1ea90526c0309cd74b7b8d0" + "Ref": "BooksApiDeployment86CA39AF4ff82f86c127f53c9de94d266b1906be" }, "StageName": "prod" } diff --git a/packages/@aws-cdk/aws-apigateway/test/test.deployment.ts b/packages/@aws-cdk/aws-apigateway/test/test.deployment.ts index 47118594132a5..0f5ce4a37732d 100644 --- a/packages/@aws-cdk/aws-apigateway/test/test.deployment.ts +++ b/packages/@aws-cdk/aws-apigateway/test/test.deployment.ts @@ -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(); diff --git a/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts b/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts index e4f91e7bcd58d..29942711ff781 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts @@ -307,7 +307,7 @@ describe('with Lambda@Edge functions', () => { { EventType: 'origin-request', LambdaFunctionARN: { - Ref: 'FunctionCurrentVersion4E2B22619c0305f954e58f25575548280c0a3629', + Ref: 'FunctionCurrentVersion4E2B2261477a5ae8059bbaa7813f752292c0f65e', }, }, ], @@ -341,7 +341,7 @@ describe('with Lambda@Edge functions', () => { { EventType: 'viewer-request', LambdaFunctionARN: { - Ref: 'FunctionCurrentVersion4E2B22619c0305f954e58f25575548280c0a3629', + Ref: 'FunctionCurrentVersion4E2B2261477a5ae8059bbaa7813f752292c0f65e', }, }, ], diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts b/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts index 9a6b99351ee7b..043e42952e7c0 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts @@ -26,7 +26,7 @@ export = nodeunit.testCase({ actions: [action], }); - cdk.ConstructNode.prepare(stack.node); + app.synth(); _assertPermissionGranted(test, stack, pipelineRole.statements, 'iam:PassRole', action.deploymentRole.roleArn); diff --git a/packages/@aws-cdk/aws-ec2/test/connections.test.ts b/packages/@aws-cdk/aws-ec2/test/connections.test.ts index 9c30e5f104cda..591d543827084 100644 --- a/packages/@aws-cdk/aws-ec2/test/connections.test.ts +++ b/packages/@aws-cdk/aws-ec2/test/connections.test.ts @@ -1,5 +1,5 @@ import { expect, haveResource } from '@aws-cdk/assert'; -import { App, ConstructNode, Stack } from '@aws-cdk/core'; +import { App, Stack } from '@aws-cdk/core'; import { nodeunitShim, Test } from 'nodeunit-shim'; import { @@ -185,7 +185,7 @@ nodeunitShim({ sg2.connections.allowFrom(sg1, Port.tcp(100)); // THEN -- both rules are in Stack2 - ConstructNode.prepare(app.node); + app.synth(); expect(stack2).to(haveResource('AWS::EC2::SecurityGroupIngress', { GroupId: { 'Fn::GetAtt': [ 'SecurityGroupDD263621', 'GroupId' ] }, @@ -216,7 +216,7 @@ nodeunitShim({ sg2.connections.allowTo(sg1, Port.tcp(100)); // THEN -- both rules are in Stack2 - ConstructNode.prepare(app.node); + app.synth(); expect(stack2).to(haveResource('AWS::EC2::SecurityGroupIngress', { GroupId: { 'Fn::ImportValue': 'Stack1:ExportsOutputFnGetAttSecurityGroupDD263621GroupIdDF6F8B09' }, @@ -249,7 +249,7 @@ nodeunitShim({ sg2.connections.allowFrom(sg1b, Port.tcp(100)); // THEN -- both egress rules are in Stack2 - ConstructNode.prepare(app.node); + app.synth(); expect(stack2).to(haveResource('AWS::EC2::SecurityGroupEgress', { GroupId: { 'Fn::ImportValue': 'Stack1:ExportsOutputFnGetAttSecurityGroupAED40ADC5GroupId1D10C76A' }, diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts index d450e85ca913a..92b6d64774751 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function.ts @@ -332,6 +332,18 @@ export class Function extends FunctionBase { ...this.currentVersionOptions, }); + // override the version's logical ID with a lazy string which includes the + // hash of the function itself, so a new version resource is created when + // the function configuration changes. + const cfn = this._currentVersion.node.defaultChild as CfnResource; + const originalLogicalId = this.stack.resolve(cfn.logicalId) as string; + + cfn.overrideLogicalId(Lazy.stringValue({ produce: _ => { + const hash = calculateFunctionHash(this); + const logicalId = trimFromStart(originalLogicalId, 255 - 32); + return `${logicalId}${hash}`; + }})); + return this._currentVersion; } @@ -739,23 +751,6 @@ export class Function extends FunctionBase { return this._logGroup; } - protected prepare() { - super.prepare(); - - // if we have a current version resource, override it's logical id - // so that it includes the hash of the function code and it's configuration. - if (this._currentVersion) { - const stack = Stack.of(this); - const cfn = this._currentVersion.node.defaultChild as CfnResource; - const originalLogicalId: string = stack.resolve(cfn.logicalId); - - const hash = calculateFunctionHash(this); - - const logicalId = trimFromStart(originalLogicalId, 255 - 32); - cfn.overrideLogicalId(`${logicalId}${hash}`); - } - } - private renderEnvironment() { if (!this.environment || Object.keys(this.environment).length === 0) { return undefined; diff --git a/packages/@aws-cdk/aws-lambda/test/integ.current-version.expected.json b/packages/@aws-cdk/aws-lambda/test/integ.current-version.expected.json index 1877a683a1740..bf269c8f6d555 100644 --- a/packages/@aws-cdk/aws-lambda/test/integ.current-version.expected.json +++ b/packages/@aws-cdk/aws-lambda/test/integ.current-version.expected.json @@ -85,7 +85,7 @@ "MyLambdaServiceRole4539ECB6" ] }, - "MyLambdaCurrentVersionE7A382CC86b18af374d6e380aa07074d2490e2df": { + "MyLambdaCurrentVersionE7A382CC721de083c6b4b6360a9c534b79eb610e": { "Type": "AWS::Lambda::Version", "Properties": { "FunctionName": { @@ -103,7 +103,7 @@ }, "Qualifier": { "Fn::GetAtt": [ - "MyLambdaCurrentVersionE7A382CC86b18af374d6e380aa07074d2490e2df", + "MyLambdaCurrentVersionE7A382CC721de083c6b4b6360a9c534b79eb610e", "Version" ] }, @@ -118,7 +118,7 @@ }, "FunctionVersion": { "Fn::GetAtt": [ - "MyLambdaCurrentVersionE7A382CC86b18af374d6e380aa07074d2490e2df", + "MyLambdaCurrentVersionE7A382CC721de083c6b4b6360a9c534b79eb610e", "Version" ] }, diff --git a/packages/@aws-cdk/aws-lambda/test/test.lambda-version.ts b/packages/@aws-cdk/aws-lambda/test/test.lambda-version.ts index 69e0f62e9c668..b7f283164b715 100644 --- a/packages/@aws-cdk/aws-lambda/test/test.lambda-version.ts +++ b/packages/@aws-cdk/aws-lambda/test/test.lambda-version.ts @@ -134,7 +134,7 @@ export = { }, 'FunctionVersion': { 'Fn::GetAtt': [ - 'FnCurrentVersion17A89ABB19ed45993ff69fd011ae9fd4ab6e2005', + 'FnCurrentVersion17A89ABBab5c765f3c55e4e61583b51b00a95742', 'Version', ], }, diff --git a/packages/@aws-cdk/aws-s3-notifications/test/notifications.test.ts b/packages/@aws-cdk/aws-s3-notifications/test/notifications.test.ts index 7a5a3cf25b96d..cadf2609692f0 100644 --- a/packages/@aws-cdk/aws-s3-notifications/test/notifications.test.ts +++ b/packages/@aws-cdk/aws-s3-notifications/test/notifications.test.ts @@ -290,8 +290,6 @@ test('a notification destination can specify a set of dependencies that must be bucket.addObjectCreatedNotification(dest); - cdk.ConstructNode.prepare(stack.node); - expect(SynthUtils.synthesize(stack).template.Resources.BucketNotifications8F2E257D).toEqual({ Type: 'Custom::S3BucketNotifications', Properties: { diff --git a/packages/@aws-cdk/core/lib/private/synthesis.ts b/packages/@aws-cdk/core/lib/private/synthesis.ts index 34aa158740242..296ba171efde9 100644 --- a/packages/@aws-cdk/core/lib/private/synthesis.ts +++ b/packages/@aws-cdk/core/lib/private/synthesis.ts @@ -1,8 +1,10 @@ import * as cxapi from '@aws-cdk/cx-api'; import * as constructs from 'constructs'; import { Construct, IConstruct, SynthesisOptions, ValidationError } from '../construct-compat'; +import { Stack } from '../stack'; import { Stage, StageSynthesisOptions } from '../stage'; import { prepareApp } from './prepare-app'; +import { TreeMetadata } from './tree-metadata'; export function synthesize(root: IConstruct, options: SynthesisOptions = { }): cxapi.CloudAssembly { // we start by calling "synth" on all nested assemblies (which will take care of all their children) @@ -103,10 +105,22 @@ function prepareTree(root: IConstruct) { * Stop at Assembly boundaries. */ function synthesizeTree(root: IConstruct, builder: cxapi.CloudAssemblyBuilder) { - visit(root, 'post', construct => construct.onSynthesize({ - outdir: builder.outdir, - assembly: builder, - })); + visit(root, 'post', construct => { + const session = { + outdir: builder.outdir, + assembly: builder, + }; + + if (construct instanceof Stack) { + construct._synthesizeTemplate(session); + } else if (construct instanceof TreeMetadata) { + construct._synthesizeTree(session); + } + + // this will soon be deprecated and removed in 2.x + // see https://github.com/aws/aws-cdk-rfcs/issues/192 + construct.onSynthesize(session); + }); } /** diff --git a/packages/@aws-cdk/core/lib/private/tree-metadata.ts b/packages/@aws-cdk/core/lib/private/tree-metadata.ts index a40481faa9b2d..b2f288d97b4b4 100644 --- a/packages/@aws-cdk/core/lib/private/tree-metadata.ts +++ b/packages/@aws-cdk/core/lib/private/tree-metadata.ts @@ -20,7 +20,11 @@ export class TreeMetadata extends Construct { super(scope, 'Tree'); } - protected synthesize(session: ISynthesisSession) { + /** + * Create tree.json + * @internal + */ + public _synthesizeTree(session: ISynthesisSession) { const lookup: { [path: string]: Node } = { }; const visit = (construct: IConstruct): Node => { diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index c7c20e8bb0c73..3f79c43aec18e 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -700,6 +700,32 @@ export class Stack extends Construct implements ITaggable { } } + /** + * Synthesizes the cloudformation template into a cloud assembly. + * @internal + */ + public _synthesizeTemplate(session: ISynthesisSession): void { + // In principle, stack synthesis is delegated to the + // StackSynthesis object. + // + // However, some parts of synthesis currently use some private + // methods on Stack, and I don't really see the value in refactoring + // this right now, so some parts still happen here. + const builder = session.assembly; + + // write the CloudFormation template as a JSON file + const outPath = path.join(builder.outdir, this.templateFile); + const text = JSON.stringify(this._toCloudFormation(), undefined, 2); + fs.writeFileSync(outPath, text); + + for (const ctx of this._missingContext) { + builder.addMissing(ctx); + } + + // Delegate adding artifacts to the Synthesizer + this.synthesizer.synthesizeStackArtifacts(session); + } + /** * Returns the naming scheme used to allocate logical IDs. By default, uses * the `HashedAddressingScheme` but this method can be overridden to customize @@ -761,28 +787,6 @@ export class Stack extends Construct implements ITaggable { } } - protected synthesize(session: ISynthesisSession): void { - // In principle, stack synthesis is delegated to the - // StackSynthesis object. - // - // However, some parts of synthesis currently use some private - // methods on Stack, and I don't really see the value in refactoring - // this right now, so some parts still happen here. - const builder = session.assembly; - - // write the CloudFormation template as a JSON file - const outPath = path.join(builder.outdir, this.templateFile); - const text = JSON.stringify(this._toCloudFormation(), undefined, 2); - fs.writeFileSync(outPath, text); - - for (const ctx of this._missingContext) { - builder.addMissing(ctx); - } - - // Delegate adding artifacts to the Synthesizer - this.synthesizer.synthesizeStackArtifacts(session); - } - /** * Returns the CloudFormation template for this stack by traversing * the tree and invoking _toCloudFormation() on all Entity objects. diff --git a/packages/@aws-cdk/core/test/test.stack.ts b/packages/@aws-cdk/core/test/test.stack.ts index 7ae1adc077798..c6a3be31af013 100644 --- a/packages/@aws-cdk/core/test/test.stack.ts +++ b/packages/@aws-cdk/core/test/test.stack.ts @@ -2,7 +2,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import { Test } from 'nodeunit'; import { App, CfnCondition, CfnInclude, CfnOutput, CfnParameter, - CfnResource, Construct, Lazy, ScopedAws, Stack, Tag, validateString } from '../lib'; + CfnResource, Construct, Lazy, ScopedAws, Stack, Tag, validateString, ISynthesisSession } from '../lib'; import { Intrinsic } from '../lib/private/intrinsic'; import { resolveReferences } from '../lib/private/refs'; import { PostResolveToken } from '../lib/util'; @@ -868,6 +868,25 @@ export = { test.done(); }, + + 'users can (still) override "synthesize()" in stack'(test: Test) { + let called = false; + + class MyStack extends Stack { + synthesize(session: ISynthesisSession) { + called = true; + test.ok(session.outdir); + test.equal(session.assembly.outdir, session.outdir); + } + } + + const app = new App(); + new MyStack(app, 'my-stack'); + + app.synth(); + test.ok(called, 'synthesize() not called for Stack'); + test.done(); + }, }; class StackWithPostProcessor extends Stack {