Merge pull request #35024 from hashicorp/b-secretsmgr-rotation

resource/aws_secretsmanager_secret_rotation: Fix bug with updates
hashicorp · Dec 21, 2023 · 57f45d7 · 57f45d7
2 parents a214c6d + 92c991e
commit 57f45d7
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 13 deletions.
diff --git a/.changelog/35024.txt b/.changelog/35024.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_secretsmanager_secret_rotation: No longer ignores changes to `rotation_rules.automatically_after_days` when `rotation_rules.schedule_expression` is set.
+```
diff --git a/internal/service/secretsmanager/secret.go b/internal/service/secretsmanager/secret.go
@@ -15,6 +15,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
@@ -142,6 +143,7 @@ func resourceSecretCreate(ctx context.Context, d *schema.ResourceData, meta inte
 
 	secretName := create.Name(d.Get("name").(string), d.Get("name_prefix").(string))
 	input := &secretsmanager.CreateSecretInput{
+		ClientRequestToken:          aws.String(id.UniqueId()), // Needed because we're handling our own retries
 		Description:                 aws.String(d.Get("description").(string)),
 		ForceOverwriteReplicaSecret: aws.Bool(d.Get("force_overwrite_replica_secret").(bool)),
 		Name:                        aws.String(secretName),
@@ -313,8 +315,9 @@ func resourceSecretUpdate(ctx context.Context, d *schema.ResourceData, meta inte
 
 	if d.HasChanges("description", "kms_key_id") {
 		input := &secretsmanager.UpdateSecretInput{
-			Description: aws.String(d.Get("description").(string)),
-			SecretId:    aws.String(d.Id()),
+			ClientRequestToken: aws.String(id.UniqueId()), // Needed because we're handling our own retries
+			Description:        aws.String(d.Get("description").(string)),
+			SecretId:           aws.String(d.Id()),
 		}
 
 		if v, ok := d.GetOk("kms_key_id"); ok {

diff --git a/internal/service/secretsmanager/secret_rotation.go b/internal/service/secretsmanager/secret_rotation.go
@@ -12,6 +12,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
@@ -53,12 +54,7 @@ func ResourceSecretRotation() *schema.Resource {
 							Optional:      true,
 							ConflictsWith: []string{"rotation_rules.0.schedule_expression"},
 							ExactlyOneOf:  []string{"rotation_rules.0.automatically_after_days", "rotation_rules.0.schedule_expression"},
-							DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
-								_, exists := d.GetOk("rotation_rules.0.schedule_expression")
-								return exists
-							},
-							DiffSuppressOnRefresh: true,
-							ValidateFunc:          validation.IntBetween(1, 1000),
+							ValidateFunc:  validation.IntBetween(1, 1000),
 						},
 						"duration": {
 							Type:         schema.TypeString,
@@ -90,8 +86,9 @@ func resourceSecretRotationCreate(ctx context.Context, d *schema.ResourceData, m
 	secretID := d.Get("secret_id").(string)
 
 	input := &secretsmanager.RotateSecretInput{
-		RotationRules: expandRotationRules(d.Get("rotation_rules").([]interface{})),
-		SecretId:      aws.String(secretID),
+		ClientRequestToken: aws.String(id.UniqueId()), // Needed because we're handling our own retries
+		RotationRules:      expandRotationRules(d.Get("rotation_rules").([]interface{})),
+		SecretId:           aws.String(secretID),
 	}
 
 	if v, ok := d.GetOk("rotation_lambda_arn"); ok {
@@ -154,8 +151,9 @@ func resourceSecretRotationUpdate(ctx context.Context, d *schema.ResourceData, m
 
 	if d.HasChanges("rotation_lambda_arn", "rotation_rules") {
 		input := &secretsmanager.RotateSecretInput{
-			RotationRules: expandRotationRules(d.Get("rotation_rules").([]interface{})),
-			SecretId:      aws.String(secretID),
+			ClientRequestToken: aws.String(id.UniqueId()), // Needed because we're handling our own retries
+			RotationRules:      expandRotationRules(d.Get("rotation_rules").([]interface{})),
+			SecretId:           aws.String(secretID),
 		}
 
 		if v, ok := d.GetOk("rotation_lambda_arn"); ok {

diff --git a/internal/service/secretsmanager/secret_rotation_test.go b/internal/service/secretsmanager/secret_rotation_test.go
@@ -41,6 +41,8 @@ func TestAccSecretsManagerSecretRotation_basic(t *testing.T) {
 					resource.TestCheckResourceAttrPair(resourceName, "rotation_lambda_arn", lambdaFunctionResourceName, "arn"),
 					resource.TestCheckResourceAttr(resourceName, "rotation_rules.#", "1"),
 					resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.automatically_after_days", strconv.Itoa(days)),
+					resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.duration", ""),
+					resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.schedule_expression", ""),
 				),
 			},
 			{
@@ -95,6 +97,53 @@ func TestAccSecretsManagerSecretRotation_scheduleExpression(t *testing.T) {
 	})
 }
 
+func TestAccSecretsManagerSecretRotation_scheduleExpressionToDays(t *testing.T) {
+	ctx := acctest.Context(t)
+	var secret secretsmanager.DescribeSecretOutput
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	const (
+		resourceName               = "aws_secretsmanager_secret_rotation.test"
+		lambdaFunctionResourceName = "aws_lambda_function.test"
+		scheduleExpression         = "rate(10 days)"
+		days                       = 7
+	)
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, secretsmanager.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckSecretRotationDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccSecretRotationConfig_scheduleExpression(rName, scheduleExpression),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckSecretRotationExists(ctx, resourceName, &secret),
+					resource.TestCheckResourceAttr(resourceName, "rotation_enabled", "true"),
+					resource.TestCheckResourceAttrPair(resourceName, "rotation_lambda_arn", lambdaFunctionResourceName, "arn"),
+					resource.TestCheckResourceAttr(resourceName, "rotation_rules.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.schedule_expression", scheduleExpression),
+					resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.automatically_after_days", "0"),
+				),
+			},
+			{
+				Config: testAccSecretRotationConfig_basic(rName, days),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckSecretRotationExists(ctx, resourceName, &secret),
+					resource.TestCheckResourceAttr(resourceName, "rotation_enabled", "true"),
+					resource.TestCheckResourceAttrPair(resourceName, "rotation_lambda_arn", lambdaFunctionResourceName, "arn"),
+					resource.TestCheckResourceAttr(resourceName, "rotation_rules.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.automatically_after_days", strconv.Itoa(days)),
+					resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.schedule_expression", ""),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccSecretsManagerSecretRotation_scheduleExpressionHours(t *testing.T) {
 	ctx := acctest.Context(t)
 	var secret secretsmanager.DescribeSecretOutput
@@ -295,6 +344,11 @@ resource "aws_secretsmanager_secret" "test" {
   name = %[1]q
 }
 
+resource "aws_secretsmanager_secret_version" "test" {
+  secret_id     = aws_secretsmanager_secret.test.id
+  secret_string = "test-string"
+}
+
 resource "aws_secretsmanager_secret_rotation" "test" {
   secret_id           = aws_secretsmanager_secret.test.id
   rotation_lambda_arn = aws_lambda_function.test.arn

diff --git a/internal/service/secretsmanager/secret_version.go b/internal/service/secretsmanager/secret_version.go
@@ -14,6 +14,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
@@ -78,7 +79,8 @@ func resourceSecretVersionCreate(ctx context.Context, d *schema.ResourceData, me
 	secretID := d.Get("secret_id").(string)
 
 	input := &secretsmanager.PutSecretValueInput{
-		SecretId: aws.String(secretID),
+		ClientRequestToken: aws.String(id.UniqueId()), // Needed because we're handling our own retries
+		SecretId:           aws.String(secretID),
 	}
 
 	if v, ok := d.GetOk("secret_string"); ok {