diff --git a/packages/@aws-cdk/core/lib/private/synthesis.ts b/packages/@aws-cdk/core/lib/private/synthesis.ts index ea6fbf7b05ffa..34aa158740242 100644 --- a/packages/@aws-cdk/core/lib/private/synthesis.ts +++ b/packages/@aws-cdk/core/lib/private/synthesis.ts @@ -58,6 +58,7 @@ function synthNestedAssemblies(root: IConstruct, options: StageSynthesisOptions) * twice for the same construct. */ function invokeAspects(root: IConstruct) { + let nestedAspectWarning = false; recurse(root, []); function recurse(construct: IConstruct, inheritedAspects: constructs.IAspect[]) { @@ -65,10 +66,17 @@ function invokeAspects(root: IConstruct) { const node: NodeWithAspectPrivatesHangingOut = construct.node._actualNode as any; const allAspectsHere = [...inheritedAspects ?? [], ...node._aspects]; - + const nodeAspectsCount = node._aspects.length; for (const aspect of allAspectsHere) { if (node.invokedAspects.includes(aspect)) { continue; } + aspect.visit(construct); + // if an aspect was added to the node while invoking another aspect it will not be invoked, emit a warning + // the `nestedAspectWarning` flag is used to prevent the warning from being emitted for every child + if (!nestedAspectWarning && nodeAspectsCount !== node._aspects.length) { + construct.node.addWarning('We detected an Aspect was added via another Aspect, and will not be applied'); + nestedAspectWarning = true; + } node.invokedAspects.push(aspect); } diff --git a/packages/@aws-cdk/core/test/test.aspect.ts b/packages/@aws-cdk/core/test/test.aspect.ts index 5d14e62017830..5b13b2a547b5b 100644 --- a/packages/@aws-cdk/core/test/test.aspect.ts +++ b/packages/@aws-cdk/core/test/test.aspect.ts @@ -1,3 +1,4 @@ +import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import { Test } from 'nodeunit'; import { App } from '../lib'; import { IAspect } from '../lib/aspect'; @@ -17,6 +18,13 @@ class VisitOnce implements IAspect { } } } + +class MyAspect implements IAspect { + public visit(node: IConstruct): void { + node.node.addMetadata('foo', 'bar'); + } +} + export = { 'Aspects are invoked only once'(test: Test) { const app = new App(); @@ -28,4 +36,42 @@ export = { test.deepEqual(root.visitCounter, 1); test.done(); }, + + 'Warn if an Aspect is added via another Aspect'(test: Test) { + const app = new App(); + const root = new MyConstruct(app, 'MyConstruct'); + const child = new MyConstruct(root, 'ChildConstruct'); + root.node.applyAspect({ + visit(construct: IConstruct) { + construct.node.applyAspect({ + visit(inner: IConstruct) { + inner.node.addMetadata('test', 'would-be-ignored'); + }, + }); + }, + }); + app.synth(); + test.deepEqual(root.node.metadata[0].type, cxschema.ArtifactMetadataEntryType.WARN); + test.deepEqual(root.node.metadata[0].data, 'We detected an Aspect was added via another Aspect, and will not be applied'); + // warning is not added to child construct + test.equal(child.node.metadata.length, 0); + test.done(); + }, + + 'Do not warn if an Aspect is added directly (not by another aspect)'(test: Test) { + const app = new App(); + const root = new MyConstruct(app, 'Construct'); + const child = new MyConstruct(root, 'ChildConstruct'); + root.node.applyAspect(new MyAspect()); + app.synth(); + test.deepEqual(root.node.metadata[0].type, 'foo'); + test.deepEqual(root.node.metadata[0].data, 'bar'); + test.deepEqual(root.node.metadata[0].type, 'foo'); + test.deepEqual(child.node.metadata[0].data, 'bar'); + // no warning is added + test.equal(root.node.metadata.length, 1); + test.equal(child.node.metadata.length, 1); + test.done(); + }, + };