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 <lpizzini7@gmail.com>

…balancer.test.ts Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

…k-load-balancer.ts Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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 <lpizzini7@gmail.com>

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

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 <lpizzini7@gmail.com>

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

cb68e88

…balancer.test.ts Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

ab8d0a4

…balancer.test.ts Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

fe33c04

…k-load-balancer.ts Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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 <lpizzini7@gmail.com>

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

297b2a6

…k-load-balancer.ts Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

602a1b2

…k-load-balancer.ts Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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
11 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(elbv2): Implement IConnectable to NLB #28494

feat(elbv2): Implement IConnectable to NLB #28494

WinterYukky commented Dec 26, 2023

aws-cdk-automation left a comment

WinterYukky Dec 26, 2023 •

edited

Loading

WinterYukky Dec 28, 2023 •

edited

Loading

lpizzinidev left a comment

WinterYukky commented Jan 5, 2024

lpizzinidev left a comment

lpizzinidev left a comment

aws-cdk-automation commented Jan 16, 2024

paulhcsun left a comment

paulhcsun left a comment

mergify bot commented Jan 16, 2024

feat(elbv2): Implement IConnectable to NLB #28494

feat(elbv2): Implement IConnectable to NLB #28494

Conversation

WinterYukky commented Dec 26, 2023

Summary

Why need this change?

Example use case

aws-cdk-automation left a comment

Choose a reason for hiding this comment

WinterYukky Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

WinterYukky Dec 28, 2023 • edited Loading

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

WinterYukky commented Jan 5, 2024

lpizzinidev left a comment

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jan 16, 2024

AWS CodeBuild CI Report

paulhcsun left a comment

Choose a reason for hiding this comment

paulhcsun left a comment

Choose a reason for hiding this comment

mergify bot commented Jan 16, 2024

WinterYukky Dec 26, 2023 •

edited

Loading

WinterYukky Dec 28, 2023 •

edited

Loading