mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-12 04:54:45 +00:00
fix(admin): wire-level access tests, MCP/agent UI, glob-safe chips
Review fix round on the group-access-display feature (Should-Fix + folded items): 1. TestAdminUsage_GroupAccessLists now asserts at the raw-JSON level, not just the typed adminUsageResponse struct: decodes rows into map[string]json.RawMessage and checks the four access-list keys are ENTIRELY ABSENT on user/total rows and an unrestricted group, PRESENT with exact values on a restricted group. A typed-struct decode alone cannot prove omitempty is doing its job -- a present-but-null field decodes into the same nil []string a genuinely absent key would. Verified by temporarily stripping the omitempty tags: the new assertions fail (four keys read back 'null'); the old struct-only assertions stayed green throughout, confirming the gap the review flagged. 2. UsageView.vue's Access block now renders MCP servers and Agents too, same Badge treatment as Providers, with muted 'All MCP servers'/ 'All agents' fallbacks -- GroupConfig carries all four lists (llmgateway.go), so the display now matches the payload instead of dropping two of the four fields silently. 3. A configured Models entry containing *, ?, or [ (path.Match's own special characters -- auth.go's matchesGlob) is a pattern, not a routable id: it now renders as a plain, non-copyable Badge (font-mono, secondary) instead of the copyable ModelChip, so a caller can no longer copy a glob pattern into a request's model field thinking it's a real id. Every Access label (Providers/Models/MCP servers/Agents) also gets a title attribute (one shared GLOB_HINT constant) making the glob-match semantics discoverable, since all four lists are glob-matched, not just Models. 4. groupSummary's providers/models/mcpServers/agents are now independent copies (cloneStringSlice), not aliases of the live group.* slices authorization reads on every request -- structurally prevents a future display-side sort/mutation from ever reordering or corrupting enforcement data. slices.Clone was tried first and reverted: it broke make yaegi-check (yaegi's stdlib symbol table does not resolve the generic slices package's type parameters under interpretation), so cloneStringSlice uses plain make/copy builtins instead -- verified yaegi-check passes with this version. 5. README.md's usage endpoint section: fixed the stale 'Overview and Usage views' reference (now 'Providers', matching the earlier tab rename) and added one line naming the new providers/models/ mcpServers/agents group-access fields, matching the section's existing terseness. 6. Renamed the 'unrestricted group carries empty/omitted lists' subtest to 'empty or omitted' -- the slash broke -run subtest path matching. 7. The Access block now uses the same dl/dt/dd markup as the sibling stat grid immediately below it, instead of a bespoke div/p structure -- one convention per panel.
This commit is contained in:
@@ -801,8 +801,10 @@ or in CI.
|
||||
`tokensOutPerMonth`, `costPerDayMicroUsd`, `costPerMonthMicroUsd` —
|
||||
alongside their configured limits, plus one extra `total` row: the
|
||||
synthetic all-traffic scope (see [Limits and accounting](
|
||||
#limits-and-accounting)), limits always `null`. The dashboard's Overview
|
||||
and Usage views poll both every 5 seconds while open.
|
||||
#limits-and-accounting)), limits always `null`. A group row also carries
|
||||
its configured `providers`/`models`/`mcpServers`/`agents` access lists,
|
||||
omitted when unrestricted. The dashboard's Providers and Usage views
|
||||
poll both every 5 seconds while open.
|
||||
- **`GET /admin/api/usage/history`** returns a bucketed series for one
|
||||
scope/metric/window — the data source for the dashboard's Charts view
|
||||
(per-user/per-group/total, stacked tokens-in/tokens-out, with a
|
||||
|
||||
+89
-5
@@ -496,14 +496,76 @@ func findUsageEntry(t *testing.T, entries []adminUsageEntryView, id string) admi
|
||||
return adminUsageEntryView{}
|
||||
}
|
||||
|
||||
// accessListKeys are adminUsageEntryView's four group-access JSON keys
|
||||
// (admin.go) — the set TestAdminUsage_GroupAccessLists checks both at the
|
||||
// typed-struct level and, since a nil Go slice and an absent JSON key
|
||||
// decode identically into a struct field either way, at the raw-JSON
|
||||
// level too (assertRawAccessList below): a struct-only assertion would
|
||||
// stay green even if the "omitempty" json tags were removed, since
|
||||
// encoding/json would then emit "null" for an unset field and
|
||||
// json.Unmarshal reads a JSON null back into a nil []string all the same.
|
||||
var accessListKeys = []string{"providers", "models", "mcpServers", "agents"}
|
||||
|
||||
// findRawUsageEntry finds row id's raw JSON object (one element of GET
|
||||
// /admin/api/usage's users/groups array, or the total object) among rows —
|
||||
// the raw-JSON counterpart to findUsageEntry, used where a typed-struct
|
||||
// decode cannot distinguish "key absent" from "key present but null" (see
|
||||
// accessListKeys' own doc comment).
|
||||
func findRawUsageEntry(t *testing.T, rows []map[string]json.RawMessage, id string) map[string]json.RawMessage {
|
||||
t.Helper()
|
||||
for _, row := range rows {
|
||||
var rowID string
|
||||
if err := json.Unmarshal(row["id"], &rowID); err != nil {
|
||||
t.Fatalf("decode id: %v", err)
|
||||
}
|
||||
if rowID == id {
|
||||
return row
|
||||
}
|
||||
}
|
||||
t.Fatalf("no raw usage entry for id %q", id)
|
||||
return nil
|
||||
}
|
||||
|
||||
// assertRawAccessList asserts one access-list key's raw JSON presence and
|
||||
// value on row: want == nil means the key must be ENTIRELY ABSENT from the
|
||||
// JSON object (not present-as-null, not present-as-"[]") — the omitempty
|
||||
// contract adminUsageEntryView's doc comment promises; a non-nil want means
|
||||
// the key must be present with exactly that value.
|
||||
func assertRawAccessList(t *testing.T, row map[string]json.RawMessage, key string, want []string) {
|
||||
t.Helper()
|
||||
raw, present := row[key]
|
||||
if want == nil {
|
||||
if present {
|
||||
t.Errorf("key %q = %s, want entirely absent (unrestricted/non-group row)", key, raw)
|
||||
}
|
||||
return
|
||||
}
|
||||
if !present {
|
||||
t.Errorf("key %q absent, want present with value %v", key, want)
|
||||
return
|
||||
}
|
||||
var got []string
|
||||
if err := json.Unmarshal(raw, &got); err != nil {
|
||||
t.Fatalf("decode %q: %v", key, err)
|
||||
}
|
||||
if !slices.Equal(got, want) {
|
||||
t.Errorf("key %q = %v, want %v", key, got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminUsage_GroupAccessLists proves GET /admin/api/usage's per-group
|
||||
// rows (group-access-display task) echo GroupConfig's Providers/Models/
|
||||
// MCPServers/Agents exactly as configured — no server-side expansion to the
|
||||
// full provider/model catalog — and that a group with none of those set
|
||||
// carries empty/omitted lists rather than a resolved "everything" set,
|
||||
// matching group.allowsX's own empty-means-all contract (auth.go). A user
|
||||
// row and the synthetic total row must never carry these fields at all —
|
||||
// they are group-only, same convention as GroupName's own user-only field.
|
||||
// carries the keys entirely absent (json:",omitempty") rather than present
|
||||
// with a resolved "everything" set, matching group.allowsX's own
|
||||
// empty-means-all contract (auth.go). A user row and the synthetic total
|
||||
// row must never carry these fields at all — they are group-only, same
|
||||
// convention as GroupName's own user-only field. Assertions run at both the
|
||||
// typed-struct level (adminUsageResponse) and the raw-JSON level
|
||||
// (assertRawAccessList) — the typed level alone cannot prove the field was
|
||||
// actually omitted from the wire, only that it decoded to nil, which a
|
||||
// present-but-null field would do identically.
|
||||
func TestAdminUsage_GroupAccessLists(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := CreateConfig()
|
||||
@@ -537,6 +599,15 @@ func TestAdminUsage_GroupAccessLists(t *testing.T) {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
|
||||
var raw struct {
|
||||
Total map[string]json.RawMessage `json:"total"`
|
||||
Users []map[string]json.RawMessage `json:"users"`
|
||||
Groups []map[string]json.RawMessage `json:"groups"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &raw); err != nil {
|
||||
t.Fatalf("decode raw: %v", err)
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
group string
|
||||
@@ -554,7 +625,7 @@ func TestAdminUsage_GroupAccessLists(t *testing.T) {
|
||||
wantAgents: []string{"agent1"},
|
||||
},
|
||||
{
|
||||
name: "unrestricted group carries empty/omitted lists",
|
||||
name: "unrestricted group carries empty or omitted lists",
|
||||
group: "open",
|
||||
},
|
||||
}
|
||||
@@ -573,6 +644,12 @@ func TestAdminUsage_GroupAccessLists(t *testing.T) {
|
||||
if !slices.Equal(entry.Agents, tc.wantAgents) {
|
||||
t.Errorf("agents = %v, want %v", entry.Agents, tc.wantAgents)
|
||||
}
|
||||
|
||||
rawEntry := findRawUsageEntry(t, raw.Groups, tc.group)
|
||||
assertRawAccessList(t, rawEntry, "providers", tc.wantProviders)
|
||||
assertRawAccessList(t, rawEntry, "models", tc.wantModels)
|
||||
assertRawAccessList(t, rawEntry, "mcpServers", tc.wantMCPServers)
|
||||
assertRawAccessList(t, rawEntry, "agents", tc.wantAgents)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -585,6 +662,13 @@ func TestAdminUsage_GroupAccessLists(t *testing.T) {
|
||||
t.Errorf("total entry must never carry group access lists, got providers=%v models=%v mcpServers=%v agents=%v",
|
||||
got.Total.Providers, got.Total.Models, got.Total.MCPServers, got.Total.Agents)
|
||||
}
|
||||
|
||||
rawAdmin1 := findRawUsageEntry(t, raw.Users, "admin1")
|
||||
rawTotal := raw.Total
|
||||
for _, key := range accessListKeys {
|
||||
assertRawAccessList(t, rawAdmin1, key, nil)
|
||||
assertRawAccessList(t, rawTotal, key, nil)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLimiterCurrentUsage_StoreDown drives limiter.currentUsage's
|
||||
|
||||
@@ -82,6 +82,24 @@ func (grp *group) allowsAgent(name string) bool {
|
||||
return matchesGlob(grp.agents, name)
|
||||
}
|
||||
|
||||
// cloneStringSlice returns an independent copy of s, preserving nil (a nil
|
||||
// s returns nil, never an empty non-nil slice — groupSummary's own
|
||||
// providers/models/mcpServers/agents fields rely on that nil-ness to stay
|
||||
// "omitempty" all the way to the JSON response, admin.go's
|
||||
// adminUsageEntryView). Plain builtins only (make/copy, no generics, no
|
||||
// non-stdlib package): the plugin runs interpreted under Yaegi (see
|
||||
// tools/yaegi-check), whose stdlib symbol table does not resolve the
|
||||
// generic "slices" package's type parameters — this is why a
|
||||
// slices.Clone call is not used here.
|
||||
func cloneStringSlice(s []string) []string {
|
||||
if s == nil {
|
||||
return nil
|
||||
}
|
||||
out := make([]string, len(s))
|
||||
copy(out, s)
|
||||
return out
|
||||
}
|
||||
|
||||
// matchesGlob reports whether candidate matches any shell-style pattern in
|
||||
// patterns (path.Match semantics). An empty pattern list matches anything.
|
||||
// A malformed pattern (path.ErrBadPattern) is treated as a non-match rather
|
||||
@@ -340,6 +358,13 @@ type userSummary struct {
|
||||
// provider/model/MCP server/agent is allowed (matchesGlob's own contract,
|
||||
// auth.go), never expanded to the full catalog here — the admin dashboard
|
||||
// echoes the configured glob list, not a resolved membership set.
|
||||
//
|
||||
// snapshot (below) clones all four slices rather than aliasing group's own
|
||||
// — group.providers/models/mcpServers/agents are the live authorization
|
||||
// data every allowsX call reads on the request path; groupSummary is a
|
||||
// read-only, dashboard-facing COPY, so a future caller that sorts or
|
||||
// otherwise mutates a groupSummary's list in place can never reorder or
|
||||
// corrupt the slice authorization itself still relies on.
|
||||
type groupSummary struct {
|
||||
limits *LimitsConfig
|
||||
name string
|
||||
@@ -371,10 +396,10 @@ func (a *authStore) snapshot() ([]userSummary, []groupSummary) {
|
||||
groups = append(groups, groupSummary{
|
||||
limits: grp.limits,
|
||||
name: name,
|
||||
providers: grp.providers,
|
||||
models: grp.models,
|
||||
mcpServers: grp.mcpServers,
|
||||
agents: grp.agents,
|
||||
providers: cloneStringSlice(grp.providers),
|
||||
models: cloneStringSlice(grp.models),
|
||||
mcpServers: cloneStringSlice(grp.mcpServers),
|
||||
agents: cloneStringSlice(grp.agents),
|
||||
memberCount: memberCounts[name],
|
||||
})
|
||||
}
|
||||
|
||||
@@ -227,6 +227,32 @@ const expandedGroupValues = computed<string[]>({
|
||||
watch(userQuery, (value) => {
|
||||
if (value.trim() === '') clearExpandOverrides(groupExpandState)
|
||||
})
|
||||
|
||||
// --- Access block (group-access-display task, review round) ---
|
||||
//
|
||||
// GLOB_HINT is the title attribute on every Access label (Providers,
|
||||
// Models, MCP servers, Agents): group.allowsX (auth.go) matches every one
|
||||
// of these four configured lists via matchesGlob/path.Match semantics, not
|
||||
// literal equality — a heading that just says "Models" leaves that
|
||||
// invisible. One constant, not four copies of the same string (vue.md:
|
||||
// "if you've written it twice, you owe an abstraction").
|
||||
const GLOB_HINT = 'Glob pattern — matches like a shell wildcard (*, ?, [...])'
|
||||
|
||||
/**
|
||||
* isGlobPattern reports whether one configured models entry is a glob
|
||||
* pattern rather than a literal model id — the same special characters
|
||||
* path.Match recognizes (auth.go's matchesGlob, which group.allowsModel
|
||||
* calls). Only Models needs this distinction rendered differently: a
|
||||
* literal id gets the copyable ModelChip treatment (it IS a routable id a
|
||||
* caller can paste into a request's `model` field); a pattern like
|
||||
* "gpt-4*" is not a valid id and must not invite copying it as one, so it
|
||||
* renders as a plain, non-copyable Badge instead (see the template below).
|
||||
* Providers/MCPServers/Agents render every entry as a plain Badge either
|
||||
* way, so they need no such split.
|
||||
*/
|
||||
function isGlobPattern(entry: string): boolean {
|
||||
return /[*?[]/.test(entry)
|
||||
}
|
||||
</script>
|
||||
|
||||
<template>
|
||||
@@ -288,35 +314,79 @@ watch(userQuery, (value) => {
|
||||
<AccordionContent>
|
||||
<!--
|
||||
Access block (group-access-display task): the group's
|
||||
configured provider/model access, display-only — not part
|
||||
of the user/group search filter above (row.original.providers/
|
||||
models are plain display data, never touched by
|
||||
groupMatches/userMatches in lib/usage-search.ts).
|
||||
configured provider/model/MCP-server/agent access,
|
||||
display-only — not part of the user/group search filter
|
||||
above (row.original.providers/models/mcpServers/agents are
|
||||
plain display data, never touched by groupMatches/
|
||||
userMatches in lib/usage-search.ts). Same dl/dt/dd
|
||||
convention as the stat grid immediately below — one
|
||||
convention per panel.
|
||||
-->
|
||||
<div class="mb-3 grid gap-3 sm:grid-cols-2">
|
||||
<dl class="mb-3 grid grid-cols-2 gap-x-6 gap-y-1.5 text-sm sm:grid-cols-4">
|
||||
<div>
|
||||
<p class="mb-1.5 text-xs font-medium text-muted-foreground">Providers</p>
|
||||
<div v-if="row.original.providers?.length" class="flex flex-wrap gap-1.5">
|
||||
<Badge
|
||||
v-for="providerName in row.original.providers"
|
||||
:key="providerName"
|
||||
as="span"
|
||||
variant="secondary"
|
||||
class="font-normal"
|
||||
>
|
||||
{{ providerName }}
|
||||
</Badge>
|
||||
</div>
|
||||
<p v-else class="text-sm text-muted-foreground">All providers</p>
|
||||
<dt class="text-xs text-muted-foreground" :title="GLOB_HINT">Providers</dt>
|
||||
<dd>
|
||||
<div v-if="row.original.providers?.length" class="flex flex-wrap gap-1.5">
|
||||
<Badge
|
||||
v-for="providerName in row.original.providers"
|
||||
:key="providerName"
|
||||
as="span"
|
||||
variant="secondary"
|
||||
class="font-mono font-normal"
|
||||
>
|
||||
{{ providerName }}
|
||||
</Badge>
|
||||
</div>
|
||||
<span v-else class="text-muted-foreground">All providers</span>
|
||||
</dd>
|
||||
</div>
|
||||
<div>
|
||||
<p class="mb-1.5 text-xs font-medium text-muted-foreground">Models</p>
|
||||
<div v-if="row.original.models?.length" class="flex flex-wrap gap-1.5">
|
||||
<ModelChip v-for="modelId in row.original.models" :key="modelId" :id="modelId" />
|
||||
</div>
|
||||
<p v-else class="text-sm text-muted-foreground">All models</p>
|
||||
<dt class="text-xs text-muted-foreground" :title="GLOB_HINT">Models</dt>
|
||||
<dd>
|
||||
<div v-if="row.original.models?.length" class="flex flex-wrap gap-1.5">
|
||||
<template v-for="modelId in row.original.models" :key="modelId">
|
||||
<ModelChip v-if="!isGlobPattern(modelId)" :id="modelId" />
|
||||
<Badge v-else as="span" variant="secondary" class="font-mono font-normal">{{ modelId }}</Badge>
|
||||
</template>
|
||||
</div>
|
||||
<span v-else class="text-muted-foreground">All models</span>
|
||||
</dd>
|
||||
</div>
|
||||
</div>
|
||||
<div>
|
||||
<dt class="text-xs text-muted-foreground" :title="GLOB_HINT">MCP servers</dt>
|
||||
<dd>
|
||||
<div v-if="row.original.mcpServers?.length" class="flex flex-wrap gap-1.5">
|
||||
<Badge
|
||||
v-for="serverName in row.original.mcpServers"
|
||||
:key="serverName"
|
||||
as="span"
|
||||
variant="secondary"
|
||||
class="font-mono font-normal"
|
||||
>
|
||||
{{ serverName }}
|
||||
</Badge>
|
||||
</div>
|
||||
<span v-else class="text-muted-foreground">All MCP servers</span>
|
||||
</dd>
|
||||
</div>
|
||||
<div>
|
||||
<dt class="text-xs text-muted-foreground" :title="GLOB_HINT">Agents</dt>
|
||||
<dd>
|
||||
<div v-if="row.original.agents?.length" class="flex flex-wrap gap-1.5">
|
||||
<Badge
|
||||
v-for="agentName in row.original.agents"
|
||||
:key="agentName"
|
||||
as="span"
|
||||
variant="secondary"
|
||||
class="font-mono font-normal"
|
||||
>
|
||||
{{ agentName }}
|
||||
</Badge>
|
||||
</div>
|
||||
<span v-else class="text-muted-foreground">All agents</span>
|
||||
</dd>
|
||||
</div>
|
||||
</dl>
|
||||
<dl class="mb-3 grid grid-cols-2 gap-x-6 gap-y-1.5 text-sm sm:grid-cols-4">
|
||||
<div>
|
||||
<dt class="text-xs text-muted-foreground">Limits</dt>
|
||||
|
||||
Reference in New Issue
Block a user