r/aws_apigateway_stage: add deployment_id to canary_settings

.changelog/39929.txt

@@ -0,0 +1,11 @@
+```release-note:breaking-change
+resource/aws_api_gateway_stage: Add `canary_settings.deployment_id` attribute as `required`
+```
+
+```release-note:bug
+resource/aws_api_gateway_deployment: Fix destroy error when canary stage still exists on resource
+```
+
+```release-note:note
+resource/aws_api_gateway_stage: `deployment_id` was added to `canary_settings` as a `required` attribute. This breaking change was necessary to make `canary_settings` functional. Without this change all canary traffic was routed to the main deployment
+```

internal/service/apigateway/deployment.go

@@ -47,9 +47,11 @@ func resourceDeployment() *schema.Resource {
 				Optional: true,
 			},
 			"canary_settings": {
-				Type:     schema.TypeList,
-				Optional: true,
-				MaxItems: 1,
+				Type:         schema.TypeList,
+				Optional:     true,
+				ForceNew:     true,
+				MaxItems:     1,
+				RequiredWith: []string{"stage_name"},
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"percent_traffic": {
@@ -83,9 +85,10 @@ func resourceDeployment() *schema.Resource {
 				ForceNew: true,
 			},
 			"stage_description": {
-				Type:     schema.TypeString,
-				Optional: true,
-				ForceNew: true,
+				Type:         schema.TypeString,
+				Optional:     true,
+				ForceNew:     true,
+				RequiredWith: []string{"stage_name"},
 			},
 			"stage_name": {
 				Type:     schema.TypeString,
@@ -120,16 +123,17 @@ func resourceDeploymentCreate(ctx context.Context, d *schema.ResourceData, meta
 		Variables:        flex.ExpandStringValueMap(d.Get("variables").(map[string]interface{})),
 	}
 
+	if v, ok := d.GetOk("canary_settings"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
+		input.CanarySettings = expandDeploymentCanarySettings(v.([]interface{})[0].(map[string]interface{}))
+	}
+
 	deployment, err := conn.CreateDeployment(ctx, input)
 
 	if err != nil {
 		return sdkdiag.AppendErrorf(diags, "creating API Gateway Deployment: %s", err)
 	}
 
 	d.SetId(aws.ToString(deployment.Id))
-	if v, ok := d.GetOk("canary_settings"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
-		input.CanarySettings = expandDeploymentCanarySettings(v.([]interface{})[0].(map[string]interface{}))
-	}
 
 	return append(diags, resourceDeploymentRead(ctx, d, meta)...)
 }
@@ -236,6 +240,22 @@ func resourceDeploymentDelete(ctx context.Context, d *schema.ResourceData, meta
 		RestApiId:    aws.String(restAPIID),
 	})
 
+	if errs.IsAErrorMessageContains[*types.BadRequestException](err, "Active stages with canary settings pointing to this deployment must be moved or deleted") {
+		_, err = conn.DeleteStage(ctx, &apigateway.DeleteStageInput{
+			StageName: aws.String(stageName),
+			RestApiId: aws.String(restAPIID),
+		})
+
+		if err != nil {
+			return sdkdiag.AppendErrorf(diags, "deleting API Gateway Stage (%s): %s", stageName, err)
+		}
+
+		_, err = conn.DeleteDeployment(ctx, &apigateway.DeleteDeploymentInput{
+			DeploymentId: aws.String(d.Id()),
+			RestApiId:    aws.String(restAPIID),
+		})
+	}
+
 	if errs.IsA[*types.NotFoundException](err) {
 		return diags
 	}

internal/service/apigateway/deployment_test.go

@@ -337,23 +337,36 @@ func TestAccAPIGatewayDeployment_deploymentCanarySettings(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	url := "https://example.com"
 	resourceName := "aws_api_gateway_deployment.test"
+	resourceNameDeploymentMain := "aws_api_gateway_deployment.test_main"
+	resourceNameStage := "aws_api_gateway_stage.test"
 
-	resource.Test(t, resource.TestCase{
+	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:                 func() { acctest.PreCheck(ctx, t); acctest.PreCheckAPIGatewayTypeEDGE(t) },
 		ErrorCheck:               acctest.ErrorCheck(t, names.APIGatewayServiceID),
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
 		CheckDestroy:             testAccCheckDeploymentDestroy(ctx),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccStageConfig_deploymentCanarySettings(rName, url),
+				Config: testAccDeploymentConfig_canarySettings(rName, url),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckDeploymentExists(ctx, resourceName, &deployment),
+					resource.TestCheckResourceAttrPair(resourceNameStage, "deployment_id", resourceNameDeploymentMain, names.AttrID),
 					resource.TestCheckResourceAttr(resourceName, "variables.one", "1"),
 					resource.TestCheckResourceAttr(resourceName, "canary_settings.0.percent_traffic", "33.33"),
 					resource.TestCheckResourceAttr(resourceName, "canary_settings.0.stage_variable_overrides.one", "3"),
 					resource.TestCheckResourceAttr(resourceName, "canary_settings.0.use_stage_cache", acctest.CtTrue),
 				),
 			},
+			{
+				RefreshState: true,
+				Check: resource.ComposeTestCheckFunc(
+					// check that canary settings on the deployment match what is on the stage
+					resource.TestCheckResourceAttrPair(resourceNameStage, "deployment_id", resourceNameDeploymentMain, names.AttrID),
+					resource.TestCheckResourceAttrPair(resourceName, "canary_settings.0.percent_traffic", resourceNameStage, "canary_settings.0.percent_traffic"),
+					resource.TestCheckResourceAttrPair(resourceName, "canary_settings.0.stage_variable_overrides.one", resourceNameStage, "canary_settings.0.stage_variable_overrides.one"),
+					resource.TestCheckResourceAttrPair(resourceName, "canary_settings.0.use_stage_cache", resourceNameStage, "canary_settings.0.use_stage_cache"),
+				),
+			},
 		},
 	})
 }
