fix(proxy): list virtual model aliases on /v1/models - #897
Conversation
`GET /v1/models` filtered out models carrying a `routing` block, so a Model Group never appeared in the caller's discovery list even when the caller API key was authorized for it. Deployments that publish a group as the only public entry point — targets kept internal by scoping the key to the group name — showed callers an empty model list, and any OpenAI-compatible client that populates its model picker from `/v1/models` had nothing to select. The filter was also inconsistent with itself: it excluded only `routing`, so semantic routers and ensembles — equally multi-target virtual aliases — were listed all along. The listing is now defined as the set of names a caller may send as `model`, which is what an OpenAI client expects `data[].id` to be. Every dispatch shape qualifies; wildcard aliases stay excluded because `provider/*` is a pattern rather than a requestable name. The `allowed_models` ACL that already authorizes the request decides what appears, so a key scoped to a group returns the group and none of its targets — no new permission surface and no target disclosure. A group has no `provider` of its own, so it surfaces as `owned_by: "aisix"` rather than borrowing a target's provider. Behavior change: keys with a `*` model ACL now see group entries they did not see before. This is additive for clients; assertions pinning an exact model list will need updating.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesModel discovery behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant APIClient as API client
participant AisixProxy as aisix-proxy
participant ModelCollection as model collection
participant ModelGroup as Model Group
APIClient->>AisixProxy: GET /v1/models with API key
AisixProxy->>ModelCollection: collect concrete model names
ModelCollection->>ModelGroup: include authorized routing alias
ModelCollection-->>AisixProxy: sorted authorized models
AisixProxy-->>APIClient: OpenAI model list
APIClient->>AisixProxy: chat completion using group ID
AisixProxy->>ModelGroup: dispatch request
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Four more sibling specs still stated that routing models never appear on /v1/models, two of them as the stated justification for their readiness gate. Correct all four; the gates themselves stay as they are, since each remains right for a reason that survives the change. Pin `owned_by` to the gateway instead of "some non-empty string". The loose assertion could not catch the leak its own comment guards against: resolving a group's owner from its first target's provider still yields a non-empty string, so the e2e passed while a target's provider leaked into the listing.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/src/cases/models-list-model-group-e2e.test.ts`:
- Around line 127-137: Update the readiness callback passed to
waitConfigPropagation around the client.chat.completions.create call to return
false only for recognized transient configuration-propagation errors. Preserve
successful assistant-role validation, but rethrow authentication, upstream,
transport, invalid-response, and other unexpected errors so they are not hidden
as a timeout.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71441695-20f7-470a-95bb-5fe4007bf2f1
📒 Files selected for processing (7)
crates/aisix-proxy/src/models.rstests/e2e/src/cases/canary-routing-e2e.test.tstests/e2e/src/cases/cost-aware-routing-e2e.test.tstests/e2e/src/cases/model-group-member-ratelimit-e2e.test.tstests/e2e/src/cases/models-list-model-group-e2e.test.tstests/e2e/src/cases/multidim-ratelimit-policy-e2e.test.tstests/e2e/src/cases/upstream-timeout-defaults-e2e.test.ts
The unrestricted-key test waited for `ml-primary` and then asserted `ml-secondary` and `ml-group` too, but both are written after it — the gate could not imply they had landed and the test leaned on the previous one having already warmed the snapshot. Both caller keys are seeded after every model, so under revision-order apply a key authenticating at all proves the models are in the snapshot. Gate on that instead. It also drops the catch-all around the chat probe, which turned an authentication, transport or upstream failure into a generic 30s propagation timeout; the chat call is now a plain assertion that surfaces the real error.
1 participant
Fixes api7/AISIX-Cloud#1237
Problem
GET /v1/modelsdropped every model carrying aroutingblock, so a Model Group never appeared in the caller's discovery list — even for a caller API key explicitly authorized to use it.That breaks the deployment the feature exists for: publish a group as the stable public entry point and keep the direct targets internal by scoping the caller key to the group name. Such a key got back an empty model list, and any OpenAI-compatible client that populates its model picker from
/v1/modelshad nothing to offer.The filter was also inconsistent with itself. It keyed on the
routingblock alone, so semantic routers and ensembles — equally multi-target virtual aliases — were listed all along. With a*-scoped key over one direct model, one ensemble, one semantic router and one group, the listing was:So "routing constructs aren't discovery-list entries" was never the actual rule; excluding
routingalone was.Change
Define the listing as the set of names a caller may send as
model, which is what the OpenAI Models API saysdata[].idis — "the model identifier, which can be referenced in the API endpoints" (https://platform.openai.com/docs/api-reference/models/list). Every dispatch shape qualifies.To be precise about the scope of that phrase: a virtual alias resolves on the routes that support virtual dispatch, while
/v1/realtimeand the files/batches/fine-tuning surface reject one and require a direct model. That gap predates this change — ensembles and semantic routers were already listed — and matches the OpenAI contract, where/v1/modelsis a global listing rather than a per-route one. The docs PR states the limit where the listing is described. The one exclusion is wildcard aliases, sinceprovider/*is a pattern rather than a requestable name; mainstream OpenAI-compatible gateways likewise return the public entry-point alias and keep the deployments behind it internal, and expand wildcard routes only on an explicit opt-in.Authorization is untouched. The request path already gates on the requested name (
can_access(&req.model)) with no per-target re-check, so the group name is already the ACL unit — the listing now applies the same judgement. A key scoped to a group returns the group and none of its targets: no new permission surface, no target disclosure.A group carries no
providerof its own, so it surfaces asowned_by: "aisix"rather than borrowing a target's.Behavior change
Callers with a
*model ACL now see group entries they did not see before. Keys whoseallowed_modelsis a narrower glob gain the same visibility for aliases matching it — e.g.openai/*now also lists a group namedopenai/premium-pool. No authorization changes in either case: those names were already callable by those keys, they simply were not enumerated. This is additive for clients; only assertions pinning an exact model list need updating.No config surface is added — this is the filter on a read-only endpoint — so there is no paired control-plane work.
Tests
tests/e2e/src/cases/models-list-model-group-e2e.test.tsdrives the reported scenario against a real gateway + etcd + upstream: a group over two direct targets, one caller key scoped to the group and one unrestricted. Before the fix:The group-only case then calls the id it read off the listing, so an entry that is listed but not callable would fail the test.
Unit tests pin all four dispatch shapes appearing in one listing, the
owned_byfallback for a group, and the group-scoped key seeing only the group.Six sibling e2e comments stated the old contract in prose and are corrected. Their readiness gates stay as they are — each remains right for a reason that survives the change.
Docs: api7/docs#2036
Summary by CodeRabbit
New Features
/v1/modelsnow includes available routing, semantic, ensemble, and model-group aliases.Bug Fixes
Tests