Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secret rotation for manage_master_user_password in RDS cluster #32405

Closed
eugeneoei opened this issue Jul 7, 2023 · 16 comments
Closed

Secret rotation for manage_master_user_password in RDS cluster #32405

eugeneoei opened this issue Jul 7, 2023 · 16 comments
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/secretsmanager Issues and PRs that pertain to the secretsmanager service.

Comments

@eugeneoei
Copy link

Description

hello,

in the resource aws_rds_cluster, setting the manage_master_user_password argument allows RDS to manage the master user password in Secrets Manager. the rotation schedule of this secret can be set in AWS console without the need for a rotation function which is managed by RDS.

sm-rotation-function

will it be possible to set the rotation schedule of this AWS managed secret through terraform?

References

i have looked at the resource aws_secretsmanager_secret_rotation, however the rotation_lambda_arn argument is a required field.

through CLI, it is possible to set the rotation using the following as stated here in the AWS docs:

aws secretsmanager rotate-secret \
    --secret-id MySecret \
    --rotation-rules "{\"ScheduleExpression\": \"cron(0 16 1,15 * ? *)\", \"Duration\": \"2h\"}"

Would you like to implement a fix?

None

@eugeneoei eugeneoei added the needs-triage Waiting for first response or review from a maintainer. label Jul 7, 2023
@github-actions
Copy link

github-actions bot commented Jul 7, 2023

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

@justinretzolk
Copy link
Member

Hey @eugeneoei 👋 Thank you for taking the time to raise this! It looks like this would be an enhancement to the aws_secretsmanager_secret_rotation resource to remove the requirement to pass the Lambda ARN, as the underlying SDK does not require this value (and omitting it allows for managed rotation, such as you're looking for). I'll mark this as an enhancement so that the maintainers or community can look into implementing this when it can be prioritized.

@justinretzolk justinretzolk added enhancement Requests to existing resources that expand the functionality or scope. service/secretsmanager Issues and PRs that pertain to the secretsmanager service. and removed needs-triage Waiting for first response or review from a maintainer. labels Jul 7, 2023
@eugeneoei
Copy link
Author

hi @justinretzolk , thank you for following up on this. look forward to the enhancement! 😄

@ksitnik-tc
Copy link

👍 I'm also looking to use this enhancement!

@tutunak
Copy link

tutunak commented Jul 27, 2023

It's a must have option. e.g. to disable it or to change the schedule

@jbennett-nex
Copy link
Contributor

jbennett-nex commented Jul 31, 2023

I had a crack at a PR for this at the weekend. Seems its not as simple as just making the ARN optional. In the update method there is a call to cancel the rotation if the lambda ARN is not present, this will always be the case for a managed secret.

We could do a lookup on the tag on the secret resource to check if it is managed by another service. Thoughts?

The code I am referring to. Note it calls cancel rotation if the ARN is not provided which would become an Optional field in my changes.

func resourceSecretRotationUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
	var diags diag.Diagnostics
	conn := meta.(*conns.AWSClient).SecretsManagerConn(ctx)
	secretID := d.Get("secret_id").(string)

	if d.HasChanges("rotation_lambda_arn", "rotation_rules") {
		if v, ok := d.GetOk("rotation_lambda_arn"); ok && v.(string) != "" {
			input := &secretsmanager.RotateSecretInput{
				RotationLambdaARN: aws.String(v.(string)),
				RotationRules:     expandRotationRules(d.Get("rotation_rules").([]interface{})),
				SecretId:          aws.String(secretID),
			}

			// AccessDeniedException: Secrets Manager cannot invoke the specified Lambda function.
			_, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 1*time.Minute, func() (interface{}, error) {
				return conn.RotateSecretWithContext(ctx, input)
			}, "AccessDeniedException")

			if err != nil {
				return sdkdiag.AppendErrorf(diags, "updating Secrets Manager Secret Rotation (%s): %s", d.Id(), err)
			}
		} else {
			input := &secretsmanager.CancelRotateSecretInput{
				SecretId: aws.String(d.Id()),
			}

			_, err := conn.CancelRotateSecretWithContext(ctx, input)

			if err != nil {
				return sdkdiag.AppendErrorf(diags, "cancelling Secrets Manager Secret Rotation (%s): %s", d.Id(), err)
			}
		}
	}

@jjosef
Copy link

jjosef commented Aug 17, 2023

Any news on this? I can take a crack at a PR if needed.

@saptadip
Copy link

saptadip commented Sep 6, 2023

I am also waiting for a fix. Any update?

@debu99
Copy link

debu99 commented Sep 24, 2023

any update

@arditmorina
Copy link

Me too, waiting for feature implementation

@tom2sf3
Copy link

tom2sf3 commented Nov 1, 2023

Any progress?

@jar-b
Copy link
Member

jar-b commented Dec 1, 2023

Relates #34108

I believe this issue was addressed with #34180 (released in v5.24.0), but was missed and not linked in the closing PR.

This related design decision contains a partial example which can be modified to bring the managed secret rotation under management with Terraform.

resource "aws_db_instance" "example" {
  # additional configuration omitted for brevity

  manage_master_user_password = true
}

# Use the output of the `master_user_secret` object, which includes `secret_arn`,
# to manage the rotation rules.
resource "aws_secretsmanager_secret_rotation" "example" {
  secret_id = aws_db_instance.example.master_user_secret[0].secret_arn

  rotation_rules {
    automatically_after_days = 30
  }
}

# Optionally fetch the secret data if attributes need to be used as inputs
# elsewhere.
data "aws_secretsmanager_secret" "example" {
  arn = aws_db_instance.example.master_user_secret[0].secret_arn
}

If this interpretation of the original issue is correct and the change above resolves it, please let us know and we can close this.

@debu99
Copy link

debu99 commented Dec 3, 2023

how to disable rotation?

@Petra-K63
Copy link

Hi @jar-b ,

I was waiting for this fix so thank you for the feedback as I missed to see #34180

I took your partial example specifying automatically_after_days to adapt the rotation of our managed RDS credential. Unfortunately this runs into conflicts when the rotation is manipulated via the GUI (which is allowed for a managed secret as pointed out by @eugeneoei above) because the GUI seems to use schedule_expression to set the rotation. According to secrets_manager_rotation either automatically_after_days or schedule_expression must be specified.

Therefore we tried schedule_expression instead to set the rotation and this seems to be working fine.

resource "aws_db_instance" "example" {
  # additional configuration omitted for brevity

  manage_master_user_password = true
}

# Use the output of the `master_user_secret` object, which includes `secret_arn`,
# to manage the rotation rules.
resource "aws_secretsmanager_secret_rotation" "example" {
  secret_id = aws_db_instance.example.master_user_secret[0].secret_arn

  rotation_rules {
    schedule_expression = rate(30 days)
  }
}

# Optionally fetch the secret data if attributes need to be used as inputs
# elsewhere.
data "aws_secretsmanager_secret" "example" {
  arn = aws_db_instance.example.master_user_secret[0].secret_arn
}

So with using schedule_expression the change #34180 resolves from my point of view the original issue.

@jar-b
Copy link
Member

jar-b commented Dec 6, 2023

Thanks for the correction and confirmation, @Petra-K63!

Closed by #34180.

@jar-b jar-b closed this as completed Dec 6, 2023
Copy link

github-actions bot commented Jan 6, 2024

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/secretsmanager Issues and PRs that pertain to the secretsmanager service.
Projects
None yet
Development

No branches or pull requests