Merge pull request #23786 from kevinscholz/fix-recreate-aws_ecs_service

r/aws_ecs_service: fix `load_balancer` & `service_registries` to update without destroy & recreate

Author: ewbankkit

.changelog/23786.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_ecs_service: Ensure that `load_balancer` and `service_registries` can be updated in-place
+```

internal/service/ecs/service.go

@@ -182,31 +182,24 @@ func ResourceService() *schema.Resource {
 				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
-						"elb_name": {
-							Type:     schema.TypeString,
-							Optional: true,
-							ForceNew: true,
-						},
-
-						"target_group_arn": {
-							Type:         schema.TypeString,
-							Optional:     true,
-							ForceNew:     true,
-							ValidateFunc: verify.ValidARN,
-						},
-
 						"container_name": {
 							Type:     schema.TypeString,
 							Required: true,
-							ForceNew: true,
 						},
-
 						"container_port": {
 							Type:         schema.TypeInt,
 							Required:     true,
-							ForceNew:     true,
 							ValidateFunc: validation.IntBetween(0, 65536),
 						},
+						"elb_name": {
+							Type:     schema.TypeString,
+							Optional: true,
+						},
+						"target_group_arn": {
+							Type:         schema.TypeString,
+							Optional:     true,
+							ValidateFunc: verify.ValidARN,
+						},
 					},
 				},
 				Set: resourceLoadBalancerHash,
@@ -222,6 +215,11 @@ func ResourceService() *schema.Resource {
 				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
+						"assign_public_ip": {
+							Type:     schema.TypeBool,
+							Optional: true,
+							Default:  false,
+						},
 						"security_groups": {
 							Type:     schema.TypeSet,
 							Optional: true,
@@ -234,11 +232,6 @@ func ResourceService() *schema.Resource {
 							Elem:     &schema.Schema{Type: schema.TypeString},
 							Set:      schema.HashString,
 						},
-						"assign_public_ip": {
-							Type:     schema.TypeBool,
-							Optional: true,
-							Default:  false,
-						},
 					},
 				},
 			},
@@ -248,11 +241,6 @@ func ResourceService() *schema.Resource {
 				MaxItems: 5,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
-						"type": {
-							Type:         schema.TypeString,
-							Required:     true,
-							ValidateFunc: validation.StringInSlice(ecs.PlacementStrategyType_Values(), false),
-						},
 						"field": {
 							Type:     schema.TypeString,
 							Optional: true,
@@ -267,6 +255,11 @@ func ResourceService() *schema.Resource {
 								return strings.EqualFold(old, new)
 							},
 						},
+						"type": {
+							Type:         schema.TypeString,
+							Required:     true,
+							ValidateFunc: validation.StringInSlice(ecs.PlacementStrategyType_Values(), false),
+						},
 					},
 				},
 			},
@@ -319,24 +312,20 @@ func ResourceService() *schema.Resource {
 					Schema: map[string]*schema.Schema{
 						"container_name": {
 							Type:     schema.TypeString,
-							ForceNew: true,
 							Optional: true,
 						},
 						"container_port": {
 							Type:         schema.TypeInt,
-							ForceNew:     true,
 							Optional:     true,
 							ValidateFunc: validation.IntBetween(0, 65536),
 						},
 						"port": {
 							Type:         schema.TypeInt,
-							ForceNew:     true,
 							Optional:     true,
 							ValidateFunc: validation.IntBetween(0, 65536),
 						},
 						"registry_arn": {
 							Type:         schema.TypeString,
-							ForceNew:     true,
 							Required:     true,
 							ValidateFunc: verify.ValidARN,
 						},
@@ -1081,7 +1070,9 @@ func resourceServiceUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 
 		if d.HasChange("load_balancer") {
-			input.LoadBalancers = expandLoadBalancers(d.Get("load_balancer").([]interface{}))
+			if v, ok := d.Get("load_balancer").(*schema.Set); ok && v != nil {
+				input.LoadBalancers = expandLoadBalancers(v.List())
+			}
 		}
 
 		if d.HasChange("propagate_tags") {

internal/service/ecs/service_test.go

@@ -557,7 +557,7 @@ func TestAccECSService_deploymentCircuitBreaker(t *testing.T) {
 
 // Regression for https://github.com/hashicorp/terraform/issues/3444
 func TestAccECSService_loadBalancerChanges(t *testing.T) {
-	var service ecs.Service
+	var s1, s2 ecs.Service
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_ecs_service.test"
 
@@ -570,13 +570,14 @@ func TestAccECSService_loadBalancerChanges(t *testing.T) {
 			{
 				Config: testAccServiceLBChangesConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckServiceExists(resourceName, &service),
+					testAccCheckServiceExists(resourceName, &s1),
 				),
 			},
 			{
 				Config: testAccServiceLBChangesModifiedConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckServiceExists(resourceName, &service),
+					testAccCheckServiceExists(resourceName, &s2),
+					testAccCheckServiceNotRecreated(&s2, &s1),
 				),
 			},
 		},
@@ -2789,15 +2790,27 @@ resource "aws_iam_role_policy" "ecs_service" {
 EOF
 }
 
-resource "aws_elb" "test" {
+resource "aws_lb_target_group" "test" {
+  name     = aws_lb.test.name
+  port     = %[5]d
+  protocol = "HTTP"
+  vpc_id   = aws_vpc.test.id
+}
+
+resource "aws_lb" "test" {
+  name     = %[1]q
   internal = true
   subnets  = aws_subnet.test[*].id
+}
 
-  listener {
-    instance_port     = %[5]d
-    instance_protocol = "http"
-    lb_port           = 80
-    lb_protocol       = "http"
+resource "aws_lb_listener" "front_end" {
+  load_balancer_arn = aws_lb.test.id
+  port              = "80"
+  protocol          = "HTTP"
+
+  default_action {
+    target_group_arn = aws_lb_target_group.test.id
+    type             = "forward"
   }
 }
 
@@ -2809,9 +2822,9 @@ resource "aws_ecs_service" "test" {
   iam_role        = aws_iam_role.ecs_service.name
 
   load_balancer {
-    elb_name       = aws_elb.test.id
-    container_name = %[3]q
-    container_port = %[4]d
+    target_group_arn = aws_lb_target_group.test.id
+    container_name   = %[3]q
+    container_port   = %[4]d
   }
 
   depends_on = [aws_iam_role_policy.ecs_service]