fix(operator): make per-model API-key auth work end-to-end on AI Gateway v0.5 (#116)#117
Conversation
|
Tested this live on a fresh AWS Nebari cluster (Envoy Gateway v1.6.2 + AI Gateway v0.5, GitOps via ArgoCD, operator pinned to the branch build), with one served LLMModel and OpenRouter PassthroughModels. Surfaced four problems. Three are fixed on this branch; the fourth is filed for follow-up. What works
Problems found
All three code fixes are stacked on this branch. Re-validating end to end now with a clean teardown and reinstall. |
eb62722 to
d9f5a15
Compare
Add apiKeyAuth.forwardClientIDHeader + sanitize and a deny-by-default authorization block listing each model's own client IDs to the external SecurityPolicy. A key valid on the shared listener now gets 403 against a model it was not minted for. Covers LLMModel and PassthroughModel. Closes #116
Watch managed api-keys Secrets and enqueue the owning model so minting or revoking a key promptly updates the per-route authorization allow-list.
…on AI Gateway v0.5 The AI Gateway v0.5 controller does not register a model whose match rule carries any header beyond x-ai-eg-model. The operator added a Host matcher (defence-in-depth on top of sectionName) to every model route, which made every request 404 "model not configured in the Gateway", blocking all model serving on EG v1.6.7 / AI Gateway v0.5. Remove it from the served (buildAIGatewayRoute) and passthrough declared-model rules; sectionName already scopes each route to its listener. The passthrough catch-all rule is unchanged (opt-in, no x-ai-eg-model). Verified end-to-end on kind for served and passthrough models.
The llm-d inference scheduler (EPP) v0.8.x watches InferenceObjective and
InferenceModelRewrite (inference.networking.x-k8s.io). Without RBAC the
informers never sync and the EPP crash-loops ("failed waiting for
*v1alpha2.InferenceObjective Informer to sync"), so served models never get a
healthy upstream. Add get/list/watch on the x-k8s.io inference resources to the
per-model EPP Role.
EG's api_key_auth filter runs before the AI Gateway ext_proc resolves the model, so authentication happens on whichever catch-all route wins, not the model's own route. With per-route credentials only the winning model's keys authenticated; every other model's keys got 401. Pool every model's api-keys Secret into each external SecurityPolicy's credentialRefs so any valid key authenticates on the shared listener, while the per-route authorization block (deny-by-default + the model's own client IDs) still scopes each request to its model. All Secrets are same-namespace (#59). The controllers gather the full Secret set and re-reconcile all models when an api-keys Secret changes. Completes per-model API-key auth for #116: a key minted for one model returns 200 for that model and 403 for every other, verified end-to-end on kind for served and passthrough models.
The dev mock-vllm only served /health and /v1/models; add a 200 OpenAI-compatible response for POST /v1/chat/completions (and /v1/completions) so the dev stack can exercise the full served-model request path through the gateway.
…erms the EPP Role needs The per-model EPP Role grants get/list/watch on inferenceobjectives, inferencemodelrewrites, inferencepools and inferencepoolimports (inference.networking.x-k8s.io). Kubernetes privilege-escalation prevention blocks the operator from creating a Role that grants permissions the operator does not itself hold, so on a Helm/ArgoCD install every served LLMModel failed to reconcile with "attempting to grant RBAC permissions not currently held" - the EPP Role, and with it the model's SecurityPolicy and AIGatewayRoute, were never created. Add the matching get/list/watch rule to the operator ClusterRole so it can create the EPP Role.
llm-d-cuda images use the NVIDIA CUDA wrapper as their entrypoint and ship
no default CMD, so the served pod must supply the command itself. The vllm
container set only Args (the vLLM flags); the wrapper exec'd them as the
command ("exec: --: invalid option") and the pod crash-looped. Set the
standard vLLM OpenAI-server command so the flags are its arguments.
The client-id sequence was counted per-model, so the first key a user minted for any model was always "user-<name>-1". The operator pools every model's api-keys Secret into each model's SecurityPolicy credentialRefs (model-scoped auth) and uses the matched data key as the x-llm-client-id for per-model authorization. Identical client IDs across models collide in that pooled set: one model's key authenticates and authorizes for another, and a user's other keys fail to authenticate at all (only one value survives per duplicated key). Include the model in the client ID (user-<name>-<model>-<n>) so each is globally unique. Add a regression test asserting a user's keys for two models get distinct client IDs.
11dcd11 to
5b12f07
Compare
|
📄 Docs preview for |
golangci-lint (goconst) on current main flagged the repeated "Deny"/"Allow" authorization-action literals and the "x-llm-client-id" header literal, which already has the apiKeyClientIDHeader constant. Add authzActionAllow/authzActionDeny constants in auth.go and reference all three constants from auth.go and the reconciler tests. No behavior change - the rendered SecurityPolicy is identical.
…e IDs Two defects in the client-ID scheme, both regression-tested: - "user-<name>-<model>-<n>" with hyphen separators is ambiguous: the pairs (mary, jane-chat) and (mary-jane, chat) composed the same clientID in two different Secrets, recreating the #122 pooled-credential collision. An FNV-1a hash of the raw (username, model) pair is now part of every clientID, so distinct pairs always mint distinct IDs. - The sequence number came from counting existing keys, so minting after a mid-sequence revocation reused a still-live clientID and silently overwrote that key in the Secret and its ConfigMap metadata. The sequence now probes upward until the ID is free in both.
Adds spec.serving.command to the LLMModel CRD. The default stays the explicit vLLM OpenAI-server entrypoint required by the llm-d-cuda image (its NVIDIA wrapper entrypoint ships no default CMD); the override lets a custom serving.image use its own launcher. The dev mock-vllm model needs it: forcing the vLLM command broke the kind dev served path, since the mock image has no vllm module and crash-looped with ModuleNotFoundError. dev/manifests/test-model.yaml now sets command: ["python", "/server.py"]. configuration.md documents the field and joins the content-parity exclusion list, following the same pattern as architecture.md.
The #121 fix added inference.networking.x-k8s.io get/list/watch to the Helm chart ClusterRole but not to dev/manifests/operator.yaml, so on the kind dev stack RBAC escalation prevention still blocked the per-model EPP Role and every served model failed to reconcile ("attempting to grant RBAC permissions not currently held"). Mirrors the chart rule.
The LLMModel and PassthroughModel controllers carried near-verbatim copies of the api-keys Secret helpers (client-ID read, pooled Secret listing, watch predicate, map function). They now share internal/controller/apikeys.go, the "-api-keys" suffix is single-sourced next to reconcilers.APIKeySecretName, and a failed List in the map function is logged instead of silently dropping the re-render.
…ract architecture.md still described the removed Host matcher, the old two-header precedence rationale, and the pre-#117 client-ID format. It now documents the behavior validated live on EG v1.6.7 / AI Gateway v0.5: - dispatch is decided by the ext_proc model registry, so served models win regardless of route age (verified against an older catchAll: true route) - catchAll is currently inert: unregistered model ids 404 at the ext_proc before route matching runs - key activation is split into its authentication (Secret sync, immediate) and authorization (operator re-render) halves - the data-model examples use the pair-hashed client-ID format The buildPassthroughRoute comments state the same validated behavior.
Follow-up drift sweep across docs/src/content/docs against the code at this branch's head: - architecture.md: explain why the pooled apiKeyAuth credentialRefs live on each per-route SecurityPolicy instead of one gateway-level policy (route-level policies take precedence over gateway-level ones rather than merging, and each route needs its own policy for the per-model authorization anyway), and note the deliberate exception to the cluster-singleton rule; fix the resources-table row that still said the external policy references only the per-model Secret; the PassthroughModel setup bullet now mentions the api-keys Secret watch. - installation.md: the wrong-model-key troubleshooting entry sat under 401, but pooled authentication makes that case a 403; split into 401 (key in no Secret) and 403 (key valid, not on this model's allow-list, or allow-list not yet re-rendered). - local-development.md and the dev passthrough manifest: note the transient 403 window between patching a key into the Secret and the operator re-rendering the allow-list.
Resolves the architecture.md conflict in the Key manager "How it works" section: keep main's Model B (SPA-managed Keycloak) login flow and JWKS env-var paragraph, and preserve this branch's step 6 describing pooled apiKeyAuth authentication plus the operator re-rendering the SecurityPolicy authorization allow-list.
The client-ID appended a 32-bit FNV-1a hash of the (username, model) pair, plus a 32-bit FNV-1a hash when sanitizing usernames. A 32-bit space makes collisions cheap to construct, and a duplicate client ID across two models' pooled api-keys Secrets reintroduces the cross-model authorization bypass (Envoy Gateway uses the first of any duplicated client ID in credentialRefs; see #116, #122). Replace both with a 128-bit-truncated SHA-256. The hash input already uses a NUL separator so it was unambiguous; only the output width was weak. The client ID is a public authorization principal, not a secret, so a deterministic hash keeps IDs reproducible for tests and debugging while making collisions computationally infeasible. Update the black-box mirror helpers and the white-box drift-detector literals to match.
test-model-scope.sh drives the real external gateway data path: it deploys two served mock models, injects a known API key into each model's api-keys Secret, and asserts the 200/403/401 matrix. The security-critical rows prove a client-supplied x-llm-client-id header (including duplicate, mixed-case, and comma-joined forms) cannot grant access to a model the key is not scoped to: Envoy Gateway's api_key_auth filter sets the forwarded client-ID header with a replace-all "set", discarding any client value before authorization runs. Locks that behavior in against a gateway/Envoy version bump.
|
I haven't tested or validated this yet but gpt 5.6-sol found a few interesting things: Findings
|
…api-key-auth # Conflicts: # docs/src/content/docs/architecture.mdx
What this fixes
Closes #116. API keys were gateway-scoped, not model-scoped: any valid key worked for every model on the shared external listener (
llm.<baseDomain>). Closing that gap end-to-end on the current stack (Envoy Gateway v1.6.7 / AI Gateway v0.5) turned out to need four changes, three of which surfaced during live validation.The four changes
1. Per-model authorization (the #116 core). Each model's external
SecurityPolicygets a deny-by-defaultauthorizationblock whose Allow rule lists only the client IDs in that model's own api-keys Secret, matched onapiKeyAuth.forwardClientIDHeader: x-llm-client-id(withsanitize: true). Authentication is shared across the listener; authorization is per-route, so a key forwards its client ID and is allowed only for the model it was minted for.2. Pooled authentication. EG's
api_key_authfilter runs before the AI Gateway ext_proc resolves the model, so authentication happens on whichever catch-all route wins, not the model's own route. With per-route credentials, only one model's keys could authenticate and every other model's keys got 401. Each externalSecurityPolicynow pools every model's api-keys Secret intocredentialRefs(all same-namespace, per #59), so any valid key authenticates on the shared listener and the per-route authorization (change 1) does the scoping. This is the design #116 assumed; it just is not the default on EG v1.6.7. The controllers gather the full Secret set and re-reconcile all models when an api-keys Secret is added or removed.3. Routing fix for AI Gateway v0.5. The AI Gateway v0.5 controller does not register a model whose
AIGatewayRoutematch carries any header beyondx-ai-eg-model. The operator added aHostmatcher (defence-in-depth on top ofsectionName), which made every request 404 "model not configured in the Gateway" and blocked all model serving. Removed it;sectionNamealready scopes each route to its listener.4. EPP RBAC. The llm-d inference scheduler (EPP) v0.8.x watches
InferenceObjectiveandInferenceModelRewrite(inference.networking.x-k8s.io). The per-model EPP Role lacked RBAC for them, so the informers never synced and the EPP crash-looped, leaving served models with no healthy upstream. Addedget/list/watch.Also: the dev
mock-vllmnow answersPOST /v1/chat/completionsso the dev stack can exercise the full served path.Validation
Verified end-to-end on a kind cluster (EG v1.6.7 + AI Gateway v0.5), operator-driven, for both served (mock-vllm) and passthrough (real OpenRouter) models:
catchAll: false): 404; a model with no keys: 403 for any keyAutomated: reconciler unit tests cover the rendered SecurityPolicy (forwardClientIDHeader, sanitize, deny-by-default authz, the per-model allow-list, and pooled credentialRefs) for both the served and passthrough paths.
Not yet validated against a live system: the internal/JWT endpoint, which needs a real Keycloak OIDC issuer (not available on a bare kind cluster). It is covered by unit tests and is not touched by these changes.