Skip to content

Commit bee00fd

Browse files
committed
Use ListSecretVersionIds when secret is write-only
1 parent cae81ed commit bee00fd

File tree

3 files changed

+199
-22
lines changed

3 files changed

+199
-22
lines changed

.changelog/44876.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
resource/aws_secretsmanager_secret_version: Avoid sending GetSecretValue calls when the secret is write-only
3+
```

internal/service/secretsmanager/secret_version.go

Lines changed: 107 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func resourceSecretVersionCreate(ctx context.Context, d *schema.ResourceData, me
151151
d.SetId(secretVersionCreateResourceID(secretID, versionID))
152152

153153
_, err = tfresource.RetryWhenNotFound(ctx, propagationTimeout, func(ctx context.Context) (any, error) {
154-
return findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID)
154+
return checkExists(ctx, conn, secretID, versionID, secretStringWO != "")
155155
})
156156

157157
if err != nil {
@@ -161,6 +161,25 @@ func resourceSecretVersionCreate(ctx context.Context, d *schema.ResourceData, me
161161
return append(diags, resourceSecretVersionRead(ctx, d, meta)...)
162162
}
163163

164+
type secretVersionExistsOutput struct {
165+
VersionStages []string
166+
}
167+
168+
func checkExists(ctx context.Context, conn *secretsmanager.Client, secretID, versionID string, hasWriteOnly bool) (*secretVersionExistsOutput, error) {
169+
if hasWriteOnly {
170+
_, output, err := findSecretVersionEntryByTwoPartKey(ctx, conn, secretID, versionID)
171+
if err != nil {
172+
return nil, err
173+
}
174+
return &secretVersionExistsOutput{VersionStages: output.VersionStages}, nil
175+
}
176+
output, err := findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID)
177+
if err != nil {
178+
return nil, err
179+
}
180+
return &secretVersionExistsOutput{VersionStages: output.VersionStages}, nil
181+
}
182+
164183
func resourceSecretVersionRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
165184
var diags diag.Diagnostics
166185
conn := meta.(*conns.AWSClient).SecretsManagerClient(ctx)
@@ -170,26 +189,6 @@ func resourceSecretVersionRead(ctx context.Context, d *schema.ResourceData, meta
170189
return sdkdiag.AppendFromErr(diags, err)
171190
}
172191

173-
output, err := findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID)
174-
175-
if !d.IsNewResource() && tfresource.NotFound(err) {
176-
log.Printf("[WARN] Secrets Manager Secret Version (%s) not found, removing from state", d.Id())
177-
d.SetId("")
178-
return diags
179-
}
180-
181-
if err != nil {
182-
return sdkdiag.AppendErrorf(diags, "reading Secrets Manager Secret Version (%s): %s", d.Id(), err)
183-
}
184-
185-
d.Set(names.AttrARN, output.ARN)
186-
d.Set("secret_binary", inttypes.Base64EncodeOnce(output.SecretBinary))
187-
d.Set("secret_id", secretID)
188-
d.Set("secret_string", output.SecretString)
189-
d.Set("version_id", output.VersionId)
190-
d.Set("version_stages", output.VersionStages)
191-
192-
// unset secret_string if the value is configured as write-only
193192
hasWriteOnly := flex.HasWriteOnlyValue(d, "secret_string_wo")
194193
secretStringWO, di := flex.GetWriteOnlyStringValue(d, cty.GetAttrPath("secret_string_wo"))
195194
diags = append(diags, di...)
@@ -201,6 +200,48 @@ func resourceSecretVersionRead(ctx context.Context, d *schema.ResourceData, meta
201200
hasWriteOnly = true
202201
}
203202

203+
d.Set("secret_id", secretID)
204+
205+
if hasWriteOnly {
206+
arn, versionEntry, err := findSecretVersionEntryByTwoPartKey(ctx, conn, secretID, versionID)
207+
208+
if !d.IsNewResource() && tfresource.NotFound(err) {
209+
log.Printf("[WARN] Secrets Manager Secret Version (%s) not found, removing from state", d.Id())
210+
d.SetId("")
211+
return diags
212+
}
213+
214+
if err != nil {
215+
return sdkdiag.AppendErrorf(diags, "reading Secrets Manager Secret Version (%s): %s", d.Id(), err)
216+
}
217+
218+
d.Set(names.AttrARN, arn)
219+
d.Set("secret_binary", nil)
220+
221+
if versionEntry != nil {
222+
d.Set("version_id", versionEntry.VersionId)
223+
d.Set("version_stages", versionEntry.VersionStages)
224+
}
225+
} else {
226+
output, err := findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID)
227+
228+
if !d.IsNewResource() && tfresource.NotFound(err) {
229+
log.Printf("[WARN] Secrets Manager Secret Version (%s) not found, removing from state", d.Id())
230+
d.SetId("")
231+
return diags
232+
}
233+
234+
if err != nil {
235+
return sdkdiag.AppendErrorf(diags, "reading Secrets Manager Secret Version (%s): %s", d.Id(), err)
236+
}
237+
238+
d.Set(names.AttrARN, output.ARN)
239+
d.Set("secret_binary", inttypes.Base64EncodeOnce(output.SecretBinary))
240+
d.Set("secret_string", output.SecretString)
241+
d.Set("version_id", output.VersionId)
242+
d.Set("version_stages", output.VersionStages)
243+
}
244+
204245
if hasWriteOnly {
205246
d.Set("has_secret_string_wo", true)
206247
d.Set("secret_string", nil)
@@ -335,7 +376,8 @@ func resourceSecretVersionDelete(ctx context.Context, d *schema.ResourceData, me
335376
}
336377

337378
_, err = tfresource.RetryUntilNotFound(ctx, propagationTimeout, func(ctx context.Context) (any, error) {
338-
output, err := findSecretVersionByTwoPartKey(ctx, conn, secretID, versionID)
379+
hasWriteOnly := flex.HasWriteOnlyValue(d, "secret_string_wo")
380+
output, err := checkExists(ctx, conn, secretID, versionID, hasWriteOnly)
339381

340382
if err != nil {
341383
return nil, err
@@ -355,6 +397,49 @@ func resourceSecretVersionDelete(ctx context.Context, d *schema.ResourceData, me
355397
return diags
356398
}
357399

400+
func findSecretVersionEntryByTwoPartKey(ctx context.Context, conn *secretsmanager.Client, secretID, versionID string) (*string, *types.SecretVersionsListEntry, error) {
401+
input := &secretsmanager.ListSecretVersionIdsInput{
402+
SecretId: aws.String(secretID),
403+
IncludeDeprecated: aws.Bool(true),
404+
}
405+
406+
paginator := secretsmanager.NewListSecretVersionIdsPaginator(conn, input)
407+
408+
for paginator.HasMorePages() {
409+
page, err := paginator.NextPage(ctx)
410+
411+
if errs.IsA[*types.ResourceNotFoundException](err) ||
412+
errs.IsAErrorMessageContains[*types.InvalidRequestException](err, "because it was deleted") ||
413+
errs.IsAErrorMessageContains[*types.InvalidRequestException](err, "because it was marked for deletion") {
414+
return nil, nil, &retry.NotFoundError{
415+
LastError: err,
416+
LastRequest: input,
417+
}
418+
}
419+
420+
if err != nil {
421+
return nil, nil, err
422+
}
423+
424+
if page == nil {
425+
continue
426+
}
427+
428+
for i := range page.Versions {
429+
version := &page.Versions[i]
430+
431+
if aws.ToString(version.VersionId) == versionID {
432+
return page.ARN, version, nil
433+
}
434+
}
435+
}
436+
437+
return nil, nil, &retry.NotFoundError{
438+
LastError: tfresource.NewEmptyResultError(input),
439+
LastRequest: input,
440+
}
441+
}
442+
358443
func findSecretVersion(ctx context.Context, conn *secretsmanager.Client, input *secretsmanager.GetSecretValueInput) (*secretsmanager.GetSecretValueOutput, error) {
359444
output, err := conn.GetSecretValue(ctx, input)
360445

internal/service/secretsmanager/secret_version_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package secretsmanager_test
66
import (
77
"context"
88
"fmt"
9+
"os"
910
"testing"
1011

1112
"github.com/aws/aws-sdk-go-v2/aws"
@@ -17,6 +18,7 @@ import (
1718
"github.com/hashicorp/terraform-plugin-testing/tfversion"
1819
"github.com/hashicorp/terraform-provider-aws/internal/acctest"
1920
"github.com/hashicorp/terraform-provider-aws/internal/conns"
21+
"github.com/hashicorp/terraform-provider-aws/internal/envvar"
2022
tfsecretsmanager "github.com/hashicorp/terraform-provider-aws/internal/service/secretsmanager"
2123
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
2224
itypes "github.com/hashicorp/terraform-provider-aws/internal/types"
@@ -312,6 +314,47 @@ func TestAccSecretsManagerSecretVersion_stringWriteOnly(t *testing.T) {
312314
})
313315
}
314316

317+
func TestAccSecretsManagerSecretVersion_stringWriteOnlyLimitedPermissions(t *testing.T) {
318+
ctx := acctest.Context(t)
319+
var version secretsmanager.GetSecretValueOutput
320+
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
321+
resourceName := "aws_secretsmanager_secret_version.test"
322+
secretResourceName := "aws_secretsmanager_secret.test"
323+
324+
resource.ParallelTest(t, resource.TestCase{
325+
PreCheck: func() {
326+
acctest.PreCheck(ctx, t)
327+
testAccPreCheck(ctx, t)
328+
acctest.PreCheckAssumeRoleARN(t)
329+
},
330+
ErrorCheck: acctest.ErrorCheck(t, names.SecretsManagerServiceID),
331+
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
332+
tfversion.SkipBelow(tfcversion.Must(tfcversion.NewVersion("1.11.0"))),
333+
},
334+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
335+
CheckDestroy: testAccCheckSecretVersionDestroy(ctx),
336+
Steps: []resource.TestStep{
337+
{
338+
Config: testAccSecretVersionConfig_stringWriteOnlyLimitedPermissions(rName, "test-secret", 1),
339+
Check: resource.ComposeTestCheckFunc(
340+
testAccCheckSecretVersionExists(ctx, resourceName, &version),
341+
testAccCheckSecretVersionWriteOnlyValueEqual(t, &version, "test-secret"),
342+
resource.TestCheckResourceAttr(resourceName, "has_secret_string_wo", "true"),
343+
resource.TestCheckResourceAttrPair(resourceName, names.AttrARN, secretResourceName, names.AttrARN),
344+
),
345+
},
346+
{
347+
Config: testAccSecretVersionConfig_string(rName),
348+
Check: resource.ComposeTestCheckFunc(
349+
testAccCheckSecretVersionExists(ctx, resourceName, &version),
350+
resource.TestCheckResourceAttr(resourceName, "secret_string", "test-string"),
351+
resource.TestCheckResourceAttrPair(resourceName, names.AttrARN, secretResourceName, names.AttrARN),
352+
),
353+
},
354+
},
355+
})
356+
}
357+
315358
func testAccCheckSecretVersionDestroy(ctx context.Context) resource.TestCheckFunc {
316359
return func(s *terraform.State) error {
317360
conn := acctest.Provider.Meta().(*conns.AWSClient).SecretsManagerClient(ctx)
@@ -410,6 +453,52 @@ resource "aws_secretsmanager_secret_version" "test" {
410453
`, rName, secret, version)
411454
}
412455

