Merge pull request #21776 from hashicorp/b-elb-policy-predefined-ssl

r/aws_load_balancer_policy: suppress computed `policy_attribute` diffs

.changelog/21776.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_load_balancer_policy: Suppress `policy_attribute` differences
+```

internal/service/elb/enum.go

@@ -0,0 +1,6 @@
+package elb
+
+const (
+	ReferenceSecurityPolicy  = "Reference-Security-Policy"
+	SSLNegotiationPolicyType = "SSLNegotiationPolicyType"
+)

internal/service/elb/flex.go

@@ -174,15 +174,19 @@ func ExpandPolicyAttributes(configured []interface{}) []*elb.PolicyAttribute {
 
 // Flattens an array of PolicyAttributes into a []interface{}
 func FlattenPolicyAttributes(list []*elb.PolicyAttributeDescription) []interface{} {
-	attributes := []interface{}{}
+	var attributes []interface{}
+
 	for _, attrdef := range list {
+		if attrdef == nil {
+			continue
+		}
+
 		attribute := map[string]string{
-			"name":  *attrdef.AttributeName,
-			"value": *attrdef.AttributeValue,
+			"name":  aws.StringValue(attrdef.AttributeName),
+			"value": aws.StringValue(attrdef.AttributeValue),
 		}
 
 		attributes = append(attributes, attribute)
-
 	}
 
 	return attributes

internal/service/elb/policy.go

@@ -42,6 +42,10 @@ func ResourcePolicy() *schema.Resource {
 			"policy_attribute": {
 				Type:     schema.TypeSet,
 				Optional: true,
+				// If policy_attribute(s) are not specified,
+				// default values per policy type (see https://awscli.amazonaws.com/v2/documentation/api/latest/reference/elb/describe-load-balancer-policies.html)
+				// will be returned by the API; thus, this TypeSet is marked as Computed.
+				Computed: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"name": {
@@ -55,6 +59,10 @@ func ResourcePolicy() *schema.Resource {
 						},
 					},
 				},
+				// For policy types like "SSLNegotiationPolicyType" that can reference predefined policies
+				// via the "Reference-Security-Policy" policy_attribute (https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/elb-security-policy-table.html),
+				// differences caused by additional attributes returned by the API are suppressed.
+				DiffSuppressFunc: suppressPolicyAttributeDiffs,
 			},
 		},
 	}
@@ -63,23 +71,14 @@ func ResourcePolicy() *schema.Resource {
 func resourcePolicyCreate(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).ELBConn
 
-	attributes := []*elb.PolicyAttribute{}
-	if attributedata, ok := d.GetOk("policy_attribute"); ok {
-		attributeSet := attributedata.(*schema.Set).List()
-		for _, attribute := range attributeSet {
-			data := attribute.(map[string]interface{})
-			attributes = append(attributes, &elb.PolicyAttribute{
-				AttributeName:  aws.String(data["name"].(string)),
-				AttributeValue: aws.String(data["value"].(string)),
-			})
-		}
-	}
-
 	lbspOpts := &elb.CreateLoadBalancerPolicyInput{
 		LoadBalancerName: aws.String(d.Get("load_balancer_name").(string)),
 		PolicyName:       aws.String(d.Get("policy_name").(string)),
 		PolicyTypeName:   aws.String(d.Get("policy_type_name").(string)),
-		PolicyAttributes: attributes,
+	}
+
+	if v, ok := d.GetOk("policy_attribute"); ok && v.(*schema.Set).Len() > 0 {
+		lbspOpts.PolicyAttributes = ExpandPolicyAttributes(v.(*schema.Set).List())
 	}
 
 	if _, err := conn.CreateLoadBalancerPolicy(lbspOpts); err != nil {
@@ -128,21 +127,12 @@ func resourcePolicyRead(d *schema.ResourceData, meta interface{}) error {
 	policyTypeName := policyDesc.PolicyTypeName
 	policyAttributes := policyDesc.PolicyAttributeDescriptions
 
-	attributes := []map[string]string{}
-	for _, a := range policyAttributes {
-		pair := make(map[string]string)
-		pair["name"] = *a.AttributeName
-		pair["value"] = *a.AttributeValue
-		if (*policyTypeName == "SSLNegotiationPolicyType") && (*a.AttributeValue == "false") {
-			continue
-		}
-		attributes = append(attributes, pair)
-	}
-
 	d.Set("policy_name", policyName)
 	d.Set("policy_type_name", policyTypeName)
 	d.Set("load_balancer_name", loadBalancerName)
-	d.Set("policy_attribute", attributes)
+	if err := d.Set("policy_attribute", FlattenPolicyAttributes(policyAttributes)); err != nil {
+		return fmt.Errorf("error setting policy_attribute: %w", err)
+	}
 
 	return nil
 }
@@ -364,3 +354,22 @@ func resourcePolicyUnassign(policyName, loadBalancerName string, conn *elb.ELB)
 
 	return reassignments, nil
 }
+
+func suppressPolicyAttributeDiffs(k, old, new string, d *schema.ResourceData) bool {
+	// Show difference for new resource
+	if d.Id() == "" {
+		return false
+	}
+
+	// Show differences if configured attributes are not in state
+	if old == "0" && new != "0" {
+		return false
+	}
+
+	o, n := d.GetChange("policy_attribute")
+	oldAttributes := o.(*schema.Set)
+	newAttributes := n.(*schema.Set)
+
+	// Suppress differences if the attributes returned from the API contain those configured
+	return oldAttributes.Intersection(newAttributes).Len() == newAttributes.Len()
+}

internal/service/elb/policy_test.go

@@ -2,6 +2,7 @@ package elb_test
 
 import (
 	"fmt"
+	"regexp"
 	"strconv"
 	"strings"
 	"testing"
@@ -75,6 +76,143 @@ func TestAccELBPolicy_disappears(t *testing.T) {
 	})
 }
 
+func TestAccELBPolicy_LBCookieStickinessPolicyType_computedAttributesOnly(t *testing.T) {
+	var policy elb.PolicyDescription
+	resourceName := "aws_load_balancer_policy.test"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	policyTypeName := "LBCookieStickinessPolicyType"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { acctest.PreCheck(t) },
+		ErrorCheck:   acctest.ErrorCheck(t, elb.EndpointsID),
+		Providers:    acctest.Providers,
+		CheckDestroy: testAccCheckPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccPolicyConfig_policyTypeNameOnly(rName, policyTypeName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &policy),
+					resource.TestCheckResourceAttr(resourceName, "policy_type_name", policyTypeName),
+					resource.TestCheckResourceAttr(resourceName, "policy_attribute.#", "1"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccELBPolicy_SSLNegotiationPolicyType_computedAttributesOnly(t *testing.T) {
+	var policy elb.PolicyDescription
+	resourceName := "aws_load_balancer_policy.test"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { acctest.PreCheck(t) },
+		ErrorCheck:   acctest.ErrorCheck(t, elb.EndpointsID),
+		Providers:    acctest.Providers,
+		CheckDestroy: testAccCheckPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccPolicyConfig_policyTypeNameOnly(rName, tfelb.SSLNegotiationPolicyType),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &policy),
+					resource.TestCheckResourceAttr(resourceName, "policy_type_name", tfelb.SSLNegotiationPolicyType),
+					resource.TestMatchResourceAttr(resourceName, "policy_attribute.#", regexp.MustCompile(`[^0]+`)),
+				),
+			},
+		},
+	})
+}
+
+func TestAccELBPolicy_SSLNegotiationPolicyType_customPolicy(t *testing.T) {
+	var policy elb.PolicyDescription
+	resourceName := "aws_load_balancer_policy.test"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { acctest.PreCheck(t) },
+		ErrorCheck:   acctest.ErrorCheck(t, elb.EndpointsID),
+		Providers:    acctest.Providers,
+		CheckDestroy: testAccCheckPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccPolicyConfig_customSSLSecurityPolicy(rName, "Protocol-TLSv1.1", "DHE-RSA-AES256-SHA256"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &policy),
+					resource.TestCheckResourceAttr(resourceName, "policy_name", rName),
+					resource.TestCheckResourceAttr(resourceName, "policy_type_name", tfelb.SSLNegotiationPolicyType),
+					resource.TestCheckResourceAttr(resourceName, "policy_attribute.#", "2"),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "policy_attribute.*", map[string]string{
+						"name":  "Protocol-TLSv1.1",
+						"value": "true",
+					}),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "policy_attribute.*", map[string]string{
+						"name":  "DHE-RSA-AES256-SHA256",
+						"value": "true",
+					}),
+				),
+			},
+			{
+				Config: testAccPolicyConfig_customSSLSecurityPolicy(rName, "Protocol-TLSv1.2", "ECDHE-ECDSA-AES128-GCM-SHA256"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &policy),
+					resource.TestCheckResourceAttr(resourceName, "policy_name", rName),
+					resource.TestCheckResourceAttr(resourceName, "policy_type_name", tfelb.SSLNegotiationPolicyType),
+					resource.TestCheckResourceAttr(resourceName, "policy_attribute.#", "2"),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "policy_attribute.*", map[string]string{
+						"name":  "Protocol-TLSv1.2",
+						"value": "true",
+					}),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "policy_attribute.*", map[string]string{
+						"name":  "ECDHE-ECDSA-AES128-GCM-SHA256",
+						"value": "true",
+					}),
+				),
+			},
+		},
+	})
+}
+
+func TestAccELBPolicy_SSLSecurityPolicy_predefined(t *testing.T) {
+	var policy elb.PolicyDescription
+	resourceName := "aws_load_balancer_policy.test"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	predefinedSecurityPolicy := "ELBSecurityPolicy-TLS-1-2-2017-01"
+	predefinedSecurityPolicyUpdated := "ELBSecurityPolicy-2016-08"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { acctest.PreCheck(t) },
+		ErrorCheck:   acctest.ErrorCheck(t, elb.EndpointsID),
+		Providers:    acctest.Providers,
+		CheckDestroy: testAccCheckPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccPolicyConfig_predefinedSSLSecurityPolicy(rName, predefinedSecurityPolicy),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &policy),
+					resource.TestCheckResourceAttr(resourceName, "policy_attribute.#", "1"),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "policy_attribute.*", map[string]string{
+						"name":  tfelb.ReferenceSecurityPolicy,
+						"value": predefinedSecurityPolicy,
+					}),
+					resource.TestCheckResourceAttr(resourceName, "policy_type_name", tfelb.SSLNegotiationPolicyType),
+				),
+			},
+			{
+				Config: testAccPolicyConfig_predefinedSSLSecurityPolicy(rName, predefinedSecurityPolicyUpdated),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckPolicyExists(resourceName, &policy),
+					resource.TestCheckResourceAttr(resourceName, "policy_attribute.#", "1"),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "policy_attribute.*", map[string]string{
+						"name":  tfelb.ReferenceSecurityPolicy,
+						"value": predefinedSecurityPolicyUpdated,
+					}),
+					resource.TestCheckResourceAttr(resourceName, "policy_type_name", tfelb.SSLNegotiationPolicyType),
+				),
+			},
+		},
+	})
+}
+
 func TestAccELBPolicy_updateWhileAssigned(t *testing.T) {
 	var policy elb.PolicyDescription
 	loadBalancerResourceName := "aws_elb.test-lb"
@@ -273,6 +411,101 @@ resource "aws_load_balancer_policy" "test-policy" {
 `, rInt))
 }
 
