Skip to content

Commit

Permalink
feat(s3): server access logs (#5072)
Browse files Browse the repository at this point in the history
* L2: enable server access logs in a S3 bucket

use serverAccessLogs and reference the L1 construct directly through LazyValue

* un-nested properties

* Update bucket.ts

* added integration tests

* add AC to the log group in the integs

* ACL support for logDelivery

* updated the test to new format

* Throw error if ACL would be changed while calling allowLogDelivery()

* allowLogDelivery method moved to Bucket and made private

* Bucket's accessControl is not anymore public

* README entry and removed accessControl from the import

Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Jan 13, 2020
1 parent 6c3d4c4 commit c9b074b
Show file tree
Hide file tree
Showing 5 changed files with 193 additions and 2 deletions.
22 changes: 22 additions & 0 deletions packages/@aws-cdk/aws-s3/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,28 @@ When `blockPublicPolicy` is set to `true`, `grantPublicRead()` throws an error.

[block public access settings]: https://docs.aws.amazon.com/AmazonS3/latest/dev/access-control-block-public-access.html

### Logging configuration

Use `serverAccessLogsBucket` to describe where server access logs are to be stored.

```ts
const accessLogsBucket = new Bucket(this, 'AccessLogsBucket');

const bucket = new Bucket(this, 'MyBucket', {
serverAccessLogsBucket: accessLogsBucket,
});
```

It's also possible to specify a prefix for Amazon S3 to assign to all log object keys.

```ts
const bucket = new Bucket(this, 'MyBucket', {
serverAccessLogsBucket: accessLogsBucket,
serverAccessLogsPrefix: 'logs'
});
```

[S3 Server access logging]: https://docs.aws.amazon.com/AmazonS3/latest/dev/ServerLogs.html

### Website redirection

Expand Down
50 changes: 49 additions & 1 deletion packages/@aws-cdk/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,18 @@ export interface BucketProps {
* @default - No CORS configuration.
*/
readonly cors?: CorsRule[];

/**
* Destination bucket for the server access logs.
* @default - Access logs are disabled
*/
readonly serverAccessLogsBucket?: IBucket;

/**
* Optional log file prefix to use for the bucket's access logs.
* @default - No log file prefix
*/
readonly serverAccessLogsPrefix?: string;
}

/**
Expand Down Expand Up @@ -982,6 +994,7 @@ export class Bucket extends BucketBase {
public policy?: BucketPolicy;
protected autoCreatePolicy = true;
protected disallowPublicAccess?: boolean;
private accessControl?: BucketAccessControl;
private readonly lifecycleRules: LifecycleRule[] = [];
private readonly versioned?: boolean;
private readonly notifications: BucketNotifications;
Expand All @@ -1006,7 +1019,8 @@ export class Bucket extends BucketBase {
publicAccessBlockConfiguration: props.blockPublicAccess,
metricsConfigurations: Lazy.anyValue({ produce: () => this.parseMetricConfiguration() }),
corsConfiguration: Lazy.anyValue({ produce: () => this.parseCorsConfiguration() }),
accessControl: props.accessControl,
accessControl: Lazy.stringValue({ produce: () => this.accessControl }),
loggingConfiguration: this.parseServerAccessLogs(props),
});

resource.applyRemovalPolicy(props.removalPolicy);
Expand All @@ -1029,6 +1043,11 @@ export class Bucket extends BucketBase {
this.bucketRegionalDomainName = resource.attrRegionalDomainName;

this.disallowPublicAccess = props.blockPublicAccess && props.blockPublicAccess.blockPublicPolicy;
this.accessControl = props.accessControl;

if (props.serverAccessLogsBucket instanceof Bucket) {
props.serverAccessLogsBucket.allowLogDelivery();
}

// Add all bucket metric configurations rules
(props.metrics || []).forEach(this.addMetric.bind(this));
Expand Down Expand Up @@ -1273,6 +1292,21 @@ 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) {
return undefined;
}

return {
destinationBucketName: props.serverAccessLogsBucket.bucketName,
logFilePrefix: props.serverAccessLogsPrefix,
};
}

private parseMetricConfiguration(): CfnBucket.MetricsConfigurationProperty[] | undefined {
if (!this.metrics || this.metrics.length === 0) {
return undefined;
Expand Down Expand Up @@ -1358,6 +1392,20 @@ export class Bucket extends BucketBase {
routingRules
};
}

/**
* Allows the LogDelivery group to write, fails if ACL was set differently.
*
* @see
* https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl
*/
private allowLogDelivery() {
if (this.accessControl && this.accessControl !== BucketAccessControl.LOG_DELIVERY_WRITE) {
throw new Error("Cannot enable log delivery to this bucket because the bucket's ACL has been set and can't be changed");
}

this.accessControl = BucketAccessControl.LOG_DELIVERY_WRITE;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"Resources": {
"MyAccessLogsBucketF7FE6635": {
"Type": "AWS::S3::Bucket",
"Properties": {
"AccessControl": "LogDeliveryWrite"
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
},
"MyBucketF68F3FF0": {
"Type": "AWS::S3::Bucket",
"Properties": {
"LoggingConfiguration": {
"DestinationBucketName": {
"Ref": "MyAccessLogsBucketF7FE6635"
},
"LogFilePrefix": "example"
}
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
}
}
}
19 changes: 19 additions & 0 deletions packages/@aws-cdk/aws-s3/test/integ.bucket.server-access-logs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/usr/bin/env node
import * as cdk from '@aws-cdk/core';
import * as s3 from '../lib';

const app = new cdk.App();

const stack = new cdk.Stack(app, 'aws-cdk-s3-access-logs');

const accessLogBucket = new s3.Bucket(stack, 'MyAccessLogsBucket', {
removalPolicy: cdk.RemovalPolicy.DESTROY,
});

new s3.Bucket(stack, 'MyBucket', {
serverAccessLogsBucket: accessLogBucket,
serverAccessLogsPrefix: 'example',
removalPolicy: cdk.RemovalPolicy.DESTROY,
});

app.synth();
79 changes: 78 additions & 1 deletion packages/@aws-cdk/aws-s3/test/test.bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1723,5 +1723,82 @@ export = {
// THEN
new s3.Bucket(stack, 'b', { encryptionKey: key });
test.done();
}
},

'Bucket with Server Access Logs'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

// WHEN
const accessLogBucket = new s3.Bucket(stack, 'AccessLogs');
new s3.Bucket(stack, 'MyBucket', {
serverAccessLogsBucket: accessLogBucket,
});

// THEN
expect(stack).to(haveResource('AWS::S3::Bucket', {
LoggingConfiguration: {
DestinationBucketName: {
Ref: 'AccessLogs8B620ECA',
},
}
}));

test.done();
},

'Bucket with Server Access Logs with Prefix'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

// WHEN
const accessLogBucket = new s3.Bucket(stack, 'AccessLogs');
new s3.Bucket(stack, 'MyBucket', {
serverAccessLogsBucket: accessLogBucket,
serverAccessLogsPrefix: 'hello',
});

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

test.done();
},

'Access log prefix given without bucket'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

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

test.done();
},

'Bucket Allow Log delivery changes bucket Access Control should fail'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

// WHEN
const accessLogBucket = new s3.Bucket(stack, 'AccessLogs', {
accessControl: s3.BucketAccessControl.AUTHENTICATED_READ,
});
test.throws(() =>
new s3.Bucket(stack, 'MyBucket', {
serverAccessLogsBucket: accessLogBucket,
serverAccessLogsPrefix: 'hello',
accessControl: s3.BucketAccessControl.AUTHENTICATED_READ,
})
, /Cannot enable log delivery to this bucket because the bucket's ACL has been set and can't be changed/);

test.done();
},
};

0 comments on commit c9b074b

Please sign in to comment.