From 639e0addf1c2034db8ec3370c5a5e741c2421f6a Mon Sep 17 00:00:00 2001
From: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
Date: Wed, 10 Aug 2016 17:37:34 +0900
Subject: [PATCH] Fix. Correct how CORS rules are handled.

This commit fixes an issue where CORS rules would not be read and thus refreshed
correctly should there be a change introduced externally e.g. CORS configuration
was edited outside of Terraform.

Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
---
 .../providers/aws/resource_aws_s3_bucket.go   | 27 +++++++---
 .../aws/resource_aws_s3_bucket_test.go        | 50 +++++++++++++++++++
 2 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/builtin/providers/aws/resource_aws_s3_bucket.go b/builtin/providers/aws/resource_aws_s3_bucket.go
index 0978009a9bc0..2a8dfc5cfa4c 100644
--- a/builtin/providers/aws/resource_aws_s3_bucket.go
+++ b/builtin/providers/aws/resource_aws_s3_bucket.go
@@ -471,20 +471,32 @@ func resourceAwsS3BucketRead(d *schema.ResourceData, meta interface{}) error {
 	cors, err := s3conn.GetBucketCors(&s3.GetBucketCorsInput{
 		Bucket: aws.String(d.Id()),
 	})
-	log.Printf("[DEBUG] S3 bucket: %s, read CORS: %v", d.Id(), cors)
 	if err != nil {
+		// An S3 Bucket might not have CORS configuration set.
+		if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() != "NoSuchCORSConfiguration" {
+			return err
+		}
+		log.Printf("[WARN] S3 bucket: %s, no CORS configuration could be found.", d.Id())
+	}
+	log.Printf("[DEBUG] S3 bucket: %s, read CORS: %v", d.Id(), cors)
+	if cors.CORSRules != nil {
 		rules := make([]map[string]interface{}, 0, len(cors.CORSRules))
 		for _, ruleObject := range cors.CORSRules {
 			rule := make(map[string]interface{})
-			rule["allowed_headers"] = ruleObject.AllowedHeaders
-			rule["allowed_methods"] = ruleObject.AllowedMethods
-			rule["allowed_origins"] = ruleObject.AllowedOrigins
-			rule["expose_headers"] = ruleObject.ExposeHeaders
-			rule["max_age_seconds"] = ruleObject.MaxAgeSeconds
+			rule["allowed_headers"] = flattenStringList(ruleObject.AllowedHeaders)
+			rule["allowed_methods"] = flattenStringList(ruleObject.AllowedMethods)
+			rule["allowed_origins"] = flattenStringList(ruleObject.AllowedOrigins)
+			// Both the "ExposeHeaders" and "MaxAgeSeconds" might not be set.
+			if ruleObject.AllowedOrigins != nil {
+				rule["expose_headers"] = flattenStringList(ruleObject.ExposeHeaders)
+			}
+			if ruleObject.MaxAgeSeconds != nil {
+				rule["max_age_seconds"] = int(*ruleObject.MaxAgeSeconds)
+			}
 			rules = append(rules, rule)
 		}
 		if err := d.Set("cors_rule", rules); err != nil {
-			return fmt.Errorf("error reading S3 bucket \"%s\" CORS rules: %s", d.Id(), err)
+			return err
 		}
 	}
 
@@ -567,7 +579,6 @@ func resourceAwsS3BucketRead(d *schema.ResourceData, meta interface{}) error {
 	accelerate, err := s3conn.GetBucketAccelerateConfiguration(&s3.GetBucketAccelerateConfigurationInput{
 		Bucket: aws.String(d.Id()),
 	})
-	log.Printf("[DEBUG] S3 bucket: %s, read Acceleration: %v", d.Id(), accelerate)
 	if err != nil {
 		// Amazon S3 Transfer Acceleration might not be supported in the
 		// given region, for example, China (Beijing) and the Government
diff --git a/builtin/providers/aws/resource_aws_s3_bucket_test.go b/builtin/providers/aws/resource_aws_s3_bucket_test.go
index 25f64479a0a4..5773af4c3947 100644
--- a/builtin/providers/aws/resource_aws_s3_bucket_test.go
+++ b/builtin/providers/aws/resource_aws_s3_bucket_test.go
@@ -395,11 +395,61 @@ func TestAccAWSS3Bucket_Versioning(t *testing.T) {
 
 func TestAccAWSS3Bucket_Cors(t *testing.T) {
 	rInt := acctest.RandInt()
+
+	updateBucketCors := func(n string) resource.TestCheckFunc {
+		return func(s *terraform.State) error {
+			rs, ok := s.RootModule().Resources[n]
+			if !ok {
+				return fmt.Errorf("Not found: %s", n)
+			}
+
+			conn := testAccProvider.Meta().(*AWSClient).s3conn
+			_, err := conn.PutBucketCors(&s3.PutBucketCorsInput{
+				Bucket: aws.String(rs.Primary.ID),
+				CORSConfiguration: &s3.CORSConfiguration{
+					CORSRules: []*s3.CORSRule{
+						&s3.CORSRule{
+							AllowedHeaders: []*string{aws.String("*")},
+							AllowedMethods: []*string{aws.String("GET")},
+							AllowedOrigins: []*string{aws.String("https://www.example.com")},
+						},
+					},
+				},
+			})
+			if err != nil {
+				if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() != "NoSuchCORSConfiguration" {
+					return err
+				}
+			}
+			return nil
+		}
+	}
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckAWSS3BucketDestroy,
 		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSS3BucketConfigWithCORS(rInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSS3BucketExists("aws_s3_bucket.bucket"),
+					testAccCheckAWSS3BucketCors(
+						"aws_s3_bucket.bucket",
+						[]*s3.CORSRule{
+							&s3.CORSRule{
+								AllowedHeaders: []*string{aws.String("*")},
+								AllowedMethods: []*string{aws.String("PUT"), aws.String("POST")},
+								AllowedOrigins: []*string{aws.String("https://www.example.com")},
+								ExposeHeaders:  []*string{aws.String("x-amz-server-side-encryption"), aws.String("ETag")},
+								MaxAgeSeconds:  aws.Int64(3000),
+							},
+						},
+					),
+					updateBucketCors("aws_s3_bucket.bucket"),
+				),
+				ExpectNonEmptyPlan: true,
+			},
 			resource.TestStep{
 				Config: testAccAWSS3BucketConfigWithCORS(rInt),
 				Check: resource.ComposeTestCheckFunc(