Skip to content

Commit c2d6847

Browse files
slipdexicrix0rrr
slipdexic
authored and
rix0rrr
committed
feat(cloudtrail): accept existing S3 bucket (#3680)
Make CloudTrail accept existing S3 bucket to write to. Fixes #3651.
1 parent 210ed8f commit c2d6847

File tree

4 files changed

+207
-17
lines changed

4 files changed

+207
-17
lines changed

‎packages/@aws-cdk/aws-cloudtrail/lib/index.ts

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ export interface TrailProps {
9393
* @default - No prefix.
9494
*/
9595
readonly s3KeyPrefix?: string;
96+
97+
/** The Amazon S3 bucket
98+
*
99+
* @default - if not supplied a bucket will be created with all the correct permisions
100+
*/
101+
readonly bucket?: s3.IBucket
96102
}
97103

98104
export enum ReadWriteType {
@@ -109,6 +115,10 @@ export enum ReadWriteType {
109115
*
110116
* const cloudTrail = new CloudTrail(this, 'MyTrail');
111117
*
118+
* NOTE the above example creates an UNENCRYPTED bucket by default,
119+
* If you are required to use an Encrypted bucket you can supply a preconfigured bucket
120+
* via TrailProps
121+
*
112122
*/
113123
export class Trail extends Resource {
114124

@@ -122,33 +132,36 @@ export class Trail extends Resource {
122132
*/
123133
public readonly trailSnsTopicArn: string;
124134

135+
private s3bucket: s3.IBucket;
125136
private eventSelectors: EventSelector[] = [];
126137

127138
constructor(scope: Construct, id: string, props: TrailProps = {}) {
128139
super(scope, id, {
129140
physicalName: props.trailName,
130141
});
131142

132-
const s3bucket = new s3.Bucket(this, 'S3', {encryption: s3.BucketEncryption.UNENCRYPTED});
133143
const cloudTrailPrincipal = new iam.ServicePrincipal("cloudtrail.amazonaws.com");
134144

135-
s3bucket.addToResourcePolicy(new iam.PolicyStatement({
136-
resources: [s3bucket.bucketArn],
137-
actions: ['s3:GetBucketAcl'],
138-
principals: [cloudTrailPrincipal],
139-
}));
140-
141-
s3bucket.addToResourcePolicy(new iam.PolicyStatement({
142-
resources: [s3bucket.arnForObjects(`AWSLogs/${Stack.of(this).account}/*`)],
143-
actions: ["s3:PutObject"],
144-
principals: [cloudTrailPrincipal],
145-
conditions: {
146-
StringEquals: {'s3:x-amz-acl': "bucket-owner-full-control"}
147-
}
148-
}));
145+
this.s3bucket = props.bucket || new s3.Bucket(this, 'S3', {encryption: s3.BucketEncryption.UNENCRYPTED});
146+
147+
this.s3bucket.addToResourcePolicy(new iam.PolicyStatement({
148+
resources: [this.s3bucket.bucketArn],
149+
actions: ['s3:GetBucketAcl'],
150+
principals: [cloudTrailPrincipal],
151+
}));
152+
153+
this.s3bucket.addToResourcePolicy(new iam.PolicyStatement({
154+
resources: [this.s3bucket.arnForObjects(`AWSLogs/${Stack.of(this).account}/*`)],
155+
actions: ["s3:PutObject"],
156+
principals: [cloudTrailPrincipal],
157+
conditions: {
158+
StringEquals: {'s3:x-amz-acl': "bucket-owner-full-control"}
159+
}
160+
}));
149161

150162
let logGroup: logs.CfnLogGroup | undefined;
151163
let logsRole: iam.IRole | undefined;
164+
152165
if (props.sendToCloudWatchLogs) {
153166
logGroup = new logs.CfnLogGroup(this, "LogGroup", {
154167
retentionInDays: props.cloudWatchLogsRetention || logs.RetentionDays.ONE_YEAR
@@ -161,6 +174,7 @@ export class Trail extends Resource {
161174
resources: [logGroup.attrArn],
162175
}));
163176
}
177+
164178
if (props.managementEvents) {
165179
const managementEvent = {
166180
includeManagementEvents: true,
@@ -177,7 +191,7 @@ export class Trail extends Resource {
177191
includeGlobalServiceEvents: props.includeGlobalServiceEvents == null ? true : props.includeGlobalServiceEvents,
178192
trailName: this.physicalName,
179193
kmsKeyId: props.kmsKey && props.kmsKey.keyArn,
180-
s3BucketName: s3bucket.bucketName,
194+
s3BucketName: this.s3bucket.bucketName,
181195
s3KeyPrefix: props.s3KeyPrefix,
182196
cloudWatchLogsLogGroupArn: logGroup && logGroup.attrArn,
183197
cloudWatchLogsRoleArn: logsRole && logsRole.roleArn,
@@ -192,7 +206,7 @@ export class Trail extends Resource {
192206
});
193207
this.trailSnsTopicArn = trail.attrSnsTopicArn;
194208

195-
const s3BucketPolicy = s3bucket.node.findChild("Policy").node.findChild("Resource") as s3.CfnBucketPolicy;
209+
const s3BucketPolicy = this.s3bucket.node.findChild("Policy").node.findChild("Resource") as s3.CfnBucketPolicy;
196210
trail.node.addDependency(s3BucketPolicy);
197211

198212
// If props.sendToCloudWatchLogs is set to true then the trail needs to depend on the created logsRole

‎packages/@aws-cdk/aws-cloudtrail/test/integ.cloudtrail-supplied-bucket.lit.expected.json

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
{
2+
"Resources": {
3+
"Bucket83908E77": {
4+
"Type": "AWS::S3::Bucket",
5+
"UpdateReplacePolicy": "Delete",
6+
"DeletionPolicy": "Delete"
7+
},
8+
"S3486F821D": {
9+
"Type": "AWS::S3::Bucket",
10+
"UpdateReplacePolicy": "Retain",
11+
"DeletionPolicy": "Retain"
12+
},
13+
"S3Policy2E4AA1D6": {
14+
"Type": "AWS::S3::BucketPolicy",
15+
"Properties": {
16+
"Bucket": {
17+
"Ref": "S3486F821D"
18+
},
19+
"PolicyDocument": {
20+
"Statement": [
21+
{
22+
"Action": "s3:GetBucketAcl",
23+
"Effect": "Allow",
24+
"Principal": {
25+
"Service": "cloudtrail.amazonaws.com"
26+
},
27+
"Resource": {
28+
"Fn::GetAtt": [
29+
"S3486F821D",
30+
"Arn"
31+
]
32+
}
33+
},
34+
{
35+
"Action": "s3:PutObject",
36+
"Condition": {
37+
"StringEquals": {
38+
"s3:x-amz-acl": "bucket-owner-full-control"
39+
}
40+
},
41+
"Effect": "Allow",
42+
"Principal": {
43+
"Service": "cloudtrail.amazonaws.com"
44+
},
45+
"Resource": {
46+
"Fn::Join": [
47+
"",
48+
[
49+
{
50+
"Fn::GetAtt": [
51+
"S3486F821D",
52+
"Arn"
53+
]
54+
},
55+
"/AWSLogs/",
56+
{
57+
"Ref": "AWS::AccountId"
58+
},
59+
"/*"
60+
]
61+
]
62+
}
63+
}
64+
],
65+
"Version": "2012-10-17"
66+
}
67+
}
68+
},
69+
"Trail022F0CF2": {
70+
"Type": "AWS::CloudTrail::Trail",
71+
"Properties": {
72+
"IsLogging": true,
73+
"S3BucketName": {
74+
"Ref": "S3486F821D"
75+
},
76+
"EnableLogFileValidation": true,
77+
"EventSelectors": [
78+
{
79+
"DataResources": [
80+
{
81+
"Type": "AWS::S3::Object",
82+
"Values": [
83+
{
84+
"Fn::Join": [
85+
"",
86+
[
87+
{
88+
"Fn::GetAtt": [
89+
"Bucket83908E77",
90+
"Arn"
91+
]
92+
},
93+
"/"
94+
]
95+
]
96+
}
97+
]
98+
}
99+
]
100+
}
101+
],
102+
"IncludeGlobalServiceEvents": true,
103+
"IsMultiRegionTrail": true
104+
},
105+
"DependsOn": [
106+
"S3Policy2E4AA1D6"
107+
]
108+
}
109+
}
110+
}

‎packages/@aws-cdk/aws-cloudtrail/test/integ.cloudtrail-supplied-bucket.lit.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import iam = require('@aws-cdk/aws-iam');
2+
import s3 = require('@aws-cdk/aws-s3');
3+
import cdk = require('@aws-cdk/core');
4+
import { Stack } from '@aws-cdk/core';
5+
6+
import cloudtrail = require('../lib');
7+
8+
const app = new cdk.App();
9+
const stack = new cdk.Stack(app, 'integ-cloudtrail');
10+
11+
const bucket = new s3.Bucket(stack, 'Bucket', { removalPolicy: cdk.RemovalPolicy.DESTROY });
12+
13+
// using exctecy the same code as inside the cloudtrail class to produce the supplied bucket and policy
14+
const cloudTrailPrincipal = new iam.ServicePrincipal("cloudtrail.amazonaws.com");
15+
16+
const Trailbucket = new s3.Bucket(stack, 'S3', {encryption: s3.BucketEncryption.UNENCRYPTED});
17+
18+
Trailbucket.addToResourcePolicy(new iam.PolicyStatement({
19+
resources: [Trailbucket.bucketArn],
20+
actions: ['s3:GetBucketAcl'],
21+
principals: [cloudTrailPrincipal],
22+
}));
23+
24+
Trailbucket.addToResourcePolicy(new iam.PolicyStatement({
25+
resources: [Trailbucket.arnForObjects(`AWSLogs/${Stack.of(stack).account}/*`)],
26+
actions: ["s3:PutObject"],
27+
principals: [cloudTrailPrincipal],
28+
conditions: {
29+
StringEquals: {'s3:x-amz-acl': "bucket-owner-full-control"}
30+
}
31+
}));
32+
33+
const trail = new cloudtrail.Trail(stack, 'Trail', {bucket: Trailbucket});
34+
35+
trail.addS3EventSelector([bucket.arnForObjects('')]);
36+
37+
app.synth();

‎packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { expect, haveResource, not, SynthUtils } from '@aws-cdk/assert';
2+
import iam = require('@aws-cdk/aws-iam');
23
import { RetentionDays } from '@aws-cdk/aws-logs';
4+
import s3 = require('@aws-cdk/aws-s3');
35
import { Stack } from '@aws-cdk/core';
46
import { Test } from 'nodeunit';
57
import { ReadWriteType, Trail } from '../lib';
@@ -67,6 +69,33 @@ export = {
6769
test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']);
6870
test.done();
6971
},
72+
'with s3bucket'(test: Test) {
73+
const stack = getTestStack();
74+
const Trailbucket = new s3.Bucket(stack, 'S3');
75+
const cloudTrailPrincipal = new iam.ServicePrincipal("cloudtrail.amazonaws.com");
76+
Trailbucket.addToResourcePolicy(new iam.PolicyStatement({
77+
resources: [Trailbucket.bucketArn],
78+
actions: ['s3:GetBucketAcl'],
79+
principals: [cloudTrailPrincipal],
80+
}));
81+
82+
Trailbucket.addToResourcePolicy(new iam.PolicyStatement({
83+
resources: [Trailbucket.arnForObjects(`AWSLogs/${Stack.of(stack).account}/*`)],
84+
actions: ["s3:PutObject"],
85+
principals: [cloudTrailPrincipal],
86+
conditions: {
87+
StringEquals: {'s3:x-amz-acl': "bucket-owner-full-control"}
88+
}
89+
}));
90+
91+
new Trail(stack, 'Trail', {bucket: Trailbucket});
92+
93+
expect(stack).to(haveResource("AWS::CloudTrail::Trail"));
94+
expect(stack).to(haveResource("AWS::S3::Bucket"));
95+
expect(stack).to(haveResource("AWS::S3::BucketPolicy"));
96+
expect(stack).to(not(haveResource("AWS::Logs::LogGroup")));
97+
test.done();
98+
},
7099
'with cloud watch logs': {
71100
'enabled'(test: Test) {
72101
const stack = getTestStack();

0 commit comments

Comments
 (0)