New resource: aws_secretsmanager_secret_rotation

[kelvin-acosta]

Context: We use this lambda for multi user secret rotation https://github.com/aws-samples/aws-secrets-manager-rotation-lambdas/blob/master/SecretsManagerRDSPostgreSQLRotationMultiUser/lambda_function.py

We currently use two resources to create a rotating secret in secrets manager.

1. aws_secretsmanager_secret
2. aws_secretsmanager_secret_version

The way the aws_secretsmanager_secret resource currently works is that you can create a secret and configure rotation for it. Creating a secret version is separate. When rotation is enabled it causes the secret to rotate once immediately. This is happening before we get to store the secret, which is where we start run into our issue.

The docs say Configuring rotation causes the secret to rotate once as soon as you store the secret. But instead it is actually true that Configuring rotation causes the secret to rotate once as soon as rotation is enabled/configured.

The lamdba then kicks off before the secret version has been created and throws an error stating that the secret version, with which the lambda kicked off, has no stage for rotation of secret because by the time the lambda is running, a secret version resource has been created and the secret version is now different.

Concerns: I had to remove the ability to enable secret rotation in the resource aws_secretsmanager_secret because then there would be two resources managing state of the rotation configuration.

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

FEATURES:

* New Resource: aws_secretsmanager_secret_rotation
* New Datasource: aws_secretsmanager_secret_rotation

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAwsSecretsManager'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAwsSecretsManager -timeout 120m
=== RUN   TestAccAwsSecretsManagerSecretRotation
=== PAUSE TestAccAwsSecretsManagerSecretRotation
=== RUN   TestAccAwsSecretsManagerSecret_Basic
=== PAUSE TestAccAwsSecretsManagerSecret_Basic
=== RUN   TestAccAwsSecretsManagerSecret_withNamePrefix
=== PAUSE TestAccAwsSecretsManagerSecret_withNamePrefix
=== RUN   TestAccAwsSecretsManagerSecret_Description
=== PAUSE TestAccAwsSecretsManagerSecret_Description
=== RUN   TestAccAwsSecretsManagerSecret_KmsKeyID
=== PAUSE TestAccAwsSecretsManagerSecret_KmsKeyID
=== RUN   TestAccAwsSecretsManagerSecret_RecoveryWindowInDays_Recreate
=== PAUSE TestAccAwsSecretsManagerSecret_RecoveryWindowInDays_Recreate
=== RUN   TestAccAwsSecretsManagerSecret_Tags
=== PAUSE TestAccAwsSecretsManagerSecret_Tags
=== RUN   TestAccAwsSecretsManagerSecret_policy
=== PAUSE TestAccAwsSecretsManagerSecret_policy
=== RUN   TestAccAwsSecretsManagerSecretVersion_BasicString
=== PAUSE TestAccAwsSecretsManagerSecretVersion_BasicString
=== RUN   TestAccAwsSecretsManagerSecretVersion_Base64Binary
=== PAUSE TestAccAwsSecretsManagerSecretVersion_Base64Binary
=== RUN   TestAccAwsSecretsManagerSecretVersion_VersionStages
=== PAUSE TestAccAwsSecretsManagerSecretVersion_VersionStages
=== CONT  TestAccAwsSecretsManagerSecretRotation
=== CONT  TestAccAwsSecretsManagerSecret_RecoveryWindowInDays_Recreate
=== CONT  TestAccAwsSecretsManagerSecret_Tags
=== CONT  TestAccAwsSecretsManagerSecretVersion_VersionStages
=== CONT  TestAccAwsSecretsManagerSecretVersion_Base64Binary
=== CONT  TestAccAwsSecretsManagerSecret_KmsKeyID
=== CONT  TestAccAwsSecretsManagerSecret_Description
=== CONT  TestAccAwsSecretsManagerSecret_withNamePrefix
=== CONT  TestAccAwsSecretsManagerSecret_Basic
=== CONT  TestAccAwsSecretsManagerSecret_policy
=== CONT  TestAccAwsSecretsManagerSecretVersion_BasicString
--- PASS: TestAccAwsSecretsManagerSecret_policy (8.71s)
--- PASS: TestAccAwsSecretsManagerSecret_withNamePrefix (9.29s)
--- PASS: TestAccAwsSecretsManagerSecret_Basic (9.63s)
--- PASS: TestAccAwsSecretsManagerSecretVersion_BasicString (9.77s)
--- PASS: TestAccAwsSecretsManagerSecretVersion_Base64Binary (10.02s)
--- PASS: TestAccAwsSecretsManagerSecret_Description (13.59s)
--- PASS: TestAccAwsSecretsManagerSecretVersion_VersionStages (18.65s)
--- PASS: TestAccAwsSecretsManagerSecret_Tags (22.40s)
--- PASS: TestAccAwsSecretsManagerSecretRotation (29.93s)
--- PASS: TestAccAwsSecretsManagerSecret_RecoveryWindowInDays_Recreate (30.98s)
--- PASS: TestAccAwsSecretsManagerSecret_KmsKeyID (34.82s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	35.943s

...

[ryancausey]

This will potentially fix #10619.

[bflad]

Hi @kelvin-acosta 👋 Thank you for submitting this.

The idea here is valid to create another resource, however just removing the current configuration options will cause major issues for existing Terraform configurations and state files, even if we introduced this change in a major version release. Terraform Providers have an explicit deprecation process which the maintainers follow, which can be seen here: https://www.terraform.io/docs/extend/best-practices/deprecations.html#provider-attribute-removal

To move this pull request forward, the recommended actions are:

• Reverting all changes to the existing data sources, resources, testing, and documentation
• Adding Deprecated: "Use the aws_secretsmanager_secret_rotation resource instead" and Computed: true to affected aws_secretmanager_secret resource attributes: rotation_enabled, rotation_lambda_arn, and rotation_rules
• Adding information about the deprecation to the aws_secretmanager_secret resource documentation in the existing Rotation Configuration section and for each of those attributes, e.g.

* `rotation_rules` - (Optional) ... existing text ... **DEPRECATED:** Use the [`aws_secretsmanager_secret_rotation` resource](/docs/providers/aws/r/secretsmanager_secret_rotation.html) instead.

Please reach out if you have any questions or do not have time to implement this.

[kelvin-acosta]

bump

[kelvin-acosta]

Is there anything else I can do here to get this moving forward?

[osulli]

@bflad and @kelvin-acosta - please can somebody, likely Brian, define what more needs doing?

Looks like there is a merge conflict, but scrolling through the posts and comments it appears everything is complete.

We are very keen to see this merged so we can start using existing lambdas with Terraform.

[bflad]

Sorry for the delay here! We do want to get this in this or next week's release.

[osulli]

Sorry for the delay here! We do want to get this in this or next week's release.

Thank you for your reply! Looking forward to it.

[kelvin-acosta]

Awesome, let me know what I can do here!

[bflad]

Thanks so much for sticking through the changes and fixing this for everyone, @kelvin-acosta 🚀 Test failures will be addressed post-merge, per other review comment.

Output from acceptance testing:

--- PASS: TestAccAwsSecretsManagerSecret_policy (16.61s)
--- PASS: TestAccAwsSecretsManagerSecret_withNamePrefix (18.88s)
--- PASS: TestAccAwsSecretsManagerSecret_Basic (19.04s)
--- PASS: TestAccAwsSecretsManagerSecret_Description (29.87s)
--- PASS: TestAccAwsSecretsManagerSecret_KmsKeyID (38.81s)
--- PASS: TestAccAwsSecretsManagerSecret_RecoveryWindowInDays_Recreate (45.96s)
    TestAccAwsSecretsManagerSecret_RotationLambdaARN: testing.go:684: Step 2 error: Check failed: Check 2/3 error: aws_secretsmanager_secret.test: Attribute 'rotation_enabled' expected "false", got "true"
--- PASS: TestAccAwsSecretsManagerSecret_Tags (52.22s)
--- FAIL: TestAccAwsSecretsManagerSecret_RotationLambdaARN (56.77s)
    TestAccAwsSecretsManagerSecret_RotationRules: testing.go:684: Step 2 error: Check failed: Check 2/3 error: aws_secretsmanager_secret.test: Attribute 'rotation_enabled' expected "false", got "true"
--- FAIL: TestAccAwsSecretsManagerSecret_RotationRules (64.77s)

--- PASS: TestAccAwsSecretsManagerSecretRotation (60.53s)

--- PASS: TestAccDataSourceAwsSecretsManagerSecretRotation_Basic (49.62s)

--- PASS: TestAccDataSourceAwsSecretsManagerSecret_Basic (8.24s)
--- PASS: TestAccDataSourceAwsSecretsManagerSecret_ARN (20.25s)
--- PASS: TestAccDataSourceAwsSecretsManagerSecret_Policy (20.68s)
--- PASS: TestAccDataSourceAwsSecretsManagerSecret_Name (21.14s)

[bflad]

Since these attributes are Computed: true now (which is required for supporting the new resource), it will no longer be possible to remove this configuration and have Terraform remove the rotation in this resource. I will add some additional verbiage in the resource documentation and CHANGELOG around this behavior change.

[ghost]

This has been released in version 2.67.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!