(rds): unable to alter master user password when using DatabaseClusterFromSnapshot with snapshotCredentials

[MitchWijt]

Describe the bug

About 2 months ago a this PR got merged, which states that we should be able to alter the master user password of the snapshot using the snapshotCredentials prop. However when using this prop CDK creates a new secret in Secrets Manager, but the master user password still remains unchanged. It seems like the DatabaseSecret that is created is not being used.

Looking at the code of the PR, the masterUserPassword gets changed in the cfnDbCluster while also having the snapshotIdentifier prop. However the docs state to NOT use the masterUserPassword prop together with the snapshotIdentifier prop.

Expected Behavior

When using the snapshotCredentials property with rds.SnapshotCredentials.fromGeneratedSecret() inside the DatabaseClusterFromSnapshot construct. I expect the master user password to be changed to the password that is generated in Secrets Manager.

Current Behavior

A DatabaseSecret is created inside Secrets Manger, however the master user password of the snapshot remains unchanged.

Reproduction Steps

1. Create a RDS DB using Aurora Serverless using the DatabaseCluster construct in CDK
2. Create snapshot
3. Add DatabaseClusterFromSnapshot construct to CDK using the snapshotCredentials with SnapshotCredentials.fromGeneratedSecret(), and remove the DatabaseCluster construct from CDK
4. Try logging in the DB as master user using the generated DatabaseSecret that is created from the DatabaseClusterFromSnapshot

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.27.0

Framework Version

No response

Node.js Version

16

OS

Linux

Language

Typescript

Language Version

No response

Other information

No response

[corymhall]

@MitchWijt can you provide an example that I can deploy to reproduce? We have an integration test that tests this scenario and I tried to reproduce using this test and was unable to.

https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-rds/test/integ.cluster-snapshot.ts

One thing I noticed is that all DB instances have to be created before the password is updated to the new generated password.

[MitchWijt]

@corymhall The problem seems to be in the secret rotation. We have Permissions Boundary Aspects on our stack. However CDK tries to create a role for the rotating lambda function without these Aspects in a nested stack. Which is the reason it fails.

Its mentioned in this issue

But I do not see a solution for it at the moment.

As a sidenote. Since by default the DatabaseClusterFromSnapshot copies the master username and master password from the original cluster. Wouldn't it be more logical if the secret property returned from DatabaseClusterFromSnapshot would contain the correct DatabaseSecret with the same password and username as from the original cluster, unless snapshotCredentials is being used?

Currently the secret returned is unreliable since it can't be used to login to the Cluster that is created

[MitchWijt]

Should a rotation be needed in order for the password to be updated to the new generated password? If not, then the password is not being updated on my end.

Example:
1. Deploy this:

new rds.DatabaseCluster(this, `DatabaseCluster`, 
{
            defaultDatabaseName: "postgres",
            engine: rds.DatabaseClusterEngine.auroraPostgres({
                version: AuroraPostgresEngineVersion.VER_13_6,
            }),
            instances: 1,
            backup: {
                retention: Duration.days(7)
            },
            removalPolicy: RemovalPolicy.RETAIN,
            instanceProps: {
                vpc: vpc,
                instanceType: CustomInstanceType.SERVERLESS as unknown as ec2.InstanceType,
                autoMinorVersionUpgrade: true,
                publiclyAccessible: false,
                vpcSubnets: {
                    subnets: vpc.isolatedSubnets
                },
            },
            cloudwatchLogsExports: ["postgresql"],
        }
)

2. Create Snapshot

3. Create cluster from Snapshot and deploy

new rds.DatabaseClusterFromSnapshot(this, `DatabaseClusterFromSnapshot`, {
                ...samePropsAsStep1,
                snapshotCredentials: rds.SnapshotCredentials.fromGeneratedSecret('postgres', {
                    excludeCharacters: '`+{}[]()\'"/\\'
                }),
                snapshotIdentifier: props.snapshotIdentifier
            })

4. Unable to login using generated secret

[corymhall]

