From 51f805272a37bb1988ae3f23f238dd681f4d4c52 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 14 Jan 2019 13:37:12 +0100 Subject: [PATCH] Change replacement diffing to not trigger IAM changes dialog We used to modify the new template in place, causing changes to "logicalIDs" of replaced resources, which would trigger downstream changes and potential downstream replacements, etc. This would lead to the changed logical ID popping up in the IAM diff which was not desirable. In this change, we do the same processing but we throw away the template after all changes have been propagated, and only copy the resultant property STATUSES onto the diff object that gets returned to the user. This leads to the same displayed result without the changes to the template actually propagating. In the course of this modification, the diff classes have been changed to also have objects in places of resources and properties that didn't actually change (so that they could be modified in-place), and diff objects have a boolean telling whether they are actual changes or not. Fixes #1495. --- .../assert/lib/assertions/match-template.ts | 2 +- .../cloudformation-diff/lib/diff-template.ts | 79 +++++-- .../cloudformation-diff/lib/diff/index.ts | 20 +- .../cloudformation-diff/lib/diff/types.ts | 210 +++++++++++++----- .../cloudformation-diff/lib/diff/util.ts | 1 - .../cloudformation-diff/lib/format.ts | 14 +- .../test/test.diff-template.ts | 40 ++-- packages/aws-cdk/lib/diff.ts | 2 +- 8 files changed, 246 insertions(+), 122 deletions(-) diff --git a/packages/@aws-cdk/assert/lib/assertions/match-template.ts b/packages/@aws-cdk/assert/lib/assertions/match-template.ts index 7ec358081fdb2..5e7f3d5d0556c 100644 --- a/packages/@aws-cdk/assert/lib/assertions/match-template.ts +++ b/packages/@aws-cdk/assert/lib/assertions/match-template.ts @@ -55,7 +55,7 @@ class StackMatchesTemplateAssertion extends Assertion { private isDiffAcceptable(diff: cfnDiff.TemplateDiff): boolean { switch (this.matchStyle) { case MatchStyle.EXACT: - return diff.count === 0; + return diff.differenceCount === 0; case MatchStyle.NO_REPLACES: for (const key of Object.keys(diff.resources.changes)) { const change = diff.resources.changes[key]!; diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index b9d47e7080456..f10bb0a1b3885 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -39,42 +39,77 @@ const DIFF_HANDLERS: HandlerRegistry = { * the template +newTemplate+. */ export function diffTemplate(currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }): types.TemplateDiff { + // Base diff + const theDiff = calculateTemplateDiff(currentTemplate, newTemplate); + // We're going to modify this in-place - newTemplate = deepCopy(newTemplate); - - while (true) { - const differences: types.ITemplateDiff = {}; - const unknown: { [key: string]: types.Difference } = {}; - for (const key of unionOf(Object.keys(currentTemplate), Object.keys(newTemplate)).sort()) { - const oldValue = currentTemplate[key]; - const newValue = newTemplate[key]; - if (deepEqual(oldValue, newValue)) { continue; } - const handler: DiffHandler = DIFF_HANDLERS[key] - || ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV)); - handler(differences, oldValue, newValue); + const newTemplateCopy = deepCopy(newTemplate); - } - if (Object.keys(unknown).length > 0) { differences.unknown = new types.DifferenceCollection(unknown); } + let didPropagateReferenceChanges; + let diffWithReplacements; + do { + diffWithReplacements = calculateTemplateDiff(currentTemplate, newTemplateCopy); // Propagate replacements for replaced resources - let didPropagateReferenceChanges = false; - if (differences.resources) { - differences.resources.forEach((logicalId, change) => { + didPropagateReferenceChanges = false; + if (diffWithReplacements.resources) { + diffWithReplacements.resources.forEachDifference((logicalId, change) => { if (change.changeImpact === types.ResourceImpact.WILL_REPLACE) { - if (propagateReplacedReferences(newTemplate, logicalId)) { + if (propagateReplacedReferences(newTemplateCopy, logicalId)) { didPropagateReferenceChanges = true; } } }); } + } while (didPropagateReferenceChanges); + + // Copy "replaced" states from `diffWithReplacements` to `theDiff`. + diffWithReplacements.resources + .filter(r => isReplacement(r!.changeImpact)) + .forEachDifference((logicalId, downstreamReplacement) => { + const resource = theDiff.resources.get(logicalId); + + if (resource.changeImpact !== downstreamReplacement.changeImpact) { + propagatePropertyReplacement(downstreamReplacement, resource); + } + }); + + return theDiff; +} + +function isReplacement(impact: types.ResourceImpact) { + return impact === types.ResourceImpact.MAY_REPLACE || impact === types.ResourceImpact.WILL_REPLACE; +} - // We're done only if we didn't have to propagate any more replacements. - if (!didPropagateReferenceChanges) { - return new types.TemplateDiff(differences); +/** + * For all properties in 'source' that have a "replacement" impact, propagate that impact to "dest" + */ +function propagatePropertyReplacement(source: types.ResourceDifference, dest: types.ResourceDifference) { + for (const [propertyName, diff] of Object.entries(source.propertyUpdates)) { + if (diff.changeImpact && isReplacement(diff.changeImpact)) { + // Use the propertydiff of source in target. The result of this happens to be clear enough. + dest.setPropertyChange(propertyName, diff); } } } +function calculateTemplateDiff(currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }): types.TemplateDiff { + const differences: types.ITemplateDiff = {}; + const unknown: { [key: string]: types.Difference } = {}; + for (const key of unionOf(Object.keys(currentTemplate), Object.keys(newTemplate)).sort()) { + const oldValue = currentTemplate[key]; + const newValue = newTemplate[key]; + if (deepEqual(oldValue, newValue)) { continue; } + const handler: DiffHandler = DIFF_HANDLERS[key] + || ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV)); + handler(differences, oldValue, newValue); + + } + if (Object.keys(unknown).length > 0) { differences.unknown = new types.DifferenceCollection(unknown); } + + return new types.TemplateDiff(differences); +} + /** * Compare two CloudFormation resources and return semantic differences between them */ @@ -109,7 +144,7 @@ function propagateReplacedReferences(template: object, logicalId: string): boole if (key === 'Ref') { if (obj.Ref === logicalId) { - obj.Ref = logicalId + '(replaced)'; + obj.Ref = logicalId + ' (replaced)'; ret = true; } return true; diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts index 0add877280471..1bcfd6d470970 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts @@ -1,6 +1,6 @@ import cfnspec = require('@aws-cdk/cfnspec'); import types = require('./types'); -import { diffKeyedEntities } from './util'; +import { deepEqual, diffKeyedEntities } from './util'; export function diffAttribute(oldValue: any, newValue: any): types.Difference { return new types.Difference(_asString(oldValue), _asString(newValue)); @@ -31,31 +31,29 @@ export function diffResource(oldValue?: types.Resource, newValue?: types.Resourc oldType: oldValue && oldValue.Type, newType: newValue && newValue.Type }; - let propertyUpdates: { [key: string]: types.PropertyDifference } = {}; - let otherChanges: { [key: string]: types.Difference } = {}; + let propertyDiffs: { [key: string]: types.PropertyDifference } = {}; + let otherDiffs: { [key: string]: types.Difference } = {}; if (resourceType.oldType !== undefined && resourceType.oldType === resourceType.newType) { // Only makes sense to inspect deeper if the types stayed the same const typeSpec = cfnspec.filteredSpecification(resourceType.oldType); const impl = typeSpec.ResourceTypes[resourceType.oldType]; - propertyUpdates = diffKeyedEntities(oldValue!.Properties, + propertyDiffs = diffKeyedEntities(oldValue!.Properties, newValue!.Properties, (oldVal, newVal, key) => _diffProperty(oldVal, newVal, key, impl)); - otherChanges = diffKeyedEntities(oldValue, newValue, _diffOther); - delete otherChanges.Properties; + otherDiffs = diffKeyedEntities(oldValue, newValue, _diffOther); + delete otherDiffs.Properties; } return new types.ResourceDifference(oldValue, newValue, { - resourceType, propertyUpdates, otherChanges, - oldProperties: oldValue && oldValue.Properties, - newProperties: newValue && newValue.Properties, + resourceType, propertyDiffs, otherDiffs, }); function _diffProperty(oldV: any, newV: any, key: string, resourceSpec?: cfnspec.schema.ResourceType) { - let changeImpact; + let changeImpact = types.ResourceImpact.NO_CHANGE; const spec = resourceSpec && resourceSpec.Properties && resourceSpec.Properties[key]; - if (spec) { + if (spec && !deepEqual(oldV, newV)) { switch (spec.UpdateType) { case 'Immutable': changeImpact = types.ResourceImpact.WILL_REPLACE; diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts index f222e56171ac3..3294b9c76b067 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -62,7 +62,7 @@ export class TemplateDiff implements ITemplateDiff { }); } - public get count() { + public get differenceCount() { let count = 0; if (this.awsTemplateFormatVersion !== undefined) { @@ -75,19 +75,19 @@ export class TemplateDiff implements ITemplateDiff { count += 1; } - count += this.conditions.count; - count += this.mappings.count; - count += this.metadata.count; - count += this.outputs.count; - count += this.parameters.count; - count += this.resources.count; - count += this.unknown.count; + count += this.conditions.differenceCount; + count += this.mappings.differenceCount; + count += this.metadata.differenceCount; + count += this.outputs.differenceCount; + count += this.parameters.differenceCount; + count += this.resources.differenceCount; + count += this.unknown.differenceCount; return count; } public get isEmpty(): boolean { - return this.count === 0; + return this.differenceCount === 0; } /** @@ -257,10 +257,24 @@ export interface ResourceChange { newProperties?: PropertyMap; } +export interface IDifference { + readonly oldValue: ValueType | undefined; + readonly newValue: ValueType | undefined; + readonly isDifferent: boolean; + readonly isAddition: boolean; + readonly isRemoval: boolean; + readonly isUpdate: boolean; +} + /** * Models an entity that changed between two versions of a CloudFormation template. */ -export class Difference { +export class Difference implements IDifference { + /** + * Whether this is an actual different or the values are actually the same + */ + public readonly isDifferent: boolean; + /** * @param oldValue the old value, cannot be equal (to the sense of +deepEqual+) to +newValue+. * @param newValue the new value, cannot be equal (to the sense of +deepEqual+) to +oldValue+. @@ -269,11 +283,7 @@ export class Difference { if (oldValue === undefined && newValue === undefined) { throw new AssertionError({ message: 'oldValue and newValue are both undefined!' }); } - if (deepEqual(oldValue, newValue)) { - const oldStr = JSON.stringify(oldValue); - const newStr = JSON.stringify(newValue); - throw new NoDifferenceError(`oldValue (${oldStr}) and newValue (${newStr}) are equal!`); - } + this.isDifferent = !deepEqual(oldValue, newValue); } /** @returns +true+ if the element is new to the template. */ @@ -302,11 +312,21 @@ export class PropertyDifference extends Difference { } } -export class DifferenceCollection> { - constructor(public readonly changes: { [logicalId: string]: T | undefined }) {} +export class DifferenceCollection> { + constructor(private readonly diffs: { [logicalId: string]: T }) {} - public get count(): number { - return this.logicalIds.length; + public get changes(): { [logicalId: string]: T } { + return onlyChanges(this.diffs); + } + + public get differenceCount(): number { + return Object.values(this.changes).length; + } + + public get(logicalId: string): T { + const ret = this.diffs[logicalId]; + if (!ret) { throw new Error(`No object with logical ID '${logicalId}'`); } + return ret; } public get logicalIds(): string[] { @@ -318,7 +338,7 @@ export class DifferenceCollection> { * returns `true`. */ public filter(predicate: (diff: T | undefined) => boolean): DifferenceCollection { - const newChanges: { [logicalId: string]: T | undefined } = { }; + const newChanges: { [logicalId: string]: T } = { }; for (const id of Object.keys(this.changes)) { const diff = this.changes[id]; @@ -341,7 +361,7 @@ export class DifferenceCollection> { * * @param cb */ - public forEach(cb: (logicalId: string, change: T) => any): void { + public forEachDifference(cb: (logicalId: string, change: T) => any): void { const removed = new Array<{ logicalId: string, change: T }>(); const added = new Array<{ logicalId: string, change: T }>(); const updated = new Array<{ logicalId: string, change: T }>(); @@ -355,7 +375,7 @@ export class DifferenceCollection> { removed.push({ logicalId, change }); } else if (change.isUpdate) { updated.push({ logicalId, change }); - } else { + } else if (change.isDifferent) { others.push({ logicalId, change }); } } @@ -372,9 +392,9 @@ export class DifferenceCollection> { * of (relative) conciseness of the constructor's signature. */ export interface ITemplateDiff { - awsTemplateFormatVersion?: Difference; - description?: Difference; - transform?: Difference; + awsTemplateFormatVersion?: IDifference; + description?: IDifference; + transform?: IDifference; conditions?: DifferenceCollection; mappings?: DifferenceCollection; @@ -383,7 +403,7 @@ export interface ITemplateDiff { parameters?: DifferenceCollection; resources?: DifferenceCollection; - unknown?: DifferenceCollection>; + unknown?: DifferenceCollection>; } export type Condition = any; @@ -423,7 +443,9 @@ export enum ResourceImpact { /** The existing physical resource will be destroyed */ WILL_DESTROY = 'WILL_DESTROY', /** The existing physical resource will be removed from CloudFormation supervision */ - WILL_ORPHAN = 'WILL_ORPHAN' + WILL_ORPHAN = 'WILL_ORPHAN', + /** There is no change in this resource */ + NO_CHANGE = 'NO_CHANGE', } /** @@ -436,12 +458,13 @@ export enum ResourceImpact { function worstImpact(one: ResourceImpact, two?: ResourceImpact): ResourceImpact { if (!two) { return one; } const badness = { - [ResourceImpact.WILL_UPDATE]: 0, - [ResourceImpact.WILL_CREATE]: 1, - [ResourceImpact.WILL_ORPHAN]: 2, - [ResourceImpact.MAY_REPLACE]: 3, - [ResourceImpact.WILL_REPLACE]: 4, - [ResourceImpact.WILL_DESTROY]: 5, + [ResourceImpact.NO_CHANGE]: 0, + [ResourceImpact.WILL_UPDATE]: 1, + [ResourceImpact.WILL_CREATE]: 2, + [ResourceImpact.WILL_ORPHAN]: 3, + [ResourceImpact.MAY_REPLACE]: 4, + [ResourceImpact.WILL_REPLACE]: 5, + [ResourceImpact.WILL_DESTROY]: 6, }; return badness[one] > badness[two] ? one : two; } @@ -453,41 +476,67 @@ export interface Resource { [key: string]: any; } -export class ResourceDifference extends Difference { +/** + * Change to a single resource between two CloudFormation templates + * + * This class can be mutated after construction. + */ +export class ResourceDifference implements IDifference { /** - * Old property values + * Whether this resource was added */ - public readonly oldProperties?: PropertyMap; + public readonly isAddition: boolean; /** - * New property values + * Whether this resource was removed */ - public readonly newProperties?: PropertyMap; + public readonly isRemoval: boolean; /** Property-level changes on the resource */ - public readonly propertyUpdates: { [key: string]: PropertyDifference }; + private readonly propertyDiffs: { [key: string]: PropertyDifference }; + /** Changes to non-property level attributes of the resource */ - public readonly otherChanges: { [key: string]: Difference }; + private readonly otherDiffs: { [key: string]: Difference }; /** The resource type (or old and new type if it has changed) */ private readonly resourceTypes: { readonly oldType?: string, readonly newType?: string }; - constructor(oldValue: Resource | undefined, - newValue: Resource | undefined, + constructor(public readonly oldValue: Resource | undefined, + public readonly newValue: Resource | undefined, args: { resourceType: { oldType?: string, newType?: string }, - oldProperties?: PropertyMap, - newProperties?: PropertyMap, - propertyUpdates: { [key: string]: PropertyDifference }, - otherChanges: { [key: string]: Difference } + propertyDiffs: { [key: string]: PropertyDifference }, + otherDiffs: { [key: string]: Difference } } ) { - super(oldValue, newValue); this.resourceTypes = args.resourceType; - this.propertyUpdates = args.propertyUpdates; - this.otherChanges = args.otherChanges; - this.oldProperties = args.oldProperties; - this.newProperties = args.newProperties; + this.propertyDiffs = args.propertyDiffs; + this.otherDiffs = args.otherDiffs; + + this.isAddition = oldValue === undefined; + this.isRemoval = newValue === undefined; + } + + public get oldProperties(): PropertyMap | undefined { + return this.oldValue && this.oldValue.Properties; + } + + public get newProperties(): PropertyMap | undefined { + return this.newValue && this.newValue.Properties; + } + + /** + * Whether this resource was modified at all + */ + public get isDifferent(): boolean { + return this.differenceCount > 0 || this.oldResourceType !== this.newResourceType; + } + + /** + * Whether the resource was updated in-place + */ + public get isUpdate(): boolean { + return this.isDifferent && !this.isAddition && !this.isRemoval; } public get oldResourceType(): string | undefined { @@ -498,6 +547,20 @@ export class ResourceDifference extends Difference { return this.resourceTypes.newType; } + /** + * All actual property updates + */ + public get propertyUpdates(): { [key: string]: PropertyDifference } { + return onlyChanges(this.propertyDiffs); + } + + /** + * All actual "other" updates + */ + public get otherChanges(): { [key: string]: Difference } { + return onlyChanges(this.otherDiffs); + } + /** * Return whether the resource type was changed in this diff * @@ -522,6 +585,18 @@ export class ResourceDifference extends Difference { return this.resourceTypes.oldType || this.resourceTypes.newType!; } + /** + * Replace a PropertyChange in this object + * + * This affects the property diff as it is summarized to users, but it DOES + * NOT affect either the "oldValue" or "newValue" values; those still contain + * the actual template values as provided by the user (they might still be + * used for downstream processing). + */ + public setPropertyChange(propertyName: string, change: PropertyDifference) { + this.propertyDiffs[propertyName] = change; + } + public get changeImpact(): ResourceImpact { // Check the Type first if (this.resourceTypes.oldType !== this.resourceTypes.newType) { @@ -534,22 +609,28 @@ export class ResourceDifference extends Difference { return ResourceImpact.WILL_REPLACE; } - return Object.values(this.propertyUpdates) + return Object.values(this.propertyDiffs) .map(elt => elt.changeImpact) .reduce(worstImpact, ResourceImpact.WILL_UPDATE); } - public get count(): number { - return Object.keys(this.propertyUpdates).length - + Object.keys(this.otherChanges).length; + /** + * Count of actual differences (not of elements) + */ + public get differenceCount(): number { + return Object.values(this.propertyUpdates).length + + Object.values(this.otherChanges).length; } - public forEach(cb: (type: 'Property' | 'Other', name: string, value: Difference | PropertyDifference) => any) { + /** + * Invoke a callback for each actual difference + */ + public forEachDifference(cb: (type: 'Property' | 'Other', name: string, value: Difference | PropertyDifference) => any) { for (const key of Object.keys(this.propertyUpdates).sort()) { cb('Property', key, this.propertyUpdates[key]); } for (const key of Object.keys(this.otherChanges).sort()) { - cb('Other', key, this.otherChanges[key]); + cb('Other', key, this.otherDiffs[key]); } } } @@ -558,8 +639,15 @@ export function isPropertyDifference(diff: Difference): diff is PropertyDi return (diff as PropertyDifference).changeImpact !== undefined; } -class NoDifferenceError extends Error { - constructor(message: string) { - super(`No difference: ${message}`); +/** + * Filter a map of IDifferences down to only retain the actual changes + */ +function onlyChanges>(xs: {[key: string]: T}): {[key: string]: T} { + const ret: { [key: string]: T } = {}; + for (const [key, diff] of Object.entries(xs)) { + if (diff.isDifferent) { + ret[key] = diff; + } } -} + return ret; +} \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts index e0832f2bf8ff6..badd8e9ac3f48 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts @@ -54,7 +54,6 @@ export function diffKeyedEntities(oldValue: { [key: string]: any } | undefine for (const logicalId of unionOf(Object.keys(oldValue || {}), Object.keys(newValue || {}))) { const oldElement = oldValue && oldValue[logicalId]; const newElement = newValue && newValue[logicalId]; - if (deepEqual(oldElement, newElement)) { continue; } result[logicalId] = elementDiff(oldElement, newElement, logicalId); } return result; diff --git a/packages/@aws-cdk/cloudformation-diff/lib/format.ts b/packages/@aws-cdk/cloudformation-diff/lib/format.ts index e37703d0da059..fcf6c35a2b162 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/format.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/format.ts @@ -96,12 +96,12 @@ class Formatter { collection: DifferenceCollection, formatter: (type: string, id: string, diff: T) => void = this.formatDifference.bind(this)) { - if (collection.count === 0) { + if (collection.differenceCount === 0) { return; } this.printSectionHeader(title); - collection.forEach((id, diff) => formatter(entryType, id, diff)); + collection.forEachDifference((id, diff) => formatter(entryType, id, diff)); this.printSectionFooter(); } @@ -120,7 +120,7 @@ class Formatter { * @param diff the difference to be rendered. */ public formatDifference(type: string, logicalId: string, diff: Difference | undefined) { - if (!diff) { return; } + if (!diff || !diff.isDifferent) { return; } let value; @@ -144,16 +144,19 @@ class Formatter { * @param diff the change to be rendered. */ public formatResourceDifference(_type: string, logicalId: string, diff: ResourceDifference) { + if (!diff.isDifferent) { return; } + const resourceType = diff.isRemoval ? diff.oldResourceType : diff.newResourceType; // tslint:disable-next-line:max-line-length this.print(`${this.formatPrefix(diff)} ${this.formatValue(resourceType, colors.cyan)} ${this.formatLogicalId(logicalId)} ${this.formatImpact(diff.changeImpact)}`); if (diff.isUpdate) { + const differenceCount = diff.differenceCount; let processedCount = 0; - diff.forEach((_, name, values) => { + diff.forEachDifference((_, name, values) => { processedCount += 1; - this.formatTreeDiff(name, values, processedCount === diff.count); + this.formatTreeDiff(name, values, processedCount === differenceCount); }); } } @@ -193,6 +196,7 @@ class Formatter { return colors.italic(colors.yellow('orphan')); case ResourceImpact.WILL_UPDATE: case ResourceImpact.WILL_CREATE: + case ResourceImpact.NO_CHANGE: return ''; // no extra info is gained here } } diff --git a/packages/@aws-cdk/cloudformation-diff/test/test.diff-template.ts b/packages/@aws-cdk/cloudformation-diff/test/test.diff-template.ts index 01cdc3663dcaf..3b490bb0a95c6 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/test.diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/test.diff-template.ts @@ -30,7 +30,7 @@ exports.diffTemplate = { const newTemplate = JSON.parse(JSON.stringify(currentTemplate)); const differences = diffTemplate(currentTemplate, newTemplate); - test.deepEqual(differences.count, 0, 'returns an empty diff'); + test.deepEqual(differences.differenceCount, 0, 'returns an empty diff'); test.done(); }, @@ -40,8 +40,8 @@ exports.diffTemplate = { const newTemplate = { Resources: { BucketResource: { Type: 'AWS::S3::Bucket' } } }; const differences = diffTemplate(currentTemplate, newTemplate); - test.equal(differences.count, 1, 'returns a single difference'); - test.equal(differences.resources.count, 1, 'the difference is in the Resources section'); + test.equal(differences.differenceCount, 1, 'returns a single difference'); + test.equal(differences.resources.differenceCount, 1, 'the difference is in the Resources section'); const difference = differences.resources.changes.BucketResource; test.notStrictEqual(difference, undefined, 'the difference is on the BucketResource logical ID'); test.ok(difference && difference.isAddition, 'the difference reflects there was no such resource before'); @@ -65,8 +65,8 @@ exports.diffTemplate = { }; const differences = diffTemplate(currentTemplate, newTemplate); - test.equal(differences.count, 1, 'returns a single difference'); - test.equal(differences.resources.count, 1, 'the difference is in the Resources section'); + test.equal(differences.differenceCount, 1, 'returns a single difference'); + test.equal(differences.resources.differenceCount, 1, 'the difference is in the Resources section'); const difference = differences.resources.changes.BucketPolicyResource; test.notStrictEqual(difference, undefined, 'the difference is on the BucketPolicyResource logical ID'); test.ok(difference && difference.isRemoval, 'the difference reflects there is no such resource after'); @@ -95,8 +95,8 @@ exports.diffTemplate = { }; const differences = diffTemplate(currentTemplate, newTemplate); - test.equal(differences.count, 1, 'returns a single difference'); - test.equal(differences.resources.count, 1, 'the difference is in the Resources section'); + test.equal(differences.differenceCount, 1, 'returns a single difference'); + test.equal(differences.resources.differenceCount, 1, 'the difference is in the Resources section'); const difference = differences.resources.changes.BucketPolicyResource; test.notStrictEqual(difference, undefined, 'the difference is on the BucketPolicyResource logical ID'); test.ok(difference && difference.isRemoval, 'the difference reflects there is no such resource after'); @@ -137,13 +137,13 @@ exports.diffTemplate = { }; const differences = diffTemplate(currentTemplate, newTemplate); - test.equal(differences.count, 1, 'returns a single difference'); - test.equal(differences.resources.count, 1, 'the difference is in the Resources section'); + test.equal(differences.differenceCount, 1, 'returns a single difference'); + test.equal(differences.resources.differenceCount, 1, 'the difference is in the Resources section'); const difference = differences.resources.changes.BucketResource; test.notStrictEqual(difference, undefined, 'the difference is on the BucketResource logical ID'); test.equal(difference && difference.oldResourceType, 'AWS::S3::Bucket', 'the difference reports the resource type'); test.deepEqual(difference && difference.propertyUpdates, - { BucketName: { oldValue: bucketName, newValue: newBucketName, changeImpact: ResourceImpact.WILL_REPLACE } }, + { BucketName: { oldValue: bucketName, newValue: newBucketName, changeImpact: ResourceImpact.WILL_REPLACE, isDifferent: true } }, 'the difference reports property-level changes'); test.done(); }, @@ -171,7 +171,7 @@ exports.diffTemplate = { const differences = diffTemplate(currentTemplate, newTemplate); // THEN - test.equal(differences.count, 1, 'no change'); + test.equal(differences.differenceCount, 1, 'no change'); const difference = differences.resources.changes.BucketResource; test.equal(difference && difference.changeImpact, ResourceImpact.WILL_UPDATE, 'the difference reflects that the resource will be replaced'); test.done(); @@ -205,13 +205,13 @@ exports.diffTemplate = { }; const differences = diffTemplate(currentTemplate, newTemplate); - test.equal(differences.count, 1, 'returns a single difference'); - test.equal(differences.resources.count, 1, 'the difference is in the Resources section'); + test.equal(differences.differenceCount, 1, 'returns a single difference'); + test.equal(differences.resources.differenceCount, 1, 'the difference is in the Resources section'); const difference = differences.resources.changes.BucketResource; test.notStrictEqual(difference, undefined, 'the difference is on the BucketResource logical ID'); test.equal(difference && difference.oldResourceType, 'AWS::S3::Bucket', 'the difference reports the resource type'); test.deepEqual(difference && difference.propertyUpdates, - { BucketName: { oldValue: bucketName, newValue: undefined, changeImpact: ResourceImpact.WILL_REPLACE } }, + { BucketName: { oldValue: bucketName, newValue: undefined, changeImpact: ResourceImpact.WILL_REPLACE, isDifferent: true } }, 'the difference reports property-level changes'); test.done(); }, @@ -244,13 +244,13 @@ exports.diffTemplate = { }; const differences = diffTemplate(currentTemplate, newTemplate); - test.equal(differences.count, 1, 'returns a single difference'); - test.equal(differences.resources.count, 1, 'the difference is in the Resources section'); + test.equal(differences.differenceCount, 1, 'returns a single difference'); + test.equal(differences.resources.differenceCount, 1, 'the difference is in the Resources section'); const difference = differences.resources.changes.BucketResource; test.notStrictEqual(difference, undefined, 'the difference is on the BucketResource logical ID'); test.equal(difference && difference.oldResourceType, 'AWS::S3::Bucket', 'the difference reports the resource type'); test.deepEqual(difference && difference.propertyUpdates, - { BucketName: { oldValue: undefined, newValue: bucketName, changeImpact: ResourceImpact.WILL_REPLACE } }, + { BucketName: { oldValue: undefined, newValue: bucketName, changeImpact: ResourceImpact.WILL_REPLACE, isDifferent: true } }, 'the difference reports property-level changes'); test.done(); }, @@ -279,8 +279,8 @@ exports.diffTemplate = { }; const differences = diffTemplate(currentTemplate, newTemplate); - test.equal(differences.count, 1, 'returns a single difference'); - test.equal(differences.resources.count, 1, 'the difference is in the Resources section'); + test.equal(differences.differenceCount, 1, 'returns a single difference'); + test.equal(differences.resources.differenceCount, 1, 'the difference is in the Resources section'); const difference = differences.resources.changes.BucketResource; test.notStrictEqual(difference, undefined, 'the difference is on the BucketResource logical ID'); test.deepEqual(difference && difference.oldResourceType, 'AWS::IAM::Policy'); @@ -332,7 +332,7 @@ exports.diffTemplate = { const differences = diffTemplate(currentTemplate, newTemplate); // THEN - test.equal(differences.resources.count, 3, 'all resources are replaced'); + test.equal(differences.resources.differenceCount, 3, 'all resources are replaced'); test.done(); }, }; diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts index ad14e47da4de0..e086b88bb2480 100644 --- a/packages/aws-cdk/lib/diff.ts +++ b/packages/aws-cdk/lib/diff.ts @@ -37,7 +37,7 @@ export function printStackDiff(oldTemplate: any, newTemplate: cxapi.SynthesizedS print(colors.green('There were no differences')); } - return diff.count; + return diff.differenceCount; } export enum RequireApproval {