test(e2e): seed cases via canonical documents instead of the Admin API - #756
Conversation
Sweep the e2e suite's SEEDING onto SeedClient (direct canonical-document writes to etcd — the same front door the control plane uses in managed mode), leaving the Admin API to the cases that exercise it as a subject or read through it. Scope: - SeedClient grows createGuardrail / createCachePolicy / createRateLimitPolicy (kind segments pinned from the Rust source) plus generic update(kind, id, value) and delete(kind, id); EtcdClient grows single-key delete (deleterange without range_end). - The seed-vs-admin characterization gains an update/delete leg: a seed.update'd model serves the NEW upstream model name, and a seed.delete'd api key stops authenticating (fail-closed) while the admin-created control caller keeps serving. - 132 case files migrated: create/PUT seeding moved to seed.*; admin.json guardrail/cache-policy POSTs to seed.createGuardrail / seed.createCachePolicy; PUT update flows to seed.update. - Direct-write seeding is faster than the admin round trip, which exposed latent propagation races that admin-write latency had been masking. Those gates are repaired, never weakened: proxy-visible /v1/models probes, revision-order canary keys for virtual routers (which /v1/models does not list), env-scoped seed roots for custom-prefix apps, and 401-tolerant probe conditions. Assertions are untouched throughout. Held back on the Admin API deliberately: - seed-vs-admin-characterization-e2e (admin is half the comparison) - smoke.test.ts (its subject IS the admin-write -> proxy-read path) - apikey-budget-e2e (its subject is admin-side validation: the unknown-field 400 cannot be observed via direct writes) - openai-sdk-compat.test.ts + datadog-exporter-e2e (deprecation-window coverage pair, marked with a comment) - read paths stay on admin everywhere: listModels/listModelStatuses, revision GETs, the guardrail store-existence gate, and the apikey rotate RPC (server-generated secret, not a document write). Verified: full suite green twice (138 files / 288 tests per run); tsc --noEmit unchanged (the 5 known TS18048 warnings, no new ones).
📝 WalkthroughWalkthroughChangesThe E2E suite replaces most AdminClient-based fixture provisioning with etcd-backed SeedClient operations. SeedClient now supports guardrails, cache and rate-limit policies, resource updates, and deletions. Several tests also switch readiness checks to proxy model visibility and add propagation-tolerant probes. Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 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 (3)
tests/e2e/src/cases/aliyun-sls-content-capture-e2e.test.ts (1)
52-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
seedRoutingis duplicated verbatim across at least 5 test files.The identical function body appears in
datadog-exporter-e2e.test.ts,usage-client-source-e2e.test.ts,aliyun-sls-exporter-e2e.test.ts, andbedrock-credential-isolation-e2e.test.ts(confirmed via graph context). All copies create the same three provider keys, three models, and one API key with the same display names and constants. Extracting this to a shared harness helper would eliminate the maintenance burden of keeping 5+ copies in sync.🤖 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/aliyun-sls-content-capture-e2e.test.ts` around lines 52 - 98, Extract the duplicated seedRouting setup into a shared E2E harness helper, including creation of the three provider keys, models, and API key with the existing display names and constants. Update seedRouting in the affected test files to call the shared helper while preserving its current inputs and behavior.tests/e2e/src/cases/aliyun-sls-exporter-e2e.test.ts (1)
92-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the duplicated
seedRoutinghelper.This identical
seedRoutingfunction appears in at least 6 test files (otlp-knobs-e2e,datadog-exporter-e2e,usage-client-source-e2e,aliyun-sls-content-capture-e2e,bedrock-credential-isolation-e2e, and this file). Extracting it into a shared harness helper (e.g.,seedStandardRouting(seed, upstream, opts)) would eliminate copy-paste drift risk as the seeding surface evolves.🤖 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/aliyun-sls-exporter-e2e.test.ts` around lines 92 - 107, Extract the duplicated seedRouting helper into a shared test harness helper, such as seedStandardRouting, and update the six listed e2e test files to use it. Preserve the existing provider key, model, and API key seeding behavior, while supporting any file-specific options through the helper’s parameters.tests/e2e/src/cases/usage-client-source-e2e.test.ts (1)
87-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
seedRoutingis duplicated across 6+ test files.The graph context shows the same
seedRoutingfunction (identical body, same constants) inotlp-knobs-e2e.test.ts,datadog-exporter-e2e.test.ts,aliyun-sls-content-capture-e2e.test.ts,aliyun-sls-exporter-e2e.test.ts,bedrock-credential-isolation-e2e.test.ts, and this file. Since this PR already touches all of them, consider extractingseedRoutinginto a shared harness helper (e.g., intests/e2e/src/harness/seed.ts) to eliminate the copy-paste. TheCALLER_KEY_HASHandPROVIDER_SECRETconstants would need to be passed as parameters or co-located in the harness.🤖 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/usage-client-source-e2e.test.ts` around lines 87 - 102, Extract the duplicated seedRouting helper into a shared e2e harness module, such as harness/seed.ts, and update each affected test file to import and use it. Preserve the existing provider/model/API-key setup, passing or co-locating CALLER_KEY_HASH and PROVIDER_SECRET so all callers retain their current behavior.
🤖 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 `@tests/e2e/src/cases/passthrough-e2e.test.ts`:
- Line 219: Update the SeedClient.createProviderKey() call in the Anthropic
passthrough test setup to explicitly set both provider and adapter to the
Anthropic values instead of relying on the OpenAI defaults. Match the
provider-key configuration used by the other Anthropic tests so bridge
selection, metrics, and logs use the correct identity.
---
Nitpick comments:
In `@tests/e2e/src/cases/aliyun-sls-content-capture-e2e.test.ts`:
- Around line 52-98: Extract the duplicated seedRouting setup into a shared E2E
harness helper, including creation of the three provider keys, models, and API
key with the existing display names and constants. Update seedRouting in the
affected test files to call the shared helper while preserving its current
inputs and behavior.
In `@tests/e2e/src/cases/aliyun-sls-exporter-e2e.test.ts`:
- Around line 92-107: Extract the duplicated seedRouting helper into a shared
test harness helper, such as seedStandardRouting, and update the six listed e2e
test files to use it. Preserve the existing provider key, model, and API key
seeding behavior, while supporting any file-specific options through the
helper’s parameters.
In `@tests/e2e/src/cases/usage-client-source-e2e.test.ts`:
- Around line 87-102: Extract the duplicated seedRouting helper into a shared
e2e harness module, such as harness/seed.ts, and update each affected test file
to import and use it. Preserve the existing provider/model/API-key setup,
passing or co-locating CALLER_KEY_HASH and PROVIDER_SECRET so all callers retain
their current 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: 8b5b2e1e-4370-409e-96d0-f05a2daea0fd
📒 Files selected for processing (134)
tests/e2e/src/cases/aliyun-sls-content-capture-e2e.test.tstests/e2e/src/cases/aliyun-sls-exporter-e2e.test.tstests/e2e/src/cases/allowed-models-e2e.test.tstests/e2e/src/cases/anthropic-cache-tpm-commit-e2e.test.tstests/e2e/src/cases/anthropic-content-blocks-cross-provider-e2e.test.tstests/e2e/src/cases/anthropic-count-tokens-e2e.test.tstests/e2e/src/cases/anthropic-streaming-usage-e2e.test.tstests/e2e/src/cases/anthropic-tools-cross-provider-e2e.test.tstests/e2e/src/cases/anthropic-upstream-e2e.test.tstests/e2e/src/cases/api-base-tolerance-e2e.test.tstests/e2e/src/cases/apikey-lifecycle-e2e.test.tstests/e2e/src/cases/audio-images-usage-e2e.test.tstests/e2e/src/cases/audio-timeout-e2e.test.tstests/e2e/src/cases/auth-baseline-e2e.test.tstests/e2e/src/cases/background-health-e2e.test.tstests/e2e/src/cases/batch-files-finetuning-e2e.test.tstests/e2e/src/cases/bedrock-anonymize-mask-e2e.test.tstests/e2e/src/cases/bedrock-credential-isolation-e2e.test.tstests/e2e/src/cases/body-edges-e2e.test.tstests/e2e/src/cases/cache-edges-model-and-disabled-e2e.test.tstests/e2e/src/cases/cache-env-namespace-e2e.test.tstests/e2e/src/cases/cache-fingerprint-collision-e2e.test.tstests/e2e/src/cases/cache-hit-output-guardrail-e2e.test.tstests/e2e/src/cases/cache-policy-backend-e2e.test.tstests/e2e/src/cases/cache-policy-e2e.test.tstests/e2e/src/cases/cache-scenarios-e2e.test.tstests/e2e/src/cases/cache-streaming-not-cached-e2e.test.tstests/e2e/src/cases/cache-ttl-eviction-e2e.test.tstests/e2e/src/cases/caller-credential-isolation-e2e.test.tstests/e2e/src/cases/canary-routing-e2e.test.tstests/e2e/src/cases/chat-anthropic-stream-input-tokens-e2e.test.tstests/e2e/src/cases/completions-legacy-e2e.test.tstests/e2e/src/cases/completions-output-guardrail-e2e.test.tstests/e2e/src/cases/completions-tpm-commit-e2e.test.tstests/e2e/src/cases/concurrency-e2e.test.tstests/e2e/src/cases/cooldown-contract-e2e.test.tstests/e2e/src/cases/cost-aware-routing-e2e.test.tstests/e2e/src/cases/cross-provider-matrix-e2e.test.tstests/e2e/src/cases/datadog-exporter-e2e.test.tstests/e2e/src/cases/deepseek-reasoning-content-e2e.test.tstests/e2e/src/cases/embeddings-e2e.test.tstests/e2e/src/cases/embeddings-missing-prompt-tokens-e2e.test.tstests/e2e/src/cases/embeddings-native-providers-e2e.test.tstests/e2e/src/cases/error-envelope-normalization-e2e.test.tstests/e2e/src/cases/fallback-e2e.test.tstests/e2e/src/cases/fallback-edges-e2e.test.tstests/e2e/src/cases/fallback-on-statuses-e2e.test.tstests/e2e/src/cases/guardrail-aliyun-e2e.test.tstests/e2e/src/cases/guardrail-disabled-bypass-e2e.test.tstests/e2e/src/cases/guardrail-keyword-e2e.test.tstests/e2e/src/cases/guardrail-keyword-message-locations-e2e.test.tstests/e2e/src/cases/guardrail-lakera-e2e.test.tstests/e2e/src/cases/guardrail-model-scope-e2e.test.tstests/e2e/src/cases/guardrail-monitor-mode-e2e.test.tstests/e2e/src/cases/guardrail-monitor-provider-failure-e2e.test.tstests/e2e/src/cases/guardrail-monitor-telemetry-e2e.test.tstests/e2e/src/cases/guardrail-openai-moderation-e2e.test.tstests/e2e/src/cases/guardrail-output-e2e.test.tstests/e2e/src/cases/guardrail-pii-redaction-e2e.test.tstests/e2e/src/cases/guardrail-presidio-e2e.test.tstests/e2e/src/cases/images-generations-e2e.test.tstests/e2e/src/cases/invalid-upstream-config-400-e2e.test.tstests/e2e/src/cases/invalid-upstream-credentials-401-e2e.test.tstests/e2e/src/cases/json-mode-e2e.test.tstests/e2e/src/cases/latency-aware-routing-e2e.test.tstests/e2e/src/cases/latency-histograms-e2e.test.tstests/e2e/src/cases/least-busy-passthrough-endpoints-e2e.test.tstests/e2e/src/cases/least-busy-routing-e2e.test.tstests/e2e/src/cases/messages-cross-provider-extras-e2e.test.tstests/e2e/src/cases/messages-cross-provider-stream-usage-e2e.test.tstests/e2e/src/cases/messages-input-guardrail-e2e.test.tstests/e2e/src/cases/messages-model-group-e2e.test.tstests/e2e/src/cases/messages-output-guardrail-e2e.test.tstests/e2e/src/cases/messages-stream-delta-only-usage-e2e.test.tstests/e2e/src/cases/messages-streaming-output-guardrail-e2e.test.tstests/e2e/src/cases/messages-streaming-toolcall-guardrail-e2e.test.tstests/e2e/src/cases/messages-system-role-e2e.test.tstests/e2e/src/cases/metric-cardinality-model-label-e2e.test.tstests/e2e/src/cases/metrics-dimensions-890-e2e.test.tstests/e2e/src/cases/model-ip-restriction-e2e.test.tstests/e2e/src/cases/openai-embeddings-base64-e2e.test.tstests/e2e/src/cases/openai-sdk-compat.test.tstests/e2e/src/cases/openai-tools-roundtrip-e2e.test.tstests/e2e/src/cases/openrouter-stream-reasoning-e2e.test.tstests/e2e/src/cases/otlp-knobs-e2e.test.tstests/e2e/src/cases/passthrough-e2e.test.tstests/e2e/src/cases/passthrough-guardrail-e2e.test.tstests/e2e/src/cases/passthrough-model-acl-e2e.test.tstests/e2e/src/cases/per-attempt-telemetry-e2e.test.tstests/e2e/src/cases/prometheus-metrics-e2e.test.tstests/e2e/src/cases/provider-key-rotation-e2e.test.tstests/e2e/src/cases/ratelimit-cluster-e2e.test.tstests/e2e/src/cases/ratelimit-e2e.test.tstests/e2e/src/cases/realtime-ws-e2e.test.tstests/e2e/src/cases/request-id-header-e2e.test.tstests/e2e/src/cases/request-id-uuid-e2e.test.tstests/e2e/src/cases/rerank-e2e.test.tstests/e2e/src/cases/responses-cross-provider-e2e.test.tstests/e2e/src/cases/responses-endpoint-e2e.test.tstests/e2e/src/cases/responses-streaming-usage-e2e.test.tstests/e2e/src/cases/retry-backoff-e2e.test.tstests/e2e/src/cases/retry-on-429-vs-background-ignore-e2e.test.tstests/e2e/src/cases/routing-strategies-e2e.test.tstests/e2e/src/cases/runtime-mixed-filtering-e2e.test.tstests/e2e/src/cases/runtime-status-e2e.test.tstests/e2e/src/cases/seed-vs-admin-characterization-e2e.test.tstests/e2e/src/cases/semantic-routing-e2e.test.tstests/e2e/src/cases/server-header-identity-e2e.test.tstests/e2e/src/cases/sls-content-capture-masked-e2e.test.tstests/e2e/src/cases/sls-content-capture-nontext-e2e.test.tstests/e2e/src/cases/sls-content-capture-responses-completions-e2e.test.tstests/e2e/src/cases/sls-failure-content-e2e.test.tstests/e2e/src/cases/sls-structured-truncation-e2e.test.tstests/e2e/src/cases/streaming-edges-e2e.test.tstests/e2e/src/cases/streaming-tpm-commit-e2e.test.tstests/e2e/src/cases/streaming-usage-accumulation-e2e.test.tstests/e2e/src/cases/tag-routing-e2e.test.tstests/e2e/src/cases/team-member-ratelimit-e2e.test.tstests/e2e/src/cases/timeout-fallback-e2e.test.tstests/e2e/src/cases/token-usage-passthrough-e2e.test.tstests/e2e/src/cases/tokens-by-client-total-1002-e2e.test.tstests/e2e/src/cases/tools-cross-provider-e2e.test.tstests/e2e/src/cases/upstream-401-error-code-e2e.test.tstests/e2e/src/cases/upstream-retry-after-passthrough-e2e.test.tstests/e2e/src/cases/usage-cache-counters-e2e.test.tstests/e2e/src/cases/usage-client-source-e2e.test.tstests/e2e/src/cases/vision-input-e2e.test.tstests/e2e/src/cases/vision-messages-e2e.test.tstests/e2e/src/cases/vision-messages-edges-e2e.test.tstests/e2e/src/cases/weighted-routing-distribution-e2e.test.tstests/e2e/src/cases/weighted-routing-edit-e2e.test.tstests/e2e/src/cases/wildcard-routing-e2e.test.tstests/e2e/src/harness/etcd.tstests/e2e/src/harness/seed.ts
|
|
||
| test("Anthropic passthrough: gateway uses x-api-key + anthropic-version, NOT Bearer", async (ctx) => { | ||
| if (!etcdReachable || !app || !admin) { | ||
| if (!etcdReachable || !app || !seed) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the Anthropic model's provider field and check if other Anthropic
# passthrough/admin tests set provider/adapter on the provider key.
sed -n '247,260p' tests/e2e/src/cases/passthrough-e2e.test.ts
# Compare with api-base-tolerance's Anthropic PK for reference
sed -n '184,191p' tests/e2e/src/cases/api-base-tolerance-e2e.test.tsRepository: api7/aisix
Length of output: 788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section around the provider key in the passthrough test.
sed -n '232,255p' tests/e2e/src/cases/passthrough-e2e.test.ts
# Inspect the provider-key helper to see its defaults.
rg -n "createProviderKey|provider:\s*\"openai\"|adapter:\s*\"openai\"" -S tests src . --glob '!**/dist/**' --glob '!**/build/**'Repository: api7/aisix
Length of output: 50366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Relevant passthrough Anthropic block, including the model and request setup.
sed -n '232,265p' tests/e2e/src/cases/passthrough-e2e.test.ts
# Seed helper defaulting for provider keys.
sed -n '35,55p' tests/e2e/src/harness/seed.ts
# Search for any passthrough logic that reads provider_key.provider or provider_key.adapter.
rg -n "provider_key.*provider|provider_key.*adapter|\.provider\b.*provider_key|\.adapter\b.*provider_key" tests/e2e crates/aisix-proxy/src -SRepository: api7/aisix
Length of output: 10705
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Passthrough bridge setup and provider-key shapes.
sed -n '770,820p' crates/aisix-proxy/src/passthrough.rs
# Bridge resolution helper.
sed -n '40,90p' crates/aisix-proxy/src/dispatch.rs
# Anthropic upstream test pattern for provider-key defaults.
sed -n '60,90p' tests/e2e/src/cases/anthropic-upstream-e2e.test.ts
# One more Anthropic passthrough-style test to compare seeding style.
sed -n '70,95p' tests/e2e/src/cases/anthropic-tools-cross-provider-e2e.test.tsRepository: api7/aisix
Length of output: 6135
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show how the passthrough route resolves bridge/provider key behavior.
rg -n "resolve_bridge|provider_key\.provider|provider_key\.adapter|dispatch_two_tier|passthrough/anthropic" crates/aisix-proxy/src/passthrough.rs crates/aisix-proxy/src/dispatch.rs -S
# Narrow read around the passthrough handler that reaches bridge resolution.
sed -n '1,220p' crates/aisix-proxy/src/passthrough.rsRepository: api7/aisix
Length of output: 9886
Set the Anthropic provider key fields explicitly. SeedClient.createProviderKey() falls back to provider: "openai" / adapter: "openai", so this PK is seeded under the wrong provider identity. Match the other Anthropic tests here to keep bridge selection, metrics, and logs aligned.
🤖 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/passthrough-e2e.test.ts` at line 219, Update the
SeedClient.createProviderKey() call in the Anthropic passthrough test setup to
explicitly set both provider and adapter to the Anthropic values instead of
relying on the OpenAI defaults. Match the provider-key configuration used by the
other Anthropic tests so bridge selection, metrics, and logs use the correct
identity.
|
Independent review: no HIGH/MEDIUM findings. Confirmed globally that zero assertion lines changed across the 10.8k-line diff (only poll gates were strengthened), the readiness idioms are snapshot-sound (canary writes ride etcd revision order; /v1/models probes read the proxy snapshot), the etcd-root fixes match |
What
Migrates the e2e suite's seeding from the Admin API (
AdminClient) toSeedClient(#750): canonical resource documents written straight to etcd at<prefix>/<kind>/<id>— the same direct-document front door the control plane uses in managed mode. The Admin API remains in the suite exactly where it is the subject or the read lens.seed.createX(...)/seed.update(...).createGuardrail,createCachePolicy,createRateLimitPolicy(kind segments pinned fromcrates/aisix-admin/src/etcd_store.rs*_SUBKEYconstants andcrates/aisix-core/src/models/rate_limit_policy.rs), plus genericupdate(kind, id, value)anddelete(kind, id). EtcdClient grows a single-keydelete(etcd v3deleterangewithrange_endomitted applies to onlykey).seed.update'd model demonstrably forwards the newmodel_nameto the upstream, and aseed.delete'd api key stops authenticating (fail-closed, polled on the negative condition) while an admin-created control caller keeps serving.Why
Seeding through the same write path production uses in managed mode makes the suite exercise the loader/watcher contract directly, and drops a synchronous HTTP hop per seeded resource. PR #750 pinned seed≡admin equivalence (behavioral + serde round-trip + raw bytes); this PR banks on that pin.
Propagation races the sweep exposed (fixed, not papered over)
Direct writes land faster than admin round-trips, which exposed latent readiness races in ~15 cases that admin-write latency had been masking — e.g. gates that polled
admin.listModels()(a store-level read that never proved DP-snapshot propagation), chat probes without try/catch that aborted on the transitional 401, and astatus >= 400gate that couldn't distinguish "guardrail live (422)" from "caller key not yet propagated (401)". Repairs strengthen the gates; no assertion was weakened:/v1/modelswith the caller bearer — it authenticates only once the key is in the DP snapshot and lists a model only once the snapshot has it, touching no upstream./v1/models, so routing cases either write the router before its targets or seed a throwaway canary api key after the resources under test — once the canary authenticates, everything written before it is in the snapshot (watch events apply in revision order). This never dispatches through the router, so cooldown warm-up and per-target counts stay clean.cache-env-namespace,ratelimit-cluster) now seed the exact root the DP reads (<prefix>/<env_id>/ the shared replica prefix) instead of the harness default.Deliberately NOT migrated
seed-vs-admin-characterization-e2esmoke.test.tsapikey-budget-e2eopenai-sdk-compat.test.ts,datadog-exporter-e2e// Deliberately seeds via the Admin API: deprecation-window coverage.listModels/listModelStatuses, revision GETs, the guardrail store-existence gate, admin-auth probesapikeys/:id/rotateteam-member-ratelimitrawetcd.putVerification
138 files / 288 testsper run (~232s each), against a binary built from currentmain.tsc --noEmit: unchanged — the 5 pre-existing TS18048 warnings, zero new diagnostics..only/.skipintroduced; assertions untouched.🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores