Skip to content

Commit fd181c7

Browse files
authored
fix(core): policy validation trace incorrect for larger constructs (#26466)
Ran into an issue where the construct trace was incorrect in larger projects, specifically where there are constructs that contain multiple constructs. To get the construct trace tree we first construct a new tree that only contains the path to the terminal construct (the one with the trace). We drop all the other constructs in the tree that don't relate. For example, if we had a tree like: ``` ->App -->MyStage --->MyStack1 ---->MyConstruct ----->Resource --->MyStack2 ---->MyConstruct ----->Resource --->MyStack3 ---->MyConstruct ----->Resource ``` And we want to get a new tree for `/App/MyStage/MyStack2/MyConstruct/Resource` it should look like this: ``` ->App -->MyStage --->MyStack2 ---->MyConstruct ----->Resource ``` We weren't correctly generating the new tree correctly and would always end up with the tree being the first item in the list. I've updated one of the tests, and also tested this on a more complex application. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 87ec525 commit fd181c7

File tree

3 files changed

+53
-15
lines changed

3 files changed

+53
-15
lines changed

packages/aws-cdk-lib/core/lib/private/tree-metadata.ts

+28-13
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import { ISynthesisSession } from '../stack-synthesizers';
1010
import { IInspectable, TreeInspector } from '../tree';
1111

1212
const FILE_PATH = 'tree.json';
13-
13+
type Mutable<T> = {
14+
-readonly [P in keyof T]: Mutable<T[P]>;
15+
};
1416
/**
1517
* Construct that is automatically attached to the top-level `App`.
1618
* This generates, as part of synthesis, a file containing the construct tree and the metadata for each node in the tree.
@@ -109,21 +111,34 @@ export class TreeMetadata extends Construct {
109111
* tree that leads to a specific construct so drop any nodes not in that path
110112
*
111113
* @param node Node the current tree node
112-
* @param child Node the previous tree node and the current node's child node
113-
* @returns Node the new tree
114+
* @returns Node the root node of the new tree
114115
*/
115-
private renderTreeWithChildren(node: Node, child?: Node): Node {
116-
if (node.parent) {
117-
return this.renderTreeWithChildren(node.parent, node);
118-
} else if (child) {
119-
return {
120-
...node,
121-
children: {
122-
[child.id]: child,
123-
},
116+
private renderTreeWithChildren(node: Node): Node {
117+
/**
118+
* @param currentNode - The current node being evaluated
119+
* @param currentNodeChild - The previous node which should be the only child of the current node
120+
* @returns The node with all children removed except for the path to the current node
121+
*/
122+
function renderTreeWithSingleChild(currentNode: Mutable<Node>, currentNodeChild: Mutable<Node>) {
123+
currentNode.children = {
124+
[currentNodeChild.id]: currentNodeChild,
124125
};
126+
if (currentNode.parent) {
127+
currentNode.parent = renderTreeWithSingleChild(currentNode.parent, currentNode);
128+
}
129+
return currentNode;
125130
}
126-
return node;
131+
132+
const currentNode = node.parent ? renderTreeWithSingleChild(node.parent, node) : node;
133+
// now that we have the new tree we need to return the root node
134+
let root = currentNode;
135+
do {
136+
if (root.parent) {
137+
root = root.parent;
138+
}
139+
} while (root.parent);
140+
141+
return root;
127142
}
128143

129144
/**

packages/aws-cdk-lib/core/lib/validation/private/construct-tree.ts

+16-2
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,26 @@ export class ConstructTree {
147147
return node;
148148
}
149149

150+
/**
151+
* @param node - the root node of the tree
152+
* @returns the terminal node in the tree
153+
*/
154+
private lastChild(node: Node): Node {
155+
if (node.children) {
156+
return this.lastChild(this.getChild(node.children));
157+
}
158+
return node;
159+
}
160+
150161
/**
151162
* Get a ConstructTrace from the cache for a given construct
152163
*
153164
* Construct the stack trace of constructs. This will start with the
154165
* root of the tree and go down to the construct that has the violation
155166
*/
156167
public getTrace(node: Node, locations?: string[]): ConstructTrace | undefined {
157-
const trace = this._traceCache.get(node.path);
168+
const lastChild = this.lastChild(node);
169+
const trace = this._traceCache.get(lastChild.path);
158170
if (trace) {
159171
return trace;
160172
}
@@ -177,7 +189,9 @@ export class ConstructTree {
177189
libraryVersion: node.constructInfo?.version,
178190
location: thisLocation ?? "Run with '--debug' to include location info",
179191
};
180-
this._traceCache.set(constructTrace.path, constructTrace);
192+
// set the cache for the last child path. If the last child path is different then
193+
// we have a different tree and need to retrieve the trace again
194+
this._traceCache.set(lastChild.path, constructTrace);
181195
return constructTrace;
182196
}
183197

packages/aws-cdk-lib/core/test/validation/trace.test.ts

+9
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ class MyL2Resource extends Resource implements IMyL2Resource {
113113
public readonly constructPath: string;
114114
constructor(scope: Construct, id: string) {
115115
super(scope, id);
116+
new core.CfnResource(this, 'Resource1', {
117+
type: 'AWS::CDK::TestResource',
118+
properties: {
119+
testProp1: 'testValue',
120+
},
121+
});
116122
const resource = new core.CfnResource(this, 'Resource', {
117123
type: 'AWS::CDK::TestResource',
118124
properties: {
@@ -127,6 +133,7 @@ class MyConstruct extends Construct {
127133
public readonly constructPath: string;
128134
constructor(scope: Construct, id: string) {
129135
super(scope, id);
136+
new MyL2Resource(this, 'MyL2Resource1');
130137
const myResource = new MyL2Resource(this, 'MyL2Resource');
131138
this.constructPath = myResource.constructPath;
132139
}
@@ -136,6 +143,8 @@ class MyStack extends core.Stack {
136143
public readonly constructPath: string;
137144
constructor(scope: Construct, id: string, props?: core.StackProps) {
138145
super(scope, id, props);
146+
new MyConstruct(this, 'MyConstruct2');
147+
new MyConstruct(this, 'MyConstruct3');
139148
const myConstruct = new MyConstruct(this, 'MyConstruct');
140149
this.constructPath = myConstruct.constructPath;
141150
}

0 commit comments

Comments
 (0)