From 7960f1c45d32dbf641f0f471e881ea7363b1a249 Mon Sep 17 00:00:00 2001 From: Miguel Pilar Date: Fri, 2 Mar 2018 16:55:37 -0400 Subject: [PATCH] enable_http2 flag for LBs and small refactor - refactored to use a switch statement a la #3578 - added enable_http2 as a flag with tests - made the boolean flag tests have 3 steps: on, off, on. to test creation, removal and enabling of a running LB - increased timeout and delay for aws_internet_gateway detachment refresh - increased wait time to wait for network interfaces to be destroyed after deleting an NLB (this, combined with the above, caused the EIP hanging resource I saw yesterday) - made all test fixtures use lb_test as the name for the aws_lb (to prevent the typo issue I had) --- aws/resource_aws_internet_gateway.go | 13 +-- aws/resource_aws_lb.go | 106 +++++++++++--------- aws/resource_aws_lb_test.go | 145 ++++++++++++++++++++++++--- website/docs/r/lb.html.markdown | 1 + 4 files changed, 198 insertions(+), 67 deletions(-) diff --git a/aws/resource_aws_internet_gateway.go b/aws/resource_aws_internet_gateway.go index a86f11670cb..1ca94b9bc19 100644 --- a/aws/resource_aws_internet_gateway.go +++ b/aws/resource_aws_internet_gateway.go @@ -236,12 +236,13 @@ func resourceAwsInternetGatewayDetach(d *schema.ResourceData, meta interface{}) // Wait for it to be fully detached before continuing log.Printf("[DEBUG] Waiting for internet gateway (%s) to detach", d.Id()) stateConf := &resource.StateChangeConf{ - Pending: []string{"detaching"}, - Target: []string{"detached"}, - Refresh: detachIGStateRefreshFunc(conn, d.Id(), vpcID.(string)), - Timeout: 15 * time.Minute, - Delay: 10 * time.Second, - NotFoundChecks: 30, + Pending: []string{"detaching"}, + Target: []string{"detached"}, + Refresh: detachIGStateRefreshFunc(conn, d.Id(), vpcID.(string)), + Timeout: 15 * time.Minute, + Delay: 20 * time.Second, + NotFoundChecks: 30, + ContinuousTargetOccurence: 2, } if _, err := stateConf.WaitForState(); err != nil { return fmt.Errorf( diff --git a/aws/resource_aws_lb.go b/aws/resource_aws_lb.go index 8834da8990f..f9f180292a7 100644 --- a/aws/resource_aws_lb.go +++ b/aws/resource_aws_lb.go @@ -163,6 +163,12 @@ func resourceAwsLb() *schema.Resource { Default: false, }, + "enable_http2": { + Type: schema.TypeBool, + Optional: true, + Default: true, + }, + "ip_address_type": { Type: schema.TypeString, Computed: true, @@ -324,56 +330,62 @@ func resourceAwsLbUpdate(d *schema.ResourceData, meta interface{}) error { attributes := make([]*elbv2.LoadBalancerAttribute, 0) - if d.HasChange("access_logs") { - logs := d.Get("access_logs").([]interface{}) - if len(logs) == 1 { - log := logs[0].(map[string]interface{}) - - attributes = append(attributes, - &elbv2.LoadBalancerAttribute{ - Key: aws.String("access_logs.s3.enabled"), - Value: aws.String(strconv.FormatBool(log["enabled"].(bool))), - }, - &elbv2.LoadBalancerAttribute{ - Key: aws.String("access_logs.s3.bucket"), - Value: aws.String(log["bucket"].(string)), - }) - - if prefix, ok := log["prefix"]; ok { + switch d.Get("load_balancer_type").(string) { + case "application": + if d.HasChange("access_logs") { + logs := d.Get("access_logs").([]interface{}) + if len(logs) == 1 { + log := logs[0].(map[string]interface{}) + + attributes = append(attributes, + &elbv2.LoadBalancerAttribute{ + Key: aws.String("access_logs.s3.enabled"), + Value: aws.String(strconv.FormatBool(log["enabled"].(bool))), + }, + &elbv2.LoadBalancerAttribute{ + Key: aws.String("access_logs.s3.bucket"), + Value: aws.String(log["bucket"].(string)), + }) + + if prefix, ok := log["prefix"]; ok { + attributes = append(attributes, &elbv2.LoadBalancerAttribute{ + Key: aws.String("access_logs.s3.prefix"), + Value: aws.String(prefix.(string)), + }) + } + } else if len(logs) == 0 { attributes = append(attributes, &elbv2.LoadBalancerAttribute{ - Key: aws.String("access_logs.s3.prefix"), - Value: aws.String(prefix.(string)), + Key: aws.String("access_logs.s3.enabled"), + Value: aws.String("false"), }) } - } else if len(logs) == 0 { + } + if d.HasChange("idle_timeout") { attributes = append(attributes, &elbv2.LoadBalancerAttribute{ - Key: aws.String("access_logs.s3.enabled"), - Value: aws.String("false"), + Key: aws.String("idle_timeout.timeout_seconds"), + Value: aws.String(fmt.Sprintf("%d", d.Get("idle_timeout").(int))), + }) + } + if d.HasChange("enable_http2") { + attributes = append(attributes, &elbv2.LoadBalancerAttribute{ + Key: aws.String("routing.http2.enabled"), + Value: aws.String(strconv.FormatBool(d.Get("enable_http2").(bool))), + }) + } + case "network": + if d.HasChange("enable_cross_zone_load_balancing") { + attributes = append(attributes, &elbv2.LoadBalancerAttribute{ + Key: aws.String("load_balancing.cross_zone.enabled"), + Value: aws.String(fmt.Sprintf("%t", d.Get("enable_cross_zone_load_balancing").(bool))), + }) + } + default: + if d.HasChange("enable_deletion_protection") { + attributes = append(attributes, &elbv2.LoadBalancerAttribute{ + Key: aws.String("deletion_protection.enabled"), + Value: aws.String(fmt.Sprintf("%t", d.Get("enable_deletion_protection").(bool))), }) } - } - - if d.HasChange("enable_deletion_protection") { - attributes = append(attributes, &elbv2.LoadBalancerAttribute{ - Key: aws.String("deletion_protection.enabled"), - Value: aws.String(fmt.Sprintf("%t", d.Get("enable_deletion_protection").(bool))), - }) - } - - // It's important to know that Idle timeout is not supported for Network Loadbalancers - if d.Get("load_balancer_type").(string) != "network" && d.HasChange("idle_timeout") { - attributes = append(attributes, &elbv2.LoadBalancerAttribute{ - Key: aws.String("idle_timeout.timeout_seconds"), - Value: aws.String(fmt.Sprintf("%d", d.Get("idle_timeout").(int))), - }) - } - - // It's important to know that Idle timeout is only supported for Network Loadbalancers - if d.Get("load_balancer_type").(string) == "network" && d.HasChange("enable_cross_zone_load_balancing") { - attributes = append(attributes, &elbv2.LoadBalancerAttribute{ - Key: aws.String("load_balancing.cross_zone.enabled"), - Value: aws.String(fmt.Sprintf("%t", d.Get("enable_cross_zone_load_balancing").(bool))), - }) } if len(attributes) != 0 { @@ -552,7 +564,7 @@ func waitForNLBNetworkInterfacesToDetach(conn *ec2.EC2, lbArn string) error { // OperationNotPermitted: You are not allowed to manage 'ela-attach' attachments. // yet presence of these ENIs may prevent us from deleting EIPs associated w/ the NLB - return resource.Retry(1*time.Minute, func() *resource.RetryError { + return resource.Retry(5*time.Minute, func() *resource.RetryError { out, err := conn.DescribeNetworkInterfaces(&ec2.DescribeNetworkInterfacesInput{ Filters: []*ec2.Filter{ { @@ -695,6 +707,10 @@ func flattenAwsLbResource(d *schema.ResourceData, meta interface{}, lb *elbv2.Lo protectionEnabled := (*attr.Value) == "true" log.Printf("[DEBUG] Setting LB Deletion Protection Enabled: %t", protectionEnabled) d.Set("enable_deletion_protection", protectionEnabled) + case "enable_http2": + http2Enabled := (*attr.Value) == "true" + log.Printf("[DEBUG] Setting ALB HTTP/2 Enabled: %t", http2Enabled) + d.Set("enable_http2", http2Enabled) case "load_balancing.cross_zone.enabled": crossZoneLbEnabled := (*attr.Value) == "true" log.Printf("[DEBUG] Setting NLB Cross Zone Load Balancing Enabled: %t", crossZoneLbEnabled) diff --git a/aws/resource_aws_lb_test.go b/aws/resource_aws_lb_test.go index 8ed851df4ee..589ab8c3a6a 100644 --- a/aws/resource_aws_lb_test.go +++ b/aws/resource_aws_lb_test.go @@ -121,16 +121,16 @@ func TestAccAWSLB_networkLoadbalancerEIP(t *testing.T) { { Config: testAccAWSLBConfig_networkLoadBalancerEIP(lbName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckAWSLBExists("aws_lb.test", &conf), - resource.TestCheckResourceAttr("aws_lb.test", "name", lbName), - resource.TestCheckResourceAttr("aws_lb.test", "internal", "false"), - resource.TestCheckResourceAttr("aws_lb.test", "ip_address_type", "ipv4"), - resource.TestCheckResourceAttrSet("aws_lb.test", "zone_id"), - resource.TestCheckResourceAttrSet("aws_lb.test", "dns_name"), - resource.TestCheckResourceAttrSet("aws_lb.test", "arn"), - resource.TestCheckResourceAttr("aws_lb.test", "enable_cross_zone_load_balancing", "true"), - resource.TestCheckResourceAttr("aws_lb.test", "load_balancer_type", "network"), - resource.TestCheckResourceAttr("aws_lb.test", "subnet_mapping.#", "2"), + testAccCheckAWSLBExists("aws_lb.lb_test", &conf), + resource.TestCheckResourceAttr("aws_lb.lb_test", "name", lbName), + resource.TestCheckResourceAttr("aws_lb.lb_test", "internal", "false"), + resource.TestCheckResourceAttr("aws_lb.lb_test", "ip_address_type", "ipv4"), + resource.TestCheckResourceAttrSet("aws_lb.lb_test", "zone_id"), + resource.TestCheckResourceAttrSet("aws_lb.lb_test", "dns_name"), + resource.TestCheckResourceAttrSet("aws_lb.lb_test", "arn"), + resource.TestCheckResourceAttr("aws_lb.lb_test", "load_balancer_type", "network"), + resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_deletion_protection", "false"), + resource.TestCheckResourceAttr("aws_lb.lb_test", "subnet_mapping.#", "2"), ), }, }, @@ -266,7 +266,7 @@ func TestAccAWSLB_tags(t *testing.T) { } func TestAccAWSLB_networkLoadbalancer_updateCrossZone(t *testing.T) { - var pre, post elbv2.LoadBalancer + var pre, mid, post elbv2.LoadBalancer lbName := fmt.Sprintf("testaccawslb-nlbcz-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)) resource.Test(t, resource.TestCase{ @@ -285,9 +285,54 @@ func TestAccAWSLB_networkLoadbalancer_updateCrossZone(t *testing.T) { { Config: testAccAWSLBConfig_networkLoadbalancer(lbName, false), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckAWSLBExists("aws_lb.lb_test", &post), + testAccCheckAWSLBExists("aws_lb.lb_test", &mid), resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_cross_zone_load_balancing", "false"), - testAccCheckAWSlbARNs(&pre, &post), + testAccCheckAWSlbARNs(&pre, &mid), + ), + }, + { + Config: testAccAWSLBConfig_networkLoadbalancer(lbName, true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSLBExists("aws_lb.lb_test", &post), + resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_cross_zone_load_balancing", "true"), + testAccCheckAWSlbARNs(&mid, &post), + ), + }, + }, + }) +} + +func TestAccAWSLB_applicationLoadBalancer_updateHttp2(t *testing.T) { + var pre, mid, post elbv2.LoadBalancer + lbName := fmt.Sprintf("testaccawsalb-http2-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_lb.lb_test", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSLBDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSLBConfig_basicWithHttp2Toggle(lbName, true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSLBExists("aws_lb.lb_test", &pre), + resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "true"), + ), + }, + { + Config: testAccAWSLBConfig_basicWithHttp2Toggle(lbName, false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSLBExists("aws_lb.lb_test", &mid), + resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "false"), + testAccCheckAWSlbARNs(&pre, &mid), + ), + }, + { + Config: testAccAWSLBConfig_basicWithHttp2Toggle(lbName, true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAWSLBExists("aws_lb.lb_test", &post), + resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "true"), + testAccCheckAWSlbARNs(&mid, &post), ), }, }, @@ -874,6 +919,75 @@ resource "aws_security_group" "alb_test" { }`, lbName) } +func testAccAWSLBConfig_basicWithHttp2Toggle(lbName string, http2 bool) string { + return fmt.Sprintf(`resource "aws_lb" "lb_test" { + name = "%s" + internal = true + security_groups = ["${aws_security_group.alb_test.id}"] + subnets = ["${aws_subnet.alb_test.*.id}"] + + idle_timeout = 30 + enable_deletion_protection = false + + enable_http2 = %t + + tags { + Name = "TestAccAWSALB_basic" + } +} + +variable "subnets" { + default = ["10.0.1.0/24", "10.0.2.0/24"] + type = "list" +} + +data "aws_availability_zones" "available" {} + +resource "aws_vpc" "alb_test" { + cidr_block = "10.0.0.0/16" + + tags { + Name = "terraform-testacc-lb-basic" + } +} + +resource "aws_subnet" "alb_test" { + count = 2 + vpc_id = "${aws_vpc.alb_test.id}" + cidr_block = "${element(var.subnets, count.index)}" + map_public_ip_on_launch = true + availability_zone = "${element(data.aws_availability_zones.available.names, count.index)}" + + tags { + Name = "TestAccAWSALB_basic" + } +} + +resource "aws_security_group" "alb_test" { + name = "allow_all_alb_test" + description = "Used for ALB Testing" + vpc_id = "${aws_vpc.alb_test.id}" + + ingress { + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = ["0.0.0.0/0"] + } + + egress { + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = ["0.0.0.0/0"] + } + + tags { + Name = "TestAccAWSALB_basic" + } +}`, lbName, http2) +} + func testAccAWSLBConfig_networkLoadbalancer_subnets(lbName string) string { return fmt.Sprintf(`resource "aws_vpc" "alb_test" { cidr_block = "10.0.0.0/16" @@ -1011,10 +1125,9 @@ resource "aws_route_table_association" "a" { route_table_id = "${aws_route_table.public.id}" } -resource "aws_lb" "test" { +resource "aws_lb" "lb_test" { name = "%s" load_balancer_type = "network" - enable_cross_zone_load_balancing = true subnet_mapping { subnet_id = "${aws_subnet.public.0.id}" allocation_id = "${aws_eip.lb.0.id}" @@ -1026,7 +1139,7 @@ resource "aws_lb" "test" { } resource "aws_eip" "lb" { - count = "${length(data.aws_availability_zones.available.names)}" + count = "${length(data.aws_availability_zones.available.names)>2?2:length(data.aws_availability_zones.available.names)}" } `, lbName) } diff --git a/website/docs/r/lb.html.markdown b/website/docs/r/lb.html.markdown index 3e2bcb1cf5d..221576487fa 100644 --- a/website/docs/r/lb.html.markdown +++ b/website/docs/r/lb.html.markdown @@ -62,6 +62,7 @@ will for load balancers of type `network` will force a recreation of the resourc the AWS API. This will prevent Terraform from deleting the load balancer. Defaults to `false`. * `enable_cross_zone_load_balancing` - (Optional) If true, cross-zone load balancing of the load balancer will be enabled. This is a `network` load balancer feature. Defaults to `false`. +* `enable_http2` - (Optional) Indicates whether HTTP/2 is enabled in `application` load balancers. Defaults to `true`. * `ip_address_type` - (Optional) The type of IP addresses used by the subnets for your load balancer. The possible values are `ipv4` and `dualstack` * `tags` - (Optional) A mapping of tags to assign to the resource.