Uh oh!
There was an error while loading. Please reload this page.
feat(provider-azure-openai): wire D6 chat + stream + api-key auth (#302 Phase F) - #319
Merged
Conversation
…rs + api-key auth (D6, #302 Phase F) Replaces the skeleton's `BridgeError::Config("not yet implemented")` stubs with real HTTP dispatch against Azure OpenAI Service. The wire shape is OpenAI chat-completions; Azure differs on three axes that this crate now handles end-to-end: 1. URL pattern — deployment-keyed: `https://<resource>.openai.azure.com/openai/deployments/<deployment>/chat/completions?api-version=<version>` built by AzureUpstreamRef::chat_completions_url() (kept as-is from the skeleton; pinned by chat_completions_url_matches_azure_api_path) 2. Auth header — `api-key: <secret>` (NOT `Authorization: Bearer`). Set by build_request_headers() with defense-in-depth via the reserved-headers list in aisix-provider-openai::overrides (covers `api-key`, `authorization`, `x-api-key` so an operator's `default_headers` override cannot exfil traffic by rewriting auth) 3. Response extension — Azure injects `prompt_filter_results` / `content_filter_results` blocks on responses. The reused `OpenAiResponse` / `OpenAiStreamChunk` parsers tolerate these transparently (no `deny_unknown_fields` on the parser types), pinned by chat_tolerates_content_filter_results_in_response Implementation strategy: promote the OpenAI wire types and conversion helpers (build_request, messages_from, response_into_chat_response, stream_chunk_into_chat_chunk, OpenAiResponse, OpenAiStreamChunk) from `pub(crate)` to `pub` and reuse them — they're JSON contract types, not OpenaiBridge-specific implementation. This keeps a single source of truth for the chat-completions wire format. The OpenaiBridge's own dispatch is unchanged. The override apply pipeline (param_renames / param_constraints / default_body_fields / default_headers / content_list_to_string / stream_done_marker / reasoning_field) is reused verbatim from aisix_provider_openai::overrides. SSE decoding mirrors OpenaiBridge's build_chunk_stream. Deferred to follow-ups (tracked in lib.rs Status section): - D6.4 — per-PK `api_version` override (currently DEFAULT_API_VERSION GA pin `2024-10-21`) - D6.6 — AAD Bearer auth as second auth scheme ## Test coverage 34 unit tests cover: - AzureUpstreamRef parsing (canonical https, bare resource shorthand, trailing slash tolerance, pasted-endpoint tolerance, URL-injection rejection across deployment/resource/query/slash/hash, host-suffix enforcement, missing api_base error message) - DEFAULT_API_VERSION shape (GA, YYYY-MM-DD, no `-preview`) - build_request_headers: api-key set (NOT Authorization), reserved- headers list blocks api-key/authorization overrides, SSE Accept, invalid api-key chars rejected, invalid request_id chars rejected - Bridge wire-shape dispatch against wiremock: - api-key header + deployment URL + api-version query reach upstream - JSON body's `model` field = deployment name - content_filter_results / prompt_filter_results tolerated - param_renames applied (max_tokens → max_completion_tokens) - 4xx maps to UpstreamStatus with body - 429 maps with Retry-After parsed - req.model ignored, ctx.model.model_name used for URL deployment - SSE streaming yields chunks until [DONE] - bridge.chat() end-to-end reaches network layer for canonical api_base
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 selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements a functional Azure OpenAI bridge by exposing shared OpenAI wire types as public API, adding HTTP dispatch logic with Azure-specific configuration validation and request handling, comprehensive test coverage via wiremock for both streaming and non-streaming operations, and updating documentation to reflect completion status. ChangesAzure OpenAI Bridge Implementation
🎯 4 (Complex) | ⏱️ ~45 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 |
…+ L1/L3/L5) PR #319 audit surfaced gaps the original PR's tests didn't catch because most "dispatch" tests bypassed `bridge.chat()` via a `run_dispatch_against_mock` helper that rebuilt the request from scratch. This commit closes those gaps: ## HIGH H1 — All dispatch tests now go through bridge.chat() / chat_stream() - Added a #[cfg(test)] `url_override` field + `with_url_override` constructor on AzureOpenAiBridge so wiremock can stand in for `<resource>.openai.azure.com`. resolve/validation/header/body paths still run normally against the canonical api_base. - Removed `run_dispatch_against_mock` helper (rebuilt request, bypassed bridge entry point). - Rewrote 7 dispatch tests to call bridge.chat() / chat_stream() directly. H2 — Streaming deadline now enforced per-chunk, not only on the initial POST. `build_chunk_stream` now takes (deadline, started) and wraps each `stream.next().await` in `timeout_at`. Pinned by new `chat_stream_enforces_per_chunk_deadline` test (200ms deadline, 2s mock body delay → BridgeError::Timeout). H3 — Stream test extended with inline `prompt_filter_results` (top- level Azure prelude chunk) + per-choice `content_filter_results`. Pins OpenAiStreamChunk tolerance for Azure's in-stream filter blocks, not just response-body filter blocks. ## MEDIUM M1 — Upstream error body no longer echoed verbatim. Azure error envelopes include the deployment name + resource hostname; piping them into customer-visible BridgeError::UpstreamStatus.message leaks operator-internal taxonomy. `map_http_error` now maps to canned status-keyed phrases ("upstream deployment or model not found", "upstream rate limited", etc.). The upstream body is drained and discarded; full body still reachable via tracing on the operator side via request_id correlation. - New test `chat_maps_upstream_400_to_canned_message_not_body_echo` asserts the body's deployment-name + resource leak into the error message is blocked. - New test `chat_maps_404_to_deployment_not_found_canned_message`. - Updated `chat_maps_429_with_retry_after_and_canned_message` to assert both the redacted message AND retry-after. M2 — param_renames test now asserts the renamed key carries the ORIGINAL VALUE (100), not just that the key swap happened. A buggy apply_param_renames that nukes the old key without inserting a value would have passed the old assertion. M3 — chat_dispatch_sends_api_key_header_and_deployment_url now asserts `Authorization` is absent at the wire (not just absent from the helper output) by extending CapturingResponder to also capture inbound headers. M4 — chat_body_full_shape_on_the_wire replaces the body_partial_json({"model": "gpt4o-prod"}) check with a full body shape inspection: `messages` array length + role + content, `stream: false` for non-streaming. body_partial_json no longer used; import removed. ## LOW L1 — Added doc comment to wire.rs's module-level header explaining why request/response/stream-chunk types are pub (sibling crate reuse, not a stability promise). L3 — chat_against_full_bridge_dispatch (renamed to chat_against_real_azure_reaches_network) marked `#[ignore]` — it called real Azure DNS, flaky on CI runners with corporate proxies. Run manually via `cargo test -- --ignored`. L5 — `truncate` removed (no longer used after M1's body-discard). L5's UTF-8 boundary-panic risk is moot now. ## Result cargo test -p aisix-provider-azure-openai → 35 passed, 1 ignored cargo clippy --workspace -- -D warnings → clean cargo fmt --check → clean
moonming
commented
May 17, 2026
MemberAuthor
Audit follow-up pushed (1e076da): Independent audit surfaced 3 HIGH + 4 MEDIUM + 5 LOW findings; all addressed: HIGH (all fixed)
MEDIUM (all fixed)
LOW (all fixed)
Verification |
Uh oh!
There was an error while loading. Please reload this page.
This was referenced May 17, 2026
Merged
Merged
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
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.
Summary
Replaces the skeleton's
BridgeError::Config(\"not yet implemented\")stubs inaisix-provider-azure-openaiwith real HTTP dispatch against Azure OpenAI Service. Closes the Phase F (D6) wire-implementation work on api7/AISIX-Cloud#302.The wire shape is OpenAI chat-completions; Azure differs on three axes that this PR handles end-to-end:
https://<resource>.openai.azure.com/openai/deployments/<deployment>/chat/completions?api-version=<version>(theAzureUpstreamRef::chat_completions_url()helper that landed in feat(provider-azure-openai): add aisix-provider-azure-openai skeleton crate (D6, #302 §3 Phase F) #313 is unchanged)api-key: <secret>(NOTAuthorization: Bearer), with defense-in-depth via the reserved-headers list inaisix-provider-openai::overrides(coversapi-key,authorization,x-api-key)prompt_filter_results/content_filter_resultsblocks; the reusedOpenAiResponse/OpenAiStreamChunkparsers tolerate these transparently (nodeny_unknown_fields)Implementation strategy
Promote the OpenAI wire types and conversion helpers from
pub(crate)topuband reuse them in the Azure crate — they're JSON contract types, notOpenaiBridge-specific implementation. This keeps a single source of truth for the chat-completions wire format.Items promoted to
pubinaisix-provider-openai::wire:OpenAiRequest/OpenAiMessage/OpenAiContent— request typesOpenAiResponse/OpenAiChoice/OpenAiResponseMessage/OpenAiUsage/OpenAiPromptDetails/OpenAiCompletionDetails— response typesOpenAiStreamChunk/OpenAiStreamChoice/OpenAiStreamDelta— stream typesbuild_request/messages_from/response_into_chat_response/stream_chunk_into_chat_chunk— convertersThe
OpenaiBridge's own dispatch is unchanged. Embedding wire types staypub(crate)(out of scope for D6).The override apply pipeline (param_renames / param_constraints / default_body_fields / default_headers / content_list_to_string / stream_done_marker / reasoning_field) is reused verbatim from
aisix_provider_openai::overrides. SSE decoding mirrorsOpenaiBridge::build_chunk_stream.Deferred (tracked in
lib.rsStatus section)api_versionoverride. Today the bridge pinsDEFAULT_API_VERSION = \"2024-10-21\"(GA). Follow-up will accept an explicit version fromprovider_key.api_basequery string or a dedicated PK field.auth_schemefield becoming routable.References
Test plan
34 unit tests, all passing. Coverage:
AzureUpstreamRefparsing (12 tests — unchanged from #313 skeleton)openai.azure.comonly)DEFAULT_API_VERSIONis GA shape (YYYY-MM-DD, no-preview)build_request_headers(6 tests — new)api-keyset,AuthorizationNOT setAccept: text/event-streamset only when streamingdefault_headers.api-keyoverride blocked by reserved-headers listdefault_headers.authorizationoverride blockedx-custom-trace)Wire-shape dispatch against wiremock (8 tests — new)
api-keyheader + deployment URL +api-versionquery reach upstreammodelfield = deployment name (Azure ignores it, but kept for log clarity)content_filter_results/prompt_filter_resultsin response toleratedparam_renamesapplied to outbound body (max_tokens→max_completion_tokens)UpstreamStatuswith body contentRetry-Afterparsedctx.model.model_nameused for URL deployment[DONE]Negative pre-dispatch (2 tests — new)
api_baseerrors before HTTPsecreterrors before HTTPCompile-only proof (1 test — new)
bridge.chat()reaches the network layer for a canonicalapi_base(not Config-errored)Test plan TODO
cargo test -p aisix-provider-azure-openaipasses locally (34/34)cargo clippy --workspace --all-targets -- -D warningscleancargo fmt --allcleancargo test --workspacecargo clippycargo fmt --checkSummary by CodeRabbit
New Features
Documentation