Description
Describe the bug
Aspects can modify the constructs' tree dynamically. The tree is modified when the Aspect is being visited.
If the modification happens in a higher node in the tree, then the aspects in this new branch are not processed, since the Aspects' processing recursion has already passed that level.
This is the case with CDK Pipelines, the "Source" stage is added in a higher location in the tree, causing tags and other aspects to not be added/processed for some elements of the dynamically generated tree.
In theory this can happen with other constructs that use a similar technique to dynamically extend the constructs' tree.
Expected Behavior
// Constructs' tree
Stack (with Tag aspect)
+- Bucket
+- Construct (with dynamic extension Aspect, adds Bucket to Stack)
After synthesis the tree should look like
// Constructs' tree
Stack (tagged)
+- Bucket (tagged)
+- Construct (tagged)
+- Bucket (tagged)
Current Behavior
// Constructs' tree
Stack (with Tag aspect)
+- Bucket
+- Construct (with dynamic extension Aspect, adds Bucket to Stack)
After synthesis the tree looks like
// Constructs' tree
Stack (tagged)
+- Bucket (tagged)
+- Construct (tagged)
+- Bucket (NOT TAGGED) <<<====
Reproduction Steps
Init sample app with:
mkdir /tmp/cdk-issue
cd /tmp/cdk-issue
cdk init sample-app --language typescript
Replace content of /tmp/cdk-issue/lib/cdk-issue-stack.ts
with the following:
import { Aspects, Tags, IAspect, Stack, StackProps } from 'aws-cdk-lib';
import { Bucket } from 'aws-cdk-lib/aws-s3';
import { Construct, IConstruct } from 'constructs';
export class CdkIssueStack extends Stack {
constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);
Tags.of(this).add('test-tag', 'test-value');
new MyConstruct(this, 'myConstruct');
}
}
class MyConstruct extends Construct {
constructor(scope: IConstruct, id: string) {
super(scope, id);
const stack = Stack.of(scope);
const s3Bucket = new Bucket(stack, 'bucket-with-tags');
Aspects.of(this).add(new MyDynAspect());
}
}
class MyDynAspect implements IAspect {
private processed = false;
public visit(node: IConstruct): void {
if (!this.processed) {
//IMPORTANT: The bucket is added to the parent of the construct where the aspect it was initialized
const stack = Stack.of(node);
const s3Bucket = new Bucket(stack, 'bucket-without-tags-that-should-have');
this.processed = true;
}
}
}
npm run build
cdk synth
===> Tags should be present on both buckets, but tags are set only on fist bucket.
Possible Solution
In aws-cdk-lib/core/lib/private/synthesis.ts
Enhance function invokeAspects
as follows:
invokeAspects = function (root: IConstruct): void {
const invokedByPath: { [nodePath: string]: IAspect[] } = {};
let nestedAspectWarning = false;
// CHANGE: repeat full recursion until no more invocations happen
let noOfInvocationsInLastFullRecursion;
do {
noOfInvocationsInLastFullRecursion = recurse(root, []);
} while(noOfInvocationsInLastFullRecursion > 0);
function recurse(construct: IConstruct, inheritedAspects: IAspect[]): number {
let noOfInvocations = 0;
const node = construct.node;
const aspects = Aspects.of(construct);
const allAspectsHere = [...inheritedAspects ?? [], ...aspects.all];
const nodeAspectsCount = aspects.all.length;
for (const aspect of allAspectsHere) {
let invoked = invokedByPath[node.path];
if (!invoked) {
invoked = invokedByPath[node.path] = [];
}
if (invoked.includes(aspect)) { continue; }
aspect.visit(construct);
noOfInvocations++;
// 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 !== aspects.all.length) {
Annotations.of(construct).addWarning('We detected an Aspect was added via another Aspect, and will not be applied');
nestedAspectWarning = true;
}
// mark as invoked for this node
invoked.push(aspect);
}
for (const child of construct.node.children) {
if (!Stage.isStage(child)) {
noOfInvocations += recurse(child, allAspectsHere);
}
}
return noOfInvocations;
}
}
The above addition will repeat the recursion on the full tree as long as any new invocation happens.
Additional Information/Context
This affects CDK Pipelines, tagging (and other Aspects) are not propagated to all stages, which is a key blocker within our company.
CDK CLI Version
2.31.2
Framework Version
No response
Node.js Version
v14.19.1
OS
All
Language
Typescript, Python, .NET, Java, Go
Language Version
all
Other information
No response