AlinsRan commented on code in PR #2804:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2804#discussion_r3635518775


##########
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:
   Fixed in d7a05234. Listeners on a port with a conflicting `tls.mode` are now 
skipped in `ParseRouteParentRefs` (the route stays not-Accepted, reason 
`NotAllowedByListeners`) and `getAttachedRoutesForListener` returns 0 for them, 
so such a route is neither attached nor translated/programmed and 
`AttachedRoutes` is 0. Added a `ParseRouteParentRefs` unit test for the 
conflicting-port case.



##########
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:
   Fixed in d7a05234. `frontendTLSValidation` / 
`frontendTLSValidationForListener` now return nil unless `listener.Protocol == 
HTTPSProtocolType`, which covers the translator, the CA-loading path, and 
status validation. Left the indexer protocol-agnostic since it only wires 
watch→reconcile triggers on referenced CAs. Added a test asserting a TLS 
listener gets no client 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 {
+       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:
   Fixed in d7a05234 by rejecting the unsupported mode. 
`translateFrontendValidation` now returns an error for `AllowInsecureFallback`: 
APISIX `ssl.client` only exposes `ca`/`depth`/`skip_mtls_uri_regex` (setting 
`ca` turns on strict `ssl_verify_client`) and has no "cert optional / accept on 
failure" flag, so the mode isn't representable. `AllowValidOnly` and unset keep 
the strict path. Test added.



##########
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 in d7a05234. `validateListenerFrontendValidation` now counts valid CA 
refs: any invalid ref sets `ResolvedRefs=False` with `InvalidCACertificateRef` 
(unresolvable/malformed) or `InvalidCACertificateKind` (unsupported kind), and 
when none resolve it sets `Accepted=False`/`NoValidCACertificate`. A single 
valid ref keeps `Accepted=True`. Tests cover all three cases. One choice to 
flag: I mapped a wrong `Group` to `InvalidCACertificateKind` as well (same 
"unsupported resource type" bucket as kind) — happy to split it out if you'd 
prefer a distinct 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]

Reply via email to