feat(rds): add support for manageMasterUserPassword in L2 construct
Issue # (if applicable)
Closes #29239
Reason for this change
In order to make the properties in the manageMasterUserPassword of the L1 Construct CfnDBCluster configurable in the L2 Construct.
Even when using a CFN override to set manageMasterUserPassword, the L2 cluster construct would create an additional unused secret. This PR adds native support for the manageMasterUserPassword CloudFormation attribute and eliminates the extra secret that the workaround caused.
Description of changes
To ensure existing clusters wouldn't have a new cfn property added and set to false, I had to use a few conditional statements. I also wanted to make sure that the created secret supported a customer defined username and kms key.
Description of how you validated changes
I deployed a database cluster using the new code and logged into it using the managed password with custom username. I also manually triggered rotations on the secret and ensured that the newly generated secret also worked for logging into the database.
Checklist
- [ x] My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Thanks 👍 Left some comments for adjustments. Also, can you please add unit test coverage?
Done, sorry about the wait!
AWS CodeBuild CI Report
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: 02cebb7
- Result: FAILED
- Build Logs (available for 30 days)
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
This failure doesn't seem to be related to code I've committed.
This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.
Any idea when this might be merged in? I've been using the CfnDBCluster ManageMasterUserPassword work around, but suddenly today it's decided not to work any more. The unused secret seems to be attaching to the cluster first, and then when the real secret is trying to attach it blows up because there already is one. CloudFormation just gives a Internal Failure (RequestToken: 6e94374a-4094-6627-e639-c8c95a165caf) which isn't particularly helpful unfortunately.
Any idea when this might be merged in? I've been using the
CfnDBClusterManageMasterUserPasswordwork around, but suddenly today it's decided not to work any more. The unused secret seems to be attaching to the cluster first, and then when the real secret is trying to attach it blows up because there already is one. CloudFormation just gives aInternal Failure (RequestToken: 6e94374a-4094-6627-e639-c8c95a165caf)which isn't particularly helpful unfortunately.
@antonfelich I'm still waiting on a community approval, please feel free to review it!
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.
AWS CodeBuild CI Report
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: 73e742155863b5ab76217c7b70e4c9833daf9c83
- Result: SUCCEEDED
- Build Logs (available for 30 days)
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 78.67%. Comparing base (
7fdd974) to head (73e7421). Report is 375 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #30997 +/- ##
==========================================
+ Coverage 78.66% 78.67% +0.01%
==========================================
Files 107 107
Lines 7237 7237
Branches 1329 1329
==========================================
+ Hits 5693 5694 +1
+ Misses 1358 1357 -1
Partials 186 186
| Flag | Coverage Δ | |
|---|---|---|
| suite.unit | 78.67% <ø> (+0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Components | Coverage Δ | |
|---|---|---|
| packages/aws-cdk | 78.67% <ø> (+0.01%) |
:arrow_up: |
This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.