Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor aws_lb resource and data source to use keyvaluetags + enums #11419

Merged
merged 3 commits into from
Jan 2, 2020
Merged

Refactor aws_lb resource and data source to use keyvaluetags + enums #11419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
028382e
refactor to use keyvaluetags
DrFaust92 Dec 24, 2019
db03cb1
refactor test
DrFaust92 Dec 24, 2019
4e81232
fix
DrFaust92 Dec 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 52 additions & 49 deletions aws/data_source_aws_lb_test.go
View file
Open in desktop
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
Open in desktop
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