Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(s3): allow accessLogsPrefix without accessLogsBucket #6709

Merged
merged 4 commits into from
Apr 13, 2020
Merged

fix(s3): allow accessLogsPrefix without accessLogsBucket #6709

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16981a0
fix(s3): allow independent serverAccessLogsPrefix without serverAcces…
lanwen Mar 12, 2020
3899a2a
docs alignment
lanwen Mar 30, 2020
6d75350
fix indent
lanwen Apr 12, 2020
b89a84c
Merge branch 'master' into fix_6599
mergify[bot] Apr 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions packages/@aws-cdk/aws-s3/lib/bucket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,12 +905,13 @@ export interface BucketProps {

/**
* Destination bucket for the server access logs.
* @default - Access logs are disabled
* @default - If "serverAccessLogsPrefix" undefined - access logs disabled, otherwise - log to current bucket.
*/
readonly serverAccessLogsBucket?: IBucket;

/**
* Optional log file prefix to use for the bucket's access logs.
* If defined without "serverAccessLogsBucket", enables access logs to current bucket with this prefix.
* @default - No log file prefix
*/
readonly serverAccessLogsPrefix?: string;
Expand Down Expand Up @@ -1294,16 +1295,12 @@ export class Bucket extends BucketBase {
}

private parseServerAccessLogs(props: BucketProps): CfnBucket.LoggingConfigurationProperty | undefined {
if (props.serverAccessLogsPrefix && !props.serverAccessLogsBucket) {
throw new Error('"serverAccessLogsBucket" is required if "serverAccessLogsPrefix" is set');
}

if (!props.serverAccessLogsBucket) {
if (!props.serverAccessLogsBucket && !props.serverAccessLogsPrefix) {
return undefined;
}

return {
destinationBucketName: props.serverAccessLogsBucket.bucketName,
destinationBucketName: props.serverAccessLogsBucket?.bucketName,
logFilePrefix: props.serverAccessLogsPrefix,
};
}
Expand Down
11 changes: 8 additions & 3 deletions packages/@aws-cdk/aws-s3/test/test.bucket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1780,11 +1780,16 @@ export = {
// GIVEN
const stack = new cdk.Stack();

// THEN
test.throws(() => new s3.Bucket(stack, 'MyBucket', {
new s3.Bucket(stack, 'MyBucket', {
serverAccessLogsPrefix: 'hello'
}), /"serverAccessLogsBucket" is required if "serverAccessLogsPrefix" is set/);
});

// THEN
expect(stack).to(haveResource('AWS::S3::Bucket', {
LoggingConfiguration: {
LogFilePrefix: 'hello'
}
}));
test.done();
},

Expand Down