Skip to content

Commit f603c97

Browse files
fix(core): cdk diff on large templates fails when passing in toolkitStackName and qualifier (#31636)
Closes #29179 ### Reason for this change If your account is bootstrapped with a custom `toolkitStackName` or `qualifier`, then running `cdk diff` does not work for large diff templates of over 50 KiB in size. ### Description of changes The `toolkitStackName` was not passed down all the way through the `cdk diff` code path - this PR fixes the issue by adding the optional `toolkitStackName` property to the `DiffOptions` interface in `cdk-toolkit.ts` and passing that value all the way through the diff code path to the `uploadBodyParameterAndCreateChangeSet` function in `cloudformation.ts`, where the template asset is built and published. ### Description of how you validated changes I created a CDK app locally and ran the commands (exactly following the reproduction steps from the original issue) using the CLI build from this PR and was able to successfully run `cdk diff` without needing to pass in the `--toolkit-stack-name` flag. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 5e7e61f commit f603c97

File tree

6 files changed

+65
-5
lines changed

6 files changed

+65
-5
lines changed

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,11 @@ interface CommonCdkBootstrapCommandOptions {
258258
* @default - none
259259
*/
260260
readonly tags?: string;
261+
262+
/**
263+
* @default - the default CDK qualifier
264+
*/
265+
readonly qualifier?: string;
261266
}
262267

263268
export interface CdkLegacyBootstrapCommandOptions extends CommonCdkBootstrapCommandOptions {
@@ -408,7 +413,7 @@ export class TestFixture extends ShellHelper {
408413
if (options.bootstrapBucketName) {
409414
args.push('--bootstrap-bucket-name', options.bootstrapBucketName);
410415
}
411-
args.push('--qualifier', this.qualifier);
416+
args.push('--qualifier', options.qualifier ?? this.qualifier);
412417
if (options.cfnExecutionPolicy) {
413418
args.push('--cloudformation-execution-policies', options.cfnExecutionPolicy);
414419
}

packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,9 +438,17 @@ class IamRolesStack extends cdk.Stack {
438438
// Environment variabile is used to create a bunch of roles to test
439439
// that large diff templates are uploaded to S3 to create the changeset.
440440
for(let i = 1; i <= Number(process.env.NUMBER_OF_ROLES) ; i++) {
441-
new iam.Role(this, `Role${i}`, {
441+
const role = new iam.Role(this, `Role${i}`, {
442442
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
443443
});
444+
const cfnRole = role.node.defaultChild;
445+
446+
// For any extra IAM roles created, add a ton of metadata so that the template size is > 50 KiB.
447+
if (i > 1) {
448+
for(let i = 1; i <= 30 ; i++) {
449+
cfnRole.addMetadata('a'.repeat(1000), 'v');
450+
}
451+
}
444452
}
445453
}
446454
}
@@ -792,6 +800,7 @@ switch (stackSet) {
792800

793801
new LambdaStack(app, `${stackPrefix}-lambda`);
794802

803+
// This stack is used to test diff with large templates by creating a role with a ton of metadata
795804
new IamRolesStack(app, `${stackPrefix}-iam-roles`);
796805

797806
if (process.env.ENABLE_VPC_TESTING == 'IMPORT') {

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,18 +1034,18 @@ integTest(
10341034
integTest(
10351035
'cdk diff with large changeset does not fail',
10361036
withDefaultFixture(async (fixture) => {
1037-
// GIVEN - small initial stack with only ane IAM role
1037+
// GIVEN - small initial stack with only one IAM role
10381038
await fixture.cdkDeploy('iam-roles', {
10391039
modEnv: {
10401040
NUMBER_OF_ROLES: '1',
10411041
},
10421042
});
10431043

1044-
// WHEN - adding 200 roles to the same stack to create a large diff
1044+
// WHEN - adding an additional role with a ton of metadata to create a large diff
10451045
const diff = await fixture.cdk(['diff', fixture.fullStackName('iam-roles')], {
10461046
verbose: true,
10471047
modEnv: {
1048-
NUMBER_OF_ROLES: '200',
1048+
NUMBER_OF_ROLES: '2',
10491049
},
10501050
});
10511051

@@ -1055,6 +1055,40 @@ integTest(
10551055
}),
10561056
);
10571057

1058+
integTest('cdk diff with large changeset and custom toolkit stack name and qualifier does not fail', withoutBootstrap(async (fixture) => {
1059+
// Bootstrapping with custom toolkit stack name and qualifier
1060+
const qualifier = 'abc1111';
1061+
const toolkitStackName = 'custom-stack2';
1062+
await fixture.cdkBootstrapModern({
1063+
verbose: true,
1064+
toolkitStackName: toolkitStackName,
1065+
qualifier: qualifier,
1066+
});
1067+
1068+
// Deploying small initial stack with only one IAM role
1069+
await fixture.cdkDeploy('iam-roles', {
1070+
modEnv: {
1071+
NUMBER_OF_ROLES: '1',
1072+
},
1073+
options: [
1074+
'--toolkit-stack-name', toolkitStackName,
1075+
'--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`,
1076+
],
1077+
});
1078+
1079+
// WHEN - adding a role with a ton of metadata to create a large diff
1080+
const diff = await fixture.cdk(['diff', '--toolkit-stack-name', toolkitStackName, '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, fixture.fullStackName('iam-roles')], {
1081+
verbose: true,
1082+
modEnv: {
1083+
NUMBER_OF_ROLES: '2',
1084+
},
1085+
});
1086+
1087+
// Assert that the CLI assumes the file publishing role:
1088+
expect(diff).toMatch(/Assuming role .*file-publishing-role/);
1089+
expect(diff).toContain('success: Published');
1090+
}));
1091+
10581092
integTest(
10591093
'cdk diff --security-only successfully outputs sso-permission-set-without-managed-policy information',
10601094
withDefaultFixture(async (fixture) => {

packages/aws-cdk/lib/api/util/cloudformation.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ export type PrepareChangeSetOptions = {
305305
sdkProvider: SdkProvider;
306306
stream: NodeJS.WritableStream;
307307
parameters: { [name: string]: string | undefined };
308+
toolkitStackName?: string;
308309
resourcesToImport?: ResourcesToImport;
309310
}
310311

@@ -375,9 +376,11 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
375376
for (const entry of file_entries) {
376377
await options.deployments.buildSingleAsset(artifact, assetManifest, entry, {
377378
stack: options.stack,
379+
toolkitStackName: options.toolkitStackName,
378380
});
379381
await options.deployments.publishSingleAsset(assetManifest, entry, {
380382
stack: options.stack,
383+
toolkitStackName: options.toolkitStackName,
381384
});
382385
}
383386
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ export class CdkToolkit {
186186
parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]),
187187
resourcesToImport,
188188
stream,
189+
toolkitStackName: options.toolkitStackName,
189190
});
190191
} else {
191192
debug(`the stack '${stack.stackName}' has not been deployed to CloudFormation or describeStacks call failed, skipping changeset creation.`);
@@ -1062,6 +1063,13 @@ export interface DiffOptions {
10621063
*/
10631064
stackNames: string[];
10641065

1066+
/**
1067+
* Name of the toolkit stack, if not the default name
1068+
*
1069+
* @default 'CDKToolkit'
1070+
*/
1071+
readonly toolkitStackName?: string;
1072+
10651073
/**
10661074
* Only select the given stack
10671075
*

packages/aws-cdk/lib/cli.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
505505
compareAgainstProcessedTemplate: args.processed,
506506
quiet: args.quiet,
507507
changeSet: args['change-set'],
508+
toolkitStackName: toolkitStackName,
508509
});
509510

510511
case 'bootstrap':

0 commit comments

Comments
 (0)