From ed9512fac317bd708a0826f04998f1f60ced9baf Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 2 Apr 2021 16:14:58 -0400 Subject: [PATCH] service/secretsmanager: Handle read-after-create eventual consistency in resources (#18462) Reference: https://github.com/hashicorp/terraform-provider-aws/issues/16482 Reference: https://github.com/hashicorp/terraform-provider-aws/issues/16796 Output from acceptance testing in AWS Commercial: ``` --- FAIL: TestAccAwsSecretsManagerSecret_policy (40.13s) # https://github.com/hashicorp/terraform-provider-aws/issues/18461 --- PASS: TestAccAwsSecretsManagerSecret_basic (19.14s) --- PASS: TestAccAwsSecretsManagerSecret_Description (28.90s) --- PASS: TestAccAwsSecretsManagerSecret_KmsKeyID (36.04s) --- PASS: TestAccAwsSecretsManagerSecret_RecoveryWindowInDays_Recreate (27.96s) --- PASS: TestAccAwsSecretsManagerSecret_RotationLambdaARN (103.44s) --- PASS: TestAccAwsSecretsManagerSecret_RotationRules (55.08s) --- PASS: TestAccAwsSecretsManagerSecret_Tags (48.81s) --- PASS: TestAccAwsSecretsManagerSecret_withNamePrefix (19.14s) --- PASS: TestAccAwsSecretsManagerSecretPolicy_basic (46.93s) --- PASS: TestAccAwsSecretsManagerSecretPolicy_blockPublicPolicy (49.17s) --- PASS: TestAccAwsSecretsManagerSecretPolicy_disappears (27.04s) --- PASS: TestAccAwsSecretsManagerSecretRotation_basic (54.74s) --- PASS: TestAccAwsSecretsManagerSecretVersion_Base64Binary (20.22s) --- PASS: TestAccAwsSecretsManagerSecretVersion_BasicString (19.64s) --- PASS: TestAccAwsSecretsManagerSecretVersion_VersionStages (42.20s) ``` Output from acceptance testing in AWS GovCloud (US): ``` --- FAIL: TestAccAwsSecretsManagerSecret_policy (41.26s) # https://github.com/hashicorp/terraform-provider-aws/issues/18461 --- PASS: TestAccAwsSecretsManagerSecret_basic (25.60s) --- PASS: TestAccAwsSecretsManagerSecret_Description (37.53s) --- PASS: TestAccAwsSecretsManagerSecret_KmsKeyID (45.10s) --- PASS: TestAccAwsSecretsManagerSecret_RecoveryWindowInDays_Recreate (37.67s) --- PASS: TestAccAwsSecretsManagerSecret_RotationLambdaARN (70.43s) --- PASS: TestAccAwsSecretsManagerSecret_RotationRules (58.87s) --- PASS: TestAccAwsSecretsManagerSecret_Tags (64.49s) --- PASS: TestAccAwsSecretsManagerSecret_withNamePrefix (23.75s) --- PASS: TestAccAwsSecretsManagerSecretPolicy_basic (49.66s) --- PASS: TestAccAwsSecretsManagerSecretPolicy_blockPublicPolicy (64.81s) --- PASS: TestAccAwsSecretsManagerSecretPolicy_disappears (29.34s) --- PASS: TestAccAwsSecretsManagerSecretRotation_basic (68.89s) --- PASS: TestAccAwsSecretsManagerSecretVersion_Base64Binary (21.89s) --- PASS: TestAccAwsSecretsManagerSecretVersion_BasicString (24.62s) --- PASS: TestAccAwsSecretsManagerSecretVersion_VersionStages (54.57s) ``` --- .changelog/18462.txt | 15 +++++ .../service/secretsmanager/waiter/waiter.go | 4 +- aws/resource_aws_secretsmanager_secret.go | 45 +++++++++++--- ...source_aws_secretsmanager_secret_policy.go | 45 +++++++++++--- ...e_aws_secretsmanager_secret_policy_test.go | 2 +- ...urce_aws_secretsmanager_secret_rotation.go | 44 +++++++++++--- ...resource_aws_secretsmanager_secret_test.go | 2 +- ...ource_aws_secretsmanager_secret_version.go | 58 +++++++++++++++---- 8 files changed, 174 insertions(+), 41 deletions(-) create mode 100644 .changelog/18462.txt diff --git a/.changelog/18462.txt b/.changelog/18462.txt new file mode 100644 index 00000000000..537f8aba095 --- /dev/null +++ b/.changelog/18462.txt @@ -0,0 +1,15 @@ +```release-note:bug +resource/aws_secretsmanager_secret: Handle read-after-create eventual consistency +``` + +```release-note:bug +resource/aws_secretsmanager_secret_policy: Handle read-after-create eventual consistency +``` + +```release-note:bug +resource/aws_secretsmanager_secret_rotation: Handle read-after-create eventual consistency +``` + +```release-note:bug +resource/aws_secretsmanager_secret_version: Handle read-after-create eventual consistency +``` diff --git a/aws/internal/service/secretsmanager/waiter/waiter.go b/aws/internal/service/secretsmanager/waiter/waiter.go index 087f9e82ee8..31333284cb6 100644 --- a/aws/internal/service/secretsmanager/waiter/waiter.go +++ b/aws/internal/service/secretsmanager/waiter/waiter.go @@ -5,6 +5,6 @@ import ( ) const ( - // Maximum amount of time to wait for Secrets Manager deletions to propagate - DeletionPropagationTimeout = 2 * time.Minute + // Maximum amount of time to wait for Secrets Manager changes to propagate + PropagationTimeout = 2 * time.Minute ) diff --git a/aws/resource_aws_secretsmanager_secret.go b/aws/resource_aws_secretsmanager_secret.go index a22215c3473..3d16345de8a 100644 --- a/aws/resource_aws_secretsmanager_secret.go +++ b/aws/resource_aws_secretsmanager_secret.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/secretsmanager" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" @@ -14,6 +15,7 @@ import ( "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/secretsmanager/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsSecretsManagerSecret() *schema.Resource { @@ -131,7 +133,7 @@ func resourceAwsSecretsManagerSecretCreate(d *schema.ResourceData, meta interfac // Retry for secret recreation after deletion var output *secretsmanager.CreateSecretOutput - err := resource.Retry(waiter.DeletionPropagationTimeout, func() *resource.RetryError { + err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { var err error output, err = conn.CreateSecret(input) // Temporarily retry on these errors to support immediate secret recreation: @@ -218,15 +220,40 @@ func resourceAwsSecretsManagerSecretRead(d *schema.ResourceData, meta interface{ SecretId: aws.String(d.Id()), } - log.Printf("[DEBUG] Reading Secrets Manager Secret: %s", input) - output, err := conn.DescribeSecret(input) - if err != nil { - if isAWSErr(err, secretsmanager.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] Secrets Manager Secret %q not found - removing from state", d.Id()) - d.SetId("") - return nil + var output *secretsmanager.DescribeSecretOutput + + err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { + var err error + + output, err = conn.DescribeSecret(input) + + if d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) } - return fmt.Errorf("error reading Secrets Manager Secret: %w", err) + + return nil + }) + + if tfresource.TimedOut(err) { + output, err = conn.DescribeSecret(input) + } + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] Secrets Manager Secret (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error reading Secrets Manager Secret (%s): %w", d.Id(), err) + } + + if output == nil { + return fmt.Errorf("error reading Secrets Manager Secret (%s): empty response", d.Id()) } d.Set("arn", output.ARN) diff --git a/aws/resource_aws_secretsmanager_secret_policy.go b/aws/resource_aws_secretsmanager_secret_policy.go index 035d3268856..07ab8b027e3 100644 --- a/aws/resource_aws_secretsmanager_secret_policy.go +++ b/aws/resource_aws_secretsmanager_secret_policy.go @@ -6,11 +6,14 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/secretsmanager" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/secretsmanager/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsSecretsManagerSecretPolicy() *schema.Resource { @@ -89,15 +92,41 @@ func resourceAwsSecretsManagerSecretPolicyRead(d *schema.ResourceData, meta inte input := &secretsmanager.GetResourcePolicyInput{ SecretId: aws.String(d.Id()), } - log.Printf("[DEBUG] Reading Secrets Manager Secret Policy: %#v", input) - res, err := conn.GetResourcePolicy(input) - if err != nil { - if isAWSErr(err, secretsmanager.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] SecretsManager Secret Policy (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil + + var res *secretsmanager.GetResourcePolicyOutput + + err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { + var err error + + res, err = conn.GetResourcePolicy(input) + + if d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) } - return fmt.Errorf("error reading Secrets Manager Secret policy: %w", err) + + return nil + }) + + if tfresource.TimedOut(err) { + res, err = conn.GetResourcePolicy(input) + } + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] Secrets Manager Secret Policy (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error reading Secrets Manager Secret Policy (%s): %w", d.Id(), err) + } + + if res == nil { + return fmt.Errorf("error reading Secrets Manager Secret Policy (%s): empty response", d.Id()) } if res.ResourcePolicy != nil { diff --git a/aws/resource_aws_secretsmanager_secret_policy_test.go b/aws/resource_aws_secretsmanager_secret_policy_test.go index 305cad4d0c6..12d1c106933 100644 --- a/aws/resource_aws_secretsmanager_secret_policy_test.go +++ b/aws/resource_aws_secretsmanager_secret_policy_test.go @@ -179,7 +179,7 @@ func testAccCheckAwsSecretsManagerSecretPolicyDestroy(s *terraform.State) error var output *secretsmanager.DescribeSecretOutput - err := resource.Retry(waiter.DeletionPropagationTimeout, func() *resource.RetryError { + err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { var err error output, err = conn.DescribeSecret(secretInput) diff --git a/aws/resource_aws_secretsmanager_secret_rotation.go b/aws/resource_aws_secretsmanager_secret_rotation.go index 21f2da3d451..d5ad3079dc3 100644 --- a/aws/resource_aws_secretsmanager_secret_rotation.go +++ b/aws/resource_aws_secretsmanager_secret_rotation.go @@ -7,8 +7,11 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/secretsmanager" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/secretsmanager/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsSecretsManagerSecretRotation() *schema.Resource { @@ -99,15 +102,40 @@ func resourceAwsSecretsManagerSecretRotationRead(d *schema.ResourceData, meta in SecretId: aws.String(d.Id()), } - log.Printf("[DEBUG] Reading Secrets Manager Secret Rotation: %s", input) - output, err := conn.DescribeSecret(input) - if err != nil { - if isAWSErr(err, secretsmanager.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] Secrets Manager Secret Rotation %q not found - removing from state", d.Id()) - d.SetId("") - return nil + var output *secretsmanager.DescribeSecretOutput + + err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { + var err error + + output, err = conn.DescribeSecret(input) + + if d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) { + return resource.RetryableError(err) } - return fmt.Errorf("error reading Secrets Manager Secret Rotation: %s", err) + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if tfresource.TimedOut(err) { + output, err = conn.DescribeSecret(input) + } + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] Secrets Manager Secret Rotation (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error reading Secrets Manager Secret Rotation (%s): %w", d.Id(), err) + } + + if output == nil { + return fmt.Errorf("error reading Secrets Manager Secret Rotation (%s): empty response", d.Id()) } d.Set("secret_id", d.Id()) diff --git a/aws/resource_aws_secretsmanager_secret_test.go b/aws/resource_aws_secretsmanager_secret_test.go index 6f4067058a3..895868aca9c 100644 --- a/aws/resource_aws_secretsmanager_secret_test.go +++ b/aws/resource_aws_secretsmanager_secret_test.go @@ -448,7 +448,7 @@ func testAccCheckAwsSecretsManagerSecretDestroy(s *terraform.State) error { var output *secretsmanager.DescribeSecretOutput - err := resource.Retry(waiter.DeletionPropagationTimeout, func() *resource.RetryError { + err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { var err error output, err = conn.DescribeSecret(input) diff --git a/aws/resource_aws_secretsmanager_secret_version.go b/aws/resource_aws_secretsmanager_secret_version.go index f1f100fb873..072e9e50eb8 100644 --- a/aws/resource_aws_secretsmanager_secret_version.go +++ b/aws/resource_aws_secretsmanager_secret_version.go @@ -8,7 +8,11 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/secretsmanager" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/secretsmanager/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsSecretsManagerSecretVersion() *schema.Resource { @@ -114,20 +118,50 @@ func resourceAwsSecretsManagerSecretVersionRead(d *schema.ResourceData, meta int VersionId: aws.String(versionID), } - log.Printf("[DEBUG] Reading Secrets Manager Secret Version: %s", input) - output, err := conn.GetSecretValue(input) - if err != nil { - if isAWSErr(err, secretsmanager.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] Secrets Manager Secret Version %q not found - removing from state", d.Id()) - d.SetId("") - return nil + var output *secretsmanager.GetSecretValueOutput + + err = resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { + var err error + + output, err = conn.GetSecretValue(input) + + if d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) { + return resource.RetryableError(err) } - if isAWSErr(err, secretsmanager.ErrCodeInvalidRequestException, "You can’t perform this operation on the secret because it was deleted") { - log.Printf("[WARN] Secrets Manager Secret Version %q not found - removing from state", d.Id()) - d.SetId("") - return nil + + if d.IsNewResource() && tfawserr.ErrMessageContains(err, secretsmanager.ErrCodeInvalidRequestException, "You can’t perform this operation on the secret because it was deleted") { + return resource.RetryableError(err) } - return fmt.Errorf("error reading Secrets Manager Secret Version: %s", err) + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if tfresource.TimedOut(err) { + output, err = conn.GetSecretValue(input) + } + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] Secrets Manager Secret Version (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if !d.IsNewResource() && tfawserr.ErrMessageContains(err, secretsmanager.ErrCodeInvalidRequestException, "You can’t perform this operation on the secret because it was deleted") { + log.Printf("[WARN] Secrets Manager Secret Version (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error reading Secrets Manager Secret Version (%s): %w", d.Id(), err) + } + + if output == nil { + return fmt.Errorf("error reading Secrets Manager Secret Version (%s): empty response", d.Id()) } d.Set("secret_id", secretID)