Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #401 (2021-02-10 09:21:39)

Don't set healthcheck timeout on NLB
For some reason custom healthcheck timeout is not supported when target group uses TCP protocol. This is the error shown in Cloudformation when creating an NLB:

```
Custom health check timeouts are not supported for health checks for target groups with the TCP protocol (Service: AmazonElasticLoadBalancing; Status Code: 400; Error Code: InvalidConfigurationRequest; Request ID: xxx; Proxy: null)
```

Fix it by only setting custom healthcheck timeout when `protocol != TCP`


mikkeloscar: @jonathanbeber you're welcome to add tests. Otherwise I can also do it next week :)
jonathanbeber: @mikkeloscar will do it.
jonathanbeber: :+1: 
mikkeloscar: :+1: 

Patch:
diff --git a/aws/cf_template.go b/aws/cf_template.go
index eb6d869..e450945 100644
--- a/aws/cf_template.go
+++ b/aws/cf_template.go
@@ -277,18 +277,23 @@ func generateTemplate(spec *stackSpec) (string, error) {
 			Value: cloudformation.String(fmt.Sprintf("%d", spec.deregistrationDelayTimeoutSeconds)),
 		},
 	}
-	template.AddResource("TG", &cloudformation.ElasticLoadBalancingV2TargetGroup{
+	targetGroup := &cloudformation.ElasticLoadBalancingV2TargetGroup{
 		TargetGroupAttributes: &targetGroupAttributes,
 
 		HealthCheckIntervalSeconds: cloudformation.Ref(parameterTargetGroupHealthCheckIntervalParameter).Integer(),
-		HealthCheckTimeoutSeconds:  cloudformation.Ref(parameterTargetGroupHealthCheckTimeoutParameter).Integer(),
 		HealthCheckPath:            cloudformation.Ref(parameterTargetGroupHealthCheckPathParameter).String(),
 		HealthCheckPort:            cloudformation.Ref(parameterTargetGroupHealthCheckPortParameter).String(),
 		HealthCheckProtocol:        cloudformation.String("HTTP"),
 		Port:                       cloudformation.Ref(parameterTargetTargetPortParameter).Integer(),
 		Protocol:                   cloudformation.String(protocol),
 		VPCID:                      cloudformation.Ref(parameterTargetGroupVPCIDParameter).String(),
-	})
+	}
+
+	// custom target group healthcheck only supported when the target group protocol is != TCP
+	if protocol != "TCP" {
+		targetGroup.HealthCheckTimeoutSeconds = cloudformation.Ref(parameterTargetGroupHealthCheckTimeoutParameter).Integer()
+	}
+	template.AddResource("TG", targetGroup)
 
 	if spec.loadbalancerType == LoadBalancerTypeApplication && spec.wafWebAclId != "" {
 		if strings.HasPrefix(spec.wafWebAclId, "arn:aws:wafv2:") {


Test patch:
diff --git a/aws/cf_template_test.go b/aws/cf_template_test.go
index 98ed35c..4c1a153 100644
--- a/aws/cf_template_test.go
+++ b/aws/cf_template_test.go
@@ -209,6 +209,30 @@ func TestGenerateTemplate(t *testing.T) {
 				require.Equal(t, &expected, props.TargetGroupAttributes)
 			},
 		},
+		{
+			name: "Does not set healthcheck timeout on NLBs",
+			spec: &stackSpec{
+				loadbalancerType: LoadBalancerTypeNetwork,
+			},
+			validate: func(t *testing.T, template *cloudformation.Template) {
+				require.NotNil(t, template.Resources, "TG")
+				tg, ok := template.Resources["TG"].Properties.(*cloudformation.ElasticLoadBalancingV2TargetGroup)
+				require.True(t, ok, "couldn't convert resource to ElasticLoadBalancingV2TargetGroup")
+				require.Nil(t, tg.HealthCheckTimeoutSeconds)
+			},
+		},
+		{
+			name: "Set healthcheck timeout when not NLB",
+			spec: &stackSpec{
+				loadbalancerType: LoadBalancerTypeApplication,
+			},
+			validate: func(t *testing.T, template *cloudformation.Template) {
+				require.NotNil(t, template.Resources, "TG")
+				tg, ok := template.Resources["TG"].Properties.(*cloudformation.ElasticLoadBalancingV2TargetGroup)
+				require.True(t, ok, "couldn't convert resource to ElasticLoadBalancingV2TargetGroup")
+				require.NotNil(t, tg.HealthCheckTimeoutSeconds)
+			},
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			generated, err := generateTemplate(test.spec)