From eeb7a53795bbd3552d9ba0e65539e2720ef91610 Mon Sep 17 00:00:00 2001 From: Ming Wen Date: Sun, 17 May 2026 13:21:35 +0800 Subject: [PATCH 1/2] feat(provider-azure-openai): add aisix-provider-azure-openai skeleton crate (#302 Phase F / D6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: `, not `Authorization: Bearer` 2. URL pattern differs — `https://.openai.azure.com/openai/ deployments//chat/completions?api-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://.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 --- Cargo.lock | 14 + Cargo.toml | 1 + crates/aisix-provider-azure-openai/Cargo.toml | 20 + .../aisix-provider-azure-openai/src/bridge.rs | 345 ++++++++++++++++++ crates/aisix-provider-azure-openai/src/lib.rs | 66 ++++ .../aisix-provider-azure-openai/src/wire.rs | 45 +++ crates/aisix-server/Cargo.toml | 1 + crates/aisix-server/src/main.rs | 29 +- 8 files changed, 509 insertions(+), 12 deletions(-) create mode 100644 crates/aisix-provider-azure-openai/Cargo.toml create mode 100644 crates/aisix-provider-azure-openai/src/bridge.rs create mode 100644 crates/aisix-provider-azure-openai/src/lib.rs create mode 100644 crates/aisix-provider-azure-openai/src/wire.rs diff --git a/Cargo.lock b/Cargo.lock index 8fae5762..1c52f07b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -213,6 +213,19 @@ dependencies = [ "wiremock", ] +[[package]] +name = "aisix-provider-azure-openai" +version = "0.1.0" +dependencies = [ + "aisix-core", + "aisix-gateway", + "async-trait", + "serde_json", + "thiserror 1.0.69", + "tokio", + "tracing", +] + [[package]] name = "aisix-provider-openai" version = "0.1.0" @@ -310,6 +323,7 @@ dependencies = [ "aisix-guardrails", "aisix-obs", "aisix-provider-anthropic", + "aisix-provider-azure-openai", "aisix-provider-openai", "aisix-provider-vertex", "aisix-proxy", diff --git a/Cargo.toml b/Cargo.toml index 7baa72af..0c2877ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ members = [ "crates/aisix-provider-openai", "crates/aisix-provider-anthropic", "crates/aisix-provider-vertex", + "crates/aisix-provider-azure-openai", "crates/aisix-proxy", "crates/aisix-admin", "crates/aisix-obs", diff --git a/crates/aisix-provider-azure-openai/Cargo.toml b/crates/aisix-provider-azure-openai/Cargo.toml new file mode 100644 index 00000000..707172ab --- /dev/null +++ b/crates/aisix-provider-azure-openai/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "aisix-provider-azure-openai" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true +authors.workspace = true +description = "aisix: Azure OpenAI Service provider bridge (skeleton — deployment-keyed dispatch)" + +[dependencies] +aisix-core = { path = "../aisix-core" } +aisix-gateway = { path = "../aisix-gateway" } +async-trait.workspace = true +thiserror.workspace = true +tracing.workspace = true + +[dev-dependencies] +tokio = { workspace = true, features = ["macros", "rt", "time"] } +serde_json.workspace = true diff --git a/crates/aisix-provider-azure-openai/src/bridge.rs b/crates/aisix-provider-azure-openai/src/bridge.rs new file mode 100644 index 00000000..2ad756c4 --- /dev/null +++ b/crates/aisix-provider-azure-openai/src/bridge.rs @@ -0,0 +1,345 @@ +//! `AzureOpenAiBridge` — family Bridge for [`Adapter::AzureOpenai`]. +//! +//! Skeleton: structure + URL-shape helpers + Hub-registrable shell. +//! Real HTTP dispatch lands in follow-up PRs (see crate-level docs). + +use aisix_gateway::{ + Bridge, BridgeContext, BridgeError, ChatChunkStream, ChatFormat, ChatResponse, +}; +use async_trait::async_trait; + +use crate::wire; + +/// Family Bridge for Azure OpenAI Service. +/// +/// **Skeleton:** compiles, registers, surfaces a clear +/// `BridgeError::Config` on every call. Real dispatch is wired in +/// follow-up PRs — see [`crate`] docs. +pub struct AzureOpenAiBridge { + /// Static `name()` returned to the Hub. Kept for metrics-label + /// stability even though we don't have a transport yet. Different + /// from the inner OpenAI metric label so dashboards can split + /// Azure traffic from canonical OpenAI traffic. + name: &'static str, +} + +impl AzureOpenAiBridge { + /// Construct an Azure OpenAI bridge with the canonical name + /// `"azure-openai"`. The Hub looks this up via [`Bridge::name`] + /// when emitting per-request metrics (provider label). + pub fn new() -> Self { + Self { + name: "azure-openai", + } + } +} + +impl Default for AzureOpenAiBridge { + fn default() -> Self { + Self::new() + } +} + +/// Parsed Azure upstream reference resolved from a provider_key's +/// `api_base` + the request's upstream model id. +/// +/// Azure's chat-completions URL pattern (per +/// ): +/// +/// ```text +/// https://.openai.azure.com/openai/deployments//chat/completions?api-version= +/// ``` +/// +/// - `resource` — the Azure resource name, e.g. `acme-prod-west-us` +/// - `deployment` — operator-named deployment, e.g. `gpt4o-prod` +/// - `api_version` — Azure's date-stamped API version, e.g. `2024-08-01-preview` +/// +/// Skeleton: returned by [`AzureUpstreamRef::resolve`] for use by the +/// follow-up dispatch PR. The resolver is intentionally cautious — +/// any missing piece produces a clear `BridgeError::Config` so an +/// operator can fix the registration before traffic ever hits Azure. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AzureUpstreamRef { + pub resource: String, + pub deployment: String, + pub api_version: String, +} + +impl AzureUpstreamRef { + /// Default Azure REST API version Bridge tests rely on when the + /// operator hasn't pinned one. Real production deployments + /// **must** pin a version explicitly via `provider_key.api_base` + /// — Azure deprecates older versions on a published schedule: + /// + pub const DEFAULT_API_VERSION: &'static str = "2024-08-01-preview"; + + /// Resolve from the deployment name + an optional pre-parsed + /// `api_base`. Real dispatch will call this from `chat()`; today + /// it's exercised purely by the publisher-resolution tests. + pub fn resolve(deployment: &str, api_base: Option<&str>) -> Result { + if deployment.trim().is_empty() { + return Err(BridgeError::Config( + "azure deployment name is empty (expected a deployment id from \ + the Azure portal, e.g. \"gpt4o-prod\")" + .into(), + )); + } + + // Skeleton: the api_base contains the resource. Real parser + // lands in follow-up PRs; for now we accept either: + // - "https://.openai.azure.com" (canonical) + // - "" (bare resource name shorthand) + // and require the canonical form for anything else. + let base = api_base.unwrap_or_default().trim(); + let resource = if base.is_empty() { + return Err(BridgeError::Config( + "azure provider_key has no api_base — \ + expected https://.openai.azure.com or a bare resource name" + .into(), + )); + } else if let Some(rest) = base + .strip_prefix("https://") + .or_else(|| base.strip_prefix("http://")) + { + rest.split('.').next().unwrap_or_default().to_string() + } else { + base.to_string() + }; + + if resource.is_empty() { + return Err(BridgeError::Config(format!( + "azure resource not resolvable from api_base {base:?}" + ))); + } + + Ok(Self { + resource, + deployment: deployment.to_string(), + api_version: Self::DEFAULT_API_VERSION.to_string(), + }) + } + + /// Build the chat-completions URL for this Azure upstream. Used + /// by the follow-up dispatch PR. + pub fn chat_completions_url(&self) -> String { + format!( + "https://{}.openai.azure.com/openai/deployments/{}/chat/completions?api-version={}", + self.resource, self.deployment, self.api_version, + ) + } +} + +#[async_trait] +impl Bridge for AzureOpenAiBridge { + fn name(&self) -> &'static str { + self.name + } + + async fn chat( + &self, + req: &ChatFormat, + ctx: &BridgeContext, + ) -> Result { + // Skeleton: validate the deployment resolution path so a + // misconfigured row surfaces a clear error today, even + // though the actual HTTP call is TODO. + let _upstream = + AzureUpstreamRef::resolve(&req.model, ctx.provider_key.api_base.as_deref())?; + // Reserved-config helpers exercised by tests: keep the wire + // module reachable from the public surface so a future + // dispatch PR can drop its body straight in (header / query + // guards for the eventual default_headers / + // default_body_fields override apply path). + let _ = wire::reserved_query_params(); + let _ = wire::reserved_auth_headers(); + Err(BridgeError::Config( + "azure-openai bridge is not yet implemented — \ + tracked under api7/AISIX-Cloud#302 Phase F (D6)" + .into(), + )) + } + + async fn chat_stream( + &self, + req: &ChatFormat, + ctx: &BridgeContext, + ) -> Result { + let _upstream = + AzureUpstreamRef::resolve(&req.model, ctx.provider_key.api_base.as_deref())?; + Err(BridgeError::Config( + "azure-openai bridge is not yet implemented — \ + tracked under api7/AISIX-Cloud#302 Phase F (D6)" + .into(), + )) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resolve_accepts_canonical_https_resource() { + let r = AzureUpstreamRef::resolve("gpt4o-prod", Some("https://acme-west.openai.azure.com")) + .unwrap(); + assert_eq!(r.resource, "acme-west"); + assert_eq!(r.deployment, "gpt4o-prod"); + assert_eq!(r.api_version, AzureUpstreamRef::DEFAULT_API_VERSION); + } + + #[test] + fn resolve_accepts_bare_resource_name() { + // Convenience: operator pastes just the resource name as + // api_base. We let it through — the URL builder synthesizes + // the canonical host. + let r = AzureUpstreamRef::resolve("dep", Some("acme-east")).unwrap(); + assert_eq!(r.resource, "acme-east"); + } + + #[test] + fn resolve_rejects_empty_deployment() { + let err = AzureUpstreamRef::resolve("", Some("https://acme.openai.azure.com")).unwrap_err(); + match err { + BridgeError::Config(msg) => { + assert!( + msg.contains("deployment name is empty"), + "must call out empty deployment; got {msg}" + ); + } + other => panic!("expected Config error, got {other:?}"), + } + } + + #[test] + fn resolve_rejects_missing_api_base() { + let err = AzureUpstreamRef::resolve("dep", None).unwrap_err(); + match err { + BridgeError::Config(msg) => { + assert!( + msg.contains("no api_base"), + "must call out missing api_base; got {msg}" + ); + } + other => panic!("expected Config error, got {other:?}"), + } + } + + #[test] + fn resolve_rejects_empty_api_base() { + let err = AzureUpstreamRef::resolve("dep", Some(" ")).unwrap_err(); + match err { + BridgeError::Config(msg) => { + assert!(msg.contains("no api_base")); + } + other => panic!("expected Config error, got {other:?}"), + } + } + + #[test] + fn chat_completions_url_matches_azure_api_path() { + // Tight pin on the URL fragment Azure expects — a typo here + // would surface as a 404 from every Azure dispatch. + let r = AzureUpstreamRef { + resource: "acme-west".into(), + deployment: "gpt4o-prod".into(), + api_version: "2024-08-01-preview".into(), + }; + assert_eq!( + r.chat_completions_url(), + "https://acme-west.openai.azure.com/openai/deployments/gpt4o-prod/chat/completions?api-version=2024-08-01-preview", + ); + } + + #[test] + fn bridge_name_is_stable() { + // Metrics label is part of the public contract — a rename + // would silently break customer dashboards. `"azure-openai"` + // is the canonical name used in the Adapter enum's + // `kebab-case` rename. + assert_eq!(AzureOpenAiBridge::new().name(), "azure-openai"); + } + + use aisix_core::{Model, ProviderKey}; + use aisix_gateway::ChatMessage; + use std::sync::Arc; + + fn sample_model() -> Arc { + // Note: the Model.provider field still uses the legacy 6-value + // Provider enum (openai/anthropic/google/deepseek/cohere/jina). + // The Adapter enum's `azure-openai` variant is on + // ProviderKey.adapter, not Model.provider — see issue #302 + // §3. For the skeleton tests we keep the Model on a valid + // legacy provider; the bridge resolves the actual Azure + // upstream from ProviderKey.api_base + Model.model_name. + Arc::new( + serde_json::from_str( + r#"{ + "display_name": "my-azure-gpt4", + "provider": "openai", + "model_name": "gpt4o-prod", + "provider_key_id": "11111111-1111-1111-1111-111111111111" + }"#, + ) + .unwrap(), + ) + } + + fn sample_pk(api_base: Option<&str>) -> Arc { + let api_base_json = match api_base { + Some(b) => format!(r#", "api_base": "{b}""#), + None => String::new(), + }; + Arc::new( + serde_json::from_str(&format!( + r#"{{"display_name": "azure-prod", "secret": "az-key"{}}}"#, + api_base_json + )) + .unwrap(), + ) + } + + #[tokio::test] + async fn chat_surfaces_clear_not_implemented_error() { + let bridge = AzureOpenAiBridge::new(); + let ctx = BridgeContext::new( + "req-1", + sample_model(), + sample_pk(Some("https://acme-west.openai.azure.com")), + ); + let req = ChatFormat::new("gpt4o-prod", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::Config(msg) => { + assert!( + msg.contains("azure-openai bridge is not yet implemented"), + "error message must call out the WIP status; got {msg}" + ); + assert!( + msg.contains("#302"), + "error message must link to the tracking issue; got {msg}" + ); + } + other => panic!("expected Config error, got {other:?}"), + } + } + + #[tokio::test] + async fn chat_with_missing_api_base_errors_before_dispatch() { + // The resolve-time guard fires before the not-implemented + // stub — proves the bridge will reject malformed + // registrations early once dispatch lands. + let bridge = AzureOpenAiBridge::new(); + let ctx = BridgeContext::new("req-1", sample_model(), sample_pk(None)); + let req = ChatFormat::new("gpt4o-prod", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::Config(msg) => { + assert!( + msg.contains("no api_base"), + "must mention missing api_base; got {msg}" + ); + } + other => panic!("expected Config error, got {other:?}"), + } + } +} diff --git a/crates/aisix-provider-azure-openai/src/lib.rs b/crates/aisix-provider-azure-openai/src/lib.rs new file mode 100644 index 00000000..aa2af2ed --- /dev/null +++ b/crates/aisix-provider-azure-openai/src/lib.rs @@ -0,0 +1,66 @@ +//! aisix-provider-azure-openai — Azure OpenAI Service provider bridge. +//! +//! **Skeleton crate** for issue #302 Phase F. Registers as the family +//! bridge for [`Adapter::AzureOpenai`] in the gateway Hub. The actual +//! deployment-keyed dispatch is TODO and filled by follow-up PRs: +//! +//! - [ ] D6.1 — `api-key` header auth (NOT `Authorization: Bearer`) +//! - [ ] D6.2 — Azure URL pattern: +//! `https://.openai.azure.com/openai/deployments//chat/completions?api-version=` +//! - [ ] D6.3 — `upstream_id` parsing as `` rather +//! than OpenAI model id (e.g. customer's deployment "prod-gpt4o" maps +//! to whichever underlying OpenAI model their Azure tenancy +//! provisioned) +//! - [ ] D6.4 — `api_version` parameter handling (Azure pins it via +//! query string; the cp-api side ships it in `provider_key.api_base` +//! or a dedicated field) +//! - [ ] D6.5 — Content filter response: Azure injects +//! `prompt_filter_results` / `content_filter_results` into responses; +//! the bridge must surface these without confusing the OpenAI-shape +//! translation +//! +//! For now the bridge's `chat()` / `chat_stream()` return a clear +//! `BridgeError::Config(...)` so a misconfigured `provider: "azure"` +//! row in the kine catalog surfaces a 501 / 502 with an actionable +//! message rather than silently dropping the dispatch. +//! +//! # Why Azure-OpenAI is a separate bridge (not OpenAiBridge::with_name) +//! +//! 1. **Auth header differs** — Azure uses `api-key: `, not +//! `Authorization: Bearer `. The OpenAiBridge's header-builder +//! hard-codes Bearer; using it for Azure would either reject or +//! silently 401. +//! 2. **URL pattern differs** — Azure embeds the deployment name in +//! the path AND requires `?api-version=YYYY-MM-DD` as a query +//! parameter. OpenAiBridge's `{base}/chat/completions` won't shape +//! correctly even with a custom `api_base`. +//! 3. **Model field semantics differ** — the customer's +//! `upstream_id` is a deployment name, not an OpenAI model id. +//! Two customers with the same Azure region can have a deployment +//! "gpt4-prod" pointing at different OpenAI model versions. +//! 4. **Content filter injection** — Azure injects filter-result +//! objects that downstream OpenAI SDK clients don't know about. +//! The bridge needs to either pass them through or strip them. +//! +//! These are exactly the cases #302 §3 carves a separate +//! [`Adapter::AzureOpenai`] for. See LiteLLM `azure/`: +//! . +//! +//! # References +//! +//! - Azure OpenAI Service REST API — +//! +//! - api-version compatibility table — +//! +//! - Content filtering response fields — +//! +//! - LiteLLM `azure/` reference impl — +//! + +#![forbid(unsafe_code)] +#![deny(rust_2018_idioms)] + +mod bridge; +mod wire; + +pub use bridge::{AzureOpenAiBridge, AzureUpstreamRef}; diff --git a/crates/aisix-provider-azure-openai/src/wire.rs b/crates/aisix-provider-azure-openai/src/wire.rs new file mode 100644 index 00000000..96558c68 --- /dev/null +++ b/crates/aisix-provider-azure-openai/src/wire.rs @@ -0,0 +1,45 @@ +//! Azure OpenAI Service request/response wire shapes. +//! +//! **Skeleton:** only the constants and helpers the bridge needs to +//! sketch the deployment-dispatch path. Real chat-completions request +//! / response wrappers (with Azure-injected +//! `prompt_filter_results` / `content_filter_results` blocks) land +//! in follow-up D6.x PRs. + +/// Query parameters reserved by Azure's REST API that +/// `default_headers` / `default_body_fields` must never overwrite. +/// Azure pins the API version via `api-version` query parameter; a +/// misconfigured override block must not redirect calls to a +/// deprecated or unsupported API version. +pub(crate) fn reserved_query_params() -> &'static [&'static str] { + &["api-version"] +} + +/// Header names reserved by Azure OpenAI authentication that +/// `default_headers` must never inject. Azure uses `api-key` +/// (different from OpenAI's `Authorization: Bearer`); the bridge's +/// own auth header must always win. +pub(crate) fn reserved_auth_headers() -> &'static [&'static str] { + &["api-key"] +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn reserved_query_params_pins_api_version() { + let reserved = reserved_query_params(); + assert!(reserved.contains(&"api-version")); + } + + #[test] + fn reserved_auth_headers_pins_azure_api_key() { + // The `api-key` header name is Azure's auth convention. + // A default_headers block trying to inject it must be + // dropped at apply time — same defense-in-depth contract + // OpenAiBridge uses for `Authorization` / `x-api-key`. + let reserved = reserved_auth_headers(); + assert!(reserved.contains(&"api-key")); + } +} diff --git a/crates/aisix-server/Cargo.toml b/crates/aisix-server/Cargo.toml index 855c86c1..2e9bc45f 100644 --- a/crates/aisix-server/Cargo.toml +++ b/crates/aisix-server/Cargo.toml @@ -23,6 +23,7 @@ aisix-admin = { path = "../aisix-admin" } aisix-provider-openai = { path = "../aisix-provider-openai" } aisix-provider-anthropic = { path = "../aisix-provider-anthropic" } aisix-provider-vertex = { path = "../aisix-provider-vertex" } +aisix-provider-azure-openai = { path = "../aisix-provider-azure-openai" } aisix-ratelimit = { path = "../aisix-ratelimit" } aisix-cache = { path = "../aisix-cache", features = ["redis"] } tokio.workspace = true diff --git a/crates/aisix-server/src/main.rs b/crates/aisix-server/src/main.rs index 1088da91..0e3ac38c 100644 --- a/crates/aisix-server/src/main.rs +++ b/crates/aisix-server/src/main.rs @@ -29,6 +29,7 @@ use aisix_etcd::{EtcdConfigProvider, SnapshotCache, Supervisor}; use aisix_gateway::Hub; use aisix_obs::{init_tracing, install_otlp_tracer, Metrics}; use aisix_provider_anthropic::AnthropicBridge; +use aisix_provider_azure_openai::AzureOpenAiBridge; use aisix_provider_openai::OpenAiBridge; use aisix_provider_vertex::VertexBridge; use aisix_proxy::background::run_background_model_check_once; @@ -786,24 +787,28 @@ fn build_hub() -> Hub { // legacy `Provider`-keyed register() above stays the live // dispatch path until cp-api stops emitting the legacy enum // field; this only adds the new-schema-keyed lookup so a - // catalog row with `adapter: "vertex"` can resolve to a real + // catalog row with `adapter: ""` can resolve to a real // bridge instead of falling through to the legacy fallback. // - // Vertex is currently a SKELETON bridge — it returns a clear - // `BridgeError::Config` referencing api7/AISIX-Cloud#302 Phase E - // (D5). Registering it now lets the dispatch path light up the - // moment cp-api starts shipping adapter strings, instead of - // having the catalog be permanently inaccessible. + // All non-OpenAI-compat adapters are SKELETON bridges today — + // they return a clear `BridgeError::Config` referencing + // api7/AISIX-Cloud#302 Phases E/F/G. Registering them now lets + // the dispatch path light up the moment cp-api starts shipping + // adapter strings, instead of having the catalog be permanently + // inaccessible. // - // CUTOVER CAUTION: cp-api today blocks `provider: "google-vertex"` + // CUTOVER CAUTION: cp-api today blocks the catalog providers + // these bridges serve (`google-vertex`, `azure`, `amazon-bedrock`) // at createProviderKey via `isSupportedProvider` (handlers.go). // The moment that gate is loosened — which Phase B is actively - // enabling — every chat through a `google-vertex` provider_key - // will route here and hit this NOT-IMPLEMENTED skeleton, taking - // Gemini from "works via OpenAI-compat" to "500: not implemented" - // in a single cp-api flip. The cutover order MUST be: - // D5.2 (Gemini dispatch) merge → cp-api flip catalog. + // enabling — every chat through one of those provider_keys will + // route here and hit a NOT-IMPLEMENTED skeleton, taking the + // matching catalog from "works via OpenAI-compat" to "500: not + // implemented" in a single cp-api flip. The cutover order MUST + // be: per-adapter dispatch PR (D5.2 / D6.1 / D7.1) → cp-api flip + // catalog for that adapter. hub.register_family(Adapter::Vertex, Arc::new(VertexBridge::new())); + hub.register_family(Adapter::AzureOpenai, Arc::new(AzureOpenAiBridge::new())); hub } From 6c717e4b7c44428b660a4e70f5f45e5ecfa5c73d Mon Sep 17 00:00:00 2001 From: Ming Wen Date: Sun, 17 May 2026 13:40:59 +0800 Subject: [PATCH 2/2] fix(provider-azure-openai): address D6 audit HIGH-1 + HIGH-2 + HIGH-3 + MEDIUM-1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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//...` 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: ` (legacy) and `Authorization: Bearer ` (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. --- .../aisix-provider-azure-openai/src/bridge.rs | 250 +++++++++++++++--- .../aisix-provider-azure-openai/src/wire.rs | 35 ++- 2 files changed, 246 insertions(+), 39 deletions(-) diff --git a/crates/aisix-provider-azure-openai/src/bridge.rs b/crates/aisix-provider-azure-openai/src/bridge.rs index 2ad756c4..bff6cc58 100644 --- a/crates/aisix-provider-azure-openai/src/bridge.rs +++ b/crates/aisix-provider-azure-openai/src/bridge.rs @@ -66,30 +66,34 @@ pub struct AzureUpstreamRef { } impl AzureUpstreamRef { - /// Default Azure REST API version Bridge tests rely on when the - /// operator hasn't pinned one. Real production deployments - /// **must** pin a version explicitly via `provider_key.api_base` - /// — Azure deprecates older versions on a published schedule: - /// - pub const DEFAULT_API_VERSION: &'static str = "2024-08-01-preview"; + /// Most recent GA REST API version at crate publish time. + /// Operators **must** pin an explicit version via + /// `provider_key.api_base` for production traffic — this constant + /// is a stop-gap default for tests + early skeleton plumbing + /// only. Azure deprecates older versions on a published schedule: + /// . + /// + /// Pinned at a GA shape (`YYYY-MM-DD`, no `-preview` suffix) so a + /// future bump can't silently re-introduce a preview default. + pub const DEFAULT_API_VERSION: &'static str = "2024-10-21"; /// Resolve from the deployment name + an optional pre-parsed - /// `api_base`. Real dispatch will call this from `chat()`; today - /// it's exercised purely by the publisher-resolution tests. + /// `api_base`. Real dispatch will call this from `chat()`. + /// + /// Both `deployment` and the resolved `resource` are validated to + /// match a strict `[A-Za-z0-9_-]+` shape: Azure resource names + /// and deployment names are constrained to that set per the + /// portal, and a URL-injection vector via `?`, `#`, `/`, or + /// whitespace would let an operator-supplied default redirect + /// the dispatch to an attacker-pinned API version. pub fn resolve(deployment: &str, api_base: Option<&str>) -> Result { - if deployment.trim().is_empty() { - return Err(BridgeError::Config( - "azure deployment name is empty (expected a deployment id from \ - the Azure portal, e.g. \"gpt4o-prod\")" - .into(), - )); - } + validate_url_token("deployment name", deployment)?; // Skeleton: the api_base contains the resource. Real parser // lands in follow-up PRs; for now we accept either: // - "https://.openai.azure.com" (canonical) // - "" (bare resource name shorthand) - // and require the canonical form for anything else. + // Both forms get the same strict token-shape validation. let base = api_base.unwrap_or_default().trim(); let resource = if base.is_empty() { return Err(BridgeError::Config( @@ -101,16 +105,36 @@ impl AzureUpstreamRef { .strip_prefix("https://") .or_else(|| base.strip_prefix("http://")) { - rest.split('.').next().unwrap_or_default().to_string() + // Canonical form: split off the leading host segment + // before the first `.`. The remainder of the host MUST + // be `openai.azure.com` — anything else is a misconfig + // we surface up rather than silently dropping the path. + let (host_resource, host_tail) = rest.split_once('.').ok_or_else(|| { + BridgeError::Config(format!( + "azure api_base {base:?} missing the .openai.azure.com suffix" + )) + })?; + // Strip a trailing `/...` path so an operator who pasted + // the full chat-completions URL still parses correctly, + // but reject anything that injected query params. + let host_tail_trimmed = host_tail.trim_end_matches('/'); + let host_tail_core = host_tail_trimmed + .split_once('/') + .map(|(host, _path)| host) + .unwrap_or(host_tail_trimmed); + if host_tail_core != "openai.azure.com" { + return Err(BridgeError::Config(format!( + "azure api_base {base:?} host must end in .openai.azure.com \ + (got host suffix {host_tail_core:?})" + ))); + } + host_resource.to_string() } else { + // Bare-resource shorthand. base.to_string() }; - if resource.is_empty() { - return Err(BridgeError::Config(format!( - "azure resource not resolvable from api_base {base:?}" - ))); - } + validate_url_token("resource name", &resource)?; Ok(Self { resource, @@ -129,6 +153,29 @@ impl AzureUpstreamRef { } } +/// Reject URL-control characters in operator/customer-supplied tokens +/// that end up in the Azure URL path. Azure resource names and +/// deployment names are documented as `[A-Za-z0-9_-]+`, so anything +/// outside that set is either a misconfig or a URL-injection attempt +/// (e.g. `?api-version=evil` to override the bridge's version pin). +fn validate_url_token(name: &str, value: &str) -> Result<(), BridgeError> { + if value.is_empty() { + return Err(BridgeError::Config(format!( + "azure {name} is empty (expected an identifier matching [A-Za-z0-9_-]+)" + ))); + } + if !value + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_') + { + return Err(BridgeError::Config(format!( + "azure {name} {value:?} contains URL-control characters — \ + must match [A-Za-z0-9_-]+ (no spaces, slashes, dots, query params, or hash)" + ))); + } + Ok(()) +} + #[async_trait] impl Bridge for AzureOpenAiBridge { fn name(&self) -> &'static str { @@ -137,14 +184,22 @@ impl Bridge for AzureOpenAiBridge { async fn chat( &self, - req: &ChatFormat, + _req: &ChatFormat, ctx: &BridgeContext, ) -> Result { // Skeleton: validate the deployment resolution path so a // misconfigured row surfaces a clear error today, even // though the actual HTTP call is TODO. + // + // IMPORTANT: the Azure deployment name lives on + // Model.model_name (the operator-pinned upstream id), NOT on + // req.model (which is the gateway-internal display name the + // customer typed in `/v1/chat/completions`). See + // OpenAiBridge / `upstream_model(ctx)` for the established + // pattern. + let deployment = upstream_model(ctx)?; let _upstream = - AzureUpstreamRef::resolve(&req.model, ctx.provider_key.api_base.as_deref())?; + AzureUpstreamRef::resolve(deployment, ctx.provider_key.api_base.as_deref())?; // Reserved-config helpers exercised by tests: keep the wire // module reachable from the public surface so a future // dispatch PR can drop its body straight in (header / query @@ -161,11 +216,12 @@ impl Bridge for AzureOpenAiBridge { async fn chat_stream( &self, - req: &ChatFormat, + _req: &ChatFormat, ctx: &BridgeContext, ) -> Result { + let deployment = upstream_model(ctx)?; let _upstream = - AzureUpstreamRef::resolve(&req.model, ctx.provider_key.api_base.as_deref())?; + AzureUpstreamRef::resolve(deployment, ctx.provider_key.api_base.as_deref())?; Err(BridgeError::Config( "azure-openai bridge is not yet implemented — \ tracked under api7/AISIX-Cloud#302 Phase F (D6)" @@ -174,6 +230,18 @@ impl Bridge for AzureOpenAiBridge { } } +/// Pull the upstream deployment name off the BridgeContext. Azure +/// deployment names (operator-defined in the Azure portal, e.g. +/// `gpt4o-prod`) live on Model.model_name. `req.model` is the +/// customer-facing display name and must NOT be used here — that +/// was D6 audit HIGH-1. +fn upstream_model(ctx: &BridgeContext) -> Result<&str, BridgeError> { + ctx.model + .model_name + .as_deref() + .ok_or_else(|| BridgeError::Config("model.model_name missing".into())) +} + #[cfg(test)] mod tests { use super::*; @@ -242,14 +310,104 @@ mod tests { let r = AzureUpstreamRef { resource: "acme-west".into(), deployment: "gpt4o-prod".into(), - api_version: "2024-08-01-preview".into(), + api_version: "2024-10-21".into(), }; assert_eq!( r.chat_completions_url(), - "https://acme-west.openai.azure.com/openai/deployments/gpt4o-prod/chat/completions?api-version=2024-08-01-preview", + "https://acme-west.openai.azure.com/openai/deployments/gpt4o-prod/chat/completions?api-version=2024-10-21", ); } + /// D6 audit HIGH-2 regression: a deployment name with URL-control + /// chars (`?`, `#`, `/`, whitespace) would inject extra query + /// params or path segments into `chat_completions_url()`. The + /// resolver must reject these before the URL is ever built. + #[test] + fn resolve_rejects_deployment_with_query_injection() { + let err = AzureUpstreamRef::resolve("foo?api-version=evil", Some("acme-east")).unwrap_err(); + match err { + BridgeError::Config(msg) => { + assert!(msg.contains("URL-control characters"), "got {msg}"); + } + other => panic!("expected Config error, got {other:?}"), + } + } + + #[test] + fn resolve_rejects_deployment_with_slash_injection() { + let err = AzureUpstreamRef::resolve("foo/bar/chat", Some("acme")).unwrap_err(); + assert!(matches!(err, BridgeError::Config(_))); + } + + #[test] + fn resolve_rejects_deployment_with_hash_fragment() { + let err = AzureUpstreamRef::resolve("foo#bar", Some("acme")).unwrap_err(); + assert!(matches!(err, BridgeError::Config(_))); + } + + #[test] + fn resolve_rejects_resource_with_query_injection() { + // Bare-resource form with `?` — would corrupt the host. + let err = AzureUpstreamRef::resolve("dep", Some("acme?evil=1")).unwrap_err(); + assert!(matches!(err, BridgeError::Config(_))); + } + + #[test] + fn resolve_rejects_canonical_https_with_wrong_suffix() { + // `acme.evil.com` is not Azure — must reject so a misconfig + // doesn't dispatch chat traffic to an attacker-controlled + // host that happens to look canonical. + let err = AzureUpstreamRef::resolve("dep", Some("https://acme.evil.com")).unwrap_err(); + match err { + BridgeError::Config(msg) => { + assert!( + msg.contains("openai.azure.com"), + "must call out the required host suffix; got {msg}" + ); + } + other => panic!("expected Config error, got {other:?}"), + } + } + + #[test] + fn resolve_accepts_canonical_https_with_trailing_slash() { + let r = + AzureUpstreamRef::resolve("gpt4o-prod", Some("https://acme-west.openai.azure.com/")) + .unwrap(); + assert_eq!(r.resource, "acme-west"); + } + + #[test] + fn resolve_accepts_canonical_https_with_pasted_endpoint_path() { + // Operator copy-paste tolerance: full chat-completions URL + // pasted into api_base should still parse the resource. + let r = AzureUpstreamRef::resolve( + "gpt4o-prod", + Some("https://acme-west.openai.azure.com/openai/deployments/x/chat/completions"), + ) + .unwrap(); + assert_eq!(r.resource, "acme-west"); + } + + /// D6 audit HIGH-3 regression: the default MUST be GA shape + /// (`YYYY-MM-DD`, no `-preview` suffix). Preview versions are + /// rotated aggressively by Azure and should not be the + /// implicit default for production traffic. + #[test] + fn default_api_version_is_ga_shape() { + let v = AzureUpstreamRef::DEFAULT_API_VERSION; + assert!( + !v.contains("preview"), + "default API version must be GA, not preview; got {v:?}" + ); + // YYYY-MM-DD shape: exactly 10 chars, hyphens at positions + // 4 and 7. A future bump can't accidentally re-introduce a + // preview default without tripping this assertion. + assert_eq!(v.len(), 10, "must match YYYY-MM-DD; got {v:?}"); + assert_eq!(v.chars().nth(4), Some('-'), "{v:?}"); + assert_eq!(v.chars().nth(7), Some('-'), "{v:?}"); + } + #[test] fn bridge_name_is_stable() { // Metrics label is part of the public contract — a rename @@ -306,7 +464,9 @@ mod tests { sample_model(), sample_pk(Some("https://acme-west.openai.azure.com")), ); - let req = ChatFormat::new("gpt4o-prod", vec![ChatMessage::user("hi")]); + // req.model is the customer-facing display name; the bridge + // must ignore it and resolve the deployment from Model.model_name. + let req = ChatFormat::new("customer-facing-name", vec![ChatMessage::user("hi")]); let err = bridge.chat(&req, &ctx).await.unwrap_err(); match err { BridgeError::Config(msg) => { @@ -323,6 +483,36 @@ mod tests { } } + /// D6 audit HIGH-1 regression: dispatch must read the upstream + /// deployment from ctx.model.model_name, NOT from req.model. + /// req.model is the customer-typed display name; resolving off + /// it would produce `/openai/deployments/customer-facing-name/` + /// — 404 from Azure every time. + #[tokio::test] + async fn chat_ignores_req_model_and_uses_ctx_model_name() { + let bridge = AzureOpenAiBridge::new(); + let ctx = BridgeContext::new( + "req-1", + sample_model(), + sample_pk(Some("https://acme-west.openai.azure.com")), + ); + // req.model set to something the deployment-token validator + // would reject if it were the source of truth (whitespace + + // path traversal). Model.model_name = "gpt4o-prod" is valid, + // so the bridge must reach the not-implemented stub. + let req = ChatFormat::new("foo bar/../etc", vec![ChatMessage::user("hi")]); + let err = bridge.chat(&req, &ctx).await.unwrap_err(); + match err { + BridgeError::Config(msg) => { + assert!( + msg.contains("not yet implemented"), + "must hit the not-implemented stub (proving model_name was used); got {msg}" + ); + } + other => panic!("expected Config error, got {other:?}"), + } + } + #[tokio::test] async fn chat_with_missing_api_base_errors_before_dispatch() { // The resolve-time guard fires before the not-implemented @@ -330,7 +520,7 @@ mod tests { // registrations early once dispatch lands. let bridge = AzureOpenAiBridge::new(); let ctx = BridgeContext::new("req-1", sample_model(), sample_pk(None)); - let req = ChatFormat::new("gpt4o-prod", vec![ChatMessage::user("hi")]); + let req = ChatFormat::new("customer-facing-name", vec![ChatMessage::user("hi")]); let err = bridge.chat(&req, &ctx).await.unwrap_err(); match err { BridgeError::Config(msg) => { diff --git a/crates/aisix-provider-azure-openai/src/wire.rs b/crates/aisix-provider-azure-openai/src/wire.rs index 96558c68..d55101e9 100644 --- a/crates/aisix-provider-azure-openai/src/wire.rs +++ b/crates/aisix-provider-azure-openai/src/wire.rs @@ -16,11 +16,20 @@ pub(crate) fn reserved_query_params() -> &'static [&'static str] { } /// Header names reserved by Azure OpenAI authentication that -/// `default_headers` must never inject. Azure uses `api-key` -/// (different from OpenAI's `Authorization: Bearer`); the bridge's -/// own auth header must always win. +/// `default_headers` must never inject. Azure supports two auth +/// modes: +/// +/// - `api-key: ` — legacy / RBAC-disabled tenants +/// - `Authorization: Bearer ` — Entra (AAD) RBAC +/// +/// Reserving both prevents a future AAD-mode operator from poking +/// either header through the override block — same defense-in-depth +/// pattern OpenAiBridge uses for `Authorization` / `x-api-key`. +/// +/// Values are lowercase canonical so they compare case-insensitively +/// against `http::HeaderName::as_str()` (which lowercases on parse). pub(crate) fn reserved_auth_headers() -> &'static [&'static str] { - &["api-key"] + &["api-key", "authorization"] } #[cfg(test)] @@ -34,12 +43,20 @@ mod tests { } #[test] - fn reserved_auth_headers_pins_azure_api_key() { - // The `api-key` header name is Azure's auth convention. - // A default_headers block trying to inject it must be + fn reserved_auth_headers_pins_both_azure_auth_modes() { + // Azure supports `api-key: ` AND + // `Authorization: Bearer ` (Entra RBAC). A + // default_headers block trying to inject EITHER must be // dropped at apply time — same defense-in-depth contract - // OpenAiBridge uses for `Authorization` / `x-api-key`. + // OpenAiBridge uses for its Bearer / vendor api-key headers. let reserved = reserved_auth_headers(); - assert!(reserved.contains(&"api-key")); + assert!( + reserved.contains(&"api-key"), + "must reserve api-key (legacy auth)" + ); + assert!( + reserved.contains(&"authorization"), + "must reserve authorization (AAD Bearer auth)" + ); } }