Uh oh!
There was an error while loading. Please reload this page.
feat(config): file-based resource source (resources.yaml) for standalone mode - #759
Merged
Conversation
…one mode
A single-container gateway can now load every dynamic resource from one
declarative resources.yaml instead of etcd + Admin API writes:
- aisix-core::filesource: read -> ${VAR} interpolation (string scalars,
$$ escape, partial values, missing/empty var = error) -> file sugar
(models[].provider_key name ref, api_keys[].display_name identity +
key_env -> SHA-256 key_hash, rate_limit_policies[].scope_ref name
resolution) -> the SAME canonical JSON-Schema validators and typed
serde models as the etcd path -> cross-reference checks (non-glob
model refs must exist) -> AisixSnapshot. Ids are UUIDv5 of
"<kind>/<identity>" under a fixed documented namespace, stable
across reloads; duplicate identities and unknown top-level keys are
load errors; all errors are aggregated, not first-only.
- config: new optional top-level resources_file; mutually exclusive
with configured etcd.endpoints and with managed mode; etcd-only
behavior unchanged when absent.
- server: file-mode boot fails fast with the aggregated report; SIGHUP
re-runs the pipeline and atomically swaps the snapshot on success,
keeps last-good and WARNs on failure (no file watcher by design);
new `aisix validate --resources <file>` runs the identical pipeline
without booting listeners.
- admin: listener stays up in file mode — reads (lists/gets, status,
health, openapi, playground) serve from the live snapshot via
FileManagedStore; every resource write (incl. rotate) answers 409
naming the file through one router-level guard.
- e2e: harness gains file mode (spawnApp resourcesFile, no etcd
contact) + fast-fail on early binary exit; new cases cover smoke,
playground, admin write-guard, file-vs-etcd differential behavior,
SIGHUP reload (valid + invalid edit), and fail-fast boot.
Canonical schemas are untouched (dump-schema / dump-openapi no-diff).📝 WalkthroughWalkthroughAdds a file-based resource source with YAML interpolation, deterministic IDs, validation, SIGHUP reloads, offline validation, read-only admin endpoints, and gateway E2E coverage. ChangesFile resource source
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Operator
participant aisix-server
participant filesource
participant SnapshotHandle
participant AdminRouter
Operator->>aisix-server: Start with resources_file
aisix-server->>filesource: Load and validate resources
filesource-->>aisix-server: AisixSnapshot
aisix-server->>SnapshotHandle: Seed snapshot
Operator->>aisix-server: Send SIGHUP
aisix-server->>filesource: Reload resources file
filesource->>SnapshotHandle: Replace snapshot on success
Operator->>AdminRouter: Read or mutate resource
AdminRouter->>SnapshotHandle: Read current snapshot
AdminRouter-->>Operator: Resource data or HTTP 409
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/aisix-core/src/filesource/mod.rs (1)
134-511: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting the pipeline passes into helpers.
load_from_strruns the full pipeline (~380 lines) inline. It reads clearly thanks to the pass banners, but each pass (root/format gate, pass 1 prepare, pass 2 desugar+validate+decode, pass 3 cross-ref, materialize) is a natural function boundary that would shrink this to a readable orchestrator and make each stage independently testable. Optional given the strong test coverage.As per coding guidelines: "Keep functions small and focused - aim for functions under 20-30 lines".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aisix-core/src/filesource/mod.rs` around lines 134 - 511, Refactor load_from_str into a short orchestration function by extracting the root/format validation, preparation, desugar/validation/decoding, cross-reference checks, and snapshot materialization into focused helper functions. Preserve the existing pass order, error aggregation, and behavior while passing shared state explicitly; keep finish and the extracted stages independently testable.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-admin/src/lib.rs`:
- Around line 213-240: The file_managed_write_guard currently runs before
AdminAuth and exposes the file-managed path to unauthenticated mutating
requests. Rework the router composition so file_managed_write_guard is applied
inside the authenticated admin route group or otherwise after the AdminAuth
boundary, while preserving its existing read and non-resource bypass behavior
and returning 401 for unauthenticated writes.
---
Nitpick comments:
In `@crates/aisix-core/src/filesource/mod.rs`:
- Around line 134-511: Refactor load_from_str into a short orchestration
function by extracting the root/format validation, preparation,
desugar/validation/decoding, cross-reference checks, and snapshot
materialization into focused helper functions. Preserve the existing pass order,
error aggregation, and behavior while passing shared state explicitly; keep
finish and the extracted stages independently testable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a70928b-1b3d-4d90-9110-30c115d0e1ae
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
Cargo.tomlconfig.example.yamlcrates/aisix-admin/src/error.rscrates/aisix-admin/src/file_store.rscrates/aisix-admin/src/lib.rscrates/aisix-admin/src/state.rscrates/aisix-admin/src/store.rscrates/aisix-core/Cargo.tomlcrates/aisix-core/src/config.rscrates/aisix-core/src/filesource/desugar.rscrates/aisix-core/src/filesource/mod.rscrates/aisix-core/src/filesource/tests.rscrates/aisix-core/src/filesource/yaml.rscrates/aisix-core/src/lib.rscrates/aisix-server/src/main.rstests/e2e/src/cases/file-resource-source-e2e.test.tstests/e2e/src/harness/app.ts
Uh oh!
There was an error while loading. Please reload this page.
…equests The router-level 409 guard runs before per-handler AdminAuth (layers wrap routes), so an unauthenticated mutating request in file mode got the 409 — whose body names the resources-file path — instead of 401. The guard now short-circuits only when the request carries a valid admin key (shared header check hoisted out of the AdminAuth extractor); unauthenticated and wrong-key writes fall through to the handler's 401 with no path disclosure. Unit + e2e regression coverage added.
…ng provider_key_id, guardrail-fallback pin Independent-audit follow-ups (no HIGH; all MEDIUMs addressed): - reject two api_keys entries resolving to the same key_hash: the runtime credential index is hash-keyed, so a duplicate plaintext silently last-wins at auth time — now a load error naming both entries (never the hash or plaintext), matching the invariant the Admin API enforces on create. - reject an explicit models[].provider_key_id that doesn't equal the derived id of a file-defined provider key: in file mode every id is derived, so any other value is guaranteed dangling and previously only surfaced per-request. Correctly-derived explicit ids still load (determinism makes generated files legal). - pin the guardrail dependency: file-defined guardrails execute through the zero-attachment env-global behavior of the guardrail index — the removal-slated compat block now carries a NOTE that the file source relies on it, and a new e2e case asserts a file-defined guardrail actually fires (422, upstream untouched). - run the SIGHUP reload pipeline (blocking file IO + parse/validate) on a blocking task instead of an async worker. - e2e: validate-subcommand failure path (exit 1 + aggregated report on stderr), CI-loud skip for the differential case, doc note that error aggregation is per-entry granular.
moonming
commented
Jul 13, 2026
MemberAuthor
Review triage (automated review + independent audit), all addressed as of 0d4fdc1: Fixed in code
Declined with rationale
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A standalone file-based resource source: a single-container gateway can now load every dynamic resource — provider keys, models, API keys, guardrails, MCP servers, A2A agents, cache policies, observability exporters, rate-limit policies — from one
resources.yaml, instead of running etcd and writing resources through the Admin API. This adds a second, fully declarative way to run standalone; the etcd source is completely unchanged.Enable it in
config.yaml:The file format (v1) was pinned after a review of mainstream declarative gateway-config conventions; the format is versioned (
_format_version) so later revisions can evolve it explicitly.Format rules (all enforced at load)
schemas/resources/*.schema.json) and flows through the same JSON-Schema validators and typed serde models as the etcd path. Canonical schemas are untouched by this PR (dump-schema/dump-openapibyte-identical).${VAR}interpolation in any string value: partial values work (https://${HOST}/v1),$$escapes a literal$, bare$VARis not interpolated, and a missing/empty variable is a load error naming file, kind, entry, and field path. Interpolation runs on parsed string scalars, so environment values can never inject YAML structure.models[].provider_key— provider-key name → derivedprovider_key_id; mutually exclusive with an explicitprovider_key_id; unknown names error and list what is defined.api_keys[].display_name— required file-side identity, stripped before validation (the canonicalapi_keydocument has no name field).key_envXORkey_hash;key_envnames an environment variable whose value is hashed (SHA-256 lowercase hex, identical toApiKey::hash_bearer) and dropped — the plaintext never reaches logs, error messages, or the store. A variable whose value itself looks like${...}(double indirection) is rejected.rate_limit_policies[].scope_ref— forscope: api_key/scope: model, a name resolved to the referenced entry's derived id;team/member/team_memberpass through verbatim.idfield. Every entry's identity is itsdisplay_name(provider_keys, models, api_keys) orname(the other six;mcp_servers/a2a_agentsalso acceptdisplay_nameper their schemas). The derived id is UUIDv5 of"<kind>/<identity>"under a fixed namespace (FILE_RESOURCE_NAMESPACE = 63e50ab2-677a-54d3-8d1e-c0cb29ceae94, itselfuuid5(NAMESPACE_URL, "https://github.com/api7/aisix#resources-file"), pinned by test) — stable across reloads and processes. Anidfield in the file is rejected on every kind, including the schema-open ones.key_hashwould silently last-wins in the runtime credential index, so the load rejects them (the same invariant the Admin API enforces on create) — the error names the entries, never the hash or plaintext.api_keys[].allowed_modelsentry,routing.targets[].model, ensemble panel/judge ref, and semanticembedding_model/default/route-target/failure-target must match a defined model name (entries containing*are glob patterns and exempt).scope_refresolution follows the same rule forapi_key/modelscopes. An explicitmodels[].provider_key_idmust equal the derived id of a file-defined provider key (ids are deterministic, so generated files can carry them; any other value is guaranteed dangling and is rejected with a pointer to theprovider_keyname sugar).Mode wiring
resources_fileis a new optional top-level config field. When set, theetcdsection must be absent or unconfigured; setting both is a boot error, as is combining it withmanaged.enabled(managed mode is control-plane-driven by definition). Withoutresources_file, behavior is exactly today's etcd mode./admin/v1/models/status,/admin/v1/health, OpenAPI,/readyz) serve from the live snapshot, and the Playground works. Every resource write (POST/PUT/DELETE, including key rotation) answers 409 with a message naming the file, enforced at one router-level chokepoint (plus read-only store errors as defense in depth). Auth ordering: the guard only short-circuits for requests carrying a valid admin key — unauthenticated or wrong-key writes get the same401as etcd mode, and the 409 body (which names the operator's file path) is only ever shown to authenticated admins.Fail-fast boot & SIGHUP reload
aisix validate --resources <file>runs the same pipeline without booting listeners: exit 0 on success, non-zero with the same aggregated report otherwise.${VAR}references resolve against the validating process's environment.Deliberately absent
idfields in the file — ids are always derived; hand-managed ids would break the determinism that keeps references and rate-limit counters stable across reloads.allowed_tools/allowed_agentsentries. These are permission masks (glob patterns matched at request time), not references: an entry granting a tool or agent that is not (yet) defined is a no-op grant, exactly as in etcd mode where an ACL may legitimately predate the resource it names. The load-time reference checks deliberately cover the dispatch-critical refs (models, provider keys, scope_ref), where a dangle means a broken request path rather than an unused grant.Verification
cargo fmt --check,cargo clippy --workspace --all-targets -- -D warnings: clean.cargo test --workspace(with live etcd + redis integration env): 2274 passed, 0 failed.$$/ bare-$/ missing-var, sugar resolution, key_env hashing + XOR + no-leak, duplicate identity + duplicate credential,idrejection, cross-refs with glob exemption, explicit provider_key_id resolution, scope_ref per scope, UUID determinism), 5 config wiring, 2 file store, 4 admin file-mode routing (409 coverage incl. rotate, snapshot-backed reads, unauthenticated-write 401 without path disclosure, guard inert in etcd mode), 1 auth helper, 3 server CLI/validate.tests/e2e/src/cases/file-resource-source-e2e.test.ts, 8 cases): file-mode smoke (full chain viakey_envcaller + mock upstream; allowed_models 403 with upstream untouched), Playground in file mode, admin write-guard (authenticated 409 + snapshot reads + unauthenticated 401 with no path leak), file-defined guardrail fires (422, upstream untouched), differential file-mode vs etcd-mode (same logical resources → identical/v1/modelslisting, chat 200 body, 403 envelope), SIGHUP reload (valid edit applies; invalid edit keeps last-good with nothing partially applied), fail-fast boot (non-zero exit + aggregated named errors on stderr),aisix validateexit codes + stderr report.cargo run -p aisix-core --bin dump-schemaandcargo run -p aisix-admin --bin dump-openapi: no diff — the canonical schemas are intentionally untouched.Review trail
An independent audit (no HIGH findings) and the automated review both ran against the first push; every MEDIUM is addressed in code on this branch: auth-ordering on the write guard (unauthenticated writes now 401 without path disclosure), duplicate-credential rejection, explicit provider_key_id resolution, the guardrail zero-attachment dependency pinned in a source comment + e2e,
validatefailure-path e2e, reload pipeline moved to a blocking task, and a CI-loud skip for the differential case. The one deliberate non-change (ACL masks, above) is justified in “Deliberately absent”.