diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 96954d43d88a9..5c9986244b26b 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -3,11 +3,11 @@ import { CloudFormation } from 'aws-sdk'; import * as impl from './diff'; import * as types from './diff/types'; import { deepEqual, diffKeyedEntities, unionOf } from './diff/util'; -import { ChangeSetReplacement, ResourceReplacements } from './format'; export * from './diff/types'; -type DiffHandler = (diff: types.ITemplateDiff, oldValue: any, newValue: any, replacement?: ResourceReplacements) => void; +// eslint-disable-next-line max-len +type DiffHandler = (diff: types.ITemplateDiff, oldValue: any, newValue: any, replacement?: types.ResourceReplacements, keepMetadata?: boolean) => void; type HandlerRegistry = { [section: string]: DiffHandler }; const DIFF_HANDLERS: HandlerRegistry = { @@ -25,8 +25,8 @@ const DIFF_HANDLERS: HandlerRegistry = { diff.conditions = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffCondition)), Transform: (diff, oldValue, newValue) => diff.transform = impl.diffAttribute(oldValue, newValue), - Resources: (diff, oldValue, newValue, replacements?: ResourceReplacements) => - diff.resources = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffResource, replacements)), + Resources: (diff, oldValue, newValue, replacements?: types.ResourceReplacements, keepMetadata?: boolean) => + diff.resources = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffResource, replacements, keepMetadata)), Outputs: (diff, oldValue, newValue) => diff.outputs = new types.DifferenceCollection(diffKeyedEntities(oldValue, newValue, impl.diffOutput)), }; @@ -48,7 +48,7 @@ export function diffTemplate( ): types.TemplateDiff { const replacements = changeSet ? findResourceReplacements(changeSet): undefined; // Base diff - const theDiff = calculateTemplateDiff(currentTemplate, newTemplate, replacements); + const theDiff = calculateTemplateDiff(currentTemplate, newTemplate, replacements, changeSet ? false : true); // We're going to modify this in-place const newTemplateCopy = deepCopy(newTemplate); @@ -104,7 +104,8 @@ function propagatePropertyReplacement(source: types.ResourceDifference, dest: ty function calculateTemplateDiff( currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }, - replacements?: ResourceReplacements, + replacements?: types.ResourceReplacements, + keepMetadata?: boolean, ): types.TemplateDiff { const differences: types.ITemplateDiff = {}; const unknown: { [key: string]: types.Difference } = {}; @@ -116,7 +117,7 @@ function calculateTemplateDiff( } const handler: DiffHandler = DIFF_HANDLERS[key] || ((_diff, oldV, newV) => unknown[key] = impl.diffUnknown(oldV, newV)); - handler(differences, oldValue, newValue, replacements); + handler(differences, oldValue, newValue, replacements, keepMetadata); } if (Object.keys(unknown).length > 0) { differences.unknown = new types.DifferenceCollection(unknown); @@ -189,10 +190,10 @@ function deepCopy(x: any): any { return x; } -function findResourceReplacements(changeSet: CloudFormation.DescribeChangeSetOutput): ResourceReplacements { - const replacements: ResourceReplacements = {}; +function findResourceReplacements(changeSet: CloudFormation.DescribeChangeSetOutput): types.ResourceReplacements { + const replacements: types.ResourceReplacements = {}; for (const resourceChange of changeSet.Changes ?? []) { - const propertiesReplaced: { [propName: string]: ChangeSetReplacement } = {}; + const propertiesReplaced: { [propName: string]: types.ChangeSetReplacement } = {}; for (const propertyChange of resourceChange.ResourceChange?.Details ?? []) { if (propertyChange.Target?.Attribute === 'Properties') { const requiresReplacement = propertyChange.Target.RequiresRecreation === 'Always'; @@ -203,7 +204,7 @@ function findResourceReplacements(changeSet: CloudFormation.DescribeChangeSetOut // see 'Replacement': https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_ResourceChange.html propertiesReplaced[propertyChange.Target.Name!] = 'Conditionally'; } else { - propertiesReplaced[propertyChange.Target.Name!] = propertyChange.Target.RequiresRecreation as ChangeSetReplacement; + propertiesReplaced[propertyChange.Target.Name!] = propertyChange.Target.RequiresRecreation as types.ChangeSetReplacement; } } } diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts index d2332f3b7ed54..556c218c8c95d 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts @@ -1,7 +1,6 @@ import { Resource } from '@aws-cdk/service-spec-types'; import * as types from './types'; import { deepEqual, diffKeyedEntities, loadResourceModel } from './util'; -import { ResourceReplacement } from '../format'; export function diffAttribute(oldValue: any, newValue: any): types.Difference { return new types.Difference(_asString(oldValue), _asString(newValue)); @@ -31,7 +30,8 @@ export function diffResource( oldValue?: types.Resource, newValue?: types.Resource, _key?: string, - replacement?: ResourceReplacement, + replacement?: types.ResourceReplacement, + keepMetadata?: boolean, ): types.ResourceDifference { const resourceType = { oldType: oldValue && oldValue.Type, @@ -49,15 +49,18 @@ export function diffResource( otherDiffs = diffKeyedEntities(oldValue, newValue, _diffOther); delete otherDiffs.Properties; - // TODO: should only happen with the right flag set - delete otherDiffs.Metadata; + // eslint-disable-next-line no-console + console.log('keepMetadata??' + keepMetadata); + if (keepMetadata === false) { + delete otherDiffs.Metadata; + } } return new types.ResourceDifference(oldValue, newValue, { resourceType, propertyDiffs, otherDiffs, }); - function _diffProperty(oldV: any, newV: any, key: string, resourceSpec?: Resource, changeSetReplacement?: ResourceReplacement) { + function _diffProperty(oldV: any, newV: any, key: string, resourceSpec?: Resource, changeSetReplacement?: types.ResourceReplacement) { let changeImpact: types.ResourceImpact = types.ResourceImpact.NO_CHANGE; if (changeSetReplacement === undefined) { changeImpact = _resourceSpecImpact(oldV, newV, key, resourceSpec); diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts index 7bd91b58f6ee4..b0cb638a4105d 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -667,6 +667,15 @@ export function isPropertyDifference(diff: Difference): diff is PropertyDi return (diff as PropertyDifference).changeImpact !== undefined; } +export type ResourceReplacements = { [logicalId: string]: ResourceReplacement }; + +export interface ResourceReplacement { + resourceReplaced: boolean, + propertiesReplaced: { [propertyName: string]: ChangeSetReplacement }; +} + +export type ChangeSetReplacement = 'Always' | 'Never' | 'Conditionally'; + /** * Filter a map of IDifferences down to only retain the actual changes */ diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts index 768b18b6a3d6e..0b255d345beec 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts @@ -1,6 +1,6 @@ import { loadAwsServiceSpecSync } from '@aws-cdk/aws-service-spec'; import { Resource, SpecDatabase } from '@aws-cdk/service-spec-types'; -import { ResourceReplacement, ResourceReplacements } from '../format'; +import * as types from './types'; /** * Compares two objects for equality, deeply. The function handles arguments that are @@ -177,8 +177,10 @@ function yamlIntrinsicEqual(lvalue: any, rvalue: any): boolean { export function diffKeyedEntities( oldValue: { [key: string]: any } | undefined, newValue: { [key: string]: any } | undefined, - elementDiff: (oldElement: any, newElement: any, key: string, replacements?: ResourceReplacement) => T, - replacements?: ResourceReplacements): { [name: string]: T } { + elementDiff: (oldElement: any, newElement: any, key: string, replacements?: types.ResourceReplacement, keepMetadata?: boolean) => T, + replacements?: types.ResourceReplacements, + keepMetadata?: boolean, +): { [name: string]: T } { const result: { [name: string]: T } = {}; for (const logicalId of unionOf(Object.keys(oldValue || {}), Object.keys(newValue || {}))) { const oldElement = oldValue && oldValue[logicalId]; @@ -189,7 +191,7 @@ export function diffKeyedEntities( continue; } - result[logicalId] = elementDiff(oldElement, newElement, logicalId, replacements ? replacements[logicalId]: undefined); + result[logicalId] = elementDiff(oldElement, newElement, logicalId, replacements ? replacements[logicalId]: undefined, keepMetadata); } return result; } diff --git a/packages/@aws-cdk/cloudformation-diff/lib/format.ts b/packages/@aws-cdk/cloudformation-diff/lib/format.ts index 87b6f2ba6022e..ce84c2e40f31c 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/format.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/format.ts @@ -18,18 +18,6 @@ export interface FormatStream extends NodeJS.WritableStream { columns?: number; } -/** - * maps logical IDs to their need to be replaced - */ -export type ResourceReplacements = { [logicalId: string]: ResourceReplacement }; - -export interface ResourceReplacement { - resourceReplaced: boolean, - propertiesReplaced: { [propertyName: string]: ChangeSetReplacement }; -} - -export type ChangeSetReplacement = 'Always' | 'Never' | 'Conditionally'; - /** * Renders template differences to the process' console. * diff --git a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts index c6b92a0680fc0..c004c8c2ddb8f 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts @@ -702,6 +702,41 @@ test('diffing any two arbitrary templates should not crash', () => { }); }); +test('metadata changes are rendered in the diff', () => { + // GIVEN + const currentTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + BucketName: 'magic-bucket', + Metadata: { + 'aws:cdk:path': '/foo/BucketResource', + }, + }, + }, + }; + + // WHEN + const newTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + BucketName: 'magic-bucket', + Metadata: { + 'aws:cdk:path': '/bar/BucketResource', + }, + }, + }, + }; + + // THEN + let differences = diffTemplate(currentTemplate, newTemplate); + expect(differences.differenceCount).toBe(1); + + differences = diffTemplate(newTemplate, currentTemplate); + expect(differences.resources.differenceCount).toBe(1); +}); + describe('changeset', () => { test('changeset overrides spec replacements', () => { // GIVEN @@ -1016,4 +1051,36 @@ describe('changeset', () => { // THEN expect(differences.differenceCount).toBe(0); }); + + test('metadata changes are obscured from the diff', () => { + // GIVEN + const currentTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + BucketName: 'magic-bucket', + Metadata: { + 'aws:cdk:path': '/foo/BucketResource', + }, + }, + }, + }; + + // WHEN + const newTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + BucketName: 'magic-bucket', + Metadata: { + 'aws:cdk:path': '/bar/BucketResource', + }, + }, + }, + }; + + // THEN + let differences = diffTemplate(currentTemplate, newTemplate, {}); + expect(differences.differenceCount).toBe(0); + }); }); diff --git a/packages/aws-cdk/lib/api/util/template-body-parameter.ts b/packages/aws-cdk/lib/api/util/template-body-parameter.ts index 02f1d8c81dfb9..cc8381ac2d44a 100644 --- a/packages/aws-cdk/lib/api/util/template-body-parameter.ts +++ b/packages/aws-cdk/lib/api/util/template-body-parameter.ts @@ -143,4 +143,4 @@ function restUrlFromManifest(url: string, environment: cxapi.Environment, sdk: I const urlSuffix: string = sdk.getEndpointSuffix(environment.region); return `https://s3.${environment.region}.${urlSuffix}/${bucketName}/${objectKey}`; -} \ No newline at end of file +}