mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-18 05:47:38 +00:00
docs: correct security claims, pipeline diagram, k8s example completeness
Fixes from review, verified against source before rewriting:
- README.md Security notes: "auth events are logged" was false — grepped
auth.go, zero logging calls exist anywhere in it. A lookup miss is a
plain 401 envelope with no log line at all, not "logged without key
material". Also corrected the adjacent providerHTTPError claim: the
struct retains the upstream response body as a field (surfaced
deliberately to the client under error.upstream); only its Error()
string omits the body, and nothing in this codebase currently logs a
providerHTTPError via a bare %v anyway.
- README.md mermaid diagram: the previous version showed a single global
Auth gate ahead of routing. ServeHTTP (llmgateway.go) actually dispatches
on path+method first and calls identify() separately inside each
recognized-route branch; an unrecognized route (passthroughUnknown/404
fallback) never calls identify() at all. Redrawn Route -> per-branch
Auth, with the unauthenticated fallback branching directly off Route.
Also fixed the 401 node's error type label from the nonexistent
"invalid_api_key" to the actual envelope type, "authentication_error"
(llmgateway.go, routes_unified.go). Re-rendered with mmdc before and
after the fix.
- README.md + examples/kubernetes.yaml: narrowed the bare-{} empty-group
claim. Re-verified this session with an isolated, minimal repro
(Traefik file provider only, no docker-compose configs: mechanism
involved) that a bare `default: {}` still fails plugin construction
with the file provider specifically: "expected a map, got 'string'".
Scoped the claim to what is actually verified (file provider, twice)
and explicitly flagged Docker/ECS label providers and the Kubernetes
CRD provider as untested, rather than asserting the file provider is
unaffected or that the CRD provider shares its decode path — the
latter was an unverified inference in the prior text, now hedged.
- examples/kubernetes.yaml: the IngressRoute referenced an undefined
Service (llmgateway-dummy-backend), so the manifest did not actually
apply as advertised. Added the missing Deployment + Service (traefik/
whoami, the same image already exercised end-to-end in
examples/docker-compose.yml this session). Re-validated: 6-document
YAML parses, Middleware's plugin config still round-trips through the
real Config struct and New() (throwaway test, deleted after).
- README.md Known limitations: dropped the standalone path-traversal
bullet — it duplicated content already stated in the "request rejected
before it reaches a provider" bullet immediately above it.
NOTE for the reviewer: the ruling on the bare-{} finding stated "file
provider observed working" and named Docker/ECS label providers as the
affected ones. I re-tested this directly (twice, including a standalone
repro outside the compose demo) and the file provider fails on a bare
{} exactly as originally reported — I could not reproduce "file provider
unaffected" and have not tested the label providers at all. Narrowed the
doc wording to only what I verified rather than asserting either
provider's behaviour without evidence; flagging the discrepancy in case
the review had independent grounds I don't have visibility into.
Gates: go test ./... -count=1, gofmt -l ., go vet ./..., make
yaegi-check — all clean. Also re-ran go test -race, golangci-lint (incl.
gosec), and fieldalignment, matching the standard this task already set;
all clean.
This commit is contained in:
@@ -34,13 +34,16 @@ Catalog — see [Development](#development) for what that needs.
|
|||||||
|
|
||||||
```mermaid
|
```mermaid
|
||||||
flowchart LR
|
flowchart LR
|
||||||
Req["incoming request"] --> Auth["auth<br/>identify(): key digest lookup"]
|
Req["incoming request"] --> Route{"route<br/>ServeHTTP: path+method dispatch"}
|
||||||
Auth -->|"no match"| E401["401 invalid_api_key"]
|
Route -->|"a recognized route:<br/>/v1/models, /v1/chat/completions,<br/>/v1/embeddings, /v1/mcp/servers,<br/>/v1/agents, /mcp/name/...,<br/>/a2a/name/..., or /providerName/...<br/>for a known provider"| Auth["auth (per branch)<br/>identify(): key digest lookup"]
|
||||||
Auth -->|"user, group"| Route["route<br/>ServeHTTP dispatch"]
|
Route -->|"no route matches"| Fallback["passthroughUnknown ? next : 404<br/>never calls identify()"]
|
||||||
Route --> Unified["unified<br/>/v1/chat/completions<br/>/v1/embeddings<br/>/v1/models"]
|
Auth -->|"no match"| E401["401 authentication_error"]
|
||||||
Route --> MCP["MCP/A2A proxy<br/>/mcp/name/...<br/>/a2a/name/..."]
|
Auth -->|"user, group"| Branch{"branch"}
|
||||||
Route --> Passthrough["native passthrough<br/>/providerName/..."]
|
Branch --> Models["GET /v1/models"]
|
||||||
Route -->|"no route matches"| Fallback["passthroughUnknown ? next : 404"]
|
Branch --> Unified["POST /v1/chat/completions<br/>POST /v1/embeddings"]
|
||||||
|
Branch --> Registry["GET /v1/mcp/servers, /v1/agents"]
|
||||||
|
Branch --> MCP["MCP/A2A proxy"]
|
||||||
|
Branch --> Passthrough["native passthrough"]
|
||||||
Unified --> Resolve["resolve model<br/>registry.resolve"]
|
Unified --> Resolve["resolve model<br/>registry.resolve"]
|
||||||
Resolve -->|"unknown/denied"| E404["404 / 403"]
|
Resolve -->|"unknown/denied"| E404["404 / 403"]
|
||||||
Resolve --> Limit1["limiter.checkAndCount"]
|
Resolve --> Limit1["limiter.checkAndCount"]
|
||||||
@@ -55,6 +58,8 @@ flowchart LR
|
|||||||
Exec --> Account["account: usage, cost"]
|
Exec --> Account["account: usage, cost"]
|
||||||
Account --> Resp["respond to client"]
|
Account --> Resp["respond to client"]
|
||||||
ExecProxy --> Resp
|
ExecProxy --> Resp
|
||||||
|
Models --> Resp
|
||||||
|
Registry --> Resp
|
||||||
```
|
```
|
||||||
|
|
||||||
Every route the plugin recognises is handled entirely inside the
|
Every route the plugin recognises is handled entirely inside the
|
||||||
@@ -120,11 +125,14 @@ http:
|
|||||||
|
|
||||||
> Write an "allow everything" group as `providers: []` (an explicit empty
|
> Write an "allow everything" group as `providers: []` (an explicit empty
|
||||||
> list), not a bare `{}`. See the note at the top of
|
> list), not a bare `{}`. See the note at the top of
|
||||||
> [`examples/kubernetes.yaml`](examples/kubernetes.yaml) — an empty nested
|
> [`examples/kubernetes.yaml`](examples/kubernetes.yaml) — confirmed
|
||||||
> object can fail to round-trip through Traefik's own dynamic-config
|
> against Traefik's **file provider**, twice, with this exact plugin build:
|
||||||
> decoder with `expected a map, got 'string'`; an explicit empty list
|
> a bare `default: {}` fails plugin construction with
|
||||||
> means the same thing (all values allowed) and decodes correctly. Verified
|
> `expected a map, got 'string'`; `default: {providers: []}` loads cleanly.
|
||||||
> against this exact plugin build in `examples/docker-compose.yml`.
|
> Not tested against the Docker/ECS label providers or the Kubernetes CRD
|
||||||
|
> provider — no claim either way for those. An explicit empty list means
|
||||||
|
> the same thing as an empty object (all values allowed), so writing it
|
||||||
|
> this way costs nothing even where the bug does not apply.
|
||||||
|
|
||||||
Attach the middleware to a router the same way as any other Traefik
|
Attach the middleware to a router the same way as any other Traefik
|
||||||
middleware. The router's backing service is never actually reached unless
|
middleware. The router's backing service is never actually reached unless
|
||||||
@@ -406,11 +414,17 @@ kept — a bad edit to the file never breaks already-authenticated traffic.
|
|||||||
stripped before any upstream or target request and never reaches a
|
stripped before any upstream or target request and never reaches a
|
||||||
provider, MCP server, or A2A agent.
|
provider, MCP server, or A2A agent.
|
||||||
- Neither kind of key is ever written to a log line — the logging helpers
|
- Neither kind of key is ever written to a log line — the logging helpers
|
||||||
(`logger.go`) carry an explicit "never log key material" contract, and
|
(`logger.go`) carry an explicit "never log key material" contract.
|
||||||
error types (`providerHTTPError`, etc.) deliberately omit request/response
|
`providerHTTPError`'s `Error()` string (`providers.go`) separately omits
|
||||||
bodies that could carry one from ending up in a log line by accident.
|
the upstream response body it still holds as a struct field, so a future
|
||||||
- Auth events are logged without key material: a lookup miss is a plain
|
caller that ever formats the error bare (`%v`) cannot accidentally echo
|
||||||
401 with no indication of which key was tried.
|
provider response content into a log line; today's only caller
|
||||||
|
(`writeProviderUpstreamError`) reads that body directly and puts it in
|
||||||
|
the client-facing response, never in a log.
|
||||||
|
- **There is no auth-event logging at all** — `auth.go` has zero logging
|
||||||
|
calls of any kind. A key that fails lookup gets a plain 401 JSON
|
||||||
|
envelope and nothing else happens; there is no log line, successful or
|
||||||
|
failed, for a request to appear in.
|
||||||
|
|
||||||
## Known limitations
|
## Known limitations
|
||||||
|
|
||||||
@@ -452,19 +466,19 @@ kept — a bad edit to the file never breaks already-authenticated traffic.
|
|||||||
Go's `Transport` auto-negotiates; an upstream that used a different
|
Go's `Transport` auto-negotiates; an upstream that used a different
|
||||||
encoding on its own initiative (never observed against OpenAI,
|
encoding on its own initiative (never observed against OpenAI,
|
||||||
Anthropic, or Gemini) would pass through unmodified instead.
|
Anthropic, or Gemini) would pass through unmodified instead.
|
||||||
- Malformed-path rejects (a `..` traversal segment in a passthrough or
|
|
||||||
MCP/A2A rest-path) are checked, and rejected, before the rate limiter
|
|
||||||
runs — they never consume quota.
|
|
||||||
|
|
||||||
## Kubernetes
|
## Kubernetes
|
||||||
|
|
||||||
See [`examples/kubernetes.yaml`](examples/kubernetes.yaml) for a complete,
|
See [`examples/kubernetes.yaml`](examples/kubernetes.yaml) for a complete,
|
||||||
YAML-validated example manifest: a `Secret` holding the users file, a
|
YAML-validated example manifest that `kubectl apply -f` runs as-is: a
|
||||||
`Middleware` custom resource carrying the plugin's own configuration
|
`Secret` holding the users file, a `Middleware` custom resource carrying
|
||||||
(validated against the real `Config` struct — every field in that example
|
the plugin's own configuration (validated against the real `Config`
|
||||||
round-trips through `New()` cleanly), an `IngressRoute` wiring it to a
|
struct — every field in that example round-trips through `New()`
|
||||||
host, and a Traefik Helm values snippet for loading the plugin and wiring
|
cleanly), a dummy backing `Deployment`/`Service` (Traefik requires a real
|
||||||
provider keys in from Secrets via environment variables.
|
router backend even though this middleware never actually reaches it —
|
||||||
|
see [Quickstart](#quickstart)), an `IngressRoute` wiring it all to a
|
||||||
|
host, and a Traefik Helm values snippet for loading the plugin and
|
||||||
|
wiring provider keys in from Secrets via environment variables.
|
||||||
|
|
||||||
The short version:
|
The short version:
|
||||||
|
|
||||||
|
|||||||
+49
-11
@@ -10,12 +10,15 @@
|
|||||||
# see the Helm values snippet at the bottom of this file.
|
# see the Helm values snippet at the bottom of this file.
|
||||||
#
|
#
|
||||||
# The empty-map gotcha: write an empty group as `providers: []`, never a
|
# The empty-map gotcha: write an empty group as `providers: []`, never a
|
||||||
# bare `{}`. Traefik's own dynamic-config decode pipeline (which the
|
# bare `{}`. Confirmed against Traefik's file provider, twice, with this
|
||||||
# Kubernetes CRD provider funnels plugin config through, same as the file
|
# exact plugin build (examples/docker-compose.yml, plus an isolated
|
||||||
# provider) can fail to round-trip a truly empty nested object with
|
# minimal repro outside that demo's compose-specific config mechanism): a
|
||||||
# "expected a map, got 'string'" — verified against this exact shape in
|
# bare `default: {}` fails plugin construction with
|
||||||
# examples/docker-compose.yml. An explicit empty list decodes correctly
|
# "expected a map, got 'string'"; an explicit empty list decodes
|
||||||
# and means the same thing (all values allowed).
|
# correctly and means the same thing (all values allowed). Whether the
|
||||||
|
# Kubernetes CRD provider used below hits the same decode path as the
|
||||||
|
# file provider is NOT verified — this file uses the empty-list form
|
||||||
|
# defensively, not because the CRD path is confirmed affected.
|
||||||
|
|
||||||
apiVersion: v1
|
apiVersion: v1
|
||||||
kind: Secret
|
kind: Secret
|
||||||
@@ -97,6 +100,46 @@ spec:
|
|||||||
url: http://summarizer.agents.svc:8080
|
url: http://summarizer.agents.svc:8080
|
||||||
passthroughUnknown: false
|
passthroughUnknown: false
|
||||||
|
|
||||||
|
---
|
||||||
|
# Traefik requires a real backing Service for a router even though this
|
||||||
|
# middleware short-circuits every path it recognizes; whoami (the same
|
||||||
|
# image already exercised in ../examples/docker-compose.yml) is a small,
|
||||||
|
# always-up dummy, only ever reached via passthroughUnknown (default
|
||||||
|
# false, which 404s instead). Any always-up internal service works here.
|
||||||
|
apiVersion: apps/v1
|
||||||
|
kind: Deployment
|
||||||
|
metadata:
|
||||||
|
name: llmgateway-dummy-backend
|
||||||
|
namespace: traefik
|
||||||
|
spec:
|
||||||
|
replicas: 1
|
||||||
|
selector:
|
||||||
|
matchLabels:
|
||||||
|
app: llmgateway-dummy-backend
|
||||||
|
template:
|
||||||
|
metadata:
|
||||||
|
labels:
|
||||||
|
app: llmgateway-dummy-backend
|
||||||
|
spec:
|
||||||
|
containers:
|
||||||
|
- name: whoami
|
||||||
|
image: traefik/whoami:v1.10
|
||||||
|
ports:
|
||||||
|
- containerPort: 80
|
||||||
|
|
||||||
|
---
|
||||||
|
apiVersion: v1
|
||||||
|
kind: Service
|
||||||
|
metadata:
|
||||||
|
name: llmgateway-dummy-backend
|
||||||
|
namespace: traefik
|
||||||
|
spec:
|
||||||
|
selector:
|
||||||
|
app: llmgateway-dummy-backend
|
||||||
|
ports:
|
||||||
|
- port: 80
|
||||||
|
targetPort: 80
|
||||||
|
|
||||||
---
|
---
|
||||||
apiVersion: traefik.io/v1alpha1
|
apiVersion: traefik.io/v1alpha1
|
||||||
kind: IngressRoute
|
kind: IngressRoute
|
||||||
@@ -111,11 +154,6 @@ spec:
|
|||||||
kind: Rule
|
kind: Rule
|
||||||
middlewares:
|
middlewares:
|
||||||
- name: llmgateway
|
- name: llmgateway
|
||||||
# Traefik requires a real backing Service for a router even though
|
|
||||||
# this middleware short-circuits every path it recognizes; a small
|
|
||||||
# dummy such as traefik/whoami (see ../examples/docker-compose.yml)
|
|
||||||
# or any always-up internal service works, since it is only ever
|
|
||||||
# reached via passthroughUnknown (default false, which 404s instead).
|
|
||||||
services:
|
services:
|
||||||
- name: llmgateway-dummy-backend
|
- name: llmgateway-dummy-backend
|
||||||
port: 80
|
port: 80
|
||||||
|
|||||||
Reference in New Issue
Block a user