-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(rds): make rds secret name configurable #13626
Merged
Merged
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
08cc369
make rds secret configurable
hedrall a0e16d7
Update packages/@aws-cdk/aws-rds/test/cluster.test.ts
hedrall ad93b90
Update packages/@aws-cdk/aws-rds/test/cluster.test.ts
hedrall 0404b1d
Update packages/@aws-cdk/aws-rds/test/instance.test.ts
hedrall 8797a19
Update packages/@aws-cdk/aws-rds/test/instance.test.ts
hedrall 7e5ee80
Update packages/@aws-cdk/aws-rds/lib/props.ts
hedrall 03fba96
delete needless assignment
hedrall d15b5e5
Update packages/@aws-cdk/aws-rds/lib/props.ts
hedrall 0cfabcb
fix comment
hedrall cae3461
fix comment
hedrall 6845b33
add readme change
hedrall 3d83a35
Clarify in the ReadMe that secretName is optional
skinny85 ca2cc88
Merge branch 'master' into make-rds-secret-configurable
mergify[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to say, I find this documentation super confusing 🤨.
This is an input property, right? So either you provide the name of the Secret, or it will be generated by CloudFormation.
What does this paragraph mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry. I did not consider this point very deeply, and used the comments for the properties of Secret itself.
https://github.com/hedrall/aws-cdk/blob/03fba9619d7f3ad4f5a27d8123ff826a0ed26bce/packages/%40aws-cdk/aws-secretsmanager/lib/secret.ts#L33-L39
I would like to look into this more, including how it relates to the Feature flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to steal the documentation from
SecretProps
, rather than fromISecret
.https://github.com/hedrall/aws-cdk/blob/03fba9619d7f3ad4f5a27d8123ff826a0ed26bce/packages/%40aws-cdk/aws-secretsmanager/lib/secret.ts#L123-L129
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
However, I just noticed that this seems to be an old specification.
I tried destroying the deployed secret and renaming the deployed secret, but in both cases the secret was deleted immediately.
I think that
cloud formation
use a flag like as cli 'force-delete-without-recovery’.So, I change comment simply.
( Should I also change the comment of SecretProps? )