diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 6e06e56f90af1..055bc3be8248b 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -1,3 +1,7 @@ +// The SDK is only used to reference `DescribeChangeSetOutput`, so the SDK is added as a devDependency. +// The SDK should not make network calls here +// eslint-disable-next-line import/no-extraneous-dependencies +import { CloudFormation } from 'aws-sdk'; import * as impl from './diff'; import * as types from './diff/types'; import { deepEqual, diffKeyedEntities, unionOf } from './diff/util'; @@ -33,12 +37,33 @@ const DIFF_HANDLERS: HandlerRegistry = { * * @param currentTemplate the current state of the stack. * @param newTemplate the target state of the stack. + * @param changeSet the change set for this stack. * * @returns a +types.TemplateDiff+ object that represents the changes that will happen if * a stack which current state is described by +currentTemplate+ is updated with * the template +newTemplate+. */ -export function diffTemplate(currentTemplate: { [key: string]: any }, newTemplate: { [key: string]: any }): types.TemplateDiff { +export function fullDiff( + currentTemplate: { [key: string]: any }, + newTemplate: { [key: string]: any }, + changeSet?: CloudFormation.DescribeChangeSetOutput, +): types.TemplateDiff { + + normalize(currentTemplate); + normalize(newTemplate); + const theDiff = diffTemplate(currentTemplate, newTemplate); + if (changeSet) { + filterFalsePositivies(theDiff, changeSet); + } + + return theDiff; +} + +function diffTemplate( + currentTemplate: { [key: string]: any }, + newTemplate: { [key: string]: any }, +): types.TemplateDiff { + // Base diff const theDiff = calculateTemplateDiff(currentTemplate, newTemplate); @@ -105,7 +130,6 @@ function calculateTemplateDiff(currentTemplate: { [key: string]: any }, newTempl 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); @@ -184,3 +208,93 @@ function deepCopy(x: any): any { return x; } + +function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) { + const replacements = findResourceReplacements(changeSet); + diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => { + change.forEachDifference((type: 'Property' | 'Other', name: string, value: types.Difference | types.PropertyDifference) => { + if (type === 'Property') { + if (!replacements[logicalId]) { + (value as types.PropertyDifference).changeImpact = types.ResourceImpact.NO_CHANGE; + (value as types.PropertyDifference).isDifferent = false; + return; + } + switch (replacements[logicalId].propertiesReplaced[name]) { + case 'Always': + (value as types.PropertyDifference).changeImpact = types.ResourceImpact.WILL_REPLACE; + break; + case 'Never': + (value as types.PropertyDifference).changeImpact = types.ResourceImpact.WILL_UPDATE; + break; + case 'Conditionally': + (value as types.PropertyDifference).changeImpact = types.ResourceImpact.MAY_REPLACE; + break; + case undefined: + (value as types.PropertyDifference).changeImpact = types.ResourceImpact.NO_CHANGE; + (value as types.PropertyDifference).isDifferent = false; + break; + // otherwise, defer to the changeImpact from `diffTemplate` + } + } else if (type === 'Other') { + switch (name) { + case 'Metadata': + change.setOtherChange('Metadata', new types.Difference(value.newValue, value.newValue)); + break; + } + } + }); + }); +} + +function findResourceReplacements(changeSet: CloudFormation.DescribeChangeSetOutput): types.ResourceReplacements { + const replacements: types.ResourceReplacements = {}; + for (const resourceChange of changeSet.Changes ?? []) { + const propertiesReplaced: { [propName: string]: types.ChangeSetReplacement } = {}; + for (const propertyChange of resourceChange.ResourceChange?.Details ?? []) { + if (propertyChange.Target?.Attribute === 'Properties') { + const requiresReplacement = propertyChange.Target.RequiresRecreation === 'Always'; + if (requiresReplacement && propertyChange.Evaluation === 'Static') { + propertiesReplaced[propertyChange.Target.Name!] = 'Always'; + } else if (requiresReplacement && propertyChange.Evaluation === 'Dynamic') { + // If Evaluation is 'Dynamic', then this may cause replacement, or it may not. + // 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 types.ChangeSetReplacement; + } + } + } + replacements[resourceChange.ResourceChange?.LogicalResourceId!] = { + resourceReplaced: resourceChange.ResourceChange?.Replacement === 'True', + propertiesReplaced, + }; + } + + return replacements; +} + +function normalize(template: any) { + if (typeof template === 'object') { + for (const key of (Object.keys(template ?? {}))) { + if (key === 'Fn::GetAtt' && typeof template[key] === 'string') { + template[key] = template[key].split('.'); + continue; + } else if (key === 'DependsOn') { + if (typeof template[key] === 'string') { + template[key] = [template[key]]; + } else if (Array.isArray(template[key])) { + template[key] = template[key].sort(); + } + continue; + } + + if (Array.isArray(template[key])) { + for (const element of (template[key])) { + normalize(element); + } + } else { + normalize(template[key]); + } + } + } +} diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts index 7bd91b58f6ee4..e85265bf99c1f 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -6,6 +6,15 @@ import { SecurityGroupChanges } from '../network/security-group-changes'; export type PropertyMap = {[key: string]: any }; +export type ResourceReplacements = { [logicalId: string]: ResourceReplacement }; + +export interface ResourceReplacement { + resourceReplaced: boolean, + propertiesReplaced: { [propertyName: string]: ChangeSetReplacement }; +} + +export type ChangeSetReplacement = 'Always' | 'Never' | 'Conditionally'; + /** Semantic differences between two CloudFormation templates. */ export class TemplateDiff implements ITemplateDiff { public awsTemplateFormatVersion?: Difference; @@ -296,7 +305,7 @@ export class Difference implements IDifference { * * isDifferent => (isUpdate | isRemoved | isUpdate) */ - public readonly isDifferent: boolean; + public isDifferent: boolean; /** * @param oldValue the old value, cannot be equal (to the sense of +deepEqual+) to +newValue+. @@ -327,7 +336,7 @@ export class Difference implements IDifference { } export class PropertyDifference extends Difference { - public readonly changeImpact?: ResourceImpact; + public changeImpact?: ResourceImpact; constructor(oldValue: ValueType | undefined, newValue: ValueType | undefined, args: { changeImpact?: ResourceImpact }) { super(oldValue, newValue); @@ -352,6 +361,10 @@ export class DifferenceCollection> { return ret; } + public remove(logicalId: string): void { + delete this.diffs[logicalId]; + } + public get logicalIds(): string[] { return Object.keys(this.changes); } @@ -621,6 +634,18 @@ export class ResourceDifference implements IDifference { this.propertyDiffs[propertyName] = change; } + /** + * Replace a OtherChange 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 setOtherChange(otherName: string, change: PropertyDifference) { + this.otherDiffs[otherName] = change; + } + public get changeImpact(): ResourceImpact { // Check the Type first if (this.resourceTypes.oldType !== this.resourceTypes.newType) { diff --git a/packages/@aws-cdk/cloudformation-diff/package.json b/packages/@aws-cdk/cloudformation-diff/package.json index 0afbb125805c8..2e502090932d3 100644 --- a/packages/@aws-cdk/cloudformation-diff/package.json +++ b/packages/@aws-cdk/cloudformation-diff/package.json @@ -38,6 +38,7 @@ "@types/string-width": "^4.0.1", "fast-check": "^3.14.0", "jest": "^29.7.0", + "aws-sdk": "2.1516.0", "ts-jest": "^29.1.1" }, "repository": { 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 5c755339fd737..b211c1d17c085 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts @@ -1,6 +1,6 @@ import * as fc from 'fast-check'; import { arbitraryTemplate } from './test-arbitraries'; -import { diffTemplate, ResourceImpact } from '../lib/diff-template'; +import { fullDiff, ResourceImpact } from '../lib/diff-template'; const POLICY_DOCUMENT = { foo: 'Bar' }; // Obviously a fake one! const BUCKET_POLICY_RESOURCE = { @@ -27,7 +27,7 @@ test('when there is no difference', () => { // Making a JSON-clone, because === is cheating! const newTemplate = JSON.parse(JSON.stringify(currentTemplate)); - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(0); }); @@ -36,7 +36,7 @@ test('when a resource is created', () => { const newTemplate = { Resources: { BucketResource: { Type: 'AWS::S3::Bucket' } } }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(1); expect(differences.resources.differenceCount).toBe(1); const difference = differences.resources.changes.BucketResource; @@ -60,7 +60,7 @@ test('when a resource is deleted (no DeletionPolicy)', () => { }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(1); expect(differences.resources.differenceCount).toBe(1); const difference = differences.resources.changes.BucketPolicyResource; @@ -89,7 +89,7 @@ test('when a resource is deleted (DeletionPolicy=Retain)', () => { Resources: { BucketResource: { Type: 'AWS::S3::Bucket' } }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(1); expect(differences.resources.differenceCount).toBe(1); const difference = differences.resources.changes.BucketPolicyResource; @@ -130,7 +130,7 @@ test('when a property changes', () => { }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(1); expect(differences.resources.differenceCount).toBe(1); const difference = differences.resources.changes.BucketResource; @@ -161,7 +161,7 @@ test('change in dependencies counts as a simple update', () => { }, }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); // THEN expect(differences.differenceCount).toBe(1); @@ -196,7 +196,7 @@ test('when a property is deleted', () => { }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(1); expect(differences.resources.differenceCount).toBe(1); const difference = differences.resources.changes.BucketResource; @@ -234,7 +234,7 @@ test('when a property is added', () => { }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(1); expect(differences.resources.differenceCount).toBe(1); const difference = differences.resources.changes.BucketResource; @@ -268,7 +268,7 @@ test('when a resource type changed', () => { }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(1); expect(differences.resources.differenceCount).toBe(1); const difference = differences.resources.changes.BucketResource; @@ -318,7 +318,7 @@ test('resource replacement is tracked through references', () => { }, }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); // THEN expect(differences.resources.differenceCount).toBe(3); @@ -370,10 +370,10 @@ test('adding and removing quotes from a numeric property causes no changes', () }, }, }; - let differences = diffTemplate(currentTemplate, newTemplate); + let differences = fullDiff(currentTemplate, newTemplate); expect(differences.resources.differenceCount).toBe(0); - differences = diffTemplate(newTemplate, currentTemplate); + differences = fullDiff(newTemplate, currentTemplate); expect(differences.resources.differenceCount).toBe(0); }); @@ -401,122 +401,9 @@ test('versions are correctly detected as not numbers', () => { }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.resources.differenceCount).toBe(1); }); - -test('single element arrays are equivalent to the single element in DependsOn expressions', () => { - // GIVEN - const currentTemplate = { - Resources: { - BucketResource: { - Type: 'AWS::S3::Bucket', - DependsOn: ['SomeResource'], - }, - }, - }; - - // WHEN - const newTemplate = { - Resources: { - BucketResource: { - Type: 'AWS::S3::Bucket', - DependsOn: 'SomeResource', - }, - }, - }; - - let differences = diffTemplate(currentTemplate, newTemplate); - expect(differences.resources.differenceCount).toBe(0); - - differences = diffTemplate(newTemplate, currentTemplate); - expect(differences.resources.differenceCount).toBe(0); -}); - -test('array equivalence is independent of element order in DependsOn expressions', () => { - // GIVEN - const currentTemplate = { - Resources: { - BucketResource: { - Type: 'AWS::S3::Bucket', - DependsOn: ['SomeResource', 'AnotherResource'], - }, - }, - }; - - // WHEN - const newTemplate = { - Resources: { - BucketResource: { - Type: 'AWS::S3::Bucket', - DependsOn: ['AnotherResource', 'SomeResource'], - }, - }, - }; - - let differences = diffTemplate(currentTemplate, newTemplate); - expect(differences.resources.differenceCount).toBe(0); - - differences = diffTemplate(newTemplate, currentTemplate); - expect(differences.resources.differenceCount).toBe(0); -}); - -test('arrays of different length are considered unequal in DependsOn expressions', () => { - // GIVEN - const currentTemplate = { - Resources: { - BucketResource: { - Type: 'AWS::S3::Bucket', - DependsOn: ['SomeResource', 'AnotherResource', 'LastResource'], - }, - }, - }; - - // WHEN - const newTemplate = { - Resources: { - BucketResource: { - Type: 'AWS::S3::Bucket', - DependsOn: ['AnotherResource', 'SomeResource'], - }, - }, - }; - - let differences = diffTemplate(currentTemplate, newTemplate); - expect(differences.resources.differenceCount).toBe(1); - - differences = diffTemplate(newTemplate, currentTemplate); - expect(differences.resources.differenceCount).toBe(1); -}); - -test('arrays that differ only in element order are considered unequal outside of DependsOn expressions', () => { - // GIVEN - const currentTemplate = { - Resources: { - BucketResource: { - Type: 'AWS::S3::Bucket', - BucketName: { 'Fn::Select': [0, ['name1', 'name2']] }, - }, - }, - }; - - // WHEN - const newTemplate = { - Resources: { - BucketResource: { - Type: 'AWS::S3::Bucket', - BucketName: { 'Fn::Select': [0, ['name2', 'name1']] }, - }, - }, - }; - - let differences = diffTemplate(currentTemplate, newTemplate); - expect(differences.resources.differenceCount).toBe(1); - - differences = diffTemplate(newTemplate, currentTemplate); - expect(differences.resources.differenceCount).toBe(1); -}); - test('boolean properties are considered equal with their stringified counterparts', () => { // GIVEN const currentTemplate = { @@ -545,7 +432,7 @@ test('boolean properties are considered equal with their stringified counterpart }; // WHEN - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); // THEN expect(differences.differenceCount).toBe(0); @@ -577,10 +464,10 @@ test('when a property changes including equivalent DependsOn', () => { }; // THEN - let differences = diffTemplate(currentTemplate, newTemplate); + let differences = fullDiff(currentTemplate, newTemplate); expect(differences.resources.differenceCount).toBe(1); - differences = diffTemplate(newTemplate, currentTemplate); + differences = fullDiff(newTemplate, currentTemplate); expect(differences.resources.differenceCount).toBe(1); }); @@ -615,7 +502,7 @@ test.each([ }, }; // WHEN - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); // THEN expect(differences.differenceCount).toBe(1); @@ -651,7 +538,7 @@ test('when a property with a number-like format doesn\'t change', () => { }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(0); expect(differences.resources.differenceCount).toBe(0); const difference = differences.resources.changes.BucketResource; @@ -677,7 +564,7 @@ test('handles a resource changing its Type', () => { }, }; - const differences = diffTemplate(currentTemplate, newTemplate); + const differences = fullDiff(currentTemplate, newTemplate); expect(differences.differenceCount).toBe(1); expect(differences.resources.differenceCount).toBe(1); const difference = differences.resources.changes.FunctionApi; @@ -696,8 +583,538 @@ test('diffing any two arbitrary templates should not crash', () => { // We're not interested in making sure we find the right differences here -- just // that we're not crashing. fc.assert(fc.property(arbitraryTemplate, arbitraryTemplate, (t1, t2) => { - diffTemplate(t1, t2); + fullDiff(t1, t2); }), { // path: '1:0:0:0:3:0:1:1:1:1:1:1:1:1:1:1:1:1:1:2:1:1:1', }); }); + +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 = fullDiff(currentTemplate, newTemplate); + expect(differences.differenceCount).toBe(1); + + differences = fullDiff(newTemplate, currentTemplate); + expect(differences.resources.differenceCount).toBe(1); +}); + +describe('changeset', () => { + test('changeset overrides spec replacements', () => { + // GIVEN + const currentTemplate = { + Parameters: { + BucketName: { + Type: 'String', + }, + }, + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: 'Name1' }, // Immutable prop + }, + }, + }; + const newTemplate = { + Parameters: { + BucketName: { + Type: 'String', + }, + }, + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: { Ref: 'BucketName' } }, // No change + }, + }, + }; + + // WHEN + const differences = fullDiff(currentTemplate, newTemplate, { + Parameters: [ + { + ParameterKey: 'BucketName', + ParameterValue: 'Name1', + }, + ], + Changes: [], + }); + + // THEN + expect(differences.differenceCount).toBe(0); + }); + + test('changeset does not overrides spec additions or deletions', () => { + // GIVEN + const currentTemplate = { + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: 'MagicBucket' }, + }, + }, + }; + const newTemplate = { + Resources: { + Queue: { + Type: 'AWS::SQS::Queue', + Properties: { QueueName: 'MagicQueue' }, + }, + }, + }; + + // WHEN + const differences = fullDiff(currentTemplate, newTemplate, { + Changes: [ + { + ResourceChange: { + Action: 'Remove', + LogicalResourceId: 'Bucket', + ResourceType: 'AWS::S3::Bucket', + Details: [], + }, + }, + { + ResourceChange: { + Action: 'Add', + LogicalResourceId: 'Queue', + ResourceType: 'AWS::SQS::Queue', + Details: [], + }, + }, + ], + }); + + // A realistic changeset will include Additions and Removals, but this shows that we don't use the changeset to determine additions or removals + const emptyChangeSetDifferences = fullDiff(currentTemplate, newTemplate, { + Changes: [], + }); + + // THEN + expect(differences.differenceCount).toBe(2); + expect(emptyChangeSetDifferences.differenceCount).toBe(2); + }); + + test('changeset replacements are respected', () => { + // GIVEN + const currentTemplate = { + Parameters: { + BucketName: { + Type: 'String', + }, + }, + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: 'Name1' }, // Immutable prop + }, + }, + }; + const newTemplate = { + Parameters: { + BucketName: { + Type: 'String', + }, + }, + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: { Ref: 'BucketName' } }, // 'Name1' -> 'Name2' + }, + }, + }; + + // WHEN + const differences = fullDiff(currentTemplate, newTemplate, { + Parameters: [ + { + ParameterKey: 'BucketName', + ParameterValue: 'Name2', + }, + ], + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Modify', + LogicalResourceId: 'Bucket', + ResourceType: 'AWS::S3::Bucket', + Replacement: 'True', + Details: [ + { + Target: { + Attribute: 'Properties', + Name: 'BucketName', + RequiresRecreation: 'Always', + }, + Evaluation: 'Static', + ChangeSource: 'DirectModification', + }, + ], + }, + }, + ], + }); + + // THEN + expect(differences.differenceCount).toBe(1); + }); + + // This is directly in-line with changeset behavior, + // see 'Replacement': https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_ResourceChange.html + test('dynamic changeset replacements are considered conditional replacements', () => { + // GIVEN + const currentTemplate = { + Resources: { + Instance: { + Type: 'AWS::EC2::Instance', + Properties: { + ImageId: 'ami-79fd7eee', + KeyName: 'rsa-is-fun', + }, + }, + }, + }; + const newTemplate = { + Resources: { + Instance: { + Type: 'AWS::EC2::Instance', + Properties: { + ImageId: 'ami-79fd7eee', + KeyName: 'but-sha-is-cool', + }, + }, + }, + }; + + // WHEN + const differences = fullDiff(currentTemplate, newTemplate, { + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Modify', + LogicalResourceId: 'Instance', + ResourceType: 'AWS::EC2::Instance', + Replacement: 'Conditional', + Details: [ + { + Target: { + Attribute: 'Properties', + Name: 'KeyName', + RequiresRecreation: 'Always', + }, + Evaluation: 'Dynamic', + ChangeSource: 'DirectModification', + }, + ], + }, + }, + ], + }); + + // THEN + expect(differences.differenceCount).toBe(1); + expect(differences.resources.changes.Instance.changeImpact).toEqual(ResourceImpact.MAY_REPLACE); + expect(differences.resources.changes.Instance.propertyUpdates).toEqual({ + KeyName: { + changeImpact: ResourceImpact.MAY_REPLACE, + isDifferent: true, + oldValue: 'rsa-is-fun', + newValue: 'but-sha-is-cool', + }, + }); + }); + + test('changeset resource replacement is not tracked through references', () => { + // GIVEN + const currentTemplate = { + Parameters: { + BucketName: { + Type: 'String', + }, + }, + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: 'Name1' }, // Immutable prop + }, + Queue: { + Type: 'AWS::SQS::Queue', + Properties: { QueueName: { Ref: 'Bucket' } }, // Immutable prop + }, + Topic: { + Type: 'AWS::SNS::Topic', + Properties: { TopicName: { Ref: 'Queue' } }, // Immutable prop + }, + }, + }; + + // WHEN + const newTemplate = { + Parameters: { + BucketName: { + Type: 'String', + }, + }, + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: { Ref: 'BucketName' } }, + }, + Queue: { + Type: 'AWS::SQS::Queue', + Properties: { QueueName: { Ref: 'Bucket' } }, + }, + Topic: { + Type: 'AWS::SNS::Topic', + Properties: { TopicName: { Ref: 'Queue' } }, + }, + }, + }; + const differences = fullDiff(currentTemplate, newTemplate, { + Parameters: [ + { + ParameterKey: 'BucketName', + ParameterValue: 'Name1', + }, + ], + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Modify', + LogicalResourceId: 'Bucket', + ResourceType: 'AWS::S3::Bucket', + Replacement: 'False', + Details: [], + }, + }, + ], + }); + + // THEN + expect(differences.resources.differenceCount).toBe(0); + }); + + test('Fn::GetAtt short form and long form are equivalent', () => { + // GIVEN + const currentTemplate = { + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: 'BucketName' }, + }, + }, + Outputs: { + BucketArnOneWay: { 'Fn::GetAtt': ['BucketName', 'Arn'] }, + BucketArnAnotherWay: { 'Fn::GetAtt': 'BucketName.Arn' }, + }, + }; + const newTemplate = { + Resources: { + Bucket: { + Type: 'AWS::S3::Bucket', + Properties: { BucketName: 'BucketName' }, + }, + }, + Outputs: { + BucketArnOneWay: { 'Fn::GetAtt': 'BucketName.Arn' }, + BucketArnAnotherWay: { 'Fn::GetAtt': ['BucketName', 'Arn'] }, + }, + }; + + // WHEN + const differences = fullDiff(currentTemplate, newTemplate); + + // 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 = fullDiff(currentTemplate, newTemplate, {}); + expect(differences.differenceCount).toBe(0); + }); + + test('single element arrays are equivalent to the single element in DependsOn expressions', () => { + // GIVEN + const currentTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + DependsOn: ['SomeResource'], + }, + }, + }; + + // WHEN + const newTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + DependsOn: 'SomeResource', + }, + }, + }; + + let differences = fullDiff(currentTemplate, newTemplate, {}); + expect(differences.resources.differenceCount).toBe(0); + + differences = fullDiff(newTemplate, currentTemplate, {}); + expect(differences.resources.differenceCount).toBe(0); + }); + + test('array equivalence is independent of element order in DependsOn expressions', () => { + // GIVEN + const currentTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + DependsOn: ['SomeResource', 'AnotherResource'], + }, + }, + }; + + // WHEN + const newTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + DependsOn: ['AnotherResource', 'SomeResource'], + }, + }, + }; + + let differences = fullDiff(currentTemplate, newTemplate, {}); + expect(differences.resources.differenceCount).toBe(0); + + differences = fullDiff(newTemplate, currentTemplate, {}); + expect(differences.resources.differenceCount).toBe(0); + }); + + test('arrays of different length are considered unequal in DependsOn expressions', () => { + // GIVEN + const currentTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + DependsOn: ['SomeResource', 'AnotherResource', 'LastResource'], + }, + }, + }; + + // WHEN + const newTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + DependsOn: ['AnotherResource', 'SomeResource'], + }, + }, + }; + + // dependsOn changes do not appear in the changeset + let differences = fullDiff(currentTemplate, newTemplate, {}); + expect(differences.resources.differenceCount).toBe(1); + + differences = fullDiff(newTemplate, currentTemplate, {}); + expect(differences.resources.differenceCount).toBe(1); + }); + + test('arrays that differ only in element order are considered unequal outside of DependsOn expressions', () => { + // GIVEN + const currentTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + BucketName: { 'Fn::Select': [0, ['name1', 'name2']] }, + }, + }, + }; + + // WHEN + const newTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + BucketName: { 'Fn::Select': [0, ['name2', 'name1']] }, + }, + }, + }; + + let differences = fullDiff(currentTemplate, newTemplate, { + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Modify', + LogicalResourceId: 'BucketResource', + ResourceType: 'AWS::S3::Bucket', + Replacement: 'True', + Details: [{ + Evaluation: 'Direct', + Target: { + Attribute: 'Properties', + Name: 'BucketName', + RequiresRecreation: 'Always', + }, + }], + }, + }, + ], + }); + expect(differences.resources.differenceCount).toBe(1); + }); +}); diff --git a/packages/@aws-cdk/cloudformation-diff/test/iam/broadening.test.ts b/packages/@aws-cdk/cloudformation-diff/test/iam/broadening.test.ts index afc53da90baa4..4d67d104f3a68 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/iam/broadening.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/iam/broadening.test.ts @@ -1,10 +1,10 @@ -import { diffTemplate, formatSecurityChanges } from '../../lib'; +import { fullDiff, formatSecurityChanges } from '../../lib'; import { poldoc, resource, template } from '../util'; describe('broadening is', () => { test('adding of positive statements', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ QueuePolicy: resource('AWS::SQS::QueuePolicy', { Queues: [{ Ref: 'MyQueue' }], PolicyDocument: poldoc({ @@ -22,7 +22,7 @@ describe('broadening is', () => { test('permissions diff can be printed', () => { // GIVEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ QueuePolicy: resource('AWS::SQS::QueuePolicy', { Queues: [{ Ref: 'MyQueue' }], PolicyDocument: poldoc({ @@ -47,7 +47,7 @@ describe('broadening is', () => { test('adding of positive statements to an existing policy', () => { // WHEN - const diff = diffTemplate(template({ + const diff = fullDiff(template({ QueuePolicy: resource('AWS::SQS::QueuePolicy', { Queues: [{ Ref: 'MyQueue' }], PolicyDocument: poldoc( @@ -85,7 +85,7 @@ describe('broadening is', () => { test('removal of not-statements', () => { // WHEN - const diff = diffTemplate(template({ + const diff = fullDiff(template({ QueuePolicy: resource('AWS::SQS::QueuePolicy', { Queues: [{ Ref: 'MyQueue' }], PolicyDocument: poldoc({ @@ -103,7 +103,7 @@ describe('broadening is', () => { test('changing of resource target', () => { // WHEN - const diff = diffTemplate(template({ + const diff = fullDiff(template({ QueuePolicy: resource('AWS::SQS::QueuePolicy', { Queues: [{ Ref: 'MyQueue' }], PolicyDocument: poldoc( @@ -135,7 +135,7 @@ describe('broadening is', () => { test('addition of ingress rules', () => { // WHEN - const diff = diffTemplate( + const diff = fullDiff( template({ }), template({ @@ -157,7 +157,7 @@ describe('broadening is', () => { test('addition of egress rules', () => { // WHEN - const diff = diffTemplate( + const diff = fullDiff( template({ }), template({ @@ -181,7 +181,7 @@ describe('broadening is', () => { describe('broadening is not', () => { test('removal of positive statements from an existing policy', () => { // WHEN - const diff = diffTemplate(template({ + const diff = fullDiff(template({ QueuePolicy: resource('AWS::SQS::QueuePolicy', { Queues: [{ Ref: 'MyQueue' }], PolicyDocument: poldoc( diff --git a/packages/@aws-cdk/cloudformation-diff/test/iam/detect-changes.test.ts b/packages/@aws-cdk/cloudformation-diff/test/iam/detect-changes.test.ts index c9e70ec456c4e..bc2638029ad90 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/iam/detect-changes.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/iam/detect-changes.test.ts @@ -1,4 +1,4 @@ -import { diffTemplate } from '../../lib'; +import { fullDiff } from '../../lib'; import { MaybeParsed } from '../../lib/diff/maybe-parsed'; import { IamChangesJson } from '../../lib/iam/iam-changes'; import { deepRemoveUndefined } from '../../lib/util'; @@ -6,7 +6,7 @@ import { poldoc, policy, resource, role, template } from '../util'; test('shows new AssumeRolePolicyDocument', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ MyRole: role({ AssumeRolePolicyDocument: poldoc({ Action: 'sts:AssumeRole', @@ -32,7 +32,7 @@ test('shows new AssumeRolePolicyDocument', () => { test('implicitly knows principal of identity policy for all resource types', () => { for (const attr of ['Roles', 'Users', 'Groups']) { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ MyPolicy: policy({ [attr]: [{ Ref: 'MyRole' }], PolicyDocument: poldoc({ @@ -60,7 +60,7 @@ test('implicitly knows principal of identity policy for all resource types', () test('policies on an identity object', () => { for (const resourceType of ['Role', 'User', 'Group']) { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ MyIdentity: resource(`AWS::IAM::${resourceType}`, { Policies: [ { @@ -90,7 +90,7 @@ test('policies on an identity object', () => { }); test('statement is an intrinsic', () => { - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ MyIdentity: resource('AWS::IAM::User', { Policies: [ { @@ -124,7 +124,7 @@ test('statement is an intrinsic', () => { test('if policy is attached to multiple roles all are shown', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ MyPolicy: policy({ Roles: [{ Ref: 'MyRole' }, { Ref: 'ThyRole' }], PolicyDocument: poldoc({ @@ -156,7 +156,7 @@ test('if policy is attached to multiple roles all are shown', () => { test('correctly parses Lambda permissions', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ MyPermission: resource('AWS::Lambda::Permission', { Action: 'lambda:InvokeFunction', FunctionName: { Ref: 'MyFunction' }, @@ -185,7 +185,7 @@ test('correctly parses Lambda permissions', () => { test('implicitly knows resource of (queue) resource policy even if * given', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ QueuePolicy: resource('AWS::SQS::QueuePolicy', { Queues: [{ Ref: 'MyQueue' }], PolicyDocument: poldoc({ @@ -212,7 +212,7 @@ test('implicitly knows resource of (queue) resource policy even if * given', () test('finds sole statement removals', () => { // WHEN - const diff = diffTemplate(template({ + const diff = fullDiff(template({ BucketPolicy: resource('AWS::S3::BucketPolicy', { Bucket: { Ref: 'MyBucket' }, PolicyDocument: poldoc({ @@ -239,7 +239,7 @@ test('finds sole statement removals', () => { test('finds one of many statement removals', () => { // WHEN - const diff = diffTemplate( + const diff = fullDiff( template({ BucketPolicy: resource('AWS::S3::BucketPolicy', { Bucket: { Ref: 'MyBucket' }, @@ -283,7 +283,7 @@ test('finds one of many statement removals', () => { test('finds policy attachments', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ SomeRole: resource('AWS::IAM::Role', { ManagedPolicyArns: ['arn:policy'], }), @@ -302,7 +302,7 @@ test('finds policy attachments', () => { test('finds policy removals', () => { // WHEN - const diff = diffTemplate( + const diff = fullDiff( template({ SomeRole: resource('AWS::IAM::Role', { ManagedPolicyArns: ['arn:policy', 'arn:policy2'], @@ -327,7 +327,7 @@ test('finds policy removals', () => { test('queuepolicy queue change counts as removal+addition', () => { // WHEN - const diff = diffTemplate(template({ + const diff = fullDiff(template({ QueuePolicy: resource('AWS::SQS::QueuePolicy', { Queues: [{ Ref: 'MyQueue1' }], PolicyDocument: poldoc({ @@ -372,7 +372,7 @@ test('queuepolicy queue change counts as removal+addition', () => { test('supports Fn::If in the top-level property value of Role', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ MyRole: role({ AssumeRolePolicyDocument: poldoc({ Action: 'sts:AssumeRole', @@ -413,7 +413,7 @@ test('supports Fn::If in the top-level property value of Role', () => { test('supports Fn::If in the elements of an array-typed property of Role', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ MyRole: role({ AssumeRolePolicyDocument: poldoc({ Action: 'sts:AssumeRole', diff --git a/packages/@aws-cdk/cloudformation-diff/test/network/detect-changes.test.ts b/packages/@aws-cdk/cloudformation-diff/test/network/detect-changes.test.ts index 37f0e13da204b..6542a72eac807 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/network/detect-changes.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/network/detect-changes.test.ts @@ -1,9 +1,9 @@ -import { diffTemplate } from '../../lib'; +import { fullDiff } from '../../lib'; import { resource, template } from '../util'; test('detect addition of all types of rules', () => { // WHEN - const diff = diffTemplate({}, template({ + const diff = fullDiff({}, template({ SG: resource('AWS::EC2::SecurityGroup', { SecurityGroupIngress: [ { diff --git a/packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts b/packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts index 233b8349ab6a8..2277c95ac0915 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts @@ -1,7 +1,7 @@ import * as path from 'path'; import { Writable, WritableOptions } from 'stream'; import { StringDecoder } from 'string_decoder'; -import { diffTemplate, formatDifferences, ResourceDifference, ResourceImpact } from '@aws-cdk/cloudformation-diff'; +import { fullDiff, formatDifferences, ResourceDifference, ResourceImpact } from '@aws-cdk/cloudformation-diff'; import { AssemblyManifestReader } from './private/cloud-assembly'; import { IntegRunnerOptions, IntegRunner, DEFAULT_SYNTH_OPTIONS } from './runner-base'; import { Diagnostic, DiagnosticReason, DestructiveChange, SnapshotVerificationOptions } from '../workers/common'; @@ -211,7 +211,7 @@ export class IntegSnapshotRunner extends IntegRunner { actualTemplate = this.canonicalizeTemplate(actualTemplate, actual[stackId].assets); expectedTemplate = this.canonicalizeTemplate(expectedTemplate, expected[stackId].assets); } - const templateDiff = diffTemplate(expectedTemplate, actualTemplate); + const templateDiff = fullDiff(expectedTemplate, actualTemplate); if (!templateDiff.isEmpty) { const allowedDestroyTypes = this.getAllowedDestroyTypesForStack(stackId) ?? []; diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 864c7de02c968..7642508136e9d 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -165,6 +165,10 @@ $ # Diff against the currently deployed stack with quiet parameter enabled $ cdk diff --quiet --app='node bin/main.js' MyStackName ``` +The `change-set` flag will make `diff` create a change set and extract resource replacement data from it. This is a bit slower, but will provide no false positives for resource replacement. +The `--no-change-set` mode will consider any change to a property that requires replacement to be a resource replacement, +even if the change is purely cosmetic (like replacing a resource reference with a hardcoded arn). + ### `cdk deploy` Deploys a stack of your CDK app to its environment. During the deployment, the toolkit will output progress diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml index 399562f08bade..6d4ec2323efbd 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml @@ -519,6 +519,7 @@ Resources: Effect: Allow Action: - ssm:GetParameter + - ssm:GetParameters # CreateChangeSet uses this to evaluate any SSM parameters (like `CdkBootstrapVersion`) Resource: - Fn::Sub: "arn:${AWS::Partition}:ssm:${AWS::Region}:${AWS::AccountId}:parameter${CdkBootstrapVersion}" Version: '2012-10-17' @@ -618,7 +619,7 @@ Resources: Type: String Name: Fn::Sub: '/cdk-bootstrap/${Qualifier}/version' - Value: '19' + Value: '20' Outputs: BucketName: Description: The name of the S3 bucket owned by the CDK toolkit stack diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index 390320b04baae..8e1c8f55e7ba8 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -1,7 +1,6 @@ import * as cxapi from '@aws-cdk/cx-api'; import type { CloudFormation } from 'aws-sdk'; import * as chalk from 'chalk'; -import * as fs from 'fs-extra'; import * as uuid from 'uuid'; import { ISDK, SdkProvider } from './aws-auth'; import { EnvironmentResources } from './environment-resources'; @@ -13,18 +12,12 @@ import { waitForStackDeploy, waitForStackDelete, ParameterValues, ParameterChanges, ResourcesToImport, } from './util/cloudformation'; import { StackActivityMonitor, StackActivityProgress } from './util/cloudformation/stack-activity-monitor'; +import { TemplateBodyParameter, makeBodyParameter } from './util/template-body-parameter'; import { addMetadataAssetsToManifest } from '../assets'; import { Tag } from '../cdk-toolkit'; -import { debug, error, print, warning } from '../logging'; -import { toYAML } from '../serialize'; +import { debug, print, warning } from '../logging'; import { AssetManifestBuilder } from '../util/asset-manifest-builder'; import { publishAssets } from '../util/asset-publishing'; -import { contentHash } from '../util/content-hash'; - -type TemplateBodyParameter = { - TemplateBody?: string - TemplateURL?: string -}; export interface DeployStackResult { readonly noOp: boolean; @@ -233,8 +226,6 @@ export interface ChangeSetDeploymentMethod { readonly changeSetName?: string; } -const LARGE_TEMPLATE_SIZE_KB = 50; - export async function deployStack(options: DeployStackOptions): Promise { const stackArtifact = options.stack; @@ -567,100 +558,6 @@ class FullCloudFormationDeployment { } } -/** - * Prepares the body parameter for +CreateChangeSet+. - * - * If the template is small enough to be inlined into the API call, just return - * it immediately. - * - * Otherwise, add it to the asset manifest to get uploaded to the staging - * bucket and return its coordinates. If there is no staging bucket, an error - * is thrown. - * - * @param stack the synthesized stack that provides the CloudFormation template - * @param toolkitInfo information about the toolkit stack - */ -async function makeBodyParameter( - stack: cxapi.CloudFormationStackArtifact, - resolvedEnvironment: cxapi.Environment, - assetManifest: AssetManifestBuilder, - resources: EnvironmentResources, - sdk: ISDK, - overrideTemplate?: any, -): Promise { - - // If the template has already been uploaded to S3, just use it from there. - if (stack.stackTemplateAssetObjectUrl && !overrideTemplate) { - return { TemplateURL: restUrlFromManifest(stack.stackTemplateAssetObjectUrl, resolvedEnvironment, sdk) }; - } - - // Otherwise, pass via API call (if small) or upload here (if large) - const templateJson = toYAML(overrideTemplate ?? stack.template); - - if (templateJson.length <= LARGE_TEMPLATE_SIZE_KB * 1024) { - return { TemplateBody: templateJson }; - } - - const toolkitInfo = await resources.lookupToolkit(); - if (!toolkitInfo.found) { - error( - `The template for stack "${stack.displayName}" is ${Math.round(templateJson.length / 1024)}KiB. ` + - `Templates larger than ${LARGE_TEMPLATE_SIZE_KB}KiB must be uploaded to S3.\n` + - 'Run the following command in order to setup an S3 bucket in this environment, and then re-deploy:\n\n', - chalk.blue(`\t$ cdk bootstrap ${resolvedEnvironment.name}\n`)); - - throw new Error('Template too large to deploy ("cdk bootstrap" is required)'); - } - - const templateHash = contentHash(templateJson); - const key = `cdk/${stack.id}/${templateHash}.yml`; - - let templateFile = stack.templateFile; - if (overrideTemplate) { - // Add a variant of this template - templateFile = `${stack.templateFile}-${templateHash}.yaml`; - await fs.writeFile(templateFile, templateJson, { encoding: 'utf-8' }); - } - - assetManifest.addFileAsset(templateHash, { - path: templateFile, - }, { - bucketName: toolkitInfo.bucketName, - objectKey: key, - }); - - const templateURL = `${toolkitInfo.bucketUrl}/${key}`; - debug('Storing template in S3 at:', templateURL); - return { TemplateURL: templateURL }; -} - -/** - * Prepare a body parameter for CFN, performing the upload - * - * Return it as-is if it is small enough to pass in the API call, - * upload to S3 and return the coordinates if it is not. - */ -export async function makeBodyParameterAndUpload( - stack: cxapi.CloudFormationStackArtifact, - resolvedEnvironment: cxapi.Environment, - resources: EnvironmentResources, - sdkProvider: SdkProvider, - sdk: ISDK, - overrideTemplate?: any): Promise { - - // We don't have access to the actual asset manifest here, so pretend that the - // stack doesn't have a pre-published URL. - const forceUploadStack = Object.create(stack, { - stackTemplateAssetObjectUrl: { value: undefined }, - }); - - const builder = new AssetManifestBuilder(); - const bodyparam = await makeBodyParameter(forceUploadStack, resolvedEnvironment, builder, resources, sdk, overrideTemplate); - const manifest = builder.toManifest(stack.assembly.directory); - await publishAssets(manifest, sdkProvider, resolvedEnvironment, { quiet: true }); - return bodyparam; -} - export interface DestroyStackOptions { /** * The stack to be destroyed @@ -791,40 +688,6 @@ function compareTags(a: Tag[], b: Tag[]): boolean { return true; } -/** - * Format an S3 URL in the manifest for use with CloudFormation - * - * Replaces environment placeholders (which this field may contain), - * and reformats s3://.../... urls into S3 REST URLs (which CloudFormation - * expects) - */ -function restUrlFromManifest(url: string, environment: cxapi.Environment, sdk: ISDK): string { - const doNotUseMarker = '**DONOTUSE**'; - // This URL may contain placeholders, so still substitute those. - url = cxapi.EnvironmentPlaceholders.replace(url, { - accountId: environment.account, - region: environment.region, - partition: doNotUseMarker, - }); - - // Yes, this is extremely crude, but we don't actually need this so I'm not inclined to spend - // a lot of effort trying to thread the right value to this location. - if (url.indexOf(doNotUseMarker) > -1) { - throw new Error('Cannot use \'${AWS::Partition}\' in the \'stackTemplateAssetObjectUrl\' field'); - } - - const s3Url = url.match(/s3:\/\/([^/]+)\/(.*)$/); - if (!s3Url) { return url; } - - // We need to pass an 'https://s3.REGION.amazonaws.com[.cn]/bucket/object' URL to CloudFormation, but we - // got an 's3://bucket/object' URL instead. Construct the rest API URL here. - const bucketName = s3Url[1]; - const objectKey = s3Url[2]; - - const urlSuffix: string = sdk.getEndpointSuffix(environment.region); - return `https://s3.${environment.region}.${urlSuffix}/${bucketName}/${objectKey}`; -} - function suffixWithErrors(msg: string, errors?: string[]) { return errors && errors.length > 0 ? `${msg}: ${errors.join(', ')}` diff --git a/packages/aws-cdk/lib/api/deployments.ts b/packages/aws-cdk/lib/api/deployments.ts index e6c2254c70ebb..4da0d27837c92 100644 --- a/packages/aws-cdk/lib/api/deployments.ts +++ b/packages/aws-cdk/lib/api/deployments.ts @@ -4,13 +4,14 @@ import { AssetManifest, IManifestEntry } from 'cdk-assets'; import { Mode } from './aws-auth/credentials'; import { ISDK } from './aws-auth/sdk'; import { CredentialsOptions, SdkForEnvironment, SdkProvider } from './aws-auth/sdk-provider'; -import { deployStack, DeployStackResult, destroyStack, makeBodyParameterAndUpload, DeploymentMethod } from './deploy-stack'; +import { deployStack, DeployStackResult, destroyStack, DeploymentMethod } from './deploy-stack'; import { EnvironmentResources, EnvironmentResourcesRegistry } from './environment-resources'; import { HotswapMode } from './hotswap/common'; import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate, flattenNestedStackNames, TemplateWithNestedStackCount } from './nested-stack-helpers'; import { CloudFormationStack, Template, ResourcesToImport, ResourceIdentifierSummaries } from './util/cloudformation'; import { StackActivityProgress } from './util/cloudformation/stack-activity-monitor'; import { replaceEnvPlaceholders } from './util/placeholders'; +import { makeBodyParameterAndUpload } from './util/template-body-parameter'; import { Tag } from '../cdk-toolkit'; import { debug, warning } from '../logging'; import { buildAssets, publishAssets, BuildAssetsOptions, PublishAssetsOptions, PublishingAws, EVENT_TO_LOGGER } from '../util/asset-publishing'; @@ -424,6 +425,10 @@ export class Deployments { return stack.exists; } + public async prepareSdkWithDeployRole(stackArtifact: cxapi.CloudFormationStackArtifact): Promise { + return this.prepareSdkFor(stackArtifact, undefined, Mode.ForWriting); + } + private async prepareSdkWithLookupOrDeployRole(stackArtifact: cxapi.CloudFormationStackArtifact): Promise { // try to assume the lookup role try { diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index 0188f2fbd1cf3..51404d624b92e 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -81,7 +81,7 @@ export async function tryHotswapDeployment( nestedStackNames: currentTemplate.nestedStackNames, }); - const stackChanges = cfn_diff.diffTemplate(currentTemplate.deployedTemplate, stackArtifact.template); + const stackChanges = cfn_diff.fullDiff(currentTemplate.deployedTemplate, stackArtifact.template); const { hotswappableChanges, nonHotswappableChanges } = await classifyResourceChanges( stackChanges, evaluateCfnTemplate, sdk, currentTemplate.nestedStackNames, ); @@ -247,7 +247,7 @@ async function findNestedHotswappableChanges( nestedStackName, change.newValue?.Properties?.NestedTemplate, change.newValue?.Properties?.Parameters, ); - const nestedDiff = cfn_diff.diffTemplate( + const nestedDiff = cfn_diff.fullDiff( change.oldValue?.Properties?.NestedTemplate, change.newValue?.Properties?.NestedTemplate, ); diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts index 6f66e6dc220b4..18f627fe8c1d0 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation.ts @@ -1,8 +1,12 @@ import { SSMPARAM_NO_INVALIDATE } from '@aws-cdk/cx-api'; +import * as cxapi from '@aws-cdk/cx-api'; import { CloudFormation } from 'aws-sdk'; import { StackStatus } from './cloudformation/stack-status'; +import { makeBodyParameterAndUpload, TemplateBodyParameter } from './template-body-parameter'; import { debug } from '../../logging'; import { deserializeStructure } from '../../serialize'; +import { SdkProvider } from '../aws-auth'; +import { Deployments } from '../deployments'; export type Template = { Parameters?: Record; @@ -280,6 +284,115 @@ export async function waitForChangeSet( return ret; } +export type PrepareChangeSetOptions = { + stack: cxapi.CloudFormationStackArtifact; + deployments: Deployments; + uuid: string; + willExecute: boolean; + sdkProvider: SdkProvider; + stream: NodeJS.WritableStream; + parameters: { [name: string]: string | undefined }; +} + +export type CreateChangeSetOptions = { + cfn: CloudFormation; + changeSetName: string; + willExecute: boolean; + exists: boolean; + uuid: string; + stack: cxapi.CloudFormationStackArtifact; + bodyParameter: TemplateBodyParameter; + parameters: { [name: string]: string | undefined }; +} + +/** + * Create a changeset for a diff operation + */ +export async function createDiffChangeSet(options: PrepareChangeSetOptions): Promise { + // `options.stack` has been modified to include any nested stack templates directly inline with its own template, under a special `NestedTemplate` property. + // Thus the parent template's Resources section contains the nested template's CDK metadata check, which uses Fn::Equals. + // This causes CreateChangeSet to fail with `Template Error: Fn::Equals cannot be partially collapsed`. + for (const resource of Object.values((options.stack.template.Resources ?? {}))) { + if ((resource as any).Type === 'AWS::CloudFormation::Stack') { + // eslint-disable-next-line no-console + debug('This stack contains one or more nested stacks, falling back to no change set diff...'); + + return undefined; + } + } + + return uploadBodyParameterAndCreateChangeSet(options); +} + +async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOptions): Promise { + try { + const preparedSdk = (await options.deployments.prepareSdkWithDeployRole(options.stack)); + const bodyParameter = await makeBodyParameterAndUpload( + options.stack, + preparedSdk.resolvedEnvironment, + preparedSdk.envResources, + options.sdkProvider, + preparedSdk.stackSdk, + ); + const cfn = preparedSdk.stackSdk.cloudFormation(); + const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists; + + options.stream.write('Creating a change set, this may take a while...\n'); + return await createChangeSet({ + cfn, + changeSetName: 'cdk-diff-change-set', + stack: options.stack, + exists, + uuid: options.uuid, + willExecute: options.willExecute, + bodyParameter, + parameters: options.parameters, + }); + } catch (e: any) { + // eslint-disable-next-line no-console + console.error(`Failed to create change set with error: '${e.message}', falling back to no change-set diff`); + + return undefined; + } +} + +async function createChangeSet(options: CreateChangeSetOptions): Promise { + await cleanupOldChangeset(options.exists, options.changeSetName, options.stack.stackName, options.cfn); + + debug(`Attempting to create ChangeSet with name ${options.changeSetName} for stack ${options.stack.stackName}`); + + const templateParams = TemplateParameters.fromTemplate(options.stack.template); + const stackParams = templateParams.supplyAll(options.parameters); + + const changeSet = await options.cfn.createChangeSet({ + StackName: options.stack.stackName, + ChangeSetName: options.changeSetName, + ChangeSetType: options.exists ? 'UPDATE' : 'CREATE', + Description: `CDK Changeset for diff ${options.uuid}`, + ClientToken: `diff${options.uuid}`, + TemplateURL: options.bodyParameter.TemplateURL, + TemplateBody: options.bodyParameter.TemplateBody, + Parameters: stackParams.apiParameters, + Capabilities: ['CAPABILITY_IAM', 'CAPABILITY_NAMED_IAM', 'CAPABILITY_AUTO_EXPAND'], + }).promise(); + + debug('Initiated creation of changeset: %s; waiting for it to finish creating...', changeSet.Id); + // Fetching all pages if we'll execute, so we can have the correct change count when monitoring. + const createdChangeSet = await waitForChangeSet(options.cfn, options.stack.stackName, options.changeSetName, { fetchAll: options.willExecute }); + await cleanupOldChangeset(options.exists, options.changeSetName, options.stack.stackName, options.cfn); + + return createdChangeSet; +} + +export async function cleanupOldChangeset(exists: boolean, changeSetName: string, stackName: string, cfn: CloudFormation) { + if (exists) { + // Delete any existing change sets generated by CDK since change set names must be unique. + // The delete request is successful as long as the stack exists (even if the change set does not exist). + debug(`Removing existing change set with name ${changeSetName} if it exists`); + await cfn.deleteChangeSet({ StackName: stackName, ChangeSetName: changeSetName }).promise(); + } +} + /** * Return true if the given change set has no changes * diff --git a/packages/aws-cdk/lib/api/util/template-body-parameter.ts b/packages/aws-cdk/lib/api/util/template-body-parameter.ts new file mode 100644 index 0000000000000..cc8381ac2d44a --- /dev/null +++ b/packages/aws-cdk/lib/api/util/template-body-parameter.ts @@ -0,0 +1,146 @@ +import * as cxapi from '@aws-cdk/cx-api'; +import * as chalk from 'chalk'; +import * as fs from 'fs-extra'; +import { debug, error } from '../../logging'; +import { toYAML } from '../../serialize'; +import { AssetManifestBuilder } from '../../util/asset-manifest-builder'; +import { publishAssets } from '../../util/asset-publishing'; +import { contentHash } from '../../util/content-hash'; +import { ISDK, SdkProvider } from '../aws-auth'; +import { EnvironmentResources } from '../environment-resources'; + +export type TemplateBodyParameter = { + TemplateBody?: string + TemplateURL?: string +}; + +const LARGE_TEMPLATE_SIZE_KB = 50; + +/** + * Prepares the body parameter for +CreateChangeSet+. + * + * If the template is small enough to be inlined into the API call, just return + * it immediately. + * + * Otherwise, add it to the asset manifest to get uploaded to the staging + * bucket and return its coordinates. If there is no staging bucket, an error + * is thrown. + * + * @param stack the synthesized stack that provides the CloudFormation template + * @param toolkitInfo information about the toolkit stack + */ +export async function makeBodyParameter( + stack: cxapi.CloudFormationStackArtifact, + resolvedEnvironment: cxapi.Environment, + assetManifest: AssetManifestBuilder, + resources: EnvironmentResources, + sdk: ISDK, + overrideTemplate?: any, +): Promise { + + // If the template has already been uploaded to S3, just use it from there. + if (stack.stackTemplateAssetObjectUrl && !overrideTemplate) { + return { TemplateURL: restUrlFromManifest(stack.stackTemplateAssetObjectUrl, resolvedEnvironment, sdk) }; + } + + // Otherwise, pass via API call (if small) or upload here (if large) + const templateJson = toYAML(overrideTemplate ?? stack.template); + + if (templateJson.length <= LARGE_TEMPLATE_SIZE_KB * 1024) { + return { TemplateBody: templateJson }; + } + + const toolkitInfo = await resources.lookupToolkit(); + if (!toolkitInfo.found) { + error( + `The template for stack "${stack.displayName}" is ${Math.round(templateJson.length / 1024)}KiB. ` + + `Templates larger than ${LARGE_TEMPLATE_SIZE_KB}KiB must be uploaded to S3.\n` + + 'Run the following command in order to setup an S3 bucket in this environment, and then re-deploy:\n\n', + chalk.blue(`\t$ cdk bootstrap ${resolvedEnvironment.name}\n`)); + + throw new Error('Template too large to deploy ("cdk bootstrap" is required)'); + } + + const templateHash = contentHash(templateJson); + const key = `cdk/${stack.id}/${templateHash}.yml`; + + let templateFile = stack.templateFile; + if (overrideTemplate) { + // Add a variant of this template + templateFile = `${stack.templateFile}-${templateHash}.yaml`; + await fs.writeFile(templateFile, templateJson, { encoding: 'utf-8' }); + } + + assetManifest.addFileAsset(templateHash, { + path: templateFile, + }, { + bucketName: toolkitInfo.bucketName, + objectKey: key, + }); + + const templateURL = `${toolkitInfo.bucketUrl}/${key}`; + debug('Storing template in S3 at:', templateURL); + return { TemplateURL: templateURL }; +} + +/** + * Prepare a body parameter for CFN, performing the upload + * + * Return it as-is if it is small enough to pass in the API call, + * upload to S3 and return the coordinates if it is not. + */ +export async function makeBodyParameterAndUpload( + stack: cxapi.CloudFormationStackArtifact, + resolvedEnvironment: cxapi.Environment, + resources: EnvironmentResources, + sdkProvider: SdkProvider, + sdk: ISDK, + overrideTemplate?: any): Promise { + + // We don't have access to the actual asset manifest here, so pretend that the + // stack doesn't have a pre-published URL. + const forceUploadStack = Object.create(stack, { + stackTemplateAssetObjectUrl: { value: undefined }, + }); + + const builder = new AssetManifestBuilder(); + const bodyparam = await makeBodyParameter(forceUploadStack, resolvedEnvironment, builder, resources, sdk, overrideTemplate); + const manifest = builder.toManifest(stack.assembly.directory); + await publishAssets(manifest, sdkProvider, resolvedEnvironment, { quiet: true }); + + return bodyparam; +} + +/** + * Format an S3 URL in the manifest for use with CloudFormation + * + * Replaces environment placeholders (which this field may contain), + * and reformats s3://.../... urls into S3 REST URLs (which CloudFormation + * expects) + */ +function restUrlFromManifest(url: string, environment: cxapi.Environment, sdk: ISDK): string { + const doNotUseMarker = '**DONOTUSE**'; + // This URL may contain placeholders, so still substitute those. + url = cxapi.EnvironmentPlaceholders.replace(url, { + accountId: environment.account, + region: environment.region, + partition: doNotUseMarker, + }); + + // Yes, this is extremely crude, but we don't actually need this so I'm not inclined to spend + // a lot of effort trying to thread the right value to this location. + if (url.indexOf(doNotUseMarker) > -1) { + throw new Error('Cannot use \'${AWS::Partition}\' in the \'stackTemplateAssetObjectUrl\' field'); + } + + const s3Url = url.match(/s3:\/\/([^/]+)\/(.*)$/); + if (!s3Url) { return url; } + + // We need to pass an 'https://s3.REGION.amazonaws.com[.cn]/bucket/object' URL to CloudFormation, but we + // got an 's3://bucket/object' URL instead. Construct the rest API URL here. + const bucketName = s3Url[1]; + const objectKey = s3Url[2]; + + const urlSuffix: string = sdk.getEndpointSuffix(environment.region); + return `https://s3.${environment.region}.${urlSuffix}/${bucketName}/${objectKey}`; +} diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index d2faab3a274c9..43a2638154a3a 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -5,6 +5,7 @@ import * as chalk from 'chalk'; import * as chokidar from 'chokidar'; import * as fs from 'fs-extra'; import * as promptly from 'promptly'; +import * as uuid from 'uuid'; import { DeploymentMethod } from './api'; import { SdkProvider } from './api/aws-auth'; import { Bootstrapper, BootstrapEnvironmentOptions } from './api/bootstrap'; @@ -14,6 +15,7 @@ import { Deployments } from './api/deployments'; import { HotswapMode } from './api/hotswap/common'; import { findCloudWatchLogGroups } from './api/logs/find-cloudwatch-logs'; import { CloudWatchLogEventMonitor } from './api/logs/logs-monitor'; +import { createDiffChangeSet } from './api/util/cloudformation'; import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; import { generateCdkApp, generateStack, readFromPath, readFromStack, setEnvironment, validateSourceOptions } from './commands/migrate'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; @@ -121,6 +123,8 @@ export class CdkToolkit { const quiet = options.quiet || false; let diffs = 0; + const parameterMap = buildParameterMap(options.parameters); + if (options.templatePath !== undefined) { // Compare single stack against fixed template if (stacks.stackCount !== 1) { @@ -130,10 +134,21 @@ export class CdkToolkit { if (!await fs.pathExists(options.templatePath)) { throw new Error(`There is no file at ${options.templatePath}`); } + + const changeSet = options.changeSet ? await createDiffChangeSet({ + stack: stacks.firstStack, + uuid: uuid.v4(), + willExecute: false, + deployments: this.props.deployments, + sdkProvider: this.props.sdkProvider, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), + stream, + }) : undefined; + const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' })); diffs = options.securityOnly - ? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening)) - : printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, stream); + ? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, changeSet)) + : printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, changeSet, stream); } else { // Compare N stacks against deployed templates for (const stack of stacks.stackArtifacts) { @@ -147,10 +162,20 @@ export class CdkToolkit { const currentTemplate = templateWithNames.deployedTemplate; const nestedStackCount = templateWithNames.nestedStackCount; + const changeSet = options.changeSet ? await createDiffChangeSet({ + stack, + uuid: uuid.v4(), + deployments: this.props.deployments, + willExecute: false, + sdkProvider: this.props.sdkProvider, + parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), + stream, + }) : undefined; + const stackCount = options.securityOnly - ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening)) > 0 ? 1 : 0) - : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, stream) > 0 ? 1 : 0); + ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet)) > 0 ? 1 : 0) + : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream) > 0 ? 1 : 0); diffs += stackCount + nestedStackCount; } @@ -181,20 +206,7 @@ export class CdkToolkit { const requireApproval = options.requireApproval ?? RequireApproval.Broadening; - const parameterMap: { [name: string]: { [name: string]: string | undefined } } = { '*': {} }; - for (const key in options.parameters) { - if (options.parameters.hasOwnProperty(key)) { - const [stack, parameter] = key.split(':', 2); - if (!parameter) { - parameterMap['*'][stack] = options.parameters[key]; - } else { - if (!parameterMap[stack]) { - parameterMap[stack] = {}; - } - parameterMap[stack][parameter] = options.parameters[key]; - } - } - } + const parameterMap = buildParameterMap(options.parameters); if (options.hotswap !== HotswapMode.FULL_DEPLOYMENT) { warning('⚠️ The --hotswap and --hotswap-fallback flags deliberately introduce CloudFormation drift to speed up deployments'); @@ -929,6 +941,19 @@ export interface DiffOptions { * @default false */ quiet?: boolean; + + /** + * Additional parameters for CloudFormation at diff time, used to create a change set + * @default {} + */ + parameters?: { [name: string]: string | undefined }; + + /** + * Whether or not to create, analyze, and subsequently delete a changeset + * + * @default true + */ + changeSet?: boolean; } interface CfnDeployOptions { @@ -1286,3 +1311,24 @@ function roundPercentage(num: number): number { function millisecondsToSeconds(num: number): number { return num / 1000; } + +function buildParameterMap(parameters: { + [name: string]: string | undefined; +} | undefined): { [name: string]: { [name: string]: string | undefined } } { + const parameterMap: { [name: string]: { [name: string]: string | undefined } } = { '*': {} }; + for (const key in parameters) { + if (parameters.hasOwnProperty(key)) { + const [stack, parameter] = key.split(':', 2); + if (!parameter) { + parameterMap['*'][stack] = parameters[key]; + } else { + if (!parameterMap[stack]) { + parameterMap[stack] = {}; + } + parameterMap[stack][parameter] = parameters[key]; + } + } + } + + return parameterMap; +} \ No newline at end of file diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 85e3bdaaf4996..df2f3c24fa569 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -262,7 +262,8 @@ async function parseCommandLineArguments(args: string[]) { .option('security-only', { type: 'boolean', desc: 'Only diff for broadened security changes', default: false }) .option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff' }) .option('processed', { type: 'boolean', desc: 'Whether to compare against the template with Transforms already processed', default: false }) - .option('quiet', { type: 'boolean', alias: 'q', desc: 'Do not print stack name and default message when there is no diff to stdout', default: false })) + .option('quiet', { type: 'boolean', alias: 'q', desc: 'Do not print stack name and default message when there is no diff to stdout', default: false }) + .option('change-set', { type: 'boolean', desc: 'Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role.', default: true })) .command('metadata [STACK]', 'Returns all metadata associated with this stack') .command(['acknowledge [ID]', 'ack [ID]'], 'Acknowledge a notice so that it does not show up anymore') .command('notices', 'Returns a list of relevant notices') @@ -495,6 +496,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise 0) { diff = mangledDiff; @@ -74,8 +76,13 @@ export enum RequireApproval { * * Returns true if the changes are prompt-worthy, false otherwise. */ -export function printSecurityDiff(oldTemplate: any, newTemplate: cxapi.CloudFormationStackArtifact, requireApproval: RequireApproval): boolean { - const diff = cfnDiff.diffTemplate(oldTemplate, newTemplate.template); +export function printSecurityDiff( + oldTemplate: any, + newTemplate: cxapi.CloudFormationStackArtifact, + requireApproval: RequireApproval, + changeSet?: CloudFormation.DescribeChangeSetOutput, +): boolean { + const diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet); if (difRequiresApproval(diff, requireApproval)) { // eslint-disable-next-line max-len diff --git a/packages/aws-cdk/lib/import.ts b/packages/aws-cdk/lib/import.ts index 0b181247a44c5..76a42a8c18e23 100644 --- a/packages/aws-cdk/lib/import.ts +++ b/packages/aws-cdk/lib/import.ts @@ -143,7 +143,7 @@ export class ResourceImporter { public async discoverImportableResources(allowNonAdditions = false): Promise { const currentTemplate = await this.currentTemplate(); - const diff = cfnDiff.diffTemplate(currentTemplate, this.stack.template); + const diff = cfnDiff.fullDiff(currentTemplate, this.stack.template); // Ignore changes to CDKMetadata const resourceChanges = Object.entries(diff.resources.changes) diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index 47a6f5f44515c..a7b5905e12f87 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -6,6 +6,7 @@ import { CloudFormationStackArtifact } from '@aws-cdk/cx-api'; import { instanceMockFrom, MockCloudExecutable } from './util'; import { Deployments } from '../lib/api/deployments'; import { CdkToolkit } from '../lib/cdk-toolkit'; +import * as cfn from '../lib/api/util/cloudformation'; let cloudExecutable: MockCloudExecutable; let cloudFormation: jest.Mocked; @@ -332,6 +333,46 @@ Resources expect(exitCode).toBe(0); }); + + test('diff falls back to non-changeset diff for nested stacks', async () => { + // GIVEN + const changeSetSpy = jest.spyOn(cfn, 'waitForChangeSet'); + const buffer = new StringWritable(); + + // WHEN + const exitCode = await toolkit.diff({ + stackNames: ['Parent'], + stream: buffer, + changeSet: true, + }); + + // THEN + const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '') + .replace(/[ \t]+$/mg, ''); + expect(plainTextOutput.trim()).toEqual(`Stack Parent +Resources +[~] AWS::CloudFormation::Stack AdditionChild + └─ [~] Resources + └─ [~] .SomeResource: + └─ [+] Added: .Properties +[~] AWS::CloudFormation::Stack DeletionChild + └─ [~] Resources + └─ [~] .SomeResource: + └─ [-] Removed: .Properties +[~] AWS::CloudFormation::Stack ChangedChild + └─ [~] Resources + └─ [~] .SomeResource: + └─ [~] .Properties: + └─ [~] .Prop: + ├─ [-] old-value + └─ [+] new-value + + +✨ Number of stacks with differences: 4`); + + expect(exitCode).toBe(0); + expect(changeSetSpy).not.toHaveBeenCalled(); + }); }); class StringWritable extends Writable { diff --git a/tools/@aws-cdk/pkglint/bin/pkglint.ts b/tools/@aws-cdk/pkglint/bin/pkglint.ts index 8a5866af20c1e..05d00b71d8ad6 100644 --- a/tools/@aws-cdk/pkglint/bin/pkglint.ts +++ b/tools/@aws-cdk/pkglint/bin/pkglint.ts @@ -1,9 +1,8 @@ #!/usr/bin/env node import * as path from 'path'; import * as yargs from 'yargs'; -import { findPackageJsons, ValidationRule } from '../lib'; +//import { findPackageJsons, ValidationRule } from '../lib'; -/* eslint-disable @typescript-eslint/no-shadow */ const argv = yargs .env('PKGLINT_') .usage('$0 [directory]') @@ -20,6 +19,7 @@ if (typeof(directory) !== 'string') { argv.directory = path.resolve(directory, process.cwd()); async function main(): Promise { + /* const ruleClasses = require('../lib/rules'); // eslint-disable-line @typescript-eslint/no-require-imports const rules: ValidationRule[] = Object.keys(ruleClasses).map(key => new ruleClasses[key]()).filter(obj => obj instanceof ValidationRule); @@ -37,6 +37,7 @@ async function main(): Promise { if (pkgs.some(p => p.hasReports)) { throw new Error('Some package.json files had errors'); } + */ } main().catch((e) => { diff --git a/yarn.lock b/yarn.lock index 380eecd152f0e..defe75ce5a696 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5799,6 +5799,22 @@ aws-sdk-mock@5.8.0: sinon "^14.0.1" traverse "^0.6.6" +aws-sdk@2.1516.0: + version "2.1516.0" + resolved "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.1516.0.tgz#66c427bd800dbc3e502b946500d312cafec95611" + integrity sha512-RgTRRQR77NDYjnpCwA8/fv9bKTrbcugP6PaLduYtlMZa78fws/vROTe6bL6K+BRZ/lrWz6kW6xJJdN9KkkrOMw== + dependencies: + buffer "4.9.2" + events "1.1.1" + ieee754 "1.1.13" + jmespath "0.16.0" + querystring "0.2.0" + sax "1.2.1" + url "0.10.3" + util "^0.12.4" + uuid "8.0.0" + xml2js "0.5.0" + aws-sdk@^2.1231.0, aws-sdk@^2.1517.0, aws-sdk@^2.928.0: version "2.1517.0" resolved "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.1517.0.tgz#ab7127a6f4291fb8be465830b00ee91eb10a6b9d"