router: respecting HTTPRoute listener by avinxshKD · Pull Request #1517 · volcano-sh/kthena

avinxshKD · 2026-08-06T10:21:48Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

HTTPRoute matching was scoped to the Gateway, not the listener handling the request. This allowed a route with parentRefs[].sectionName to match requests received by another listener on the same Gateway.

This change passes the selected listener name through the request context and filters HTTPRoutes before hostname and path matching. Listener-scoped routes also fail closed when listener context is missing.

Which issue(s) this PR fixes:

Fixes #1476

Bug evidence (required for bug-related PRs):

Reproduction steps and the affected Gateway/HTTPRoute configuration are documented in #1476.

Before this change, listener selection stored only the Gateway key:

kthena/cmd/kthena-router/app/router.go

Line 258 in 5d19b80

c.Set(router.GatewayKey, matched.GatewayKey)

Request matching then loaded every HTTPRoute indexed under that Gateway:

kthena/pkg/kthena-router/router/httproute_match.go

Lines 73 to 74 in 5d19b80

    
           func (r *Router) findHTTPRouteMatch(c *gin.Context, gatewayKey string) (httpRouteMatchResult, bool) { 
        
           	httpRoutes := r.store.GetHTTPRoutesByGateway(gatewayKey)

For a Gateway with public and private listeners, a route attached with sectionName: private could therefore participate in matching on public when its hostname and path matched.

The selected listener is now added to the request context:

kthena/cmd/kthena-router/app/router.go

Lines 258 to 259 in c3da190

    
           c.Set(router.GatewayKey, matched.GatewayKey) 
        
           c.Set(router.GatewayListenerNameKey, matched.ListenerName)

Routes are filtered against the matching Gateway parentRef and sectionName before normal request matching:

kthena/pkg/kthena-router/router/httproute_match.go

Lines 84 to 118 in c3da190

    
           		if !httpRouteMatchesGatewayListenerName(route, gatewayKey, listenerName) { 
        
           			continue 
        
           		} 
        
           		if !matchHTTPRouteHostnames(route.Spec.Hostnames, c.Request.Host) { 
        
           			continue 
        
           		} 
        
           		result, matched := findBestHTTPRouteRuleMatch(route, c.Request.URL.Path) 
        
           		if matched { 
        
           			return result, true 
        
           		} 
        
           	} 
        
           	return httpRouteMatchResult{}, false 
        
           } 
        
           func httpRouteMatchesGatewayListenerName(route *gatewayv1.HTTPRoute, gatewayKey, listenerName string) bool { 
        
           	for _, parentRef := range route.Spec.ParentRefs { 
        
           		if parentRef.Group != nil && string(*parentRef.Group) != gatewayv1.GroupName { 
        
           			continue 
        
           		} 
        
           		if parentRef.Kind != nil && *parentRef.Kind != "Gateway" { 
        
           			continue 
        
           		} 
        
           		gatewayNamespace := route.Namespace 
        
           		if parentRef.Namespace != nil { 
        
           			gatewayNamespace = string(*parentRef.Namespace) 
        
           		} 
        
           		if gatewayNamespace+"/"+string(parentRef.Name) != gatewayKey { 
        
           			continue 
        
           		} 
        
           		if parentRef.SectionName == nil || string(*parentRef.SectionName) == listenerName { 
        
           			return true 
        
           		} 
        
           	} 
        
           	return false 
        
           }

This was identified by tracing the production request path. It has not been reproduced in a cluster.

Special notes for your reviewer:

Tests run:

