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

[RDS] Issue with credentials & Secret attachment #11040

Closed
Cloudrage opened this issue Oct 22, 2020 · 4 comments · Fixed by #11237
Closed

[RDS] Issue with credentials & Secret attachment #11040

Cloudrage opened this issue Oct 22, 2020 · 4 comments · Fixed by #11237
Assignees
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. in-progress This issue is being actively worked on. needs-triage This issue or PR still needs to be triaged.

Comments

@Cloudrage
Copy link

Cloudrage commented Oct 22, 2020

When updating a SecretsManager Secret defined in "credentials" prop of an RS Instance, we lost the attachment.

Reproduction Steps

Create an RDS Instance with "credentials" prop targeting a Secret.
Update the Secret.

What did you expect to happen?

I want to update my Secret but keep the attachment ith the DB.

What actually happened?

You lost the attachment and it's not possible to add it because we have an error :
Secret is already attached to a target.

So a workaround is to update the DB with no/or another Secret.
But in that case, DB Instance is replaced/recreated (???!!!)

Environment

  • CLI Version : 1.68.0
  • Framework Version: 6.14.8
  • Node.js Version: v12.15.0
  • OS : Linux
  • Language (Version): TypeScript

Other

In my case, I've update the DB without the prop "credentials", then, update it again with my Secret (the Original one).
But it recreate @ll my DB instances 2 times and lost datas of course...

credentials: rds.Credentials.fromSecret(SecretsManager)


This is 🐛 Bug Report

@Cloudrage Cloudrage added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 22, 2020
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Oct 22, 2020
@Cloudrage Cloudrage changed the title [RDS] Issue with credentials & attachement [RDS] Issue with credentials & Secret attachment Oct 22, 2020
@skinny85
Copy link
Contributor

@Cloudrage is this a duplicate of #10800?

@Cloudrage
Copy link
Author

Hello @skinny85 ,

Not really sure but indeed, when updating the Secret, all fields generated by the attachment was lost.

jogold added a commit to jogold/aws-cdk that referenced this issue Oct 31, 2020
…tachment

When the `excludeCharacters` prop is updated the secret is correctly
regenerated but the database is not updated because the
`MasterUserPassword` prop does not change. Moreover the connection
fields created by the attachment are lost because the secret is
regenerated but the `AWS::SecretsManager::SecretAttachment` doesn't
"rerun".

Introduce a new `fromFixedUsername()` static method in `Credentials`.
When used the `MasterUsername` prop of the database references the
username as a string instead of a dynamic reference to the username
field of the secret. Moreover the logical id of the secret is a hash of
its customization options. This way the secret gets replaced when it's
customized, the database gets updated and the attachement reruns. This
without updating the `MasterUsername` prop, avoiding a replacement of
the database.

Closes aws#11040
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Nov 2, 2020
@skinny85
Copy link
Contributor

skinny85 commented Nov 2, 2020

Ok. Let's close this one then, and move the conversation to #10800 to have it all in one place.

@skinny85 skinny85 closed this as completed Nov 2, 2020
@github-actions
Copy link

github-actions bot commented Nov 2, 2020

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

mergify bot pushed a commit that referenced this issue Nov 6, 2020
…tachment (#11237)

When the `excludeCharacters` prop is updated the secret is correctly
regenerated but the database is not updated because the
`MasterUserPassword` prop does not change. Moreover the connection
fields created by the attachment are lost because the
`AWS::SecretsManager::SecretAttachment` doesn't "rerun" (it references
the same secret).

Introduce `fromGeneratedSecret()` and `fromPassword()` static methods
in `Credentials`. When used the `MasterUsername` prop of the database
references the username as a string instead of a dynamic reference to
the username field of the secret. Moreover the logical id of the secret
is a hash of its customization options. This way the secret gets replaced
when it's customized, the database gets updated and the attachement reruns.
This without updating the `MasterUsername` prop, avoiding a replacement
of the database.

For instances that were created from a snapshot the `MasterUsername` prop
is not specified so there's no replacement risk. Add a new static method
`fromGeneratedSecret()` in `SnapshotCredentials` to replace the secret
when its customization options change (also hash in logical id).

Deprecate `Credentials.fromUsername()` but keep it as default to avoid
a breaking change that would replace existing databases that were not
created from a snapshot.

Deprecate `SnapshotCredentials.fromGeneratedPassword()` which doesn't
replace the secret when customization options are changed.

Closes #11040

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. in-progress This issue is being actively worked on. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants