Skip to content

feat(core): Model, ApiKey, RateLimit entities + JSON Schema validators - #3

Merged
moonming merged 1 commit into
mainfrom
feat/data-models
Apr 17, 2026
Merged

feat(core): Model, ApiKey, RateLimit entities + JSON Schema validators#3
moonming merged 1 commit into
mainfrom
feat/data-models

Conversation

@moonming

Copy link
Copy Markdown
Collaborator

Summary

Adds the three entities from spec §3 that the admin plane writes and
the data plane reads on every request, alongside their Draft 2020-12
JSON Schemas and the composite `AisixSnapshot`.

  • Model — `/` routing target. `Provider` enum
    (openai/anthropic/gemini/deepseek) with default base URLs;
    `ProviderConfig` supports an `api_base` override.
  • ApiKey — bearer credential with `allowed_models` whitelist.
    Empty array denies all per spec §3 authz rule.
  • RateLimit — tpm / tpd / rpm / rpd / concurrency, all optional.
  • schema.rs — validators compiled once via `Lazy<Arc>`,
    reused on admin write and etcd watch read paths. `SchemaError`
    collapses to the first failing JSON pointer.
  • AisixSnapshot — `ResourceTable` + `ResourceTable`;
    the atomic-swap target for the future watch supervisor.

Reuses the generic `ResourceTable` / `SnapshotHandle` primitives that
landed in #2.

Test plan

  • `cargo test -p aisix-core` — 42 tests pass
  • `cargo clippy -p aisix-core --all-targets -- -D warnings` clean
  • `cargo fmt --check` clean
  • CI green across all 6 jobs

Adds the three entities introduced in spec §3 alongside their Draft
2020-12 JSON Schemas and the composite `AisixSnapshot` that the data
plane reads from.

- `Model`: `<provider>/<upstream>` routing target with ProviderConfig,
  optional timeout and rate_limit. `Provider` enum covers openai /
  anthropic / gemini / deepseek with default base URLs.
- `ApiKey`: bearer credential with `allowed_models` whitelist. Empty
  array denies all (spec §3 authz rule).
- `RateLimit`: tpm/tpd/rpm/rpd/concurrency, all optional.
- `schema.rs`: validators compiled once via `Lazy<Arc<Schemas>>`,
  reused on admin write path and etcd watch read path. `SchemaError`
  collapses to the first failing JSON pointer.
- `AisixSnapshot`: `ResourceTable<Model>` + `ResourceTable<ApiKey>`,
  atomic-swap target for the watch supervisor.

42 unit tests covering serde round-trip, schema happy/unhappy paths,
provider prefix regex, deny-all-empty authz, Resource trait wiring,
unknown-field rejection, rate-limit negative rejection.
Copilot AI review requested due to automatic review settings April 17, 2026 05:12
@moonming
moonming merged commit fe7646f into main Apr 17, 2026
10 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the first concrete etcd-backed domain entities to aisix-core (Model, ApiKey, RateLimit) along with JSON Schema validation helpers and a composite AisixSnapshot that will be atomically swapped by a future watch supervisor.

Changes:

  • Introduces Model, ApiKey, RateLimit entities implementing the Resource trait (with snapshot-ready runtime ids).
  • Adds Draft 2020-12 JSON Schema validators compiled once and reused across admin write + watch read paths.
  • Defines AisixSnapshot as the concrete snapshot shape (tables for Model and ApiKey) and re-exports the new APIs from aisix-core.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
