Skip to content

Commit 0c3ee61

Browse files
pierDipipierDipi
authored
RHOAIENG-46780: feat(gateway): filter gateway refs based on listener allowedRoutes (opendatahub-io#771)
* feat(gateway): filter gateway refs based on listener allowedRoutes Implement filterAllowedRefs to enforce Gateway API allowedRoutes semantics: only gateway refs whose listeners permit routes from the LLMInferenceService namespace are retained. Thread the gateway object through getEffectiveGatewayRefs and getGatewayRefs so the filter has access to listener configuration. Handles All, Same, and Selector (permissive fallback since namespace labels are unavailable in the controller context). Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * test(gateway): add unit and integration tests for allowedRoutes filtering Add unit tests for filterAllowedRefs, listenerAllowsNamespace, and gatewayAllowsNamespace covering All/Same/Selector semantics, nil defaults, empty ref namespaces, mixed refs, and no-listener gateways. Add integration tests verifying that cross-namespace refs are rejected when listeners default to Same, accepted when listeners use All, and accepted for same-namespace refs. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * namespace selector Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * fix tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * lint Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * One more defensive test Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Add namespace watch Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * fix: target namespace watch and clean up cross-ns test resources Replace cluster-wide Gateway list in enqueueGatewaysFromNamespace with targeted approach: extract gateway refs from LLMInferenceServices in the changed namespace (resolving BaseRefs), then only enqueue those that exist and have Selector-based listeners. Add DeferCleanup for cross-namespace LLMInferenceService instances to prevent stale refs from affecting cluster-wide listings. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * fix: handle transient errors in namespace watch gateway lookup Only skip IsNotFound errors when looking up gateways in the namespace label change handler. Log and enqueue other errors so the reconciler can retry on transient failures. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * fix: propagate context to allowedRoutes filter functions for error logging Thread context.Context through filterAllowedRefs, gatewayAllowsNamespace, and listenerAllowsNamespace so malformed namespace selector errors are logged via the structured logger instead of silently swallowed. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * fix: clean up gateways in cross-namespace allowedRoutes tests Add DeferCleanup to delete Gateways created in cross-namespace tests to prevent interference via cluster-wide watches. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
1 parent ef7b85b commit 0c3ee61

File tree

4 files changed

+687
-10
lines changed

4 files changed

+687
-10
lines changed

‎internal/controller/serving/llm/gateway_controller.go‎

Lines changed: 168 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package llm
1919
import (
2020
"context"
2121
"fmt"
22+
"maps"
2223
"os"
2324
"slices"
2425
"strings"
@@ -30,6 +31,8 @@ import (
3031
corev1 "k8s.io/api/core/v1"
3132
apierrors "k8s.io/apimachinery/pkg/api/errors"
3233
"k8s.io/apimachinery/pkg/api/meta"
34+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
35+
"k8s.io/apimachinery/pkg/labels"
3336
"k8s.io/apimachinery/pkg/runtime"
3437
"k8s.io/apimachinery/pkg/types"
3538
"k8s.io/client-go/tools/record"
@@ -81,6 +84,7 @@ func NewGatewayReconciler(client client.Client, scheme *runtime.Scheme, recorder
8184
// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch
8285
// +kubebuilder:rbac:groups=networking.istio.io,resources=envoyfilters,verbs=get;list;watch;create;update;patch;delete
8386
// +kubebuilder:rbac:groups=kuadrant.io,resources=authpolicies,verbs=get;list;watch;create;update;patch;delete
87+
// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
8488

8589
func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
8690
logger := log.FromContext(ctx).WithValues("Gateway", req.Name, "namespace", req.Namespace)
@@ -335,7 +339,7 @@ func (r *GatewayReconciler) isGatewayReferencedByLLMService(ctx context.Context,
335339
}
336340

337341
for i := range llmSvcList.Items {
338-
for _, ref := range r.getEffectiveGatewayRefs(ctx, &llmSvcList.Items[i]) {
342+
for _, ref := range r.getEffectiveGatewayRefs(ctx, gateway, &llmSvcList.Items[i]) {
339343
ns := string(ref.Namespace)
340344
if ns == "" {
341345
ns = llmSvcList.Items[i].Namespace
@@ -351,9 +355,11 @@ func (r *GatewayReconciler) isGatewayReferencedByLLMService(ctx context.Context,
351355
// getEffectiveGatewayRefs returns the effective gateway references for an LLMInferenceService.
352356
// The service's own gateway refs take precedence over those inherited from BaseRef configs,
353357
// matching the "last one wins" merge semantics used by kserve's MergeSpecs.
354-
func (r *GatewayReconciler) getEffectiveGatewayRefs(ctx context.Context, llmSvc *kservev1alpha2.LLMInferenceService) []kservev1alpha2.UntypedObjectReference {
358+
func (r *GatewayReconciler) getEffectiveGatewayRefs(ctx context.Context, gateway *gatewayapiv1.Gateway, llmSvc *kservev1alpha2.LLMInferenceService) []kservev1alpha2.UntypedObjectReference {
359+
nsLabels := r.fetchNamespaceLabels(ctx, llmSvc.Namespace)
360+
355361
// Service's own refs take precedence (replace config refs)
356-
if refs := getGatewayRefs(llmSvc); len(refs) > 0 {
362+
if refs := getGatewayRefs(ctx, gateway, llmSvc, nsLabels); len(refs) > 0 {
357363
return refs
358364
}
359365

@@ -373,7 +379,80 @@ func (r *GatewayReconciler) getEffectiveGatewayRefs(ctx context.Context, llmSvc
373379
}
374380
}
375381

376-
return refs
382+
return filterAllowedRefs(ctx, gateway, refs, llmSvc.Namespace, nsLabels)
383+
}
384+
385+
// fetchNamespaceLabels retrieves the labels for the given namespace.
386+
// Returns nil if the namespace cannot be fetched.
387+
func (r *GatewayReconciler) fetchNamespaceLabels(ctx context.Context, namespace string) map[string]string {
388+
ns := &corev1.Namespace{}
389+
if err := r.Client.Get(ctx, client.ObjectKey{Name: namespace}, ns); err != nil {
390+
log.FromContext(ctx).V(1).Info("Failed to fetch namespace labels, Selector-based allowedRoutes will be denied", "namespace", namespace, "error", err)
391+
return nil
392+
}
393+
return ns.Labels
394+
}
395+
396+
func filterAllowedRefs(ctx context.Context, gateway *gatewayapiv1.Gateway, refs []kservev1alpha2.UntypedObjectReference, targetNS string, nsLabels map[string]string) []kservev1alpha2.UntypedObjectReference {
397+
if gateway == nil {
398+
return refs
399+
}
400+
401+
var allowed []kservev1alpha2.UntypedObjectReference
402+
for _, ref := range refs {
403+
refNs := string(ref.Namespace)
404+
if refNs == "" {
405+
refNs = targetNS
406+
}
407+
if string(ref.Name) != gateway.Name || refNs != gateway.Namespace {
408+
// Ref points to a different gateway — keep it, we can't evaluate.
409+
allowed = append(allowed, ref)
410+
continue
411+
}
412+
413+
if gatewayAllowsNamespace(ctx, gateway, targetNS, nsLabels) {
414+
allowed = append(allowed, ref)
415+
}
416+
}
417+
return allowed
418+
}
419+
420+
// gatewayAllowsNamespace returns true if at least one listener on the gateway
421+
// permits routes from targetNS according to the Gateway API allowedRoutes spec.
422+
func gatewayAllowsNamespace(ctx context.Context, gateway *gatewayapiv1.Gateway, targetNS string, nsLabels map[string]string) bool {
423+
for _, listener := range gateway.Spec.Listeners {
424+
if listenerAllowsNamespace(ctx, listener, gateway.Namespace, targetNS, nsLabels) {
425+
return true
426+
}
427+
}
428+
return false
429+
}
430+
431+
func listenerAllowsNamespace(ctx context.Context, l gatewayapiv1.Listener, gwNamespace, targetNS string, nsLabels map[string]string) bool {
432+
if l.AllowedRoutes == nil || l.AllowedRoutes.Namespaces == nil || l.AllowedRoutes.Namespaces.From == nil {
433+
// Default is "Same" per Gateway API spec.
434+
return gwNamespace == targetNS
435+
}
436+
437+
switch *l.AllowedRoutes.Namespaces.From {
438+
case gatewayapiv1.NamespacesFromAll:
439+
return true
440+
case gatewayapiv1.NamespacesFromSame:
441+
return gwNamespace == targetNS
442+
case gatewayapiv1.NamespacesFromSelector:
443+
if l.AllowedRoutes.Namespaces.Selector == nil {
444+
return false
445+
}
446+
selector, err := metav1.LabelSelectorAsSelector(l.AllowedRoutes.Namespaces.Selector)
447+
if err != nil {
448+
log.FromContext(ctx).Error(err, "Failed to parse listener namespace selector, denying access",
449+
"listener", l.Name, "gwNamespace", gwNamespace)
450+
return false
451+
}
452+
return selector.Matches(labels.Set(nsLabels))
453+
default:
454+
return false
455+
}
377456
}
378457

379458
// fetchLLMISvcConfig fetches LLMInferenceServiceConfig by name, first from the given namespace,
@@ -408,7 +487,7 @@ func (r *GatewayReconciler) enqueueGatewaysFromLLMInferenceService() handler.Eve
408487
}
409488

410489
var requests []reconcile.Request
411-
for _, ref := range r.getEffectiveGatewayRefs(ctx, llmSvc) {
490+
for _, ref := range r.getEffectiveGatewayRefs(ctx, nil, llmSvc) {
412491
namespace := string(ref.Namespace)
413492
if namespace == "" {
414493
namespace = llmSvc.Namespace
@@ -463,7 +542,7 @@ func (r *GatewayReconciler) enqueueGatewaysFromLLMInferenceServiceConfig() handl
463542
if !referencesConfig(svc, cfg.Name) {
464543
continue
465544
}
466-
for _, ref := range r.getEffectiveGatewayRefs(ctx, svc) {
545+
for _, ref := range r.getEffectiveGatewayRefs(ctx, nil, svc) {
467546
namespace := string(ref.Namespace)
468547
if namespace == "" {
469548
namespace = svc.Namespace
@@ -481,6 +560,73 @@ func (r *GatewayReconciler) enqueueGatewaysFromLLMInferenceServiceConfig() handl
481560
})
482561
}
483562

563+
// enqueueGatewaysFromNamespace returns an event handler that, on namespace label
564+
// changes, finds gateways referenced by LLMInferenceServices in the changed
565+
// namespace and enqueues those with Selector-based listeners.
566+
func (r *GatewayReconciler) enqueueGatewaysFromNamespace() handler.EventHandler {
567+
return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
568+
ns := object.(*corev1.Namespace)
569+
570+
llmSvcList := &kservev1alpha2.LLMInferenceServiceList{}
571+
if err := r.Client.List(ctx, llmSvcList, client.InNamespace(ns.Name)); err != nil {
572+
log.FromContext(ctx).Error(err, "Failed to list LLMInferenceServices for namespace label change", "namespace", ns.Name)
573+
return nil
574+
}
575+
if len(llmSvcList.Items) == 0 {
576+
return nil
577+
}
578+
579+
// Collect unique gateway refs from all LLMInferenceServices in this namespace.
580+
seen := make(map[types.NamespacedName]struct{})
581+
var candidates []types.NamespacedName
582+
for i := range llmSvcList.Items {
583+
for _, ref := range r.getEffectiveGatewayRefs(ctx, nil, &llmSvcList.Items[i]) {
584+
namespace := string(ref.Namespace)
585+
if namespace == "" {
586+
namespace = ns.Name
587+
}
588+
key := types.NamespacedName{Name: string(ref.Name), Namespace: namespace}
589+
if _, exists := seen[key]; !exists {
590+
seen[key] = struct{}{}
591+
candidates = append(candidates, key)
592+
}
593+
}
594+
}
595+
596+
// Only enqueue gateways that exist and have Selector-based listeners.
597+
var requests []reconcile.Request
598+
for _, key := range candidates {
599+
gw := &gatewayapiv1.Gateway{}
600+
if err := r.Client.Get(ctx, key, gw); err != nil {
601+
if apierrors.IsNotFound(err) {
602+
continue
603+
}
604+
log.FromContext(ctx).Error(err, "Failed to get Gateway for namespace label change", "gateway", key.Name, "namespace", key.Namespace)
605+
requests = append(requests, reconcile.Request{NamespacedName: key})
606+
continue
607+
}
608+
if hasSelectorListeners(gw) {
609+
requests = append(requests, reconcile.Request{NamespacedName: key})
610+
}
611+
}
612+
return requests
613+
})
614+
}
615+
616+
// hasSelectorListeners returns true if any listener on the gateway uses
617+
// NamespacesFromSelector in its allowedRoutes.
618+
func hasSelectorListeners(gw *gatewayapiv1.Gateway) bool {
619+
for _, l := range gw.Spec.Listeners {
620+
if l.AllowedRoutes != nil &&
621+
l.AllowedRoutes.Namespaces != nil &&
622+
l.AllowedRoutes.Namespaces.From != nil &&
623+
*l.AllowedRoutes.Namespaces.From == gatewayapiv1.NamespacesFromSelector {
624+
return true
625+
}
626+
}
627+
return false
628+
}
629+
484630
func getConfigGatewayRefs(cfg *kservev1alpha2.LLMInferenceServiceConfig) []kservev1alpha2.UntypedObjectReference {
485631
if cfg.Spec.Router != nil && cfg.Spec.Router.Gateway.HasRefs() {
486632
return cfg.Spec.Router.Gateway.Refs
@@ -559,7 +705,7 @@ func configGatewayRefsChanged(oldCfg, newCfg *kservev1alpha2.LLMInferenceService
559705
// gatewayRefsChanged reports whether gateway refs or BaseRefs differ between two LLMInferenceService objects.
560706
// BaseRefs are compared because they can introduce gateway refs via LLMInferenceServiceConfig.
561707
func gatewayRefsChanged(old, new *kservev1alpha2.LLMInferenceService) bool {
562-
if !gatewayRefsEqual(getGatewayRefs(old), getGatewayRefs(new)) {
708+
if !gatewayRefsEqual(getGatewayRefs(context.Background(), nil, old, nil), getGatewayRefs(context.Background(), nil, new, nil)) {
563709
return true
564710
}
565711

@@ -587,9 +733,9 @@ func gatewayRefsChanged(old, new *kservev1alpha2.LLMInferenceService) bool {
587733
return false
588734
}
589735

590-
func getGatewayRefs(llmSvc *kservev1alpha2.LLMInferenceService) []kservev1alpha2.UntypedObjectReference {
736+
func getGatewayRefs(ctx context.Context, gateway *gatewayapiv1.Gateway, llmSvc *kservev1alpha2.LLMInferenceService, nsLabels map[string]string) []kservev1alpha2.UntypedObjectReference {
591737
if llmSvc.Spec.Router != nil && llmSvc.Spec.Router.Gateway.HasRefs() {
592-
return slices.Clone(llmSvc.Spec.Router.Gateway.Refs)
738+
return filterAllowedRefs(ctx, gateway, slices.Clone(llmSvc.Spec.Router.Gateway.Refs), llmSvc.Namespace, nsLabels)
593739
}
594740
return nil
595741
}
@@ -692,6 +838,19 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager, setupLog logr.Log
692838
return true
693839
},
694840
})).
841+
Watches(&corev1.Namespace{},
842+
r.enqueueGatewaysFromNamespace(),
843+
ctrlbuilder.WithPredicates(predicate.Funcs{
844+
CreateFunc: func(_ event.CreateEvent) bool {
845+
return true
846+
},
847+
UpdateFunc: func(e event.UpdateEvent) bool {
848+
return !maps.Equal(e.ObjectOld.GetLabels(), e.ObjectNew.GetLabels())
849+
},
850+
DeleteFunc: func(_ event.DeleteEvent) bool {
851+
return false
852+
},
853+
})).
695854
Named("gateway-auth-bootstrap").
696855
Complete(r)
697856
}

‎internal/controller/serving/llm/gateway_controller_test.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,83 @@ var _ = Describe("Gateway Controller", func() {
232232
})
233233
})
234234

235+
Context("Gateway allowedRoutes filtering", func() {
236+
It("should NOT create resources when gateway has Same-only listeners and LLMInferenceService is in a different namespace", func(ctx SpecContext) {
237+
// Create a second namespace for the LLMInferenceService
238+
otherNamespace := testutils.Namespaces.Create(ctx, envTest.Client)
239+
otherNs := otherNamespace.Name
240+
241+
gatewayName := pkgtest.GenerateUniqueTestName("same-only-gateway")
242+
// Default WithListener creates listeners without AllowedRoutes → defaults to Same
243+
gw := createGateway(ctx, gatewayName)
244+
DeferCleanup(func(ctx SpecContext) {
245+
_ = envTest.Client.Delete(ctx, gw)
246+
})
247+
248+
// Create LLMInferenceService in a different namespace, referencing the gateway
249+
llmSvc := fixture.LLMInferenceService("test-llmisvc",
250+
fixture.InNamespace[*kservev1alpha2.LLMInferenceService](otherNs),
251+
fixture.WithGatewayRefs(fixture.LLMGatewayRef(gatewayName, testNs)),
252+
)
253+
Expect(envTest.Client.Create(ctx, llmSvc)).Should(Succeed())
254+
DeferCleanup(func(ctx SpecContext) {
255+
_ = envTest.Client.Delete(ctx, llmSvc)
256+
})
257+
258+
fixture.VerifyGatewayEnvoyFilterNotExist(ctx, envTest.Client, testNs, gatewayName)
259+
fixture.VerifyGatewayAuthPolicyNotExist(ctx, envTest.Client, testNs, gatewayName)
260+
})
261+
262+
It("should create resources when gateway has All listener and LLMInferenceService is in a different namespace", func(ctx SpecContext) {
263+
otherNamespace := testutils.Namespaces.Create(ctx, envTest.Client)
264+
otherNs := otherNamespace.Name
265+
266+
gatewayName := pkgtest.GenerateUniqueTestName("all-listener-gateway")
267+
from := gatewayapiv1.NamespacesFromAll
268+
gw := fixture.Gateway(gatewayName,
269+
fixture.InNamespace[*gatewayapiv1.Gateway](testNs),
270+
fixture.WithClassName(GatewayClassName),
271+
fixture.WithListeners(gatewayapiv1.Listener{
272+
Name: "http",
273+
Port: 80,
274+
Protocol: gatewayapiv1.HTTPProtocolType,
275+
AllowedRoutes: &gatewayapiv1.AllowedRoutes{
276+
Namespaces: &gatewayapiv1.RouteNamespaces{
277+
From: &from,
278+
},
279+
},
280+
}),
281+
)
282+
Expect(envTest.Client.Create(ctx, gw)).Should(Succeed())
283+
DeferCleanup(func(ctx SpecContext) {
284+
_ = envTest.Client.Delete(ctx, gw)
285+
})
286+
287+
// Create LLMInferenceService in a different namespace
288+
llmSvc := fixture.LLMInferenceService("test-llmisvc",
289+
fixture.InNamespace[*kservev1alpha2.LLMInferenceService](otherNs),
290+
fixture.WithGatewayRefs(fixture.LLMGatewayRef(gatewayName, testNs)),
291+
)
292+
Expect(envTest.Client.Create(ctx, llmSvc)).Should(Succeed())
293+
DeferCleanup(func(ctx SpecContext) {
294+
_ = envTest.Client.Delete(ctx, llmSvc)
295+
})
296+
297+
fixture.VerifyGatewayEnvoyFilterExists(ctx, envTest.Client, testNs, gatewayName)
298+
fixture.VerifyGatewayAuthPolicyExists(ctx, envTest.Client, testNs, gatewayName)
299+
})
300+
301+
It("should create resources when gateway has Same listener and LLMInferenceService is in the same namespace", func(ctx SpecContext) {
302+
gatewayName := pkgtest.GenerateUniqueTestName("same-ns-gateway")
303+
// Default listener has no AllowedRoutes → defaults to Same
304+
createGateway(ctx, gatewayName)
305+
createLLMServiceForGateway(ctx, gatewayName)
306+
307+
fixture.VerifyGatewayEnvoyFilterExists(ctx, envTest.Client, testNs, gatewayName)
308+
fixture.VerifyGatewayAuthPolicyExists(ctx, envTest.Client, testNs, gatewayName)
309+
})
310+
})
311+
235312
Context("LLMInferenceService gateway ref changes", func() {
236313
It("should create resources when gateway ref is added to LLMInferenceService", func(ctx SpecContext) {
237314
gatewayName := pkgtest.GenerateUniqueTestName("managed-gateway")

0 commit comments

Comments
 (0)