Merge pull request #15613 from hashicorp/b-fed-lb-sticky

lb_target_group: Allow invalid stickiness config, avoid breaking change
hashicorp · Nov 10, 2020 · 2713c91 · 2713c91
2 parents 90075b8 + c50d984
commit 2713c91
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 20 deletions.
diff --git a/aws/resource_aws_lb_target_group.go b/aws/resource_aws_lb_target_group.go
@@ -152,6 +152,16 @@ func resourceAwsLbTargetGroup() *schema.Resource {
 								"lb_cookie", // Only for ALBs
 								"source_ip", // Only for NLBs
 							}, false),
+							DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
+								switch d.Get("protocol").(string) {
+								case elbv2.ProtocolEnumTcp, elbv2.ProtocolEnumUdp, elbv2.ProtocolEnumTcpUdp, elbv2.ProtocolEnumTls:
+									if new == "lb_cookie" && !d.Get("stickiness.0.enabled").(bool) {
+										log.Printf("[WARN] invalid configuration, this will fail in a future version: stickiness enabled %v, protocol %s, type %s", d.Get("stickiness.0.enabled").(bool), d.Get("protocol").(string), new)
+										return true
+									}
+								}
+								return false
+							},
 						},
 						"cookie_duration": {
 							Type:         schema.TypeInt,
@@ -447,23 +457,27 @@ func resourceAwsLbTargetGroupUpdate(d *schema.ResourceData, meta interface{}) er
 			if len(stickinessBlocks) == 1 {
 				stickiness := stickinessBlocks[0].(map[string]interface{})
 
-				attrs = append(attrs,
-					&elbv2.TargetGroupAttribute{
-						Key:   aws.String("stickiness.enabled"),
-						Value: aws.String(strconv.FormatBool(stickiness["enabled"].(bool))),
-					},
-					&elbv2.TargetGroupAttribute{
-						Key:   aws.String("stickiness.type"),
-						Value: aws.String(stickiness["type"].(string)),
-					})
-
-				switch d.Get("protocol").(string) {
-				case elbv2.ProtocolEnumHttp, elbv2.ProtocolEnumHttps:
+				if !stickiness["enabled"].(bool) && stickiness["type"].(string) == "lb_cookie" && d.Get("protocol").(string) != elbv2.ProtocolEnumHttp && d.Get("protocol").(string) != elbv2.ProtocolEnumHttps {
+					log.Printf("[WARN] invalid configuration, this will fail in a future version: stickiness enabled %v, protocol %s, type %s", stickiness["enabled"].(bool), d.Get("protocol").(string), stickiness["type"].(string))
+				} else {
 					attrs = append(attrs,
 						&elbv2.TargetGroupAttribute{
-							Key:   aws.String("stickiness.lb_cookie.duration_seconds"),
-							Value: aws.String(fmt.Sprintf("%d", stickiness["cookie_duration"].(int))),
+							Key:   aws.String("stickiness.enabled"),
+							Value: aws.String(strconv.FormatBool(stickiness["enabled"].(bool))),
+						},
+						&elbv2.TargetGroupAttribute{
+							Key:   aws.String("stickiness.type"),
+							Value: aws.String(stickiness["type"].(string)),
 						})
+
+					switch d.Get("protocol").(string) {
+					case elbv2.ProtocolEnumHttp, elbv2.ProtocolEnumHttps:
+						attrs = append(attrs,
+							&elbv2.TargetGroupAttribute{
+								Key:   aws.String("stickiness.lb_cookie.duration_seconds"),
+								Value: aws.String(fmt.Sprintf("%d", stickiness["cookie_duration"].(int))),
+							})
+					}
 				}
 			} else if len(stickinessBlocks) == 0 {
 				attrs = append(attrs, &elbv2.TargetGroupAttribute{

diff --git a/aws/resource_aws_lb_target_group_test.go b/aws/resource_aws_lb_target_group_test.go
@@ -1047,6 +1047,15 @@ func TestAccAWSLBTargetGroup_stickinessValidNLB(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "stickiness.0.type", "source_ip"),
 				),
 			},
+			{
+				// this test should be invalid but allowed to avoid breaking changes
+				Config: testAccAWSLBTargetGroupConfig_stickinessValidity("TCP", "lb_cookie", false),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckAWSLBTargetGroupExists(resourceName, &conf),
+					resource.TestCheckResourceAttr(resourceName, "stickiness.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "stickiness.0.enabled", "false"),
+				),
+			},
 			{
 				Config: testAccAWSLBTargetGroupConfig_stickinessValidity("TCP", "source_ip", true),
 				Check: resource.ComposeAggregateTestCheckFunc(
@@ -1130,11 +1139,6 @@ func TestAccAWSLBTargetGroup_stickinessInvalidNLB(t *testing.T) {
 				Config:      testAccAWSLBTargetGroupConfig_stickinessValidity("TCP_UDP", "lb_cookie", true),
 				ExpectError: regexp.MustCompile("Stickiness type 'lb_cookie' is not supported for target groups with"),
 			},
-			{
-				Config:             testAccAWSLBTargetGroupConfig_stickinessValidity("TCP_UDP", "lb_cookie", false),
-				PlanOnly:           true,
-				ExpectNonEmptyPlan: true,
-			},
 		},
 	})
 }

diff --git a/website/docs/r/lb_target_group.html.markdown b/website/docs/r/lb_target_group.html.markdown
@@ -85,7 +85,7 @@ Stickiness Blocks (`stickiness`) support the following:
 * `cookie_duration` - (Optional) Only used when the type is `lb_cookie`. 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`.
+~> **NOTE:** Currently, an NLB (i.e., protocol of `HTTP` or `HTTPS`) can have an invalid `stickiness` block with `type` set to `lb_cookie` as long as `enabled` is set to `false`. However, please update your configurations to avoid errors in a future version of the provider: either remove the invalid `stickiness` block or set the `type` to `source_ip`.
 
 Health Check Blocks (`health_check`):