Merge pull request #31915 from szilveszter/b-fix-secret-rotation-upda…

…te-failing r/aws_secretsmanager_secret_rotation: Fix failing schedule expression updates
hashicorp · Jul 3, 2023 · 929a1c0 · 929a1c0
2 parents be00acc + e6b3462
commit 929a1c0
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 2 deletions.
diff --git a/.changelog/31915.txt b/.changelog/31915.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_secretsmanager_secret_rotation: Fix `InvalidParameterException: You cannot specify both rotation frequency and schedule expression together` errors on resource Update
+```
diff --git a/internal/service/secretsmanager/secret_rotation.go b/internal/service/secretsmanager/secret_rotation.go
@@ -182,7 +182,6 @@ func resourceSecretRotationDelete(ctx context.Context, d *schema.ResourceData, m
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).SecretsManagerConn(ctx)
 
-	log.Printf("[DEBUG] Deleting Secrets Manager Rotation: %s", d.Id())
 	_, err := conn.CancelRotateSecretWithContext(ctx, &secretsmanager.CancelRotateSecretInput{
 		SecretId: aws.String(d.Get("secret_id").(string)),
 	})
@@ -224,7 +223,8 @@ func flattenRotationRules(rules *secretsmanager.RotationRulesType) []interface{}
 
 	m := map[string]interface{}{}
 
-	if v := rules.AutomaticallyAfterDays; v != nil {
+	if v := rules.AutomaticallyAfterDays; v != nil && rules.ScheduleExpression == nil {
+		// Only populate automatically_after_days if schedule_expression is not set, otherwise we won't be able to update the resource
 		m["automatically_after_days"] = int(aws.Int64Value(v))
 	}
 

diff --git a/internal/service/secretsmanager/secret_rotation_test.go b/internal/service/secretsmanager/secret_rotation_test.go
@@ -109,6 +109,52 @@ func TestAccSecretsManagerSecretRotation_scheduleExpression(t *testing.T) {
 	})
 }
 
+func TestAccSecretsManagerSecretRotation_scheduleExpressionHours(t *testing.T) {
+	ctx := acctest.Context(t)
+	var secret secretsmanager.DescribeSecretOutput
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_secretsmanager_secret_rotation.test"
+	lambdaFunctionResourceName := "aws_lambda_function.test1"
+	scheduleExpression := "rate(6 hours)"
+	scheduleExpression02 := "rate(10 hours)"
+	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{
+			// Test creating secret rotation resource
+			{
+				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),
+					testSecretValueIsCurrent(ctx, rName),
+				),
+			},
+			{
+				Config: testAccSecretRotationConfig_scheduleExpression(rName, scheduleExpression02),
+				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", scheduleExpression02),
+				),
+			},
+			// Test importing secret rotation
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccSecretsManagerSecretRotation_duration(t *testing.T) {
 	ctx := acctest.Context(t)
 	var secret secretsmanager.DescribeSecretOutput
@@ -358,3 +404,36 @@ resource "aws_secretsmanager_secret_rotation" "test" {
 }
 `, rName, automaticallyAfterDays, duration))
 }
+
+func testSecretValueIsCurrent(ctx context.Context, rName string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		conn := acctest.Provider.Meta().(*conns.AWSClient).SecretsManagerConn(ctx)
+		// Write secret value to clear in-rotation state, otherwise updating the secret rotation
+		// will fail with "A previous rotation isn't complete. That rotation will be reattempted."
+		put_secret_input := &secretsmanager.PutSecretValueInput{
+			SecretId:     aws.String(rName),
+			SecretString: aws.String("secret-value"),
+		}
+		_, err := conn.PutSecretValueWithContext(ctx, put_secret_input)
+		if err != nil {
+			return err
+		}
+		input := &secretsmanager.DescribeSecretInput{
+			SecretId: aws.String(rName),
+		}
+		output, err := conn.DescribeSecretWithContext(ctx, input)
+		if err != nil {
+			return err
+		} else {
+			// Ensure that the current version of the secret is in the AWSCURRENT stage
+			for _, stage := range output.VersionIdsToStages {
+				if *stage[0] == "AWSCURRENT" {
+					return nil
+				} else {
+					return fmt.Errorf("Secret version is not in AWSCURRENT stage: %s", *stage[0])
+				}
+			}
+			return nil
+		}
+	}
+}