Skip to content

Commit d26bbf0

Browse files
comcalvicomcalvi
committed
nested diffs\!
1 parent d89866d commit d26bbf0

5 files changed

Lines changed: 55 additions & 15 deletions

File tree

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

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,13 @@ export function fullDiff(
6161
normalize(newTemplate);
6262
const theDiff = diffTemplate(currentTemplate, newTemplate);
6363
if (changeSet) {
64-
filterFalsePositivies(theDiff, changeSet);
64+
filterFalsePositivies(theDiff, findResourceReplacements(changeSet));
6565
}
6666

6767
return theDiff;
6868
}
6969

70-
function diffTemplate(
70+
export function diffTemplate(
7171
currentTemplate: { [key: string]: any },
7272
newTemplate: { [key: string]: any },
7373
): types.TemplateDiff {
@@ -217,8 +217,7 @@ function deepCopy(x: any): any {
217217
return x;
218218
}
219219

220-
function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput | NestedChangeSet) {
221-
const replacements = findResourceReplacements(changeSet);
220+
function filterFalsePositivies(diff: types.TemplateDiff, replacements: types.ResourceReplacements) {
222221
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
223222
change.forEachDifference((type: 'Property' | 'Other', name: string, value: types.Difference<any> | types.PropertyDifference<any>) => {
224223
if (type === 'Property') {
@@ -227,6 +226,16 @@ function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormati
227226
(value as types.PropertyDifference<any>).isDifferent = false;
228227
return;
229228
}
229+
// to make this work, diff needs to be aware that there can be nested templates. Currently, we don't really have this.
230+
231+
/*
232+
if (name === 'NestedTemplate') {
233+
(value as types.PropertyDifference<any>).changeImpact = types.ResourceImpact.WILL_UPDATE;
234+
filterFalsePositivies(diff, replacements[logicalId].nestedReplacements!);
235+
return;
236+
}
237+
*/
238+
230239
switch (replacements[logicalId].propertiesReplaced[name]) {
231240
case 'Always':
232241
(value as types.PropertyDifference<any>).changeImpact = types.ResourceImpact.WILL_REPLACE;
@@ -243,6 +252,12 @@ function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormati
243252
break;
244253
// otherwise, defer to the changeImpact from `diffTemplate`
245254
}
255+
256+
/*
257+
if (replacements[logicalId].nestedReplacements) {
258+
filterFalsePositivies(diff, replacements[logicalId].nestedReplacements!);
259+
}
260+
*/
246261
} else if (type === 'Other') {
247262
switch (name) {
248263
case 'Metadata':
@@ -254,14 +269,15 @@ function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormati
254269
});
255270
}
256271

257-
function findResourceReplacements(changeSet: NestedChangeSet, replacements: types.ResourceReplacements = {}): types.ResourceReplacements {
258-
for (const resourceChange of changeSet.Changes ?? []) {
272+
function findResourceReplacements(changeSet: NestedChangeSet): types.ResourceReplacements {
273+
const replacements: types.ResourceReplacements = {};
274+
for (const change of changeSet.Changes ?? []) {
259275
const propertiesReplaced: { [propName: string]: types.ChangeSetReplacement } = {};
260-
if (resourceChange.ResourceChange?.NestedChanges) {
261-
// eslint-disable-next-line max-len
262-
findResourceReplacements(resourceChange.ResourceChange.NestedChanges, replacements[resourceChange.ResourceChange!.LogicalResourceId!] as types.ResourceReplacements);
276+
const resourceChange = change.ResourceChange;
277+
if (!resourceChange) {
278+
continue;
263279
}
264-
for (const propertyChange of resourceChange.ResourceChange?.Details ?? []) {
280+
for (const propertyChange of resourceChange.Details ?? []) {
265281
if (propertyChange.Target?.Attribute === 'Properties') {
266282
const requiresReplacement = propertyChange.Target.RequiresRecreation === 'Always';
267283
if (requiresReplacement && propertyChange.Evaluation === 'Static') {
@@ -275,9 +291,10 @@ function findResourceReplacements(changeSet: NestedChangeSet, replacements: type
275291
}
276292
}
277293
}
278-
replacements[resourceChange.ResourceChange?.LogicalResourceId!] = {
279-
resourceReplaced: resourceChange.ResourceChange?.Replacement === 'True',
294+
replacements[resourceChange.LogicalResourceId!] = {
295+
resourceReplaced: resourceChange.Replacement === 'True',
280296
propertiesReplaced,
297+
nestedReplacements: resourceChange.NestedChanges ? findResourceReplacements(resourceChange.NestedChanges) : undefined,
281298
};
282299
}
283300

‎packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Resource } from '@aws-cdk/service-spec-types';
22
import * as types from './types';
33
import { deepEqual, diffKeyedEntities, loadResourceModel } from './util';
4+
import { diffTemplate } from '../diff-template';
45

56
export function diffAttribute(oldValue: any, newValue: any): types.Difference<string> {
67
return new types.Difference<string>(_asString(oldValue), _asString(newValue));
@@ -33,6 +34,7 @@ export function diffResource(oldValue?: types.Resource, newValue?: types.Resourc
3334
};
3435
let propertyDiffs: { [key: string]: types.PropertyDifference<any> } = {};
3536
let otherDiffs: { [key: string]: types.Difference<any> } = {};
37+
let nestedDiff: types.ITemplateDiff = {};
3638

3739
if (resourceType.oldType !== undefined && resourceType.oldType === resourceType.newType) {
3840
// Only makes sense to inspect deeper if the types stayed the same
@@ -43,10 +45,14 @@ export function diffResource(oldValue?: types.Resource, newValue?: types.Resourc
4345

4446
otherDiffs = diffKeyedEntities(oldValue, newValue, _diffOther);
4547
delete otherDiffs.Properties;
48+
49+
if (resourceType.newType === 'AWS::CloudFormation::Stack') {
50+
nestedDiff = diffTemplate(oldValue?.Properties!.NestedTemplate, newValue?.Properties!.NestedTemplate);
51+
}
4652
}
4753

4854
return new types.ResourceDifference(oldValue, newValue, {
49-
resourceType, propertyDiffs, otherDiffs,
55+
resourceType, propertyDiffs, otherDiffs, nestedDiff,
5056
});
5157

5258
function _diffProperty(oldV: any, newV: any, key: string, resourceSpec?: Resource) {

‎packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@ import { SecurityGroupChanges } from '../network/security-group-changes';
66

77
export type PropertyMap = {[key: string]: any };
88

9-
export type ResourceReplacements = { [logicalId: string]: ResourceReplacement | ResourceReplacements };
9+
export type ResourceReplacements = { [logicalId: string]: ResourceReplacement };
1010

1111
export interface ResourceReplacement {
12-
resourceReplaced: boolean,
12+
resourceReplaced: boolean;
1313
propertiesReplaced: { [propertyName: string]: ChangeSetReplacement };
14+
nestedReplacements?: ResourceReplacements;
1415
}
1516

1617
export type ChangeSetReplacement = 'Always' | 'Never' | 'Conditionally';
@@ -534,6 +535,9 @@ export class ResourceDifference implements IDifference<Resource> {
534535
/** Changes to non-property level attributes of the resource */
535536
private readonly otherDiffs: { [key: string]: Difference<any> };
536537

538+
/** Changes to the nested stack template, iff this resource is a nested stack */
539+
private readonly nestedDiff: ITemplateDiff;
540+
537541
/** The resource type (or old and new type if it has changed) */
538542
private readonly resourceTypes: { readonly oldType?: string, readonly newType?: string };
539543

@@ -544,11 +548,13 @@ export class ResourceDifference implements IDifference<Resource> {
544548
resourceType: { oldType?: string, newType?: string },
545549
propertyDiffs: { [key: string]: PropertyDifference<any> },
546550
otherDiffs: { [key: string]: Difference<any> }
551+
nestedDiff: ITemplateDiff,
547552
},
548553
) {
549554
this.resourceTypes = args.resourceType;
550555
this.propertyDiffs = args.propertyDiffs;
551556
this.otherDiffs = args.otherDiffs;
557+
this.nestedDiff = args.nestedDiff;
552558

553559
this.isAddition = oldValue === undefined;
554560
this.isRemoval = newValue === undefined;
@@ -598,6 +604,12 @@ export class ResourceDifference implements IDifference<Resource> {
598604
return onlyChanges(this.otherDiffs);
599605
}
600606

607+
//public get nestedChanges(): { [key: string]: Difference<any> } {
608+
//return onlyChanges(this.nestedDiff);
609+
public get nestedChanges(): ITemplateDiff {
610+
return this.nestedDiff;
611+
}
612+
601613
/**
602614
* Return whether the resource type was changed in this diff
603615
*

‎packages/@aws-cdk/cloudformation-diff/lib/format.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ class Formatter {
158158

159159
const resourceType = diff.isRemoval ? diff.oldResourceType : diff.newResourceType;
160160

161+
if (Object.keys(diff.nestedChanges).length > 0) {
162+
formatDifferences(this.stream, diff.nestedChanges as TemplateDiff, this.logicalToPathMap, this.context);
163+
}
164+
161165
// eslint-disable-next-line max-len
162166
this.print(`${this.formatPrefix(diff)} ${this.formatValue(resourceType, chalk.cyan)} ${this.formatLogicalId(logicalId)} ${this.formatImpact(diff.changeImpact)}`);
163167

‎packages/aws-cdk/lib/api/hotswap-deployments.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ function makeRenameDifference(
276276
},
277277
propertyDiffs: (addChange as any).propertyDiffs,
278278
otherDiffs: (addChange as any).otherDiffs,
279+
nestedDiff: {}, // TODO: how does this work with hotswap???
279280
},
280281
);
281282
}

0 commit comments

Comments
 (0)