Skip to content

Commit a83ac5f

Browse files
rix0rrrrix0rrr
authoredOct 25, 2018
fix(cloudformation-diff): track replacements (#1003)
Track second-order effects of resource replacements through references. If a resource gets replaced and it's referenced by some other resource in an immutable field, that field gets replaced as well, and so on. ALSO IN THIS COMMIT - Configure lerna to not update package-lock.json on every bootstrap. Fixes #1001
1 parent 8c90350 commit a83ac5f

File tree

3 files changed

+168
-18
lines changed

3 files changed

+168
-18
lines changed
 

Diff for: ‎lerna.json

+5
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
"packages/@aws-cdk/*",
77
"tools/*"
88
],
9+
"command": {
10+
"bootstrap": {
11+
"npmClientArgs": ["--no-package-lock"]
12+
}
13+
},
914
"rejectCycles": "true",
1015
"version": "0.13.0"
1116
}

Diff for: ‎packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts

+97-11
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,40 @@ const DIFF_HANDLERS: HandlerRegistry = {
3939
* the template +newTemplate+.
4040
*/
4141
export function diffTemplate(currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }): types.TemplateDiff {
42-
const differences: types.ITemplateDiff = {};
43-
const unknown: { [key: string]: types.Difference<any> } = {};
44-
for (const key of unionOf(Object.keys(currentTemplate), Object.keys(newTemplate)).sort()) {
45-
const oldValue = currentTemplate[key];
46-
const newValue = newTemplate[key];
47-
if (deepEqual(oldValue, newValue)) { continue; }
48-
const handler: DiffHandler = DIFF_HANDLERS[key]
49-
|| ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV));
50-
handler(differences, oldValue, newValue);
42+
// We're going to modify this in-place
43+
newTemplate = deepCopy(newTemplate);
44+
45+
while (true) {
46+
const differences: types.ITemplateDiff = {};
47+
const unknown: { [key: string]: types.Difference<any> } = {};
48+
for (const key of unionOf(Object.keys(currentTemplate), Object.keys(newTemplate)).sort()) {
49+
const oldValue = currentTemplate[key];
50+
const newValue = newTemplate[key];
51+
if (deepEqual(oldValue, newValue)) { continue; }
52+
const handler: DiffHandler = DIFF_HANDLERS[key]
53+
|| ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV));
54+
handler(differences, oldValue, newValue);
55+
56+
}
57+
if (Object.keys(unknown).length > 0) { differences.unknown = new types.DifferenceCollection(unknown); }
58+
59+
// Propagate replacements for replaced resources
60+
let didPropagateReferenceChanges = false;
61+
if (differences.resources) {
62+
differences.resources.forEach((logicalId, change) => {
63+
if (change.changeImpact === types.ResourceImpact.WILL_REPLACE) {
64+
if (propagateReplacedReferences(newTemplate, logicalId)) {
65+
didPropagateReferenceChanges = true;
66+
}
67+
}
68+
});
69+
}
70+
71+
// We're done only if we didn't have to propagate any more replacements.
72+
if (!didPropagateReferenceChanges) {
73+
return new types.TemplateDiff(differences);
74+
}
5175
}
52-
if (Object.keys(unknown).length > 0) { differences.unknown = new types.DifferenceCollection(unknown); }
53-
return new types.TemplateDiff(differences);
5476
}
5577

5678
/**
@@ -59,3 +81,67 @@ export function diffTemplate(currentTemplate: { [key: string]: any }, newTemplat
5981
export function diffResource(oldValue: types.Resource, newValue: types.Resource): types.ResourceDifference {
6082
return impl.diffResource(oldValue, newValue);
6183
}
84+
85+
/**
86+
* Replace all references to the given logicalID on the given template, in-place
87+
*
88+
* Returns true iff any references were replaced.
89+
*/
90+
function propagateReplacedReferences(template: object, logicalId: string): boolean {
91+
let ret = false;
92+
93+
function recurse(obj: any) {
94+
if (Array.isArray(obj)) {
95+
obj.forEach(recurse);
96+
}
97+
98+
if (typeof obj === 'object' && obj !== null) {
99+
if (!replaceReference(obj)) {
100+
Object.values(obj).forEach(recurse);
101+
}
102+
}
103+
}
104+
105+
function replaceReference(obj: any) {
106+
const keys = Object.keys(obj);
107+
if (keys.length !== 1) { return false; }
108+
const key = keys[0];
109+
110+
if (key === 'Ref') {
111+
if (obj.Ref === logicalId) {
112+
obj.Ref = logicalId + '(replaced)';
113+
ret = true;
114+
}
115+
return true;
116+
}
117+
118+
if (key.startsWith('Fn::')) {
119+
if (Array.isArray(obj[key]) && obj[key].length > 0 && obj[key][0] === logicalId) {
120+
obj[key][0] = logicalId + '(replaced)';
121+
ret = true;
122+
}
123+
return true;
124+
}
125+
126+
return false;
127+
}
128+
129+
recurse(template);
130+
return ret;
131+
}
132+
133+
function deepCopy(x: any): any {
134+
if (Array.isArray(x)) {
135+
return x.map(deepCopy);
136+
}
137+
138+
if (typeof x === 'object' && x !== null) {
139+
const ret: any = {};
140+
for (const key of Object.keys(x)) {
141+
ret[key] = deepCopy(x[key]);
142+
}
143+
return ret;
144+
}
145+
146+
return x;
147+
}

