Skip to content

Commit

Permalink
service/elbv2: Refactor to use keyvaluetags package and SDK enums, ad…
Browse files Browse the repository at this point in the history 
…d plan-time validation (#11419)

Output from acceptance testing:

```
--- PASS: TestAccAWSLB_ALB_AccessLogs (290.82s)
--- PASS: TestAccAWSLB_ALB_AccessLogs_Prefix (234.28s)
--- PASS: TestAccAWSLB_ALB_basic (183.55s)
--- PASS: TestAccAWSLB_applicationLoadBalancer_updateDeletionProtection (267.31s)
--- PASS: TestAccAWSLB_applicationLoadBalancer_updateHttp2 (237.66s)
--- PASS: TestAccAWSLB_generatedName (208.78s)
--- PASS: TestAccAWSLB_generatesNameForZeroValue (196.87s)
--- PASS: TestAccAWSLB_namePrefix (193.50s)
--- PASS: TestAccAWSLB_networkLoadbalancer_subnet_change (216.71s)
--- PASS: TestAccAWSLB_networkLoadbalancer_updateCrossZone (325.01s)
--- PASS: TestAccAWSLB_networkLoadbalancerEIP (245.80s)
--- PASS: TestAccAWSLB_NLB_AccessLogs (331.75s)
--- PASS: TestAccAWSLB_NLB_AccessLogs_Prefix (275.50s)
--- PASS: TestAccAWSLB_NLB_basic (211.14s)
--- PASS: TestAccAWSLB_noSecurityGroup (266.95s)
--- PASS: TestAccAWSLB_tags (258.93s)
--- PASS: TestAccAWSLB_updatedIpAddressType (215.52s)
--- PASS: TestAccAWSLB_updatedSecurityGroups (227.98s)
--- PASS: TestAccAWSLBBackwardsCompatibility (175.67s)
--- PASS: TestAccDataSourceAWSLB_basic (173.24s)
--- PASS: TestAccDataSourceAWSLBBackwardsCompatibility (252.83s)
```
  • Loading branch information
@DrFaust92 @bflad
DrFaust92 authored and bflad committed Jan 2, 2020
1 parent 60f51ed commit 91d4c37
Show file tree
Hide file tree
Showing 3 changed files with 215 additions and 180 deletions.
101 changes: 52 additions & 49 deletions aws/data_source_aws_lb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (

func TestAccDataSourceAWSLB_basic(t *testing.T) {
lbName := fmt.Sprintf("testaccawslb-basic-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum))
dataSourceName := "data.aws_lb.alb_test_with_arn"
dataSourceName2 := "data.aws_lb.alb_test_with_name"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -18,30 +20,30 @@ func TestAccDataSourceAWSLB_basic(t *testing.T) {
{
Config: testAccDataSourceAWSLBConfigBasic(lbName),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_arn", "name", lbName),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_arn", "internal", "true"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_arn", "subnets.#", "2"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_arn", "security_groups.#", "1"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_arn", "tags.%", "1"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_arn", "tags.TestName", "TestAccAWSALB_basic"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_arn", "enable_deletion_protection", "false"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_arn", "idle_timeout", "30"),
resource.TestCheckResourceAttrSet("data.aws_lb.alb_test_with_arn", "vpc_id"),
resource.TestCheckResourceAttrSet("data.aws_lb.alb_test_with_arn", "zone_id"),
resource.TestCheckResourceAttrSet("data.aws_lb.alb_test_with_arn", "dns_name"),
resource.TestCheckResourceAttrSet("data.aws_lb.alb_test_with_arn", "arn"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_name", "name", lbName),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_name", "internal", "true"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_name", "subnets.#", "2"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_name", "security_groups.#", "1"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_name", "tags.%", "1"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_name", "tags.TestName", "TestAccAWSALB_basic"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_name", "enable_deletion_protection", "false"),
resource.TestCheckResourceAttr("data.aws_lb.alb_test_with_name", "idle_timeout", "30"),
resource.TestCheckResourceAttrSet("data.aws_lb.alb_test_with_name", "vpc_id"),
resource.TestCheckResourceAttrSet("data.aws_lb.alb_test_with_name", "zone_id"),
resource.TestCheckResourceAttrSet("data.aws_lb.alb_test_with_name", "dns_name"),
resource.TestCheckResourceAttrSet("data.aws_lb.alb_test_with_name", "arn"),
resource.TestCheckResourceAttr(dataSourceName, "name", lbName),
resource.TestCheckResourceAttr(dataSourceName, "internal", "true"),
resource.TestCheckResourceAttr(dataSourceName, "subnets.#", "2"),
resource.TestCheckResourceAttr(dataSourceName, "security_groups.#", "1"),
resource.TestCheckResourceAttr(dataSourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(dataSourceName, "tags.TestName", "TestAccAWSALB_basic"),
resource.TestCheckResourceAttr(dataSourceName, "enable_deletion_protection", "false"),
resource.TestCheckResourceAttr(dataSourceName, "idle_timeout", "30"),
resource.TestCheckResourceAttrSet(dataSourceName, "vpc_id"),
resource.TestCheckResourceAttrSet(dataSourceName, "zone_id"),
resource.TestCheckResourceAttrSet(dataSourceName, "dns_name"),
resource.TestCheckResourceAttrSet(dataSourceName, "arn"),
resource.TestCheckResourceAttr(dataSourceName2, "name", lbName),
resource.TestCheckResourceAttr(dataSourceName2, "internal", "true"),
resource.TestCheckResourceAttr(dataSourceName2, "subnets.#", "2"),
resource.TestCheckResourceAttr(dataSourceName2, "security_groups.#", "1"),
resource.TestCheckResourceAttr(dataSourceName2, "tags.%", "1"),
resource.TestCheckResourceAttr(dataSourceName2, "tags.TestName", "TestAccAWSALB_basic"),
resource.TestCheckResourceAttr(dataSourceName2, "enable_deletion_protection", "false"),
resource.TestCheckResourceAttr(dataSourceName2, "idle_timeout", "30"),
resource.TestCheckResourceAttrSet(dataSourceName2, "vpc_id"),
resource.TestCheckResourceAttrSet(dataSourceName2, "zone_id"),
resource.TestCheckResourceAttrSet(dataSourceName2, "dns_name"),
resource.TestCheckResourceAttrSet(dataSourceName2, "arn"),
),
},
},
Expand All @@ -50,38 +52,39 @@ func TestAccDataSourceAWSLB_basic(t *testing.T) {

func TestAccDataSourceAWSLBBackwardsCompatibility(t *testing.T) {
lbName := fmt.Sprintf("testaccawsalb-basic-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum))

dataSourceName1 := "data.aws_alb.alb_test_with_arn"
dataSourceName2 := "data.aws_alb.alb_test_with_name"
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccDataSourceAWSLBConfigBackardsCompatibility(lbName),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_arn", "name", lbName),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_arn", "internal", "true"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_arn", "subnets.#", "2"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_arn", "security_groups.#", "1"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_arn", "tags.%", "1"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_arn", "tags.TestName", "TestAccAWSALB_basic"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_arn", "enable_deletion_protection", "false"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_arn", "idle_timeout", "30"),
resource.TestCheckResourceAttrSet("data.aws_alb.alb_test_with_arn", "vpc_id"),
resource.TestCheckResourceAttrSet("data.aws_alb.alb_test_with_arn", "zone_id"),
resource.TestCheckResourceAttrSet("data.aws_alb.alb_test_with_arn", "dns_name"),
resource.TestCheckResourceAttrSet("data.aws_alb.alb_test_with_arn", "arn"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_name", "name", lbName),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_name", "internal", "true"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_name", "subnets.#", "2"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_name", "security_groups.#", "1"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_name", "tags.%", "1"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_name", "tags.TestName", "TestAccAWSALB_basic"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_name", "enable_deletion_protection", "false"),
resource.TestCheckResourceAttr("data.aws_alb.alb_test_with_name", "idle_timeout", "30"),
resource.TestCheckResourceAttrSet("data.aws_alb.alb_test_with_name", "vpc_id"),
resource.TestCheckResourceAttrSet("data.aws_alb.alb_test_with_name", "zone_id"),
resource.TestCheckResourceAttrSet("data.aws_alb.alb_test_with_name", "dns_name"),
resource.TestCheckResourceAttrSet("data.aws_alb.alb_test_with_name", "arn"),
resource.TestCheckResourceAttr(dataSourceName1, "name", lbName),
resource.TestCheckResourceAttr(dataSourceName1, "internal", "true"),
resource.TestCheckResourceAttr(dataSourceName1, "subnets.#", "2"),
resource.TestCheckResourceAttr(dataSourceName1, "security_groups.#", "1"),
resource.TestCheckResourceAttr(dataSourceName1, "tags.%", "1"),
resource.TestCheckResourceAttr(dataSourceName1, "tags.TestName", "TestAccAWSALB_basic"),
resource.TestCheckResourceAttr(dataSourceName1, "enable_deletion_protection", "false"),
resource.TestCheckResourceAttr(dataSourceName1, "idle_timeout", "30"),
resource.TestCheckResourceAttrSet(dataSourceName1, "vpc_id"),
resource.TestCheckResourceAttrSet(dataSourceName1, "zone_id"),
resource.TestCheckResourceAttrSet(dataSourceName1, "dns_name"),
resource.TestCheckResourceAttrSet(dataSourceName1, "arn"),
resource.TestCheckResourceAttr(dataSourceName2, "name", lbName),
resource.TestCheckResourceAttr(dataSourceName2, "internal", "true"),
resource.TestCheckResourceAttr(dataSourceName2, "subnets.#", "2"),
resource.TestCheckResourceAttr(dataSourceName2, "security_groups.#", "1"),
resource.TestCheckResourceAttr(dataSourceName2, "tags.%", "1"),
resource.TestCheckResourceAttr(dataSourceName2, "tags.TestName", "TestAccAWSALB_basic"),
resource.TestCheckResourceAttr(dataSourceName2, "enable_deletion_protection", "false"),
resource.TestCheckResourceAttr(dataSourceName2, "idle_timeout", "30"),
resource.TestCheckResourceAttrSet(dataSourceName2, "vpc_id"),
resource.TestCheckResourceAttrSet(dataSourceName2, "zone_id"),
resource.TestCheckResourceAttrSet(dataSourceName2, "dns_name"),
resource.TestCheckResourceAttrSet(dataSourceName2, "arn"),
),
},
},
Expand Down
54 changes: 32 additions & 22 deletions aws/resource_aws_lb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/helper/hashcode"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

func resourceAwsLb() *schema.Resource {
Expand Down Expand Up @@ -73,7 +75,11 @@ func resourceAwsLb() *schema.Resource {
Type: schema.TypeString,
ForceNew: true,
Optional: true,
Default: "application",
Default: elbv2.LoadBalancerTypeEnumApplication,
ValidateFunc: validation.StringInSlice([]string{
elbv2.LoadBalancerTypeEnumApplication,
elbv2.LoadBalancerTypeEnumNetwork,
}, false),
},

"security_groups": {
Expand Down Expand Up @@ -162,27 +168,31 @@ func resourceAwsLb() *schema.Resource {
Type: schema.TypeInt,
Optional: true,
Default: 60,
DiffSuppressFunc: suppressIfLBType("network"),
DiffSuppressFunc: suppressIfLBType(elbv2.LoadBalancerTypeEnumNetwork),
},

"enable_cross_zone_load_balancing": {
Type: schema.TypeBool,
Optional: true,
Default: false,
DiffSuppressFunc: suppressIfLBType("application"),
DiffSuppressFunc: suppressIfLBType(elbv2.LoadBalancerTypeEnumApplication),
},

"enable_http2": {
Type: schema.TypeBool,
Optional: true,
Default: true,
DiffSuppressFunc: suppressIfLBType("network"),
DiffSuppressFunc: suppressIfLBType(elbv2.LoadBalancerTypeEnumNetwork),
},

"ip_address_type": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{
elbv2.IpAddressTypeIpv4,
elbv2.IpAddressTypeDualstack,
}, false),
},

"vpc_id": {
Expand Down Expand Up @@ -213,6 +223,7 @@ func suppressIfLBType(t string) schema.SchemaDiffSuppressFunc {

func resourceAwsLbCreate(d *schema.ResourceData, meta interface{}) error {
elbconn := meta.(*AWSClient).elbv2conn
tags := keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().Elbv2Tags()

var name string
if v, ok := d.GetOk("name"); ok {
Expand All @@ -227,7 +238,10 @@ func resourceAwsLbCreate(d *schema.ResourceData, meta interface{}) error {
elbOpts := &elbv2.CreateLoadBalancerInput{
Name: aws.String(name),
Type: aws.String(d.Get("load_balancer_type").(string)),
Tags: tagsFromMapELBv2(d.Get("tags").(map[string]interface{})),
}

if len(tags) > 0 {
elbOpts.Tags = tags
}

if scheme, ok := d.GetOk("internal"); ok && scheme.(bool) {
Expand Down Expand Up @@ -338,9 +352,11 @@ func resourceAwsLbRead(d *schema.ResourceData, meta interface{}) error {
func resourceAwsLbUpdate(d *schema.ResourceData, meta interface{}) error {
elbconn := meta.(*AWSClient).elbv2conn

if !d.IsNewResource() {
if err := setElbV2Tags(elbconn, d); err != nil {
return fmt.Errorf("Error Modifying Tags on ALB: %s", err)
if d.HasChange("tags") {
o, n := d.GetChange("tags")

if err := keyvaluetags.Elbv2UpdateTags(elbconn, d.Id(), o, n); err != nil {
return fmt.Errorf("error updating ALB (%s) tags: %s", d.Id(), err)
}
}

Expand Down Expand Up @@ -379,7 +395,7 @@ func resourceAwsLbUpdate(d *schema.ResourceData, meta interface{}) error {
}

switch d.Get("load_balancer_type").(string) {
case "application":
case elbv2.LoadBalancerTypeEnumApplication:
if d.HasChange("idle_timeout") || d.IsNewResource() {
attributes = append(attributes, &elbv2.LoadBalancerAttribute{
Key: aws.String("idle_timeout.timeout_seconds"),
Expand All @@ -392,7 +408,7 @@ func resourceAwsLbUpdate(d *schema.ResourceData, meta interface{}) error {
Value: aws.String(strconv.FormatBool(d.Get("enable_http2").(bool))),
})
}
case "network":
case elbv2.LoadBalancerTypeEnumNetwork:
if d.HasChange("enable_cross_zone_load_balancing") || d.IsNewResource() {
attributes = append(attributes, &elbv2.LoadBalancerAttribute{
Key: aws.String("load_balancing.cross_zone.enabled"),
Expand Down Expand Up @@ -702,20 +718,14 @@ func flattenAwsLbResource(d *schema.ResourceData, meta interface{}, lb *elbv2.Lo
return fmt.Errorf("error setting subnet_mapping: %s", err)
}

respTags, err := elbconn.DescribeTags(&elbv2.DescribeTagsInput{
ResourceArns: []*string{lb.LoadBalancerArn},
})
if err != nil {
return fmt.Errorf("Error retrieving LB Tags: %s", err)
}
tags, err := keyvaluetags.Elbv2ListTags(elbconn, d.Id())

var et []*elbv2.Tag
if len(respTags.TagDescriptions) > 0 {
et = respTags.TagDescriptions[0].Tags
if err != nil {
return fmt.Errorf("error listing tags for (%s): %s", d.Id(), err)
}

if err := d.Set("tags", tagsToMapELBv2(et)); err != nil {
log.Printf("[WARN] Error setting tags for AWS LB (%s): %s", d.Id(), err)
if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
}

attributesResp, err := elbconn.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{
Expand Down Expand Up @@ -782,7 +792,7 @@ func customizeDiffNLBSubnets(diff *schema.ResourceDiff, v interface{}) error {
// Application Load Balancers, so the logic below is simple individual checks.
// If other differences arise we'll want to refactor to check other
// conditions in combinations, but for now all we handle is subnets
if lbType := diff.Get("load_balancer_type").(string); lbType != "network" {
if lbType := diff.Get("load_balancer_type").(string); lbType != elbv2.LoadBalancerTypeEnumNetwork {
return nil
}

Expand Down
Loading

0 comments on commit 91d4c37

Please sign in to comment.