Skip to content

Commit a261c9d

Browse files
fix(assertions): throw error or warn when synth is called multiple times on mutated construct tree (#31865)
Closes #24689 ### Reason for this change Calling `Template.fromStack(stack)` twice on the same stack object will throw the error `Unable to find artifact with id` if the stack is mutated after the first `Template.fromStack(stack)` call. This is because synth should only be called once - from the comment on the `synth` function: > _Once an assembly has been synthesized, it cannot be modified. Subsequent calls will return the same assembly._ Second call of `Template.fromStack(stack)` tries to find the mutated stack since `app.synth()` caches and returns the assembly from the first synth call. ### Description of changes This PR checks to see whether or not the construct tree has been each time `synth()` is called - if it has, then we either throw an error or a warning. We will only throw a warning if synth is being called from `process.once('beforeExit')`. ### Description of how you validated changes Unit test with the customer's same example case, asserting than an error is thrown when synth is called twice after a stack has been mutated after the first synth call. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 46e51f5 commit a261c9d

File tree

4 files changed

+107
-23
lines changed

4 files changed

+107
-23
lines changed

packages/aws-cdk-lib/core/lib/app.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ export class App extends Stage {
192192
if (autoSynth) {
193193
// synth() guarantees it will only execute once, so a default of 'true'
194194
// doesn't bite manual calling of the function.
195-
process.once('beforeExit', () => this.synth());
195+
process.once('beforeExit', () => this.synth({ errorOnDuplicateSynth: false }));
196196
}
197197

198198
this._treeMetadata = props.treeMetadata ?? true;

packages/aws-cdk-lib/core/lib/stage.ts

+60-1
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,11 @@ export class Stage extends Construct {
146146
*/
147147
private assembly?: cxapi.CloudAssembly;
148148

149+
/**
150+
* The cached set of construct paths. Empty if assembly was not yet built.
151+
*/
152+
private constructPathsCache: Set<string>;
153+
149154
/**
150155
* Validation plugins to run during synthesis. If any plugin reports any violation,
151156
* synthesis will be interrupted and the report displayed to the user.
@@ -163,6 +168,7 @@ export class Stage extends Construct {
163168

164169
Object.defineProperty(this, STAGE_SYMBOL, { value: true });
165170

171+
this.constructPathsCache = new Set<string>();
166172
this.parentStage = Stage.of(this);
167173

168174
this.region = props.env?.region ?? this.parentStage?.region;
@@ -210,16 +216,62 @@ export class Stage extends Construct {
210216
* calls will return the same assembly.
211217
*/
212218
public synth(options: StageSynthesisOptions = { }): cxapi.CloudAssembly {
213-
if (!this.assembly || options.force) {
219+
220+
let newConstructPaths = this.listAllConstructPaths(this);
221+
222+
// If the assembly cache is uninitiazed, run synthesize and reset construct paths cache
223+
if (this.constructPathsCache.size == 0 || !this.assembly || options.force) {
214224
this.assembly = synthesize(this, {
215225
skipValidation: options.skipValidation,
216226
validateOnSynthesis: options.validateOnSynthesis,
217227
});
228+
newConstructPaths = this.listAllConstructPaths(this);
229+
this.constructPathsCache = newConstructPaths;
218230
}
219231

232+
// If the construct paths set has changed
233+
if (!this.constructPathSetsAreEqual(this.constructPathsCache, newConstructPaths)) {
234+
const errorMessage = 'Synthesis has been called multiple times and the construct tree was modified after the first synthesis.';
235+
if (options.errorOnDuplicateSynth ?? true) {
236+
throw new Error(errorMessage + ' This is not allowed. Remove multple synth() calls and do not modify the construct tree after the first synth().');
237+
} else {
238+
// eslint-disable-next-line no-console
239+
console.error(errorMessage + ' Only the results of the first synth() call are used, and modifications done after it are ignored. Avoid construct tree mutations after synth() has been called unless this is intentional.');
240+
}
241+
}
242+
243+
// Reset construct paths cache
244+
this.constructPathsCache = newConstructPaths;
245+
220246
return this.assembly;
221247
}
222248

249+
// Function that lists all construct paths and returns them as a set
250+
private listAllConstructPaths(construct: IConstruct): Set<string> {
251+
const paths = new Set<string>();
252+
function recurse(root: IConstruct) {
253+
paths.add(root.node.path);
254+
for (const child of root.node.children) {
255+
if (!Stage.isStage(child)) {
256+
recurse(child);
257+
}
258+
}
259+
}
260+
recurse(construct);
261+
return paths;
262+
}
263+
264+
// Checks if sets of construct paths are equal
265+
private constructPathSetsAreEqual(set1: Set<string>, set2: Set<string>): boolean {
266+
if (set1.size !== set2.size) return false;
267+
for (const id of set1) {
268+
if (!set2.has(id)) {
269+
return false;
270+
}
271+
}
272+
return true;
273+
}
274+
223275
private createBuilder(outdir?: string) {
224276
// cannot specify "outdir" if we are a nested stage
225277
if (this.parentStage && outdir) {
@@ -259,4 +311,11 @@ export interface StageSynthesisOptions {
259311
* @default false
260312
*/
261313
readonly force?: boolean;
314+
315+
/**
316+
* Whether or not to throw a warning instead of an error if the construct tree has
317+
* been mutated since the last synth.
318+
* @default true
319+
*/
320+
readonly errorOnDuplicateSynth?: boolean;
262321
}

packages/aws-cdk-lib/core/test/synthesis.test.ts

+25
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as os from 'os';
33
import * as path from 'path';
44
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
55
import { Construct } from 'constructs';
6+
import { Template } from '../../assertions';
67
import * as cxschema from '../../cloud-assembly-schema';
78
import * as cxapi from '../../cx-api';
89
import * as cdk from '../lib';
@@ -362,6 +363,30 @@ describe('synthesis', () => {
362363

363364
});
364365

366+
test('calling synth multiple times errors if construct tree is mutated', () => {
367+
const app = new cdk.App();
368+
369+
const stages = [
370+
{
371+
stage: 'PROD',
372+
},
373+
{
374+
stage: 'BETA',
375+
},
376+
];
377+
378+
// THEN - no error the first time synth is called
379+
let stack = new cdk.Stack(app, `${stages[0].stage}-Stack`, {});
380+
expect(() => {
381+
Template.fromStack(stack);
382+
}).not.toThrow();
383+
384+
// THEN - error is thrown since synth was called with mutated stack name
385+
stack = new cdk.Stack(app, `${stages[1].stage}-Stack`, {});
386+
expect(() => {
387+
Template.fromStack(stack);
388+
}).toThrow('Synthesis has been called multiple times and the construct tree was modified after the first synthesis');
389+
});
365390
});
366391

367392
function list(outdir: string) {

packages/aws-cdk-lib/pipelines/test/compliance/synths.test.ts

+21-21
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,27 @@ test('CodeBuild: environment variables specified in multiple places are correctl
186186
}),
187187
});
188188

189+
new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk-2', {
190+
synth: new cdkp.CodeBuildStep('Synth', {
191+
input: cdkp.CodePipelineSource.gitHub('test/test', 'main'),
192+
primaryOutputDirectory: '.',
193+
env: {
194+
SOME_ENV_VAR: 'SomeValue',
195+
},
196+
installCommands: [
197+
'install1',
198+
'install2',
199+
],
200+
commands: ['synth'],
201+
buildEnvironment: {
202+
environmentVariables: {
203+
INNER_VAR: { value: 'InnerValue' },
204+
},
205+
privileged: true,
206+
},
207+
}),
208+
});
209+
189210
// THEN
190211
Template.fromStack(pipelineStack).hasResourceProperties('AWS::CodeBuild::Project', {
191212
Environment: Match.objectLike({
@@ -217,27 +238,6 @@ test('CodeBuild: environment variables specified in multiple places are correctl
217238
},
218239
});
219240

220-
new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk-2', {
221-
synth: new cdkp.CodeBuildStep('Synth', {
222-
input: cdkp.CodePipelineSource.gitHub('test/test', 'main'),
223-
primaryOutputDirectory: '.',
224-
env: {
225-
SOME_ENV_VAR: 'SomeValue',
226-
},
227-
installCommands: [
228-
'install1',
229-
'install2',
230-
],
231-
commands: ['synth'],
232-
buildEnvironment: {
233-
environmentVariables: {
234-
INNER_VAR: { value: 'InnerValue' },
235-
},
236-
privileged: true,
237-
},
238-
}),
239-
});
240-
241241
// THEN
242242
Template.fromStack(pipelineStack).hasResourceProperties('AWS::CodeBuild::Project', {
243243
Environment: Match.objectLike({

0 commit comments

Comments
 (0)