b/aws_redshiftserverless_workgroup: Fix base_capacity, max_capacity ordered updates and max_capacity limit removal

.changelog/36032.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_redshiftserverless_workgroup: Fix updating both `base_capacity` and `max_capacity`
+```
+
+```release-note:bug
+resource/aws_redshiftserverless_workgroup: Fix `max_capacity` removal
+```

internal/service/redshiftserverless/workgroup.go

@@ -157,7 +157,6 @@ func ResourceWorkgroup() *schema.Resource {
 			"max_capacity": {
 				Type:     schema.TypeInt,
 				Optional: true,
-				Computed: true,
 			},
 			"namespace_name": {
 				Type:     schema.TypeString,
@@ -306,15 +305,72 @@ func resourceWorkgroupUpdate(ctx context.Context, d *schema.ResourceData, meta i
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).RedshiftServerlessConn(ctx)
 
-	// You can't update multiple parameters in one request.
-
-	if d.HasChange("base_capacity") {
-		input := &redshiftserverless.UpdateWorkgroupInput{
-			BaseCapacity:  aws.Int64(int64(d.Get("base_capacity").(int))),
-			WorkgroupName: aws.String(d.Id()),
+	checkCapacityChange := func(key string) (bool, int, int) {
+		o, n := d.GetChange(key)
+		oldCapacity, newCapacity := o.(int), n.(int)
+		hasCapacityChange := newCapacity != oldCapacity
+		return hasCapacityChange, oldCapacity, newCapacity
+	}
+
+	// You can't update multiple workgroup parameters in one request.
+	// This is particularly important when adjusting base_capacity and max_capacity due to their interdependencies:
+	// - base_capacity cannot be increased to a value greater than the current max_capacity.
+	// - max_capacity cannot be decreased to a value smaller than the current base_capacity.
+	// The value 0 of max_capacity in the state signifies "not set".
+	// Sending max_capacity value of -1 to AWS API removes max_capacity limit, but -1 cannot be used as max_capacity in the state,
+	// because AWS API never returns -1 as the value of unset max_capacity. There would be a diff on subsequent apply,
+	// resulting in errors due to the lack of AWS API idempotency.
+	// Some validations, such as increasing base_capacity beyond an unchanged max_capacity, are deferred to the AWS API.
+
+	hasBaseCapacityChange, _, newBaseCapacity := checkCapacityChange("base_capacity")
+	hasMaxCapacityChange, oldMaxCapacity, newMaxCapacity := checkCapacityChange("max_capacity")
+
+	switch {
+	case hasMaxCapacityChange && newMaxCapacity == 0:
+		if err :=
+			updateWorkgroup(ctx, conn,
+				&redshiftserverless.UpdateWorkgroupInput{MaxCapacity: aws.Int64(-1), WorkgroupName: aws.String(d.Id())},
+				d.Timeout(schema.TimeoutUpdate)); err != nil {
+			return sdkdiag.AppendFromErr(diags, err)
 		}
-
-		if err := updateWorkgroup(ctx, conn, input, d.Timeout(schema.TimeoutUpdate)); err != nil {
+	case hasBaseCapacityChange && hasMaxCapacityChange && (oldMaxCapacity == 0 || newBaseCapacity <= oldMaxCapacity):
+		if err :=
+			updateWorkgroup(ctx, conn,
+				&redshiftserverless.UpdateWorkgroupInput{BaseCapacity: aws.Int64(int64(newBaseCapacity)), WorkgroupName: aws.String(d.Id())},
+				d.Timeout(schema.TimeoutUpdate)); err != nil {
+			return sdkdiag.AppendFromErr(diags, err)
+		}
+		if err :=
+			updateWorkgroup(ctx, conn,
+				&redshiftserverless.UpdateWorkgroupInput{MaxCapacity: aws.Int64(int64(newMaxCapacity)), WorkgroupName: aws.String(d.Id())},
+				d.Timeout(schema.TimeoutUpdate)); err != nil {
+			return sdkdiag.AppendFromErr(diags, err)
+		}
+	case hasBaseCapacityChange && hasMaxCapacityChange && newBaseCapacity > oldMaxCapacity:
+		if err :=
+			updateWorkgroup(ctx, conn,
+				&redshiftserverless.UpdateWorkgroupInput{MaxCapacity: aws.Int64(int64(newMaxCapacity)), WorkgroupName: aws.String(d.Id())},
+				d.Timeout(schema.TimeoutUpdate)); err != nil {
+			return sdkdiag.AppendFromErr(diags, err)
+		}
+		if err :=
+			updateWorkgroup(ctx, conn,
+				&redshiftserverless.UpdateWorkgroupInput{BaseCapacity: aws.Int64(int64(newBaseCapacity)), WorkgroupName: aws.String(d.Id())},
+				d.Timeout(schema.TimeoutUpdate)); err != nil {
+			return sdkdiag.AppendFromErr(diags, err)
+		}
+	case hasBaseCapacityChange:
+		if err :=
+			updateWorkgroup(ctx, conn,
+				&redshiftserverless.UpdateWorkgroupInput{BaseCapacity: aws.Int64(int64(newBaseCapacity)), WorkgroupName: aws.String(d.Id())},
+				d.Timeout(schema.TimeoutUpdate)); err != nil {
+			return sdkdiag.AppendFromErr(diags, err)
+		}
+	case hasMaxCapacityChange:
+		if err :=
+			updateWorkgroup(ctx, conn,
+				&redshiftserverless.UpdateWorkgroupInput{MaxCapacity: aws.Int64(int64(newMaxCapacity)), WorkgroupName: aws.String(d.Id())},
+				d.Timeout(schema.TimeoutUpdate)); err != nil {
 			return sdkdiag.AppendFromErr(diags, err)
 		}
 	}
@@ -341,17 +397,6 @@ func resourceWorkgroupUpdate(ctx context.Context, d *schema.ResourceData, meta i
 		}
 	}
 
-	if d.HasChange("max_capacity") {
-		input := &redshiftserverless.UpdateWorkgroupInput{
-			MaxCapacity:   aws.Int64(int64(d.Get("max_capacity").(int))),
-			WorkgroupName: aws.String(d.Id()),
-		}
-
-		if err := updateWorkgroup(ctx, conn, input, d.Timeout(schema.TimeoutUpdate)); err != nil {
-			return sdkdiag.AppendFromErr(diags, err)
-		}
-	}
-
 	if d.HasChange("port") {
 		input := &redshiftserverless.UpdateWorkgroupInput{
 			Port:          aws.Int64(int64(d.Get("port").(int))),

internal/service/redshiftserverless/workgroup_test.go

@@ -50,6 +50,8 @@ func TestAccRedshiftServerlessWorkgroup_basic(t *testing.T) {
 	})
 }
 
+// Tests the complex logic involved in updating 'base_capacity' and 'max_capacity'.
+// The order of updates is crucial and is determined by their current state values.
 func TestAccRedshiftServerlessWorkgroup_baseAndMaxCapacityAndPubliclyAccessible(t *testing.T) {
 	ctx := acctest.Context(t)
 	resourceName := "aws_redshiftserverless_workgroup.test"
@@ -62,28 +64,57 @@ func TestAccRedshiftServerlessWorkgroup_baseAndMaxCapacityAndPubliclyAccessible(
 		CheckDestroy:             testAccCheckWorkgroupDestroy(ctx),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 64, 128, true),
+				Config: testAccWorkgroupConfig_baseCapacity(rName, 128),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckWorkgroupExists(ctx, resourceName),
+					resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+					resource.TestCheckResourceAttr(resourceName, "max_capacity", "0"),
+				),
+			},
+			{
+				Config: testAccWorkgroupConfig_baseCapacity(rName, 256),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "base_capacity", "256"),
+					resource.TestCheckResourceAttr(resourceName, "max_capacity", "0"),
+				),
+			},
+			{
+				Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 64, 128, false),
+				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "base_capacity", "64"),
 					resource.TestCheckResourceAttr(resourceName, "max_capacity", "128"),
-					resource.TestCheckResourceAttr(resourceName, "publicly_accessible", "true"),
 				),
 			},
 			{
-				ResourceName:      resourceName,
-				ImportState:       true,
-				ImportStateVerify: true,
+				Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 128, 256, false),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+					resource.TestCheckResourceAttr(resourceName, "max_capacity", "256"),
+				),
 			},
 			{
-				Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 128, 5632, false),
+				Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 512, 5632, false),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckWorkgroupExists(ctx, resourceName),
-					resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+					resource.TestCheckResourceAttr(resourceName, "base_capacity", "512"),
 					resource.TestCheckResourceAttr(resourceName, "max_capacity", "5632"),
 					resource.TestCheckResourceAttr(resourceName, "publicly_accessible", "false"),
 				),
 			},
+			{
+				Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 128, 256, true),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+					resource.TestCheckResourceAttr(resourceName, "max_capacity", "256"),
+					resource.TestCheckResourceAttr(resourceName, "publicly_accessible", "true"),
+				),
+			},
+			{
+				Config: testAccWorkgroupConfig_baseCapacity(rName, 128),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+					resource.TestCheckResourceAttr(resourceName, "max_capacity", "0"),
+				),
+			},
 		},
 	})
 }
@@ -361,6 +392,20 @@ resource "aws_redshiftserverless_workgroup" "test" {
 `, rName, baseCapacity, maxCapacity, publiclyAccessible)
 }
 
+func testAccWorkgroupConfig_baseCapacity(rName string, baseCapacity int) string {
+	return fmt.Sprintf(`
+resource "aws_redshiftserverless_namespace" "test" {
+  namespace_name = %[1]q
+}
+
+resource "aws_redshiftserverless_workgroup" "test" {
+  namespace_name = aws_redshiftserverless_namespace.test.namespace_name
+  workgroup_name = %[1]q
+  base_capacity  = %[2]d
+}
+`, rName, baseCapacity)
+}
+
 func testAccWorkgroupConfig_configParameters(rName, maxQueryExecutionTime string) string {
 	return fmt.Sprintf(`
 resource "aws_redshiftserverless_namespace" "test" {