Skip to content

Commit 904e3a3

Browse files
authored
fix(s3): updating blockPublicAccess to enable default true settings (under feature flag) (#33702)
V3 but I think we got there ### Issue # Closes #32811 ### Reason for this change By default when you create an s3 bucket, all public access is already blocked. However if you then use CDK to specify 1 or more access point you want to unblock, all undefined block types will be auto set to false, and when it deploys you will see everything uncheck even if you only wanted to uncheck 1 thing. So to fix this we should instead default all values to true when at least 1 option is specified, to mimic to experience when a user in the console unchecks the boxes. ### Description of changes deprecating `BLOCK_ACLS` method of `BlockPublicAccess`. Adding `BLOCK_ACLS_ONLY`. ``` public static readonly BLOCK_ACLS_ONLY = new BlockPublicAccess({ blockPublicAcls: true, blockPublicPolicy: false, ignorePublicAcls: true, restrictPublicBuckets: false, }); ``` This is just a general revamp to match what the feature will bring, it's separate from the feature itself. The point being that for any shortcut methods like this, we should be specifying all 4 options to ensure the default true behavior remains. Created function `setBlockPublicAccessDefaults()` ``` /** * Function to set the blockPublicAccessOptions to a true default if not defined. * If no blockPublicAccessOptions are specified at all, this is already the case as an s3 default in aws * @see https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-control-block-public-access.html */ private setBlockPublicAccessDefaults(blockPublicAccessOptions: BlockPublicAccessOptions) { return { blockPublicAcls: blockPublicAccessOptions.blockPublicAcls ?? true, blockPublicPolicy: blockPublicAccessOptions.blockPublicPolicy ?? true, ignorePublicAcls: blockPublicAccessOptions.ignorePublicAcls ?? true, restrictPublicBuckets: blockPublicAccessOptions.restrictPublicBuckets ?? true, }; } ``` but this method is only called if the FF is enabled ``` let blockPublicAccess: BlockPublicAccessOptions | undefined = props.blockPublicAccess; if (props.blockPublicAccess && FeatureFlags.of(this).isEnabled(cxapi.S3_BLOCK_PUBLIC_ACCESS_OPTION_AUTO_TRUE)) { blockPublicAccess = this.setBlockPublicAccessDefaults(props.blockPublicAccess); } ``` Of course the FF itself was added. ### Description of how you validated changes Added tests that are duplicates of others, just testing for both behaviors with and without the FF. ``` describe('bucket with custom block public access setting', () => { ... test('S3_BLOCK_PUBLIC_ACCESS_OPTION_AUTO_TRUE Enabled', () => { const app = new cdk.App({ context: { [cxapi.S3_BLOCK_PUBLIC_ACCESS_OPTION_AUTO_TRUE]: true, }, }); const stack = new cdk.Stack(app); new s3.Bucket(stack, 'MyBucket', { blockPublicAccess: new s3.BlockPublicAccess({ restrictPublicBuckets: false }), }); Template.fromStack(stack).templateMatches({ 'Resources': { 'MyBucketF68F3FF0': { 'Type': 'AWS::S3::Bucket', 'Properties': { 'PublicAccessBlockConfiguration': { 'BlockPublicAcls': true, 'BlockPublicPolicy': true, 'IgnorePublicAcls': true, 'RestrictPublicBuckets': false, }, }, 'DeletionPolicy': 'Retain', 'UpdateReplacePolicy': 'Retain', }, }, }); }); ``` ``` describe('bucket with custom block public access setting', () => { ... test('S3_BLOCK_PUBLIC_ACCESS_OPTION_AUTO_TRUE Enabled', () => { const app = new cdk.App({ context: { [cxapi.S3_BLOCK_PUBLIC_ACCESS_OPTION_AUTO_TRUE]: true, }, }); const stack = new cdk.Stack(app); new s3.Bucket(stack, 'MyBucket', { blockPublicAccess: new s3.BlockPublicAccess({ restrictPublicBuckets: false }), }); Template.fromStack(stack).templateMatches({ 'Resources': { 'MyBucketF68F3FF0': { 'Type': 'AWS::S3::Bucket', 'Properties': { 'PublicAccessBlockConfiguration': { 'BlockPublicAcls': true, 'BlockPublicPolicy': true, 'IgnorePublicAcls': true, 'RestrictPublicBuckets': false, }, }, 'DeletionPolicy': 'Retain', 'UpdateReplacePolicy': 'Retain', }, }, }); }); ``` Also added an integ that just tests different combinations of the blocking. https://github.com/aws/aws-cdk/blob/51ffe2112e048f5866e5c0d811377b4deca7920d/packages/%40aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-block-access.ts#L1-L42 There was no `BlockPublicAccess` integ before so I did not add the context for the FF disabled anywhere. The tests should still be working since it's not used that often. But if the team needs me to, I can add a 2nd integ with the old behavior ### 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 4dc523f commit 904e3a3

16 files changed

+895
-27
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-block-access.js.snapshot/aws-cdk-s3-bucket-block-access.assets.json

Lines changed: 19 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
{
2+
"Resources": {
3+
"blockAcls40B656E5": {
4+
"Type": "AWS::S3::Bucket",
5+
"Properties": {
6+
"PublicAccessBlockConfiguration": {
7+
"BlockPublicAcls": true,
8+
"BlockPublicPolicy": true,
9+
"IgnorePublicAcls": true,
10+
"RestrictPublicBuckets": true
11+
}
12+
},
13+
"UpdateReplacePolicy": "Retain",
14+
"DeletionPolicy": "Retain"
15+
},
16+
"blockAclsOnly4E7ABDF2": {
17+
"Type": "AWS::S3::Bucket",
18+
"Properties": {
19+
"PublicAccessBlockConfiguration": {
20+
"BlockPublicAcls": true,
21+
"BlockPublicPolicy": false,
22+
"IgnorePublicAcls": true,
23+
"RestrictPublicBuckets": false
24+
}
25+
},
26+
"UpdateReplacePolicy": "Retain",
27+
"DeletionPolicy": "Retain"
28+
},
29+
"blockAll363E56BB": {
30+
"Type": "AWS::S3::Bucket",
31+
"Properties": {
32+
"PublicAccessBlockConfiguration": {
33+
"BlockPublicAcls": true,
34+
"BlockPublicPolicy": true,
35+
"IgnorePublicAcls": true,
36+
"RestrictPublicBuckets": true
37+
}
38+
},
39+
"UpdateReplacePolicy": "Retain",
40+
"DeletionPolicy": "Retain"
41+
},
42+
"blockOnly2C4374FF0": {
43+
"Type": "AWS::S3::Bucket",
44+
"Properties": {
45+
"PublicAccessBlockConfiguration": {
46+
"BlockPublicAcls": false,
47+
"BlockPublicPolicy": false,
48+
"IgnorePublicAcls": true,
49+
"RestrictPublicBuckets": true
50+
}
51+
},
52+
"UpdateReplacePolicy": "Retain",
53+
"DeletionPolicy": "Retain"
54+
},
55+
"blockOnly1C2CBF6E1": {
56+
"Type": "AWS::S3::Bucket",
57+
"Properties": {
58+
"PublicAccessBlockConfiguration": {
59+
"BlockPublicAcls": true,
60+
"BlockPublicPolicy": false,
61+
"IgnorePublicAcls": true,
62+
"RestrictPublicBuckets": true
63+
}
64+
},
65+
"UpdateReplacePolicy": "Retain",
66+
"DeletionPolicy": "Retain"
67+
}
68+
},
69+
"Parameters": {
70+
"BootstrapVersion": {
71+
"Type": "AWS::SSM::Parameter::Value<String>",
72+
"Default": "/cdk-bootstrap/hnb659fds/version",
73+
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
74+
}
75+
},
76+
"Rules": {
77+
"CheckBootstrapVersion": {
78+
"Assertions": [
79+
{
80+
"Assert": {
81+
"Fn::Not": [
82+
{
83+
"Fn::Contains": [
84+
[
85+
"1",
86+
"2",
87+
"3",
88+
"4",
89+
"5"
90+
],
91+
{
92+
"Ref": "BootstrapVersion"
93+
}
94+
]
95+
}
96+
]
97+
},
98+
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
99+
}
100+
]
101+
}
102+
}
103+
}

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-block-access.js.snapshot/cdk.out

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-block-access.js.snapshot/integ.json

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-block-access.js.snapshot/manifest.json

Lines changed: 177 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-block-access.js.snapshot/testBlockingBucketAccessDefaultTestDeployAssert2F4ACAD8.assets.json

Lines changed: 19 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3/test/integ.bucket-block-access.js.snapshot/testBlockingBucketAccessDefaultTestDeployAssert2F4ACAD8.template.json

Lines changed: 36 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)