Skip to content

Commit

Permalink
chore: do not use "synthesize" and "prepare" in the cdk (#9410)
Browse files Browse the repository at this point in the history
In 2.x we plan to deprecate support for the `synthesize()` and `prepare()` hooks in `Construct`. See [RFC] for motivation.

This change does not remove support for these hooks, but it does remove any usage of these hooks from the AWS Construct Library.

- aws-apigateway: the calculated logical ID of Deployment resources is now done through a Lazy instead of in `prepare()`.
- aws-lambda: the calculated logical ID of Version resources is now done through a Lazy instead of in `prepare()`.
- core: `Stack.synthesize()` is now called `_synthesizeTemplate()` and is explicitly called from `app.synth()`.
- core: `TreeEtadata.synthesize()` is now called `_synthesizeTree()` and is explicitly called from `app.synth()`.

The logical IDs of Lambda Version and API Gateway Deployment resources will be different after this change due to a latent bug in the previous implementation. The `prepare()` methods are called _before_ resource dependencies are resolved, which means that the hash in the logical ID did not include dependencies. Now it includes dependencies and therefore these IDs have changed. Since both of these resources are stateless, this does not introduce risk to production systems. See more details [here].


Furthermore: all calls to `ConstructNode.prepare()` were converted to `app.synth()`.

Related: aws/aws-cdk-rfcs#192

[RFC]: https://github.com/aws/aws-cdk-rfcs/blob/master/text/0192-remove-constructs-compat.md
[here]: #9410 (comment)

BREAKING CHANGE: `lambda.Version` and `apigateway.Deployment` resources with auto-generated IDs will be replaced as we fixed a bug which ignored resource dependencies when generating these logical IDs.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Elad Ben-Israel committed Aug 10, 2020
1 parent 03f68f1 commit 4b081c7
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 15 deletions.
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
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ describe('with Lambda@Edge functions', () => {
{
EventType: 'origin-request',
LambdaFunctionARN: {
Ref: 'FunctionCurrentVersion4E2B22619c0305f954e58f25575548280c0a3629',
Ref: 'FunctionCurrentVersion4E2B2261477a5ae8059bbaa7813f752292c0f65e',
},
},
],
Expand Down Expand Up @@ -341,7 +341,7 @@ describe('with Lambda@Edge functions', () => {
{
EventType: 'viewer-request',
LambdaFunctionARN: {
Ref: 'FunctionCurrentVersion4E2B22619c0305f954e58f25575548280c0a3629',
Ref: 'FunctionCurrentVersion4E2B2261477a5ae8059bbaa7813f752292c0f65e',
},
},
],
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ec2/test/connections.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,9 @@ export class Function extends FunctionBase {
// 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: ctx => {
const originalLogicalId: string = ctx.resolve(cfn.logicalId);
cfn.overrideLogicalId(Lazy.stringValue({ produce: _ => {
const hash = calculateFunctionHash(this);
const logicalId = trimFromStart(originalLogicalId, 255 - 32);
return `${logicalId}${hash}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"MyLambdaServiceRole4539ECB6"
]
},
"MyLambdaCurrentVersionE7A382CC86b18af374d6e380aa07074d2490e2df": {
"MyLambdaCurrentVersionE7A382CC721de083c6b4b6360a9c534b79eb610e": {
"Type": "AWS::Lambda::Version",
"Properties": {
"FunctionName": {
Expand All @@ -103,7 +103,7 @@
},
"Qualifier": {
"Fn::GetAtt": [
"MyLambdaCurrentVersionE7A382CC86b18af374d6e380aa07074d2490e2df",
"MyLambdaCurrentVersionE7A382CC721de083c6b4b6360a9c534b79eb610e",
"Version"
]
},
Expand All @@ -118,7 +118,7 @@
},
"FunctionVersion": {
"Fn::GetAtt": [
"MyLambdaCurrentVersionE7A382CC86b18af374d6e380aa07074d2490e2df",
"MyLambdaCurrentVersionE7A382CC721de083c6b4b6360a9c534b79eb610e",
"Version"
]
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda/test/test.lambda-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export = {
},
'FunctionVersion': {
'Fn::GetAtt': [
'FnCurrentVersion17A89ABB19ed45993ff69fd011ae9fd4ab6e2005',
'FnCurrentVersion17A89ABBab5c765f3c55e4e61583b51b00a95742',
'Version',
],
},
Expand Down
19 changes: 19 additions & 0 deletions packages/@aws-cdk/core/test/test.stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,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 {
Expand Down

0 comments on commit 4b081c7

Please sign in to comment.