mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-14 05:18:54 +00:00
fix(aliases): dedupe listing against emitted entries, not raw owners
listFor's alias dedupe keyed on the raw owners map, so a colliding discovered model whose provider is group-denied caused BOTH the real entry and the alias to be skipped - the id vanished from the listing entirely even though resolve still serves it via the alias. Track ids actually appended to out in a listed map instead, and skip the alias only when listed[alias] is true. Extends TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision with a provider-denied variant: group providers:["openai"], collision on the denied "acme" provider -> alias listed exactly once, owned_by openai.
This commit is contained in:
+14
-4
@@ -664,10 +664,19 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any {
|
|||||||
}
|
}
|
||||||
|
|
||||||
out := make([]map[string]any, 0, len(owners))
|
out := make([]map[string]any, 0, len(owners))
|
||||||
|
// listed tracks every id actually APPENDED to out below — not merely
|
||||||
|
// present in the raw, authorization-blind owners map — so the alias
|
||||||
|
// dedupe loop further down only skips an alias whose real-model entry
|
||||||
|
// this group can actually see (review fix, second pass): keying the
|
||||||
|
// dedupe on owners itself made a group-denied collision vanish the id
|
||||||
|
// from the listing entirely (neither the real entry nor the alias
|
||||||
|
// appeared), even though resolve still serves it through the alias.
|
||||||
|
listed := make(map[string]bool, len(owners))
|
||||||
for id, provs := range owners {
|
for id, provs := range owners {
|
||||||
winner := provs[0]
|
winner := provs[0]
|
||||||
if grp.allowsModel(id) && grp.allowsProvider(winner) {
|
if grp.allowsModel(id) && grp.allowsProvider(winner) {
|
||||||
out = append(out, modelObject(id, winner))
|
out = append(out, modelObject(id, winner))
|
||||||
|
listed[id] = true
|
||||||
}
|
}
|
||||||
if len(provs) < 2 {
|
if len(provs) < 2 {
|
||||||
continue
|
continue
|
||||||
@@ -680,6 +689,7 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any {
|
|||||||
// for the equivalent client request.
|
// for the equivalent client request.
|
||||||
if (grp.allowsModel(pid) || grp.allowsModel(id)) && grp.allowsProvider(p) {
|
if (grp.allowsModel(pid) || grp.allowsModel(id)) && grp.allowsProvider(p) {
|
||||||
out = append(out, modelObject(pid, p))
|
out = append(out, modelObject(pid, p))
|
||||||
|
listed[pid] = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -699,15 +709,15 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any {
|
|||||||
// An alias id that also happens to collide with a since-discovered
|
// An alias id that also happens to collide with a since-discovered
|
||||||
// model's own bare id (impossible for an EXPLICIT model —
|
// model's own bare id (impossible for an EXPLICIT model —
|
||||||
// validateModelAliases already rejects that at construction) is
|
// validateModelAliases already rejects that at construction) is
|
||||||
// SKIPPED here (review fix), not appended a second time: owners
|
// SKIPPED here (review fix) only when that real model's own entry was
|
||||||
// already holds a real-model entry for that id from the loop above,
|
// actually EMITTED above (listed[alias], not merely present in
|
||||||
// and adding another with the same id would list it twice —
|
// owners) — otherwise adding it a second time would list it twice,
|
||||||
// duplicate, order-nondeterministic-looking entries in an
|
// duplicate, order-nondeterministic-looking entries in an
|
||||||
// OpenAI-shaped model list. resolve's own precedence (its doc
|
// OpenAI-shaped model list. resolve's own precedence (its doc
|
||||||
// comment) still has the alias win at REQUEST time regardless of
|
// comment) still has the alias win at REQUEST time regardless of
|
||||||
// which entry the listing shows; only the listing itself is deduped.
|
// which entry the listing shows; only the listing itself is deduped.
|
||||||
for alias, target := range m.aliases {
|
for alias, target := range m.aliases {
|
||||||
if _, collides := owners[alias]; collides {
|
if listed[alias] {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
_, _, canonical, err := m.resolveAliasTarget(alias, target, grp)
|
_, _, canonical, err := m.resolveAliasTarget(alias, target, grp)
|
||||||
|
|||||||
@@ -1254,6 +1254,14 @@ func TestModelRegistry_ListFor_Alias_NotAuthorized_Omitted(t *testing.T) {
|
|||||||
// and a second, duplicate one from the alias loop. resolve's own
|
// and a second, duplicate one from the alias loop. resolve's own
|
||||||
// precedence (registry.go's resolve doc comment) must still have the
|
// precedence (registry.go's resolve doc comment) must still have the
|
||||||
// alias win at request time regardless of which entry the listing shows.
|
// alias win at request time regardless of which entry the listing shows.
|
||||||
|
//
|
||||||
|
// A second scenario (review fix, second pass) shares this fixture: when
|
||||||
|
// grp cannot reach the COLLIDING provider at all (provider-denied), the
|
||||||
|
// real model's own entry is never emitted in the first place — the
|
||||||
|
// dedupe must key off entries actually LISTED, not the raw,
|
||||||
|
// authorization-blind owners map, or the id would vanish from the
|
||||||
|
// listing entirely (neither the real entry nor the alias appears) even
|
||||||
|
// though resolve still serves it through the alias.
|
||||||
func TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision(t *testing.T) {
|
func TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
adapters := map[string]providerAdapter{
|
adapters := map[string]providerAdapter{
|
||||||
@@ -1299,4 +1307,27 @@ func TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision(t *testin
|
|||||||
if canonical != "openai/gpt-test" {
|
if canonical != "openai/gpt-test" {
|
||||||
t.Errorf("canonical = %q, want %q", canonical, "openai/gpt-test")
|
t.Errorf("canonical = %q, want %q", canonical, "openai/gpt-test")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Provider-denied variant: grp can reach only "openai" — "acme", the
|
||||||
|
// colliding discovered model's own provider, is denied, so its raw
|
||||||
|
// entry is never emitted at all. The alias must still be listed
|
||||||
|
// exactly once, owned by its own target's provider ("openai") — the
|
||||||
|
// earlier owners-keyed dedupe would have wrongly skipped it too here,
|
||||||
|
// vanishing "shared-id" from the listing entirely.
|
||||||
|
narrowGrp := &group{name: "openai-only", providers: []string{"openai"}}
|
||||||
|
gotNarrow := reg.listFor(narrowGrp)
|
||||||
|
var narrowCount int
|
||||||
|
var ownedBy string
|
||||||
|
for _, entry := range gotNarrow {
|
||||||
|
if entry["id"] == "shared-id" {
|
||||||
|
narrowCount++
|
||||||
|
ownedBy, _ = entry["owned_by"].(string)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if narrowCount != 1 {
|
||||||
|
t.Fatalf("listFor (provider-denied collision) has %d entries for id %q, want exactly 1 (the alias)", narrowCount, "shared-id")
|
||||||
|
}
|
||||||
|
if ownedBy != "openai" {
|
||||||
|
t.Errorf("owned_by = %q, want %q (the alias's own target provider)", ownedBy, "openai")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user