From d97361519bfb6a3ebb53939b1e7da4db0a507dee Mon Sep 17 00:00:00 2001 From: Calvin Combs <66279577+comcalvi@users.noreply.github.com> Date: Tue, 30 Jan 2024 19:51:11 -0800 Subject: [PATCH] feat(CLI): Diff Supports Import Change Sets (#28787) #28336 made diff create and parse change sets to determine accurate resource replacement information. This PR expands the change set parsing to support import type change sets. This shows the output of diff with a single resource import: Screenshot 2024-01-19 at 2 44 09 PM ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../cloudformation-diff/lib/diff-template.ts | 21 +++++ .../cloudformation-diff/lib/diff/types.ts | 12 +++ .../cloudformation-diff/lib/format.ts | 11 ++- .../test/diff-template.test.ts | 65 +++++++++++++ .../aws-cdk/lib/api/util/cloudformation.ts | 20 +++- packages/aws-cdk/lib/cdk-toolkit.ts | 25 ++++- packages/aws-cdk/lib/import.ts | 16 +++- packages/aws-cdk/test/diff.test.ts | 94 +++++++++++++++++++ 8 files changed, 252 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 433eda0b5a10d..d84cc83a082d7 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -53,6 +53,7 @@ export function fullDiff( const theDiff = diffTemplate(currentTemplate, newTemplate); if (changeSet) { filterFalsePositivies(theDiff, changeSet); + addImportInformation(theDiff, changeSet); } return theDiff; @@ -208,6 +209,15 @@ function deepCopy(x: any): any { return x; } +function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) { + const imports = findResourceImports(changeSet); + diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => { + if (imports.includes(logicalId)) { + change.isImport = true; + } + }); +} + function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) { const replacements = findResourceReplacements(changeSet); diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => { @@ -245,6 +255,17 @@ function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormati }); } +function findResourceImports(changeSet: CloudFormation.DescribeChangeSetOutput): string[] { + const importedResourceLogicalIds = []; + for (const resourceChange of changeSet.Changes ?? []) { + if (resourceChange.ResourceChange?.Action === 'Import') { + importedResourceLogicalIds.push(resourceChange.ResourceChange.LogicalResourceId!); + } + } + + return importedResourceLogicalIds; +} + function findResourceReplacements(changeSet: CloudFormation.DescribeChangeSetOutput): types.ResourceReplacements { const replacements: types.ResourceReplacements = {}; for (const resourceChange of changeSet.Changes ?? []) { diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts index e85265bf99c1f..af536400f942b 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -480,6 +480,8 @@ export enum ResourceImpact { WILL_DESTROY = 'WILL_DESTROY', /** The existing physical resource will be removed from CloudFormation supervision */ WILL_ORPHAN = 'WILL_ORPHAN', + /** The existing physical resource will be added to CloudFormation supervision */ + WILL_IMPORT = 'WILL_IMPORT', /** There is no change in this resource */ NO_CHANGE = 'NO_CHANGE', } @@ -495,6 +497,7 @@ function worstImpact(one: ResourceImpact, two?: ResourceImpact): ResourceImpact if (!two) { return one; } const badness = { [ResourceImpact.NO_CHANGE]: 0, + [ResourceImpact.WILL_IMPORT]: 0, [ResourceImpact.WILL_UPDATE]: 1, [ResourceImpact.WILL_CREATE]: 2, [ResourceImpact.WILL_ORPHAN]: 3, @@ -528,6 +531,11 @@ export class ResourceDifference implements IDifference { */ public readonly isRemoval: boolean; + /** + * Whether this resource was imported + */ + public isImport?: boolean; + /** Property-level changes on the resource */ private readonly propertyDiffs: { [key: string]: PropertyDifference }; @@ -552,6 +560,7 @@ export class ResourceDifference implements IDifference { this.isAddition = oldValue === undefined; this.isRemoval = newValue === undefined; + this.isImport = undefined; } public get oldProperties(): PropertyMap | undefined { @@ -647,6 +656,9 @@ export class ResourceDifference implements IDifference { } public get changeImpact(): ResourceImpact { + if (this.isImport) { + return ResourceImpact.WILL_IMPORT; + } // Check the Type first if (this.resourceTypes.oldType !== this.resourceTypes.newType) { if (this.resourceTypes.oldType === undefined) { return ResourceImpact.WILL_CREATE; } diff --git a/packages/@aws-cdk/cloudformation-diff/lib/format.ts b/packages/@aws-cdk/cloudformation-diff/lib/format.ts index ce84c2e40f31c..7935f774fd468 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/format.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/format.ts @@ -79,6 +79,7 @@ const ADDITION = chalk.green('[+]'); const CONTEXT = chalk.grey('[ ]'); const UPDATE = chalk.yellow('[~]'); const REMOVAL = chalk.red('[-]'); +const IMPORT = chalk.blue('[←]'); class Formatter { constructor( @@ -159,7 +160,7 @@ class Formatter { const resourceType = diff.isRemoval ? diff.oldResourceType : diff.newResourceType; // eslint-disable-next-line max-len - this.print(`${this.formatPrefix(diff)} ${this.formatValue(resourceType, chalk.cyan)} ${this.formatLogicalId(logicalId)} ${this.formatImpact(diff.changeImpact)}`); + this.print(`${this.formatResourcePrefix(diff)} ${this.formatValue(resourceType, chalk.cyan)} ${this.formatLogicalId(logicalId)} ${this.formatImpact(diff.changeImpact)}`); if (diff.isUpdate) { const differenceCount = diff.differenceCount; @@ -171,6 +172,12 @@ class Formatter { } } + public formatResourcePrefix(diff: ResourceDifference) { + if (diff.isImport) { return IMPORT; } + + return this.formatPrefix(diff); + } + public formatPrefix(diff: Difference) { if (diff.isAddition) { return ADDITION; } if (diff.isUpdate) { return UPDATE; } @@ -204,6 +211,8 @@ class Formatter { return chalk.italic(chalk.bold(chalk.red('destroy'))); case ResourceImpact.WILL_ORPHAN: return chalk.italic(chalk.yellow('orphan')); + case ResourceImpact.WILL_IMPORT: + return chalk.italic(chalk.blue('import')); case ResourceImpact.WILL_UPDATE: case ResourceImpact.WILL_CREATE: case ResourceImpact.NO_CHANGE: 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 b211c1d17c085..805e8a9a7767e 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts @@ -1117,4 +1117,69 @@ describe('changeset', () => { }); expect(differences.resources.differenceCount).toBe(1); }); + + test('imports are respected for new stacks', async () => { + // GIVEN + const currentTemplate = {}; + + // WHEN + const newTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + }, + }, + }; + + let differences = fullDiff(currentTemplate, newTemplate, { + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Import', + LogicalResourceId: 'BucketResource', + }, + }, + ], + }); + expect(differences.resources.differenceCount).toBe(1); + expect(differences.resources.get('BucketResource').changeImpact === ResourceImpact.WILL_IMPORT); + }); + + test('imports are respected for existing stacks', async () => { + // GIVEN + const currentTemplate = { + Resources: { + OldResource: { + Type: 'AWS::Something::Resource', + }, + }, + }; + + // WHEN + const newTemplate = { + Resources: { + OldResource: { + Type: 'AWS::Something::Resource', + }, + BucketResource: { + Type: 'AWS::S3::Bucket', + }, + }, + }; + + let differences = fullDiff(currentTemplate, newTemplate, { + Changes: [ + { + Type: 'Resource', + ResourceChange: { + Action: 'Import', + LogicalResourceId: 'BucketResource', + }, + }, + ], + }); + expect(differences.resources.differenceCount).toBe(1); + expect(differences.resources.get('BucketResource').changeImpact === ResourceImpact.WILL_IMPORT); + }); }); diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts index e965c27db1940..b61991bf51699 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation.ts @@ -292,6 +292,7 @@ export type PrepareChangeSetOptions = { sdkProvider: SdkProvider; stream: NodeJS.WritableStream; parameters: { [name: string]: string | undefined }; + resourcesToImport?: ResourcesToImport; } export type CreateChangeSetOptions = { @@ -303,6 +304,8 @@ export type CreateChangeSetOptions = { stack: cxapi.CloudFormationStackArtifact; bodyParameter: TemplateBodyParameter; parameters: { [name: string]: string | undefined }; + resourcesToImport?: ResourcesToImport; + role?: string; } /** @@ -337,7 +340,9 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp const cfn = preparedSdk.stackSdk.cloudFormation(); const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists; + const executionRoleArn = preparedSdk.cloudFormationRoleArn; options.stream.write('Hold on while we create a read-only change set to get a diff with accurate replacement information (use --no-change-set to use a less accurate but faster template-only diff)\n'); + return await createChangeSet({ cfn, changeSetName: 'cdk-diff-change-set', @@ -347,6 +352,8 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp willExecute: options.willExecute, bodyParameter, parameters: options.parameters, + resourcesToImport: options.resourcesToImport, + role: executionRoleArn, }); } catch (e: any) { debug(e.message); @@ -367,12 +374,14 @@ async function createChangeSet(options: CreateChangeSetOptions): Promise 0 ? 1 : 0) @@ -205,6 +215,12 @@ export class CdkToolkit { const elapsedSynthTime = new Date().getTime() - startSynthTime; print('\n✨ Synthesis time: %ss\n', formatTime(elapsedSynthTime)); + if (stackCollection.stackCount === 0) { + // eslint-disable-next-line no-console + console.error('This app contains no stacks'); + return; + } + await this.tryMigrateResources(stackCollection, options); const requireApproval = options.requireApproval ?? RequireApproval.Broadening; @@ -884,7 +900,7 @@ export class CdkToolkit { private async tryMigrateResources(stacks: StackCollection, options: DeployOptions): Promise { const stack = stacks.stackArtifacts[0]; const migrateDeployment = new ResourceImporter(stack, this.props.deployments); - const resourcesToImport = await this.tryGetResources(migrateDeployment); + const resourcesToImport = await this.tryGetResources(await migrateDeployment.resolveEnvironment()); if (resourcesToImport) { print('%s: creating stack for resource migration...', chalk.bold(stack.displayName)); @@ -918,11 +934,10 @@ export class CdkToolkit { print('\n✨ Resource migration time: %ss\n', formatTime(elapsedDeployTime)); } - private async tryGetResources(migrateDeployment: ResourceImporter) { + private async tryGetResources(environment: cxapi.Environment): Promise { try { const migrateFile = fs.readJsonSync('migrate.json', { encoding: 'utf-8' }); const sourceEnv = (migrateFile.Source as string).split(':'); - const environment = await migrateDeployment.resolveEnvironment(); if (sourceEnv[0] === 'localfile' || (sourceEnv[4] === environment.account && sourceEnv[3] === environment.region)) { return migrateFile.Resources; @@ -930,6 +945,8 @@ export class CdkToolkit { } catch (e) { // Nothing to do } + + return undefined; } } diff --git a/packages/aws-cdk/lib/import.ts b/packages/aws-cdk/lib/import.ts index 99a54654ecfda..cd6f70cebb03f 100644 --- a/packages/aws-cdk/lib/import.ts +++ b/packages/aws-cdk/lib/import.ts @@ -385,13 +385,21 @@ export class ResourceImporter { * @returns template with import resources only */ private removeNonImportResources() { - const template = this.stack.template; - delete template.Resources.CDKMetadata; - delete template.Outputs; - return template; + return removeNonImportResources(this.stack); } } +/** + * Removes CDKMetadata and Outputs in the template so that only resources for importing are left. + * @returns template with import resources only + */ +export function removeNonImportResources(stack: cxapi.CloudFormationStackArtifact) { + const template = stack.template; + delete template.Resources.CDKMetadata; + delete template.Outputs; + return template; +} + /** * Information about a resource in the template that is importable */ diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index a7b5905e12f87..a92c6d53551c3 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -12,6 +12,100 @@ let cloudExecutable: MockCloudExecutable; let cloudFormation: jest.Mocked; let toolkit: CdkToolkit; +describe('imports', () => { + beforeEach(() => { + jest.spyOn(cfn, 'createDiffChangeSet').mockImplementation(async () => { + return { + Changes: [ + { + ResourceChange: { + Action: 'Import', + LogicalResourceId: 'Queue', + }, + }, + { + ResourceChange: { + Action: 'Import', + LogicalResourceId: 'Bucket', + }, + }, + { + ResourceChange: { + Action: 'Import', + LogicalResourceId: 'Queue2', + }, + }, + ], + }; + }); + cloudExecutable = new MockCloudExecutable({ + stacks: [{ + stackName: 'A', + template: { + Resources: { + Queue: { + Type: 'AWS::SQS::Queue', + }, + Queue2: { + Type: 'AWS::SQS::Queue', + }, + Bucket: { + Type: 'AWS::S3::Bucket', + }, + }, + }, + }], + }); + + cloudFormation = instanceMockFrom(Deployments); + + toolkit = new CdkToolkit({ + cloudExecutable, + deployments: cloudFormation, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + }); + + // Default implementations + cloudFormation.readCurrentTemplateWithNestedStacks.mockImplementation((_stackArtifact: CloudFormationStackArtifact) => { + return Promise.resolve({ + deployedTemplate: {}, + nestedStackCount: 0, + }); + }); + cloudFormation.deployStack.mockImplementation((options) => Promise.resolve({ + noOp: true, + outputs: {}, + stackArn: '', + stackArtifact: options.stack, + })); + }); + + test('imports', async () => { + // GIVEN + const buffer = new StringWritable(); + + // WHEN + const exitCode = await toolkit.diff({ + stackNames: ['A'], + stream: buffer, + changeSet: true, + }); + + // THEN + const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ''); + expect(plainTextOutput).toContain(`Stack A +Resources +[←] AWS::SQS::Queue Queue import +[←] AWS::SQS::Queue Queue2 import +[←] AWS::S3::Bucket Bucket import +`); + + expect(buffer.data.trim()).toContain('✨ Number of stacks with differences: 1'); + expect(exitCode).toBe(0); + }); +}); + describe('non-nested stacks', () => { beforeEach(() => { cloudExecutable = new MockCloudExecutable({