[gw api] Implement ALB target of NLB #4430

zac-nixon · 2025-11-04T04:26:16Z

Description

This is a huge PR, I'd recommend to review the individual commits for clarity. I'll also comment through out the PR. This PR adds the ability to connect an ALB Gateway to an NLB Gateway via ALB target of NLB.

Checklist

Added tests that cover your change (if possible)
Added/modified documentation as required (such as the README.md, or the docs directory)
Manually tested
Made sure the title of the PR is a good description that can go into the release notes

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Backfilled missing tests for code in same general area 🎉
Refactored something and made the world a better place 🌟

k8s-ci-robot · 2025-11-04T04:26:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zac-nixon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [zac-nixon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

zac-nixon · 2025-11-04T04:28:36Z

apis/gateway/v1beta1/targetgroupconfig_types.go

 )

-// +kubebuilder:validation:Enum=http;https;tcp
+// +kubebuilder:validation:Enum=HTTP;HTTPS;TCP


This enum validation was incorrect, which made the field unusable as the casing should be all upper case.

zac-nixon · 2025-11-04T04:29:46Z

controllers/gateway/eventhandlers/target_group_configuration_events.go

+	}
+
+	// TODO - We should probably use an indexer here, we have a task to do this.
+	if tgconfig.Spec.TargetReference.Kind != nil && *tgconfig.Spec.TargetReference.Kind == "Gateway" && h.tcpRouteEventChan != nil {


This is required to support TG config updates that impact ALB typed target groups. We need to find out all TCP Routes that reference the Gateway which the config references it. See the added tests.

zac-nixon · 2025-11-04T04:31:08Z

controllers/gateway/eventhandlers/target_group_configuration_events.go

+					if string(beRef.Name) == tgconfig.Spec.TargetReference.Name {
+
+						// The route backend ns
+						var routeNs string


This is to support optional namespace, which mandates that when the backend ns is omitted we should use the route namespace.

zac-nixon · 2025-11-04T04:32:02Z

controllers/gateway/eventhandlers/tcp_route_events.go

 		queue.Add(reconcile.Request{NamespacedName: gw})
 	}
+
+	gateways, err = gatewayutils.GetImpactedGatewaysFromBackendRefs(ctx, h.k8sClient, route.Spec.Rules, route.Namespace, constants.NLBGatewayController)


This implements updates for any Gateways that are attached to by a TCP route.

zac-nixon · 2025-11-04T04:33:06Z

controllers/ingress/group_controller.go

 	var secrets []types.NamespacedName
 	var backendSGRequired bool
 	var err error
-	var frontendNlbTargetGroupDesiredState *core.FrontendNlbTargetGroupDesiredState


I refactored the FE NLB synthesizer to read the FE NLB state from the stack. This means that I don't have to pass a bunch of state around. Instead, the synthesizer just looks within the stack.

zac-nixon · 2025-11-04T04:33:55Z

pkg/deploy/stack_deployer.go


-	if controllerName == ingressController {
+	if d.enableFrontendNLB {
+		var desiredFENLBState []*elbv2model.FrontendNlbTargetGroupDesiredState


This reads the FE NLB state from the stack, I found it easier than passing the state around the Gateway controller.

I like the change you made. Making it a resource and read it from stack is better. : )

zac-nixon · 2025-11-04T04:34:32Z

pkg/gateway/model/base_model_builder.go

 		psa.AddToStack(stack, lb.LoadBalancerARN())
 	}

+	_ = elbv2model.NewFrontendNlbTargetGroupDesiredState(stack, tgBuilder.getLocalFrontendNlbData())


This adds the FE NLB state into the stack.

zac-nixon · 2025-11-04T04:37:58Z

pkg/gateway/model/model_build_target_group.go

 	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"


I refactored this tg builder to support building target groups from either a gateway or service. the goal was to abstract backend data retrieval so we can use the same tg builder code for service and gateway based targets.

This required moving some functionality to the backend target group configurator I added into the route utils package.

This is great.

wweiwei-li · 2025-11-05T01:04:02Z

pkg/deploy/stack_deployer.go

+		var desiredFENLBState []*elbv2model.FrontendNlbTargetGroupDesiredState
+		stack.ListResources(&desiredFENLBState)
+		var frontendNLBState *elbv2model.FrontendNlbTargetGroupDesiredState
+		if len(desiredFENLBState) == 1 {


if len(desiredFENLBState) be 0, looks like we are passing nil frontendNLBState. We need make some updates to handle it in NewFrontendNlbTargetSynthesizer

good point, ill add a check to be safe. based off my current read of the code it's not possible but we should be defensive.

wweiwei-li · 2025-11-05T01:05:42Z

pkg/model/elbv2/frontend_nlb_target_group.go

+
+func NewFrontendNlbTargetGroupDesiredState(stack core.Stack, stateConfig map[string]*FrontendNlbTargetGroupState) *FrontendNlbTargetGroupDesiredState {
+	desiredState := &FrontendNlbTargetGroupDesiredState{
+		ResourceMeta: core.NewResourceMeta(stack, FrontNLBResourceId, FrontNLBResourceId),


Should the resource type be AWS::ElasticLoadBalancingV2::TargetGroup

I chose a different resource type because it's not a target group.

TargetGroups map[string]*FrontendNlbTargetGroupState type FrontendNlbTargetGroupState struct { Name string ARN core.StringToken // Port -> NLB Listener Port Port int32 TargetARN core.StringToken // TargetPort -> ALB Listener Port TargetPort int32 }

It's the struct that you created, to map tg name to the frontend nlb.

wweiwei-li · 2025-11-05T01:10:24Z

I think all your changes made to ingress look good to me. I am still reviewing gateway changes.

shraddhabang · 2025-11-05T03:16:26Z

pkg/deploy/elbv2/frontend_nlb_target_synthesizer.go

 			})

 			if err != nil {
-				s.logger.Error(err, "Got this error!")


Can we log debug error for troubleshooting to know the exact error?

I removed this because it's an expected error message (when the ALB is still being provisioned)

shraddhabang · 2025-11-05T03:18:50Z

test/e2e/ingress/vanilla_ingress_test.go

-					Name: svc.Name,
-					Port: networking.ServiceBackendPort{
-						Number: 80,
+	/*


Leftover comments. Please uncomment this.

wweiwei-li · 2025-11-05T17:56:50Z

pkg/gateway/routeutils/backend_gateway.go

+		}
+	}
+
+	if arn == "" {


I was wondering what if GetALBARN() fails. We handled when arn is not ready here. Nice

wweiwei-li · 2025-11-05T17:57:22Z

pkg/gateway/gatewayutils/gateway_utils.go

+		}
+	}
+	var err error
+	if resultSet.Len() > 0 {


should this be errorSet > 0 ?

yes 💯 let me add a unit test here to validate this path.

actually, while writing the unit tests and your comment below. I realized that this function doesn't do anything. This was detecting changes to the TCP route and trying to enqueue ALB Gateways. ALB Gateways don't need to care about changes to the NLB

wweiwei-li · 2025-11-05T17:59:22Z

controllers/gateway/eventhandlers/tcp_route_events.go

 		queue.Add(reconcile.Request{NamespacedName: gw})
 	}
+
+	gateways, err = gatewayutils.GetImpactedGatewaysFromBackendRefs(ctx, h.k8sClient, route.Spec.Rules, route.Namespace, constants.NLBGatewayController)


Should this be constants.ALBGatewayController ?

NLB is correct here because TCP routes attach to the NLB. It's a little confusing because the TCP route ends up pointing to the ALB.

wweiwei-li · 2025-11-06T19:15:09Z

/lgtm

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 4, 2025

k8s-ci-robot requested a review from M00nF1sh November 4, 2025 04:26

k8s-ci-robot requested a review from shraddhabang November 4, 2025 04:26

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 4, 2025

zac-nixon commented Nov 4, 2025

View reviewed changes

zac-nixon force-pushed the znixon/gw-api-alb-nlb-try2 branch from e85f2d2 to fd65071 Compare November 4, 2025 16:47

zac-nixon added 9 commits November 4, 2025 08:47

remove unused parameters

50b997a

refactor service interactions into its own struct

0819b45

refactor front nlb data passing to deployer

8fb9a04

working alb as target of nlb in gateway api

9bd4c27

fix enum check, the casing was incorrect

2f442de

add validation to ensure it is really an ALB gateway

76da6d4

handle tg config updates for gateway based backends

819879e

add e2e test for gateway api alb target of nlb

66fa78c

remove ununused value

2f550be

zac-nixon force-pushed the znixon/gw-api-alb-nlb-try2 branch from fd65071 to 2f550be Compare November 4, 2025 16:47

wweiwei-li reviewed Nov 5, 2025

View reviewed changes

shraddhabang reviewed Nov 5, 2025

View reviewed changes

wweiwei-li reviewed Nov 5, 2025

View reviewed changes

review fixes

5599bda

k8s-ci-robot assigned wweiwei-li Nov 6, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 6, 2025

k8s-ci-robot merged commit 88f7c1c into kubernetes-sigs:main Nov 6, 2025
9 checks passed

[gw api] Implement ALB target of NLB #4430

[gw api] Implement ALB target of NLB #4430

Conversation

zac-nixon commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Uh oh!

k8s-ci-robot commented Nov 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zac-nixon Nov 4, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wweiwei-li commented Nov 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wweiwei-li commented Nov 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zac-nixon commented Nov 4, 2025 •

edited

Loading

zac-nixon Nov 4, 2025 •

edited

Loading