feat(cli): allow disabling of Public Access Block Configuration on bo…

…otstrap Bucket (aws#8171) ---- closes aws#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*
ConradMearns · Jun 19, 2020 · 33f4746 · 33f4746
1 parent 387c1a8
commit 33f4746
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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;
 }
diff --git 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
@@ -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
@@ -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
@@ -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',
       },
     }));
   });