+func testAccPolicyConfig_policyTypeNameOnly(rName, policyType string) string {
+	return acctest.ConfigCompose(
+		acctest.ConfigAvailableAZsNoOptIn(),
+		fmt.Sprintf(`
+resource "aws_elb" "test" {
+  name               = %[1]q
+  availability_zones = [data.aws_availability_zones.available.names[0]]
+
+  listener {
+    instance_port     = 80
+    instance_protocol = "http"
+    lb_port           = 80
+    lb_protocol       = "http"
+  }
+
+  tags = {
+    Name = "tf-acc-test"
+  }
+}
+
+resource "aws_load_balancer_policy" "test" {
+  load_balancer_name = aws_elb.test.name
+  policy_name        = %[1]q
+  policy_type_name   = %[2]q
+}
+`, rName, policyType))
+}
+
+func testAccPolicyConfig_customSSLSecurityPolicy(rName, protocol, cipher string) string {
+	return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
+resource "aws_elb" "test" {
+  name               = %[1]q
+  availability_zones = [data.aws_availability_zones.available.names[0]]
+
+  listener {
+    instance_port     = 80
+    instance_protocol = "http"
+    lb_port           = 80
+    lb_protocol       = "http"
+  }
+
+  tags = {
+    Name = "tf-acc-test"
+  }
+}
+
+resource "aws_load_balancer_policy" "test" {
+  load_balancer_name = aws_elb.test.name
+  policy_name        = %[1]q
+  policy_type_name   = "SSLNegotiationPolicyType"
+
+  policy_attribute {
+    name  = %[2]q
+    value = "true"
+  }
+
+  policy_attribute {
+    name  = %[3]q
+    value = "true"
+  }
+}
+`, rName, protocol, cipher))
+}
+
+func testAccPolicyConfig_predefinedSSLSecurityPolicy(rName, securityPolicy string) string {
+	return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
+resource "aws_elb" "test" {
+  name               = %[1]q
+  availability_zones = [data.aws_availability_zones.available.names[0]]
+
+  listener {
+    instance_port     = 80
+    instance_protocol = "http"
+    lb_port           = 80
+    lb_protocol       = "http"
+  }
+
+  tags = {
+    Name = "tf-acc-test"
+  }
+}
+
+resource "aws_load_balancer_policy" "test" {
+  load_balancer_name = aws_elb.test.name
+  policy_name        = %[1]q
+  policy_type_name   = "SSLNegotiationPolicyType"
+
+  policy_attribute {
+    name  = "Reference-Security-Policy"
+    value = %[2]q
+  }
+}
+`, rName, securityPolicy))
+}
+
 func testAccPolicyConfig_updateWhileAssigned0(rInt int) string {
 	return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
 resource "aws_elb" "test-lb" {