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 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
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 @@ export class CdkToolkit {
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');
}

if (options.templatePath !== undefined) {
// Compare single stack against fixed template
if (stacks.stackCount !== 1) {
Expand Down Expand Up @@ -220,9 +224,13 @@ export class CdkToolkit {
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 @@ export interface DiffOptions {
* @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 @@ -105,6 +105,26 @@ describe('fixed template', () => {
✨ Number of stacks with differences: 1
`);
});

test('--import-existing-resources is propagated to createChangeSet', async () => {
// GIVEN
const buffer = new StringWritable();
const createDiffChangeSet = jest.spyOn(cfn, 'createDiffChangeSet');

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

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

describe('imports', () => {
Expand Down Expand Up @@ -196,7 +216,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 @@ -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