fix: allow to add subnets/-mappings to a Network LoadBalancer

.changelog/33205.txt

@@ -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
+```

internal/service/elbv2/load_balancer.go

@@ -51,7 +51,9 @@ func ResourceLoadBalancer() *schema.Resource {
 		},
 
 		CustomizeDiff: customdiff.Sequence(
+			customizeDiffALB,
 			customizeDiffNLB,
+			customizeDiffGWLB,
 			verify.SetTagsDiff,
 		),
 
@@ -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": {
@@ -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(),
@@ -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)
@@ -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 {
@@ -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
 	//
@@ -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
 }