feat(elbv2): Implement IConnectable to NLB #28494

WinterYukky · 2023-12-26T16:01:06Z

Summary

Implement an IConnectable interface to a NetworkLoadBalancer.

Why need this change?

AWS CDK has great features for abstraction. IConnectable interface is one of this. IConnectable simplifies the management of security groups. AWS CDK add support security group to NLB at #27978. However, Currently NLB not implement IConnectable, so customers can't use useful interface in AWS CDK.

Example use case

declare const lb: elbv2.NetworkLoadBalancer;
declare const asg: autoscaling.AutoScalingGroup;

asg.connections.allowFrom(lb, ec2.Port.tcp(8088)); // currently can not

Closes #26735

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.

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts

WinterYukky · 2023-12-26T16:24:00Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/nlb/network-load-balancer.ts

+      securityGroups: Lazy.list({
+        produce: () => this.connections.securityGroups.length >= 1 ? this.connections.securityGroups.map(sg => sg.securityGroupId) : undefined,
+      }),


If securityGroups becomes an empty array from undefined, an update will be applied and deployment will not be possible, so in the case of an empty array, it is undefined for backwards compatibility.
ref: https://docs.aws.amazon.com/elasticloadbalancing/latest/network/load-balancer-security-groups.html#security-group-considerations

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

WinterYukky · 2023-12-28T02:22:52Z

packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.nlb.ts

+const backend = new elbv2.ApplicationLoadBalancer(stack, 'Backend', {
+  vpc,
+});
+backend.addListener('Listener', {
+  protocol: elbv2.ApplicationProtocol.HTTP,
+  defaultAction: elbv2.ListenerAction.fixedResponse(200, {
+    contentType: 'application/json',
+    messageBody: JSON.stringify({
+      Message: 'I am ALB!',
+    }),
+  }),
+});


I changed target to ALB from IP to test reachability client -> nlb -> backend. This test can check security group settings via http api call.

lpizzinidev

Thanks 👍
I left some suggestions for adjustments, feel free to comment on those.

packages/@aws-cdk/cli-lib-alpha/THIRD_PARTY_LICENSES

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/nlb/network-load-balancer.ts

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts

…k-load-balancer.ts Co-authored-by: Luca Pizzini <[email protected]>

…balancer.test.ts Co-authored-by: Luca Pizzini <[email protected]>

…k-load-balancer.ts Co-authored-by: Luca Pizzini <[email protected]>

This reverts commit 0aa750c.

WinterYukky · 2024-01-05T07:44:38Z

@lpizzinidev Thanks your reviewing!!
Your comments are great and I fixed the codes lined your comments. Could you re:review?
(feature flag about create default security group is out of this topic so I'll create another PR)

lpizzinidev

Nice 👍
I left some comments for a final cleanup and adjustments.

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/nlb/network-load-balancer.ts

…k-load-balancer.ts Co-authored-by: Luca Pizzini <[email protected]>

lpizzinidev

Thanks 👍

…after initialization

aws-cdk-automation · 2024-01-16T01:34:24Z

AWS CodeBuild CI Report

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

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

paulhcsun

Thanks for the contribution @WinterYukky! This will be a very useful addition for the community.

As always, thanks for reviewing @lpizzinidev!

paulhcsun

Thanks for the contribution @WinterYukky! This will be a very useful addition for the community.

As always, thanks for reviewing @lpizzinidev!

mergify · 2024-01-16T01:43: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).

