The service CIDR is 10.106.0.0/16, so the ipBlock 10.96.0.0/12 never covered
ingress-nginx (10.101.128.185). The JWKS fetch to zitadel.riotpiao.com therefore
hung until the context deadline: every caller got 403, and the pod failed its
liveness probe before serving traffic.
Add an egress rule for TCP 443 to the ingress-nginx namespace and point jwksUrl
back at the public host. The ZITADEL service port 8080 answers 404 for
/oauth/v2/keys, so the service endpoint is not usable.
The public host resolves through the CoreDNS rewrite to ingress-nginx and the
fetch timed out, so the validator answered 403 with a context-deadline error on
every caller. Point jwksUrl at zitadel.iam.svc.cluster.local:8080, which the
NetworkPolicy already allows.
cmd/gateway/main.go:144 constructs the adapter validator only when both
cfg.Auth.Enabled and cfg.Auth.JWKSURL are set. The config left jwksUrl unset on
purpose, so jwtValidator stayed nil and every adapter request passed with one
warning line:
auth required but jwtValidator nil; request unauthenticated
That made auth.required cosmetic: sqs, workflow, memory, s3 and iam all answered
unauthenticated calls. Set the JWKS URL to the ZITADEL key endpoint.
workflow, memory, s3 and iam served requests without any Authorization header.
Only sqs and the LLM models checked a token, so four upstreams were reachable
from inside the cluster with no authentication at all.
Set auth.required true on all five adapters. The dispatcher then enforces one
capability per method, derived from the verb
(internal/serviceadapter/router.go:142):
GET/HEAD/OPTIONS -> "<service>:read"
anything else -> "<service>:write"
Those strings must exist as ZITADEL role keys with the colon. The project now
holds memory:read, memory:write, workflow:read, workflow:execute, s3:read,
s3:write, iam:read, iam:write, sqs:read and sqs:write, and the riotpiao user
grant carries them, so a browser session keeps working.
Correct the old comment: ZITADEL does store colon role keys. Verified by listing
the project roles, which returned all eleven colon keys.
Replace the handwritten Authentik JWT checks with github.com/zitadel/zitadel-go/v3.
What the SDK now owns:
- OIDC discovery (client.Discover) to find issuer and JWKS URI
- remote, auto-refreshing JWKS (rp.NewRemoteKeySet)
- signature, expiry and issuer checks (op.NewAccessTokenVerifier,
op.VerifyAccessToken)
- the verified claim set and role lookup (oauth.IntrospectionContext,
IsGrantedRole)
What stays local:
- a multi-audience allow-list, because this gateway is a shared resource
server and the SDK's oauth.WithJWT accepts exactly one client ID
- an adapter from IntrospectionContext to jwt.MapClaims so the rest of the
gateway keeps one claim representation
Other changes:
- issuer is configuration-driven; the hardcoded authentik.riotpiao.com
check is gone
- aud may be a string or an array (ZITADEL sends an array)
- CheckPermissions also reads urn:zitadel:iam:org:project:roles, the claim
ZITADEL uses for project roles
- requiredCapability accepts a comma-separated list, so llm:inference and
llm-inference both satisfy the LLM gate
- a five-segment token is reported as an encrypted JWE with the fix in the
message, instead of failing as a bad signature
Config: auth.zitadelDomain turns the SDK path on; jwksUrl stays supported for
the legacy path and for tests.
Follow-up to #65, which is already merged. The review of that PR returned BLOCK with two high-severity defects; the fix commit was pushed **after** the merge landed, so none of it is in `main`.
## What happened
- `main` @ `369d51a` is the squash of the first commit only (`c5a91cf`).
- `d71c884` (review fixes) is on the old branch and was never merged — PR #65 is closed.
- Verified against `main` directly: `ADMISSION_MAX_INFLIGHT` 0 hits, `func (rw *responseWriter) Flush` 0 hits, `ModifyResponse` 0 hits, `Snapshot` 0 hits.
- The image in production (`forgejo.riotpiao.com/rock/api-gateway@sha256:64f2bfc1…`, 3/3 replicas) is built from that squash, so the defects below are **live right now**.
## Live state confirmed
`https://api.riotpiao.com/metrics` on the running pods:
```
observer_degraded 0
vllm_requests_running 1
vllm_kv_usage_perc 0.008293092588704587
admission_parked_total 0
engine_state_stale_total 0
```
The observer itself works: it is reading the real vLLM exposition body and the startup self-check passed (`observer_degraded 0`). No requests have parked yet, so the two defects below have not been observable in the wild — they are proven from code, not from a live failure.
## Defects fixed here
**1. The parked-stream keep-alive never runs in production (high).** `internal/tracing/middleware.go` wraps the `ResponseWriter` in a struct that embeds the interface and implements only `WriteHeader`/`Write`, so `w.(http.Flusher)` in `internal/proxy/proxy.go` is false for every real request (`cmd/gateway/main.go` → `tracing.Middleware` → `router`). `prewritten` therefore never becomes true, the heartbeat never starts, and a parked streaming request writes zero bytes for up to `ADMISSION_MAX_PARK_SECONDS` (120) — the client timeout the feature exists to prevent. Fixed by adding `Flush()` and `Unwrap()` to the wrapper.
**2. Upstream status masked once the SSE status is spent (high, latent behind 1).** `httputil.ReverseProxy` calls `rw.WriteHeader(res.StatusCode)` unconditionally; after the park wrote 200, net/http keeps 200 and the upstream error body is appended raw into the event stream, which a streaming client reads as an empty completion and reports as success. Fixed with a `ModifyResponse` that frames an upstream `>= 400` as an `event: error` when the status has already been written.
**3. Lost wakeup in the park loop.** `Get()` and `Tick()` were taken under two separate lock acquisitions, so a poll landing between them pushed the waiter onto the next generation's channel — up to one extra poll interval (2s) parked. `Snapshot()` now returns state and channel under one lock.
**4. `observer_degraded` pinned at 1 forever.** It was raised only on the *first* poll's parse failure and `SetObserverDegraded(false)` had no call site, so one truncated response pinned the gauge for the pod's life, while genuine metric-name drift appearing later was never reported at all. `markDegraded` now tracks current parsability in both directions and only touches the exporter on a change.
**5. `ADMISSION_MAX_INFLIGHT` silently defaulted.** Now set explicitly to `4` in `k8s/deployment.yaml` with the coupling to the engine's `--max-num-seqs` documented, instead of quietly inheriting `VLLM_MAX_NUM_SEQS`.
## Verification
`go build ./...` clean · `go vet ./...` clean · `go test ./...` 368 passed, 0 failed · `structural-check.sh` 0 errors (1 pre-existing `:latest` warning) · `go.mod` unchanged.
Cherry-picked onto `main` rather than merged from the old branch, so this diff is exactly the five fixes and nothing else: 5 files, +80/−8.
## Still open
The reviewer's BLOCK also rested on nine functions over CRAP>30 purely from 0% coverage — the deliberate "no new tests" decision. Worth recording that defect 1 is a one-line type assertion that a `httptest` case through the real middleware chain would have caught immediately. Raising it here so the decision is visible rather than implied.
---------
Co-authored-by: Poimen <[email protected]>
Reviewed-on: #66
Co-authored-by: poimen <[email protected]>
# Allow HTTPS to ingress-nginx. The gateway fetches JWKS and the OIDC
# discovery document from https://zitadel.riotpiao.com, which CoreDNS rewrites
# to this service. The service CIDR is 10.106.0.0/16, so the 10.96.0.0/12
# ipBlock below never covered it: the fetch hung until the context deadline and
# the pod failed its liveness probe.
- to:
- namespaceSelector:
matchLabels:
kubernetes.io/metadata.name:ingress-nginx
ports:
- protocol:TCP
port:443
# Allow to kubernetes API server (ArgoCD Application patching via in-cluster SA).
# Allow to kubernetes API server (ArgoCD Application patching via in-cluster SA).
# kube-apiserver runs on host network (192.168.1.162/166:6443) — must use ipBlock.
# kube-apiserver runs on host network (192.168.1.162/166:6443) — must use ipBlock.
# ClusterIP 10.96.0.1:443 also routes there, covered by the 10.96.0.0/12 CIDR.
# ClusterIP 10.96.0.1:443 also routes there, covered by the 10.96.0.0/12 CIDR.
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Admin Bot
