Skip to content

Commit

Permalink
Merge pull request #36980 from hashicorp/b-aws_appconfig_deployment.C…
Browse files Browse the repository at this point in the history
…onflictException

r/aws_appconfig_deployment: Handle `ConflictException` in resource Create
  • Loading branch information
ewbankkit authored Apr 18, 2024
2 parents 9ca1c99 + c9172fa commit 31d05e7
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 75 deletions.
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

0 comments on commit 31d05e7

Please sign in to comment.