mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-12 04:54:45 +00:00
fix: v0.2 final review wave
Transcriptions Yaegi coverage via integration mock, flattened admin usage batch, hardened CSP, sanitized provider lastErr, source-archive prefix template, vendor export-ignore warning, release paths.
This commit is contained in:
@@ -3,6 +3,17 @@
|
||||
# (.goreleaser.yaml), so these entries are the actual filter for the
|
||||
# release source tarball — not the archive block's own file globs, which
|
||||
# have no negation syntax and cannot exclude anything.
|
||||
# WARNING (v0.2 final review wave, 2026-08-20): vendor/ is safe to
|
||||
# export-ignore ONLY because every current go.mod dependency
|
||||
# (github.com/stretchr/testify, its go.yaml.in/yaml/v3 indirect) is a
|
||||
# TEST-only dependency, imported solely by *_test.go files — which are
|
||||
# themselves export-ignored below, so the released tarball never contains
|
||||
# code that imports them and never needs vendor/ to build. go.mod itself
|
||||
# is NOT export-ignored and ships in every release tarball as-is. The
|
||||
# moment any non-test .go file (production code, not *_test.go) imports a
|
||||
# third-party package, this line MUST be removed: otherwise the tarball
|
||||
# ships a go.mod requiring a module its own vendor/ was stripped out of,
|
||||
# and the extracted source cannot build standalone.
|
||||
vendor/ export-ignore
|
||||
integration/ export-ignore
|
||||
tools/ export-ignore
|
||||
|
||||
@@ -8,6 +8,11 @@ on:
|
||||
- "**.go"
|
||||
- "go.mod"
|
||||
- "go.sum"
|
||||
- ".goreleaser.yaml"
|
||||
- ".gitattributes"
|
||||
- ".traefik.yml"
|
||||
- "version.go"
|
||||
- "workflow-prepare.sh"
|
||||
workflow_dispatch:
|
||||
|
||||
permissions:
|
||||
|
||||
@@ -20,6 +20,13 @@ source:
|
||||
enabled: true
|
||||
format: tar.gz
|
||||
name_template: "{{ .ProjectName }}_v{{ .Version }}_source"
|
||||
# Extracting the tarball must land everything under one directory, not
|
||||
# loose at the archive root, the same way GitHub's own source tarballs
|
||||
# behave — without this, `tar xzf` scatters the plugin's files straight
|
||||
# into the extracting directory. Verified with `goreleaser release
|
||||
# --snapshot --clean`: the tarball's top-level entry is exactly
|
||||
# traefik-llmgateway_v{version}/ (v0.2 final review wave, 2026-08-20).
|
||||
prefix_template: "{{ .ProjectName }}_v{{ .Version }}/"
|
||||
|
||||
checksum:
|
||||
name_template: "{{ .ProjectName }}_v{{ .Version }}_checksums.txt"
|
||||
|
||||
@@ -171,7 +171,17 @@ config accepts them as YAML, which decodes to the same JSON shape.
|
||||
| `cache` | `CacheConfig` | `{}` (disabled) | Opt-in Redis-backed response cache for unified non-streaming chat/embeddings — see [Caching](#caching). Omitted or `enabled: false` means no caching, byte-identical to a gateway built before this field existed. |
|
||||
| `admin` | `*AdminConfig` | `nil` (disabled) | Read-only admin dashboard — see [Admin](#admin). `nil` or `enabled: false` means the `/admin*` routes are not registered at all. |
|
||||
| `modelAliases` | `map[string]string` | `{}` | Operator-defined alias id → target model id — see [Model aliases](#model-aliases). Omitted or empty means no aliases, byte-identical to a gateway built before this field existed. |
|
||||
| `passthroughUnknown` | `bool` | `false` | `false`: a request matching none of the plugin's routes gets a 404 JSON envelope. `true`: it falls through to the router's own backing service. |
|
||||
| `passthroughUnknown` | `bool` | `false` | `false`: a request matching none of the plugin's routes gets a 404 JSON envelope. `true`: it falls through to the router's own backing service. **Does not cover the three media routes** — see the upgrade note below. |
|
||||
|
||||
> **Upgrading from v0.1**: `POST /v1/images/generations`, `POST
|
||||
> /v1/audio/speech`, and `POST /v1/audio/transcriptions` are now handled by
|
||||
> the gateway itself, unconditionally — see [Image and audio
|
||||
> endpoints](#image-and-audio-endpoints). They no longer fall through to
|
||||
> `next` even when `passthroughUnknown: true`. A v0.1 deployment that
|
||||
> served its own backend on any of these three exact paths, relying on
|
||||
> passthrough to reach it, must move that backend to a different path
|
||||
> before upgrading to v0.2 — the gateway now intercepts these paths first,
|
||||
> every time.
|
||||
|
||||
Provider, `mcpServers`, and `agents` map **keys** (names) must match
|
||||
`^[a-zA-Z0-9._-]+$` and must not be `v1`, `mcp`, or `a2a` — those names are
|
||||
@@ -348,10 +358,15 @@ kept — a bad edit to the file never breaks already-authenticated traffic.
|
||||
through the explicit `provider/model` form regardless of who won the
|
||||
bare id, and `GET /v1/models` lists both the bare (winner's) form and
|
||||
every `provider/model` form.
|
||||
- Authorization always requires both `group.allowsModel` and
|
||||
- **On the unified routes** (`modelRegistry.resolve`, used by
|
||||
`/v1/chat/completions`, `/v1/embeddings`, and the three media
|
||||
endpoints), authorization requires both `group.allowsModel` and
|
||||
`group.allowsProvider` for the resolved provider — a model that exists
|
||||
but the caller's group cannot reach returns 403, distinct from a model no
|
||||
configured provider knows at all (404).
|
||||
configured provider knows at all (404). Native passthrough
|
||||
(`/{provider}/...`) resolves no model at all and checks
|
||||
`group.allowsProvider` only — see the authorization row in [Unified vs.
|
||||
passthrough](#unified-vs-passthrough).
|
||||
- **Discovery**: `GET {baseUrl}/v1/models` for an `openai`-type provider,
|
||||
`GET {baseUrl}/v1/models` for `anthropic`, `GET {baseUrl}/v1beta/models`
|
||||
for `gemini`. Runs once at plugin construction (bounded to 5s per
|
||||
@@ -489,9 +504,11 @@ predates this field keeps making exactly one upstream attempt per
|
||||
request.
|
||||
|
||||
- **Scope**: every adapter upstream call — chat, embeddings, model
|
||||
listing — across all three provider types. Native passthrough
|
||||
(`/{provider}/...`) and the MCP/A2A proxy are raw reverse proxies and
|
||||
are never retried; the client owns retry semantics there.
|
||||
listing, and the three media endpoints (`images/generations`,
|
||||
`audio/speech`, `audio/transcriptions`) — across all three provider
|
||||
types. Native passthrough (`/{provider}/...`) and the MCP/A2A proxy are
|
||||
raw reverse proxies and are never retried; the client owns retry
|
||||
semantics there.
|
||||
- **Attempts is retries, not tries**: `attempts` counts retries
|
||||
performed **after** the first try. The default, `1`, allows one retry
|
||||
— two tries total, not one. The maximum, `3`, allows three retries —
|
||||
@@ -579,6 +596,7 @@ cached bodies for the same request.
|
||||
| | Unified (`/v1/...`) | Native passthrough (`/{provider}/...`) |
|
||||
|---|---|---|
|
||||
| Wire format | Always OpenAI-shaped in, OpenAI-shaped out. | The provider's own native format, untouched. |
|
||||
| Authorization | `group.allowsModel` AND `group.allowsProvider`, both checked against the resolved provider (`modelRegistry.resolve` — see [Model routing](#model-routing)). | `group.allowsProvider` only. Passthrough resolves no model at all, so there is nothing for `allowsModel` to gate. |
|
||||
| Translation | `openai`-type: body forwarded verbatim (model id rewritten). `anthropic`/`gemini`: full bidirectional translation, including streaming, chunk by chunk. | None — a raw reverse proxy. |
|
||||
| Model alias in the response | Translated providers (`anthropic`, `gemini`) echo back the client's exact requested model string in the response's `model` field, even though the upstream call used the resolved provider model id. An `openai`-type response is a verbatim passthrough of the upstream body, so it carries whatever model id the upstream itself returned — this asymmetry is intentional, not a bug. | The upstream's own `model` field, verbatim — there is no alias to echo. |
|
||||
| Embeddings | `openai`-type: passthrough. `gemini`: mapped to `:embedContent`/`:batchEmbedContents`. `anthropic`: **501** — Anthropic's API has no embeddings endpoint. | Whatever the provider itself supports at that path; the gateway does not gate it. |
|
||||
@@ -592,6 +610,14 @@ cached bodies for the same request.
|
||||
`/v1/chat/completions` — auth, model routing, group authorization, limits,
|
||||
adapter call — minus response caching and token/cost accounting (below).
|
||||
|
||||
> **Upgrading from v0.1**: these three routes are now handled by the
|
||||
> gateway unconditionally. They are matched and served before
|
||||
> `passthroughUnknown` is ever consulted, so a request to any of them
|
||||
> **never** falls through to `next` — not even with `passthroughUnknown:
|
||||
> true`. If a v0.1 deployment pointed its router's backing service at one
|
||||
> of these exact paths and relied on passthrough to reach it, that backend
|
||||
> must move to a different path before upgrading.
|
||||
|
||||
| | `openai`-type | `gemini` | `anthropic` |
|
||||
|---|---|---|---|
|
||||
| `images/generations` | Native forward. | Translated to Imagen's `:predict` (below). | **501**. |
|
||||
@@ -674,7 +700,10 @@ request to any of them falls through to the plugin's existing
|
||||
gate: unauthenticated → 401, authenticated non-admin → 403, admin →
|
||||
serve. `overview` returns the provider list, groups, the configured
|
||||
model alias table (`alias`/`target` pairs, sorted by alias — see
|
||||
[Model aliases](#model-aliases)), redis/cache status, and the plugin
|
||||
[Model aliases](#model-aliases)), redis/cache/retry status (`retry`'s
|
||||
`enabled`/`attempts`/`backoff` are the EFFECTIVE values after
|
||||
[Retry](#retry)'s own defaulting, not the raw config — `attempts` and
|
||||
`backoff` are both omitted when retry is disabled), and the plugin
|
||||
version string. `usage` returns every user's and every group's
|
||||
current-window counter values (req/min, req/day, tok/day, tok/month,
|
||||
cost/day, cost/month) alongside their configured limits.
|
||||
@@ -793,6 +822,18 @@ request to any of them falls through to the plugin's existing
|
||||
move a token counter and always record cost as 0, regardless of what a
|
||||
provider itself would charge for them — a real gap for an operator
|
||||
billing on these endpoints, not just an accounting nuance.
|
||||
- **An upstream response over 32MiB (`maxResponseBytes`, `providers.go`)
|
||||
is silently truncated, not rejected.** Every adapter response read — the
|
||||
unified chat/embeddings JSON, and the three media endpoints — is capped
|
||||
at this limit. `audio/speech`'s streaming copy (`openaiAdapter.audioSpeech`,
|
||||
`provider_openai.go`) already committed the upstream's 200 status before
|
||||
it starts copying bytes, so a response over the cap simply ends early: no
|
||||
error, no truncation signal of any kind, a shorter audio file
|
||||
indistinguishable from a genuinely short one. `images/generations` and
|
||||
`audio/transcriptions` (`forwardMediaBody`) read their whole response
|
||||
before writing a status, so they hit the identical cap with the same
|
||||
silent-truncation-into-200 outcome. 32MiB is generous for any of these
|
||||
payloads in practice, but nothing enforces or signals the cutoff.
|
||||
- **The response cache shares entries across every user and group that
|
||||
can reach a model.** `cache`'s key (see [Caching](#caching)) carries no
|
||||
user identity by design, so two different callers sending the same
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -22,8 +23,16 @@ const (
|
||||
// folded review item, 2026-08-20 review): no external assets of any
|
||||
// kind, only this page's own inline script/style, and fetch calls
|
||||
// restricted to same-origin — the dashboard works air-gapped and cannot
|
||||
// be coerced into loading anything off-host.
|
||||
const adminCSP = "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; connect-src 'self'"
|
||||
// be coerced into loading anything off-host. frame-ancestors/base-uri/
|
||||
// form-action are all 'none' (v0.2 final review wave, 2026-08-20): the
|
||||
// page carries a password-type key-entry input, so it must never be
|
||||
// embeddable in another site's frame (clickjacking), never have its
|
||||
// <base> href hijacked to retarget a relative script/fetch URL, and never
|
||||
// have its auth-form submitted anywhere but nowhere at all — the form's
|
||||
// own submit handler already intercepts and cancels the real submit
|
||||
// (adminPageHTML's "submit" listener calls preventDefault()), so
|
||||
// form-action has nothing legitimate to allow.
|
||||
const adminCSP = "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; connect-src 'self'; frame-ancestors 'none'; base-uri 'none'; form-action 'none'"
|
||||
|
||||
// adminEnabled reports whether cfg's Admin block is present and enabled —
|
||||
// the single gate ServeHTTP checks before matching any /admin* route at
|
||||
@@ -139,6 +148,29 @@ func sanitizeBaseURL(raw string) string {
|
||||
return u.String()
|
||||
}
|
||||
|
||||
// sanitizeProviderErr applies sanitizeBaseURL's userinfo/query-stripping
|
||||
// treatment to lastErr before it is ever echoed by GET /admin/api/overview
|
||||
// (v0.2 final review wave, 2026-08-20): a provider's discovery/refresh
|
||||
// error commonly embeds the request URL it failed against (Go's
|
||||
// net/http wraps a *url.Error carrying it verbatim), so a baseUrl
|
||||
// misconfigured with embedded credentials would otherwise leak straight
|
||||
// back out through this field, even though adminProviderView.BaseURL
|
||||
// itself is already sanitized.
|
||||
//
|
||||
// This is best-effort, not a general URL scrubber: it looks only for
|
||||
// rawBaseURL appearing verbatim inside lastErr and replaces every such
|
||||
// occurrence with sanitizeBaseURL's cleaned form. An error naming some
|
||||
// OTHER URL — a redirect target, a differently-formatted variant of the
|
||||
// same host — is not caught; recognizing every URL shape a wrapped error
|
||||
// might embed, without risking mangling ordinary error text, is not
|
||||
// attempted here.
|
||||
func sanitizeProviderErr(lastErr, rawBaseURL string) string {
|
||||
if lastErr == "" || rawBaseURL == "" || !strings.Contains(lastErr, rawBaseURL) {
|
||||
return lastErr
|
||||
}
|
||||
return strings.ReplaceAll(lastErr, rawBaseURL, sanitizeBaseURL(rawBaseURL))
|
||||
}
|
||||
|
||||
// adminProviderView is one provider's read-only view in GET
|
||||
// /admin/api/overview (spec §4, v0.2). baseUrl is not secret — the
|
||||
// spec's NEVER-exposed list is API keys (not even digests), provider
|
||||
@@ -179,6 +211,21 @@ type adminCacheView struct {
|
||||
Enabled bool `json:"enabled"`
|
||||
}
|
||||
|
||||
// adminRetryView is the retry config summary in GET /admin/api/overview
|
||||
// (v0.2 final review wave, 2026-08-20): the EFFECTIVE enabled/attempts/
|
||||
// backoff a `retry: {enabled: true}` block resolves to, after
|
||||
// newRetryPolicy's own zero-value defaulting (retry.go) — not the raw,
|
||||
// possibly-empty RetryConfig fields, so an operator sees what actually
|
||||
// governs upstream calls, not just what they left unset. Attempts and
|
||||
// Backoff are both omitted (zero value / empty string) when retry is
|
||||
// disabled, mirroring adminCacheView's own omitempty convention for a
|
||||
// disabled feature's now-meaningless detail fields.
|
||||
type adminRetryView struct {
|
||||
Backoff string `json:"backoff,omitempty"`
|
||||
Enabled bool `json:"enabled"`
|
||||
Attempts int `json:"attempts,omitempty"`
|
||||
}
|
||||
|
||||
// adminGroupView is one group's summary in GET /admin/api/overview.
|
||||
type adminGroupView struct {
|
||||
Limits *LimitsConfig `json:"limits,omitempty"`
|
||||
@@ -205,6 +252,7 @@ type adminOverviewResponse struct {
|
||||
Aliases []adminAliasView `json:"aliases"`
|
||||
Redis adminRedisView `json:"redis"`
|
||||
Cache adminCacheView `json:"cache"`
|
||||
Retry adminRetryView `json:"retry"`
|
||||
}
|
||||
|
||||
// buildAdminOverview assembles adminOverviewResponse from the registry,
|
||||
@@ -220,7 +268,7 @@ func (g *Gateway) buildAdminOverview() adminOverviewResponse {
|
||||
BaseURL: sanitizeBaseURL(s.baseURL),
|
||||
ModelCount: s.modelCount,
|
||||
LastRefresh: s.lastRefresh,
|
||||
LastErr: s.lastErr,
|
||||
LastErr: sanitizeProviderErr(s.lastErr, s.baseURL),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -232,6 +280,19 @@ func (g *Gateway) buildAdminOverview() adminOverviewResponse {
|
||||
ttl = g.cache.ttl.String()
|
||||
}
|
||||
|
||||
// newRetryPolicy is pure (no I/O) and already ran once, successfully,
|
||||
// when this Gateway was constructed (providers.go's buildAdapters) —
|
||||
// g.cfg.Retry cannot have changed since a running Gateway never
|
||||
// mutates its own config, so this second call is guaranteed to return
|
||||
// the identical result with a nil error; the error return is
|
||||
// intentionally discarded rather than propagated.
|
||||
rp, _ := newRetryPolicy(g.cfg.Retry)
|
||||
retryView := adminRetryView{Enabled: rp.enabled}
|
||||
if rp.enabled {
|
||||
retryView.Attempts = rp.attempts
|
||||
retryView.Backoff = rp.backoff.String()
|
||||
}
|
||||
|
||||
_, groupSummaries := g.auth.snapshot()
|
||||
groups := make([]adminGroupView, len(groupSummaries))
|
||||
for i, gs := range groupSummaries {
|
||||
@@ -248,6 +309,7 @@ func (g *Gateway) buildAdminOverview() adminOverviewResponse {
|
||||
Providers: providers,
|
||||
Redis: adminRedisView{Configured: configured, LastErr: redisLastErr, LastErrAt: redisLastErrAt},
|
||||
Cache: adminCacheView{Enabled: cacheEnabled, TTL: ttl},
|
||||
Retry: retryView,
|
||||
Groups: groups,
|
||||
Aliases: aliases,
|
||||
Version: pluginVersion,
|
||||
@@ -309,25 +371,28 @@ func usageEntryView(su scopeUsage, limits *LimitsConfig) adminUsageEntryView {
|
||||
// buildAdminUsage assembles adminUsageResponse: authStore.snapshot lists
|
||||
// every currently active user and every configured group (names, group
|
||||
// membership, and limits only — never a key or its digest), and
|
||||
// limiter.currentUsage reads each one's six current-window counters —
|
||||
// one batched round trip per scope (limits.go's storeGetMulti) rather
|
||||
// than six separate ones. Unlike buildLimitScopes (routes_unified.go),
|
||||
// this never omits an entity for having nil limits — the dashboard shows
|
||||
// usage for every user and group, limited or not.
|
||||
// limiter.currentUsage reads every one's six current-window counters in
|
||||
// ONE storeGetMulti round trip total (v0.2 final review wave, 2026-08-20;
|
||||
// see limiter.currentUsage's own doc comment for the tradeoff this
|
||||
// supersedes) — users and groups are concatenated into a single scopes
|
||||
// slice before that one call, then the flat result is sliced back into
|
||||
// the two response sections at the same split point, order preserved.
|
||||
// Unlike buildLimitScopes (routes_unified.go), this never omits an
|
||||
// entity for having nil limits — the dashboard shows usage for every
|
||||
// user and group, limited or not.
|
||||
func (g *Gateway) buildAdminUsage() adminUsageResponse {
|
||||
userSummaries, groupSummaries := g.auth.snapshot()
|
||||
|
||||
userScopes := make([]limitScope, len(userSummaries))
|
||||
for i, us := range userSummaries {
|
||||
userScopes[i] = limitScope{kind: "user", id: us.name, limits: us.limits}
|
||||
scopes := make([]limitScope, 0, len(userSummaries)+len(groupSummaries))
|
||||
for _, us := range userSummaries {
|
||||
scopes = append(scopes, limitScope{kind: "user", id: us.name, limits: us.limits})
|
||||
}
|
||||
groupScopes := make([]limitScope, len(groupSummaries))
|
||||
for i, gs := range groupSummaries {
|
||||
groupScopes[i] = limitScope{kind: "group", id: gs.name, limits: gs.limits}
|
||||
for _, gs := range groupSummaries {
|
||||
scopes = append(scopes, limitScope{kind: "group", id: gs.name, limits: gs.limits})
|
||||
}
|
||||
|
||||
userUsage := g.limiter.currentUsage(userScopes)
|
||||
groupUsage := g.limiter.currentUsage(groupScopes)
|
||||
allUsage := g.limiter.currentUsage(scopes)
|
||||
userUsage, groupUsage := allUsage[:len(userSummaries)], allUsage[len(userSummaries):]
|
||||
|
||||
users := make([]adminUsageEntryView, len(userUsage))
|
||||
for i, su := range userUsage {
|
||||
@@ -571,6 +636,8 @@ const adminPageHTML = `<!doctype html>
|
||||
infra.appendChild(el("div", redisText, "infra-line"));
|
||||
var cacheText = "Cache: " + (data.cache.enabled ? "enabled (ttl " + data.cache.ttl + ")" : "disabled");
|
||||
infra.appendChild(el("div", cacheText, "infra-line"));
|
||||
var retryText = "Retry: " + (data.retry.enabled ? "enabled (attempts " + data.retry.attempts + ", backoff " + data.retry.backoff + ")" : "disabled");
|
||||
infra.appendChild(el("div", retryText, "infra-line"));
|
||||
infra.appendChild(el("div", "Version: " + data.version, "infra-line muted"));
|
||||
|
||||
groupMeta = {};
|
||||
|
||||
+46
-7
@@ -263,6 +263,12 @@ func TestAdminOverview_SortedShapeAndVersion(t *testing.T) {
|
||||
if got.Redis.Configured {
|
||||
t.Error("redis.configured must be false: cfg.Redis was never configured")
|
||||
}
|
||||
if got.Retry.Enabled {
|
||||
t.Error("retry.enabled must be false: cfg.Retry was never configured")
|
||||
}
|
||||
if got.Retry.Attempts != 0 || got.Retry.Backoff != "" {
|
||||
t.Errorf("retry.attempts/backoff must be omitted when disabled, got %+v", got.Retry)
|
||||
}
|
||||
|
||||
wantAliases := []adminAliasView{
|
||||
{Alias: "aaliased/y", Target: "alpha/a-model-1"},
|
||||
@@ -278,6 +284,38 @@ func TestAdminOverview_SortedShapeAndVersion(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminOverview_RetryEffectiveValues proves GET /admin/api/overview's
|
||||
// retry block reports the EFFECTIVE, defaulted values newRetryPolicy
|
||||
// (retry.go) resolves an enabled retry block to, not the raw config —
|
||||
// Attempts and Backoff are both left at their zero value here specifically
|
||||
// to exercise that defaulting (v0.2 final review wave, 2026-08-20).
|
||||
func TestAdminOverview_RetryEffectiveValues(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newAdminTestConfig()
|
||||
cfg.Retry = RetryConfig{Enabled: true}
|
||||
h, _ := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, adminRequest(http.MethodGet, adminOverviewPath, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
|
||||
var got adminOverviewResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if !got.Retry.Enabled {
|
||||
t.Fatal("retry.enabled must be true")
|
||||
}
|
||||
if got.Retry.Attempts != defaultRetryAttempts {
|
||||
t.Errorf("retry.attempts = %d, want the default %d (raw config left Attempts at 0)", got.Retry.Attempts, defaultRetryAttempts)
|
||||
}
|
||||
if got.Retry.Backoff != defaultRetryBackoff {
|
||||
t.Errorf("retry.backoff = %q, want the default %q (raw config left Backoff empty)", got.Retry.Backoff, defaultRetryBackoff)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdminOverview_ProviderLastErrAfterFailedRefresh(t *testing.T) {
|
||||
t.Parallel()
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -478,7 +516,7 @@ func TestAdminPage_CSPHeaderAndFetchURLs(t *testing.T) {
|
||||
if ct := rec.Header().Get("Content-Type"); !strings.Contains(ct, "text/html") {
|
||||
t.Errorf("content-type = %q, want text/html", ct)
|
||||
}
|
||||
wantCSP := "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; connect-src 'self'"
|
||||
wantCSP := "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; connect-src 'self'; frame-ancestors 'none'; base-uri 'none'; form-action 'none'"
|
||||
if got := rec.Header().Get("Content-Security-Policy"); got != wantCSP {
|
||||
t.Errorf("CSP = %q, want %q", got, wantCSP)
|
||||
}
|
||||
@@ -506,7 +544,7 @@ func TestAdmin_JSONSecurityHeaders(t *testing.T) {
|
||||
cfg := newAdminTestConfig()
|
||||
h, _ := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
wantCSP := "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; connect-src 'self'"
|
||||
wantCSP := "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; connect-src 'self'; frame-ancestors 'none'; base-uri 'none'; form-action 'none'"
|
||||
for _, p := range []string{adminOverviewPath, adminUsagePath} {
|
||||
t.Run(p, func(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
@@ -660,8 +698,9 @@ func TestAdminOverview_BaseURLStripsCredentials(t *testing.T) {
|
||||
|
||||
// countingMultiStore is a counterStore stub whose getMulti serves a fixed
|
||||
// value per key and counts how many times it was called — it proves
|
||||
// limiter.currentUsage batches a scope's six counter reads into one
|
||||
// getMulti call rather than issuing them as six separate get calls.
|
||||
// limiter.currentUsage flattens every scope's six counter reads into ONE
|
||||
// getMulti call total (v0.2 final review wave, 2026-08-20), rather than
|
||||
// six separate get calls or even one getMulti call per scope.
|
||||
// incrBy/get are never exercised by currentUsage and just return zero
|
||||
// values.
|
||||
type countingMultiStore struct {
|
||||
@@ -682,7 +721,7 @@ func (s *countingMultiStore) getMulti(keys []string) ([]int64, error) {
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func TestLimiterCurrentUsage_BatchesOneGetMultiCallPerScope(t *testing.T) {
|
||||
func TestLimiterCurrentUsage_BatchesOneGetMultiCallForAllScopes(t *testing.T) {
|
||||
t.Parallel()
|
||||
fixedNow := time.Date(2026, 8, 20, 10, 0, 0, 0, time.UTC)
|
||||
store := &countingMultiStore{values: map[string]int64{
|
||||
@@ -703,8 +742,8 @@ func TestLimiterCurrentUsage_BatchesOneGetMultiCallPerScope(t *testing.T) {
|
||||
}
|
||||
got := l.currentUsage(scopes)
|
||||
|
||||
if store.getMultiCalls != len(scopes) {
|
||||
t.Errorf("getMultiCalls = %d, want %d (one pipelined getMulti per scope, not six separate get calls)", store.getMultiCalls, len(scopes))
|
||||
if store.getMultiCalls != 1 {
|
||||
t.Errorf("getMultiCalls = %d, want 1 (all scopes' keys flattened into one pipelined getMulti call)", store.getMultiCalls)
|
||||
}
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("len(got) = %d, want 2", len(got))
|
||||
|
||||
@@ -21,6 +21,7 @@ import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"mime/multipart"
|
||||
"net/http"
|
||||
"os"
|
||||
"os/exec"
|
||||
@@ -89,6 +90,61 @@ func doJSON(t *testing.T, method, url, apiKey string, body any) (*http.Response,
|
||||
return resp, out
|
||||
}
|
||||
|
||||
// buildMultipartAudio builds a multipart/form-data body carrying a dummy
|
||||
// "file" part and a "model" part set to model, mirroring what an
|
||||
// OpenAI-SDK client sends to POST /v1/audio/transcriptions. It returns the
|
||||
// encoded body and its matching Content-Type (including the boundary
|
||||
// parameter multipart.Writer generated).
|
||||
func buildMultipartAudio(t *testing.T, model string) ([]byte, string) {
|
||||
t.Helper()
|
||||
var buf bytes.Buffer
|
||||
mw := multipart.NewWriter(&buf)
|
||||
fw, err := mw.CreateFormFile("file", "audio.wav")
|
||||
if err != nil {
|
||||
t.Fatalf("create form file part: %v", err)
|
||||
}
|
||||
if _, err := fw.Write([]byte("mock-audio-bytes")); err != nil {
|
||||
t.Fatalf("write file part: %v", err)
|
||||
}
|
||||
if err := mw.WriteField("model", model); err != nil {
|
||||
t.Fatalf("write model field: %v", err)
|
||||
}
|
||||
if err := mw.Close(); err != nil {
|
||||
t.Fatalf("close multipart writer: %v", err)
|
||||
}
|
||||
return buf.Bytes(), mw.FormDataContentType()
|
||||
}
|
||||
|
||||
// doMultipart issues one multipart/form-data POST, mirroring doJSON's
|
||||
// contract: the returned *http.Response has already had its body read and
|
||||
// closed, and a non-JSON or empty body decodes to a nil map.
|
||||
func doMultipart(t *testing.T, url, apiKey, contentType string, body []byte) (*http.Response, map[string]any) {
|
||||
t.Helper()
|
||||
|
||||
req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(body))
|
||||
if err != nil {
|
||||
t.Fatalf("new request POST %s: %v", url, err)
|
||||
}
|
||||
req.Header.Set("Authorization", "Bearer "+apiKey)
|
||||
req.Header.Set("Content-Type", contentType)
|
||||
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
t.Fatalf("do request POST %s: %v", url, err)
|
||||
}
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
raw, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
t.Fatalf("read response body from POST %s: %v", url, err)
|
||||
}
|
||||
var out map[string]any
|
||||
if len(raw) > 0 {
|
||||
_ = json.Unmarshal(raw, &out)
|
||||
}
|
||||
return resp, out
|
||||
}
|
||||
|
||||
// firstChoiceContent extracts choices[0].message.content from an
|
||||
// OpenAI-shaped chat completion response body, failing the test if the
|
||||
// shape doesn't match.
|
||||
@@ -827,6 +883,39 @@ func TestAudioSpeech(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAudioTranscriptions covers the review wave's highest-value Yaegi gap:
|
||||
// POST /v1/audio/transcriptions' multipart handling under real
|
||||
// Traefik+Yaegi. A provider-prefixed model ("openai/gpt-mock") drives
|
||||
// rewriteMultipartModel (routes_media.go), which rebuilds the multipart
|
||||
// body with the "model" part rewritten to the resolved bare upstream id
|
||||
// before it reaches the mock — asserted via mock/main.go's
|
||||
// handleOpenAIAudioTranscriptions echoing "model_received" back. A second,
|
||||
// already-bare request ("gpt-mock") exercises the byte-identical replay
|
||||
// path instead: no rebuild, same bare id reported back verbatim.
|
||||
func TestAudioTranscriptions(t *testing.T) {
|
||||
t.Run("provider-prefixed model is rewritten before upload", func(t *testing.T) {
|
||||
body, contentType := buildMultipartAudio(t, "openai/gpt-mock")
|
||||
resp, decoded := doMultipart(t, traefik1URL+"/v1/audio/transcriptions", aliceKey, contentType, body)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("status = %d, body=%#v", resp.StatusCode, decoded)
|
||||
}
|
||||
if got, _ := decoded["model_received"].(string); got != "gpt-mock" {
|
||||
t.Errorf("model_received = %q, want %q (rewritten from the provider-prefixed id)", got, "gpt-mock")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("bare model replays the original body unchanged", func(t *testing.T) {
|
||||
body, contentType := buildMultipartAudio(t, "gpt-mock")
|
||||
resp, decoded := doMultipart(t, traefik1URL+"/v1/audio/transcriptions", aliceKey, contentType, body)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("status = %d, body=%#v", resp.StatusCode, decoded)
|
||||
}
|
||||
if got, _ := decoded["model_received"].(string); got != "gpt-mock" {
|
||||
t.Errorf("model_received = %q, want %q (byte-identical replay, no rewrite needed)", got, "gpt-mock")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestAdminDashboard covers v0.2 integration coverage (spec §4): the
|
||||
// unauthenticated HTML shell carries no data, the two JSON routes gate
|
||||
// on admin vs. non-admin vs. unauthenticated, and GET /admin/api/usage
|
||||
|
||||
@@ -84,6 +84,7 @@ func newServerMux(mode string) (*http.ServeMux, error) {
|
||||
mux.HandleFunc("/flaky/reset", handleFlakyReset)
|
||||
mux.HandleFunc("/v1/images/generations", handleOpenAIImages)
|
||||
mux.HandleFunc("/v1/audio/speech", handleOpenAIAudioSpeech)
|
||||
mux.HandleFunc("/v1/audio/transcriptions", handleOpenAIAudioTranscriptions)
|
||||
case "anthropic":
|
||||
mux.HandleFunc("/v1/models", handleAnthropicModels)
|
||||
mux.HandleFunc("/v1/messages", handleAnthropicMessages)
|
||||
@@ -309,6 +310,27 @@ func handleOpenAIAudioSpeech(w http.ResponseWriter, r *http.Request) {
|
||||
_, _ = w.Write(mockAudioBytes)
|
||||
}
|
||||
|
||||
// handleOpenAIAudioTranscriptions answers POST /v1/audio/transcriptions by
|
||||
// parsing the multipart body Traefik forwarded and echoing back the "model"
|
||||
// form field it finds — this is the review wave's highest-value Yaegi
|
||||
// coverage gap closed: the audio-transcriptions integration test sends a
|
||||
// provider-prefixed model ("openai/gpt-mock") and asserts this handler
|
||||
// receives the bare, rewritten form ("gpt-mock"), proving
|
||||
// rewriteMultipartModel (routes_media.go) actually rebuilds the multipart
|
||||
// body under real Traefik+Yaegi, not just in the compiled unit tests. A
|
||||
// second, already-bare request exercises the byte-identical replay path
|
||||
// instead, so this same field still reads back "gpt-mock" with no rebuild.
|
||||
func handleOpenAIAudioTranscriptions(w http.ResponseWriter, r *http.Request) {
|
||||
if err := r.ParseMultipartForm(1 << 20); err != nil { //nolint:gosec // G120: maxMemory is set (1MiB) and the whole handler is behind readCapped's 10MiB gateway-side cap (routes_media.go) before this mock ever sees the body; test-only fixture server, not production code
|
||||
http.Error(w, `{"error":{"message":"invalid multipart body","type":"invalid_request_error"}}`, http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
writeJSON(w, map[string]any{
|
||||
"text": "mock transcription",
|
||||
"model_received": r.FormValue("model"),
|
||||
})
|
||||
}
|
||||
|
||||
// --- anthropic mode ----------------------------------------------------------
|
||||
|
||||
func handleAnthropicModels(w http.ResponseWriter, _ *http.Request) {
|
||||
|
||||
@@ -723,42 +723,60 @@ func usageWindowKeys(sc limitScope, now time.Time) []string {
|
||||
// does, and it is read-only: unlike checkAndCount, it never increments
|
||||
// anything.
|
||||
//
|
||||
// One storeGetMulti call per scope (controller-approved amendment,
|
||||
// 2026-08-20 review): the original per-metric l.getCounter loop issued
|
||||
// six separate round trips per scope against the single shared Redis
|
||||
// connection — GET /admin/api/usage serialized 6×(users+groups) of them.
|
||||
// Batching all scopes' keys into one single storeGetMulti call would cut
|
||||
// that further, to one round trip total, but was not taken here: it
|
||||
// would need buildAdminUsage to flatten and later re-split two
|
||||
// differently-sized scope lists (users, groups) around one shared call,
|
||||
// for a further win only realized when both lists are non-trivially
|
||||
// large. One call per scope already turns 6N round trips into N, keeps
|
||||
// currentUsage's per-scope iteration shape identical to
|
||||
// checkAndCount/evaluateScope's, and keeps a single scope's store
|
||||
// failure from forcing every other scope in the same request to look
|
||||
// storeDown too. Order is preserved: currentUsage(scopes)[i] corresponds
|
||||
// to scopes[i].
|
||||
// ONE storeGetMulti call for the whole scopes slice (v0.2 final review
|
||||
// wave, 2026-08-20; supersedes the "one call per scope" amendment this
|
||||
// comment previously described, 2026-08-20 review): every scope's six
|
||||
// keys are flattened into a single round trip against the shared store,
|
||||
// rather than N separate ones. The reviewer-confirmed reason for this
|
||||
// second amendment: GET /admin/api/usage's per-scope loop was contending
|
||||
// with request admission on the same store connection as live traffic,
|
||||
// and N round trips (even pipelined 6-at-a-time) scales with the number
|
||||
// of configured users and groups in a way one round trip does not.
|
||||
// buildAdminUsage (admin.go) drives this by concatenating its users and
|
||||
// groups into one scopes slice before calling in, then slicing the flat
|
||||
// result back into its two response sections at the same split point.
|
||||
//
|
||||
// Tradeoff given up by this second amendment: a transient store error
|
||||
// now marks every scope in the call storeDown together, where the
|
||||
// previous per-scope loop could let one scope's failure land mid-batch
|
||||
// while a later scope's own independent call still succeeded. That
|
||||
// isolation was never a documented guarantee of this method, only an
|
||||
// incidental property of how it was written — cutting N round trips to 1
|
||||
// is the reviewer-confirmed right side of this tradeoff. Order is
|
||||
// preserved: currentUsage(scopes)[i] corresponds to scopes[i].
|
||||
func (l *limiter) currentUsage(scopes []limitScope) []scopeUsage {
|
||||
now := l.now()
|
||||
out := make([]scopeUsage, len(scopes))
|
||||
for i, sc := range scopes {
|
||||
keys := usageWindowKeys(sc, now)
|
||||
vals, ok := l.storeGetMulti(keys)
|
||||
// len(vals) != len(keys) is reachable only from a non-conforming
|
||||
// counterStore implementation (a real respClient.getMulti and
|
||||
// memoryStore.getMulti both always return one value per key) —
|
||||
// guarded defensively so a future or test-only store's short
|
||||
// slice reports storeDown instead of panicking on an
|
||||
// out-of-range index below (review sweep, 2026-08-20).
|
||||
if !ok || len(vals) != len(keys) {
|
||||
if len(scopes) == 0 {
|
||||
return out
|
||||
}
|
||||
|
||||
now := l.now()
|
||||
allKeys := make([]string, 0, len(scopes)*6)
|
||||
for _, sc := range scopes {
|
||||
allKeys = append(allKeys, usageWindowKeys(sc, now)...)
|
||||
}
|
||||
|
||||
vals, ok := l.storeGetMulti(allKeys)
|
||||
// len(vals) != len(allKeys) is reachable only from a non-conforming
|
||||
// counterStore implementation (a real respClient.getMulti and
|
||||
// memoryStore.getMulti both always return one value per key) —
|
||||
// guarded defensively so a future or test-only store's short slice
|
||||
// reports every scope storeDown instead of panicking on an
|
||||
// out-of-range index below (review sweep, 2026-08-20).
|
||||
if !ok || len(vals) != len(allKeys) {
|
||||
for i, sc := range scopes {
|
||||
out[i] = scopeUsage{kind: sc.kind, id: sc.id, storeDown: true}
|
||||
continue
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
for i, sc := range scopes {
|
||||
v := vals[i*6 : i*6+6]
|
||||
out[i] = scopeUsage{
|
||||
kind: sc.kind, id: sc.id,
|
||||
requestsPerMinute: vals[0], requestsPerDay: vals[1],
|
||||
tokensPerDay: vals[2], tokensPerMonth: vals[3],
|
||||
costPerDayMicros: vals[4], costPerMonthMicros: vals[5],
|
||||
requestsPerMinute: v[0], requestsPerDay: v[1],
|
||||
tokensPerDay: v[2], tokensPerMonth: v[3],
|
||||
costPerDayMicros: v[4], costPerMonthMicros: v[5],
|
||||
}
|
||||
}
|
||||
return out
|
||||
|
||||
Reference in New Issue
Block a user