Skip to content

Commit

Permalink
Merge pull request #33205 from elchead/allow-to-add-subnets-to-lb
Browse files Browse the repository at this point in the history 
fix: allow to add subnets/-mappings to a Network LoadBalancer
  • Loading branch information
@ewbankkit
ewbankkit authored Dec 12, 2023
2 parents d2df929 + fbf19f1 commit 4f230ce
Show file tree
Hide file tree
Showing 4 changed files with 462 additions and 73 deletions.
19 changes: 19 additions & 0 deletions .changelog/33205.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
```release-note:enhancement
resource/aws_lb: Allow the number of `subnets` for Network Load Balancers to be increased without recreating the resource
```

```release-note:enhancement
resource/aws_lb: Add plan-time validation that exactly one of either `subnets` or `subnet_mapping` is configured
```

```release-note:enhancement
resource/aws_lb: Allow the number of `subnet_mapping`s for Network Load Balancers to be increased without recreating the resource
```

```release-note:enhancement
resource/aws_lb: Allow the number of `subnet_mapping`s for Application Load Balancers to be changed without recreating the resource
```

```release-note:bug
resource/aws_lb: Correct in-place update of `security_groups` for Network Load Balancers when the new value is Computed
```
199 changes: 161 additions & 38 deletions internal/service/elbv2/load_balancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ func ResourceLoadBalancer() *schema.Resource {
},

CustomizeDiff: customdiff.Sequence(
customizeDiffALB,
customizeDiffNLB,
customizeDiffGWLB,
verify.SetTagsDiff,
),

Expand Down Expand Up @@ -227,18 +229,15 @@ func ResourceLoadBalancer() *schema.Resource {
Type: schema.TypeSet,
Optional: true,
Computed: true,
ForceNew: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"allocation_id": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
"ipv6_address": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: validation.IsIPv6Address,
},
"outpost_id": {
Expand All @@ -248,22 +247,22 @@ func ResourceLoadBalancer() *schema.Resource {
"private_ipv4_address": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: validation.IsIPv4Address,
},
"subnet_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
},
},
ExactlyOneOf: []string{"subnet_mapping", "subnets"},
},
"subnets": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
Type: schema.TypeSet,
Optional: true,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
ExactlyOneOf: []string{"subnet_mapping", "subnets"},
},
names.AttrTags: tftags.TagsSchema(),
names.AttrTagsAll: tftags.TagsSchemaComputed(),
Expand Down Expand Up @@ -532,10 +531,21 @@ func resourceLoadBalancerUpdate(ctx context.Context, d *schema.ResourceData, met
}
}

