From a6d126668344bd20c61965e941fae6aaf09cc451 Mon Sep 17 00:00:00 2001 From: Clint Date: Thu, 14 Dec 2017 16:20:51 -0600 Subject: [PATCH] r/aws_lb_target_group: various fixes to behavior based on protocol type (#2380) * fix up nlb/alb issues * regression / acceptance tests * matcher is optional, not required --- aws/resource_aws_lb_target_group.go | 111 +++++++++--- aws/resource_aws_lb_target_group_test.go | 170 +++++++++++++++++++ website/docs/r/lb_target_group.html.markdown | 16 +- 3 files changed, 264 insertions(+), 33 deletions(-) diff --git a/aws/resource_aws_lb_target_group.go b/aws/resource_aws_lb_target_group.go index 4f1fa2c57c61..21826891d424 100644 --- a/aws/resource_aws_lb_target_group.go +++ b/aws/resource_aws_lb_target_group.go @@ -18,13 +18,13 @@ import ( func resourceAwsLbTargetGroup() *schema.Resource { return &schema.Resource{ + // NLBs have restrictions on them at this time + CustomizeDiff: resourceAwsLbTargetGroupCustomizeDiff, + Create: resourceAwsLbTargetGroupCreate, Read: resourceAwsLbTargetGroupRead, Update: resourceAwsLbTargetGroupUpdate, Delete: resourceAwsLbTargetGroupDelete, - - // Health Check Interval is not updatable for TCP-based Target Groups - CustomizeDiff: resourceAwsLbTargetGroupCustomizeDiff, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -132,6 +132,7 @@ func resourceAwsLbTargetGroup() *schema.Resource { "path": { Type: schema.TypeString, Optional: true, + Computed: true, ValidateFunc: validateAwsLbTargetGroupHealthCheckPath, }, @@ -155,7 +156,7 @@ func resourceAwsLbTargetGroup() *schema.Resource { "timeout": { Type: schema.TypeInt, Optional: true, - Default: 10, + Computed: true, ValidateFunc: validateAwsLbTargetGroupHealthCheckTimeout, }, @@ -168,6 +169,7 @@ func resourceAwsLbTargetGroup() *schema.Resource { "matcher": { Type: schema.TypeString, + Computed: true, Optional: true, }, @@ -214,12 +216,22 @@ func resourceAwsLbTargetGroupCreate(d *schema.ResourceData, meta interface{}) er params.HealthCheckProtocol = aws.String(healthCheck["protocol"].(string)) params.HealthyThresholdCount = aws.Int64(int64(healthCheck["healthy_threshold"].(int))) params.UnhealthyThresholdCount = aws.Int64(int64(healthCheck["unhealthy_threshold"].(int))) + t := healthCheck["timeout"].(int) + if t != 0 { + params.HealthCheckTimeoutSeconds = aws.Int64(int64(t)) + } - if *params.Protocol != "TCP" { - params.HealthCheckTimeoutSeconds = aws.Int64(int64(healthCheck["timeout"].(int))) - params.HealthCheckPath = aws.String(healthCheck["path"].(string)) - params.Matcher = &elbv2.Matcher{ - HttpCode: aws.String(healthCheck["matcher"].(string)), + if *params.HealthCheckProtocol != "TCP" { + p := healthCheck["path"].(string) + if p != "" { + params.HealthCheckPath = aws.String(p) + } + + m := healthCheck["matcher"].(string) + if m != "" { + params.Matcher = &elbv2.Matcher{ + HttpCode: aws.String(m), + } } } } @@ -269,10 +281,12 @@ func resourceAwsLbTargetGroupUpdate(d *schema.ResourceData, meta interface{}) er } if d.HasChange("health_check") { - healthChecks := d.Get("health_check").([]interface{}) - var params *elbv2.ModifyTargetGroupInput + healthChecks := d.Get("health_check").([]interface{}) if len(healthChecks) == 1 { + params = &elbv2.ModifyTargetGroupInput{ + TargetGroupArn: aws.String(d.Id()), + } healthCheck := healthChecks[0].(map[string]interface{}) params = &elbv2.ModifyTargetGroupInput{ @@ -283,25 +297,27 @@ func resourceAwsLbTargetGroupUpdate(d *schema.ResourceData, meta interface{}) er UnhealthyThresholdCount: aws.Int64(int64(healthCheck["unhealthy_threshold"].(int))), } + t := healthCheck["timeout"].(int) + if t != 0 { + params.HealthCheckTimeoutSeconds = aws.Int64(int64(t)) + } + healthCheckProtocol := strings.ToLower(healthCheck["protocol"].(string)) - if healthCheckProtocol != "tcp" { + if healthCheckProtocol != "tcp" && !d.IsNewResource() { params.Matcher = &elbv2.Matcher{ HttpCode: aws.String(healthCheck["matcher"].(string)), } params.HealthCheckPath = aws.String(healthCheck["path"].(string)) params.HealthCheckIntervalSeconds = aws.Int64(int64(healthCheck["interval"].(int))) - params.HealthCheckTimeoutSeconds = aws.Int64(int64(healthCheck["timeout"].(int))) - } - } else { - params = &elbv2.ModifyTargetGroupInput{ - TargetGroupArn: aws.String(d.Id()), } } - _, err := elbconn.ModifyTargetGroup(params) - if err != nil { - return errwrap.Wrapf("Error modifying Target Group: {{err}}", err) + if params != nil { + _, err := elbconn.ModifyTargetGroup(params) + if err != nil { + return errwrap.Wrapf("Error modifying Target Group: {{err}}", err) + } } } @@ -547,7 +563,11 @@ func flattenAwsLbTargetGroupResource(d *schema.ResourceData, meta interface{}, t } } - if err := d.Set("stickiness", []interface{}{stickinessMap}); err != nil { + setStickyMap := []interface{}{} + if len(stickinessMap) > 0 { + setStickyMap = []interface{}{stickinessMap} + } + if err := d.Set("stickiness", setStickyMap); err != nil { return err } @@ -570,19 +590,54 @@ func flattenAwsLbTargetGroupResource(d *schema.ResourceData, meta interface{}, t func resourceAwsLbTargetGroupCustomizeDiff(diff *schema.ResourceDiff, v interface{}) error { protocol := diff.Get("protocol").(string) - if protocol != "TCP" { - return nil + 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") + } + // Network Load Balancers have many special qwirks to them. + // See http://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_CreateTargetGroup.html + if healthChecks := diff.Get("health_check").([]interface{}); len(healthChecks) == 1 { + healthCheck := healthChecks[0].(map[string]interface{}) + // Cannot set custom matcher on TCP health checks + if m := healthCheck["matcher"].(string); m != "" { + return fmt.Errorf("%s: custom matcher is not supported for target_groups with TCP protocol", diff.Id()) + } + // Cannot set custom path on TCP health checks + if m := healthCheck["path"].(string); m != "" { + return fmt.Errorf("%s: custom path is not supported for target_groups with TCP protocol", diff.Id()) + } + // Cannot set custom timeout on TCP health checks + if t := healthCheck["timeout"].(int); t != 0 && diff.Id() == "" { + // timeout has a default value, so only check this if this is a network + // LB and is a first run + return fmt.Errorf("%s: custom timeout is not supported for target_groups with TCP protocol", diff.Id()) + } + } + } + + if strings.Contains(protocol, "HTTP") { + if healthChecks := diff.Get("health_check").([]interface{}); len(healthChecks) == 1 { + healthCheck := healthChecks[0].(map[string]interface{}) + // HTTP(S) Target Groups cannot use TCP health checks + if p := healthCheck["protocol"].(string); strings.ToLower(p) == "tcp" { + return fmt.Errorf("HTTP Target Groups cannot use TCP health checks") + } + } } if diff.Id() == "" { return nil } - if diff.HasChange("health_check.0.interval") { - old, new := diff.GetChange("health_check.0.interval") - return fmt.Errorf("Health check interval cannot be updated from %d to %d for TCP based Target Group %s,"+ - " use 'terraform taint' to recreate the resource if you wish", - old, new, diff.Id()) + if protocol == "TCP" { + if diff.HasChange("health_check.0.interval") { + old, new := diff.GetChange("health_check.0.interval") + return fmt.Errorf("Health check interval cannot be updated from %d to %d for TCP based Target Group %s,"+ + " use 'terraform taint' to recreate the resource if you wish", + old, new, diff.Id()) + } } return nil } diff --git a/aws/resource_aws_lb_target_group_test.go b/aws/resource_aws_lb_target_group_test.go index 7c9b114fe017..5a563cc56c94 100644 --- a/aws/resource_aws_lb_target_group_test.go +++ b/aws/resource_aws_lb_target_group_test.go @@ -531,6 +531,111 @@ func TestAccAWSLBTargetGroup_updateSticknessEnabled(t *testing.T) { }) } +func TestAccAWSLBTargetGroup_defaults_application(t *testing.T) { + var conf elbv2.TargetGroup + targetGroupName := fmt.Sprintf("test-target-group-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_lb_target_group.test", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSLBTargetGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccALB_defaults(targetGroupName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSLBTargetGroupExists("aws_lb_target_group.test", &conf), + resource.TestCheckResourceAttrSet("aws_lb_target_group.test", "arn"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "name", targetGroupName), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "port", "443"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "protocol", "HTTP"), + resource.TestCheckResourceAttrSet("aws_lb_target_group.test", "vpc_id"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "deregistration_delay", "200"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.#", "1"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.interval", "10"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.port", "8081"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.protocol", "HTTP"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.timeout", "5"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.healthy_threshold", "3"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.unhealthy_threshold", "3"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "tags.%", "1"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "tags.Name", "TestAccAWSLBTargetGroup_application_LB_defaults"), + ), + }, + }, + }) +} + +func TestAccAWSLBTargetGroup_defaults_network(t *testing.T) { + var conf elbv2.TargetGroup + targetGroupName := fmt.Sprintf("test-target-group-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)) + healthCheckInvalid1 := ` + path = "/health" + interval = 10 + port = 8081 + protocol = "TCP" + ` + healthCheckInvalid2 := ` + interval = 10 + port = 8081 + protocol = "TCP" + matcher = "200" + ` + healthCheckInvalid3 := ` + interval = 10 + port = 8081 + protocol = "TCP" + timeout = 4 + ` + healthCheckValid := ` + interval = 10 + port = 8081 + protocol = "TCP" + ` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_lb_target_group.test", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSLBTargetGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNLB_defaults(targetGroupName, healthCheckInvalid1), + ExpectError: regexp.MustCompile("custom path is not supported for target_groups with TCP protocol"), + }, + { + Config: testAccNLB_defaults(targetGroupName, healthCheckInvalid2), + ExpectError: regexp.MustCompile("custom matcher is not supported for target_groups with TCP protocol"), + }, + { + Config: testAccNLB_defaults(targetGroupName, healthCheckInvalid3), + ExpectError: regexp.MustCompile("custom timeout is not supported for target_groups with TCP protocol"), + }, + { + Config: testAccNLB_defaults(targetGroupName, healthCheckValid), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSLBTargetGroupExists("aws_lb_target_group.test", &conf), + resource.TestCheckResourceAttrSet("aws_lb_target_group.test", "arn"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "name", targetGroupName), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "port", "443"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "protocol", "TCP"), + resource.TestCheckResourceAttrSet("aws_lb_target_group.test", "vpc_id"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "deregistration_delay", "200"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.#", "1"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.interval", "10"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.port", "8081"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.protocol", "TCP"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.timeout", "10"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.healthy_threshold", "3"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.unhealthy_threshold", "3"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "tags.%", "1"), + resource.TestCheckResourceAttr("aws_lb_target_group.test", "tags.Name", "TestAccAWSLBTargetGroup_application_LB_defaults"), + ), + }, + }, + }) +} + func testAccCheckAWSLBTargetGroupExists(n string, res *elbv2.TargetGroup) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -648,6 +753,71 @@ func testAccCheckAWSLBTargetGroupDestroy(s *terraform.State) error { return nil } +func testAccALB_defaults(name string) string { + return fmt.Sprintf(` +resource "aws_lb_target_group" "test" { + name = "%s" + port = 443 + protocol = "HTTP" + vpc_id = "${aws_vpc.test.id}" + + deregistration_delay = 200 + + # HTTP Only + stickiness { + type = "lb_cookie" + cookie_duration = 10000 + } + + health_check { + interval = 10 + port = 8081 + protocol = "HTTP" + healthy_threshold = 3 + unhealthy_threshold = 3 + } + tags { + Name = "TestAccAWSLBTargetGroup_application_LB_defaults" + } +} + +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags { + Name = "TestAccAWSLBTargetGroup_application_LB_defaults" + } +}`, name) +} + +func testAccNLB_defaults(name, healthCheckBlock string) string { + return fmt.Sprintf(` +resource "aws_lb_target_group" "test" { + name = "%s" + port = 443 + protocol = "TCP" + vpc_id = "${aws_vpc.test.id}" + + deregistration_delay = 200 + + health_check { + %s + } + + tags { + Name = "TestAccAWSLBTargetGroup_application_LB_defaults" + } +} + +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags { + Name = "TestAccAWSLBTargetGroup_application_LB_defaults" + } +}`, name, healthCheckBlock) +} + func testAccAWSLBTargetGroupConfig_basic(targetGroupName string) string { return fmt.Sprintf(`resource "aws_lb_target_group" "test" { name = "%s" diff --git a/website/docs/r/lb_target_group.html.markdown b/website/docs/r/lb_target_group.html.markdown index abd06a515d97..ac4e52156265 100644 --- a/website/docs/r/lb_target_group.html.markdown +++ b/website/docs/r/lb_target_group.html.markdown @@ -53,16 +53,22 @@ 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` -Health Check Blocks (`health_check`) support the following: +Health Check Blocks (`health_check`): + +~> **Note:** The Health Check parameters you can set vary by the `protocol` of +the Target Group. Many parameters cannot be set to custom values for `network` +load balancers at this time. See +http://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_CreateTargetGroup.html +for a complete reference. * `interval` - (Optional) The approximate amount of time, in seconds, between health checks of an individual target. Minimum value 5 seconds, Maximum value 300 seconds. Default 30 seconds. * `path` - (Optional) The destination for the health check request. Default `/`. * `port` - (Optional) The port to use to connect with the target. Valid values are either ports 1-65536, or `traffic-port`. Defaults to `traffic-port`. * `protocol` - (Optional) The protocol to use to connect with the target. Defaults to `HTTP`. -* `timeout` - (Optional) The amount of time, in seconds, during which no response means a failed health check. Defaults to 5 seconds. -* `healthy_threshold` - (Optional) The number of consecutive health checks successes required before considering an unhealthy target healthy. Defaults to 3. -* `unhealthy_threshold` - (Optional) The number of consecutive health check failures required before considering the target unhealthy. Defaults to 3. -* `matcher` (Optional) The HTTP codes to use when checking for a successful response from a target. Defaults to `200`. You can specify multiple values (for example, "200,202") or a range of values (for example, "200-299"). +* `timeout` - (Optional) The amount of time, in seconds, during which no response means a failed health check. For Application Load Balancers, the range is 2 to 60 seconds and the default is 5 seconds. For Network Load Balancers, you cannot set a custom value, and the default is 10 seconds for TCP and HTTPS health checks and 6 seconds for HTTP health checks. +* `healthy_threshold` - (Optional) The number of consecutive health checks successes required before considering an unhealthy target healthy. Defaults to 5. +* `unhealthy_threshold` - (Optional) The number of consecutive health check failures required before considering the target unhealthy. Defaults to 2. +* `matcher` (Optional, only supported on Application Load Balancers): The HTTP codes to use when checking for a successful response from a target. You can specify multiple values (for example, "200,202") or a range of values (for example, "200-299"). ## Attributes Reference