resource/aws_lb_target_group: Attempt to resolve catch-22 around NLB's stickiness

aws/resource_aws_lb_target_group.go

@@ -330,7 +330,11 @@ func resourceAwsLbTargetGroupUpdate(d *schema.ResourceData, meta interface{}) er
 		})
 	}
 
-	if d.HasChange("stickiness") {
+	// In CustomizeDiff we allow LB stickiness to be declared for TCP target
+	// groups, so long as it's not enabled. This allows for better support for
+	// modules, but also means we need to completely skip sending the data to the
+	// API if it's defined on a TCP target group.
+	if d.HasChange("stickiness") && d.Get("protocol") != "TCP" {
 		stickinessBlocks := d.Get("stickiness").([]interface{})
 		if len(stickinessBlocks) == 1 {
 			stickiness := stickinessBlocks[0].(map[string]interface{})
@@ -541,8 +545,45 @@ func flattenAwsLbTargetGroupResource(d *schema.ResourceData, meta interface{}, t
 		return errwrap.Wrapf("Error retrieving Target Group Attributes: {{err}}", err)
 	}
 
+	// We only read in the stickiness attributes if the target group is not
+	// TCP-based. This ensures we don't end up causing a spurious diff if someone
+	// has defined the stickiness block on a TCP target group (albeit with
+	// false), for which this update would clobber the state coming from config
+	// for.
+	//
+	// This is a workaround to support module design where the module needs to
+	// support HTTP and TCP target groups.
+	switch {
+	case *targetGroup.Protocol != "TCP":
+		if err = flattenAwsLbTargetGroupStickiness(d, attrResp.Attributes); err != nil {
+			return err
+		}
+	case *targetGroup.Protocol == "TCP" && len(d.Get("stickiness").([]interface{})) < 1:
+		if err = d.Set("stickiness", []interface{}{}); err != nil {
+			return err
+		}
+	}
+
+	tagsResp, err := elbconn.DescribeTags(&elbv2.DescribeTagsInput{
+		ResourceArns: []*string{aws.String(d.Id())},
+	})
+	if err != nil {
+		return errwrap.Wrapf("Error retrieving Target Group Tags: {{err}}", err)
+	}
+	for _, t := range tagsResp.TagDescriptions {
+		if *t.ResourceArn == d.Id() {
+			if err := d.Set("tags", tagsToMapELBv2(t.Tags)); err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
+func flattenAwsLbTargetGroupStickiness(d *schema.ResourceData, attributes []*elbv2.TargetGroupAttribute) error {
 	stickinessMap := map[string]interface{}{}
-	for _, attr := range attrResp.Attributes {
+	for _, attr := range attributes {
 		switch *attr.Key {
 		case "stickiness.enabled":
 			enabled, err := strconv.ParseBool(*attr.Value)
@@ -574,31 +615,18 @@ func flattenAwsLbTargetGroupResource(d *schema.ResourceData, meta interface{}, t
 	if err := d.Set("stickiness", setStickyMap); err != nil {
 		return err
 	}
-
-	tagsResp, err := elbconn.DescribeTags(&elbv2.DescribeTagsInput{
-		ResourceArns: []*string{aws.String(d.Id())},
-	})
-	if err != nil {
-		return errwrap.Wrapf("Error retrieving Target Group Tags: {{err}}", err)
-	}
-	for _, t := range tagsResp.TagDescriptions {
-		if *t.ResourceArn == d.Id() {
-			if err := d.Set("tags", tagsToMapELBv2(t.Tags)); err != nil {
-				return err
-			}
-		}
-	}
-
 	return nil
 }
 
 func resourceAwsLbTargetGroupCustomizeDiff(diff *schema.ResourceDiff, v interface{}) error {
 	protocol := diff.Get("protocol").(string)
 	if protocol == "TCP" {
 		// TCP load balancers do not support stickiness
-		stickinessBlocks := diff.Get("stickiness").([]interface{})
-		if len(stickinessBlocks) != 0 {
-			return fmt.Errorf("Network Load Balancers do not support Stickiness")
+		if stickinessBlocks := diff.Get("stickiness").([]interface{}); len(stickinessBlocks) == 1 {
+			stickiness := stickinessBlocks[0].(map[string]interface{})
+			if val := stickiness["enabled"].(bool); val {
+				return fmt.Errorf("Network Load Balancers do not support Stickiness")
+			}
 		}
 	}
 

aws/resource_aws_lb_target_group_test.go

@@ -703,6 +703,43 @@ func TestAccAWSLBTargetGroup_defaults_network(t *testing.T) {
 	})
 }
 
+func TestAccAWSLBTargetGroup_stickinessWithTCPDisabled(t *testing.T) {
+	var conf elbv2.TargetGroup
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:      func() { testAccPreCheck(t) },
+		IDRefreshName: "aws_lb_target_group.test",
+		Providers:     testAccProviders,
+		CheckDestroy:  testAccCheckAWSLBTargetGroupDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSLBTargetGroupConfig_stickinessWithTCP(false),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckAWSLBTargetGroupExists("aws_lb_target_group.test", &conf),
+					resource.TestCheckResourceAttr("aws_lb_target_group.test", "stickiness.#", "1"),
+					resource.TestCheckResourceAttr("aws_lb_target_group.test", "stickiness.0.enabled", "false"),
+					resource.TestCheckResourceAttr("aws_lb_target_group.test", "stickiness.0.type", "lb_cookie"),
+					resource.TestCheckResourceAttr("aws_lb_target_group.test", "stickiness.0.cookie_duration", "86400"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccAWSLBTargetGroup_stickinessWithTCPEnabledShouldError(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccAWSLBTargetGroupConfig_stickinessWithTCP(true),
+				PlanOnly:    true,
+				ExpectError: regexp.MustCompile("Network Load Balancers do not support Stickiness"),
+			},
+		},
+	})
+}
+
 func testAccCheckAWSLBTargetGroupExists(n string, res *elbv2.TargetGroup) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -1359,3 +1396,27 @@ resource "aws_vpc" "test" {
 	}
 }
 `
+
+func testAccAWSLBTargetGroupConfig_stickinessWithTCP(enabled bool) string {
+	return fmt.Sprintf(`
+resource "aws_lb_target_group" "test" {
+  name_prefix = "tf-"
+  port        = 25
+  protocol    = "TCP"
+  vpc_id      = "${aws_vpc.test.id}"
+
+  stickiness {
+    type    = "lb_cookie"
+    enabled = %t
+  }
+}
+
+resource "aws_vpc" "test" {
+  cidr_block = "10.0.0.0/16"
+
+  tags {
+    Name = "testAccAWSLBTargetGroupConfig_namePrefix"
+  }
+}
+`, enabled)
+}

website/docs/r/lb_target_group.html.markdown

@@ -53,6 +53,8 @@ Stickiness Blocks (`stickiness`) support the following:
 * `cookie_duration` - (Optional) The time period, in seconds, during which requests from a client should be routed to the same target. After this time period expires, the load balancer-generated cookie is considered stale. The range is 1 second to 1 week (604800 seconds). The default value is 1 day (86400 seconds).
 * `enabled` - (Optional) Boolean to enable / disable `stickiness`. Default is `true`
 
+~> **NOTE:** To help facilitate the authoring of modules that support target groups of any protocol, you can define `stickiness` regardless of the protocol chosen. However, for `TCP` target groups, `enabled` must be `false`.
+
 Health Check Blocks (`health_check`):
 
 ~> **Note:** The Health Check parameters you can set vary by the `protocol` of