Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/aws_appconfig_deployment: Handle ConflictException in resource Create #36980

Merged
merged 10 commits into from
Apr 18, 2024
3 changes: 3 additions & 0 deletions .changelog/36980.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_appconfig_deployment: Fix `ConflictException` errors on resource Create
```
44 changes: 43 additions & 1 deletion internal/conns/apiretry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@ package conns

import (
"errors"
"net/http"
"testing"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/aws/retry"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
appconfigtypes "github.com/aws/aws-sdk-go-v2/service/appconfig/types"
smithy "github.com/aws/smithy-go"
smithyhttp "github.com/aws/smithy-go/transport/http"
"github.com/hashicorp/terraform-provider-aws/internal/errs"
)

Expand All @@ -24,18 +29,55 @@ func TestAddIsErrorRetryables(t *testing.T) {
testCases := []struct {
name string
err error
f retry.IsErrorRetryableFunc
expected bool
}{
{
name: "no error",
f: f,
},
{
name: "non-retryable",
err: errors.New(`this is not retryable`),
f: f,
},
{
name: "retryable",
err: errors.New(`this is testing`),
f: f,
expected: true,
},
{
// https://github.com/hashicorp/terraform-provider-aws/issues/36975.
name: "appconfig ConflictException",
err: &smithy.OperationError{
ServiceID: "AppConfig",
OperationName: "StartDeployment",
Err: &awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusConflict,
},
},
Err: &appconfigtypes.ConflictException{
Message: aws.String("Deployment number 1 already exists"),
},
},
RequestID: "43e844da-818b-458e-aae2-553960ccc4d6",
},
},
f: func(err error) aws.Ternary {
if err, ok := errs.As[*smithy.OperationError](err); ok {
switch err.OperationName {
case "StartDeployment":
if errs.IsA[*appconfigtypes.ConflictException](err) {
return aws.TrueTernary
}
}
}
return aws.UnknownTernary
},
expected: true,
},
}
Expand All @@ -45,7 +87,7 @@ func TestAddIsErrorRetryables(t *testing.T) {
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

got := AddIsErrorRetryables(retry.NewStandard(), retry.IsErrorRetryableFunc(f)).IsErrorRetryable(testCase.err)
got := AddIsErrorRetryables(retry.NewStandard(), testCase.f).IsErrorRetryable(testCase.err)
if got, want := got, testCase.expected; got != want {
t.Errorf("IsErrorRetryable(%q) = %v, want %v", testCase.err, got, want)
}
Expand Down
14 changes: 9 additions & 5 deletions internal/service/appconfig/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"log"
"strconv"
"strings"
"time"

"github.com/YakDriver/regexache"
"github.com/aws/aws-sdk-go-v2/aws"
Expand All @@ -22,6 +23,7 @@ import (
"github.com/hashicorp/terraform-provider-aws/internal/errs"
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
"github.com/hashicorp/terraform-provider-aws/internal/verify"
"github.com/hashicorp/terraform-provider-aws/names"
)
Expand Down Expand Up @@ -124,16 +126,18 @@ func resourceDeploymentCreate(ctx context.Context, d *schema.ResourceData, meta
input.KmsKeyIdentifier = aws.String(v.(string))
}

output, err := conn.StartDeployment(ctx, input)
const (
timeout = 30 * time.Minute // AWS SDK for Go v1 compatibility.
)
outputRaw, err := tfresource.RetryWhenIsA[*awstypes.ConflictException](ctx, timeout, func() (interface{}, error) {
return conn.StartDeployment(ctx, input)
})

if err != nil {
return sdkdiag.AppendErrorf(diags, "starting AppConfig Deployment: %s", err)
}

if output == nil {
return sdkdiag.AppendErrorf(diags, "starting AppConfig Deployment: empty response")
}

output := outputRaw.(*appconfig.StartDeploymentOutput)
appID := aws.ToString(output.ApplicationId)
envID := aws.ToString(output.EnvironmentId)

Expand Down
116 changes: 89 additions & 27 deletions internal/service/appconfig/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ func TestAccAppConfigDeployment_basic(t *testing.T) {
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.AppConfigServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
// AppConfig Deployments cannot be destroyed, but we want to ensure
// the Application and its dependents are removed.
CheckDestroy: testAccCheckApplicationDestroy(ctx),
CheckDestroy: acctest.CheckDestroyNoop,
Steps: []resource.TestStep{
{
Config: testAccDeploymentConfig_name(rName),
Expand Down Expand Up @@ -77,9 +75,7 @@ func TestAccAppConfigDeployment_kms(t *testing.T) {
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.AppConfigServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
// AppConfig Deployments cannot be destroyed, but we want to ensure
// the Application and its dependents are removed.
CheckDestroy: testAccCheckApplicationDestroy(ctx),
CheckDestroy: acctest.CheckDestroyNoop,
Steps: []resource.TestStep{
{
Config: testAccDeploymentConfig_kms(rName),
Expand Down Expand Up @@ -112,9 +108,7 @@ func TestAccAppConfigDeployment_predefinedStrategy(t *testing.T) {
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.AppConfigServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
// AppConfig Deployments cannot be destroyed, but we want to ensure
// the Application and its dependents are removed.
CheckDestroy: testAccCheckApplicationDestroy(ctx),
CheckDestroy: acctest.CheckDestroyNoop,
Steps: []resource.TestStep{
{
Config: testAccDeploymentConfig_predefinedStrategy(rName, strategy),
Expand Down Expand Up @@ -146,7 +140,7 @@ func TestAccAppConfigDeployment_tags(t *testing.T) {
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.AppConfigServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: nil,
CheckDestroy: acctest.CheckDestroyNoop,
Steps: []resource.TestStep{
{
Config: testAccDeploymentConfig_tags1(rName, "key1", "value1"),
Expand Down Expand Up @@ -182,6 +176,31 @@ func TestAccAppConfigDeployment_tags(t *testing.T) {
})
}

func TestAccAppConfigDeployment_multiple(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resource1Name := "aws_appconfig_deployment.test.0"
resource2Name := "aws_appconfig_deployment.test.1"
resource3Name := "aws_appconfig_deployment.test.2"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.AppConfigServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: acctest.CheckDestroyNoop,
Steps: []resource.TestStep{
{
Config: testAccDeploymentConfig_multiple(rName, 3),
Check: resource.ComposeTestCheckFunc(
testAccCheckDeploymentExists(ctx, resource1Name),
testAccCheckDeploymentExists(ctx, resource2Name),
testAccCheckDeploymentExists(ctx, resource3Name),
),
},
},
})
}

func testAccCheckDeploymentExists(ctx context.Context, resourceName string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[resourceName]
Expand Down Expand Up @@ -221,7 +240,7 @@ func testAccCheckDeploymentExists(ctx context.Context, resourceName string) reso
}
}

func testAccDeploymentBaseConfig(rName string) string {
func testAccDeploymentConfig_base(rName string) string {
return fmt.Sprintf(`
resource "aws_appconfig_application" "test" {
name = %[1]q
Expand Down Expand Up @@ -259,7 +278,7 @@ resource "aws_appconfig_hosted_configuration_version" "test" {
`, rName)
}

func testAccDeploymentKMSConfig(rName string) string {
func testAccDeploymentConfig_baseKMS(rName string) string {
return fmt.Sprintf(`
resource "aws_kms_key" "test" {
description = %[1]q
Expand Down Expand Up @@ -304,9 +323,7 @@ resource "aws_appconfig_hosted_configuration_version" "test" {
}

func testAccDeploymentConfig_name(rName string) string {
return acctest.ConfigCompose(
testAccDeploymentBaseConfig(rName),
fmt.Sprintf(`
return acctest.ConfigCompose(testAccDeploymentConfig_base(rName), fmt.Sprintf(`
resource "aws_appconfig_deployment" "test"{
application_id = aws_appconfig_application.test.id
configuration_profile_id = aws_appconfig_configuration_profile.test.configuration_profile_id
Expand All @@ -319,9 +336,7 @@ resource "aws_appconfig_deployment" "test"{
}

func testAccDeploymentConfig_kms(rName string) string {
return acctest.ConfigCompose(
testAccDeploymentKMSConfig(rName),
fmt.Sprintf(`
return acctest.ConfigCompose(testAccDeploymentConfig_baseKMS(rName), fmt.Sprintf(`
resource "aws_appconfig_deployment" "test"{
application_id = aws_appconfig_application.test.id
configuration_profile_id = aws_appconfig_configuration_profile.test.configuration_profile_id
Expand All @@ -335,9 +350,7 @@ resource "aws_appconfig_deployment" "test"{
}

func testAccDeploymentConfig_predefinedStrategy(rName, strategy string) string {
return acctest.ConfigCompose(
testAccDeploymentBaseConfig(rName),
fmt.Sprintf(`
return acctest.ConfigCompose(testAccDeploymentConfig_base(rName), fmt.Sprintf(`
resource "aws_appconfig_deployment" "test"{
application_id = aws_appconfig_application.test.id
configuration_profile_id = aws_appconfig_configuration_profile.test.configuration_profile_id
Expand All @@ -350,9 +363,7 @@ resource "aws_appconfig_deployment" "test"{
}

func testAccDeploymentConfig_tags1(rName, tagKey1, tagValue1 string) string {
return acctest.ConfigCompose(
testAccDeploymentBaseConfig(rName),
fmt.Sprintf(`
return acctest.ConfigCompose(testAccDeploymentConfig_base(rName), fmt.Sprintf(`
resource "aws_appconfig_deployment" "test"{
application_id = aws_appconfig_application.test.id
configuration_profile_id = aws_appconfig_configuration_profile.test.configuration_profile_id
Expand All @@ -368,9 +379,7 @@ resource "aws_appconfig_deployment" "test"{
}

func testAccDeploymentConfig_tags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string {
return acctest.ConfigCompose(
testAccDeploymentBaseConfig(rName),
fmt.Sprintf(`
return acctest.ConfigCompose(testAccDeploymentConfig_base(rName), fmt.Sprintf(`
resource "aws_appconfig_deployment" "test"{
application_id = aws_appconfig_application.test.id
configuration_profile_id = aws_appconfig_configuration_profile.test.configuration_profile_id
Expand All @@ -385,3 +394,56 @@ resource "aws_appconfig_deployment" "test"{
}
`, rName, tagKey1, tagValue1, tagKey2, tagValue2))
}

func testAccDeploymentConfig_multiple(rName string, n int) string {
return fmt.Sprintf(`
resource "aws_appconfig_application" "test" {
name = %[1]q
}

resource "aws_appconfig_environment" "test" {
name = %[1]q
application_id = aws_appconfig_application.test.id
}

resource "aws_appconfig_configuration_profile" "test" {
count = %[2]d

application_id = aws_appconfig_application.test.id
name = "%[1]s-${count.index}"
location_uri = "hosted"
}

resource "aws_appconfig_deployment_strategy" "test" {
name = %[1]q
deployment_duration_in_minutes = 3
growth_factor = 10
replicate_to = "NONE"
}

resource "aws_appconfig_hosted_configuration_version" "test" {
count = %[2]d

application_id = aws_appconfig_application.test.id
configuration_profile_id = aws_appconfig_configuration_profile.test[count.index].configuration_profile_id
content_type = "application/json"

content = jsonencode({
foo = "bar"
})

description = "%[1]s-${count.index}"
}

resource "aws_appconfig_deployment" "test" {
count = %[2]d

application_id = aws_appconfig_application.test.id
configuration_profile_id = aws_appconfig_configuration_profile.test[count.index].configuration_profile_id
configuration_version = aws_appconfig_hosted_configuration_version.test[count.index].version_number
description = "%[1]s-${count.index}"
deployment_strategy_id = aws_appconfig_deployment_strategy.test.id
environment_id = aws_appconfig_environment.test.environment_id
}
`, rName, n)
}
41 changes: 0 additions & 41 deletions internal/service/appconfig/service_package.go

This file was deleted.

13 changes: 13 additions & 0 deletions internal/service/appconfig/service_package_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading