From ba72d74b761704d79fa4a05a0050ea55659b8af9 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Thu, 11 Jan 2024 16:50:24 -0800 Subject: [PATCH 1/4] feat(migrate): enable import of resources on apps created from cdk migrate --- .../cli-integ/resources/cdk-apps/app/app.js | 10 ++- .../tests/cli-integ-tests/cli.integtest.ts | 37 +++++++++++ packages/aws-cdk/lib/api/deployments.ts | 9 ++- packages/aws-cdk/lib/cdk-toolkit.ts | 65 +++++++++++++++++-- packages/aws-cdk/lib/import.ts | 58 +++++++++++++++-- packages/aws-cdk/test/import.test.ts | 50 ++++++++++++-- 6 files changed, 214 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index eeb1ad83673e2..3d98923972acb 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -65,7 +65,7 @@ class YourStack extends cdk.Stack { } } -class ImportableStack extends cdk.Stack { +class MigrateStack extends cdk.Stack { constructor(parent, id, props) { super(parent, id, props); @@ -82,6 +82,12 @@ class ImportableStack extends cdk.Stack { }); } + } +} + +class ImportableStack extends MigrateStack { + constructor(parent, id, props) { + super(parent, id, props); new cdk.CfnWaitConditionHandle(this, 'Handle'); } } @@ -470,6 +476,8 @@ switch (stackSet) { new ImportableStack(app, `${stackPrefix}-importable-stack`); + new MigrateStack(app, `${stackPrefix}-migrate-stack`); + new ExportValueStack(app, `${stackPrefix}-export-value-stack`); new BundlingStage(app, `${stackPrefix}-bundling-stage`); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index b64a3c085bd34..2b6e4d251fb8b 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -1213,6 +1213,43 @@ integTest('test resource import', withDefaultFixture(async (fixture) => { } })); +integTest('test migrate deployment for app with localfile source in migrate.json', withDefaultFixture(async (fixture) => { + const outputsFile = path.join(fixture.integTestDir, 'outputs', 'outputs.json'); + await fs.mkdir(path.dirname(outputsFile), { recursive: true }); + + // Initial deploy + await fixture.cdkDeploy('migrate-stack', { + modEnv: { ORPHAN_TOPIC: '1' }, + options: ['--outputs-file', outputsFile], + }); + + const outputs = JSON.parse((await fs.readFile(outputsFile, { encoding: 'utf-8' })).toString()); + const queueName = outputs.QueueName; + const queueLogicalId = outputs.QueueLogicalId; + fixture.log(`Created queue ${queueName} in stack ${fixture.fullStackName}`); + + // Write the migrate file based on the ID from step one, then deploy the app with migrate + const migrateFile = path.join(fixture.integTestDir, 'migrate.json'); + await fs.writeFile( + migrateFile, JSON.stringify( + { Source: 'localfile', Resources: [{ ResourceType: 'AWS::SQS::Queue', LogicalResourceId: queueLogicalId, ResourceIdentifier: { QueueName: queueName } }] }, + ), + { encoding: 'utf-8' }, + ); + + await fixture.cdkDestroy('migrate-stack'); + fixture.log(`Deleted stack ${fixture.fullStackName}, orphaning ${queueName}`); + + // Create new stack from existing queue + try { + fixture.log(`Deploying new stack ${fixture.fullStackName}, migrating ${queueName} into stack`); + await fixture.cdkDeploy('migrate-stack'); + } finally { + // Cleanup + await fixture.cdkDestroy('migrate-stack'); + } +})); + integTest('hotswap deployment supports Lambda function\'s description and environment variables', withDefaultFixture(async (fixture) => { // GIVEN const stackArn = await fixture.cdkDeploy('lambda-hotswap', { diff --git a/packages/aws-cdk/lib/api/deployments.ts b/packages/aws-cdk/lib/api/deployments.ts index 09264f393fc3c..925cdebd45e15 100644 --- a/packages/aws-cdk/lib/api/deployments.ts +++ b/packages/aws-cdk/lib/api/deployments.ts @@ -317,6 +317,13 @@ export class Deployments { this.environmentResources = new EnvironmentResourcesRegistry(props.toolkitStackName); } + /** + * Resolves the environment for a stack. + */ + public async resolveEnvironment(stack: cxapi.CloudFormationStackArtifact): Promise { + return this.sdkProvider.resolveEnvironment(stack.environment); + } + public async readCurrentTemplateWithNestedStacks( rootStackArtifact: cxapi.CloudFormationStackArtifact, retrieveProcessedTemplate: boolean = false, @@ -470,7 +477,7 @@ export class Deployments { throw new Error(`The stack ${stack.displayName} does not have an environment`); } - const resolvedEnvironment = await this.sdkProvider.resolveEnvironment(stack.environment); + const resolvedEnvironment = await this.resolveEnvironment(stack); // Substitute any placeholders with information about the current environment const arns = await replaceEnvPlaceholders({ diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 4a8a40949ab4d..f1ed8872cf959 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -15,7 +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 { createDiffChangeSet, ResourcesToImport } 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'; @@ -205,6 +205,8 @@ export class CdkToolkit { const elapsedSynthTime = new Date().getTime() - startSynthTime; print('\n✨ Synthesis time: %ss\n', formatTime(elapsedSynthTime)); + await this.tryMigrateResources(stackCollection, options); + const requireApproval = options.requireApproval ?? RequireApproval.Broadening; const parameterMap = buildParameterMap(options.parameters); @@ -539,9 +541,7 @@ export class CdkToolkit { // Import the resources according to the given mapping print('%s: importing resources into stack...', chalk.bold(stack.displayName)); const tags = tagsForStack(stack); - await resourceImporter.importResources(actualImport, { - stack, - deployName: stack.stackName, + await resourceImporter.importResourcesFromMap(actualImport, { roleArn: options.roleArn, toolkitStackName: options.toolkitStackName, tags, @@ -874,6 +874,63 @@ export class CdkToolkit { stackName: assetNode.parentStack.stackName, })); } + + /** + * Checks to see if a migrate.json file exists. If it does and the source is either `filepath` or + * is in the same environment as the stack deployment, a new stack is created and the resources are + * migrated to the stack using an IMPORT changeset. The normal deployment will resume after this is complete + * to add back in any outputs and the CDKMetadata. + */ + 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); + + if (resourcesToImport) { + print('%s: creating stack for resource migration...', chalk.bold(stack.displayName)); + print('%s: importing resources into stack...', chalk.bold(stack.displayName)); + + await this.performResourceMigration(migrateDeployment, resourcesToImport, options); + + fs.rmSync('migrate.json'); + print('%s: applying CDKMetadata and Outputs to stack (if applicable)...', chalk.bold(stack.displayName)); + } + } + + private async tryGetResources(migrateDeployment: ResourceImporter) { + 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; + } + } catch (e) { + // Nothing to do + } + } + + /** + * Creates a new stack with just the resources to be migrated + */ + private async performResourceMigration(migrateDeployment: ResourceImporter, resourcesToImport: ResourcesToImport, options: DeployOptions) { + const startDeployTime = new Date().getTime(); + let elapsedDeployTime = 0; + + // Initial Deployment + await migrateDeployment.importResourcesFromMigrate(resourcesToImport, { + roleArn: options.roleArn, + toolkitStackName: options.toolkitStackName, + deploymentMethod: options.deploymentMethod, + usePreviousParameters: true, + progress: options.progress, + rollback: options.rollback, + }); + + elapsedDeployTime = new Date().getTime() - startDeployTime; + print('\n✨ Resource migration time: %ss\n', formatTime(elapsedDeployTime)); + } } export interface DiffOptions { diff --git a/packages/aws-cdk/lib/import.ts b/packages/aws-cdk/lib/import.ts index 76a42a8c18e23..99a54654ecfda 100644 --- a/packages/aws-cdk/lib/import.ts +++ b/packages/aws-cdk/lib/import.ts @@ -1,13 +1,23 @@ +import { DeployOptions } from '@aws-cdk/cloud-assembly-schema'; import * as cfnDiff from '@aws-cdk/cloudformation-diff'; import { ResourceDifference } from '@aws-cdk/cloudformation-diff'; import * as cxapi from '@aws-cdk/cx-api'; import * as chalk from 'chalk'; import * as fs from 'fs-extra'; import * as promptly from 'promptly'; -import { Deployments, DeployStackOptions } from './api/deployments'; +import { DeploymentMethod } from './api'; +import { Deployments } from './api/deployments'; import { ResourceIdentifierProperties, ResourcesToImport } from './api/util/cloudformation'; +import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; +import { Tag } from './cdk-toolkit'; import { error, print, success, warning } from './logging'; +export interface ImportDeploymentOptions extends DeployOptions { + deploymentMethod?: DeploymentMethod; + progress?: StackActivityProgress; + tags?: Tag[]; +} + /** * Set of parameters that uniquely identify a physical resource of a given type * for the import operation, example: @@ -112,14 +122,34 @@ export class ResourceImporter { * @param importMap Mapping from CDK construct tree path to physical resource import identifiers * @param options Options to pass to CloudFormation deploy operation */ - public async importResources(importMap: ImportMap, options: DeployStackOptions) { + public async importResourcesFromMap(importMap: ImportMap, options: ImportDeploymentOptions) { const resourcesToImport: ResourcesToImport = await this.makeResourcesToImport(importMap); const updatedTemplate = await this.currentTemplateWithAdditions(importMap.importResources); + await this.importResources(updatedTemplate, resourcesToImport, options); + } + + /** + * Based on the app and resources file generated by cdk migrate. Removes all items from the template that + * cannot be included in an import change-set for new stacks and performs the import operation, + * creating the new stack. + * + * @param resourcesToImport The mapping created by cdk migrate + * @param options Options to pass to CloudFormation deploy operation + */ + public async importResourcesFromMigrate(resourcesToImport: ResourcesToImport, options: ImportDeploymentOptions) { + const updatedTemplate = this.removeNonImportResources(); + + await this.importResources(updatedTemplate, resourcesToImport, options); + } + + private async importResources(overrideTemplate: any, resourcesToImport: ResourcesToImport, options: ImportDeploymentOptions) { try { const result = await this.cfn.deployStack({ + stack: this.stack, + deployName: this.stack.stackName, ...options, - overrideTemplate: updatedTemplate, + overrideTemplate, resourcesToImport, }); @@ -127,9 +157,9 @@ export class ResourceImporter { ? ' ✅ %s (no changes)' : ' ✅ %s'; - success('\n' + message, options.stack.displayName); + success('\n' + message, this.stack.displayName); } catch (e) { - error('\n ❌ %s failed: %s', chalk.bold(options.stack.displayName), e); + error('\n ❌ %s failed: %s', chalk.bold(this.stack.displayName), e); throw e; } } @@ -176,6 +206,13 @@ export class ResourceImporter { }; } + /** + * Resolves the environment of a stack. + */ + public async resolveEnvironment(): Promise { + return this.cfn.resolveEnvironment(this.stack); + } + /** * Get currently deployed template of the given stack (SINGLETON) * @@ -342,6 +379,17 @@ export class ResourceImporter { private describeResource(logicalId: string): string { return this.stack.template?.Resources?.[logicalId]?.Metadata?.['aws:cdk:path'] ?? logicalId; } + + /** + * Removes CDKMetadata and Outputs in the template so that only resources for importing are left. + * @returns template with import resources only + */ + private removeNonImportResources() { + const template = this.stack.template; + delete template.Resources.CDKMetadata; + delete template.Outputs; + return template; + } } /** diff --git a/packages/aws-cdk/test/import.test.ts b/packages/aws-cdk/test/import.test.ts index 0d73cef3949cd..6a4e95487a832 100644 --- a/packages/aws-cdk/test/import.test.ts +++ b/packages/aws-cdk/test/import.test.ts @@ -164,9 +164,7 @@ test('asks human to confirm automic import if identifier is in template', async }; // WHEN - await importer.importResources(importMap, { - stack: STACK_WITH_QUEUE, - }); + await importer.importResourcesFromMap(importMap, {}); expect(createChangeSetInput?.ResourcesToImport).toEqual([ { @@ -177,6 +175,50 @@ test('asks human to confirm automic import if identifier is in template', async ]); }); +test('importing resources from migrate strips cdk metadata and outputs', async () => { + // GIVEN + + const MyQueue = { + Type: 'AWS::SQS::Queue', + Properties: {}, + }; + const stack = { + stackName: 'StackWithQueue', + template: { + Resources: { + MyQueue, + CDKMetadata: { + Type: 'AWS::CDK::Metadata', + Properties: { + Analytics: 'exists', + }, + }, + }, + Outputs: { + Output: { + Description: 'There is an output', + Value: 'OutputValue', + }, + }, + }, + }; + + givenCurrentStack(stack.stackName, stack); + const importer = new ResourceImporter(testStack(stack), deployments); + const migrateMap = [{ + LogicalResourceId: 'MyQueue', + ResourceIdentifier: { QueueName: 'TheQueueName' }, + ResourceType: 'AWS::SQS::Queue', + }]; + + // WHEN + await importer.importResourcesFromMigrate(migrateMap, STACK_WITH_QUEUE.template); + + // THEN + expect(createChangeSetInput?.ResourcesToImport).toEqual(migrateMap); + expect(createChangeSetInput?.TemplateBody).toEqual('Resources:\n MyQueue:\n Type: AWS::SQS::Queue\n Properties: {}\n'); +}); + test('only use one identifier if multiple are in template', async () => { // GIVEN const stack = stackWithGlobalTable({ @@ -289,7 +331,7 @@ async function importTemplateFromClean(stack: ReturnType) { const importer = new ResourceImporter(stack, deployments); const { additions } = await importer.discoverImportableResources(); const importable = await importer.askForResourceIdentifiers(additions); - await importer.importResources(importable, { stack }); + await importer.importResourcesFromMap(importable, {}); return importable; } From 02a2fd29368bb802eeb193dd825298eae7a1c011 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Fri, 12 Jan 2024 10:19:29 -0800 Subject: [PATCH 2/4] incorrect format for getting the values from the outputs file --- .../cli-integ/tests/cli-integ-tests/cli.integtest.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index 2b6e4d251fb8b..fa0ce712b4867 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -1224,8 +1224,9 @@ integTest('test migrate deployment for app with localfile source in migrate.json }); const outputs = JSON.parse((await fs.readFile(outputsFile, { encoding: 'utf-8' })).toString()); - const queueName = outputs.QueueName; - const queueLogicalId = outputs.QueueLogicalId; + const stackName = fixture.fullStackName('migrate-stack'); + const queueName = outputs[stackName].QueueName; + const queueLogicalId = outputs[stackName].QueueLogicalId; fixture.log(`Created queue ${queueName} in stack ${fixture.fullStackName}`); // Write the migrate file based on the ID from step one, then deploy the app with migrate From f4cc8d391d2738e796b3d67e08a4468848e245a8 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Fri, 12 Jan 2024 12:20:08 -0800 Subject: [PATCH 3/4] resource identifier must be queueUrl, not queueName --- .../@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js | 5 +++++ .../cli-integ/tests/cli-integ-tests/cli.integtest.ts | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index 3d98923972acb..f8e94bc2920df 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -77,6 +77,11 @@ class MigrateStack extends cdk.Stack { new cdk.CfnOutput(this, 'QueueName', { value: queue.queueName, }); + + new cdk.CfnOutput(this, 'QueueUrl', { + value: queue.queueUrl, + }); + new cdk.CfnOutput(this, 'QueueLogicalId', { value: queue.node.defaultChild.logicalId, }); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index fa0ce712b4867..a6a92b0a7cde2 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -1226,14 +1226,15 @@ integTest('test migrate deployment for app with localfile source in migrate.json const outputs = JSON.parse((await fs.readFile(outputsFile, { encoding: 'utf-8' })).toString()); const stackName = fixture.fullStackName('migrate-stack'); const queueName = outputs[stackName].QueueName; + const queueUrl = outputs[stackName].QueueUrl; const queueLogicalId = outputs[stackName].QueueLogicalId; - fixture.log(`Created queue ${queueName} in stack ${fixture.fullStackName}`); + fixture.log(`Created queue ${queueUrl} in stack ${fixture.fullStackName}`); // Write the migrate file based on the ID from step one, then deploy the app with migrate const migrateFile = path.join(fixture.integTestDir, 'migrate.json'); await fs.writeFile( migrateFile, JSON.stringify( - { Source: 'localfile', Resources: [{ ResourceType: 'AWS::SQS::Queue', LogicalResourceId: queueLogicalId, ResourceIdentifier: { QueueName: queueName } }] }, + { Source: 'localfile', Resources: [{ ResourceType: 'AWS::SQS::Queue', LogicalResourceId: queueLogicalId, ResourceIdentifier: { QueueUrl: queueUrl } }] }, ), { encoding: 'utf-8' }, ); From 05a7e7cdb99f2a4d14b19503d0fa6f13d4593278 Mon Sep 17 00:00:00 2001 From: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com> Date: Fri, 12 Jan 2024 14:17:51 -0800 Subject: [PATCH 4/4] update order of functions --- packages/aws-cdk/lib/cdk-toolkit.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index f1ed8872cf959..79d11364f1605 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -897,20 +897,6 @@ export class CdkToolkit { } } - private async tryGetResources(migrateDeployment: ResourceImporter) { - 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; - } - } catch (e) { - // Nothing to do - } - } - /** * Creates a new stack with just the resources to be migrated */ @@ -931,6 +917,20 @@ export class CdkToolkit { elapsedDeployTime = new Date().getTime() - startDeployTime; print('\n✨ Resource migration time: %ss\n', formatTime(elapsedDeployTime)); } + + private async tryGetResources(migrateDeployment: ResourceImporter) { + 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; + } + } catch (e) { + // Nothing to do + } + } } export interface DiffOptions {