feat(admin): add admin.enabled switch to run without the admin listener - #791

Merged
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch
Jul 21, 2026
Merged

feat(admin): add admin.enabled switch to run without the admin listener#791
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch

Conversation

@moonming

@moonmingmoonming commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

Add an admin.enabled startup-config switch (default true) that lets a standalone gateway run without binding the admin listener, and wire the e2e harness to spawn in that mode. This is the foundation for retiring the Admin API: it makes the admin-listener-off configuration a runnable, tested reality during the coexistence window, before any admin code is deleted.

Why

The gateway already runs admin-less in managed mode (config comes from the control plane over etcd). But in standalone etcd/file mode the admin listener was always bound — there was no way to preview the post-removal world, and the e2e harness had no way to prove the request path works without it. admin.enabled = false closes that gap:

  • Operators can run the admin-free configuration on the coexistence release, de-risking the eventual code removal.
  • The e2e suite gains a spawn mode that exercises the declarative-only path (resources seeded straight to etcd via SeedClient, never the Admin API).

admin.enabled is a process-level startup-config toggle (like proxy.addr), not a per-resource setting, so it carries no control-plane/schema work.

Changes

  • AdminConfig.enabled: bool (crates/aisix-core/src/config.rs), defaults to true. When false, Config::validate no longer requires admin_keys/admin.addr (there is no surface to authenticate), mirroring the existing managed-mode relaxation.
  • Bind gating (crates/aisix-server/src/main.rs): the admin listener is spawned only when its store is present andadmin.enabled. /status/config, /status/ready, /status/models, /metrics (metrics listener) and the proxy /livez are unaffected — the metrics/status listener is bound independently and still reads models through the same store handle, so /status/models keeps working with the admin listener off.
  • Harness (tests/e2e/src/harness/app.ts): AppOverrides.admin?: boolean (default true). With admin: false, the generated config carries admin.enabled = false and readiness gates on the proxy /livez + the metrics listener instead of /admin/v1/health.
  • New e2e (admin-disabled-e2e.test.ts): spawns admin: false, seeds a provider key + model + caller key only through etcd, and asserts (1) a proxy chat request succeeds, (2) /status/config reports the applied config on the metrics listener, (3) the admin port is not bound (connection refused).
  • Config unit tests: enabled defaults to true; enabled: false relaxes the admin_keys requirement.

Verification

  • cargo fmt, cargo clippy -p aisix-core -p aisix-server -p aisix-admin --all-targets -D warnings, cargo test -p aisix-core --lib (config, +2 new) and -p aisix-admin --lib (114) green.
  • New admin-disabled-e2e green (3/3). Regression: allowed-models and seed-vs-admin-characterization (exercises both the admin and etcd-seed paths) green — with admin defaulting to true, the admin-on config + readiness are byte-identical to before, so existing tests are unaffected. Full e2e suite green locally.