crates/aisix-core/src/models/snapshot.rs Adds AisixSnapshot composite snapshot + basic tests
crates/aisix-core/src/models/schema.rs Adds compiled JSON Schema validators + tests
crates/aisix-core/src/models/rate_limit.rs Adds shared RateLimit struct + tests
crates/aisix-core/src/models/model.rs Adds Model/Provider/ProviderConfig + helper methods + tests
crates/aisix-core/src/models/apikey.rs Adds ApiKey + authz helper + tests
crates/aisix-core/src/models/mod.rs Wires up the new models module and re-exports
crates/aisix-core/src/lib.rs Exposes models module and re-exports public types/helpers
crates/aisix-core/Cargo.toml Adds dependencies needed for schema/regex validation
Cargo.lock Locks new transitive dependencies introduced by jsonschema/regex/etc.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +70 to +73
/// Request timeout in ms. 0 or absent = no timeout.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub timeout: Option<u64>,

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field docs say “0 or absent = no timeout”, but with timeout: Option<u64> deserializing "timeout": 0 results in Some(0), not None. If callers forget the special-case, this can be misinterpreted as an immediate timeout. Consider encoding the spec rule in the type (e.g., Option<NonZeroU64> with custom serde mapping 0→None) or providing a helper like effective_timeout() that returns None when the value is 0.

Copilot uses AI. Check for mistakes.
tracing.workspace = true
config.workspace = true
humantime-serde.workspace = true
jsonschema.workspace = true

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding jsonschema to aisix-core pulls in reqwest transitively (visible in Cargo.lock under jsonschema dependencies). That conflicts with the crate-level intent documented in crates/aisix-core/src/lib.rs (“no … reqwest”) and increases the core crate’s dependency surface (TLS/HTTP client, tokio, etc.). Consider either (a) configuring the jsonschema dependency to avoid HTTP-resolution/client features if possible, (b) gating schema validation behind a feature, or (c) moving schema validation to a higher-level crate so aisix-core stays framework-agnostic as documented.

Suggested change
jsonschema.workspace = true
jsonschema = { workspace = true, default-features = false }

