Repository navigation
Replies: 2 comments 1 reply
|
Thanks for putting this together, Argus. This is really solid! The model makes sense to me and I like shipping safety first, then efficiency in Phase 2. Just one thing on the design I'd like us to sort before Phase 1. The crashed-server case. On a graceful shutdown the collectors are stopped, but on an ungraceful crash they can keep running with nothing tying their lifecycle to the server. On a plain host that can leave orphaned collectors still holding Valkey connections, and since Phase 1 spawns more of them (one per node per identity), a crash loop could compound. It's a fairly rare case, but there's no cleanup path described, so worth deciding deliberately. |
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
RBAC part of #552.
1. Summary
When several people use one Valkey Admin server against the same Valkey endpoint, they share a single Valkey client and a single set of metrics collectors. Whoever connected or retried last decides the Valkey identity that runs everyone's commands. A user reported this: their
ACL LISTwas rejected with an error naming another user,admin_user. The same sharing lets a restricted user read hot keys, command logs and MONITOR output gathered with someone else's credentials.We propose to fix this in two phases. Phase 1 gives each Valkey identity its own clients and collectors, so nothing is shared between different users. It is the smallest change that removes the problem, and it costs more processes and connections. Phase 2 brings back shared metrics collection for operator-configured instances, using an admin account and a server-side permission check, while each user keeps their own client for commands.
2. Tenets
3. Background
Valkey Admin keeps all connection state in one server process. The maps that matter are:
host-port-dbN)host-port)Release 1.2.0 (#500) stopped a session from reusing a cached client it never connected to. It did not separate two sessions that each connected successfully to the same endpoint. Both sessions then own the same client slot. A second user's connect overwrites the slot without closing the old client, and a retry by the first user flips it back. Collectors are keyed by node alone, so any user who owns a connection to a node reads data gathered under the credentials that spawned its collector. In Kubernetes, that is the sidecar's credentials.
A Valkey ACL checks each command against the identity of the client that sends it. Valkey does not propagate ACLs between nodes: each node loads its own
aclfileorvalkey.confusers, and runtime changes stay on the node that received them.4. Customers
We see two kinds of deployment.
Shared-instance teams. Many people with different roles work against a small, stable set of Valkey instances. They want shared, efficient monitoring and want Valkey ACLs (and, from Valkey 9.2, ACL roles) to decide who sees what.
Individual developers. A few people per instance, and instances come and go often. They connect to new endpoints from the UI and care little about shared monitoring.
Both need the same guarantee: what a user does and sees in Valkey Admin matches what their own Valkey user is allowed to do.
5. Goals and non-goals
Goals for Phase 1:
Non-goals for Phase 1:
6. Proposal
Phase 1: isolate by Valkey identity
Owner identity. Every piece of server state that holds a client, a collector or per-cluster data is keyed by an owner identity: the auth type and the Valkey username the user connected as, with an empty username meaning
default. For ElastiCache IAM the identity also includes the region and replication group. Users who authenticate as the same Valkey user on the same connection share one client and one set of collectors. Users authenticated as different Valkey users share nothing. The frontend keeps identifying connections the same way.Reusing a client. The owner identity is not secret, so it never grants access by itself. A new client authenticates when it's created, as today. To reuse an existing client, the server first checks the presented credentials with a fresh
AUTHagainst that node, and hands over the client only if Valkey accepts them. Valkey Admin never stores or compares secrets. This means:Collectors. Each identity's collectors keep their own metrics history and their own collector settings. Today all collectors share one settings file, so one user's Monitor or retention change would reach every other user's collectors. History is stored under a readable, reversible encoding of the owner identity, so it survives restarts and password changes, and operators can tell whose history is whose. Collectors stop when their owner no longer has a connection to the node. The server caps the number of collector processes it runs. When the cap is reached, a new connection still works for commands and the cluster dashboard, and its metrics panels say the collector limit was reached. The collector itself does not change: the server already controls where each collector stores data and which settings file it reads, and already gives each collector its own registration key.
Browser sessions. Ownership moves from the session cookie to the browser's live connection to the server. That connection may act only on Valkey connections it opened with credentials Valkey accepted. When it drops briefly, the frontend already re-sends the credentials it holds in memory, and the server verifies them and reattaches to the existing client. Valkey clients and collectors are released shortly after the last browser using them goes away. The one capability lost is resuming after a full page reload, or in a new tab, without the password. Web never keeps a real password across a reload, so the user re-enters it, and the frontend already prompts for a missing password. IAM, GCP IAM and passwordless connections reconnect as before. Electron is unaffected when the OS keystore is available. Without one, it prompts after a reload as Web does.
Restarts. A server restart clears clients, collectors, topology and browser bindings, because all of them live in server memory. Users reconnect, and the server rebuilds them under the same owner identities, with nothing to regenerate. A graceful shutdown stops every spawned collector. If the server crashes instead, collectors it spawned keep running indefinitely and can't re-register with the new server, so they waste Valkey and server capacity until someone stops them. In Docker this doesn't arise, because the crash stops the container, so we will move to stop support for bare-metal server deployment. This doesn't affect the Electron deployment as Electron will close child processes on a crash.
Kubernetes. Each Valkey pod runs one metrics sidecar under one set of credentials, so sidecars can't be made per user. In Kubernetes, Phase 1 disables collector-backed features (dashboard history, hot keys, big keys, command logs, Monitor and collector config) and shows a clear message in their place. Commands and the cluster dashboard keep working through each user's own client. Phase 2 restores these features for Kubernetes through the operator-configured admin account.
What Phase 1 removes:
Phase 2: shared collectors for operator-configured instances
Operators configure their Valkey instances before launch, with an admin account used only for metrics collection. Each configured instance gets one shared collector process that polls each primary through a Glide cluster client, matching Glide's default of sending
INFOto all primaries. MONITOR runs in per-primary subprocesses that report aggregates to the collector.Users keep their own client per connection for every command, exactly as in Phase 1. We considered one unified client per instance that runs
ACL DRYRUNas the user before each command, and rejected it (Section 8). With per-user clients, Valkey checks every command against the user's own ACL as it runs, including commands inside scripts and transactions, and ACL changes apply immediately.ACL DRYRUNappears in Phase 2 only to gate aggregates. The shared collector gathers aggregates as the admin account, so Valkey never checks the requesting user's ACL against the commands behind them.ACL DRYRUNasks Valkey whether that user would be allowed to run a command, without running it. Probing with the user's own client doesn't work for every aggregate: MONITOR has no harmless probe, and checking access to all keys needsACL GETUSER.A user sees aggregate data only if their own Valkey user could run every command the aggregates rely on (for example
INFO,COMMANDLOG GET,MONITOR,SCAN), including access to all keys, on every node. The check usesACL DRYRUNandACL GETUSERthrough the admin account, takes the weakest result across the cluster's nodes, and is cached in the collector untilACL DIGESTchanges. The server enforces it.A user's connection uses a shared collector only when the address the user authenticated to is in the operator's configured node list. Connections the operator didn't configure fall back to Phase 1 per-user collectors, and the UI warns that this mode is inefficient.
Phase 2 depends on a change in Valkey Glide. Today, a command sent without an address, such as
INFO, goes to all target nodes, but the reply succeeds only if every node succeeds: one error or timeout fails the whole call, and the results from the other nodes are lost. Phase 2 needs the cluster client to return every node's reply, with a timed-out or failed node included as an error entry alongside the others. Without it, one slow primary blanks metrics for the whole instance.7. Cost of Phase 1
Phase 1 trades efficiency for safety:
ORCHESTRATOR_RATE_LIMIT_MAX.ORCHESTRATOR_RATE_LIMIT_MAXsetting so that they can set the cap smartly, rather than discovering the ceiling in production. A sizing guide will be added for this setting.AUTHper connect or resume.These costs fall on the deployments with the most users per instance, which are the shared-instance teams Phase 2 is designed for.
8. Alternatives considered
One unified client per Valkey instance, with
ACL DRYRUNbefore each user command. This saves connections, two per node for each user. Rejected for both phases:EVAL,EVALSHAandFCALLrun with the admin's rights, whatever the user's ACL says.ACL DRYRUNdoesn't check whether the user is enabled.SELECT,CLIENTorHELLO, would affect every user on the shared client.CLIENT LIST,ACL LOGand command logs would show the admin account, not the user.Gate access in the frontend only. The WebSocket API accepts any action from any socket, so hiding UI elements stops nothing. Rejected as a control.
Isolate per browser session instead of per identity. This is simpler, but the same person in two browsers would get two sets of collectors, and people who legitimately share one Valkey user would gain nothing. Rejected in favor of keying by Valkey identity.
An owner tag made from a keyed hash of the credentials. The server would hash auth type, username and secret with a key generated at startup, and reuse a client when the hashes match. It avoids the extra
AUTHround trip, but Valkey Admin rather than Valkey decides who may reuse a client. The old password keeps matching after a password change, a disabled user still matches, and a user with two valid passwords gets two clients and two sets of collectors. The key changes on every restart, so data directories named by the tag would be orphaned each time, while persisting the key would make directory names usable for offline password guessing. Rejected in favor ofAUTH-gated reuse.Keep sessions alongside owner identities. This lets a reload or new tab resume without the password. It keeps a 24-hour cookie that grants use of authenticated Valkey clients, a second ownership check that overlaps
AUTH-gated reuse, and the expiry sweep and listener that drive teardown. It also keeps a closed tab's clients and collectors alive until the session expires. Session ownership alone is what allowed the reported bug, because both users owned the same connectionId. Rejected: re-entering the password after a reload is an acceptable cost.Go straight to Phase 2. Phase 2 needs new configuration, a new collector design, a permission check and a Glide change, so it would leave the impersonation open for longer. Rejected as the first step.
Keep serving Kubernetes sidecar data to every user. This keeps metrics working in Kubernetes but leaves the sidecar's credentials deciding what every user sees, which breaks tenet 2. Rejected for Phase 1.
Appendix A. Shared state that Phase 1 scopes by owner identity
Every piece of server state that today is shared between users of the same endpoint: Valkey clients and connects in progress, cluster topology and the credentials used to spawn collectors, the collector registry and each collector's registration key, CPU baselines, config pushes in progress, the record of which browsers are watching each connection, and each collector's history and settings on disk. Per-cluster bookkeeping that is already held per browser connection only needs to carry the owner identity.
Appendix B. ACL facts this design relies on
ACL SETUSERisn't replicated.ACL DRYRUNchecks command, key, channel and database rules, including roles, but not whether the user is enabled.SCANhas no key spec, so key patterns don't limit which key names it returns.db=) arrive in Valkey 9.1.0 and roles (role=) in 9.2.0.All reactions