feat(rds): use user-defined security group for db user rotation function #23087

clueleaf · 2022-11-25T12:51:19Z

closes #23086

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2022-11-25T12:51:24Z

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

clueleaf · 2022-11-25T12:54:02Z

Integration test is not added because I think the modification is tested by unit tests.

mrgrain

This looks good.

Please also add or extend an integration test. E.g. packages/@aws-cdk/aws-rds/test/integ.cluster-rotation.lit.ts (although note this is an old style tests and it will need converting to the latest syntax)

https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md#integration-tests
https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md

mrgrain · 2022-11-25T14:35:24Z

packages/@aws-cdk/aws-rds/README.md

@@ -340,6 +340,7 @@ declare const instance: rds.DatabaseInstance;
 instance.addRotationSingleUser({
  automaticallyAfter: cdk.Duration.days(7), // defaults to 30 days
  excludeCharacters: '!@#$%^&*', // defaults to the set " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
+  securityGroup: mySecurityGroup, // optional, defaults to an auto-created security group


please use declare to explain what mySecurityGroup is
(like instance above this)

Thank you for the comment. Fixed!

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

Pull request has been modified.

mrgrain · 2022-11-28T13:16:53Z

@Mergifyio update

mergify · 2022-11-28T13:17:29Z

update

❌ Base branch update has failed

refusing to allow a GitHub App to create or update workflow .github/workflows/close-stale-issues.yml without workflows permission
err-code: 3088F

mrgrain · 2022-11-29T01:14:56Z

@clueleaf Something up with the build here. Can you check how that's related to your change?

clueleaf · 2022-11-29T05:05:52Z

@mrgrain
Don't fully understand but it seems like IntegTest declaration was unnecessary?

mrgrain · 2022-11-29T15:00:38Z

Don't fully understand but it seems like IntegTest declaration was unnecessary?

@clueleaf I think I know why. The .lit.ts files are also used as examples in the docs. Adding IntegTest messed up the examples parser. The proper fix would be to:

Use IntegTest
Remove the .lit part
Replace the usage of the example with an dedicated inline example

or

Create a new testfile without .lit.

But this is fine for now. Thanks for bearing with me!

mergify · 2022-11-29T15:02:07Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-11-29T16:03:11Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 63758db
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-11-29T16:03:20Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…ion (aws#23087) closes aws#23086 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

clueleaf added 3 commits November 25, 2022 21:12

add securityGroup property

6a764cd

edit readme

44bf2aa

edit unit tests

0d15756

github-actions bot added the feature-request A feature should be added or improved. label Nov 25, 2022

aws-cdk-automation requested a review from a team November 25, 2022 12:51

github-actions bot added p2 repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK labels Nov 25, 2022

aws-cdk-automation previously requested changes Nov 25, 2022

View reviewed changes

mrgrain previously requested changes Nov 25, 2022

View reviewed changes

clueleaf added 3 commits November 26, 2022 09:06

add security group declaration

f48a0dc

add unit tests for addRotationMultiUser()

4b8a4e0

add integ test for custom rotation function options

8dd91dc

clueleaf requested a review from mrgrain November 26, 2022 02:08

github-actions bot added the effort/small Small work item – less than a day of effort label Nov 26, 2022

Merge branch 'main' into feat/rotate_secret_security_group

b05e10e

mrgrain added 2 commits November 28, 2022 13:18

Merge branch 'main' into feat/rotate_secret_security_group

fca3de7

Merge branch 'main' into feat/rotate_secret_security_group

79b6cfd

removed explicit IntegTest

163b61f

mrgrain approved these changes Nov 29, 2022

View reviewed changes

Merge branch 'main' into feat/rotate_secret_security_group

63758db

mergify bot merged commit 9d8f69e into aws:main Nov 29, 2022

clueleaf deleted the feat/rotate_secret_security_group branch November 29, 2022 22:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rds): use user-defined security group for db user rotation function #23087

feat(rds): use user-defined security group for db user rotation function #23087

clueleaf commented Nov 25, 2022 •

edited

Loading

gitpod-io bot commented Nov 25, 2022

aws-cdk-automation left a comment

clueleaf commented Nov 25, 2022

mrgrain left a comment

mrgrain Nov 25, 2022

clueleaf Nov 26, 2022

mrgrain commented Nov 28, 2022

mergify bot commented Nov 28, 2022

mrgrain commented Nov 29, 2022

clueleaf commented Nov 29, 2022

mrgrain commented Nov 29, 2022

mergify bot commented Nov 29, 2022

aws-cdk-automation commented Nov 29, 2022

mergify bot commented Nov 29, 2022

feat(rds): use user-defined security group for db user rotation function #23087

feat(rds): use user-defined security group for db user rotation function #23087

Conversation

clueleaf commented Nov 25, 2022 • edited Loading

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Nov 25, 2022

aws-cdk-automation left a comment

Choose a reason for hiding this comment

clueleaf commented Nov 25, 2022

mrgrain left a comment

Choose a reason for hiding this comment

mrgrain Nov 25, 2022

Choose a reason for hiding this comment

clueleaf Nov 26, 2022

Choose a reason for hiding this comment

mrgrain commented Nov 28, 2022

mergify bot commented Nov 28, 2022

❌ Base branch update has failed

mrgrain commented Nov 29, 2022

clueleaf commented Nov 29, 2022

mrgrain commented Nov 29, 2022

mergify bot commented Nov 29, 2022

aws-cdk-automation commented Nov 29, 2022

AWS CodeBuild CI Report

mergify bot commented Nov 29, 2022

clueleaf commented Nov 25, 2022 •

edited

Loading