diff --git a/packages/@aws-cdk/aws-cloudtrail/lib/index.ts b/packages/@aws-cdk/aws-cloudtrail/lib/index.ts index 792d86b37f958..48929535daeb8 100644 --- a/packages/@aws-cdk/aws-cloudtrail/lib/index.ts +++ b/packages/@aws-cdk/aws-cloudtrail/lib/index.ts @@ -138,7 +138,7 @@ export class CloudTrail extends cdk.Construct { .addServicePrincipal(cloudTrailPrincipal)); s3bucket.addToResourcePolicy(new iam.PolicyStatement() - .addResource(s3bucket.arnForObjects(new cdk.FnConcat('/AWSLogs/', new cdk.AwsAccountId()))) + .addResource(s3bucket.arnForObjects(new cdk.FnConcat('AWSLogs/', new cdk.AwsAccountId(), "/*"))) .addActions("s3:PutObject") .addServicePrincipal(cloudTrailPrincipal) .setCondition("StringEquals", {'s3:x-amz-acl': "bucket-owner-full-control"})); @@ -182,6 +182,8 @@ export class CloudTrail extends cdk.Construct { eventSelectors: this.eventSelectors }); this.cloudTrailArn = trail.trailArn; + const s3BucketPolicy = s3bucket.findChild("Policy").findChild("Resource") as s3.cloudformation.BucketPolicyResource; + trail.addDependency(s3BucketPolicy); } /** diff --git a/packages/@aws-cdk/aws-cloudtrail/package.json b/packages/@aws-cdk/aws-cloudtrail/package.json index 8c5e5c4a2ae2c..2edbd87eb18d3 100644 --- a/packages/@aws-cdk/aws-cloudtrail/package.json +++ b/packages/@aws-cdk/aws-cloudtrail/package.json @@ -70,4 +70,4 @@ "@aws-cdk/aws-kms": "^0.18.1", "@aws-cdk/cdk": "^0.18.1" } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts b/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts index 84aa74638261c..59783aee5764f 100644 --- a/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts +++ b/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts @@ -3,16 +3,68 @@ import { Stack } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; import { CloudTrail, LogRetention, ReadWriteType } from '../lib'; +const ExpectedBucketPolicyProperties = { + PolicyDocument: { + Statement: [ + { + Action: "s3:GetBucketAcl", + Effect: "Allow", + Principal: { + Service: "cloudtrail.amazonaws.com" + }, + Resource: { + "Fn::GetAtt": [ + "MyAmazingCloudTrailS3A580FE27", + "Arn" + ] + } + }, + { + Action: "s3:PutObject", + Condition: { + StringEquals: { + "s3:x-amz-acl": "bucket-owner-full-control" + } + }, + Effect: "Allow", + Principal: { + Service: "cloudtrail.amazonaws.com" + }, + Resource: { + "Fn::Join": [ + "", + [ + { + "Fn::GetAtt": [ + "MyAmazingCloudTrailS3A580FE27", + "Arn" + ] + }, + "/AWSLogs/", + { + Ref: "AWS::AccountId" + }, + "/*" + ] + ] + } + } + ], + Version: "2012-10-17" + } +}; + export = { 'constructs the expected resources': { 'with no properties'(test: Test) { const stack = getTestStack(); new CloudTrail(stack, 'MyAmazingCloudTrail'); - expect(stack).to(haveResource("AWS::CloudTrail::Trail")); expect(stack).to(haveResource("AWS::S3::Bucket")); - expect(stack).to(haveResource("AWS::S3::BucketPolicy")); + expect(stack).to(haveResource("AWS::S3::BucketPolicy", ExpectedBucketPolicyProperties)); expect(stack).to(not(haveResource("AWS::Logs::LogGroup"))); + const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D; + test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']); test.done(); }, 'with cloud watch logs': { @@ -24,13 +76,14 @@ export = { expect(stack).to(haveResource("AWS::CloudTrail::Trail")); expect(stack).to(haveResource("AWS::S3::Bucket")); - expect(stack).to(haveResource("AWS::S3::BucketPolicy")); + expect(stack).to(haveResource("AWS::S3::BucketPolicy", ExpectedBucketPolicyProperties)); expect(stack).to(haveResource("AWS::Logs::LogGroup")); expect(stack).to(haveResource("AWS::IAM::Role")); expect(stack).to(haveResource("AWS::Logs::LogGroup", { RetentionInDays: 365 })); - + const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D; + test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']); test.done(); }, 'enabled and custom retention'(test: Test) { @@ -42,12 +95,14 @@ export = { expect(stack).to(haveResource("AWS::CloudTrail::Trail")); expect(stack).to(haveResource("AWS::S3::Bucket")); - expect(stack).to(haveResource("AWS::S3::BucketPolicy")); + expect(stack).to(haveResource("AWS::S3::BucketPolicy", ExpectedBucketPolicyProperties)); expect(stack).to(haveResource("AWS::Logs::LogGroup")); expect(stack).to(haveResource("AWS::IAM::Role")); expect(stack).to(haveResource("AWS::Logs::LogGroup", { RetentionInDays: 7 })); + const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D; + test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']); test.done(); }, }, @@ -60,7 +115,7 @@ export = { expect(stack).to(haveResource("AWS::CloudTrail::Trail")); expect(stack).to(haveResource("AWS::S3::Bucket")); - expect(stack).to(haveResource("AWS::S3::BucketPolicy")); + expect(stack).to(haveResource("AWS::S3::BucketPolicy", ExpectedBucketPolicyProperties)); expect(stack).to(not(haveResource("AWS::Logs::LogGroup"))); expect(stack).to(not(haveResource("AWS::IAM::Role"))); @@ -74,13 +129,14 @@ export = { test.equals(dataResource.Type, "AWS::S3::Object", "Expected the data resrouce type to be AWS::S3::Object"); test.equals(dataResource.Values.length, 1, "Expected there to be one value"); test.equals(dataResource.Values[0], "arn:aws:s3:::", "Expected the first type value to be the S3 type"); + test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']); test.done(); }, 'with management event'(test: Test) { const stack = getTestStack(); - new CloudTrail(stack, 'MyAmazingCloudTrail', {managementEvents: ReadWriteType.WriteOnly}); + new CloudTrail(stack, 'MyAmazingCloudTrail', { managementEvents: ReadWriteType.WriteOnly }); const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D; test.equals(trail.Properties.EventSelectors.length, 1);