Skip to content

Commit

Permalink
fix(aws-cloudtrail): correct created log policy when sendToCloudWatch…
Browse files Browse the repository at this point in the history
…Logs is true (#1966)
  • Loading branch information
RobinsonAndrew authored and Sam Goodwin committed Mar 7, 2019
1 parent 558d81f commit f06ff8e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 6 deletions.
10 changes: 8 additions & 2 deletions packages/@aws-cdk/aws-cloudtrail/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,9 @@ export class CloudTrail extends cdk.Construct {

logsRole = new iam.Role(this, 'LogsRole', { assumedBy: new iam.ServicePrincipal(cloudTrailPrincipal) });

const streamArn = `${logsRole.roleArn}:log-stream:*`;
logsRole.addToPolicy(new iam.PolicyStatement()
.addActions("logs:PutLogEvents", "logs:CreateLogStream")
.addResource(streamArn));
.addResource(logGroup.logGroupArn));
}
if (props.managementEvents) {
const managementEvent = {
Expand Down Expand Up @@ -181,6 +180,13 @@ export class CloudTrail extends cdk.Construct {
this.cloudTrailArn = trail.trailArn;
const s3BucketPolicy = s3bucket.node.findChild("Policy").node.findChild("Resource") as s3.CfnBucketPolicy;
trail.node.addDependency(s3BucketPolicy);

// If props.sendToCloudWatchLogs is set to true then the trail needs to depend on the created logsRole
// so that it can create the log stream for the log group. This ensures the logsRole is created and propagated
// before the trail tries to create the log stream.
if (logsRole !== undefined) {
trail.node.addDependency(logsRole);
}
}

/**
Expand Down
11 changes: 7 additions & 4 deletions packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ const ExpectedBucketPolicyProperties = {
}
};

const logsRolePolicyName = 'MyAmazingCloudTrailLogsRoleDefaultPolicy61DC49E7';
const logsRoleName = 'MyAmazingCloudTrailLogsRoleF2CCF977';

export = {
'constructs the expected resources': {
'with no properties'(test: Test) {
Expand Down Expand Up @@ -83,15 +86,15 @@ export = {
Effect: 'Allow',
Action: ['logs:PutLogEvents', 'logs:CreateLogStream'],
Resource: {
'Fn::Join': ['', [{ 'Fn::GetAtt': ['MyAmazingCloudTrailLogsRoleF2CCF977', 'Arn'] }, ':log-stream:*']],
'Fn::GetAtt': ['MyAmazingCloudTrailLogGroupAAD65144', 'Arn'],
}
}]
},
PolicyName: 'MyAmazingCloudTrailLogsRoleDefaultPolicy61DC49E7',
PolicyName: logsRolePolicyName,
Roles: [{ Ref: 'MyAmazingCloudTrailLogsRoleF2CCF977' }],
}));
const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']);
test.deepEqual(trail.DependsOn, [logsRolePolicyName, logsRoleName, 'MyAmazingCloudTrailS3Policy39C120B0']);
test.done();
},
'enabled and custom retention'(test: Test) {
Expand All @@ -110,7 +113,7 @@ export = {
RetentionInDays: 7
}));
const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']);
test.deepEqual(trail.DependsOn, [logsRolePolicyName, logsRoleName, 'MyAmazingCloudTrailS3Policy39C120B0']);
test.done();
},
},
Expand Down

0 comments on commit f06ff8e

Please sign in to comment.