Diff for: ‎packages/@aws-cdk/cloudformation-diff/test/test.diff-template.ts

+66-7
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,9 @@ exports.diffTemplate = {
109109
const bucketName = 'ShineyBucketName';
110110
const currentTemplate = {
111111
Resources: {
112-
BucketPolicyResource: BUCKET_POLICY_RESOURCE,
112+
QueueResource: {
113+
Type: 'AWS::SQS::Queue'
114+
},
113115
BucketResource: {
114116
Type: 'AWS::S3::Bucket',
115117
Properties: {
@@ -122,7 +124,9 @@ exports.diffTemplate = {
122124
const newBucketName = `${bucketName}-v2`;
123125
const newTemplate = {
124126
Resources: {
125-
BucketPolicyResource: BUCKET_POLICY_RESOURCE,
127+
QueueResource: {
128+
Type: 'AWS::SQS::Queue'
129+
},
126130
BucketResource: {
127131
Type: 'AWS::S3::Bucket',
128132
Properties: {
@@ -148,7 +152,9 @@ exports.diffTemplate = {
148152
const bucketName = 'ShineyBucketName';
149153
const currentTemplate = {
150154
Resources: {
151-
BucketPolicyResource: BUCKET_POLICY_RESOURCE,
155+
QueueResource: {
156+
Type: 'AWS::SQS::Queue'
157+
},
152158
BucketResource: {
153159
Type: 'AWS::S3::Bucket',
154160
Properties: {
@@ -160,7 +166,9 @@ exports.diffTemplate = {
160166

161167
const newTemplate = {
162168
Resources: {
163-
BucketPolicyResource: BUCKET_POLICY_RESOURCE,
169+
QueueResource: {
170+
Type: 'AWS::SQS::Queue'
171+
},
164172
BucketResource: {
165173
Type: 'AWS::S3::Bucket'
166174
}
@@ -183,7 +191,9 @@ exports.diffTemplate = {
183191
const bucketName = 'ShineyBucketName';
184192
const currentTemplate = {
185193
Resources: {
186-
BucketPolicyResource: BUCKET_POLICY_RESOURCE,
194+
QueueResource: {
195+
Type: 'AWS::SQS::Queue'
196+
},
187197
BucketResource: {
188198
Type: 'AWS::S3::Bucket'
189199
}
@@ -192,7 +202,9 @@ exports.diffTemplate = {
192202

193203
const newTemplate = {
194204
Resources: {
195-
BucketPolicyResource: BUCKET_POLICY_RESOURCE,
205+
QueueResource: {
206+
Type: 'AWS::SQS::Queue'
207+
},
196208
BucketResource: {
197209
Type: 'AWS::S3::Bucket',
198210
Properties: {
@@ -247,5 +259,52 @@ exports.diffTemplate = {
247259
'the difference reflects the type change');
248260
test.equal(difference && difference.changeImpact, ResourceImpact.WILL_REPLACE, 'the difference reflects that the resource will be replaced');
249261
test.done();
250-
}
262+
},
263+
264+
'resource replacement is tracked through references': (test: Test) => {
265+
// If a resource is replaced, then that change shows that references are
266+
// going to change. This may lead to replacement of downstream resources
267+
// if the reference is used in an immutable property, and so on.
268+
269+
// GIVEN
270+
const currentTemplate = {
271+
Resources: {
272+
Bucket: {
273+
Type: 'AWS::S3::Bucket',
274+
Properties: { BucketName: 'Name1', }, // Immutable prop
275+
},
276+
Queue: {
277+
Type: 'AWS::SQS::Queue',
278+
Properties: { QueueName: { Ref: 'Bucket' }}, // Immutable prop
279+
},
280+
Topic: {
281+
Type: 'AWS::SNS::Topic',
282+
Properties: { TopicName: { Ref: 'Queue' }}, // Immutable prop
283+
}
284+
}
285+
};
286+
287+
// WHEN
288+
const newTemplate = {
289+
Resources: {
290+
Bucket: {
291+
Type: 'AWS::S3::Bucket',
292+
Properties: { BucketName: 'Name2', },
293+
},
294+
Queue: {
295+
Type: 'AWS::SQS::Queue',
296+
Properties: { QueueName: { Ref: 'Bucket' }},
297+
},
298+
Topic: {
299+
Type: 'AWS::SNS::Topic',
300+
Properties: { TopicName: { Ref: 'Queue' }},
301+
}
302+
}
303+
};
304+
const differences = diffTemplate(currentTemplate, newTemplate);
305+
306+
// THEN
307+
test.equal(differences.resources.count, 3, 'all resources are replaced');
308+
test.done();
309+
},
251310
};

0 commit comments

Comments
 (0)