feat(natsstore): serve all tenants from a single muxed K/V bucket

[omer9564]

What & why

Replaces the bucket-per-tenant model with a single muxed bucket (config.bucket), keyed "<tenant>.<key...>". At ~40k tenants, one KV bucket per tenant means one JetStream RAFT group per tenant (~40k HA assets, >10GiB RAM, ~20× over the ~2k-HA-assets/server budget). Collapsing to one bucket removes that fixed per-RAFT-group cost.

The Rego interface is unchanged — data still lands at data.nats.kv.<tenant>.<...>, and nats.kv.watch_bucket / nats.kv.get_data keep their names and arity. The only conceptual shift: argument 1 is now the tenant token (the leading key segment in the one bucket) instead of a bucket name. Policies don't change.

No backward compatibility retained (intentional, per request). bucket is now required; old per-bucket configs won't validate.

Core changes

• config: add required bucket; rename root_bucket → root_tenant (a tenant whose subtree mounts at the OPA data root).
• NATSKeyToOPAPath: derive the tenant from the key's first token (drop the out-of-band bucket arg); root mode strips the tenant token.
• nats_client: open the single bucket once (cached); add tenantKeys(), a prefix-filtered lister so loads are O(tenant), never O(whole bucket) — the key correctness/perf change.
• watcher: watch "<tenant>.>" (single filter → scopeable consumer) instead of ">"; identity is the tenant.
• builtins: loadTenantAsGJSON reads only the tenant slice and strips the "<tenant>." prefix via the new pure buildTenantJSON helper.

Tests (TDD for the pure logic)

• Reshaped path-mapping / config / JSON-builder unit tests (watched each fail first, then implemented).
• New muxed_integration_test.go (real NATS): tenantKeys isolation, data placement at data.nats.kv.<tenant>, t2 never leaking into a t1 load, root-tenant stripping.
• Migrated the example (one DATA bucket, <tenant>.* keys) + both config files + README; account perms scoped to $KV.DATA.>.

Verification

• pkg/natsstore suite: ok (incl. the new real-NATS integration test).
• cmd/opa-nats docker-compose integration test: PASS (54s) — real OPA + NATS, bucket_watched flips false→true, get_data(bucket,"members") returns the members map, data.nats.kv[bucket_id] returns the full tenant tree. Same interface, same placement.
• go build, go vet, gofmt, golangci-lint (0 issues), all pre-commit hooks pass.

Operational note

At the cloud, each watched tenant is one ordered consumer on KV_DATA, capped by max_bucket_watchers (LRU). WatchFiltered could batch tenants later if fan-in becomes a concern.

🤖 Generated with Claude Code

[zeevmoney]

See comments

[zeevmoney]

Automated Review - the PR looks clean, found nothing.

[zeevmoney]

Code review — informational findings (not inline)

These are from a re-review of omer/single-muxed-kv @ 67c901e. They live on lines this PR doesn't change, so they can't be posted as inline review comments — but two of them are directly exercised by this PR's root_tenant path. None duplicate the (now-resolved) prior review threads. The one in-diff finding was posted as an inline review comment separately.

#	Severity	Location	Description
1	MEDIUM	pkg/natsstore/bucket_watcher_manager.go:123 (cleanOPAStore)	Always removes nats/kv/<tenant>, ignoring isRoot. A root watcher writes to stripped data-root paths, so on root-watcher Stop/Reconfigure the root data is left resident and the RemoveOp targets a path that was never written. The example config sets root_tenant: "permit-groups-default", so this path is live.
2	MEDIUM	pkg/natsstore/builtin_functions.go:89 + bucket_watcher_manager.go:79	Root tenant is bulk-loaded twice: EnsureBucketLoaded → LoadBucketDataBulk, then CreateRootWatcher → Start → LoadBucketDataBulk again. Doubles the per-tenant read the O(tenant) design is meant to minimize.
3	LOW	pkg/natsstore/bucket_watcher_manager.go:111-121 (cleanOPAStore defer)	The defer runs Abort then unconditionally Commit on the same txn (inner err := shadows the named return). This is not a panic — the inmem store's underlying() rejects the now-stale txn, so the abort is correct and only a spurious "Failed to commit clean transaction: stale transaction" error is logged. Worth tidying so the abort path returns early.
4	LOW	pkg/natsstore/data_transformer.go:327 (InjectDataToOPAStore)	BasePaths: []string{bucketName} is the tenant token, while writes go to nats/kv/<tenant>/… (or stripped root). Inert for the inmem store (BasePaths is ignored for write txns) but misleading; will mislead/misbehave if a backend ever honors it.
5	LOW	pkg/natsstore/plugin.go:117	logger.Warn("Warning: Cache miss … .\n", …) — embedded \n and a redundant Warning: prefix; violates the project "structured logging, no Printf-style" rule.
6	LOW	pkg/natsstore/bucket_watcher_manager.go:32	gw.cancel is stored but never called; in-flight tenantKeys/LoadBucketDataBulk bound to gw.ctx can't be cancelled on Stop (the watch loop still exits via stopReq, so this is latent rather than a leak today).
7	MEDIUM	pkg/natsstore/*_test.go (coverage)	No test for: t1 vs t10 prefix-confusion isolation, cleanOPAStore (either path), root-tenant watch-update writes, or ErrBucketNotFound handling. Real-NATS isolation is only asserted behind the integration build tag, so default go test ./... proves isolation only via the pure buildTenantJSON unit test.

All pre-existing except where noted; raised here for visibility, non-blocking. Happy to open a follow-up issue or push fixes for #1 (root cleanOPAStore) and #2 (duplicate root load) if useful.

[zeevmoney]

Approved, see one comment and note this one:

#4 (comment)

See if it's safe to merge or defer it to a future issue.