fix(rds): can not lookup VPC Security Groups with fromLookup method #34906

mazyu36 · 2025-07-04T11:16:21Z

Issue # (if applicable)

N/A

Reason for this change

Currently, the fromLookup method uses DBSecurityGroups, which is designed for EC2-Classic resources.
However, since EC2-Classic was retired, this property is no longer relevant.

Reference:

Description of changes

Added VPCSecurityGroups to the fromLookup method to properly handle VPC security group lookups.

Describe any new or updated permissions being added

N/A

Description of how you validated changes

Add unit tests and an integ test.

Checklist

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

go-to-k · 2025-07-29T04:51:49Z

packages/aws-cdk-lib/aws-rds/test/instance.from-lookup.test.ts

+describe('DatabaseInstanceBase from lookup with empty DBSecurityGroups and valid VPCSecurityGroups', () => {
+  test('return correct instance info', () => {
+    // GIVEN
+    const resultObjs = [
+      {
+        'DBInstanceArn': 'arn:aws:rds:us-east-1:123456789012:db:instance-1',
+        'Endpoint.Address': 'instance-1.testserver.us-east-1.rds.amazonaws.com',
+        'Endpoint.Port': '5432',
+        'DbiResourceId': 'db-ABCDEFGHI',
+        'VPCSecurityGroups': ['sg-1', 'sg-2'],
+        'Identifier': 'instance-1',
+      },


with empty DBSecurityGroups

DBSecurityGroups is a property that ceases to exist and does not appear in the resultObjs, what is the intent of this test?

Even if this is what you intended, wouldn't it confuse the developer at a later time since it is the name of a property that doesn't exist now and is unspecified, not empty?

Thank you.
You are correct. This test is unnecessary.
I have removed it.

go-to-k

Thanks. LGTM.

vishaalmehrishi

Thank you for your contribution! I've requested some changes to ensure backward compatibility.

packages/aws-cdk-lib/aws-rds/lib/instance.ts

Pull request has been modified.

packages/aws-cdk-lib/aws-rds/lib/instance.ts

Pull request has been modified.

mergify · 2025-08-04T15:02:34Z

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).

mergify · 2025-08-04T15:12:06Z

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated.

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

vishaalmehrishi · 2025-08-04T15:19:13Z

@mazyu36 you might need to update your fork/branch with the latest from main. Mergify is not able to rebase because of permissions: https://github.com/aws/aws-cdk/pull/34906/checks?check_run_id=47345276993

mazyu36 · 2025-08-04T15:27:25Z

@Mergifyio update

mergify · 2025-08-04T15:27:30Z

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/pr-build.yml without workflows permission

Pull request has been modified.

mergify · 2025-08-04T15:50:38Z

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 · 2025-08-04T16:00:46Z

AWS CodeBuild CI Report

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

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

github-actions · 2025-08-04T16:01:23Z

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.

mazyu36 added 2 commits July 4, 2025 18:24

fix

9600719

remove DBSecurityGroups

a04981e

github-actions bot added the p2 label Jul 4, 2025

aws-cdk-automation requested a review from a team July 4, 2025 11:16

github-actions bot added the distinguished-contributor [Pilot] contributed 50+ PRs to the CDK label Jul 4, 2025

mazyu36 changed the title ~~fix(rds): can not lookup VPC Security Groups by fromLookup method~~ fix(rds): can not lookup VPC Security Groups with fromLookup method Jul 4, 2025

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jul 4, 2025

Merge branch 'main' into fix/rds-instance-lookup-sg

7999f1d

go-to-k reviewed Jul 29, 2025

View reviewed changes

remove unit test

1d286bb

go-to-k approved these changes Jul 29, 2025

View reviewed changes

aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Jul 29, 2025

github-actions bot mentioned this pull request Aug 1, 2025

Monthly PR metrics report #35128

Closed

vishaalmehrishi self-assigned this Aug 4, 2025

vishaalmehrishi previously requested changes Aug 4, 2025

View reviewed changes

packages/aws-cdk-lib/aws-rds/lib/instance.ts Show resolved Hide resolved

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Aug 4, 2025

incorporate review comment

82670c9

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Aug 4, 2025

vishaalmehrishi previously requested changes Aug 4, 2025

View reviewed changes

packages/aws-cdk-lib/aws-rds/lib/instance.ts Outdated Show resolved Hide resolved

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Aug 4, 2025

fix

a5d4c47

vishaalmehrishi previously approved these changes Aug 4, 2025

View reviewed changes

Merge branch 'main' into fix/rds-instance-lookup-sg

e7fe3ca

vishaalmehrishi approved these changes Aug 4, 2025

View reviewed changes

mergify bot merged commit ae87e26 into aws:main Aug 4, 2025
22 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2025

fix(rds): can not lookup VPC Security Groups with fromLookup method #34906

fix(rds): can not lookup VPC Security Groups with fromLookup method #34906

Uh oh!

Conversation

mazyu36 commented Jul 4, 2025

Issue # (if applicable)

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

Uh oh!

go-to-k Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

mazyu36 Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

go-to-k left a comment

Choose a reason for hiding this comment

Uh oh!

vishaalmehrishi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mergify bot commented Aug 4, 2025

Uh oh!

mergify bot commented Aug 4, 2025

Uh oh!

vishaalmehrishi commented Aug 4, 2025

Uh oh!

mazyu36 commented Aug 4, 2025

Uh oh!

mergify bot commented Aug 4, 2025

❌ Mergify doesn't have permission to update

Uh oh!

mergify bot commented Aug 4, 2025

Uh oh!

aws-cdk-automation commented Aug 4, 2025

AWS CodeBuild CI Report

Uh oh!

Uh oh!

github-actions bot commented Aug 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants