aws-cdk
aws-cdk copied to clipboard
feat(elasticloadbalancingv2): default security group settings for NLB (under feature flag)
Issue # (if applicable)
Closes #34606.
Reason for this change
Currently, CDK's L2 constructs allow setting security groups for NLBs, but this requires explicit configuration.
declare const sg1: ec2.ISecurityGroup;
const lb = new elbv2.NetworkLoadBalancer(this, 'LB', {
vpc,
securityGroups: [sg1], // configure SG explicitly
});
This was not originally intended - NLB security group support was implemented later, and the current specification exists to maintain backward compatibility.
https://github.com/aws/aws-cdk/pull/27978 https://github.com/aws/aws-cdk/pull/28494
However, when comparing NLBs without security groups to NLBs with security groups configured, the latter has significantly more advantages. Furthermore, once an NLB is created without security groups, it's impossible to add security group configuration later.
Therefore, I propose using feature flags to make security group configuration the default for NLBs in CDK.
Description of changes
- Add
@aws-cdk/aws-elasticloadbalancingv2:networkLoadBalancerWithSecurityGroupByDefaultfeature flag - Create security groups by default when feature flags are enabled
Describe any new or updated permissions being added
None
Description of how you validated changes
Add both unit and integ tests
Other information
This implementation was also proposed in the issue, but it was not implemented because it was difficult to detect when referenced from other Connectables as follows case2.
declare const nlb: elbv2.INetworkLoadBalancer;
declare const other: IConnectable;
// case1
nlb.connections.allowTo(other, ec2.Port.tcp(1234));
// case2
other.connections.allowTo(nlb, ec2.Port.tcp(2181));
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
AWS CodeBuild CI Report
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: 8cb8370814ddfd0f24e8ff2df151c921e1932dc1
- Result: FAILED
- Build Logs (available for 30 days)
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
@mazyu36 Thank you for your review! I've updated the jsdoc of disableSecurityGroups. Could you please confirm the description again?
Thanks always!
@Abogical Thank you for your comments and I'm sorry for my late response. I've updated the snapshot.
Build CI has failed... I'll fix it later.
You'll need to run rosetta for the docs.
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).
@Abogical Thank you very much!
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).
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.
@mergifyio update
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
@Abogical Could you please approve again?
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).
@mergifyio update
update
☑️ Nothing to do, the required conditions are not met
- [ ]
#commits-behind > 0[📌 update requirement] - [X]
-closed[📌 update requirement] - [X]
-conflict[📌 update requirement] - [X]
queue-position = -1[📌 update requirement]
@mergifyio requeue
requeue
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically
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.