AlinsRan commented on code in PR #2804:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2804#discussion_r3635519354
##########
internal/adc/translator/translator.go:
##########
@@ -66,6 +66,27 @@ func hasExplicitListenerTarget(parentRefs
[]gatewayv1.ParentReference) bool {
return false
}
+// collectServerPortMatchPorts returns the set of listener ports that should be
+// enforced through a server_port var.
+//
+// Listeners carrying a hostname are isolated by that hostname (service.hosts),
+// which is the correct discriminator when several listeners share a single
port.
+// A server_port var adds no isolation for them and actively breaks routing: it
+// pins the route to the Gateway's declared listener port, which need not equal
+// the port APISIX actually accepts the connection on (node_listen), turning
+// every request into a 404. Only hostname-less listeners rely on port-based
+// isolation, so only their ports contribute here.
+func collectServerPortMatchPorts(listeners []gatewayv1.Listener)
map[int32]struct{} {
+ ports := make(map[int32]struct{})
+ for _, listener := range listeners {
+ if listener.Hostname != nil && *listener.Hostname != "" {
Review Comment:
This looks already handled on the current branch. `addServerPortVars` emits
`server_port in (p1, p2, …)` collecting ports from all of `tctx.Listeners` (so
multiple explicit ParentRefs are preserved), not a single-port `==`. A hostname
listener's port is dropped only when the route's hostnames don't intersect it,
in which case that listener legitimately doesn't accept the route. Regression
coverage is in `TestTranslateHTTPRouteServerPortVarsByMode` (the multi-port
`in` cases, including colliding listener names across gateways).
Separately, I noticed a possible *host* collapse in `grpcroute.go`
(`service.Hosts` narrowed to the hostname listener when a hostname-less
parentRef also matches) — I left it out of this change; happy to track it as
its own issue if you agree it's real.
##########
internal/controller/utils.go:
##########
@@ -383,6 +383,10 @@ func ParseRouteParentRefs(
}
if ok, _ := routeMatchesListenerType(route, listener);
!ok {
+ // The listener exists but its protocol cannot
carry this route kind,
+ // which the spec reports as
NotAllowedByListeners. A parentRef that
+ // matches no listener at all stays
NoMatchingParent.
+ reason =
gatewayv1.RouteReasonNotAllowedByListeners
Review Comment:
Fixed in d7a05234. `ParseRouteParentRefs` now aggregates `hostnameMismatch`
/ `notAllowed` across all listeners and selects the reason after the loop
(`hostnameMismatch` > `notAllowed` > `NoMatchingParent`), so the result is
independent of listener order and a compatible-but-hostname-mismatched listener
keeps `NoMatchingListenerHostname`. Added a test that runs both listener orders
and asserts the same reason.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