go test -p 1 -count=1 ./pkg/kthena-router/router -run TestRouter_FindHTTPRouteMatch`
go test -p 1 ./pkg/kthena-router/...
go test -p 1 ./cmd/kthena-router/app

Does this PR introduce a user-facing change?:

Kthena router now respects HTTPRoute parentRefs sectionName during request matching

Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

avinxshKD · 2026-08-06T10:22:46Z

@LiZhenCheng9527 @acsoto PTAL, this keeps listener-scoped HTTPRoutes from matching traffic on other listeners of the same Gateway.

acsoto · 2026-08-06T12:33:37Z

+		}
+		if gatewayNamespace+"/"+string(parentRef.Name) != gatewayKey {
+			continue
+		}


ParentReference.Port is already honored by the controller, but this request-time check ignores it. A route targeting port 8081 can still match traffic on another port.

listener port is now added to the request context and checked during route matching.

acsoto · 2026-08-06T12:33:44Z

+		}
+		if gatewayNamespace+"/"+string(parentRef.Name) != gatewayKey {
+			continue
+		}


An unscoped parentRef is still treated as accepted by every listener. Since the controller stores the route when any listener accepts it, it can match on another listener whose allowedRoutes rejects it.

Agree, reconciliation now stores explicit parentRefs only for listeners that accepted the route.

LiZhenCheng9527 · 2026-08-06T12:57:20Z


+func httpRouteMatchesGatewayListenerName(route *gatewayv1.HTTPRoute, gatewayKey, listenerName string) bool {
+	for _, parentRef := range route.Spec.ParentRefs {
+		if parentRef.Group != nil && string(*parentRef.Group) != gatewayv1.GroupName {


nit: The comparison of Group and Kind should be abstracted out to create a new function.

Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

avinxshKD · 2026-08-06T16:37:23Z

@LiZhenCheng9527 @acsoto thanks for revewing sir. I have updated, ptal when get chance.

acsoto · 2026-08-07T03:03:53Z

e2e fail

avinxshKD · 2026-08-07T05:50:23Z

e2e fail

i feel the failure is flaky and not caused by this change

YaoZengzeng · 2026-08-07T08:10:10Z


 			c.Set(router.GatewayKey, matched.GatewayKey)
+			c.Set(router.GatewayListenerNameKey, matched.ListenerName)
+			c.Set(router.GatewayListenerPortKey, int(matched.Port))


Good — this is the missing half of the fix from #1476. I confirmed matched.ListenerName / matched.Port are populated directly from listener.Name / listener.Port (router.go:479-480), which is the exact same source the controller writes into the stored parentRef's SectionName / Port. So the request-side equality check in httpRouteMatchesGatewayListener compares like-for-like values and won't spuriously mismatch.

thankyou so much sir😄

YaoZengzeng · 2026-08-07T08:10:10Z

+			acceptedParentRef := parentRef
+			sectionName := listener.Name
+			port := listener.Port
+			acceptedParentRef.SectionName = &sectionName


Correct Gateway API semantics: a parentRef without sectionName is expanded into one accepted parentRef per accepting listener, each stamped with the concrete SectionName+Port. This is what lets request-time matching be listener-scoped. Two things I verified are safe: (1) the datastore Gateway index keys off parentRef.Name+namespace (store.go:2222-2235), both preserved, so lookups still resolve; (2) syncHandler reads httpRoute.Spec.ParentRefs from the original informer object, not this rewritten storedRoute copy, and there's no HTTPRoute status writer consuming the mutated refs — so stamping synthetic sectionName/port has no status side effect.

YaoZengzeng · 2026-08-07T08:10:10Z

+		if gatewayNamespace+"/"+string(parentRef.Name) != gatewayKey {
+			continue
+		}
+		if listenerName == "" && listenerPort == 0 {


Fail-closed logic is sound. Because the controller now always stamps SectionName+Port on every stored route, this SectionName == nil && Port == nil branch can only be true for a legacy/unprocessed route — so when listener context is missing, nothing matches, which is the safe default. Nicely covered by the skips unprocessed route with listener context and skips listener-scoped route without listener context test cases. Verified the full router + controller + app test suites pass.

YaoZengzeng · 2026-08-07T08:10:10Z

+	return false
+}
+
+func isGatewayParentRef(parentRef gatewayv1.ParentReference) bool {


Minor (non-blocking): isGatewayParentRef is now defined identically in three packages — here, controller (httproute_controller.go), and datastore (store.go:2244). Consider hoisting a single shared helper to keep the parentRef group/kind semantics from drifting between the accept path and the match path. Fine to defer.

sir i feel, ill defer the shared-helper cleanup to keep this fix scoped.

avinxshKD · 2026-08-07T12:16:53Z

Thankyou so much for revewing @YaoZengzeng
can it be approved? if all good 😄

cc @LiZhenCheng9527 @acsoto

acsoto · 2026-08-08T07:18:12Z

/lgtm

avinxshKD · 2026-08-10T04:22:31Z

@LiZhenCheng9527

LiZhenCheng9527 · 2026-08-10T06:17:46Z

/approve

volcano-sh-bot · 2026-08-10T06:17:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LiZhenCheng9527

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~cmd/OWNERS~~ [LiZhenCheng9527]
~~pkg/OWNERS~~ [LiZhenCheng9527]

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

router: respecting HTTPRoute listener

c3da190

Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>

Copilot AI lite review requested due to automatic review settings August 6, 2026 10:21

volcano-sh-bot added the kind/bug label Aug 6, 2026

volcano-sh-bot requested review from LiZhenCheng9527 and hzxuzhonghu August 6, 2026 10:21

Copilot AI reviewed Aug 6, 2026

volcano-sh-bot added the size/M label Aug 6, 2026

acsoto reviewed Aug 6, 2026

View reviewed changes

LiZhenCheng9527 reviewed Aug 6, 2026

View reviewed changes

router: handle HTTPRoute listener constraints

eb36077

Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>

Copilot AI review requested due to automatic review settings August 6, 2026 16:33

Copilot AI reviewed Aug 6, 2026

volcano-sh-bot added size/L and removed size/M labels Aug 6, 2026

avinxshKD closed this Aug 6, 2026

avinxshKD reopened this Aug 6, 2026

YaoZengzeng reviewed Aug 7, 2026

View reviewed changes

volcano-sh-bot assigned acsoto Aug 8, 2026

volcano-sh-bot added the lgtm label Aug 8, 2026

volcano-sh-bot added the approved label Aug 10, 2026

volcano-sh-bot merged commit de4800e into volcano-sh:main Aug 10, 2026
28 of 30 checks passed

	func (r Router) findHTTPRouteMatch(c gin.Context, gatewayKey string) (httpRouteMatchResult, bool) {
	httpRoutes := r.store.GetHTTPRoutesByGateway(gatewayKey)

	c.Set(router.GatewayKey, matched.GatewayKey)
	c.Set(router.GatewayListenerNameKey, matched.ListenerName)

	if !httpRouteMatchesGatewayListenerName(route, gatewayKey, listenerName) {
	continue
	}
	if !matchHTTPRouteHostnames(route.Spec.Hostnames, c.Request.Host) {
	continue
	}
	result, matched := findBestHTTPRouteRuleMatch(route, c.Request.URL.Path)
	if matched {
	return result, true
	}
	}
	return httpRouteMatchResult{}, false
	}

	func httpRouteMatchesGatewayListenerName(route *gatewayv1.HTTPRoute, gatewayKey, listenerName string) bool {
	for _, parentRef := range route.Spec.ParentRefs {
	if parentRef.Group != nil && string(*parentRef.Group) != gatewayv1.GroupName {
	continue
	}
	if parentRef.Kind != nil && *parentRef.Kind != "Gateway" {
	continue
	}
	gatewayNamespace := route.Namespace
	if parentRef.Namespace != nil {
	gatewayNamespace = string(*parentRef.Namespace)
	}
	if gatewayNamespace+"/"+string(parentRef.Name) != gatewayKey {
	continue
	}
	if parentRef.SectionName == nil \|\| string(*parentRef.SectionName) == listenerName {
	return true
	}
	}
	return false
	}

Uh oh!

Conversation

avinxshKD commented Aug 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

avinxshKD commented Aug 6, 2026

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

avinxshKD commented Aug 6, 2026

Uh oh!

acsoto commented Aug 7, 2026

Uh oh!

avinxshKD commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

avinxshKD commented Aug 7, 2026

Uh oh!

acsoto commented Aug 8, 2026

Uh oh!

avinxshKD commented Aug 10, 2026

Uh oh!

LiZhenCheng9527 commented Aug 10, 2026

Uh oh!

volcano-sh-bot commented Aug 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

avinxshKD commented Aug 7, 2026 •

edited

Loading