From 816cfc04649ea1db729f55ef18d6d2fc6704b356 Mon Sep 17 00:00:00 2001
From: Romain Marcadier-Muller <romain.muller@telecomnancy.net>
Date: Mon, 25 Feb 2019 12:03:47 +0100
Subject: [PATCH] fix(cloudtrail): Invalid resource for policy when using
 sendToCloudWatchLogs (#1851)

Sets `this.cloudWatchLogsGroupArn` before using it, such that a correct
resource ARN is used in the policy generated for CloudTrail to be able
to create and use the required log stream.

Fixes #1848
---
 packages/@aws-cdk/aws-cloudtrail/lib/index.ts   | 17 +++++++----------
 .../aws-cloudtrail/test/test.cloudtrail.ts      | 16 ++++++++++++++--
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/packages/@aws-cdk/aws-cloudtrail/lib/index.ts b/packages/@aws-cdk/aws-cloudtrail/lib/index.ts
index 6a2cc83ae06c7..3e299e102ce0d 100644
--- a/packages/@aws-cdk/aws-cloudtrail/lib/index.ts
+++ b/packages/@aws-cdk/aws-cloudtrail/lib/index.ts
@@ -122,8 +122,6 @@ export enum LogRetention {
 export class CloudTrail extends cdk.Construct {
 
   public readonly cloudTrailArn: string;
-  private readonly cloudWatchLogsRoleArn?: string;
-  private readonly cloudWatchLogsGroupArn?: string;
   private eventSelectors: EventSelector[] = [];
 
   constructor(scope: cdk.Construct, id: string, props: CloudTrailProps = {}) {
@@ -143,20 +141,19 @@ export class CloudTrail extends cdk.Construct {
       .addServicePrincipal(cloudTrailPrincipal)
       .setCondition("StringEquals", {'s3:x-amz-acl': "bucket-owner-full-control"}));
 
+    let logGroup: logs.CfnLogGroup | undefined;
+    let logsRole: iam.IRole | undefined;
     if (props.sendToCloudWatchLogs) {
-      const logGroup = new logs.CfnLogGroup(this, "LogGroup", {
+      logGroup = new logs.CfnLogGroup(this, "LogGroup", {
         retentionInDays: props.cloudWatchLogsRetentionTimeDays || LogRetention.OneYear
       });
-      this.cloudWatchLogsGroupArn = logGroup.logGroupArn;
 
-      const logsRole = new iam.Role(this, 'LogsRole', {assumedBy: new iam.ServicePrincipal(cloudTrailPrincipal) });
+      logsRole = new iam.Role(this, 'LogsRole', { assumedBy: new iam.ServicePrincipal(cloudTrailPrincipal) });
 
-      const streamArn = `${this.cloudWatchLogsRoleArn}:log-stream:*`;
+      const streamArn = `${logsRole.roleArn}:log-stream:*`;
       logsRole.addToPolicy(new iam.PolicyStatement()
         .addActions("logs:PutLogEvents", "logs:CreateLogStream")
         .addResource(streamArn));
-      this.cloudWatchLogsRoleArn = logsRole.roleArn;
-
     }
     if (props.managementEvents) {
       const managementEvent =  {
@@ -176,8 +173,8 @@ export class CloudTrail extends cdk.Construct {
       kmsKeyId:  props.kmsKey && props.kmsKey.keyArn,
       s3BucketName: s3bucket.bucketName,
       s3KeyPrefix: props.s3KeyPrefix,
-      cloudWatchLogsLogGroupArn: this.cloudWatchLogsGroupArn,
-      cloudWatchLogsRoleArn: this.cloudWatchLogsRoleArn,
+      cloudWatchLogsLogGroupArn: logGroup && logGroup.logGroupArn,
+      cloudWatchLogsRoleArn: logsRole && logsRole.roleArn,
       snsTopicName: props.snsTopic,
       eventSelectors: this.eventSelectors
     });
diff --git a/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts b/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts
index ae6066ec43ff4..90f909c495013 100644
--- a/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts
+++ b/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts
@@ -75,8 +75,20 @@ export = {
         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
+        expect(stack).to(haveResource("AWS::Logs::LogGroup", { RetentionInDays: 365 }));
+        expect(stack).to(haveResource("AWS::IAM::Policy", {
+          PolicyDocument: {
+            Version: '2012-10-17',
+            Statement: [{
+              Effect: 'Allow',
+              Action: ['logs:PutLogEvents', 'logs:CreateLogStream'],
+              Resource: {
+                'Fn::Join': ['', [{ 'Fn::GetAtt': ['MyAmazingCloudTrailLogsRoleF2CCF977', 'Arn'] }, ':log-stream:*']],
+              }
+            }]
+          },
+          PolicyName: 'MyAmazingCloudTrailLogsRoleDefaultPolicy61DC49E7',
+          Roles: [{ Ref: 'MyAmazingCloudTrailLogsRoleF2CCF977' }],
         }));
         const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
         test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']);