@@ -610,12 +623,30 @@ resource "aws_lambda_function" "test" {
 `, rName))
 }
 
-func testAccStageConfig_deploymentCanarySettings(rName, url string) string {
+func testAccDeploymentConfig_canarySettings(rName, url string) string {
 	return acctest.ConfigCompose(testAccDeploymentConfig_base(rName, url), `
+resource "aws_api_gateway_stage" "test" {
+  rest_api_id   = aws_api_gateway_rest_api.test.id
+  stage_name    = "prod"
+  deployment_id = aws_api_gateway_deployment.test_main.id
+
+  lifecycle {
+    ignore_changes = [variables, canary_settings]
+  }
+}
+
+resource "aws_api_gateway_deployment" "test_main" {
+  depends_on = [aws_api_gateway_integration.test]
+
+  description = "test-api-gateway"
+  rest_api_id = aws_api_gateway_rest_api.test.id
+}
+
 resource "aws_api_gateway_deployment" "test" {
   depends_on = [aws_api_gateway_integration.test]
 
   rest_api_id = aws_api_gateway_rest_api.test.id
+  stage_name  = aws_api_gateway_stage.test.stage_name
   canary_settings {
     percent_traffic = "33.33"
     stage_variable_overrides = {

internal/service/apigateway/stage.go

@@ -91,6 +91,10 @@ func resourceStage() *schema.Resource {
 				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
+						"deployment_id": {
+							Type:     schema.TypeString,
+							Required: true,
+						},
 						"percent_traffic": {
 							Type:     schema.TypeFloat,
 							Optional: true,
@@ -189,7 +193,7 @@ func resourceStageCreate(ctx context.Context, d *schema.ResourceData, meta inter
 	}
 
 	if v, ok := d.GetOk("canary_settings"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
-		input.CanarySettings = expandCanarySettings(v.([]interface{})[0].(map[string]interface{}), deploymentID)
+		input.CanarySettings = expandCanarySettings(v.([]interface{})[0].(map[string]interface{}))
 	}
 
 	if v, ok := d.GetOk(names.AttrDescription); ok {
@@ -332,14 +336,6 @@ func resourceStageUpdate(ctx context.Context, d *schema.ResourceData, meta inter
 				Path:  aws.String("/deploymentId"),
 				Value: aws.String(d.Get("deployment_id").(string)),
 			})
-
-			if _, ok := d.GetOk("canary_settings"); ok {
-				operations = append(operations, types.PatchOperation{
-					Op:    types.OpReplace,
-					Path:  aws.String("/canarySettings/deploymentId"),
-					Value: aws.String(d.Get("deployment_id").(string)),
-				})
-			}
 		}
 		if d.HasChange(names.AttrDescription) {
 			operations = append(operations, types.PatchOperation{
@@ -561,13 +557,13 @@ func flattenAccessLogSettings(accessLogSettings *types.AccessLogSettings) []map[
 	return result
 }
 
-func expandCanarySettings(tfMap map[string]interface{}, deploymentId string) *types.CanarySettings {
+func expandCanarySettings(tfMap map[string]interface{}) *types.CanarySettings {
 	if tfMap == nil {
 		return nil
 	}
 
 	apiObject := &types.CanarySettings{
-		DeploymentId: aws.String(deploymentId),
+		DeploymentId: aws.String(tfMap["deployment_id"].(string)),
 	}
 
 	if v, ok := tfMap["percent_traffic"].(float64); ok {
@@ -600,6 +596,7 @@ func flattenCanarySettings(canarySettings *types.CanarySettings) []interface{} {
 
 	settings["percent_traffic"] = canarySettings.PercentTraffic
 	settings["use_stage_cache"] = canarySettings.UseStageCache
+	settings["deployment_id"] = canarySettings.DeploymentId
 
 	return []interface{}{settings}
 }
@@ -621,6 +618,7 @@ func appendCanarySettingsPatchOperations(operations []types.PatchOperation, oldC
 			"percent_traffic":          0.0,
 			"stage_variable_overrides": make(map[string]interface{}),
 			"use_stage_cache":          false,
+			"deployment_id":            "",
 		}
 	}
 
@@ -648,6 +646,14 @@ func appendCanarySettingsPatchOperations(operations []types.PatchOperation, oldC
 		})
 	}
 
+	oldDeploymentID, newDeploymentID := oldSettings["deployment_id"].(string), newSettings["deployment_id"].(string)
+	if oldDeploymentID != newDeploymentID {
+		operations = append(operations, types.PatchOperation{
+			Op:    types.OpReplace,
+			Path:  aws.String("/canarySettings/deploymentId"),
+			Value: aws.String(newDeploymentID),
+		})
+	}
 	return operations
 }