diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f4503b52..85d7a34a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,6 +46,44 @@ still build and pass its structural checks implementation details. 4. Make sure the checks above pass locally before pushing. +### Changing resource models: unknown fields vs. new enum values + +The gateway reads its resources leniently from etcd and strictly on write +(issue #871). That split makes two kinds of schema change behave very +differently on a data plane that has not been upgraded yet, and each needs a +different discipline: + +- **Adding a field** is the safe, expected change. An older gateway loads the + document with the new field ignored and reports it as partially compatible + (`GET /status/config` `partially_compatible[]`, the heartbeat, and the + `aisix_config_partially_compatible_resources` metric). Never assume a new + field is enforced fleet-wide until every data plane runs a version that + knows it — this matters most for restriction-type fields (an old gateway + keeps allowing what the new field would forbid). Two kinds are exempt from + this tolerance: `guardrail` and `observability_exporter` documents flatten + their fields into closed tagged shapes, so ANY new field there still + whole-row rejects on older gateways — treat additions to those two like + enum values below. +- **Adding an enum value** (a routing strategy, an adapter, a guardrail + `kind`, …) is NOT forward compatible, by design: a value the gateway cannot + interpret has no old behavior to fall back to, so the whole document stays + rejected on older versions. Do not "fix" that by opening the enum. Every + new enum value needs an explicit rollout decision, made in the PR that adds + it: + 1. **Version-gate at the control plane** (preferred for values that change + serving behavior): the control plane only offers the value once the + environment's data planes are on a version that knows it, using the + version the heartbeat already reports. + 2. **Ship a degradable fallback** via `#[serde(other)]` — only when a + fallback is semantically safe (e.g. an advisory label where "unknown" + is a reasonable interpretation). Never for values that select serving + behavior: silently running a different routing strategy than configured + is worse than rejecting the document. + 3. **Accept the rejection** for values that are new capabilities: an old + gateway that cannot serve the capability rejecting the row loudly (the + rejection reaches `rejected[]` and the heartbeat) can be the correct + outcome — state which of the three you chose and why in the PR. + ### Commit and PR style Commit subjects follow Conventional Commits, matching the existing history: diff --git a/Cargo.lock b/Cargo.lock index 29a04142..b89e4d40 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -126,6 +126,7 @@ dependencies = [ "etcd-client", "futures", "serde", + "serde_ignored", "serde_json", "tempfile", "thiserror 1.0.69", @@ -4376,6 +4377,16 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_ignored" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "115dffd5f3853e06e746965a20dcbae6ee747ae30b543d91b0e089668bb07798" +dependencies = [ + "serde", + "serde_core", +] + [[package]] name = "serde_json" version = "1.0.149" diff --git a/Cargo.toml b/Cargo.toml index e2541c5e..4704ead9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,6 +81,10 @@ webpki-roots = "0.26" # Serialization / validation serde = { version = "1", features = ["derive"] } serde_json = "1" +# Collects the paths of fields serde ignored during deserialization — +# how the etcd loader reports unknown fields from a newer control plane +# as "partially compatible" instead of dropping them silently (#871). +serde_ignored = "0.1" jsonschema = "0.28" schemars = "0.8" diff --git a/crates/aisix-admin/src/lib.rs b/crates/aisix-admin/src/lib.rs index 4122e420..bb235f02 100644 --- a/crates/aisix-admin/src/lib.rs +++ b/crates/aisix-admin/src/lib.rs @@ -800,6 +800,8 @@ mod tests { resource_counts: Default::default(), }), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -836,6 +838,12 @@ mod tests { last_error: "schema validation failed at `/display_name`".into(), seen_at: chrono::Utc::now(), }], + partially_compatible: vec![aisix_core::config_status::PartialCompatResource { + resource_kind: "api_keys".into(), + field: "quota_profile".into(), + count: 2, + }], + partially_compatible_rows_by_kind: [("api_keys".to_string(), 2)].into_iter().collect(), is_reload: true, wholly_rejected: false, }); @@ -866,6 +874,12 @@ mod tests { assert_eq!(v["applied"]["resource_counts"]["models"], 1); assert_eq!(v["rejected"][0]["resource_kind"], "models"); assert_eq!(v["rejected"][0]["last_error_kind"], "schema_failed"); + // The partially-compatible companion list (#871) rides next to + // rejected[] so a matching config_hash can't hide that some + // served rows carry fields this DP does not enforce. + assert_eq!(v["partially_compatible"][0]["resource_kind"], "api_keys"); + assert_eq!(v["partially_compatible"][0]["field"], "quota_profile"); + assert_eq!(v["partially_compatible"][0]["count"], 2); } #[tokio::test] @@ -883,6 +897,8 @@ mod tests { resource_counts: [("models".to_string(), 2)].into_iter().collect(), }), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); diff --git a/crates/aisix-core/src/bin/dump-schema.rs b/crates/aisix-core/src/bin/dump-schema.rs index 9568e66c..a491c853 100644 --- a/crates/aisix-core/src/bin/dump-schema.rs +++ b/crates/aisix-core/src/bin/dump-schema.rs @@ -38,38 +38,35 @@ fn main() { fs::create_dir_all(&out_dir).expect("create schemas/resources dir"); // Every resource with a runtime validator goes through the SAME - // `*_root_schema()` producer the validator uses, so the published schema == - // the enforced schema by construction. `ensemble`/`rate_limit`/`routing` - // have no standalone validator (they are nested struct types) so they dump - // straight from the struct via `schema_for!`. - dump_value(&out_dir, "api_key", schema::apikey_root_schema()); - dump_value(&out_dir, "cache_policy", schema::cache_policy_root_schema()); - dump_value(&out_dir, "model", schema::model_root_schema()); - dump_value( - &out_dir, + // `resource_root_schema(name, strict: true)` producer the write-path + // validators compile, so the published schema == the enforced write + // contract by construction. The published files deliberately carry the + // STRICT shape: they document the Admin API write contract (unknown + // fields are a 400) and the etcd loader's lenient read tolerance is a + // runtime behavior, not a contract callers may write against. + // `ensemble`/`rate_limit`/`routing` have no standalone validator (they + // are nested struct types) so they dump straight from the struct via + // `schema_for!`, closed the same way. + for resource in [ + "api_key", + "cache_policy", + "model", "rate_limit_policy", - schema::rate_limit_policy_root_schema(), - ); - dump_value(&out_dir, "provider_key", schema::provider_key_root_schema()); - dump_value( - &out_dir, + "provider_key", "observability_exporter", - schema::observability_exporter_root_schema(), - ); - dump_value(&out_dir, "guardrail", schema::guardrail_root_schema()); - dump_value( - &out_dir, + "guardrail", "guardrail_attachment", - schema::guardrail_attachment_root_schema(), - ); - dump_value(&out_dir, "mcp_server", schema::mcp_server_root_schema()); - dump_value(&out_dir, "mcp_policy", schema::mcp_policy_root_schema()); - dump_value(&out_dir, "a2a_agent", schema::a2a_agent_root_schema()); - dump_value( - &out_dir, + "mcp_server", + "mcp_policy", + "a2a_agent", "oidc_provider", - schema::oidc_provider_root_schema(), - ); + ] { + dump_value( + &out_dir, + resource, + schema::resource_root_schema(resource, true), + ); + } dump::(&out_dir, "ensemble"); dump::(&out_dir, "rate_limit"); @@ -81,14 +78,48 @@ fn main() { fn dump(out_dir: &Path, name: &str) { // Serialize the `RootSchema` directly to preserve schemars' native key // ordering. (Routing through `serde_json::Value` would re-sort keys.) - let mut json = - serde_json::to_string_pretty(&schemars::schema_for!(T)).expect("serialize schema"); + // These nested types belong to closed resources, so re-close the root + // and every struct-shaped definition on the typed schema — the same + // strictness `schema::close_unknown_fields` applies to the resource + // documents, kept typed here so the key order stays schemars-native. + let mut root = schemars::schema_for!(T); + close_object_schema(&mut root.schema); + for def in root.definitions.values_mut() { + if let schemars::schema::Schema::Object(obj) = def { + close_object_schema(obj); + } + } + let mut json = serde_json::to_string_pretty(&root).expect("serialize schema"); json.push('\n'); let path = out_dir.join(format!("{name}.schema.json")); fs::write(&path, json).unwrap_or_else(|e| panic!("write {}: {e}", path.display())); println!("wrote {}", path.display()); } +/// Insert `additionalProperties: false` on a struct-shaped schema object +/// (one that lists `properties`), unless it already pins a value. Recurses +/// into `anyOf` branches so an untagged enum's object variant closes too — +/// serde silently swallows unknown fields inside untagged content, so the +/// schema closure is the only non-silent guard there (the resource +/// producers apply the same rule, e.g. `OnEmbeddingFailure` in `model`). +fn close_object_schema(schema: &mut schemars::schema::SchemaObject) { + if let Some(sub) = schema.subschemas.as_deref_mut() { + if let Some(any_of) = sub.any_of.as_mut() { + for branch in any_of.iter_mut() { + if let schemars::schema::Schema::Object(b) = branch { + close_object_schema(b); + } + } + } + } + let Some(object) = schema.object.as_deref_mut() else { + return; + }; + if !object.properties.is_empty() && object.additional_properties.is_none() { + object.additional_properties = Some(Box::new(schemars::schema::Schema::Bool(false))); + } +} + /// Write a pre-assembled schema `Value`. Used for resources whose canonical /// schema is built by a dedicated producer rather than a bare `schema_for!` /// (e.g. `model`, which injects the cross-field `oneOf`). diff --git a/crates/aisix-core/src/config_status.rs b/crates/aisix-core/src/config_status.rs index 13ffcb97..ebb5b4c7 100644 --- a/crates/aisix-core/src/config_status.rs +++ b/crates/aisix-core/src/config_status.rs @@ -157,6 +157,24 @@ pub struct IncomingRejection { pub seen_at: DateTime, } +/// One partially compatible observation, aggregated per (kind, field): +/// `count` resources of `resource_kind` are currently served with `field` +/// ignored because this gateway version does not know it — typically a +/// field added by a newer control plane (issue #871). Shown next to +/// `rejected[]` on `GET /status/config` so a matching `config_hash` can +/// never silently hide that enforcement differs from the stored +/// documents. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct PartialCompatResource { + /// Plural resource kind (`api_keys`, `provider_keys`, …). + pub resource_kind: String, + /// Dotted path of the ignored field inside the document, with array + /// indices normalized to `[]` (e.g. `routing.targets[].priority`). + pub field: String, + /// Number of served resources of this kind carrying this field. + pub count: usize, +} + /// The result of a snapshot the gateway actually applied (served). #[derive(Debug, Clone)] pub struct AppliedSnapshot { @@ -182,6 +200,13 @@ pub struct LoadObservation { pub applied: Option, /// Rejected entries observed in this snapshot. pub rejected: Vec, + /// Partially compatible observations for the served snapshot, + /// aggregated per (kind, field). Replaces the previous set wholesale. + pub partially_compatible: Vec, + /// Served resources per kind that carry at least one ignored field. + /// Row-based (a resource with two unknown fields counts once), for + /// the `aisix_config_partially_compatible_resources` gauge. + pub partially_compatible_rows_by_kind: BTreeMap, /// Whether this load counts as a config reload for /// `aisix_config_reloads_total` (full (re)syncs and file loads do; /// incremental etcd events do not). @@ -239,6 +264,11 @@ struct ConfigStatusInner { // Retained rejections, keyed by source identity to keep first_seen stable. rejected: BTreeMap, + // Partially compatible observations for the served snapshot (#871). + // Replaced wholesale on every load; the load paths own the retention. + partially_compatible: Vec, + partially_compatible_rows_by_kind: BTreeMap, + // Metric counters. reloads_total: u64, reload_failures: BTreeMap<&'static str, u64>, @@ -273,6 +303,8 @@ impl ConfigStatus { last_reload_success_at: None, last_failure: None, rejected: BTreeMap::new(), + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: BTreeMap::new(), reloads_total: 0, reload_failures: BTreeMap::new(), })), @@ -325,6 +357,8 @@ impl ConfigStatus { ); } inner.rejected = merged; + inner.partially_compatible = obs.partially_compatible; + inner.partially_compatible_rows_by_kind = obs.partially_compatible_rows_by_kind; let clean = inner.rejected.is_empty(); inner.last_reload_successful = clean; @@ -479,6 +513,9 @@ impl ConfigStatusInner { rejected.sort_by(|a, b| { (&a.resource_kind, &a.resource_id).cmp(&(&b.resource_kind, &b.resource_id)) }); + let mut partially_compatible = self.partially_compatible.clone(); + partially_compatible + .sort_by(|a, b| (&a.resource_kind, &a.field).cmp(&(&b.resource_kind, &b.field))); ConfigStatusView { state: self.derive_state(), source, @@ -486,6 +523,7 @@ impl ConfigStatusInner { last_reload, last_failure, rejected, + partially_compatible, } } @@ -502,6 +540,7 @@ impl ConfigStatusInner { reloads_total: self.reloads_total, reload_failures: self.reload_failures.iter().map(|(k, v)| (*k, *v)).collect(), rejected_by_kind, + partially_compatible_by_kind: self.partially_compatible_rows_by_kind.clone(), observed_revision: if etcd { self.observed_revision } else { None }, applied_revision: if etcd { self.applied_revision } else { None }, config_hash: self.config_hash.clone(), @@ -522,6 +561,13 @@ pub struct ConfigStatusView { /// `null` when no failure has occurred this boot. pub last_failure: Option, pub rejected: Vec, + /// Resources served with unknown fields ignored, aggregated per + /// (kind, field) and sorted. Empty when every served document matched + /// its schema exactly. The companion to `applied.config_hash`: the + /// hash covers these rows (they are served), so this list is what + /// distinguishes "fully synced" from "synced with fields this + /// gateway version does not enforce". + pub partially_compatible: Vec, } #[derive(Debug, Clone, Serialize)] @@ -570,6 +616,8 @@ pub struct ConfigMetricsView { pub reloads_total: u64, pub reload_failures: BTreeMap<&'static str, u64>, pub rejected_by_kind: BTreeMap, + /// Served resources per kind carrying at least one ignored field. + pub partially_compatible_by_kind: BTreeMap, pub observed_revision: Option, pub applied_revision: Option, pub config_hash: Option, @@ -706,6 +754,8 @@ mod tests { observed_revision: Some(7), applied: Some(applied("h", &[("models", 2)])), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -731,6 +781,8 @@ mod tests { observed_revision: Some(1), applied: Some(applied("applied1", &[("models", 1)])), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -742,6 +794,8 @@ mod tests { observed_revision: Some(2), applied: Some(applied("applied2", &[("models", 1)])), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -754,6 +808,8 @@ mod tests { observed_revision: Some(3), applied: None, rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: true, }); @@ -768,6 +824,8 @@ mod tests { observed_revision: Some(3), applied: Some(applied("h", &[])), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -789,6 +847,8 @@ mod tests { "schema_failed", "schema validation failed at `/display_name`", )], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -809,6 +869,8 @@ mod tests { observed_revision: None, applied: Some(applied("good", &[("models", 1)])), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -824,6 +886,8 @@ mod tests { "schema_failed", "boom", )], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: true, }); @@ -844,6 +908,8 @@ mod tests { "non_json", "not json", )], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -858,6 +924,8 @@ mod tests { observed_revision: Some(1), applied: Some(applied(hash, &[("models", 1)])), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: false, wholly_rejected: false, }; @@ -883,6 +951,8 @@ mod tests { "schema_failed", "boom", )], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -893,6 +963,8 @@ mod tests { observed_revision: Some(2), applied: Some(applied("a2", &[("models", 1)])), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -919,6 +991,8 @@ mod tests { observed_revision: Some(1), applied: Some(applied("a", &[])), rejected: vec![r], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -937,6 +1011,8 @@ mod tests { observed_revision: Some(2), applied: Some(applied("a", &[])), rejected: vec![r2], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -960,6 +1036,8 @@ mod tests { resource_counts: [("models".to_string(), 1)].into_iter().collect(), }), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -978,6 +1056,8 @@ mod tests { observed_revision: Some(11), applied: Some(applied("e", &[("models", 1)])), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); @@ -1005,6 +1085,8 @@ mod tests { "y", ), ], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload: true, wholly_rejected: false, }); diff --git a/crates/aisix-core/src/filesource/status.rs b/crates/aisix-core/src/filesource/status.rs index b69843f9..368e4452 100644 --- a/crates/aisix-core/src/filesource/status.rs +++ b/crates/aisix-core/src/filesource/status.rs @@ -64,6 +64,8 @@ pub fn load_resources_file_tracked( resource_counts: resource_counts(snapshot), }), rejected: vec![], + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload, wholly_rejected: false, }); @@ -81,6 +83,8 @@ pub fn load_resources_file_tracked( // No snapshot applied — the previous one keeps serving. applied: None, rejected, + partially_compatible: Vec::new(), + partially_compatible_rows_by_kind: Default::default(), is_reload, // The whole file was rejected; last-good retained. wholly_rejected: true, diff --git a/crates/aisix-core/src/models/a2a_agent.rs b/crates/aisix-core/src/models/a2a_agent.rs index f1b5113b..6841e147 100644 --- a/crates/aisix-core/src/models/a2a_agent.rs +++ b/crates/aisix-core/src/models/a2a_agent.rs @@ -20,7 +20,6 @@ use serde::{Deserialize, Serialize}; use crate::resource::Resource; #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq)] -#[serde(deny_unknown_fields)] pub struct A2aAgent { /// Operator-facing label, unique within the gateway. It is the path segment /// under which the agent is exposed to callers as `/a2a/`, so it must @@ -164,25 +163,26 @@ mod tests { } #[test] - fn rejects_oauth2_auth_type_and_oauth_fields() { + fn rejects_oauth2_auth_type_but_tolerates_removed_oauth_fields() { // `auth_type` accepts only `none` / `bearer` / `api_key` — the same // closed set as the control plane's a2a_agent resource. assert!(serde_json::from_str::( r#"{"display_name":"a","url":"https://x/a2a","auth_type":"oauth2","secret":"cs-1"}"#, ) .is_err()); - // The OAuth-specific fields were removed with the `oauth2` arm, so a - // document carrying one is rejected as an unknown field. + // The OAuth-specific fields were removed with the `oauth2` arm; serde + // now tolerates them as unknown fields for forward compatibility (the + // write path still rejects them via `validate_a2a_agent` in + // `models/schema.rs`). for field in [ r#""client_id":"cid""#, r#""token_url":"https://auth/x/token""#, r#""scopes":["read","write"]"#, ] { let doc = format!(r#"{{"display_name":"a","url":"https://x/a2a",{field}}}"#); - assert!( - serde_json::from_str::(&doc).is_err(), - "field must be rejected as unknown: {doc}" - ); + let a: A2aAgent = serde_json::from_str(&doc) + .unwrap_or_else(|e| panic!("field must be tolerated as unknown: {doc}: {e}")); + assert_eq!(a.name, "a"); } } @@ -213,10 +213,13 @@ mod tests { } #[test] - fn rejects_unknown_fields() { - let r: Result = - serde_json::from_str(r#"{"display_name":"x","url":"u","extra":1}"#); - assert!(r.is_err()); + fn tolerates_unknown_fields_for_forward_compat() { + // A newer control plane may ship fields ahead of this DP; serde must + // accept them (the write path still rejects them via + // `validate_a2a_agent` in `models/schema.rs`). + let a: A2aAgent = + serde_json::from_str(r#"{"display_name":"x","url":"u","extra":1}"#).unwrap(); + assert_eq!(a.name, "x"); } // ---- `display_name` → `name` rename ---- diff --git a/crates/aisix-core/src/models/apikey.rs b/crates/aisix-core/src/models/apikey.rs index 30d99f1d..9e8bd190 100644 --- a/crates/aisix-core/src/models/apikey.rs +++ b/crates/aisix-core/src/models/apikey.rs @@ -19,7 +19,6 @@ use super::rate_limit::{McpRateLimit, RateLimit}; use crate::resource::Resource; #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct ApiKey { /// SHA-256 hexadecimal hash of the plaintext bearer. The proxy hashes /// incoming bearer tokens before lookup. @@ -395,11 +394,18 @@ mod tests { } #[test] - fn mcp_access_rejects_unknown_inner_fields() { - let r: Result = serde_json::from_str( + fn mcp_access_tolerates_unknown_inner_fields_for_forward_compat() { + // cp-api may ship new `mcp_access` fields ahead of the DP rolling + // out; serde must accept them (the write path still rejects them + // via `validate_apikey` in models/schema.rs). + let k: ApiKey = serde_json::from_str( r#"{"key_hash":"h","allowed_models":[],"mcp_access":{"mode":"inherit","widen":["*"]}}"#, + ) + .unwrap(); + assert_eq!( + k.mcp_access.unwrap().mode, + crate::models::McpAccessMode::Inherit ); - assert!(r.is_err()); } #[test] @@ -490,10 +496,13 @@ mod tests { } #[test] - fn rejects_unknown_fields() { - let r: Result = - serde_json::from_str(r#"{"key_hash":"x","allowed_models":[],"extra":1}"#); - assert!(r.is_err()); + fn tolerates_unknown_fields_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde + // must accept them (the write path still rejects them via + // `validate_apikey` in models/schema.rs). + let k: ApiKey = + serde_json::from_str(r#"{"key_hash":"x","allowed_models":[],"extra":1}"#).unwrap(); + assert_eq!(k.key_hash, "x"); } #[test] diff --git a/crates/aisix-core/src/models/embedding.rs b/crates/aisix-core/src/models/embedding.rs index 62b31a16..169b4e13 100644 --- a/crates/aisix-core/src/models/embedding.rs +++ b/crates/aisix-core/src/models/embedding.rs @@ -20,7 +20,6 @@ fn default_normalize() -> bool { /// Embedding-modality metadata for a direct Model. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq)] -#[serde(deny_unknown_fields)] pub struct EmbeddingConfig { /// Output vector dimensionality. Used to validate vectors, key the /// example-vector cache, and (for endpoints that support it) request a @@ -54,9 +53,12 @@ mod tests { } #[test] - fn rejects_unknown_field() { - let r: Result = - serde_json::from_str(r#"{"dimensions": 1024, "bogus": true}"#); - assert!(r.is_err()); + fn tolerates_unknown_field_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde must + // accept them. The write path still rejects them via the strict + // schema validators (validate_model in models/schema.rs). + let e: EmbeddingConfig = + serde_json::from_str(r#"{"dimensions": 1024, "bogus": true}"#).unwrap(); + assert_eq!(e.dimensions, 1024); } } diff --git a/crates/aisix-core/src/models/ensemble.rs b/crates/aisix-core/src/models/ensemble.rs index 9bcc59aa..7ef4ce98 100644 --- a/crates/aisix-core/src/models/ensemble.rs +++ b/crates/aisix-core/src/models/ensemble.rs @@ -15,7 +15,6 @@ use serde::{Deserialize, Serialize}; /// One member of an ensemble panel. `model` references a direct model alias. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct PanelMember { /// Model alias for a direct model that receives one panel request. #[schemars(length(min = 1))] @@ -47,7 +46,6 @@ impl PanelMember { /// The judge model that synthesizes the panel responses into one answer. /// `model` references a direct model alias. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq)] -#[serde(deny_unknown_fields)] pub struct Judge { /// Model alias for the direct model that synthesizes panel responses. #[schemars(length(min = 1))] @@ -74,7 +72,6 @@ impl Judge { const DEFAULT_MIN_RESPONSES: usize = 2; #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct EnsembleConfig { /// Direct models called concurrently for each ensemble request. #[schemars(length(min = 1))] @@ -181,22 +178,26 @@ mod tests { } #[test] - fn rejects_unknown_ensemble_field() { - let r: Result = - serde_json::from_str(r#"{"panel":[{"model":"a"}],"judge":{"model":"j"},"foo":1}"#); - assert!(r.is_err()); + fn tolerates_unknown_ensemble_field_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde must + // accept them. The write path still rejects them via validate_model + // in models/schema.rs. + let e: EnsembleConfig = + serde_json::from_str(r#"{"panel":[{"model":"a"}],"judge":{"model":"j"},"foo":1}"#) + .unwrap(); + assert_eq!(e.judge.model, "j"); } #[test] - fn rejects_unknown_panel_member_field() { - let r: Result = serde_json::from_str(r#"{"model":"a","bogus":true}"#); - assert!(r.is_err()); + fn tolerates_unknown_panel_member_field_for_forward_compat() { + let m: PanelMember = serde_json::from_str(r#"{"model":"a","bogus":true}"#).unwrap(); + assert_eq!(m.model, "a"); } #[test] - fn rejects_unknown_judge_field() { - let r: Result = serde_json::from_str(r#"{"model":"j","bogus":true}"#); - assert!(r.is_err()); + fn tolerates_unknown_judge_field_for_forward_compat() { + let j: Judge = serde_json::from_str(r#"{"model":"j","bogus":true}"#).unwrap(); + assert_eq!(j.model, "j"); } #[test] diff --git a/crates/aisix-core/src/models/guardrail.rs b/crates/aisix-core/src/models/guardrail.rs index e619ed55..849bb53d 100644 --- a/crates/aisix-core/src/models/guardrail.rs +++ b/crates/aisix-core/src/models/guardrail.rs @@ -1000,6 +1000,17 @@ pub struct GuardrailAttachment { #[serde(default = "default_enabled")] pub enabled: bool, + /// Environment the attachment belongs to. Written by the managed + /// control plane for its own scoping; the gateway does not read it. + // + // Declared so the field counts as known instead of being reported + // as partially compatible on every managed deployment (the CP + // writes it unconditionally): every attachment in a gateway's + // snapshot already belongs to its environment, so there is nothing + // to consume. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub env_id: Option, + #[serde(skip)] pub(crate) runtime_id: String, } diff --git a/crates/aisix-core/src/models/mcp_policy.rs b/crates/aisix-core/src/models/mcp_policy.rs index b469d98f..f6399b9e 100644 --- a/crates/aisix-core/src/models/mcp_policy.rs +++ b/crates/aisix-core/src/models/mcp_policy.rs @@ -46,7 +46,6 @@ pub enum McpPolicyMode { } #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct McpPolicy { /// Which API keys the policy applies to: the whole environment or one /// team. @@ -114,7 +113,6 @@ pub enum McpAccessMode { /// key's effective grant is computed from the applicable MCP access policies /// and `mode`, and `allowed_tools` is not consulted. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct McpAccess { /// How the key combines with the applicable policies: `inherit`, /// `restrict`, or `deny`. @@ -192,10 +190,13 @@ mod tests { } #[test] - fn rejects_unknown_fields() { - let r: Result = - serde_json::from_str(r#"{"scope":"env","mode":"all","extra":1}"#); - assert!(r.is_err()); + fn tolerates_unknown_fields_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde must + // accept them. The write path still rejects them via the strict + // schema validators (validate_mcp_policy in models/schema.rs). + let p: McpPolicy = + serde_json::from_str(r#"{"scope":"env","mode":"all","extra":1}"#).unwrap(); + assert_eq!(p.mode, McpPolicyMode::All); } #[test] @@ -250,8 +251,13 @@ mod tests { } #[test] - fn mcp_access_rejects_unknown_fields_and_modes() { - assert!(serde_json::from_str::(r#"{"mode":"inherit","extra":1}"#).is_err()); + fn mcp_access_tolerates_unknown_fields_for_forward_compat_but_rejects_unknown_modes() { + // cp-api may ship new fields ahead of the DP rolling out; serde must + // accept them (the write path still rejects them via the strict + // schema validators in models/schema.rs). Unknown enum values stay + // hard errors. + let a: McpAccess = serde_json::from_str(r#"{"mode":"inherit","extra":1}"#).unwrap(); + assert_eq!(a.mode, McpAccessMode::Inherit); assert!(serde_json::from_str::(r#"{"mode":"legacy"}"#).is_err()); } diff --git a/crates/aisix-core/src/models/mcp_server.rs b/crates/aisix-core/src/models/mcp_server.rs index 423c9eb5..c81ad2df 100644 --- a/crates/aisix-core/src/models/mcp_server.rs +++ b/crates/aisix-core/src/models/mcp_server.rs @@ -17,7 +17,6 @@ use crate::resource::Resource; // `Eq` is deliberately absent: `spec` holds a `serde_json::Value`, which is // only `PartialEq` (JSON numbers are floats). #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct McpServer { /// Operator-facing label, unique within the gateway. It is used as the /// namespace prefix for this server's tools, which are exposed to clients as @@ -263,10 +262,13 @@ mod tests { } #[test] - fn rejects_unknown_fields() { - let r: Result = - serde_json::from_str(r#"{"display_name":"x","url":"u","extra":1}"#); - assert!(r.is_err()); + fn tolerates_unknown_fields_for_forward_compat() { + // A newer control plane may ship fields ahead of this DP; serde must + // accept them. The write path still rejects them via the strict + // `schema::validate_mcp_server`. + let s: McpServer = + serde_json::from_str(r#"{"display_name":"x","url":"u","extra":1}"#).unwrap(); + assert_eq!(s.name, "x"); } // ---- `display_name` → `name` rename ---- diff --git a/crates/aisix-core/src/models/mod.rs b/crates/aisix-core/src/models/mod.rs index 03a21d2b..c324ba5f 100644 --- a/crates/aisix-core/src/models/mod.rs +++ b/crates/aisix-core/src/models/mod.rs @@ -65,10 +65,15 @@ pub use rate_limit::{McpRateLimit, RateLimit}; pub use rate_limit_policy::{PolicyScope, PolicyWindow, RateLimitPolicy}; pub use routing::{Routing, RoutingStrategy, RoutingTarget, WhenAllUnavailablePolicy}; pub use schema::{ - validate_a2a_agent, validate_apikey, validate_cache_policy, validate_guardrail, - validate_guardrail_attachment, validate_mcp_policy, validate_mcp_server, validate_model, - validate_observability_exporter, validate_oidc_provider, validate_provider_key, - validate_rate_limit_policy, SchemaError, + validate_a2a_agent, validate_a2a_agent_lenient, validate_apikey, validate_apikey_lenient, + validate_cache_policy, validate_cache_policy_lenient, validate_guardrail, + validate_guardrail_attachment, validate_guardrail_attachment_lenient, + validate_guardrail_lenient, validate_mcp_policy, validate_mcp_policy_lenient, + validate_mcp_server, validate_mcp_server_lenient, validate_model, validate_model_lenient, + validate_observability_exporter, validate_observability_exporter_lenient, + validate_oidc_provider, validate_oidc_provider_lenient, validate_provider_key, + validate_provider_key_lenient, validate_rate_limit_policy, validate_rate_limit_policy_lenient, + SchemaError, }; pub use semantic::{ Aggregation, DistanceMetric, EmbeddingFailureMode, OnEmbeddingFailure, Semantic, SemanticMatch, diff --git a/crates/aisix-core/src/models/model.rs b/crates/aisix-core/src/models/model.rs index 7c9d13e4..55854393 100644 --- a/crates/aisix-core/src/models/model.rs +++ b/crates/aisix-core/src/models/model.rs @@ -41,7 +41,6 @@ pub enum Adapter { /// Per-token cost for budget tracking. Both values are in USD per 1,000 tokens. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct ModelCost { /// Prompt token cost in USD per 1,000 tokens. #[schemars(range(min = 0.0))] @@ -61,7 +60,6 @@ impl ModelCost { } #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq)] -#[serde(deny_unknown_fields)] pub struct BackgroundModelCheck { /// Whether background health checks are enabled for this model. pub enabled: bool, @@ -88,7 +86,6 @@ pub struct BackgroundModelCheck { /// Request-path cooldown settings for a direct model after retryable upstream failures. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq, Default)] -#[serde(deny_unknown_fields)] pub struct CooldownConfig { /// Whether cooldown is active for this model. Set to `false` to keep the model in rotation regardless of upstream failures. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -183,7 +180,6 @@ impl CacheTtl { /// prompt-cache discounts without changing their requests. Requests that /// already set their own cache-control markers are forwarded unchanged. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq)] -#[serde(deny_unknown_fields)] pub struct AutoPromptCaching { /// Whether automatic prompt-cache injection is active for this model. pub enabled: bool, @@ -200,7 +196,6 @@ impl AutoPromptCaching { } #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct Model { /// Operator-facing unique label. Surfaces on `/v1/models`, /// `req.model` on chat completions, `ApiKey.allowed_models`, and @@ -517,15 +512,19 @@ mod tests { } #[test] - fn rejects_unknown_top_level_fields() { - let r: Result = serde_json::from_str( + fn tolerates_unknown_top_level_fields_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde must + // accept them. The write path still rejects them via `validate_model` + // in models/schema.rs. + let m: Model = serde_json::from_str( r#"{ "display_name":"x","provider":"openai","model_name":"g", "provider_key_id":"pk-1", "foo": 1 }"#, - ); - assert!(r.is_err()); + ) + .unwrap(); + assert_eq!(m.display_name, "x"); } #[test] diff --git a/crates/aisix-core/src/models/oidc_provider.rs b/crates/aisix-core/src/models/oidc_provider.rs index 46442095..996250f5 100644 --- a/crates/aisix-core/src/models/oidc_provider.rs +++ b/crates/aisix-core/src/models/oidc_provider.rs @@ -42,7 +42,6 @@ impl BoundClaimExpect { } #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct OidcProvider { /// Human-readable provider name, unique within the environment /// (e.g. `"corp-keycloak"`). @@ -202,11 +201,15 @@ mod tests { } #[test] - fn rejects_unknown_fields() { - let r: Result = serde_json::from_str( + fn tolerates_unknown_fields_for_forward_compat() { + // A newer control plane may ship fields ahead of this DP; serde + // must accept them. The write path still rejects them via the + // strict schema validator (validate_oidc_provider in models/schema.rs). + let p: OidcProvider = serde_json::from_str( r#"{"name":"x","issuer":"https://x","audiences":["a"],"extra":1}"#, - ); - assert!(r.is_err()); + ) + .unwrap(); + assert_eq!(p.name, "x"); } #[test] diff --git a/crates/aisix-core/src/models/provider_key.rs b/crates/aisix-core/src/models/provider_key.rs index 4295e146..84391bd6 100644 --- a/crates/aisix-core/src/models/provider_key.rs +++ b/crates/aisix-core/src/models/provider_key.rs @@ -27,7 +27,6 @@ use crate::resource::Resource; // NaN / Number-equality semantics. Tests compare via `assert_eq!` // which only needs `PartialEq`. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct ProviderKey { /// Operator-facing label, unique within the gateway. Surfaces in /// the Admin API list view and in dashboard UIs that wrap this @@ -105,7 +104,6 @@ pub struct ProviderKey { // settings themselves, sharing one connection pool across every Provider // Key configured the same way. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq, Hash)] -#[serde(deny_unknown_fields)] pub struct ProviderKeyTls { /// PEM-encoded certificate authority certificates trusted as issuers for /// this endpoint, in addition to the gateway's default trust store. A @@ -212,7 +210,6 @@ impl TelemetryKind { /// Telemetry attribution tags emitted with requests routed through this provider key. #[derive(Debug, Clone, Default, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq)] -#[serde(deny_unknown_fields)] pub struct TelemetryTags { /// Provider-key category, such as `"catalog"` for curated providers or /// `"byo"` for bring-your-own providers. @@ -243,7 +240,6 @@ pub struct TelemetryTags { /// request body parameters, clamp supported numeric parameters, add fallback /// outbound headers, or add fallback outbound body fields. #[derive(Debug, Clone, Default, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct RequestOverrides { /// `apply_param_renames` input. Top-level body keys named on the left are renamed to the right. Leave empty to preserve request parameter names. #[serde(default, skip_serializing_if = "HashMap::is_empty")] @@ -282,7 +278,6 @@ pub struct RequestOverrides { /// Numeric range clamps applied to chat-completion request bodies. #[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct ParamConstraints { /// Upper bound for `temperature`. Values above this are clamped /// to this value. If omitted, no upper bound is applied. @@ -299,7 +294,6 @@ pub struct ParamConstraints { /// stream termination behavior, flatten list-style content when needed, select /// an error envelope strategy, or lift provider-specific reasoning content. #[derive(Debug, Clone, Default, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq)] -#[serde(deny_unknown_fields)] pub struct ResponseOverrides { /// Stream `[DONE]` terminator expectation. If omitted, either presence /// or absence of the terminator is accepted. @@ -373,10 +367,14 @@ mod tests { } #[test] - fn rejects_unknown_fields() { - let r: Result = - serde_json::from_str(r#"{"display_name":"x","secret":"k","extra":1}"#); - assert!(r.is_err()); + fn tolerates_unknown_fields_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde + // must accept them. The write path still rejects them via the + // strict schema validator (validate_provider_key in + // models/schema.rs). + let p: ProviderKey = + serde_json::from_str(r#"{"display_name":"x","secret":"k","extra":1}"#).unwrap(); + assert_eq!(p.display_name, "x"); } // ---- `secret` → `api_key` rename ---- @@ -506,17 +504,20 @@ mod tests { } #[test] - fn telemetry_tags_rejects_unknown_field() { - // TelemetryTags is `deny_unknown_fields` — stops cp-api from - // silently shipping a new tag the DP can't see. - let r: Result = serde_json::from_str( + fn telemetry_tags_tolerates_unknown_field_for_forward_compat() { + // cp-api may ship a new tag ahead of the DP rolling out; serde + // must accept it. The write path still rejects it via the + // strict schema validator (validate_provider_key in + // models/schema.rs). + let p: ProviderKey = serde_json::from_str( r#"{ "display_name": "x", "secret": "k", - "telemetry_tags": { "unknown_tag": "v" } + "telemetry_tags": { "unknown_tag": "v", "featured": true } }"#, - ); - assert!(r.is_err()); + ) + .unwrap(); + assert!(p.telemetry_tags.featured); } #[test] @@ -650,17 +651,21 @@ mod tests { } #[test] - fn request_overrides_rejects_unknown_field() { - // deny_unknown_fields on RequestOverrides stops a typo in - // cp-api JSON from silently no-oping the apply call. - let r: Result = serde_json::from_str( + fn request_overrides_tolerates_unknown_field_for_forward_compat() { + // cp-api may ship new override fields ahead of the DP rolling + // out; serde must accept them. Typos on the write path are + // still rejected by the strict schema validator + // (validate_provider_key in models/schema.rs). + let p: ProviderKey = serde_json::from_str( r#"{ "display_name": "x", "secret": "k", - "request": { "param_rename": {} } + "request": { "param_rename": {}, "default_headers": { "X-Foo": "bar" } } }"#, - ); - assert!(r.is_err()); + ) + .unwrap(); + let req = p.request.expect("request was Some"); + assert_eq!(req.default_headers.get("X-Foo"), Some(&"bar".to_string())); } #[test] @@ -701,15 +706,20 @@ mod tests { } #[test] - fn response_overrides_rejects_unknown_field() { - let r: Result = serde_json::from_str( + fn response_overrides_tolerates_unknown_field_for_forward_compat() { + // cp-api may ship new override fields ahead of the DP rolling + // out; serde must accept them (the strict write-path schema + // still rejects them — validate_provider_key in models/schema.rs). + let p: ProviderKey = serde_json::from_str( r#"{ "display_name": "x", "secret": "k", - "response": { "reasoning_fields": "delta.foo" } + "response": { "reasoning_fields": "delta.foo", "error_envelope": "openai" } }"#, - ); - assert!(r.is_err()); + ) + .unwrap(); + let resp = p.response.expect("response was Some"); + assert_eq!(resp.error_envelope.as_deref(), Some("openai")); } #[test] @@ -754,9 +764,13 @@ mod tests { } #[test] - fn param_constraints_rejects_unknown_field() { - let r: Result = serde_json::from_str(r#"{"top_p_max": 0.9}"#); - assert!(r.is_err()); + fn param_constraints_tolerates_unknown_field_for_forward_compat() { + // cp-api may ship a new clamp ahead of the DP rolling out; + // serde must accept it (the strict write-path schema still + // rejects it — validate_provider_key in models/schema.rs). + let c: ParamConstraints = + serde_json::from_str(r#"{"top_p_max": 0.9, "temperature_max": 1.0}"#).unwrap(); + assert_eq!(c.temperature_max, Some(1.0)); } // ---- Issue #411 strip_headers deserialize/normalize ---- diff --git a/crates/aisix-core/src/models/rate_limit.rs b/crates/aisix-core/src/models/rate_limit.rs index 2c552f4f..6a47b329 100644 --- a/crates/aisix-core/src/models/rate_limit.rs +++ b/crates/aisix-core/src/models/rate_limit.rs @@ -14,7 +14,6 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct RateLimit { /// Tokens per 60-second window. #[serde(skip_serializing_if = "Option::is_none")] @@ -65,7 +64,6 @@ impl RateLimit { /// consumed — the shape leaves `tpm`/`tpd` out rather than accepting a knob /// that is silently inert. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct McpRateLimit { /// Tool calls per 1-second window. #[serde(skip_serializing_if = "Option::is_none")] @@ -134,8 +132,11 @@ mod tests { } #[test] - fn rejects_unknown_fields() { - let r: Result = serde_json::from_str(r#"{"rpm": 10, "extra": 1}"#); - assert!(r.is_err()); + fn tolerates_unknown_fields_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde must + // accept them (the write path still rejects them via the strict + // schema validators in models/schema.rs). + let rl: RateLimit = serde_json::from_str(r#"{"rpm": 10, "extra": 1}"#).unwrap(); + assert_eq!(rl.rpm, Some(10)); } } diff --git a/crates/aisix-core/src/models/rate_limit_policy.rs b/crates/aisix-core/src/models/rate_limit_policy.rs index 0a425044..4681622a 100644 --- a/crates/aisix-core/src/models/rate_limit_policy.rs +++ b/crates/aisix-core/src/models/rate_limit_policy.rs @@ -75,7 +75,6 @@ impl std::fmt::Display for PolicyWindow { } #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct RateLimitPolicy { #[schemars(length(min = 1))] pub name: String, @@ -161,8 +160,11 @@ mod tests { } #[test] - fn rejects_unknown_fields() { - let r: Result = serde_json::from_str( + fn tolerates_unknown_fields_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde must + // accept them. The write path still rejects them via the strict + // schema validator (validate_rate_limit_policy in models/schema.rs). + let p: RateLimitPolicy = serde_json::from_str( r#"{ "name": "x", "scope": "team", @@ -170,8 +172,9 @@ mod tests { "window": "minute", "extra": true }"#, - ); - assert!(r.is_err()); + ) + .unwrap(); + assert_eq!(p.name, "x"); } #[test] diff --git a/crates/aisix-core/src/models/routing.rs b/crates/aisix-core/src/models/routing.rs index cead1ea0..2c9f579b 100644 --- a/crates/aisix-core/src/models/routing.rs +++ b/crates/aisix-core/src/models/routing.rs @@ -70,7 +70,6 @@ impl RoutingStrategy { /// One destination in a routing configuration. `model` references a direct model alias. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq, Eq)] -#[serde(deny_unknown_fields)] pub struct RoutingTarget { /// Model alias for a direct model that can receive routed traffic. #[schemars(length(min = 1))] @@ -153,7 +152,6 @@ pub enum WhenAllUnavailablePolicy { } #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema)] -#[serde(deny_unknown_fields)] pub struct Routing { /// Strategy used to select a target for each request. #[serde(default)] @@ -382,16 +380,22 @@ mod tests { } #[test] - fn rejects_unknown_routing_fields() { - let r: Result = - serde_json::from_str(r#"{"strategy":"failover","targets":[{"model":"a"}],"foo":1}"#); - assert!(r.is_err()); + fn tolerates_unknown_routing_fields_for_forward_compat() { + // A newer control plane may ship fields ahead of this DP; serde must + // accept them. The write path still rejects them via the strict schema + // validator of the enclosing resource (validate_model in models/schema.rs). + let r: Routing = + serde_json::from_str(r#"{"strategy":"failover","targets":[{"model":"a"}],"foo":1}"#) + .unwrap(); + assert_eq!(r.strategy, RoutingStrategy::Failover); } #[test] - fn rejects_unknown_target_fields() { - let r: Result = - serde_json::from_str(r#"{"model":"a","weight":2,"extra":true}"#); - assert!(r.is_err()); + fn tolerates_unknown_target_fields_for_forward_compat() { + // Same forward-compat contract as above, for the nested target struct. + let t: RoutingTarget = + serde_json::from_str(r#"{"model":"a","weight":2,"extra":true}"#).unwrap(); + assert_eq!(t.model, "a"); + assert_eq!(t.weight, Some(2)); } } diff --git a/crates/aisix-core/src/models/schema.rs b/crates/aisix-core/src/models/schema.rs index bc3fa9b7..9779cfd5 100644 --- a/crates/aisix-core/src/models/schema.rs +++ b/crates/aisix-core/src/models/schema.rs @@ -10,6 +10,26 @@ //! 5. etcd txn commit //! ``` //! +//! Two validator sets exist since issue #871 (strict write / lenient read): +//! +//! - **Strict** ([`SCHEMAS`], the plain `validate_*` functions): unknown +//! fields are rejected. Used by every write path (Admin API, file source) +//! so typos keep failing loud with a 400, and published as the resource +//! schema files in `schemas/resources/` — the write contract. +//! - **Lenient** ([`LENIENT_SCHEMAS`], the `validate_*_lenient` functions): +//! unknown fields pass; every other constraint (types, required, ranges, +//! closed enums) still applies. Used only by the etcd snapshot loader so a +//! document written by a newer control plane loads with its extra fields +//! ignored — and reported — instead of whole-row rejected. +//! +//! Both sets build from the same per-resource producers; strictness is a +//! mechanical [`close_unknown_fields`] pass over the produced value, so the +//! two can never drift field-wise. Deliberately closed subschemas (the +//! `observability_exporter` branches and the guardrail tagged sub-enums, +//! injected inside the producers) stay closed in BOTH sets: serde silently +//! ignores unknown fields inside those tagged shapes, so an open schema +//! there would be an unreportable — silent — tolerance. +//! //! The watch path reuses step 2 on incoming events — malformed payloads are //! skipped with a warning and do not take down the gateway. @@ -20,7 +40,8 @@ use std::sync::Arc; use thiserror::Error; /// Cached compiled schemas. Compiling on every write would be wasteful; the -/// schemas are static, so we build them once. +/// schemas are static, so we build them once. This is the **strict** set: +/// unknown fields fail validation wherever the resource model closes them. pub struct Schemas { pub model: Validator, pub apikey: Validator, @@ -36,47 +57,105 @@ pub struct Schemas { pub oidc_provider: Validator, } -pub static SCHEMAS: Lazy> = Lazy::new(|| Arc::new(Schemas::compile())); +pub static SCHEMAS: Lazy> = Lazy::new(|| Arc::new(Schemas::compile(true))); + +/// The **lenient** twin of [`SCHEMAS`]: same producers, without the +/// [`close_unknown_fields`] pass. Only the etcd snapshot loader validates +/// against this set (issue #871); every write path stays on [`SCHEMAS`]. +pub static LENIENT_SCHEMAS: Lazy> = Lazy::new(|| Arc::new(Schemas::compile(false))); + +/// Whether a resource's write contract closes unknown top-level fields. +/// `cache_policy`, `guardrail`, `guardrail_attachment` and +/// `observability_exporter` historically ship open root schemas (documented +/// on their producers), so the strict closing pass skips them. Shared by +/// [`Schemas::compile`] and the resource schema published by `dump-schema`, +/// so the enforced write contract and the published one cannot drift. +fn closes_on_write(resource: &str) -> bool { + !matches!( + resource, + "cache_policy" | "guardrail" | "guardrail_attachment" | "observability_exporter" + ) +} + +/// The canonical schema of one resource, as enforced on the given path. +/// `strict` selects the write contract (unknown fields rejected wherever the +/// resource closes them); `!strict` the etcd read contract (unknown fields +/// tolerated). This is the single producer both validator sets and the +/// `dump-schema` binary build from. +pub fn resource_root_schema(resource: &str, strict: bool) -> Value { + let mut schema = match resource { + "model" => model_root_schema(), + "api_key" => apikey_root_schema(), + "provider_key" => provider_key_root_schema(), + "guardrail" => guardrail_root_schema(), + "guardrail_attachment" => guardrail_attachment_root_schema(), + "cache_policy" => cache_policy_root_schema(), + "observability_exporter" => observability_exporter_root_schema(), + "rate_limit_policy" => rate_limit_policy_root_schema(), + "mcp_server" => mcp_server_root_schema(), + "mcp_policy" => mcp_policy_root_schema(), + "a2a_agent" => a2a_agent_root_schema(), + "oidc_provider" => oidc_provider_root_schema(), + other => panic!("unknown resource {other:?}"), + }; + if strict && closes_on_write(resource) { + close_unknown_fields(&mut schema); + } + schema +} impl Schemas { - fn compile() -> Self { + fn compile(strict: bool) -> Self { + let build = |resource: &str| { + jsonschema::options() + .build(&resource_root_schema(resource, strict)) + .unwrap_or_else(|e| panic!("{resource} schema is well-formed: {e}")) + }; Self { - model: jsonschema::options() - .build(&model_root_schema()) - .expect("model schema is well-formed"), - apikey: jsonschema::options() - .build(&apikey_root_schema()) - .expect("apikey schema is well-formed"), - provider_key: jsonschema::options() - .build(&provider_key_root_schema()) - .expect("provider_key schema is well-formed"), - guardrail: jsonschema::options() - .build(&guardrail_root_schema()) - .expect("guardrail schema is well-formed"), - guardrail_attachment: jsonschema::options() - .build(&guardrail_attachment_root_schema()) - .expect("guardrail_attachment schema is well-formed"), - cache_policy: jsonschema::options() - .build(&cache_policy_root_schema()) - .expect("cache_policy schema is well-formed"), - observability_exporter: jsonschema::options() - .build(&observability_exporter_root_schema()) - .expect("observability_exporter schema is well-formed"), - rate_limit_policy: jsonschema::options() - .build(&rate_limit_policy_root_schema()) - .expect("rate_limit_policy schema is well-formed"), - mcp_server: jsonschema::options() - .build(&mcp_server_root_schema()) - .expect("mcp_server schema is well-formed"), - mcp_policy: jsonschema::options() - .build(&mcp_policy_root_schema()) - .expect("mcp_policy schema is well-formed"), - a2a_agent: jsonschema::options() - .build(&a2a_agent_root_schema()) - .expect("a2a_agent schema is well-formed"), - oidc_provider: jsonschema::options() - .build(&oidc_provider_root_schema()) - .expect("oidc_provider schema is well-formed"), + model: build("model"), + apikey: build("api_key"), + provider_key: build("provider_key"), + guardrail: build("guardrail"), + guardrail_attachment: build("guardrail_attachment"), + cache_policy: build("cache_policy"), + observability_exporter: build("observability_exporter"), + rate_limit_policy: build("rate_limit_policy"), + mcp_server: build("mcp_server"), + mcp_policy: build("mcp_policy"), + a2a_agent: build("a2a_agent"), + oidc_provider: build("oidc_provider"), + } + } +} + +/// Close a produced resource schema against unknown fields: insert +/// `additionalProperties: false` on the root object and on every +/// `definitions` entry that is a plain object schema (has `properties`). +/// +/// This reproduces exactly what `#[serde(deny_unknown_fields)]` made +/// `schemars` emit before issue #871 moved strictness out of the structs: +/// +/// - conditional/overlay subschemas (`oneOf`/`anyOf`/`allOf`/`if` branches) +/// are never touched — closing an `if`/`then` overlay would reject every +/// field the overlay does not list; +/// - an existing `additionalProperties` value is preserved, whether the +/// deliberate `false` on hand-closed branches or the value schema of a +/// map-typed field; +/// - enum-shaped definitions (no `properties`) are skipped. +pub fn close_unknown_fields(schema: &mut Value) { + fn close_object(node: &mut Value) { + let Some(obj) = node.as_object_mut() else { + return; + }; + if obj.contains_key("properties") && !obj.contains_key("additionalProperties") { + obj.insert("additionalProperties".to_string(), json!(false)); + } + } + + close_object(schema); + if let Some(Value::Object(defs)) = schema.get_mut("definitions") { + for def in defs.values_mut() { + close_object(def); } } } @@ -155,6 +234,60 @@ pub fn validate_oidc_provider(value: &Value) -> Result<(), SchemaError> { validate(&SCHEMAS.oidc_provider, value) } +// ---- lenient variants (etcd snapshot loader only, issue #871) ---- +// +// Unknown fields pass; every other constraint still applies. The loader +// pairs these with `serde_ignored` so tolerated fields are collected and +// reported as partially compatible rather than silently dropped. + +pub fn validate_model_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.model, value) +} + +pub fn validate_apikey_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.apikey, value) +} + +pub fn validate_provider_key_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.provider_key, value) +} + +pub fn validate_guardrail_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.guardrail, value) +} + +pub fn validate_cache_policy_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.cache_policy, value) +} + +pub fn validate_observability_exporter_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.observability_exporter, value) +} + +pub fn validate_rate_limit_policy_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.rate_limit_policy, value) +} + +pub fn validate_guardrail_attachment_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.guardrail_attachment, value) +} + +pub fn validate_mcp_server_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.mcp_server, value) +} + +pub fn validate_a2a_agent_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.a2a_agent, value) +} + +pub fn validate_mcp_policy_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.mcp_policy, value) +} + +pub fn validate_oidc_provider_lenient(value: &Value) -> Result<(), SchemaError> { + validate(&LENIENT_SCHEMAS.oidc_provider, value) +} + /// Build a resource's canonical JSON Schema from its struct via `schemars`, /// the single source of field shapes and per-field constraints. /// @@ -190,6 +323,27 @@ pub fn model_root_schema() -> Value { .as_object_mut() .expect("model root schema is a JSON object") .insert("oneOf".to_string(), super::model::model_one_of()); + // `OnEmbeddingFailure` is `#[serde(untagged)]` with an object variant + // (`{ "target": … }`): serde buffers untagged content and silently + // swallows unknown fields inside it, invisible to both the write + // path's serde step and the loader's `serde_ignored` reporting. The + // schema closure is therefore the only non-silent guard — same + // reasoning as the tagged-enum branch closures below, applied to + // both validator sets. + if let Some(any_of) = schema + .get_mut("definitions") + .and_then(|d| d.get_mut("OnEmbeddingFailure")) + .and_then(|b| b.get_mut("anyOf")) + .and_then(Value::as_array_mut) + { + for branch in any_of.iter_mut() { + if branch.get("type").and_then(Value::as_str) == Some("object") { + if let Some(obj) = branch.as_object_mut() { + obj.insert("additionalProperties".to_string(), json!(false)); + } + } + } + } schema } @@ -2979,6 +3133,95 @@ mod tests { validate_mcp_server(&v).unwrap(); } + // ---- strict-write / lenient-read split (issue #871) ---- + + #[test] + fn lenient_set_tolerates_unknown_fields_strict_set_rejects() { + let v = json!({ + "key_hash": "9df37f5e7cbc3c391d872742b5f286c242e733a09add9eeaa4d26a599bd90b20", + "allowed_models": ["a"], + "future_field": true + }); + assert!(validate_apikey(&v).is_err(), "write contract stays strict"); + validate_apikey_lenient(&v).expect("read contract tolerates unknown fields"); + } + + #[test] + fn lenient_set_still_enforces_every_other_constraint() { + // Missing required field. + assert!(validate_apikey_lenient(&json!({"allowed_models": []})).is_err()); + // Unknown enum value. + let v = json!({ + "display_name": "r", + "routing": {"strategy": "quantum", "targets": [{"model": "a"}]} + }); + assert!(validate_model_lenient(&v).is_err()); + // Range violation. + let v = json!({ + "display_name": "", "provider": "openai", + "model_name": "g", "provider_key_id": "pk" + }); + assert!(validate_model_lenient(&v).is_err()); + } + + #[test] + fn lenient_set_keeps_deliberate_closures_closed() { + // The observability-exporter branches guard the credential_ref + // indirection against a smuggled plaintext secret, and serde + // cannot report ignored fields inside tagged-enum content — so + // these closures must hold on the READ path too, or the + // tolerance would be silent. + let exporter = json!({ + "name": "o", "kind": "otlp_http", + "endpoint": "https://otel.example/v1/traces", + "smuggled_secret": "sk-x" + }); + assert!(validate_observability_exporter_lenient(&exporter).is_err()); + + // Same for the guardrail tagged sub-enums: serde silently + // swallows unknown fields inside inline-tagged variants. + let guardrail = json!({ + "name": "kw", "kind": "keyword", + "patterns": [{"kind": "literal", "value": "x", "extra": 1}] + }); + assert!(validate_guardrail_lenient(&guardrail).is_err()); + } + + #[test] + fn on_embedding_failure_object_variant_is_closed_on_both_paths() { + // `OnEmbeddingFailure` is untagged with an object variant: serde + // buffers untagged content and silently swallows unknown fields + // inside it — invisible to serde_ignored too. The producer closes + // the object branch so the typo is caught on write AND stays a + // loud (RED) rejection on read instead of a silent tolerance. + let v = json!({ + "display_name": "prod-chat", + "semantic": { + "embedding_model": "e", + "routes": [{"name": "a", "target": "m", "examples": ["x"]}], + "default": "d", + "match": {"threshold": 0.5}, + "on_embedding_failure": {"target": "t", "sneaky": 1} + } + }); + assert!(validate_model(&v).is_err()); + assert!(validate_model_lenient(&v).is_err()); + + // The legitimate shapes keep validating on both paths. + let ok = json!({ + "display_name": "prod-chat", + "semantic": { + "embedding_model": "e", + "routes": [{"name": "a", "target": "m", "examples": ["x"]}], + "default": "d", + "match": {"threshold": 0.5}, + "on_embedding_failure": {"target": "t"} + } + }); + validate_model(&ok).unwrap(); + validate_model_lenient(&ok).unwrap(); + } + #[test] fn mcp_server_rejects_zero_timeout_ms() { // A zero deadline times out every upstream op instantly and silently diff --git a/crates/aisix-core/src/models/semantic.rs b/crates/aisix-core/src/models/semantic.rs index e1cad0e8..8542e5fb 100644 --- a/crates/aisix-core/src/models/semantic.rs +++ b/crates/aisix-core/src/models/semantic.rs @@ -49,7 +49,6 @@ pub enum Aggregation { /// embeddings define the route. A request that scores high enough against /// them dispatches to `target`. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct SemanticRoute { /// Operator-facing route label. Surfaced in the `x-aisix-route` /// response header and access logs (e.g. `prod-chat -> route:legal`). @@ -78,7 +77,6 @@ pub struct SemanticRoute { /// Matching parameters shared across every route in a semantic router. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct SemanticMatch { /// Similarity metric. v1: cosine. #[serde(default)] @@ -131,7 +129,6 @@ impl Default for OnEmbeddingFailure { /// Semantic-routing config: pick a target by request meaning. #[derive(Debug, Clone, Serialize, Deserialize, schemars::JsonSchema, PartialEq)] -#[serde(deny_unknown_fields)] pub struct Semantic { /// Alias of an `embedding`-modality Model used to embed the request /// and (at apply time) the route examples. @@ -288,18 +285,24 @@ mod tests { } #[test] - fn rejects_unknown_semantic_field() { - let r: Result = serde_json::from_str( + fn tolerates_unknown_semantic_field_for_forward_compat() { + // cp-api may ship new fields ahead of the DP rolling out; serde must + // accept them. The write path still rejects them via the strict + // schema validators (validate_* in models/schema.rs). + let s: Semantic = serde_json::from_str( r#"{"embedding_model":"e","routes":[{"name":"a","target":"m","examples":["x"]}],"default":"d","match":{"threshold":0.5},"foo":1}"#, - ); - assert!(r.is_err()); + ) + .unwrap(); + assert_eq!(s.embedding_model, "e"); } #[test] - fn rejects_unknown_route_field() { - let r: Result = - serde_json::from_str(r#"{"name":"a","target":"m","examples":["x"],"bogus":true}"#); - assert!(r.is_err()); + fn tolerates_unknown_route_field_for_forward_compat() { + // Same forward-compat contract as above, at the route level. + let r: SemanticRoute = + serde_json::from_str(r#"{"name":"a","target":"m","examples":["x"],"bogus":true}"#) + .unwrap(); + assert_eq!(r.target, "m"); } #[test] diff --git a/crates/aisix-etcd/Cargo.toml b/crates/aisix-etcd/Cargo.toml index 8ae78411..963f85b5 100644 --- a/crates/aisix-etcd/Cargo.toml +++ b/crates/aisix-etcd/Cargo.toml @@ -16,6 +16,7 @@ futures.workspace = true async-trait.workspace = true serde.workspace = true serde_json.workspace = true +serde_ignored.workspace = true thiserror.workspace = true tracing.workspace = true base64.workspace = true diff --git a/crates/aisix-etcd/src/loader.rs b/crates/aisix-etcd/src/loader.rs index 4870b3a7..c1e00503 100644 --- a/crates/aisix-etcd/src/loader.rs +++ b/crates/aisix-etcd/src/loader.rs @@ -2,22 +2,35 @@ //! //! Flow: //! 1. parse the key → `(kind, id)` -//! 2. validate the value against the kind's JSON Schema -//! 3. deserialise into the typed struct via serde (cheap after schema -//! passes) +//! 2. validate the value against the kind's **lenient** JSON Schema — +//! types, required fields, ranges and closed enums are enforced; +//! unknown fields are not +//! 3. deserialise into the typed struct via `serde_ignored`, collecting +//! the paths of any fields serde had to ignore //! 4. insert into the appropriate [`ResourceTable`] //! -//! Malformed payloads are logged at WARN level and skipped, not fatal — -//! this matches spec §2: "the gateway does not abort on a single bad -//! entry; it serves the rest." +//! Every row lands in one of three compatibility states (issue #871): +//! +//! - **incompatible** (RED): step 2 or 3 fails — the row is skipped and +//! logged at ERROR, as today's contract genuinely cannot represent it; +//! - **partially compatible** (YELLOW): the row loaded but carried fields +//! this build does not know — typically written by a newer control +//! plane. It serves with those fields ignored, and the ignored paths +//! are reported through [`BuildStats::partially_compatible`]; +//! - fully compatible (GREEN): exact match, no signal. +//! +//! Rejected payloads are skipped, not fatal — this matches spec §2: +//! "the gateway does not abort on a single bad entry; it serves the +//! rest." use aisix_core::models::{ - validate_a2a_agent, validate_apikey, validate_cache_policy, validate_guardrail, - validate_guardrail_attachment, validate_mcp_policy, validate_mcp_server, validate_model, - validate_observability_exporter, validate_oidc_provider, validate_provider_key, - validate_rate_limit_policy, A2aAgent, ApiKey, CachePolicy, Guardrail, GuardrailAttachment, - McpPolicy, McpServer, Model, ObservabilityExporter, OidcProvider, ProviderKey, RateLimitPolicy, - SchemaError, + validate_a2a_agent_lenient, validate_apikey_lenient, validate_cache_policy_lenient, + validate_guardrail_attachment_lenient, validate_guardrail_lenient, validate_mcp_policy_lenient, + validate_mcp_server_lenient, validate_model_lenient, validate_observability_exporter_lenient, + validate_oidc_provider_lenient, validate_provider_key_lenient, + validate_rate_limit_policy_lenient, A2aAgent, ApiKey, CachePolicy, Guardrail, + GuardrailAttachment, McpPolicy, McpServer, Model, ObservabilityExporter, OidcProvider, + ProviderKey, RateLimitPolicy, SchemaError, }; use aisix_core::resource::ResourceEntry; use aisix_core::AisixSnapshot; @@ -39,8 +52,9 @@ pub enum RejectionKind { NonJson, /// JSON parsed but failed the kind's JSON Schema. SchemaFailed, - /// JSON Schema passed but `serde_json::from_value` refused — usually - /// a `deny_unknown_fields` mismatch between schema and Rust struct. + /// JSON Schema passed but serde deserialization refused — e.g. a + /// duplicate field via a rename alias, or an unknown field inside a + /// tagged enum whose shape stays closed. ParseFailed, /// Key referenced a `kind` segment we don't know about. Logged at /// debug normally but counted here so unknown kinds show up too. @@ -91,6 +105,64 @@ impl RejectedEntry { } } +/// One unknown-field observation from a row that still loaded — the +/// YELLOW ("partially compatible") state of issue #871. Aggregated per +/// (kind, field path) with a row count, never per row, so a fleet-wide +/// additive CP change produces one entry per field instead of one per +/// resource. Kept apart from [`RejectedEntry`] so YELLOW volume can +/// never evict RED entries from the retained rejection buffer. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct PartialCompatEntry { + /// Resource kind segment as it appears in the etcd key + /// (e.g. `api_keys`). + pub kind: String, + /// Dotted path of the ignored field inside the document + /// (e.g. `quota_profile` or `rate_limit.burst`). Array indices are + /// normalized to `[]` (`routing.targets[].priority`) so the entry + /// count is bounded by the document shape, not the data volume. + pub field: String, + /// Number of rows of this kind carrying this unknown field in the + /// build. + pub count: usize, +} + +/// The unknown fields one loaded row carried, keyed by its full etcd +/// key. This is the per-row form the supervisor merges into its +/// retained YELLOW state on incremental watch events (a re-put of the +/// same key replaces its entry; a delete removes it); the aggregated +/// [`PartialCompatEntry`] form is derived from it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PartialCompatRow { + /// Full etcd key of the row. + pub key: String, + /// Resource kind segment (e.g. `api_keys`). + pub kind: String, + /// Ignored field paths, index-normalized and deduplicated, sorted. + pub fields: Vec, +} + +/// Aggregate per-row unknown-field observations into the reporting form: +/// one entry per (kind, field path) with the number of rows carrying it, +/// sorted by kind then field. +pub fn aggregate_partial_compat(rows: &[PartialCompatRow]) -> Vec { + let mut counts: std::collections::BTreeMap<(&str, &str), usize> = Default::default(); + for row in rows { + for field in &row.fields { + *counts + .entry((row.kind.as_str(), field.as_str())) + .or_insert(0) += 1; + } + } + counts + .into_iter() + .map(|((kind, field), count)| PartialCompatEntry { + kind: kind.to_string(), + field: field.to_string(), + count, + }) + .collect() +} + /// Counts of rejected entries during a build, plus the rejection /// list itself. The counts stay handy for metrics; the list is what /// the heartbeat sends upstream so the dashboard can show "your DP @@ -111,6 +183,13 @@ pub struct BuildStats { /// upstream provider feeds in; the supervisor caps its retained /// buffer separately. pub rejections: Vec, + /// Unknown-field observations from rows that loaded anyway, + /// aggregated per (kind, field path). Empty when every row matched + /// its schema exactly. + pub partially_compatible: Vec, + /// The same observations in per-row form (etcd key → ignored + /// fields), for the supervisor's incremental watch-event merging. + pub partial_rows: Vec, } /// Build a fresh snapshot from raw entries. Never fails — bad rows are @@ -156,7 +235,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_model, + validate_model_lenient, &mut stats, ) { snapshot.models.insert(entry); @@ -168,7 +247,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_apikey, + validate_apikey_lenient, &mut stats, ) { snapshot.apikeys.insert(entry); @@ -180,7 +259,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_provider_key, + validate_provider_key_lenient, &mut stats, ) { snapshot.provider_keys.insert(entry); @@ -192,7 +271,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_guardrail, + validate_guardrail_lenient, &mut stats, ) { snapshot.guardrails.insert(entry); @@ -204,7 +283,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_guardrail_attachment, + validate_guardrail_attachment_lenient, &mut stats, ) { snapshot.guardrail_attachments.insert(entry); @@ -216,7 +295,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_cache_policy, + validate_cache_policy_lenient, &mut stats, ) { snapshot.cache_policies.insert(entry); @@ -228,7 +307,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_observability_exporter, + validate_observability_exporter_lenient, &mut stats, ) { snapshot.observability_exporters.insert(entry); @@ -240,7 +319,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_rate_limit_policy, + validate_rate_limit_policy_lenient, &mut stats, ) { snapshot.rate_limit_policies.insert(entry); @@ -252,7 +331,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_mcp_server, + validate_mcp_server_lenient, &mut stats, ) { snapshot.mcp_servers.insert(entry); @@ -264,7 +343,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_mcp_policy, + validate_mcp_policy_lenient, &mut stats, ) { snapshot.mcp_policies.insert(entry); @@ -276,7 +355,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_a2a_agent, + validate_a2a_agent_lenient, &mut stats, ) { snapshot.a2a_agents.insert(entry); @@ -288,7 +367,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui raw.revision, parsed, &value, - validate_oidc_provider, + validate_oidc_provider_lenient, &mut stats, ) { snapshot.oidc_providers.insert(entry); @@ -306,6 +385,7 @@ pub fn build_snapshot(prefix: &str, entries: &[RawEntry]) -> (AisixSnapshot, Bui } } + stats.partially_compatible = aggregate_partial_compat(&stats.partial_rows); (snapshot, stats) } @@ -320,8 +400,13 @@ fn validate_and_parse( where T: DeserializeOwned, { + // RED: the lenient schema still enforces types, required fields, + // ranges and closed enums — a failure here means this build cannot + // represent the row at all, so it is skipped. ERROR, not warn: under + // the supported CP-before-DP upgrade order this is the signal that a + // resource stopped applying on this instance. if let Err(err) = validate(value) { - tracing::warn!(key = %key, error = %err, "schema validation failed; skipping"); + tracing::error!(key = %key, error = %err, "schema validation failed; skipping (incompatible row)"); stats.schema_rejected += 1; stats.rejections.push(RejectedEntry::new( key, @@ -331,15 +416,40 @@ where return None; } - match serde_json::from_value::(value.clone()) { + let mut ignored: Vec = Vec::new(); + match serde_ignored::deserialize::<_, _, T>(value, |path| { + ignored.push(normalize_ignored_path(&path.to_string())); + }) { Ok(t) => { stats.accepted += 1; + if !ignored.is_empty() { + // YELLOW: loaded, but fields this build does not know were + // ignored — typically written by a newer control plane. + ignored.sort_unstable(); + ignored.dedup(); + // A single document can carry an arbitrary number of + // unknown fields with arbitrary-length names; everything + // captured here flows into logs, the retained map, the + // status JSON and the heartbeat body. Cap per row — the + // sentinel keeps the truncation visible in every report. + if ignored.len() > MAX_REPORTED_FIELDS_PER_ROW { + ignored.truncate(MAX_REPORTED_FIELDS_PER_ROW); + ignored.push("...truncated".to_string()); + } + warn_partial_compat_deduped(key, parsed.kind, &ignored); + stats.partial_rows.push(PartialCompatRow { + key: key.to_string(), + kind: parsed.kind.to_string(), + fields: ignored, + }); + } Some(ResourceEntry::new(parsed.id, t, revision)) } Err(err) => { - // Schema passed but serde refused — usually a deny_unknown_fields - // mismatch. Treat as schema-rejected for stats purposes. - tracing::warn!(key = %key, error = %err, "serde parse failed after schema pass"); + // RED: schema passed but serde refused — a duplicate field via + // a rename alias, or an unknown field inside a tagged enum + // whose shape stays closed. + tracing::error!(key = %key, error = %err, "serde parse failed after schema pass (incompatible row)"); stats.parse_rejected += 1; stats.rejections.push(RejectedEntry::new( key, @@ -351,6 +461,74 @@ where } } +/// Cap on ignored-field paths reported per row. A row over the cap keeps +/// its first entries (sorted) plus a `...truncated` sentinel, so the +/// truncation shows up in every downstream report instead of silently +/// under-counting. +const MAX_REPORTED_FIELDS_PER_ROW: usize = 64; + +/// Normalize a `serde_ignored` path into document terms: array indices +/// become `[]` so the aggregated report stays bounded by the document +/// shape (`targets.0.x` and `targets.1.x` are one field, not two), and +/// the `?` segments serde_ignored emits for `Option` wrapping layers — +/// invisible in the JSON — are dropped (`rate_limit.?.burst` → +/// `rate_limit.burst`). +/// +/// Lossy by design: a map key that is itself all digits (or literally +/// `?`) aliases with the normalized forms and merges counts, in the +/// aggregate and in the WARN line alike. The actionable signal is the +/// field name per kind, not which array element carried it. +fn normalize_ignored_path(path: &str) -> String { + path.split('.') + .filter(|seg| *seg != "?") + .map(|seg| { + if seg.parse::().is_ok() { + "[]" + } else { + seg + } + }) + .collect::>() + .join(".") +} + +/// WARN once per (kind, field-set) for the process lifetime. Resyncs +/// rebuild the whole snapshot on a cadence; without dedup every cycle +/// would re-log every YELLOW row. The set is capped: past the cap new +/// combinations keep logging (never silently dropped) but are no longer +/// remembered, so a pathological fleet re-logs on each resync instead +/// of growing memory without bound. +fn warn_partial_compat_deduped(key: &str, kind: &str, fields: &[String]) { + use std::collections::HashSet; + use std::sync::{Mutex, OnceLock}; + + const MAX_REMEMBERED: usize = 1024; + static WARNED: OnceLock>> = OnceLock::new(); + + let fields_joined = fields.join(","); + let entry = (kind.to_string(), fields_joined); + // Poison-tolerant: this set only dedupes log lines, so a panic while + // the lock was held (e.g. inside a tracing subscriber) must not wedge + // every subsequent snapshot build in the supervisor task. + let mut warned = WARNED + .get_or_init(|| Mutex::new(HashSet::new())) + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if warned.contains(&entry) { + return; + } + tracing::warn!( + key = %key, + kind = %kind, + ignored_fields = %entry.1, + "row loaded with unknown fields ignored (partially compatible; \ + likely written by a newer control plane)" + ); + if warned.len() < MAX_REMEMBERED { + warned.insert(entry); + } +} + #[cfg(test)] mod tests { use super::*; @@ -518,60 +696,261 @@ mod tests { } #[test] - fn provider_key_aws_region_payload_currently_rejected() { - // Documents the current gap: an `amazon-bedrock` - // provider_key payload must carry `aws_region` per - // adapter_map.yaml:30, but the ProviderKey struct in - // `aisix-core::models::provider_key` is - // `#[serde(deny_unknown_fields)]` and has no `aws_region` - // field. Today the loader REJECTS the entry, so a customer - // creating a Bedrock provider_key via cp-api never sees - // the row reach the DP. Tracked as a follow-up to Adapter - // family e2e coverage (Tier 3 + Tier 4-7 in #398). - // - // When ProviderKey gains adapter-family extra-config fields - // (or an `extra: HashMap` escape hatch), this - // test should flip to assert `accepted=1` instead. + fn provider_key_aws_region_payload_loads_partially_compatible() { + // Flip of the pre-#871 `*_currently_rejected` pin. cp-api's + // adapter_map admits Bedrock provider_key payloads carrying a + // top-level `aws_region`; the DP adapter never reads that field + // — the Bedrock bridge takes its region from the credential JSON + // inside `api_key` (`aisix-provider-bedrock/src/bridge.rs`, + // `BedrockSecret.region`). So the field stays off the model + // (a struct field nothing consumes would be dead config) and the + // row loads with the field ignored and reported, instead of the + // pre-#871 whole-row rejection that silently kept the key from + // ever reaching dispatch. let entries = vec![raw( "/aisix/provider_keys/pk-bedrock", br#"{"display_name":"bedrock-pk","secret":"x","provider":"amazon-bedrock","aws_region":"us-east-1"}"#, 1, )]; - let (_snap, stats) = build_snapshot("/aisix", &entries); - assert_eq!(stats.accepted, 0); - assert_eq!(stats.schema_rejected, 1); - assert_eq!(stats.rejections.len(), 1); - assert_eq!(stats.rejections[0].kind, RejectionKind::SchemaFailed); + let (snap, stats) = build_snapshot("/aisix", &entries); + assert_eq!(stats.accepted, 1, "rejections: {:?}", stats.rejections); + assert!(stats.rejections.is_empty()); + assert!(snap.provider_keys.get_by_id("pk-bedrock").is_some()); + assert_eq!( + stats.partially_compatible, + vec![PartialCompatEntry { + kind: "provider_keys".into(), + field: "aws_region".into(), + count: 1, + }] + ); } #[test] - fn provider_key_gcp_project_payload_currently_rejected() { - // Same gap as aws_region: `google-vertex` needs - // gcp_project + gcp_region per adapter_map.yaml. Today the - // loader rejects. + fn provider_key_gcp_project_payload_loads_partially_compatible() { + // Same decision as aws_region: the Vertex bridge reads project + // and region from the credential JSON inside `api_key` + // (`VertexSecret.project` / `.region`), never from top-level + // fields — YELLOW load, fields reported. let entries = vec![raw( "/aisix/provider_keys/pk-vertex", br#"{"display_name":"vertex-pk","secret":"x","provider":"google-vertex","gcp_project":"my-proj","gcp_region":"us-central1"}"#, 1, )]; - let (_snap, stats) = build_snapshot("/aisix", &entries); - assert_eq!(stats.accepted, 0); - assert_eq!(stats.schema_rejected, 1); + let (snap, stats) = build_snapshot("/aisix", &entries); + assert_eq!(stats.accepted, 1, "rejections: {:?}", stats.rejections); + assert!(snap.provider_keys.get_by_id("pk-vertex").is_some()); + assert_eq!( + stats.partially_compatible, + vec![ + PartialCompatEntry { + kind: "provider_keys".into(), + field: "gcp_project".into(), + count: 1, + }, + PartialCompatEntry { + kind: "provider_keys".into(), + field: "gcp_region".into(), + count: 1, + }, + ] + ); } #[test] - fn provider_key_azure_resource_payload_currently_rejected() { - // Same gap as aws_region: `azure` needs - // azure_resource_name + api_version per adapter_map.yaml. - // Today the loader rejects. + fn provider_key_azure_resource_payload_loads_partially_compatible() { + // Same decision as aws_region: the Azure bridge derives the + // resource name from `api_base` and pins its own API version — + // neither top-level field is consumed — YELLOW load, fields + // reported. let entries = vec![raw( "/aisix/provider_keys/pk-azure", br#"{"display_name":"azure-pk","secret":"x","provider":"azure","azure_resource_name":"my-azure","api_version":"2024-02-01"}"#, 1, )]; + let (snap, stats) = build_snapshot("/aisix", &entries); + assert_eq!(stats.accepted, 1, "rejections: {:?}", stats.rejections); + assert!(snap.provider_keys.get_by_id("pk-azure").is_some()); + assert_eq!( + stats.partially_compatible, + vec![ + PartialCompatEntry { + kind: "provider_keys".into(), + field: "api_version".into(), + count: 1, + }, + PartialCompatEntry { + kind: "provider_keys".into(), + field: "azure_resource_name".into(), + count: 1, + }, + ] + ); + } + + // ---- forward-compat: lenient parse + tri-state (issue #871) ---- + // + // Under the supported rolling-upgrade order (CP first, DPs + // behind), an upgraded CP adds a field to a resource and an older + // DP receives the document. The DP must load the row with the + // unknown field ignored (YELLOW / partially compatible) and report + // exactly which field it ignored — not whole-row reject, which + // silently drops the resource on the next resync/restart. + + #[test] + fn api_key_unknown_field_is_accepted_and_reported_partially_compatible() { + let entries = vec![raw( + "/aisix/api_keys/k-forward", + br#"{ + "key_hash": "1460db1b6902f8b1fc2a40d9381a24d0fd22c3bc1b2c6f999c521da73776fbe0", + "allowed_models": ["my-gpt4"], + "quota_profile": "gold" + }"#, + 1, + )]; + let (snap, stats) = build_snapshot("/aisix", &entries); + + // YELLOW: the row loads and serves with the unknown field ignored. + assert_eq!(stats.accepted, 1, "rejections: {:?}", stats.rejections); + assert_eq!(stats.schema_rejected, 0); + assert_eq!(stats.parse_rejected, 0); + assert!(stats.rejections.is_empty()); + assert_eq!(snap.apikeys.len(), 1); + let entry = snap.apikeys.get_by_id("k-forward").unwrap(); + assert_eq!(entry.value.allowed_models, vec!["my-gpt4"]); + + // ...and the ignored field is reported, aggregated per + // (kind, field path) with a row count. + assert_eq!( + stats.partially_compatible, + vec![PartialCompatEntry { + kind: "api_keys".into(), + field: "quota_profile".into(), + count: 1, + }] + ); + } + + #[test] + fn nested_unknown_field_reports_dotted_path() { + let entries = vec![raw( + "/aisix/api_keys/k-nested", + br#"{ + "key_hash": "1460db1b6902f8b1fc2a40d9381a24d0fd22c3bc1b2c6f999c521da73776fbe0", + "allowed_models": ["m"], + "rate_limit": {"rpm": 60, "burst": 10} + }"#, + 1, + )]; + let (_snap, stats) = build_snapshot("/aisix", &entries); + assert_eq!(stats.accepted, 1, "rejections: {:?}", stats.rejections); + assert_eq!( + stats.partially_compatible, + vec![PartialCompatEntry { + kind: "api_keys".into(), + field: "rate_limit.burst".into(), + count: 1, + }] + ); + } + + #[test] + fn unknown_enum_value_stays_incompatible() { + // A value this build cannot interpret has no lenient fallback: + // a routing strategy from a newer CP is RED, not YELLOW. + let entries = vec![raw( + "/aisix/models/m-newstrat", + br#"{"display_name":"r","routing":{"strategy":"quantum","targets":[{"model":"a"}]}}"#, + 1, + )]; let (_snap, stats) = build_snapshot("/aisix", &entries); assert_eq!(stats.accepted, 0); assert_eq!(stats.schema_rejected, 1); + assert_eq!(stats.rejections[0].kind, RejectionKind::SchemaFailed); + assert!(stats.partially_compatible.is_empty()); + } + + #[test] + fn partial_compat_aggregates_across_rows_of_a_kind() { + let doc = br#"{ + "key_hash": "1460db1b6902f8b1fc2a40d9381a24d0fd22c3bc1b2c6f999c521da73776fbe0", + "allowed_models": ["m"], + "quota_profile": "gold" + }"#; + let entries = vec![ + raw("/aisix/api_keys/k-1", doc, 1), + raw("/aisix/api_keys/k-2", doc, 2), + ]; + let (_snap, stats) = build_snapshot("/aisix", &entries); + assert_eq!(stats.accepted, 2); + assert_eq!( + stats.partially_compatible, + vec![PartialCompatEntry { + kind: "api_keys".into(), + field: "quota_profile".into(), + count: 2, + }] + ); + // The per-row form keeps one record per etcd key. + assert_eq!(stats.partial_rows.len(), 2); + assert_eq!(stats.partial_rows[0].key, "/aisix/api_keys/k-1"); + assert_eq!(stats.partial_rows[1].key, "/aisix/api_keys/k-2"); + } + + #[test] + fn guardrail_attachment_in_cp_projection_shape_is_fully_compatible() { + // The managed control plane writes `env_id` on every attachment + // document (its own tenancy scoping; the gateway does not read + // it). The field is declared on the model as known-and-ignored, + // so a same-version managed fleet reports ZERO partially + // compatible rows — a standing false version-skew alarm here + // would train operators to ignore the YELLOW signal entirely. + let entries = vec![raw( + "/aisix/guardrail_attachments/ga-1", + br#"{ + "guardrail_id": "11111111-1111-1111-1111-111111111111", + "scope_type": "env", + "scope_id": null, + "priority": 0, + "enabled": true, + "env_id": "22222222-2222-2222-2222-222222222222" + }"#, + 1, + )]; + let (snap, stats) = build_snapshot("/aisix", &entries); + assert_eq!(stats.accepted, 1, "rejections: {:?}", stats.rejections); + assert_eq!(snap.guardrail_attachments.len(), 1); + assert!( + stats.partially_compatible.is_empty(), + "env_id is a registered cross-plane field, not version skew: {:?}", + stats.partially_compatible + ); + } + + #[test] + fn per_row_unknown_field_report_is_capped_with_a_visible_sentinel() { + // One document can carry arbitrarily many unknown fields with + // arbitrary-length names; everything captured flows into logs, + // the retained map, the status JSON and the heartbeat body. + let mut doc = serde_json::json!({ + "key_hash": "1460db1b6902f8b1fc2a40d9381a24d0fd22c3bc1b2c6f999c521da73776fbe0", + "allowed_models": ["m"] + }); + for i in 0..200 { + doc.as_object_mut() + .unwrap() + .insert(format!("unknown_field_{i:03}"), serde_json::json!(1)); + } + let entries = vec![raw( + "/aisix/api_keys/k-flood", + doc.to_string().as_bytes(), + 1, + )]; + let (_snap, stats) = build_snapshot("/aisix", &entries); + assert_eq!(stats.accepted, 1); + let fields = &stats.partial_rows[0].fields; + assert_eq!(fields.len(), 65, "64 fields + the truncation sentinel"); + assert_eq!(fields.last().map(String::as_str), Some("...truncated")); } // ---- renamed-field dual acceptance ---- diff --git a/crates/aisix-etcd/src/supervisor.rs b/crates/aisix-etcd/src/supervisor.rs index eff991ee..7484a0c6 100644 --- a/crates/aisix-etcd/src/supervisor.rs +++ b/crates/aisix-etcd/src/supervisor.rs @@ -16,7 +16,8 @@ //! read path reading a fully-formed `Arc` the whole time. use aisix_core::config_status::{ - hash_entries, AppliedSnapshot, ConfigStatus, IncomingRejection, LoadObservation, SourceKind, + hash_entries, AppliedSnapshot, ConfigStatus, IncomingRejection, LoadObservation, + PartialCompatResource, SourceKind, }; use aisix_core::snapshot::SnapshotHandle; use aisix_core::AisixSnapshot; @@ -31,7 +32,7 @@ use tokio::task::JoinHandle; use crate::backoff::ExpBackoff; use crate::key; -use crate::loader::{self, BuildStats, RejectedEntry}; +use crate::loader::{self, BuildStats, PartialCompatEntry, PartialCompatRow, RejectedEntry}; use crate::provider::{ConfigProvider, ProviderError, RawEntry, WatchEvent}; use crate::snapshot_cache::SnapshotCache; @@ -130,6 +131,13 @@ pub struct WatchStatusSnapshot { /// memory. Newest rejection wins on overflow (drops the oldest). const MAX_RETAINED_REJECTIONS: usize = 256; +/// Maximum partially-compatible rows the supervisor retains, in its own +/// buffer so YELLOW volume can never evict RED entries from the rejection +/// buffer above (#871). Bounded by the number of config rows in practice; +/// past the cap new rows are still logged by the loader but drop out of +/// the aggregated report, with a WARN so the truncation is never silent. +const MAX_RETAINED_PARTIAL_ROWS: usize = 1024; + /// One supervisor instance. Consumers call [`Supervisor::run`] once and /// drop the returned handle on shutdown. pub struct Supervisor { @@ -166,6 +174,15 @@ pub struct Supervisor { /// per-event because they only see one row. rejections: Mutex>, + /// Rows currently served with unknown fields ignored (partially + /// compatible, #871), keyed by etcd key so incremental watch events + /// merge cleanly: a Put replaces (or clears) the key's entry, a + /// Delete removes it, a resync replaces the map wholesale. Reported + /// aggregated per (kind, field) on `/status/config` and the + /// heartbeat. Separate from `rejections` by design — see + /// [`MAX_RETAINED_PARTIAL_ROWS`]. + partial_compat: Mutex>, + // JoinHandles for in-flight `flush_cache` writes. Tests use // [`Self::await_pending_cache_writes`] to deterministically wait // for these without relying on a wall-clock sleep, which proved @@ -199,6 +216,7 @@ impl Supervisor

