From e475fdcda3fd1a58e4e39664380b2352c63d2684 Mon Sep 17 00:00:00 2001
From: Radek Simko <radek.simko@gmail.com>
Date: Sat, 5 Aug 2017 21:20:38 +0200
Subject: [PATCH 1/2] r/cloudtrail: Add acc test for changing CW settings

---
 aws/resource_aws_cloudtrail_test.go | 209 ++++++++++++++++++++++++++++
 1 file changed, 209 insertions(+)

diff --git a/aws/resource_aws_cloudtrail_test.go b/aws/resource_aws_cloudtrail_test.go
index 08655ea671b..09c13190f59 100644
--- a/aws/resource_aws_cloudtrail_test.go
+++ b/aws/resource_aws_cloudtrail_test.go
@@ -18,6 +18,7 @@ func TestAccAWSCloudTrail(t *testing.T) {
 	testCases := map[string]map[string]func(t *testing.T){
 		"Trail": {
 			"basic":         testAccAWSCloudTrail_basic,
+			"cloudwatch":    testAccAWSCloudTrail_cloudwatch,
 			"enableLogging": testAccAWSCloudTrail_enable_logging,
 			"isMultiRegion": testAccAWSCloudTrail_is_multi_region,
 			"logValidation": testAccAWSCloudTrail_logValidation,
@@ -71,6 +72,35 @@ func testAccAWSCloudTrail_basic(t *testing.T) {
 	})
 }
 
+func testAccAWSCloudTrail_cloudwatch(t *testing.T) {
+	var trail cloudtrail.Trail
+	randInt := acctest.RandInt()
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSCloudTrailDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSCloudTrailConfigCloudWatch(randInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudTrailExists("aws_cloudtrail.test", &trail),
+					resource.TestCheckResourceAttrSet("aws_cloudtrail.test", "cloud_watch_logs_group_arn"),
+					resource.TestCheckResourceAttrSet("aws_cloudtrail.test", "cloud_watch_logs_role_arn"),
+				),
+			},
+			{
+				Config: testAccAWSCloudTrailConfigCloudWatchModified(randInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudTrailExists("aws_cloudtrail.test", &trail),
+					resource.TestCheckResourceAttrSet("aws_cloudtrail.test", "cloud_watch_logs_group_arn"),
+					resource.TestCheckResourceAttrSet("aws_cloudtrail.test", "cloud_watch_logs_role_arn"),
+				),
+			},
+		},
+	})
+}
+
 func testAccAWSCloudTrail_enable_logging(t *testing.T) {
 	var trail cloudtrail.Trail
 	cloudTrailRandInt := acctest.RandInt()
@@ -501,6 +531,185 @@ POLICY
 `, cloudTrailRandInt, cloudTrailRandInt, cloudTrailRandInt, cloudTrailRandInt)
 }
 
+func testAccAWSCloudTrailConfigCloudWatch(randInt int) string {
+	return fmt.Sprintf(`
+resource "aws_cloudtrail" "test" {
+  name = "tf-acc-test-%d"
+  s3_bucket_name = "${aws_s3_bucket.test.id}"
+
+  cloud_watch_logs_group_arn = "${aws_cloudwatch_log_group.test.arn}"
+  cloud_watch_logs_role_arn = "${aws_iam_role.test.arn}"
+}
+
+resource "aws_s3_bucket" "test" {
+  bucket = "tf-test-trail-%d"
+  force_destroy = true
+  policy = <<POLICY
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Sid": "AWSCloudTrailAclCheck",
+      "Effect": "Allow",
+      "Principal": "*",
+      "Action": "s3:GetBucketAcl",
+      "Resource": "arn:aws:s3:::tf-test-trail-%d"
+    },
+    {
+      "Sid": "AWSCloudTrailWrite",
+      "Effect": "Allow",
+      "Principal": "*",
+      "Action": "s3:PutObject",
+      "Resource": "arn:aws:s3:::tf-test-trail-%d/*",
+      "Condition": {
+        "StringEquals": {
+          "s3:x-amz-acl": "bucket-owner-full-control"
+        }
+      }
+    }
+  ]
+}
+POLICY
+}
+
+resource "aws_cloudwatch_log_group" "test" {
+  name = "tf-acc-test-cloudtrail-%d"
+}
+
+resource "aws_iam_role" "test" {
+  name = "tf-acc-test-cloudtrail-%d"
+  assume_role_policy = <<POLICY
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Sid": "",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": "cloudtrail.amazonaws.com"
+      },
+      "Action": "sts:AssumeRole"
+    }
+  ]
+}
+POLICY
+}
+
+resource "aws_iam_role_policy" "test" {
+  name = "tf-acc-test-cloudtrail-%d"
+  role = "${aws_iam_role.test.id}"
+  policy = <<POLICY
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Sid": "AWSCloudTrailCreateLogStream",
+      "Effect": "Allow",
+      "Action": [
+        "logs:CreateLogStream",
+        "logs:PutLogEvents"
+      ],
+      "Resource": "${aws_cloudwatch_log_group.test.arn}"
+    }
+  ]
+}
+POLICY
+}
+`, randInt, randInt, randInt, randInt, randInt, randInt, randInt)
+}
+
+func testAccAWSCloudTrailConfigCloudWatchModified(randInt int) string {
+	return fmt.Sprintf(`
+resource "aws_cloudtrail" "test" {
+  name = "tf-acc-test-%d"
+  s3_bucket_name = "${aws_s3_bucket.test.id}"
+
+  cloud_watch_logs_group_arn = "${aws_cloudwatch_log_group.second.arn}"
+  cloud_watch_logs_role_arn = "${aws_iam_role.test.arn}"
+}
+
+resource "aws_s3_bucket" "test" {
+  bucket = "tf-test-trail-%d"
+  force_destroy = true
+  policy = <<POLICY
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Sid": "AWSCloudTrailAclCheck",
+      "Effect": "Allow",
+      "Principal": "*",
+      "Action": "s3:GetBucketAcl",
+      "Resource": "arn:aws:s3:::tf-test-trail-%d"
+    },
+    {
+      "Sid": "AWSCloudTrailWrite",
+      "Effect": "Allow",
+      "Principal": "*",
+      "Action": "s3:PutObject",
+      "Resource": "arn:aws:s3:::tf-test-trail-%d/*",
+      "Condition": {
+        "StringEquals": {
+          "s3:x-amz-acl": "bucket-owner-full-control"
+        }
+      }
+    }
+  ]
+}
+POLICY
+}
+
+resource "aws_cloudwatch_log_group" "test" {
+  name = "tf-acc-test-cloudtrail-%d"
+}
+
+resource "aws_cloudwatch_log_group" "second" {
+  name = "tf-acc-test-cloudtrail-second-%d"
+}
+
+resource "aws_iam_role" "test" {
+  name = "tf-acc-test-cloudtrail-%d"
+  assume_role_policy = <<POLICY
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Sid": "",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": "cloudtrail.amazonaws.com"
+      },
+      "Action": "sts:AssumeRole"
+    }
+  ]
+}
+POLICY
+}
+
+resource "aws_iam_role_policy" "test" {
+  name = "tf-acc-test-cloudtrail-%d"
+  role = "${aws_iam_role.test.id}"
+  policy = <<POLICY
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Sid": "AWSCloudTrailCreateLogStream",
+      "Effect": "Allow",
+      "Action": [
+        "logs:CreateLogStream",
+        "logs:PutLogEvents"
+      ],
+      "Resource": "${aws_cloudwatch_log_group.second.arn}"
+    }
+  ]
+}
+POLICY
+}
+
+`, randInt, randInt, randInt, randInt, randInt, randInt, randInt, randInt)
+}
+
 func testAccAWSCloudTrailConfigMultiRegion(cloudTrailRandInt int) string {
 	return fmt.Sprintf(`
 resource "aws_cloudtrail" "foobar" {

From 7da0f7dd1a405029677d4a60cc4c0a147193c3d4 Mon Sep 17 00:00:00 2001
From: Radek Simko <radek.simko@gmail.com>
Date: Sat, 5 Aug 2017 21:04:49 +0200
Subject: [PATCH 2/2] r/cloudtrail: Fix CloudWatch role ARN/group updates

---
 aws/resource_aws_cloudtrail.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/aws/resource_aws_cloudtrail.go b/aws/resource_aws_cloudtrail.go
index 8c59a5d9408..08b224833d7 100644
--- a/aws/resource_aws_cloudtrail.go
+++ b/aws/resource_aws_cloudtrail.go
@@ -227,10 +227,10 @@ func resourceAwsCloudTrailUpdate(d *schema.ResourceData, meta interface{}) error
 	if d.HasChange("s3_key_prefix") {
 		input.S3KeyPrefix = aws.String(d.Get("s3_key_prefix").(string))
 	}
-	if d.HasChange("cloud_watch_logs_role_arn") {
+	if d.HasChange("cloud_watch_logs_role_arn") || d.HasChange("cloud_watch_logs_group_arn") {
+		// Both of these need to be provided together
+		// in the update call otherwise API complains
 		input.CloudWatchLogsRoleArn = aws.String(d.Get("cloud_watch_logs_role_arn").(string))
-	}
-	if d.HasChange("cloud_watch_logs_group_arn") {
 		input.CloudWatchLogsLogGroupArn = aws.String(d.Get("cloud_watch_logs_group_arn").(string))
 	}
 	if d.HasChange("include_global_service_events") {