AlinsRan commented on code in PR #2804:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2804#discussion_r3643412964
##########
internal/controller/utils.go:
##########
@@ -951,9 +1033,10 @@ func getListenerStatus(
}
// frontendValidation (downstream mTLS) only applies to
Terminate listeners.
- if listener.TLS.FrontendValidation != nil &&
+ // In Gateway API v1.6 it is declared at the Gateway
level (spec.tls.frontend).
+ if validation :=
frontendTLSValidationForListener(gateway, listener); validation != nil &&
(listener.TLS.Mode == nil || *listener.TLS.Mode
== gatewayv1.TLSModeTerminate) {
- validateListenerFrontendValidation(ctx, mrgc,
gateway, listener.TLS.FrontendValidation, &conditionResolvedRefs,
&conditionProgrammed)
+ validateListenerFrontendValidation(ctx, mrgc,
gateway, validation, &conditionResolvedRefs, &conditionProgrammed)
Review Comment:
Fixed. `validateListenerFrontendValidation` now also takes the `Accepted`
condition: each bad ref sets `ResolvedRefs=False` with
`InvalidCACertificateRef` / `InvalidCACertificateKind` (and `RefNotPermitted`
for cross-namespace), and when no CA ref resolves it sets
`Accepted=False/NoValidCACertificate`. (659b192a)
Note: this fixes the status conditions. The data-plane path
(`processListenerConfig`) does not yet gate cross-namespace CA refs by
ReferenceGrant, which is symmetric with the existing server `certificateRefs`
handling — I'll follow up on aligning both rather than only the CA side.
##########
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:
Addressed with a different approach — flagging for your take rather than
resolving. Instead of emitting a multi-port predicate, hostname-carrying
listeners are now excluded from the `server_port` set entirely (they are
isolated by `service.hosts`), and a `listener_port_match_mode` knob (default
off) gates port-predicate injection for the hostname-less case. So a request
accepted through the hostname listener is no longer 404'd by a foreign port
predicate. Splitting routes per listener would also work but is heavier — happy
to go that way if you'd prefer. (a5e18fff)
##########
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. `ParseRouteParentRefs` now aggregates `hostnameMismatch` /
`notAllowed` across all listeners and selects the reason after the loop,
preferring `NoMatchingListenerHostname` when a compatible listener otherwise
allows the Route — so the outcome no longer depends on listener order.
(659b192a)
--
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]