{ status: WatchStatus::new(), config_status: ConfigStatus::new(SourceKind::Etcd), rejections: Mutex::new(Vec::new()), + partial_compat: Mutex::new(HashMap::new()), pending_writes: Mutex::new(Vec::new()), } } @@ -250,6 +268,8 @@ impl Supervisor

{ } let revision = *self.revision.lock().unwrap(); let resource_counts = resource_counts(&self.handle.load()); + let (partially_compatible, partially_compatible_rows_by_kind) = + self.partial_compat_observation(); self.config_status.record_load(LoadObservation { source_hash, @@ -260,6 +280,8 @@ impl Supervisor

{ resource_counts, }), rejected, + partially_compatible, + partially_compatible_rows_by_kind, is_reload, // etcd always publishes the accepted subset (even an empty one); // it never retains a previous snapshot wholesale, so a wholly- @@ -328,6 +350,81 @@ impl Supervisor

{ guard.len() != before } + /// Aggregated partially-compatible observations for the currently + /// served snapshot: one entry per (kind, field) with the number of + /// rows carrying it, sorted. Read by the heartbeat path (cloned, no + /// lock held across the HTTP call). + pub fn recent_partial_compat(&self) -> Vec { + let guard = self.partial_compat.lock().unwrap(); + let rows: Vec = guard.values().cloned().collect(); + drop(guard); + loader::aggregate_partial_compat(&rows) + } + + /// Replace the retained partially-compatible state wholesale. Called + /// by the resync paths, which re-process every entry. + fn set_partial_rows(&self, rows: Vec) { + let mut guard = self.partial_compat.lock().unwrap(); + guard.clear(); + for row in rows { + if guard.len() >= MAX_RETAINED_PARTIAL_ROWS { + tracing::warn!( + cap = MAX_RETAINED_PARTIAL_ROWS, + "partially-compatible rows exceed the retention cap; \ + the aggregated report is truncated" + ); + break; + } + guard.insert(row.key.clone(), row); + } + } + + /// Merge one apply_put outcome into the retained partially-compatible + /// state: the row's new unknown-field set replaces its previous one, + /// and a row that now matches exactly clears its entry. + fn update_partial_row(&self, key: &str, row: Option) { + let mut guard = self.partial_compat.lock().unwrap(); + match row { + Some(row) => { + if !guard.contains_key(key) && guard.len() >= MAX_RETAINED_PARTIAL_ROWS { + tracing::warn!( + key = %key, + cap = MAX_RETAINED_PARTIAL_ROWS, + "partially-compatible rows exceed the retention cap; \ + this row is served but missing from the aggregated report" + ); + return; + } + guard.insert(key.to_string(), row); + } + None => { + guard.remove(key); + } + } + } + + /// The retained partially-compatible state in the two wire shapes + /// [`LoadObservation`] carries: the per-(kind, field) aggregate and + /// the per-kind row counts. + fn partial_compat_observation(&self) -> (Vec, BTreeMap) { + let guard = self.partial_compat.lock().unwrap(); + let rows: Vec = guard.values().cloned().collect(); + drop(guard); + let aggregated = loader::aggregate_partial_compat(&rows) + .into_iter() + .map(|e| PartialCompatResource { + resource_kind: e.kind, + field: e.field, + count: e.count, + }) + .collect(); + let mut rows_by_kind: BTreeMap = BTreeMap::new(); + for row in &rows { + *rows_by_kind.entry(row.kind.clone()).or_insert(0) += 1; + } + (aggregated, rows_by_kind) + } + /// Drain the JoinHandles for any in-flight cache writes spawned /// by [`Self::flush_cache`] and await them. Test-only synchroniser: /// production code never needs to block on disk persistence. @@ -424,6 +521,9 @@ impl Supervisor

{ // Build a tiny snapshot out of just the new entry, then merge. let (tiny, mut stats) = loader::build_snapshot(&self.prefix, std::slice::from_ref(entry)); if stats.accepted == 0 { + // Note: a previously retained partially-compatible entry for + // this key is deliberately kept — the row's last-good value + // (loaded with those fields ignored) is still what serves. // The loader already attached a RejectedEntry for whatever // path failed (bad key / non-JSON / schema / parse). Move // them into the supervisor's retained buffer so the next @@ -492,6 +592,14 @@ impl Supervisor

{ new }); self.remove_rejection_for_key(&entry.key); + // Refresh this key's partially-compatible signal: replaced when + // the new value still carries unknown fields, cleared when it now + // matches the schema exactly. + let partial = stats + .partial_rows + .drain(..) + .find(|row| row.key == entry.key); + self.update_partial_row(&entry.key, partial); // Mirror the put into the cache-tracking map and flush. // Track the highest revision we've observed so the cache file @@ -551,6 +659,9 @@ impl Supervisor

{ _ => false, }; let removed_rejection = self.remove_rejection_for_key(key_str); + // A deleted key no longer serves, so its partially-compatible + // signal (if any) goes with it. + self.update_partial_row(key_str, None); drop(snap); if !present { if removed_rejection { @@ -654,8 +765,10 @@ impl Supervisor

{ self.status.record_apply(cur_rev); // Resync re-processes the entire entry set so the prior // per-key rejection list is no longer accurate — replace it - // wholesale with what this build produced (issue #115). + // wholesale with what this build produced (issue #115). Same for + // the partially-compatible state (#871). self.set_rejections(stats.rejections.clone()); + self.set_partial_rows(stats.partial_rows.clone()); // A full resync is a config reload — publish the observability view // (source/config hashes, counts, rejected list) and count it. self.sync_config_status(true); @@ -1530,4 +1643,90 @@ mod tests { "delete must clear a rejection even when the bad row never entered the snapshot", ); } + + // ---- partially-compatible retention (issue #871) ---- + + /// A model document carrying a field this build does not know: loads + /// (YELLOW) with the field reported. + const YELLOW_MODEL: &[u8] = br#"{ + "display_name": "my-gpt4", + "provider": "openai", + "model_name": "gpt-4o", + "provider_key_id": "11111111-1111-1111-1111-111111111111", + "future_knob": true + }"#; + + #[tokio::test] + async fn partial_compat_tracked_on_put_and_cleared_on_exact_match() { + let provider = Arc::new(FakeProvider::new(vec![], 0)); + let sup = Supervisor::new(provider, "/aisix"); + sup.load_once().await.unwrap(); + + assert!(sup.apply_put(&entry("/aisix/models/m-1", YELLOW_MODEL, 1))); + assert_eq!(sup.handle().load().models.len(), 1, "YELLOW row serves"); + let agg = sup.recent_partial_compat(); + assert_eq!(agg.len(), 1); + assert_eq!(agg[0].kind, "models"); + assert_eq!(agg[0].field, "future_knob"); + assert_eq!(agg[0].count, 1); + // The status view carries the companion list next to rejected[]. + let view = sup.config_status().view(); + assert_eq!(view.partially_compatible.len(), 1); + assert_eq!(view.partially_compatible[0].resource_kind, "models"); + assert_eq!(view.partially_compatible[0].field, "future_knob"); + assert!(view.rejected.is_empty()); + + // Re-put with an exact-match document: the signal clears. + assert!(sup.apply_put(&entry("/aisix/models/m-1", VALID_MODEL, 2))); + assert!(sup.recent_partial_compat().is_empty()); + assert!(sup.config_status().view().partially_compatible.is_empty()); + } + + #[tokio::test] + async fn partial_compat_cleared_on_delete() { + let provider = Arc::new(FakeProvider::new(vec![], 0)); + let sup = Supervisor::new(provider, "/aisix"); + sup.load_once().await.unwrap(); + + assert!(sup.apply_put(&entry("/aisix/models/m-1", YELLOW_MODEL, 1))); + assert_eq!(sup.recent_partial_compat().len(), 1); + assert!(sup.apply_delete("/aisix/models/m-1")); + assert!(sup.recent_partial_compat().is_empty()); + } + + #[tokio::test] + async fn partial_compat_replaced_wholesale_on_resync() { + let provider = Arc::new(FakeProvider::new(vec![], 0)); + let sup = Supervisor::new(provider, "/aisix"); + sup.load_once().await.unwrap(); + + assert!(sup.apply_put(&entry("/aisix/models/m-1", YELLOW_MODEL, 1))); + assert_eq!(sup.recent_partial_compat().len(), 1); + + // Resync to a clean entry set: prior per-key YELLOW state is + // no longer accurate and must be dropped, mirroring rejections. + sup.apply_resync(&[entry("/aisix/models/m-2", VALID_MODEL, 2)]); + assert!(sup.recent_partial_compat().is_empty()); + + // Resync back to a YELLOW set repopulates it. + sup.apply_resync(&[entry("/aisix/models/m-3", YELLOW_MODEL, 3)]); + let agg = sup.recent_partial_compat(); + assert_eq!(agg.len(), 1); + assert_eq!(agg[0].count, 1); + } + + #[tokio::test] + async fn partial_compat_kept_when_update_for_same_key_is_rejected() { + let provider = Arc::new(FakeProvider::new(vec![], 0)); + let sup = Supervisor::new(provider, "/aisix"); + sup.load_once().await.unwrap(); + + assert!(sup.apply_put(&entry("/aisix/models/m-1", YELLOW_MODEL, 1))); + // A rejected update keeps the previous (YELLOW-loaded) value + // serving, so the partially-compatible signal must survive too. + assert!(!sup.apply_put(&entry("/aisix/models/m-1", BAD_PROVIDER_MODEL, 2))); + assert_eq!(sup.handle().load().models.len(), 1); + assert_eq!(sup.recent_partial_compat().len(), 1); + assert_eq!(sup.recent_rejections().len(), 1); + } } diff --git a/crates/aisix-obs/src/metrics.rs b/crates/aisix-obs/src/metrics.rs index 66ffa19f..2b9592e0 100644 --- a/crates/aisix-obs/src/metrics.rs +++ b/crates/aisix-obs/src/metrics.rs @@ -178,6 +178,12 @@ pub const M_CONFIG_LAST_RELOAD_SUCCESS_TIMESTAMP: &str = pub const M_CONFIG_RELOADS_TOTAL: &str = "aisix_config_reloads_total"; pub const M_CONFIG_RELOAD_FAILURES_TOTAL: &str = "aisix_config_reload_failures_total"; pub const M_CONFIG_REJECTED_RESOURCES: &str = "aisix_config_rejected_resources"; +/// Served resources per kind carrying fields this gateway version does not +/// know (loaded with those fields ignored — partially compatible, #871). +/// Non-zero typically means a newer control plane is writing ahead of this +/// data plane's rollout. +pub const M_CONFIG_PARTIALLY_COMPATIBLE_RESOURCES: &str = + "aisix_config_partially_compatible_resources"; pub const M_CONFIG_OBSERVED_REVISION: &str = "aisix_config_observed_revision"; pub const M_CONFIG_APPLIED_REVISION: &str = "aisix_config_applied_revision"; pub const M_CONFIG_HASH_INFO: &str = "aisix_config_hash_info"; @@ -343,6 +349,7 @@ struct MetricsInner { struct ConfigLabelState { last_hash: Option, last_rejected_kinds: std::collections::HashSet, + last_partial_kinds: std::collections::HashSet, } const REQUEST_SERIES_CACHE_CAPACITY: usize = 1024; @@ -693,6 +700,21 @@ impl Metrics { .set(*count as f64); } labels.last_rejected_kinds = view.rejected_by_kind.keys().cloned().collect(); + + // Partially-compatible gauge per kind, same zeroing discipline. + // Per-field detail deliberately stays off the labels (field paths + // are not a bounded set) — it lives in `/status/config`. + for kind in &labels.last_partial_kinds { + if !view.partially_compatible_by_kind.contains_key(kind) { + metrics::gauge!(M_CONFIG_PARTIALLY_COMPATIBLE_RESOURCES, "kind" => kind.clone()) + .set(0.0); + } + } + for (kind, count) in &view.partially_compatible_by_kind { + metrics::gauge!(M_CONFIG_PARTIALLY_COMPATIBLE_RESOURCES, "kind" => kind.clone()) + .set(*count as f64); + } + labels.last_partial_kinds = view.partially_compatible_by_kind.keys().cloned().collect(); }); } @@ -3027,6 +3049,7 @@ mod tests { reloads_total: 3, reload_failures: std::collections::BTreeMap::new(), rejected_by_kind: std::collections::BTreeMap::new(), + partially_compatible_by_kind: std::collections::BTreeMap::new(), observed_revision: Some(42), applied_revision: Some(42), config_hash: Some("abc123".into()), @@ -3041,6 +3064,8 @@ mod tests { view.last_reload_successful = false; view.reload_failures.insert("validate", 2); view.rejected_by_kind.insert("models".to_string(), 1); + view.partially_compatible_by_kind + .insert("api_keys".to_string(), 3); m.sync_config_status(&view); let out = m.render(); @@ -3053,6 +3078,9 @@ mod tests { assert!(out.contains(&format!( "{M_CONFIG_REJECTED_RESOURCES}{{kind=\"models\"}} 1" ))); + assert!(out.contains(&format!( + "{M_CONFIG_PARTIALLY_COMPATIBLE_RESOURCES}{{kind=\"api_keys\"}} 3" + ))); assert!(out.contains(&format!("{M_CONFIG_OBSERVED_REVISION} 42"))); assert!(out.contains(&format!("{M_CONFIG_APPLIED_REVISION} 42"))); assert!(out.contains(&format!("{M_CONFIG_HASH_INFO}{{hash=\"abc123\"}} 1"))); @@ -3080,6 +3108,9 @@ mod tests { let mut first = config_metrics_view(aisix_core::SourceKind::Etcd); first.config_hash = Some("hash-A".into()); first.rejected_by_kind.insert("models".to_string(), 2); + first + .partially_compatible_by_kind + .insert("api_keys".to_string(), 1); m.sync_config_status(&first); // The applied config changes and the models rejection clears. @@ -3096,6 +3127,10 @@ mod tests { assert!(out.contains(&format!( "{M_CONFIG_REJECTED_RESOURCES}{{kind=\"models\"}} 0" ))); + // Same zeroing discipline for the partially-compatible gauge. + assert!(out.contains(&format!( + "{M_CONFIG_PARTIALLY_COMPATIBLE_RESOURCES}{{kind=\"api_keys\"}} 0" + ))); } /// Issue #408 audit MEDIUM-2: pin every boundary of diff --git a/crates/aisix-server/src/heartbeat.rs b/crates/aisix-server/src/heartbeat.rs index da115b19..e37323f2 100644 --- a/crates/aisix-server/src/heartbeat.rs +++ b/crates/aisix-server/src/heartbeat.rs @@ -31,7 +31,7 @@ use std::sync::Arc; use std::sync::LazyLock; use std::time::{Duration, Instant}; -use aisix_etcd::loader::RejectedEntry; +use aisix_etcd::loader::{PartialCompatEntry, RejectedEntry}; use aisix_obs::SinkStatsSnapshot; use anyhow::{anyhow, Context}; use serde::Serialize; @@ -94,6 +94,14 @@ pub type ExporterHealthFetcher = Arc HashMap Option + Send + Sync>; +/// Per-tick source of the supervisor's partially-compatible aggregate: +/// resources served with unknown fields ignored, per (kind, field) with +/// row counts (#871, `Supervisor::recent_partial_compat`). Reported as +/// `partially_compatible_resources` so cp-api can tell an operator +/// "these DPs load your documents but do not enforce field X" — the +/// signal that a fleet is mid-rollout behind a newer control plane. +pub type PartialCompatFetcher = Arc Vec + Send + Sync>; + /// File paths to the on-disk mTLS bundle the heartbeat client presents /// to cp-api. Same three files written by cert-bundle provisioning and /// re-used on every subsequent boot when the bundle is already on disk. @@ -150,6 +158,10 @@ pub struct HeartbeatConfig { /// `config_hash` from the body — cp-api tolerates its absence. See /// #774. pub config_hash_fetcher: Option, + /// Optional source of the supervisor's partially-compatible + /// aggregate. `None` (tests / no supervisor) omits the field, same + /// tolerance contract as the other optional fields. See #871. + pub partial_compat_fetcher: Option, } impl std::fmt::Debug for HeartbeatConfig { @@ -177,6 +189,10 @@ impl std::fmt::Debug for HeartbeatConfig { "config_hash_fetcher", &self.config_hash_fetcher.as_ref().map(|_| ""), ) + .field( + "partial_compat_fetcher", + &self.partial_compat_fetcher.as_ref().map(|_| ""), + ) .finish() } } @@ -204,6 +220,7 @@ impl HeartbeatConfig { applied_revision_fetcher: None, exporter_health_fetcher: None, config_hash_fetcher: None, + partial_compat_fetcher: None, } } @@ -231,6 +248,12 @@ impl HeartbeatConfig { self.config_hash_fetcher = Some(fetcher); self } + + /// Wire the supervisor's partially-compatible aggregate source (#871). + pub fn with_partial_compat_fetcher(mut self, fetcher: PartialCompatFetcher) -> Self { + self.partial_compat_fetcher = Some(fetcher); + self + } } /// Spawn the heartbeat worker. Returns the JoinHandle so `main` can @@ -329,6 +352,13 @@ struct HeartbeatBody<'a> { /// historical body shape. See issue #115. #[serde(skip_serializing_if = "Vec::is_empty")] rejected_resources: Vec, + /// Resources served with unknown fields ignored, aggregated per + /// (kind, field) with row counts (#871). Omitted when empty for the + /// same historical-shape tolerance as `rejected_resources`; cp-api's + /// heartbeat handler accepts unknown fields, so an older CP simply + /// ignores this until AISIX-Cloud#1227 surfaces it. + #[serde(skip_serializing_if = "Vec::is_empty")] + partially_compatible_resources: Vec, } /// Cap on the `last_error` excerpt forwarded per exporter. The pipeline @@ -396,6 +426,29 @@ impl From<&RejectedEntry> for RejectedResourceWire { } } +/// On-the-wire shape of one partially-compatible aggregate entry (#871). +/// Kept separate from `aisix_etcd::loader::PartialCompatEntry` so the +/// loader's internal representation can evolve without a wire bump. +/// `kind` is the plural resource kind; `field` a dotted document path +/// with array indices normalized to `[]`; `count` the number of served +/// rows of that kind carrying the field. +#[derive(Debug, Serialize)] +struct PartialCompatWire { + kind: String, + field: String, + count: usize, +} + +impl From for PartialCompatWire { + fn from(e: PartialCompatEntry) -> Self { + Self { + kind: e.kind, + field: e.field, + count: e.count, + } + } +} + async fn send(client: &reqwest::Client, cfg: &HeartbeatConfig, uptime: i64) -> anyhow::Result<()> { let rejections: Vec = cfg .rejection_fetcher @@ -426,6 +479,12 @@ async fn send(client: &reqwest::Client, cfg: &HeartbeatConfig, uptime: i64) -> a .unwrap_or_default(); // Deterministic order — the fetcher hands back a HashMap. exporter_health.sort_by(|a, b| a.name.cmp(&b.name)); + // Already (kind, field)-sorted by the supervisor's aggregation. + let partially_compatible_resources: Vec = cfg + .partial_compat_fetcher + .as_ref() + .map(|fetcher| fetcher().into_iter().map(PartialCompatWire::from).collect()) + .unwrap_or_default(); let resp = client .post(&cfg.url) @@ -442,6 +501,7 @@ async fn send(client: &reqwest::Client, cfg: &HeartbeatConfig, uptime: i64) -> a config_hash, exporter_health, rejected_resources: rejections, + partially_compatible_resources, }) .send() .await @@ -725,6 +785,45 @@ mod tests { body.get("rejected_resources").is_none(), "empty rejected_resources must stay off the wire", ); + // Same for the partially-compatible aggregate: unwired fetcher + // (or an empty aggregate) keeps the historical body shape. + assert!( + body.get("partially_compatible_resources").is_none(), + "empty partially_compatible_resources must stay off the wire", + ); + } + + #[tokio::test] + async fn send_includes_partially_compatible_resources_when_wired() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/dp/heartbeat")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "ok": true + }))) + .mount(&server) + .await; + + let dir = tempfile::tempdir().unwrap(); + let mtls = write_test_bundle(dir.path()); + let cfg = cfg_with_bundle(format!("{}/dp/heartbeat", server.uri()), mtls) + .with_partial_compat_fetcher(Arc::new(|| { + vec![aisix_etcd::loader::PartialCompatEntry { + kind: "api_keys".into(), + field: "quota_profile".into(), + count: 3, + }] + })); + send(&plain_client(), &cfg, 7).await.unwrap(); + + let received = server.received_requests().await.unwrap(); + let body: serde_json::Value = serde_json::from_slice(&received[0].body).unwrap(); + assert_eq!( + body["partially_compatible_resources"], + serde_json::json!([ + {"kind": "api_keys", "field": "quota_profile", "count": 3} + ]), + ); } /// Without fetchers (tests / no supervisor), the new fields still diff --git a/crates/aisix-server/src/main.rs b/crates/aisix-server/src/main.rs index a46f6efa..322230c0 100644 --- a/crates/aisix-server/src/main.rs +++ b/crates/aisix-server/src/main.rs @@ -804,6 +804,10 @@ async fn run(mut cfg: Config) -> anyhow::Result<()> { h = h.with_config_hash_fetcher(Arc::new(move || { config_status_for_heartbeat.applied_config_hash() })); + let supervisor_for_partial = Arc::clone(supervisor); + h = h.with_partial_compat_fetcher(Arc::new(move || { + supervisor_for_partial.recent_partial_compat() + })); let fan_out = proxy_state.otlp_fan_out.clone(); h = h.with_exporter_health_fetcher(Arc::new(move || fan_out.exporter_stats())); heartbeat::spawn(h, cancel_rx.clone()) diff --git a/schemas/README.md b/schemas/README.md index 0d8c2ec7..b3d43885 100644 --- a/schemas/README.md +++ b/schemas/README.md @@ -34,22 +34,36 @@ etcd key prefix uses the plural `Resource::kind()` value deliberately distinct because the schema file is a per-type artifact while the etcd prefix groups a collection of instances. -## Forward-compatibility +## Strictness: these files describe the write contract + +The published schemas carry the **write contract**: the Admin API rejects +a payload that fails them (including unknown fields, where a resource +closes them) with a 400. They are generated from the same producers the +write-path validators compile, so the published and enforced shapes +cannot drift. + +The gateway's **etcd read path is deliberately more lenient** (#871): a +stored document carrying fields outside these schemas still loads, with +the unknown fields ignored and reported as partially compatible on +`GET /status/config`, the heartbeat, and the +`aisix_config_partially_compatible_resources` metric. This keeps an +older gateway serving documents written by a newer control plane. Every +other constraint in these files — types, required fields, ranges, closed +enum value sets — applies on both paths. Three top-level resources intentionally **omit** -`additionalProperties: false`: +`additionalProperties: false` even on the write contract: - `guardrail.schema.json` — the discriminated-union `kind` field uses serde's `flatten + tag` pattern, which is incompatible with a strict outer deny; strict typo-rejection happens earlier via `aisix-core::models::schema::validate_guardrail`. -- `cache_policy.schema.json` — cp-api may ship forward-compat fields - ahead of a DP rollout, e.g. a new backend variant. -- `observability_exporter.schema.json` — same forward-compat reason as - `cache_policy`. - -Downstream consumers that default to strict validation should permit -unknown keys for these three resources; the other six are strict. +- `cache_policy.schema.json` — historically open on write as well. +- `observability_exporter.schema.json` — the top level is open, but the + per-`kind` branches stay closed on both paths: an unknown field there + could smuggle a plaintext credential past the `credential_ref` + indirection, and serde cannot report ignored fields inside the + tagged union, so an open branch would be a silent tolerance. ## Regenerating diff --git a/schemas/resources/guardrail_attachment.schema.json b/schemas/resources/guardrail_attachment.schema.json index 57d7bd99..60b4f3c4 100644 --- a/schemas/resources/guardrail_attachment.schema.json +++ b/schemas/resources/guardrail_attachment.schema.json @@ -19,6 +19,13 @@ "description": "When `false`, AISIX ignores this attachment.", "type": "boolean" }, + "env_id": { + "description": "Environment the attachment belongs to. Written by the managed control plane for its own scoping; the gateway does not read it.", + "type": [ + "string", + "null" + ] + }, "guardrail_id": { "description": "UUID of the guardrail definition this attachment points to.", "minLength": 1, diff --git a/schemas/resources/model.schema.json b/schemas/resources/model.schema.json index 438fa70e..219de767 100644 --- a/schemas/resources/model.schema.json +++ b/schemas/resources/model.schema.json @@ -290,6 +290,7 @@ "description": "`\"default\"` or `\"fail\"`." }, { + "additionalProperties": false, "description": "`{ \"target\": \"\" }` — route to a specific safe model.", "properties": { "target": { diff --git a/schemas/resources/semantic.schema.json b/schemas/resources/semantic.schema.json index bb05fc51..b76cf863 100644 --- a/schemas/resources/semantic.schema.json +++ b/schemas/resources/semantic.schema.json @@ -122,7 +122,8 @@ "type": "string", "minLength": 1 } - } + }, + "additionalProperties": false } ] }, diff --git a/tests/e2e/src/cases/config-forward-compat-e2e.test.ts b/tests/e2e/src/cases/config-forward-compat-e2e.test.ts new file mode 100644 index 00000000..f4587d53 --- /dev/null +++ b/tests/e2e/src/cases/config-forward-compat-e2e.test.ts @@ -0,0 +1,266 @@ +import { createHash, randomUUID } from "node:crypto"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + EtcdClient, + ProxyClient, + SeedClient, + spawnApp, + startOpenAiUpstream, + waitConfigPropagation, + type OpenAiUpstream, + type SpawnedApp, +} from "../harness/index.js"; + +// E2E for config forward compatibility (issue #871). Under the supported +// rolling-upgrade order the control plane upgrades first and may write +// resource documents carrying fields this data-plane version does not +// know. The observable contract: +// +// - such a document LOADS and behaves (an api_key authenticates real +// traffic) with the unknown fields ignored — not whole-row rejected; +// - the tolerance is never silent: `GET /status/config` reports the +// ignored fields as `partially_compatible[]` next to `rejected[]`, +// and the metrics listener exposes a per-kind gauge; +// - a converged same-version deployment (documents written by this +// version's own canonical shapes) reports ZERO partially-compatible +// rows — the strictness that catches typos is preserved; +// - the Admin API write path still rejects unknown fields with 400. + +const CALLER_PLAINTEXT = "sk-forward-compat-caller"; +const CALLER_KEY_HASH = createHash("sha256").update(CALLER_PLAINTEXT).digest("hex"); + +interface StatusConfig { + state: string; + applied?: { resource_counts: Record }; + rejected: Array<{ resource_kind: string; resource_id: string }>; + partially_compatible: Array<{ + resource_kind: string; + field: string; + count: number; + }>; +} + +async function getStatusConfig(app: SpawnedApp): Promise { + const res = await fetch(`${app.metricsUrl}/status/config`); + expect(res.status).toBe(200); + return (await res.json()) as StatusConfig; +} + +async function scrape(app: SpawnedApp): Promise { + const res = await fetch(`${app.metricsUrl}/metrics`); + expect(res.status).toBe(200); + return res.text(); +} + +describe("config forward-compat: unknown fields from a newer control plane", () => { + let app: SpawnedApp | undefined; + let upstream: OpenAiUpstream | undefined; + let etcd: EtcdClient | undefined; + let etcdReachable = false; + let yellowKeyId: string; + let pkId: string; + + beforeAll(async () => { + etcd = new EtcdClient(); + etcdReachable = await etcd.ping(); + if (!etcdReachable) return; + + upstream = await startOpenAiUpstream(); + app = await spawnApp({ admin: true }); + const seed = new SeedClient(etcd, app.etcdPrefix); + + const pk = await seed.createProviderKey({ + display_name: "fc-pk", + secret: "sk-mock", + api_base: `${upstream.baseUrl}/v1`, + }); + pkId = pk.id; + await seed.createModel({ + display_name: "fc-model", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: pk.id, + }); + }); + + afterAll(async () => { + await app?.exit(); + await upstream?.close(); + }); + + test("an api_key document with an unknown field authenticates and is reported partially compatible", async (ctx) => { + if (!etcdReachable || !app || !etcd) { + ctx.skip(); + return; + } + + // A document as a newer CP would write it: canonical api_key fields + // plus one this DP version has never heard of. + yellowKeyId = randomUUID(); + await etcd.put( + `${app.etcdPrefix}/api_keys/${yellowKeyId}`, + JSON.stringify({ + key_hash: CALLER_KEY_HASH, + allowed_models: ["fc-model"], + quota_profile: "gold", + }), + ); + + let cfg: StatusConfig | undefined; + await waitConfigPropagation(async () => { + cfg = await getStatusConfig(app!); + return (cfg.applied?.resource_counts.api_keys ?? 0) >= 1; + }); + + // The credential WORKS — the user journey the strict reader broke: + // pre-#871 this row was whole-row rejected and the key 401'd + // identically to "no such key". + const proxy = new ProxyClient(app.proxyUrl, CALLER_PLAINTEXT); + const chat = await proxy.chat({ + model: "fc-model", + messages: [{ role: "user", content: "does the forward-compat key work?" }], + }); + expect(chat.status, JSON.stringify(chat.body)).toBe(200); + + // A second traffic-bearing kind: a model document with an unknown + // field must also load and serve chat. + const yellowModelId = randomUUID(); + await etcd.put( + `${app.etcdPrefix}/models/${yellowModelId}`, + JSON.stringify({ + display_name: "fc-model-yellow", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: pkId, + future_model_knob: true, + }), + ); + await etcd.put( + `${app.etcdPrefix}/api_keys/${randomUUID()}`, + JSON.stringify({ + key_hash: createHash("sha256").update(`${CALLER_PLAINTEXT}-2`).digest("hex"), + allowed_models: ["fc-model-yellow"], + }), + ); + await waitConfigPropagation(async () => { + cfg = await getStatusConfig(app!); + return (cfg.applied?.resource_counts.models ?? 0) >= 2; + }); + const proxy2 = new ProxyClient(app.proxyUrl, `${CALLER_PLAINTEXT}-2`); + const chat2 = await proxy2.chat({ + model: "fc-model-yellow", + messages: [{ role: "user", content: "does the forward-compat model serve?" }], + }); + expect(chat2.status, JSON.stringify(chat2.body)).toBe(200); + await etcd.delete(`${app.etcdPrefix}/models/${yellowModelId}`); + + // The tolerance is reported, not silent: the exact ignored field with + // a row count, next to an empty rejected[]. The row is served, so the + // state stays synced rather than degraded. + cfg = await getStatusConfig(app); + expect(cfg.state).toBe("synced"); + expect(cfg.rejected).toHaveLength(0); + expect(cfg.partially_compatible).toContainEqual({ + resource_kind: "api_keys", + field: "quota_profile", + count: 1, + }); + + // And on the metrics listener as a per-kind gauge. + const text = await scrape(app); + expect(text).toMatch( + /aisix_config_partially_compatible_resources\{kind="api_keys"\} 1/, + ); + }); + + test("a value the gateway cannot interpret stays rejected (unknown enum value)", async (ctx) => { + if (!etcdReachable || !app || !etcd) { + ctx.skip(); + return; + } + + // An unknown VALUE has no lenient fallback — there is no old behavior + // to run for a routing strategy this version cannot interpret. The + // row must reject (RED), not load partially. + const badId = randomUUID(); + await etcd.put( + `${app.etcdPrefix}/models/${badId}`, + JSON.stringify({ + display_name: "fc-router", + routing: { + strategy: "strategy-from-the-future", + targets: [{ model: "fc-model" }], + }, + }), + ); + + let cfg: StatusConfig | undefined; + await waitConfigPropagation(async () => { + cfg = await getStatusConfig(app!); + return cfg.rejected.some((r) => r.resource_id === badId); + }); + expect(cfg!.state).toBe("degraded"); + expect(cfg!.rejected.find((r) => r.resource_id === badId)!.resource_kind).toBe( + "models", + ); + + await etcd.delete(`${app.etcdPrefix}/models/${badId}`); + }); + + test("deleting the forward-compat row clears the report; converged config has zero partially-compatible rows", async (ctx) => { + if (!etcdReachable || !app || !etcd) { + ctx.skip(); + return; + } + + await etcd.delete(`${app.etcdPrefix}/api_keys/${yellowKeyId}`); + + // Zero-YELLOW invariant at equal versions: every remaining document + // was written through this version's own canonical shapes + // (SeedClient), so nothing may report as partially compatible. A + // failure here means the seed shapes and the DP models drifted — + // exactly the typo class the old strictness caught. + let cfg: StatusConfig | undefined; + await waitConfigPropagation(async () => { + cfg = await getStatusConfig(app!); + return cfg.partially_compatible.length === 0 && cfg.state === "synced"; + }); + expect(cfg!.applied?.resource_counts.models).toBe(1); + expect(cfg!.applied?.resource_counts.provider_keys).toBe(1); + expect(cfg!.rejected).toHaveLength(0); + + // The gauge zeroes rather than lingering at its stale value. + const text = await scrape(app); + expect(text).toMatch( + /aisix_config_partially_compatible_resources\{kind="api_keys"\} 0/, + ); + }); + + test("the Admin API write path still rejects unknown fields with 400", async (ctx) => { + if (!etcdReachable || !app) { + ctx.skip(); + return; + } + + // Strict write / lenient read: leniency is a property of the etcd + // READ path only. A human (or script) writing through the Admin API + // gets typo protection unchanged. + const res = await fetch(`${app.adminUrl}/admin/v1/models`, { + method: "POST", + headers: { + authorization: `Bearer ${app.adminKey}`, + "content-type": "application/json", + }, + body: JSON.stringify({ + display_name: "fc-typo", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: randomUUID(), + dispaly_name_typo: true, + }), + }); + expect(res.status).toBe(400); + const body = (await res.json()) as { error_msg?: string }; + expect(body.error_msg ?? "").toMatch(/schema validation|unknown field/i); + }); +});