From ac1e63e9b44f380b6389b2321bd39e8fe0301ce5 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 15 Nov 2018 23:09:54 +0200 Subject: [PATCH 1/2] feat(toolkit): by default hide AWS::CDK::Metadata from "cdk diff" `DifferenceCollection#applyFilter` can be used to delete any changes that do not pass the predicate from the collection. Changed `DifferenceCollection#count` to do a lazy calculation since now `changes` is mutable. The toolkit switch `cdk diff --strict` disables this behavior. Fixes #465 --- .../cloudformation-diff/lib/diff/types.ts | 59 ++++++++++++------- packages/aws-cdk/bin/cdk.ts | 9 +-- packages/aws-cdk/lib/diff.ts | 13 +++- 3 files changed, 56 insertions(+), 25 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts index f921531e39919..4503b83c27905 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -15,45 +15,48 @@ export class TemplateDiff implements ITemplateDiff { /** The differences in unknown/unexpected parts of the template */ public readonly unknown: DifferenceCollection>; - public readonly count: number; - constructor(args: ITemplateDiff) { - let count = 0; if (args.awsTemplateFormatVersion !== undefined) { this.awsTemplateFormatVersion = args.awsTemplateFormatVersion; - count += 1; } if (args.description !== undefined) { this.description = args.description; - count += 1; } if (args.transform !== undefined) { this.transform = args.transform; - count += 1; } this.conditions = args.conditions || new DifferenceCollection({}); - count += this.conditions.count; - this.mappings = args.mappings || new DifferenceCollection({}); - count += this.mappings.count; - this.metadata = args.metadata || new DifferenceCollection({}); - count += this.metadata.count; - this.outputs = args.outputs || new DifferenceCollection({}); - count += this.outputs.count; - this.parameters = args.parameters || new DifferenceCollection({}); - count += this.parameters.count; - this.resources = args.resources || new DifferenceCollection({}); - count += this.resources.count; - this.unknown = args.unknown || new DifferenceCollection({}); + } + + public get count() { + let count = 0; + + if (this.awsTemplateFormatVersion !== undefined) { + count += 1; + } + if (this.description !== undefined) { + count += 1; + } + if (this.transform !== undefined) { + 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; - this.count = count; + return count; } public get isEmpty(): boolean { @@ -107,7 +110,7 @@ export class PropertyDifference extends Difference { } export class DifferenceCollection> { - constructor(public readonly changes: { [logicalId: string]: T | undefined }) {} + constructor(public changes: { [logicalId: string]: T | undefined }) {} public get count(): number { return this.logicalIds.length; @@ -117,6 +120,22 @@ export class DifferenceCollection> { return Object.keys(this.changes); } + /** + * Removes all changes that do not match the specified filter. + */ + public applyFilter(filter: (diff: T | undefined) => boolean) { + const newChanges: { [logicalId: string]: T | undefined } = { }; + for (const id of Object.keys(this.changes)) { + const diff = this.changes[id]; + + if (filter(diff)) { + newChanges[id] = diff; + } + } + + this.changes = newChanges; + } + public forEach(cb: (logicalId: string, change: T) => any): void { for (const logicalId of this.logicalIds) { cb(logicalId, this.changes[logicalId]!); diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 17b9b3f27e21d..81bd691a7e632 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -65,7 +65,8 @@ async function parseCommandLineArguments() { .command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', yargs => yargs .option('force', { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' })) .command('diff [STACK]', 'Compares the specified stack with the deployed stack or a local template file', yargs => yargs - .option('template', { type: 'string', desc: 'the path to the CloudFormation template to compare with' })) + .option('template', { type: 'string', desc: 'the path to the CloudFormation template to compare with' }) + .option('strict', { type: 'boolean', desc: 'do not filter out AWS::CDK::Metadata resources', default: false })) .command('metadata [STACK]', 'Returns all metadata associated with this stack') .command('init [TEMPLATE]', 'Create a new, empty CDK project from a template. Invoked without TEMPLATE, the app template will be used.', yargs => yargs .option('language', { type: 'string', alias: 'l', desc: 'the language to be used for the new project (default can be configured in ~/.cdk.json)', choices: initTemplateLanuages }) @@ -200,7 +201,7 @@ async function initCommandLine() { return await cliList({ long: args.long }); case 'diff': - return await diffStack(await findStack(args.STACK), args.template); + return await diffStack(await findStack(args.STACK), args.template, args.strict); case 'bootstrap': return await cliBootstrap(args.ENVIRONMENTS, toolkitStackName, args.roleArn); @@ -592,10 +593,10 @@ async function initCommandLine() { } } - async function diffStack(stackName: string, templatePath?: string): Promise { + async function diffStack(stackName: string, templatePath: string | undefined, strict: boolean): Promise { const stack = await synthesizeStack(stackName); const currentTemplate = await readCurrentTemplate(stack, templatePath); - if (printStackDiff(currentTemplate, stack) === 0) { + if (printStackDiff(currentTemplate, stack, strict) === 0) { return 0; } else { return 1; diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts index b952790916f7f..d802bd0328167 100644 --- a/packages/aws-cdk/lib/diff.ts +++ b/packages/aws-cdk/lib/diff.ts @@ -11,8 +11,19 @@ import { print } from './logging'; * * @returns the count of differences that were rendered. */ -export function printStackDiff(oldTemplate: any, newTemplate: cxapi.SynthesizedStack): number { +export function printStackDiff(oldTemplate: any, newTemplate: cxapi.SynthesizedStack, strict: boolean): number { const diff = cfnDiff.diffTemplate(oldTemplate, newTemplate.template); + + // filter out 'AWS::CDK::Metadata' resources from the template + if (diff.resources && !strict) { + diff.resources.applyFilter(change => { + if (!change) { return true; } + if (change.newResourceType === 'AWS::CDK::Metadata') { return false; } + if (change.oldResourceType === 'AWS::CDK::Metadata') { return false; } + return true; + }); + } + if (!diff.isEmpty) { cfnDiff.formatDifferences(process.stderr, diff); } else { From 9e05b8fe3c1560b5ed472d1b1b8ae8960939ee13 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 18 Nov 2018 13:07:26 +0200 Subject: [PATCH 2/2] Revert DifferenceCollection to immutable (applyFilter => filter) --- .../cloudformation-diff/lib/diff/types.ts | 31 ++++++++++--------- packages/aws-cdk/lib/diff.ts | 12 +++---- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts index 4503b83c27905..1a6c54f8ac6d9 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -3,17 +3,17 @@ import { deepEqual } from './util'; /** Semantic differences between two CloudFormation templates. */ export class TemplateDiff implements ITemplateDiff { - public readonly awsTemplateFormatVersion?: Difference; - public readonly description?: Difference; - public readonly transform?: Difference; - public readonly conditions: DifferenceCollection; - public readonly mappings: DifferenceCollection; - public readonly metadata: DifferenceCollection; - public readonly outputs: DifferenceCollection; - public readonly parameters: DifferenceCollection; - public readonly resources: DifferenceCollection; + public awsTemplateFormatVersion?: Difference; + public description?: Difference; + public transform?: Difference; + public conditions: DifferenceCollection; + public mappings: DifferenceCollection; + public metadata: DifferenceCollection; + public outputs: DifferenceCollection; + public parameters: DifferenceCollection; + public resources: DifferenceCollection; /** The differences in unknown/unexpected parts of the template */ - public readonly unknown: DifferenceCollection>; + public unknown: DifferenceCollection>; constructor(args: ITemplateDiff) { if (args.awsTemplateFormatVersion !== undefined) { @@ -110,7 +110,7 @@ export class PropertyDifference extends Difference { } export class DifferenceCollection> { - constructor(public changes: { [logicalId: string]: T | undefined }) {} + constructor(public readonly changes: { [logicalId: string]: T | undefined }) {} public get count(): number { return this.logicalIds.length; @@ -121,19 +121,20 @@ export class DifferenceCollection> { } /** - * Removes all changes that do not match the specified filter. + * Returns a new TemplateDiff which only contains changes for which `predicate` + * returns `true`. */ - public applyFilter(filter: (diff: T | undefined) => boolean) { + public filter(predicate: (diff: T | undefined) => boolean): DifferenceCollection { const newChanges: { [logicalId: string]: T | undefined } = { }; for (const id of Object.keys(this.changes)) { const diff = this.changes[id]; - if (filter(diff)) { + if (predicate(diff)) { newChanges[id] = diff; } } - this.changes = newChanges; + return new DifferenceCollection(newChanges); } public forEach(cb: (logicalId: string, change: T) => any): void { diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts index d802bd0328167..bf823da2f97b4 100644 --- a/packages/aws-cdk/lib/diff.ts +++ b/packages/aws-cdk/lib/diff.ts @@ -16,12 +16,12 @@ export function printStackDiff(oldTemplate: any, newTemplate: cxapi.SynthesizedS // filter out 'AWS::CDK::Metadata' resources from the template if (diff.resources && !strict) { - diff.resources.applyFilter(change => { - if (!change) { return true; } - if (change.newResourceType === 'AWS::CDK::Metadata') { return false; } - if (change.oldResourceType === 'AWS::CDK::Metadata') { return false; } - return true; - }); + diff.resources = diff.resources.filter(change => { + if (!change) { return true; } + if (change.newResourceType === 'AWS::CDK::Metadata') { return false; } + if (change.oldResourceType === 'AWS::CDK::Metadata') { return false; } + return true; + }); } if (!diff.isEmpty) {