fix(rds): publiclyAccessible=false set on an instance is ignored when cluster is placed in a public subnet

[juanheyns]

This change fixes the incorrect behavior as explained in #28037.

Using null coalescing operator in TypeScript will use the original value of publiclyAccessible set on the ServerlessV2ClusterInstanceProps.

Closes #28037.

---

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

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

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

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

[lpizzinidev]

Thanks for the contribution!
Feel free to comment on the proposed changes.
Also, the documentation for publiclyAccessible should be updated here and here.
Finally, the title should describe the bug (not the solution), as per guidelines.

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

[lpizzinidev]

@juanheyns
Thanks for the clarification on the default behavior.
If you could please update the snapshot for the integration test (the build is failing because of that) I will then approve the PR.

[juanheyns]

@juanheyns Thanks for the clarification on the default behavior. If you could please update the snapshot for the integration test (the build is failing because of that) I will then approve the PR.

Hi @lpizzinidev I can see it is failing because of the integration test, I updated it with yarn integ --update-on-failed test/aws-rds/test/integ.cluster-public-subnets.js. See 2f410b8

I am wondering if I need to configure a different account to deploy the integration test into? If I am missing something with running the test locally please point me to the documentation on how to set it up.

[lpizzinidev]

@juanheyns
It looks like the integration test snapshot has not been updated after the last changes.
It still contains the original resources.
You should build it and execute it again:

1. cd packages/@aws-cdk-testing/framework-integ
2. yarn build
3. yarn integ --update-on-failed test/aws-rds/test/integ.cluster-public-subnets.js

[juanheyns]

@juanheyns It looks like the integration test snapshot has not been updated after the last changes. It still contains the original resources. You should build it and execute it again:

1. cd packages/@aws-cdk-testing/framework-integ
2. yarn build
3. yarn integ --update-on-failed test/aws-rds/test/integ.cluster-public-subnets.js

@lpizzinidev my bad, I forgot to build the stack first.

[juanheyns]

@lpizzinidev All checks are passing now, ready to review.

[scanlonp]

The logic change for aurora cluster instance looks good to me. I think the standardization in instance is good even though the logic is the same. Good tests.
Thanks @lpizzinidev for the review comments! And thank you @juanheyns for the contribution!

[mergify]

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]

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

Pull request has been modified.

[aws-cdk-automation]

AWS CodeBuild CI Report

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

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

[mergify]

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