Merge pull request #32987 from fhke/f-aws_lb-network-lb-security-group

Update `aws_lb` resource to cover edge cases with network load balancer security groups.

.changelog/32987.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/aws_lb: Changes to `security_groups` for Network Load Balancers force a new resource if either the old or new set of security group IDs is empty
+```

internal/service/elbv2/listener_data_source_test.go

@@ -146,7 +146,7 @@ func TestAccELBV2ListenerDataSource_DefaultAction_forward(t *testing.T) {
 }
 
 func testAccListenerDataSourceConfig_basic(rName string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"
@@ -202,7 +202,7 @@ data "aws_lb_listener" "from_lb_and_port" {
 }
 
 func testAccListenerDataSourceConfig_backwardsCompatibility(rName string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_alb_listener" "test" {
   load_balancer_arn = aws_alb.test.id
   protocol          = "HTTP"
@@ -258,7 +258,7 @@ data "aws_alb_listener" "from_lb_and_port" {
 }
 
 func testAccListenerDataSourceConfig_https(rName, certificate, key string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTPS"

internal/service/elbv2/listener_rule_test.go

@@ -605,11 +605,11 @@ func TestAccELBV2ListenerRule_priority(t *testing.T) {
 			},
 			{
 				Config:      testAccListenerRuleConfig_priority50001(rName),
-				ExpectError: regexp.MustCompile(`creating LB Listener Rule: ValidationError`),
+				ExpectError: regexp.MustCompile(`creating ELBv2 Listener Rule: ValidationError`),
 			},
 			{
 				Config:      testAccListenerRuleConfig_priorityInUse(rName),
-				ExpectError: regexp.MustCompile(`creating LB Listener Rule: PriorityInUse`),
+				ExpectError: regexp.MustCompile(`creating ELBv2 Listener Rule: PriorityInUse`),
 			},
 		},
 	})

internal/service/elbv2/listener_test.go

@@ -694,7 +694,7 @@ func testAccCheckListenerDestroy(ctx context.Context) resource.TestCheckFunc {
 	}
 }
 
-func testAccListenerBaseConfig(rName string) string {
+func testAccListenerConfig_base(rName string) string {
 	return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
 resource "aws_vpc" "test" {
   cidr_block = "10.0.0.0/16"
@@ -743,7 +743,7 @@ resource "aws_security_group" "test" {
 }
 
 func testAccListenerConfig_basic(rName string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"
@@ -794,7 +794,7 @@ resource "aws_lb_target_group" "test" {
 }
 
 func testAccListenerConfig_forwardWeighted(rName, rName2 string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"
@@ -878,7 +878,7 @@ resource "aws_lb_target_group" "test2" {
 }
 
 func testAccListenerConfig_changeForwardWeightedStickiness(rName, rName2 string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"
@@ -967,7 +967,7 @@ resource "aws_lb_target_group" "test2" {
 }
 
 func testAccListenerConfig_changeForwardWeightedToBasic(rName, rName2 string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"
@@ -1040,7 +1040,7 @@ resource "aws_lb_target_group" "test2" {
 }
 
 func testAccListenerConfig_basicUdp(rName string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "UDP"
@@ -1093,7 +1093,7 @@ resource "aws_internet_gateway" "test" {
 }
 
 func testAccListenerConfig_backwardsCompatibility(rName string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_alb_listener" "test" {
   load_balancer_arn = aws_alb.test.id
   protocol          = "HTTP"
@@ -1144,7 +1144,7 @@ resource "aws_alb_target_group" "test" {
 }
 
 func testAccListenerConfig_https(rName, key, certificate string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTPS"
@@ -1269,7 +1269,7 @@ resource "aws_lb_listener" "test" {
 }
 
 func testAccListenerConfig_protocolTLS(rName, key, certificate string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_acm_certificate" "test" {
   certificate_body = "%[2]s"
   private_key      = "%[3]s"
@@ -1326,7 +1326,7 @@ resource "aws_lb_listener" "test" {
 }
 
 func testAccListenerConfig_redirect(rName string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"
@@ -1360,7 +1360,7 @@ resource "aws_lb" "test" {
 }
 
 func testAccListenerConfig_fixedResponse(rName string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"
@@ -1394,7 +1394,7 @@ resource "aws_lb" "test" {
 }
 
 func testAccListenerConfig_cognito(rName, key, certificate string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb" "test" {
   name                       = %[1]q
   internal                   = false
@@ -1498,7 +1498,7 @@ resource "aws_lb_listener" "test" {
 }
 
 func testAccListenerConfig_oidc(rName, key, certificate string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb" "test" {
   name                       = %[1]q
   internal                   = false
@@ -1580,7 +1580,7 @@ resource "aws_lb_listener" "test" {
 }
 
 func testAccListenerConfig_defaultActionOrder(rName, key, certificate string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTPS"
@@ -1655,7 +1655,7 @@ resource "aws_lb_target_group" "test" {
 }
 
 func testAccListenerConfig_tags1(rName, tagKey1, tagValue1 string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"
@@ -1710,7 +1710,7 @@ resource "aws_lb_target_group" "test" {
 }
 
 func testAccListenerConfig_tags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string {
-	return acctest.ConfigCompose(testAccListenerBaseConfig(rName), fmt.Sprintf(`
+	return acctest.ConfigCompose(testAccListenerConfig_base(rName), fmt.Sprintf(`
 resource "aws_lb_listener" "test" {
   load_balancer_arn = aws_lb.test.id
   protocol          = "HTTP"

internal/service/elbv2/load_balancer.go

@@ -50,9 +50,8 @@ func ResourceLoadBalancer() *schema.Resource {
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 
-		// Subnets are ForceNew for Network Load Balancers
 		CustomizeDiff: customdiff.Sequence(
-			customizeDiffNLBSubnets,
+			customizeDiffNLB,
 			verify.SetTagsDiff,
 		),
 
@@ -209,9 +208,9 @@ func ResourceLoadBalancer() *schema.Resource {
 			},
 			"security_groups": {
 				Type:     schema.TypeSet,
-				Elem:     &schema.Schema{Type: schema.TypeString},
-				Computed: true,
 				Optional: true,
+				Computed: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},
 			},
 			"subnet_mapping": {
 				Type:     schema.TypeSet,
@@ -1013,14 +1012,17 @@ func flattenResource(ctx context.Context, d *schema.ResourceData, meta interface
 	return nil
 }
 
-// Load balancers of type 'network' cannot have their subnets updated at
-// this time. If the type is 'network' and subnets have changed, mark the
-// diff as a ForceNew operation
-func customizeDiffNLBSubnets(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
+// Load balancers of type 'network' cannot have their subnets updated,
+// cannot have security groups added if none are present, and cannot have
+// all security groups removed. If the type is 'network' and any of these
+// conditions are met, mark the diff as a ForceNew operation.
+func customizeDiffNLB(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
 	// 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
+	//   OR security groups are being added where none currently exist
+	//   OR all security groups are being removed
 	//
 	// Any other combination should be treated as normal. At this time, subnet
 	// handling is the only known difference between Network Load Balancers and
@@ -1035,26 +1037,26 @@ func customizeDiffNLBSubnets(_ context.Context, diff *schema.ResourceDiff, v int
 		return nil
 	}
 
+	// Get diff for subnets.
 	o, n := diff.GetChange("subnets")
-	if o == nil {
-		o = new(schema.Set)
-	}
-	if n == nil {
-		n = new(schema.Set)
-	}
-	os := o.(*schema.Set)
-	ns := n.(*schema.Set)
-	remove := os.Difference(ns).List()
-	add := ns.Difference(os).List()
-	if len(remove) > 0 || len(add) > 0 {
-		if err := diff.SetNew("subnets", n); err != nil {
+	os, ns := o.(*schema.Set), n.(*schema.Set)
+
+	if add, del := ns.Difference(os).List(), os.Difference(ns).List(); len(del) > 0 || len(add) > 0 {
+		if err := diff.ForceNew("subnets"); err != nil {
 			return err
 		}
+	}
 
-		if err := diff.ForceNew("subnets"); err != nil {
+	// Get diff for security groups.
+	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
 }