Copilot uses AI. Check for mistakes.
Comment on lines +55 to +61
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
#[serde(deny_unknown_fields)]
pub struct ProviderConfig {
pub api_key: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub api_base: Option<String>,
}

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProviderConfig contains an upstream api_key secret but derives Debug. Any logging/panic that prints this struct (or a parent Model) will leak credentials into logs. Consider using a secret wrapper type (e.g., secrecy::SecretString) and/or implementing a custom Debug that redacts the key.

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +18
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct ApiKey {
/// The actual secret bearer value. Secondary-indexed for O(1) auth.
pub key: String,

/// Whitelisted Model names. An **empty array** denies every model —
/// it is not a shortcut for "all" (spec §3 authz rule).
pub allowed_models: Vec<String>,

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApiKey holds the bearer secret in key but derives Debug. This makes it easy to accidentally leak API keys into logs (including via Debug on ResourceEntry<ApiKey> / snapshots). Consider wrapping the key in a redacting secret type and/or implementing a custom Debug impl that omits/redacts the secret.

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +20
/// Composite of every typed [`ResourceTable`] the gateway reads on the hot
/// path. Cheap to construct empty; populated by the loader.
#[derive(Debug, Default)]
pub struct AisixSnapshot {
pub models: ResourceTable<Model>,
pub apikeys: ResourceTable<ApiKey>,
}

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AisixSnapshot derives Debug but will contain Model / ApiKey values (including secrets like upstream api_key and bearer keys). If a snapshot is ever logged with {:?}, it can leak credentials. Consider removing Debug here or providing a redacted debug representation for snapshot/table contents.

Copilot uses AI. Check for mistakes.
Comment on lines +79 to +83
"name": { "type": "string", "minLength": 1 },
"model": {
"type": "string",
"pattern": "^(anthropic|deepseek|gemini|openai)/.+$"
},

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model-id validation regex is duplicated: Model uses MODEL_ID_RE while the JSON Schema hard-codes the same pattern string. Keeping these in sync over time is error-prone (e.g., adding a provider would require updating both). Consider defining a single shared constant for the pattern (or deriving the schema pattern from the Provider enum) so both runtime checks and schema validation stay consistent.

Copilot uses AI. Check for mistakes.
@moonming
moonming deleted the feat/data-models branch April 17, 2026 05:16
moonming added a commit that referenced this pull request Apr 17, 2026
Wires the 10-step startup sequence from spec §1 into the aisix binary:

1. Parse CLI args (--config, with AISIX_CONFIG env fallback)
2. Load + validate bootstrap config
3. init_tracing — aisix-obs now installs an EnvFilter-backed subscriber
4. EtcdConfigProvider::connect (5s × 5 retry, reused from PR #3)
5. Initial snapshot load via Supervisor::load_once
6. Spawn Supervisor::run on a dedicated task (cancel via watch channel)
7. Build proxy router (aisix-proxy now exposes build_router + ProxyState)
8. Build admin router (aisix-admin now exposes build_router + AdminState)
9. Bind + serve both listeners with axum::serve + graceful shutdown
10. SIGINT/SIGTERM → flip cancel channel → drain both serves → join supervisor

For now both routers only mount /health so the startup wiring is
observable without reaching into feature code that hasn't landed yet.
The health handler reports the current snapshot table sizes so the
bootstrap path is end-to-end verifiable.

Ancillary changes:
- ObsError with Filter + AlreadyInitialised variants (thiserror)
- aisix-obs drops #![forbid(unsafe_code)] so test code can use the
  now-unsafe std::env::remove_var without a lint override
- ProxyState / AdminState are Clone and cheap (Arc-backed handles and
  Arc<[String]> admin keys)

6 new unit tests (CLI parsing, ObsError display, proxy health JSON
shape, admin health JSON shape, admin-keys Arc sharing) land on top of
the existing 68 — 75 passing workspace-wide.
moonming added a commit that referenced this pull request Apr 17, 2026
)

Wires the 10-step startup sequence from spec §1 into the aisix binary:

1. Parse CLI args (--config, with AISIX_CONFIG env fallback)
2. Load + validate bootstrap config
3. init_tracing — aisix-obs now installs an EnvFilter-backed subscriber
4. EtcdConfigProvider::connect (5s × 5 retry, reused from PR #3)
5. Initial snapshot load via Supervisor::load_once
6. Spawn Supervisor::run on a dedicated task (cancel via watch channel)
7. Build proxy router (aisix-proxy now exposes build_router + ProxyState)
8. Build admin router (aisix-admin now exposes build_router + AdminState)
9. Bind + serve both listeners with axum::serve + graceful shutdown
10. SIGINT/SIGTERM → flip cancel channel → drain both serves → join supervisor

For now both routers only mount /health so the startup wiring is
observable without reaching into feature code that hasn't landed yet.
The health handler reports the current snapshot table sizes so the
bootstrap path is end-to-end verifiable.

Ancillary changes:
- ObsError with Filter + AlreadyInitialised variants (thiserror)
- aisix-obs drops #![forbid(unsafe_code)] so test code can use the
  now-unsafe std::env::remove_var without a lint override
- ProxyState / AdminState are Clone and cheap (Arc-backed handles and
  Arc<[String]> admin keys)

6 new unit tests (CLI parsing, ObsError display, proxy health JSON
shape, admin health JSON shape, admin-keys Arc sharing) land on top of
the existing 68 — 75 passing workspace-wide.
moonming added a commit that referenced this pull request May 18, 2026
…ped reads, redact 5xx message, Vertex content-type guard

Five concrete fixes from the Copilot inline review on PR #323. Two
stale comments (#3, #4 — already fixed in commit 3) are skipped.

**#1+#7 — Azure OpenAI-compatible code preservation.**
Azure's envelope omits `error.type` and carries only `error.code`.
The bridge previously put the upstream code into `view.kind` and
left `view.code` as `None`. For OpenAI-compat tokens Azure inherits
unchanged (e.g. `rate_limit_exceeded`), this meant downstream OpenAI
clients received `error.type=rate_limit_exceeded` but
`error.code=null` — exactly the SDK-retry break issue #322 is about.

Fix:
- Azure parser populates BOTH `view.kind` AND `view.code` from the
  upstream `error.code` field.
- `render_openai_envelope`'s AzureOpenAI branch now prefers the
  translation-table-derived code (so explicit Azure tokens like
  `DeploymentNotFound` → `model_not_found` still win), falling back
  to `view.code` for OpenAI-compat pass-through.

**#2 — Drain the response stream after hitting the cap.**
`read_body_capped` previously broke out of the read loop the moment
`limit` bytes were buffered. With reqwest/hyper that leaves unread
bytes in the response and prevents connection reuse — during a burst
of upstream errors the gateway would churn TCP connections instead
of recycling the keep-alive pool. Fix: keep iterating the stream,
discarding chunks past the cap. Memory stays bounded by `limit`.

**#5 — Redact upstream `error.message` on 5xx.**
The 5xx branch of `render_bridge_upstream_envelope` was forwarding
`BridgeError::UpstreamStatus.message` verbatim — which for OpenAI /
Anthropic comes from the parsed upstream `error.message`. Upstream
5xx bodies routinely embed operator-internal detail (engine names,
shard ids, queue depth). Fix: on 5xx, emit a canned
`"upstream returned {status}"` message; the full upstream body
remains in operator logs via tracing.

**#6 — Stale "follow-up" comment.**
The docstring on `render_bridge_upstream_envelope` claimed cross-wire
translation would ship in a follow-up, but it already shipped in
commit 2. Rewrite the comment to describe current behaviour
(4xx → `error_translate`; 5xx → canned envelope; `Unknown` wire →
legacy generic envelope).

**#8 — Content-type guard on Vertex (and Azure, while at it).**
`capture_upstream_error_http` already gates serde parsing on
`Content-Type: application/json` so a 64 KB HTML error page from a
fronting WAF doesn't waste CPU on a doomed JSON parse. The Vertex
and Azure bridges call serde directly because they need a custom
parse path (canned message for redaction) — same guard now applies.
Promoted `content_type_is_json` and added a `response_is_json`
helper to the gateway's public surface; both bridges call it before
`parse_*_error_*`.

New tests:
- `upstream_openai_5xx_with_json_envelope_collapses_and_redacts_message`
  pins the 5xx redaction (asserts `engine offline` / `shard 47` /
  `engine_overloaded` don't reach the customer envelope).
- `chat_429_preserves_openai_compatible_code_for_sdk_retry` (Azure)
  pins that `parsed.code` carries the OpenAI-compat upstream code.
- `chat_400_non_json_body_skips_envelope_parse` (Azure) and
  `chat_gemini_non_json_body_skips_envelope_parse` (Vertex) pin the
  new content-type guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
moonming added a commit that referenced this pull request May 22, 2026
…dor consts

Round-3 audit follow-up + scope expansion confirmed by user. Cuts the
last remnants of the pre-#302 vendor-enumeration design that the prior
clean-cut PR left as soft-deprecated:

#1 Provider enum
   - Delete the enum + its Adapter mapping + the
     `every_provider_variant_has_as_str_and_adapter` test.
   - Drop the `Provider` re-export from `aisix_core::{lib, models}`.

#2 OpenAiBridge::with_name + `name` field
   - Drop the builder method and the per-instance `name` field; the
     bridge is the singular OpenAI family bridge and reports `"openai"`
     unconditionally.
   - Same surgery on AnthropicBridge for symmetry (its `with_name`
     was unused at every call site).

#3 Per-vendor default-base consts + match arms
   - Delete DEEPSEEK_DEFAULT_BASE, GOOGLE_DEFAULT_BASE, COHERE_DEFAULT_BASE
     plus the 11 long-tail consts (GROQ/MISTRAL/TOGETHERAI/FIREWORKS/
     PERPLEXITY/MOONSHOTAI/ALIBABA/ZHIPUAI/BASETEN/HUGGINGFACE/CEREBRAS).
   - Replace OpenAiBridge::default_base() with the hardcoded OpenAI
     bare-host fallback inside resolve_base; the safety guard now
     returns a Config error for any non-`"openai"` vendor whose PK
     reaches the bridge with an empty api_base, preventing a silent
     credential leak to api.openai.com.

#4 normalize_canonical_deepseek + normalize_canonical_cohere
   - Delete both helpers + DEEPSEEK_CANONICAL_HOSTS / COHERE_CANONICAL_HOSTS.
   - normalize_api_base loses its `provider` parameter; the canonical
     `/v1` synthesis only applies to api.openai.com. Operators paste
     the documented URL for every other vendor (cp-api populates it
     from adapter_map's `default_base_url`).

Compat shim: build_hub() keeps `register_specialized("openai", …)` +
`register_specialized("anthropic", …)` so pre-Phase-A ProviderKeys that
carry `provider` but not `adapter` still dispatch through the
two-tier lookup. cp-api admits every catalog vendor post-Phase-A
with `adapter` populated, so the family bridge above covers them
without any specialized entry needed.

Tests deleted: every test that exercised the deleted surface
(deepseek/cohere canonical-host normalization, gemini/cohere/long-tail
with_name vendor-default targeting, with_name x-aisix-bridge header
variant). Kept a single non-OpenAI host pass-through test that pins
the family bridge's verbatim treatment of any non-openai api_base
(corporate proxies, alternative deployments).

Doc updates: stale `Provider::default_base_url` / `Provider::Cohere` /
`OpenAiBridge::with_name` references in dispatch.rs, rerank.rs,
provider_key.rs, azure-openai/lib.rs replaced with the post-clean-cut
language.

Net: −464 LOC. Workspace cargo test + clippy clean.
moonming added a commit that referenced this pull request May 22, 2026
…es + sanitize_tag

Audit on the previous push found 3 MEDIUM findings. This commit addresses
two of them (#1 and #3); #2 (test for snapshot→emit populate path) is
covered by the 4 new sanitize_tag unit tests + existing test plan.

MEDIUM-1 RESOLVED: my PR-body justification for deferring messages.rs
was factually wrong — `emit_anthropic_usage_event` already takes
`provider_key_id: &str` (called from 3 sites that already pass it).
Adding the same snapshot-lookup-and-populate block as chat.rs is
mechanical, no signature change required. Done in messages.rs:921-939.
This means /v1/messages flows now also emit the 5 telemetry tag
fields, not just /v1/chat/completions.

MEDIUM-3 RESOLVED: added `sanitize_tag(s: String) -> String` helper in
chat.rs (pub(crate) so messages.rs can re-use). Strips ASCII control
characters and caps length at 256 chars. Applied to all 4
operator-defined tag strings emitted by both emit_usage_event and
emit_anthropic_usage_event. Defence-in-depth against a malicious
operator crafting a label like
`"production\ninjected-internal-key: secret"` that, while
JSON-escaped on this gateway↔cp-api hop, could forge log lines on
downstream consumers that re-stringify the tag. The right place to
enforce a strict admission policy is at PK CRUD validation in cp-api
— sanitize_tag is a belt-and-suspenders guard, not a replacement.

4 new unit tests in `sanitize_tag_tests`:
  - empty stays empty
  - strips \n, \r, \0
  - caps at 256 chars
  - preserves normal ASCII + Unicode (labels in any language)

`cargo test -p aisix-proxy --lib` → 287 passed (was 283 + 4 new).
`cargo check --workspace` → clean.

PR body's "messages.rs deferred" claim was wrong; I've also removed
the duplicate `let snap = state.snapshot.load();` in messages.rs that
the new block superseded (matching the same dedupe chat.rs got).
jarvis9443 added a commit that referenced this pull request Jun 2, 2026
Output guardrails only inspected message.content, so client-visible
output that lives elsewhere bypassed content/DLP checks:

- tool_calls / Anthropic tool_use (normalized into message.extra) are
  now folded into a single guardrail-inspected text view via
  ChatResponse::guardrail_output_text(), used by the keyword, text-
  moderation, Bedrock, and Prompt Shield output checks (#3/#18/#21).
  Reasoning/thinking content is intentionally left out of scope.

- Non-streaming cache hits now run the resolved output guardrail chain
  before returning the stored body, instead of replaying it unchecked
  (#28). Streaming output guardrails already run end-of-stream.

Part of #448 (findings #3, #18, #21, #28)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants