diff --git a/.changelog/44876.txt b/.changelog/44876.txt new file mode 100644 index 000000000000..f1925305d635 --- /dev/null +++ b/.changelog/44876.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_secretsmanager_secret_version: Avoid sending GetSecretValue calls when the secret is write-only +``` diff --git a/internal/service/secretsmanager/secret_version.go b/internal/service/secretsmanager/secret_version.go index 8a1867d7d268..1911627b1f35 100644 --- a/internal/service/secretsmanager/secret_version.go +++ b/internal/service/secretsmanager/secret_version.go @@ -151,7 +151,7 @@ func resourceSecretVersionCreate(ctx context.Context, d *schema.ResourceData, me d.SetId(secretVersionCreateResourceID(secretID, versionID)) _, err = tfresource.RetryWhenNotFound(ctx, propagationTimeout, func(ctx context.Context) (any, error) { - return findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID) + return checkExists(ctx, conn, secretID, versionID, secretStringWO != "") }) if err != nil { @@ -161,6 +161,25 @@ func resourceSecretVersionCreate(ctx context.Context, d *schema.ResourceData, me return append(diags, resourceSecretVersionRead(ctx, d, meta)...) } +type secretVersionExistsOutput struct { + VersionStages []string +} + +func checkExists(ctx context.Context, conn *secretsmanager.Client, secretID, versionID string, hasWriteOnly bool) (*secretVersionExistsOutput, error) { + if hasWriteOnly { + _, output, err := findSecretVersionEntryByTwoPartKey(ctx, conn, secretID, versionID) + if err != nil { + return nil, err + } + return &secretVersionExistsOutput{VersionStages: output.VersionStages}, nil + } + output, err := findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID) + if err != nil { + return nil, err + } + return &secretVersionExistsOutput{VersionStages: output.VersionStages}, nil +} + func resourceSecretVersionRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).SecretsManagerClient(ctx) @@ -170,26 +189,6 @@ func resourceSecretVersionRead(ctx context.Context, d *schema.ResourceData, meta return sdkdiag.AppendFromErr(diags, err) } - output, err := findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID) - - if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] Secrets Manager Secret Version (%s) not found, removing from state", d.Id()) - d.SetId("") - return diags - } - - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading Secrets Manager Secret Version (%s): %s", d.Id(), err) - } - - d.Set(names.AttrARN, output.ARN) - d.Set("secret_binary", inttypes.Base64EncodeOnce(output.SecretBinary)) - d.Set("secret_id", secretID) - d.Set("secret_string", output.SecretString) - d.Set("version_id", output.VersionId) - d.Set("version_stages", output.VersionStages) - - // unset secret_string if the value is configured as write-only hasWriteOnly := flex.HasWriteOnlyValue(d, "secret_string_wo") secretStringWO, di := flex.GetWriteOnlyStringValue(d, cty.GetAttrPath("secret_string_wo")) diags = append(diags, di...) @@ -201,6 +200,48 @@ func resourceSecretVersionRead(ctx context.Context, d *schema.ResourceData, meta hasWriteOnly = true } + d.Set("secret_id", secretID) + + if hasWriteOnly { + arn, versionEntry, err := findSecretVersionEntryByTwoPartKey(ctx, conn, secretID, versionID) + + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] Secrets Manager Secret Version (%s) not found, removing from state", d.Id()) + d.SetId("") + return diags + } + + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading Secrets Manager Secret Version (%s): %s", d.Id(), err) + } + + d.Set(names.AttrARN, arn) + d.Set("secret_binary", nil) + + if versionEntry != nil { + d.Set("version_id", versionEntry.VersionId) + d.Set("version_stages", versionEntry.VersionStages) + } + } else { + output, err := findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID) + + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] Secrets Manager Secret Version (%s) not found, removing from state", d.Id()) + d.SetId("") + return diags + } + + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading Secrets Manager Secret Version (%s): %s", d.Id(), err) + } + + d.Set(names.AttrARN, output.ARN) + d.Set("secret_binary", inttypes.Base64EncodeOnce(output.SecretBinary)) + d.Set("secret_string", output.SecretString) + d.Set("version_id", output.VersionId) + d.Set("version_stages", output.VersionStages) + } + if hasWriteOnly { d.Set("has_secret_string_wo", true) d.Set("secret_string", nil) @@ -335,7 +376,8 @@ func resourceSecretVersionDelete(ctx context.Context, d *schema.ResourceData, me } _, err = tfresource.RetryUntilNotFound(ctx, propagationTimeout, func(ctx context.Context) (any, error) { - output, err := findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID) + hasWriteOnly := flex.HasWriteOnlyValue(d, "secret_string_wo") + output, err := checkExists(ctx, conn, secretID, versionID, hasWriteOnly) if err != nil { return nil, err @@ -355,6 +397,49 @@ func resourceSecretVersionDelete(ctx context.Context, d *schema.ResourceData, me return diags } +func findSecretVersionEntryByTwoPartKey(ctx context.Context, conn *secretsmanager.Client, secretID, versionID string) (*string, *types.SecretVersionsListEntry, error) { + input := &secretsmanager.ListSecretVersionIdsInput{ + SecretId: aws.String(secretID), + IncludeDeprecated: aws.Bool(true), + } + + paginator := secretsmanager.NewListSecretVersionIdsPaginator(conn, input) + + for paginator.HasMorePages() { + page, err := paginator.NextPage(ctx) + + if errs.IsA[*types.ResourceNotFoundException](err) || + errs.IsAErrorMessageContains[*types.InvalidRequestException](err, "because it was deleted") || + errs.IsAErrorMessageContains[*types.InvalidRequestException](err, "because it was marked for deletion") { + return nil, nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, nil, err + } + + if page == nil { + continue + } + + for i := range page.Versions { + version := &page.Versions[i] + + if aws.ToString(version.VersionId) == versionID { + return page.ARN, version, nil + } + } + } + + return nil, nil, &retry.NotFoundError{ + LastError: tfresource.NewEmptyResultError(input), + LastRequest: input, + } +} + func findSecretVersion(ctx context.Context, conn *secretsmanager.Client, input *secretsmanager.GetSecretValueInput) (*secretsmanager.GetSecretValueOutput, error) { output, err := conn.GetSecretValue(ctx, input) diff --git a/internal/service/secretsmanager/secret_version_test.go b/internal/service/secretsmanager/secret_version_test.go index 3c01c5bd5db2..c8fe82fd4a20 100644 --- a/internal/service/secretsmanager/secret_version_test.go +++ b/internal/service/secretsmanager/secret_version_test.go @@ -6,6 +6,7 @@ package secretsmanager_test import ( "context" "fmt" + "os" "testing" "github.com/aws/aws-sdk-go-v2/aws" @@ -17,6 +18,7 @@ import ( "github.com/hashicorp/terraform-plugin-testing/tfversion" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/envvar" tfsecretsmanager "github.com/hashicorp/terraform-provider-aws/internal/service/secretsmanager" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" itypes "github.com/hashicorp/terraform-provider-aws/internal/types" @@ -312,6 +314,47 @@ func TestAccSecretsManagerSecretVersion_stringWriteOnly(t *testing.T) { }) } +func TestAccSecretsManagerSecretVersion_stringWriteOnlyLimitedPermissions(t *testing.T) { + ctx := acctest.Context(t) + var version secretsmanager.GetSecretValueOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_secretsmanager_secret_version.test" + secretResourceName := "aws_secretsmanager_secret.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + testAccPreCheck(ctx, t) + acctest.PreCheckAssumeRoleARN(t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.SecretsManagerServiceID), + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.SkipBelow(tfcversion.Must(tfcversion.NewVersion("1.11.0"))), + }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckSecretVersionDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccSecretVersionConfig_stringWriteOnlyLimitedPermissions(rName, "test-secret", 1), + Check: resource.ComposeTestCheckFunc( + testAccCheckSecretVersionExists(ctx, resourceName, &version), + testAccCheckSecretVersionWriteOnlyValueEqual(t, &version, "test-secret"), + resource.TestCheckResourceAttr(resourceName, "has_secret_string_wo", "true"), + resource.TestCheckResourceAttrPair(resourceName, names.AttrARN, secretResourceName, names.AttrARN), + ), + }, + { + Config: testAccSecretVersionConfig_string(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckSecretVersionExists(ctx, resourceName, &version), + resource.TestCheckResourceAttr(resourceName, "secret_string", "test-string"), + resource.TestCheckResourceAttrPair(resourceName, names.AttrARN, secretResourceName, names.AttrARN), + ), + }, + }, + }) +} + func testAccCheckSecretVersionDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).SecretsManagerClient(ctx) @@ -410,6 +453,52 @@ resource "aws_secretsmanager_secret_version" "test" { `, rName, secret, version) } +func testAccSecretVersionConfig_stringWriteOnlyLimitedPermissions(rName, secret string, version int) string { + policy := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "secretsmanager:DeleteSecret", + "secretsmanager:DescribeSecret", + "secretsmanager:GetResourcePolicy", + "secretsmanager:ListSecretVersionIds", + "secretsmanager:PutSecretValue" + ], + "Resource": "*" + } + ] +}` + + return acctest.ConfigCompose( + fmt.Sprintf(` +provider "aws" { + alias = "limited" + + assume_role { + role_arn = %[1]q + policy = <