Skip to content

Commit

Permalink
remove metadata, but only when the flag is passed
Browse files Browse the repository at this point in the history
  • Loading branch information
comcalvi committed Dec 13, 2023
1 parent da00e1a commit 212f603
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 33 deletions.
23 changes: 12 additions & 11 deletions packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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)),
};
Expand All @@ -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);
Expand Down Expand Up @@ -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<any> } = {};
Expand All @@ -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);
Expand Down Expand Up @@ -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';
Expand All @@ -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;
}
}
}
Expand Down
13 changes: 8 additions & 5 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts
Original file line number Diff line number Diff line change
@@ -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<string> {
return new types.Difference<string>(_asString(oldValue), _asString(newValue));
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,15 @@ export function isPropertyDifference<T>(diff: Difference<T>): diff is PropertyDi
return (diff as PropertyDifference<T>).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
*/
Expand Down
10 changes: 6 additions & 4 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -177,8 +177,10 @@ function yamlIntrinsicEqual(lvalue: any, rvalue: any): boolean {
export function diffKeyedEntities<T>(
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];
Expand All @@ -189,7 +191,7 @@ export function diffKeyedEntities<T>(
continue;
}

result[logicalId] = elementDiff(oldElement, newElement, logicalId, replacements ? replacements[logicalId]: undefined);
result[logicalId] = elementDiff(oldElement, newElement, logicalId, replacements ? replacements[logicalId]: undefined, keepMetadata);
}
return result;
}
Expand Down
12 changes: 0 additions & 12 deletions packages/@aws-cdk/cloudformation-diff/lib/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
67 changes: 67 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
});
});
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/api/util/template-body-parameter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
}
}

0 comments on commit 212f603

Please sign in to comment.