@MitchWijt I tried again today and it's not working for me anymore. I'm not sure if something changed, or I just messed something up the last time I tried. After looking into it more, I'm not sure how it ever could have worked. In all of my testing, including with the integration test, it always uses the previous database password (which is inline with the documentation).

It seems like this issue impacts a significant number of customers, and I've tagged it as P1, which means it should be on our near-term roadmap.

We welcome community contributions! If you are able, we encourage you to contribute (https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) a bug fix or new feature to the CDK. If you decide to contribute, please start an engineering discussion in this issue to ensure there is a commonly understood design before submitting code. This will minimize the number of review cycles and get your code merged faster.

[jogold]

I redeployed the integration test just now and indeed it doesn't work anymore... very strange. I can assure that when #20504 was merged it worked.

I now have to check if the behavior has also changed for DB instances created from snapshots.

I think the solution could be to use a custom resource to make a ModifyDBCluster call to update the master password. Not sure it is a effort/small.

[corymhall]

Should we not even try to update the password? I think if CloudFormation does not support this functionality that we should not try to hack around it.

[MitchWijt]

@corymhall I do agree with you. If it's indeed not supported, there is probably a reason for it. However, currently the secret that is returned from the snapshot has an incorrect password, even when not using the snapshotCredentials props. It should at-least help I think. If a secret gets created that has the old password which is then returned from the cluster as the secret.

Example:

1. Cluster + Secret gets created
2. Snapshot is made
3. Cluster from snapshot gets created with snapshotIdentifier + new Secret (which contains the original password)

This way users can rely on the .secret prop that is being returned from the cluster and use that in their other processes. At the moment the secret that is returned can't be used for anything, since it's incorrect.

[corymhall]

Yeah we should expose the original secret if we can, and if we can't then undefined. You may be creating a cluster from a cluster that was created via CDK and has a secret, but you also might be creating a cluster from a manually created cluster that doesn't have the password stored in a secret.

[amcquistan]

This cross referenced issue provides a minimally scoped reproducable v2 CDK project. #22199

[IllarionovDimitri]

In 2.47.0 still an issue. It will be a new password generated which doesn't fit to snapshot password. As soon as the new password is replaced with original one, which was in the stack during snapshot creation, everything works fine

[ryparker]

It seems like the current implementation of snapshotCredentials relies on an expectation that Cfn will automatically change the DB cluster password shortly after the DB cluster is restored from snapshot. From my testing this is not the case and I cannot find any Cfn documentation that states that this should be the expected behavior. Based on my testing you cannot restore a DB cluster and replace the DB password in a single deploy. I tested this with both SnapshotCredentials.fromGeneratedSecret() and SnapshotCredentials.fromPassword(). However if you apply the snapshotCredentials after deploying the DatabaseClusterFromSnapshot then the DB password will update as expected.

Yeah we should expose the original secret if we can, and if we can't then undefined.

I don't know of a way to expose the original secret when restoring from a snapshot. My understanding is that the original secret is never recreated when restoring from a snapshot. It's up to the user to remember the password, keep the original secret from being deleted, or change the database password using AWS console/RDS API.

Solutions:

A.) File this as an issue with Cfn. This will require evidence that Cfn supports restoring a DB cluster and updating the password in a single deploy.

B.) Update documentation to inform the user that they must deploy the DatabaseClusterFromSnapshot first before providing a new password via the snapshotCredentials prop.

C.) We create and maintain a custom resource that changes the DB password shortly after the DB cluster is restored. Which as Cory mentioned is not ideal.

I'm leaning towards B. Would that be acceptable?

[MitchWijt]

@ryparker
needing 2 deploys for a piece of your infra to be setup correctly seems like an anti-pattern to me. Since there are also events being pushed out whenever a cluster is created successfully, as you can see here: https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_Events.Messages.html#USER_Events.Messages.cluster

This means whenever other services depend on these events, for example event 005 which is DB instance created that they will all fail whenever a deploy finished of DatabaseClusterFromSnapshot because the password that it receives is incorrect.

in my eyes option A seems like a more viable solution that would also be and feel more intuitive for users using these constructs.

[corymhall]

