diff --git a/.changelog/39929.txt b/.changelog/39929.txt new file mode 100644 index 00000000000..8dccd5bbe29 --- /dev/null +++ b/.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 +``` \ No newline at end of file diff --git a/internal/service/apigateway/deployment.go b/internal/service/apigateway/deployment.go index ea810b29bfb..086d37a8fa2 100644 --- a/internal/service/apigateway/deployment.go +++ b/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,6 +123,10 @@ 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 { @@ -127,9 +134,6 @@ func resourceDeploymentCreate(ctx context.Context, d *schema.ResourceData, meta } 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 } diff --git a/internal/service/apigateway/deployment_test.go b/internal/service/apigateway/deployment_test.go index 2bc0108025a..02d252bf0b1 100644 --- a/internal/service/apigateway/deployment_test.go +++ b/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 = { diff --git a/internal/service/apigateway/stage.go b/internal/service/apigateway/stage.go index 07d1304a4df..0b2c19ce41f 100644 --- a/internal/service/apigateway/stage.go +++ b/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 } diff --git a/internal/service/apigateway/stage_test.go b/internal/service/apigateway/stage_test.go index e87ec2ca8d9..906af2e8daa 100644 --- a/internal/service/apigateway/stage_test.go +++ b/internal/service/apigateway/stage_test.go @@ -423,6 +423,8 @@ func testAccStage_canarySettings(t *testing.T) { var conf apigateway.GetStageOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_stage.test" + deployment2 := "aws_api_gateway_deployment.test2" + deployment3 := "aws_api_gateway_deployment.test3" resource.Test(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t); acctest.PreCheckAPIGatewayTypeEDGE(t) }, @@ -438,6 +440,7 @@ func testAccStage_canarySettings(t *testing.T) { 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), + resource.TestCheckResourceAttrPair(resourceName, "canary_settings.0.deployment_id", deployment2, names.AttrID), ), }, { @@ -447,7 +450,7 @@ func testAccStage_canarySettings(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccStageConfig_basic(rName), + Config: testAccStageConfig_canarySettingsRemoved(rName), Check: resource.ComposeTestCheckFunc( testAccCheckStageExists(ctx, resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "canary_settings.#", "0"), @@ -459,8 +462,30 @@ func testAccStage_canarySettings(t *testing.T) { testAccCheckStageExists(ctx, resourceName, &conf), resource.TestCheckResourceAttr(resourceName, "variables.one", "1"), resource.TestCheckResourceAttr(resourceName, "canary_settings.0.percent_traffic", "66.66"), - resource.TestCheckResourceAttr(resourceName, "canary_settings.0.stage_variable_overrides.four", "5"), + resource.TestCheckResourceAttr(resourceName, "canary_settings.0.stage_variable_overrides.one", "5"), + resource.TestCheckResourceAttr(resourceName, "canary_settings.0.use_stage_cache", acctest.CtFalse), + resource.TestCheckResourceAttrPair(resourceName, "canary_settings.0.deployment_id", deployment2, names.AttrID), + ), + }, + { + Config: testAccStageConfig_canarySettingsNewDeployment(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckStageExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "variables.one", "1"), + resource.TestCheckResourceAttr(resourceName, "canary_settings.0.percent_traffic", "66.66"), + resource.TestCheckResourceAttr(resourceName, "canary_settings.0.stage_variable_overrides.one", "5"), resource.TestCheckResourceAttr(resourceName, "canary_settings.0.use_stage_cache", acctest.CtFalse), + resource.TestCheckResourceAttrPair(resourceName, "canary_settings.0.deployment_id", deployment3, names.AttrID), + ), + }, + { + Config: testAccStageConfig_canarySettingsPromote(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckStageExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttrPair(resourceName, "deployment_id", deployment3, names.AttrID), + resource.TestCheckResourceAttr(resourceName, "canary_settings.#", "0"), + resource.TestCheckResourceAttr(resourceName, "variables.%", "2"), + resource.TestCheckResourceAttr(resourceName, "variables.one", "5"), ), }, }, @@ -718,13 +743,25 @@ resource "aws_wafregional_web_acl_association" "test" { } func testAccStageConfig_canarySettings(rName string) string { - return acctest.ConfigCompose(testAccStageConfig_base(rName), ` + return acctest.ConfigCompose(testAccStageConfig_base(rName), fmt.Sprintf(` +resource "aws_api_gateway_deployment" "test2" { + depends_on = [aws_api_gateway_integration.test] + + rest_api_id = aws_api_gateway_rest_api.test.id + description = %[1]q + + variables = { + "a" = "2" + } +} + 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.id canary_settings { + deployment_id = aws_api_gateway_deployment.test2.id percent_traffic = "33.33" stage_variable_overrides = { one = "3" @@ -736,20 +773,58 @@ resource "aws_api_gateway_stage" "test" { two = "2" } } -`) +`, rName)) +} + +func testAccStageConfig_canarySettingsRemoved(rName string) string { + return acctest.ConfigCompose(testAccStageConfig_base(rName), fmt.Sprintf(` +resource "aws_api_gateway_deployment" "test2" { + depends_on = [aws_api_gateway_integration.test] + + rest_api_id = aws_api_gateway_rest_api.test.id + description = %[1]q + + variables = { + "a" = "2" + } +} + +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.id + + variables = { + one = "1" + two = "2" + } +} +`, rName)) } func testAccStageConfig_canarySettingsUpdated(rName string) string { - return acctest.ConfigCompose(testAccStageConfig_base(rName), ` + return acctest.ConfigCompose(testAccStageConfig_base(rName), fmt.Sprintf(` +resource "aws_api_gateway_deployment" "test2" { + depends_on = [aws_api_gateway_integration.test] + + rest_api_id = aws_api_gateway_rest_api.test.id + description = %[1]q + + variables = { + "a" = "2" + } +} + 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.id canary_settings { + deployment_id = aws_api_gateway_deployment.test2.id percent_traffic = "66.66" stage_variable_overrides = { - four = "5" + one = "5" } use_stage_cache = "false" } @@ -758,5 +833,76 @@ resource "aws_api_gateway_stage" "test" { two = "2" } } -`) +`, rName)) +} + +func testAccStageConfig_canarySettingsNewDeployment(rName string) string { + return acctest.ConfigCompose(testAccStageConfig_base(rName), fmt.Sprintf(` +resource "aws_api_gateway_deployment" "test2" { + depends_on = [aws_api_gateway_integration.test] + + rest_api_id = aws_api_gateway_rest_api.test.id + description = %[1]q + + variables = { + "a" = "2" + } +} + +resource "aws_api_gateway_deployment" "test3" { + depends_on = [aws_api_gateway_integration.test] + + rest_api_id = aws_api_gateway_rest_api.test.id + description = %[1]q + + variables = { + "a" = "2" + } +} + +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.id + + canary_settings { + percent_traffic = "66.66" + stage_variable_overrides = { + one = "5" + } + use_stage_cache = "false" + deployment_id = aws_api_gateway_deployment.test3.id + } + variables = { + one = "1" + two = "2" + } +} +`, rName)) +} + +func testAccStageConfig_canarySettingsPromote(rName string) string { + return acctest.ConfigCompose(testAccStageConfig_base(rName), fmt.Sprintf(` +resource "aws_api_gateway_deployment" "test3" { + depends_on = [aws_api_gateway_integration.test] + + rest_api_id = aws_api_gateway_rest_api.test.id + description = %[1]q + + variables = { + "a" = "2" + } +} + +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.test3.id + + variables = { + one = "5" + two = "2" + } +} +`, rName)) } diff --git a/website/docs/r/api_gateway_stage.html.markdown b/website/docs/r/api_gateway_stage.html.markdown index a47b88546fe..57d786d62ea 100644 --- a/website/docs/r/api_gateway_stage.html.markdown +++ b/website/docs/r/api_gateway_stage.html.markdown @@ -125,6 +125,7 @@ For more information on configuring the log format rules visit the AWS [document ### Canary Settings +* `deployment_id` - (Required) ID of the deployment that the canary points to. * `percent_traffic` - (Optional) Percent `0.0` - `100.0` of traffic to divert to the canary deployment. * `stage_variable_overrides` - (Optional) Map of overridden stage `variables` (including new variables) for the canary deployment. * `use_stage_cache` - (Optional) Whether the canary deployment uses the stage cache. Defaults to false.