Skip to content

Commit c7c75f8

Browse files
authored
fix(s3): bucketKey does not support SSE-S3 (#30184)
### Issue # (if applicable) Closes #30183 ### Reason for this change ### Description of changes ### Description of how you validated changes ### 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 7624c62 commit c7c75f8

File tree

10 files changed

+112
-22
lines changed

10 files changed

+112
-22
lines changed

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

+1-1
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-encryption.js.snapshot/aws-cdk-s3-bucket-encryption.assets.json

+3-3
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-encryption.js.snapshot/aws-cdk-s3-bucket-encryption.template.json

+17
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,23 @@
1515
},
1616
"UpdateReplacePolicy": "Retain",
1717
"DeletionPolicy": "Retain"
18+
},
19+
"MySSES3Bucket6973690D": {
20+
"Type": "AWS::S3::Bucket",
21+
"Properties": {
22+
"BucketEncryption": {
23+
"ServerSideEncryptionConfiguration": [
24+
{
25+
"BucketKeyEnabled": true,
26+
"ServerSideEncryptionByDefault": {
27+
"SSEAlgorithm": "AES256"
28+
}
29+
}
30+
]
31+
}
32+
},
33+
"UpdateReplacePolicy": "Retain",
34+
"DeletionPolicy": "Retain"
1835
}
1936
},
2037
"Parameters": {

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

+1-1
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-encryption.js.snapshot/integ.json

+1-1
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-encryption.js.snapshot/manifest.json

+10-2
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-encryption.js.snapshot/tree.json

+35-2
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-encryption.ts

+5
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ new s3.Bucket(stack, 'MyDSSEBucket', {
1010
encryption: s3.BucketEncryption.DSSE_MANAGED,
1111
});
1212

13+
new s3.Bucket(stack, 'MySSES3Bucket', {
14+
encryption: s3.BucketEncryption.S3_MANAGED,
15+
bucketKeyEnabled: true,
16+
});
17+
1318
new integ.IntegTest(app, 'IntegTestDSSEBucket', {
1419
testCases: [stack],
1520
});

packages/aws-cdk-lib/aws-s3/lib/bucket.ts

+9-8
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,7 @@ export interface BucketProps {
14401440
* attendant cost implications of that).
14411441
* - If enabled, S3 will use its own time-limited key instead.
14421442
*
1443-
* Only relevant, when Encryption is set to `BucketEncryption.KMS` or `BucketEncryption.KMS_MANAGED`.
1443+
* Only relevant, when Encryption is not set to `BucketEncryption.UNENCRYPTED`.
14441444
*
14451445
* @default - false
14461446
*/
@@ -2105,6 +2105,7 @@ export class Bucket extends BucketBase {
21052105
* | KMS | undefined | e | SSE-KMS, bucketKeyEnabled = e | new key |
21062106
* | KMS_MANAGED | undefined | e | SSE-KMS, bucketKeyEnabled = e | undefined |
21072107
* | S3_MANAGED | undefined | false | SSE-S3 | undefined |
2108+
* | S3_MANAGED | undefined | e | SSE-S3, bucketKeyEnabled = e | undefined |
21082109
* | UNENCRYPTED | undefined | true | ERROR! | ERROR! |
21092110
* | UNENCRYPTED | k | e | ERROR! | ERROR! |
21102111
* | KMS_MANAGED | k | e | ERROR! | ERROR! |
@@ -2127,12 +2128,9 @@ export class Bucket extends BucketBase {
21272128
throw new Error(`encryptionKey is specified, so 'encryption' must be set to KMS or DSSE (value: ${encryptionType})`);
21282129
}
21292130

2130-
// if bucketKeyEnabled is set, encryption must be set to KMS or DSSE.
2131-
if (
2132-
props.bucketKeyEnabled &&
2133-
![BucketEncryption.KMS, BucketEncryption.KMS_MANAGED, BucketEncryption.DSSE, BucketEncryption.DSSE_MANAGED].includes(encryptionType)
2134-
) {
2135-
throw new Error(`bucketKeyEnabled is specified, so 'encryption' must be set to KMS or DSSE (value: ${encryptionType})`);
2131+
// if bucketKeyEnabled is set, encryption can not be BucketEncryption.UNENCRYPTED
2132+
if (props.bucketKeyEnabled && encryptionType === BucketEncryption.UNENCRYPTED) {
2133+
throw new Error(`bucketKeyEnabled is specified, so 'encryption' must be set to KMS, DSSE or S3 (value: ${encryptionType})`);
21362134
}
21372135

21382136
if (encryptionType === BucketEncryption.UNENCRYPTED) {
@@ -2161,7 +2159,10 @@ export class Bucket extends BucketBase {
21612159
if (encryptionType === BucketEncryption.S3_MANAGED) {
21622160
const bucketEncryption = {
21632161
serverSideEncryptionConfiguration: [
2164-
{ serverSideEncryptionByDefault: { sseAlgorithm: 'AES256' } },
2162+
{
2163+
bucketKeyEnabled: props.bucketKeyEnabled,
2164+
serverSideEncryptionByDefault: { sseAlgorithm: 'AES256' },
2165+
},
21652166
],
21662167
};
21672168

packages/aws-cdk-lib/aws-s3/test/bucket.test.ts

+30-4
Original file line numberDiff line numberDiff line change
@@ -574,15 +574,41 @@ describe('bucket', () => {
574574
});
575575
});
576576

577-
test('throws error if bucketKeyEnabled is set, but encryption is not KMS', () => {
577+
test('bucketKeyEnabled can be enabled with SSE-S3', () => {
578578
const stack = new cdk.Stack();
579579

580+
// WHEN
581+
new s3.Bucket(stack, 'MyBucket', { bucketKeyEnabled: true, encryption: s3.BucketEncryption.S3_MANAGED });
582+
Template.fromStack(stack).hasResourceProperties('AWS::S3::Bucket', {
583+
BucketEncryption: {
584+
ServerSideEncryptionConfiguration: [
585+
{
586+
ServerSideEncryptionByDefault: { SSEAlgorithm: 'AES256' },
587+
BucketKeyEnabled: true,
588+
},
589+
],
590+
},
591+
});
592+
593+
});
594+
test('bucketKeyEnabled can not be enabled with UNENCRYPTED', () => {
595+
const stack = new cdk.Stack();
596+
597+
// WHEN
580598
expect(() => {
581-
new s3.Bucket(stack, 'MyBucket', { bucketKeyEnabled: true, encryption: s3.BucketEncryption.S3_MANAGED });
582-
}).toThrow("bucketKeyEnabled is specified, so 'encryption' must be set to KMS or DSSE (value: S3_MANAGED)");
599+
new s3.Bucket(stack, 'MyBucket', {
600+
bucketKeyEnabled: true,
601+
encryption: s3.BucketEncryption.UNENCRYPTED,
602+
});
603+
}).toThrow(/bucketKeyEnabled is specified, so 'encryption' must be set to KMS, DSSE or S3/);
604+
});
605+
606+
test('bucketKeyEnabled can NOT be enabled with encryption undefined', () => {
607+
const stack = new cdk.Stack();
608+
583609
expect(() => {
584610
new s3.Bucket(stack, 'MyBucket3', { bucketKeyEnabled: true });
585-
}).toThrow("bucketKeyEnabled is specified, so 'encryption' must be set to KMS or DSSE (value: UNENCRYPTED)");
611+
}).toThrow("bucketKeyEnabled is specified, so 'encryption' must be set to KMS, DSSE or S3 (value: UNENCRYPTED)");
586612

587613
});
588614

0 commit comments

Comments
 (0)