Restrict Secret informer to bbr-managed labeled Secrets only

Signed-off-by: Abdallah Samara <abdallahsamabd@gmail.com>

Author: abdallahsamabd

deploy/README.md

@@ -3,3 +3,30 @@
 A chart to deploy payload-processing.
 
 <!-- TODO we should pin to odh payload processing released tag -->
+
+## RBAC and Secrets Access
+
+The `apikey-injection` plugin watches Kubernetes Secrets to inject provider API
+keys (e.g. OpenAI, Anthropic) into outbound inference requests. Only Secrets
+labeled `inference.llm-d.ai/ipp-managed: "true"` are processed.
+
+### Single-namespace mode (`multiNamespace: false`, the default)
+
+A **Role** and **RoleBinding** are created in the release namespace. This matches
+[Kubernetes RBAC good practices](https://kubernetes.io/docs/concepts/security/rbac-good-practices/)
+(least privilege, prefer namespace-scoped bindings) and limits exposure called
+out in [#201](https://github.com/opendatahub-io/ai-gateway-payload-processing/issues/201).
+
+### Multi-namespace mode (`multiNamespace: true`, opt-in)
+
+A **ClusterRole** is created with `get`/`list`/`watch` on `secrets`. Kubernetes
+RBAC cannot scope `secrets` by label, so static analysis tools will still flag
+broad Secret access; only enable when ExternalModels and credential Secrets
+actually span namespaces.
+
+Defense in depth on the workload:
+
+- The Secret informer uses a **label selector** at list/watch time so only
+  `ipp-managed` Secrets enter the cache (reduces blast radius if the process is
+  compromised; it does not replace least-privilege RBAC). `Reconcile` still
+  checks the label after `Get` before storing credentials.

deploy/payload-processing/templates/rbac.yaml

@@ -6,7 +6,11 @@ kind: ClusterRole
 metadata:
   name: {{ .Release.Name }}-externalmodels-reader
 rules:
-  # apikey-injection plugin: watches labeled Secrets
+  # apikey-injection plugin: watches Secrets that carry the label
+  # "inference.llm-d.ai/ipp-managed=true" to inject provider API keys.
+  # A ClusterRole is required because Kubernetes RBAC does not support
+  # label-based scoping; the apikey-injection Secret informer still applies a
+  # label selector at list/watch time so only ipp-managed Secrets are cached.
   - apiGroups: [""]
     resources: ["secrets"]
     verbs: ["get", "list", "watch"]
@@ -35,7 +39,9 @@ metadata:
   name: {{ .Release.Name }}-externalmodels-reader
   namespace: {{ .Release.Namespace }}
 rules:
-  # apikey-injection plugin: watches labeled Secrets
+  # apikey-injection plugin: watches Secrets labeled
+  # "inference.llm-d.ai/ipp-managed=true" in this namespace.
+  # Scoped to a single namespace via Role (preferred over ClusterRole).
   - apiGroups: [""]
     resources: ["secrets"]
     verbs: ["get", "list", "watch"]

deploy/payload-processing/values.yaml

@@ -9,7 +9,9 @@ upstreamBbr:
       pullPolicy: IfNotPresent
     port: 9004
     healthCheckPort: 9005
-    multiNamespace: true
+    # false = namespaced Role for secrets + externalmodels (default, least privilege; see #201).
+    # true = ClusterRole when ExternalModels/credentials span multiple namespaces.
+    multiNamespace: false
 
     flags:
       # Log verbosity

docs/providers/anthropic.md

@@ -60,7 +60,7 @@ metadata:
   name: anthropic-api-key
   namespace: llm
   labels:
-    inference.networking.k8s.io/bbr-managed: "true"
+    inference.llm-d.ai/ipp-managed: "true"
 type: Opaque
 stringData:
   api-key: "sk-ant-api03-..."

docs/providers/azure-openai.md

@@ -52,7 +52,7 @@ metadata:
   name: azure-api-key
   namespace: llm
   labels:
-    inference.networking.k8s.io/bbr-managed: "true"
+    inference.llm-d.ai/ipp-managed: "true"
 type: Opaque
 stringData:
   api-key: "<AZURE_API_KEY>"

docs/providers/bedrock-openai.md

@@ -67,7 +67,7 @@ metadata:
   name: bedrock-api-key
   namespace: llm
   labels:
-    inference.networking.k8s.io/bbr-managed: "true"
+    inference.llm-d.ai/ipp-managed: "true"
 type: Opaque
 stringData:
   api-key: "<BEDROCK_API_KEY>"

docs/providers/openai.md

@@ -41,7 +41,7 @@ metadata:
   name: openai-api-key
   namespace: llm
   labels:
-    inference.networking.k8s.io/bbr-managed: "true"
+    inference.llm-d.ai/ipp-managed: "true"
 type: Opaque
 stringData:
   api-key: "sk-proj-..."

docs/providers/vertex-openai.md

@@ -93,7 +93,7 @@ metadata:
   name: vertex-api-key
   namespace: llm
   labels:
-    inference.networking.k8s.io/bbr-managed: "true"
+    inference.llm-d.ai/ipp-managed: "true"
 type: Opaque
 stringData:
   api-key: "ya29.a0..."  # OAuth token from gcloud auth print-access-token

pkg/plugins/apikey-injection/plugin.go

@@ -20,11 +20,17 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 	"sigs.k8s.io/gateway-api-inference-extension/pkg/bbr/framework"
 	errcommon "sigs.k8s.io/gateway-api-inference-extension/pkg/common/error"
 	logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/common/observability/logging"
@@ -45,23 +51,37 @@ var _ framework.RequestProcessor = &ApiKeyInjectionPlugin{}
 
 // APIKeyInjectionFactory defines the factory function for ApiKeyInjectionPlugin.
 func APIKeyInjectionFactory(name string, _ json.RawMessage, handle framework.Handle) (framework.BBRPlugin, error) {
-	plugin, err := NewAPIKeyInjectionPlugin(handle.ReconcilerBuilder, handle.ClientReader())
+	plugin, err := NewAPIKeyInjectionPlugin(handle.Context(), handle.ReconcilerBuilder, handle.ClientReader())
 	if err != nil {
 		return nil, fmt.Errorf("failed to create plugin '%s' - %w", APIKeyInjectionPluginType, err)
 	}
 
 	return plugin.WithName(name), nil
 }
 
-// NewAPIKeyInjectionPlugin creates a new apiKeyInjectionPlugin and returns its pointer
-func NewAPIKeyInjectionPlugin(reconcilerBuilder func() *builder.Builder, clientReader client.Reader) (*ApiKeyInjectionPlugin, error) {
+// NewAPIKeyInjectionPlugin creates a new apiKeyInjectionPlugin and returns its pointer.
+// It sets up a label-filtered informer cache so only Secrets matching the
+// ipp-managed label are listed from the API server; Reconcile still checks the
+// label after Get before updating the store.
+func NewAPIKeyInjectionPlugin(ctx context.Context, reconcilerBuilder func() *builder.Builder, clientReader client.Reader) (*ApiKeyInjectionPlugin, error) {
 	store := newSecretStore()
 	reconciler := &secretReconciler{
 		Reader: clientReader,
 		store:  store,
 	}
 
-	if err := reconcilerBuilder().For(&corev1.Secret{}).WithEventFilter(managedLabelPredicate()).Complete(reconciler); err != nil {
+	filteredCache, err := newFilteredSecretCache(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	var secretObj client.Object = &corev1.Secret{}
+	if err := reconcilerBuilder().
+		Named("apikey-injection-secret-watcher").
+		WatchesRawSource(
+			source.Kind(filteredCache, secretObj, &handler.EnqueueRequestForObject{}),
+		).
+		Complete(reconciler); err != nil {
 		return nil, fmt.Errorf("failed to register Secret reconciler for plugin '%s' - %w", APIKeyInjectionPluginType, err)
 	}
 
@@ -71,9 +91,9 @@ func NewAPIKeyInjectionPlugin(reconcilerBuilder func() *builder.Builder, clientR
 			Name: APIKeyInjectionPluginType,
 		},
 		authHeadersGenerators: map[string]auth.AuthHeadersGenerator{
-			provider.OpenAI:        &auth.SimpleAuthGenerator{HeaderName: "Authorization", HeaderValuePrefix: "Bearer "},
-			provider.Anthropic:     &auth.SimpleAuthGenerator{HeaderName: "x-api-key"},
-			provider.AzureOpenAI:   &auth.SimpleAuthGenerator{HeaderName: "api-key"},
+			provider.OpenAI:      &auth.SimpleAuthGenerator{HeaderName: "Authorization", HeaderValuePrefix: "Bearer "},
+			provider.Anthropic:   &auth.SimpleAuthGenerator{HeaderName: "x-api-key"},
+			provider.AzureOpenAI: &auth.SimpleAuthGenerator{HeaderName: "api-key"},
 			// provider.Vertex uses the native GenerateContent API — not used in 3.4 ExternalModel flow.
 			// provider.Vertex:     &auth.SimpleAuthGenerator{HeaderName: "Authorization", HeaderValuePrefix: "Bearer "},
 			provider.VertexOpenAI:  &auth.SimpleAuthGenerator{HeaderName: "Authorization", HeaderValuePrefix: "Bearer "},
@@ -145,3 +165,42 @@ func (p *ApiKeyInjectionPlugin) ProcessRequest(ctx context.Context, cycleState *
 	log.FromContext(ctx).V(logutil.VERBOSE).Info("auth headers injected", "provider", providerName)
 	return nil
 }
+
+// newFilteredSecretCache creates a controller-runtime cache that restricts the
+// Secret informer to only list/watch Secrets labeled with the managed label.
+// This is a defense-in-depth measure: even though the RBAC ClusterRole grants
+// broad Secret access (required for cross-namespace watches), the informer
+// never fetches or caches unrelated Secrets.
+func newFilteredSecretCache(ctx context.Context) (cache.Cache, error) {
+	cfg, err := ctrl.GetConfig()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get rest config for filtered Secret cache: %w", err)
+	}
+
+	filteredCache, err := cache.New(cfg, cache.Options{
+		ByObject: map[client.Object]cache.ByObject{
+			&corev1.Secret{}: {
+				Label: labels.SelectorFromSet(labels.Set{
+					managedLabel: "true",
+				}),
+			},
+		},
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create filtered Secret cache: %w", err)
+	}
+
+	go func() {
+		if err := filteredCache.Start(ctx); err != nil {
+			ctrl.Log.WithName(APIKeyInjectionPluginType).Error(err, "filtered Secret cache stopped unexpectedly")
+		}
+	}()
+
+	syncCtx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+	defer cancel()
+	if !filteredCache.WaitForCacheSync(syncCtx) {
+		return nil, fmt.Errorf("filtered Secret cache failed to sync within deadline")
+	}
+
+	return filteredCache, nil
+}

pkg/plugins/apikey-injection/reconciler.go

@@ -24,34 +24,20 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
 const (
 	// managedLabel selects Secrets managed by the apikey-injection plugin.
-	// Only Secrets carrying this label are watched by the reconciler.
-	managedLabel = "inference.networking.k8s.io/bbr-managed"
+	// The Secret informer uses this as a list/watch label selector; Reconcile
+	// also requires this label before caching Secret data.
+	managedLabel = "inference.llm-d.ai/ipp-managed"
 )
 
 func hasManagedLabel(object client.Object) bool {
 	return object.GetLabels()[managedLabel] == "true"
 }
 
-// managedLabelPredicate filters events to only Secrets labeled with
-// "inference.networking.k8s.io/bbr-managed" = "true".
-// For updates, it accepts the event when either the old or new object carries
-// the label so that label-removal is visible to the reconciler.
-func managedLabelPredicate() predicate.Predicate {
-	return predicate.Funcs{
-		CreateFunc:  func(e event.CreateEvent) bool { return hasManagedLabel(e.Object) },
-		UpdateFunc:  func(e event.UpdateEvent) bool { return hasManagedLabel(e.ObjectOld) || hasManagedLabel(e.ObjectNew) },
-		DeleteFunc:  func(e event.DeleteEvent) bool { return hasManagedLabel(e.Object) },
-		GenericFunc: func(e event.GenericEvent) bool { return hasManagedLabel(e.Object) },
-	}
-}
-
 // secretReconciler watches Secrets and updates the secretStore.
 type secretReconciler struct {
 	client.Reader