ci: add schema drift check for resource JSON Schemas - #309
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR enables automated JSON Schema generation for aisix-core resource model types. It adds a ChangesSchema Generation Infrastructure and Output
🎯 3 (Moderate) | ⏱️ ~20 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
Adds a new `schema-drift` job to the CI workflow that runs `cargo run -p aisix-core --bin dump-schema` and asserts `git diff --exit-code schemas/` is clean. PRs that modify resource struct in `crates/aisix-core/src/models/` but forget to regenerate the schema files now fail CI with a fix instruction in the error message. ## Why Refs #304 item #1. The `dump-schema` tool and `schemas/resources/*.schema.json` files were introduced in #308; without an enforcement mechanism the committed schemas can silently diverge from the Rust types as the resource graph evolves (especially during issue #302 Phase A, which is actively mutating ProviderKey / Model). This job is that enforcement. ## Job placement Sits as a peer to `lint` — fast, independent, no service deps. Runs in parallel with `lint` / `rust-unit` / `build-bin`. Not a `needs:` target of any downstream job, so a drift failure does not block the e2e or coverage signals. ## Verification - Positive path: `cargo run -p aisix-core --bin dump-schema` on the HEAD of this PR succeeds and `git diff --exit-code schemas/` is empty (no drift in tree) - Negative path: locally introduced a synthetic drift by truncating `schemas/resources/api_key.schema.json` to `{}`. `git diff --exit-code schemas/` returned non-zero — the check fires as expected. Reverted with `git checkout schemas/resources/api_key.schema.json`. - YAML parses with `python3 -c "import yaml; yaml.safe_load(open(...))"`. ## Stack Builds on #308 (which adds the binary + initial schemas). Base will switch to `main` once #308 merges. Refs #304 (#1).
16a7ddb to
ddefbd5
Compare
The hand-written OpenAPI 3.1 document in `crates/aisix-admin/src/openapi.rs` previously inlined its own copy of every resource schema (`Model`, `ApiKey`, `ProviderKey`, `Guardrail`, `CachePolicy`, `ObservabilityExporter`, `RateLimit`, `Routing`, plus the nested `ModelCost` / `BackgroundModelCheck`). That left three places to keep in sync whenever a resource field changed: the Rust struct, the inline OpenAPI schema, and the cp-api / dashboard side. This PR cuts the duplication. The Rust struct is now the single source of truth; `dump-schema` (PR #308) writes canonical draft-07 JSON Schemas into `schemas/resources/*.schema.json`; CI (PR #309) enforces those files match the structs. This commit: 1. Removes the ten inlined resource schemas from `OPENAPI_JSON_BASE` (the const formerly named `OPENAPI_JSON`). 2. Embeds the eight canonical schema files at compile time via `include_str!` into a new `RESOURCE_SCHEMAS` const. 3. Adds `merged_openapi()` — runs once on first request, parses the base spec, parses each embedded schema, hoists `definitions/*` into top-level `components.schemas`, rewrites `$ref: #/definitions/X` to `$ref: #/components/schemas/X` (JSON Schema draft-07 → OpenAPI 3.1), and caches the result in an `OnceLock<String>`. 4. Changes `openapi_json()` to serve the merged doc instead of the raw `OPENAPI_JSON_BASE`. 5. Updates the three openapi unit tests to parse `merged_openapi()`. ## What this means for `/admin/openapi.json` The served document keeps the same wrapper schemas (`ModelEntry`, `ApiKeyEntry`, `ModelStatusView`, `ModelKind`, `RuntimeStatus`, `SystemTime`, `AdminError`) and gains 16 new top-level component schemas hoisted from the resource definitions (`Adapter`, `BedrockConfig`, `CacheBackend`, `CooldownConfig`, `GuardrailHookPoint`, `KeywordPattern`, `OnAllFilteredPolicy`, `ParamConstraints`, `Provider`, `RequestOverrides`, `ResponseOverrides`, `RoutingStrategy`, `RoutingTarget`, `StreamDoneMarker`, `TelemetryTags`, etc.). The resource schemas themselves are now precise reflections of the Rust types — e.g. `Guardrail` uses a proper `oneOf` discriminator on `kind` instead of the previous flat `additionalProperties: true` hand-wave; `Provider` lists its 6 variants from the actual enum; `Adapter` lists the 5 wire-shape kebab-case values from #302 Phase A. ## Verification - `cargo check -p aisix-admin` clean - `cargo clippy --workspace --all-targets -- -D warnings` clean - `cargo fmt --all -- --check` clean - `cargo test -p aisix-admin --lib` — all 7 openapi tests pass, including the regression test `openapi_apikey_schema_excludes_max_budget_usd` - External validation: parsed the merged doc, collected 43 `$ref` references across 32 distinct targets, all resolve inside `#/components/schemas/*` (0 unresolved) ## Why nested `if let` instead of let-chains Workspace is on `edition = "2021"`. The merge logic uses one level of nesting in two spots; not pretty, but `edition = "2024"` is a separate decision not in this PR's scope. ## Stack Builds on: - #307 (JsonSchema derives on resource structs) - #308 (dump-schema binary + initial schema files) - #309 (CI drift enforcement) Merge order: 307 → 308 → 309 → this PR. Base will switch to `main` once #308 merges. Refs #304 (#1).
Summary
Adds a
schema-driftjob to.github/workflows/ci.ymlthat:cargo run -p aisix-core --bin dump-schemagit diff --exit-code schemas/is cleanPRs that modify a resource struct in
crates/aisix-core/src/models/but forget to regenerate the schema files now fail CI with a fix instruction in the error message:Why
Refs #304 item #1. The
dump-schematool and the nineschemas/resources/*.schema.jsonfiles landed in #308. Without an enforcement mechanism the committed schemas can silently diverge from the Rust types as the resource graph evolves — especially relevant during issue #302 Phase A, which is actively mutatingProviderKeyandModel. This job is that enforcement.Job placement
Sits as a peer to
lint— fast, independent, no service dependencies. Runs in parallel withlint,rust-unit, andbuild-bin. Not aneeds:target of any downstream job, so a drift failure does not block e2e or coverage signals.Verification
Positive path:
cargo run -p aisix-core --bin dump-schemaon HEAD succeedsgit diff --exit-code schemas/is empty (no drift in tree)Negative path (proving the check actually catches drift):
schemas/resources/api_key.schema.jsonto{}git diff --exit-code schemas/returned non-zero ✓git checkout schemas/resources/api_key.schema.jsongit diff --exit-code schemas/clean again ✓Workflow file:
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))"parses successfullyDiff (20 added lines)
Stack
Builds on #308 (binary + initial schemas), which builds on #307 (JsonSchema derives). Merge order: #307 → #308 → this PR.
Refs #304 (#1).
Summary by CodeRabbit
Release Notes
New Features
Chores