456+
func testAccSecretVersionConfig_stringWriteOnlyLimitedPermissions(rName, secret string, version int) string {
457+
policy := `{
458+
"Version": "2012-10-17",
459+
"Statement": [
460+
{
461+
"Effect": "Allow",
462+
"Action": [
463+
"secretsmanager:DeleteSecret",
464+
"secretsmanager:DescribeSecret",
465+
"secretsmanager:GetResourcePolicy",
466+
"secretsmanager:ListSecretVersionIds",
467+
"secretsmanager:PutSecretValue"
468+
],
469+
"Resource": "*"
470+
}
471+
]
472+
}`
473+
474+
return acctest.ConfigCompose(
475+
fmt.Sprintf(`
476+
provider "aws" {
477+
alias = "limited"
478+
479+
assume_role {
480+
role_arn = %[1]q
481+
policy = <<POLICY
482+
%[2]s
483+
POLICY
484+
}
485+
}
486+
`, os.Getenv(envvar.AccAssumeRoleARN), policy),
487+
fmt.Sprintf(`
488+
resource "aws_secretsmanager_secret" "test" {
489+
name = %[1]q
490+
}
491+
492+
resource "aws_secretsmanager_secret_version" "test" {
493+
provider = aws.limited
494+
secret_id = aws_secretsmanager_secret.test.id
495+
secret_string_wo = %[2]q
496+
secret_string_wo_version = %[3]d
497+
}
498+
`, rName, secret, version),
499+
)
500+
}
501+
413502
func testAccSecretVersionConfig_binary(rName string) string {
414503
return fmt.Sprintf(`
415504
resource "aws_secretsmanager_secret" "test" {

0 commit comments

Comments
 (0)