[feat gw-api]implement httproute matching and weighted target group #4241

shuqz · 2025-06-23T20:41:54Z

Description

added httproute matching logic
added precedence order logic to assign priority
added weighted target group support
sourceIp is not cover in this PR -> this will be covered in lbConfig, another PR
i am giving pathType priority as exact > regex > pathPrefix, but this can be changed

Checklist

Added tests that cover your change (if possible)
run gateway E2E and it passed
Added/modified documentation as required (such as the README.md, or the docs directory)
Manually tested

a comprohensive test example, provided two YAML (both prefers to same parentRef):
----------------------------------------------------------
metadata:
  name: http-app-5
  namespace: sample-gateway-alb
spec:
  parentRefs:
    - name: sample-gw-alb
      port: 80
  hostnames:
    - "test.a.example.com"
    - "test.example.com"
  rules: 
    - backendRefs:
        - name: echoserver
          port: 80
          weight: 70
        - name: echoserver2
          port: 80
          weight: 30
      matches:
        - path:
            type: PathPrefix
            value: /PathPrefixShouldSecondPriority
        - path:
            type: Exact
            value: /exactHighPriority
        - path:
            type: PathPrefix
            value: /PathPrefixLowPriority
----------------------------------------------------------
metadata:
  name: http-app-6
  namespace: sample-gateway-alb
spec:
  parentRefs:
    - name: sample-gw-alb
      port: 80
  hostnames:
    - "test.a.example.com"
    - "*.example.com"
    - "*.com"
  rules: # no matches specified
    - backendRefs:
        - name: echoserver
          port: 80
          weight: 50
        - name: echoserver2
          port: 80
          weight: 0
      matches:
        - path:
            type: Exact
            value: /exactPath
        - headers:
            - name: "firstMatch"
              value: "someValue"
        - path: # Regex path match
            type: RegularExpression
            value: "/v+?/users"
    - backendRefs:
        - name: echoserver2
          port: 80
      matches:
        - path: # not specify type, should be treated as pathPrefix
            value: "/pathPrefixMatch"
          headers:
            - name: "version"
              type: RegularExpression
              value: "*[0-9]a-Z*"

result:

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-06-23T20:42:04Z

Hi @shuqz. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

pkg/gateway/model/model_build_listener.go

zac-nixon · 2025-06-23T22:22:12Z

pkg/gateway/model/model_build_listener.go

+				return tgErr
 			}
+			actions := buildL4ListenerDefaultActions(targetGroup) // TODO: update this and weighted target group support
+			albRules = append(albRules, ingress.Rule{


It would be best to not mingle the ingress and gateway packages. Would it be possible to take some time to refactor what we need out of the ingress package into a common package.

i will take a look

zac-nixon · 2025-06-23T22:22:57Z

pkg/gateway/model/utilities.go

 	})
 }
