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..a7cc6d703de2e 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 changes(): { [logicalId: string]: T } { + return onlyChanges(this.diffs); + } + + public get differenceCount(): number { + return Object.values(this.changes).length; + } - public get count(): number { - return this.logicalIds.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,32 @@ export class ResourceDifference extends Difference { return ResourceImpact.WILL_REPLACE; } - return Object.values(this.propertyUpdates) + // Base impact (before we mix in the worst of the property impacts); + // WILL_UPDATE if we have "other" changes, NO_CHANGE if there are no "other" changes. + const baseImpact = Object.keys(this.otherChanges).length > 0 ? ResourceImpact.WILL_UPDATE : ResourceImpact.NO_CHANGE; + + return Object.values(this.propertyDiffs) .map(elt => elt.changeImpact) - .reduce(worstImpact, ResourceImpact.WILL_UPDATE); + .reduce(worstImpact, baseImpact); } - 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 +643,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-table.ts b/packages/@aws-cdk/cloudformation-diff/lib/format-table.ts new file mode 100644 index 0000000000000..cd9a05c4b478f --- /dev/null +++ b/packages/@aws-cdk/cloudformation-diff/lib/format-table.ts @@ -0,0 +1,111 @@ +import colors = require('colors/safe'); +import stringWidth = require('string-width'); +import table = require('table'); + +/** + * Render a two-dimensional array to a visually attractive table + * + * First row is considered the table header. + */ +export function formatTable(cells: string[][], columns: number | undefined): string { + return table.table(cells, { + border: TABLE_BORDER_CHARACTERS, + columns: buildColumnConfig(columns !== undefined ? calculcateColumnWidths(cells, columns) : undefined), + drawHorizontalLine: (line) => { + // Numbering like this: [line 0] [header = row[0]] [line 1] [row 1] [line 2] [content 2] [line 3] + return (line < 2 || line === cells.length) || lineBetween(cells[line - 1], cells[line]); + } + }).trimRight(); +} + +/** + * Whether we should draw a line between two rows + * + * Draw horizontal line if 2nd column values are different. + */ +function lineBetween(rowA: string[], rowB: string[]) { + return rowA[1] !== rowB[1]; +} + +function buildColumnConfig(widths: number[] | undefined): { [index: number]: table.ColumnConfig } | undefined { + if (widths === undefined) { return undefined; } + + const ret: { [index: number]: table.ColumnConfig } = {}; + widths.forEach((width, i) => { + ret[i] = { width, useWordWrap: true } as any; // 'useWordWrap' is not in @types/table + if (width === undefined) { + delete ret[i].width; + } + }); + + return ret; +} + +/** + * Calculate column widths given a terminal width + * + * We do this by calculating a fair share for every column. Extra width smaller + * than the fair share is evenly distributed over all columns that exceed their + * fair share. + */ +function calculcateColumnWidths(rows: string[][], terminalWidth: number): number[] { + // use 'string-width' to not count ANSI chars as actual character width + const columns = rows[0].map((_, i) => Math.max(...rows.map(row => stringWidth(row[i])))); + + // If we have no terminal width, do nothing + const contentWidth = terminalWidth - 2 - columns.length * 3; + + // If we don't exceed the terminal width, do nothing + if (sum(columns) <= contentWidth) { return columns; } + + const fairShare = Math.min(contentWidth / columns.length); + const smallColumns = columns.filter(w => w < fairShare); + + let distributableWidth = contentWidth - sum(smallColumns); + const fairDistributable = Math.floor(distributableWidth / (columns.length - smallColumns.length)); + + const ret = new Array(); + for (const requestedWidth of columns) { + if (requestedWidth < fairShare) { + // Small column gets what they want + ret.push(requestedWidth); + } else { + // Last column gets all remaining, otherwise get fair redist share + const width = distributableWidth < 2 * fairDistributable ? distributableWidth : fairDistributable; + ret.push(width); + distributableWidth -= width; + } + } + + return ret; +} + +function sum(xs: number[]): number { + let total = 0; + for (const x of xs) { + total += x; + } + return total; +} + +// What color the table is going to be +const tableColor = colors.gray; + +// Unicode table characters with a color +const TABLE_BORDER_CHARACTERS = { + topBody: tableColor('─'), + topJoin: tableColor('┬'), + topLeft: tableColor('┌'), + topRight: tableColor('┐'), + bottomBody: tableColor('─'), + bottomJoin: tableColor('┴'), + bottomLeft: tableColor('└'), + bottomRight: tableColor('┘'), + bodyLeft: tableColor('│'), + bodyRight: tableColor('│'), + bodyJoin: tableColor('│'), + joinBody: tableColor('─'), + joinLeft: tableColor('├'), + joinRight: tableColor('┤'), + joinJoin: tableColor('┼') +}; \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-diff/lib/format.ts b/packages/@aws-cdk/cloudformation-diff/lib/format.ts index c808dbc822497..fcf6c35a2b162 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/format.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/format.ts @@ -1,10 +1,10 @@ import cxapi = require('@aws-cdk/cx-api'); -import Table = require('cli-table'); import colors = require('colors/safe'); import { format } from 'util'; import { Difference, isPropertyDifference, ResourceDifference, ResourceImpact } from './diff-template'; import { DifferenceCollection, TemplateDiff } from './diff/types'; import { deepEqual } from './diff/util'; +import { formatTable } from './format-table'; import { IamChanges } from './iam/iam-changes'; import { SecurityGroupChanges } from './network/security-group-changes'; @@ -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); }); } } @@ -186,13 +189,14 @@ class Formatter { case ResourceImpact.MAY_REPLACE: return colors.italic(colors.yellow('may be replaced')); case ResourceImpact.WILL_REPLACE: - return colors.italic(colors.bold(colors.yellow('replace'))); + return colors.italic(colors.bold(colors.red('replace'))); case ResourceImpact.WILL_DESTROY: return colors.italic(colors.bold(colors.red('destroy'))); case ResourceImpact.WILL_ORPHAN: 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 } } @@ -352,12 +356,12 @@ class Formatter { if (changes.statements.hasChanges) { this.printSectionHeader('IAM Statement Changes'); - this.print(renderTable(this.deepSubstituteBracedLogicalIds(changes.summarizeStatements()))); + this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarizeStatements()), this.stream.columns)); } if (changes.managedPolicies.hasChanges) { this.printSectionHeader('IAM Policy Changes'); - this.print(renderTable(this.deepSubstituteBracedLogicalIds(changes.summarizeManagedPolicies()))); + this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarizeManagedPolicies()), this.stream.columns)); } } @@ -365,7 +369,7 @@ class Formatter { if (!changes.hasChanges) { return; } this.printSectionHeader('Security Group Changes'); - this.print(renderTable(this.deepSubstituteBracedLogicalIds(changes.summarize()))); + this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarize()), this.stream.columns)); } public deepSubstituteBracedLogicalIds(rows: string[][]): string[][] { @@ -382,47 +386,6 @@ class Formatter { } } -/** - * Render a two-dimensional array to a visually attractive table - * - * First row is considered the table header. - */ -function renderTable(cells: string[][]): string { - const head = cells.splice(0, 1)[0]; - - const table = new Table({ head, style: { head: [] } }); - table.push(...cells); - return stripHorizontalLines(table.toString()).trimRight(); -} - -/** - * Strip horizontal lines in the table rendering if the second-column values are the same - * - * We couldn't find a table library that BOTH does newlines-in-cells correctly AND - * has an option to enable/disable separator lines on a per-row basis. So we're - * going to do some character post-processing on the table instead. - */ -function stripHorizontalLines(tableRendering: string) { - const lines = tableRendering.split('\n'); - - let i = 3; - while (i < lines.length - 3) { - if (secondColumnValue(lines[i]) === secondColumnValue(lines[i + 2])) { - lines.splice(i + 1, 1); - i += 1; - } else { - i += 2; - } - } - - return lines.join('\n'); - - function secondColumnValue(line: string) { - const cols = colors.stripColors(line).split('│').filter(x => x !== ''); - return cols[1]; - } -} - /** * A patch as returned by ``diff.structuredPatch``. */ diff --git a/packages/@aws-cdk/cloudformation-diff/lib/index.ts b/packages/@aws-cdk/cloudformation-diff/lib/index.ts index 44f7a1f8cea49..34a07c7559fb7 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/index.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/index.ts @@ -1,3 +1,4 @@ export * from './diff-template'; export * from './format'; +export * from './format-table'; export { deepEqual } from './diff/util'; diff --git a/packages/@aws-cdk/cloudformation-diff/package.json b/packages/@aws-cdk/cloudformation-diff/package.json index 5c1c1454d720b..c3789f258552d 100644 --- a/packages/@aws-cdk/cloudformation-diff/package.json +++ b/packages/@aws-cdk/cloudformation-diff/package.json @@ -25,14 +25,16 @@ "dependencies": { "@aws-cdk/cfnspec": "^0.22.0", "@aws-cdk/cx-api": "^0.22.0", - "cli-table": "^0.3.1", + "string-width": "^2.1.1", + "table": "^5.2.1", "colors": "^1.2.1", "diff": "^4.0.1", "fast-deep-equal": "^2.0.1", "source-map-support": "^0.5.6" }, "devDependencies": { - "@types/cli-table": "^0.3.0", + "@types/table": "^4.0.5", + "@types/string-width": "^2.0.0", "cdk-build-tools": "^0.22.0", "fast-check": "^1.8.0", "pkglint": "^0.22.0" 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/integ-tests/test-cdk-ls.sh b/packages/aws-cdk/integ-tests/test-cdk-ls.sh index 9958052b99e74..f219eb2f42f90 100755 --- a/packages/aws-cdk/integ-tests/test-cdk-ls.sh +++ b/packages/aws-cdk/integ-tests/test-cdk-ls.sh @@ -7,9 +7,11 @@ source ${scriptdir}/common.bash setup assert "cdk ls" < { function listContext(context: any) { const keys = contextKeys(context); + if (keys.length === 0) { + print(`This CDK application does not have any saved context values yet.`); + print(''); + print(`Context will automatically be saved when you synthesize CDK apps`); + print(`that use environment context information like AZ information, VPCs,`); + print(`SSM parameters, and so on.`); + + return; + } + // Print config by default const data: any[] = [[colors.green('#'), colors.green('Key'), colors.green('Value')]]; for (const [i, key] of keys) { @@ -61,7 +71,7 @@ function listContext(context: any) { print(`Context found in ${colors.blue(DEFAULTS)}:\n`); - print(renderTable(data, { colWidths: [2, 50, 50] })); + print(renderTable(data, process.stdout.columns)); // tslint:disable-next-line:max-line-length print(`Run ${colors.blue('cdk context --reset KEY_OR_NUMBER')} to remove a context key. It will be refreshed on the next CDK synthesis run.`); diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts index 5a56a3fc6b1a0..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 { @@ -61,7 +61,7 @@ export function printSecurityDiff(oldTemplate: any, newTemplate: cxapi.Synthesiz warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${requireApproval}).`); warning(`Please confirm you intend to make the following modifications:\n`); - cfnDiff.formatSecurityChanges(process.stderr, diff, buildLogicalToPathMap(newTemplate)); + cfnDiff.formatSecurityChanges(process.stdout, diff, buildLogicalToPathMap(newTemplate)); return true; } return false; diff --git a/packages/aws-cdk/lib/util/tables.ts b/packages/aws-cdk/lib/util/tables.ts index 56a42f13fb105..bfc059bef40fc 100644 --- a/packages/aws-cdk/lib/util/tables.ts +++ b/packages/aws-cdk/lib/util/tables.ts @@ -1,13 +1,7 @@ -import Table = require('cli-table'); +import cfnDiff = require('@aws-cdk/cloudformation-diff'); -export interface RenderTableOptions { - colWidths?: number[]; -} - -export function renderTable(cells: string[][], options: RenderTableOptions = {}): string { - const head = cells.splice(0, 1)[0]; - - const table = new Table({ head, style: { head: [] }, colWidths: options.colWidths }); - table.push(...cells); - return table.toString(); -} +export function renderTable(cells: string[][], columns?: number) { + // The cfnDiff module has logic for terminal-width aware table + // formatting (and nice colors), let's just reuse that. + return cfnDiff.formatTable(cells, columns); +} \ No newline at end of file diff --git a/packages/aws-cdk/package.json b/packages/aws-cdk/package.json index d88f4e79ca779..f67843cc6c278 100644 --- a/packages/aws-cdk/package.json +++ b/packages/aws-cdk/package.json @@ -32,11 +32,11 @@ "license": "Apache-2.0", "devDependencies": { "@types/archiver": "^2.1.2", - "@types/cli-table": "^0.3.0", "@types/fs-extra": "^5.0.4", "@types/minimatch": "^3.0.3", "@types/mockery": "^1.4.29", "@types/request": "^2.47.1", + "@types/table": "^4.0.5", "@types/semver": "^5.5.0", "@types/uuid": "^3.4.3", "@types/yaml": "^1.0.0", @@ -52,7 +52,6 @@ "archiver": "^2.1.1", "aws-sdk": "^2.259.1", "camelcase": "^5.0.0", - "cli-table": "^0.3.1", "colors": "^1.2.1", "decamelize": "^2.0.0", "fs-extra": "^7.0.0", @@ -63,7 +62,7 @@ "request": "^2.83.0", "semver": "^5.5.0", "source-map-support": "^0.5.6", - "table": "^5.1.0", + "table": "^5.2.1", "yaml": "^1.1.0", "yargs": "^9.0.1" },