From 4bf17ff090a096995f4ae29707b90a766295b4ee Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Tue, 20 Mar 2018 22:52:12 -0400 Subject: [PATCH 1/3] fix lb read of http2 attribute add acceptance checks on attribute tests --- aws/resource_aws_lb.go | 12 +++---- aws/resource_aws_lb_test.go | 62 +++++++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/aws/resource_aws_lb.go b/aws/resource_aws_lb.go index f6d978b34ede..a80639d04af8 100644 --- a/aws/resource_aws_lb.go +++ b/aws/resource_aws_lb.go @@ -332,7 +332,7 @@ func resourceAwsLbUpdate(d *schema.ResourceData, meta interface{}) error { switch d.Get("load_balancer_type").(string) { case "application": - if d.HasChange("access_logs") { + if d.HasChange("access_logs") || d.IsNewResource() { logs := d.Get("access_logs").([]interface{}) if len(logs) == 1 { log := logs[0].(map[string]interface{}) @@ -360,20 +360,20 @@ func resourceAwsLbUpdate(d *schema.ResourceData, meta interface{}) error { }) } } - if d.HasChange("idle_timeout") { + if d.HasChange("idle_timeout") || d.IsNewResource() { attributes = append(attributes, &elbv2.LoadBalancerAttribute{ Key: aws.String("idle_timeout.timeout_seconds"), Value: aws.String(fmt.Sprintf("%d", d.Get("idle_timeout").(int))), }) } - if d.HasChange("enable_http2") { + if d.HasChange("enable_http2") || d.IsNewResource() { 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") { + if d.HasChange("enable_cross_zone_load_balancing") || d.IsNewResource() { 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))), @@ -381,7 +381,7 @@ func resourceAwsLbUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("enable_deletion_protection") { + if d.HasChange("enable_deletion_protection") || d.IsNewResource() { attributes = append(attributes, &elbv2.LoadBalancerAttribute{ Key: aws.String("deletion_protection.enabled"), Value: aws.String(fmt.Sprintf("%t", d.Get("enable_deletion_protection").(bool))), @@ -707,7 +707,7 @@ 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": + case "routing.http2.enabled": http2Enabled := (*attr.Value) == "true" log.Printf("[DEBUG] Setting ALB HTTP/2 Enabled: %t", http2Enabled) d.Set("enable_http2", http2Enabled) diff --git a/aws/resource_aws_lb_test.go b/aws/resource_aws_lb_test.go index 59a1dd70a072..08049cd93ac8 100644 --- a/aws/resource_aws_lb_test.go +++ b/aws/resource_aws_lb_test.go @@ -279,6 +279,7 @@ func TestAccAWSLB_networkLoadbalancer_updateCrossZone(t *testing.T) { Config: testAccAWSLBConfig_networkLoadbalancer(lbName, true), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &pre), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "load_balancing.cross_zone.enabled", "true"), resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_cross_zone_load_balancing", "true"), ), }, @@ -286,6 +287,7 @@ func TestAccAWSLB_networkLoadbalancer_updateCrossZone(t *testing.T) { Config: testAccAWSLBConfig_networkLoadbalancer(lbName, false), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &mid), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "load_balancing.cross_zone.enabled", "false"), resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_cross_zone_load_balancing", "false"), testAccCheckAWSlbARNs(&pre, &mid), ), @@ -294,6 +296,7 @@ func TestAccAWSLB_networkLoadbalancer_updateCrossZone(t *testing.T) { Config: testAccAWSLBConfig_networkLoadbalancer(lbName, true), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &post), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "load_balancing.cross_zone.enabled", "true"), resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_cross_zone_load_balancing", "true"), testAccCheckAWSlbARNs(&mid, &post), ), @@ -313,25 +316,28 @@ func TestAccAWSLB_applicationLoadBalancer_updateHttp2(t *testing.T) { CheckDestroy: testAccCheckAWSLBDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSLBConfig_enableHttp2(lbName, true), + Config: testAccAWSLBConfig_enableHttp2(lbName, false), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &pre), - resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "true"), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "routing.http2.enabled", "false"), + resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "false"), ), }, { - Config: testAccAWSLBConfig_enableHttp2(lbName, false), + Config: testAccAWSLBConfig_enableHttp2(lbName, true), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &mid), - resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "false"), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "routing.http2.enabled", "true"), + resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "true"), testAccCheckAWSlbARNs(&pre, &mid), ), }, { - Config: testAccAWSLBConfig_enableHttp2(lbName, true), + Config: testAccAWSLBConfig_enableHttp2(lbName, false), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &post), - resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "true"), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "routing.http2.enabled", "false"), + resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_http2", "false"), testAccCheckAWSlbARNs(&mid, &post), ), }, @@ -353,6 +359,7 @@ func TestAccAWSLB_applicationLoadBalancer_updateDeletionProtection(t *testing.T) Config: testAccAWSLBConfig_enableDeletionProtection(lbName, false), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &pre), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "deletion_protection.enabled", "false"), resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_deletion_protection", "false"), ), }, @@ -360,6 +367,7 @@ func TestAccAWSLB_applicationLoadBalancer_updateDeletionProtection(t *testing.T) Config: testAccAWSLBConfig_enableDeletionProtection(lbName, true), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &mid), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "deletion_protection.enabled", "true"), resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_deletion_protection", "true"), testAccCheckAWSlbARNs(&pre, &mid), ), @@ -368,6 +376,7 @@ func TestAccAWSLB_applicationLoadBalancer_updateDeletionProtection(t *testing.T) Config: testAccAWSLBConfig_enableDeletionProtection(lbName, false), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &post), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "deletion_protection.enabled", "false"), resource.TestCheckResourceAttr("aws_lb.lb_test", "enable_deletion_protection", "false"), testAccCheckAWSlbARNs(&mid, &post), ), @@ -511,6 +520,9 @@ func TestAccAWSLB_accesslogs(t *testing.T) { Config: testAccAWSLBConfig_basic(lbName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &conf), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.enabled", "false"), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.bucket", ""), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.prefix", ""), resource.TestCheckResourceAttr("aws_lb.lb_test", "name", lbName), resource.TestCheckResourceAttr("aws_lb.lb_test", "internal", "true"), resource.TestCheckResourceAttr("aws_lb.lb_test", "subnets.#", "2"), @@ -529,6 +541,9 @@ func TestAccAWSLB_accesslogs(t *testing.T) { Config: testAccAWSLBConfig_accessLogs(true, lbName, bucketName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &conf), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.enabled", "true"), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.bucket", bucketName), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.prefix", "testAccAWSALBConfig_accessLogs"), resource.TestCheckResourceAttr("aws_lb.lb_test", "name", lbName), resource.TestCheckResourceAttr("aws_lb.lb_test", "internal", "true"), resource.TestCheckResourceAttr("aws_lb.lb_test", "subnets.#", "2"), @@ -551,6 +566,9 @@ func TestAccAWSLB_accesslogs(t *testing.T) { Config: testAccAWSLBConfig_accessLogs(false, lbName, bucketName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &conf), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.enabled", "false"), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.bucket", ""), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.prefix", ""), resource.TestCheckResourceAttr("aws_lb.lb_test", "name", lbName), resource.TestCheckResourceAttr("aws_lb.lb_test", "internal", "true"), resource.TestCheckResourceAttr("aws_lb.lb_test", "subnets.#", "2"), @@ -637,6 +655,38 @@ func testAccCheckAWSLBExists(n string, res *elbv2.LoadBalancer) resource.TestChe } } +func testAccCheckAWSLBAttribute(n, key, value string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return errors.New("No LB ID is set") + } + + conn := testAccProvider.Meta().(*AWSClient).elbv2conn + attributesResp, err := conn.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{ + LoadBalancerArn: aws.String(rs.Primary.ID), + }) + if err != nil { + return errwrap.Wrapf("Error retrieving LB Attributes: {{err}}", err) + } + + for _, attr := range attributesResp.Attributes { + if *attr.Key == key { + if *attr.Value == value { + return nil + } else { + return fmt.Errorf(`LB attribute %s expected: "%s" actual: "%s"`, key, value, *attr.Value) + } + } + } + return fmt.Errorf("LB attribute %s does not exist on LB: %s", key, rs.Primary.ID) + } +} + func testAccCheckAWSLBDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).elbv2conn From 8812cde2cb932c8e2f7e508adc6b716f2245d004 Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Thu, 22 Mar 2018 15:42:38 -0400 Subject: [PATCH 2/3] fix acceptance test --- aws/resource_aws_lb_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_lb_test.go b/aws/resource_aws_lb_test.go index 08049cd93ac8..2cc308173f97 100644 --- a/aws/resource_aws_lb_test.go +++ b/aws/resource_aws_lb_test.go @@ -567,8 +567,8 @@ func TestAccAWSLB_accesslogs(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAWSLBExists("aws_lb.lb_test", &conf), testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.enabled", "false"), - testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.bucket", ""), - testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.prefix", ""), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.bucket", bucketName), + testAccCheckAWSLBAttribute("aws_lb.lb_test", "access_logs.s3.prefix", "testAccAWSALBConfig_accessLogs"), resource.TestCheckResourceAttr("aws_lb.lb_test", "name", lbName), resource.TestCheckResourceAttr("aws_lb.lb_test", "internal", "true"), resource.TestCheckResourceAttr("aws_lb.lb_test", "subnets.#", "2"), From 81bc63f8a82b652f377761f3ad32d105633621aa Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Tue, 27 Mar 2018 11:05:34 -0400 Subject: [PATCH 3/3] use %q instead of "%s" --- aws/resource_aws_lb_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_lb_test.go b/aws/resource_aws_lb_test.go index 2cc308173f97..c5b91b524062 100644 --- a/aws/resource_aws_lb_test.go +++ b/aws/resource_aws_lb_test.go @@ -679,7 +679,7 @@ func testAccCheckAWSLBAttribute(n, key, value string) resource.TestCheckFunc { if *attr.Value == value { return nil } else { - return fmt.Errorf(`LB attribute %s expected: "%s" actual: "%s"`, key, value, *attr.Value) + return fmt.Errorf("LB attribute %s expected: %q actual: %q", key, value, *attr.Value) } } }