Uh oh!
There was an error while loading. Please reload this page.
test(admin): real-etcd integration tests for every admin handler - #99
Merged
Conversation
The in-memory ConfigStore tests in aisix-admin/src/lib.rs only exercise compile-time correctness of the EtcdConfigStore trait contract. They don't catch: - subkey constant drift between EtcdConfigStore::*_SUBKEY and the match arms in aisix-etcd::loader (a regression in either side silently breaks the watch path) - JSON shape drift between the Admin write and the loader's serde parse (a field rename that misses one side gets logged + skipped in production rather than failing loudly) - gRPC + revision plumbing in EtcdConfigStore itself This commit adds a new integration-tests file crates/aisix-admin/tests/etcd_integration.rs that drives the full HTTP → AdminState → EtcdConfigStore → real etcd path for all six resources (Models, ApiKeys, ProviderKeys, Guardrails, CachePolicies, ObservabilityExporters), then a load-bearing seventh test that: 1. Seeds one of every kind through the Admin HTTP layer 2. Reads the raw KV bytes back via etcd_client 3. Runs them through aisix_etcd::loader::build_snapshot 4. Asserts every snapshot table is populated and BuildStats reports zero rejections That last assertion is the one the previous in-memory tests couldn't make — it pins the byte-level compatibility between the admin write path and the watch read path. Skip semantics - Tests no-op when ADMIN_TEST_ETCD_URL is unset (mirrors the CACHE_TEST_REDIS_URL pattern in crates/aisix-cache/tests/redis_integration.rs). Local `cargo test` without docker still passes. - CI: ci.yml's rust-unit job grows a `bitnami/etcd:3.5` service alongside the existing redis service and sets ADMIN_TEST_ETCD_URL so the etcd-backed path actually runs in every PR. Per-test prefix is randomised (`/aisix-admin-it/<nanos>-<thread>`) so concurrent tests don't collide. The prefix is left in etcd at test exit — the container is ephemeral so cleanup isn't worth the additional teardown noise. Verified - `cargo fmt --all --check` clean - `cargo clippy --workspace --tests -- -D warnings` clean - `cargo check -p aisix-admin --tests` clean (compile-only here; the real run happens in CI against the etcd service. Local docker isn't running on this machine so I deferred the live exec to CI.)
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds CI provisioning for a real etcd instance and a gated Tokio-based integration test suite that exercises Admin HTTP CRUD for multiple resource types and validates loader snapshot compatibility by reading raw etcd KV entries. ChangesEtcd Integration Testing
Sequence Diagram(s)sequenceDiagram
participant Client
participant AdminHTTP
participant AdminState
participant Etcd
participant TestHarness
participant Loader
rect rgba(200,200,255,0.5)
Client->>AdminHTTP: POST resource (auth)
AdminHTTP->>AdminState: persist via EtcdConfigStore
AdminState->>Etcd: PUT KV (prefix/resource)
Etcd-->>AdminState: OK
end
rect rgba(200,255,200,0.5)
Client->>AdminHTTP: GET list
AdminHTTP->>AdminState: range keys
AdminState->>Etcd: RANGE KV
Etcd-->>AdminState: KV list
AdminState-->>AdminHTTP: resources
AdminHTTP-->>Client: JSON list
end
rect rgba(255,200,200,0.5)
TestHarness->>Etcd: direct RANGE raw prefix
TestHarness->>Loader: build_snapshot(raw KV)
Loader-->>TestHarness: Snapshot (tables & counts)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
Bitnami retired the bitnami/etcd:3.5 floating tag from Docker Hub sometime in 2025; the GHA runner's anonymous docker pull now fails with 'manifest unknown' against that image. Switch to the upstream Quay-hosted image — same set of ETCD_* env vars works there.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds real-etcd integration tests for the Admin service to validate end-to-end CRUD and loader compatibility, and wires an etcd service into CI so these tests run continuously.
Changes:
- Added
crates/aisix-admin/tests/etcd_integration.rscovering CRUD round-trips for six admin resources plus a loader compatibility “all tables populated” assertion. - Updated CI workflow to run an etcd service and set
ADMIN_TEST_ETCD_URLso the integration tests execute in CI.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/aisix-admin/tests/etcd_integration.rs | Adds real etcd-backed integration tests validating Admin → EtcdConfigStore → etcd → loader behavior. |
| .github/workflows/ci.yml | Provisions an etcd service in CI and exports ADMIN_TEST_ETCD_URL to enable the new integration tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+101
to
+102
| /// and assert the round-trip lands real entries in etcd. Returns `true` | ||
| /// on success so the per-resource macro can assert. |
| // LIST again — empty | ||
| let app = build_router(state); | ||
| let resp = app.oneshot(auth_get(list_uri)).await.unwrap(); |
| ports: ["2379:2379"] | ||
| env: | ||
| ETCD_LISTEN_CLIENT_URLS: http://0.0.0.0:2379 | ||
| ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379 |
Comment on lines
+53
to
+57
| async fn build_state_with_real_etcd(url: &str, prefix: &str) -> AdminState { | ||
| let client = etcd_client::Client::connect([url], None) | ||
| .await | ||
| .expect("etcd connect"); | ||
| let store: Arc<dyn ConfigStore> = Arc::new(EtcdConfigStore::new(client, prefix)); |
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
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.
Summary
The in-memory
ConfigStoretests inaisix-admin/src/lib.rsonly check that the trait contract compiles. They don't catch:EtcdConfigStore::*_SUBKEYand the match arms inaisix-etcd::loader::build_snapshot— a regression on either side silently breaks the watch path.EtcdConfigStoreitself.This PR adds
crates/aisix-admin/tests/etcd_integration.rsdriving the full HTTP →AdminState→EtcdConfigStore→ real etcd path for all six resources (Models, ApiKeys, ProviderKeys, Guardrails, CachePolicies, ObservabilityExporters), plus a load-bearing seventh test that:etcd_clientaisix_etcd::loader::build_snapshotBuildStatsreportsschema_rejected=0, parse_rejected=0, unknown_kind=0That last assertion is the one the in-memory tests couldn't make — it pins the byte-level compatibility between the admin write path and the watch read path.
Skip semantics
ADMIN_TEST_ETCD_URLis unset (mirrors theCACHE_TEST_REDIS_URLpattern incrates/aisix-cache/tests/redis_integration.rs). Localcargo testwithout docker still passes.ci.yml'srust-unitjob gains abitnami/etcd:3.5service alongside the existing redis service and setsADMIN_TEST_ETCD_URLso the etcd-backed path actually runs on every PR.Per-test prefix is randomised (
/aisix-admin-it/<nanos>-<thread>) so concurrent tests don't collide on the etcd service.Test plan
cargo fmt --all --checkcleancargo clippy --workspace --tests -- -D warningscleancargo check -p aisix-admin --testsclean (Docker Desktop isn't running on my dev box; deferred the live exec to CI which has docker-as-a-service).rust unit + coverageshould pick up the etcd service and run all 7 tests against it. This PR's main verification happens in CI.Summary by CodeRabbit
New Features
Tests
Chores