if d.HasChange("subnets") {
if d.HasChanges("subnet_mapping", "subnets") {
input := &elbv2.SetSubnetsInput{
LoadBalancerArn: aws.String(d.Id()),
Subnets: flex.ExpandStringSet(d.Get("subnets").(*schema.Set)),
}

if d.HasChange("subnet_mapping") {
if v, ok := d.GetOk("subnet_mapping"); ok && v.(*schema.Set).Len() > 0 {
input.SubnetMappings = expandSubnetMappings(v.(*schema.Set).List())
}
}

if d.HasChange("subnets") {
if v, ok := d.GetOk("subnets"); ok {
input.Subnets = flex.ExpandStringSet(v.(*schema.Set))
}
}

_, err := conn.SetSubnetsWithContext(ctx, input)
Expand Down Expand Up @@ -953,28 +963,27 @@ func loadBalancerNameFromARN(s string) (string, error) {
return matches[1], nil
}

func flattenSubnetsFromAvailabilityZones(availabilityZones []*elbv2.AvailabilityZone) []string {
return tfslices.ApplyToAll(availabilityZones, func(v *elbv2.AvailabilityZone) string {
return aws.StringValue(v.SubnetId)
func flattenSubnetsFromAvailabilityZones(apiObjects []*elbv2.AvailabilityZone) []string {
return tfslices.ApplyToAll(apiObjects, func(apiObject *elbv2.AvailabilityZone) string {
return aws.StringValue(apiObject.SubnetId)
})
}

func flattenSubnetMappingsFromAvailabilityZones(availabilityZones []*elbv2.AvailabilityZone) []map[string]interface{} {
l := make([]map[string]interface{}, 0)
for _, availabilityZone := range availabilityZones {
m := make(map[string]interface{})
m["subnet_id"] = aws.StringValue(availabilityZone.SubnetId)
m["outpost_id"] = aws.StringValue(availabilityZone.OutpostId)

for _, loadBalancerAddress := range availabilityZone.LoadBalancerAddresses {
m["allocation_id"] = aws.StringValue(loadBalancerAddress.AllocationId)
m["private_ipv4_address"] = aws.StringValue(loadBalancerAddress.PrivateIPv4Address)
m["ipv6_address"] = aws.StringValue(loadBalancerAddress.IPv6Address)
func flattenSubnetMappingsFromAvailabilityZones(apiObjects []*elbv2.AvailabilityZone) []map[string]interface{} {
return tfslices.ApplyToAll(apiObjects, func(apiObject *elbv2.AvailabilityZone) map[string]interface{} {
tfMap := map[string]interface{}{
"outpost_id": aws.StringValue(apiObject.OutpostId),
"subnet_id": aws.StringValue(apiObject.SubnetId),
}
if apiObjects := apiObject.LoadBalancerAddresses; len(apiObjects) > 0 {
apiObject := apiObjects[0]
tfMap["allocation_id"] = aws.StringValue(apiObject.AllocationId)
tfMap["ipv6_address"] = aws.StringValue(apiObject.IPv6Address)
tfMap["private_ipv4_address"] = aws.StringValue(apiObject.PrivateIPv4Address)
}

l = append(l, m)
}
return l
return tfMap
})
}

func SuffixFromARN(arn *string) string {
Expand All @@ -999,7 +1008,7 @@ func customizeDiffNLB(_ context.Context, diff *schema.ResourceDiff, v interface{
// The current criteria for determining if the operation should be ForceNew:
// - lb of type "network"
// - existing resource (id is not "")
// - there are actual changes to be made in the subnets
// - there are subnet removals
// OR security groups are being added where none currently exist
// OR all security groups are being removed
//
Expand All @@ -1016,25 +1025,139 @@ func customizeDiffNLB(_ context.Context, diff *schema.ResourceDiff, v interface{
return nil
}

// Get diff for subnets.
o, n := diff.GetChange("subnets")
os, ns := o.(*schema.Set), n.(*schema.Set)
config := diff.GetRawConfig()

// Subnet diffs.
// Check for changes here -- SetNewComputed will modify HasChange.
hasSubnetMappingChanges, hasSubnetsChanges := diff.HasChange("subnet_mapping"), diff.HasChange("subnets")
if hasSubnetMappingChanges {
if v := config.GetAttr("subnet_mapping"); v.IsWhollyKnown() {
o, n := diff.GetChange("subnet_mapping")
os, ns := o.(*schema.Set), n.(*schema.Set)

deltaN := ns.Len() - os.Len()
switch {
case deltaN == 0:
// No change in number of subnet mappings, but one of the mappings did change.
fallthrough
case deltaN < 0:
// Subnet mappings removed.
if err := diff.ForceNew("subnet_mapping"); err != nil {
return err
}
case deltaN > 0:
// Subnet mappings added. Ensure that the previous mappings didn't change.
if ns.Intersection(os).Len() != os.Len() {
if err := diff.ForceNew("subnet_mapping"); err != nil {
return err
}
}
}
}

if add, del := ns.Difference(os).List(), os.Difference(ns).List(); len(del) > 0 || len(add) > 0 {
if err := diff.ForceNew("subnets"); err != nil {
if err := diff.SetNewComputed("subnets"); err != nil {
return err
}
}
if hasSubnetsChanges {
if v := config.GetAttr("subnets"); v.IsWhollyKnown() {
o, n := diff.GetChange("subnets")
os, ns := o.(*schema.Set), n.(*schema.Set)

// In-place increase in number of subnets only.
if deltaN := ns.Len() - os.Len(); deltaN <= 0 {
if err := diff.ForceNew("subnets"); err != nil {
return err
}
}
}

if err := diff.SetNewComputed("subnet_mapping"); err != nil {
return err
}
}

// Get diff for security groups.
o, n = diff.GetChange("security_groups")
os, ns = o.(*schema.Set), n.(*schema.Set)
if diff.HasChange("security_groups") {
if v := config.GetAttr("security_groups"); v.IsWhollyKnown() {
o, n := diff.GetChange("security_groups")
os, ns := o.(*schema.Set), n.(*schema.Set)

if (os.Len() == 0 && ns.Len() > 0) || (ns.Len() == 0 && os.Len() > 0) {
if err := diff.ForceNew("security_groups"); err != nil {
return err
}
}
}
}

return nil
}

func customizeDiffALB(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
if lbType := diff.Get("load_balancer_type").(string); lbType != elbv2.LoadBalancerTypeEnumApplication {
return nil
}

if diff.Id() == "" {
return nil
}

config := diff.GetRawConfig()

// Subnet diffs.
// Check for changes here -- SetNewComputed will modify HasChange.
hasSubnetMappingChanges, hasSubnetsChanges := diff.HasChange("subnet_mapping"), diff.HasChange("subnets")
if hasSubnetMappingChanges {
if v := config.GetAttr("subnet_mapping"); v.IsWhollyKnown() {
o, n := diff.GetChange("subnet_mapping")
os, ns := o.(*schema.Set), n.(*schema.Set)

deltaN := ns.Len() - os.Len()
switch {
case deltaN == 0:
// No change in number of subnet mappings, but one of the mappings did change.
if err := diff.ForceNew("subnet_mapping"); err != nil {
return err
}
case deltaN < 0:
// Subnet mappings removed. Ensure that the remaining mappings didn't change.
if os.Intersection(ns).Len() != ns.Len() {
if err := diff.ForceNew("subnet_mapping"); err != nil {
return err
}
}
case deltaN > 0:
// Subnet mappings added. Ensure that the previous mappings didn't change.
if ns.Intersection(os).Len() != os.Len() {
if err := diff.ForceNew("subnet_mapping"); err != nil {
return err
}
}
}
}

if (os.Len() == 0 && ns.Len() > 0) || (ns.Len() == 0 && os.Len() > 0) {
if err := diff.ForceNew("security_groups"); err != nil {
if err := diff.SetNewComputed("subnets"); err != nil {
return err
}
}
if hasSubnetsChanges {
if err := diff.SetNewComputed("subnet_mapping"); err != nil {
return err
}
}

return nil
}

func customizeDiffGWLB(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
if lbType := diff.Get("load_balancer_type").(string); lbType != elbv2.LoadBalancerTypeEnumGateway {
return nil
}

if diff.Id() == "" {
return nil
}

return nil
}
Expand Down
Loading

0 comments on commit 4f230ce

Please sign in to comment.