…by default (under feature flag) (#34675) ### 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. ```ts 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. #27978 #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:networkLoadBalancerWithSecurityGroupByDefault` feature 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](#34606 (comment)) 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. ```ts 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](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

WinterYukky added 2 commits December 26, 2023 15:26

feat(elbv2): Implement IConnectable to NLB

8c2d773

test(elbv2): remove unnecessary test

c2fc279

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Dec 26, 2023

aws-cdk-automation requested a review from a team December 26, 2023 16:01

github-actions bot added the admired-contributor [Pilot] contributed between 13-24 PRs to the CDK label Dec 26, 2023

aws-cdk-automation previously requested changes Dec 26, 2023

View reviewed changes

docs(elbv2): add description of NLB using SecurityGroup

0b67abd

WinterYukky commented Dec 26, 2023

View reviewed changes

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts Show resolved Hide resolved

WinterYukky commented Dec 26, 2023

View reviewed changes

WinterYukky added 2 commits December 27, 2023 08:41

test(elbv2): update to using security group and assertions by http api

a9d15d2

feat(elbv2): supported imported security groups

91f9e1f

test(elbv2): fix dummy value

643584a

WinterYukky commented Dec 28, 2023

View reviewed changes

WinterYukky added 2 commits December 28, 2023 03:21

test(elbv2): add fromNetworkLoadBalancerAttributes integ test

a4b5324

test(elbv2): use nlb connections

745f931

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

github-actions bot mentioned this pull request Jan 1, 2024

Monthly PR metrics report - Dec 2023 #28544

Closed

lpizzinidev suggested changes Jan 2, 2024

View reviewed changes

aws-cdk-automation removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Jan 2, 2024

WinterYukky and others added 7 commits January 4, 2024 09:18

revert: THIRD_PARTY_LICENSES

6a19bbc

chore: mark securityGroups as deprecated

953a282

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/nlb/networ…

20ce6db

…k-load-balancer.ts Co-authored-by: Luca Pizzini <[email protected]>

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/nlb/load-…

cb68e88

…balancer.test.ts Co-authored-by: Luca Pizzini <[email protected]>

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/nlb/load-…

ab8d0a4

…balancer.test.ts Co-authored-by: Luca Pizzini <[email protected]>

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/nlb/networ…

fe33c04

…k-load-balancer.ts Co-authored-by: Luca Pizzini <[email protected]>

Merge branch 'main' into feat/elbv2/implement-IConnectable-NLB

bea84c0

WinterYukky added 4 commits January 4, 2024 16:28

chore(elbv2): backwards compatible on securityGroups

0aa750c

Revert "chore(elbv2): backwards compatible on securityGroups"

da632ef

This reverts commit 0aa750c.

fix: keep original security groups

2f68a0a

test: update integ

6c6f905

lpizzinidev suggested changes Jan 5, 2024

View reviewed changes

WinterYukky and others added 5 commits January 6, 2024 00:02

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/nlb/networ…

6e0d64e

…k-load-balancer.ts Co-authored-by: Luca Pizzini <[email protected]>

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/nlb/networ…

297b2a6

…k-load-balancer.ts Co-authored-by: Luca Pizzini <[email protected]>

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/nlb/networ…

602a1b2

…k-load-balancer.ts Co-authored-by: Luca Pizzini <[email protected]>

chore: originalSecurityGroups to boolean

70a88e5

chore: simplify securityGroups

0aff8cb

lpizzinidev approved these changes Jan 5, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 5, 2024

WinterYukky and others added 4 commits January 5, 2024 18:03

fix: security groups are undefined even if security groups are added …

500ecce

…after initialization

Merge branch 'main' into feat/elbv2/implement-IConnectable-NLB

569910f

Merge branch 'main' into feat/elbv2/implement-IConnectable-NLB

0db974c

Merge branch 'main' into feat/elbv2/implement-IConnectable-NLB

2a13aa1

paulhcsun approved these changes Jan 16, 2024

View reviewed changes

mergify bot merged commit 1e69cc6 into aws:main Jan 16, 2024

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 16, 2024

adam-clauss mentioned this pull request Oct 3, 2024

(ElasticLoadBalancingV2): Security group missing when specifying Connections rules #31644

Closed

1 task

badmintoncryer mentioned this pull request Jun 2, 2025

elestivloadbalancingv2: default security group settings for NLB (Network Load Balancer) #34606

Closed

2 tasks

badmintoncryer mentioned this pull request Jun 10, 2025

feat(elasticloadbalancingv2): create security group settings for NLB by default (under feature flag) #34675

Merged

1 task

feat(elbv2): Implement IConnectable to NLB #28494

feat(elbv2): Implement IConnectable to NLB #28494

Uh oh!

Conversation

WinterYukky commented Dec 26, 2023

Summary

Why need this change?

Example use case

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WinterYukky Dec 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WinterYukky Dec 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lpizzinidev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WinterYukky commented Jan 5, 2024

Uh oh!

lpizzinidev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lpizzinidev left a comment

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Jan 16, 2024

AWS CodeBuild CI Report

Uh oh!

paulhcsun left a comment

Choose a reason for hiding this comment

Uh oh!

paulhcsun left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Jan 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

WinterYukky Dec 26, 2023 •

edited

Loading

WinterYukky Dec 28, 2023 •

edited

Loading