Scope / follow-ups (this is P0-1 step 1 of the Admin API removal — AISIX-Cloud#1007 Phase 4)

This PR delivers the switch. It does not yet flip the suite: several tests still seed through AdminClient, and a few deliberately exercise the Admin API (characterization, file-mode 409 rejection, key rotation, auth baseline). Follow-ups:

  1. Migrate the remaining seed-only holdouts (AdminClientSeedClient) and relocate the harness's default readiness probe off /admin/v1/health.
  2. Segregate the Admin-API-testing cases as the held-back set that runs admin-on until removal, and add a suite lane that runs the rest admin-off.
  3. Document admin.enabled in the configuration-files reference (api7/docs).

Summary by CodeRabbit

  • New Features

    • Added an option to disable the admin listener while keeping the gateway operational.
    • Gateway readiness no longer depends on the admin health endpoint when the listener is disabled.
    • Configuration seeded through etcd remains available for requests and status reporting.
  • Bug Fixes

    • Improved configuration validation so admin settings are only required when the admin listener is enabled.
  • Tests

    • Added end-to-end coverage for gateway operation without the admin listener.

A standalone gateway could only run admin-less in managed mode, which
needs the full mTLS/control-plane bootstrap. admin.enabled = false
(default true) lets etcd/file mode skip binding the admin listener too —
the shape the gateway takes once the Admin API is removed. The proxy,
the metrics/status listener, and /status/models are unaffected; the
metrics listener still reads models through the same store handle, and
Config::validate drops the admin_keys/admin.addr requirement when there
is no surface to authenticate.
This is the enabler for retiring the Admin API: it makes the
admin-listener-off configuration runnable and testable during the
coexistence window, before any admin code is deleted. The e2e harness
gains an admin: false spawn mode that seeds resources straight to etcd
(never the Admin API) and gates readiness on the proxy /livez plus the
metrics listener; a new admin-disabled e2e proves an etcd-seeded request
path works with the admin listener off.
@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@moonming, you've reached your PR review limit, so we couldn't start this review.

Next review available in:37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f922993a-9c11-4ee6-a649-49bacc68fa38

📥 Commits

Reviewing files that changed from the base of the PR and between 22f4187 and 34d4d0f.

📒 Files selected for processing (4)
  • crates/aisix-core/src/config.rs
  • crates/aisix-server/src/main.rs
  • tests/e2e/src/cases/admin-disabled-e2e.test.ts
  • tests/e2e/src/harness/app.ts
📝 Walkthrough

Walkthrough

Changes

Admin listener control

Layer / File(s)Summary
Admin enablement configuration and validation
crates/aisix-core/src/config.rs, crates/aisix-admin/src/..., crates/aisix-admin/tests/etcd_integration.rs
AdminConfig.enabled defaults to true; validation skips admin requirements when disabled, and test configurations explicitly enable the admin subsystem.
Conditional admin listener startup
crates/aisix-server/src/main.rs
The server binds the admin listener only when configured and logs whether it is disabled or omitted in managed mode.
Disabled-admin test coverage and harness
tests/e2e/src/harness/app.ts, tests/e2e/src/cases/admin-disabled-e2e.test.ts
The E2E harness supports disabling admin readiness and binding, while tests verify etcd-seeded requests, metrics, and an unavailable admin health endpoint.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant E2ETest
participant AppHarness
participant aisix
participant etcd
participant Upstream
E2ETest->>AppHarness: spawn with admin false
AppHarness->>aisix: generate disabled admin configuration
E2ETest->>etcd: seed gateway configuration
aisix->>Upstream: proxy chat completion
E2ETest->>aisix: query metrics and admin health
Loading

Suggested reviewers:jarvis9443

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
E2e Test Quality Review⚠️ WarningThe suite is order-dependent: only the first test waits for config propagation, while later tests assert /status/config and port refusal without their own readiness.Move waitConfigPropagation into beforeAll after seeding, or call it in each test so each case is independently runnable and not time/order dependent.
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly summarizes the main change: adding an admin.enabled switch to disable the admin listener.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Security Check✅ PassedNo CRITICAL/HIGH/MEDIUM issues found; the change only gates the admin listener and uses derived status views without exposing secrets.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-disable-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes.

Address the cold-audit findings on the admin.enabled switch:
- MEDIUM: file-mode admin-off routes through a distinct store branch
(FileManagedStore) that had no coverage — every new test was etcd-only.
Add a file-mode config unit test and a file-source admin-off e2e leg
(declarative resources.yaml, one proxy request, /status/config reports
the file-loaded model, admin port refused).
- LOW: skip the redundant admin etcd client at boot when admin is off
(is_managed() || !admin.enabled) — /status/models then reads the
snapshot, as in managed mode, and boot no longer fails on a connection
it immediately drops.
- LOW: assert the admin port is specifically ECONNREFUSED, not any throw.
- LOW: assert exact resource_counts (models/provider_keys/api_keys == 1)
under the unique etcd prefix, not >= 1.
- LOW: document that admin-off + prometheus-off reduces readiness to the
proxy /livez.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent cold audit (diverse-lens: correctness / security+breaking / e2e-coverage, 3 agents → synthesis). All findings addressed in f086784.

MEDIUM — file-mode admin-off had zero coverage. File mode binds the admin surface through a distinct store branch (FileManagedStore vs EtcdConfigStore), and every new test was etcd-only; the doc advertises admin-off "even in standalone (etcd or file) mode". Behavior was correct today (all three lenses confirmed the runtime is right and default-preserving), but the file branch had no driver → blocks under the e2e-contract merge gate. Fixed: added admin_disabled_relaxes_admin_key_requirement_in_file_mode config unit test + a file-source admin-off e2e leg (declarative resources.yaml, one proxy request, /status/config reports the file-loaded model, admin port refused).

LOW (batched into the same commit):

  • Redundant admin etcd client connected at boot when admin-off (etcd mode) → guard extended to is_managed() || !admin.enabled; /status/models then reads the snapshot (as in managed mode), and boot no longer risks failing on a connection it immediately drops.
  • Admin-unbound assertion accepted any throw → now asserts the specific ECONNREFUSED cause, so a stray listener or a DNS/abort error can't satisfy it.
  • /status/config count asserted >= 1 → now exact (models/provider_keys/api_keys == 1) under the unique etcd prefix.
  • admin-off + prometheus-off reduces readiness to proxy /livez → documented at the readiness site (latent; the default keeps prometheus on).

The correctness and security lenses independently confirmed enabled = true reproduces prior behavior bit-for-bit (non-breaking) and admin-off drops no security control other than the admin surface itself. Verification: cargo fmt/clippy/aisix-core config (35, +new file-mode) green; admin-disabled-e2e 6/6 (etcd + file legs) green; full suite green earlier (3 transient etcd/ECONNRESET flakes, pass on isolated re-run).

…n-off
Close the last coverage corner the audit noted: the file-mode
FileManagedStore /status/models read was only exercised admin-on. The
file admin-off leg now reads the runtime health view on the metrics
listener and asserts the file-loaded model row — the read surface that
must stay live once the admin listener is gone.
@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(admin): add admin.enabled switch to run without the admin listener - #791

Merged
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch
Jul 21, 2026
Merged

feat(admin): add admin.enabled switch to run without the admin listener#791
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch

Conversation

@moonming

@moonmingmoonming commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

Add an admin.enabled startup-config switch (default true) that lets a standalone gateway run without binding the admin listener, and wire the e2e harness to spawn in that mode. This is the foundation for retiring the Admin API: it makes the admin-listener-off configuration a runnable, tested reality during the coexistence window, before any admin code is deleted.

Why

The gateway already runs admin-less in managed mode (config comes from the control plane over etcd). But in standalone etcd/file mode the admin listener was always bound — there was no way to preview the post-removal world, and the e2e harness had no way to prove the request path works without it. admin.enabled = false closes that gap:

  • Operators can run the admin-free configuration on the coexistence release, de-risking the eventual code removal.
  • The e2e suite gains a spawn mode that exercises the declarative-only path (resources seeded straight to etcd via SeedClient, never the Admin API).

admin.enabled is a process-level startup-config toggle (like proxy.addr), not a per-resource setting, so it carries no control-plane/schema work.

Changes

  • AdminConfig.enabled: bool (crates/aisix-core/src/config.rs), defaults to true. When false, Config::validate no longer requires admin_keys/admin.addr (there is no surface to authenticate), mirroring the existing managed-mode relaxation.
  • Bind gating (crates/aisix-server/src/main.rs): the admin listener is spawned only when its store is present andadmin.enabled. /status/config, /status/ready, /status/models, /metrics (metrics listener) and the proxy /livez are unaffected — the metrics/status listener is bound independently and still reads models through the same store handle, so /status/models keeps working with the admin listener off.
  • Harness (tests/e2e/src/harness/app.ts): AppOverrides.admin?: boolean (default true). With admin: false, the generated config carries admin.enabled = false and readiness gates on the proxy /livez + the metrics listener instead of /admin/v1/health.
  • New e2e (admin-disabled-e2e.test.ts): spawns admin: false, seeds a provider key + model + caller key only through etcd, and asserts (1) a proxy chat request succeeds, (2) /status/config reports the applied config on the metrics listener, (3) the admin port is not bound (connection refused).
  • Config unit tests: enabled defaults to true; enabled: false relaxes the admin_keys requirement.

Verification

  • cargo fmt, cargo clippy -p aisix-core -p aisix-server -p aisix-admin --all-targets -D warnings, cargo test -p aisix-core --lib (config, +2 new) and -p aisix-admin --lib (114) green.
  • New admin-disabled-e2e green (3/3). Regression: allowed-models and seed-vs-admin-characterization (exercises both the admin and etcd-seed paths) green — with admin defaulting to true, the admin-on config + readiness are byte-identical to before, so existing tests are unaffected. Full e2e suite green locally.

Scope / follow-ups (this is P0-1 step 1 of the Admin API removal — AISIX-Cloud#1007 Phase 4)

This PR delivers the switch. It does not yet flip the suite: several tests still seed through AdminClient, and a few deliberately exercise the Admin API (characterization, file-mode 409 rejection, key rotation, auth baseline). Follow-ups:

  1. Migrate the remaining seed-only holdouts (AdminClientSeedClient) and relocate the harness's default readiness probe off /admin/v1/health.
  2. Segregate the Admin-API-testing cases as the held-back set that runs admin-on until removal, and add a suite lane that runs the rest admin-off.
  3. Document admin.enabled in the configuration-files reference (api7/docs).

Summary by CodeRabbit

  • New Features

    • Added an option to disable the admin listener while keeping the gateway operational.
    • Gateway readiness no longer depends on the admin health endpoint when the listener is disabled.
    • Configuration seeded through etcd remains available for requests and status reporting.
  • Bug Fixes

    • Improved configuration validation so admin settings are only required when the admin listener is enabled.
  • Tests

    • Added end-to-end coverage for gateway operation without the admin listener.

A standalone gateway could only run admin-less in managed mode, which
needs the full mTLS/control-plane bootstrap. admin.enabled = false
(default true) lets etcd/file mode skip binding the admin listener too —
the shape the gateway takes once the Admin API is removed. The proxy,
the metrics/status listener, and /status/models are unaffected; the
metrics listener still reads models through the same store handle, and
Config::validate drops the admin_keys/admin.addr requirement when there
is no surface to authenticate.
This is the enabler for retiring the Admin API: it makes the
admin-listener-off configuration runnable and testable during the
coexistence window, before any admin code is deleted. The e2e harness
gains an admin: false spawn mode that seeds resources straight to etcd
(never the Admin API) and gates readiness on the proxy /livez plus the
metrics listener; a new admin-disabled e2e proves an etcd-seeded request
path works with the admin listener off.
@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@moonming, you've reached your PR review limit, so we couldn't start this review.

Next review available in:37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f922993a-9c11-4ee6-a649-49bacc68fa38

📥 Commits

Reviewing files that changed from the base of the PR and between 22f4187 and 34d4d0f.

📒 Files selected for processing (4)
  • crates/aisix-core/src/config.rs
  • crates/aisix-server/src/main.rs
  • tests/e2e/src/cases/admin-disabled-e2e.test.ts
  • tests/e2e/src/harness/app.ts
📝 Walkthrough

Walkthrough

Changes

Admin listener control

Layer / File(s)Summary
Admin enablement configuration and validation
crates/aisix-core/src/config.rs, crates/aisix-admin/src/..., crates/aisix-admin/tests/etcd_integration.rs
AdminConfig.enabled defaults to true; validation skips admin requirements when disabled, and test configurations explicitly enable the admin subsystem.
Conditional admin listener startup
crates/aisix-server/src/main.rs
The server binds the admin listener only when configured and logs whether it is disabled or omitted in managed mode.
Disabled-admin test coverage and harness
tests/e2e/src/harness/app.ts, tests/e2e/src/cases/admin-disabled-e2e.test.ts
The E2E harness supports disabling admin readiness and binding, while tests verify etcd-seeded requests, metrics, and an unavailable admin health endpoint.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant E2ETest
participant AppHarness
participant aisix
participant etcd
participant Upstream
E2ETest->>AppHarness: spawn with admin false
AppHarness->>aisix: generate disabled admin configuration
E2ETest->>etcd: seed gateway configuration
aisix->>Upstream: proxy chat completion
E2ETest->>aisix: query metrics and admin health
Loading

Suggested reviewers:jarvis9443

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
E2e Test Quality Review⚠️ WarningThe suite is order-dependent: only the first test waits for config propagation, while later tests assert /status/config and port refusal without their own readiness.Move waitConfigPropagation into beforeAll after seeding, or call it in each test so each case is independently runnable and not time/order dependent.
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly summarizes the main change: adding an admin.enabled switch to disable the admin listener.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Security Check✅ PassedNo CRITICAL/HIGH/MEDIUM issues found; the change only gates the admin listener and uses derived status views without exposing secrets.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-disable-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes.

Address the cold-audit findings on the admin.enabled switch:
- MEDIUM: file-mode admin-off routes through a distinct store branch
(FileManagedStore) that had no coverage — every new test was etcd-only.
Add a file-mode config unit test and a file-source admin-off e2e leg
(declarative resources.yaml, one proxy request, /status/config reports
the file-loaded model, admin port refused).
- LOW: skip the redundant admin etcd client at boot when admin is off
(is_managed() || !admin.enabled) — /status/models then reads the
snapshot, as in managed mode, and boot no longer fails on a connection
it immediately drops.
- LOW: assert the admin port is specifically ECONNREFUSED, not any throw.
- LOW: assert exact resource_counts (models/provider_keys/api_keys == 1)
under the unique etcd prefix, not >= 1.
- LOW: document that admin-off + prometheus-off reduces readiness to the
proxy /livez.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent cold audit (diverse-lens: correctness / security+breaking / e2e-coverage, 3 agents → synthesis). All findings addressed in f086784.

MEDIUM — file-mode admin-off had zero coverage. File mode binds the admin surface through a distinct store branch (FileManagedStore vs EtcdConfigStore), and every new test was etcd-only; the doc advertises admin-off "even in standalone (etcd or file) mode". Behavior was correct today (all three lenses confirmed the runtime is right and default-preserving), but the file branch had no driver → blocks under the e2e-contract merge gate. Fixed: added admin_disabled_relaxes_admin_key_requirement_in_file_mode config unit test + a file-source admin-off e2e leg (declarative resources.yaml, one proxy request, /status/config reports the file-loaded model, admin port refused).

LOW (batched into the same commit):

  • Redundant admin etcd client connected at boot when admin-off (etcd mode) → guard extended to is_managed() || !admin.enabled; /status/models then reads the snapshot (as in managed mode), and boot no longer risks failing on a connection it immediately drops.
  • Admin-unbound assertion accepted any throw → now asserts the specific ECONNREFUSED cause, so a stray listener or a DNS/abort error can't satisfy it.
  • /status/config count asserted >= 1 → now exact (models/provider_keys/api_keys == 1) under the unique etcd prefix.
  • admin-off + prometheus-off reduces readiness to proxy /livez → documented at the readiness site (latent; the default keeps prometheus on).

The correctness and security lenses independently confirmed enabled = true reproduces prior behavior bit-for-bit (non-breaking) and admin-off drops no security control other than the admin surface itself. Verification: cargo fmt/clippy/aisix-core config (35, +new file-mode) green; admin-disabled-e2e 6/6 (etcd + file legs) green; full suite green earlier (3 transient etcd/ECONNRESET flakes, pass on isolated re-run).

…n-off
Close the last coverage corner the audit noted: the file-mode
FileManagedStore /status/models read was only exercised admin-on. The
file admin-off leg now reads the runtime health view on the metrics
listener and asserts the file-loaded model row — the read surface that
must stay live once the admin listener is gone.
@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(admin): add admin.enabled switch to run without the admin listener - #791

Merged
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch
Jul 21, 2026
Merged

feat(admin): add admin.enabled switch to run without the admin listener#791
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch

Conversation

@moonming

@moonmingmoonming commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

Add an admin.enabled startup-config switch (default true) that lets a standalone gateway run without binding the admin listener, and wire the e2e harness to spawn in that mode. This is the foundation for retiring the Admin API: it makes the admin-listener-off configuration a runnable, tested reality during the coexistence window, before any admin code is deleted.

Why

The gateway already runs admin-less in managed mode (config comes from the control plane over etcd). But in standalone etcd/file mode the admin listener was always bound — there was no way to preview the post-removal world, and the e2e harness had no way to prove the request path works without it. admin.enabled = false closes that gap:

  • Operators can run the admin-free configuration on the coexistence release, de-risking the eventual code removal.
  • The e2e suite gains a spawn mode that exercises the declarative-only path (resources seeded straight to etcd via SeedClient, never the Admin API).

admin.enabled is a process-level startup-config toggle (like proxy.addr), not a per-resource setting, so it carries no control-plane/schema work.

Changes

  • AdminConfig.enabled: bool (crates/aisix-core/src/config.rs), defaults to true. When false, Config::validate no longer requires admin_keys/admin.addr (there is no surface to authenticate), mirroring the existing managed-mode relaxation.
  • Bind gating (crates/aisix-server/src/main.rs): the admin listener is spawned only when its store is present andadmin.enabled. /status/config, /status/ready, /status/models, /metrics (metrics listener) and the proxy /livez are unaffected — the metrics/status listener is bound independently and still reads models through the same store handle, so /status/models keeps working with the admin listener off.
  • Harness (tests/e2e/src/harness/app.ts): AppOverrides.admin?: boolean (default true). With admin: false, the generated config carries admin.enabled = false and readiness gates on the proxy /livez + the metrics listener instead of /admin/v1/health.
  • New e2e (admin-disabled-e2e.test.ts): spawns admin: false, seeds a provider key + model + caller key only through etcd, and asserts (1) a proxy chat request succeeds, (2) /status/config reports the applied config on the metrics listener, (3) the admin port is not bound (connection refused).
  • Config unit tests: enabled defaults to true; enabled: false relaxes the admin_keys requirement.

Verification

  • cargo fmt, cargo clippy -p aisix-core -p aisix-server -p aisix-admin --all-targets -D warnings, cargo test -p aisix-core --lib (config, +2 new) and -p aisix-admin --lib (114) green.
  • New admin-disabled-e2e green (3/3). Regression: allowed-models and seed-vs-admin-characterization (exercises both the admin and etcd-seed paths) green — with admin defaulting to true, the admin-on config + readiness are byte-identical to before, so existing tests are unaffected. Full e2e suite green locally.

Scope / follow-ups (this is P0-1 step 1 of the Admin API removal — AISIX-Cloud#1007 Phase 4)

This PR delivers the switch. It does not yet flip the suite: several tests still seed through AdminClient, and a few deliberately exercise the Admin API (characterization, file-mode 409 rejection, key rotation, auth baseline). Follow-ups:

  1. Migrate the remaining seed-only holdouts (AdminClientSeedClient) and relocate the harness's default readiness probe off /admin/v1/health.
  2. Segregate the Admin-API-testing cases as the held-back set that runs admin-on until removal, and add a suite lane that runs the rest admin-off.
  3. Document admin.enabled in the configuration-files reference (api7/docs).

Summary by CodeRabbit

  • New Features

    • Added an option to disable the admin listener while keeping the gateway operational.
    • Gateway readiness no longer depends on the admin health endpoint when the listener is disabled.
    • Configuration seeded through etcd remains available for requests and status reporting.
  • Bug Fixes

    • Improved configuration validation so admin settings are only required when the admin listener is enabled.
  • Tests

    • Added end-to-end coverage for gateway operation without the admin listener.

A standalone gateway could only run admin-less in managed mode, which
needs the full mTLS/control-plane bootstrap. admin.enabled = false
(default true) lets etcd/file mode skip binding the admin listener too —
the shape the gateway takes once the Admin API is removed. The proxy,
the metrics/status listener, and /status/models are unaffected; the
metrics listener still reads models through the same store handle, and
Config::validate drops the admin_keys/admin.addr requirement when there
is no surface to authenticate.
This is the enabler for retiring the Admin API: it makes the
admin-listener-off configuration runnable and testable during the
coexistence window, before any admin code is deleted. The e2e harness
gains an admin: false spawn mode that seeds resources straight to etcd
(never the Admin API) and gates readiness on the proxy /livez plus the
metrics listener; a new admin-disabled e2e proves an etcd-seeded request
path works with the admin listener off.
@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@moonming, you've reached your PR review limit, so we couldn't start this review.

Next review available in:37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f922993a-9c11-4ee6-a649-49bacc68fa38

📥 Commits

Reviewing files that changed from the base of the PR and between 22f4187 and 34d4d0f.

📒 Files selected for processing (4)
  • crates/aisix-core/src/config.rs
  • crates/aisix-server/src/main.rs
  • tests/e2e/src/cases/admin-disabled-e2e.test.ts
  • tests/e2e/src/harness/app.ts
📝 Walkthrough

Walkthrough

Changes

Admin listener control

Layer / File(s)Summary
Admin enablement configuration and validation
crates/aisix-core/src/config.rs, crates/aisix-admin/src/..., crates/aisix-admin/tests/etcd_integration.rs
AdminConfig.enabled defaults to true; validation skips admin requirements when disabled, and test configurations explicitly enable the admin subsystem.
Conditional admin listener startup
crates/aisix-server/src/main.rs
The server binds the admin listener only when configured and logs whether it is disabled or omitted in managed mode.
Disabled-admin test coverage and harness
tests/e2e/src/harness/app.ts, tests/e2e/src/cases/admin-disabled-e2e.test.ts
The E2E harness supports disabling admin readiness and binding, while tests verify etcd-seeded requests, metrics, and an unavailable admin health endpoint.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant E2ETest
participant AppHarness
participant aisix
participant etcd
participant Upstream
E2ETest->>AppHarness: spawn with admin false
AppHarness->>aisix: generate disabled admin configuration
E2ETest->>etcd: seed gateway configuration
aisix->>Upstream: proxy chat completion
E2ETest->>aisix: query metrics and admin health
Loading

Suggested reviewers:jarvis9443

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
E2e Test Quality Review⚠️ WarningThe suite is order-dependent: only the first test waits for config propagation, while later tests assert /status/config and port refusal without their own readiness.Move waitConfigPropagation into beforeAll after seeding, or call it in each test so each case is independently runnable and not time/order dependent.
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly summarizes the main change: adding an admin.enabled switch to disable the admin listener.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Security Check✅ PassedNo CRITICAL/HIGH/MEDIUM issues found; the change only gates the admin listener and uses derived status views without exposing secrets.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-disable-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes.

Address the cold-audit findings on the admin.enabled switch:
- MEDIUM: file-mode admin-off routes through a distinct store branch
(FileManagedStore) that had no coverage — every new test was etcd-only.
Add a file-mode config unit test and a file-source admin-off e2e leg
(declarative resources.yaml, one proxy request, /status/config reports
the file-loaded model, admin port refused).
- LOW: skip the redundant admin etcd client at boot when admin is off
(is_managed() || !admin.enabled) — /status/models then reads the
snapshot, as in managed mode, and boot no longer fails on a connection
it immediately drops.
- LOW: assert the admin port is specifically ECONNREFUSED, not any throw.
- LOW: assert exact resource_counts (models/provider_keys/api_keys == 1)
under the unique etcd prefix, not >= 1.
- LOW: document that admin-off + prometheus-off reduces readiness to the
proxy /livez.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent cold audit (diverse-lens: correctness / security+breaking / e2e-coverage, 3 agents → synthesis). All findings addressed in f086784.

MEDIUM — file-mode admin-off had zero coverage. File mode binds the admin surface through a distinct store branch (FileManagedStore vs EtcdConfigStore), and every new test was etcd-only; the doc advertises admin-off "even in standalone (etcd or file) mode". Behavior was correct today (all three lenses confirmed the runtime is right and default-preserving), but the file branch had no driver → blocks under the e2e-contract merge gate. Fixed: added admin_disabled_relaxes_admin_key_requirement_in_file_mode config unit test + a file-source admin-off e2e leg (declarative resources.yaml, one proxy request, /status/config reports the file-loaded model, admin port refused).

LOW (batched into the same commit):

  • Redundant admin etcd client connected at boot when admin-off (etcd mode) → guard extended to is_managed() || !admin.enabled; /status/models then reads the snapshot (as in managed mode), and boot no longer risks failing on a connection it immediately drops.
  • Admin-unbound assertion accepted any throw → now asserts the specific ECONNREFUSED cause, so a stray listener or a DNS/abort error can't satisfy it.
  • /status/config count asserted >= 1 → now exact (models/provider_keys/api_keys == 1) under the unique etcd prefix.
  • admin-off + prometheus-off reduces readiness to proxy /livez → documented at the readiness site (latent; the default keeps prometheus on).

The correctness and security lenses independently confirmed enabled = true reproduces prior behavior bit-for-bit (non-breaking) and admin-off drops no security control other than the admin surface itself. Verification: cargo fmt/clippy/aisix-core config (35, +new file-mode) green; admin-disabled-e2e 6/6 (etcd + file legs) green; full suite green earlier (3 transient etcd/ECONNRESET flakes, pass on isolated re-run).

…n-off
Close the last coverage corner the audit noted: the file-mode
FileManagedStore /status/models read was only exercised admin-on. The
file admin-off leg now reads the runtime health view on the metrics
listener and asserts the file-loaded model row — the read surface that
must stay live once the admin listener is gone.
@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(admin): add admin.enabled switch to run without the admin listener - #791

Merged
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch
Jul 21, 2026
Merged

feat(admin): add admin.enabled switch to run without the admin listener#791
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch

Conversation

@moonming

@moonmingmoonming commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

Add an admin.enabled startup-config switch (default true) that lets a standalone gateway run without binding the admin listener, and wire the e2e harness to spawn in that mode. This is the foundation for retiring the Admin API: it makes the admin-listener-off configuration a runnable, tested reality during the coexistence window, before any admin code is deleted.

Why

The gateway already runs admin-less in managed mode (config comes from the control plane over etcd). But in standalone etcd/file mode the admin listener was always bound — there was no way to preview the post-removal world, and the e2e harness had no way to prove the request path works without it. admin.enabled = false closes that gap:

  • Operators can run the admin-free configuration on the coexistence release, de-risking the eventual code removal.
  • The e2e suite gains a spawn mode that exercises the declarative-only path (resources seeded straight to etcd via SeedClient, never the Admin API).

admin.enabled is a process-level startup-config toggle (like proxy.addr), not a per-resource setting, so it carries no control-plane/schema work.

Changes

  • AdminConfig.enabled: bool (crates/aisix-core/src/config.rs), defaults to true. When false, Config::validate no longer requires admin_keys/admin.addr (there is no surface to authenticate), mirroring the existing managed-mode relaxation.
  • Bind gating (crates/aisix-server/src/main.rs): the admin listener is spawned only when its store is present andadmin.enabled. /status/config, /status/ready, /status/models, /metrics (metrics listener) and the proxy /livez are unaffected — the metrics/status listener is bound independently and still reads models through the same store handle, so /status/models keeps working with the admin listener off.
  • Harness (tests/e2e/src/harness/app.ts): AppOverrides.admin?: boolean (default true). With admin: false, the generated config carries admin.enabled = false and readiness gates on the proxy /livez + the metrics listener instead of /admin/v1/health.
  • New e2e (admin-disabled-e2e.test.ts): spawns admin: false, seeds a provider key + model + caller key only through etcd, and asserts (1) a proxy chat request succeeds, (2) /status/config reports the applied config on the metrics listener, (3) the admin port is not bound (connection refused).
  • Config unit tests: enabled defaults to true; enabled: false relaxes the admin_keys requirement.

Verification

  • cargo fmt, cargo clippy -p aisix-core -p aisix-server -p aisix-admin --all-targets -D warnings, cargo test -p aisix-core --lib (config, +2 new) and -p aisix-admin --lib (114) green.
  • New admin-disabled-e2e green (3/3). Regression: allowed-models and seed-vs-admin-characterization (exercises both the admin and etcd-seed paths) green — with admin defaulting to true, the admin-on config + readiness are byte-identical to before, so existing tests are unaffected. Full e2e suite green locally.

Scope / follow-ups (this is P0-1 step 1 of the Admin API removal — AISIX-Cloud#1007 Phase 4)

This PR delivers the switch. It does not yet flip the suite: several tests still seed through AdminClient, and a few deliberately exercise the Admin API (characterization, file-mode 409 rejection, key rotation, auth baseline). Follow-ups:

  1. Migrate the remaining seed-only holdouts (AdminClientSeedClient) and relocate the harness's default readiness probe off /admin/v1/health.
  2. Segregate the Admin-API-testing cases as the held-back set that runs admin-on until removal, and add a suite lane that runs the rest admin-off.
  3. Document admin.enabled in the configuration-files reference (api7/docs).

Summary by CodeRabbit

  • New Features

    • Added an option to disable the admin listener while keeping the gateway operational.
    • Gateway readiness no longer depends on the admin health endpoint when the listener is disabled.
    • Configuration seeded through etcd remains available for requests and status reporting.
  • Bug Fixes

    • Improved configuration validation so admin settings are only required when the admin listener is enabled.
  • Tests

    • Added end-to-end coverage for gateway operation without the admin listener.

A standalone gateway could only run admin-less in managed mode, which
needs the full mTLS/control-plane bootstrap. admin.enabled = false
(default true) lets etcd/file mode skip binding the admin listener too —
the shape the gateway takes once the Admin API is removed. The proxy,
the metrics/status listener, and /status/models are unaffected; the
metrics listener still reads models through the same store handle, and
Config::validate drops the admin_keys/admin.addr requirement when there
is no surface to authenticate.
This is the enabler for retiring the Admin API: it makes the
admin-listener-off configuration runnable and testable during the
coexistence window, before any admin code is deleted. The e2e harness
gains an admin: false spawn mode that seeds resources straight to etcd
(never the Admin API) and gates readiness on the proxy /livez plus the
metrics listener; a new admin-disabled e2e proves an etcd-seeded request
path works with the admin listener off.
@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@moonming, you've reached your PR review limit, so we couldn't start this review.

Next review available in:37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f922993a-9c11-4ee6-a649-49bacc68fa38

📥 Commits

Reviewing files that changed from the base of the PR and between 22f4187 and 34d4d0f.

📒 Files selected for processing (4)
  • crates/aisix-core/src/config.rs
  • crates/aisix-server/src/main.rs
  • tests/e2e/src/cases/admin-disabled-e2e.test.ts
  • tests/e2e/src/harness/app.ts
📝 Walkthrough

Walkthrough

Changes

Admin listener control

Layer / File(s)Summary
Admin enablement configuration and validation
crates/aisix-core/src/config.rs, crates/aisix-admin/src/..., crates/aisix-admin/tests/etcd_integration.rs
AdminConfig.enabled defaults to true; validation skips admin requirements when disabled, and test configurations explicitly enable the admin subsystem.
Conditional admin listener startup
crates/aisix-server/src/main.rs
The server binds the admin listener only when configured and logs whether it is disabled or omitted in managed mode.
Disabled-admin test coverage and harness
tests/e2e/src/harness/app.ts, tests/e2e/src/cases/admin-disabled-e2e.test.ts
The E2E harness supports disabling admin readiness and binding, while tests verify etcd-seeded requests, metrics, and an unavailable admin health endpoint.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant E2ETest
participant AppHarness
participant aisix
participant etcd
participant Upstream
E2ETest->>AppHarness: spawn with admin false
AppHarness->>aisix: generate disabled admin configuration
E2ETest->>etcd: seed gateway configuration
aisix->>Upstream: proxy chat completion
E2ETest->>aisix: query metrics and admin health
Loading

Suggested reviewers:jarvis9443

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
E2e Test Quality Review⚠️ WarningThe suite is order-dependent: only the first test waits for config propagation, while later tests assert /status/config and port refusal without their own readiness.Move waitConfigPropagation into beforeAll after seeding, or call it in each test so each case is independently runnable and not time/order dependent.
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly summarizes the main change: adding an admin.enabled switch to disable the admin listener.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Security Check✅ PassedNo CRITICAL/HIGH/MEDIUM issues found; the change only gates the admin listener and uses derived status views without exposing secrets.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-disable-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes.

Address the cold-audit findings on the admin.enabled switch:
- MEDIUM: file-mode admin-off routes through a distinct store branch
(FileManagedStore) that had no coverage — every new test was etcd-only.
Add a file-mode config unit test and a file-source admin-off e2e leg
(declarative resources.yaml, one proxy request, /status/config reports
the file-loaded model, admin port refused).
- LOW: skip the redundant admin etcd client at boot when admin is off
(is_managed() || !admin.enabled) — /status/models then reads the
snapshot, as in managed mode, and boot no longer fails on a connection
it immediately drops.
- LOW: assert the admin port is specifically ECONNREFUSED, not any throw.
- LOW: assert exact resource_counts (models/provider_keys/api_keys == 1)
under the unique etcd prefix, not >= 1.
- LOW: document that admin-off + prometheus-off reduces readiness to the
proxy /livez.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent cold audit (diverse-lens: correctness / security+breaking / e2e-coverage, 3 agents → synthesis). All findings addressed in f086784.

MEDIUM — file-mode admin-off had zero coverage. File mode binds the admin surface through a distinct store branch (FileManagedStore vs EtcdConfigStore), and every new test was etcd-only; the doc advertises admin-off "even in standalone (etcd or file) mode". Behavior was correct today (all three lenses confirmed the runtime is right and default-preserving), but the file branch had no driver → blocks under the e2e-contract merge gate. Fixed: added admin_disabled_relaxes_admin_key_requirement_in_file_mode config unit test + a file-source admin-off e2e leg (declarative resources.yaml, one proxy request, /status/config reports the file-loaded model, admin port refused).

LOW (batched into the same commit):

  • Redundant admin etcd client connected at boot when admin-off (etcd mode) → guard extended to is_managed() || !admin.enabled; /status/models then reads the snapshot (as in managed mode), and boot no longer risks failing on a connection it immediately drops.
  • Admin-unbound assertion accepted any throw → now asserts the specific ECONNREFUSED cause, so a stray listener or a DNS/abort error can't satisfy it.
  • /status/config count asserted >= 1 → now exact (models/provider_keys/api_keys == 1) under the unique etcd prefix.
  • admin-off + prometheus-off reduces readiness to proxy /livez → documented at the readiness site (latent; the default keeps prometheus on).

The correctness and security lenses independently confirmed enabled = true reproduces prior behavior bit-for-bit (non-breaking) and admin-off drops no security control other than the admin surface itself. Verification: cargo fmt/clippy/aisix-core config (35, +new file-mode) green; admin-disabled-e2e 6/6 (etcd + file legs) green; full suite green earlier (3 transient etcd/ECONNRESET flakes, pass on isolated re-run).

…n-off
Close the last coverage corner the audit noted: the file-mode
FileManagedStore /status/models read was only exercised admin-on. The
file admin-off leg now reads the runtime health view on the metrics
listener and asserts the file-loaded model row — the read surface that
must stay live once the admin listener is gone.
@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(admin): add admin.enabled switch to run without the admin listener - #791

Merged
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch
Jul 21, 2026
Merged

feat(admin): add admin.enabled switch to run without the admin listener#791
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch

Conversation

@moonming

@moonmingmoonming commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

Add an admin.enabled startup-config switch (default true) that lets a standalone gateway run without binding the admin listener, and wire the e2e harness to spawn in that mode. This is the foundation for retiring the Admin API: it makes the admin-listener-off configuration a runnable, tested reality during the coexistence window, before any admin code is deleted.

Why

The gateway already runs admin-less in managed mode (config comes from the control plane over etcd). But in standalone etcd/file mode the admin listener was always bound — there was no way to preview the post-removal world, and the e2e harness had no way to prove the request path works without it. admin.enabled = false closes that gap:

  • Operators can run the admin-free configuration on the coexistence release, de-risking the eventual code removal.
  • The e2e suite gains a spawn mode that exercises the declarative-only path (resources seeded straight to etcd via SeedClient, never the Admin API).

admin.enabled is a process-level startup-config toggle (like proxy.addr), not a per-resource setting, so it carries no control-plane/schema work.

Changes

  • AdminConfig.enabled: bool (crates/aisix-core/src/config.rs), defaults to true. When false, Config::validate no longer requires admin_keys/admin.addr (there is no surface to authenticate), mirroring the existing managed-mode relaxation.
  • Bind gating (crates/aisix-server/src/main.rs): the admin listener is spawned only when its store is present andadmin.enabled. /status/config, /status/ready, /status/models, /metrics (metrics listener) and the proxy /livez are unaffected — the metrics/status listener is bound independently and still reads models through the same store handle, so /status/models keeps working with the admin listener off.
  • Harness (tests/e2e/src/harness/app.ts): AppOverrides.admin?: boolean (default true). With admin: false, the generated config carries admin.enabled = false and readiness gates on the proxy /livez + the metrics listener instead of /admin/v1/health.
  • New e2e (admin-disabled-e2e.test.ts): spawns admin: false, seeds a provider key + model + caller key only through etcd, and asserts (1) a proxy chat request succeeds, (2) /status/config reports the applied config on the metrics listener, (3) the admin port is not bound (connection refused).
  • Config unit tests: enabled defaults to true; enabled: false relaxes the admin_keys requirement.

Verification

  • cargo fmt, cargo clippy -p aisix-core -p aisix-server -p aisix-admin --all-targets -D warnings, cargo test -p aisix-core --lib (config, +2 new) and -p aisix-admin --lib (114) green.
  • New admin-disabled-e2e green (3/3). Regression: allowed-models and seed-vs-admin-characterization (exercises both the admin and etcd-seed paths) green — with admin defaulting to true, the admin-on config + readiness are byte-identical to before, so existing tests are unaffected. Full e2e suite green locally.

Scope / follow-ups (this is P0-1 step 1 of the Admin API removal — AISIX-Cloud#1007 Phase 4)

This PR delivers the switch. It does not yet flip the suite: several tests still seed through AdminClient, and a few deliberately exercise the Admin API (characterization, file-mode 409 rejection, key rotation, auth baseline). Follow-ups:

  1. Migrate the remaining seed-only holdouts (AdminClientSeedClient) and relocate the harness's default readiness probe off /admin/v1/health.
  2. Segregate the Admin-API-testing cases as the held-back set that runs admin-on until removal, and add a suite lane that runs the rest admin-off.
  3. Document admin.enabled in the configuration-files reference (api7/docs).

Summary by CodeRabbit

  • New Features

    • Added an option to disable the admin listener while keeping the gateway operational.
    • Gateway readiness no longer depends on the admin health endpoint when the listener is disabled.
    • Configuration seeded through etcd remains available for requests and status reporting.
  • Bug Fixes

    • Improved configuration validation so admin settings are only required when the admin listener is enabled.
  • Tests

    • Added end-to-end coverage for gateway operation without the admin listener.

A standalone gateway could only run admin-less in managed mode, which
needs the full mTLS/control-plane bootstrap. admin.enabled = false
(default true) lets etcd/file mode skip binding the admin listener too —
the shape the gateway takes once the Admin API is removed. The proxy,
the metrics/status listener, and /status/models are unaffected; the
metrics listener still reads models through the same store handle, and
Config::validate drops the admin_keys/admin.addr requirement when there
is no surface to authenticate.
This is the enabler for retiring the Admin API: it makes the
admin-listener-off configuration runnable and testable during the
coexistence window, before any admin code is deleted. The e2e harness
gains an admin: false spawn mode that seeds resources straight to etcd
(never the Admin API) and gates readiness on the proxy /livez plus the
metrics listener; a new admin-disabled e2e proves an etcd-seeded request
path works with the admin listener off.
@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@moonming, you've reached your PR review limit, so we couldn't start this review.

Next review available in:37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f922993a-9c11-4ee6-a649-49bacc68fa38

📥 Commits

Reviewing files that changed from the base of the PR and between 22f4187 and 34d4d0f.

📒 Files selected for processing (4)
  • crates/aisix-core/src/config.rs
  • crates/aisix-server/src/main.rs
  • tests/e2e/src/cases/admin-disabled-e2e.test.ts
  • tests/e2e/src/harness/app.ts
📝 Walkthrough

Walkthrough

Changes

Admin listener control

Layer / File(s)Summary
Admin enablement configuration and validation
crates/aisix-core/src/config.rs, crates/aisix-admin/src/..., crates/aisix-admin/tests/etcd_integration.rs
AdminConfig.enabled defaults to true; validation skips admin requirements when disabled, and test configurations explicitly enable the admin subsystem.
Conditional admin listener startup
crates/aisix-server/src/main.rs
The server binds the admin listener only when configured and logs whether it is disabled or omitted in managed mode.
Disabled-admin test coverage and harness
tests/e2e/src/harness/app.ts, tests/e2e/src/cases/admin-disabled-e2e.test.ts
The E2E harness supports disabling admin readiness and binding, while tests verify etcd-seeded requests, metrics, and an unavailable admin health endpoint.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant E2ETest
participant AppHarness
participant aisix
participant etcd
participant Upstream
E2ETest->>AppHarness: spawn with admin false
AppHarness->>aisix: generate disabled admin configuration
E2ETest->>etcd: seed gateway configuration
aisix->>Upstream: proxy chat completion
E2ETest->>aisix: query metrics and admin health
Loading

Suggested reviewers:jarvis9443

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
E2e Test Quality Review⚠️ WarningThe suite is order-dependent: only the first test waits for config propagation, while later tests assert /status/config and port refusal without their own readiness.Move waitConfigPropagation into beforeAll after seeding, or call it in each test so each case is independently runnable and not time/order dependent.
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly summarizes the main change: adding an admin.enabled switch to disable the admin listener.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Security Check✅ PassedNo CRITICAL/HIGH/MEDIUM issues found; the change only gates the admin listener and uses derived status views without exposing secrets.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-disable-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes.

Address the cold-audit findings on the admin.enabled switch:
- MEDIUM: file-mode admin-off routes through a distinct store branch
(FileManagedStore) that had no coverage — every new test was etcd-only.
Add a file-mode config unit test and a file-source admin-off e2e leg
(declarative resources.yaml, one proxy request, /status/config reports
the file-loaded model, admin port refused).
- LOW: skip the redundant admin etcd client at boot when admin is off
(is_managed() || !admin.enabled) — /status/models then reads the
snapshot, as in managed mode, and boot no longer fails on a connection
it immediately drops.
- LOW: assert the admin port is specifically ECONNREFUSED, not any throw.
- LOW: assert exact resource_counts (models/provider_keys/api_keys == 1)
under the unique etcd prefix, not >= 1.
- LOW: document that admin-off + prometheus-off reduces readiness to the
proxy /livez.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent cold audit (diverse-lens: correctness / security+breaking / e2e-coverage, 3 agents → synthesis). All findings addressed in f086784.

MEDIUM — file-mode admin-off had zero coverage. File mode binds the admin surface through a distinct store branch (FileManagedStore vs EtcdConfigStore), and every new test was etcd-only; the doc advertises admin-off "even in standalone (etcd or file) mode". Behavior was correct today (all three lenses confirmed the runtime is right and default-preserving), but the file branch had no driver → blocks under the e2e-contract merge gate. Fixed: added admin_disabled_relaxes_admin_key_requirement_in_file_mode config unit test + a file-source admin-off e2e leg (declarative resources.yaml, one proxy request, /status/config reports the file-loaded model, admin port refused).

LOW (batched into the same commit):

  • Redundant admin etcd client connected at boot when admin-off (etcd mode) → guard extended to is_managed() || !admin.enabled; /status/models then reads the snapshot (as in managed mode), and boot no longer risks failing on a connection it immediately drops.
  • Admin-unbound assertion accepted any throw → now asserts the specific ECONNREFUSED cause, so a stray listener or a DNS/abort error can't satisfy it.
  • /status/config count asserted >= 1 → now exact (models/provider_keys/api_keys == 1) under the unique etcd prefix.
  • admin-off + prometheus-off reduces readiness to proxy /livez → documented at the readiness site (latent; the default keeps prometheus on).

The correctness and security lenses independently confirmed enabled = true reproduces prior behavior bit-for-bit (non-breaking) and admin-off drops no security control other than the admin surface itself. Verification: cargo fmt/clippy/aisix-core config (35, +new file-mode) green; admin-disabled-e2e 6/6 (etcd + file legs) green; full suite green earlier (3 transient etcd/ECONNRESET flakes, pass on isolated re-run).

…n-off
Close the last coverage corner the audit noted: the file-mode
FileManagedStore /status/models read was only exercised admin-on. The
file admin-off leg now reads the runtime health view on the metrics
listener and asserts the file-loaded model row — the read surface that
must stay live once the admin listener is gone.
@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(admin): add admin.enabled switch to run without the admin listener - #791

Merged
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch
Jul 21, 2026
Merged

feat(admin): add admin.enabled switch to run without the admin listener#791
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch

Conversation

@moonming

@moonmingmoonming commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

Add an admin.enabled startup-config switch (default true) that lets a standalone gateway run without binding the admin listener, and wire the e2e harness to spawn in that mode. This is the foundation for retiring the Admin API: it makes the admin-listener-off configuration a runnable, tested reality during the coexistence window, before any admin code is deleted.

Why

The gateway already runs admin-less in managed mode (config comes from the control plane over etcd). But in standalone etcd/file mode the admin listener was always bound — there was no way to preview the post-removal world, and the e2e harness had no way to prove the request path works without it. admin.enabled = false closes that gap:

  • Operators can run the admin-free configuration on the coexistence release, de-risking the eventual code removal.
  • The e2e suite gains a spawn mode that exercises the declarative-only path (resources seeded straight to etcd via SeedClient, never the Admin API).

admin.enabled is a process-level startup-config toggle (like proxy.addr), not a per-resource setting, so it carries no control-plane/schema work.

Changes

  • AdminConfig.enabled: bool (crates/aisix-core/src/config.rs), defaults to true. When false, Config::validate no longer requires admin_keys/admin.addr (there is no surface to authenticate), mirroring the existing managed-mode relaxation.
  • Bind gating (crates/aisix-server/src/main.rs): the admin listener is spawned only when its store is present andadmin.enabled. /status/config, /status/ready, /status/models, /metrics (metrics listener) and the proxy /livez are unaffected — the metrics/status listener is bound independently and still reads models through the same store handle, so /status/models keeps working with the admin listener off.
  • Harness (tests/e2e/src/harness/app.ts): AppOverrides.admin?: boolean (default true). With admin: false, the generated config carries admin.enabled = false and readiness gates on the proxy /livez + the metrics listener instead of /admin/v1/health.
  • New e2e (admin-disabled-e2e.test.ts): spawns admin: false, seeds a provider key + model + caller key only through etcd, and asserts (1) a proxy chat request succeeds, (2) /status/config reports the applied config on the metrics listener, (3) the admin port is not bound (connection refused).
  • Config unit tests: enabled defaults to true; enabled: false relaxes the admin_keys requirement.

Verification

  • cargo fmt, cargo clippy -p aisix-core -p aisix-server -p aisix-admin --all-targets -D warnings, cargo test -p aisix-core --lib (config, +2 new) and -p aisix-admin --lib (114) green.
  • New admin-disabled-e2e green (3/3). Regression: allowed-models and seed-vs-admin-characterization (exercises both the admin and etcd-seed paths) green — with admin defaulting to true, the admin-on config + readiness are byte-identical to before, so existing tests are unaffected. Full e2e suite green locally.

Scope / follow-ups (this is P0-1 step 1 of the Admin API removal — AISIX-Cloud#1007 Phase 4)

This PR delivers the switch. It does not yet flip the suite: several tests still seed through AdminClient, and a few deliberately exercise the Admin API (characterization, file-mode 409 rejection, key rotation, auth baseline). Follow-ups:

  1. Migrate the remaining seed-only holdouts (AdminClientSeedClient) and relocate the harness's default readiness probe off /admin/v1/health.
  2. Segregate the Admin-API-testing cases as the held-back set that runs admin-on until removal, and add a suite lane that runs the rest admin-off.
  3. Document admin.enabled in the configuration-files reference (api7/docs).

Summary by CodeRabbit

  • New Features

    • Added an option to disable the admin listener while keeping the gateway operational.
    • Gateway readiness no longer depends on the admin health endpoint when the listener is disabled.
    • Configuration seeded through etcd remains available for requests and status reporting.
  • Bug Fixes

    • Improved configuration validation so admin settings are only required when the admin listener is enabled.
  • Tests

    • Added end-to-end coverage for gateway operation without the admin listener.

A standalone gateway could only run admin-less in managed mode, which
needs the full mTLS/control-plane bootstrap. admin.enabled = false
(default true) lets etcd/file mode skip binding the admin listener too —
the shape the gateway takes once the Admin API is removed. The proxy,
the metrics/status listener, and /status/models are unaffected; the
metrics listener still reads models through the same store handle, and
Config::validate drops the admin_keys/admin.addr requirement when there
is no surface to authenticate.
This is the enabler for retiring the Admin API: it makes the
admin-listener-off configuration runnable and testable during the
coexistence window, before any admin code is deleted. The e2e harness
gains an admin: false spawn mode that seeds resources straight to etcd
(never the Admin API) and gates readiness on the proxy /livez plus the
metrics listener; a new admin-disabled e2e proves an etcd-seeded request
path works with the admin listener off.
@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@moonming, you've reached your PR review limit, so we couldn't start this review.

Next review available in:37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f922993a-9c11-4ee6-a649-49bacc68fa38

📥 Commits

Reviewing files that changed from the base of the PR and between 22f4187 and 34d4d0f.

📒 Files selected for processing (4)
  • crates/aisix-core/src/config.rs
  • crates/aisix-server/src/main.rs
  • tests/e2e/src/cases/admin-disabled-e2e.test.ts
  • tests/e2e/src/harness/app.ts
📝 Walkthrough

Walkthrough

Changes

Admin listener control

Layer / File(s)Summary
Admin enablement configuration and validation
crates/aisix-core/src/config.rs, crates/aisix-admin/src/..., crates/aisix-admin/tests/etcd_integration.rs
AdminConfig.enabled defaults to true; validation skips admin requirements when disabled, and test configurations explicitly enable the admin subsystem.
Conditional admin listener startup
crates/aisix-server/src/main.rs
The server binds the admin listener only when configured and logs whether it is disabled or omitted in managed mode.
Disabled-admin test coverage and harness
tests/e2e/src/harness/app.ts, tests/e2e/src/cases/admin-disabled-e2e.test.ts
The E2E harness supports disabling admin readiness and binding, while tests verify etcd-seeded requests, metrics, and an unavailable admin health endpoint.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant E2ETest
participant AppHarness
participant aisix
participant etcd
participant Upstream
E2ETest->>AppHarness: spawn with admin false
AppHarness->>aisix: generate disabled admin configuration
E2ETest->>etcd: seed gateway configuration
aisix->>Upstream: proxy chat completion
E2ETest->>aisix: query metrics and admin health
Loading

Suggested reviewers:jarvis9443

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
E2e Test Quality Review⚠️ WarningThe suite is order-dependent: only the first test waits for config propagation, while later tests assert /status/config and port refusal without their own readiness.Move waitConfigPropagation into beforeAll after seeding, or call it in each test so each case is independently runnable and not time/order dependent.
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly summarizes the main change: adding an admin.enabled switch to disable the admin listener.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Security Check✅ PassedNo CRITICAL/HIGH/MEDIUM issues found; the change only gates the admin listener and uses derived status views without exposing secrets.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-disable-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes.

Address the cold-audit findings on the admin.enabled switch:
- MEDIUM: file-mode admin-off routes through a distinct store branch
(FileManagedStore) that had no coverage — every new test was etcd-only.
Add a file-mode config unit test and a file-source admin-off e2e leg
(declarative resources.yaml, one proxy request, /status/config reports
the file-loaded model, admin port refused).
- LOW: skip the redundant admin etcd client at boot when admin is off
(is_managed() || !admin.enabled) — /status/models then reads the
snapshot, as in managed mode, and boot no longer fails on a connection
it immediately drops.
- LOW: assert the admin port is specifically ECONNREFUSED, not any throw.
- LOW: assert exact resource_counts (models/provider_keys/api_keys == 1)
under the unique etcd prefix, not >= 1.
- LOW: document that admin-off + prometheus-off reduces readiness to the
proxy /livez.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent cold audit (diverse-lens: correctness / security+breaking / e2e-coverage, 3 agents → synthesis). All findings addressed in f086784.

MEDIUM — file-mode admin-off had zero coverage. File mode binds the admin surface through a distinct store branch (FileManagedStore vs EtcdConfigStore), and every new test was etcd-only; the doc advertises admin-off "even in standalone (etcd or file) mode". Behavior was correct today (all three lenses confirmed the runtime is right and default-preserving), but the file branch had no driver → blocks under the e2e-contract merge gate. Fixed: added admin_disabled_relaxes_admin_key_requirement_in_file_mode config unit test + a file-source admin-off e2e leg (declarative resources.yaml, one proxy request, /status/config reports the file-loaded model, admin port refused).

LOW (batched into the same commit):

  • Redundant admin etcd client connected at boot when admin-off (etcd mode) → guard extended to is_managed() || !admin.enabled; /status/models then reads the snapshot (as in managed mode), and boot no longer risks failing on a connection it immediately drops.
  • Admin-unbound assertion accepted any throw → now asserts the specific ECONNREFUSED cause, so a stray listener or a DNS/abort error can't satisfy it.
  • /status/config count asserted >= 1 → now exact (models/provider_keys/api_keys == 1) under the unique etcd prefix.
  • admin-off + prometheus-off reduces readiness to proxy /livez → documented at the readiness site (latent; the default keeps prometheus on).

The correctness and security lenses independently confirmed enabled = true reproduces prior behavior bit-for-bit (non-breaking) and admin-off drops no security control other than the admin surface itself. Verification: cargo fmt/clippy/aisix-core config (35, +new file-mode) green; admin-disabled-e2e 6/6 (etcd + file legs) green; full suite green earlier (3 transient etcd/ECONNRESET flakes, pass on isolated re-run).

…n-off
Close the last coverage corner the audit noted: the file-mode
FileManagedStore /status/models read was only exercised admin-on. The
file admin-off leg now reads the runtime health view on the metrics
listener and asserts the file-loaded model row — the read surface that
must stay live once the admin listener is gone.
@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(admin): add admin.enabled switch to run without the admin listener - #791

Merged
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch
Jul 21, 2026
Merged

feat(admin): add admin.enabled switch to run without the admin listener#791
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch

Conversation

@moonming

@moonmingmoonming commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

Add an admin.enabled startup-config switch (default true) that lets a standalone gateway run without binding the admin listener, and wire the e2e harness to spawn in that mode. This is the foundation for retiring the Admin API: it makes the admin-listener-off configuration a runnable, tested reality during the coexistence window, before any admin code is deleted.

Why

The gateway already runs admin-less in managed mode (config comes from the control plane over etcd). But in standalone etcd/file mode the admin listener was always bound — there was no way to preview the post-removal world, and the e2e harness had no way to prove the request path works without it. admin.enabled = false closes that gap:

  • Operators can run the admin-free configuration on the coexistence release, de-risking the eventual code removal.
  • The e2e suite gains a spawn mode that exercises the declarative-only path (resources seeded straight to etcd via SeedClient, never the Admin API).

admin.enabled is a process-level startup-config toggle (like proxy.addr), not a per-resource setting, so it carries no control-plane/schema work.

Changes

  • AdminConfig.enabled: bool (crates/aisix-core/src/config.rs), defaults to true. When false, Config::validate no longer requires admin_keys/admin.addr (there is no surface to authenticate), mirroring the existing managed-mode relaxation.
  • Bind gating (crates/aisix-server/src/main.rs): the admin listener is spawned only when its store is present andadmin.enabled. /status/config, /status/ready, /status/models, /metrics (metrics listener) and the proxy /livez are unaffected — the metrics/status listener is bound independently and still reads models through the same store handle, so /status/models keeps working with the admin listener off.
  • Harness (tests/e2e/src/harness/app.ts): AppOverrides.admin?: boolean (default true). With admin: false, the generated config carries admin.enabled = false and readiness gates on the proxy /livez + the metrics listener instead of /admin/v1/health.
  • New e2e (admin-disabled-e2e.test.ts): spawns admin: false, seeds a provider key + model + caller key only through etcd, and asserts (1) a proxy chat request succeeds, (2) /status/config reports the applied config on the metrics listener, (3) the admin port is not bound (connection refused).
  • Config unit tests: enabled defaults to true; enabled: false relaxes the admin_keys requirement.

Verification

  • cargo fmt, cargo clippy -p aisix-core -p aisix-server -p aisix-admin --all-targets -D warnings, cargo test -p aisix-core --lib (config, +2 new) and -p aisix-admin --lib (114) green.
  • New admin-disabled-e2e green (3/3). Regression: allowed-models and seed-vs-admin-characterization (exercises both the admin and etcd-seed paths) green — with admin defaulting to true, the admin-on config + readiness are byte-identical to before, so existing tests are unaffected. Full e2e suite green locally.

Scope / follow-ups (this is P0-1 step 1 of the Admin API removal — AISIX-Cloud#1007 Phase 4)

This PR delivers the switch. It does not yet flip the suite: several tests still seed through AdminClient, and a few deliberately exercise the Admin API (characterization, file-mode 409 rejection, key rotation, auth baseline). Follow-ups:

  1. Migrate the remaining seed-only holdouts (AdminClientSeedClient) and relocate the harness's default readiness probe off /admin/v1/health.
  2. Segregate the Admin-API-testing cases as the held-back set that runs admin-on until removal, and add a suite lane that runs the rest admin-off.
  3. Document admin.enabled in the configuration-files reference (api7/docs).

Summary by CodeRabbit

  • New Features

    • Added an option to disable the admin listener while keeping the gateway operational.
    • Gateway readiness no longer depends on the admin health endpoint when the listener is disabled.
    • Configuration seeded through etcd remains available for requests and status reporting.
  • Bug Fixes

    • Improved configuration validation so admin settings are only required when the admin listener is enabled.
  • Tests

    • Added end-to-end coverage for gateway operation without the admin listener.

A standalone gateway could only run admin-less in managed mode, which
needs the full mTLS/control-plane bootstrap. admin.enabled = false
(default true) lets etcd/file mode skip binding the admin listener too —
the shape the gateway takes once the Admin API is removed. The proxy,
the metrics/status listener, and /status/models are unaffected; the
metrics listener still reads models through the same store handle, and
Config::validate drops the admin_keys/admin.addr requirement when there
is no surface to authenticate.
This is the enabler for retiring the Admin API: it makes the
admin-listener-off configuration runnable and testable during the
coexistence window, before any admin code is deleted. The e2e harness
gains an admin: false spawn mode that seeds resources straight to etcd
(never the Admin API) and gates readiness on the proxy /livez plus the
metrics listener; a new admin-disabled e2e proves an etcd-seeded request
path works with the admin listener off.
@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@moonming, you've reached your PR review limit, so we couldn't start this review.

Next review available in:37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f922993a-9c11-4ee6-a649-49bacc68fa38

📥 Commits

Reviewing files that changed from the base of the PR and between 22f4187 and 34d4d0f.

📒 Files selected for processing (4)
  • crates/aisix-core/src/config.rs
  • crates/aisix-server/src/main.rs
  • tests/e2e/src/cases/admin-disabled-e2e.test.ts
  • tests/e2e/src/harness/app.ts
📝 Walkthrough

Walkthrough

Changes

Admin listener control

Layer / File(s)Summary
Admin enablement configuration and validation
crates/aisix-core/src/config.rs, crates/aisix-admin/src/..., crates/aisix-admin/tests/etcd_integration.rs
AdminConfig.enabled defaults to true; validation skips admin requirements when disabled, and test configurations explicitly enable the admin subsystem.
Conditional admin listener startup
crates/aisix-server/src/main.rs
The server binds the admin listener only when configured and logs whether it is disabled or omitted in managed mode.
Disabled-admin test coverage and harness
tests/e2e/src/harness/app.ts, tests/e2e/src/cases/admin-disabled-e2e.test.ts
The E2E harness supports disabling admin readiness and binding, while tests verify etcd-seeded requests, metrics, and an unavailable admin health endpoint.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant E2ETest
participant AppHarness
participant aisix
participant etcd
participant Upstream
E2ETest->>AppHarness: spawn with admin false
AppHarness->>aisix: generate disabled admin configuration
E2ETest->>etcd: seed gateway configuration
aisix->>Upstream: proxy chat completion
E2ETest->>aisix: query metrics and admin health
Loading

Suggested reviewers:jarvis9443

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
E2e Test Quality Review⚠️ WarningThe suite is order-dependent: only the first test waits for config propagation, while later tests assert /status/config and port refusal without their own readiness.Move waitConfigPropagation into beforeAll after seeding, or call it in each test so each case is independently runnable and not time/order dependent.
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly summarizes the main change: adding an admin.enabled switch to disable the admin listener.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Security Check✅ PassedNo CRITICAL/HIGH/MEDIUM issues found; the change only gates the admin listener and uses derived status views without exposing secrets.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-disable-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes.

Address the cold-audit findings on the admin.enabled switch:
- MEDIUM: file-mode admin-off routes through a distinct store branch
(FileManagedStore) that had no coverage — every new test was etcd-only.
Add a file-mode config unit test and a file-source admin-off e2e leg
(declarative resources.yaml, one proxy request, /status/config reports
the file-loaded model, admin port refused).
- LOW: skip the redundant admin etcd client at boot when admin is off
(is_managed() || !admin.enabled) — /status/models then reads the
snapshot, as in managed mode, and boot no longer fails on a connection
it immediately drops.
- LOW: assert the admin port is specifically ECONNREFUSED, not any throw.
- LOW: assert exact resource_counts (models/provider_keys/api_keys == 1)
under the unique etcd prefix, not >= 1.
- LOW: document that admin-off + prometheus-off reduces readiness to the
proxy /livez.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent cold audit (diverse-lens: correctness / security+breaking / e2e-coverage, 3 agents → synthesis). All findings addressed in f086784.

MEDIUM — file-mode admin-off had zero coverage. File mode binds the admin surface through a distinct store branch (FileManagedStore vs EtcdConfigStore), and every new test was etcd-only; the doc advertises admin-off "even in standalone (etcd or file) mode". Behavior was correct today (all three lenses confirmed the runtime is right and default-preserving), but the file branch had no driver → blocks under the e2e-contract merge gate. Fixed: added admin_disabled_relaxes_admin_key_requirement_in_file_mode config unit test + a file-source admin-off e2e leg (declarative resources.yaml, one proxy request, /status/config reports the file-loaded model, admin port refused).

LOW (batched into the same commit):

  • Redundant admin etcd client connected at boot when admin-off (etcd mode) → guard extended to is_managed() || !admin.enabled; /status/models then reads the snapshot (as in managed mode), and boot no longer risks failing on a connection it immediately drops.
  • Admin-unbound assertion accepted any throw → now asserts the specific ECONNREFUSED cause, so a stray listener or a DNS/abort error can't satisfy it.
  • /status/config count asserted >= 1 → now exact (models/provider_keys/api_keys == 1) under the unique etcd prefix.
  • admin-off + prometheus-off reduces readiness to proxy /livez → documented at the readiness site (latent; the default keeps prometheus on).

The correctness and security lenses independently confirmed enabled = true reproduces prior behavior bit-for-bit (non-breaking) and admin-off drops no security control other than the admin surface itself. Verification: cargo fmt/clippy/aisix-core config (35, +new file-mode) green; admin-disabled-e2e 6/6 (etcd + file legs) green; full suite green earlier (3 transient etcd/ECONNRESET flakes, pass on isolated re-run).

…n-off
Close the last coverage corner the audit noted: the file-mode
FileManagedStore /status/models read was only exercised admin-on. The
file admin-off leg now reads the runtime health view on the metrics
listener and asserts the file-loaded model row — the read surface that
must stay live once the admin listener is gone.
@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(admin): add admin.enabled switch to run without the admin listener - #791

Merged
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch
Jul 21, 2026
Merged

feat(admin): add admin.enabled switch to run without the admin listener#791
moonming merged 3 commits into
mainfrom
feat/admin-disable-switch

Conversation

@moonming

@moonmingmoonming commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

Add an admin.enabled startup-config switch (default true) that lets a standalone gateway run without binding the admin listener, and wire the e2e harness to spawn in that mode. This is the foundation for retiring the Admin API: it makes the admin-listener-off configuration a runnable, tested reality during the coexistence window, before any admin code is deleted.

Why

The gateway already runs admin-less in managed mode (config comes from the control plane over etcd). But in standalone etcd/file mode the admin listener was always bound — there was no way to preview the post-removal world, and the e2e harness had no way to prove the request path works without it. admin.enabled = false closes that gap:

  • Operators can run the admin-free configuration on the coexistence release, de-risking the eventual code removal.
  • The e2e suite gains a spawn mode that exercises the declarative-only path (resources seeded straight to etcd via SeedClient, never the Admin API).

admin.enabled is a process-level startup-config toggle (like proxy.addr), not a per-resource setting, so it carries no control-plane/schema work.

Changes

  • AdminConfig.enabled: bool (crates/aisix-core/src/config.rs), defaults to true. When false, Config::validate no longer requires admin_keys/admin.addr (there is no surface to authenticate), mirroring the existing managed-mode relaxation.
  • Bind gating (crates/aisix-server/src/main.rs): the admin listener is spawned only when its store is present andadmin.enabled. /status/config, /status/ready, /status/models, /metrics (metrics listener) and the proxy /livez are unaffected — the metrics/status listener is bound independently and still reads models through the same store handle, so /status/models keeps working with the admin listener off.
  • Harness (tests/e2e/src/harness/app.ts): AppOverrides.admin?: boolean (default true). With admin: false, the generated config carries admin.enabled = false and readiness gates on the proxy /livez + the metrics listener instead of /admin/v1/health.
  • New e2e (admin-disabled-e2e.test.ts): spawns admin: false, seeds a provider key + model + caller key only through etcd, and asserts (1) a proxy chat request succeeds, (2) /status/config reports the applied config on the metrics listener, (3) the admin port is not bound (connection refused).
  • Config unit tests: enabled defaults to true; enabled: false relaxes the admin_keys requirement.

Verification

  • cargo fmt, cargo clippy -p aisix-core -p aisix-server -p aisix-admin --all-targets -D warnings, cargo test -p aisix-core --lib (config, +2 new) and -p aisix-admin --lib (114) green.
  • New admin-disabled-e2e green (3/3). Regression: allowed-models and seed-vs-admin-characterization (exercises both the admin and etcd-seed paths) green — with admin defaulting to true, the admin-on config + readiness are byte-identical to before, so existing tests are unaffected. Full e2e suite green locally.

Scope / follow-ups (this is P0-1 step 1 of the Admin API removal — AISIX-Cloud#1007 Phase 4)

This PR delivers the switch. It does not yet flip the suite: several tests still seed through AdminClient, and a few deliberately exercise the Admin API (characterization, file-mode 409 rejection, key rotation, auth baseline). Follow-ups:

  1. Migrate the remaining seed-only holdouts (AdminClientSeedClient) and relocate the harness's default readiness probe off /admin/v1/health.
  2. Segregate the Admin-API-testing cases as the held-back set that runs admin-on until removal, and add a suite lane that runs the rest admin-off.
  3. Document admin.enabled in the configuration-files reference (api7/docs).

Summary by CodeRabbit

  • New Features

    • Added an option to disable the admin listener while keeping the gateway operational.
    • Gateway readiness no longer depends on the admin health endpoint when the listener is disabled.
    • Configuration seeded through etcd remains available for requests and status reporting.
  • Bug Fixes

    • Improved configuration validation so admin settings are only required when the admin listener is enabled.
  • Tests

    • Added end-to-end coverage for gateway operation without the admin listener.

A standalone gateway could only run admin-less in managed mode, which
needs the full mTLS/control-plane bootstrap. admin.enabled = false
(default true) lets etcd/file mode skip binding the admin listener too —
the shape the gateway takes once the Admin API is removed. The proxy,
the metrics/status listener, and /status/models are unaffected; the
metrics listener still reads models through the same store handle, and
Config::validate drops the admin_keys/admin.addr requirement when there
is no surface to authenticate.
This is the enabler for retiring the Admin API: it makes the
admin-listener-off configuration runnable and testable during the
coexistence window, before any admin code is deleted. The e2e harness
gains an admin: false spawn mode that seeds resources straight to etcd
(never the Admin API) and gates readiness on the proxy /livez plus the
metrics listener; a new admin-disabled e2e proves an etcd-seeded request
path works with the admin listener off.
@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@moonming, you've reached your PR review limit, so we couldn't start this review.

Next review available in:37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f922993a-9c11-4ee6-a649-49bacc68fa38

📥 Commits

Reviewing files that changed from the base of the PR and between 22f4187 and 34d4d0f.

📒 Files selected for processing (4)
  • crates/aisix-core/src/config.rs
  • crates/aisix-server/src/main.rs
  • tests/e2e/src/cases/admin-disabled-e2e.test.ts
  • tests/e2e/src/harness/app.ts
📝 Walkthrough

Walkthrough

Changes

Admin listener control

Layer / File(s)Summary
Admin enablement configuration and validation
crates/aisix-core/src/config.rs, crates/aisix-admin/src/..., crates/aisix-admin/tests/etcd_integration.rs
AdminConfig.enabled defaults to true; validation skips admin requirements when disabled, and test configurations explicitly enable the admin subsystem.
Conditional admin listener startup
crates/aisix-server/src/main.rs
The server binds the admin listener only when configured and logs whether it is disabled or omitted in managed mode.
Disabled-admin test coverage and harness
tests/e2e/src/harness/app.ts, tests/e2e/src/cases/admin-disabled-e2e.test.ts
The E2E harness supports disabling admin readiness and binding, while tests verify etcd-seeded requests, metrics, and an unavailable admin health endpoint.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant E2ETest
participant AppHarness
participant aisix
participant etcd
participant Upstream
E2ETest->>AppHarness: spawn with admin false
AppHarness->>aisix: generate disabled admin configuration
E2ETest->>etcd: seed gateway configuration
aisix->>Upstream: proxy chat completion
E2ETest->>aisix: query metrics and admin health
Loading

Suggested reviewers:jarvis9443

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
E2e Test Quality Review⚠️ WarningThe suite is order-dependent: only the first test waits for config propagation, while later tests assert /status/config and port refusal without their own readiness.Move waitConfigPropagation into beforeAll after seeding, or call it in each test so each case is independently runnable and not time/order dependent.
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly summarizes the main change: adding an admin.enabled switch to disable the admin listener.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Security Check✅ PassedNo CRITICAL/HIGH/MEDIUM issues found; the change only gates the admin listener and uses derived status views without exposing secrets.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-disable-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai full review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes.

Address the cold-audit findings on the admin.enabled switch:
- MEDIUM: file-mode admin-off routes through a distinct store branch
(FileManagedStore) that had no coverage — every new test was etcd-only.
Add a file-mode config unit test and a file-source admin-off e2e leg
(declarative resources.yaml, one proxy request, /status/config reports
the file-loaded model, admin port refused).
- LOW: skip the redundant admin etcd client at boot when admin is off
(is_managed() || !admin.enabled) — /status/models then reads the
snapshot, as in managed mode, and boot no longer fails on a connection
it immediately drops.
- LOW: assert the admin port is specifically ECONNREFUSED, not any throw.
- LOW: assert exact resource_counts (models/provider_keys/api_keys == 1)
under the unique etcd prefix, not >= 1.
- LOW: document that admin-off + prometheus-off reduces readiness to the
proxy /livez.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent cold audit (diverse-lens: correctness / security+breaking / e2e-coverage, 3 agents → synthesis). All findings addressed in f086784.

MEDIUM — file-mode admin-off had zero coverage. File mode binds the admin surface through a distinct store branch (FileManagedStore vs EtcdConfigStore), and every new test was etcd-only; the doc advertises admin-off "even in standalone (etcd or file) mode". Behavior was correct today (all three lenses confirmed the runtime is right and default-preserving), but the file branch had no driver → blocks under the e2e-contract merge gate. Fixed: added admin_disabled_relaxes_admin_key_requirement_in_file_mode config unit test + a file-source admin-off e2e leg (declarative resources.yaml, one proxy request, /status/config reports the file-loaded model, admin port refused).

LOW (batched into the same commit):

  • Redundant admin etcd client connected at boot when admin-off (etcd mode) → guard extended to is_managed() || !admin.enabled; /status/models then reads the snapshot (as in managed mode), and boot no longer risks failing on a connection it immediately drops.
  • Admin-unbound assertion accepted any throw → now asserts the specific ECONNREFUSED cause, so a stray listener or a DNS/abort error can't satisfy it.
  • /status/config count asserted >= 1 → now exact (models/provider_keys/api_keys == 1) under the unique etcd prefix.
  • admin-off + prometheus-off reduces readiness to proxy /livez → documented at the readiness site (latent; the default keeps prometheus on).

The correctness and security lenses independently confirmed enabled = true reproduces prior behavior bit-for-bit (non-breaking) and admin-off drops no security control other than the admin surface itself. Verification: cargo fmt/clippy/aisix-core config (35, +new file-mode) green; admin-disabled-e2e 6/6 (etcd + file legs) green; full suite green earlier (3 transient etcd/ECONNRESET flakes, pass on isolated re-run).

…n-off
Close the last coverage corner the audit noted: the file-mode
FileManagedStore /status/models read was only exercised admin-on. The
file admin-off leg now reads the runtime health view on the metrics
listener and asserts the file-loaded model row — the read surface that
must stay live once the admin listener is gone.
@moonming

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@moonming