Uh oh!
There was an error while loading. Please reload this page.
feat(provider-azure-openai): add aisix-provider-azure-openai skeleton crate (D6, #302 §3 Phase F) - #313
Conversation
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds a new aisix-provider-azure-openai crate implementing a skeletal AzureOpenAiBridge with upstream parsing, URL construction, reserved wire helpers, tests, and registers the bridge in the server hub for adapter-based routing. The bridge currently validates configuration and returns "not implemented" errors. ChangesAzure OpenAI Provider Bridge
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
moonming
commented
May 17, 2026
Audit response — 3 HIGH + 2 MEDIUM + 2 LOWFixed in `bf05ae3`
Justified without code changes
Test results``` All HIGH/MEDIUM either fixed or justified per CLAUDE.md §8 merge gate. HIGH-1 also applies to D5 (#312) and D7 (#314) — D7 was written after this audit landed so it already has the fix; D5 backport coming as a follow-up commit on that PR. |
…el.model_name) D6 audit on PR #313 surfaced a wire-shape bug that applies equally to D5's skeleton: chat() and chat_stream() resolved the publisher from req.model (customer-typed display name) instead of from ctx.model.model_name (operator-pinned upstream id). Once dispatch lands in D5.2 (Gemini publisher), the URL builder would have produced `.../publishers/google/models/<customer-facing- name>:streamGenerateContent` and 404 on every Vertex request. The skeleton tests previously pinned the wrong contract (passed `req.model="gemini-1.5-pro"` so the matcher accidentally got the right input), so a future dispatch PR would have inherited the broken wire. Fix: - Add upstream_model(ctx) helper mirroring OpenAiBridge. - chat() / chat_stream() resolve from ctx.model.model_name. - req.model is now _req and explicitly ignored. - sample_model() renamed to sample_model_with(model_name) so tests can pin "display_name differs from model_name" by construction. - New regression test chat_ignores_req_model_and_uses_ctx_model_name sets req.model="gpt-4o" (would fail publisher resolution if it were the source of truth) and asserts the not-implemented stub fires — proving model_name was the actual input. - New defense test chat_with_missing_model_name_errors_before_dispatch ensures Option<String> = None on Model.model_name surfaces a clear error rather than panicking. D6 (#313) already fixed; D7 (#314) was authored after D6's audit landed so it already has the fix.
… crate (#302 Phase F / D6) Wave 5 D6 — scaffolds the Azure OpenAI Service family bridge so a `provider_key` row with `adapter: "azure-openai"` resolves to a real bridge instead of falling through to the legacy fallback. Actual HTTP dispatch + GCP-style auth lands in follow-up D6.x PRs. Why Azure-OpenAI is a separate bridge (not OpenAiBridge::with_name): 1. Auth header differs — `api-key: <key>`, not `Authorization: Bearer` 2. URL pattern differs — `https://<resource>.openai.azure.com/openai/ deployments/<deployment>/chat/completions?api-version=<version>` 3. Model field semantics — upstream_id is a deployment name (operator- defined), not an OpenAI model id 4. Content filter injection — Azure injects prompt_filter_results / content_filter_results that the OpenAI SDK doesn't expect Implementation: - AzureOpenAiBridge struct with name "azure-openai" - AzureUpstreamRef::resolve(deployment, api_base) parses + validates: canonical https://<resource>.openai.azure.com OR bare resource name - AzureUpstreamRef::chat_completions_url() builds the per-request URL - DEFAULT_API_VERSION constant pinned to current stable (with doc comment linking to Azure's deprecation schedule) - chat() / chat_stream() return BridgeError::Config referencing #302 - Hub::register_family(Adapter::AzureOpenai, ...) in build_hub() - wire::reserved_query_params (api-version) + reserved_auth_headers (api-key) — same defense-in-depth pattern as OpenAiBridge's RESERVED_DEFAULT_HEADERS, for the eventual override apply path Tests (11 passing): - resolve_accepts_canonical_https_resource / bare_resource_name - resolve_rejects_empty_deployment / missing_api_base / empty_api_base - chat_completions_url_matches_azure_api_path (URL fragment pinned — any typo in resource/deployment/api-version positioning would surface as a 404 from every Azure dispatch) - bridge_name_is_stable - chat_surfaces_clear_not_implemented_error - chat_with_missing_api_base_errors_before_dispatch (proves resolve- time guard fires before the not-implemented stub) - wire reserved_query_params / reserved_auth_headers coverage References (per CLAUDE.md §7): - Azure OpenAI REST API — https://learn.microsoft.com/en-us/azure/ai-services/openai/reference - api-version deprecation schedule — https://learn.microsoft.com/en-us/azure/ai-services/openai/api-version-deprecation - Content filter shape — https://learn.microsoft.com/en-us/azure/ai-services/openai/concepts/content-filter - LiteLLM azure/ reference — https://github.com/BerriAI/litellm/tree/main/litellm/llms/azure Out of scope, tracked under #302 Phase F follow-ups: - D6.1 api-key header auth - D6.2 Full Azure URL pattern dispatch - D6.3 upstream_id-as-deployment-name parsing - D6.4 api_version parameter handling - D6.5 Content filter response surfacing
… + MEDIUM-1 HIGH-1 — chat() resolved deployment from req.model (display name) instead of ctx.model.model_name (operator-pinned upstream id). Once dispatch lands the URL builder would produce `/openai/deployments/<customer-display-name>/...` and 404 on every request. Fix: introduce upstream_model(ctx) helper mirroring OpenAiBridge, resolve deployment from Model.model_name. New regression test chat_ignores_req_model_and_uses_ctx_model_name pins the contract with req.model="foo bar/../etc" (would be rejected by the URL-token validator if it were the source of truth) — the not-implemented stub fires instead, proving model_name was used. HIGH-2 — `chat_completions_url()` URL-injection via operator/ customer-controlled strings. Format! of unvalidated `resource` + `deployment` into the URL host + path lets: - `api_base = "acme?evil=1"` corrupt the host - `deployment = "foo?api-version=evil"` override the api-version - `api_base = "https://acme.evil.com"` redirect to attacker host Fix: validate_url_token() enforces [A-Za-z0-9_-]+ on both deployment and resource; canonical-https resolver now requires the host suffix to be exactly `openai.azure.com` (rejecting `acme.evil.com`). Tests cover query-injection in deployment, slash-injection, hash-fragment, query-injection in bare-resource, and the wrong-suffix host case. HIGH-3 — DEFAULT_API_VERSION was "2024-08-01-preview" (preview!). Azure rotates preview versions aggressively per the published deprecation schedule; shipping a preview as the implicit default means silent breakage on Azure's cadence. Fix: bumped to GA shape "2024-10-21". New test default_api_version_is_ga_shape asserts the constant matches `YYYY-MM-DD` exactly (no `-preview` suffix) so a future bump can't accidentally re-introduce a preview default. MEDIUM-1 — wire::reserved_auth_headers() only listed `api-key`. Azure supports both `api-key: <key>` (legacy) and `Authorization: Bearer <aad-token>` (Entra RBAC); a future AAD-mode operator would have silently been able to inject Authorization via default_headers. Fix: list now includes both. Test renamed + extended to assert both are present. Justifications (LOW + MEDIUM-2): - LOW-1 (BridgeError::Config semantically wrong for "not implemented"): kept as-is, same justification as D5's LOW-1 — a new BridgeError::NotImplemented variant ripples through every Bridge impl + proxy error mapping. Will revisit alongside D5/D7 if the variant becomes needed for other reasons. - LOW-2 (sample_model uses "provider": "openai" for an Azure bridge): doc comment notes this is by design; the legacy Provider enum doesn't have an Azure variant; Adapter::AzureOpenai routing happens off ProviderKey.adapter, not Model.provider. - MEDIUM-2 (no build_hub integration test): filed as a shared follow-up across D5/D6/D7 skeletons.
bf05ae3 to
6c717e4Compare
Summary
Phase F / D6 of api7/AISIX-Cloud#302 — scaffolds the Azure OpenAI Service family bridge so a `provider_key` row with `adapter: "azure-openai"` resolves to a real bridge in the Hub.
Skeleton PR: crate exists, compiles, registered in workspace + Hub. Real HTTP dispatch + auth lands in follow-up D6.1–D6.5.
Why a separate bridge (not OpenAiBridge::with_name)
OpenAiBridge's header builder and URL composition hard-code Bearer + simple path. Reusing it for Azure would either 401 or 404 every request.
What lands
Tests (11 passing)
References (per CLAUDE.md §7)
Out of scope — D6 follow-up tracker
Test plan
Related
Summary by CodeRabbit