From e1a1253f4cf35e56bb8e796d6e13b10f8faf45ff Mon Sep 17 00:00:00 2001 From: Sumu Date: Thu, 3 Oct 2024 11:38:25 -0400 Subject: [PATCH 1/6] WIP: initial integ test Signed-off-by: Sumu --- .../tests/cli-integ-tests/cli.integtest.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) 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 8efebdec07875..95ec9edb1d51c 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 @@ -1056,6 +1056,41 @@ integTest( }), ); +integTest('cdk diff with large changeset and custom toolkit stack name and qualifier does not fail', withoutBootstrap(async (fixture) => { + // Bootstrapping with custom toolkit stack name and qualifier + const qualifier = 'abc1234'; + await fixture.cdkBootstrapModern({ + verbose: true, + toolkitStackName: 'custom-stack', + qualifier: qualifier, + }); + + // Deploying small initial stack with only ane IAM role + await fixture.cdkDeploy('iam-roles', { + modEnv: { + NUMBER_OF_ROLES: '1', + }, + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, + ], + }); + + // WHEN - adding 200 roles to the same stack to create a large diff + const diff = await fixture.cdk(['diff', fixture.fullStackName('iam-roles')], { + verbose: true, + modEnv: { + NUMBER_OF_ROLES: '200', + }, + options: [ + '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, + ], + }); + + // Assert that the CLI assumes the file publishing role: + expect(diff).toMatch(/Assuming role .*file-publishing-role/); + expect(diff).toContain('success: Published'); +})); + integTest( 'cdk diff --security-only successfully outputs sso-permission-set-without-managed-policy information', withDefaultFixture(async (fixture) => { From dd5dde407311e6bb1365e5daa3943ef0d0edc8fd Mon Sep 17 00:00:00 2001 From: Sumu Date: Thu, 3 Oct 2024 11:58:42 -0400 Subject: [PATCH 2/6] add qualifier to bootstrapoptions Signed-off-by: Sumu --- packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts b/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts index c28f5eccb4d4b..fca2d76ac74e4 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts @@ -273,6 +273,11 @@ interface CommonCdkBootstrapCommandOptions { * @default - none */ readonly tags?: string; + + /** + * @default - the default CDK qualifier + */ + readonly qualifier?: string; } export interface CdkLegacyBootstrapCommandOptions extends CommonCdkBootstrapCommandOptions { @@ -423,7 +428,7 @@ export class TestFixture extends ShellHelper { if (options.bootstrapBucketName) { args.push('--bootstrap-bucket-name', options.bootstrapBucketName); } - args.push('--qualifier', this.qualifier); + args.push('--qualifier', options.qualifier ?? this.qualifier); if (options.cfnExecutionPolicy) { args.push('--cloudformation-execution-policies', options.cfnExecutionPolicy); } From d5973955dfe3acc5d0830d8d87e74a95e99bd022 Mon Sep 17 00:00:00 2001 From: Sumu Date: Thu, 3 Oct 2024 15:21:33 -0400 Subject: [PATCH 3/6] fix: pass toolkitStackName down to publishAsset within diff code path Signed-off-by: Sumu --- packages/aws-cdk/lib/api/util/cloudformation.ts | 3 +++ packages/aws-cdk/lib/cdk-toolkit.ts | 8 ++++++++ packages/aws-cdk/lib/cli.ts | 1 + 3 files changed, 12 insertions(+) diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts index 27822ca4c8378..85a2742449da0 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation.ts @@ -305,6 +305,7 @@ export type PrepareChangeSetOptions = { sdkProvider: SdkProvider; stream: NodeJS.WritableStream; parameters: { [name: string]: string | undefined }; + toolkitStackName?: string; resourcesToImport?: ResourcesToImport; } @@ -375,9 +376,11 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp for (const entry of file_entries) { await options.deployments.buildSingleAsset(artifact, assetManifest, entry, { stack: options.stack, + toolkitStackName: options.toolkitStackName, }); await options.deployments.publishSingleAsset(assetManifest, entry, { stack: options.stack, + toolkitStackName: options.toolkitStackName, }); } } diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index e8f59b2fbda5b..37eb53b54dd1b 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -182,6 +182,7 @@ export class CdkToolkit { parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), resourcesToImport, stream, + toolkitStackName: options.toolkitStackName, }); } else { debug(`the stack '${stack.stackName}' has not been deployed to CloudFormation or describeStacks call failed, skipping changeset creation.`); @@ -1099,6 +1100,13 @@ export interface DiffOptions { */ stackNames: string[]; + /** + * Name of the toolkit stack, if not the default name + * + * @default 'CDKToolkit' + */ + readonly toolkitStackName?: string; + /** * Only select the given stack * diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 9a91e6257db76..0e28a41a4c106 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -526,6 +526,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise Date: Thu, 3 Oct 2024 15:37:21 -0400 Subject: [PATCH 4/6] WIP - integ test Signed-off-by: Sumu --- .../cli-integ/tests/cli-integ-tests/cli.integtest.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 95ec9edb1d51c..6ce3005417c61 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 @@ -1059,10 +1059,13 @@ integTest( integTest('cdk diff with large changeset and custom toolkit stack name and qualifier does not fail', withoutBootstrap(async (fixture) => { // Bootstrapping with custom toolkit stack name and qualifier const qualifier = 'abc1234'; + const toolkitStackName = 'custom-stack'; + const bootstrapBucketName = 'cdk-abc1234-assets-587443617500-us-east-1'; await fixture.cdkBootstrapModern({ verbose: true, - toolkitStackName: 'custom-stack', + toolkitStackName: toolkitStackName, qualifier: qualifier, + bootstrapBucketName: bootstrapBucketName, }); // Deploying small initial stack with only ane IAM role @@ -1071,6 +1074,8 @@ integTest('cdk diff with large changeset and custom toolkit stack name and quali NUMBER_OF_ROLES: '1', }, options: [ + '--context', `bootstrapBucket=${bootstrapBucketName}`, + '--toolkit-stack-name', toolkitStackName, '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, ], }); @@ -1082,6 +1087,8 @@ integTest('cdk diff with large changeset and custom toolkit stack name and quali NUMBER_OF_ROLES: '200', }, options: [ + '--context', `bootstrapBucket=${bootstrapBucketName}`, + '--toolkit-stack-name', toolkitStackName, '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, ], }); From a3dc6eda422fea3a5de3b55069df3da4c7c1348c Mon Sep 17 00:00:00 2001 From: Sumu Date: Fri, 4 Oct 2024 11:31:49 -0400 Subject: [PATCH 5/6] update app.js to add ton of metadata instead of 200 roles, update cli integ test to succeed Signed-off-by: Sumu --- .../cli-integ/resources/cdk-apps/app/app.js | 10 ++++++++- .../tests/cli-integ-tests/cli.integtest.ts | 21 +++++++------------ 2 files changed, 17 insertions(+), 14 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 dd2797823198f..445b10ba4de89 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 @@ -438,9 +438,17 @@ class IamRolesStack extends cdk.Stack { // Environment variabile is used to create a bunch of roles to test // that large diff templates are uploaded to S3 to create the changeset. for(let i = 1; i <= Number(process.env.NUMBER_OF_ROLES) ; i++) { - new iam.Role(this, `Role${i}`, { + const role = new iam.Role(this, `Role${i}`, { assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'), }); + const cfnRole = role.node.defaultChild; + + // For any extra IAM roles created, add a ton of metadata so that the template size is > 50 KiB. + if (i > 1) { + for(let i = 1; i <= 30 ; i++) { + cfnRole.addMetadata('a'.repeat(1000), 'v'); + } + } } } } 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 6ce3005417c61..7408845e2e610 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 @@ -1046,7 +1046,7 @@ integTest( const diff = await fixture.cdk(['diff', fixture.fullStackName('iam-roles')], { verbose: true, modEnv: { - NUMBER_OF_ROLES: '200', + NUMBER_OF_ROLES: '2', }, }); @@ -1058,14 +1058,14 @@ integTest( integTest('cdk diff with large changeset and custom toolkit stack name and qualifier does not fail', withoutBootstrap(async (fixture) => { // Bootstrapping with custom toolkit stack name and qualifier - const qualifier = 'abc1234'; - const toolkitStackName = 'custom-stack'; - const bootstrapBucketName = 'cdk-abc1234-assets-587443617500-us-east-1'; + const qualifier = 'abc1111'; + const toolkitStackName = 'custom-stack2'; + // const bootstrapBucketName = 'cdk-abc1234-assets-587443617500-us-east-1'; await fixture.cdkBootstrapModern({ verbose: true, toolkitStackName: toolkitStackName, qualifier: qualifier, - bootstrapBucketName: bootstrapBucketName, + // bootstrapBucketName: bootstrapBucketName, }); // Deploying small initial stack with only ane IAM role @@ -1074,23 +1074,18 @@ integTest('cdk diff with large changeset and custom toolkit stack name and quali NUMBER_OF_ROLES: '1', }, options: [ - '--context', `bootstrapBucket=${bootstrapBucketName}`, + // '--context', `bootstrapBucket=${bootstrapBucketName}`, '--toolkit-stack-name', toolkitStackName, '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, ], }); // WHEN - adding 200 roles to the same stack to create a large diff - const diff = await fixture.cdk(['diff', fixture.fullStackName('iam-roles')], { + const diff = await fixture.cdk(['diff', '--toolkit-stack-name', toolkitStackName, '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, fixture.fullStackName('iam-roles')], { verbose: true, modEnv: { - NUMBER_OF_ROLES: '200', + NUMBER_OF_ROLES: '2', }, - options: [ - '--context', `bootstrapBucket=${bootstrapBucketName}`, - '--toolkit-stack-name', toolkitStackName, - '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, - ], }); // Assert that the CLI assumes the file publishing role: From cb448a319d39354ae5d44da689d29c5c0201fad6 Mon Sep 17 00:00:00 2001 From: Sumu Date: Fri, 4 Oct 2024 11:58:15 -0400 Subject: [PATCH 6/6] WIP: cleanup comments Signed-off-by: Sumu --- .../cli-integ/resources/cdk-apps/app/app.js | 1 + .../cli-integ/tests/cli-integ-tests/cli.integtest.ts | 11 ++++------- 2 files changed, 5 insertions(+), 7 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 445b10ba4de89..6ac252b4ac5aa 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 @@ -800,6 +800,7 @@ switch (stackSet) { new LambdaStack(app, `${stackPrefix}-lambda`); + // 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`); if (process.env.ENABLE_VPC_TESTING == 'IMPORT') { 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 7408845e2e610..90413cb0a06be 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 @@ -1035,14 +1035,14 @@ integTest( integTest( 'cdk diff with large changeset does not fail', withDefaultFixture(async (fixture) => { - // GIVEN - small initial stack with only ane IAM role + // GIVEN - small initial stack with only one IAM role await fixture.cdkDeploy('iam-roles', { modEnv: { NUMBER_OF_ROLES: '1', }, }); - // WHEN - adding 200 roles to the same stack to create a large diff + // WHEN - adding an additional role with a ton of metadata to create a large diff const diff = await fixture.cdk(['diff', fixture.fullStackName('iam-roles')], { verbose: true, modEnv: { @@ -1060,27 +1060,24 @@ integTest('cdk diff with large changeset and custom toolkit stack name and quali // Bootstrapping with custom toolkit stack name and qualifier const qualifier = 'abc1111'; const toolkitStackName = 'custom-stack2'; - // const bootstrapBucketName = 'cdk-abc1234-assets-587443617500-us-east-1'; await fixture.cdkBootstrapModern({ verbose: true, toolkitStackName: toolkitStackName, qualifier: qualifier, - // bootstrapBucketName: bootstrapBucketName, }); - // Deploying small initial stack with only ane IAM role + // Deploying small initial stack with only one IAM role await fixture.cdkDeploy('iam-roles', { modEnv: { NUMBER_OF_ROLES: '1', }, options: [ - // '--context', `bootstrapBucket=${bootstrapBucketName}`, '--toolkit-stack-name', toolkitStackName, '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, ], }); - // WHEN - adding 200 roles to the same stack to create a large diff + // WHEN - adding a role with a ton of metadata to create a large diff const diff = await fixture.cdk(['diff', '--toolkit-stack-name', toolkitStackName, '--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`, fixture.fullStackName('iam-roles')], { verbose: true, modEnv: {