Skip to content

fix(plugin-etcd): classify etcd faults by their gRPC code, not the HTTP status - #3011

Merged
datlechin merged 2 commits into
mainfrom
fix/etcd-auth-fault-classification
Sep 20, 2026
Merged

datlechin merged 2 commits into
mainfrom
fix/etcd-auth-fault-classification

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #2994.

Root cause

EtcdHttpClient.detectApiPrefix() asked one question, "does this gateway prefix exist?", by reading the answer to a different one, "did this unauthenticated maintenance/status call succeed?". It switched on the raw HTTP status: 404 tried the next candidate, 200 accepted, 401 with a configured username accepted, and everything else threw.

etcd's gateway has no HTTP status that means "you need a token". grpc-gateway's HTTPStatusFromCode maps both InvalidArgument(3) and FailedPrecondition(9) onto 400, and etcd reports a missing token as InvalidArgument(3) etcdserver: user name is empty. Once Maintenance.Status became auth-gated, the probe's first call answered 400 and hit the catch-all throw, before connect() ever reached authenticate().

That gate is an era, not a version. Measured by bisecting etcd releases: Status is ungated through 3.5.31, gated behind any valid token from 3.5.32, gated behind the root role on 3.6.0 through 3.6.11 (PR #14663), and relaxed back to any valid token on 3.6.12 (PR #21666). So a fix keyed on "3.6 needs root" would be wrong by 3.6.12.

The same mistake ran through the rest of the client: performRequest re-authenticated on HTTP 401 alone, authenticate() threw the raw JSON body, and watch() never looked at the status at all.

Measured against real etcd

Containers on 3.2.32, 3.3.27, 3.4.34, 3.5.17 and 3.6.1.

Scenario HTTP code message
3.6 auth on, no token, maintenance/status 400 3 etcdserver: user name is empty
3.5 auth on, no token, maintenance/status 200 - not gated on 3.5.17
wrong password 400 3 etcdserver: authentication failed, invalid user ID or password
authenticate against an auth-disabled server 400 9 etcdserver: authentication is not enabled
garbage, expired or deleted-user token 401 16 etcdserver: invalid auth token
Authorization: Bearer <token> 401 16 raw token only, no scheme prefix
non-admin token, maintenance/status on 3.6.1 403 7 etcdserver: permission denied
role revoked under a live token 403 7 etcdserver: permission denied

Prefix routing, which is why the probe stays:

etcd /v3 /v3beta /v3alpha
3.2.32 404 404 200
3.3.27 404 200 200
3.4.34 200 200 404
3.5.17 200 200 404
3.6.1 200 200 404

docs/databases/etcd.mdx promises etcd 3.2 and later, and 3.2 serves only /v3alpha, so hardcoding /v3 would drop two documented versions. The probe is kept and rewritten instead.

The fix

One fault model, owned by one type, used by every transport.

  • EtcdServerFault decodes both body shapes (3.5 emits error, code and message; 3.6 emits code and message) and classifies by the gRPC code, never the HTTP status.
  • EtcdRequestRecovery is the retry decision, mirroring clientv3's shouldRefreshToken: refresh on code 16, on code 3 with a stale auth-store revision, and on code 3 with a missing token when credentials exist. Never on code 7, which clientv3 also refuses (PR #12135 was closed unmerged).
  • EtcdGatewayRoute answers the routing question alone. 404 is the only rejection; a JSON object body is what proves the path reached etcd.
  • authenticate() coalesces onto one Task the way SnowflakeConnection.connectIfNeeded does. The old _isAuthenticating flag returned without a token, so a concurrent caller retried with the stale one.
  • Liveness is etcdctl endpoint health's own check: a linearizable kv/range on key health, counting permission denied as healthy, because reaching the RBAC check proves the quorum read served the request. Not maintenance/status, which is admin-gated on 3.6.0 to 3.6.11, and not cluster/member/list, which answers 200 with no token at all on most releases and so cannot see a dead token.
  • A server with authentication off is tolerated: authenticate() clears the token and connects, which is what etcdctl --user does.

Three of the eight changes are defects in the transport being rewritten rather than parts of the reported bug, and each has its own CHANGELOG line: watch() fed an error body to the event parser and reported zero events instead of an auth failure; the single shared cancel slot let one tab's completion clear another tab's handle; and watch --timeout above 60 seconds raced the session's own 60-second request timeout and died with "The request timed out".

scripts/check-etcd-auth-faults.sh diffs the classification against a live server, in the shape of check-redis-command-routing.sh. It is a manual check, not a CI gate, and it says in its header which row it cannot drive.

Review findings applied

A second-model review of the diff found four defects in it, all fixed here:

  • watch --timeout -1 reached UInt64(timeout * 1_000_000_000), a trapping conversion, and crashed the app from the command editor. --timeout is now range-checked in the parser, with the transport clamping as a second line.
  • The first cut of the cancel fix only made the clear side identity-checked; the write side still clobbered. It is now a keyed set, and Stop cancels what is actually in flight.
  • The watch timeout could fire before its data task was adopted, dropping the cancel. TaskHandle now records the request and honours it on adopt.
  • A docs sentence still said the server version goes blank for a non-admin user, which the new /version fallback fixes.

Codex was out of credits (Reviewer failed to output a response, no job recorded), so the second read came from Skill(code-review) instead.

Verified

All in an isolated worktree, since this checkout is shared with other in-flight work.

Check Result
verify.sh generate PASS
verify.sh build PASS
verify.sh test (4 new suites + watch parser) PASS, 36 of 36
verify.sh test (10 pre-existing etcd suites) PASS, 74 of 74
verify.sh docs PASS
swiftlint --strict on the five plugin files 0 violations
shellcheck --severity=warning on the new script clean
verify.sh plugins EtcdDriverPlugin compiles and links

AllPlugins does not go green on main for an unrelated reason: #3001 added OracleCoreError.transactionLost without updating an exhaustive switch. #3008 fixes that and has to land first for this PR's CI to pass.

No UI automation: nothing here changes a user-facing flow that XCUITest can drive. The behaviour is a network contract, and the tests cover it at the classification boundary.

@mintlify

mintlify Bot commented Sep 20, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 20, 2026, 12:13 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit 5b9ed7c into main Sep 20, 2026
6 of 8 checks passed
@datlechin
datlechin deleted the fix/etcd-auth-fault-classification branch September 20, 2026 12:13
Sign up for free to 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.

ETCD server error after enabling authentication: Unexpected HTTP 400 from v3/maintenance/status

1 participant