I've not looked too much into this yet, but RDS just announced a new integration with secretsmanager https://aws.amazon.com/about-aws/whats-new/2022/12/amazon-rds-integration-aws-secrets-manager/

[scottbisker]

What's completely bizarre is that DatabaseInstanceFromSnapshot works as expected when providing the snapshotCredentials property. Even though Cfn clearly states that setting the master username or password is not supported https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbinstance.html#cfn-rds-dbinstance-dbsnapshotidentifier . Seems like a Cfn inconsistency to me. This event excerpt is from CloudTrail immediately after the DBInstance is created.

The Event Type is ModifyDBInstance

"eventTime": "2023-01-12T22:48:04Z",
"eventSource": "rds.amazonaws.com",
"eventName": "ModifyDBInstance",
"awsRegion": "us-west-2",
"sourceIPAddress": "cloudformation.amazonaws.com",
"userAgent": "cloudformation.amazonaws.com",
"requestParameters": {
"dBInstanceIdentifier": "xxxxxxxxxxxx",
"allocatedStorage": 1000,
"applyImmediately": true,
"masterUserPassword": "****",
"engineVersion": "8.0.28",
"allowMajorVersionUpgrade": false,
"iops": 3500
},

There is no such event after a DBClusterFromSnapshot is created.

[atali]

I am trying to restore a snapshot from prod db to my dev db, and I need to reset the password of the db with a new generated password and be saved into secretsmanager. I am stuck until this bug is fixed

[ryparker]

I've not looked too much into this yet, but RDS just announced a new integration with secretsmanager aws.amazon.com/about-aws/whats-new/2022/12/amazon-rds-integration-aws-secrets-manager

Tested this today:
1.) Created a new DBCluster with ManageMasterUserPassword: true
2.) Viewed the DBCluster's managed credentials secret in secret manager and copied the password.
3.) Created a snapshot
4.) Deleted the cluster (this also deleted the managed secret as expected/documented)
5.) Restored from the cluster snapshot (the secret was not recreated)
6.) Logged in using the original password from step 2. (this worked)

The only problem is that when ManageMasterUserPassword is enabled, you cannot provide a custom password and the original credentials secret is never recreated.

Constraints: Can't manage the master user password with AWS Secrets Manager if MasterUserPassword is specified.

DBCluster's ManageMasterUserPassword prop docs

In summary the new ManageMasterUserPassword prop seems like a new feature that we should add in addition to the existing Credential options, however it doesn't serve as a solution to this issue.

[ryparker]

I've created a Cfn issue for the desired behavior: aws-cloudformation/cloudformation-coverage-roadmap#1500

[pflueras]

It seems this bug is fixed for me. I'm using the following code:

const mainRdsCluster = new rds.DatabaseClusterFromSnapshot(this, 'testRdsCluster', {
      snapshotIdentifier: 'testRdsSnapshot',
      ...
      snapshotCredentials: rds.SnapshotCredentials.fromGeneratedSecret('admin_user'),
      ...
}

Please confirm or infirm above information.
Thank you in advance!

[MitchWijt]

@pflueras Are you able to use that secret to login to the cluster that was created from snapshot?

[pflueras]

Before: the actual master password of the cluster was given by the RDS snapshot. The password of the newly created secret (SnapshotCredentials.fromGeneratedSecret) was incorrect. I had to fix the password from the secret to get the secret functional.

After: the actual master password is given by the newly created secret (SnapshotCredentials.fromGeneratedSecret). The secret is fully functional.

I am not sure I answered to your question..

[scottbisker]

@pflueras Did you confirm in Cloudtrail that Cloudformation actually reset the cluster password after it was restored? From my experience, when restoring a DBCluster from a snapshot the original snapshot master password is retained and Cloudformation never attempts to reset the password once the cluster is restored.

[colifran]

@pflueras I've been investigating this and it seems from some initial testing that the bug is fixed for me too. I'm going to continue testing this today with some different variations, but I'm wondering if anyone else has had a chance to retry this with success too. @MitchWijt would you be able to retry on your end to see if this is now working for you?

[tim-finnigan]

Linking related issue: #21730

[github-actions]

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.