diff --git a/crates/aisix-proxy/src/models.rs b/crates/aisix-proxy/src/models.rs index d1868aad..934c59fe 100644 --- a/crates/aisix-proxy/src/models.rs +++ b/crates/aisix-proxy/src/models.rs @@ -5,6 +5,13 @@ //! matches the OpenAI `/v1/models` contract so any client that uses //! `client.models.list()` sees the models available to it. //! +//! The listing is defined as *the names a caller may put in a request's +//! `model` field*, so every dispatch shape qualifies: direct models and +//! the virtual aliases (routing / semantic / ensemble) alike. A Model +//! Group is the stable public entry point its operator intends callers +//! to use, and the same `allowed_models` ACL that authorizes the request +//! decides whether it appears here. +//! //! Each Model surfaces as: //! ```json //! { @@ -56,16 +63,14 @@ pub async fn list_models( let snapshot = state.snapshot.load(); - // Collect the names of all non-routing models (routing aliases are - // implementation detail, not something callers PUT into requests). - // Wildcard aliases (`provider/*`) are patterns, not concrete ids a caller - // can request by name, so they're excluded too. - // Then filter to what the authenticated key may access. + // Collect every model name a caller can request. Wildcard aliases + // (`provider/*`) are patterns rather than concrete ids, so they're the one + // exclusion. Then filter to what the authenticated key may access. let all_names: Vec = snapshot .models .entries() .into_iter() - .filter(|e| !e.value.is_routing() && !e.value.display_name.contains('*')) + .filter(|e| !e.value.display_name.contains('*')) .map(|e| e.value.display_name.clone()) .collect(); @@ -187,7 +192,7 @@ mod tests { } #[tokio::test] - async fn wildcard_key_sees_all_non_routing_models() { + async fn wildcard_key_sees_all_concrete_models() { let snap = new_snap(); snap.models.insert(model_entry("m1", "gpt4")); snap.models.insert(model_entry("m2", "claude")); @@ -259,22 +264,126 @@ mod tests { assert_eq!(data.len(), 0); } + /// Every virtual dispatch shape — routing, semantic, ensemble — is a name + /// a caller may request, so all three list alongside direct models. #[tokio::test] - async fn routing_models_are_excluded_from_list() { + async fn virtual_aliases_are_listed_alongside_direct_models() { let snap = new_snap(); snap.models.insert(model_entry("m1", "gpt4")); - // Insert a routing model. - let routing_cfg = serde_json::json!({ + snap.models.insert(model_entry("m2", "embed")); + + let routing: Model = serde_json::from_value(serde_json::json!({ "display_name": "smart-router", "routing": { "strategy": "failover", "targets": [{"model": "gpt4"}] } - }); - let routing: Model = serde_json::from_value(routing_cfg).unwrap(); + })) + .unwrap(); snap.models.insert(ResourceEntry::new("r1", routing, 1)); + + let semantic: Model = serde_json::from_value(serde_json::json!({ + "display_name": "topic-router", + "semantic": { + "embedding_model": "embed", + "routes": [{"name": "legal", "target": "gpt4", "examples": ["contract review"]}], + "default": "gpt4", + "match": {"threshold": 0.8} + } + })) + .unwrap(); + snap.models.insert(ResourceEntry::new("s1", semantic, 1)); + + let ensemble: Model = serde_json::from_value(serde_json::json!({ + "display_name": "panel", + "ensemble": { + "panel": [{"model": "gpt4"}], + "judge": {"model": "gpt4"} + } + })) + .unwrap(); + snap.models.insert(ResourceEntry::new("e1", ensemble, 1)); + snap.apikeys.insert(apikey_entry("sk-caller", &["*"])); + let app = build_app(snap); + let req = Request::builder() + .method("GET") + .uri("/v1/models") + .header("authorization", "Bearer sk-caller") + .body(axum::body::Body::empty()) + .unwrap(); + + let resp = tower::ServiceExt::oneshot(app, req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let bytes = to_bytes(resp.into_body(), 65536).await.unwrap(); + let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + let ids: Vec<&str> = v["data"] + .as_array() + .unwrap() + .iter() + .map(|m| m["id"].as_str().unwrap()) + .collect(); + assert_eq!( + ids, + ["embed", "gpt4", "panel", "smart-router", "topic-router"] + ); + } + + /// A Model Group carries no `provider` of its own, so it falls back to the + /// gateway as owner rather than leaking a target's provider. + #[tokio::test] + async fn routing_model_is_owned_by_the_gateway() { + let snap = new_snap(); + snap.models.insert(model_entry("m1", "gpt4")); + let routing: Model = serde_json::from_value(serde_json::json!({ + "display_name": "smart-router", + "routing": { + "strategy": "failover", + "targets": [{"model": "gpt4"}] + } + })) + .unwrap(); + snap.models.insert(ResourceEntry::new("r1", routing, 1)); + snap.apikeys + .insert(apikey_entry("sk-caller", &["smart-router"])); + + let app = build_app(snap); + let req = Request::builder() + .method("GET") + .uri("/v1/models") + .header("authorization", "Bearer sk-caller") + .body(axum::body::Body::empty()) + .unwrap(); + + let resp = tower::ServiceExt::oneshot(app, req).await.unwrap(); + let bytes = to_bytes(resp.into_body(), 65536).await.unwrap(); + let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + let item = &v["data"][0]; + assert_eq!(item["id"], "smart-router"); + assert_eq!(item["object"], "model"); + assert_eq!(item["owned_by"], "aisix"); + } + + /// The Model-Group-only deployment: the key is authorized for the group + /// alone, so the group is the whole listing and its targets stay hidden. + #[tokio::test] + async fn key_scoped_to_a_group_sees_the_group_and_not_its_targets() { + let snap = new_snap(); + snap.models.insert(model_entry("m1", "gpt4")); + snap.models.insert(model_entry("m2", "claude")); + let routing: Model = serde_json::from_value(serde_json::json!({ + "display_name": "my-gpt-group", + "routing": { + "strategy": "failover", + "targets": [{"model": "gpt4"}, {"model": "claude"}] + } + })) + .unwrap(); + snap.models.insert(ResourceEntry::new("r1", routing, 1)); + snap.apikeys + .insert(apikey_entry("sk-caller", &["my-gpt-group"])); + let app = build_app(snap); let req = Request::builder() .method("GET") @@ -288,9 +397,8 @@ mod tests { let bytes = to_bytes(resp.into_body(), 65536).await.unwrap(); let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); let data = v["data"].as_array().unwrap(); - // Only gpt4, not smart-router. assert_eq!(data.len(), 1); - assert_eq!(data[0]["id"], "gpt4"); + assert_eq!(data[0]["id"], "my-gpt-group"); } #[tokio::test] diff --git a/tests/e2e/src/cases/canary-routing-e2e.test.ts b/tests/e2e/src/cases/canary-routing-e2e.test.ts index 99c86f60..ceca7384 100644 --- a/tests/e2e/src/cases/canary-routing-e2e.test.ts +++ b/tests/e2e/src/cases/canary-routing-e2e.test.ts @@ -101,8 +101,7 @@ describe("sticky (A/B / canary) weighted routing e2e", () => { const canary = await startOpenAiUpstream({ nonStreamBody: okBody("canary-served") }); upstreams.push(stable, canary); // Router BEFORE its targets: watch events apply in revision order, so - // once /v1/models lists both targets the router is in the snapshot too - // (virtual models don't appear in /v1/models themselves). + // once /v1/models lists both targets the router is in the snapshot too. await seed.createModel({ display_name: "canary-router", routing: { diff --git a/tests/e2e/src/cases/cost-aware-routing-e2e.test.ts b/tests/e2e/src/cases/cost-aware-routing-e2e.test.ts index 796807c0..b7817c45 100644 --- a/tests/e2e/src/cases/cost-aware-routing-e2e.test.ts +++ b/tests/e2e/src/cases/cost-aware-routing-e2e.test.ts @@ -98,8 +98,7 @@ describe("cost-aware (least_cost) routing e2e", () => { // Declare the expensive target FIRST — least_cost must reorder by price, // not honor declaration order. The router document is written BEFORE its // targets: watch events apply in revision order, so once both targets are - // visible the router is in the snapshot too (virtual models don't appear - // in /v1/models themselves). + // visible the router is in the snapshot too. await seed.createModel({ display_name: "cost-virtual", routing: { diff --git a/tests/e2e/src/cases/model-group-member-ratelimit-e2e.test.ts b/tests/e2e/src/cases/model-group-member-ratelimit-e2e.test.ts index 77a20d12..cff91497 100644 --- a/tests/e2e/src/cases/model-group-member-ratelimit-e2e.test.ts +++ b/tests/e2e/src/cases/model-group-member-ratelimit-e2e.test.ts @@ -33,8 +33,8 @@ const CALLER_KEY_HASH = createHash("sha256") // returns 404 while a name is absent from the snapshot and 403 once it // propagated. The 403 fires at the ACL gate, before any rate-limit // reservation, so probing never consumes the member quotas under test -// (and routing models never appear in /v1/models, so listing can't be -// the probe). +// (and this key is allowed nothing, so its /v1/models listing is always +// empty and can't be the probe). const PROBE_PLAINTEXT = "sk-1087-probe"; const PROBE_KEY_HASH = createHash("sha256") .update(PROBE_PLAINTEXT) diff --git a/tests/e2e/src/cases/models-list-model-group-e2e.test.ts b/tests/e2e/src/cases/models-list-model-group-e2e.test.ts new file mode 100644 index 00000000..ccd7c76a --- /dev/null +++ b/tests/e2e/src/cases/models-list-model-group-e2e.test.ts @@ -0,0 +1,184 @@ +import { createHash } from "node:crypto"; +import OpenAI from "openai"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + EtcdClient, + ProxyClient, + SeedClient, + spawnApp, + startOpenAiUpstream, + waitConfigPropagation, + type OpenAiUpstream, + type SpawnedApp, +} from "../harness/index.js"; + +// E2E: `GET /v1/models` is the caller's model-discovery surface, so it must +// list every name the caller is allowed to send as `model` — a Model Group +// included. The scenario is the deployment where a group is the only public +// entry point: callers get the stable group name and its targets are an +// internal detail they are not authorized for. +// +// Two keys exercise the two halves of the contract: +// - group-only key: discovery returns exactly the group, and that name is +// callable. Nothing about the group's targets leaks into the listing. +// - unrestricted key: discovery returns the group *and* the direct models, +// because the key may send any of those names. +// +// Reference: OpenAI Models API spec +// (https://platform.openai.com/docs/api-reference/models/list) — `data[].id` +// is "the model identifier, which can be referenced in the API endpoints". +// +// Note the group name is asserted to be callable, not just present: a listing +// entry a client cannot actually use would be worse than omitting it. + +const GROUP_ONLY_PLAINTEXT = "sk-ml-e2e-group-only"; +const GROUP_ONLY_KEY_HASH = createHash("sha256") + .update(GROUP_ONLY_PLAINTEXT) + .digest("hex"); + +const ALL_MODELS_PLAINTEXT = "sk-ml-e2e-all-models"; +const ALL_MODELS_KEY_HASH = createHash("sha256") + .update(ALL_MODELS_PLAINTEXT) + .digest("hex"); + +interface ModelListBody { + object?: unknown; + data?: { id?: unknown; object?: unknown; created?: unknown; owned_by?: unknown }[]; +} + +function idsOf(body: unknown): string[] { + const data = (body as ModelListBody | null)?.data ?? []; + return data.map((m) => String(m.id)); +} + +describe("models listing e2e: a Model Group is a discoverable entry point", () => { + let app: SpawnedApp | undefined; + let upstream: OpenAiUpstream | undefined; + let seed: SeedClient | undefined; + let etcdReachable = false; + + beforeAll(async () => { + const etcd = new EtcdClient(); + etcdReachable = await etcd.ping(); + if (!etcdReachable) return; + + upstream = await startOpenAiUpstream(); + app = await spawnApp(); + seed = new SeedClient(etcd, app.etcdPrefix); + + const pk = await seed.createProviderKey({ + display_name: "ml-e2e-pk", + secret: "sk-mock", + api_base: `${upstream.baseUrl}/v1`, + }); + await seed.createModel({ + display_name: "ml-primary", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: pk.id, + }); + await seed.createModel({ + display_name: "ml-secondary", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: pk.id, + }); + // The public entry point. Its targets are the two direct models above. + await seed.createModel({ + display_name: "ml-group", + routing: { + strategy: "failover", + targets: [{ model: "ml-primary" }, { model: "ml-secondary" }], + }, + }); + + await seed.createApiKey({ + key_hash: GROUP_ONLY_KEY_HASH, + allowed_models: ["ml-group"], + }); + await seed.createApiKey({ + key_hash: ALL_MODELS_KEY_HASH, + allowed_models: ["*"], + }); + }); + + afterAll(async () => { + await app?.exit(); + await upstream?.close(); + }); + + test("group-only key discovers the group, can call it, and never sees its targets", async (ctx) => { + if (!etcdReachable || !app) { + ctx.skip(); + return; + } + + const probe = new ProxyClient(app.proxyUrl, GROUP_ONLY_PLAINTEXT); + const client = new OpenAI({ + apiKey: GROUP_ONLY_PLAINTEXT, + baseURL: `${app.proxyUrl}/v1`, + maxRetries: 0, + }); + + // Both caller keys are seeded after every model and watch events apply in + // revision order, so this key authenticating at all means the provider + // key, both targets and the group are already in the snapshot. Gating on + // the key rather than on a model name keeps the gate independent of what + // the assertions check, and keeps a regression an assertion diff rather + // than a propagation timeout. + await waitConfigPropagation(async () => { + const res = await probe.listModels(); + return res.status === 200; + }); + + const res = await probe.listModels(); + expect(res.status).toBe(200); + expect((res.body as ModelListBody).object).toBe("list"); + // Exactly the group: the targets exist in the snapshot but this key is + // not authorized for them, so discovery must not disclose them. + expect(idsOf(res.body)).toEqual(["ml-group"]); + + // OpenAI Models spec shape for the group entry. + const entry = (res.body as ModelListBody).data?.[0]; + expect(entry?.object).toBe("model"); + expect(typeof entry?.created).toBe("number"); + // A group has no provider of its own, so it is owned by the gateway. + // A target's provider surfacing here would be exactly the target + // detail this listing must not disclose. + expect(entry?.owned_by).toBe("aisix"); + + // The discovered name is usable as-is: a client that picked `ml-group` + // off the listing sends it straight back as `model`. + const completion = await client.chat.completions.create({ + model: idsOf(res.body)[0], + messages: [{ role: "user", content: "hello from the listing" }], + }); + expect(completion.choices[0]?.message.role).toBe("assistant"); + }); + + test("unrestricted key discovers the group alongside the direct models", async (ctx) => { + if (!etcdReachable || !app) { + ctx.skip(); + return; + } + + const probe = new ProxyClient(app.proxyUrl, ALL_MODELS_PLAINTEXT); + + // Same gate as above, and this key is seeded last of all: authenticating + // proves every model landed. Waiting on one target's name would not — + // the other target and the group are written after it. + await waitConfigPropagation(async () => { + const res = await probe.listModels(); + return res.status === 200; + }); + + const res = await probe.listModels(); + expect(res.status).toBe(200); + // Every name this key may send, group and targets alike. + expect(idsOf(res.body).sort()).toEqual([ + "ml-group", + "ml-primary", + "ml-secondary", + ]); + }); +}); diff --git a/tests/e2e/src/cases/multidim-ratelimit-policy-e2e.test.ts b/tests/e2e/src/cases/multidim-ratelimit-policy-e2e.test.ts index 85242f6f..c8028150 100644 --- a/tests/e2e/src/cases/multidim-ratelimit-policy-e2e.test.ts +++ b/tests/e2e/src/cases/multidim-ratelimit-policy-e2e.test.ts @@ -57,12 +57,12 @@ const KEY_MEMBER_A2 = "sk-892-m-a2"; const KEY_MEMBER_B = "sk-892-m-b"; const KEY_TPM = "sk-892-tpm"; const KEY_ROUTE = "sk-892-route"; -// Readiness probe for the routing group: routing models never appear in -// /v1/models, so group propagation is probed with a key allowed to -// access NOTHING — 404 while the group is absent from the snapshot, 403 -// once it propagated. The 403 fires at the ACL gate, before any -// rate-limit reservation, so probing never consumes the buckets under -// test. +// Readiness probe for the routing group: this key is allowed to access +// NOTHING, so its /v1/models listing is always empty and cannot gate on +// the group — probe with a chat call instead, 404 while the group is +// absent from the snapshot and 403 once it propagated. The 403 fires at +// the ACL gate, before any rate-limit reservation, so probing never +// consumes the buckets under test. const KEY_PROBE = "sk-892-probe"; function chatBody(content: string, totalTokens = 8) { @@ -253,9 +253,9 @@ describe("conditional rate limit policies e2e (AISIX-Cloud#892)", () => { }); } - // Routing groups are invisible to /v1/models — wait until a chat call - // with the no-access probe key flips from 404 (not propagated) to 403 - // (in snapshot, ACL-rejected before any reservation). + // The no-access probe key lists nothing, so wait until a chat call with + // it flips from 404 (not propagated) to 403 (in snapshot, ACL-rejected + // before any reservation). async function waitGroupPropagated(name: string): Promise { if (!app) throw new Error("app not initialized"); const probe = new ProxyClient(app.proxyUrl, KEY_PROBE); diff --git a/tests/e2e/src/cases/upstream-timeout-defaults-e2e.test.ts b/tests/e2e/src/cases/upstream-timeout-defaults-e2e.test.ts index dd6a00d5..521b1135 100644 --- a/tests/e2e/src/cases/upstream-timeout-defaults-e2e.test.ts +++ b/tests/e2e/src/cases/upstream-timeout-defaults-e2e.test.ts @@ -215,10 +215,10 @@ describe("deployment-wide upstream timeout default", () => { key_hash: CALLER_KEY_HASH, allowed_models: ["td-group"], }); - // Routing models are not listed on /v1/models — gate on a probe call - // instead (the pattern timeout-fallback-e2e uses). A 504 means the - // virtual model and its member are both loaded; before that the - // gateway answers 404. + // The group showing up on /v1/models would only prove the group itself + // propagated — gate on a probe call instead (the pattern + // timeout-fallback-e2e uses). A 504 means the virtual model and its + // member are both loaded; before that the gateway answers 404. await waitConfigPropagation(async () => { const res = await callChat(grouped!, "td-group"); if (res.status !== 504) {