provider/aws: CloudFront post-merge review updates

builtin/providers/aws/cloudfront_distribution_configuration_structure.go

@@ -82,42 +82,74 @@ func expandDistributionConfig(d *schema.ResourceData) *cloudfront.DistributionCo
 // aws_cloudfront_distribution resource.
 //
 // Used by the aws_cloudfront_distribution Read function.
-func flattenDistributionConfig(d *schema.ResourceData, distributionConfig *cloudfront.DistributionConfig) {
-	d.Set("origins", flattenOrigins(distributionConfig.Origins))
-	d.Set("enabled", *distributionConfig.Enabled)
-	d.Set("default_cache_behavior", flattenDefaultCacheBehavior(distributionConfig.DefaultCacheBehavior))
-	d.Set("viewer_certificate", flattenViewerCertificate(distributionConfig.ViewerCertificate))
-	d.Set("price_class", *distributionConfig.PriceClass)
+func flattenDistributionConfig(d *schema.ResourceData, distributionConfig *cloudfront.DistributionConfig) error {
+	var err error
+
+	d.Set("enabled", distributionConfig.Enabled)
+	d.Set("price_class", distributionConfig.PriceClass)
+
+	err = d.Set("default_cache_behavior", flattenDefaultCacheBehavior(distributionConfig.DefaultCacheBehavior))
+	if err != nil {
+		return err
+	}
+	err = d.Set("viewer_certificate", flattenViewerCertificate(distributionConfig.ViewerCertificate))
+	if err != nil {
+		return err
+	}
 
 	if distributionConfig.CallerReference != nil {
-		d.Set("caller_reference", *distributionConfig.CallerReference)
+		d.Set("caller_reference", distributionConfig.CallerReference)
 	}
 	if distributionConfig.Comment != nil {
 		if *distributionConfig.Comment != "" {
-			d.Set("comment", *distributionConfig.Comment)
+			d.Set("comment", distributionConfig.Comment)
 		}
 	}
 	if distributionConfig.DefaultRootObject != nil {
-		d.Set("default_root_object", *distributionConfig.DefaultRootObject)
+		d.Set("default_root_object", distributionConfig.DefaultRootObject)
 	}
+	if distributionConfig.WebACLId != nil {
+		d.Set("web_acl_id", distributionConfig.WebACLId)
+	}
+
 	if distributionConfig.CustomErrorResponses != nil {
-		d.Set("custom_error_response", flattenCustomErrorResponses(distributionConfig.CustomErrorResponses))
+		err = d.Set("custom_error_response", flattenCustomErrorResponses(distributionConfig.CustomErrorResponses))
+		if err != nil {
+			return err
+		}
 	}
 	if distributionConfig.CacheBehaviors != nil {
-		d.Set("cache_behavior", flattenCacheBehaviors(distributionConfig.CacheBehaviors))
+		err = d.Set("cache_behavior", flattenCacheBehaviors(distributionConfig.CacheBehaviors))
+		if err != nil {
+			return err
+		}
 	}
 	if distributionConfig.Logging != nil && *distributionConfig.Logging.Enabled {
-		d.Set("logging_config", flattenLoggingConfig(distributionConfig.Logging))
+		err = d.Set("logging_config", flattenLoggingConfig(distributionConfig.Logging))
+		if err != nil {
+			return err
+		}
 	}
 	if distributionConfig.Aliases != nil {
-		d.Set("aliases", flattenAliases(distributionConfig.Aliases))
+		err = d.Set("aliases", flattenAliases(distributionConfig.Aliases))
+		if err != nil {
+			return err
+		}
 	}
 	if distributionConfig.Restrictions != nil {
-		d.Set("restrictions", flattenRestrictions(distributionConfig.Restrictions))
+		err = d.Set("restrictions", flattenRestrictions(distributionConfig.Restrictions))
+		if err != nil {
+			return err
+		}
 	}
-	if distributionConfig.WebACLId != nil {
-		d.Set("web_acl_id", *distributionConfig.WebACLId)
+	if *distributionConfig.Origins.Quantity > 0 {
+		err = d.Set("origin", flattenOrigins(distributionConfig.Origins))
+		if err != nil {
+			return err
+		}
 	}
+
+	return nil
 }
 
 func expandDefaultCacheBehavior(m map[string]interface{}) *cloudfront.DefaultCacheBehavior {
@@ -872,10 +904,12 @@ func flattenViewerCertificate(vc *cloudfront.ViewerCertificate) *schema.Set {
 	if vc.IAMCertificateId != nil {
 		m["iam_certificate_id"] = *vc.IAMCertificateId
 		m["ssl_support_method"] = *vc.SSLSupportMethod
-	} else if vc.ACMCertificateArn != nil {
+	}
+	if vc.ACMCertificateArn != nil {
 		m["acm_certificate_arn"] = *vc.ACMCertificateArn
 		m["ssl_support_method"] = *vc.SSLSupportMethod
-	} else {
+	}
+	if vc.CloudFrontDefaultCertificate != nil {
 		m["cloudfront_default_certificate"] = *vc.CloudFrontDefaultCertificate
 	}
 	if vc.MinimumProtocolVersion != nil {
@@ -889,16 +923,16 @@ func flattenViewerCertificate(vc *cloudfront.ViewerCertificate) *schema.Set {
 func viewerCertificateHash(v interface{}) int {
 	var buf bytes.Buffer
 	m := v.(map[string]interface{})
-	if v, ok := m["iam_certificate_id"]; ok {
+	if v, ok := m["iam_certificate_id"]; ok && v.(string) != "" {
 		buf.WriteString(fmt.Sprintf("%s-", v.(string)))
 		buf.WriteString(fmt.Sprintf("%s-", m["ssl_support_method"].(string)))
-	} else if v, ok := m["acm_certificate_arn"]; ok {
+	} else if v, ok := m["acm_certificate_arn"]; ok && v.(string) != "" {
 		buf.WriteString(fmt.Sprintf("%s-", v.(string)))
 		buf.WriteString(fmt.Sprintf("%s-", m["ssl_support_method"].(string)))
 	} else {
 		buf.WriteString(fmt.Sprintf("%t-", m["cloudfront_default_certificate"].(bool)))
 	}
-	if v, ok := m["minimum_protocol_version"]; ok {
+	if v, ok := m["minimum_protocol_version"]; ok && v.(string) != "" {
 		buf.WriteString(fmt.Sprintf("%s-", v.(string)))
 	}
 	return hashcode.String(buf.String())

builtin/providers/aws/cloudfront_distribution_configuration_structure_test.go

@@ -193,23 +193,31 @@ func customErrorResponsesConfFirst() map[string]interface{} {
 
 func viewerCertificateConfSetCloudFrontDefault() map[string]interface{} {
 	return map[string]interface{}{
+		"acm_certificate_arn":            "",
 		"cloudfront_default_certificate": true,
+		"iam_certificate_id":             "",
+		"minimum_protocol_version":       "",
+		"ssl_support_method":             "",
 	}
 }
 
 func viewerCertificateConfSetIAM() map[string]interface{} {
 	return map[string]interface{}{
-		"iam_certificate_id":       "iamcert-01234567",
-		"ssl_support_method":       "vip",
-		"minimum_protocol_version": "TLSv1",
+		"acm_certificate_arn":            "",
+		"cloudfront_default_certificate": false,
+		"iam_certificate_id":             "iamcert-01234567",
+		"ssl_support_method":             "vip",
+		"minimum_protocol_version":       "TLSv1",
 	}
 }
 
 func viewerCertificateConfSetACM() map[string]interface{} {
 	return map[string]interface{}{
-		"acm_certificate_arn":      "arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012",
-		"ssl_support_method":       "sni-only",
-		"minimum_protocol_version": "TLSv1",
+		"acm_certificate_arn":            "arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012",
+		"cloudfront_default_certificate": false,
+		"iam_certificate_id":             "",
+		"ssl_support_method":             "sni-only",
+		"minimum_protocol_version":       "TLSv1",
 	}
 }
 
@@ -987,3 +995,33 @@ func TestCloudFrontStructure_falttenViewerCertificate_acm_certificate_arn(t *tes
 		t.Fatalf("Expected out to be %v, got %v, diff: %v", in, out, diff)
 	}
 }
+
+func TestCloudFrontStructure_viewerCertificateHash_IAM(t *testing.T) {
+	in := viewerCertificateConfSetIAM()
+	out := viewerCertificateHash(in)
+	expected := 1157261784
+
+	if expected != out {
+		t.Fatalf("Expected %v, got %v", expected, out)
+	}
+}
+
+func TestCloudFrontStructure_viewerCertificateHash_ACM(t *testing.T) {
+	in := viewerCertificateConfSetACM()
+	out := viewerCertificateHash(in)
+	expected := 2883600425
+
+	if expected != out {
+		t.Fatalf("Expected %v, got %v", expected, out)
+	}
+}
+
+func TestCloudFrontStructure_viewerCertificateHash_default(t *testing.T) {
+	in := viewerCertificateConfSetCloudFrontDefault()
+	out := viewerCertificateHash(in)
+	expected := 69840937
+
+	if expected != out {
+		t.Fatalf("Expected %v, got %v", expected, out)
+	}
+}

builtin/providers/aws/resource_aws_cloudfront_distribution.go

@@ -492,15 +492,21 @@ func resourceAwsCloudFrontDistributionRead(d *schema.ResourceData, meta interfac
 	}
 
 	// Update attributes from DistributionConfig
-	flattenDistributionConfig(d, resp.Distribution.DistributionConfig)
+	err = flattenDistributionConfig(d, resp.Distribution.DistributionConfig)
+	if err != nil {
+		return err
+	}
 	// Update other attributes outside of DistributionConfig
 	d.SetId(*resp.Distribution.Id)
-	d.Set("active_trusted_signers", flattenActiveTrustedSigners(resp.Distribution.ActiveTrustedSigners))
-	d.Set("status", *resp.Distribution.Status)
-	d.Set("domain_name", *resp.Distribution.DomainName)
+	err = d.Set("active_trusted_signers", flattenActiveTrustedSigners(resp.Distribution.ActiveTrustedSigners))
+	if err != nil {
+		return err
+	}
+	d.Set("status", resp.Distribution.Status)
+	d.Set("domain_name", resp.Distribution.DomainName)
 	d.Set("last_modified_time", aws.String(resp.Distribution.LastModifiedTime.String()))
-	d.Set("in_progress_validation_batches", *resp.Distribution.InProgressInvalidationBatches)
-	d.Set("etag", *resp.ETag)
+	d.Set("in_progress_validation_batches", resp.Distribution.InProgressInvalidationBatches)
+	d.Set("etag", resp.ETag)
 	return nil
 }
 
@@ -537,7 +543,10 @@ func resourceAwsCloudFrontDistributionDelete(d *schema.ResourceData, meta interf
 	}
 
 	// Distribution needs to be in deployed state again before it can be deleted.
-	resourceAwsCloudFrontDistributionWaitUntilDeployed(d.Id(), meta)
+	err = resourceAwsCloudFrontDistributionWaitUntilDeployed(d.Id(), meta)
+	if err != nil {
+		return err
+	}
 
 	// now delete
 	params := &cloudfront.DeleteDistributionInput{

builtin/providers/aws/resource_aws_cloudfront_distribution_test.go

@@ -3,6 +3,7 @@ package aws
 import (
 	"fmt"
 	"math/rand"
+	"os"
 	"testing"
 	"time"
 
@@ -12,6 +13,11 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
+// TestAccAWSCloudFrontDistribution_S3Origin runs an
+// aws_cloudfront_distribution acceptance test with a single S3 origin.
+//
+// If you are testing manually and can't wait for deletion, set the
+// TF_TEST_CLOUDFRONT_RETAIN environment variable.
 func TestAccAWSCloudFrontDistribution_S3Origin(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -30,6 +36,11 @@ func TestAccAWSCloudFrontDistribution_S3Origin(t *testing.T) {
 	})
 }
 
+// TestAccAWSCloudFrontDistribution_customOriginruns an
+// aws_cloudfront_distribution acceptance test with a single custom origin.
+//
+// If you are testing manually and can't wait for deletion, set the
+// TF_TEST_CLOUDFRONT_RETAIN environment variable.
 func TestAccAWSCloudFrontDistribution_customOrigin(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -48,6 +59,11 @@ func TestAccAWSCloudFrontDistribution_customOrigin(t *testing.T) {
 	})
 }
 
+// TestAccAWSCloudFrontDistribution_multiOrigin runs an
+// aws_cloudfront_distribution acceptance test with multiple origins.
+//
+// If you are testing manually and can't wait for deletion, set the
+// TF_TEST_CLOUDFRONT_RETAIN environment variable.
 func TestAccAWSCloudFrontDistribution_multiOrigin(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -66,6 +82,11 @@ func TestAccAWSCloudFrontDistribution_multiOrigin(t *testing.T) {
 	})
 }
 
+// TestAccAWSCloudFrontDistribution_noOptionalItemsConfig runs an
+// aws_cloudfront_distribution acceptance test with no optional items set.
+//
+// If you are testing manually and can't wait for deletion, set the
+// TF_TEST_CLOUDFRONT_RETAIN environment variable.
 func TestAccAWSCloudFrontDistribution_noOptionalItemsConfig(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -89,10 +110,15 @@ func testAccCheckCloudFrontDistributionDestroy(s *terraform.State) error {
 		if rs.Type != "aws_cloudfront_distribution" {
 			continue
 		}
-		dist, _ := testAccAuxCloudFrontGetDistributionConfig(s, k)
-
-		if *dist.DistributionConfig.Enabled != false {
-			return fmt.Errorf("CloudFront distribution should be disabled")
+		dist, err := testAccAuxCloudFrontGetDistributionConfig(s, k)
+		if err == nil {
+			if _, ok := os.LookupEnv("TF_TEST_CLOUDFRONT_RETAIN"); ok {
+				if *dist.DistributionConfig.Enabled != false {
+					return fmt.Errorf("CloudFront distribution should be disabled")
+				}
+				return nil
+			}
+			return fmt.Errorf("CloudFront distribution did not destroy")
 		}
 	}
 	return nil
@@ -130,6 +156,13 @@ func testAccAuxCloudFrontGetDistributionConfig(s *terraform.State, cloudFrontRes
 	return res.Distribution, nil
 }
 
+func testAccAWSCloudFrontDistributionRetainConfig() string {
+	if _, ok := os.LookupEnv("TF_TEST_CLOUDFRONT_RETAIN"); ok {
+		return "retain_on_delete = true"
+	}
+	return ""
+}
+
 var testAccAWSCloudFrontDistributionS3Config = fmt.Sprintf(`
 variable rand_id {
 	default = %d
@@ -179,9 +212,9 @@ resource "aws_cloudfront_distribution" "s3_distribution" {
 	viewer_certificate {
 		cloudfront_default_certificate = true
 	}
-	retain_on_delete = true
+	%s
 }
-`, rand.New(rand.NewSource(time.Now().UnixNano())).Int())
+`, rand.New(rand.NewSource(time.Now().UnixNano())).Int(), testAccAWSCloudFrontDistributionRetainConfig())
 
 var testAccAWSCloudFrontDistributionCustomConfig = fmt.Sprintf(`
 variable rand_id {
@@ -234,9 +267,9 @@ resource "aws_cloudfront_distribution" "custom_distribution" {
 	viewer_certificate {
 		cloudfront_default_certificate = true
 	}
-	retain_on_delete = true
+	%s
 }
-`, rand.New(rand.NewSource(time.Now().UnixNano())).Int())
+`, rand.New(rand.NewSource(time.Now().UnixNano())).Int(), testAccAWSCloudFrontDistributionRetainConfig())
 
 var testAccAWSCloudFrontDistributionMultiOriginConfig = fmt.Sprintf(`
 variable rand_id {
@@ -336,9 +369,9 @@ resource "aws_cloudfront_distribution" "multi_origin_distribution" {
 	viewer_certificate {
 		cloudfront_default_certificate = true
 	}
-	retain_on_delete = true
+	%s
 }
-`, rand.New(rand.NewSource(time.Now().UnixNano())).Int())
+`, rand.New(rand.NewSource(time.Now().UnixNano())).Int(), testAccAWSCloudFrontDistributionRetainConfig())
 
 var testAccAWSCloudFrontDistributionNoOptionalItemsConfig = fmt.Sprintf(`
 variable rand_id {
@@ -383,6 +416,6 @@ resource "aws_cloudfront_distribution" "no_optional_items" {
 	viewer_certificate {
 		cloudfront_default_certificate = true
 	}
-	retain_on_delete = true
+	%s
 }
-`, rand.New(rand.NewSource(time.Now().UnixNano())).Int())
+`, rand.New(rand.NewSource(time.Now().UnixNano())).Int(), testAccAWSCloudFrontDistributionRetainConfig())

builtin/providers/aws/resource_aws_cloudfront_origin_access_identity.go

@@ -71,8 +71,8 @@ func resourceAwsCloudFrontOriginAccessIdentityRead(d *schema.ResourceData, meta
 	flattenOriginAccessIdentityConfig(d, resp.CloudFrontOriginAccessIdentity.CloudFrontOriginAccessIdentityConfig)
 	// Update other attributes outside of DistributionConfig
 	d.SetId(*resp.CloudFrontOriginAccessIdentity.Id)
-	d.Set("etag", *resp.ETag)
-	d.Set("s3_canonical_user_id", *resp.CloudFrontOriginAccessIdentity.S3CanonicalUserId)
+	d.Set("etag", resp.ETag)
+	d.Set("s3_canonical_user_id", resp.CloudFrontOriginAccessIdentity.S3CanonicalUserId)
 	d.Set("cloudfront_access_identity_path", fmt.Sprintf("origin-access-identity/cloudfront/%s", *resp.CloudFrontOriginAccessIdentity.Id))
 	return nil
 }
@@ -124,7 +124,7 @@ func expandOriginAccessIdentityConfig(d *schema.ResourceData) *cloudfront.Origin
 
 func flattenOriginAccessIdentityConfig(d *schema.ResourceData, originAccessIdentityConfig *cloudfront.OriginAccessIdentityConfig) {
 	if originAccessIdentityConfig.Comment != nil {
-		d.Set("comment", *originAccessIdentityConfig.Comment)
+		d.Set("comment", originAccessIdentityConfig.Comment)
 	}
-	d.Set("caller_reference", *originAccessIdentityConfig.CallerReference)
+	d.Set("caller_reference", originAccessIdentityConfig.CallerReference)
 }