Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): support --import-existing-resources flag in cdk diff #32831

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var constructs = require('constructs');
if (process.env.PACKAGE_LAYOUT_VERSION === '1') {
var cdk = require('@aws-cdk/core');
var ec2 = require('@aws-cdk/aws-ec2');
var ecr = require('@aws-cdk/aws-ecr');
var ecs = require('@aws-cdk/aws-ecs');
var s3 = require('@aws-cdk/aws-s3');
var ssm = require('@aws-cdk/aws-ssm');
Expand All @@ -20,6 +21,7 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') {
DefaultStackSynthesizer,
LegacyStackSynthesizer,
aws_ec2: ec2,
aws_ecr: ecr,
aws_ecs: ecs,
aws_sso: sso,
aws_s3: s3,
Expand Down Expand Up @@ -787,6 +789,21 @@ class AppSyncHotswapStack extends cdk.Stack {
}
}

class SimplifiedImportStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
// create a random resource because we cannot deploy a stack without any resources.
new sns.Topic(this, 'RandomTopic');

if (process.env.IMPORTED_REPOSITORY_NAME) {
new ecr.Repository(this, 'ImportedRepository', {
repositoryName: process.env.IMPORTED_REPOSITORY_NAME,
removalPolicy: cdk.RemovalPolicy.RETAIN,
});
}
}
}

class MetadataStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
Expand Down Expand Up @@ -836,6 +853,8 @@ switch (stackSet) {
// This stack is used to test diff with large templates by creating a role with a ton of metadata
new IamRolesStack(app, `${stackPrefix}-iam-roles`);

new SimplifiedImportStack(app, `${stackPrefix}-simplified-import`);

if (process.env.ENABLE_VPC_TESTING == 'IMPORT') {
// this stack performs a VPC lookup so we gate synth
const env = { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
UpdateStackCommand,
waitUntilStackUpdateComplete,
} from '@aws-sdk/client-cloudformation';
import { CreateRepositoryCommand, DeleteRepositoryCommand } from '@aws-sdk/client-ecr';
import { DescribeServicesCommand } from '@aws-sdk/client-ecs';
import {
CreateRoleCommand,
Expand Down Expand Up @@ -1523,6 +1524,31 @@ integTest(
}),
);

integTest('cdk diff --import-existing-resources print diff of imported resources', withDefaultFixture(async (fixture) => {
const repositoryName = `${fixture.stackNamePrefix}-repository`;
await fixture.aws.ecr.send(new CreateRepositoryCommand({ repositoryName: repositoryName }));

try {
await fixture.cdkDeploy('simplified-import', {
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The integ-test steps:

  1. Create an AWS resource (e.g. ECR repository) to be imported
  2. Deploy a stack without importing resource (because cdk diff only works for an existing stack)
  3. Run cdk diff with the stack template that contains the resource to be imported
  4. assert cdk diff output
  5. destroy stack
  6. delete the AWS resource created at step 1


const diff = await fixture.cdk(['diff --import-existing-resources', fixture.fullStackName('simplified-import')], {
modEnv: {
IMPORTED_REPOSITORY_NAME: repositoryName,
},
});
const plainTextOutput = diff.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
expect(plainTextOutput).toContain('[←] AWS::ECR::Repository ImportedRepository ImportedRepository1921D649 import');
} finally {
await fixture.cdkDestroy('simplified-import');
await fixture.aws.ecr.send(
new DeleteRepositoryCommand({
repositoryName,
}),
);
}
}));

integTest(
'deploy stack with docker asset',
withDefaultFixture(async (fixture) => {
Expand Down
10 changes: 9 additions & 1 deletion packages/aws-cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,15 @@ This automatically imports resources in your CDK application that represent
unmanaged resources in your account. It reduces the manual effort of import operations and
avoids deployment failures due to naming conflicts with unmanaged resources in your account.

Use the `--method=prepare-change-set` flag to review which resources are imported or not before deploying a changeset.
You can also use `cdk diff` to review which resources will be imported:

```console
$ cdk diff --import-existing-resources
```

Please note that `cdk diff --import-existing-resources` only works when the stack has already been created; it will not work for a new stack that does not exist yet.

You can also use the `--method=prepare-change-set` flag to review which resources are imported or not before deploying a changeset.
You can inspect the change set created by CDK from the management console or other external tools.

```console
Expand Down
4 changes: 4 additions & 0 deletions packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ export type PrepareChangeSetOptions = {
stream: NodeJS.WritableStream;
parameters: { [name: string]: string | undefined };
resourcesToImport?: ResourcesToImport;
importExistingResources?: boolean;
}

export type CreateChangeSetOptions = {
Expand All @@ -338,6 +339,7 @@ export type CreateChangeSetOptions = {
bodyParameter: TemplateBodyParameter;
parameters: { [name: string]: string | undefined };
resourcesToImport?: ResourceToImport[];
importExistingResources?: boolean;
role?: string;
};

Expand Down Expand Up @@ -419,6 +421,7 @@ async function uploadBodyParameterAndCreateChangeSet(
bodyParameter,
parameters: options.parameters,
resourcesToImport: options.resourcesToImport,
importExistingResources: options.importExistingResources,
role: executionRoleArn,
});
} catch (e: any) {
Expand Down Expand Up @@ -476,6 +479,7 @@ async function createChangeSet(options: CreateChangeSetOptions): Promise<Describ
TemplateBody: options.bodyParameter.TemplateBody,
Parameters: stackParams.apiParameters,
ResourcesToImport: options.resourcesToImport,
ImportExistingResources: options.importExistingResources,
RoleARN: options.role,
Capabilities: ['CAPABILITY_IAM', 'CAPABILITY_NAMED_IAM', 'CAPABILITY_AUTO_EXPAND'],
});
Expand Down
15 changes: 15 additions & 0 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@
let diffs = 0;
const parameterMap = buildParameterMap(options.parameters);

if (options.importExistingResources && !(options.changeSet ?? true)) {
throw new ToolkitError('--import-existing-resources cannot be enabled without changeSet');

Check warning on line 164 in packages/aws-cdk/lib/cdk-toolkit.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/cdk-toolkit.ts#L164

Added line #L164 was not covered by tests
}

if (options.templatePath !== undefined) {
// Compare single stack against fixed template
if (stacks.stackCount !== 1) {
Expand Down Expand Up @@ -220,9 +224,13 @@
sdkProvider: this.props.sdkProvider,
parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]),
resourcesToImport,
importExistingResources: options.importExistingResources,
stream,
});
} else {
if (options.importExistingResources) {
throw new ToolkitError(`--import-existing-resources diff cannot be enabled for a stack that does not exist yet. StackName: ${stack.stackName}`);
}
Copy link
Contributor Author

@tmokmss tmokmss Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to #29492, cdk diff will not create changeset when the target stack does not exist. Without changeset, CDK cannot know which resource will be imported, so I just throw an error instead.

debug(
`the stack '${stack.stackName}' has not been deployed to CloudFormation or describeStacks call failed, skipping changeset creation.`,
);
Expand Down Expand Up @@ -1469,6 +1477,13 @@
* @default true
*/
changeSet?: boolean;

/**
* Indicates if the stack set imports resources that already exist.
*
* @default - false
*/
readonly importExistingResources?: boolean;
}

interface CfnDeployOptions {
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
compareAgainstProcessedTemplate: args.processed,
quiet: args.quiet,
changeSet: args['change-set'],
importExistingResources: args.importExistingResources,
toolkitStackName: toolkitStackName,
});

Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ export async function makeConfig(): Promise<CliConfig> {
'processed': { type: 'boolean', desc: 'Whether to compare against the template with Transforms already processed', default: false },
'quiet': { type: 'boolean', alias: 'q', desc: 'Do not print stack name and default message when there is no diff to stdout', default: false },
'change-set': { type: 'boolean', alias: 'changeset', 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 },
'import-existing-resources': { type: 'boolean', desc: 'Indicates if the stack set imports resources that already exist.', default: false },
},
},
metadata: {
Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk/lib/convert-to-user-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ export function convertYargsToUserInput(args: any): UserInput {
processed: args.processed,
quiet: args.quiet,
changeSet: args.changeSet,
importExistingResources: args.importExistingResources,
STACKS: args.STACKS,
};
break;
Expand Down Expand Up @@ -396,6 +397,7 @@ export function convertConfigToUserInput(config: any): UserInput {
processed: config.diff?.processed,
quiet: config.diff?.quiet,
changeSet: config.diff?.changeSet,
importExistingResources: config.diff?.importExistingResources,
};
const metadataOptions = {};
const acknowledgeOptions = {};
Expand Down
5 changes: 5 additions & 0 deletions packages/aws-cdk/lib/parse-command-line-arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,11 @@ export function parseCommandLineArguments(args: Array<string>): any {
type: 'boolean',
alias: 'changeset',
desc: 'Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role.',
})
.option('import-existing-resources', {
default: false,
type: 'boolean',
desc: 'Indicates if the stack set imports resources that already exist.',
}),
)
.command('metadata [STACK]', 'Returns all metadata associated with this stack')
Expand Down
7 changes: 7 additions & 0 deletions packages/aws-cdk/lib/user-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,13 @@ export interface DiffOptions {
*/
readonly changeSet?: boolean;

/**
* Indicates if the stack set imports resources that already exist.
*
* @default - false
*/
readonly importExistingResources?: boolean;

/**
* Positional argument for diff
*/
Expand Down
39 changes: 38 additions & 1 deletion packages/aws-cdk/test/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ describe('imports', () => {
fs.rmSync('migrate.json');
});

test('imports render correctly for a nonexistant stack without creating a changeset', async () => {
test('imports render correctly for a nonexistent stack without creating a changeset', async () => {
// GIVEN
const buffer = new StringWritable();

Expand Down Expand Up @@ -248,6 +248,26 @@ Resources
expect(buffer.data.trim()).toContain('✨ Number of stacks with differences: 1');
expect(exitCode).toBe(0);
});

test('--import-existing-resources is propagated to createChangeSet', async () => {
// GIVEN
const buffer = new StringWritable();
cloudFormation.stackExists = jest.fn().mockReturnValue(Promise.resolve(true));

// WHEN
const exitCode = await toolkit.diff({
stackNames: ['A'],
stream: buffer,
changeSet: true,
importExistingResources: true,
});

// THEN
expect(exitCode).toBe(0);
expect(createDiffChangeSet).toHaveBeenCalledWith(expect.objectContaining({
importExistingResources: true,
}));
});
});

describe('non-nested stacks', () => {
Expand Down Expand Up @@ -561,6 +581,23 @@ describe('stack exists checks', () => {
expect(cloudFormation.stackExists).not.toHaveBeenCalled();
});

test('diff throws error when --import-existing-resources is passed but stack does not exist', async () => {
// GIVEN
const buffer = new StringWritable();

// WHEN
await expect(() =>
toolkit.diff({
stackNames: ['A', 'A'],
stream: buffer,
fail: false,
quiet: true,
changeSet: true,
importExistingResources: true,
}),
).rejects.toThrow(/diff cannot be enabled for a stack that does not exist yet/);
});

test('diff falls back to classic diff when stack does not exist', async () => {
// GIVEN
const buffer = new StringWritable();
Expand Down
Loading