feat(cli): aisix export — emit resources.yaml from a running etcd store#761
feat(cli): aisix export — emit resources.yaml from a running etcd store#761moonming wants to merge 1 commit into
Conversation
Add an `aisix export` subcommand that reads the resources currently stored
in etcd and emits a resources.yaml the file source (`resources_file`) can
load back — the inverse of the file loader and the migration/backup path
for a standalone deployment moving from Admin-API-plus-etcd to the
declarative file.
Export decodes each kind through the same typed models the gateway loads
(via the shared snapshot builder), then rewrites the set into idiomatic
file form:
- resugar references: model.provider_key_id -> `provider_key: <name>`;
rate_limit_policy.scope_ref for api_key/model scopes -> the referenced
entry's name (team-family scopes pass through). Dangling references are
kept verbatim and warned, never silently dropped.
- ids are dropped (the file derives them from names); api_keys get a
deterministic `apikey-<hash…>` display_name synthesized from their
already-hashed key_hash, which is emitted verbatim.
- duplicate identities within a kind are surfaced as warnings.
Secrets are never emitted in cleartext by default: provider_key api_key,
mcp/a2a secret, guardrail moderation credentials, and OTLP exporter auth
headers are replaced with derived `${VAR}` placeholders, and a companion
"set these before loading" list is printed to stderr (never into the
file). `--reveal-secrets` opts into inline values for air-gapped
same-host migration.
Emission reuses the file source's own YAML library so the output is
guaranteed to re-parse. Round-trip proven end to end by a new e2e:
seed etcd -> `aisix export` -> load the file into a file-mode gateway ->
assert identical /v1/models, chat completion, and allowed_models 403
against the etcd-mode gateway, plus no live credential at default
settings.
📝 WalkthroughWalkthroughAdds ChangesResource export workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as aisix export
participant Etcd as etcd
participant Loader as Snapshot loader
participant Builder as build_export_document
participant Emitter as emit_yaml
participant FileGateway as file-mode gateway
CLI->>Etcd: load resources under prefix
Etcd-->>Loader: return stored documents
Loader-->>Builder: provide AisixSnapshot
Builder-->>Emitter: provide redacted ExportDocument
Emitter-->>CLI: return resources YAML
CLI->>FileGateway: start with YAML and environment values
FileGateway-->>CLI: serve equivalent gateway behavior
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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
crates/aisix-server/src/export/secrets.rs (1)
76-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a shared redaction-context struct instead of repeating the 5-arg tail across
redact_top_level,redact_by_key, andredact_headers.All three functions carry the same
(kind_token, kind, identity, reveal, out)tail. Bundling them into a smallstruct RedactionCtx<'a> { kind_token: &'a str, kind: &'static str, identity: &'a str, reveal: bool, out: &'a mut Vec<SecretPlaceholder> }would cut duplication and remove the risk of transposing two same-typed positional&strargs (kind_tokenvskind) at a call site.♻️ Sketch
pub struct RedactionCtx<'a> { pub kind_token: &'a str, pub kind: &'static str, pub identity: &'a str, pub reveal: bool, pub out: &'a mut Vec<SecretPlaceholder>, } pub fn redact_top_level(doc: &mut Value, field: &str, ctx: &mut RedactionCtx) { ... }🤖 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-server/src/export/secrets.rs` around lines 76 - 183, Introduce a shared RedactionCtx<'a> containing kind_token, kind, identity, reveal, and out, then update redact_top_level, redact_by_key, and redact_headers to accept the context instead of their repeated five-argument tail. Adjust recursive redact_by_key calls and all callers to pass the context while preserving existing redaction behavior and placeholder output.tests/e2e/src/cases/export-roundtrip-e2e.test.ts (1)
110-114: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
afterAllcleanup isn't isolated — an early failure skips the remaining teardown.If
fileApp?.exit()rejects,etcdApp?.exit()andupstream?.close()never run, potentially leaking the spawned processes for subsequent test files.♻️ Proposed fix
afterAll(async () => { - await fileApp?.exit(); - await etcdApp?.exit(); - await upstream?.close(); + await Promise.allSettled([fileApp?.exit(), etcdApp?.exit(), upstream?.close()]); });🤖 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 `@tests/e2e/src/cases/export-roundtrip-e2e.test.ts` around lines 110 - 114, Update the afterAll teardown to isolate cleanup failures so a rejection from fileApp?.exit() does not prevent etcdApp?.exit() or upstream?.close() from running. Ensure each resource cleanup is attempted independently while preserving the existing optional-resource behavior.crates/aisix-server/src/export/yaml_emit.rs (1)
20-29: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCentralize the resources-file format version
yaml_emit.rsandfilesource/mod.rsboth pin"1"separately. Keep a single shared constant so a future version bump updates the emitter and loader together.🤖 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-server/src/export/yaml_emit.rs` around lines 20 - 29, Centralize the resources-file format version currently duplicated in yaml_emit.rs and filesource/mod.rs into one shared public constant. Update emit_yaml and the loader’s version validation to reference that constant, removing both local "1" definitions so future version changes require a single update.
🤖 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-server/src/export/mod.rs`:
- Around line 84-96: Update the output-file handling in the export command
around args.output and Export::reveal_secrets so files written with
--reveal-secrets are created with owner-only permissions (0600) before any
secret-containing data is written. Preserve the existing write error reporting
and normal output behavior for exports without revealed secrets.
In `@crates/aisix-server/src/export/secrets.rs`:
- Around line 46-66: Add a post-pass in document.rs::build_export_document over
ExportDocument::secret_placeholders to detect repeated env_var values. Emit a
warning, matching the existing duplicate file-identity check, whenever two
distinct placeholders resolve to the same environment variable, including enough
identity context to identify both secrets. Preserve the existing placeholder
generation and other validation behavior.
In `@crates/aisix-server/src/main.rs`:
- Around line 87-89: Update the prefix argument definition in the main CLI
configuration to reuse the canonical default from aisix_core::EtcdConfig instead
of hardcoding "/aisix". Preserve the existing long option and String type while
ensuring aisix export stays aligned with future EtcdConfig prefix changes.
In `@tests/e2e/src/cases/export-roundtrip-e2e.test.ts`:
- Around line 56-66: Update the beforeAll setup around EtcdClient.ping so
unavailable etcd always follows the existing skip flow, including when
process.env.CI is set. Remove the hard Error path and ensure the test’s
established ctx.skip() handling remains responsible for skipping without
failing.
---
Nitpick comments:
In `@crates/aisix-server/src/export/secrets.rs`:
- Around line 76-183: Introduce a shared RedactionCtx<'a> containing kind_token,
kind, identity, reveal, and out, then update redact_top_level, redact_by_key,
and redact_headers to accept the context instead of their repeated five-argument
tail. Adjust recursive redact_by_key calls and all callers to pass the context
while preserving existing redaction behavior and placeholder output.
In `@crates/aisix-server/src/export/yaml_emit.rs`:
- Around line 20-29: Centralize the resources-file format version currently
duplicated in yaml_emit.rs and filesource/mod.rs into one shared public
constant. Update emit_yaml and the loader’s version validation to reference that
constant, removing both local "1" definitions so future version changes require
a single update.
In `@tests/e2e/src/cases/export-roundtrip-e2e.test.ts`:
- Around line 110-114: Update the afterAll teardown to isolate cleanup failures
so a rejection from fileApp?.exit() does not prevent etcdApp?.exit() or
upstream?.close() from running. Ensure each resource cleanup is attempted
independently while preserving the existing optional-resource behavior.
🪄 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: d4ad96d1-be9d-4d5c-9125-90254ff5a78a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
crates/aisix-server/Cargo.tomlcrates/aisix-server/src/export/document.rscrates/aisix-server/src/export/mod.rscrates/aisix-server/src/export/secrets.rscrates/aisix-server/src/export/yaml_emit.rscrates/aisix-server/src/main.rstests/e2e/src/cases/export-roundtrip-e2e.test.ts
| match &args.output { | ||
| Some(path) => { | ||
| std::fs::write(path, &yaml) | ||
| .map_err(|e| anyhow::anyhow!("writing {}: {e}", path.display()))?; | ||
| eprintln!( | ||
| "Exported {resource_count} resource(s) to {}", | ||
| path.display() | ||
| ); | ||
| } | ||
| None => { | ||
| print!("{yaml}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Output file is written with default (umask) permissions — tighten for the --reveal-secrets case.
std::fs::write leaves the file's permissions to the process umask, typically group/world-readable on many hosts. The Export::reveal_secrets doc comment (main.rs) explicitly anticipates this flag being used on a live host, at which point this file contains real upstream credentials in the clear. It should be created with owner-only permissions.
🔒 Proposed fix
-use std::path::PathBuf;
+use std::io::Write;
+use std::path::PathBuf;
use std::time::Duration;
+#[cfg(unix)]
+use std::os::unix::fs::OpenOptionsExt;
...
match &args.output {
Some(path) => {
- std::fs::write(path, &yaml)
- .map_err(|e| anyhow::anyhow!("writing {}: {e}", path.display()))?;
+ let mut opts = std::fs::OpenOptions::new();
+ opts.write(true).create(true).truncate(true);
+ #[cfg(unix)]
+ opts.mode(0o600);
+ let mut file = opts
+ .open(path)
+ .map_err(|e| anyhow::anyhow!("writing {}: {e}", path.display()))?;
+ file.write_all(yaml.as_bytes())
+ .map_err(|e| anyhow::anyhow!("writing {}: {e}", path.display()))?;
eprintln!(
"Exported {resource_count} resource(s) to {}",
path.display()
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| match &args.output { | |
| Some(path) => { | |
| std::fs::write(path, &yaml) | |
| .map_err(|e| anyhow::anyhow!("writing {}: {e}", path.display()))?; | |
| eprintln!( | |
| "Exported {resource_count} resource(s) to {}", | |
| path.display() | |
| ); | |
| } | |
| None => { | |
| print!("{yaml}"); | |
| } | |
| } | |
| match &args.output { | |
| Some(path) => { | |
| let mut opts = std::fs::OpenOptions::new(); | |
| opts.write(true).create(true).truncate(true); | |
| #[cfg(unix)] | |
| opts.mode(0o600); | |
| let mut file = opts | |
| .open(path) | |
| .map_err(|e| anyhow::anyhow!("writing {}: {e}", path.display()))?; | |
| file.write_all(yaml.as_bytes()) | |
| .map_err(|e| anyhow::anyhow!("writing {}: {e}", path.display()))?; | |
| eprintln!( | |
| "Exported {resource_count} resource(s) to {}", | |
| path.display() | |
| ); | |
| } | |
| None => { | |
| print!("{yaml}"); | |
| } | |
| } |
🤖 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-server/src/export/mod.rs` around lines 84 - 96, Update the
output-file handling in the export command around args.output and
Export::reveal_secrets so files written with --reveal-secrets are created with
owner-only permissions (0600) before any secret-containing data is written.
Preserve the existing write error reporting and normal output behavior for
exports without revealed secrets.
| pub fn sanitize(label: &str) -> String { | ||
| label | ||
| .chars() | ||
| .map(|c| { | ||
| if c.is_ascii_alphanumeric() { | ||
| c.to_ascii_uppercase() | ||
| } else { | ||
| '_' | ||
| } | ||
| }) | ||
| .collect() | ||
| } | ||
|
|
||
| /// `AISIXSECRET_<KIND>_<IDENTITY>_<FIELD>` — the derived variable name. | ||
| pub fn secret_var(kind_token: &str, identity: &str, field: &str) -> String { | ||
| format!( | ||
| "{SECRET_ENV_PREFIX}_{kind_token}_{}_{}", | ||
| sanitize(identity), | ||
| sanitize(field), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
sanitize/secret_var can collide across different identities or fields, aliasing two distinct secrets to one placeholder.
sanitize maps every non-alphanumeric byte to _, so secret_var("PROVIDER_KEY", "openai-prod", "api_key") and secret_var("PROVIDER_KEY", "openai.prod", "api_key") both resolve to AISIXSECRET_PROVIDER_KEY_OPENAI_PROD_API_KEY. Two provider keys named e.g. openai-prod and openai_prod would receive the same exported placeholder; setting that one env var then feeds the same credential to both, silently misassigning one of them. Nothing in document.rs::build_export_document checks the collected placeholders for duplicate env_var values the way it already does for duplicate file identities (document.rs lines 340-345).
Add a post-pass over ExportDocument::secret_placeholders in build_export_document that warns (like the existing duplicate-identity check) when two placeholders share an env_var.
Also applies to: 60-66
🤖 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-server/src/export/secrets.rs` around lines 46 - 66, Add a
post-pass in document.rs::build_export_document over
ExportDocument::secret_placeholders to detect repeated env_var values. Emit a
warning, matching the existing duplicate file-identity check, whenever two
distinct placeholders resolve to the same environment variable, including enough
identity context to identify both secrets. Preserve the existing placeholder
generation and other validation behavior.
| /// Key prefix the resources are stored under. | ||
| #[arg(long, default_value = "/aisix")] | ||
| prefix: String, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '"/aisix"' --type=rust -B2 -A2Repository: api7/aisix
Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate any canonical default etcd prefix definitions or usages.
rg -n --hidden --glob '!target' --glob '!dist' --glob '!build' \
'aisix|default.*prefix|prefix.*default|"/aisix"' \
crates api7 -g '*.rs' -g '*.toml' -g '*.yaml' -g '*.yml'Repository: api7/aisix
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the CLI and export wiring around the prefix field.
ast-grep outline crates/aisix-server/src/main.rs --view expandedRepository: api7/aisix
Length of output: 3118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --glob '!target' --glob '!dist' --glob '!build' \
'aisix|default.*prefix|prefix.*default|"/aisix"' \
crates api7 -g '*.rs' -g '*.toml' -g '*.yaml' -g '*.yml'Repository: api7/aisix
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline crates/aisix-server/src/main.rs --view expandedRepository: api7/aisix
Length of output: 3118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any canonical default etcd-prefix constant or shared default value.
rg -n --hidden --glob '!target' --glob '!dist' --glob '!build' \
'DEFAULT.*PREFIX|default.*prefix|prefix.*default|"/aisix"|aisix_prefix|etcd.*prefix' \
crates api7 -g '*.rs' -g '*.toml' -g '*.yaml' -g '*.yml'Repository: api7/aisix
Length of output: 10040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '85,330p' crates/aisix-core/src/config.rsRepository: api7/aisix
Length of output: 10456
Reuse the canonical etcd prefix default crates/aisix-server/src/main.rs:88-89 Use the same default as aisix_core::EtcdConfig instead of hardcoding "/aisix" here; that keeps aisix export aligned if the prefix changes later.
🤖 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-server/src/main.rs` around lines 87 - 89, Update the prefix
argument definition in the main CLI configuration to reuse the canonical default
from aisix_core::EtcdConfig instead of hardcoding "/aisix". Preserve the
existing long option and String type while ensuring aisix export stays aligned
with future EtcdConfig prefix changes.
| beforeAll(async () => { | ||
| const etcd = new EtcdClient(); | ||
| etcdReachable = await etcd.ping(); | ||
| if (!etcdReachable) { | ||
| // The whole point of this case is the etcd → file equivalence; a | ||
| // silent skip on CI would let the migration contract rot invisibly. | ||
| if (process.env.CI) { | ||
| throw new Error("export round-trip requires etcd on CI (AISIX_E2E_ETCD)"); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don't hard-fail on etcd unavailability even under CI — follow the established skip convention.
This throws a hard Error when etcd is unreachable and process.env.CI is set, rather than deferring uniformly to the test's own ctx.skip(). Based on learnings, "use ctx.skip() (instead of throwing or failing the test) when etcd is not reachable/available" and "individual test files should not deviate from this convention by throwing hard errors on etcd unavailability."
✅ Proposed fix
const etcd = new EtcdClient();
etcdReachable = await etcd.ping();
if (!etcdReachable) {
- // The whole point of this case is the etcd → file equivalence; a
- // silent skip on CI would let the migration contract rot invisibly.
- if (process.env.CI) {
- throw new Error("export round-trip requires etcd on CI (AISIX_E2E_ETCD)");
- }
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeAll(async () => { | |
| const etcd = new EtcdClient(); | |
| etcdReachable = await etcd.ping(); | |
| if (!etcdReachable) { | |
| // The whole point of this case is the etcd → file equivalence; a | |
| // silent skip on CI would let the migration contract rot invisibly. | |
| if (process.env.CI) { | |
| throw new Error("export round-trip requires etcd on CI (AISIX_E2E_ETCD)"); | |
| } | |
| return; | |
| } | |
| beforeAll(async () => { | |
| const etcd = new EtcdClient(); | |
| etcdReachable = await etcd.ping(); | |
| if (!etcdReachable) { | |
| return; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@tests/e2e/src/cases/export-roundtrip-e2e.test.ts` around lines 56 - 66,
Update the beforeAll setup around EtcdClient.ping so unavailable etcd always
follows the existing skip flow, including when process.env.CI is set. Remove the
hard Error path and ensure the test’s established ctx.skip() handling remains
responsible for skipping without failing.
Source: Learnings
1 participant
What
Adds an
aisix exportsubcommand — the inverse of the file source (resources_file). It reads the resources currently stored in etcd and emits aresources.yamlthat the file loader can load back, so a standalone deployment can move from Admin-API-plus-etcd to the declarative file (or take a portable, declarative backup of its live config).Default output is stdout; the secret-placeholder list and any warnings go to stderr.
How it stays faithful to the loader
Export decodes every kind through the same typed models the gateway loads (the shared snapshot builder), so the exported set is exactly what the running gateway would serve. It then rewrites each entry into idiomatic file form — the exact sugar the file source desugars on the way in:
model.provider_key_id→provider_key: <that provider key's display_name>rate_limit_policy.scope_refforapi_key/modelscopes → the referenced entry's name;team/member/team_memberpass through verbatim.idis emitted. Canonical api-key documents carry no name, so a deterministicapikey-<hash…>display_nameis synthesized from the entry's already-hashedkey_hash(emitted verbatim — it round-trips exactly and is not plaintext).guardrail_attachments(which the file format has no collection for), are reported on stderr rather than dropped silently.Secret policy
No live credential is written in cleartext by default. Each secret-bearing field is replaced with a derived, stable, greppable
${VAR}placeholder, and a companion "set these before loading" list is printed to stderr — never into the file:provider_key.api_key,mcp_server.secret,a2a_agent.secretapi_key,access_key_secret, and the nested Bedrocksecret_access_key)headersapi_keysemitkey_hashdirectly (already a hash — round-trips exactly; no placeholder).--reveal-secretsopts into emitting the real stored values inline; it is documented as unsafe and intended only for air-gapped, same-host migration.The placeholder names use the
AISIXSECRET_…prefix rather thanAISIX_…: the gateway's config loader claims theAISIX_prefix and rejects unknown keys, so anAISIX_…secret variable set in the data plane's environment would be misread as a bad config override and fail boot (the same reason the existingSLS_CRED_…/OBJSTORE_CRED_…conventions keep secret variables off that prefix).Emission reuses the file source's own YAML library, so the output is guaranteed to re-parse (including the mandatory quoted
_format_version: "1").Tests
key_hashpassthrough, duplicate-name warn, id-stripping, secret placeholder derivation + no-cleartext-by-default +--reveal-secrets, recursive/nested guardrail + header redaction, and YAML round-trip (format-version stays a string, placeholders survive).tests/e2e/src/cases/export-roundtrip-e2e.test.ts): seed etcd → runaisix export→ load the exported file into a file-mode gateway (supplying the real secret through the emitted placeholder variable) → assert identical observable behavior (/v1/models, a chat completion, and anallowed_models403) against the etcd-mode gateway, and assert the exported file contains no live credential at default settings. This closes the loop: etcd → export → file → same behavior.Full workspace suite green (2299 passed) run twice;
cargo fmt --checkandcargo clippy --workspace --all-targets -D warningsclean; resource schemas untouched.Summary by CodeRabbit
New Features
aisix exportcommand to export gateway resources from etcd to YAML.Tests