From 33f4746b3da9ccd5dbc2bcf879feabf05e52baf0 Mon Sep 17 00:00:00 2001 From: mvp-cloud <64612801+mvp-cloud@users.noreply.github.com> Date: Fri, 19 Jun 2020 06:39:14 -0400 Subject: [PATCH] feat(cli): allow disabling of Public Access Block Configuration on bootstrap Bucket (#8171) ---- closes https://github.com/aws/aws-cdk/issues/5028 Adding optional boolean parameter `--public-access-block-configuration` to cdk bootstrap command to explicitly enable or disable Public Access Block Configuration on the bootstrap S3 bucket. By default this still remains enabled. Organizational and other account based restrictions can cause bootstrapping to fail if this is present. Example commands **Enabled Public Access Block Configuration** `cdk bootstrap` `cdk bootstrap --public-access-block-configuration true` **Disabled Public Access Block Configuration** `cdk bootstrap --public-access-block-configuration false` *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- design/cdk-bootstrap.md | 3 +++ packages/aws-cdk/README.md | 5 ++-- packages/aws-cdk/bin/cdk.ts | 2 ++ .../api/bootstrap/bootstrap-environment.ts | 1 + .../lib/api/bootstrap/bootstrap-props.ts | 7 ++++++ .../lib/api/bootstrap/bootstrap-template.yaml | 20 ++++++++++++---- .../lib/api/bootstrap/legacy-template.ts | 22 ++++++++++++++---- packages/aws-cdk/test/api/bootstrap.test.ts | 23 +++++++++++++++++++ packages/aws-cdk/test/api/bootstrap2.test.ts | 16 +++++++++++++ 9 files changed, 88 insertions(+), 11 deletions(-) diff --git a/design/cdk-bootstrap.md b/design/cdk-bootstrap.md index 718f475b51426..d2a3434ef5059 100644 --- a/design/cdk-bootstrap.md +++ b/design/cdk-bootstrap.md @@ -102,6 +102,9 @@ and need to be kept for backwards compatibility reasons: * `--bootstrap-kms-key-id`: optional identifier of the KMS key used for encrypting the file assets S3 bucket. +* `--public-access-block-configuration`: allows you to explicitly enable or disable public access bucket block configuration + on the file assets S3 Bucket (enabled by default). + #### New options These options will be added to the `bootstrap` command: diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 1d3b1ca3b85db..ad31e14b4214a 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -239,7 +239,8 @@ $ cdk destroy --app='node bin/main.js' MyStackName #### `cdk bootstrap` Deploys a `CDKToolkit` CloudFormation stack into the specified environment(s), that provides an S3 bucket that `cdk deploy` will use to store synthesized templates and the related assets, before triggering a CloudFormation stack -update. The name of the deployed stack can be configured using the `--toolkit-stack-name` argument. +update. The name of the deployed stack can be configured using the `--toolkit-stack-name` argument. The S3 Bucket +Public Access Block Configuration can be configured using the `--public-access-block-configuration` argument. ```console $ # Deploys to all environments @@ -279,6 +280,6 @@ Some of the interesting keys that can be used in the JSON configuration files: }, "toolkitStackName": "foo", // Customize 'bootstrap' stack name (--toolkit-stack-name=foo) "toolkitBucketName": "fooBucket", // Customize 'bootstrap' bucket name (--toolkit-bucket-name=fooBucket) - "versionReporting": false // Opt-out of version reporting (--no-version-reporting) + "versionReporting": false, // Opt-out of version reporting (--no-version-reporting) } ``` diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index 37c5a0887d52c..b30b6346ada84 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -67,6 +67,7 @@ async function parseCommandLineArguments() { .option('bootstrap-bucket-name', { type: 'string', alias: ['b', 'toolkit-bucket-name'], desc: 'The name of the CDK toolkit bucket; bucket will be created and must not exist', default: undefined }) .option('bootstrap-kms-key-id', { type: 'string', desc: 'AWS KMS master key ID used for the SSE-KMS encryption', default: undefined }) .option('qualifier', { type: 'string', desc: 'Unique string to distinguish multiple bootstrap stacks', default: undefined }) + .option('public-access-block-configuration', { type: 'boolean', desc: 'Block public access configuration on CDK toolkit bucket (enabled by default) ', default: true }) .option('tags', { type: 'array', alias: 't', desc: 'Tags to add for the stack (KEY=VALUE)', nargs: 1, requiresArg: true, default: [] }) .option('execute', {type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true}) .option('trust', { type: 'array', desc: 'The AWS account IDs that should be trusted to perform deployments into this environment (may be repeated)', default: [], nargs: 1, requiresArg: true, hidden: true }) @@ -233,6 +234,7 @@ async function initCommandLine() { bucketName: configuration.settings.get(['toolkitBucket', 'bucketName']), kmsKeyId: configuration.settings.get(['toolkitBucket', 'kmsKeyId']), qualifier: args.qualifier, + publicAccessBlockConfiguration: args.publicAccessBlockConfiguration, tags: configuration.settings.get(['tags']), execute: args.execute, trustedAccounts: args.trust, diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts index 2bb024d8abea1..71e85d77e9d69 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts @@ -62,6 +62,7 @@ export async function bootstrapEnvironment2( TrustedAccounts: params.trustedAccounts?.join(','), CloudFormationExecutionPolicies: params.cloudFormationExecutionPolicies?.join(','), Qualifier: params.qualifier, + PublicAccessBlockConfiguration: params.publicAccessBlockConfiguration || params.publicAccessBlockConfiguration === undefined ? 'true' : 'false', }, environment, sdkProvider, diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts b/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts index fd284a4fec83b..2a90bd9033908 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-props.ts @@ -70,4 +70,11 @@ export interface BootstrappingParameters { * @default - Default qualifier */ readonly qualifier?: string; + + /** + * Whether or not to enable S3 Staging Bucket Public Access Block Configuration + * + * @default true + */ + readonly publicAccessBlockConfiguration?: boolean; } \ No newline at end of file diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml index c73539d7a29a2..c3b744c83d9e2 100644 --- a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml +++ b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml @@ -28,6 +28,11 @@ Parameters: Description: An identifier to distinguish multiple bootstrap stacks in the same environment Default: hnb659fds Type: String + PublicAccessBlockConfiguration: + Description: Whether or not to enable S3 Staging Bucket Public Access Block Configuration + Default: 'true' + Type: 'String' + AllowedValues: ['true', 'false'] Conditions: HasTrustedAccounts: Fn::Not: @@ -57,6 +62,10 @@ Conditions: - Fn::Equals: - '' - Ref: ContainerAssetsRepositoryName + UsePublicAccessBlockConfiguration: + Fn::Equals: + - 'true' + - Ref: PublicAccessBlockConfiguration Resources: FileAssetsBucketEncryptionKey: Type: AWS::KMS::Key @@ -128,10 +137,13 @@ Resources: - Fn::Sub: "${FileAssetsBucketEncryptionKey.Arn}" - Fn::Sub: "${FileAssetsBucketKmsKeyId}" PublicAccessBlockConfiguration: - BlockPublicAcls: true - BlockPublicPolicy: true - IgnorePublicAcls: true - RestrictPublicBuckets: true + Fn::If: + - UsePublicAccessBlockConfiguration + - BlockPublicAcls: true + BlockPublicPolicy: true + IgnorePublicAcls: true + RestrictPublicBuckets: true + - Ref: AWS::NoValue UpdateReplacePolicy: Retain StagingBucketPolicy: Type: 'AWS::S3::BucketPolicy' diff --git a/packages/aws-cdk/lib/api/bootstrap/legacy-template.ts b/packages/aws-cdk/lib/api/bootstrap/legacy-template.ts index 9bd99a8792836..67bed3406b07a 100644 --- a/packages/aws-cdk/lib/api/bootstrap/legacy-template.ts +++ b/packages/aws-cdk/lib/api/bootstrap/legacy-template.ts @@ -3,6 +3,13 @@ import { BootstrappingParameters, BUCKET_DOMAIN_NAME_OUTPUT, BUCKET_NAME_OUTPUT export function legacyBootstrapTemplate(params: BootstrappingParameters): any { return { Description: 'The CDK Toolkit Stack. It was created by `cdk bootstrap` and manages resources necessary for managing your Cloud Applications with AWS CDK.', + Conditions: { + UsePublicAccessBlockConfiguration: { + 'Fn::Equals' : [ + params.publicAccessBlockConfiguration || params.publicAccessBlockConfiguration === undefined ? 'true' : 'false', + 'true', + ]}, + }, Resources: { StagingBucket: { Type: 'AWS::S3::Bucket', @@ -18,11 +25,16 @@ export function legacyBootstrapTemplate(params: BootstrappingParameters): any { }], }, PublicAccessBlockConfiguration: { - BlockPublicAcls: true, - BlockPublicPolicy: true, - IgnorePublicAcls: true, - RestrictPublicBuckets: true, - }, + 'Fn::If' : [ + 'UsePublicAccessBlockConfiguration', + { + BlockPublicAcls: true, + BlockPublicPolicy: true, + IgnorePublicAcls: true, + RestrictPublicBuckets: true, + }, + 'AWS::NoValue', + ]}, }, }, StagingBucketPolicy: { diff --git a/packages/aws-cdk/test/api/bootstrap.test.ts b/packages/aws-cdk/test/api/bootstrap.test.ts index ddc3086877c7e..adf36daa5eb4c 100644 --- a/packages/aws-cdk/test/api/bootstrap.test.ts +++ b/packages/aws-cdk/test/api/bootstrap.test.ts @@ -60,6 +60,7 @@ test('do bootstrap', async () => { expect(bucketProperties.BucketName).toBeUndefined(); expect(bucketProperties.BucketEncryption.ServerSideEncryptionConfiguration[0].ServerSideEncryptionByDefault.KMSMasterKeyID) .toBeUndefined(); + expect(changeSetTemplate.Conditions.UsePublicAccessBlockConfiguration['Fn::Equals'][0]).toBe('true'); expect(ret.noOp).toBeFalsy(); expect(executed).toBeTruthy(); }); @@ -78,6 +79,7 @@ test('do bootstrap using custom bucket name', async () => { expect(bucketProperties.BucketName).toBe('foobar'); expect(bucketProperties.BucketEncryption.ServerSideEncryptionConfiguration[0].ServerSideEncryptionByDefault.KMSMasterKeyID) .toBeUndefined(); + expect(changeSetTemplate.Conditions.UsePublicAccessBlockConfiguration['Fn::Equals'][0]).toBe('true'); expect(ret.noOp).toBeFalsy(); expect(executed).toBeTruthy(); }); @@ -96,6 +98,26 @@ test('do bootstrap using KMS CMK', async () => { expect(bucketProperties.BucketName).toBeUndefined(); expect(bucketProperties.BucketEncryption.ServerSideEncryptionConfiguration[0].ServerSideEncryptionByDefault.KMSMasterKeyID) .toBe('myKmsKey'); + expect(changeSetTemplate.Conditions.UsePublicAccessBlockConfiguration['Fn::Equals'][0]).toBe('true'); + expect(ret.noOp).toBeFalsy(); + expect(executed).toBeTruthy(); +}); + +test('bootstrap disable bucket Public Access Block Configuration', async () => { + // WHEN + const ret = await bootstrapEnvironment(env, sdk, { + toolkitStackName: 'mockStack', + parameters: { + publicAccessBlockConfiguration: false, + }, + }); + + // THEN + const bucketProperties = changeSetTemplate.Resources.StagingBucket.Properties; + expect(bucketProperties.BucketName).toBeUndefined(); + expect(bucketProperties.BucketEncryption.ServerSideEncryptionConfiguration[0].ServerSideEncryptionByDefault.KMSMasterKeyID) + .toBeUndefined(); + expect(changeSetTemplate.Conditions.UsePublicAccessBlockConfiguration['Fn::Equals'][0]).toBe('false'); expect(ret.noOp).toBeFalsy(); expect(executed).toBeTruthy(); }); @@ -114,6 +136,7 @@ test('do bootstrap with custom tags for toolkit stack', async () => { expect(bucketProperties.BucketName).toBeUndefined(); expect(bucketProperties.BucketEncryption.ServerSideEncryptionConfiguration[0].ServerSideEncryptionByDefault.KMSMasterKeyID) .toBeUndefined(); + expect(changeSetTemplate.Conditions.UsePublicAccessBlockConfiguration['Fn::Equals'][0]).toBe('true'); expect(ret.noOp).toBeFalsy(); expect(executed).toBeTruthy(); }); diff --git a/packages/aws-cdk/test/api/bootstrap2.test.ts b/packages/aws-cdk/test/api/bootstrap2.test.ts index d3fa823588052..8409a60cf2945 100644 --- a/packages/aws-cdk/test/api/bootstrap2.test.ts +++ b/packages/aws-cdk/test/api/bootstrap2.test.ts @@ -41,6 +41,7 @@ describe('Bootstrapping v2', () => { expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ parameters: { FileAssetsBucketName: 'my-bucket-name', + PublicAccessBlockConfiguration: 'true', }, })); }); @@ -55,6 +56,21 @@ describe('Bootstrapping v2', () => { expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ parameters: { FileAssetsBucketKmsKeyId: 'my-kms-key-id', + PublicAccessBlockConfiguration: 'true', + }, + })); + }); + + test('passes false to PublicAccessBlockConfiguration', async () => { + await bootstrapEnvironment2(env, sdk, { + parameters: { + publicAccessBlockConfiguration: false, + }, + }); + + expect(mockDeployStack).toHaveBeenCalledWith(expect.objectContaining({ + parameters: { + PublicAccessBlockConfiguration: 'false', }, })); });