+
+type RulePrecedence struct {


nit: consider putting this into it's own file, as to not bloat the general utilities file.

zac-nixon · 2025-06-23T22:29:30Z

pkg/gateway/model/utilities.go

+	for _, route := range routes {
+		routeNamespacedName := route.GetRouteNamespacedName().String()
+		routeCreateTimestamp := getRouteCreateTimestamp(route)
+		highestPrecedenceHostname := getHighestPrecedenceHostname(route.GetHostnames())


This is interesting. Were you following a specific specification here?

Nvm, found it here. https://gateway-api.sigs.k8s.io/reference/spec/#httproute

I guess I need some clarification, because I only see these specific rules under the GRPC route.
https://gateway-api.sigs.k8s.io/reference/spec/#grpcroutespec

synced offline regarding this, added the missing piece in new commit

pkg/gateway/routeutils/route_rule_condition.go

zac-nixon · 2025-06-24T20:10:13Z

pkg/gateway/routeutils/route_rule_condition.go

+	if pathType == gwv1.PathMatchPathPrefix {
+		// the paths `/abc`, `/abc/`, and `/abc/def` would all match the prefix `/abc`, but the path `/abcd` would not.
+		if pathValue == "/" {
+			pathValue = "/*"


Can you explain why this is needed? I was checking against the Ingress implementation and we translate / -> / without adding the wildcard.

i was thinking, when pathType and value is not specified, it will be automatically assigned as prefix and /. and in this case, we should pass /* to ALB to present that this allows all path.

// +optional // +kubebuilder:default={type: "PathPrefix", value: "/"}

Is this what you're referring to?

// If no matches are specified, the default is a prefix // path match on "/", which has the effect of matching every // HTTP request.

synced offline

zac-nixon · 2025-06-24T20:13:56Z

pkg/gateway/routeutils/route_rule_precedence.go

+	})
+}
+
+func getHostnameListPrecedenceOrder(hostnameOne, hostnameTwo []string) int {


nit: adding a comment stating that the behavior is to NOT tiebreak when lists are not equal length but have the same elements, e.g

routeA: hostname: - "test.example.com" -> highest - "test.a.com" - "*.a.com" routeB: hostname: - "test.example.com"

ah i have a comment regarding this before return 0 at the end, but i can add a general comment for this function

NIT : func getHostnameListPrecedenceOrder(hostnameListOne, hostnameListTwo []string)

zac-nixon · 2025-06-24T23:34:54Z

/lgtm

shraddhabang · 2025-06-25T21:07:16Z

pkg/gateway/routeutils/route_rule_precedence.go

+
+	for _, route := range routes {
+		routeNamespacedName := route.GetRouteNamespacedName().String()
+		routeCreateTimestamp := getRouteCreateTimestamp(route)


NIT : Add this getRouteCreateTimestamp as part of routeMetadataDescriptor interface as its part of metadata of the routes.

type routeMetadataDescriptor interface { GetRouteNamespacedName() types.NamespacedName GetRouteKind() RouteKind GetHostnames() []gwv1.Hostname GetParentRefs() []gwv1.ParentReference GetRawRoute() interface{} GetBackendRefs() []gwv1.BackendRef GetRouteGeneration() int64 GetRouteCreateTimestamp() time.Time // Add this method }

pkg/gateway/model/model_build_listener.go

pkg/gateway/routeutils/route_rule_precedence.go

shraddhabang · 2025-06-26T00:08:29Z

pkg/gateway/model/model_build_listener.go

+// this is only for L7 ALB
 func (l listenerBuilderImpl) buildListenerRules(stack core.Stack, ls *elbv2model.Listener, lb *elbv2model.LoadBalancer, securityGroups securityGroupOutput, gw *gwv1.Gateway, port int32, lbCfg elbv2gw.LoadBalancerConfiguration, routes map[int32][]routeutils.RouteDescriptor) error {
+	// sort all rules based on precedence
+	routesWithPrecedenceOrder := routeutils.SortAllRulesByPrecedence(routes[port])


NIT: renameroutesWithPrecedenceOrder to rulesWithPrecedenceOrder

shraddhabang · 2025-06-26T00:14:37Z

pkg/gateway/model/model_build_listener.go

+		rule := ruleWithPrecedence.Rule
+		var hostnamesStringList []string
+		// add hostname to condition
+		if hostnames := route.GetHostnames(); len(hostnames) > 0 {


We have this info already in ruleWithPrecedence. No need to create hostnamesStringList again from route.
Replace this with hostnamesStringList :=ruleWithPrecedence.Hostnames

pkg/gateway/routeutils/route_rule_condition.go

shuqz · 2025-06-26T23:43:41Z

synced with shraddhabang we will defer validation check to API directly, resolving all validation related comments.

add weighted target group support and factor previous commit address feedbacks and fix bug in prefix path build

k8s-ci-robot · 2025-06-27T00:37:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: shuqz, 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-06-27T00:37:23Z

/lgtm

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

k8s-ci-robot requested review from oliviassss and zac-nixon June 23, 2025 20:42

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 23, 2025

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 23, 2025

zac-nixon reviewed Jun 23, 2025

View reviewed changes

shuqz force-pushed the shuqz-httproute branch from 353d19f to 16a5588 Compare June 24, 2025 17:59

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 24, 2025

shuqz changed the title ~~[feat gw-api]implement httproute matching~~ [feat gw-api]implement httproute matching and weighted target group Jun 24, 2025

zac-nixon reviewed Jun 24, 2025

View reviewed changes

zac-nixon approved these changes Jun 24, 2025

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2025

k8s-ci-robot assigned zac-nixon Jun 24, 2025

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

shraddhabang reviewed Jun 25, 2025

View reviewed changes

pkg/gateway/model/model_build_listener.go Outdated Show resolved Hide resolved

shraddhabang reviewed Jun 25, 2025

View reviewed changes

pkg/gateway/routeutils/route_rule_precedence.go Show resolved Hide resolved

shraddhabang reviewed Jun 26, 2025

View reviewed changes

pkg/gateway/routeutils/route_rule_condition.go Show resolved Hide resolved

shraddhabang reviewed Jun 26, 2025

View reviewed changes

pkg/gateway/routeutils/route_rule_condition.go Show resolved Hide resolved

shraddhabang reviewed Jun 26, 2025

View reviewed changes

pkg/gateway/routeutils/route_rule_condition.go Show resolved Hide resolved

[feat gw-api]implement httproute matching

9bb29d4

add weighted target group support and factor previous commit address feedbacks and fix bug in prefix path build

shuqz force-pushed the shuqz-httproute branch from 16a5588 to 9bb29d4 Compare June 27, 2025 00:26

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 27, 2025

zac-nixon approved these changes Jun 27, 2025

View reviewed changes

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

k8s-ci-robot merged commit bc93298 into kubernetes-sigs:main Jun 27, 2025
9 checks passed

shuqz mentioned this pull request Jul 30, 2025

🥳 aws-load-balancer-controller v2.13.4 Automated Release! 🥑 aws/eks-charts#1235

Merged

[feat gw-api]implement httproute matching and weighted target group #4241

[feat gw-api]implement httproute matching and weighted target group #4241

Uh oh!

Conversation

shuqz commented Jun 23, 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 Jun 23, 2025

Uh oh!

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!

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 commented Jun 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shuqz commented Jun 26, 2025

Uh oh!

k8s-ci-robot commented Jun 27, 2025

Uh oh!

zac-nixon commented Jun 27, 2025

Uh oh!

Uh oh!

Uh oh!

shuqz commented Jun 23, 2025 •

edited

Loading