Skip to content

Commit

Permalink
r/aws_lb_target_group: various fixes to behavior based on protocol ty…
Browse files Browse the repository at this point in the history
…pe (#2380)

* fix up nlb/alb issues
* regression / acceptance tests
* matcher is optional, not required
  • Loading branch information
catsby authored Dec 14, 2017
1 parent 8994a34 commit a6d1266
Show file tree
Hide file tree
Showing 3 changed files with 264 additions and 33 deletions.
111 changes: 83 additions & 28 deletions aws/resource_aws_lb_target_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -132,6 +132,7 @@ func resourceAwsLbTargetGroup() *schema.Resource {
"path": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validateAwsLbTargetGroupHealthCheckPath,
},

Expand All @@ -155,7 +156,7 @@ func resourceAwsLbTargetGroup() *schema.Resource {
"timeout": {
Type: schema.TypeInt,
Optional: true,
Default: 10,
Computed: true,
ValidateFunc: validateAwsLbTargetGroupHealthCheckTimeout,
},

Expand All @@ -168,6 +169,7 @@ func resourceAwsLbTargetGroup() *schema.Resource {

"matcher": {
Type: schema.TypeString,
Computed: true,
Optional: true,
},

Expand Down Expand Up @@ -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),
}
}
}
}
Expand Down Expand Up @@ -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{
Expand All @@ -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)
}
}
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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
}
170 changes: 170 additions & 0 deletions aws/resource_aws_lb_target_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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"
Expand Down
Loading

0 comments on commit a6d1266

Please sign in to comment.