diff --git a/AGENTS.md b/AGENTS.md index 2a3c307a..b4c5df44 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ One binary: Eighteen internal packages under `internal/` (plus `internal/testutil/` for shared test helpers): - **`api/`** — Chi HTTP router, JWT/JWKS middleware (from `auth/`), ingest/query/structured-query/SSE/schema/DLQ/pipes handlers -- **`app/`** — the process wiring: `New` builds every component from the boot config and the settings directory (each one wired in one place — what it opens, what it loops, what it releases — with the settings store handed to its wiring function whole, the injection point of the per-tenant registry of #583: store-keyed getters for the handlers, `perTenant` for the async paths), `Run` drives the long-lived ones under one `errgroup` until the context is cancelled or one fails, `Close` releases them in reverse order. `cmd/wavehouse` and `tests/integration` both boot through it +- **`app/`** — the process wiring: `New` builds every component from the boot config and the settings directory (each one wired in one place — what it opens, what it loops, what it releases — with the settings registry handed to its wiring function whole, the injection point of the per-tenant registry of #583: store-keyed getters for the handlers, `perTenant` for the async paths, and `defaultSetting`/`onDefaultAdopt` for the resources a process still has one of, which follow tenant `0`), `Run` drives the long-lived ones under one `errgroup` until the context is cancelled or one fails, `Close` releases them in reverse order. `cmd/wavehouse` and `tests/integration` both boot through it - **`auth/`** — JWT auth middleware: HMAC **or** JWKS verification with `alg` pinned to the active verifier, role extraction from a configurable claim path; always runs, never rejects (bad token → empty role + stashed reason) - **`cache/`** — `Cache` interface → `LocalCache` (Ristretto) + `SharedCache` (TBD) + `TieredCache` (singleflight) - **`chconn/`** — `Manager`, the one ClickHouse `driver.Conn` every consumer holds; `Reconfigure` swaps the connection behind it after a settings reload changes the wiring (never dials; the old connection closes after a `query_timeout` grace) @@ -43,9 +43,9 @@ Eighteen internal packages under `internal/` (plus `internal/testutil/` for shar - **`pipes/`** — Named query pipes: `NamedQuery` type + `BindParams` + `Source` (read per request; `settings.Store` in production, `Static(q...)` in tests) - **`policy/`** — Hasura-style access control, **role-first**: `TablePolicy` is `map[string]RolePermissions`, and a role's grant splits by operation into `SelectPermissions` (columns, row `filter`, aggregations, the `max_*` limits) and `InsertPermissions` (columns, `check`) — so a field only one side honors does not exist on the other. `Evaluate()` resolves ONE operation and leaves the other side **nil** (`Select *ResolvedSelect` / `Insert *ResolvedInsert`), which every accessor fails closed on — nil is "not resolved", distinct from an empty side, which is "unrestricted" (what the admin return builds). Claim templating (`{{ jwt.claim.path }}`) resolves during that call. Policies come from `Source`, a `func() *Policy` read per call (`settings.Store.Policy` in production, `Static(p)` in tests) - **`query/`** — Structured query AST types + SQL builder with schema validation, structural policy predicate/limit emission, timestamp bucketing -- **`settings/`** — the settings directory: `Validate` (strict JSON, per-file rules, cross-file role references), `Store` (the adopted snapshot + serialized `Reload`, typed accessors read per call, `AfterAdopt` hooks), `Registry` (tenant id → `Store`; holds the one store under `tenant.Default`), the fsnotify `Watch`, and the embedded (`go:embed`) seed `wavehouse bootstrap` writes +- **`settings/`** — the settings directory, in either shape ([#583](https://github.com/Wave-RF/WaveHouse/issues/583)): flat (the four files: tenant `0` alone) or nested (one folder per tenant, never mixed). `Validate` detects the shape and checks it — `ValidateDir` per directory (strict JSON, per-file rules, cross-file role references), folder names against `tenant.Parse`, a nested finding's `File` led by its folder; `Store` is a passive holder (one tenant's adopted snapshot, typed accessors read per call); `Registry` (tenant id → `Store`) owns `Open`, the serialized `Reload`/`ReloadTenant`, the `AfterAdopt` hooks, and the fsnotify `Watch` (flat only). Flat refuses an invalid directory at boot and keeps the previous snapshot on a rejected reload; nested fails closed per tenant (a rejected folder stops being served, the rest carry on, a whole-tree reload mirrors the folders, and a finding about the root itself rejects the reload whole). Plus the embedded (`go:embed`) seed `wavehouse bootstrap` writes - **`stream/`** — SSE fan-out: rows travel POSITIONALLY, so each connection is told its projected column list in an `event: schema` frame before its first row and again on drift — **not** guaranteed after a gap-fill across a column change, which can leave a connection reading live rows against a stale list until it reconnects ([#543](https://github.com/Wave-RF/WaveHouse/issues/543)) — (tracked per connection; replay tracks its own). The event `Hub` (registers subscribers by `(topic, role)`; `Broadcast` projects + serializes each event once per role, the #294 delivery hot path — a role carrying a row-level `filter` keeps the shared projection but delivers per subscriber, each subscriber's claims evaluated against the row, #319), `Subscriber` (per-connection outbound `Frame` queue, `Send`/`Frames`; claims fixed at construction, immutable), the `Bucket` fan-out set (`subscriberSet`, one per `(topic, role)`), the `Heartbeater` keepalive wheel, and `Metrics` (the `wavehouse_sse_*` stream instruments) -- **`tenant/`** — the tenant identifier ([#583](https://github.com/Wave-RF/WaveHouse/issues/583)): `ID` (a validated string), `Parse` (letters, digits, `_`, `-`; ≤ 64 bytes — safe as a folder name and as an MQ subject token), `Default` (`"0"`), and `Header` (`X-Tenant-ID`). Imports nothing from the rest of the repo. `api.TenantMW` resolves the header against `settings.Registry` before auth on every `/v1` route outside `/v1/ops/*` (`400` malformed, `404` unknown) and puts the resolved `*settings.Store` in the request context; handlers read it once (`api.StoreFromContext`) and pass it down as an argument, and nothing below a handler reads context. The async paths (ingest worker, sweeper, stream hub, schema registry) are constructed with a `tenant.ID` and their getters take it +- **`tenant/`** — the tenant identifier ([#583](https://github.com/Wave-RF/WaveHouse/issues/583)): `ID` (a validated string), `Parse` (letters, digits, `_`, `-`; ≤ 64 bytes — safe as a folder name and as an MQ subject token), `Default` (`"0"`), and `Header` (`X-Tenant-ID`). Imports nothing from the rest of the repo. `api.TenantMW` resolves the header against `settings.Registry` before auth on every `/v1` route outside `/v1/ops/*` (`400` malformed, `404` unknown, a bare `503` for a nested tenant whose folder was rejected) and puts the resolved `*settings.Store` in the request context; the ops routes that address one tenant (`GET /v1/ops/pipes[/{name}]`, `POST /v1/ops/settings/reload`) take a strictly parsed `?tenant=` instead; handlers read it once (`api.StoreFromContext`) and pass it down as an argument, and nothing below a handler reads context. The async paths (ingest worker, sweeper, stream hub, schema registry) are constructed with a `tenant.ID` and their getters take it ## Key Design Decisions @@ -61,7 +61,7 @@ The invariant index — what must stay true. Full narrative and rationale live i 8. **Optional dedup** — opt-in via `dedupe.enabled` in the settings directory's `config.json` (hot-reloadable: a reload opens or closes the Pebble store via `dedupe.Managed`); `dedupe.id_field` there selects the JSON key, overridable per table. 9. **Singleflight** — `TieredCache` coalesces concurrent misses (`x/sync/singleflight`) to prevent cache stampede. 10. **Active Sweeper** — purges NATS messages that are both ACKed (written to CH) and older than the gap window; SSE gap-fill uses `DeliverByStartTime`, no in-process ring buffer. -11. **Hasura-style access control: fail-closed (security)** — `policy.IsAdmin` (role == `admin_role`, **exact case-sensitive**, default `"admin"`) is the single admin check, shared by `Evaluate`/`ResolveRole`/`Validate`/the `/v1/ops` gate/`RoleAllowed`. Empty/absent role matches nothing (no `"*"` wildcard); `Validate` rejects empty role keys; a `nil` policy (deleted) denies **everyone incl. admin** via a role — a total lockout for token-based callers, so recovery is writing `policies.json` and reloading, never an implicit admin grant (**exception:** the operator key's `auth.IsOperator` bit passes the `/v1/ops` gate even under a `nil` policy — a deliberate break-glass that can `POST /v1/ops/settings/reload` over HTTP, see #7). `default_role` is the one sanctioned roleless exception (`ResolveRole` maps empty → it pre-eval); `default_role == admin_role` is permitted but dev-only and loudly warned (`policy.DefaultRoleGrantsAdmin`). Preserve when touching `internal/policy` (policy twin of #13; see #159). Detail: architecture.md § `policy/`. +11. **Hasura-style access control: fail-closed (security)** — `policy.IsAdmin` (role == `admin_role`, **exact case-sensitive**, default `"admin"`) is the single admin check, shared by `Evaluate`/`ResolveRole`/`Validate`/the `/v1/ops` gate/`RoleAllowed`. Empty/absent role matches nothing (no `"*"` wildcard); `Validate` rejects empty role keys; a `nil` policy (deleted) denies **everyone incl. admin** via a role — a total lockout for token-based callers, so recovery is writing `policies.json` and reloading, never an implicit admin grant (**exception:** the operator key's `auth.IsOperator` bit passes the `/v1/ops` gate even under a `nil` policy — a deliberate break-glass that can `POST /v1/ops/settings/reload` over HTTP, see #7). Over a nested settings directory the `/v1/ops` gate reads no policy at all — those routes reach every tenant, so the operator key alone passes and an admin-role token gets `403`; `api.NewRouter` decides that from the registry's shape, not from what was wired. `default_role` is the one sanctioned roleless exception (`ResolveRole` maps empty → it pre-eval); `default_role == admin_role` is permitted but dev-only and loudly warned (`policy.DefaultRoleGrantsAdmin`). Preserve when touching `internal/policy` (policy twin of #13; see #159). Detail: architecture.md § `policy/`. 12. **Structured queries: column authz fail-closed (security)** — `POST /v1/query?table={table}`: typed AST validated against schema, permission-enforced, timestamp-bucketed for cache, `DefaultMaxRows` (10,000) cap. Every column reference — projection, aggregation args, `filters`, `group_by`, `order_by`, `time_range` — is authorized inside `query.Build` (the single chokepoint that enumerates them all), so no clause can skip the role's `allow_columns`/`deny_columns` check (#223). A `select_all` read by a *column-restricted* role expands to its allowed columns via `policy.AllowedProjection`, never a bare `SELECT *`; *unrestricted*/admin roles keep `SELECT *` (`policy.RestrictsColumns` decides). Omitting `columns` selects nothing (`ErrEmptyProjection` → `200 []`); `["*"]` is the literal column `*` (schema-gated, not a wildcard); a table-granted role with no readable columns fails closed (`ErrNoReadableColumns` → `403`). Structured and live-stream (`stream.projectIndices`) reads share the one per-column decision `policy.IsColumnAllowed`, so column visibility can't drift. Row visibility has the same one-source guarantee (#319): `Evaluate` resolves a role's row-`filter` once (`resolvePredicates`), and both surfaces consume that single resolution — the query path renders it to SQL (`predicatesToSQL`), the stream evaluates it in memory per subscriber (`ResolvedPermissions.RowVisible`, whose type-aware comparison fails closed on anything it can't prove about the ingested payload — `policy.ColumnSpec`, with `DateTime`/`DateTime64` operands compared as instants through the ingest grammar (`discovery.Column.TimeParser`) and claim constants rendered canonically and digit-exact by the one shared rule `policy.CanonicalScalar` (#457 — which also refuses a float64 at/past 2^53 rather than match a neighboring ID, and whose ok=false — an absent claim, a structured value, no canonical form — makes the predicate match no rows on BOTH surfaces: `1 = 0` in SQL, every row withheld in memory); numeric comparison runs in the column's STORAGE domain (`policy.NumericSpec`, classified by `discovery.NumericStorageOf` — Float width rounding, Decimal scale truncation, integer exactness, both operands narrowed as ClickHouse narrows stored value and bound constant, out-of-range operands refused rather than modeled; the `tests/integration` differential oracle holds in-range verdicts equal to a live ClickHouse's and the never-admit-where-SQL-hides direction for the refused out-of-range ones); an event whose insert later fails into the DLQ is the one residual payload-vs-stored asymmetry, documented in the access-control enforcement caution) — so row visibility can't drift either. Preserve when touching `internal/query` or the structured-query handler. Detail: architecture.md § `query/`. 13. **Named query pipes: fail-closed (security)** — pre-defined SQL templates (Tinybird-style) with param binding + caching; `GET/POST /v1/pipes/{name}` sit outside `RequireAdmin`, so per-pipe `allowed_roles` is the *only* execute-path gate, via `policy.RoleAllowed`: exact allowlist membership (no `"*"`), admin always passes, empty/absent role and empty-string entries authorize nobody, and no `allowed_roles` → admin-only. Preserve and exercise via `testutil.RunRoleMatrix` / `StandardRoleMatrix` (see #159). Detail: architecture.md § `pipes/`. 14. **TypeScript SDK** — `@wavehouse/sdk`: typed query builder, real-time SSE over `fetch`, live queries (incrementable/decomposable/poll aggregation), codegen CLI. Exactly one runtime dependency — `eventsource-parser` (SSE framing, itself dependency-free); adding a second needs the same scrutiny the first got. The canonical client (see §SDK Sync). diff --git a/CHANGELOG.md b/CHANGELOG.md index 977c02d0..7f0ceca1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Added +- **A nested settings directory serves one tenant per folder, failing closed per tenant** (`internal/settings/{tree,registry,store,validate,watch}.go` (`tree.go` new, + tests), `internal/api/{tenant,router,pipes,settings}.go`, `internal/app/{app,wire}.go`, `cmd/wavehouse/validate.go`, `clients/ts/src/{pipes,settings,types,index}.ts`, `tests/e2e/sdk/admin.test.ts`, `docs/src/content/docs/{deployment,api,architecture}.md`, `docs/src/content/docs/{settings-directory,reverse-proxy,access-control}.mdx`, `docs/src/content/docs/sdk/{admin,pipes,reference,streaming}.md`): story 2 of the multi-tenant epic ([#583](https://github.com/Wave-RF/WaveHouse/issues/583)), with no behavior change for a settings directory that holds the four files — the same findings byte for byte, the same boot refusal, the same keep-previous on a rejected reload, the same watcher and `SIGHUP`, the same `wavehouse validate` exit codes. `settings.Validate` now reads the directory's shape off its entries — any of the four file names makes it flat, otherwise a folder makes it nested — and checks either one: each tenant folder goes through the per-directory checks (now `ValidateDir`), its name through the tenant-id grammar, and its findings carry the folder (`acme/policies.json`). The shapes never mix: a flat directory rejects a folder and a nested one rejects a loose file. `settings.Open` returns the `Registry`, which now owns `Reload`, the new `ReloadTenant`, the `AfterAdopt` hooks (handed the tenants a reload adopted), and the watcher; a `Store` is a passive holder of one tenant's document. A nested directory fails closed per tenant, at boot and on reload alike: a folder with an error finding stops its tenant being served — the tenant routes answer a bare `503 {"error": "tenant settings are invalid"}`, since they resolve before authentication and the findings quote the settings — while every other tenant carries on, with no previous-snapshot fallback; a request already admitted finishes on the document it started with, and the tenant keeps one `*Store` across the rejection. A whole-directory reload mirrors the folders (a new one is served, a removed one is a `404`), and a finding about the directory itself — a loose file, an unreadable directory, a changed shape — refuses boot and rejects a reload whole, leaving every tenant as it was. A nested directory gets no watcher; `SIGHUP` reloads the whole tree in both shapes. `POST /v1/ops/settings/reload` and the admin pipe reads (`GET /v1/ops/pipes[/{name}]`) take an optional `?tenant=`, parsed strictly — a query string that does not parse, or an empty, repeated, or malformed `tenant`, is a `400`, never a read of the default tenant — with `404` for an unknown tenant; absent means the whole directory on the reload and tenant `0` on the reads. The reload response keeps its shape: over a nested directory `adopted: false` with a `422` can mean adopted in part, the rejected folders being the ones with an error among their `findings`. Over a nested directory the `/v1/ops/*` gate admits the operator key alone — those routes reach every tenant — and an admin-role token gets `403`; `api.NewRouter` decides that from the registry's shape, whatever policy source was wired. Booting a nested directory with no `auth.operator_key` therefore leaves `SIGHUP` as the only reload, and boot warns about it; a whole-directory reload that drops a tenant names it in the log; and `Registry.Watch` refuses a nested directory itself. The resources a process still has one of (ClickHouse connection, dedupe store, MQ byte budget, auth verifier, CORS list) follow the settings tenant `0` last adopted, their reload hooks running only when tenant `0` is adopted, so another tenant's reload never moves them and a `0` folder that a reload rejects or removes leaves all of them as they were — the CORS list, which is read per request, included; a nested directory without a `0` folder boots with them unconfigured (no ClickHouse address, `/livez` degraded) and says so once at boot, and the async paths (ingest worker, sweeper, stream hub, schema refresh) stay wired to tenant `0` until story 5. The SSE keepalive wheel is the one shared resource that weighs every tenant: it runs at the shortest `stream.keepalive_interval` among the tenants being served ([#597](https://github.com/Wave-RF/WaveHouse/issues/597) tracks honoring each tenant's own). The SDK's `wh.pipes.list()`, `wh.pipes.get()`, and `wh.settings.reload()` take a `tenant` option (the new `OpsRequestOptions`) sent as `?tenant=`, since `options.headers` cannot set a query parameter. + - **Requests resolve to a tenant before authentication, and the tenant is threaded through every settings read** (`internal/tenant/` (new, + tests), `internal/settings/registry.go` (new, + tests), `internal/api/tenant.go` (new, + tests), `internal/api/{router,ingest,structured_query,pipes}.go`, `internal/ingest/{worker,sweeper}.go`, `internal/stream/hub.go`, `internal/discovery/discovery.go`, `internal/app/{app,wire}.go`): story 1 of the multi-tenant epic ([#583](https://github.com/Wave-RF/WaveHouse/issues/583)), with no behavior change for a deployment that sends no tenant header. `internal/tenant` defines the id — a validated string (letters, digits, `_`, `-`; at most 64 bytes, so it is safe as a folder name and as an MQ subject token), the reserved default `0`, and the `X-Tenant-ID` header name — and imports nothing from the rest of the repository. `api.TenantMW` runs ahead of the auth middleware on every `/v1` route outside `/v1/ops/*`: an absent or empty header is tenant `0`, a malformed id or a repeated header is a `400`, a well-formed id the new `settings.Registry` does not hold is a `404`, and the resolved `*settings.Store` rides the request context. Every answer from the middleware, the `400` and `404` included, carries `Vary: X-Tenant-ID` so a shared cache can't replay one tenant's response to another. The registry holds the one store `settings.Open` adopted, keyed `0`. Handlers read the store once and pass it down as an argument — the ingest, structured-query, and pipe getters (`PolicySource`, `DedupeSettings`, `bucketSecs`, `defaultMaxRows`, the pipes source) now take it as a parameter — and a tenant route reached without a resolved tenant answers `500` rather than fall back to one. The ingest worker, sweeper, stream hub, and schema registry are constructed with a `tenant.ID` (`tenant.Default` in `internal/app`) and their settings getters take it; a tenant the registry does not hold is logged and read as the getter's zero value, with two fail-safes — the worker's DLQ switch reads as on (an unreadable message is parked, never dropped) and the schema auto-refresh keeps its cadence rather than hand `time.NewTicker` a zero interval. The probes, `/version`, the metrics path, and `/v1/ops/*` stay tenant-exempt, with the ops tree behind the auth middleware and the admin gate exactly as before; the admin pipe reads (`GET /v1/ops/pipes[/{name}]`) serve the default tenant. `X-Tenant-ID` joins the CORS `Access-Control-Allow-Headers` list so a browser client can send it; the SDK needs no change (`options.headers`). - **Schema discovery captures each table's DDL, its columns' ordinals and default expressions, and the server version** (`internal/discovery/discovery.go`, `internal/testutil/testutil.go`): `Column` gains `DefaultExpression` and `Position` (both from a widened `system.columns` select), `TableSchema` gains `DDL` from `system.tables.create_table_query`, and `SchemaRegistry` gains `ServerVersion()` from a `SELECT version()` probe next to the existing `SELECT timezone()`. Groundwork for the native type layer, captured on the same refresh as the columns so a stale version cannot outlive the schemas it describes. That is a publication guarantee, not a same-server one: `chconn.Manager` resolves the connection per call, so a reload changing `clickhouse.addr` mid-refresh can still pair a version from one server with schemas from another — narrow, and self-correcting on the next refresh. `DDL` is `json:"-"` and does **not** appear in `/v1/ops/schema`: that endpoint marshals `TableSchema` straight to the client, and an external-engine table (S3, MySQL, PostgreSQL, Kafka) renders its wiring there unconditionally — endpoint, bucket or host, database, username, S3 access key id. ClickHouse masks the password itself as `[HIDDEN]` from ~23.9 (verified on 26.7.3), so the exposure is the topology rather than the secret — except on an older server, or one with `display_secrets_in_show_and_select` enabled. `position` and `default_expression` are additive fields in the response. A table listed in `system.tables` with no `system.columns` rows is skipped rather than published column-less, and both new queries fail the refresh on error exactly as `timezone()` and `system.columns` do — callers keep the prior cache and retry. @@ -62,6 +64,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- **The `?token=` strip no longer repairs a query string that does not parse** (`internal/auth/auth.go`): `bearerToken` removed a query-string token by parsing the query, deleting `token`, and re-encoding what was left — and `url.ParseQuery` skips a pair it cannot read, so the re-encoding erased that pair. A handler that parses the query strictly in order to refuse a malformed one would then see a clean query: `GET /v1/ops/pipes?tenant=acme;x=1&token=…` would have answered `200` with the default tenant's pipes. The token is read exactly as before and a query that parses is rewritten exactly as before; a query that does not parse now loses its token pairs and nothing else, byte for byte. Pinned through `api.NewRouter` with the real authenticator, since a handler-level test never runs the middleware that rewrote the URL. + - **SSE gap-fill re-reads the policy per replayed row** (`internal/stream/hub.go`): `ReplayProjector` captured the policy once when the replay began, so a policy adopted mid-fill — a revoked grant, say — applied only after the fill ended. It now reads it per event, as `Broadcast` does on the live path. - **`classify-paths.sh` no longer reads a `grep` failure as "no match"** (`scripts/classify-paths.sh`, `scripts/classify-paths.test.sh`): both decisions were `if printf … | grep -qE …; then A; else B; fi`. `grep` exits `0` on match, `1` on no match and **`2` on error** (can't fork/exec, read error, bad pattern), and the `else` branch collapsed `1` and `2` into the same answer — `set -euo pipefail` does not help, since `set -e` is suppressed for a command used as an `if` condition. Observed twice in local `make ci` runs whose static checks run at `-j 14`: a different single case failed each time (`mixed-docs-go` answering `docs=false`, then `dep-bump-go` answering `code=false`) while every other case passed, which is the signature of a transient `grep` failure rather than a pattern bug. The test caught it only because it asserts expected values; **the production path has no such check** — CI's `changes` job gates the docs pipeline on this answer, so a `docs=false` produced by an errored `grep` silently skips the docs build and still reports success. The two greps now go through a `matches` helper that aborts with a diagnostic on any exit above 1, and the test suite stubs `grep` onto `PATH` to prove the abort fires (that case fails against the previous script). A second instance of the same class, found reviewing the first fix: the helper piped its input into `grep -q`, which exits at the first match — so once the file list outgrew the pipe buffer (a few thousand paths) the upstream `printf` died of SIGPIPE, `pipefail` reported 141, and the new error arm aborted on an ordinary large change set. Reproduced at 5,000 paths. It now reads from a here-string instead, and the suite pins that case. `scripts/ci/classify-changes.sh` also stopped reading the classifier through process substitution, which discarded its exit status: a classifier that aborted left `code`/`docs` empty, every `needs.changes.outputs.code == 'true'` job skipped, and the `CI` aggregator reported green having run nothing. It now captures the status, and fails closed — running everything — on a failed *or* partial classification, matching the rule already used for an empty file list. Also here, unrelated and one line: `biome.json` declared `$schema` 2.4.15 while the lockfile pins the 2.5.8 CLI, so `biome check --error-on-warnings` failed on the config itself for any change touching TypeScript. Bumped to match; it changes no lint rule. diff --git a/clients/ts/src/http.ts b/clients/ts/src/http.ts index e6d1edc7..bc67f2e0 100644 --- a/clients/ts/src/http.ts +++ b/clients/ts/src/http.ts @@ -1,5 +1,5 @@ import { networkError, parseErrorResponse } from "./errors.js"; -import type { HttpContext, WaveHouseError } from "./types.js"; +import type { HttpContext, OpsRequestOptions, WaveHouseError } from "./types.js"; import { resolveURL } from "./url.js"; interface RequestSpec { @@ -60,6 +60,18 @@ export function mergeHeaders( return merged; } +/** + * The `?tenant=` query an admin call sends for `opts.tenant`. An empty string + * is sent, not dropped: the server refuses it, where dropping it would turn a + * caller's bug into a call that addresses the default tenant — or, on a + * reload, every tenant. + * + * @internal + */ +export function tenantParam(opts?: OpsRequestOptions): Record | undefined { + return opts?.tenant === undefined ? undefined : { tenant: opts.tenant }; +} + /** The documented shape for a cancelled request: a Result, never a throw. */ function abortedResult(): HttpResult { return { diff --git a/clients/ts/src/index.ts b/clients/ts/src/index.ts index 0641b302..96e7d27a 100644 --- a/clients/ts/src/index.ts +++ b/clients/ts/src/index.ts @@ -36,6 +36,7 @@ export type { // Ingest InsertRecordResult, InsertResult, + OpsRequestOptions, OrderClause, ParamDef, // Pipes diff --git a/clients/ts/src/namespaces.test.ts b/clients/ts/src/namespaces.test.ts index 1815c6fd..48df893f 100644 --- a/clients/ts/src/namespaces.test.ts +++ b/clients/ts/src/namespaces.test.ts @@ -172,6 +172,19 @@ describe("SettingsNamespace", () => { expect(fetchSpy.mock.calls[0][0]).toContain("/v1/ops/settings/reload"); }); + it("reload() sends opts.tenant as ?tenant=, and nothing without it", async () => { + const body = { adopted: true, findings: [] }; + fetchSpy.mockImplementation(async () => new Response(JSON.stringify(body), { status: 200 })); + const ns = new SettingsNamespace(makeCtx()); + + await ns.reload({ tenant: "acme" }); + await ns.reload(); + + const urls = fetchSpy.mock.calls.map((call) => new URL(call[0])); + expect(urls[0].pathname + urls[0].search).toBe("/v1/ops/settings/reload?tenant=acme"); + expect(urls[1].search).toBe(""); + }); + it("reload() surfaces a 422 rejection as an error", async () => { const body = { adopted: false, diff --git a/clients/ts/src/pipes.test.ts b/clients/ts/src/pipes.test.ts index 83a39cd5..476877e3 100644 --- a/clients/ts/src/pipes.test.ts +++ b/clients/ts/src/pipes.test.ts @@ -90,6 +90,24 @@ describe("PipesNamespace", () => { expect(fetchSpy.mock.calls[0][0]).toContain("/v1/ops/pipes"); }); + it("list() and get() send opts.tenant as ?tenant=, and nothing without it", async () => { + fetchSpy.mockImplementation(async () => new Response("[]", { status: 200 })); + const ns = new PipesNamespace(makeCtx()); + + await ns.list({ tenant: "acme" }); + await ns.get("p1", { tenant: "acme" }); + await ns.list(); + // An empty id is the caller's bug: it is sent for the server to refuse, + // never dropped into a read of the default tenant. + await ns.list({ tenant: "" }); + + const urls = fetchSpy.mock.calls.map((call) => new URL(call[0])); + expect(urls[0].pathname + urls[0].search).toBe("/v1/ops/pipes?tenant=acme"); + expect(urls[1].pathname + urls[1].search).toBe("/v1/ops/pipes/p1?tenant=acme"); + expect(urls[2].search).toBe(""); + expect(urls[3].search).toBe("?tenant="); + }); + it("get() GETs /v1/ops/pipes/{name}", async () => { fetchSpy.mockResolvedValue( new Response(JSON.stringify({ name: "p1", sql: "SELECT 1" }), { status: 200 }), diff --git a/clients/ts/src/pipes.ts b/clients/ts/src/pipes.ts index 4a74b304..0297e851 100644 --- a/clients/ts/src/pipes.ts +++ b/clients/ts/src/pipes.ts @@ -1,7 +1,14 @@ import { err, ok } from "./errors.js"; -import { request } from "./http.js"; +import { request, tenantParam } from "./http.js"; import type { StreamController } from "./stream/controller.js"; -import type { HttpContext, Pipe, PipeRequestOptions, Result, StreamOptions } from "./types.js"; +import type { + HttpContext, + OpsRequestOptions, + Pipe, + PipeRequestOptions, + Result, + StreamOptions, +} from "./types.js"; type CreateStreamFn = (table: string, opts?: StreamOptions) => StreamController; @@ -71,22 +78,24 @@ export class PipesNamespace { this._ctx = ctx; } - /** List all registered pipes. */ - async list(opts?: { signal?: AbortSignal }): Promise> { + /** List all registered pipes — of `opts.tenant`, the default tenant without it. */ + async list(opts?: OpsRequestOptions): Promise> { const { data, error } = await request(this._ctx, { method: "GET", path: "/v1/ops/pipes", + params: tenantParam(opts), signal: opts?.signal, }); if (error) return err(error); return ok(data!); } - /** Get a single pipe definition by name. */ - async get(name: string, opts?: { signal?: AbortSignal }): Promise> { + /** Get a single pipe definition by name — of `opts.tenant`, the default tenant without it. */ + async get(name: string, opts?: OpsRequestOptions): Promise> { const { data, error } = await request(this._ctx, { method: "GET", path: `/v1/ops/pipes/${encodeURIComponent(name)}`, + params: tenantParam(opts), signal: opts?.signal, }); if (error) return err(error); diff --git a/clients/ts/src/settings.ts b/clients/ts/src/settings.ts index aa05c4d3..ee73fb41 100644 --- a/clients/ts/src/settings.ts +++ b/clients/ts/src/settings.ts @@ -1,6 +1,6 @@ import { err, ok } from "./errors.js"; -import { request } from "./http.js"; -import type { HttpContext, Result, SettingsReloadResult } from "./types.js"; +import { request, tenantParam } from "./http.js"; +import type { HttpContext, OpsRequestOptions, Result, SettingsReloadResult } from "./types.js"; /** * Namespace for the server's hot-reloadable settings directory. Requires the @@ -24,11 +24,15 @@ export class SettingsNamespace { * included); a rejected directory is a 422 error whose `details` carries the * same `{ adopted: false, findings }` body, and the previous settings stay * in effect. + * + * `opts.tenant` reloads that tenant's folder alone; without it the whole + * directory is reloaded. */ - async reload(opts?: { signal?: AbortSignal }): Promise> { + async reload(opts?: OpsRequestOptions): Promise> { const { data, error } = await request(this._ctx, { method: "POST", path: "/v1/ops/settings/reload", + params: tenantParam(opts), signal: opts?.signal, }); if (error) return err(error); diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index 4736c12a..d4de4b8b 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -413,7 +413,11 @@ export interface PolicyFilter { /** One validation finding from the server's settings directory. */ export interface SettingsFinding { severity: "error" | "warning"; - /** Settings file the finding is about; absent for directory-level findings. */ + /** + * Settings file the finding is about; absent for directory-level findings. + * In a directory of tenant folders it leads with the folder + * (`acme/policies.json`). + */ file?: string; /** Dotted JSON path within the file; absent for whole-file findings. */ path?: string; @@ -422,7 +426,13 @@ export interface SettingsFinding { /** Body of POST /v1/ops/settings/reload. */ export interface SettingsReloadResult { - /** Whether the directory was adopted; false leaves the previous settings in effect. */ + /** + * Whether everything the reload covered was adopted; false leaves the + * previous settings in effect. Over a directory of tenant folders false can + * mean adopted in part: the tenants with an error among their `findings` + * were not adopted and are no longer served, and the rest were — `findings` + * carries every folder's warnings too. + */ adopted: boolean; /** Every finding from the validation pass (warnings included on success). */ findings: SettingsFinding[]; @@ -458,6 +468,22 @@ export interface PipeRequestOptions { limit?: never; } +/** + * Options for a call to one of the admin routes that address a tenant: + * `wh.pipes.list()`, `wh.pipes.get()`, and `wh.settings.reload()`. + */ +export interface OpsRequestOptions { + signal?: AbortSignal; + /** + * The tenant the call addresses, sent as `?tenant=`. The admin routes ignore + * the `X-Tenant-ID` header, so `options.headers` cannot select one. Omitted, + * the reads serve the default tenant (`0`) and `reload()` reloads every + * tenant. An id the server does not accept — the empty string included — is + * a `400`, never a silent fallback to the default. + */ + tenant?: string; +} + // --- Stream options --- export interface StreamOptions { diff --git a/cmd/wavehouse/validate.go b/cmd/wavehouse/validate.go index 35a4319d..da5d49ba 100644 --- a/cmd/wavehouse/validate.go +++ b/cmd/wavehouse/validate.go @@ -12,18 +12,19 @@ import ( ) // runValidate implements `wavehouse validate [dir]`: validate a settings -// directory (roles.json, policies.json, pipes.json, config.json) without -// starting the server, so an operator or CI can gate a config change before it -// reaches a running instance. The directory comes from the argument, falling -// back to WH_SETTINGS_DIR. Exit codes: 0 valid (warnings allowed), 1 invalid, -// 2 usage. +// directory (roles.json, policies.json, pipes.json, config.json — or one +// folder per tenant, each holding those four) without starting the server, so +// an operator or CI can gate a config change before it reaches a running +// instance. The directory comes from the argument, falling back to +// WH_SETTINGS_DIR. Exit codes: 0 valid (warnings allowed), 1 invalid, 2 usage. func runValidate(args []string) int { fs := flag.NewFlagSet("validate", flag.ContinueOnError) fs.Usage = func() { _, _ = fmt.Fprintf(fs.Output(), `usage: wavehouse validate [dir] -Validate a settings directory (%s) without -starting the server. With no dir argument, the directory comes from %s. +Validate a settings directory (%s, or one +folder per tenant, each holding those files) without starting the server. +With no dir argument, the directory comes from %s. Exit codes: 0 valid (warnings allowed), 1 invalid, 2 usage. `, strings.Join(settings.Files(), ", "), config.EnvSettingsDir) diff --git a/cmd/wavehouse/validate_test.go b/cmd/wavehouse/validate_test.go index 36d96cf1..5a7efdec 100644 --- a/cmd/wavehouse/validate_test.go +++ b/cmd/wavehouse/validate_test.go @@ -36,6 +36,20 @@ func TestRunValidate(t *testing.T) { assert.Equal(t, 1, runValidate([]string{writeSettingsDir(t, `{"default_role": "ghost"}`)})) }) + // A nested root — one folder per tenant — shares the exit codes: every + // folder valid is 0, one invalid folder is 1. + t.Run("nested directory", func(t *testing.T) { + nested := func(policies map[string]string) string { + root := t.TempDir() + for folder, p := range policies { + require.NoError(t, os.Rename(writeSettingsDir(t, p), filepath.Join(root, folder))) + } + return root + } + assert.Equal(t, 0, runValidate([]string{nested(map[string]string{"acme": `{}`, "globex": `{}`})})) + assert.Equal(t, 1, runValidate([]string{nested(map[string]string{"acme": `{}`, "globex": `{"default_role": "ghost"}`})})) + }) + t.Run("env fallback", func(t *testing.T) { t.Setenv("WH_SETTINGS_DIR", writeSettingsDir(t, `{}`)) assert.Equal(t, 0, runValidate(nil)) diff --git a/docs/src/content/docs/access-control.mdx b/docs/src/content/docs/access-control.mdx index b54d271a..0e44434a 100644 --- a/docs/src/content/docs/access-control.mdx +++ b/docs/src/content/docs/access-control.mdx @@ -62,7 +62,7 @@ Setting `default_role` equal to `admin_role` is permitted — it makes every una `admin_role` (default: `"admin"`) is the one role that bypasses the entire policy: - It is granted **full, unrestricted access** to every table and operation. An admin is **never** column-scoped or row-scoped, even by a policy entry that explicitly names the admin role — admin is an unconditional bypass, not a scoped grant. -- It is the gate for the whole `/v1/ops/*` tree — raw SQL, pipe inspection, settings reload, schema discovery, and DLQ stats. The gate reads `admin_role` live from the policy, so changing it applies without a restart. +- It is the gate for the whole `/v1/ops/*` tree — raw SQL, pipe inspection, settings reload, schema discovery, and DLQ stats. The gate reads `admin_role` live from the policy, so changing it applies without a restart — except over a [nested settings directory](/deployment#the-nested-settings-directory), where it reads no policy at all and the [operator key](#operator-key) alone passes. There is no separate `service` role. To reach an admin endpoint or to read data beyond what `default_role` grants, present a valid token whose `role_claim` is the admin role (or another granted role) — **or** present the non-JWT [operator key](#operator-key) described below. @@ -213,7 +213,7 @@ On a structured query (`POST /v1/query?table={table}`) the allowlist is a **hard This produces `WHERE (tenant_id = ?)` with the caller's `app_metadata.tenant_id` claim bound as the parameter — so a `viewer` only ever sees rows for their own tenant, and the value comes from the signed token, not from anything the client sends. :::note[Two meanings of "tenant"] -On this page a *tenant* is a row-scoping value carried in the signed token. The [`X-Tenant-ID` header](/deployment#multi-tenant-deployments) is a different axis: it selects which settings directory — and so which `policies.json` — serves the request. It is client-supplied, resolved before authentication, and isolates no rows. A settings directory is one such tenant (`0`), so most deployments never send it. +On this page a *tenant* is a row-scoping value carried in the signed token. The [`X-Tenant-ID` header](/deployment#multi-tenant-deployments) is a different axis: it selects which set of settings files — and so which `policies.json` — serves the request. It is client-supplied, resolved before authentication, and isolates no rows. A settings directory that holds the four files is one such tenant (`0`), so most deployments never send it. ::: Supported comparison operators: diff --git a/docs/src/content/docs/api.md b/docs/src/content/docs/api.md index e153827b..00bcee25 100644 --- a/docs/src/content/docs/api.md +++ b/docs/src/content/docs/api.md @@ -150,7 +150,7 @@ Status code: `503 Service Unavailable` ### `GET /v1/health` — Liveness ping (public, content-free) -Returns **`200 OK` with an empty body** once the gateway is past boot, or **`503 Service Unavailable`** (also empty) while boot-time schema discovery is still failing. Like every `/v1` route outside `/v1/ops/*` it [resolves a tenant](/deployment#multi-tenant-deployments) first, so a bad `X-Tenant-ID` answers `400`/`404` before the probe runs. No authentication required and no response body — the caller only branches on the status code, so there's nothing to JSON-encode or cache per request. +Returns **`200 OK` with an empty body** once the gateway is past boot, or **`503 Service Unavailable`** (also empty) while boot-time schema discovery is still failing. Like every `/v1` route outside `/v1/ops/*` it [resolves a tenant](/deployment#multi-tenant-deployments) first, so a malformed or unknown `X-Tenant-ID` answers `400`/`404` before the probe runs, and — over a [nested settings directory](/deployment#the-nested-settings-directory) — a tenant whose settings folder was rejected answers a `503` that carries the usual JSON error body rather than this route's empty one. No authentication required and no response body — the caller only branches on the status code, so there's nothing to JSON-encode or cache per request. This is what the SDK's `wh.sys.health()` calls, and the endpoint to use when choosing among multiple servers in a distributed setup. It mirrors `/livez` under the hood but is intentionally a `/v1` API route rather than a Kubernetes probe path: an operator may filter the bare probe paths (`/livez`, `/readyz`, `/healthz`) out at the reverse proxy since they're internal probes, so the SDK relies on `/v1/health`, which is documented public API surface meant to stay reachable. It does **not** ping ClickHouse — readiness-based load balancing is the proxy/LB's job (via `/readyz`), not the client's. @@ -638,7 +638,7 @@ curl -N "http://localhost:8080/v1/stream?table=clicks&since=2026-03-24T11:00:00Z ### Admin Endpoints -Every admin-gated surface lives under the `/v1/ops/*` prefix, behind a single `RequireAdmin` gate: schema discovery, DLQ stats, and the pipe and settings-reload endpoints below, plus the raw-SQL passthrough [`POST /v1/ops/query`](#post-v1opsquery--query-clickhouse) documented with the query endpoints above. They require the policy `admin_role` (`"admin"` by default, exact case-sensitive match) — or the non-JWT [operator key](#authentication), which reaches the same surface without a token; other callers get 401 (present-but-invalid token) / 403, and the quickstart's trial `public` role cannot call any of them. There is no separate `service` role. The JWT middleware always runs — a tokenless request (or a valid token without a role claim) resolves to the `default_role` (not the admin role unless `default_role` is deliberately set to it — a loudly-warned dev-only setting) and is denied `403`, while a present-but-invalid token keeps its stashed verification error and is denied `401`. +Every admin-gated surface lives under the `/v1/ops/*` prefix, behind a single `RequireAdmin` gate: schema discovery, DLQ stats, and the pipe and settings-reload endpoints below, plus the raw-SQL passthrough [`POST /v1/ops/query`](#post-v1opsquery--query-clickhouse) documented with the query endpoints above. They require the policy `admin_role` (`"admin"` by default, exact case-sensitive match) — or the non-JWT [operator key](#authentication), which reaches the same surface without a token; other callers get 401 (present-but-invalid token) / 403, and the quickstart's trial `public` role cannot call any of them. Over a [nested settings directory](/deployment#the-nested-settings-directory) these routes reach every tenant, so the operator key alone opens them and an admin-role token gets `403`. There is no separate `service` role. The JWT middleware always runs — a tokenless request (or a valid token without a role claim) resolves to the `default_role` (not the admin role unless `default_role` is deliberately set to it — a loudly-warned dev-only setting) and is denied `403`, while a present-but-invalid token keeps its stashed verification error and is denied `401`. No admin endpoint in this section accepts a request body — they are reads and triggers; the settings directory's files are the only write path. The raw-SQL `POST /v1/ops/query` carries the 16 MiB bulk-payload cap documented with the query endpoints above. @@ -758,6 +758,14 @@ The policy has no endpoints: it is the settings directory's [`policies.json`](/s Returns every adopted named query pipe — the settings directory's [`pipes.json`](/settings-directory#pipesjson). Pipes have no write endpoints: edit the file and reload. +Both pipe reads take an optional `?tenant=` naming the [tenant](/deployment#the-nested-settings-directory) whose pipes are read; without it they read the default tenant `0`, which is the whole settings directory unless it is nested. The query string is parsed strictly, so that a request is never answered for a tenant it did not name: + +| Status | When | +| ------ | ---- | +| `400` | The query string does not parse (`?tenant=acme;x=1`, a bad `%` escape), or `tenant` is empty, repeated, or not a tenant id | +| `404` | No such tenant | +| `503` | The tenant's settings folder was rejected | + #### `GET /v1/ops/pipes/{name}` — Get Named Pipe Returns a specific named pipe definition: @@ -792,6 +800,8 @@ Re-validates the [settings directory](/settings-directory) — `roles.json`, `po `200` when adopted (warnings allowed); `422` when validation rejected the directory — the previous settings stay in effect, and `findings` says why. +An optional `?tenant=` reloads that tenant's folder of a [nested settings directory](/deployment#the-nested-settings-directory) and nothing else; it is parsed as strictly as on the [pipe reads](#get-v1opspipes--list-named-pipes) (`400`), and an unknown tenant is a `404`. Over a nested directory a rejected folder is not kept on its previous settings, and a `422` for the whole directory can mean adopted in part — see that section. + ## Event Message Format ### Internal Wire Format (NATS) diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index f9f4c4c2..2969714a 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -66,7 +66,7 @@ internal/ ├── pipes/ Named query pipes (NamedQuery type, parameter binding, Source) ├── policy/ Hasura-style access control (policy types, evaluation, Source) ├── query/ Structured query AST, SQL builder, and timestamp bucketing -├── settings/ Settings directory: validate the JSON files, hold the adopted snapshot, reload on watch / SIGHUP / API +├── settings/ Settings directory: validate the JSON files, hold each tenant's adopted snapshot, reload on watch / SIGHUP / API ├── stream/ SSE fan-out: event Hub (project once per role), Subscriber queue, Bucket fan-out, keepalive Heartbeater wheel └── tenant/ Tenant id: the type, its grammar, the reserved default, the request header name ``` @@ -75,9 +75,9 @@ internal/ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with RequestID, a CORS middleware, and a custom JSON recoverer (`jsonRecoverer`) that emits a JSON `500` on panic instead of chi's plain-text `middleware.Recoverer`. -- **router.go** — Route definitions. Public: `/livez`, `/readyz`, and the content-free `/v1/health` SDK ping (plus the permanent `/healthz` alias and the deprecated `/health`, `/ready` aliases). Policy-gated: `/v1/ingest?table={table}`, `/v1/query?table={table}` (structured), `/v1/pipes/{name}` (named pipes), `/v1/stream`. Admin-only (`RequireAdmin` — role == `policy.admin_role`, or a request bearing the operator key's operator bit, which passes even under a nil policy): `/v1/ops/schema/*`, `/v1/ops/dlq/stats`, `GET /v1/ops/pipes[/{name}]`, `/v1/ops/settings/reload`, `/v1/ops/query` (raw SQL — same gate as the rest of `/v1/ops/*`). +- **router.go** — Route definitions. Public: `/livez`, `/readyz`, and the content-free `/v1/health` SDK ping (plus the permanent `/healthz` alias and the deprecated `/health`, `/ready` aliases). Policy-gated: `/v1/ingest?table={table}`, `/v1/query?table={table}` (structured), `/v1/pipes/{name}` (named pipes), `/v1/stream`. Admin-only (`RequireAdmin` — role == `policy.admin_role`, or a request bearing the operator key's operator bit, which passes even under a nil policy; over a nested settings directory `NewRouter` mounts the gate with no policy at all, whatever `Dependencies.PolicySource` was wired, so the operator key alone passes): `/v1/ops/schema/*`, `/v1/ops/dlq/stats`, `GET /v1/ops/pipes[/{name}]`, `/v1/ops/settings/reload`, `/v1/ops/query` (raw SQL — same gate as the rest of `/v1/ops/*`). - **auth middleware** — the JWT/JWKS authentication middleware is its own package, [`auth/`](#auth--authentication); the router runs it on every `/v1/*` route. -- **tenant.go** — `TenantMW` resolves the request's tenant ahead of the auth middleware on every `/v1` route outside `/v1/ops/*`: the [`X-Tenant-ID`](/deployment#multi-tenant-deployments) header (absent means `tenant.Default`), validated by `tenant.Parse` (`400`), looked up in the `settings.Registry` (`404` on a miss), and the resolved `*settings.Store` stored in the request context (`WithStore` / `StoreFromContext` — here rather than in `tenant/`, because `settings` names `tenant.ID`). A handler reads the store once and passes it down as an argument — the per-tenant getters it holds take it as a parameter (`(*settings.Store).Policy`, `.DedupeFor`, `.DefaultMaxRows`, … in production) — and nothing below a handler reads the context; a tenant route reached without a resolved store answers `500` rather than fall back to a tenant. The probes, `/version`, the metrics path, and `/v1/ops/*` are tenant-exempt; the admin pipe reads serve the default tenant's store. +- **tenant.go** — `TenantMW` resolves the request's tenant ahead of the auth middleware on every `/v1` route outside `/v1/ops/*`: the [`X-Tenant-ID`](/deployment#multi-tenant-deployments) header (absent means `tenant.Default`), validated by `tenant.Parse` (`400`), looked up in the `settings.Registry` (`resolveStore`: `404` for an id it does not hold, a bare `503` for a tenant whose folder was rejected — the findings stay out of a body answered before authentication), and the resolved `*settings.Store` stored in the request context (`WithStore` / `StoreFromContext` — here rather than in `tenant/`, because `settings` names `tenant.ID`). A handler reads the store once and passes it down as an argument — the per-tenant getters it holds take it as a parameter (`(*settings.Store).Policy`, `.DedupeFor`, `.DefaultMaxRows`, … in production) — and nothing below a handler reads the context; a tenant route reached without a resolved store answers `500` rather than fall back to a tenant. The probes, `/version`, the metrics path, and `/v1/ops/*` are tenant-exempt; an ops route that addresses one tenant — the admin pipe reads and the settings reload — names it in `?tenant=` (`opsTenant`), parsed strictly so that a query `url.ParseQuery` would half-read is a `400` rather than a read of the default tenant, which is what it means when absent on the reads (on the reload, absent is the whole directory). - **pipes.go** — Named query pipe handlers: admin listing (`GET /v1/ops/pipes[/{name}]`, read per request from its `pipes.Source`) and execution with parameter binding. `pipes.json` is the only write path. - **structured_query.go** — Handler for `POST /v1/query?table={table}`: validates query AST, enforces permissions, builds and executes SQL. - **ingest.go** — Accepts `POST /v1/ingest?table={table}` in three body shapes: one flat JSON object, a JSON array of them, or NDJSON. The **required** `Content-Type` chooses the format *family* — `application/json` versus the four NDJSON spellings — and within the JSON family the body's first non-whitespace byte picks array versus single object; the bytes never choose the family. Anything that is not exactly one readable media type is a `415`, decided before the body is read: the header is parsed per RFC 9110 §8.3, and because `Content-Type` is a singleton field, repeated header lines must all resolve to the same format and a value carrying a comma is refused unless the value as a whole parses as one media type — a comma inside a *quoted* parameter value is data, so `application/json; a=", application/x-ndjson; b="` is accepted. It then reads the whole (`MaxBytesReader`-capped) body into a pooled buffer and runs the per-format record readers over those bytes, so the `413` lands before any record is processed and peak memory per request is O(body) rather than O(record). Then it validates each record against the discovered schema, optional dedup, and publishes each row through `mq.Publisher` on `mq.Topic{Table, Scope}` (raw names — the subject it becomes is `internal/mq`'s; a full queue comes back as `mq.ErrQueueFull`, which is the `503` + `Retry-After`). When dedup is on, a row missing the configured `id_field` can't be deduped: it is logged at `WARN` and counted by `wavehouse_ingest_dedupe_missing_id_total` (labeled by `table`), then published un-deduped — or rejected when `dedupe.require_id` is set ([#219](https://github.com/Wave-RF/WaveHouse/issues/219)). @@ -89,8 +89,8 @@ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with Request ### `app/` — Process wiring -- **app.go** — `New(ctx, Options)` builds every component from the boot config (`Options.Config`) and the settings directory it names, in dependency order: settings store, observability, ClickHouse connection, schema discovery, dedupe store, embedded NATS (ingest + DLQ streams), cache, sweeper, streaming (hub, MQ→hub bridge, keepalive wheel), ingest worker, auth, reload triggers, HTTP. Each is one `component` value — what it opens, what it loops, what it releases — so a failure part-way releases what was already opened and returns the error. `Run(ctx)` drives every loop under one `errgroup` until `ctx` is canceled (a clean stop: every loop drains, the API server and the ingest worker within `server.shutdown_timeout`; open SSE streams are ended as the drain begins rather than waited on) or a component fails, which stops the rest and returns that error. `Close(ctx)` releases what `New` opened, newest first, under the caller's release budget (`ReleaseTimeout`, 5s), a real bound: a remote implementation's close gives up at the deadline itself, and a close that ignores the context (the local stores) is abandoned at it, with the components below it left unreleased rather than overlapping it, both named in the error — and then flushes telemetry under its own 3s budget, so the flush that reports on the stop is never handed a deadline a slow close already spent. The SIGHUP registration is released last of all. `Handler`, `Registry`, and `MQ` expose the pieces a harness needs; `Options.Listener` lets one serve the API on its own listener instead of `server.port`. -- **wire.go** — one `wire*` function per component, each handed the settings store whole and deriving the per-call getters the internal packages take (`DLQFor`, `DedupeFor`, `GapWindow`, …) and registering its `AfterAdopt` hook there where it has one. Those wiring functions are where the per-tenant registry of [#583](https://github.com/Wave-RF/WaveHouse/issues/583) is injected, not `main`: `wireSettings` builds the `settings.Registry`, the HTTP handlers get store-keyed getters (method expressions such as `(*settings.Store).Policy`), and `perTenant` adapts a store accessor into the `func(tenant.ID) T` getter the async packages take — a tenant the registry does not hold is logged and read as the zero value, except in `dlqFor`, the ingest worker's DLQ switch, where it reads as on so a message the worker cannot read is parked rather than dropped. The reload triggers (SIGHUP, the directory watcher) only start in `Run`, after `New` has registered every hook, so the watcher's first reload already drives all of them. The `mq.max_bytes_gb` hook only hands the adopted budget to `mq.Broker.SetMaxBytes` under the App's stop context; how it is split across the streams, the time bounds, and the rollback are `internal/mq`'s. +- **app.go** — `New(ctx, Options)` builds every component from the boot config (`Options.Config`) and the settings directory it names, in dependency order: settings registry, observability, ClickHouse connection, schema discovery, dedupe store, embedded NATS (ingest + DLQ streams), cache, sweeper, streaming (hub, MQ→hub bridge, keepalive wheel), ingest worker, auth, reload triggers, HTTP. Each is one `component` value — what it opens, what it loops, what it releases — so a failure part-way releases what was already opened and returns the error. `Run(ctx)` drives every loop under one `errgroup` until `ctx` is canceled (a clean stop: every loop drains, the API server and the ingest worker within `server.shutdown_timeout`; open SSE streams are ended as the drain begins rather than waited on) or a component fails, which stops the rest and returns that error. `Close(ctx)` releases what `New` opened, newest first, under the caller's release budget (`ReleaseTimeout`, 5s), a real bound: a remote implementation's close gives up at the deadline itself, and a close that ignores the context (the local stores) is abandoned at it, with the components below it left unreleased rather than overlapping it, both named in the error — and then flushes telemetry under its own 3s budget, so the flush that reports on the stop is never handed a deadline a slow close already spent. The SIGHUP registration is released last of all. `Handler`, `Registry`, and `MQ` expose the pieces a harness needs; `Options.Listener` lets one serve the API on its own listener instead of `server.port`. +- **wire.go** — one `wire*` function per component, each handed the settings registry whole and deriving the per-call getters the internal packages take (`DLQFor`, `DedupeFor`, `GapWindow`, …) and registering its `AfterAdopt` hook there where it has one. Those wiring functions are where the per-tenant registry of [#583](https://github.com/Wave-RF/WaveHouse/issues/583) is injected, not `main`: `wireSettings` opens the `settings.Registry`, the HTTP handlers get store-keyed getters (method expressions such as `(*settings.Store).Policy`), and `perTenant` adapts a store accessor into the `func(tenant.ID) T` getter the async packages take — a tenant the registry is not serving is logged and read as the zero value, except in `dlqFor`, the ingest worker's DLQ switch, where it reads as on so a message the worker cannot read is parked rather than dropped. The resources one process still has one of (ClickHouse connection, dedupe store, MQ byte budget, auth verifier, CORS list) follow the default tenant: `defaultSetting` reads the store tenant `0` last adopted (`App.defaultStore`; the zero value when a nested directory has never served a tenant `0`, warned about once at boot), and `onDefaultAdopt` runs their hooks only after a reload that adopted it, so another tenant's reload never moves them and a `0` folder that a reload rejects or removes leaves all of them as they were — the hook-reconciled ones and the ones read per request (the CORS list, the operator key's admin role) alike. The keepalive wheel is shared differently: `shortestKeepalive` takes the shortest `stream.keepalive_interval` among the tenants being served, after any tenant's adoption ([#597](https://github.com/Wave-RF/WaveHouse/issues/597)). The reload triggers only start in `Run`, after `New` has registered every hook, so the watcher's first reload already drives all of them: SIGHUP in both shapes, the directory watcher for a flat directory only. The `mq.max_bytes_gb` hook only hands the adopted budget to `mq.Broker.SetMaxBytes` under the App's stop context; how it is split across the streams, the time bounds, and the rollback are `internal/mq`'s. ### `stream/` — SSE keepalive & fan-out @@ -124,7 +124,7 @@ The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https:/ - **dedupe.go** — `Deduplicator` interface: `CheckAndMark(ctx, eventID) (bool, error)`. - **embedded.go** — Uses [Pebble](https://github.com/cockroachdb/pebble) (embedded key-value store). Key = event ID. -- **managed.go** — `Managed` wraps the Pebble store behind the hot-reloadable `dedupe.enabled` switch: a `settings.Store.AfterAdopt` hook opens or closes the store after every adoption, so flipping the key is a reload, not a restart. `CheckAndMark` returns `ErrDisabled` while switched off (the ingest handler publishes un-deduped and counts it — a reload-window race, not a mode). +- **managed.go** — `Managed` wraps the Pebble store behind the hot-reloadable `dedupe.enabled` switch: a `settings.Registry.AfterAdopt` hook opens or closes the store after every adoption of the default tenant, so flipping the key is a reload, not a restart. `CheckAndMark` returns `ErrDisabled` while switched off (the ingest handler publishes un-deduped and counts it — a reload-window race, not a mode). ### `discovery/` — Schema Discovery & Validation @@ -164,7 +164,7 @@ The package's design invariants — stdout always 100%, WARN+ERROR always export - **rowfilter.go** — the in-memory row-visibility twin of the SQL `WHERE`: `HasRowFilter`, `RowVisible` (evaluates the resolved predicates against a decoded event, per subscriber), and `ColumnSpec` — the per-column comparison contract (`ColumnKind` `Numeric`/`Text`/`Time`/`Opaque`, plus each kind's parameters: the caller-supplied instant parser for `Time`, the `NumericSpec` storage model for `Numeric`) whose zero value is the fail-closed floor: numeric columns compare in the column's **storage domain** (operands rendered by canonical.go, compared by numeric.go — next two bullets), `String` bytewise, `DateTime`/`DateTime64` chronologically (both operands through the ingest grammar; either side unreadable ⇒ withheld), and everything else (including any column with no usable schema) admits byte-equality only, failing `!=`/`>`/`<` closed. Both `HasRowFilter` and `RowVisible` fail closed on a denied or unresolved grant: `HasRowFilter` is the gate in front of `RowVisible`, so it must answer *true* there or the whole-bucket fast path skips the check entirely. - **canonical.go** — the one rendering layer for comparison operands: every value a `filter` or `check` compares — a JWT claim (`CanonicalScalar`), a policy-authored literal (`CanonicalNumericLiteral`), an ingested payload value (`numericCanonical`) — converges on one exact canonical decimal form (positional, digit-bounded, never a float64 round-trip), so what a read filter binds and what the stream compares can't drift; `scalarString` is the deliberate exception, the raw byte rendering that `Text`/`Opaque` equality compares. - **numeric.go** — compares canonical forms the way the column that stores them would: `compareCanonicalDecimals` orders by exact digit-string arithmetic, and `NumericSpec` first narrows both operands the way ClickHouse narrows the stored value and the bound constant — `Float32`/`Float64` width rounding, `Decimal` scale truncation, integers exact at any width, with an operand outside the column's width or a `Decimal`'s precision budget refused rather than modeled; the `tests/integration` differential oracle holds in-range verdicts equal to a live ClickHouse's and asserts the never-admit-where-SQL-hides direction for the refused out-of-range operands. -- **source.go** — `Source`, a `func() *Policy` the auth middleware and the `/v1/ops` gate read per call, so a settings reload applies to the very next request; in production it is the default tenant's `settings.Store.Policy`, and `Static(p)` fixes one for tests. The tenant-aware surfaces take a keyed variant that resolves to the same `Store.Policy`: `api.PolicySource` (`func(*settings.Store) *policy.Policy`) for ingest, structured query and pipes, and `stream.PolicySource` (`func(tenant.ID) *policy.Policy`) for the hub. A `nil` result is a deliberate lockout. +- **source.go** — `Source`, a `func() *Policy` the auth middleware and the `/v1/ops` gate (over a flat settings directory) read per call, so a settings reload applies to the very next request; in production it is the default tenant's `settings.Store.Policy`, and `Static(p)` fixes one for tests. The tenant-aware surfaces take a keyed variant that resolves to the same `Store.Policy`: `api.PolicySource` (`func(*settings.Store) *policy.Policy`) for ingest, structured query and pipes, and `stream.PolicySource` (`func(tenant.ID) *policy.Policy`) for the hub. A `nil` result is a deliberate lockout. ### `pipes/` — Named Query Pipes @@ -177,13 +177,14 @@ The package's design invariants — stdout always 100%, WARN+ERROR always export ### `settings/` — Settings Directory -The hot-reloadable half of configuration: a directory of four JSON files (`config.json`, `roles.json`, `policies.json`, `pipes.json`) that the server validates at boot and re-adopts while running — `Validate` is the single gate (strict decode, per-file shape rules, and the cross-file check that every role a policy grant or pipe allowlist names is declared in `roles.json`), and `Store` holds the adopted `Document` as one atomic snapshot. `Store` is also the runtime authority for access control and pipes: it implements `policy.Source` (`Store.Policy`, nil when `policies.json` is `{}` — fail closed) and `pipes.Source`, and there is no other copy — the files are the only write path. See [Settings Directory](/settings-directory). +The hot-reloadable half of configuration: a directory of four JSON files (`config.json`, `roles.json`, `policies.json`, `pipes.json`) — or, [nested](/deployment#the-nested-settings-directory), one folder of those four per tenant — that the server validates at boot and re-adopts while running. `Validate` is the single gate (strict decode, per-file shape rules, and the cross-file check that every role a policy grant or pipe allowlist names is declared in `roles.json`), a `Store` holds one tenant's adopted `Document` as one atomic snapshot, and the `Registry` maps tenant ids to stores and owns every reload. `Store` is also the runtime authority for access control and pipes: it implements `policy.Source` (`Store.Policy`, nil when `policies.json` is `{}` — fail closed) and `pipes.Source`, and there is no other copy — the files are the only write path. See [Settings Directory](/settings-directory). -- **validate.go** — `Validate(dir)` reads, decodes, and checks the directory in one pass (strict JSON — unknown fields and duplicate keys are errors; per-file shape rules; cross-file role references) and returns every `Finding` at once. Shared by `wavehouse validate`, boot, and every reload. +- **validate.go** — `ValidateDir(dir)` reads, decodes, and checks one directory of the four files — a flat root, or one tenant's folder — in one pass (strict JSON — unknown fields and duplicate keys are errors; per-file shape rules; cross-file role references) and returns every `Finding` at once. Shared, by way of `Validate`, by `wavehouse validate`, boot, and every reload. - **finding.go** — `Finding` / `Severity`: errors make the directory invalid, warnings don't block adoption. The JSON shape is part of the ops API (`POST /v1/ops/settings/reload` returns them). -- **store.go** — `Store` owns the adopted snapshot. `Open` validates and adopts at boot; `Reload` re-validates and swaps the document atomically when there are no errors (a rejected reload keeps the previous snapshot). Consumers read typed accessors per call (`ClickHouse()`, `Auth()`, `DedupeFor(table)`, `DLQFor(table)`, `Keepalive()`, …) rather than holding values, and `AfterAdopt` registers hooks (dedupe store open/close, keepalive-wheel rebuild) that run after each successful reload. -- **registry.go** — `Registry` maps a tenant id to its `Store` (`For(id)`). It holds the one store `Open` adopted, under `tenant.Default`; reload and the watcher stay on the `Store`. -- **watch.go** — fsnotify on the *directory* (not the files, so atomic-writer replaces and Kubernetes ConfigMap symlink swaps aren't lost), debounced into one reload; reloads once as soon as the watch exists so an edit between the boot read and the watch is never missed. `SIGHUP` and the reload endpoint funnel through the same serialized `Reload`. +- **tree.go** — `Validate(root)` reads the directory's shape off its entries and checks either one: a root holding any of the four file names is flat and goes through `ValidateDir` untouched; otherwise a root holding a folder is nested, each folder name going through `tenant.Parse` and each folder through `ValidateDir`, with the folder leading every finding's `File` (`acme/policies.json`). The `Tree` it returns is nil when the finding is about the root itself — it cannot be listed, or a nested root holds a loose file or an entry that cannot be stat'ed. +- **store.go** — `Store` is a passive holder: one tenant's adopted document behind an atomic pointer, swapped by the registry. Consumers read typed accessors per call (`ClickHouse()`, `Auth()`, `DedupeFor(table)`, `DLQFor(table)`, `Keepalive()`, …) rather than holding values. +- **registry.go** — `Registry` maps a tenant id to its `Store` and owns everything that changes one. `Open` validates and adopts at boot; `Reload` re-validates the whole directory and `ReloadTenant` one tenant's folder, serialized with each other; `AfterAdopt` hooks run after a reload with the tenants it adopted; `For(id)` and `All()` see only the tenants being served, and `Resolve(id)` tells a rejected tenant from an unknown one. The shape is fixed at `Open`. Flat: an invalid directory refuses boot, and a rejected reload keeps the previous snapshot. Nested: fail closed per tenant — a folder with an error finding stops being served (the store keeps its document for requests already admitted, and gets the next good one) while the rest carry on; a whole-directory reload mirrors the folders; and a finding about the directory itself refuses boot or rejects the reload whole, leaving every tenant as it was. The tenant map is replaced whole by a reload, so a lookup is one lock-free load. +- **watch.go** — `Registry.Watch`, which `internal/app` starts for a flat directory only: fsnotify on the *directory* (not the files, so atomic-writer replaces and Kubernetes ConfigMap symlink swaps aren't lost), debounced into one reload; reloads once as soon as the watch exists so an edit between the boot read and the watch is never missed. `SIGHUP` and the reload endpoint funnel through the same serialized `Reload`. - **seed.go** / **seed/** — The embedded (`go:embed`) starter directory with every key at its default. The binary carries no compiled defaults: `wavehouse bootstrap [dir]` writes this seed, and the compose stack and e2e fixture ship copies of it. ### `tenant/` — Tenant Identifier @@ -205,7 +206,7 @@ The hot-reloadable half of configuration: a directory of four JSON files (`confi ```text wrap=false Client POST /v1/ingest?table={table} → Tenant resolution: X-Tenant-ID → settings.Registry → the request's *settings.Store - (absent = tenant 0; 400 malformed / 404 unknown, before auth) + (absent = tenant 0; 400 malformed / 404 unknown / 503 rejected, before auth) → JWT auth middleware (always runs; token optional) → Look up table schema from SchemaRegistry → Policy check: role allowed to insert into this table (before the body is parsed) @@ -299,7 +300,7 @@ The proxy-pattern wins are: zero classification logic on the WaveHouse side (no ```text Client GET /v1/stream → Tenant resolution: X-Tenant-ID → settings.Registry → the request's *settings.Store - (absent = tenant 0; 400 malformed / 404 unknown, before auth) + (absent = tenant 0; 400 malformed / 404 unknown / 503 rejected, before auth) → JWT auth middleware (always runs; token optional) → Announce the caller's projected column list as an `event: schema` frame (no `id:`, so it never moves Last-Event-ID) BEFORE registering, so a client diff --git a/docs/src/content/docs/deployment.md b/docs/src/content/docs/deployment.md index 358e4ac5..075828da 100644 --- a/docs/src/content/docs/deployment.md +++ b/docs/src/content/docs/deployment.md @@ -332,7 +332,7 @@ WaveHouse serves plain HTTP on `:8080` and does **not** terminate TLS, manage ce Most deployments serve one tenant and can skip this section: send no `X-Tenant-ID` header and none of it applies, with one exception — [a proxy that already sends the header](#upgrading-behind-a-proxy-that-already-sends-x-tenant-id). -A *tenant* here is a [settings directory](/settings-directory): the `X-Tenant-ID` request header selects whose `roles.json`, `policies.json`, `pipes.json`, and `config.json` serve the request. The header is client-supplied and resolved before authentication, so it is **not** a row-isolation boundary — it picks which `policies.json` applies, and scoping a caller to their own rows stays that policy's job, from a value in the signed token ([row-level security](/access-control#row-level-security)). Tenant selection and row scoping are different axes. +A *tenant* here is one set of the four [settings files](/settings-directory): the `X-Tenant-ID` request header selects whose `roles.json`, `policies.json`, `pipes.json`, and `config.json` serve the request. The header is client-supplied and resolved before authentication, so it is **not** a row-isolation boundary — it picks which `policies.json` applies, and scoping a caller to their own rows stays that policy's job, from a value in the signed token ([row-level security](/access-control#row-level-security)). Tenant selection and row scoping are different axes. Every `/v1` route outside `/v1/ops/*` resolves the tenant before it authenticates the request: @@ -340,7 +340,7 @@ Every `/v1` route outside `/v1/ops/*` resolves the tenant before it authenticate X-Tenant-ID: 0 ``` -A request without the header, or with an empty one, resolves to tenant `0`, the default tenant, whose settings are the settings directory. A settings directory defines that one tenant, so any other id is unknown. Setting the header on every request is the client's or the fronting proxy's job; WaveHouse never derives it from the token. +A request without the header, or with an empty one, resolves to tenant `0`, the default tenant. A settings directory that holds the four files itself defines that one tenant, so any other id is unknown; [a nested settings directory](#the-nested-settings-directory) defines one tenant per folder. Setting the header on every request is the client's or the fronting proxy's job; WaveHouse never derives it from the token. A tenant id is 1–64 characters of ASCII letters, digits, `_`, and `-`. It is a string, not a number, so a long numeric id keeps every digit. @@ -348,16 +348,49 @@ A tenant id is 1–64 characters of ASCII letters, digits, `_`, and `-`. It is a | ------ | ---- | ---- | | `400` | `{"error": "invalid X-Tenant-ID: …"}` | The id breaks the grammar above, or the header was sent more than once | | `404` | `{"error": "unknown tenant: "}` | The id is well formed but no such tenant exists | +| `503` | `{"error": "tenant settings are invalid"}` | The tenant exists but its settings folder was rejected ([nested directories](#the-nested-settings-directory) only) | -Both are decided before authentication, so they are returned whatever token the request carries. Every response that passes through tenant resolution — a route's own answer and these two alike — carries `Vary: X-Tenant-ID`, so a shared cache that stores one keys it on the header. A router-level `405` and the CORS preflight `204` are answered before tenant resolution and carry no such `Vary`; neither depends on the tenant. `Vary` covers the tenant and nothing else: a response also depends on who is asking, which is why [a shared cache must not store the authenticated reads](/reverse-proxy#header-and-auth-forwarding). +All three are decided before authentication, so they are returned whatever token the request carries — which is why the `503` says nothing about what was wrong with the settings. Every response that passes through tenant resolution — a route's own answer and these three alike — carries `Vary: X-Tenant-ID`, so a shared cache that stores one keys it on the header. A router-level `405` and the CORS preflight `204` are answered before tenant resolution and carry no such `Vary`; neither depends on the tenant. `Vary` covers the tenant and nothing else: a response also depends on who is asking, which is why [a shared cache must not store the authenticated reads](/reverse-proxy#header-and-auth-forwarding). The probes (`/livez`, `/readyz`, `/healthz`, and the deprecated `/health` and `/ready`), `/version`, the Prometheus metrics path, and `/v1/ops/*` are tenant-exempt: they ignore the header entirely. `X-Tenant-ID` is in the CORS `Access-Control-Allow-Headers` list, so a browser client can send it cross-origin. The SDK sends it through [`options.headers`](/sdk#custom-headers). +### The nested settings directory + +Serving more than one tenant from one process takes a settings directory that holds one folder per tenant instead of the four files: + +```text +settings/ +├── acme/ +│ ├── config.json +│ ├── pipes.json +│ ├── policies.json +│ └── roles.json +└── globex/ + ├── config.json + ├── pipes.json + ├── policies.json + └── roles.json +``` + +That is the layout a control plane writes, and it does not answer queries on its own: tenant `0`'s folder supplies the wiring the whole process shares — the ClickHouse address included — so a directory that serves no tenant `0` boots with none. "What a tenant's folder decides", below, lists what comes from where. + +The folder name is the tenant id, and each folder is a complete settings directory: everything on the [Settings Directory](/settings-directory) page applies to it as written, except where the rules below say otherwise. The two shapes don't mix — a folder beside the four files, or a loose file beside the folders, is a validation error — and a running server keeps the shape it booted with, so switching is stop, restructure, start. Dot-prefixed entries are ignored in either shape. `wavehouse validate` checks either shape with the same exit codes; a finding in a nested directory names its folder (`acme/policies.json`), and a folder whose name is not a tenant id is a finding of its own — that folder is skipped, and the rest of the directory still loads. + +**A rejected folder fails closed, for that tenant alone — tenant `0`'s excepted.** A folder that fails validation stops its tenant being served — its requests answer `503` — while every other tenant carries on, at boot and on a reload alike. Tenant `0` is the exception: the process draws its shared wiring from that folder, so rejecting it costs every tenant ("What a lost tenant `0` costs", below, says what). There is no fall back to the tenant's previous settings, unlike [the single-tenant directory](/settings-directory#loading-and-hot-reload): the recovery is fixing the folder and reloading it. A request already in flight finishes on the settings it started with. The findings go to the log and to the reload response, never into the `503`. A finding about the directory itself — a loose file, an entry or a directory that can't be read, a changed shape — is another matter: it refuses boot, and on a reload it rejects the reload whole and leaves every tenant as it was. + +**Reloading is the writer's call.** A nested directory is not watched, because a watcher would validate a folder halfway through being written and drop its tenant. Whoever writes a tenant's folder reloads it once it is complete: `POST /v1/ops/settings/reload?tenant=acme` re-validates that folder and reads nothing else. It must name a tenant the server already holds (`404` otherwise), so a folder the server does not hold yet — one added since the last whole-directory reload — is picked up by a whole-directory reload, not by naming it; a tenant it holds but rejected is reloaded by name like any other. Without the parameter — and on `SIGHUP` — the whole directory is reloaded and mirrors its folders: a new folder becomes a tenant, and a removed one becomes unknown. A whole-directory reload re-validates every folder, so it carries the exposure the watcher would: a folder caught halfway through being written can fail validation, and its tenant then stops being served until a later reload adopts it. The response is the [single-tenant one](/api#post-v1opssettingsreload--reload-settings-directory). After a whole-directory reload, `adopted: false` with a `422` can mean adopted in part: the folders with an error among their `findings` were rejected and the rest were adopted — warnings included, since `findings` carries every folder's. + +**The admin routes take the operator key only.** `/v1/ops/*` reaches every tenant, so over a nested directory no tenant's admin role opens it: the [operator key](/api#authentication) alone does, and a token carrying an admin role gets `403`. Boot a nested directory without `auth.operator_key` and no caller can reach these routes at all, which leaves `SIGHUP` as the only reload; the server warns about it at boot. `GET /v1/ops/pipes` and `GET /v1/ops/pipes/{name}` take the same `?tenant=`, and read tenant `0` without it. The other `/v1/ops/*` routes — schema, DLQ stats, raw SQL — act on the resources the whole process shares and ignore the parameter. On the three routes that take it the parameter is parsed strictly — a query string that does not parse, an empty or repeated `tenant`, or a malformed id is a `400`, never a silent read of the default tenant or, on the reload route, a reload of every tenant. The SDK sends it as the [`tenant` option](/sdk/admin#settings--whsettings). + +**What a tenant's folder decides, and what tenant `0`'s does.** A request is evaluated against its own tenant's `policies.json` and `pipes.json` (ingest, structured queries, pipes), its `query.*` keys, and its `dedupe` block — by which id, and whether its records are deduplicated at all, provided tenant `0`'s `dedupe.enabled` has the store open: with the store closed they are published un-deduped and counted by `wavehouse_ingest_dedupe_disabled_total`, which then climbs steadily rather than only across a reload, as it does for [a directory that holds the four files](/settings-directory#deduplication). The process still has one ClickHouse connection, one message queue, one dedupe store, one token verifier, and one event hub, and those follow tenant `0`'s folder: `clickhouse.*`, `auth.*`, `mq.max_bytes_gb`, `cors.allowed_origins`, `schema.refresh_interval`, `stream.gap_window_minutes`, `dlq.*`, and whether the dedupe store is open at all (tenant `0`'s `dedupe.enabled`). So every tenant reads and writes the same ClickHouse, a token that verifies is accepted under any tenant's header, and a tenant's own `policies.json` does not govern its event stream: every `GET /v1/stream` connection is authorized by tenant `0`'s policy, and delivery is by table alone, so a subscriber under any tenant's header sees every tenant's rows for that table. The one shared setting that weighs every tenant is the SSE keepalive: the wheel runs at the shortest `stream.keepalive_interval` among the tenants being served, with that tenant's `stream.keepalive_buckets`. + +**What a lost tenant `0` costs.** A `0` folder that a reload rejects or removes stops tenant `0` being served like any other, and what becomes of the shared settings depends on how they are read. `clickhouse.*`, `auth.*`, `mq.max_bytes_gb`, `cors.allowed_origins`, and tenant `0`'s `dedupe.enabled` stay as tenant `0` last adopted them. The background paths read tenant `0` as unset instead: the sweeper's gap window falls to zero, which purges the acknowledged history that gap-fill replays; every table's failed rows are parked on the DLQ; the event hub reads no policy, which withholds every `GET /v1/stream` subscriber's rows; and schema discovery keeps the refresh cadence it had. A nested directory that has never served a tenant `0` — no `0` folder, or one rejected at boot — has no ClickHouse address: it boots, reports degraded on `/livez`, and answers no query. Outside `/v1/ops/*`, a `/v1` request that sends no `X-Tenant-ID` resolves to tenant `0`, so with no `0` folder it answers `404 unknown tenant: 0` (`503` with a rejected one) — the SDK's `/v1/health` reachability ping included. + ### Upgrading behind a proxy that already sends `X-Tenant-ID` -`X-Tenant-ID` is a generic name, and some gateways and service meshes stamp one on every request. WaveHouse used to ignore it; now any value other than `0` names an unknown tenant, so **every `/v1` route outside `/v1/ops/*` answers `404 unknown tenant: `** — the SDK's `/v1/health` reachability ping included, while the bare probes and the admin surface stay green. Strip the inbound header at the edge ([header forwarding](/reverse-proxy#header-and-auth-forwarding)) unless you are using it deliberately. +`X-Tenant-ID` is a generic name, and some gateways and service meshes stamp one on every request. WaveHouse used to ignore it; now, over a settings directory that holds the four files, any value other than `0` names an unknown tenant, so **every `/v1` route outside `/v1/ops/*` answers `404 unknown tenant: `** — the SDK's `/v1/health` reachability ping included, while the bare probes and the admin surface stay green. Strip the inbound header at the edge ([header forwarding](/reverse-proxy#header-and-auth-forwarding)) unless you are using it deliberately. ## ClickHouse Schema diff --git a/docs/src/content/docs/reverse-proxy.mdx b/docs/src/content/docs/reverse-proxy.mdx index 5f425e3b..4d671c6a 100644 --- a/docs/src/content/docs/reverse-proxy.mdx +++ b/docs/src/content/docs/reverse-proxy.mdx @@ -194,7 +194,7 @@ These limit the *whole* request regardless of traffic, so no keepalive extends t - **`Authorization`** — forward verbatim. WaveHouse validates a `Bearer` JWT (resolving the role from it) or an `Operator ` [operator credential](/access-control#operator-key). Most proxies forward `Authorization` unchanged, so it's the transport to prefer for the operator key — the exception to check for is an auth-terminating layer that consumes or rewrites the header (e.g. a gateway doing its own auth). - **`X-Operator-Key`** (optional) — only relevant if you present the [operator key](/access-control#operator-key) via this alias header instead of `Authorization: Operator `. Custom request headers are forwarded by default, but confirm your proxy doesn't strip it — and note nginx silently drops header names containing **underscores** (this one uses hyphens, so it's fine as named). The `Authorization` form needs none of this. -- **`X-Tenant-ID`** — **strip it at the edge unless you are deliberately using it.** It selects the [tenant](/deployment#multi-tenant-deployments) (absent means tenant `0`) and any other value is an unknown tenant, so a gateway or mesh that stamps its own `X-Tenant-ID` turns every `/v1` route outside `/v1/ops/*` into a `404` — the SDK's `/v1/health` reachability ping included. It is resolved before authentication and never derived from the token, so a proxy that owns tenant selection must **replace** a client-supplied value, not add to it: two `X-Tenant-ID` lines are refused with `400`. That is a stricter version of the append-vs-replace hazard of `Content-Type` below — here even two agreeing lines are refused. The name is hyphenated, so nginx's underscore rule does not apply. +- **`X-Tenant-ID`** — **strip it at the edge unless you are deliberately using it.** It selects the [tenant](/deployment#multi-tenant-deployments) (absent means tenant `0`), and a settings directory that isn't nested defines no other tenant, so a gateway or mesh that stamps its own `X-Tenant-ID` turns every `/v1` route outside `/v1/ops/*` into a `404` — the SDK's `/v1/health` reachability ping included. It is resolved before authentication and never derived from the token, so a proxy that owns tenant selection must **replace** a client-supplied value, not add to it: two `X-Tenant-ID` lines are refused with `400`. That is a stricter version of the append-vs-replace hazard of `Content-Type` below — here even two agreeing lines are refused. The name is hyphenated, so nginx's underscore rule does not apply. - **`X-Forwarded-For` / `X-Forwarded-Proto` / `Host`** — set these for your own logs and any upstream that reads them. WaveHouse does not currently derive a client IP from `X-Forwarded-For` (see the caution below); forwarding it is good hygiene and is what the trusted-proxy client-IP work ([#333](https://github.com/Wave-RF/WaveHouse/issues/333)) will consume. :::caution[Don't expose `:8080` directly] diff --git a/docs/src/content/docs/sdk/admin.md b/docs/src/content/docs/sdk/admin.md index df5513e2..e2137583 100644 --- a/docs/src/content/docs/sdk/admin.md +++ b/docs/src/content/docs/sdk/admin.md @@ -3,7 +3,7 @@ title: "SDK Admin & System" description: "Schema introspection, settings reload, DLQ stats, and health checks in @wavehouse/sdk." --- -Operational surfaces of `@wavehouse/sdk`. With one exception, everything on this page sits behind the server's admin gate: the caller must resolve to the admin role (`policy.admin_role`) or present the non-JWT [operator key](/api#authentication) — the SDK has no first-class operator-key option, but [`options.headers`](/sdk#custom-headers) can carry the `X-Operator-Key` header. The exception is `wh.sys.health()`, which calls the public, content-free `/v1/health` route and needs no credentials. See [Access Control](/access-control) for how roles resolve. Examples import from `@wavehouse/sdk` or `https://esm.sh/@wavehouse/sdk` (see [Imports & Runtimes](/sdk#imports--runtimes)). +Operational surfaces of `@wavehouse/sdk`. With one exception, everything on this page sits behind the server's admin gate: the caller must resolve to the admin role (`policy.admin_role`) or present the non-JWT [operator key](/api#authentication) — the SDK has no first-class operator-key option, but [`options.headers`](/sdk#custom-headers) can carry the `X-Operator-Key` header. Over a [nested settings directory](/deployment#the-nested-settings-directory) the operator key alone opens them, and an admin-role token gets `403`. The exception is `wh.sys.health()`, which calls the public, content-free `/v1/health` route and needs no credentials. See [Access Control](/access-control) for how roles resolve. Examples import from `@wavehouse/sdk` or `https://esm.sh/@wavehouse/sdk` (see [Imports & Runtimes](/sdk#imports--runtimes)). ## Schema — `wh.schema` @@ -36,6 +36,12 @@ const { data, error } = await wh.settings.reload(); // { adopted: false, findings } and the previous settings stay in effect ``` +Over [a nested settings directory](/deployment#the-nested-settings-directory), pass `tenant` to reload that tenant's folder alone: a `422` then means the folder was rejected and the tenant is no longer served (its requests answer `503`), not that its previous settings stayed. Without `tenant` the whole directory is reloaded, and a `422` can mean adopted in part. + +```ts +const { data, error } = await wh.settings.reload({ tenant: 'acme' }); +``` + --- ## DLQ — `wh.dlq` diff --git a/docs/src/content/docs/sdk/pipes.md b/docs/src/content/docs/sdk/pipes.md index 14d370c9..1bae86bf 100644 --- a/docs/src/content/docs/sdk/pipes.md +++ b/docs/src/content/docs/sdk/pipes.md @@ -41,3 +41,10 @@ const { data: pipes } = await wh.pipes.list(); const { data: pipe } = await wh.pipes.get('top_pages'); // pipe: { name, sql, parameters, description, allowed_roles } ``` + +Both take a `tenant` option, sent as `?tenant=`, for a server with [a nested settings directory](/deployment#the-nested-settings-directory) — the admin routes ignore the `X-Tenant-ID` header, so [`options.headers`](/sdk#custom-headers) cannot select one. Without it they read the default tenant. Over a nested directory these routes take the [operator key](/api#authentication) alone; an admin-role token gets `403`. + +```ts +const { data: acmePipes } = await wh.pipes.list({ tenant: 'acme' }); +const { data: acmePipe } = await wh.pipes.get('top_pages', { tenant: 'acme' }); +``` diff --git a/docs/src/content/docs/sdk/reference.md b/docs/src/content/docs/sdk/reference.md index 32a94614..d860eb46 100644 --- a/docs/src/content/docs/sdk/reference.md +++ b/docs/src/content/docs/sdk/reference.md @@ -32,7 +32,7 @@ The SDK **never throws** for anything the server returns — all API errors come | 403 | `HTTP_403` | No | Insufficient permissions | | 404 | `HTTP_404` | No | Table, pipe, or tenant not found | | 500 | `HTTP_500` | Yes | Server error (retried per `maxRetries`) | -| 503 | `HTTP_503` | Yes | Service unavailable (auto-retries with `Retry-After`) | +| 503 | `HTTP_503` | Yes | Service unavailable, or a tenant whose settings folder was rejected (auto-retries, honoring `Retry-After` when the response carries one) | | 0 | `NETWORK_ERROR` | Yes | Network failure (retried with exponential backoff) | | 0 | `ABORTED` | No | Request canceled via `AbortSignal` | | 0 | `SSE_CONNECT_ERROR` | No | Stream could not be started (e.g. a non-absolute `baseURL`) | @@ -56,7 +56,7 @@ On REST, `ABORTED` is the one error raised *by* a backoff rather than by an atte On a stream, a retryable failure is re-dialed on a jittered exponential backoff (capped at 30s, and reset only once a connection has held for a few seconds — so a server that accepts and instantly closes still backs off), with the `status` callback moving `reconnecting` → `live`. -Rejected requests surface the real status and message rather than an opaque connection failure — in a browser going cross-origin, though, only when the rejection passes CORS and the gateway answered whatever preflight the request triggers — `Authorization`, configured `headers`, or `Last-Event-ID` once the stream resumes; a rejected preflight or a response without `Access-Control-Allow-Origin` reaches you as a retryable network error instead — indistinguishable from a drop, and retried. Any `4xx` ends the stream, since repeating the request won't usually talk whatever rejected it round — the exception being a `429` or `408` from a fronting rate limiter, which is transient even though the stream still ends, so catch it and open a new one after a delay ([#469](https://github.com/Wave-RF/WaveHouse/issues/469)). Note that **WaveHouse never rejects a stream for authentication**: `/v1/stream` is ungated, so an expired or missing token resolves to `default_role` and you get a `200` with a filtered view, not a `401`. The 4xx it raises itself are `400` for a missing or empty `table` and, when you send `X-Tenant-ID`, the `400`/`404` of [tenant resolution](/deployment#multi-tenant-deployments); any other `404` or `405` means the request never reached that route, most often a `baseURL` path prefix your proxy didn't strip. Any other 4xx comes from something in front — an auth gateway, a proxy. That silent-downgrade behavior is exactly why `auth` is re-read on every connection attempt, and [#239](https://github.com/Wave-RF/WaveHouse/issues/239) tracks enforcing expiry server-side. `SSE_CONNECT_ERROR` and `SSE_NO_STREAM_BODY` are configuration faults, so fix the cause and start a new stream. +Rejected requests surface the real status and message rather than an opaque connection failure — in a browser going cross-origin, though, only when the rejection passes CORS and the gateway answered whatever preflight the request triggers — `Authorization`, configured `headers`, or `Last-Event-ID` once the stream resumes; a rejected preflight or a response without `Access-Control-Allow-Origin` reaches you as a retryable network error instead — indistinguishable from a drop, and retried. Any `4xx` ends the stream, since repeating the request won't usually talk whatever rejected it round — the exception being a `429` or `408` from a fronting rate limiter, which is transient even though the stream still ends, so catch it and open a new one after a delay ([#469](https://github.com/Wave-RF/WaveHouse/issues/469)). Note that **WaveHouse never rejects a stream for authentication**: `/v1/stream` is ungated, so an expired or missing token resolves to `default_role` and you get a `200` with a filtered view, not a `401`. The 4xx it raises itself are `400` for a missing or empty `table` and, when you send `X-Tenant-ID`, the `400`/`404` of [tenant resolution](/deployment#multi-tenant-deployments) — which over a [nested settings directory](/deployment#the-nested-settings-directory) can also answer a `503` for a tenant whose settings were rejected, and that one is retryable, so the stream re-dials rather than ending; any other `404` or `405` means the request never reached that route, most often a `baseURL` path prefix your proxy didn't strip. Any other 4xx comes from something in front — an auth gateway, a proxy. That silent-downgrade behavior is exactly why `auth` is re-read on every connection attempt, and [#239](https://github.com/Wave-RF/WaveHouse/issues/239) tracks enforcing expiry server-side. `SSE_CONNECT_ERROR` and `SSE_NO_STREAM_BODY` are configuration faults, so fix the cause and start a new stream. `SSE_PARSE_ERROR` is the one code that isn't a connection outcome: it's reported and *skipped*, and the connection keeps reading — one bad frame shouldn't cost you the stream. For an ordinary bad frame its `retryable: true` is therefore vestigial — nothing is re-dialed. Two exceptions, one to each half of that reported-and-skipped rule. A frame the SDK can't turn into a row is skipped but never *reported* — `console.warn` and dropped, with no `error` callback: `data` that isn't valid JSON, a row arriving before any `event: schema` frame, a `row` that is valid JSON but not an array, or a row whose length disagrees with the announced column list. Every one of those is **bounded** — three per cause per connection, then one "further occurrences suppressed" line, with the malformed-schema frame below counted as its own cause. So against a server that never announces a schema you get a handful of lines rather than one per row, and a quiet console is **not** evidence the stream is healthy. A malformed schema frame is the one to watch, because it discards the list rather than keeping a stale one — so every row after it is dropped until the next announcement or a reconnect. The parser's 16 MiB buffer cap is reported but not *skipped*: an overflow terminates the parser, so the transport stops reading and reconnects rather than feeding it again. @@ -95,8 +95,8 @@ createClient(config) → WaveHouseClient │ ├── .fetch(opts?) → Promise> // { signal } only — no limit │ └── .stream(opts?) → StreamController ├── .pipes (admin) -│ ├── .list() → Promise> -│ └── .get(name) → Promise> +│ ├── .list(opts?) → Promise> +│ └── .get(name, opts?) → Promise> ├── .sql(query, opts?) → Promise> (admin) ├── .schema (admin) │ ├── .list() → Promise> diff --git a/docs/src/content/docs/sdk/streaming.md b/docs/src/content/docs/sdk/streaming.md index 0c391ccc..a4492830 100644 --- a/docs/src/content/docs/sdk/streaming.md +++ b/docs/src/content/docs/sdk/streaming.md @@ -127,7 +127,7 @@ The SSE reader and writer changed in the same release. A **new SDK against an ol `@wavehouse/sdk` publishes to npm independently of the server, so pinning the SDK in a frontend while the backend upgrades on its own schedule (or the reverse) is the normal deployment shape. ::: -A `4xx` is terminal and surfaces through `error` with the real status code rather than an opaque connection failure — in a browser going cross-origin, only when the rejection passes CORS and the gateway answered whatever preflight the request triggers (`Authorization`, configured `headers`, or `Last-Event-ID` once the stream resumes); otherwise it arrives as a retryable network error instead — indistinguishable from a drop, and retried. It won't be an *authentication* rejection from WaveHouse, which leaves `/v1/stream` ungated and answers an expired token with a filtered view rather than a `401`; the 4xx it raises itself are `400` for a missing or empty table name and, when you send `X-Tenant-ID`, the `400`/`404` of [tenant resolution](/deployment#multi-tenant-deployments) — any other `404` or `405` means the request never reached the route, usually a `baseURL` path prefix the proxy didn't strip. Anything else means something in front of it (an auth gateway, a proxy) turned the request away — and note the exception to "retrying wouldn't help": a `429` or `408` from a rate limiter *is* transient, but the stream still ends, so catch it and open a new one after a delay ([#469](https://github.com/Wave-RF/WaveHouse/issues/469)). See [Error Handling](/sdk/reference#error-handling) for every code a stream can report and which ones re-dial. +A `4xx` is terminal and surfaces through `error` with the real status code rather than an opaque connection failure — in a browser going cross-origin, only when the rejection passes CORS and the gateway answered whatever preflight the request triggers (`Authorization`, configured `headers`, or `Last-Event-ID` once the stream resumes); otherwise it arrives as a retryable network error instead — indistinguishable from a drop, and retried. It won't be an *authentication* rejection from WaveHouse, which leaves `/v1/stream` ungated and answers an expired token with a filtered view rather than a `401`; the 4xx it raises itself are `400` for a missing or empty table name and, when you send `X-Tenant-ID`, the `400`/`404` of [tenant resolution](/deployment#multi-tenant-deployments) (over a [nested settings directory](/deployment#the-nested-settings-directory) it can also answer a `503` for a tenant whose settings were rejected, which is retryable, so the stream re-dials rather than ending) — any other `404` or `405` means the request never reached the route, usually a `baseURL` path prefix the proxy didn't strip. Anything else means something in front of it (an auth gateway, a proxy) turned the request away — and note the exception to "retrying wouldn't help": a `429` or `408` from a rate limiter *is* transient, but the stream still ends, so catch it and open a new one after a delay ([#469](https://github.com/Wave-RF/WaveHouse/issues/469)). See [Error Handling](/sdk/reference#error-handling) for every code a stream can report and which ones re-dial. Streams go through `options.fetch`, `options.headers`, and `options.fetchOptions` like every other request — which is what lets a stream reach a header-gated origin. A custom `fetch` is asked more of on this path; see [Supplying your own fetch](/sdk#supplying-your-own-fetch). diff --git a/docs/src/content/docs/settings-directory.mdx b/docs/src/content/docs/settings-directory.mdx index 3ab3c498..fec19fb0 100644 --- a/docs/src/content/docs/settings-directory.mdx +++ b/docs/src/content/docs/settings-directory.mdx @@ -31,6 +31,8 @@ A reload that fails validation is logged (and reported by the endpoint) and the "Previous good settings" is the in-memory snapshot of the running process, nothing more: there is no persisted copy of the files. A restart re-validates the directory from scratch and refuses to start on the same findings the reload rejected, so bad files never survive a restart silently — fix them (or run `wavehouse validate`) before bouncing the server. +A directory that holds one folder per tenant instead of the four files is [a nested settings directory](/deployment#the-nested-settings-directory): each folder is everything this page describes, but it is not watched, a rejected folder stops its tenant being served rather than keeping the previous settings, and the keys the whole process shares are not read from the tenant's own folder: they come from tenant `0`'s, bar the SSE keepalive, which follows the shortest interval among the tenants being served — that section lists which keys. + Every adoption — boot and every reload — goes through the same `Validate`, so the policy, the roles, and the pipes are checked with the current rules each time they are read; there is no stored copy that can skip validation. All four files are adopted as one snapshot: a request is evaluated against the policy, pipes, and tunables of a single adoption, never a mix. Every file is decoded strictly: an unknown key, a duplicate key, a UTF-8 byte order mark, an all-whitespace file, or a top-level `null` is an error, so a misspelled rule can never load as "no rule". @@ -163,7 +165,7 @@ What stays in boot config is only what cannot change under a running process — Every dedupe knob lives here — there are no boot-config keys for it. The switch and its fields are resolved per record from one snapshot (table override → global value): -- `dedupe.enabled` (seed default `false`) — turns deduplication on. Hot-reloadable: a reload that flips it opens the embedded Pebble store at `/pebble` (or closes it), so no restart is needed; seen ids persist across an off/on cycle. If the store fails to open on a reload, the failure is logged and ingest fails closed (`500 dedupe failed`) until the next reload or restart — the files asked for dedupe, so publishing un-deduped is not a fallback. At boot a failed open refuses to start, like every other store. A record that lands in the instant of the flip itself is published un-deduped: if the settings already say on but the store is not yet open, it's counted by `wavehouse_ingest_dedupe_disabled_total`; in the reverse case (settings already say off, store still open) the handler skips dedupe like any other disabled record and nothing is counted. That counter should only ever tick during a reload, so a steadily climbing rate means the store and the settings have come apart. +- `dedupe.enabled` (seed default `false`) — turns deduplication on. Hot-reloadable: a reload that flips it opens the embedded Pebble store at `/pebble` (or closes it), so no restart is needed; seen ids persist across an off/on cycle. If the store fails to open on a reload, the failure is logged and ingest fails closed (`500 dedupe failed`) until the next reload or restart — the files asked for dedupe, so publishing un-deduped is not a fallback. At boot a failed open refuses to start, like every other store. A record that lands in the instant of the flip itself is published un-deduped: if the settings already say on but the store is not yet open, it's counted by `wavehouse_ingest_dedupe_disabled_total`; in the reverse case (settings already say off, store still open) the handler skips dedupe like any other disabled record and nothing is counted. Over a directory that holds the four files that counter should only ever tick during a reload, so a steadily climbing rate means the store and the settings have come apart; over [a nested directory](/deployment#the-nested-settings-directory) it climbs for any tenant whose `dedupe.enabled` is on while tenant `0`'s is off. - `dedupe.id_field` (seed default `event_id`) — JSON field name in the ingest body used as the dedup key. - `dedupe.require_id` (seed default `false`) — controls what happens to a row missing `id_field` (which can't be deduped, so idempotency wouldn't apply to it). Such a row is always logged at `WARN` and counted by `wavehouse_ingest_dedupe_missing_id_total`, in both modes. `false`: it is then published un-deduped. `true` rejects it instead (`400` for a single insert; a per-record failure in a batch) — a server-side tripwire for producers that must guarantee the id. - `dedupe.tables..{id_field, require_id}` — per-table overrides; each entry overrides only the fields it names and inherits the rest. diff --git a/internal/api/pipes.go b/internal/api/pipes.go index e1800391..470e07bd 100644 --- a/internal/api/pipes.go +++ b/internal/api/pipes.go @@ -24,12 +24,13 @@ type PipesHandler struct { // Source yields a tenant's pipes (the store itself in production). Source func(*settings.Store) pipes.Source PolicySource PolicySource // resolves empty role to default_role; may be nil - // OpsStore is the store the admin reads (List, Get) serve: /v1/ops is - // tenant-exempt, so they carry no request tenant and read the default one. - OpsStore *settings.Store - CHConn driver.Conn - Cache cache.Cache - sf singleflight.Group + // Tenants resolves the tenant the admin reads (List, Get) serve: /v1/ops + // is tenant-exempt, so they carry no request tenant and read the one + // ?tenant= names, the default one without it (opsStore). + Tenants *settings.Registry + CHConn driver.Conn + Cache cache.Cache + sf singleflight.Group // queryTimeout bounds each pipe execution, read per request // (chconn.Manager.QueryTimeout in production) so a settings reload // applies without a restart. @@ -47,20 +48,28 @@ func NewPipesHandler(source func(*settings.Store) pipes.Source, policySource Pol return &PipesHandler{Source: source, PolicySource: policySource, CHConn: conn, Cache: c, queryTimeout: queryTimeout} } -// List returns all named queries (admin endpoint). -func (h *PipesHandler) List(w http.ResponseWriter, _ *http.Request) { +// List returns all named queries of the ?tenant= (admin endpoint). +func (h *PipesHandler) List(w http.ResponseWriter, r *http.Request) { + store, ok := opsStore(w, r, h.Tenants) + if !ok { + return + } w.Header().Set("Content-Type", "application/json") - q := h.Source(h.OpsStore).Pipes() + q := h.Source(store).Pipes() if q == nil { q = []*pipes.NamedQuery{} } _ = json.NewEncoder(w).Encode(q) } -// Get returns a specific named query (admin endpoint). +// Get returns a specific named query of the ?tenant= (admin endpoint). func (h *PipesHandler) Get(w http.ResponseWriter, r *http.Request) { + store, ok := opsStore(w, r, h.Tenants) + if !ok { + return + } name := chi.URLParam(r, "name") - q := h.Source(h.OpsStore).Pipe(name) + q := h.Source(store).Pipe(name) if q == nil { writeJSONError(w, http.StatusNotFound, "pipe not found") return diff --git a/internal/api/pipes_test.go b/internal/api/pipes_test.go index 59dc892b..374386de 100644 --- a/internal/api/pipes_test.go +++ b/internal/api/pipes_test.go @@ -13,6 +13,8 @@ import ( "github.com/Wave-RF/WaveHouse/internal/auth" "github.com/Wave-RF/WaveHouse/internal/pipes" "github.com/Wave-RF/WaveHouse/internal/policy" + "github.com/Wave-RF/WaveHouse/internal/settings" + "github.com/Wave-RF/WaveHouse/internal/tenant" "github.com/Wave-RF/WaveHouse/internal/testutil" "github.com/go-chi/chi/v5" "github.com/golang-jwt/jwt/v5" @@ -48,7 +50,7 @@ func TestPipesHandler_List(t *testing.T) { &pipes.NamedQuery{Name: "recent", SQL: "SELECT * FROM clicks ORDER BY ts DESC LIMIT 10"}, ) h := NewPipesHandler(store, nil, nil, nil, noTimeout) - h.OpsStore = testStore + h.Tenants = testTenants() w := httptest.NewRecorder() r := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/v1/ops/pipes", nil) @@ -66,7 +68,7 @@ func TestPipesHandler_Get_Found(t *testing.T) { &pipes.NamedQuery{Name: "top_pages", SQL: "SELECT page FROM clicks"}, ) h := NewPipesHandler(store, nil, nil, nil, noTimeout) - h.OpsStore = testStore + h.Tenants = testTenants() w := httptest.NewRecorder() r := pipesRequest(t, http.MethodGet, "/v1/ops/pipes/top_pages", "top_pages", nil) @@ -82,7 +84,7 @@ func TestPipesHandler_Get_NotFound(t *testing.T) { t.Parallel() store := staticPipes() h := NewPipesHandler(store, nil, nil, nil, noTimeout) - h.OpsStore = testStore + h.Tenants = testTenants() w := httptest.NewRecorder() r := pipesRequest(t, http.MethodGet, "/v1/ops/pipes/nope", "nope", nil) @@ -93,11 +95,106 @@ func TestPipesHandler_Get_NotFound(t *testing.T) { testutil.AssertJSONErrorResponse(t, w) } +// The admin reads serve the default tenant, which a nested settings directory +// need not hold and may hold rejected: the tenant routes' 404 and 503, never +// a nil store. +func TestPipesHandler_AdminReads_DefaultTenantNotServed(t *testing.T) { + t.Parallel() + tests := []struct { + name string + configs map[string]string + wantStatus int + wantBody string + }{ + {name: "no 0 folder", configs: map[string]string{"acme": fullConfig(100)}, wantStatus: http.StatusNotFound, wantBody: "unknown tenant: 0"}, + {name: "rejected 0 folder", configs: map[string]string{"acme": fullConfig(100), "0": `{"unknown_key": true}`}, wantStatus: http.StatusServiceUnavailable, wantBody: "tenant settings are invalid"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + h := NewPipesHandler(staticPipes(&pipes.NamedQuery{Name: "top_pages", SQL: "SELECT 1"}), nil, nil, nil, noTimeout) + h.Tenants = nestedTenants(t, tt.configs) + + reads := map[string]func(http.ResponseWriter, *http.Request){"list": h.List, "get": h.Get} + for name, read := range reads { + w := httptest.NewRecorder() + read(w, pipesRequest(t, http.MethodGet, "/v1/ops/pipes/top_pages", "top_pages", nil)) + assert.Equal(t, tt.wantStatus, w.Code, name) + assert.Contains(t, w.Body.String(), tt.wantBody, name) + testutil.AssertJSONErrorResponse(t, w) + } + }) + } +} + +// The admin reads name their tenant in ?tenant=, parsed strictly: the store +// the pipes source receives is that tenant's, a query that does not parse is +// a 400 rather than a read of the default tenant, and a tenant that cannot be +// served gets the tenant routes' 404 or 503. +func TestPipesHandler_AdminReads_TenantParam(t *testing.T) { + t.Parallel() + tenants := nestedTenants(t, map[string]string{"0": fullConfig(100), "acme": fullConfig(100), "globex": `{"unknown_key": true}`}) + tests := []struct { + name, query string + wantStatus int + wantTenant tenant.ID // on 200, whose store the source was handed + wantBody string + }{ + {name: "no parameter is the default tenant", query: "", wantStatus: http.StatusOK, wantTenant: tenant.Default}, + {name: "named tenant", query: "tenant=acme", wantStatus: http.StatusOK, wantTenant: "acme"}, + {name: "explicit default tenant", query: "tenant=0", wantStatus: http.StatusOK, wantTenant: tenant.Default}, + {name: "an unrelated parameter changes nothing", query: "tenant=acme&pretty=1", wantStatus: http.StatusOK, wantTenant: "acme"}, + {name: "rejected tenant", query: "tenant=globex", wantStatus: http.StatusServiceUnavailable, wantBody: "tenant settings are invalid"}, + {name: "unknown tenant", query: "tenant=initech", wantStatus: http.StatusNotFound, wantBody: "unknown tenant: initech"}, + {name: "empty value is not absent", query: "tenant=", wantStatus: http.StatusBadRequest, wantBody: "invalid ?tenant: tenant id is empty"}, + {name: "repeated, even agreeing", query: "tenant=acme&tenant=acme", wantStatus: http.StatusBadRequest, wantBody: "sent more than once"}, + {name: "malformed id", query: "tenant=a.b", wantStatus: http.StatusBadRequest, wantBody: "invalid ?tenant"}, + {name: "semicolon pair", query: "tenant=acme;x=1", wantStatus: http.StatusBadRequest, wantBody: "invalid query string"}, + {name: "bad escape", query: "tenant=%zz", wantStatus: http.StatusBadRequest, wantBody: "invalid query string"}, + {name: "a malformed pair elsewhere refuses the read too", query: "tenant=acme&x=%zz", wantStatus: http.StatusBadRequest, wantBody: "invalid query string"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + for _, read := range []string{"list", "get"} { + var handed *settings.Store + h := NewPipesHandler(func(s *settings.Store) pipes.Source { + handed = s + return pipes.Static(&pipes.NamedQuery{Name: "top_pages", SQL: "SELECT 1"}) + }, nil, nil, nil, noTimeout) + h.Tenants = tenants + + w := httptest.NewRecorder() + if read == "list" { + r := pipesRequest(t, http.MethodGet, "/v1/ops/pipes", "", nil) + r.URL.RawQuery = tt.query + h.List(w, r) + } else { + r := pipesRequest(t, http.MethodGet, "/v1/ops/pipes/top_pages", "top_pages", nil) + r.URL.RawQuery = tt.query + h.Get(w, r) + } + + require.Equal(t, tt.wantStatus, w.Code, "%s: %s", read, w.Body.String()) + if tt.wantStatus != http.StatusOK { + assert.Nil(t, handed, "%s: a refused read must not reach the pipes source", read) + assert.Contains(t, w.Body.String(), tt.wantBody, read) + testutil.AssertJSONErrorResponse(t, w) + continue + } + want, ok := tenants.For(tt.wantTenant) + require.True(t, ok) + assert.Same(t, want, handed, read) + } + }) + } +} + func TestPipesHandler_List_Empty(t *testing.T) { t.Parallel() store := staticPipes() h := NewPipesHandler(store, nil, nil, nil, noTimeout) - h.OpsStore = testStore + h.Tenants = testTenants() w := httptest.NewRecorder() r := pipesRequest(t, http.MethodGet, "/v1/ops/pipes", "", nil) diff --git a/internal/api/router.go b/internal/api/router.go index 84b2e033..f8fa83e5 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -39,6 +39,8 @@ type Dependencies struct { Tenants *settings.Registry // PolicySource backs the RequireAdmin gate: the admin role (policy.AdminRole) // is read live from the adopted policy, so admin_role changes apply on reload. + // NewRouter ignores it when Tenants is nested, whatever was wired here: the + // ops gate then admits the operator key alone (see NewRouter). PolicySource policy.Source // CORSOrigins returns the allowed CORS origins, read per request so a // settings reload applies immediately (settings.Store.CORSOrigins in @@ -166,8 +168,19 @@ func NewRouter(deps Dependencies) http.Handler { // role is policy.AdminRole (configurable via admin_role, "admin" by // default), read live from the policy store so changes apply // without a restart. + // + // A nested settings directory has no one policy to read an admin + // role from, and no tenant's admin may act on another tenant — these + // routes reach every tenant — so there the gate reads no policy at + // all and the operator key alone passes; a token admin gets 403 + // (#583). Decided here rather than by what the caller wired, so + // tenant 0's policy can never end up guarding a nested ops tree. + adminPolicy := deps.PolicySource + if deps.Tenants != nil && deps.Tenants.Nested() { + adminPolicy = nil + } r.Use(deps.AuthMW) - r.Use(RequireAdmin(deps.PolicySource)) + r.Use(RequireAdmin(adminPolicy)) // Schema discovery. r.Get("/schema", deps.Schema.Get) @@ -250,7 +263,9 @@ func jsonRecoverer(next http.Handler) http.Handler { // via a role — IsAdmin(nil) is false — so no token can re-open a locked-out // deployment through this gate. The exception is the operator key: // auth.IsOperator passes this gate even under a nil policy, so an operator can -// still trigger a settings reload after fixing the files (break-glass). +// still trigger a settings reload after fixing the files (break-glass). A nil +// store is the same gate with nothing to read — the operator key alone — which +// is what NewRouter mounts over a nested settings directory. // // Authentication is decoupled from this gate: a missing/invalid/expired token // resolves to an empty (non-admin) role and is denied here. Denials go through diff --git a/internal/api/router_test.go b/internal/api/router_test.go index f1955755..441ab9dc 100644 --- a/internal/api/router_test.go +++ b/internal/api/router_test.go @@ -5,13 +5,17 @@ import ( "errors" "net/http" "net/http/httptest" + "os" + "path/filepath" "sync" "testing" "github.com/Wave-RF/WaveHouse/internal/auth" "github.com/Wave-RF/WaveHouse/internal/discovery" "github.com/Wave-RF/WaveHouse/internal/mq" + "github.com/Wave-RF/WaveHouse/internal/pipes" "github.com/Wave-RF/WaveHouse/internal/policy" + "github.com/Wave-RF/WaveHouse/internal/settings" "github.com/Wave-RF/WaveHouse/internal/stream" "github.com/Wave-RF/WaveHouse/internal/tenant" "github.com/Wave-RF/WaveHouse/internal/testutil" @@ -337,7 +341,7 @@ func TestNewRouter_RoutesRegistered(t *testing.T) { Version: NewVersionHandler("test", "test", "test"), Schema: NewSchemaHandler(reg), DLQ: NewDLQHandler(emb), - Pipes: &PipesHandler{Source: staticPipes(), PolicySource: staticPolicy(&policy.Policy{}), OpsStore: testStore}, + Pipes: &PipesHandler{Source: staticPipes(), PolicySource: staticPolicy(&policy.Policy{}), Tenants: testTenants()}, AuthMW: func(next http.Handler) http.Handler { return next }, PolicySource: policy.Static(&policy.Policy{}), } @@ -564,6 +568,144 @@ func TestNewRouter_RawSQLAdminGate(t *testing.T) { }) } +// Over a nested settings directory the ops tree reaches every tenant, so no +// tenant's admin role may open it: the operator key alone passes, and a token +// carrying the admin role gets the same 403 as anyone else. The router decides +// this from the registry's shape — the PolicySource wired below would admit +// "admin", and is what a caller binding tenant 0's policy would pass. A flat +// directory keeps the gate it always had. +func TestNewRouter_NestedOpsGateAdmitsTheOperatorKeyAlone(t *testing.T) { + t.Parallel() + reg := testutil.NewTestSchemaRegistry(t, nil) + routerOver := func(tenants *settings.Registry) http.Handler { + pipesHandler := NewPipesHandler(staticPipes(), nil, nil, nil, noTimeout) + pipesHandler.Tenants = tenants + return NewRouter(Dependencies{ + Tenants: tenants, + Ingest: NewIngestHandler(reg, &testutil.MockPublisher{}), + Query: &QueryHandler{}, + SSE: NewStreamHandler(stream.NewHub(tenant.Default, nil, nil, nil), nil), + Health: &HealthHandler{}, + Schema: NewSchemaHandler(reg), + Pipes: pipesHandler, + Settings: NewSettingsHandler(tenants), + AuthMW: func(next http.Handler) http.Handler { return next }, + PolicySource: policy.Static(&policy.Policy{}), + }) + } + flatTenants, _ := settings.Open(writeSettingsFixture(t, fullConfig(100))) + require.NotNil(t, flatTenants) + routers := map[string]http.Handler{ + "nested": routerOver(nestedTenants(t, map[string]string{"0": fullConfig(100), "acme": fullConfig(100)})), + "flat": routerOver(flatTenants), + } + routes := []struct{ method, path string }{ + {http.MethodGet, "/v1/ops/schema"}, + {http.MethodGet, "/v1/ops/pipes"}, + {http.MethodPost, "/v1/ops/settings/reload"}, + } + callers := []struct { + name string + ctx context.Context + passesNested bool + passesFlat bool + }{ + {name: "operator key", ctx: auth.WithOperator(context.Background()), passesNested: true, passesFlat: true}, + {name: "admin token", ctx: auth.WithRole(context.Background(), "admin"), passesFlat: true}, + {name: "viewer token", ctx: auth.WithRole(context.Background(), "viewer")}, + {name: "no token", ctx: context.Background()}, + } + for shape, router := range routers { + for _, caller := range callers { + for _, route := range routes { + t.Run(shape+" "+caller.name+" "+route.path, func(t *testing.T) { + t.Parallel() + rec := httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequestWithContext(caller.ctx, route.method, route.path, nil)) + passes := caller.passesFlat + if shape == "nested" { + passes = caller.passesNested + } + if passes { + assert.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + return + } + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + testutil.AssertJSONErrorResponse(t, rec) + }) + } + } + } +} + +// The ?token= strip and the strict ?tenant= parse meet only in the router. +// AuthMW runs first, and its strip used to re-encode the query on the way +// through, which erased the very pair opsTenant exists to refuse: an admin's +// `?tenant=acme;x=1&token=…` answered 200 with the default tenant's pipes, +// and on the reload route would have reloaded every tenant. A handler-level +// test cannot see that — the middleware that rewrote the URL never runs in +// one — so this goes through NewRouter with the real authenticator. The +// subtests share the directory and run in order, so nothing here is parallel. +func TestNewRouter_MalformedTenantSurvivesTheTokenStrip(t *testing.T) { + dir := writeSettingsFixture(t, fullConfig(100)) + tenants, _ := settings.Open(dir) + require.NotNil(t, tenants) + store, _ := tenants.For(tenant.Default) + authn, err := auth.NewAuthenticator(auth.Config{JWTSecret: testutil.TestJWTSecret, RoleClaim: "role"}, store.Policy) + require.NoError(t, err) + reg := testutil.NewTestSchemaRegistry(t, nil) + pipesHandler := NewPipesHandler(func(s *settings.Store) pipes.Source { return s }, nil, nil, nil, noTimeout) + pipesHandler.Tenants = tenants + router := NewRouter(Dependencies{ + Tenants: tenants, + Ingest: NewIngestHandler(reg, &testutil.MockPublisher{}), + Query: &QueryHandler{}, + SSE: NewStreamHandler(stream.NewHub(tenant.Default, nil, nil, nil), nil), + Health: &HealthHandler{}, + Schema: NewSchemaHandler(reg), + Pipes: pipesHandler, + Settings: NewSettingsHandler(tenants), + AuthMW: authn.Middleware(), + PolicySource: store.Policy, + }) + token := "token=" + testutil.MakeJWT(t, map[string]any{"role": "admin"}) + do := func(method, target string) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequestWithContext(t.Context(), method, target, nil)) + return rec + } + + t.Run("the query token still authenticates", func(t *testing.T) { + assert.Equal(t, http.StatusOK, do(http.MethodGet, "/v1/ops/pipes?"+token).Code) + assert.Equal(t, http.StatusOK, do(http.MethodGet, "/v1/ops/pipes?tenant=0&"+token).Code) + assert.Equal(t, http.StatusForbidden, do(http.MethodGet, "/v1/ops/pipes?tenant=0").Code, "and nothing else does") + }) + + t.Run("a malformed tenant beside it is refused, not read as the default tenant", func(t *testing.T) { + for _, target := range []string{ + "/v1/ops/pipes?tenant=acme;x=1&" + token, + "/v1/ops/pipes?" + token + "&tenant=acme;x=1", + "/v1/ops/pipes?tenant=%zz&" + token, + "/v1/ops/pipes/top_pages?tenant=acme;x=1&" + token, + } { + rec := do(http.MethodGet, target) + assert.Equal(t, http.StatusBadRequest, rec.Code, "%s: %s", target, rec.Body.String()) + testutil.AssertJSONErrorResponse(t, rec) + assert.NotContains(t, rec.Body.String(), "eyJ", "the refusal must not echo the token") + } + }) + + t.Run("and on the reload route reloads nothing", func(t *testing.T) { + require.NoError(t, os.WriteFile(filepath.Join(dir, settings.FileConfig), []byte(fullConfig(200)), 0o600)) + rec := do(http.MethodPost, "/v1/ops/settings/reload?tenant=acme;x=1&"+token) + assert.Equal(t, http.StatusBadRequest, rec.Code, "body: %s", rec.Body.String()) + assert.Equal(t, 100, store.DefaultMaxRows(), "a misread ?tenant= must not become a whole-tree reload") + + assert.Equal(t, http.StatusOK, do(http.MethodPost, "/v1/ops/settings/reload?"+token).Code) + assert.Equal(t, 200, store.DefaultMaxRows()) + }) +} + func TestNewRouter_OptionalDepsNil(t *testing.T) { t.Parallel() diff --git a/internal/api/settings.go b/internal/api/settings.go index a25b92f8..1be7af4e 100644 --- a/internal/api/settings.go +++ b/internal/api/settings.go @@ -13,11 +13,11 @@ import ( // none there is nothing to reload, so the route is simply absent (the same // pattern as the DLQ and policy handlers). type SettingsHandler struct { - Store *settings.Store + Tenants *settings.Registry } -func NewSettingsHandler(store *settings.Store) *SettingsHandler { - return &SettingsHandler{Store: store} +func NewSettingsHandler(tenants *settings.Registry) *SettingsHandler { + return &SettingsHandler{Tenants: tenants} } // reloadResponse is the POST /v1/ops/settings/reload body: whether the @@ -33,8 +33,29 @@ type reloadResponse struct { // serialized reload path SIGHUP and the directory watcher run. 200 when the // directory was adopted (warnings included in the body), 422 when validation // rejected it and the previous settings remain in effect. +// +// ?tenant= narrows the reload to that tenant's folder of a nested directory +// (400 malformed, 404 unknown): adopted then speaks for that folder alone, +// and a 422 means the tenant is no longer served. Without it the whole tree +// is reloaded, and over a nested directory a 422 can mean adopted in part — +// the error findings name the folders that were not +// (settings.Registry.Reload). func (h *SettingsHandler) Reload(w http.ResponseWriter, r *http.Request) { - findings, adopted := h.Store.Reload("api") + id, named, ok := opsTenant(w, r) + if !ok { + return + } + var findings []settings.Finding + var adopted bool + if named { + var known bool + if findings, adopted, known = h.Tenants.ReloadTenant(id, "api"); !known { + writeJSONError(w, http.StatusNotFound, "unknown tenant: "+id.String()) + return + } + } else { + findings, adopted = h.Tenants.Reload("api") + } if findings == nil { findings = []settings.Finding{} } diff --git a/internal/api/settings_test.go b/internal/api/settings_test.go index 2f3a3499..e9ff4ce8 100644 --- a/internal/api/settings_test.go +++ b/internal/api/settings_test.go @@ -10,6 +10,8 @@ import ( "testing" "github.com/Wave-RF/WaveHouse/internal/settings" + "github.com/Wave-RF/WaveHouse/internal/tenant" + "github.com/Wave-RF/WaveHouse/internal/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -40,9 +42,10 @@ func writeSettingsFixture(t *testing.T, configJSON string) string { // case adopted survives), so neither the parent nor the subtests are parallel. func TestSettingsReload(t *testing.T) { dir := writeSettingsFixture(t, fullConfig(100)) - store, _ := settings.Open(dir) - require.NotNil(t, store) - h := NewSettingsHandler(store) + tenants, _ := settings.Open(dir) + require.NotNil(t, tenants) + store, _ := tenants.For(tenant.Default) + h := NewSettingsHandler(tenants) post := func() (*httptest.ResponseRecorder, reloadResponse) { rec := httptest.NewRecorder() @@ -70,3 +73,109 @@ func TestSettingsReload(t *testing.T) { assert.Equal(t, 200, store.DefaultMaxRows(), "rejected reload must keep the previous snapshot") }) } + +// ?tenant= narrows the reload to one tenant's folder: the folder beside it is +// not read, a rejected folder is a 422 whose findings carry the folder, and a +// query that does not parse is a 400 that reloads nothing — never the whole +// tree the absent parameter means. Subtests are ordered on purpose, so +// neither the parent nor the subtests are parallel. +func TestSettingsReload_TenantParam(t *testing.T) { + tenants := nestedTenants(t, map[string]string{"acme": fullConfig(100), "globex": fullConfig(100)}) + acme, _ := tenants.For("acme") + globex, _ := tenants.For("globex") + h := NewSettingsHandler(tenants) + rewrite := func(folder, config string) { + require.NoError(t, os.WriteFile(filepath.Join(tenants.Dir(), folder, settings.FileConfig), []byte(config), 0o600)) + } + post := func(query string) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/v1/ops/settings/reload", nil) + req.URL.RawQuery = query + h.Reload(rec, req) + return rec + } + body := func(rec *httptest.ResponseRecorder) reloadResponse { + var b reloadResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &b)) + return b + } + + t.Run("a named tenant reloads that folder alone", func(t *testing.T) { + rewrite("acme", fullConfig(200)) + rewrite("globex", fullConfig(200)) + rec := post("tenant=acme") + assert.Equal(t, http.StatusOK, rec.Code) + assert.True(t, body(rec).Adopted) + assert.Equal(t, 200, acme.DefaultMaxRows()) + assert.Equal(t, 100, globex.DefaultMaxRows(), "the folder beside it was not read") + }) + + t.Run("a query that does not parse reloads nothing", func(t *testing.T) { + for _, query := range []string{"tenant=globex;x=1", "tenant=%zz", "tenant=", "tenant=acme&tenant=globex", "tenant=a.b"} { + rec := post(query) + assert.Equal(t, http.StatusBadRequest, rec.Code, query) + testutil.AssertJSONErrorResponse(t, rec) + } + assert.Equal(t, 100, globex.DefaultMaxRows(), "a refused reload must not fall back to the whole tree") + }) + + t.Run("an unknown tenant is a 404", func(t *testing.T) { + rec := post("tenant=initech") + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Contains(t, rec.Body.String(), "unknown tenant: initech") + }) + + t.Run("a rejected folder is a 422 naming the folder, and the tenant stops being served", func(t *testing.T) { + rewrite("globex", `{"unknown_key": true}`) + rec := post("tenant=globex") + assert.Equal(t, http.StatusUnprocessableEntity, rec.Code) + b := body(rec) + assert.False(t, b.Adopted) + require.NotEmpty(t, b.Findings) + assert.Equal(t, "globex/config.json", b.Findings[0].File) + _, ok := tenants.For("globex") + assert.False(t, ok) + _, ok = tenants.For("acme") + assert.True(t, ok) + }) + + t.Run("no parameter reloads the whole tree: adopted in part is a 422", func(t *testing.T) { + rewrite("acme", fullConfig(300)) + rec := post("") + assert.Equal(t, http.StatusUnprocessableEntity, rec.Code) + b := body(rec) + assert.False(t, b.Adopted, "globex is still rejected, so not everything was adopted") + assert.Equal(t, "globex/config.json", b.Findings[0].File) + assert.Equal(t, 300, acme.DefaultMaxRows(), "the tenant beside the rejected one was adopted") + + rewrite("globex", fullConfig(400)) + rec = post("") + assert.Equal(t, http.StatusOK, rec.Code) + assert.True(t, body(rec).Adopted) + recovered, ok := tenants.For("globex") + require.True(t, ok) + assert.Equal(t, 400, recovered.DefaultMaxRows()) + }) +} + +// A flat directory has the default tenant and no other, so ?tenant=0 is the +// reload it always was and any other id is unknown. +func TestSettingsReload_TenantParam_FlatDirectory(t *testing.T) { + dir := writeSettingsFixture(t, fullConfig(100)) + tenants, _ := settings.Open(dir) + require.NotNil(t, tenants) + store, _ := tenants.For(tenant.Default) + h := NewSettingsHandler(tenants) + post := func(query string) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/v1/ops/settings/reload", nil) + req.URL.RawQuery = query + h.Reload(rec, req) + return rec + } + + require.NoError(t, os.WriteFile(filepath.Join(dir, settings.FileConfig), []byte(fullConfig(200)), 0o600)) + assert.Equal(t, http.StatusOK, post("tenant=0").Code) + assert.Equal(t, 200, store.DefaultMaxRows()) + assert.Equal(t, http.StatusNotFound, post("tenant=acme").Code) +} diff --git a/internal/api/tenant.go b/internal/api/tenant.go index 6900b44a..afef1433 100644 --- a/internal/api/tenant.go +++ b/internal/api/tenant.go @@ -4,6 +4,7 @@ import ( "context" "log/slog" "net/http" + "net/url" "github.com/Wave-RF/WaveHouse/internal/policy" "github.com/Wave-RF/WaveHouse/internal/settings" @@ -46,12 +47,79 @@ func requestStore(w http.ResponseWriter, r *http.Request) (*settings.Store, bool return store, ok } +// resolveStore is the one answer for a tenant that cannot be served: a 404 +// for an id the registry does not hold, and a 503 for a tenant it holds whose +// settings folder was rejected (nested directories only). The 503 says +// nothing more: tenant routes resolve before authentication, and the findings +// quote the tenant's settings — they go to the log and to the reload +// response, which is behind the ops gate. +func resolveStore(w http.ResponseWriter, tenants *settings.Registry, id tenant.ID) (*settings.Store, bool) { + store, known := tenants.Resolve(id) + switch { + case store != nil: + return store, true + case known: + writeJSONError(w, http.StatusServiceUnavailable, "tenant settings are invalid") + default: + writeJSONError(w, http.StatusNotFound, "unknown tenant: "+id.String()) + } + return nil, false +} + +// opsTenantParam names the tenant an ops route addresses. The ops tree is +// tenant-exempt — no TenantMW, the header ignored — so a caller that means one +// tenant says so in the query string. +const opsTenantParam = "tenant" + +// opsTenant reads opsTenantParam, strictly: the whole query must parse. +// ParseQuery skips a pair it cannot read and keeps going, so a lenient read +// of `?tenant=acme;x=1` sees no tenant at all — the default tenant on a read, +// every tenant on a reload — and answers 200 for a request it misread. An +// empty value is refused for the same reason rather than read as absent. +// named reports whether the parameter was sent; ok is false once a 400 has +// been written. +func opsTenant(w http.ResponseWriter, r *http.Request) (id tenant.ID, named, ok bool) { + params, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + writeJSONError(w, http.StatusBadRequest, "invalid query string: "+err.Error()) + return "", false, false + } + values, named := params[opsTenantParam] + if !named { + return "", false, true + } + if len(values) > 1 { + writeJSONError(w, http.StatusBadRequest, "invalid ?"+opsTenantParam+": sent more than once") + return "", false, false + } + id, err = tenant.Parse(values[0]) + if err != nil { + writeJSONError(w, http.StatusBadRequest, "invalid ?"+opsTenantParam+": "+err.Error()) + return "", false, false + } + return id, true, true +} + +// opsStore resolves the store an ops read serves: the tenant opsTenantParam +// names, tenant.Default when it names none, with TenantMW's answers when that +// tenant cannot be served. +func opsStore(w http.ResponseWriter, r *http.Request, tenants *settings.Registry) (*settings.Store, bool) { + id, named, ok := opsTenant(w, r) + if !ok { + return nil, false + } + if !named { + id = tenant.Default + } + return resolveStore(w, tenants, id) +} + // TenantMW resolves the request's tenant before authentication runs and // stores it in the request context: the tenant.Header value, tenant.Default -// when absent. A malformed id is a 400 and a well-formed id the registry does -// not hold is a 404. A repeated header is refused rather than picked from, so -// a value a proxy sets can never be shadowed by one the client sent. Every -// answer, the 400 and 404 included, carries Vary: X-Tenant-ID so a shared +// when absent. A malformed id is a 400; an id that cannot be served is +// resolveStore's 404 or 503. A repeated header is refused rather than picked +// from, so a value a proxy sets can never be shadowed by one the client sent. +// Every answer, the refusals included, carries Vary: X-Tenant-ID so a shared // cache cannot replay one tenant's response to another — added, not set, so // the CORS Vary: Origin survives. func TenantMW(tenants *settings.Registry) func(http.Handler) http.Handler { @@ -72,9 +140,8 @@ func TenantMW(tenants *settings.Registry) func(http.Handler) http.Handler { } id = parsed } - store, ok := tenants.For(id) + store, ok := resolveStore(w, tenants, id) if !ok { - writeJSONError(w, http.StatusNotFound, "unknown tenant: "+id.String()) return } next.ServeHTTP(w, r.WithContext(WithStore(r.Context(), store))) diff --git a/internal/api/tenant_helpers_test.go b/internal/api/tenant_helpers_test.go index 4f991efd..bd1ad12b 100644 --- a/internal/api/tenant_helpers_test.go +++ b/internal/api/tenant_helpers_test.go @@ -2,6 +2,11 @@ package api import ( "net/http" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" "github.com/Wave-RF/WaveHouse/internal/pipes" "github.com/Wave-RF/WaveHouse/internal/policy" @@ -22,6 +27,20 @@ func withTenant(r *http.Request) *http.Request { // testTenants is a registry whose default tenant is testStore. func testTenants() *settings.Registry { return settings.NewRegistry(testStore) } +// nestedTenants opens a nested settings directory, one folder per entry: +// tenant folder → its config.json (fullConfig for a tenant that is served, +// anything Validate rejects for one that is not). +func nestedTenants(t *testing.T, configs map[string]string) *settings.Registry { + t.Helper() + root := t.TempDir() + for folder, config := range configs { + require.NoError(t, os.Rename(writeSettingsFixture(t, config), filepath.Join(root, folder))) + } + tenants, _ := settings.Open(root) + require.NotNil(t, tenants) + return tenants +} + // staticPolicy is a PolicySource fixed to p, whatever the tenant. func staticPolicy(p *policy.Policy) PolicySource { return func(*settings.Store) *policy.Policy { return p } diff --git a/internal/api/tenant_test.go b/internal/api/tenant_test.go index 4099ba1f..1e22c9ca 100644 --- a/internal/api/tenant_test.go +++ b/internal/api/tenant_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/Wave-RF/WaveHouse/internal/pipes" "github.com/Wave-RF/WaveHouse/internal/policy" "github.com/Wave-RF/WaveHouse/internal/settings" "github.com/Wave-RF/WaveHouse/internal/stream" @@ -62,6 +63,121 @@ func TestTenantMW(t *testing.T) { } } +// In a nested settings directory a tenant whose folder was rejected is known +// but not served: a 503, never the 404 of an unknown tenant, and never a body +// that quotes the settings — the middleware answers before authentication. +func TestTenantMW_NestedDirectory(t *testing.T) { + t.Parallel() + tenants := nestedTenants(t, map[string]string{"acme": fullConfig(100), "globex": `{"unknown_key": true}`}) + acme, ok := tenants.For("acme") + require.True(t, ok) + + tests := []struct { + name, header string + wantStatus int + wantBody string + }{ + {name: "served tenant", header: "acme", wantStatus: http.StatusOK}, + {name: "rejected tenant", header: "globex", wantStatus: http.StatusServiceUnavailable, wantBody: `{"error":"tenant settings are invalid"}`}, + {name: "unknown tenant", header: "initech", wantStatus: http.StatusNotFound, wantBody: `{"error":"unknown tenant: initech"}`}, + {name: "no header is tenant 0, which a nested directory need not hold", wantStatus: http.StatusNotFound, wantBody: `{"error":"unknown tenant: 0"}`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var resolved *settings.Store + h := TenantMW(tenants)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + resolved, _ = StoreFromContext(r.Context()) + w.WriteHeader(http.StatusOK) + })) + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/health", nil) + if tt.header != "" { + req.Header.Set(tenant.Header, tt.header) + } + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + require.Equal(t, tt.wantStatus, w.Code, "body: %s", w.Body.String()) + assert.Equal(t, []string{tenant.Header}, w.Header().Values("Vary")) + if tt.wantStatus == http.StatusOK { + assert.Same(t, acme, resolved) + return + } + assert.Nil(t, resolved, "a refused request must not reach the handler") + assert.JSONEq(t, tt.wantBody, w.Body.String()) + }) + } +} + +// Threading the tenant is what TenantMW is for, and every other double in +// this package discards the store it is handed (tenant_helpers_test.go) — so a +// handler that passed the wrong one down (nil, tenant 0's, a captured one) +// would pass all of them. This drives the real router over a two-tenant +// registry and checks the store each handler's getters actually received. The +// recorded policy is nil, which denies, so every route answers 403 straight +// after asking. +// +// The subtests share the recorder and must alternate tenants through one +// router — that is what would expose a captured store — so neither the parent +// nor the subtests are parallel. +func TestNewRouter_HandlersReceiveTheRequestTenantsStore(t *testing.T) { + tenants := nestedTenants(t, map[string]string{"acme": fullConfig(100), "globex": fullConfig(200)}) + var handed []*settings.Store + recordPolicy := func(s *settings.Store) *policy.Policy { + handed = append(handed, s) + return nil + } + recordPipes := func(s *settings.Store) pipes.Source { + handed = append(handed, s) + return pipes.Static(&pipes.NamedQuery{Name: "top_pages", SQL: "SELECT 1"}) + } + reg := testRegistry(t) + ingest := NewIngestHandler(reg, &testutil.MockPublisher{}) + ingest.PolicySource = recordPolicy + router := NewRouter(Dependencies{ + Tenants: tenants, + Ingest: ingest, + StructuredQuery: NewStructuredQueryHandler(nil, nil, reg, recordPolicy, func(*settings.Store) int { return 60 }, noTimeout, nil), + Pipes: NewPipesHandler(recordPipes, recordPolicy, nil, nil, noTimeout), + Query: &QueryHandler{}, + SSE: NewStreamHandler(stream.NewHub(tenant.Default, nil, nil, nil), nil), + Health: &HealthHandler{}, + Version: NewVersionHandler("test", "test", "test"), + Schema: NewSchemaHandler(reg), + AuthMW: func(next http.Handler) http.Handler { return next }, + PolicySource: policy.Static(&policy.Policy{}), + }) + + routes := []struct { + name, path, body string + getters int // store-keyed getters the route consults before it denies + }{ + {name: "ingest", path: "/v1/ingest?table=clicks", body: `{"page": "/"}`, getters: 1}, + {name: "structured query", path: "/v1/query?table=clicks", body: `{}`, getters: 1}, + {name: "pipe execute", path: "/v1/pipes/top_pages", body: `{}`, getters: 2}, + } + for _, route := range routes { + for _, id := range []tenant.ID{"acme", "globex", "acme"} { + t.Run(route.name+" as "+id.String(), func(t *testing.T) { + want, ok := tenants.For(id) + require.True(t, ok) + handed = nil + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, route.path, strings.NewReader(route.body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set(tenant.Header, id.String()) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + require.Equal(t, http.StatusForbidden, w.Code, "body: %s", w.Body.String()) + require.Len(t, handed, route.getters) + for _, got := range handed { + assert.Same(t, want, got) + } + }) + } + } +} + func TestStoreFromContext_Absent(t *testing.T) { t.Parallel() store, ok := StoreFromContext(httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil).Context()) diff --git a/internal/app/app.go b/internal/app/app.go index 3c8b21df..bb9a49d7 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -14,11 +14,12 @@ // testcontainer. // // Each component is wired in one place, as one component value: what New -// opens, what Run loops, and what Close releases. The settings store is +// opens, what Run loops, and what Close releases. The settings registry is // handed whole to each component's wiring function, which derives the -// per-call getters the internal packages take — so when the one store -// becomes a per-tenant registry (#583), the injection points are those -// wiring functions, not main. +// per-call getters the internal packages take: keyed by the request's store +// for the handlers, by tenant id for the async paths (perTenant), and fixed +// to the default tenant for the process-wide resources #583 has not yet made +// per tenant (defaultSetting). package app import ( @@ -30,6 +31,7 @@ import ( "net/http" "os" "os/signal" + "sync/atomic" "time" "golang.org/x/sync/errgroup" @@ -86,11 +88,14 @@ type App struct { logLevel *slog.LevelVar listener net.Listener - // store is the default tenant's settings, which the process-wide - // resources (ClickHouse, dedupe, MQ, auth, CORS, reload) still follow; - // tenants is the registry every tenant-aware path resolves through. - store *settings.Store - tenants *settings.Registry + // tenants is the registry every tenant-aware path resolves through, and + // the owner of every reload. The process-wide resources (ClickHouse, + // dedupe, MQ, auth, CORS) still follow its default tenant, through + // defaultStore: tenant 0's store as of its last adoption (defaultSetting). + tenants *settings.Registry + defaultStore atomic.Pointer[settings.Store] + // policies is the default tenant's policy, for the ops gate and the + // authenticator's operator-key path. policies policy.Source promHandler http.Handler ch *chconn.Manager diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 593188c4..02b290ed 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -25,6 +25,7 @@ import ( "github.com/Wave-RF/WaveHouse/internal/mq" "github.com/Wave-RF/WaveHouse/internal/settings" "github.com/Wave-RF/WaveHouse/internal/tenant" + "github.com/Wave-RF/WaveHouse/internal/testutil/logtest" ) // None of these tests run in parallel: New installs a process-wide default @@ -144,10 +145,11 @@ func TestNew_DegradedBootServesDiagnostics(t *testing.T) { assert.NoError(t, a.Close(context.Background()), "Close is idempotent") } -// The wired registry holds the default tenant only: no header and "0" reach -// the route, any other well-formed id is a 404, a malformed one a 400, and -// the ops tree never looks at the header. A 503 is the handler's own answer — -// boot is degraded without ClickHouse — so it proves the tenant resolved. +// A flat directory's registry holds the default tenant only: no header and +// "0" reach the route, any other well-formed id is a 404, a malformed one a +// 400, and the ops tree never looks at the header. A 503 is the handler's own +// answer — boot is degraded without ClickHouse — so it proves the tenant +// resolved. func TestNew_TenantHeaderResolvesAgainstTheRegistry(t *testing.T) { a := newApp(t, testConfig(t, writeSettings(t, nil)), Options{}) @@ -232,18 +234,273 @@ func TestReload_DrivesTheRegisteredHooks(t *testing.T) { "dedupe": map[string]any{"enabled": true, "id_field": "event_id", "require_id": false, "tables": map[string]any{}}, "mq": map[string]any{"max_bytes_gb": 2}, }) - _, adopted := a.store.Reload("test") + _, adopted := a.tenants.Reload("test") require.True(t, adopted) assert.True(t, a.dedup.Open(), "dedupe hook opened the store") // How the budget is split across the MQ's queues is internal/mq's to test. assert.Equal(t, int64(2<<30), a.mq.MaxBytes(), "mq hook applied the new byte budget") rewriteSettings(t, dir, map[string]any{"mq": map[string]any{"max_bytes_gb": 2}}) - _, adopted = a.store.Reload("test") + _, adopted = a.tenants.Reload("test") require.True(t, adopted) assert.False(t, a.dedup.Open(), "dedupe hook closed the store") } +// writeNestedSettings materializes a nested settings directory: tenant folder +// → the patch writeSettings applies to that tenant's config.json. +func writeNestedSettings(t *testing.T, tenants map[string]map[string]any) string { + t.Helper() + root := t.TempDir() + for folder, patch := range tenants { + require.NoError(t, os.Rename(writeSettings(t, patch), filepath.Join(root, folder))) + } + return root +} + +// invalidQuery is a config.json patch Validate rejects. +var invalidQuery = map[string]any{"query": map[string]any{"default_max_rows": -1, "timestamp_bucket_seconds": 60}} + +func componentNames(a *App) []string { + names := make([]string, len(a.components)) + for i, c := range a.components { + names[i] = c.name + } + return names +} + +// A nested directory boots without a 0 folder and with a rejected tenant: +// each tenant route answers for the tenant its header names, and only the +// rejected one is refused. GET /v1/pipes/{name} stands in for the tenant +// routes because its own 404 needs no ClickHouse, so it proves the handler +// ran with a resolved store. +func TestNew_NestedDirectory(t *testing.T) { + root := writeNestedSettings(t, map[string]map[string]any{"acme": nil, "globex": nil, "broken": invalidQuery}) + a := newApp(t, testConfig(t, root), Options{}) + + assert.NotContains(t, componentNames(a), "settings watcher", "a nested directory is reloaded by whoever wrote the folder, never watched") + flat := newApp(t, testConfig(t, writeSettings(t, nil)), Options{}) + assert.Contains(t, componentNames(flat), "settings watcher") + + pipe := func(header string) *httptest.ResponseRecorder { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/v1/pipes/nope", nil) + if header != "" { + req.Header.Set(tenant.Header, header) + } + rec := httptest.NewRecorder() + a.Handler().ServeHTTP(rec, req) + return rec + } + tests := []struct { + name, header string + wantStatus int + wantBody string + }{ + {name: "served tenant", header: "acme", wantStatus: http.StatusNotFound, wantBody: "pipe not found"}, + {name: "the tenant beside it", header: "globex", wantStatus: http.StatusNotFound, wantBody: "pipe not found"}, + {name: "rejected tenant", header: "broken", wantStatus: http.StatusServiceUnavailable, wantBody: "tenant settings are invalid"}, + {name: "unknown tenant", header: "initech", wantStatus: http.StatusNotFound, wantBody: "unknown tenant: initech"}, + {name: "no header is tenant 0, which this directory does not hold", wantStatus: http.StatusNotFound, wantBody: "unknown tenant: 0"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := pipe(tt.header) + assert.Equal(t, tt.wantStatus, rec.Code) + assert.Contains(t, rec.Body.String(), tt.wantBody) + }) + } + + // Fixing the folder and reloading is the recovery, with no restart. + rewriteSettings(t, filepath.Join(root, "broken"), nil) + _, adopted := a.tenants.Reload("test") + require.True(t, adopted) + assert.Contains(t, pipe("broken").Body.String(), "pipe not found") +} + +// The control plane's loop over a nested directory, through the real wiring: +// write a tenant's folder, then reload that tenant with the operator key. An +// admin token cannot — over a nested directory the ops routes reach every +// tenant, so the operator key alone opens them. +func TestNew_NestedOperatorReloadsOneTenant(t *testing.T) { + root := writeNestedSettings(t, map[string]map[string]any{"acme": nil, "broken": invalidQuery}) + cfg := testConfig(t, root) + cfg.Auth.OperatorKey = "unit-test-operator-key" + a := newApp(t, cfg, Options{}) + + // header is one name and value, or none. + do := func(method, target string, header ...string) *httptest.ResponseRecorder { + req := httptest.NewRequestWithContext(t.Context(), method, target, nil) + if len(header) == 2 { + req.Header.Set(header[0], header[1]) + } + rec := httptest.NewRecorder() + a.Handler().ServeHTTP(rec, req) + return rec + } + operator := []string{"X-Operator-Key", cfg.Auth.OperatorKey} + as := func(id string) []string { return []string{tenant.Header, id} } + + require.Equal(t, http.StatusServiceUnavailable, do(http.MethodGet, "/v1/pipes/nope", as("broken")...).Code) + + rewriteSettings(t, filepath.Join(root, "broken"), nil) + rec := do(http.MethodPost, "/v1/ops/settings/reload?tenant=broken", operator...) + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"adopted":true`) + rec = do(http.MethodGet, "/v1/pipes/nope", as("broken")...) + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Contains(t, rec.Body.String(), "pipe not found", "the tenant is served again, with no restart") + + // The admin reads name their tenant the same way; without it they read + // tenant 0, which this directory does not hold. + assert.Equal(t, http.StatusOK, do(http.MethodGet, "/v1/ops/pipes?tenant=acme", operator...).Code) + assert.Equal(t, http.StatusNotFound, do(http.MethodGet, "/v1/ops/pipes", operator...).Code) + assert.Equal(t, http.StatusNotFound, do(http.MethodPost, "/v1/ops/settings/reload?tenant=initech", operator...).Code) + assert.Equal(t, http.StatusForbidden, do(http.MethodPost, "/v1/ops/settings/reload?tenant=acme").Code) +} + +// Over a nested directory the operator key is the only credential the ops +// tree takes, and nothing watches the directory — so booting one without the +// key leaves SIGHUP as the only reload, and boot says so rather than repeat +// the flat directory's recovery advice. +func TestNew_NestedWithoutAnOperatorKeyWarnsTheOpsTreeIsClosed(t *testing.T) { + const warning = "no caller can reach those routes" + boot := func(t *testing.T, settingsDir, operatorKey string) string { + t.Helper() + guardGlobals(t) + logs := logtest.Capture(t, slog.LevelWarn) + cfg := testConfig(t, settingsDir) + cfg.Auth.OperatorKey = operatorKey + a, err := New(t.Context(), Options{Config: cfg}) + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, a.Close(context.Background())) }) + return logs.String() + } + + t.Run("nested, no key", func(t *testing.T) { + assert.Contains(t, boot(t, writeNestedSettings(t, map[string]map[string]any{"acme": nil}), ""), warning) + }) + t.Run("nested, key set", func(t *testing.T) { + assert.NotContains(t, boot(t, writeNestedSettings(t, map[string]map[string]any{"acme": nil}), "unit-test-operator-key"), warning) + }) + t.Run("flat, no key", func(t *testing.T) { + logs := boot(t, writeSettings(t, nil), "") + assert.NotContains(t, logs, warning) + assert.Contains(t, logs, "no auth.operator_key set") + }) +} + +// The process-wide resources follow tenant 0 alone: another tenant's reload +// never moves them, and a rejected 0 folder leaves them as they were rather +// than reconfiguring them from nothing. +func TestReload_NestedHooksFollowTheDefaultTenant(t *testing.T) { + dedupeOn := map[string]any{"enabled": true, "id_field": "event_id", "require_id": false, "tables": map[string]any{}} + grown := map[string]any{"dedupe": dedupeOn, "mq": map[string]any{"max_bytes_gb": 2}} + root := writeNestedSettings(t, map[string]map[string]any{ + "0": {"mq": map[string]any{"max_bytes_gb": 1}}, + "acme": {"mq": map[string]any{"max_bytes_gb": 1}}, + }) + a := newApp(t, testConfig(t, root), Options{}) + require.False(t, a.dedup.Open()) + require.Equal(t, int64(1<<30), a.mq.MaxBytes()) + // The CORS list is read per request rather than reconciled by a hook; it + // is the seed's ["*"] in every folder here. + allowOrigin := func() string { + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/version", nil) + req.Header.Set("Origin", "https://app.example.com") + rec := httptest.NewRecorder() + a.Handler().ServeHTTP(rec, req) + return rec.Header().Get("Access-Control-Allow-Origin") + } + require.Equal(t, "*", allowOrigin()) + + rewriteSettings(t, filepath.Join(root, "acme"), grown) + _, adopted := a.tenants.Reload("test") + require.True(t, adopted) + assert.False(t, a.dedup.Open(), "acme's dedupe switch is not the process's") + assert.Equal(t, int64(1<<30), a.mq.MaxBytes()) + + rewriteSettings(t, filepath.Join(root, "0"), grown) + _, adopted = a.tenants.Reload("test") + require.True(t, adopted) + assert.True(t, a.dedup.Open()) + assert.Equal(t, int64(2<<30), a.mq.MaxBytes()) + + rewriteSettings(t, filepath.Join(root, "0"), invalidQuery) + _, adopted = a.tenants.Reload("test") + require.False(t, adopted) + assert.True(t, a.dedup.Open(), "a rejected 0 folder must not read as dedupe off") + assert.Equal(t, int64(2<<30), a.mq.MaxBytes()) + assert.Equal(t, "*", allowOrigin(), "nor as an empty CORS list: that would cost every tenant its browser clients") + + // A removed 0 folder is the same: the registry forgets the tenant, the + // process keeps the wiring it last adopted. + require.NoError(t, os.RemoveAll(filepath.Join(root, "0"))) + _, adopted = a.tenants.Reload("test") + require.True(t, adopted) + _, known := a.tenants.Resolve(tenant.Default) + require.False(t, known) + assert.True(t, a.dedup.Open()) + assert.Equal(t, "*", allowOrigin()) +} + +// keepalive is a config.json patch setting the stream block's keepalive pair. +func keepalive(interval, buckets int) map[string]any { + return map[string]any{"stream": map[string]any{"keepalive_interval": interval, "keepalive_buckets": buckets, "gap_window_minutes": 15}} +} + +// One wheel keeps every tenant's streams alive, so it runs at the shortest +// keepalive_interval among the tenants being served — an upper bound the +// longer ones are inside of (#597 tracks honoring each tenant's own). A flat +// directory's single tenant gets exactly its own pair. +func TestShortestKeepalive(t *testing.T) { + open := func(t *testing.T, dir string) *settings.Registry { + t.Helper() + guardGlobals(t) + tenants, findings := settings.Open(dir) + require.NotNil(t, tenants, "findings: %v", findings) + return tenants + } + + t.Run("flat directory", func(t *testing.T) { + period, buckets := shortestKeepalive(open(t, writeSettings(t, keepalive(45, 5)))) + assert.Equal(t, 45*time.Second, period) + assert.Equal(t, 5, buckets) + }) + + t.Run("nested directory", func(t *testing.T) { + root := writeNestedSettings(t, map[string]map[string]any{"acme": keepalive(30, 3), "globex": keepalive(10, 2), "initech": keepalive(10, 7)}) + tenants := open(t, root) + period, buckets := shortestKeepalive(tenants) + assert.Equal(t, 10*time.Second, period) + assert.Equal(t, 2, buckets, "tenants tied on the interval resolve to the first in id order") + + // A rejected tenant is not being served, so its setting is not weighed. + rewriteSettings(t, filepath.Join(root, "globex"), invalidQuery) + rewriteSettings(t, filepath.Join(root, "initech"), invalidQuery) + tenants.Reload("test") + period, buckets = shortestKeepalive(tenants) + assert.Equal(t, 30*time.Second, period) + assert.Equal(t, 3, buckets) + }) + + t.Run("no tenant served falls back to the wheel's defaults", func(t *testing.T) { + period, buckets := shortestKeepalive(open(t, writeNestedSettings(t, map[string]map[string]any{"acme": invalidQuery}))) + assert.Zero(t, period) + assert.Zero(t, buckets) + }) +} + +// A finding about a nested directory itself — a loose file beside the tenant +// folders — refuses boot, like an invalid flat directory. +func TestNew_NestedLooseFileRefusesBoot(t *testing.T) { + guardGlobals(t) + root := writeNestedSettings(t, map[string]map[string]any{"acme": nil}) + require.NoError(t, os.WriteFile(filepath.Join(root, "notes.txt"), []byte("scratch"), 0o600)) + a, err := New(t.Context(), Options{Config: testConfig(t, root)}) + require.Error(t, err) + assert.Nil(t, a) + assert.Contains(t, err.Error(), "settings directory") +} + func TestNew_RefusesInvalidSettingsDirectory(t *testing.T) { guardGlobals(t) cfg := testConfig(t, t.TempDir()) // empty: every required file is missing diff --git a/internal/app/wire.go b/internal/app/wire.go index 0b8ee208..260c6092 100644 --- a/internal/app/wire.go +++ b/internal/app/wire.go @@ -10,6 +10,7 @@ import ( "os" "os/signal" "path/filepath" + "slices" "strings" "sync" "syscall" @@ -48,30 +49,99 @@ func withoutContext(release func() error) func(context.Context) error { // settings.TenantConfig). Required: config.Validate already rejected an // empty settings.dir, and an invalid directory refuses boot. The binary // carries no compiled defaults; `wavehouse bootstrap` writes the seed. A -// *reload* of an invalid directory merely keeps the previous snapshot. +// *reload* of an invalid directory merely keeps the previous snapshot. A +// nested directory (one folder per tenant, #583) fails closed per tenant +// instead, at boot and on reload alike: see settings.Registry. // // The access-control policy and the named pipes (policies.json / pipes.json) // are read per request off the adopted snapshot, so a reload applies to the // next request with no hook. func (a *App) wireSettings() error { - store, _ := settings.Open(a.cfg.Settings.Dir) - if store == nil { + tenants, _ := settings.Open(a.cfg.Settings.Dir) + if tenants == nil { return fmt.Errorf("settings directory %s invalid, refusing to start — findings above; `wavehouse validate` reproduces them, `wavehouse bootstrap` writes a starter directory", a.cfg.Settings.Dir) } - a.store = store - a.tenants = settings.NewRegistry(store) - a.policies = policy.Source(store.Policy) - if store.Policy() == nil { - slog.Warn("no policy adopted — every token-based request is denied until policies.json defines one (fail closed)") + a.tenants = tenants + // Registered first: hooks run in registration order, and every other one + // reads tenant 0 through the store this one tracks. + a.trackDefaultStore() + a.onDefaultAdopt(a.trackDefaultStore) + a.policies = func() *policy.Policy { return defaultSetting(a, (*settings.Store).Policy) } + switch _, served := tenants.For(tenant.Default); { + case !tenants.Nested(): + if a.policies() == nil { + slog.Warn("no policy adopted — every token-based request is denied until policies.json defines one (fail closed)") + } + case !served: + slog.Warn("nested settings directory with no tenant 0 being served: the ClickHouse connection, the MQ byte budget, the dedupe store, the JWT verifier, CORS, and the async paths (ingest worker, sweeper, stream hub, schema refresh) are still configured from tenant 0's config.json, so they run unconfigured — no ClickHouse address, /livez degraded — until a 0 folder is adopted") } return nil } +// trackDefaultStore remembers tenant 0's store as of its last adoption. The +// registry stops handing out a rejected tenant's store and forgets a removed +// one, but the store keeps its last adopted document either way — and that is +// what the process-wide resources go on following (defaultSetting). +func (a *App) trackDefaultStore() { + if store, ok := a.tenants.For(tenant.Default); ok { + a.defaultStore.Store(store) + } +} + +// defaultSetting reads one setting of the default tenant, which the +// process-wide resources (ClickHouse, dedupe, MQ, auth, CORS) follow until +// #583 gives each tenant its own. It reads tenant 0's last adopted document, +// so a 0 folder a reload rejected or removed leaves every one of them as it +// was — the ones a hook reconciles and the ones read per request (the CORS +// list, the operator key's admin role) alike; one tenant's bad folder must +// not cost every tenant its browser clients. A nested directory that has +// never served a tenant 0 reads T's zero value, which wireSettings warned +// about at boot. +func defaultSetting[T any](a *App, get func(*settings.Store) T) T { + store := a.defaultStore.Load() + if store == nil { + var zero T + return zero + } + return get(store) +} + +// onDefaultAdopt registers fn to run after each reload that adopts the +// default tenant, so a nested directory's other tenants never move the +// process-wide resources, and a rejected 0 folder leaves them as they were. +func (a *App) onDefaultAdopt(fn func()) { + a.tenants.AfterAdopt(func(adopted []tenant.ID) { + if slices.Contains(adopted, tenant.Default) { + fn() + } + }) +} + +// shortestKeepalive is the shape of the one keepalive wheel every tenant's +// streams share: the stream.keepalive_* pair of the tenant with the shortest +// keepalive_interval among those being served. The interval is an upper bound +// on how long a quiet stream goes unwritten, so the shortest one keeps every +// tenant's — at the cost of one tenant setting the cadence for all, which is +// why honoring each tenant's own is tracked in #597. A flat directory's one +// tenant gets exactly its own pair; with no tenant served the zeros fall back +// to the wheel's defaults. +func shortestKeepalive(tenants *settings.Registry) (period time.Duration, buckets int) { + for _, store := range tenants.All() { + // Strictly shorter, so tenants tied on the interval resolve to the + // first in id order rather than to map order. + if p, b := store.Keepalive(); period == 0 || p < period { + period, buckets = p, b + } + } + return period, buckets +} + // perTenant adapts a store accessor to the tenant-keyed getter the async // paths take: they hold a tenant id (tenant.Default today, the MQ subject's -// from #583 story 5), not a request's resolved store. Only tenant.Default -// exists, so a miss is a wiring bug: logged, and read as T's zero value — -// what a removed tenant means to each async path is story 3's to decide. +// from #583 story 5), not a request's resolved store. A miss — a nested +// directory with no 0 folder, or with a rejected one — is logged and read as +// T's zero value; what a removed tenant means to each async path is story +// 3's to decide. func perTenant[T any](tenants *settings.Registry, get func(*settings.Store) T) func(tenant.ID) T { return func(id tenant.ID) T { store, ok := tenants.For(id) @@ -163,7 +233,7 @@ func (a *App) wireObservability(ctx context.Context) { // per request. func (a *App) wireClickHouse() error { params := func() chconn.Params { - c := a.store.ClickHouse() + c := defaultSetting(a, (*settings.Store).ClickHouse) return chconn.Params{ Addr: c.Addr, HTTPPort: c.HTTPPort, HTTPScheme: c.HTTPScheme, Database: c.Database, Username: c.Username, Password: a.cfg.ClickHouse.Password, @@ -176,7 +246,7 @@ func (a *App) wireClickHouse() error { } a.ch = ch a.add(component{name: "clickhouse", close: withoutContext(ch.Close)}) - a.store.AfterAdopt(func() { + a.onDefaultAdopt(func() { if err := ch.Reconfigure(params()); err != nil { slog.Error("clickhouse reconfigure", "error", err) } @@ -238,7 +308,7 @@ func (a *App) wireDedupe() error { a.dedup = dedup a.add(component{name: "dedupe", close: withoutContext(dedup.Close)}) reconcile := func() (bool, error) { - enabled := a.store.DedupeEnabled() + enabled := defaultSetting(a, (*settings.Store).DedupeEnabled) if enabled && !dedup.Open() { config.WarnIfFreshDataDir("pebble", dir) } @@ -248,7 +318,7 @@ func (a *App) wireDedupe() error { } return enabled, nil } - a.store.AfterAdopt(func() { + a.onDefaultAdopt(func() { if enabled, err := reconcile(); err == nil { slog.Info("dedupe store reconciled with settings", "enabled", enabled) } @@ -268,7 +338,7 @@ func (a *App) wireMQ() error { dir := filepath.Join(a.cfg.DataDir, "nats") config.WarnIfFreshDataDir("nats", dir) var broker mq.Broker - broker, err := mq.NewEmbedded(dir, a.store.MQMaxBytes()) + broker, err := mq.NewEmbedded(dir, defaultSetting(a, (*settings.Store).MQMaxBytes)) if err != nil { config.LogStorageInitError("mq", dir, err) return fmt.Errorf("mq open: %w", err) @@ -289,8 +359,8 @@ func (a *App) wireMQ() error { // Rooted in the App's stop context, so a reload caught mid-hook by // SIGTERM gives up rather than holding the drain past // server.shutdown_timeout. - a.store.AfterAdopt(func() { - mb := a.store.MQMaxBytes() + a.onDefaultAdopt(func() { + mb := defaultSetting(a, (*settings.Store).MQMaxBytes) if mb == broker.MaxBytes() { return } @@ -355,9 +425,10 @@ func (a *App) wireStreaming() { // Shared keepalive wheel: one goroutine nudges idle streams so proxies // don't idle-close them. Runs for the process lifetime; a reload that // changes stream.keepalive_* rebuilds the ring in place under the live - // connections. - heartbeater := stream.NewHeartbeater(a.store.Keepalive()) - a.store.AfterAdopt(func() { heartbeater.Reconfigure(a.store.Keepalive()) }) + // connections — any tenant's reload, since every tenant's streams ride + // the one wheel (shortestKeepalive). + heartbeater := stream.NewHeartbeater(shortestKeepalive(a.tenants)) + a.tenants.AfterAdopt(func([]tenant.ID) { heartbeater.Reconfigure(shortestKeepalive(a.tenants)) }) a.heartbeater = heartbeater a.add(component{name: "keepalive", run: func(ctx context.Context) error { heartbeater.Run(ctx) @@ -408,21 +479,26 @@ func (a *App) wireIngestWorker() { func (a *App) wireAuth() (func(http.Handler) http.Handler, error) { cfg := a.cfg switch { - case cfg.Auth.JWTSecret == "" && a.store.Auth().JWKSURL == "": + case cfg.Auth.JWTSecret == "" && defaultSetting(a, (*settings.Store).Auth).JWKSURL == "": slog.Warn("no auth.jwt_secret (boot config) or auth.jwks_url (settings) set: no token can be validated, so every request resolves to the policy default_role (public access)") case cfg.Auth.JWTSecret == "change-me-in-production": slog.Warn("WH_AUTH_JWT_SECRET is using the default insecure value") } operatorKey := strings.TrimSpace(cfg.Auth.OperatorKey) - if operatorKey == "" { + switch { + case operatorKey == "" && a.tenants.Nested(): + // Not the recovery concern below: over a nested directory the key is + // the ops tree's only credential, and there is no watcher either. + slog.Warn("nested settings directory and no auth.operator_key set: the operator key is the only credential /v1/ops/* takes over a nested directory, so no caller can reach those routes — settings can only be reloaded by SIGHUP, which reloads every tenant") + case operatorKey == "": slog.Warn("no auth.operator_key set: if you lose the JWT secret, lose control of the JWKS endpoint, or lose your HMAC secret — or policies.json is emptied — every token-based request is denied and the only recovery is editing the settings directory on the host") - } else { + default: slog.Info("operator key is set: requests presenting it via 'Authorization: Operator ' (or the X-Operator-Key alias) are authorized as a full-access platform operator, and can trigger a settings reload over HTTP while the server is locked out") } authConfig := func() auth.Config { - s := a.store.Auth() + s := defaultSetting(a, (*settings.Store).Auth) return auth.Config{ JWTSecret: cfg.Auth.JWTSecret, JWKSURL: s.JWKSURL, @@ -434,18 +510,23 @@ func (a *App) wireAuth() (func(http.Handler) http.Handler, error) { if err != nil { return nil, fmt.Errorf("auth middleware init: %w", err) } - a.store.AfterAdopt(func() { authn.Reconfigure(authConfig()) }) + a.onDefaultAdopt(func() { authn.Reconfigure(authConfig()) }) return authn.Middleware(), nil } // wireReloadTriggers adds SIGHUP and the directory watcher. All three // triggers (these two and POST /v1/ops/settings/reload) funnel into the same -// serialized Store.Reload, and a rejected reload keeps the previous good +// serialized Registry.Reload, and a rejected reload keeps the previous good // snapshot. They only start in Run, after New has registered every // AfterAdopt hook (ClickHouse reconnect, dedupe store, keepalive wheel, auth // verifier): the watcher reloads once as soon as its watch exists, and that // reload must already drive every hook — a hook registered after the first // reload could miss it. +// +// A nested directory gets no watcher (#583): whoever writes a tenant's +// folder calls the reload route once the folder is complete, where a watcher +// would validate it half-written and, with no previous snapshot to fall back +// on, drop the tenant. SIGHUP reloads the whole tree in both shapes. func (a *App) wireReloadTriggers() { // Registered here and released only at the end of Close, deliberately: // Notify takes SIGHUP off its default disposition (terminate), and a @@ -471,14 +552,18 @@ func (a *App) wireReloadTriggers() { if ctx.Err() != nil { return nil } - a.store.Reload("sighup") + a.tenants.Reload("sighup") } } }}) + if a.tenants.Nested() { + slog.Info("nested settings directory: no directory watcher — reload via POST /v1/ops/settings/reload or SIGHUP") + return + } a.add(component{name: "settings watcher", run: func(ctx context.Context) error { // Watcher setup failure degrades, not fatal: SIGHUP and the ops // endpoint still reload. - if err := a.store.Watch(ctx); err != nil { + if err := a.tenants.Watch(ctx); err != nil { slog.Error("settings directory watcher failed; reload via SIGHUP or POST /v1/ops/settings/reload", "error", err) } return nil @@ -506,7 +591,7 @@ func (a *App) wireHTTP(authMW func(http.Handler) http.Handler) { streamHandler.Closing = closing pipesHandler := api.NewPipesHandler(func(s *settings.Store) pipes.Source { return s }, (*settings.Store).Policy, a.ch, a.cache, a.ch.QueryTimeout) - pipesHandler.OpsStore = a.store + pipesHandler.Tenants = a.tenants deps := api.Dependencies{ Ingest: ingestHandler, @@ -525,8 +610,8 @@ func (a *App) wireHTTP(authMW func(http.Handler) http.Handler) { AuthMW: authMW, Tenants: a.tenants, PolicySource: a.policies, - CORSOrigins: a.store.CORSOrigins, - Settings: api.NewSettingsHandler(a.store), + CORSOrigins: func() []string { return defaultSetting(a, (*settings.Store).CORSOrigins) }, + Settings: api.NewSettingsHandler(a.tenants), } prom := a.cfg.Prometheus diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 2a275bf5..6fafca71 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -7,6 +7,7 @@ import ( "fmt" "log/slog" "net/http" + "net/url" "strings" "sync" "sync/atomic" @@ -362,14 +363,28 @@ func operatorKey(r *http.Request) string { // line. That only protects *our own* logs — it has already crossed every // intermediary in the request URI, so proxies must redact query strings // themselves. +// +// The strip must not repair the query on its way through. ParseQuery skips a +// pair it cannot read and keeps going, so re-encoding what it did read erases +// that pair — and a handler that parses the query strictly in order to refuse +// a malformed one (the ops ?tenant=) would then see a clean query and serve +// the default tenant. A query that does not parse therefore loses its token +// pairs and nothing else. func bearerToken(r *http.Request) string { // Strip before either return: a header-authenticated request carrying a // stray ?token would otherwise keep the unused JWT in r.URL. var queryToken string - if params := r.URL.Query(); params.Get("token") != "" { - queryToken = params.Get("token") - params.Del("token") - r.URL.RawQuery = params.Encode() + // params holds every pair that did parse, error or not — what r.URL.Query + // returns, so which token is read does not depend on the rest of the query. + params, err := url.ParseQuery(r.URL.RawQuery) + if tok := params.Get("token"); tok != "" { + queryToken = tok + if err == nil { + params.Del("token") + r.URL.RawQuery = params.Encode() + } else { + r.URL.RawQuery = withoutTokenPairs(r.URL.RawQuery) + } } if tok, ok := authScheme(r, "Bearer"); ok { @@ -378,6 +393,25 @@ func bearerToken(r *http.Request) string { return queryToken } +// withoutTokenPairs cuts the token pairs out of a raw query and leaves every +// other byte as it was sent. A pair counts only if ParseQuery would have read +// it as one: a malformed pair that merely looks like a token stays, for the +// same strict parse to refuse. +func withoutTokenPairs(raw string) string { + pairs := strings.Split(raw, "&") + kept := pairs[:0] + for _, pair := range pairs { + key, value, _ := strings.Cut(pair, "=") + k, kerr := url.QueryUnescape(key) + _, verr := url.QueryUnescape(value) + if k == "token" && kerr == nil && verr == nil && !strings.Contains(pair, ";") { + continue + } + kept = append(kept, pair) + } + return strings.Join(kept, "&") +} + // tokenError maps a jwt parse failure to a stable, caller-safe error for the // fail-loud message: expired tokens are distinguished, everything else (bad // signature, malformed, nil) collapses to a generic invalid-token error so no diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 3197754b..bcef233b 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -33,6 +33,8 @@ type captured struct { tokenInURL string // a non-token query param, to prove the strip is surgical otherQueryParam string + // the query string the handler was left with, byte for byte + rawQuery string } // run drives cfg's middleware over a request decorated by setup, returning what @@ -60,6 +62,7 @@ func runOp(t *testing.T, cfg Config, store policy.Source, setup func(*http.Reque c.isOperator = IsOperator(r.Context()) c.tokenInURL = r.URL.Query().Get("token") c.otherQueryParam = r.URL.Query().Get("table") + c.rawQuery = r.URL.RawQuery w.WriteHeader(http.StatusOK) })) req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil) @@ -288,6 +291,37 @@ func TestMiddleware_QueryTokenStrippedWithUnusableHeader(t *testing.T) { assert.Equal(t, "clicks", c.otherQueryParam, "unrelated query params must survive the strip") } +// The strip must not repair a query that does not parse. Re-encoding what +// ParseQuery could read erases the pairs it could not, and a handler that +// parses strictly in order to refuse them (the ops ?tenant=) would then see a +// clean query: `?tenant=acme;x=1&token=…` read the default tenant. The token +// is still read and still removed; every other byte reaches the handler as +// it was sent. +func TestMiddleware_QueryToken_MalformedQuerySurvivesTheStrip(t *testing.T) { + t.Parallel() + tok := testutil.MakeJWT(t, map[string]any{"role": "viewer"}) + tests := []struct { + name, query, want string + }{ + {name: "semicolon pair before the token", query: "tenant=acme;x=1&token=" + tok, want: "tenant=acme;x=1"}, + {name: "semicolon pair after the token", query: "token=" + tok + "&tenant=acme;x=1", want: "tenant=acme;x=1"}, + {name: "bad escape", query: "tenant=%zz&token=" + tok, want: "tenant=%zz"}, + {name: "the rest is left in the order and spelling it was sent", query: "b=2&token=" + tok + "&a=%41&x=%zz", want: "b=2&a=%41&x=%zz"}, + {name: "an escaped key is the token too", query: "%74oken=" + tok + "&x=%zz", want: "x=%zz"}, + {name: "every token pair goes", query: "token=" + tok + "&x=%zz&token=second", want: "x=%zz"}, + {name: "a pair the parser could not read as a token stays", query: "token=" + tok + "&token=%zz", want: "token=%zz"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + c := run(t, cfg(), func(r *http.Request) { r.URL.RawQuery = tt.query }) + assert.Equal(t, "viewer", c.role, "a malformed pair elsewhere does not cost the request its token") + assert.Equal(t, tt.want, c.rawQuery) + assert.Empty(t, c.tokenInURL) + }) + } +} + func TestMiddleware_InvalidQueryParamToken_FallsBackWithError(t *testing.T) { t.Parallel() c := run(t, cfg(), func(r *http.Request) { r.URL.RawQuery = "token=not.a.jwt" }) diff --git a/internal/settings/finding.go b/internal/settings/finding.go index 1e5ea548..10c84534 100644 --- a/internal/settings/finding.go +++ b/internal/settings/finding.go @@ -13,8 +13,10 @@ const ( ) // Finding is one validation result, located as precisely as the failure -// allows: File is empty for directory-level findings, Path is a dotted JSON -// path ("tables.clicks.analyst") and empty for whole-file findings. +// allows: File is empty for directory-level findings and, in a nested root, +// leads with the tenant folder ("acme/policies.json", or "acme" alone for a +// finding about the folder); Path is a dotted JSON path +// ("tables.clicks.analyst") and empty for whole-file findings. // The JSON shape is part of the ops API: POST /v1/ops/settings/reload returns // findings verbatim. type Finding struct { diff --git a/internal/settings/registry.go b/internal/settings/registry.go index 007c592b..f8d22b36 100644 --- a/internal/settings/registry.go +++ b/internal/settings/registry.go @@ -1,22 +1,300 @@ package settings -import "github.com/Wave-RF/WaveHouse/internal/tenant" +import ( + "iter" + "log/slog" + "maps" + "slices" + "sync" + "sync/atomic" + + "github.com/Wave-RF/WaveHouse/internal/tenant" +) // Registry maps a tenant id to the Store holding that tenant's adopted -// settings. It holds exactly one store, under tenant.Default: the settings -// directory is one tenant's four files, and Open, Reload, and Watch stay on -// the Store behind it. +// settings, and owns everything that changes one. Open validates the +// directory and adopts it up front. Reload is the single code path every +// later trigger — SIGHUP, the directory watcher, and POST +// /v1/ops/settings/reload — funnels through. The stores are passive: a +// document pointer their getters read. +// +// What a rejected directory costs depends on its shape (#583), which is fixed +// at Open — switching shapes is stop, restructure, start: +// +// - Flat, the four files in the directory itself: tenant.Default alone. Open +// refuses an invalid directory, so a flat *Registry never exists without a +// good document behind it, and a reload swaps the snapshot only when no +// finding is an error, so a bad edit (or a deleted file, or a vanished +// directory) can never evict the last good document. +// - Nested, one folder per tenant: fail closed per tenant, at Open and on +// reload alike. A folder with an error finding stops being served — the +// registry still knows the tenant, so its requests are refused rather +// than unknown — and every other tenant carries on; there is no +// previous-snapshot fallback. A reload mirrors the folders: a new one is +// served and a removed one is forgotten. A finding about the directory +// itself (a loose file, an unreadable directory, a changed shape) rejects +// the reload whole and leaves every tenant as it was; at Open it refuses +// boot. type Registry struct { - stores map[tenant.ID]*Store + dir string + nested bool + + // mu serializes Reload: concurrent triggers queue rather than racing + // validate-then-swap sequences (a stale document must not overwrite a newer one). + mu sync.Mutex + // afterAdopt runs under mu after every reload that adopted a tenant, in + // registration order — for consumers that own a resource whose lifecycle + // follows a setting (the Pebble store behind dedupe.enabled) rather than + // reading the snapshot per call. + afterAdopt []func(adopted []tenant.ID) + + // tenants is replaced whole by a reload, never edited, so a lookup is one + // lock-free load. + tenants atomic.Pointer[map[tenant.ID]entry] +} + +// entry is one tenant the registry knows. +type entry struct { + // store is created when the tenant's folder first validates and never + // replaced, so the handle For returns keeps reading the latest adopted + // document. + store *Store + // rejected marks a nested tenant whose folder failed its last + // validation. The store keeps its document, for the requests already + // admitted under it; the registry just stops handing it out. + rejected bool +} + +// adopt returns e after a validation pass over its folder: holding doc, or +// rejected when the folder yielded none. +func (e entry) adopt(doc *Document) entry { + if doc == nil { + e.rejected = true + return e + } + if e.store == nil { + e.store = &Store{} + } + e.store.adopt(doc) + e.rejected = false + return e +} + +// Open validates dir and returns a Registry serving it. A rejected directory +// — flat and invalid, or of either shape with a finding about the directory +// itself — returns a nil Registry with the findings: the caller (boot) +// refuses to start. A nested directory with a rejected tenant folder still +// opens; that tenant alone is not served. +func Open(dir string) (*Registry, []Finding) { + r := newRegistry(map[tenant.ID]entry{}) + r.dir = dir + r.mu.Lock() + defer r.mu.Unlock() + findings, _, applied := r.reload("boot", true) + if !applied { + return nil, findings + } + return r, findings } -// NewRegistry returns a Registry serving store as tenant.Default. +// NewRegistry returns a flat Registry serving store as tenant.Default, with +// no directory behind it: what a test that fixes its settings holds. func NewRegistry(store *Store) *Registry { - return &Registry{stores: map[tenant.ID]*Store{tenant.Default: store}} + return newRegistry(map[tenant.ID]entry{tenant.Default: {store: store}}) +} + +func newRegistry(tenants map[tenant.ID]entry) *Registry { + r := &Registry{} + r.tenants.Store(&tenants) + return r } -// For returns the store of tenant id, or false when no such tenant exists. +// Dir returns the directory this registry reads. +func (r *Registry) Dir() string { return r.dir } + +// Nested reports the directory's shape: one folder per tenant (true) or the +// four files of tenant.Default (false). +func (r *Registry) Nested() bool { return r.nested } + +// For returns the store of tenant id, or false when the registry is not +// serving that tenant — it has no such tenant, or the tenant's folder was +// rejected. func (r *Registry) For(id tenant.ID) (*Store, bool) { - s, ok := r.stores[id] - return s, ok + store, _ := r.Resolve(id) + return store, store != nil +} + +// Resolve is For for a caller that answers the two misses differently: a nil +// store with known=true is a tenant whose folder was rejected, and with +// known=false a tenant the registry has never heard of. +func (r *Registry) Resolve(id tenant.ID) (store *Store, known bool) { + e, known := (*r.tenants.Load())[id] + if !known || e.rejected { + return nil, known + } + return e.store, true +} + +// All iterates over the tenants being served, in id order — for a consumer +// that owns one resource every tenant shares and has to weigh their settings +// against each other. +func (r *Registry) All() iter.Seq2[tenant.ID, *Store] { + return func(yield func(tenant.ID, *Store) bool) { + tenants := *r.tenants.Load() + for _, id := range slices.Sorted(maps.Keys(tenants)) { + if e := tenants[id]; !e.rejected && !yield(id, e.store) { + return + } + } + } +} + +// Reload re-validates the directory and adopts what it finds (see Registry +// for what a rejection costs in each shape). Warnings don't block adoption, +// matching `wavehouse validate`. The returned bool reports whether everything +// was adopted: no finding is an error. In a nested directory false can +// therefore mean adopted in part — the tenants whose folders validated were +// adopted, warnings and all, and the ones with an error finding were not. +// trigger names the path that fired ("boot", "sighup", "watch", "api") and +// tags every log line so operators can tell them apart. +func (r *Registry) Reload(trigger string) ([]Finding, bool) { + r.mu.Lock() + defer r.mu.Unlock() + findings, adopted, _ := r.reload(trigger, false) + return findings, adopted +} + +// reload is Reload under mu. boot marks Open's first pass, which takes the +// directory's shape rather than holding it to one. applied reports whether +// the registry took the tree at all. +func (r *Registry) reload(trigger string, boot bool) (findings []Finding, adopted, applied bool) { + tree, findings := Validate(r.dir) + switch { + case tree == nil: + case boot: + r.nested = tree.Nested + case tree.Nested != r.nested: + booted := "the four files" + if r.nested { + booted = "one folder per tenant" + } + findings = append(findings, Finding{Severity: SeverityError, Message: "the settings directory no longer has the shape this server booted with (" + booted + ") — switching shapes is stop, restructure, start"}) + tree = nil + } + // A flat directory with an error finding changes nothing, like a finding + // about the directory itself: the previous document stays. + if tree != nil && !tree.Nested && tree.Tenants[tenant.Default].Doc == nil { + tree = nil + } + + var adoptedIDs, rejectedIDs, removedIDs []tenant.ID + if tree != nil { + prev := *r.tenants.Load() + next := make(map[tenant.ID]entry, len(tree.Tenants)) + // Sorted so the hooks and the log name the tenants in one order every run. + for _, id := range slices.Sorted(maps.Keys(tree.Tenants)) { + next[id] = prev[id].adopt(tree.Tenants[id].Doc) + if next[id].rejected { + rejectedIDs = append(rejectedIDs, id) + } else { + adoptedIDs = append(adoptedIDs, id) + } + } + // A tenant whose folder is gone leaves the map with no finding to show + // for it, and every request of its turns into a 404: name it in the log. + for _, id := range slices.Sorted(maps.Keys(prev)) { + if _, kept := next[id]; !kept { + removedIDs = append(removedIDs, id) + } + } + r.tenants.Store(&next) + r.adopted(adoptedIDs) + } + + errs, warns := logFindings(trigger, findings) + switch { + case tree == nil && boot: + slog.Error("settings rejected", "trigger", trigger, "dir", r.dir, "errors", errs, "warnings", warns) + case tree == nil: + slog.Error("settings rejected — keeping previous settings", "trigger", trigger, "dir", r.dir, "errors", errs, "warnings", warns) + case errs > 0: + slog.Error("settings adopted in part — a tenant whose folder was rejected answers 503 until a reload adopts it", "trigger", trigger, "dir", r.dir, "adopted", len(adoptedIDs), "rejected", rejectedIDs, "removed", removedIDs, "errors", errs, "warnings", warns) + case len(removedIDs) > 0: + slog.Warn("settings adopted — a tenant whose folder is gone is no longer served", "trigger", trigger, "dir", r.dir, "tenants", len(adoptedIDs), "removed", removedIDs, "warnings", warns) + case r.nested: + slog.Info("settings adopted", "trigger", trigger, "dir", r.dir, "tenants", len(adoptedIDs), "warnings", warns) + default: + slog.Info("settings adopted", "trigger", trigger, "dir", r.dir, "warnings", warns) + } + return findings, errs == 0, tree != nil +} + +// ReloadTenant is Reload for one tenant's folder of a nested directory: the +// rest of the directory is not read, so it can neither adopt nor drop another +// tenant — what the writer of one folder calls when that folder is complete. +// The folder is adopted, or the tenant stops being served. known is false for +// a tenant the registry does not hold, and nothing is read: a whole-tree +// Reload is what picks up a new folder. A flat directory is its default +// tenant's folder, so there this is Reload. +func (r *Registry) ReloadTenant(id tenant.ID, trigger string) (findings []Finding, adopted, known bool) { + r.mu.Lock() + defer r.mu.Unlock() + prev := *r.tenants.Load() + e, known := prev[id] + if !known { + return nil, false, false + } + if !r.nested { + findings, adopted, _ = r.reload(trigger, false) + return findings, adopted, true + } + + doc, findings := validateFolder(r.dir, id.String()) + next := maps.Clone(prev) + next[id] = e.adopt(doc) + r.tenants.Store(&next) + if doc != nil { + r.adopted([]tenant.ID{id}) + } + errs, warns := logFindings(trigger, findings) + if doc == nil { + slog.Error("settings rejected — the tenant answers 503 until a reload adopts its folder", "trigger", trigger, "dir", r.dir, "tenant", id, "errors", errs, "warnings", warns) + } else { + slog.Info("settings adopted", "trigger", trigger, "dir", r.dir, "tenant", id, "warnings", warns) + } + return findings, doc != nil, true +} + +// adopted runs the AfterAdopt hooks for a reload that adopted ids. Under mu. +func (r *Registry) adopted(ids []tenant.ID) { + if len(ids) == 0 { + return + } + for _, fn := range r.afterAdopt { + fn(ids) + } +} + +// logFindings logs each finding under its trigger and counts them by severity. +func logFindings(trigger string, findings []Finding) (errs, warns int) { + for _, f := range findings { + if f.Severity == SeverityError { + errs++ + slog.Error("settings finding", "trigger", trigger, "finding", f.String()) + } else { + warns++ + slog.Warn("settings finding", "trigger", trigger, "finding", f.String()) + } + } + return errs, warns +} + +// AfterAdopt registers fn to run after each subsequent reload that adopted a +// tenant, serialized with the reload itself, with the tenants it adopted. +// Open's boot adoption has already happened by the time a caller can +// register, so the caller applies the boot state itself. +func (r *Registry) AfterAdopt(fn func(adopted []tenant.ID)) { + r.mu.Lock() + defer r.mu.Unlock() + r.afterAdopt = append(r.afterAdopt, fn) } diff --git a/internal/settings/registry_test.go b/internal/settings/registry_test.go index 733cf65b..f8b9ee8d 100644 --- a/internal/settings/registry_test.go +++ b/internal/settings/registry_test.go @@ -1,14 +1,32 @@ package settings import ( + "fmt" + "log/slog" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/Wave-RF/WaveHouse/internal/tenant" + "github.com/Wave-RF/WaveHouse/internal/testutil/logtest" ) +// newLoadedRegistry materializes a valid directory (with overrides applied) +// and returns a registry that has adopted it. +func newLoadedRegistry(t *testing.T, overrides map[string]string) *Registry { + t.Helper() + files := validFiles() + for name, content := range overrides { + files[name] = content + } + reg, findings := Open(writeDir(t, files)) + require.NotNil(t, reg, "findings: %s", findingStrings(findings)) + return reg +} + func TestRegistry_For(t *testing.T) { t.Parallel() store := newLoadedStore(t, nil) @@ -22,3 +40,483 @@ func TestRegistry_For(t *testing.T) { assert.False(t, ok, "only the default tenant exists") assert.Nil(t, got) } + +func TestRegistry_ReloadAdoptsAndRejects(t *testing.T) { + t.Parallel() + reg := newLoadedRegistry(t, map[string]string{ + FileConfig: configJSON(`{"query": {"default_max_rows": 500}}`), + }) + s, _ := reg.For(tenant.Default) + assert.Equal(t, 500, s.DefaultMaxRows()) + + // Break the directory: the reload must report the error and keep the + // previous snapshot — a bad edit can never evict the last good document. + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": -1}}`)), 0o600)) + findings, adopted := reg.Reload("test") + assert.False(t, adopted) + assert.True(t, HasErrors(findings)) + assert.Equal(t, 500, s.DefaultMaxRows(), "rejected reload must keep the previous snapshot") + + // Fix it: the next reload adopts again, into the same store — the handle a + // long-lived consumer holds keeps reading the latest document. + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 700}}`)), 0o600)) + findings, adopted = reg.Reload("test") + require.True(t, adopted, "findings: %s", findingStrings(findings)) + assert.Equal(t, 700, s.DefaultMaxRows()) +} + +func TestRegistry_ReloadWithWarningsAdopts(t *testing.T) { + t.Parallel() + reg := newLoadedRegistry(t, map[string]string{ + FilePolicies: `{}`, // empty policy: legal, warned (total lockout) + FilePipes: `{}`, // drop the pipe so its analyst role reference doesn't dangle + }) + findings, adopted := reg.Reload("test") + assert.True(t, adopted, "warnings alone must not block adoption") + assert.NotEmpty(t, findings) + assert.False(t, HasErrors(findings)) +} + +// TestOpen_RejectsInvalid pins the boot contract: an invalid directory yields +// no Registry at all — there is no "store without a document" state and no +// compiled defaults to fall back on. +func TestOpen_RejectsInvalid(t *testing.T) { + t.Parallel() + files := validFiles() + files[FileConfig] = `{}` // every key missing + reg, findings := Open(writeDir(t, files)) + assert.Nil(t, reg) + assert.True(t, HasErrors(findings)) + + reg, findings = Open(filepath.Join(t.TempDir(), "nope")) + assert.Nil(t, reg) + assert.True(t, HasErrors(findings)) +} + +// TestRegistry_SurvivesVanishedDirectory pins the runtime half of the same +// contract: once adopted, the snapshot outlives its files — deleting the +// directory is just a rejected reload. +func TestRegistry_SurvivesVanishedDirectory(t *testing.T) { + t.Parallel() + reg := newLoadedRegistry(t, map[string]string{ + FileConfig: configJSON(`{"query": {"default_max_rows": 42}}`), + }) + s, _ := reg.For(tenant.Default) + require.NoError(t, os.RemoveAll(reg.Dir())) + findings, adopted := reg.Reload("test") + assert.False(t, adopted) + assert.True(t, HasErrors(findings)) + assert.Equal(t, 42, s.DefaultMaxRows()) + _, id, req := s.DedupeFor("clicks") + assert.Equal(t, "event_id", id) + assert.False(t, req) +} + +// TestRegistry_AfterAdoptRunsOnlyOnAdoption pins the lifecycle hook contract: +// it fires after every successful reload (with the new snapshot already +// visible), names the tenant that reload adopted, and never fires on a +// rejected one. +func TestRegistry_AfterAdoptRunsOnlyOnAdoption(t *testing.T) { + t.Parallel() + reg := newLoadedRegistry(t, nil) + s, _ := reg.For(tenant.Default) + var seen []bool + reg.AfterAdopt(func(adopted []tenant.ID) { + assert.Equal(t, []tenant.ID{tenant.Default}, adopted) + seen = append(seen, s.DedupeEnabled()) + }) + + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(configJSON(`{"dedupe": {"enabled": true}}`)), 0o600)) + _, adopted := reg.Reload("test") + require.True(t, adopted) + assert.Equal(t, []bool{true}, seen, "hook sees the newly adopted snapshot") + + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 0}}`)), 0o600)) + _, adopted = reg.Reload("test") + require.False(t, adopted) + assert.Equal(t, []bool{true}, seen, "rejected reload must not fire the hook") +} + +// writeTenant (re)writes one tenant folder of a nested root. +func writeTenant(t *testing.T, root, folder string, files map[string]string) { + t.Helper() + dir := filepath.Join(root, folder) + require.NoError(t, os.MkdirAll(dir, 0o750)) + for name, content := range files { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600)) + } +} + +// maxRowsFiles is a valid tenant folder whose query.default_max_rows tells +// the tenants of a test apart. +func maxRowsFiles(maxRows int) map[string]string { + files := validFiles() + files[FileConfig] = configJSON(fmt.Sprintf(`{"query": {"default_max_rows": %d}}`, maxRows)) + return files +} + +// brokenFiles is a tenant folder Validate rejects. +func brokenFiles() map[string]string { + files := validFiles() + files[FileConfig] = configJSON(`{"query": {"default_max_rows": -1}}`) + return files +} + +func TestOpen_NestedRoot(t *testing.T) { + t.Parallel() + reg, findings := Open(writeTree(t, map[string]map[string]string{"acme": maxRowsFiles(111), "globex": maxRowsFiles(222)})) + require.NotNil(t, reg, "findings: %s", findingStrings(findings)) + assert.Empty(t, findings) + assert.True(t, reg.Nested()) + + acme, ok := reg.For("acme") + require.True(t, ok) + globex, ok := reg.For("globex") + require.True(t, ok) + assert.NotSame(t, acme, globex) + assert.Equal(t, 111, acme.DefaultMaxRows()) + assert.Equal(t, 222, globex.DefaultMaxRows()) + + // A nested root defines the tenants it holds folders for and no other: + // the default tenant exists only as a 0 folder. + _, ok = reg.For(tenant.Default) + assert.False(t, ok) + store, known := reg.Resolve(tenant.Default) + assert.Nil(t, store) + assert.False(t, known) + + assert.False(t, newLoadedRegistry(t, nil).Nested()) +} + +// Fail closed per tenant, at boot: the registry opens, the rejected tenant is +// known but not served, and the tenant beside it is. +func TestOpen_NestedRejectedFolder(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": maxRowsFiles(111), "globex": brokenFiles()}) + reg, findings := Open(root) + require.NotNil(t, reg, "one bad folder must not cost the pod its other tenants") + assert.Contains(t, findingStrings(findings), "error: globex/config.json: query.default_max_rows") + + _, ok := reg.For("acme") + assert.True(t, ok) + _, ok = reg.For("globex") + assert.False(t, ok, "a rejected tenant is not served") + store, known := reg.Resolve("globex") + assert.Nil(t, store) + assert.True(t, known, "rejected is not unknown: its requests are refused, not 404ed") + + // Fixing the folder and reloading is the whole recovery. + writeTenant(t, root, "globex", maxRowsFiles(222)) + findings, adopted := reg.Reload("test") + require.True(t, adopted, "findings: %s", findingStrings(findings)) + globex, ok := reg.For("globex") + require.True(t, ok) + assert.Equal(t, 222, globex.DefaultMaxRows()) +} + +// A nested root whose every folder is rejected still opens: nothing is +// served, and a reload of the fixed folders brings the tenants up. +func TestOpen_NestedEveryFolderRejected(t *testing.T) { + t.Parallel() + reg, findings := Open(writeTree(t, map[string]map[string]string{"acme": brokenFiles()})) + require.NotNil(t, reg) + assert.True(t, HasErrors(findings)) + _, ok := reg.For("acme") + assert.False(t, ok) +} + +// A finding about the root itself refuses boot in either shape. +func TestOpen_NestedLooseFileRefusesBoot(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": validFiles()}) + require.NoError(t, os.WriteFile(filepath.Join(root, "notes.txt"), []byte("scratch"), 0o600)) + reg, findings := Open(root) + assert.Nil(t, reg) + assert.Contains(t, findingStrings(findings), "notes.txt: unexpected file") +} + +// Fail closed per tenant, on reload: no previous-snapshot fallback. The +// rejected tenant stops being served while the reload still adopts the tenant +// beside it; a request already admitted keeps reading the document it was +// admitted under; and the recovery lands in the same store. +func TestRegistry_NestedReloadRejectsOneTenant(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": maxRowsFiles(111), "globex": maxRowsFiles(222)}) + reg, _ := Open(root) + require.NotNil(t, reg) + var hooks [][]tenant.ID + reg.AfterAdopt(func(adopted []tenant.ID) { hooks = append(hooks, adopted) }) + admitted, _ := reg.For("globex") + + writeTenant(t, root, "acme", maxRowsFiles(333)) + writeTenant(t, root, "globex", brokenFiles()) + findings, adopted := reg.Reload("test") + assert.False(t, adopted, "not everything was adopted") + assert.Contains(t, findingStrings(findings), "error: globex/config.json") + assert.Equal(t, [][]tenant.ID{{"acme"}}, hooks, "the hooks hear of the tenants that were adopted, and only those") + + acme, _ := reg.For("acme") + assert.Equal(t, 333, acme.DefaultMaxRows(), "the tenant beside the rejected one is adopted") + _, ok := reg.For("globex") + assert.False(t, ok) + _, known := reg.Resolve("globex") + assert.True(t, known) + assert.Equal(t, 222, admitted.DefaultMaxRows(), "a request already holding the store finishes on the document it started with") + + writeTenant(t, root, "globex", maxRowsFiles(444)) + _, adopted = reg.Reload("test") + require.True(t, adopted) + recovered, ok := reg.For("globex") + require.True(t, ok) + assert.Same(t, admitted, recovered, "a tenant keeps one store for life") + assert.Equal(t, 444, recovered.DefaultMaxRows()) + assert.Equal(t, [][]tenant.ID{{"acme"}, {"acme", "globex"}}, hooks) +} + +// All is the served tenants in id order: a rejected tenant is left out, like +// everywhere else, and comes back with its folder. +func TestRegistry_All(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"globex": maxRowsFiles(222), "acme": maxRowsFiles(111), "broken": brokenFiles()}) + reg, _ := Open(root) + require.NotNil(t, reg) + + served := func() (ids []tenant.ID, maxRows []int) { + for id, store := range reg.All() { + ids = append(ids, id) + maxRows = append(maxRows, store.DefaultMaxRows()) + } + return ids, maxRows + } + ids, maxRows := served() + assert.Equal(t, []tenant.ID{"acme", "globex"}, ids) + assert.Equal(t, []int{111, 222}, maxRows) + + writeTenant(t, root, "broken", maxRowsFiles(333)) + _, adopted := reg.Reload("test") + require.True(t, adopted) + ids, _ = served() + assert.Equal(t, []tenant.ID{"acme", "broken", "globex"}, ids) + + // Stopping early is the iterator's contract, not the caller's problem. + for id := range reg.All() { + assert.Equal(t, tenant.ID("acme"), id) + break + } +} + +// A reload mirrors the folders: a new one is served, a removed one is +// forgotten. A folder whose name is not a tenant id is reported and skipped. +func TestRegistry_NestedReloadMirrorsTheFolders(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": maxRowsFiles(111), "globex": maxRowsFiles(222)}) + reg, _ := Open(root) + require.NotNil(t, reg) + + writeTenant(t, root, "initech", maxRowsFiles(333)) + require.NoError(t, os.RemoveAll(filepath.Join(root, "acme"))) + findings, adopted := reg.Reload("test") + require.True(t, adopted, "findings: %s", findingStrings(findings)) + + initech, ok := reg.For("initech") + require.True(t, ok, "a new folder is a new tenant") + assert.Equal(t, 333, initech.DefaultMaxRows()) + store, known := reg.Resolve("acme") + assert.Nil(t, store) + assert.False(t, known, "a removed folder is an unknown tenant, not a rejected one") + + writeTenant(t, root, "acme.bak", validFiles()) + findings, adopted = reg.Reload("test") + assert.False(t, adopted) + assert.Contains(t, findingStrings(findings), "acme.bak: folder name is not a tenant id") + _, ok = reg.For("globex") + assert.True(t, ok, "a badly named folder costs no tenant its settings") +} + +// A tenant whose folder is gone leaves the registry with no finding to show +// for it — every request of its just turns into a 404 — so the reload's log +// line names it. Captures the default logger, so it is not parallel. +func TestRegistry_ReloadNamesARemovedTenantInTheLog(t *testing.T) { + root := writeTree(t, map[string]map[string]string{"acme": validFiles(), "globex": validFiles()}) + reg, _ := Open(root) + require.NotNil(t, reg) + logs := logtest.Capture(t, slog.LevelInfo) + + _, adopted := reg.Reload("test") + require.True(t, adopted) + assert.NotContains(t, logs.String(), "removed", "a reload that drops nobody says nothing of it") + + require.NoError(t, os.RemoveAll(filepath.Join(root, "acme"))) + _, adopted = reg.Reload("test") + require.True(t, adopted, "a removed folder is not a finding") + assert.Contains(t, logs.String(), `"level":"WARN"`) + assert.Contains(t, logs.String(), `"removed":["acme"]`) +} + +// ReloadTenant reads one tenant's folder and nothing else: the tenant beside +// it is neither adopted nor dropped, whatever state its folder is in. +func TestRegistry_ReloadTenant(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": maxRowsFiles(111), "globex": maxRowsFiles(222)}) + reg, _ := Open(root) + require.NotNil(t, reg) + var hooks [][]tenant.ID + reg.AfterAdopt(func(adopted []tenant.ID) { hooks = append(hooks, adopted) }) + acme, _ := reg.For("acme") + globex, _ := reg.For("globex") + + // Both folders change on disk; only the named one is read. + writeTenant(t, root, "acme", maxRowsFiles(333)) + writeTenant(t, root, "globex", brokenFiles()) + findings, adopted, known := reg.ReloadTenant("acme", "test") + require.True(t, known) + require.True(t, adopted, "findings: %s", findingStrings(findings)) + assert.Equal(t, 333, acme.DefaultMaxRows()) + assert.Equal(t, 222, globex.DefaultMaxRows()) + _, ok := reg.For("globex") + assert.True(t, ok, "a broken folder nobody asked to reload costs its tenant nothing") + assert.Equal(t, [][]tenant.ID{{"acme"}}, hooks) + + // Reloading the broken one is what drops it: no previous-snapshot fallback. + findings, adopted, known = reg.ReloadTenant("globex", "test") + require.True(t, known) + assert.False(t, adopted) + assert.Contains(t, findingStrings(findings), "error: globex/config.json: query.default_max_rows") + _, ok = reg.For("globex") + assert.False(t, ok) + _, known = reg.Resolve("globex") + assert.True(t, known) + _, ok = reg.For("acme") + assert.True(t, ok) + assert.Equal(t, [][]tenant.ID{{"acme"}}, hooks, "a rejected folder runs no hook") + + // And reloading the fixed one brings it back, in the store it always had. + writeTenant(t, root, "globex", maxRowsFiles(444)) + _, adopted, _ = reg.ReloadTenant("globex", "test") + require.True(t, adopted) + recovered, ok := reg.For("globex") + require.True(t, ok) + assert.Same(t, globex, recovered) + assert.Equal(t, 444, recovered.DefaultMaxRows()) + + // A tenant the registry does not hold is not looked for on disk: picking + // up a new folder is a whole-tree reload's job. + writeTenant(t, root, "initech", maxRowsFiles(555)) + findings, adopted, known = reg.ReloadTenant("initech", "test") + assert.False(t, known) + assert.False(t, adopted) + assert.Empty(t, findings) + _, ok = reg.For("initech") + assert.False(t, ok) + + // A folder that is gone is a rejected tenant here — the registry still + // holds it — and a forgotten one after a whole-tree reload. + require.NoError(t, os.RemoveAll(filepath.Join(root, "acme"))) + findings, adopted, known = reg.ReloadTenant("acme", "test") + assert.True(t, known) + assert.False(t, adopted) + require.Len(t, findings, 1) + assert.Equal(t, "acme", findings[0].File) + assert.Contains(t, findings[0].Message, "does not exist") + _, known = reg.Resolve("acme") + assert.True(t, known) + reg.Reload("test") + _, known = reg.Resolve("acme") + assert.False(t, known) +} + +// A flat directory is its default tenant's folder, so reloading that tenant +// is Reload — keep-previous on a rejection included — and it has no other. +func TestRegistry_ReloadTenant_FlatDirectory(t *testing.T) { + t.Parallel() + reg := newLoadedRegistry(t, map[string]string{FileConfig: configJSON(`{"query": {"default_max_rows": 500}}`)}) + s, _ := reg.For(tenant.Default) + + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 700}}`)), 0o600)) + _, adopted, known := reg.ReloadTenant(tenant.Default, "test") + assert.True(t, known) + assert.True(t, adopted) + assert.Equal(t, 700, s.DefaultMaxRows()) + + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(`not json`), 0o600)) + findings, adopted, known := reg.ReloadTenant(tenant.Default, "test") + assert.True(t, known) + assert.False(t, adopted) + assert.Contains(t, findingStrings(findings), "error: config.json:", "a flat directory's findings carry no folder") + got, ok := reg.For(tenant.Default) + require.True(t, ok, "a flat directory keeps its previous document") + assert.Equal(t, 700, got.DefaultMaxRows()) + + _, _, known = reg.ReloadTenant("acme", "test") + assert.False(t, known) +} + +// A finding about the root itself rejects the reload whole: nothing on disk +// is adopted, no tenant is dropped, no hook runs. +func TestRegistry_RootLevelFailureChangesNothing(t *testing.T) { + t.Parallel() + tests := []struct { + name string + damage func(t *testing.T, root string) + want string + }{ + {name: "a loose file beside the folders", want: "notes.txt: unexpected file", damage: func(t *testing.T, root string) { + require.NoError(t, os.WriteFile(filepath.Join(root, "notes.txt"), []byte("scratch"), 0o600)) + }}, + {name: "the root is gone", want: "does not exist", damage: func(t *testing.T, root string) { + require.NoError(t, os.RemoveAll(root)) + }}, + {name: "every folder is gone", want: "no longer has the shape this server booted with (one folder per tenant)", damage: func(t *testing.T, root string) { + require.NoError(t, os.RemoveAll(filepath.Join(root, "acme"))) + require.NoError(t, os.RemoveAll(filepath.Join(root, "globex"))) + }}, + {name: "the root turned flat", want: "no longer has the shape this server booted with (one folder per tenant)", damage: func(t *testing.T, root string) { + require.NoError(t, os.RemoveAll(filepath.Join(root, "acme"))) + require.NoError(t, os.RemoveAll(filepath.Join(root, "globex"))) + writeTenant(t, root, ".", validFiles()) + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": maxRowsFiles(111), "globex": maxRowsFiles(222)}) + reg, _ := Open(root) + require.NotNil(t, reg) + reg.AfterAdopt(func([]tenant.ID) { t.Error("a reload rejected whole must not run the hooks") }) + + // A change that would be adopted, were the reload not rejected whole. + writeTenant(t, root, "globex", maxRowsFiles(999)) + tt.damage(t, root) + findings, adopted := reg.Reload("test") + assert.False(t, adopted) + assert.Contains(t, findingStrings(findings), tt.want) + + for id, want := range map[tenant.ID]int{"acme": 111, "globex": 222} { + store, ok := reg.For(id) + require.True(t, ok, "%s must still be served", id) + assert.Equal(t, want, store.DefaultMaxRows()) + } + }) + } +} + +// The shape is fixed at Open in the other direction too: a flat directory +// that turns into tenant folders is a rejected reload, and the document it +// was serving stays. +func TestRegistry_FlatRootTurnedNestedIsRejected(t *testing.T) { + t.Parallel() + reg := newLoadedRegistry(t, map[string]string{FileConfig: configJSON(`{"query": {"default_max_rows": 42}}`)}) + for _, name := range Files() { + require.NoError(t, os.Remove(filepath.Join(reg.Dir(), name))) + } + writeTenant(t, reg.Dir(), "acme", validFiles()) + + findings, adopted := reg.Reload("test") + assert.False(t, adopted) + assert.Contains(t, findingStrings(findings), "no longer has the shape this server booted with (the four files)") + s, ok := reg.For(tenant.Default) + require.True(t, ok) + assert.Equal(t, 42, s.DefaultMaxRows()) + _, ok = reg.For("acme") + assert.False(t, ok) +} diff --git a/internal/settings/settings.go b/internal/settings/settings.go index 9ec5d185..6689aefc 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -4,7 +4,8 @@ // // The directory holds exactly four files: roles.json (the role registry), // policies.json (the access-control policy), pipes.json (named queries), and -// config.json (behavioral tunables migrated out of boot config). Validate is +// config.json (behavioral tunables migrated out of boot config) — or, nested, +// one folder per tenant, each holding those four (see Tree). Validate is // deliberately pure — no network, no ClickHouse, no side effects — so the same // function can gate the `wavehouse validate` CLI, boot, and a live reload. // Table/column existence is out of scope by design: WaveHouse is diff --git a/internal/settings/store.go b/internal/settings/store.go index 2010dbd2..26fe5724 100644 --- a/internal/settings/store.go +++ b/internal/settings/store.go @@ -1,8 +1,6 @@ package settings import ( - "log/slog" - "sync" "sync/atomic" "time" @@ -10,100 +8,26 @@ import ( "github.com/Wave-RF/WaveHouse/internal/policy" ) -// Store owns the settings snapshot a running instance has adopted. Open -// validates and adopts the directory up front, so a *Store never exists -// without a good document behind it. Reload is the single code path every -// later trigger — SIGHUP, the directory watcher, and POST -// /v1/ops/settings/reload — funnels through: re-validate the directory, and -// swap the snapshot only when no finding is an error, so a bad edit (or a -// deleted file, or a vanished directory) can never evict the last good -// document. Readers go through one lock-free atomic load per lookup; the -// typed accessors below each resolve from a single snapshot load, so a -// reload lands between lookups, never inside one. +// Store holds the settings snapshot one tenant has adopted, and nothing +// else: the Registry validates, swaps the document in, and owns every reload +// trigger. Readers go through one lock-free atomic load per lookup; the typed +// accessors below each resolve from a single snapshot load, so a reload lands +// between lookups, never inside one. // // There are no compiled defaults here on purpose: every key is required by // Validate, so the snapshot is exactly what the files said when they were // adopted. Defaults live in the seed directory (Seed / WriteSeed). type Store struct { - dir string - - // mu serializes Reload: concurrent triggers queue rather than racing - // validate-then-swap sequences (a stale document must not overwrite a newer one). - mu sync.Mutex snap atomic.Pointer[Document] - // afterAdopt runs under mu after every successful swap, in registration - // order — for consumers that own a resource whose lifecycle follows a - // setting (the Pebble store behind dedupe.enabled) rather than reading - // the snapshot per call. - afterAdopt []func() -} - -// Open validates dir and returns a Store holding its document. A rejected -// directory returns a nil Store with the findings — the caller (boot) -// refuses to start; it must never run without adopted settings. -func Open(dir string) (*Store, []Finding) { - s := &Store{dir: dir} - findings, adopted := s.Reload("boot") - if !adopted { - return nil, findings - } - return s, findings -} - -// Dir returns the directory this store reads. -func (s *Store) Dir() string { return s.dir } - -// Reload re-validates the directory and adopts the parsed document when no -// finding is an error (warnings don't block adoption, matching `wavehouse -// validate`). On a rejected reload the previous snapshot stays in place. -// The returned bool reports whether the document was adopted. trigger names -// the path that fired ("boot", "sighup", "watch", "api") and tags every log -// line so operators can tell them apart. -func (s *Store) Reload(trigger string) ([]Finding, bool) { - s.mu.Lock() - defer s.mu.Unlock() - doc, findings := Validate(s.dir) - adopted := doc != nil - if adopted { - s.snap.Store(doc) - for _, fn := range s.afterAdopt { - fn() - } - } - var errs, warns int - for _, f := range findings { - if f.Severity == SeverityError { - errs++ - slog.Error("settings finding", "trigger", trigger, "finding", f.String()) - } else { - warns++ - slog.Warn("settings finding", "trigger", trigger, "finding", f.String()) - } - } - switch { - case adopted: - slog.Info("settings adopted", "trigger", trigger, "dir", s.dir, "warnings", warns) - case s.snap.Load() == nil: - slog.Error("settings rejected", "trigger", trigger, "dir", s.dir, "errors", errs, "warnings", warns) - default: - slog.Error("settings rejected — keeping previous settings", "trigger", trigger, "dir", s.dir, "errors", errs, "warnings", warns) - } - return findings, adopted } -// AfterAdopt registers fn to run after each subsequent successful reload, -// serialized with the reload itself. Open's boot adoption has already -// happened by the time a caller can register, so the caller applies the -// boot state itself. -func (s *Store) AfterAdopt(fn func()) { - s.mu.Lock() - defer s.mu.Unlock() - s.afterAdopt = append(s.afterAdopt, fn) -} +// adopt swaps in a validated document. The Registry calls it under its +// reload lock. +func (s *Store) adopt(doc *Document) { s.snap.Store(doc) } -// doc returns the current snapshot. Never nil for a Store returned by Open: -// adoption happened before the Store was handed out, and a rejected reload -// leaves the previous document in place. +// doc returns the current snapshot. Never nil for a Store a Registry from +// Open hands out: adoption happened first, and a rejected reload leaves the +// previous document in place. func (s *Store) doc() *Document { return s.snap.Load() } diff --git a/internal/settings/store_test.go b/internal/settings/store_test.go index f0ed9e1e..924aac4f 100644 --- a/internal/settings/store_test.go +++ b/internal/settings/store_test.go @@ -8,55 +8,19 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/Wave-RF/WaveHouse/internal/tenant" ) // newLoadedStore materializes a valid directory (with overrides applied) and -// returns a store that has adopted it. +// returns the store of the registry that has adopted it. func newLoadedStore(t *testing.T, overrides map[string]string) *Store { t.Helper() - files := validFiles() - for name, content := range overrides { - files[name] = content - } - s, findings := Open(writeDir(t, files)) - require.NotNil(t, s, "findings: %s", findingStrings(findings)) + s, ok := newLoadedRegistry(t, overrides).For(tenant.Default) + require.True(t, ok) return s } -func TestStore_ReloadAdoptsAndRejects(t *testing.T) { - t.Parallel() - s := newLoadedStore(t, map[string]string{ - FileConfig: configJSON(`{"query": {"default_max_rows": 500}}`), - }) - assert.Equal(t, 500, s.DefaultMaxRows()) - - // Break the directory: the reload must report the error and keep the - // previous snapshot — a bad edit can never evict the last good document. - require.NoError(t, os.WriteFile(filepath.Join(s.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": -1}}`)), 0o600)) - findings, adopted := s.Reload("test") - assert.False(t, adopted) - assert.True(t, HasErrors(findings)) - assert.Equal(t, 500, s.DefaultMaxRows(), "rejected reload must keep the previous snapshot") - - // Fix it: the next reload adopts again. - require.NoError(t, os.WriteFile(filepath.Join(s.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 700}}`)), 0o600)) - findings, adopted = s.Reload("test") - require.True(t, adopted, "findings: %s", findingStrings(findings)) - assert.Equal(t, 700, s.DefaultMaxRows()) -} - -func TestStore_ReloadWithWarningsAdopts(t *testing.T) { - t.Parallel() - s := newLoadedStore(t, map[string]string{ - FilePolicies: `{}`, // empty policy: legal, warned (total lockout) - FilePipes: `{}`, // drop the pipe so its analyst role reference doesn't dangle - }) - findings, adopted := s.Reload("test") - assert.True(t, adopted, "warnings alone must not block adoption") - assert.NotEmpty(t, findings) - assert.False(t, HasErrors(findings)) -} - func TestStore_DedupeFor_Cascade(t *testing.T) { t.Parallel() s := newLoadedStore(t, map[string]string{ @@ -81,40 +45,6 @@ func TestStore_DedupeFor_Cascade(t *testing.T) { } } -// TestStore_OpenRejectsInvalid pins the boot contract: an invalid directory -// yields no Store at all — there is no "store without a document" state and -// no compiled defaults to fall back on. -func TestStore_OpenRejectsInvalid(t *testing.T) { - t.Parallel() - files := validFiles() - files[FileConfig] = `{}` // every key missing - s, findings := Open(writeDir(t, files)) - assert.Nil(t, s) - assert.True(t, HasErrors(findings)) - - s, findings = Open(filepath.Join(t.TempDir(), "nope")) - assert.Nil(t, s) - assert.True(t, HasErrors(findings)) -} - -// TestStore_SurvivesVanishedDirectory pins the runtime half of the same -// contract: once adopted, the snapshot outlives its files — deleting the -// directory is just a rejected reload. -func TestStore_SurvivesVanishedDirectory(t *testing.T) { - t.Parallel() - s := newLoadedStore(t, map[string]string{ - FileConfig: configJSON(`{"query": {"default_max_rows": 42}}`), - }) - require.NoError(t, os.RemoveAll(s.Dir())) - findings, adopted := s.Reload("test") - assert.False(t, adopted) - assert.True(t, HasErrors(findings)) - assert.Equal(t, 42, s.DefaultMaxRows()) - _, id, req := s.DedupeFor("clicks") - assert.Equal(t, "event_id", id) - assert.False(t, req) -} - // TestStore_SeedIsValid pins that the shipped starter directory passes its // own gate: `wavehouse bootstrap` must never write something // `wavehouse validate` rejects, and the defaults are readable back. @@ -122,9 +52,10 @@ func TestStore_SeedIsValid(t *testing.T) { t.Parallel() dir := filepath.Join(t.TempDir(), "settings") require.NoError(t, WriteSeed(dir)) - s, findings := Open(dir) - require.NotNil(t, s, "findings: %s", findingStrings(findings)) + reg, findings := Open(dir) + require.NotNil(t, reg, "findings: %s", findingStrings(findings)) assert.False(t, HasErrors(findings)) + s, _ := reg.For(tenant.Default) // The one expected finding: an empty policies.json is fail-closed and // says so. The seed ships no policy on purpose — a policy is a tenant's // decision (deployments/compose/settings ships the opt-in trial one). @@ -190,26 +121,6 @@ func TestStore_DLQFor_Cascade(t *testing.T) { assert.True(t, s.DLQFor("other"), "unlisted table gets the global switch") } -// TestStore_AfterAdoptRunsOnlyOnAdoption pins the lifecycle hook contract: -// it fires after every successful reload (with the new snapshot already -// visible) and never on a rejected one. -func TestStore_AfterAdoptRunsOnlyOnAdoption(t *testing.T) { - t.Parallel() - s := newLoadedStore(t, nil) - var seen []bool - s.AfterAdopt(func() { seen = append(seen, s.DedupeEnabled()) }) - - require.NoError(t, os.WriteFile(filepath.Join(s.Dir(), FileConfig), []byte(configJSON(`{"dedupe": {"enabled": true}}`)), 0o600)) - _, adopted := s.Reload("test") - require.True(t, adopted) - assert.Equal(t, []bool{true}, seen, "hook sees the newly adopted snapshot") - - require.NoError(t, os.WriteFile(filepath.Join(s.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 0}}`)), 0o600)) - _, adopted = s.Reload("test") - require.False(t, adopted) - assert.Equal(t, []bool{true}, seen, "rejected reload must not fire the hook") -} - func TestStore_PolicyAndPipesAccessors(t *testing.T) { t.Parallel() s := newLoadedStore(t, nil) // validFiles: default_role public, one pipe "top_clicks" for analyst diff --git a/internal/settings/tree.go b/internal/settings/tree.go new file mode 100644 index 00000000..7d42f7cf --- /dev/null +++ b/internal/settings/tree.go @@ -0,0 +1,139 @@ +package settings + +import ( + "fmt" + "os" + "path" + "path/filepath" + "slices" + "strings" + + "github.com/Wave-RF/WaveHouse/internal/tenant" +) + +// Tree is a validated settings root (#583). A flat root — the four files in +// the root itself — is tenant.Default alone. A nested root holds one folder +// per tenant and nothing else, the folder name being the tenant id. +type Tree struct { + Nested bool + // Tenants holds every tenant the root defines. Doc is nil for a tenant + // whose folder has an error finding. + Tenants map[tenant.ID]TenantResult +} + +// TenantResult is one tenant's share of a validation pass. +type TenantResult struct { + Doc *Document + Findings []Finding +} + +// Validate checks a settings root of either shape in one pass and returns +// every finding, the way ValidateDir does for one directory. The shape is +// read off the root: holding any of the four settings file names makes it +// flat, whatever else is there; otherwise holding a folder makes it nested; +// otherwise — empty, missing, unreadable — it is flat again, so ValidateDir +// names the problem exactly as it always has. Mixing the shapes needs no rule +// of its own: a flat root rejects a folder and a nested root rejects a loose +// file. +// +// A flat root's findings are ValidateDir's, untouched. A nested root's carry +// the tenant folder in File ("acme/policies.json"), and a folder whose name +// is not a tenant id is a finding against that folder alone. The Tree is nil +// when the finding is about the root itself — it cannot be listed, or a +// nested root holds a loose file or an entry that cannot be stat'ed — since +// no tenant can then be trusted to be what the root's author meant. +func Validate(root string) (*Tree, []Finding) { + folders, loose, listed := listRoot(root) + if len(folders) == 0 { + doc, findings := ValidateDir(root) + if !listed && doc == nil { + return nil, findings + } + return &Tree{Tenants: map[tenant.ID]TenantResult{tenant.Default: {Doc: doc, Findings: findings}}}, findings + } + + var findings []Finding + for _, entry := range loose { + // An entry that cannot be stat'ed is no more a tenant folder than a + // loose file is, but say what it is: a dangling symlink reported as a + // stray file sends its reader looking for the wrong thing. + problem := "unexpected file" + if entry.err != nil { + problem = fmt.Sprintf("stat: %v", entry.err) + } + findings = append(findings, Finding{Severity: SeverityError, File: entry.name, Message: problem + " — a nested settings directory holds only tenant folders, each named by its tenant id"}) + } + tree := &Tree{Nested: true, Tenants: make(map[tenant.ID]TenantResult, len(folders))} + for _, name := range folders { + id, err := tenant.Parse(name) + if err != nil { + findings = append(findings, Finding{Severity: SeverityError, File: name, Message: fmt.Sprintf("folder name is not a tenant id: %v", err)}) + continue + } + doc, fs := validateFolder(root, name) + tree.Tenants[id] = TenantResult{Doc: doc, Findings: fs} + findings = append(findings, fs...) + } + if len(loose) > 0 { + return nil, findings + } + return tree, findings +} + +// validateFolder is ValidateDir for one tenant folder of a nested root, with +// the folder leading each finding's File. +// +// This is the one place a tenant id becomes a filesystem path. Every caller +// already hands it an id that passed tenant.Parse (which forbids '.', '/' and +// '\'), so the check below is never reached today; it is here so the +// guarantee that a name resolves to one folder under root — never root +// itself, never outside it — lives beside the join that depends on it, +// rather than in two callers — and it is the check CodeQL's path-injection +// query recognizes as a sanitizer, which the grammar in tenant.Parse is not. +func validateFolder(root, folder string) (*Document, []Finding) { + if folder == "" || folder == "." || strings.Contains(folder, "/") || strings.Contains(folder, `\`) || strings.Contains(folder, "..") { + return nil, []Finding{{Severity: SeverityError, File: folder, Message: "folder name is not a tenant id: it must name one folder — not empty, not \".\", no path separator, no \"..\""}} + } + doc, findings := ValidateDir(filepath.Join(root, folder)) + for i := range findings { + // path, not filepath: File is part of the ops API, one spelling on every OS. + findings[i].File = path.Join(folder, findings[i].File) + } + return doc, findings +} + +// looseEntry is a root entry that is not a tenant folder: a file, or +// something that could not be stat'ed (err says why). +type looseEntry struct { + name string + err error +} + +// listRoot sorts the root's entries into tenant folders and loose entries, in +// name order. It returns no folders for a flat root — one holding any of the +// four settings file names — and listed=false when the root cannot be read. +// Dot-prefixed entries are skipped, as ValidateDir skips them. +func listRoot(root string) (folders []string, loose []looseEntry, listed bool) { + entries, err := os.ReadDir(root) + if err != nil { + return nil, nil, false + } + for _, e := range entries { + name := e.Name() + if strings.HasPrefix(name, ".") { + continue + } + if slices.Contains(Files(), name) { + return nil, nil, true + } + // Stat, not the entry's own type: a Kubernetes ConfigMap mount + // publishes each folder as a symlink into its `..data` directory. + info, err := os.Stat(filepath.Join(root, name)) + if err == nil && info.IsDir() { + folders = append(folders, name) + } else { + loose = append(loose, looseEntry{name: name, err: err}) + } + } + return folders, loose, true +} diff --git a/internal/settings/tree_test.go b/internal/settings/tree_test.go new file mode 100644 index 00000000..4c3feeb6 --- /dev/null +++ b/internal/settings/tree_test.go @@ -0,0 +1,195 @@ +package settings + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Wave-RF/WaveHouse/internal/tenant" +) + +// writeTree materializes a nested root: tenant folder → file name → content. +func writeTree(t *testing.T, tenants map[string]map[string]string) string { + t.Helper() + root := t.TempDir() + for folder, files := range tenants { + dir := filepath.Join(root, folder) + require.NoError(t, os.Mkdir(dir, 0o750)) + for name, content := range files { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600)) + } + } + return root +} + +// A flat root goes through ValidateDir untouched: same findings, same +// document, whatever is wrong with it. This is what keeps a single-tenant +// directory byte-identical across the move to a tree-shaped Validate. +func TestValidate_FlatRootIsValidateDir(t *testing.T) { + t.Parallel() + tests := []struct { + name string + root func(t *testing.T) string + wantDoc bool + }{ + {name: "valid", root: func(t *testing.T) string { return writeDir(t, validFiles()) }, wantDoc: true}, + {name: "empty root", root: func(t *testing.T) string { return t.TempDir() }}, + {name: "only a stray file", root: func(t *testing.T) string { return writeDir(t, map[string]string{"notes.txt": "x"}) }}, + {name: "missing file", root: func(t *testing.T) string { + files := validFiles() + delete(files, FileConfig) + return writeDir(t, files) + }}, + {name: "a folder beside the files", root: func(t *testing.T) string { + dir := writeDir(t, validFiles()) + require.NoError(t, os.Mkdir(filepath.Join(dir, "acme"), 0o700)) + return dir + }}, + {name: "a folder squatting on a settings file name", root: func(t *testing.T) string { + files := validFiles() + delete(files, FileRoles) + dir := writeDir(t, files) + require.NoError(t, os.Mkdir(filepath.Join(dir, FileRoles), 0o700)) + return dir + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + root := tt.root(t) + wantDoc, wantFindings := ValidateDir(root) + require.Equal(t, tt.wantDoc, wantDoc != nil, "findings: %s", findingStrings(wantFindings)) + + tree, findings := Validate(root) + assert.Equal(t, wantFindings, findings) + require.NotNil(t, tree) + assert.False(t, tree.Nested) + require.Len(t, tree.Tenants, 1) + got := tree.Tenants[tenant.Default] + assert.Equal(t, wantDoc, got.Doc) + assert.Equal(t, wantFindings, got.Findings) + }) + } +} + +// A root that cannot be listed has no shape to report: no Tree, and the +// finding ValidateDir has always given. +func TestValidate_UnusableRoot(t *testing.T) { + t.Parallel() + missing := filepath.Join(t.TempDir(), "nope") + tree, findings := Validate(missing) + assert.Nil(t, tree) + assert.Contains(t, findingStrings(findings), "does not exist") + + file := filepath.Join(t.TempDir(), "file") + require.NoError(t, os.WriteFile(file, []byte("x"), 0o600)) + tree, findings = Validate(file) + assert.Nil(t, tree) + assert.Contains(t, findingStrings(findings), "not a directory") +} + +func TestValidate_NestedRoot(t *testing.T) { + t.Parallel() + acme := validFiles() + acme[FileConfig] = configJSON(`{"query": {"default_max_rows": 111}}`) + root := writeTree(t, map[string]map[string]string{"acme": acme, "0": validFiles()}) + // Dot-prefixed entries stay invisible at the root too: the machinery of a + // ConfigMap mount sits beside the tenant folders. + require.NoError(t, os.Mkdir(filepath.Join(root, "..data"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(root, ".root.swp"), []byte("vim"), 0o600)) + + tree, findings := Validate(root) + require.Empty(t, findings) + require.NotNil(t, tree) + assert.True(t, tree.Nested) + require.Len(t, tree.Tenants, 2) + require.NotNil(t, tree.Tenants["acme"].Doc) + assert.Equal(t, 111, *tree.Tenants["acme"].Doc.Config.Query.DefaultMaxRows) + require.NotNil(t, tree.Tenants[tenant.Default].Doc, "a 0 folder is an ordinary tenant of a nested root") +} + +// One bad folder is that tenant's problem alone: its findings name the +// folder, its document is withheld, and the tenant beside it validates. +func TestValidate_NestedRejectedFolder(t *testing.T) { + t.Parallel() + globex := validFiles() + globex[FileConfig] = configJSON(`{"query": {"default_max_rows": -1}}`) + delete(globex, FilePipes) + root := writeTree(t, map[string]map[string]string{"acme": validFiles(), "globex": globex}) + + tree, findings := Validate(root) + require.NotNil(t, tree) + require.True(t, HasErrors(findings)) + out := findingStrings(findings) + assert.Contains(t, out, "error: globex/pipes.json: missing") + assert.Contains(t, out, "error: globex/config.json: query.default_max_rows: must be >= 1") + assert.NotContains(t, out, "acme") + + assert.NotNil(t, tree.Tenants["acme"].Doc) + assert.Empty(t, tree.Tenants["acme"].Findings) + assert.Nil(t, tree.Tenants["globex"].Doc) + assert.Equal(t, findings, tree.Tenants["globex"].Findings, "the flat list is the tenants' findings in folder order") +} + +// A folder whose name is not a tenant id can never be served, so it is a +// finding against that folder and defines no tenant; the rest of the root +// stands. +func TestValidate_NestedFolderNames(t *testing.T) { + t.Parallel() + tests := []struct { + name, folder, want string + }{ + {name: "dot", folder: "acme.bak", want: `error: acme.bak: folder name is not a tenant id: tenant id has '.' at byte 4`}, + {name: "space", folder: "acme corp", want: `error: acme corp: folder name is not a tenant id: tenant id has ' ' at byte 4`}, + {name: "over the length cap", folder: strings.Repeat("a", tenant.MaxLen+1), want: "folder name is not a tenant id: tenant id is 65 bytes, the limit is 64"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": validFiles(), tt.folder: validFiles()}) + tree, findings := Validate(root) + require.Len(t, findings, 1, "findings: %s", findingStrings(findings)) + assert.Contains(t, findings[0].String(), tt.want) + require.NotNil(t, tree) + require.Len(t, tree.Tenants, 1) + assert.NotNil(t, tree.Tenants["acme"].Doc) + }) + } +} + +// A loose file beside the tenant folders is a finding about the root itself: +// no Tree, though every folder is still checked so one pass reports it all. +func TestValidate_NestedLooseFile(t *testing.T) { + t.Parallel() + globex := validFiles() + delete(globex, FileRoles) + root := writeTree(t, map[string]map[string]string{"acme": validFiles(), "globex": globex}) + require.NoError(t, os.WriteFile(filepath.Join(root, "notes.txt"), []byte("scratch"), 0o600)) + + tree, findings := Validate(root) + assert.Nil(t, tree) + out := findingStrings(findings) + assert.Contains(t, out, "error: notes.txt: unexpected file — a nested settings directory holds only tenant folders") + assert.Contains(t, out, "error: globex/roles.json: missing") +} + +// validateFolder is where a tenant id becomes a path, and it refuses a name +// that could leave the root itself rather than trusting its callers to have +// parsed it: a finding against that name, nothing read. +func TestValidateFolder_RefusesAPathLikeName(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": validFiles()}) + for _, name := range []string{"../acme", "acme/..", `..\acme`, "a/b", "..", ".", ""} { + doc, findings := validateFolder(root, name) + assert.Nil(t, doc, name) + require.Len(t, findings, 1, name) + assert.Equal(t, name, findings[0].File) + assert.Contains(t, findings[0].Message, "not a tenant id") + } + doc, findings := validateFolder(root, "acme") + assert.NotNil(t, doc, "a plain name is checked as before: %s", findingStrings(findings)) +} diff --git a/internal/settings/validate.go b/internal/settings/validate.go index 6f034533..8ddd3602 100644 --- a/internal/settings/validate.go +++ b/internal/settings/validate.go @@ -16,17 +16,18 @@ import ( "github.com/Wave-RF/WaveHouse/internal/policy" ) -// Validate reads, decodes, and checks a settings directory in one pass and -// returns every finding it can discover — an operator fixing a hand-edited -// directory wants the whole list, not a fix-rerun-fix loop. No side effects: -// no network, no ClickHouse, nothing written. The Document is returned only -// when no finding is an error (warnings alone leave it usable), so what a -// node adopts is byte-for-byte what was validated — this is the single -// checking path, and every consumer (the `wavehouse validate` CLI, boot, a -// live reload) goes through it. Callers translate the result per their own -// contract: the CLI exits non-zero, boot refuses to start, and a live reload -// keeps serving the previous good document. -func Validate(dir string) (*Document, []Finding) { +// ValidateDir reads, decodes, and checks one directory of the four settings +// files — a flat root, or one tenant's folder of a nested one — in one pass +// and returns every finding it can discover — an operator fixing a +// hand-edited directory wants the whole list, not a fix-rerun-fix loop. No +// side effects: no network, no ClickHouse, nothing written. The Document is +// returned only when no finding is an error (warnings alone leave it usable), +// so what a node adopts is byte-for-byte what was validated — this is the +// single checking path, and every consumer (the `wavehouse validate` CLI, +// boot, a live reload) goes through it, by way of Validate. Callers translate +// the result per their own contract: the CLI exits non-zero, boot refuses to +// start, and a live reload keeps serving the previous good document. +func ValidateDir(dir string) (*Document, []Finding) { v := &validator{} files, ok := v.checkDir(dir) diff --git a/internal/settings/validate_test.go b/internal/settings/validate_test.go index 7ea989ef..4cb9706b 100644 --- a/internal/settings/validate_test.go +++ b/internal/settings/validate_test.go @@ -72,7 +72,7 @@ func findingStrings(findings []Finding) string { func TestValidate_ValidDirectory(t *testing.T) { t.Parallel() - doc, findings := Validate(writeDir(t, validFiles())) + doc, findings := ValidateDir(writeDir(t, validFiles())) require.Empty(t, findings, "a fully valid directory must produce no findings") require.NotNil(t, doc) @@ -92,7 +92,7 @@ func TestValidate_ValidDirectory(t *testing.T) { func TestValidate_EmptyDocuments(t *testing.T) { t.Parallel() - doc, findings := Validate(writeDir(t, map[string]string{ + doc, findings := ValidateDir(writeDir(t, map[string]string{ FileRoles: `{}`, FilePolicies: `{}`, FilePipes: `{}`, FileConfig: configJSON(`{}`), })) @@ -112,7 +112,7 @@ func TestValidate_DirectoryProblems(t *testing.T) { t.Run("missing directory", func(t *testing.T) { t.Parallel() - doc, findings := Validate(filepath.Join(t.TempDir(), "nope")) + doc, findings := ValidateDir(filepath.Join(t.TempDir(), "nope")) assert.Nil(t, doc) require.True(t, HasErrors(findings)) assert.Contains(t, findingStrings(findings), "does not exist") @@ -122,7 +122,7 @@ func TestValidate_DirectoryProblems(t *testing.T) { t.Parallel() f := filepath.Join(t.TempDir(), "file") require.NoError(t, os.WriteFile(f, []byte("x"), 0o600)) - doc, findings := Validate(f) + doc, findings := ValidateDir(f) assert.Nil(t, doc) assert.Contains(t, findingStrings(findings), "not a directory") }) @@ -131,7 +131,7 @@ func TestValidate_DirectoryProblems(t *testing.T) { t.Parallel() files := validFiles() delete(files, FileConfig) - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) assert.Nil(t, doc) assert.Contains(t, findingStrings(findings), "config.json: missing") }) @@ -141,7 +141,7 @@ func TestValidate_DirectoryProblems(t *testing.T) { files := validFiles() files["polices.json"] = `{}` // the canonical typo files["notes.txt"] = "scratch" - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) assert.Nil(t, doc) out := findingStrings(findings) assert.Contains(t, out, "polices.json: unexpected file") @@ -152,7 +152,7 @@ func TestValidate_DirectoryProblems(t *testing.T) { t.Parallel() dir := writeDir(t, validFiles()) require.NoError(t, os.Mkdir(filepath.Join(dir, "backup"), 0o700)) - doc, findings := Validate(dir) + doc, findings := ValidateDir(dir) assert.Nil(t, doc) assert.Contains(t, findingStrings(findings), "backup: unexpected directory") }) @@ -163,7 +163,7 @@ func TestValidate_DirectoryProblems(t *testing.T) { delete(files, FileRoles) dir := writeDir(t, files) require.NoError(t, os.Mkdir(filepath.Join(dir, FileRoles), 0o700)) - doc, findings := Validate(dir) + doc, findings := ValidateDir(dir) assert.Nil(t, doc) out := findingStrings(findings) assert.Contains(t, out, "roles.json: is a directory") @@ -177,7 +177,7 @@ func TestValidate_DirectoryProblems(t *testing.T) { } dir := writeDir(t, validFiles()) require.NoError(t, os.Chmod(filepath.Join(dir, FileConfig), 0o000)) - doc, findings := Validate(dir) + doc, findings := ValidateDir(dir) assert.Nil(t, doc) out := findingStrings(findings) assert.Contains(t, out, "config.json: read:") @@ -194,7 +194,7 @@ func TestValidate_DirectoryProblems(t *testing.T) { files[".roles.json.swp"] = "vim" dir := writeDir(t, files) require.NoError(t, os.Mkdir(filepath.Join(dir, "..data"), 0o700)) - doc, findings := Validate(dir) + doc, findings := ValidateDir(dir) assert.Empty(t, findings) assert.NotNil(t, doc) }) @@ -225,7 +225,7 @@ func TestValidate_FileSyntax(t *testing.T) { t.Parallel() files := validFiles() files[tt.file] = tt.body - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) assert.Nil(t, doc) require.True(t, HasErrors(findings), "findings: %s", findingStrings(findings)) assert.Contains(t, findingStrings(findings), tt.want) @@ -315,7 +315,7 @@ func TestValidate_ContentRules(t *testing.T) { t.Parallel() files := validFiles() files[tt.file] = tt.body - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) assert.Nil(t, doc) require.True(t, HasErrors(findings), "findings: %s", findingStrings(findings)) assert.Contains(t, findingStrings(findings), tt.want) @@ -331,7 +331,7 @@ func TestValidate_RoleReferences(t *testing.T) { files := validFiles() files[FilePolicies] = `{"default_role": "ghost", "tables": {"clicks": {"phantom": {"select": {}}}}}` files[FilePipes] = `{"pipes": [{"name": "a", "sql": "SELECT 1", "allowed_roles": ["specter"]}]}` - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) assert.Nil(t, doc) out := findingStrings(findings) assert.Contains(t, out, `default_role: role "ghost" is not declared`) @@ -343,7 +343,7 @@ func TestValidate_RoleReferences(t *testing.T) { t.Parallel() files := validFiles() files[FilePolicies] = `{"admin_role": "root", "tables": {}}` - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) assert.Nil(t, doc) assert.Contains(t, findingStrings(findings), `admin_role: role "root" is not declared`) }) @@ -352,7 +352,7 @@ func TestValidate_RoleReferences(t *testing.T) { t.Parallel() files := validFiles() files[FileRoles] = `{"roles": [` - _, findings := Validate(writeDir(t, files)) + _, findings := ValidateDir(writeDir(t, files)) out := findingStrings(findings) assert.NotContains(t, out, "is not declared", "reference checks against a broken registry are noise") }) @@ -379,7 +379,7 @@ func TestValidate_Warnings(t *testing.T) { t.Parallel() files := validFiles() files[tt.file] = tt.body - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) require.NotNil(t, doc, "warnings alone must leave the directory valid: %s", findingStrings(findings)) assert.False(t, HasErrors(findings)) assert.Contains(t, findingStrings(findings), tt.want) @@ -414,7 +414,7 @@ func TestValidate_LegacyPolicyLayout(t *testing.T) { t.Parallel() files := validFiles() files[FilePolicies] = tt.body - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) assert.Nil(t, doc) require.True(t, HasErrors(findings), "findings: %s", findingStrings(findings)) out := findingStrings(findings) @@ -434,7 +434,7 @@ func TestValidate_V2LayoutNotFlaggedAsLegacy(t *testing.T) { `"select": {"allow_columns": ["page"], "filter": {"region": {"_eq": "eu"}}},` + `"insert": {"check": {"region": {"_eq": "eu"}}}}}}}` files[FileRoles] = `{"roles": ["public", "analyst"]}` - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) require.NotNil(t, doc, "findings: %s", findingStrings(findings)) assert.NotContains(t, findingStrings(findings), "pre-v2") require.NotNil(t, doc.Policy.Tables["clicks"]["analyst"].Select) @@ -448,7 +448,7 @@ func TestValidate_V2LayoutNotFlaggedAsLegacy(t *testing.T) { // exactly once, so the operator gets the whole list without noise. func TestValidate_MultipleFaults(t *testing.T) { t.Parallel() - doc, findings := Validate(writeDir(t, map[string]string{ + doc, findings := ValidateDir(writeDir(t, map[string]string{ FileRoles: `{"roles": ["analyst", "analyst"]}`, // duplicate role FilePolicies: `{"default_role": "ghost", "tables": {}}`, // undeclared role FilePipes: `{"pipes": [`, // truncated JSON @@ -477,7 +477,7 @@ func TestValidate_RoleNamedAfterAnOperation_DecodesAsWritten(t *testing.T) { files[FileRoles] = `{"roles": ["select"]}` files[FilePolicies] = `{"tables":{"clicks":{"select":{"select":{"allow_columns":["page"]}}}}}` files[FilePipes] = `{}` - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) require.False(t, HasErrors(findings), "the readings agree, so this must adopt: %v", findingStrings(findings)) require.NotNil(t, doc) @@ -506,7 +506,7 @@ func TestValidate_EmptyLegacyOperationBlock(t *testing.T) { files[FileRoles] = `{"roles": ["viewer"]}` files[FilePolicies] = legacy files[FilePipes] = `{}` - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) require.True(t, HasErrors(findings)) assert.Nil(t, doc) joined := findingStrings(findings) @@ -523,7 +523,7 @@ func TestValidate_EmptyLegacyOperationBlock(t *testing.T) { files[FileRoles] = `{"roles": ["select"]}` files[FilePolicies] = legacy files[FilePipes] = `{}` - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) require.False(t, HasErrors(findings), "%v", findingStrings(findings)) require.NotNil(t, doc, "the document adopts") assert.Contains(t, findingStrings(findings), "grant sets neither select nor insert") @@ -537,7 +537,7 @@ func TestValidate_ErrorAndWarningMix(t *testing.T) { files := validFiles() files[FilePolicies] = `{"default_role": "public", "tables": {"clicks": {"admin": {"select": {}}}}}` // warning: admin grant is dead config files[FileConfig] = configJSON(`{"schema": {"refresh_interval": 0}}`) // error: bounds violation - doc, findings := Validate(writeDir(t, files)) + doc, findings := ValidateDir(writeDir(t, files)) assert.Nil(t, doc, "one error rejects the directory even when the rest only warns") require.True(t, HasErrors(findings)) out := findingStrings(findings) @@ -568,10 +568,10 @@ func FuzzSyntaxGate(f *testing.F) { // when no finding is an error. func TestValidate_DocumentGating(t *testing.T) { t.Parallel() - doc, findings := Validate(writeDir(t, validFiles())) + doc, findings := ValidateDir(writeDir(t, validFiles())) assert.NotNil(t, doc) assert.Empty(t, findings) - doc, findings = Validate(filepath.Join(t.TempDir(), "nope")) + doc, findings = ValidateDir(filepath.Join(t.TempDir(), "nope")) assert.Nil(t, doc) assert.True(t, HasErrors(findings)) } diff --git a/internal/settings/validate_unix_test.go b/internal/settings/validate_unix_test.go index b0e538af..a1e54e8b 100644 --- a/internal/settings/validate_unix_test.go +++ b/internal/settings/validate_unix_test.go @@ -23,7 +23,7 @@ func TestValidate_NonRegularFile(t *testing.T) { dir := writeDir(t, files) require.NoError(t, syscall.Mkfifo(filepath.Join(dir, FilePipes), 0o600)) - doc, findings := Validate(dir) + doc, findings := ValidateDir(dir) assert.Nil(t, doc) out := findingStrings(findings) assert.Contains(t, out, "pipes.json: not a regular file") @@ -44,7 +44,66 @@ func TestValidate_SymlinkedFiles(t *testing.T) { require.NoError(t, os.Symlink(filepath.Join(dataDir, name), filepath.Join(dir, name))) } - doc, findings := Validate(dir) + doc, findings := ValidateDir(dir) require.NotNil(t, doc, "findings: %s", findingStrings(findings)) assert.Empty(t, findings) } + +// TestValidate_SymlinkedTenantFolders is the nested form of the same mount: +// each tenant folder at the root is a symlink into the dot-prefixed data +// directory. A directory entry's own type says "symlink", so reading the +// shape off it would call every tenant a loose file. +func TestValidate_SymlinkedTenantFolders(t *testing.T) { + t.Parallel() + root := t.TempDir() + data := writeTree(t, map[string]map[string]string{"acme": validFiles(), "globex": validFiles()}) + dataDir := filepath.Join(root, "..data") + require.NoError(t, os.Rename(data, dataDir)) + for _, folder := range []string{"acme", "globex"} { + require.NoError(t, os.Symlink(filepath.Join(dataDir, folder), filepath.Join(root, folder))) + } + + tree, findings := Validate(root) + require.Empty(t, findings) + require.NotNil(t, tree) + assert.True(t, tree.Nested) + assert.Len(t, tree.Tenants, 2) +} + +// A finding about a tenant's folder itself, which ValidateDir reports with no +// file, names the folder. +func TestValidate_UnreadableTenantFolder(t *testing.T) { + t.Parallel() + if os.Geteuid() == 0 { + t.Skip("root ignores directory permission bits") + } + root := writeTree(t, map[string]map[string]string{"acme": validFiles(), "globex": validFiles()}) + locked := filepath.Join(root, "globex") + require.NoError(t, os.Chmod(locked, 0o000)) + t.Cleanup(func() { _ = os.Chmod(locked, 0o700) }) //nolint:gosec // G302: restores a test directory so TempDir cleanup can remove it + + tree, findings := Validate(root) + require.NotNil(t, tree) + require.Len(t, findings, 1, "findings: %s", findingStrings(findings)) + assert.Equal(t, "globex", findings[0].File) + assert.Contains(t, findings[0].Message, "read settings directory") + assert.Nil(t, tree.Tenants["globex"].Doc) + assert.NotNil(t, tree.Tenants["acme"].Doc) +} + +// A tenant folder's symlink whose target is gone cannot be stat'ed, so it is +// not a folder — and the finding says that, rather than sending its reader to +// look for a stray file. It is a finding about the root: no Tree. +func TestValidate_DanglingTenantSymlink(t *testing.T) { + t.Parallel() + root := writeTree(t, map[string]map[string]string{"acme": validFiles()}) + require.NoError(t, os.Symlink(filepath.Join(root, "..data", "globex"), filepath.Join(root, "globex"))) + + tree, findings := Validate(root) + assert.Nil(t, tree) + require.Len(t, findings, 1, "findings: %s", findingStrings(findings)) + assert.Equal(t, "globex", findings[0].File) + assert.Contains(t, findings[0].Message, "stat: ") + assert.Contains(t, findings[0].Message, "no such file or directory") + assert.NotContains(t, findings[0].Message, "unexpected file") +} diff --git a/internal/settings/watch.go b/internal/settings/watch.go index 6a1f3f7b..0624dd33 100644 --- a/internal/settings/watch.go +++ b/internal/settings/watch.go @@ -2,6 +2,7 @@ package settings import ( "context" + "errors" "fmt" "log/slog" "path/filepath" @@ -38,15 +39,23 @@ const watchDebounce = 250 * time.Millisecond // The setup error is returned (directory missing, fd limits); runtime watcher // errors are logged and the loop continues — SIGHUP and the ops reload // endpoint remain as triggers even if the watcher degrades. -func (s *Store) Watch(ctx context.Context) error { +// +// Flat directories only, and a nested registry is refused rather than +// trusted to its caller: "the previous good snapshot stays" does not hold +// there. A watcher would validate a tenant's folder halfway through being +// written, and with no previous-snapshot fallback that drops the tenant. +func (r *Registry) Watch(ctx context.Context) error { + if r.nested { + return errors.New("settings watcher: a nested settings directory is never watched — reload it through POST /v1/ops/settings/reload or SIGHUP") + } w, err := fsnotify.NewWatcher() if err != nil { return fmt.Errorf("settings watcher: %w", err) } defer func() { _ = w.Close() }() - dir := filepath.Clean(s.dir) + dir := filepath.Clean(r.dir) if err := w.Add(dir); err != nil { - return fmt.Errorf("settings watcher: watch %s: %w", s.dir, err) + return fmt.Errorf("settings watcher: watch %s: %w", r.dir, err) } // Best effort: a parent that can't be watched (e.g. "/" permissions) // costs only the recreate case, not the watcher. @@ -59,7 +68,7 @@ func (s *Store) Watch(ctx context.Context) error { // an edit that landed in between (a ConfigMap update during a rolling // restart, say) fired no event and would otherwise sit unnoticed, with // every pod looking healthy, until something touched the directory again. - s.Reload("watch") + r.Reload("watch") // The timer starts disarmed; each relevant event re-arms it, so the // reload fires watchDebounce after the *last* event of a burst. @@ -90,14 +99,14 @@ func (s *Store) Watch(ctx context.Context) error { if !ok { return nil } - slog.ErrorContext(ctx, "settings watcher error", "dir", s.dir, "error", werr) + slog.ErrorContext(ctx, "settings watcher error", "dir", r.dir, "error", werr) case <-timer.C: // Re-arm the directory watch before reloading: after a remove or // rename fsnotify has dropped it, and Add is a no-op while it // still exists. Failure (directory currently absent) is expected // mid-replace; the next parent event retries. _ = w.Add(dir) - s.Reload("watch") + r.Reload("watch") } } } diff --git a/internal/settings/watch_test.go b/internal/settings/watch_test.go index 94bd60c3..f810b9e7 100644 --- a/internal/settings/watch_test.go +++ b/internal/settings/watch_test.go @@ -10,43 +10,47 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/Wave-RF/WaveHouse/internal/tenant" ) -// TestStore_Watch_ReloadsOnChange pins the watcher end to end: an edit to a +// TestRegistry_Watch_ReloadsOnChange pins the watcher end to end: an edit to a // settings file lands in the snapshot without any explicit reload call. The // debounce makes exact timing untestable, so the assertion polls. -// startWatch runs s.Watch in the background and proves the watch is live +// startWatch runs reg.Watch in the background and proves the watch is live // before returning: it writes a valid config.json with maxRows and waits for // the watcher to adopt it. A fixed sleep could not distinguish "watch // registered" from "write raced ahead of w.Add", and with the watch proven // live, the caller's later mutations can't be mistaken for setup races. -func startWatch(ctx context.Context, t *testing.T, s *Store, maxRows int) <-chan error { +func startWatch(ctx context.Context, t *testing.T, reg *Registry, maxRows int) <-chan error { t.Helper() + s, _ := reg.For(tenant.Default) done := make(chan error, 1) - go func() { done <- s.Watch(ctx) }() + go func() { done <- reg.Watch(ctx) }() // Keep writing until the watcher picks one up: the write and w.Add race, // and a write that lands before the watch exists emits no event. // EventuallyWithT rather than Eventually: the condition runs off the test // goroutine, where a bare require.NoError would FailNow the wrong // goroutine and stall the poll until timeout instead of failing loudly. require.EventuallyWithT(t, func(c *assert.CollectT) { - require.NoError(c, os.WriteFile(filepath.Join(s.Dir(), FileConfig), []byte(configJSON(fmt.Sprintf(`{"query": {"default_max_rows": %d}}`, maxRows))), 0o600)) + require.NoError(c, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(configJSON(fmt.Sprintf(`{"query": {"default_max_rows": %d}}`, maxRows))), 0o600)) assert.Equal(c, maxRows, s.DefaultMaxRows()) }, 5*time.Second, 2*watchDebounce, "watcher should adopt the readiness write") return done } -func TestStore_Watch_ReloadsOnChange(t *testing.T) { +func TestRegistry_Watch_ReloadsOnChange(t *testing.T) { t.Parallel() - s := newLoadedStore(t, map[string]string{ + reg := newLoadedRegistry(t, map[string]string{ FileConfig: configJSON(`{"query": {"default_max_rows": 100}}`), }) + s, _ := reg.For(tenant.Default) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - done := startWatch(ctx, t, s, 900) + done := startWatch(ctx, t, reg, 900) // An invalid edit is debounced, rejected, and the snapshot survives. - require.NoError(t, os.WriteFile(filepath.Join(s.Dir(), FileConfig), []byte(`not json`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(`not json`), 0o600)) time.Sleep(4 * watchDebounce) assert.Equal(t, 900, s.DefaultMaxRows(), "invalid edit must keep the previous snapshot") @@ -54,22 +58,23 @@ func TestStore_Watch_ReloadsOnChange(t *testing.T) { assert.NoError(t, <-done) } -// TestStore_Watch_CatchesUpOnStart pins the boot gap: an edit that lands after +// TestRegistry_Watch_CatchesUpOnStart pins the boot gap: an edit that lands after // Open's read but before the watch exists fires no event, so Watch reloads // once as soon as the watch is registered instead of waiting for the next one. -func TestStore_Watch_CatchesUpOnStart(t *testing.T) { +func TestRegistry_Watch_CatchesUpOnStart(t *testing.T) { t.Parallel() - s := newLoadedStore(t, map[string]string{ + reg := newLoadedRegistry(t, map[string]string{ FileConfig: configJSON(`{"query": {"default_max_rows": 100}}`), }) + s, _ := reg.For(tenant.Default) // The "in-between" edit: after Open, before Watch. - require.NoError(t, os.WriteFile(filepath.Join(s.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 700}}`)), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 700}}`)), 0o600)) assert.Equal(t, 100, s.DefaultMaxRows(), "no watch yet, so nothing has adopted the edit") ctx, cancel := context.WithCancel(context.Background()) defer cancel() done := make(chan error, 1) - go func() { done <- s.Watch(ctx) }() + go func() { done <- reg.Watch(ctx) }() assert.Eventually(t, func() bool { return s.DefaultMaxRows() == 700 }, 5*time.Second, 50*time.Millisecond, "Watch should reload once on start without any further edit") @@ -77,50 +82,64 @@ func TestStore_Watch_CatchesUpOnStart(t *testing.T) { assert.NoError(t, <-done) } -// TestStore_Watch_MissingDir pins the setup contract: a nonexistent directory +// TestRegistry_Watch_MissingDir pins the setup contract: a nonexistent directory // is a returned error (internal/app logs it and degrades to SIGHUP + the ops // endpoint), not a silent no-op loop. -func TestStore_Watch_MissingDir(t *testing.T) { +func TestRegistry_Watch_MissingDir(t *testing.T) { t.Parallel() - s := newLoadedStore(t, nil) - require.NoError(t, os.RemoveAll(s.Dir())) - err := s.Watch(context.Background()) + reg := newLoadedRegistry(t, nil) + require.NoError(t, os.RemoveAll(reg.Dir())) + err := reg.Watch(context.Background()) require.Error(t, err) assert.Contains(t, err.Error(), "settings watcher") } -// TestStore_Watch_SurvivesDirectoryRecreate pins the recreate contract: +// TestRegistry_Watch_SurvivesDirectoryRecreate pins the recreate contract: // fsnotify drops a watch whose directory is removed, so without the parent // watch + re-add a delete-and-recreate would leave later edits unwatched. -func TestStore_Watch_SurvivesDirectoryRecreate(t *testing.T) { +func TestRegistry_Watch_SurvivesDirectoryRecreate(t *testing.T) { t.Parallel() - s := newLoadedStore(t, map[string]string{ + reg := newLoadedRegistry(t, map[string]string{ FileConfig: configJSON(`{"query": {"default_max_rows": 100}}`), }) + s, _ := reg.For(tenant.Default) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - done := startWatch(ctx, t, s, 200) + done := startWatch(ctx, t, reg, 200) // Remove the whole directory: a rejected reload, snapshot survives. - require.NoError(t, os.RemoveAll(s.Dir())) + require.NoError(t, os.RemoveAll(reg.Dir())) time.Sleep(4 * watchDebounce) assert.Equal(t, 200, s.DefaultMaxRows(), "vanished directory must keep the previous snapshot") // Recreate it with new content: the watcher must pick it up again. files := validFiles() files[FileConfig] = configJSON(`{"query": {"default_max_rows": 300}}`) - require.NoError(t, os.Mkdir(s.Dir(), 0o750)) + require.NoError(t, os.Mkdir(reg.Dir(), 0o750)) for name, content := range files { - require.NoError(t, os.WriteFile(filepath.Join(s.Dir(), name), []byte(content), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), name), []byte(content), 0o600)) } assert.Eventually(t, func() bool { return s.DefaultMaxRows() == 300 }, 5*time.Second, 50*time.Millisecond, "recreated directory should be adopted") // And edits inside the recreated directory are watched again. - require.NoError(t, os.WriteFile(filepath.Join(s.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 400}}`)), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(reg.Dir(), FileConfig), []byte(configJSON(`{"query": {"default_max_rows": 400}}`)), 0o600)) assert.Eventually(t, func() bool { return s.DefaultMaxRows() == 400 }, 5*time.Second, 50*time.Millisecond, "edits after recreate should be watched") cancel() assert.NoError(t, <-done) } + +// A nested directory is never watched: a watcher would validate a tenant's +// folder halfway through being written, and with no previous-snapshot +// fallback that drops the tenant. Watch refuses it itself rather than trust +// every caller to know. +func TestRegistry_Watch_RefusesANestedDirectory(t *testing.T) { + t.Parallel() + reg, _ := Open(writeTree(t, map[string]map[string]string{"acme": validFiles()})) + require.NotNil(t, reg) + err := reg.Watch(t.Context()) + require.Error(t, err) + assert.Contains(t, err.Error(), "never watched") +} diff --git a/tests/e2e/sdk/admin.test.ts b/tests/e2e/sdk/admin.test.ts index 5591004f..54589b39 100644 --- a/tests/e2e/sdk/admin.test.ts +++ b/tests/e2e/sdk/admin.test.ts @@ -103,6 +103,17 @@ describe("Admin", () => { }); }); + describe("Settings reload", () => { + it("reloads one tenant with the tenant option", async () => { + const result = await wh.settings.reload({ tenant: "0" }); + expect(result.error).toBeNull(); + expect(result.data?.adopted).toBe(true); + + const unknown = await wh.settings.reload({ tenant: "acme" }); + expect(unknown.error?.status).toBe(404); + }); + }); + describe("Pipes", () => { const pipeName = `test_pipe_${Date.now()}`; const inPipe = `test_pipe_in_${Date.now()}`; @@ -132,6 +143,27 @@ describe("Admin", () => { expect(names).toContain(pipeName); }); + // The admin reads name their tenant in ?tenant= — the header is ignored + // under /v1/ops. This stack's settings directory is the single tenant 0. + it("addresses a tenant with the tenant option", async () => { + const named = await wh.pipes.list({ tenant: "0" }); + expect(named.error).toBeNull(); + expect(named.data).toEqual((await wh.pipes.list()).data); + + const one = await wh.pipes.get(pipeName, { tenant: "0" }); + expect(one.error).toBeNull(); + expect(one.data).toMatchObject({ name: pipeName }); + + const unknown = await wh.pipes.get(pipeName, { tenant: "acme" }); + expect(unknown.error?.status).toBe(404); + expect(unknown.error?.message).toContain("unknown tenant: acme"); + + const malformed = await wh.pipes.list({ tenant: "a.b" }); + expect(malformed.error?.status).toBe(400); + const empty = await wh.pipes.list({ tenant: "" }); + expect(empty.error?.status).toBe(400); + }); + it("executes a pipe with params", async () => { const result = await wh.pipe(pipeName, { limit: 3 }); expect(result.error).toBeNull();