From 46946c5478b544f8cd147bc3c36937320cd7763c Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Wed, 22 Jan 2025 12:41:26 +0000 Subject: [PATCH 1/4] feat(cli): add --untrust option to bootstrap --- .../cli-integ/lib/with-cdk-app.ts | 11 +++ .../bootstrapping.integtest.ts | 25 +++++++ .../api/bootstrap/bootstrap-environment.ts | 23 ++++++- .../lib/api/bootstrap/bootstrap-props.ts | 8 +++ packages/aws-cdk/lib/config.ts | 1 + packages/aws-cdk/lib/convert-to-user-input.ts | 2 + .../lib/parse-command-line-arguments.ts | 7 ++ packages/aws-cdk/lib/user-input.ts | 7 ++ packages/aws-cdk/test/api/bootstrap2.test.ts | 69 +++++++++++++++++++ 9 files changed, 150 insertions(+), 3 deletions(-) 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 7bdfff1154ba2..da4ede82f2663 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 @@ -332,6 +332,10 @@ export interface CdkModernBootstrapCommandOptions extends CommonCdkBootstrapComm * @default undefined */ readonly usePreviousParameters?: boolean; + + readonly trust?: string[]; + + readonly untrust?: string[]; } export interface CdkGarbageCollectionCommandOptions { @@ -480,6 +484,13 @@ export class TestFixture extends ShellHelper { args.push('--template', options.bootstrapTemplate); } + if (options.trust != null) { + args.push('--trust', options.trust.join(',')); + } + if (options.untrust != null) { + args.push('--untrust', options.untrust.join(',')); + } + return this.cdk(args, { ...options.cliOptions, modEnv: { diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrapping.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrapping.integtest.ts index e6d2b05903f5e..cc8e66822a74b 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrapping.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrapping.integtest.ts @@ -491,3 +491,28 @@ integTest('create ECR with tag IMMUTABILITY to set on', withoutBootstrap(async ( expect(ecrResponse.repositories?.[0].imageTagMutability).toEqual('IMMUTABLE'); })); +integTest('can remove trusted account', withoutBootstrap(async (fixture) => { + const bootstrapStackName = fixture.bootstrapStackName; + + await fixture.cdkBootstrapModern({ + verbose: false, + toolkitStackName: bootstrapStackName, + cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess', + trust: ['599757620138', '730170552321'], + }); + + await fixture.cdkBootstrapModern({ + verbose: true, + toolkitStackName: bootstrapStackName, + cfnExecutionPolicy: ' arn:aws:iam::aws:policy/AdministratorAccess', + untrust: ['730170552321'], + }); + + const response = await fixture.aws.cloudFormation.send( + new DescribeStacksCommand({ StackName: bootstrapStackName }), + ); + + const trustedAccounts = response.Stacks?.[0].Parameters?.find(p => p.ParameterKey === 'TrustedAccounts')?.ParameterValue; + expect(trustedAccounts).toEqual('599757620138'); +})) + diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts index ad0acf18bf477..969c5ae20d5d5 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts @@ -102,11 +102,24 @@ export class Bootstrapper { // Ideally we'd do this inside the template, but the `Rules` section of CFN // templates doesn't seem to be able to express the conditions that we need // (can't use Fn::Join or reference Conditions) so we do it here instead. - const trustedAccounts = params.trustedAccounts ?? splitCfnArray(current.parameters.TrustedAccounts); + const allTrusted = new Set([ + ...params.trustedAccounts ?? [], + ...params.trustedAccountsForLookup ?? [] + ]); + const invalid = intersection(allTrusted, new Set(params.untrustedAccounts)); + if (invalid.size > 0) { + throw new ToolkitError(`Accounts cannot be both trusted and untrusted. Found: ${[...invalid].join(',')}`); + } + + const removeUntrusted = (accounts: string[]) => + accounts.filter(acc => !params.untrustedAccounts?.includes(acc)); + + const trustedAccounts = removeUntrusted(params.trustedAccounts ?? splitCfnArray(current.parameters.TrustedAccounts)); info(`Trusted accounts for deployment: ${trustedAccounts.length > 0 ? trustedAccounts.join(', ') : '(none)'}`); - const trustedAccountsForLookup = - params.trustedAccountsForLookup ?? splitCfnArray(current.parameters.TrustedAccountsForLookup); + const trustedAccountsForLookup = removeUntrusted( + params.trustedAccountsForLookup ?? splitCfnArray(current.parameters.TrustedAccountsForLookup), + ); info( `Trusted accounts for lookup: ${trustedAccountsForLookup.length > 0 ? trustedAccountsForLookup.join(', ') : '(none)'}`, ); @@ -376,3 +389,7 @@ function splitCfnArray(xs: string | undefined): string[] { } return xs.split(','); } + +function intersection(xs: Set, ys: Set): Set { + return new Set(Array.from(xs).filter(x => ys.has(x))); +} diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts index f32608e627a2f..cdea6a6ef4025 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts @@ -102,6 +102,14 @@ export interface BootstrappingParameters { */ readonly trustedAccountsForLookup?: string[]; + /** + * The list of AWS account IDs that should not be trusted by the bootstrapped environment. + * If these accounts are already trusted, they will be removed on bootstrapping. + * + * @default - no account will be untrusted. + */ + readonly untrustedAccounts?: string[]; + /** * The ARNs of the IAM managed policies that should be attached to the role performing CloudFormation deployments. * In most cases, this will be the AdministratorAccess policy. diff --git a/packages/aws-cdk/lib/config.ts b/packages/aws-cdk/lib/config.ts index 2ef82baf3304e..b60569f1e9498 100644 --- a/packages/aws-cdk/lib/config.ts +++ b/packages/aws-cdk/lib/config.ts @@ -87,6 +87,7 @@ export async function makeConfig(): Promise { 'execute': { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true }, 'trust': { type: 'array', desc: 'The AWS account IDs that should be trusted to perform deployments into this environment (may be repeated, modern bootstrapping only)', default: [] }, 'trust-for-lookup': { type: 'array', desc: 'The AWS account IDs that should be trusted to look up values in this environment (may be repeated, modern bootstrapping only)', default: [] }, + 'untrust': { type: 'array', desc: 'The AWS account IDs that should not be trusted by this environment (may be repeated, modern bootstrapping only)', default: [] }, 'cloudformation-execution-policies': { type: 'array', desc: 'The Managed Policy ARNs that should be attached to the role performing deployments into this environment (may be repeated, modern bootstrapping only)', default: [] }, 'force': { alias: 'f', type: 'boolean', desc: 'Always bootstrap even if it would downgrade template version', default: false }, 'termination-protection': { type: 'boolean', default: undefined, desc: 'Toggle CloudFormation termination protection on the bootstrap stacks' }, diff --git a/packages/aws-cdk/lib/convert-to-user-input.ts b/packages/aws-cdk/lib/convert-to-user-input.ts index 4b400aa844424..65f3dfdbd5b79 100644 --- a/packages/aws-cdk/lib/convert-to-user-input.ts +++ b/packages/aws-cdk/lib/convert-to-user-input.ts @@ -69,6 +69,7 @@ export function convertYargsToUserInput(args: any): UserInput { execute: args.execute, trust: args.trust, trustForLookup: args.trustForLookup, + untrust: args.untrust, cloudformationExecutionPolicies: args.cloudformationExecutionPolicies, force: args.force, terminationProtection: args.terminationProtection, @@ -309,6 +310,7 @@ export function convertConfigToUserInput(config: any): UserInput { execute: config.bootstrap?.execute, trust: config.bootstrap?.trust, trustForLookup: config.bootstrap?.trustForLookup, + untrust: config.bootstrap?.untrust, cloudformationExecutionPolicies: config.bootstrap?.cloudformationExecutionPolicies, force: config.bootstrap?.force, terminationProtection: config.bootstrap?.terminationProtection, diff --git a/packages/aws-cdk/lib/parse-command-line-arguments.ts b/packages/aws-cdk/lib/parse-command-line-arguments.ts index 20b09694290fa..0dbec7f7c2859 100644 --- a/packages/aws-cdk/lib/parse-command-line-arguments.ts +++ b/packages/aws-cdk/lib/parse-command-line-arguments.ts @@ -263,6 +263,13 @@ export function parseCommandLineArguments(args: Array): any { nargs: 1, requiresArg: true, }) + .option('untrust', { + default: [], + type: 'array', + desc: 'The AWS account IDs that should not be trusted by this environment (may be repeated, modern bootstrapping only)', + nargs: 1, + requiresArg: true, + }) .option('cloudformation-execution-policies', { default: [], type: 'array', diff --git a/packages/aws-cdk/lib/user-input.ts b/packages/aws-cdk/lib/user-input.ts index 369a7fe325515..279d0635cca2b 100644 --- a/packages/aws-cdk/lib/user-input.ts +++ b/packages/aws-cdk/lib/user-input.ts @@ -464,6 +464,13 @@ export interface BootstrapOptions { */ readonly trustForLookup?: Array; + /** + * The AWS account IDs that should not be trusted by this environment (may be repeated, modern bootstrapping only) + * + * @default - [] + */ + readonly untrust?: Array; + /** * The Managed Policy ARNs that should be attached to the role performing deployments into this environment (may be repeated, modern bootstrapping only) * diff --git a/packages/aws-cdk/test/api/bootstrap2.test.ts b/packages/aws-cdk/test/api/bootstrap2.test.ts index eb2b273e85845..8898f3799f424 100644 --- a/packages/aws-cdk/test/api/bootstrap2.test.ts +++ b/packages/aws-cdk/test/api/bootstrap2.test.ts @@ -330,6 +330,75 @@ describe('Bootstrapping v2', () => { // Did not throw }); + test('removes trusted account when it is listed as untrusted', async () => { + // GIVEN + mockTheToolkitInfo({ + Parameters: [ + { + ParameterKey: 'CloudFormationExecutionPolicies', + ParameterValue: 'arn:aws:something', + }, + { + ParameterKey: 'TrustedAccounts', + ParameterValue: '111111111111,222222222222', + }, + ], + }); + + await bootstrapper.bootstrapEnvironment(env, sdk, { + parameters: { + untrustedAccounts: ['111111111111'], + }, + }); + + expect(mockDeployStack).toHaveBeenCalledWith( + expect.objectContaining({ + parameters: expect.objectContaining({ + TrustedAccounts: '222222222222', + }), + }), + ) + }); + + test('removes trusted account for lookup when it is listed as untrusted', async () => { + // GIVEN + mockTheToolkitInfo({ + Parameters: [ + { + ParameterKey: 'CloudFormationExecutionPolicies', + ParameterValue: 'arn:aws:something', + }, + { + ParameterKey: 'TrustedAccountsForLookup', + ParameterValue: '111111111111,222222222222', + }, + ], + }); + + await bootstrapper.bootstrapEnvironment(env, sdk, { + parameters: { + untrustedAccounts: ['111111111111'], + }, + }); + + expect(mockDeployStack).toHaveBeenCalledWith( + expect.objectContaining({ + parameters: expect.objectContaining({ + TrustedAccountsForLookup: '222222222222', + }), + }), + ) + }); + + test('do not allow accounts to be listed as both trusted and untrusted', async () => { + await expect(bootstrapper.bootstrapEnvironment(env, sdk, { + parameters: { + trustedAccountsForLookup: ['123456789012'], + untrustedAccounts: ['123456789012'], + }, + })).rejects.toThrow('Accounts cannot be both trusted and untrusted. Found: 123456789012'); + }); + test('Do not allow downgrading bootstrap stack version', async () => { // GIVEN mockTheToolkitInfo({ From 9f50134ff43f01d2f03dbfea9420e9d213076e60 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Wed, 22 Jan 2025 13:03:28 +0000 Subject: [PATCH 2/4] reformat bootstrapping.integtest.ts --- .../cli-integ-tests/bootstrapping.integtest.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrapping.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrapping.integtest.ts index cc8e66822a74b..09992e4d1d798 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrapping.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrapping.integtest.ts @@ -494,12 +494,12 @@ integTest('create ECR with tag IMMUTABILITY to set on', withoutBootstrap(async ( integTest('can remove trusted account', withoutBootstrap(async (fixture) => { const bootstrapStackName = fixture.bootstrapStackName; - await fixture.cdkBootstrapModern({ - verbose: false, - toolkitStackName: bootstrapStackName, - cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess', - trust: ['599757620138', '730170552321'], - }); + await fixture.cdkBootstrapModern({ + verbose: false, + toolkitStackName: bootstrapStackName, + cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess', + trust: ['599757620138', '730170552321'], + }); await fixture.cdkBootstrapModern({ verbose: true, @@ -514,5 +514,5 @@ integTest('can remove trusted account', withoutBootstrap(async (fixture) => { const trustedAccounts = response.Stacks?.[0].Parameters?.find(p => p.ParameterKey === 'TrustedAccounts')?.ParameterValue; expect(trustedAccounts).toEqual('599757620138'); -})) +})); From 66c959dcbae1e6eca9101f89bdaca995eebb3446 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Wed, 22 Jan 2025 13:29:15 +0000 Subject: [PATCH 3/4] reformat bootstrap-environment.ts --- packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts | 2 +- packages/aws-cdk/test/api/bootstrap2.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts index 969c5ae20d5d5..96a6193bdab6a 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts @@ -104,7 +104,7 @@ export class Bootstrapper { // (can't use Fn::Join or reference Conditions) so we do it here instead. const allTrusted = new Set([ ...params.trustedAccounts ?? [], - ...params.trustedAccountsForLookup ?? [] + ...params.trustedAccountsForLookup ?? [], ]); const invalid = intersection(allTrusted, new Set(params.untrustedAccounts)); if (invalid.size > 0) { diff --git a/packages/aws-cdk/test/api/bootstrap2.test.ts b/packages/aws-cdk/test/api/bootstrap2.test.ts index 8898f3799f424..efc472c3a7867 100644 --- a/packages/aws-cdk/test/api/bootstrap2.test.ts +++ b/packages/aws-cdk/test/api/bootstrap2.test.ts @@ -357,7 +357,7 @@ describe('Bootstrapping v2', () => { TrustedAccounts: '222222222222', }), }), - ) + ); }); test('removes trusted account for lookup when it is listed as untrusted', async () => { @@ -387,7 +387,7 @@ describe('Bootstrapping v2', () => { TrustedAccountsForLookup: '222222222222', }), }), - ) + ); }); test('do not allow accounts to be listed as both trusted and untrusted', async () => { From fb7c30c973dcd9522a76008e4b12bfc51d12fa98 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Thu, 23 Jan 2025 11:08:38 +0000 Subject: [PATCH 4/4] Fixes --- packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts | 2 +- packages/aws-cdk/lib/cli.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts index 96a6193bdab6a..d14b51b185e88 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts @@ -112,7 +112,7 @@ export class Bootstrapper { } const removeUntrusted = (accounts: string[]) => - accounts.filter(acc => !params.untrustedAccounts?.includes(acc)); + accounts.filter(acc => !params.untrustedAccounts?.map(String).includes(String(acc))); const trustedAccounts = removeUntrusted(params.trustedAccounts ?? splitCfnArray(current.parameters.TrustedAccounts)); info(`Trusted accounts for deployment: ${trustedAccounts.length > 0 ? trustedAccounts.join(', ') : '(none)'}`); diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 053fcafe07654..01e5c06c3d761 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -279,6 +279,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise