Skip to content

Commit 33040d8

Browse files
otaviomacedootaviomacedo
authored
feat: digest computation for refactor considers cross-stack references (#529)
Previously the digest computation was being done stack by stack. One of the implications of this is that cross-stack references were ignored. Specifically, `Fn::ImportValue` was treated as a regular value, rather than as a reference. Change that, so we consider all stacks at once, and treating cross-stack references in the same way as within-stack references are treated, for the purposes of computing the digests of all resources. While we're at it, the purely graph related code was extracted to its own class, `ResourceGraph`. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 3d833d3 commit 33040d8

File tree

5 files changed

+341
-133
lines changed

5 files changed

+341
-133
lines changed

‎packages/@aws-cdk/toolkit-lib/lib/api/refactoring/cloudformation.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
import type { TypedMapping } from '@aws-cdk/cloudformation-diff';
22
import type * as cxapi from '@aws-cdk/cx-api';
33

4+
export interface CloudFormationResource {
5+
Type: string;
6+
Properties?: any;
7+
Metadata?: Record<string, any>;
8+
}
9+
410
export interface CloudFormationTemplate {
511
Resources?: {
6-
[logicalId: string]: {
7-
Type: string;
8-
Properties?: any;
9-
Metadata?: Record<string, any>;
10-
};
12+
[logicalId: string]: CloudFormationResource;
1113
};
14+
Outputs?: Record<string, any>;
1215
}
1316

1417
export interface CloudFormationStack {

‎packages/@aws-cdk/toolkit-lib/lib/api/refactoring/digest.ts‎

Lines changed: 36 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as crypto from 'node:crypto';
22
import { loadResourceModel } from '@aws-cdk/cloudformation-diff/lib/diff/util';
3-
import type { CloudFormationTemplate } from './cloudformation';
3+
import type { CloudFormationResource, CloudFormationStack } from './cloudformation';
4+
import { ResourceGraph } from './graph';
45

56
/**
67
* Computes the digest for each resource in the template.
@@ -25,75 +26,28 @@ import type { CloudFormationTemplate } from './cloudformation';
2526
* CloudFormation template form a directed acyclic graph, this function is
2627
* well-defined.
2728
*/
28-
export function computeResourceDigests(template: CloudFormationTemplate): Record<string, string> {
29-
const resources = template.Resources || {};
30-
const graph: Record<string, Set<string>> = {};
31-
const reverseGraph: Record<string, Set<string>> = {};
32-
33-
// 1. Build adjacency lists
34-
for (const id of Object.keys(resources)) {
35-
graph[id] = new Set();
36-
reverseGraph[id] = new Set();
37-
}
38-
39-
// 2. Detect dependencies by searching for Ref/Fn::GetAtt
40-
const findDependencies = (value: any): string[] => {
41-
if (!value || typeof value !== 'object') return [];
42-
if (Array.isArray(value)) {
43-
return value.flatMap(findDependencies);
44-
}
45-
if ('Ref' in value) {
46-
return [value.Ref];
47-
}
48-
if ('Fn::GetAtt' in value) {
49-
const refTarget = Array.isArray(value['Fn::GetAtt']) ? value['Fn::GetAtt'][0] : value['Fn::GetAtt'].split('.')[0];
50-
return [refTarget];
51-
}
52-
const result = [];
53-
if ('DependsOn' in value) {
54-
if (Array.isArray(value.DependsOn)) {
55-
result.push(...value.DependsOn);
56-
} else {
57-
result.push(value.DependsOn);
58-
}
59-
}
60-
result.push(...Object.values(value).flatMap(findDependencies));
61-
return result;
62-
};
63-
64-
for (const [id, res] of Object.entries(resources)) {
65-
const deps = findDependencies(res || {});
66-
for (const dep of deps) {
67-
if (dep in resources && dep !== id) {
68-
graph[id].add(dep);
69-
reverseGraph[dep].add(id);
70-
}
71-
}
72-
}
73-
74-
// 3. Topological sort
75-
const outDegree = Object.keys(graph).reduce((acc, k) => {
76-
acc[k] = graph[k].size;
77-
return acc;
78-
}, {} as Record<string, number>);
79-
80-
const queue = Object.keys(outDegree).filter((k) => outDegree[k] === 0);
81-
const order: string[] = [];
82-
83-
while (queue.length > 0) {
84-
const node = queue.shift()!;
85-
order.push(node);
86-
for (const nxt of reverseGraph[node]) {
87-
outDegree[nxt]--;
88-
if (outDegree[nxt] === 0) {
89-
queue.push(nxt);
90-
}
91-
}
92-
}
93-
29+
export function computeResourceDigests(stacks: CloudFormationStack[]): Record<string, string> {
30+
const exports: { [p: string]: { stackName: string; value: any } } = Object.fromEntries(
31+
stacks.flatMap((s) =>
32+
Object.values(s.template.Outputs ?? {})
33+
.filter((o) => o.Export != null && typeof o.Export.Name === 'string')
34+
.map((o) => [o.Export.Name, { stackName: s.stackName, value: o.Value }] as [string, { stackName: string; value: any }]),
35+
),
36+
);
37+
38+
const resources = Object.fromEntries(
39+
stacks.flatMap((s) =>
40+
Object.entries(s.template.Resources ?? {}).map(
41+
([id, res]) => [`${s.stackName}.${id}`, res] as [string, CloudFormationResource],
42+
),
43+
),
44+
);
45+
46+
const graph = new ResourceGraph(stacks);
47+
const nodes = graph.sortedNodes;
9448
// 4. Compute digests in sorted order
9549
const result: Record<string, string> = {};
96-
for (const id of order) {
50+
for (const id of nodes) {
9751
const resource = resources[id];
9852
const resourceProperties = resource.Properties ?? {};
9953
const model = loadResourceModel(resource.Type);
@@ -112,8 +66,8 @@ export function computeResourceDigests(template: CloudFormationTemplate): Record
11266
} else {
11367
// The resource does not have a physical ID defined, so we need to
11468
// compute the digest based on its properties and dependencies.
115-
const depDigests = Array.from(graph[id]).map((d) => result[d]);
116-
const propertiesHash = hashObject(stripReferences(stripConstructPath(resource)));
69+
const depDigests = Array.from(graph.outNeighbors(id)).map((d) => result[d]);
70+
const propertiesHash = hashObject(stripReferences(stripConstructPath(resource), exports));
11771
toHash = resource.Type + propertiesHash + depDigests.join('');
11872
}
11973

@@ -153,10 +107,10 @@ export function hashObject(obj: any): string {
153107
* Removes sub-properties containing Ref or Fn::GetAtt to avoid hashing
154108
* references themselves but keeps the property structure.
155109
*/
156-
function stripReferences(value: any): any {
110+
function stripReferences(value: any, exports: { [p: string]: { stackName: string; value: any } }): any {
157111
if (!value || typeof value !== 'object') return value;
158112
if (Array.isArray(value)) {
159-
return value.map(stripReferences);
113+
return value.map(x => stripReferences(x, exports));
160114
}
161115
if ('Ref' in value) {
162116
return { __cloud_ref__: 'Ref' };
@@ -167,9 +121,18 @@ function stripReferences(value: any): any {
167121
if ('DependsOn' in value) {
168122
return { __cloud_ref__: 'DependsOn' };
169123
}
124+
if ('Fn::ImportValue' in value) {
125+
const v = exports[value['Fn::ImportValue']].value;
126+
// Treat Fn::ImportValue as if it were a reference with the same stack
127+
if ('Ref' in v) {
128+
return { __cloud_ref__: 'Ref' };
129+
} else if ('Fn::GetAtt' in v) {
130+
return { __cloud_ref__: 'Fn::GetAtt' };
131+
}
132+
}
170133
const result: any = {};
171134
for (const [k, v] of Object.entries(value)) {
172-
result[k] = stripReferences(v);
135+
result[k] = stripReferences(v, exports);
173136
}
174137
return result;
175138
}

‎packages/@aws-cdk/toolkit-lib/lib/api/refactoring/graph.ts‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
import type { CloudFormationResource, CloudFormationStack } from './cloudformation';
2+
import { ToolkitError } from '../../toolkit/toolkit-error';
3+
4+
/**
5+
* An immutable directed graph of resources from multiple CloudFormation stacks.
6+
*/
7+
export class ResourceGraph {
8+
private readonly edges: Record<string, Set<string>> = {};
9+
private readonly reverseEdges: Record<string, Set<string>> = {};
10+
11+
constructor(stacks: Omit<CloudFormationStack, 'environment'>[]) {
12+
const exports: { [p: string]: { stackName: string; value: any } } = Object.fromEntries(
13+
stacks.flatMap((s) =>
14+
Object.values(s.template.Outputs ?? {})
15+
.filter((o) => o.Export != null && typeof o.Export.Name === 'string')
16+
.map(
17+
(o) =>
18+
[o.Export.Name, { stackName: s.stackName, value: o.Value }] as [
19+
string,
20+
{
21+
stackName: string;
22+
value: any;
23+
},
24+
],
25+
),
26+
),
27+
);
28+
29+
const resources = Object.fromEntries(
30+
stacks.flatMap((s) =>
31+
Object.entries(s.template.Resources ?? {}).map(
32+
([id, res]) => [`${s.stackName}.${id}`, res] as [string, CloudFormationResource],
33+
),
34+
),
35+
);
36+
37+
// 1. Build adjacency lists
38+
for (const id of Object.keys(resources)) {
39+
this.edges[id] = new Set();
40+
this.reverseEdges[id] = new Set();
41+
}
42+
43+
// 2. Detect dependencies by searching for Ref/Fn::GetAtt
44+
const findDependencies = (stackName: string, value: any): string[] => {
45+
if (!value || typeof value !== 'object') return [];
46+
if (Array.isArray(value)) {
47+
return value.flatMap((res) => findDependencies(stackName, res));
48+
}
49+
if ('Ref' in value) {
50+
return [`${stackName}.${value.Ref}`];
51+
}
52+
if ('Fn::GetAtt' in value) {
53+
const refTarget = Array.isArray(value['Fn::GetAtt'])
54+
? value['Fn::GetAtt'][0]
55+
: value['Fn::GetAtt'].split('.')[0];
56+
return [`${stackName}.${refTarget}`];
57+
}
58+
if ('Fn::ImportValue' in value) {
59+
const exp = exports[value['Fn::ImportValue']];
60+
const v = exp.value;
61+
if ('Fn::GetAtt' in v) {
62+
const id = Array.isArray(v['Fn::GetAtt']) ? v['Fn::GetAtt'][0] : v['Fn::GetAtt'].split('.')[0];
63+
return [`${exp.stackName}.${id}`];
64+
}
65+
if ('Ref' in v) {
66+
return [`${exp.stackName}.${v.Ref}`];
67+
}
68+
return [`${exp.stackName}.${v}`];
69+
}
70+
const result: string[] = [];
71+
if ('DependsOn' in value) {
72+
if (Array.isArray(value.DependsOn)) {
73+
result.push(...value.DependsOn.map((r: string) => `${stackName}.${r}`));
74+
} else {
75+
result.push(`${stackName}.${value.DependsOn}`);
76+
}
77+
}
78+
result.push(...Object.values(value).flatMap((res) => findDependencies(stackName, res)));
79+
return result;
80+
};
81+
82+
for (const [id, res] of Object.entries(resources)) {
83+
const stackName = id.split('.')[0];
84+
const deps = findDependencies(stackName, res || {});
85+
for (const dep of deps) {
86+
if (dep in resources && dep !== id) {
87+
this.edges[id].add(dep);
88+
this.reverseEdges[dep].add(id);
89+
}
90+
}
91+
}
92+
}
93+
94+
/**
95+
* Returns the sorted nodes in topological order.
96+
*/
97+
get sortedNodes(): string[] {
98+
const result: string[] = [];
99+
const outDegree = Object.keys(this.edges).reduce((acc, k) => {
100+
acc[k] = this.edges[k].size;
101+
return acc;
102+
}, {} as Record<string, number>);
103+
104+
const queue = Object.keys(outDegree).filter((k) => outDegree[k] === 0);
105+
106+
while (queue.length > 0) {
107+
const node = queue.shift()!;
108+
result.push(node);
109+
for (const nxt of this.reverseEdges[node]) {
110+
outDegree[nxt]--;
111+
if (outDegree[nxt] === 0) {
112+
queue.push(nxt);
113+
}
114+
}
115+
}
116+
return result;
117+
}
118+
119+
public inNeighbors(node: string): string[] {
120+
if (!(node in this.edges)) {
121+
throw new ToolkitError(`Node ${node} not found in the graph`);
122+
}
123+
return Array.from(this.reverseEdges[node] || []);
124+
}
125+
126+
public outNeighbors(node: string): string[] {
127+
if (!(node in this.edges)) {
128+
throw new ToolkitError(`Node ${node} not found in the graph`);
129+
}
130+
return Array.from(this.edges[node] || []);
131+
}
132+
}

‎packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ export async function usePrescribedMappings(
143143
export function resourceMovements(before: CloudFormationStack[], after: CloudFormationStack[]): ResourceMovement[] {
144144
return Object.values(
145145
removeUnmovedResources(
146-
zip(groupByKey(before.flatMap(resourceDigests)), groupByKey(after.flatMap(resourceDigests))),
146+
zip(groupByKey(resourceDigests(before)), groupByKey(resourceDigests(after))),
147147
),
148148
);
149149
}
@@ -221,11 +221,18 @@ function zip(
221221
/**
222222
* Computes a list of pairs [digest, location] for each resource in the stack.
223223
*/
224-
function resourceDigests(stack: CloudFormationStack): [string, ResourceLocation][] {
225-
const digests = computeResourceDigests(stack.template);
224+
function resourceDigests(stacks: CloudFormationStack[]): [string, ResourceLocation][] {
225+
// index stacks by name
226+
const stacksByName = new Map<string, CloudFormationStack>();
227+
for (const stack of stacks) {
228+
stacksByName.set(stack.stackName, stack);
229+
}
226230

227-
return Object.entries(digests).map(([logicalId, digest]) => {
228-
const location: ResourceLocation = new ResourceLocation(stack, logicalId);
231+
const digests = computeResourceDigests(stacks);
232+
233+
return Object.entries(digests).map(([loc, digest]) => {
234+
const [stackName, logicalId] = loc.split('.');
235+
const location: ResourceLocation = new ResourceLocation(stacksByName.get(stackName)!, logicalId);
229236
return [digest, location];
230237
});
231238
}

0 commit comments

Comments
 (0)