nic-6443 commented on code in PR #2804:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2804#discussion_r3635208927
##########
internal/controller/utils.go:
##########
@@ -836,51 +916,53 @@ func getListenerStatus(
supportedKinds = []gatewayv1.RouteGroupKind{}
)
+ // A port serving more than one TLS mode cannot be programmed,
so the
+ // listener is rejected rather than accepted with undefined
behaviour.
+ if listener.Protocol == gatewayv1.TLSProtocolType &&
tlsModeConflictPorts[listener.Port] {
+ conditionAccepted.Status = metav1.ConditionFalse
+ conditionAccepted.Reason =
string(gatewayv1.ListenerReasonProtocolConflict)
+ conditionAccepted.Message = "listeners on this port
disagree on tls.mode"
+ conditionConflicted.Status = metav1.ConditionTrue
+ conditionConflicted.Reason =
string(gatewayv1.ListenerReasonProtocolConflict)
+ conditionProgrammed.Status = metav1.ConditionFalse
+ conditionProgrammed.Reason =
string(gatewayv1.ListenerReasonInvalid)
+
+ statusArray = append(statusArray,
reuseUnchangedListenerStatus(gateway, i, gatewayv1.ListenerStatus{
+ Name: listener.Name,
+ Conditions: []metav1.Condition{
+ conditionProgrammed,
+ conditionAccepted,
+ conditionConflicted,
+ conditionResolvedRefs,
+ },
+ SupportedKinds: supportedKinds,
+ AttachedRoutes: attachedRoutes,
+ }))
+ continue
Review Comment:
[P1] This only stops listener-status construction. `ParseRouteParentRefs`
still matches these listeners from `Gateway.spec`, and the route reconcilers
call `Provider.Update` without consulting listener status, so a TLSRoute can
still be programmed even though the listeners report `Accepted=False` and
`Programmed=False`; `AttachedRoutes` also remains nonzero. Please make
listeners on a conflicting TLS-mode port ineligible during route
attachment/translation and report no attached routes so the conflict actually
prevents traffic.
##########
internal/adc/translator/gateway.go:
##########
@@ -134,26 +146,43 @@ func (t *Translator) translateSecret(tctx
*provider.TranslateContext, listener g
}
}
- // Only supported on TLSRoute. The certificateRefs field is ignored in
this mode.
- case gatewayv1.TLSModePassthrough:
- return sslObjs, nil
default:
- return nil, fmt.Errorf("unknown TLS mode %s",
*listener.TLS.Mode)
+ return nil, fmt.Errorf("unknown TLS mode %s", mode)
}
return sslObjs, nil
}
-// translateFrontendValidation builds the downstream mTLS client configuration
from a
-// listener's frontendValidation. The referenced CA certificates (ConfigMap,
key `ca.crt`)
-// are bundled into a single trust anchor used to validate client certificates.
+// frontendTLSValidation resolves the Gateway-level frontend TLS client-cert
+// validation that applies to the given HTTPS listener. In Gateway API v1.6,
+// frontendValidation moved from the per-listener TLS config to the
Gateway-level
+// spec.tls.frontend: Default applies to all HTTPS listeners, and a PerPort
entry
+// overrides it for listeners on the matching port.
+func frontendTLSValidation(obj *gatewayv1.Gateway, listener
gatewayv1.Listener) *gatewayv1.FrontendTLSValidation {
+ if obj.Spec.TLS == nil || obj.Spec.TLS.Frontend == nil {
+ return nil
+ }
+ frontend := obj.Spec.TLS.Frontend
+ for i := range frontend.PerPort {
+ if frontend.PerPort[i].Port == listener.Port {
+ return frontend.PerPort[i].TLS.Validation
+ }
+ }
+ return frontend.Default.Validation
+}
+
+// translateFrontendValidation builds the downstream mTLS client configuration
from the
+// Gateway's frontendValidation that applies to the listener. The referenced CA
+// certificates (ConfigMap, key `ca.crt`) are bundled into a single trust
anchor used
+// to validate client certificates.
func (t *Translator) translateFrontendValidation(tctx
*provider.TranslateContext, listener gatewayv1.Listener, obj
*gatewayv1.Gateway) (*adctypes.ClientClass, error) {
- if listener.TLS.FrontendValidation == nil ||
len(listener.TLS.FrontendValidation.CACertificateRefs) == 0 {
+ validation := frontendTLSValidation(obj, listener)
Review Comment:
[P2] `validation.Mode` is ignored. With `AllowInsecureFallback`, this path
still builds the same CA-backed `ClientClass` as `AllowValidOnly`, so APISIX
rejects clients that omit a certificate or fail verification, which is the
opposite of the requested mode. Please implement fallback semantics, or reject
the unsupported mode instead of silently programming strict mTLS.
##########
internal/adc/translator/gateway.go:
##########
@@ -134,26 +146,43 @@ func (t *Translator) translateSecret(tctx
*provider.TranslateContext, listener g
}
}
- // Only supported on TLSRoute. The certificateRefs field is ignored in
this mode.
- case gatewayv1.TLSModePassthrough:
- return sslObjs, nil
default:
- return nil, fmt.Errorf("unknown TLS mode %s",
*listener.TLS.Mode)
+ return nil, fmt.Errorf("unknown TLS mode %s", mode)
}
return sslObjs, nil
}
-// translateFrontendValidation builds the downstream mTLS client configuration
from a
-// listener's frontendValidation. The referenced CA certificates (ConfigMap,
key `ca.crt`)
-// are bundled into a single trust anchor used to validate client certificates.
+// frontendTLSValidation resolves the Gateway-level frontend TLS client-cert
+// validation that applies to the given HTTPS listener. In Gateway API v1.6,
+// frontendValidation moved from the per-listener TLS config to the
Gateway-level
+// spec.tls.frontend: Default applies to all HTTPS listeners, and a PerPort
entry
+// overrides it for listeners on the matching port.
+func frontendTLSValidation(obj *gatewayv1.Gateway, listener
gatewayv1.Listener) *gatewayv1.FrontendTLSValidation {
Review Comment:
[P2] `spec.tls.frontend.default` and `perPort` apply only to HTTPS listeners
in Gateway API v1.6. This helper resolves them for every protocol, while
`translateSecret` invokes it for a `TLS`/`Terminate` listener too. A mixed
HTTPS + TLS Gateway can therefore attach client-certificate verification to a
TLSRoute listener, or fail that listener because of an unrelated CA reference.
Please return nil unless `listener.Protocol == HTTPSProtocolType` and apply the
same guard in status validation.
##########
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:
[P2] `tctx.Listeners` can include both a hostname listener and a
hostname-less listener when one HTTPRoute/GRPCRoute has multiple explicit
ParentRefs. Dropping the hostname listener here leaves only the other port, but
this set is later installed as one global `server_port` predicate on every
generated APISIX route, so requests accepted through the hostname listener can
never match. Please preserve all targeted ports whenever a port predicate is
emitted, or split routes per listener.
##########
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:
[P2] The validator only mutates `ResolvedRefs` and `Programmed`, leaving
`Accepted=True`. When the sole CA ConfigMap is missing/malformed, or the Kind
is unsupported, v1.6 requires `ResolvedRefs=False` with
`InvalidCACertificateRef`/`InvalidCACertificateKind`, and because no valid CA
remains, `Accepted=False` with `NoValidCACertificate`. Please pass/update the
Accepted condition and use the CA-specific reasons.
##########
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:
[P2] For a ParentRef without `sectionName`, this assignment makes the final
reason depend on listener order: a protocol-compatible listener with a hostname
mismatch sets `NoMatchingListenerHostname`, then a later incompatible listener
overwrites it with `NotAllowedByListeners`; reversing the list gives another
result. Please aggregate candidate outcomes and select the reason after
evaluating all listeners, preserving the hostname-mismatch reason when a
compatible listener otherwise allows the Route.
--
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]
