Uh oh!
There was an error while loading. Please reload this page.
feat(proxy): entry-level URL rewriting via proxy.url_rewrites - #881
Merged
Conversation
The path names a registered mcp_servers entry; the endpoint presents that server alone: initialize reports its name, tools/list returns the upstream's original (un-namespaced) tool names, and tools/call accepts both the bare and the namespaced spelling. A name prefixed with a different registered server fails closed; an unregistered prefix stays a bare name. Unknown/disabled servers 404 after auth, with no fallback to the aggregate. Governance is the same pipeline as /mcp: per-tool ACL still evaluates the namespaced form (a grant means one thing on every endpoint, and the scoped surface cannot widen a key's grant), per-server rate limits and usage attribution key on the path's server rather than the tool-name prefix, and guardrails run both directions. /mcp itself is unchanged. Part of AISIX-Cloud#1219: gives per-server-URL clients a native scoped endpoint; the entry-level URL rewrite that maps legacy URL shapes onto it ships separately.
Independent review findings on the scoped endpoint:
- tools/list stripped the namespace prefix unconditionally, so an
upstream tool whose literal name starts with a registered server's
prefix was advertised under a spelling tools/call would re-strip
(mis-dispatch) or fail closed on. Strip only when the bare name
round-trips; colliding names stay namespaced — that spelling is the
callable one.
- The proxy's attribution peek parsed the name with its own logic,
which diverged from the gateway for server names ending in '_'.
Both sides now share one primitive (strip_server_prefix), which is
whole-string-prefix based, so such names namespace cleanly too.
- Pin the remaining review gaps in tests: the namespaced spelling and
the aggregated endpoint share the per-server rate-limit bucket, and
/mcp/{server} vs /mcp/ metric labels can't regress by arm reorder.An ordered list of {match, rewrite} regex rules applied to every
proxy-listener request before route matching (admin/metrics listeners
unaffected): the first matching rule rewrites the path once — no
cascading — and the request then flows through the normal endpoint
(auth, ACL, quota, metrics labelling) as if the client had sent the
rewritten path. Replacement substitutes the matched portion with
$1/${name} capture-group expansion; the query string is preserved; a
miss leaves the request untouched. Invalid regexes fail startup.
Because axum's Router::layer middleware runs after route matching, the
rewrite gets its pre-routing seat by wrapping the whole router as the
fallback of an outer router; the wrapper is only built when rules are
configured, so the default path pays nothing.
Lets operators map legacy URL shapes onto AISIX endpoints without
client changes — e.g. per-server MCP paths like /mcp-servers/{svc}/mcp
onto the /mcp/{server} endpoint, completing the migration scenario of
api7/AISIX-Cloud#1219 together with the scoped-endpoint PR.Review follow-up: the foreign set excluded disabled entries, so toggling another server's enabled flag changed which names a scope would serve bare vs fail closed on. Reserve every other registered name regardless of enabled state; the round-trip listing keeps the colliding literal names namespaced, so they stay callable. Also assert the ACL rejection wording in the scoped e2e denial case.
Independent review findings on the rewrite layer:
- proxy.url_rewrites is a struct list, which AISIX_* env vars — the only
config channel in chart-driven deployments — cannot express. The field
now also accepts one JSON array in a string, so
AISIX_PROXY__URL_REWRITES='[{...}]' works.
- Only the match regex was validated; the template could reference an
unknown capture group (expands to empty — every legacy request lands
on the wrong endpoint, silently), or carry '?'/'#'/whitespace (absorbs
the caller's query / truncates the path as a fragment). Config::validate
now rejects unknown group references, forbidden template characters,
and patterns that match the empty string (which would fire on every
request).
- Request ids now cover the rewrite layer: ensure_request_id moved
outside the wrapper, so fired/failed rewrite logs carry the request
span.
- Pin the remaining review gaps: warn-fallback serves the original path
(router-level), raw percent-encoded matching (no decode/normalize),
and first-occurrence replacement for unanchored patterns. Document the
raw-path semantics and the env JSON form.Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:2 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 ignored due to path filters (1)
📒 Files selected for processing (27)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #878, which GitHub auto-closed when its stacked base branch (
feat/mcp-scoped-endpoint, #875) was deleted on merge; same branch, now based onmain.What
Adds
proxy.url_rewrites: an ordered list of entry-level URL rewrite rules applied to every proxy-listener request before route matching (the admin and metrics listeners are unaffected).matchregex matches the request path rewrites it — once, no cascading — and the request then flows through the normal endpoint (auth, ACL, quota, metrics labelling) exactly as if the client had sent the rewritten path. A miss leaves the request untouched.matchruns against the raw, percent-encoded path (no decoding, no normalization — documented; unlike gateways that match a decoded$uri).rewritereplaces the matched portion;$1/${name}expand capture groups; the query string is preserved as sent.?/#/whitespace in the template (query absorption / fragment truncation), and patterns matching the empty string (would fire on every request). A template that still assembles an invalid path at runtime logs a warning naming the rule and serves the original path.AISIX_*vars, which cannot express a structured list) set the whole list as one JSON array:AISIX_PROXY__URL_REWRITES='[{"match":"...","rewrite":"..."}]'.Implementation note: axum's
Router::layermiddleware runs after route matching, so a URI rewritten there could never change which route matches. The rewrite therefore wraps the whole router as the fallback of an outer router, giving it a genuine pre-routing seat.Why
Lets operators map legacy URL shapes onto AISIX endpoints without client changes. The flagship scenario (api7/AISIX-Cloud#1219): clients migrating from gateways that expose one URL per MCP server (
/mcp-servers/{service}/{path}) keep their configured URLs and original tool names — one rule maps the URL onto the/mcp/{server}endpoint from #875, and the whole existing governance chain applies unchanged.Design comparison (per repo rule): mainstream gateways all ship a regex path-rewrite primitive with matched-portion replacement and capture-group templates (route-plugin, per-route rewrite, or middleware forms). Ours differs in placement only — a gateway-global ordered rule list instead of per-route config — because AISIX's routes are fixed built-in endpoints and the layer's purpose is mapping external URL space onto them; first-match-wins order replaces per-route attachment. LiteLLM offers no operator-configurable equivalent (its per-server MCP alias route is an internal fixed rewrite of the same shape), so route-rewrite plugins of general-purpose gateways are the reference baseline.
Rewriting cannot bypass governance: it only re-targets which proxy endpoint serves the request, and every endpoint enforces its own auth/ACL/quota after the rewrite; the admin surface lives on a separate listener the layer never touches.
Follow-up (tracked): the public DP Helm chart should surface
urlRewritesas first-class values at the next release sync, so chart users don't need the env JSON form — api7/api7-helm-chart#331.Tests
crates/aisix-proxy/src/rewrite.rs— unit + router-level: capture groups, matched-portion + first-occurrence semantics, named/braced references, query preservation, raw percent-encoded matching, warn-fallback serving the original path, first-rule-wins through the real router, no-rules passthrough.crates/aisix-core/src/config.rs— config load, env JSON-string form, and boot rejection of invalid regexes, unknown group references, forbidden template characters, and empty-matching patterns.tests/e2e/src/cases/url-rewrite-e2e.test.ts— real binary + etcd + real MCP upstream: the full migration scenario (legacy per-server URL + original tool name end to end), generic non-MCP mapping, routing with a query present, miss-passthrough (canonical paths intact, unmatched legacy tails 404).config.example.yaml/config.managed.yamldocument the block. Fixes api7/AISIX-Cloud#1219.