diff --git a/packages/aws-cdk/lib/import.ts b/packages/aws-cdk/lib/import.ts index 0a5795313fbc0..f3caf736373be 100644 --- a/packages/aws-cdk/lib/import.ts +++ b/packages/aws-cdk/lib/import.ts @@ -9,18 +9,18 @@ import { ResourceIdentifierProperties, ResourcesToImport } from './api/util/clou import { error, print, success, warning } from './logging'; /** - * Parameters that uniquely identify a physical resource of a given type + * Set of parameters that uniquely identify a physical resource of a given type * for the import operation, example: * * ``` * { - * "AWS::S3::Bucket": ["BucketName"], - * "AWS::IAM::Role": ["RoleName"], - * "AWS::EC2::VPC": ["VpcId"] + * "AWS::S3::Bucket": [["BucketName"]], + * "AWS::DynamoDB::GlobalTable": [["TableName"], ["TableArn"], ["TableStreamArn"]], + * "AWS::Route53::KeySigningKey": [["HostedZoneId", "Name"]], * } * ``` */ -export type ResourceIdentifiers = { [resourceType: string]: string[] }; +export type ResourceIdentifiers = { [resourceType: string]: string[][] }; /** * Mapping of CDK resources (L1 constructs) to physical resources to be imported @@ -225,12 +225,21 @@ export class ResourceImporter { const resourceIdentifierSummaries = await this.cfn.resourceIdentifierSummaries(this.stack, this.options.toolkitStackName); for (const summary of resourceIdentifierSummaries) { if ('ResourceType' in summary && summary.ResourceType && 'ResourceIdentifiers' in summary && summary.ResourceIdentifiers) { - ret[summary.ResourceType] = summary.ResourceIdentifiers; + ret[summary.ResourceType] = (summary.ResourceIdentifiers ?? [])?.map(x => x.split(',')); } } return ret; } + /** + * Ask for the importable identifier for the given resource + * + * There may be more than one identifier under which a resource can be imported. The `import` + * operation needs exactly one of them. + * + * - If we can get one from the template, we will use one. + * - Otherwise, we will ask the user for one of them. + */ private async askForResourceIdentifier( resourceIdentifiers: ResourceIdentifiers, chg: ImportableResource, @@ -244,45 +253,83 @@ export class ResourceImporter { return undefined; } - const idProps = resourceIdentifiers[resourceType]; - const resourceProps = chg.resourceDefinition.Properties ?? {}; + const idPropSets = resourceIdentifiers[resourceType]; - const fixedIdProps = idProps.filter(p => resourceProps[p]); - const fixedIdInput: ResourceIdentifierProperties = Object.fromEntries(fixedIdProps.map(p => [p, resourceProps[p]])); + // Retain only literal strings: strip potential CFN intrinsics + const resourceProps = Object.fromEntries(Object.entries(chg.resourceDefinition.Properties ?? {}) + .filter(([_, v]) => typeof v === 'string')) as Record; - const missingIdProps = idProps.filter(p => !resourceProps[p]); + // Find property sets that are fully satisfied in the template, ask the user to confirm them + const satisfiedPropSets = idPropSets.filter(ps => ps.every(p => resourceProps[p])); + for (const satisfiedPropSet of satisfiedPropSets) { + const candidateProps = Object.fromEntries(satisfiedPropSet.map(p => [p, resourceProps[p]])); + const displayCandidateProps = fmtdict(candidateProps); - if (missingIdProps.length === 0) { - // We can auto-import this, but ask the user to confirm - const props = fmtdict(fixedIdInput); - - if (!await promptly.confirm( - `${chalk.blue(resourceName)} (${resourceType}): import with ${chalk.yellow(props)} (yes/no) [default: yes]? `, + if (await promptly.confirm( + `${chalk.blue(resourceName)} (${resourceType}): import with ${chalk.yellow(displayCandidateProps)} (yes/no) [default: yes]? `, { default: 'yes' }, )) { - print(chalk.grey(`Skipping import of ${resourceName}`)); - return undefined; + return candidateProps; } } - // Ask the user to provide missing props - const userInput: ResourceIdentifierProperties = {}; - for (const missingIdProp of missingIdProps) { - const response = (await promptly.prompt( - `${chalk.blue(resourceName)} (${resourceType}): enter ${chalk.blue(missingIdProp)} to import (empty to skip):`, - { default: '', trim: true }, - )); - if (!response) { - print(chalk.grey(`Skipping import of ${resourceName}`)); - return undefined; + // If we got here and the user rejected any available identifiers, then apparently they don't want the resource at all + if (satisfiedPropSets.length > 0) { + print(chalk.grey(`Skipping import of ${resourceName}`)); + return undefined; + } + + // We cannot auto-import this, ask the user for one of the props + // The only difference between these cases is what we print: for multiple properties, we print a preamble + const prefix = `${chalk.blue(resourceName)} (${resourceType})`; + let preamble; + let promptPattern; + if (idPropSets.length > 1) { + preamble = `${prefix}: enter one of ${idPropSets.map(x => chalk.blue(x.join('+'))).join(', ')} to import (all empty to skip)`; + promptPattern = `${prefix}: enter %`; + } else { + promptPattern = `${prefix}: enter %`; + } + + // Do the input loop here + if (preamble) { + print(preamble); + } + for (const idProps of idPropSets) { + const input: Record = {}; + for (const idProp of idProps) { + // If we have a value from the template, use it as default. This will only be a partial + // identifier if present, otherwise we would have done the import already above. + const defaultValue = typeof resourceProps[idProp] ?? ''; + + const prompt = [ + promptPattern.replace(/%/, chalk.blue(idProp)), + defaultValue + ? `[${defaultValue}]` + : '(empty to skip)', + ].join(' ') + ':'; + const response = await promptly.prompt(prompt, + { default: defaultValue, trim: true }, + ); + + if (!response) { + break; + } + + input[idProp] = response; + // Also stick this property into 'resourceProps', so that it may be reused by a subsequent question + // (for a different compound identifier that involves the same property). Just a small UX enhancement. + resourceProps[idProp] = response; + } + + // If the user gave inputs for all values, we are complete + if (Object.keys(input).length === idProps.length) { + return input; } - userInput[missingIdProp] = response; } - return { - ...fixedIdInput, - ...userInput, - }; + print(chalk.grey(`Skipping import of ${resourceName}`)); + return undefined; } /** @@ -364,4 +411,4 @@ function addDefaultDeletionPolicy(resource: any): any { export interface DiscoverImportableResourcesResult { readonly additions: ImportableResource[]; readonly hasNonAdditions: boolean; -} \ No newline at end of file +} diff --git a/packages/aws-cdk/test/import.test.ts b/packages/aws-cdk/test/import.test.ts index ec970df533daf..9bbbc7b98a24f 100644 --- a/packages/aws-cdk/test/import.test.ts +++ b/packages/aws-cdk/test/import.test.ts @@ -17,31 +17,53 @@ const promptlyPrompt = promptly.prompt as jest.Mock; let createChangeSetInput: AWS.CloudFormation.CreateChangeSetInput | undefined; -const STACK_WITH_QUEUE = testStack({ - stackName: 'StackWithQueue', - template: { - Resources: { - MyQueue: { - Type: 'AWS::SQS::Queue', - Properties: {}, +function stackWithQueue(props: Record) { + return testStack({ + stackName: 'StackWithQueue', + template: { + Resources: { + MyQueue: { + Type: 'AWS::SQS::Queue', + Properties: props, + }, }, }, - }, + }); +} + +const STACK_WITH_QUEUE = stackWithQueue({}); + +const STACK_WITH_NAMED_QUEUE = stackWithQueue({ + QueueName: 'TheQueueName', }); -const STACK_WITH_NAMED_QUEUE = testStack({ - stackName: 'StackWithQueue', - template: { - Resources: { - MyQueue: { - Type: 'AWS::SQS::Queue', - Properties: { - QueueName: 'TheQueueName', +function stackWithGlobalTable(props: Record) { + return testStack({ + stackName: 'StackWithTable', + template: { + Resources: { + MyTable: { + Type: 'AWS::DynamoDB::GlobalTable', + Properties: props, }, }, }, - }, -}); + }); +} + +function stackWithKeySigningKey(props: Record) { + return testStack({ + stackName: 'StackWithKSK', + template: { + Resources: { + MyKSK: { + Type: 'AWS::Route53::KeySigningKey', + Properties: props, + }, + }, + }, + }); +} let sdkProvider: MockSdkProvider; let deployments: CloudFormationDeployments; @@ -154,6 +176,122 @@ test('asks human to confirm automic import if identifier is in template', async ]); }); +test('only use one identifier if multiple are in template', async () => { + // GIVEN + const stack = stackWithGlobalTable({ + TableName: 'TheTableName', + TableArn: 'ThisFieldDoesntExistInReality', + TableStreamArn: 'NorDoesThisOne', + }); + + // WHEN + promptlyConfirm.mockResolvedValue(true); // Confirm yes/no + await importTemplateFromClean(stack); + + // THEN + expect(createChangeSetInput?.ResourcesToImport).toEqual([ + { + LogicalResourceId: 'MyTable', + ResourceIdentifier: { TableName: 'TheTableName' }, + ResourceType: 'AWS::DynamoDB::GlobalTable', + }, + ]); +}); + +test('only ask user for one identifier if multiple possible ones are possible', async () => { + // GIVEN -- no identifiers in template, so ask user + const stack = stackWithGlobalTable({}); + + // WHEN + promptlyPrompt.mockResolvedValue('Banana'); + const importable = await importTemplateFromClean(stack); + + // THEN -- only asked once + expect(promptlyPrompt).toHaveBeenCalledTimes(1); + expect(importable.resourceMap).toEqual({ + MyTable: { TableName: 'Banana' }, + }); +}); + +test('ask identifier if the value in the template is a CFN intrinsic', async () => { + // GIVEN -- identifier in template is a CFN intrinsic so it doesn't count + const stack = stackWithQueue({ + QueueName: { Ref: 'SomeParam' }, + }); + + // WHEN + promptlyPrompt.mockResolvedValue('Banana'); + const importable = await importTemplateFromClean(stack); + + // THEN + expect(importable.resourceMap).toEqual({ + MyQueue: { QueueName: 'Banana' }, + }); +}); + +test('take compound identifiers from the template if found', async () => { + // GIVEN + const stack = stackWithKeySigningKey({ + HostedZoneId: 'z-123', + Name: 'KeyName', + }); + + // WHEN + promptlyConfirm.mockResolvedValue(true); + await importTemplateFromClean(stack); + + // THEN + expect(createChangeSetInput?.ResourcesToImport).toEqual([ + { + LogicalResourceId: 'MyKSK', + ResourceIdentifier: { HostedZoneId: 'z-123', Name: 'KeyName' }, + ResourceType: 'AWS::Route53::KeySigningKey', + }, + ]); +}); + +test('ask user for compound identifiers if not found', async () => { + // GIVEN + const stack = stackWithKeySigningKey({}); + + // WHEN + promptlyPrompt.mockReturnValue('Banana'); + await importTemplateFromClean(stack); + + // THEN + expect(createChangeSetInput?.ResourcesToImport).toEqual([ + { + LogicalResourceId: 'MyKSK', + ResourceIdentifier: { HostedZoneId: 'Banana', Name: 'Banana' }, + ResourceType: 'AWS::Route53::KeySigningKey', + }, + ]); +}); + +test('do not ask for second part of compound identifier if the user skips the first', async () => { + // GIVEN + const stack = stackWithKeySigningKey({}); + + // WHEN + promptlyPrompt.mockReturnValue(''); + const importMap = await importTemplateFromClean(stack); + + // THEN + expect(importMap.resourceMap).toEqual({}); +}); + +/** + * Do a full import cycle with the given stack template + */ +async function importTemplateFromClean(stack: ReturnType) { + givenCurrentStack(stack.stackName, { Resources: {} }); + const importer = new ResourceImporter(stack, deployments); + const { additions } = await importer.discoverImportableResources(); + const importable = await importer.askForResourceIdentifiers(additions); + await importer.importResources(importable, { stack }); + return importable; +} + function givenCurrentStack(stackName: string, template: any) { sdkProvider.stubCloudFormation({ describeStacks() { @@ -181,6 +319,14 @@ function givenCurrentStack(stackName: string, template: any) { ResourceType: 'AWS::SQS::Queue', ResourceIdentifiers: ['QueueName'], }, + { + ResourceType: 'AWS::DynamoDB::GlobalTable', + ResourceIdentifiers: ['TableName', 'TableArn', 'TableStreamArn'], + }, + { + ResourceType: 'AWS::Route53::KeySigningKey', + ResourceIdentifiers: ['HostedZoneId,Name'], + }, ], }; },