From f5f531530762d27f3572de8bf9de51af0023d4e4 Mon Sep 17 00:00:00 2001 From: taitelee Date: Tue, 8 Sep 2026 15:08:00 -0400 Subject: [PATCH 1/9] feat(config): refuse unbound WH_* env and unusable data_dir at boot --- AGENTS.md | 2 +- CHANGELOG.md | 2 + cmd/wavehouse/main.go | 14 ++- docs/src/content/docs/architecture.md | 3 +- docs/src/content/docs/configuration.mdx | 5 +- docs/src/content/docs/settings-directory.mdx | 2 +- internal/config/check.go | 113 ++++++++++++++++++ internal/config/check_test.go | 114 +++++++++++++++++++ internal/config/config.go | 24 +++- internal/config/persistence.go | 11 +- 10 files changed, 276 insertions(+), 14 deletions(-) create mode 100644 internal/config/check.go create mode 100644 internal/config/check_test.go diff --git a/AGENTS.md b/AGENTS.md index b4bc36bb..911cd33e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ Sixteen internal packages under `internal/` (plus `internal/testutil/` for share - **`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) - **`chsql/`** — dependency-free ClickHouse SQL helpers shared by `query`/`policy` (avoids an import cycle): `QuoteIdent` (backtick-quote every identifier) + `BindUnsafe` (reject names with a literal `?`) -- **`config/`** — YAML + env var config loading (cleanenv) +- **`config/`** — YAML + env var config loading (cleanenv); strict on both sides (undeclared YAML key, unbound `WH_*` variable) and probes `data_dir` writability — boot is the validator, there is no dry run - **`dedupe/`** — `Deduplicator` interface → `Embedded` (Pebble), wrapped by `Managed` whose open/closed state follows the hot-reloadable `dedupe.enabled` in the settings directory's `config.json` - **`discovery/`** — `SchemaRegistry` that introspects ClickHouse `system.columns` (name/type/nullability plus `default_expression` and 1-based `position`) and `system.tables` (each table's `create_table_query`, kept in-process and never serialized — an external-engine table renders its wiring there unconditionally — endpoint, bucket/host, database, username, S3 access key id; ClickHouse masks the password as `[HIDDEN]` from ~23.9, so the exposure is the topology, not the secret), records the server version, + `Validate()` for ingest payloads + `CanonicalizeTimestamps()` rewriting top-level `DateTime`/`DateTime64` column values to the canonical RFC 3339 UTC wire form pre-publish (Key Design Decision #19) - **`ingest/`** — Ingest worker pipeline (`worker.go`: JetStream input → per-table batch INSERT with DLQ output). The pipeline is **insert-only**. The wire format `EventMessage` (`types.go`) carries `{table_name, scope, received_timestamp, data}` and nothing else; the worker accepts whatever table name the envelope carries (table existence was already checked by the HTTP ingest handler, which `404`s an unknown table before publish; the worker doesn't re-validate), then bulk-INSERTs. In the embedded-NATS deployment (the default), the server runs with `DontListen: true` (`internal/mq/embedded.go`), so the only Publishers reachable on the `ingest.>` subjects are in-process Go code — today, only the HTTP `/v1/ingest?table={table}` handler. Non-insert mutations (`DELETE`/`UPDATE`/`TRUNCATE`/…) must go through `POST /v1/ops/query` under the admin role (the same `RequireAdmin` gate as the rest of `/v1/ops/*`), so non-admin callers never reach the proxy. A request with no token (or an invalid one) resolves to the `default_role`, which in a production config is not the admin role (setting them equal is a loudly-warned dev-only setting), so it can't reach this endpoint. Plus `Sweeper` (Active Sweeper for NATS message lifecycle) + `EventMessage`/`BufferConsumerName` types (`types.go`) diff --git a/CHANGELOG.md b/CHANGELOG.md index 429f514a..bdbdf9f1 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 +- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (`internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds and the binary doesn't otherwise read (`WH_CONFIG`, `WH_LOG_LEVEL`) — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. Only the `WH_` prefix is checked, since the environment always carries unrelated names. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes that `data_dir` is a writable directory (or absent under a writable parent, so boot can create it) by creating and removing one temp file, so a root-owned bind mount refuses boot in the first second of the log with the UID-65532 remediation attached instead of after schema discovery; the hint string is now shared with `LogStorageInitError`. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. + - **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. - **Settings-directory hot reload — boot loading, three reload triggers, and the config-key migration** (`internal/settings/` (new: `store.go`, `watch.go`, + tests), `internal/api/settings.go` (new, + tests), `internal/api/{router,ingest,structured_query}.go`, `internal/discovery/discovery.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/settings-directory.mdx` (new — the hot-reloadable half of configuration gets its own page; `configuration.mdx` is boot config only); closes the loop [#500](https://github.com/Wave-RF/WaveHouse/pull/500) opened, tracked by [#48](https://github.com/Wave-RF/WaveHouse/issues/48)): the server now *consumes* the settings directory instead of only validating it. `settings.Store` owns the adopted snapshot: `settings.dir` / `WH_SETTINGS_DIR` is now **required**, boot validates and adopts the directory (missing or invalid refuses to start); a running instance then re-validates and re-adopts on any of three triggers — a **directory watch** (fsnotify on the directory, not the files, so atomic-writer replaces and Kubernetes ConfigMap symlink swaps aren't lost; bursts debounce into one reload), **`SIGHUP`**, and **`POST /v1/ops/settings/reload`** (admin-gated; returns `{"adopted", "findings"}`, `200` adopted / `422` rejected) — all funneling through one serialized reload path. A reload that fails validation keeps the previous good snapshot (an operator mid-edit degrades to a log line, never a broken server); warnings don't block adoption, matching `wavehouse validate`. The tenant tunables **migrate out of boot config** into the directory's `config.json`: `dedupe.id_field` / `dedupe.require_id` (now with the per-table overrides under `dedupe.tables` that [#222](https://github.com/Wave-RF/WaveHouse/issues/222) asked for, resolved per record through the table → global cascade in one atomic snapshot read, so a reload lands at a record boundary and never mixes documents within one record), `query.default_max_rows` and `query.timestamp_bucket_seconds` (read per query), `schema.refresh_interval` (re-read after each tick, so a change applies from the next cycle), `stream.keepalive_interval` / `stream.keepalive_buckets` (a reload calls the new `Heartbeater.Reconfigure`, which rebuilds the keepalive wheel in place with every live subscriber carried over and re-times the running ticker) and `stream.gap_window_minutes` (the sweeper re-reads it every sweep), `mq.max_bytes_gb` (an after-adopt hook updates the `WAVEHOUSE` and `WAVEHOUSE_DLQ` stream limits in place via `EmbeddedNATS.Resize` — shrinking below the buffered size backpressures until the worker drains, nothing is dropped), `dlq.enabled` with per-table overrides under `dlq.tables` (resolved by the ingest worker at the moment a poison row is isolated: on → park it on `WAVEHOUSE_DLQ` and ack; off → leave it unacked for redelivery, never dropped; the DLQ stream and `GET /v1/ops/dlq/stats` now always exist, so the switch is purely behavioral), the **ClickHouse wiring** (`clickhouse.addr` / `http_port` / `http_scheme` / `database` / `username` / `query_timeout`: the new `chconn.Manager` is the one `driver.Conn` every consumer holds and swaps the connection behind it on reload — unconditionally, since the adopted settings are the authority and reachability already surfaces through schema discovery and `/readyz`; the replaced one closes after a `query_timeout` grace; the ingest worker, raw-SQL proxy, and schema registry read the HTTP target, timeout, and database per call), the **auth verifier wiring** (`auth.jwks_url` / `auth.role_claim`: the new `auth.Authenticator` swaps a whole verifier — key source plus its pinned algorithm allowlist — atomically per reload, unconditionally, so an unreachable JWKS fails closed until it can be fetched; `auth.Middleware` is gone — `Authenticator` is the one constructor), and the CORS allowlist (`cors.allowed_origins`, resolved per request). The corresponding YAML/env keys are **removed**: `server.cors_allowed_origins`, `query.default_max_rows`, `schema.refresh_interval`, `dedupe.enabled`, `dedupe.id_field`, `dedupe.require_id`, `stream.keepalive_interval`, `stream.keepalive_buckets`, `mq.gap_window_minutes`, `cache.timestamp_bucket_seconds`, `mq.max_bytes_gb`, `dlq.enabled`, `clickhouse.addr`, `clickhouse.http_port`, `clickhouse.http_scheme`, `clickhouse.database`, `clickhouse.username`, `clickhouse.query_timeout`, `auth.jwks_url`, `auth.role_claim` (and `WH_SERVER_CORS_ALLOWED_ORIGINS`, `WH_QUERY_DEFAULT_MAX_ROWS`, `WH_SCHEMA_REFRESH_INTERVAL`, `WH_DEDUPE_ENABLED`, `WH_DEDUPE_ID_FIELD`, `WH_DEDUPE_REQUIRE_ID`, `WH_STREAM_KEEPALIVE_INTERVAL`, `WH_STREAM_KEEPALIVE_BUCKETS`, `WH_MQ_GAP_WINDOW_MINUTES`, `WH_CACHE_TIMESTAMP_BUCKET_SECONDS`, `WH_MQ_MAX_BYTES_GB`, `WH_DLQ_ENABLED`, `WH_CH_ADDR`, `WH_CH_HTTP_PORT`, `WH_CH_HTTP_SCHEME`, `WH_CH_DATABASE`, `WH_CH_USERNAME`, `WH_CH_QUERY_TIMEOUT`, `WH_AUTH_JWKS_URL`, `WH_AUTH_ROLE_CLAIM`); the secrets — `clickhouse.password`, `auth.jwt_secret`, `auth.operator_key` — stay boot config on purpose (never in a tracked JSON file; combined with the adopted wiring on every reconnect, rotating one is a restart), and boot config is now **strict**: `config.Load` re-reads the YAML against the struct's tags and refuses to start naming every undeclared key, so a `dlq:` or `clickhouse: addr:` left behind can't be read, ignored, and believed; the binary carries **no compiled defaults** — every `config.json` key is required (validation names each missing one), so the adopted snapshot is what the files say, and once adopted it outlives its files (a deleted file or vanished directory is just a rejected reload). Defaults live in one checked-in seed directory (`internal/settings/seed/`, `go:embed`ded): the new **`wavehouse bootstrap [dir]`** writes it (refusing a non-empty directory, the `initdb` contract; the directory resolves exactly as it does for `validate` — the argument, else `WH_SETTINGS_DIR`, usage error with neither — so the two commands are interchangeable on one path and a bare `bootstrap` inside the container images seeds `/app/settings`), the dev `config.yaml` points at a gitignored `./settings` that `make dev` seeds from it, and the e2e fixture ships a copy. The container images ship **no** settings directory: `WH_SETTINGS_DIR` is preset to `/app/settings`, the operator mounts a directory there (`standalone.yaml` bind-mounts the checked-in `deployments/compose/settings/`), and a missing mount refuses to boot rather than running on defaults nobody chose. `dedupe.enabled` moves too: the new `dedupe.Managed` wraps the Pebble store and a `Store.AfterAdopt` hook opens or closes it after every adoption, so flipping the switch is a reload, not a restart (seen ids persist across an off/on cycle; a failed open on reload is logged and ingest fails closed with `500` until the next reload, since the files asked for dedupe — at boot it still refuses to start; a record caught in the instant of the flip is published un-deduped and counted by `wavehouse_ingest_dedupe_disabled_total` rather than failed, and the hook is registered before the boot apply so a reload can never leave the settings and the store out of step). The watcher reloads once as soon as its watch exists, closing the gap between the boot read and the watch — an edit landing in between (a ConfigMap update during a rolling restart) is adopted, not silently missed. `dedupe.enabled` / `WH_DEDUPE_ENABLED` are removed from boot config alongside the other keys. What stays in boot config is only what cannot change under a running process — resource sizing (`data_dir`, `cache.l1_max_cost`), the listeners, the observability exporters — and the secrets. The compose stack now bind-mounts a checked-in `deployments/compose/settings/` (the seed with `clickhouse.addr` pointed at the `clickhouse` service) instead of a volume seeded with `bootstrap`, so the quickstart is `up -d` again; the e2e orchestrator copies the fixture settings per run and patches the testcontainer's ClickHouse ports into `config.json`, since that wiring no longer has an env override. Every after-adopt hook (dedupe, keepalive wheel) is registered before the reload triggers start, so the watcher's first reload can never be missed by a hook. Consumers take functions, not values (`IngestHandler.DedupeSettings`, the structured-query handler's `defaultMaxRows` / `bucketSecs func() int`, the ingest worker's `dlqEnabled func(table) bool`, the sweeper's `gapWindow func() time.Duration`, `corsMiddleware`'s origins getter, `SchemaRegistry`'s database and refresh-interval sources, the query handlers' timeout sources), so `internal/api` stays testable without materializing settings directories. The settings directory is also the **runtime authority for access control and named pipes** (`internal/settings/store.go`, `internal/policy/source.go` (new), `internal/pipes/pipes.go`, `internal/api/{policy,pipes,router}.go`, `internal/stream/hub.go`, `internal/auth/auth.go`, `cmd/wavehouse/main.go`, `Makefile`, `deployments/compose/settings/{policies,roles}.json`, `clients/ts/src/settings.ts` (new); closes [#229](https://github.com/Wave-RF/WaveHouse/issues/229), [#33](https://github.com/Wave-RF/WaveHouse/issues/33), [#461](https://github.com/Wave-RF/WaveHouse/issues/461), [#514](https://github.com/Wave-RF/WaveHouse/issues/514), [#460](https://github.com/Wave-RF/WaveHouse/issues/460), [#363](https://github.com/Wave-RF/WaveHouse/issues/363); advances [#48](https://github.com/Wave-RF/WaveHouse/issues/48) and [#214](https://github.com/Wave-RF/WaveHouse/issues/214)): `roles.json`, `policies.json`, and `pipes.json` are adopted with `config.json` as one snapshot and re-adopted on the same three triggers, and **files are the only write path** — standalone, the operator edits them on the host; on WaveHouse Cloud the control plane writes them — so there is no stored copy that can skip validation: every adoption runs the current rules (strict decode rejecting unknown and duplicate keys, the full policy validation including the claim-template grammar, pipe name/SQL/parameter-type rules, and the cross-file check that every role a grant or `allowed_roles` names is declared in `roles.json`), and a rejected edit keeps the previous good policy and pipes in effect. `policies.json` is one policy document (`{}` = no policy, adopted fail-closed with a warning); `pipes.json` carries full definitions (`allowed_roles`, `parameters`, `description`), so a file-defined pipe is no longer admin-only by construction. Consumers read the adopted snapshot per request through `policy.Source` (a `func() *policy.Policy`; `settings.Store.Policy` in production, `policy.Static(p)` in tests) and `pipes.Source` (`settings.Store`; `pipes.Static(q...)` in tests), so a reload applies to the very next request, including the SSE hub's per-event policy read. `GET /v1/ops/policy`, `POST /v1/ops/policy/validate`, `GET /v1/ops/pipes`, `GET /v1/ops/pipes/{name}`, and pipe execution are unchanged; the operator key still passes the `/v1/ops/*` gate under no policy, now as the break-glass that inspects the policy and triggers `POST /v1/ops/settings/reload` after `policies.json` is fixed. The SDK gains `wh.settings.reload()` (`POST /v1/ops/settings/reload`, returning `{ adopted, findings }`). The compose stack's trial `public` policy moves into the bind-mounted `deployments/compose/settings/policies.json` + `roles.json`, and `make dev` copies the same two files into its seeded `./settings` so a fresh dev server works tokenless. **Removed** — the write endpoints `PUT /v1/ops/policy`, `PUT /v1/ops/pipes/{name}`, and `DELETE /v1/ops/pipes/{name}`; the NATS KV buckets `WAVEHOUSE_POLICY` and `WAVEHOUSE_PIPES` and their KV Watch sync (`internal/policy/store.go`, the pipes KV store); the boot-config keys `policy.file_path` / `WH_POLICY_FILE_PATH` and `pipes.dir` / `WH_PIPES_DIR` (a leftover `policy:` or `pipes:` YAML block now refuses boot by name, like the other moved keys) and the `.sql`-directory pipes bootstrap; `deployments/compose/dev-policy.yaml`; the SDK methods `wh.policy.set`, `wh.pipes.set`, and `wh.pipes.delete`; and the test helpers `policy.NewMemoryStore`, `pipes.NewMemoryStore`, and `testutil/natsjs.go`. diff --git a/cmd/wavehouse/main.go b/cmd/wavehouse/main.go index 99ff57b4..9197271a 100644 --- a/cmd/wavehouse/main.go +++ b/cmd/wavehouse/main.go @@ -150,7 +150,7 @@ func run() int { logger.Info("starting WaveHouse", "version", Version, "build_time", BuildTime, "git_commit", GitCommit) cfgPath := "config.yaml" - if p := os.Getenv("WH_CONFIG"); p != "" { + if p := os.Getenv(config.EnvConfig); p != "" { cfgPath = p } @@ -160,6 +160,16 @@ func run() int { return 1 } + // data_dir must be usable before anything dials out: a root-owned bind + // mount is the classic container misconfiguration, and refusing here + // puts the UID-65532 hint in the first second of the log rather than + // after ClickHouse discovery. NATS and Pebble still fail loud on their + // own if the directory changes underneath us. + if err := config.CheckDataDir(cfg.DataDir); err != nil { + logger.Error("load config", "error", err) + return 1 + } + // Settings directory — the hot-reloadable half of configuration (dedupe, // dlq, query, schema, stream, cors — see settings.TenantConfig). Required: // config.Validate already @@ -195,7 +205,7 @@ func run() int { serviceName := "wavehouse" var level slog.Level - switch strings.ToUpper(strings.TrimSpace(os.Getenv("WH_LOG_LEVEL"))) { + switch strings.ToUpper(strings.TrimSpace(os.Getenv(config.EnvLogLevel))) { case "DEBUG": level = slog.LevelDebug case "WARN": diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index adfaacfb..81ba3cf6 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -107,7 +107,8 @@ The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https:/ ### `config/` — Configuration -- **config.go** — Loads *boot* configuration from a YAML file with environment variable overrides (using [cleanenv](https://github.com/ilyakaznacheev/cleanenv)); every key has a `WH_`-prefixed env var. Boot config is only what can't change under a running process — resource sizing, listeners, observability exporters, the settings-directory path, and the secrets (`clickhouse.password`, `auth.jwt_secret`, `auth.operator_key`). Everything tenant-tunable lives in the settings directory (`settings/`). The YAML is strict: `rejectUnknownKeys` refuses to boot naming every key the struct doesn't declare, so a tunable that moved to the settings directory can't be read, ignored, and believed. See [Configuration Reference](/configuration). +- **config.go** — Loads *boot* configuration from a YAML file with environment variable overrides (using [cleanenv](https://github.com/ilyakaznacheev/cleanenv)); every key has a `WH_`-prefixed env var. Boot config is only what can't change under a running process — resource sizing, listeners, observability exporters, the settings-directory path, and the secrets (`clickhouse.password`, `auth.jwt_secret`, `auth.operator_key`). Everything tenant-tunable lives in the settings directory (`settings/`). Both sources are strict: `rejectUnknownKeys` refuses to boot naming every YAML key the struct doesn't declare, and `rejectUnboundEnv` does the same for every `WH_*` environment variable no field binds, so a tunable that moved to the settings directory can't be read, ignored, and believed. Boot is the validator for this half — there is no dry-run command. See [Configuration Reference](/configuration). +- **check.go** — `UnboundEnv` walks the struct's `env` tags (plus the two process-level names, `WH_CONFIG` and `WH_LOG_LEVEL`) against the environment; `CheckDataDir` probes that `data_dir` is a writable directory, or creatable under a writable parent, by creating and removing one temp file — run by `main` right after `Load`, so a root-owned bind mount refuses boot before anything dials out, with the UID-65532 hint attached. ### `dedupe/` — Deduplication (Optional) diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx index 43256a31..4f677ac8 100644 --- a/docs/src/content/docs/configuration.mdx +++ b/docs/src/content/docs/configuration.mdx @@ -16,7 +16,10 @@ WaveHouse is configured via a YAML file with environment variable overrides. All 1. If a config file exists at the specified path (default: `config.yaml`), it is loaded first. 2. Environment variables override any values from the YAML file. 3. If no config file exists, all values are read from environment variables. Every key has a default except `settings.dir` (`WH_SETTINGS_DIR`), which must be set either way. -4. The YAML file is **strict**: a key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. +4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way; only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. +5. Before anything dials out, `data_dir` is probed: it must be a writable directory, or absent with a writable parent so it can be created. A root-owned bind mount therefore refuses to boot in the first second of the log, with the UID-65532 remediation attached, rather than after ClickHouse discovery. + +Boot is the validator for this half of configuration: there is no dry run, and a refused boot with the offending key, variable, or path named in the error is the loud signal. The hot-reloadable half has a dry run — `wavehouse validate` — because it is edited under a running server; boot config only ever takes effect through a restart, so the restart is where it is checked. Set `WH_CONFIG` to change the config file path: diff --git a/docs/src/content/docs/settings-directory.mdx b/docs/src/content/docs/settings-directory.mdx index 454538b2..c0c454cf 100644 --- a/docs/src/content/docs/settings-directory.mdx +++ b/docs/src/content/docs/settings-directory.mdx @@ -19,7 +19,7 @@ Where the directory lives is the one boot-config key involved: `settings.dir` / ## Loading and hot reload -The server validates and adopts the directory at boot — a missing or invalid directory refuses to start, so a typo, a missing mount, or an invalid policy surfaces immediately instead of silently denying every request (run `wavehouse validate` to reproduce the findings, `wavehouse bootstrap` to create a starter directory). +The server validates and adopts the directory at boot — a missing or invalid directory refuses to start, so a typo, a missing mount, or an invalid policy surfaces immediately instead of silently denying every request (run `wavehouse validate` to reproduce the findings, `wavehouse bootstrap` to create a starter directory). Boot config has no equivalent command and needs none: it only takes effect through a restart, and the restart refuses on an undeclared YAML key, an unbound `WH_*` variable, or an unusable `data_dir` — see [Configuration — Loading Order](/configuration#loading-order). A running instance re-validates and re-adopts on any of three triggers, all funneling through one serialized reload path: diff --git a/internal/config/check.go b/internal/config/check.go new file mode 100644 index 00000000..04233711 --- /dev/null +++ b/internal/config/check.go @@ -0,0 +1,113 @@ +package config + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "reflect" + "sort" + "strings" +) + +// envPrefix is what every WaveHouse environment variable starts with, and the +// only filter that makes an unbound-name check meaningful: the environment +// always carries names that aren't ours. +const envPrefix = "WH_" + +// processEnv lists the WH_* names the binary reads outside the Config struct +// (cmd/wavehouse), so UnboundEnv doesn't flag them. +var processEnv = []string{EnvConfig, EnvLogLevel} + +// rejectUnboundEnv is the environment half of rejectUnknownKeys: an error +// naming every WH_* variable in environ that nothing reads. +func rejectUnboundEnv(environ []string) error { + unbound := UnboundEnv(environ) + if len(unbound) == 0 { + return nil + } + return fmt.Errorf("unbound environment variable(s): %s — a typo, or a key that moved to the settings directory (%s); unset it, or rename it to a key the Config struct declares", strings.Join(unbound, ", "), EnvSettingsDir) +} + +// UnboundEnv returns, sorted, every WH_* name in environ (os.Environ() form, +// "KEY=value") that no Config field's env tag binds and the binary doesn't +// read otherwise. Such a name is almost always a typo or a key that moved to +// the settings directory — `WH_DEDUPE_ENABLED=true` left in a compose file +// would otherwise be set, ignored, and believed. Only the WH_ prefix is +// checked, since the environment is shared with whatever launched the process. +func UnboundEnv(environ []string) []string { + bound := map[string]bool{} + for _, name := range processEnv { + bound[name] = true + } + collectEnvTags(reflect.TypeFor[Config](), bound) + var out []string + for _, kv := range environ { + name, _, _ := strings.Cut(kv, "=") + if strings.HasPrefix(name, envPrefix) && !bound[name] { + out = append(out, name) + } + } + sort.Strings(out) + return out +} + +// collectEnvTags records every env tag in t, recursing into nested structs. +func collectEnvTags(t reflect.Type, into map[string]bool) { + for i := range t.NumField() { + f := t.Field(i) + if tag := f.Tag.Get("env"); tag != "" { + into[tag] = true + } + if f.Type.Kind() == reflect.Struct { + collectEnvTags(f.Type, into) + } + } +} + +// CheckDataDir reports whether boot can use dir as data_dir: a directory that +// exists must be writable, and one that doesn't (boot creates it — a first +// run, or a missing mount that WarnIfFreshDataDir calls out) must have a +// writable nearest existing ancestor so that creation can succeed. Run before +// anything dials out, so a root-owned bind mount refuses boot in the first +// second instead of after ClickHouse discovery, with the UID-65532 hint +// attached. Writability is probed by creating and removing one temp file: +// the only portable test that exercises the mount's ownership and mode. +func CheckDataDir(dir string) error { + info, err := os.Stat(dir) + switch { + case errors.Is(err, fs.ErrNotExist): + // Boot creates it; check the ancestor below. + case err != nil: + return fmt.Errorf("data_dir %s: %w", dir, err) + case !info.IsDir(): + return fmt.Errorf("data_dir %s is not a directory", dir) + } + + target := dir + for { + if _, err := os.Stat(target); err == nil { + break + } + parent := filepath.Dir(target) + if parent == target { + break + } + target = parent + } + f, err := os.CreateTemp(target, ".wavehouse-datadir-probe-*") + if err != nil { + msg := fmt.Sprintf("data_dir %s is not writable", dir) + if target != dir { + msg = fmt.Sprintf("data_dir %s does not exist and %s is not writable, so it cannot be created", dir, target) + } + if errors.Is(err, fs.ErrPermission) { + msg += "; " + permissionHint + } + return fmt.Errorf("%s: %w", msg, err) + } + name := f.Name() + _ = f.Close() + return os.Remove(name) +} diff --git a/internal/config/check_test.go b/internal/config/check_test.go new file mode 100644 index 00000000..3fa77e77 --- /dev/null +++ b/internal/config/check_test.go @@ -0,0 +1,114 @@ +package config + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestUnboundEnv(t *testing.T) { + t.Parallel() + environ := []string{ + "WH_SERVER_PORT=9090", // struct tag + "WH_OTEL_LOGS_SAMPLE_RATE=0.5", // nested struct tag + "WH_SETTINGS_DIR=./settings", // struct tag + "WH_CONFIG=/etc/wh.yaml", // process-level, read by main + "WH_LOG_LEVEL=debug", // process-level, read by main + "WH_DEDUPE_ENABLED=true", // moved to the settings directory + "WH_SERVER_PROT=1", // typo + "WH_=x", // bare prefix + "PATH=/usr/bin", // not ours + "WHATEVER=1", // prefix is WH_, not WH + "NOT_WH_FOO=1", + } + assert.Equal(t, []string{"WH_", "WH_DEDUPE_ENABLED", "WH_SERVER_PROT"}, UnboundEnv(environ)) + assert.Empty(t, UnboundEnv(nil)) +} + +// Every env tag on the struct must count as bound — a field added without +// this walk picking it up would refuse every boot that sets it. +func TestUnboundEnv_EveryStructTagIsBound(t *testing.T) { + t.Parallel() + var environ []string + for name := range collectAllEnvTags(t) { + environ = append(environ, name+"=1") + } + assert.Empty(t, UnboundEnv(environ)) +} + +func collectAllEnvTags(t *testing.T) map[string]bool { + t.Helper() + tags := map[string]bool{} + collectEnvTags(reflect.TypeFor[Config](), tags) + require.Contains(t, tags, EnvSettingsDir) + require.Contains(t, tags, "WH_OTEL_TRACES_SAMPLE_RATE") + return tags +} + +func TestLoad_RejectsUnboundEnv(t *testing.T) { + t.Setenv("WH_DEDUPE_ENABLED", "true") + t.Setenv("WH_CH_ADDR", "localhost:9000") + _, err := Load("nonexistent.yaml") + require.Error(t, err) + assert.Contains(t, err.Error(), "unbound environment variable(s): WH_CH_ADDR, WH_DEDUPE_ENABLED") + assert.Contains(t, err.Error(), EnvSettingsDir) +} + +func TestCheckDataDir(t *testing.T) { + t.Parallel() + t.Run("existing writable directory", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + require.NoError(t, CheckDataDir(dir)) + entries, err := os.ReadDir(dir) + require.NoError(t, err) + assert.Empty(t, entries, "the probe file must be removed") + }) + + t.Run("missing directory under a writable parent", func(t *testing.T) { + t.Parallel() + dir := filepath.Join(t.TempDir(), "data", "nested") + require.NoError(t, CheckDataDir(dir)) + _, err := os.Stat(dir) + assert.True(t, os.IsNotExist(err), "the check must not create the directory") + }) + + t.Run("a file is not a directory", func(t *testing.T) { + t.Parallel() + path := filepath.Join(t.TempDir(), "data") + require.NoError(t, os.WriteFile(path, []byte("x"), 0o600)) + err := CheckDataDir(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "is not a directory") + }) + + t.Run("unwritable directory carries the UID hint", func(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("root writes anywhere") + } + dir := t.TempDir() + require.NoError(t, os.Chmod(dir, 0o500)) //nolint:gosec // G302: an unwritable directory is the point of the test + t.Cleanup(func() { _ = os.Chmod(dir, 0o700) }) //nolint:gosec // G302: restore so TempDir cleanup works + err := CheckDataDir(dir) + require.Error(t, err) + assert.Contains(t, err.Error(), "is not writable") + assert.Contains(t, err.Error(), "65532") + }) + + t.Run("missing directory under an unwritable parent", func(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("root writes anywhere") + } + parent := t.TempDir() + require.NoError(t, os.Chmod(parent, 0o500)) //nolint:gosec // G302: an unwritable directory is the point of the test + t.Cleanup(func() { _ = os.Chmod(parent, 0o700) }) //nolint:gosec // G302: restore so TempDir cleanup works + err := CheckDataDir(filepath.Join(parent, "data")) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot be created") + assert.Contains(t, err.Error(), parent) + }) +} diff --git a/internal/config/config.go b/internal/config/config.go index 8d90dbb2..faf598c9 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -32,6 +32,16 @@ type Config struct { // so a config test pins the tag to this constant to prevent drift. const EnvSettingsDir = "WH_SETTINGS_DIR" +// EnvConfig names the boot-config file (default: ./config.yaml) and +// EnvLogLevel the minimum log level. Neither is a Config field — the first +// locates the file the struct is read from, the second is read by main +// directly — so they are declared here as the process-level names UnboundEnv +// must not flag. +const ( + EnvConfig = "WH_CONFIG" + EnvLogLevel = "WH_LOG_LEVEL" +) + // Settings locates the hot-reloadable settings directory — the four JSON // documents (roles.json, policies.json, pipes.json, config.json) validated by // internal/settings. Boot-tier by necessity: it's the pointer the reload @@ -207,11 +217,17 @@ func (c *Config) Validate() error { } // Load reads config from a YAML file (if it exists) with env var overrides. -// The YAML is strict: a key the Config struct doesn't declare is an error -// naming every such key, so a tunable that moved to the settings directory -// (or a typo) can't be silently ignored. Environment variables can't be -// checked the same way — the environment always carries unrelated names. +// Both sources are strict: a YAML key the Config struct doesn't declare, or +// a WH_* environment variable no field binds, is an error naming every +// offender, so a tunable that moved to the settings directory (or a typo) +// can't be silently ignored. The environment always carries unrelated +// names, so only the WH_ prefix is checked there. Boot is the validator for +// this half of configuration — there is no dry run; a refused boot is the +// loud signal. func Load(path string) (*Config, error) { + if err := rejectUnboundEnv(os.Environ()); err != nil { + return nil, err + } var cfg Config if _, err := os.Stat(path); err == nil { if err := cleanenv.ReadConfig(path, &cfg); err != nil { diff --git a/internal/config/persistence.go b/internal/config/persistence.go index f4e7626c..5ac4a5e5 100644 --- a/internal/config/persistence.go +++ b/internal/config/persistence.go @@ -7,6 +7,12 @@ import ( "os" ) +// permissionHint is the remediation for the dominant permission-denied +// signature: a Docker bind mount whose host directory is owned by root +// rather than the distroless `nonroot` user. Shared by the boot-time storage +// error and the `wavehouse validate-config` data_dir probe. +const permissionHint = "if running in a container with a host bind mount, the host directory must be owned by UID 65532 (the `nonroot` user in the distroless image). Try `sudo chown -R 65532:65532 /your/host/path`. Named volumes inherit ownership automatically and don't need this." + // LogStorageInitError emits an error log for a storage-init failure, with a // UID-65532 hint when the failure looks like a permission denial — the // dominant signature of a Docker bind mount whose host directory is owned @@ -18,10 +24,7 @@ import ( func LogStorageInitError(logger *slog.Logger, kind, path string, err error) { fields := []any{"error", err, "path", path} if errors.Is(err, os.ErrPermission) { - fields = append(fields, - "hint", - "if running in a container with a host bind mount, the host directory must be owned by UID 65532 (the `nonroot` user in the distroless image). Try `sudo chown -R 65532:65532 /your/host/path`. Named volumes inherit ownership automatically and don't need this.", - ) + fields = append(fields, "hint", permissionHint) } logger.Error(kind+" init failed", fields...) } From e187f010ceb692df9ef0a493bef0ee2c33983851 Mon Sep 17 00:00:00 2001 From: taitelee Date: Tue, 8 Sep 2026 15:38:55 -0400 Subject: [PATCH 2/9] fix(config): address review on boot data_dir probe --- CHANGELOG.md | 2 +- cmd/wavehouse/main.go | 8 ++++---- docs/src/content/docs/architecture.md | 2 +- docs/src/content/docs/configuration.mdx | 2 +- internal/config/check.go | 15 +++++++++++---- internal/config/check_test.go | 12 ++++++++++++ internal/config/persistence.go | 2 +- 7 files changed, 31 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdbdf9f1..836c84fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Added -- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (`internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds and the binary doesn't otherwise read (`WH_CONFIG`, `WH_LOG_LEVEL`) — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. Only the `WH_` prefix is checked, since the environment always carries unrelated names. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes that `data_dir` is a writable directory (or absent under a writable parent, so boot can create it) by creating and removing one temp file, so a root-owned bind mount refuses boot in the first second of the log with the UID-65532 remediation attached instead of after schema discovery; the hint string is now shared with `LogStorageInitError`. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. +- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (`internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds and the binary doesn't otherwise read (`WH_CONFIG`, `WH_LOG_LEVEL`) — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. Only the `WH_` prefix is checked, since the environment always carries unrelated names. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes that `data_dir` is a writable directory (or absent under a writable parent, so boot can create it) by creating and removing one temp file, so a `data_dir` the process cannot write to refuses boot before schema discovery rather than after it; a permission denial carries the UID-65532 remediation (a bind mount owned by root is the typical cause), and that hint string is now shared with `LogStorageInitError`. An empty or blank `data_dir` — reachable through `WH_DATA_DIR=` — is refused by the probe outright, since the ancestor walk would otherwise fall back to the working directory and NATS and Pebble state would land under it. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. - **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. diff --git a/cmd/wavehouse/main.go b/cmd/wavehouse/main.go index 9197271a..20afec50 100644 --- a/cmd/wavehouse/main.go +++ b/cmd/wavehouse/main.go @@ -160,13 +160,13 @@ func run() int { return 1 } - // data_dir must be usable before anything dials out: a root-owned bind - // mount is the classic container misconfiguration, and refusing here - // puts the UID-65532 hint in the first second of the log rather than + // data_dir must be writable before anything dials out, so the refusal + // (and, for the typical cause — a bind mount owned by root rather than + // UID 65532 — the remediation) lands at the top of the log rather than // after ClickHouse discovery. NATS and Pebble still fail loud on their // own if the directory changes underneath us. if err := config.CheckDataDir(cfg.DataDir); err != nil { - logger.Error("load config", "error", err) + logger.Error("check data_dir", "error", err) return 1 } diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index 81ba3cf6..efd27f9e 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -108,7 +108,7 @@ The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https:/ ### `config/` — Configuration - **config.go** — Loads *boot* configuration from a YAML file with environment variable overrides (using [cleanenv](https://github.com/ilyakaznacheev/cleanenv)); every key has a `WH_`-prefixed env var. Boot config is only what can't change under a running process — resource sizing, listeners, observability exporters, the settings-directory path, and the secrets (`clickhouse.password`, `auth.jwt_secret`, `auth.operator_key`). Everything tenant-tunable lives in the settings directory (`settings/`). Both sources are strict: `rejectUnknownKeys` refuses to boot naming every YAML key the struct doesn't declare, and `rejectUnboundEnv` does the same for every `WH_*` environment variable no field binds, so a tunable that moved to the settings directory can't be read, ignored, and believed. Boot is the validator for this half — there is no dry-run command. See [Configuration Reference](/configuration). -- **check.go** — `UnboundEnv` walks the struct's `env` tags (plus the two process-level names, `WH_CONFIG` and `WH_LOG_LEVEL`) against the environment; `CheckDataDir` probes that `data_dir` is a writable directory, or creatable under a writable parent, by creating and removing one temp file — run by `main` right after `Load`, so a root-owned bind mount refuses boot before anything dials out, with the UID-65532 hint attached. +- **check.go** — `UnboundEnv` walks the struct's `env` tags (plus the two process-level names, `WH_CONFIG` and `WH_LOG_LEVEL`) against the environment; `CheckDataDir` probes that `data_dir` is a writable directory, or creatable under a writable parent, by creating and removing one temp file — run by `main` right after `Load`, so a `data_dir` the process cannot write to refuses boot before anything dials out; a permission denial carries the UID-65532 hint, since a bind mount owned by root is the typical cause. An empty `data_dir` (reachable through `WH_DATA_DIR=`) is refused outright rather than probing the working directory. ### `dedupe/` — Deduplication (Optional) diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx index 4f677ac8..9757e95a 100644 --- a/docs/src/content/docs/configuration.mdx +++ b/docs/src/content/docs/configuration.mdx @@ -17,7 +17,7 @@ WaveHouse is configured via a YAML file with environment variable overrides. All 2. Environment variables override any values from the YAML file. 3. If no config file exists, all values are read from environment variables. Every key has a default except `settings.dir` (`WH_SETTINGS_DIR`), which must be set either way. 4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way; only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. -5. Before anything dials out, `data_dir` is probed: it must be a writable directory, or absent with a writable parent so it can be created. A root-owned bind mount therefore refuses to boot in the first second of the log, with the UID-65532 remediation attached, rather than after ClickHouse discovery. +5. Before anything dials out, `data_dir` is probed: it must be a writable directory, or absent with a writable parent so it can be created. A `data_dir` the process cannot write to refuses to boot before ClickHouse discovery rather than after it, and a permission denial carries the UID-65532 remediation, since a bind mount owned by root is the typical cause. An empty `data_dir` is refused outright. Boot is the validator for this half of configuration: there is no dry run, and a refused boot with the offending key, variable, or path named in the error is the loud signal. The hot-reloadable half has a dry run — `wavehouse validate` — because it is edited under a running server; boot config only ever takes effect through a restart, so the restart is where it is checked. diff --git a/internal/config/check.go b/internal/config/check.go index 04233711..b571d451 100644 --- a/internal/config/check.go +++ b/internal/config/check.go @@ -70,11 +70,18 @@ func collectEnvTags(t reflect.Type, into map[string]bool) { // exists must be writable, and one that doesn't (boot creates it — a first // run, or a missing mount that WarnIfFreshDataDir calls out) must have a // writable nearest existing ancestor so that creation can succeed. Run before -// anything dials out, so a root-owned bind mount refuses boot in the first -// second instead of after ClickHouse discovery, with the UID-65532 hint -// attached. Writability is probed by creating and removing one temp file: -// the only portable test that exercises the mount's ownership and mode. +// anything dials out, so a data_dir the process cannot write to refuses boot +// before ClickHouse discovery rather than after it; a permission denial +// carries the UID-65532 hint, since a bind mount owned by root is the +// typical cause. Writability is probed by creating and removing one temp +// file: the only portable test that exercises the mount's ownership and +// mode. A blank dir — reachable through `WH_DATA_DIR=` — is refused +// outright: the ancestor walk would otherwise probe the working directory +// and pass, and NATS and Pebble state would land under it. func CheckDataDir(dir string) error { + if strings.TrimSpace(dir) == "" { + return errors.New("data_dir (WH_DATA_DIR) is required: an empty value would scatter NATS and Pebble state under the working directory") + } info, err := os.Stat(dir) switch { case errors.Is(err, fs.ErrNotExist): diff --git a/internal/config/check_test.go b/internal/config/check_test.go index 3fa77e77..9e28df3c 100644 --- a/internal/config/check_test.go +++ b/internal/config/check_test.go @@ -112,3 +112,15 @@ func TestCheckDataDir(t *testing.T) { assert.Contains(t, err.Error(), parent) }) } + +// A blank data_dir must not pass by probing the working directory — an empty +// WH_DATA_DIR reaches Load as "" and would otherwise scatter NATS and Pebble +// state under the cwd. +func TestCheckDataDir_BlankIsRefused(t *testing.T) { + t.Parallel() + for _, dir := range []string{"", " "} { + err := CheckDataDir(dir) + require.Error(t, err, "%q", dir) + assert.Contains(t, err.Error(), "data_dir (WH_DATA_DIR) is required") + } +} diff --git a/internal/config/persistence.go b/internal/config/persistence.go index 5ac4a5e5..0ee1b7b7 100644 --- a/internal/config/persistence.go +++ b/internal/config/persistence.go @@ -10,7 +10,7 @@ import ( // permissionHint is the remediation for the dominant permission-denied // signature: a Docker bind mount whose host directory is owned by root // rather than the distroless `nonroot` user. Shared by the boot-time storage -// error and the `wavehouse validate-config` data_dir probe. +// error and the data_dir probe (CheckDataDir) that runs right after Load. const permissionHint = "if running in a container with a host bind mount, the host directory must be owned by UID 65532 (the `nonroot` user in the distroless image). Try `sudo chown -R 65532:65532 /your/host/path`. Named volumes inherit ownership automatically and don't need this." // LogStorageInitError emits an error log for a storage-init failure, with a From bf871919d0330d3c27a3cec8845a355a5e1a6680 Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 9 Sep 2026 18:35:10 -0400 Subject: [PATCH 3/9] fix(config): name Kubernetes service links, mirror cleanenv tag grammar --- CHANGELOG.md | 2 +- docs/src/content/docs/configuration.mdx | 2 +- docs/src/content/docs/deployment.md | 11 +++---- internal/config/check.go | 14 ++++++--- internal/config/check_test.go | 39 +++++++++++++++++-------- 5 files changed, 45 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 836c84fc..a550bded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Added -- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (`internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds and the binary doesn't otherwise read (`WH_CONFIG`, `WH_LOG_LEVEL`) — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. Only the `WH_` prefix is checked, since the environment always carries unrelated names. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes that `data_dir` is a writable directory (or absent under a writable parent, so boot can create it) by creating and removing one temp file, so a `data_dir` the process cannot write to refuses boot before schema discovery rather than after it; a permission denial carries the UID-65532 remediation (a bind mount owned by root is the typical cause), and that hint string is now shared with `LogStorageInitError`. An empty or blank `data_dir` — reachable through `WH_DATA_DIR=` — is refused by the probe outright, since the ancestor walk would otherwise fall back to the working directory and NATS and Pebble state would land under it. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. +- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (`internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds and the binary doesn't otherwise read (`WH_CONFIG`, `WH_LOG_LEVEL`) — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. Only the `WH_` prefix is checked, since the environment always carries unrelated names; the one outside source that shares it — Kubernetes service-link variables for a Service named `wh` or `wh-*` — is named in the error with the `enableServiceLinks: false` remediation. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes that `data_dir` is a writable directory (or absent under a writable parent, so boot can create it) by creating and removing one temp file, so a `data_dir` the process cannot write to refuses boot before schema discovery rather than after it; a permission denial carries the UID-65532 remediation (a bind mount owned by root is the typical cause), and that hint string is now shared with `LogStorageInitError`. An empty or blank `data_dir` — reachable through `WH_DATA_DIR=` — is refused by the probe outright, since the ancestor walk would otherwise fall back to the working directory and NATS and Pebble state would land under it. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. - **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. diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx index 9757e95a..65e0e582 100644 --- a/docs/src/content/docs/configuration.mdx +++ b/docs/src/content/docs/configuration.mdx @@ -16,7 +16,7 @@ WaveHouse is configured via a YAML file with environment variable overrides. All 1. If a config file exists at the specified path (default: `config.yaml`), it is loaded first. 2. Environment variables override any values from the YAML file. 3. If no config file exists, all values are read from environment variables. Every key has a default except `settings.dir` (`WH_SETTINGS_DIR`), which must be set either way. -4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way; only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. +4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way; only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix: Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod for each Service that existed before it, so a Service named `wh` or `wh-*` produces `WH_SERVICE_HOST`, `WH_PORT`, … and the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else; the error says so. 5. Before anything dials out, `data_dir` is probed: it must be a writable directory, or absent with a writable parent so it can be created. A `data_dir` the process cannot write to refuses to boot before ClickHouse discovery rather than after it, and a permission denial carries the UID-65532 remediation, since a bind mount owned by root is the typical cause. An empty `data_dir` is refused outright. Boot is the validator for this half of configuration: there is no dry run, and a refused boot with the offending key, variable, or path named in the error is the loud signal. The hot-reloadable half has a dry run — `wavehouse validate` — because it is edited under a running server; boot config only ever takes effect through a restart, so the restart is where it is checked. diff --git a/docs/src/content/docs/deployment.md b/docs/src/content/docs/deployment.md index 67172ee2..d05beeed 100644 --- a/docs/src/content/docs/deployment.md +++ b/docs/src/content/docs/deployment.md @@ -186,7 +186,7 @@ On a first-ever run this is expected. On every subsequent run it should be silen ### Distroless Permission Traps (named volume vs bind mount) -WaveHouse images run as the distroless `nonroot` user (UID 65532). Bind mounts and named volumes interact with this differently, and the distroless image has no shell to `chown` things at runtime — so getting the host side wrong produces a hard-to-read permission error from NATS or Pebble at startup. +WaveHouse images run as the distroless `nonroot` user (UID 65532). Bind mounts and named volumes interact with this differently, and the distroless image has no shell to `chown` things at runtime — so getting the host side wrong refuses boot in the first lines of the log with a named `data_dir` error and the `chown` remediation attached. **Named volumes** (the recommended pattern): @@ -206,12 +206,13 @@ volumes: - /srv/wavehouse:/app/data ``` -Bind mounts do **not** copy-up — Docker exposes the host directory as-is, and the image's pre-created dir is masked entirely. If `/srv/wavehouse` is owned by `root:root` on the host (the default for a freshly `mkdir`'d directory), the binary fails at startup with a permission error from NATS: +Bind mounts do **not** copy-up — Docker exposes the host directory as-is, and the image's pre-created dir is masked entirely. If `/srv/wavehouse` is owned by `root:root` on the host (the default for a freshly `mkdir`'d directory), the binary refuses to start before it touches the settings directory or ClickHouse — `data_dir` is probed for writability right after the config loads: ```text wrap=false -ERROR mq init failed error="..." path=/app/data/nats - hint="if running in a container with a host bind mount, the host - directory must be owned by UID 65532..." +ERROR check data_dir error="data_dir /app/data is not writable; if running in a + container with a host bind mount, the host directory must be owned by + UID 65532 (the `nonroot` user in the distroless image). Try + `sudo chown -R 65532:65532 /your/host/path`. ...: permission denied" ``` The fix is one host-side command before first start: diff --git a/internal/config/check.go b/internal/config/check.go index b571d451..9eb92461 100644 --- a/internal/config/check.go +++ b/internal/config/check.go @@ -27,7 +27,7 @@ func rejectUnboundEnv(environ []string) error { if len(unbound) == 0 { return nil } - return fmt.Errorf("unbound environment variable(s): %s — a typo, or a key that moved to the settings directory (%s); unset it, or rename it to a key the Config struct declares", strings.Join(unbound, ", "), EnvSettingsDir) + return fmt.Errorf("unbound environment variable(s): %s — a typo, or a key that moved to the settings directory (%s); unset it, or rename it to a key the Config struct declares. On Kubernetes, a Service named wh or wh-* injects WH_SERVICE_HOST, WH_PORT, … into every pod started after it: set enableServiceLinks: false on the pod spec, or rename the Service", strings.Join(unbound, ", "), EnvSettingsDir) } // UnboundEnv returns, sorted, every WH_* name in environ (os.Environ() form, @@ -53,12 +53,18 @@ func UnboundEnv(environ []string) []string { return out } -// collectEnvTags records every env tag in t, recursing into nested structs. +// collectEnvTags records every name in every env tag in t, recursing into +// nested structs. It mirrors cleanenv's tag grammar: an env tag is a +// comma-separated list of names. cleanenv's `env-prefix` tag is NOT +// mirrored — Config must not use it, or a prefixed variable that cleanenv +// reads would be refused here; TestUnboundEnv_MatchesCleanenv pins that. func collectEnvTags(t reflect.Type, into map[string]bool) { for i := range t.NumField() { f := t.Field(i) - if tag := f.Tag.Get("env"); tag != "" { - into[tag] = true + for _, name := range strings.Split(f.Tag.Get("env"), ",") { + if name != "" { + into[name] = true + } } if f.Type.Kind() == reflect.Struct { collectEnvTags(f.Type, into) diff --git a/internal/config/check_test.go b/internal/config/check_test.go index 9e28df3c..904e2692 100644 --- a/internal/config/check_test.go +++ b/internal/config/check_test.go @@ -4,8 +4,10 @@ import ( "os" "path/filepath" "reflect" + "regexp" "testing" + "github.com/ilyakaznacheev/cleanenv" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -29,24 +31,37 @@ func TestUnboundEnv(t *testing.T) { assert.Empty(t, UnboundEnv(nil)) } -// Every env tag on the struct must count as bound — a field added without -// this walk picking it up would refuse every boot that sets it. -func TestUnboundEnv_EveryStructTagIsBound(t *testing.T) { +// Every variable cleanenv itself reads must count as bound — a name the +// loader honors but this walk misses would refuse every boot that sets it. +// The oracle is cleanenv's own metadata (GetDescription lists each bound +// name), not collectEnvTags, so a divergence in tag grammar (comma lists, +// env-prefix) shows up here rather than in production. +func TestUnboundEnv_MatchesCleanenv(t *testing.T) { t.Parallel() + desc, err := cleanenv.GetDescription(&Config{}, nil) + require.NoError(t, err) + names := regexp.MustCompile(`(?m)^ (WH_[A-Z0-9_]+) `).FindAllStringSubmatch(desc, -1) + require.NotEmpty(t, names) var environ []string - for name := range collectAllEnvTags(t) { - environ = append(environ, name+"=1") + seen := map[string]bool{} + for _, m := range names { + environ = append(environ, m[1]+"=1") + seen[m[1]] = true } + require.True(t, seen[EnvSettingsDir]) + require.True(t, seen["WH_OTEL_TRACES_SAMPLE_RATE"]) assert.Empty(t, UnboundEnv(environ)) } -func collectAllEnvTags(t *testing.T) map[string]bool { - t.Helper() - tags := map[string]bool{} - collectEnvTags(reflect.TypeFor[Config](), tags) - require.Contains(t, tags, EnvSettingsDir) - require.Contains(t, tags, "WH_OTEL_TRACES_SAMPLE_RATE") - return tags +func TestCollectEnvTags_CommaList(t *testing.T) { + t.Parallel() + type multi struct { + A string `env:"WH_ONE,WH_TWO"` + B string `env:""` + } + got := map[string]bool{} + collectEnvTags(reflect.TypeFor[multi](), got) + assert.Equal(t, map[string]bool{"WH_ONE": true, "WH_TWO": true}, got) } func TestLoad_RejectsUnboundEnv(t *testing.T) { From 9b5da0d857708bb59f9f3b515fb2e9fcfbb5625d Mon Sep 17 00:00:00 2001 From: taitelee Date: Wed, 9 Sep 2026 18:56:23 -0400 Subject: [PATCH 4/9] fix(config): catch dangling data_dir symlinks, state writability as the rule --- CHANGELOG.md | 2 +- docs/src/content/docs/configuration.mdx | 2 +- docs/src/content/docs/deployment.md | 6 ++--- internal/config/check.go | 9 ++++++- internal/config/check_test.go | 33 +++++++++++++++++++++++++ internal/config/persistence.go | 2 +- 6 files changed, 47 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a550bded..67de93c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Added -- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (`internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds and the binary doesn't otherwise read (`WH_CONFIG`, `WH_LOG_LEVEL`) — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. Only the `WH_` prefix is checked, since the environment always carries unrelated names; the one outside source that shares it — Kubernetes service-link variables for a Service named `wh` or `wh-*` — is named in the error with the `enableServiceLinks: false` remediation. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes that `data_dir` is a writable directory (or absent under a writable parent, so boot can create it) by creating and removing one temp file, so a `data_dir` the process cannot write to refuses boot before schema discovery rather than after it; a permission denial carries the UID-65532 remediation (a bind mount owned by root is the typical cause), and that hint string is now shared with `LogStorageInitError`. An empty or blank `data_dir` — reachable through `WH_DATA_DIR=` — is refused by the probe outright, since the ancestor walk would otherwise fall back to the working directory and NATS and Pebble state would land under it. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. +- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (`internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds — the two variables read outside the struct, `WH_CONFIG` and `WH_LOG_LEVEL`, are exempt — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. Only the `WH_` prefix is checked, since the environment always carries unrelated names; the one outside source that shares it — Kubernetes service-link variables for a Service named `wh` or `wh-*` — is named in the error with the `enableServiceLinks: false` remediation. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes that `data_dir` is a writable directory (or absent under a writable parent, so boot can create it) by creating and removing one temp file, so a `data_dir` the process cannot write to refuses boot before schema discovery rather than after it; a permission denial carries the UID-65532 remediation (a bind mount owned by root is the typical cause), and that hint string is now shared with `LogStorageInitError`. An empty or blank `data_dir` — reachable through `WH_DATA_DIR=` — is refused by the probe outright, since the ancestor walk would otherwise fall back to the working directory and NATS and Pebble state would land under it. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. - **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. diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx index 65e0e582..841062f9 100644 --- a/docs/src/content/docs/configuration.mdx +++ b/docs/src/content/docs/configuration.mdx @@ -16,7 +16,7 @@ WaveHouse is configured via a YAML file with environment variable overrides. All 1. If a config file exists at the specified path (default: `config.yaml`), it is loaded first. 2. Environment variables override any values from the YAML file. 3. If no config file exists, all values are read from environment variables. Every key has a default except `settings.dir` (`WH_SETTINGS_DIR`), which must be set either way. -4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way; only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix: Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod for each Service that existed before it, so a Service named `wh` or `wh-*` produces `WH_SERVICE_HOST`, `WH_PORT`, … and the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else; the error says so. +4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way; only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix. Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod, for each Service that existed before the pod started. A Service named `wh` or `wh-*` therefore produces `WH_SERVICE_HOST`, `WH_PORT`, and more, and the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else. The error says so. 5. Before anything dials out, `data_dir` is probed: it must be a writable directory, or absent with a writable parent so it can be created. A `data_dir` the process cannot write to refuses to boot before ClickHouse discovery rather than after it, and a permission denial carries the UID-65532 remediation, since a bind mount owned by root is the typical cause. An empty `data_dir` is refused outright. Boot is the validator for this half of configuration: there is no dry run, and a refused boot with the offending key, variable, or path named in the error is the loud signal. The hot-reloadable half has a dry run — `wavehouse validate` — because it is edited under a running server; boot config only ever takes effect through a restart, so the restart is where it is checked. diff --git a/docs/src/content/docs/deployment.md b/docs/src/content/docs/deployment.md index d05beeed..f1e08f4a 100644 --- a/docs/src/content/docs/deployment.md +++ b/docs/src/content/docs/deployment.md @@ -206,12 +206,12 @@ volumes: - /srv/wavehouse:/app/data ``` -Bind mounts do **not** copy-up — Docker exposes the host directory as-is, and the image's pre-created dir is masked entirely. If `/srv/wavehouse` is owned by `root:root` on the host (the default for a freshly `mkdir`'d directory), the binary refuses to start before it touches the settings directory or ClickHouse — `data_dir` is probed for writability right after the config loads: +Bind mounts do **not** copy-up — Docker exposes the host directory as-is, and the image's pre-created dir is masked entirely. The condition boot enforces is that UID 65532 can **write** to the directory — a freshly `mkdir`'d `root:root` directory at the default mode cannot be, which is the common case, though a root-owned directory with permissive mode bits or an ACL passes. If `/srv/wavehouse` is not writable, the binary refuses to start before it touches the settings directory or ClickHouse — `data_dir` is probed for writability right after the config loads: ```text wrap=false ERROR check data_dir error="data_dir /app/data is not writable; if running in a - container with a host bind mount, the host directory must be owned by - UID 65532 (the `nonroot` user in the distroless image). Try + container with a host bind mount, the host directory must be writable by + UID 65532 (the `nonroot` user in the distroless image); the usual fix is `sudo chown -R 65532:65532 /your/host/path`. ...: permission denied" ``` diff --git a/internal/config/check.go b/internal/config/check.go index 9eb92461..e67f9b52 100644 --- a/internal/config/check.go +++ b/internal/config/check.go @@ -98,9 +98,13 @@ func CheckDataDir(dir string) error { return fmt.Errorf("data_dir %s is not a directory", dir) } + // Walk up to the nearest existing entry with Lstat, so a dangling + // symlink — dir itself, or a component above it — stops the walk rather + // than being skipped over as "does not exist": the probe would otherwise + // pass in an unrelated ancestor and NATS would fail on the symlink later. target := dir for { - if _, err := os.Stat(target); err == nil { + if _, err := os.Lstat(target); err == nil { break } parent := filepath.Dir(target) @@ -109,6 +113,9 @@ func CheckDataDir(dir string) error { } target = parent } + if _, err := os.Stat(target); err != nil { + return fmt.Errorf("data_dir %s: %s is a dangling symlink: %w", dir, target, err) + } f, err := os.CreateTemp(target, ".wavehouse-datadir-probe-*") if err != nil { msg := fmt.Sprintf("data_dir %s is not writable", dir) diff --git a/internal/config/check_test.go b/internal/config/check_test.go index 904e2692..5decc737 100644 --- a/internal/config/check_test.go +++ b/internal/config/check_test.go @@ -92,6 +92,39 @@ func TestCheckDataDir(t *testing.T) { assert.True(t, os.IsNotExist(err), "the check must not create the directory") }) + t.Run("dangling symlink as data_dir", func(t *testing.T) { + t.Parallel() + root := t.TempDir() + link := filepath.Join(root, "data") + require.NoError(t, os.Symlink(filepath.Join(root, "gone"), link)) + err := CheckDataDir(link) + require.Error(t, err) + assert.Contains(t, err.Error(), "dangling symlink") + assert.Contains(t, err.Error(), link) + }) + + t.Run("dangling symlink above data_dir", func(t *testing.T) { + t.Parallel() + root := t.TempDir() + link := filepath.Join(root, "mnt") + require.NoError(t, os.Symlink(filepath.Join(root, "gone"), link)) + err := CheckDataDir(filepath.Join(link, "data")) + require.Error(t, err) + assert.Contains(t, err.Error(), "dangling symlink") + assert.Contains(t, err.Error(), link) + }) + + t.Run("symlink to a real directory passes", func(t *testing.T) { + t.Parallel() + root := t.TempDir() + target := filepath.Join(root, "real") + require.NoError(t, os.Mkdir(target, 0o700)) + link := filepath.Join(root, "data") + require.NoError(t, os.Symlink(target, link)) + require.NoError(t, CheckDataDir(link)) + require.NoError(t, CheckDataDir(filepath.Join(link, "nested"))) + }) + t.Run("a file is not a directory", func(t *testing.T) { t.Parallel() path := filepath.Join(t.TempDir(), "data") diff --git a/internal/config/persistence.go b/internal/config/persistence.go index 0ee1b7b7..c1640165 100644 --- a/internal/config/persistence.go +++ b/internal/config/persistence.go @@ -11,7 +11,7 @@ import ( // signature: a Docker bind mount whose host directory is owned by root // rather than the distroless `nonroot` user. Shared by the boot-time storage // error and the data_dir probe (CheckDataDir) that runs right after Load. -const permissionHint = "if running in a container with a host bind mount, the host directory must be owned by UID 65532 (the `nonroot` user in the distroless image). Try `sudo chown -R 65532:65532 /your/host/path`. Named volumes inherit ownership automatically and don't need this." +const permissionHint = "if running in a container with a host bind mount, the host directory must be writable by UID 65532 (the `nonroot` user in the distroless image); the usual fix is `sudo chown -R 65532:65532 /your/host/path`. Named volumes inherit ownership automatically and don't need this." // LogStorageInitError emits an error log for a storage-init failure, with a // UID-65532 hint when the failure looks like a permission denial — the From 278284272074dadc52637587aa8c1cd273a37ee3 Mon Sep 17 00:00:00 2001 From: taitelee Date: Thu, 10 Sep 2026 09:33:19 -0400 Subject: [PATCH 5/9] fix(config): hint on unsearchable data_dir parent, unexport unboundEnv, sync docs to review --- CHANGELOG.md | 4 ++-- config.yaml | 6 ++++-- deployments/Dockerfile | 2 +- deployments/Dockerfile.goreleaser | 2 +- docs/src/content/docs/architecture.md | 2 +- docs/src/content/docs/configuration.mdx | 4 ++-- docs/src/content/docs/deployment.md | 5 +++-- docs/src/integrations/diagram-png.mjs | 4 ++-- internal/config/check.go | 28 +++++++++++++++---------- internal/config/check_test.go | 21 ++++++++++++++++--- internal/config/config.go | 2 +- 11 files changed, 52 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5515b4e..8b53b190 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Added -- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (`internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds — the two variables read outside the struct, `WH_CONFIG` and `WH_LOG_LEVEL`, are exempt — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. Only the `WH_` prefix is checked, since the environment always carries unrelated names; the one outside source that shares it — Kubernetes service-link variables for a Service named `wh` or `wh-*` — is named in the error with the `enableServiceLinks: false` remediation. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes that `data_dir` is a writable directory (or absent under a writable parent, so boot can create it) by creating and removing one temp file, so a `data_dir` the process cannot write to refuses boot before schema discovery rather than after it; a permission denial carries the UID-65532 remediation (a bind mount owned by root is the typical cause), and that hint string is now shared with `LogStorageInitError`. An empty or blank `data_dir` — reachable through `WH_DATA_DIR=` — is refused by the probe outright, since the ancestor walk would otherwise fall back to the working directory and NATS and Pebble state would land under it. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. - - **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. - **Settings-directory hot reload — boot loading, three reload triggers, and the config-key migration** (`internal/settings/` (new: `store.go`, `watch.go`, + tests), `internal/api/settings.go` (new, + tests), `internal/api/{router,ingest,structured_query}.go`, `internal/discovery/discovery.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/settings-directory.mdx` (new — the hot-reloadable half of configuration gets its own page; `configuration.mdx` is boot config only); closes the loop [#500](https://github.com/Wave-RF/WaveHouse/pull/500) opened, tracked by [#48](https://github.com/Wave-RF/WaveHouse/issues/48)): the server now *consumes* the settings directory instead of only validating it. `settings.Store` owns the adopted snapshot: `settings.dir` / `WH_SETTINGS_DIR` is now **required**, boot validates and adopts the directory (missing or invalid refuses to start); a running instance then re-validates and re-adopts on any of three triggers — a **directory watch** (fsnotify on the directory, not the files, so atomic-writer replaces and Kubernetes ConfigMap symlink swaps aren't lost; bursts debounce into one reload), **`SIGHUP`**, and **`POST /v1/ops/settings/reload`** (admin-gated; returns `{"adopted", "findings"}`, `200` adopted / `422` rejected) — all funneling through one serialized reload path. A reload that fails validation keeps the previous good snapshot (an operator mid-edit degrades to a log line, never a broken server); warnings don't block adoption, matching `wavehouse validate`. The tenant tunables **migrate out of boot config** into the directory's `config.json`: `dedupe.id_field` / `dedupe.require_id` (now with the per-table overrides under `dedupe.tables` that [#222](https://github.com/Wave-RF/WaveHouse/issues/222) asked for, resolved per record through the table → global cascade in one atomic snapshot read, so a reload lands at a record boundary and never mixes documents within one record), `query.default_max_rows` and `query.timestamp_bucket_seconds` (read per query), `schema.refresh_interval` (re-read after each tick, so a change applies from the next cycle), `stream.keepalive_interval` / `stream.keepalive_buckets` (a reload calls the new `Heartbeater.Reconfigure`, which rebuilds the keepalive wheel in place with every live subscriber carried over and re-times the running ticker) and `stream.gap_window_minutes` (the sweeper re-reads it every sweep), `mq.max_bytes_gb` (an after-adopt hook updates the `WAVEHOUSE` and `WAVEHOUSE_DLQ` stream limits in place via `EmbeddedNATS.Resize` — shrinking below the buffered size backpressures until the worker drains, nothing is dropped), `dlq.enabled` with per-table overrides under `dlq.tables` (resolved by the ingest worker at the moment a poison row is isolated: on → park it on `WAVEHOUSE_DLQ` and ack; off → leave it unacked for redelivery, never dropped; the DLQ stream and `GET /v1/ops/dlq/stats` now always exist, so the switch is purely behavioral), the **ClickHouse wiring** (`clickhouse.addr` / `http_port` / `http_scheme` / `database` / `username` / `query_timeout`: the new `chconn.Manager` is the one `driver.Conn` every consumer holds and swaps the connection behind it on reload — unconditionally, since the adopted settings are the authority and reachability already surfaces through schema discovery and `/readyz`; the replaced one closes after a `query_timeout` grace; the ingest worker, raw-SQL proxy, and schema registry read the HTTP target, timeout, and database per call), the **auth verifier wiring** (`auth.jwks_url` / `auth.role_claim`: the new `auth.Authenticator` swaps a whole verifier — key source plus its pinned algorithm allowlist — atomically per reload, unconditionally, so an unreachable JWKS fails closed until it can be fetched; `auth.Middleware` is gone — `Authenticator` is the one constructor), and the CORS allowlist (`cors.allowed_origins`, resolved per request). The corresponding YAML/env keys are **removed**: `server.cors_allowed_origins`, `query.default_max_rows`, `schema.refresh_interval`, `dedupe.enabled`, `dedupe.id_field`, `dedupe.require_id`, `stream.keepalive_interval`, `stream.keepalive_buckets`, `mq.gap_window_minutes`, `cache.timestamp_bucket_seconds`, `mq.max_bytes_gb`, `dlq.enabled`, `clickhouse.addr`, `clickhouse.http_port`, `clickhouse.http_scheme`, `clickhouse.database`, `clickhouse.username`, `clickhouse.query_timeout`, `auth.jwks_url`, `auth.role_claim` (and `WH_SERVER_CORS_ALLOWED_ORIGINS`, `WH_QUERY_DEFAULT_MAX_ROWS`, `WH_SCHEMA_REFRESH_INTERVAL`, `WH_DEDUPE_ENABLED`, `WH_DEDUPE_ID_FIELD`, `WH_DEDUPE_REQUIRE_ID`, `WH_STREAM_KEEPALIVE_INTERVAL`, `WH_STREAM_KEEPALIVE_BUCKETS`, `WH_MQ_GAP_WINDOW_MINUTES`, `WH_CACHE_TIMESTAMP_BUCKET_SECONDS`, `WH_MQ_MAX_BYTES_GB`, `WH_DLQ_ENABLED`, `WH_CH_ADDR`, `WH_CH_HTTP_PORT`, `WH_CH_HTTP_SCHEME`, `WH_CH_DATABASE`, `WH_CH_USERNAME`, `WH_CH_QUERY_TIMEOUT`, `WH_AUTH_JWKS_URL`, `WH_AUTH_ROLE_CLAIM`); the secrets — `clickhouse.password`, `auth.jwt_secret`, `auth.operator_key` — stay boot config on purpose (never in a tracked JSON file; combined with the adopted wiring on every reconnect, rotating one is a restart), and boot config is now **strict**: `config.Load` re-reads the YAML against the struct's tags and refuses to start naming every undeclared key, so a `dlq:` or `clickhouse: addr:` left behind can't be read, ignored, and believed; the binary carries **no compiled defaults** — every `config.json` key is required (validation names each missing one), so the adopted snapshot is what the files say, and once adopted it outlives its files (a deleted file or vanished directory is just a rejected reload). Defaults live in one checked-in seed directory (`internal/settings/seed/`, `go:embed`ded): the new **`wavehouse bootstrap [dir]`** writes it (refusing a non-empty directory, the `initdb` contract; the directory resolves exactly as it does for `validate` — the argument, else `WH_SETTINGS_DIR`, usage error with neither — so the two commands are interchangeable on one path and a bare `bootstrap` inside the container images seeds `/app/settings`), the dev `config.yaml` points at a gitignored `./settings` that `make dev` seeds from it, and the e2e fixture ships a copy. The container images ship **no** settings directory: `WH_SETTINGS_DIR` is preset to `/app/settings`, the operator mounts a directory there (`standalone.yaml` bind-mounts the checked-in `deployments/compose/settings/`), and a missing mount refuses to boot rather than running on defaults nobody chose. `dedupe.enabled` moves too: the new `dedupe.Managed` wraps the Pebble store and a `Store.AfterAdopt` hook opens or closes it after every adoption, so flipping the switch is a reload, not a restart (seen ids persist across an off/on cycle; a failed open on reload is logged and ingest fails closed with `500` until the next reload, since the files asked for dedupe — at boot it still refuses to start; a record caught in the instant of the flip is published un-deduped and counted by `wavehouse_ingest_dedupe_disabled_total` rather than failed, and the hook is registered before the boot apply so a reload can never leave the settings and the store out of step). The watcher reloads once as soon as its watch exists, closing the gap between the boot read and the watch — an edit landing in between (a ConfigMap update during a rolling restart) is adopted, not silently missed. `dedupe.enabled` / `WH_DEDUPE_ENABLED` are removed from boot config alongside the other keys. What stays in boot config is only what cannot change under a running process — resource sizing (`data_dir`, `cache.l1_max_cost`), the listeners, the observability exporters — and the secrets. The compose stack now bind-mounts a checked-in `deployments/compose/settings/` (the seed with `clickhouse.addr` pointed at the `clickhouse` service) instead of a volume seeded with `bootstrap`, so the quickstart is `up -d` again; the e2e orchestrator copies the fixture settings per run and patches the testcontainer's ClickHouse ports into `config.json`, since that wiring no longer has an env override. Every after-adopt hook (dedupe, keepalive wheel) is registered before the reload triggers start, so the watcher's first reload can never be missed by a hook. Consumers take functions, not values (`IngestHandler.DedupeSettings`, the structured-query handler's `defaultMaxRows` / `bucketSecs func() int`, the ingest worker's `dlqEnabled func(table) bool`, the sweeper's `gapWindow func() time.Duration`, `corsMiddleware`'s origins getter, `SchemaRegistry`'s database and refresh-interval sources, the query handlers' timeout sources), so `internal/api` stays testable without materializing settings directories. The settings directory is also the **runtime authority for access control and named pipes** (`internal/settings/store.go`, `internal/policy/source.go` (new), `internal/pipes/pipes.go`, `internal/api/{policy,pipes,router}.go`, `internal/stream/hub.go`, `internal/auth/auth.go`, `cmd/wavehouse/main.go`, `Makefile`, `deployments/compose/settings/{policies,roles}.json`, `clients/ts/src/settings.ts` (new); closes [#229](https://github.com/Wave-RF/WaveHouse/issues/229), [#33](https://github.com/Wave-RF/WaveHouse/issues/33), [#461](https://github.com/Wave-RF/WaveHouse/issues/461), [#514](https://github.com/Wave-RF/WaveHouse/issues/514), [#460](https://github.com/Wave-RF/WaveHouse/issues/460), [#363](https://github.com/Wave-RF/WaveHouse/issues/363); advances [#48](https://github.com/Wave-RF/WaveHouse/issues/48) and [#214](https://github.com/Wave-RF/WaveHouse/issues/214)): `roles.json`, `policies.json`, and `pipes.json` are adopted with `config.json` as one snapshot and re-adopted on the same three triggers, and **files are the only write path** — standalone, the operator edits them on the host; on WaveHouse Cloud the control plane writes them — so there is no stored copy that can skip validation: every adoption runs the current rules (strict decode rejecting unknown and duplicate keys, the full policy validation including the claim-template grammar, pipe name/SQL/parameter-type rules, and the cross-file check that every role a grant or `allowed_roles` names is declared in `roles.json`), and a rejected edit keeps the previous good policy and pipes in effect. `policies.json` is one policy document (`{}` = no policy, adopted fail-closed with a warning); `pipes.json` carries full definitions (`allowed_roles`, `parameters`, `description`), so a file-defined pipe is no longer admin-only by construction. Consumers read the adopted snapshot per request through `policy.Source` (a `func() *policy.Policy`; `settings.Store.Policy` in production, `policy.Static(p)` in tests) and `pipes.Source` (`settings.Store`; `pipes.Static(q...)` in tests), so a reload applies to the very next request, including the SSE hub's per-event policy read. `GET /v1/ops/policy`, `POST /v1/ops/policy/validate`, `GET /v1/ops/pipes`, `GET /v1/ops/pipes/{name}`, and pipe execution are unchanged; the operator key still passes the `/v1/ops/*` gate under no policy, now as the break-glass that inspects the policy and triggers `POST /v1/ops/settings/reload` after `policies.json` is fixed. The SDK gains `wh.settings.reload()` (`POST /v1/ops/settings/reload`, returning `{ adopted, findings }`). The compose stack's trial `public` policy moves into the bind-mounted `deployments/compose/settings/policies.json` + `roles.json`, and `make dev` copies the same two files into its seeded `./settings` so a fresh dev server works tokenless. **Removed** — the write endpoints `PUT /v1/ops/policy`, `PUT /v1/ops/pipes/{name}`, and `DELETE /v1/ops/pipes/{name}`; the NATS KV buckets `WAVEHOUSE_POLICY` and `WAVEHOUSE_PIPES` and their KV Watch sync (`internal/policy/store.go`, the pipes KV store); the boot-config keys `policy.file_path` / `WH_POLICY_FILE_PATH` and `pipes.dir` / `WH_PIPES_DIR` (a leftover `policy:` or `pipes:` YAML block now refuses boot by name, like the other moved keys) and the `.sql`-directory pipes bootstrap; `deployments/compose/dev-policy.yaml`; the SDK methods `wh.policy.set`, `wh.pipes.set`, and `wh.pipes.delete`; and the test helpers `policy.NewMemoryStore`, `pipes.NewMemoryStore`, and `testutil/natsjs.go`. @@ -22,6 +20,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- **Boot refuses an unbound `WH_*` environment variable and an unusable `data_dir`** (BREAKING; `internal/config/check.go` (new, + tests), `internal/config/{config,persistence}.go`, `cmd/wavehouse/main.go`, `docs/src/integrations/diagram-png.mjs`): the environment half of the strict YAML loader. `config.Load` now errors, naming every offender, on a `WH_*` variable that no `Config` field binds — the two variables read outside the struct, `WH_CONFIG` and `WH_LOG_LEVEL`, are exempt — `WH_DEDUPE_ENABLED=true` left in a compose file from before the settings-directory move, or a misspelling, was set, ignored, and believed. **An existing deployment that still exports a variable this release moved to the settings directory stops booting until it is unset**; the upgrade runbook in `deployment.md` gains that audit. Only the `WH_` prefix is checked, since the environment always carries unrelated names; the one outside source that shares it — Kubernetes service-link variables for a Service named `wh` or `wh-*` — is named in the error with the `enableServiceLinks: false` remediation, and the docs build's opt-out knob is renamed from `WH_SKIP_DIAGRAM_PNG` to `DOCS_SKIP_DIAGRAM_PNG` so an exported one no longer refuses a local boot. Right after `Load`, before ClickHouse or the settings directory are touched, `config.CheckDataDir` probes `data_dir` and refuses boot on any of: an empty or blank value (reachable through `WH_DATA_DIR=`), refused outright since the ancestor walk would otherwise fall back to the working directory and NATS and Pebble state would land under it; a path that exists and is not a directory; a dangling symlink at `data_dir` or any component above it (the walk to the nearest existing ancestor uses `Lstat`, so a failed mount is not skipped over as "does not exist" and passed in an unrelated directory); and a directory the process cannot write to — or, when it does not exist, an unwritable nearest ancestor — probed by creating and removing one temp file. So an unusable `data_dir` refuses boot before schema discovery rather than after it; a permission denial — on the probe, or on reaching the path at all through a parent without search permission — carries the UID-65532 remediation (a bind mount owned by root is the typical cause), and that hint string is now shared with `LogStorageInitError`. `EnvConfig` and `EnvLogLevel` join `EnvSettingsDir` as the exported names for the process-level variables. Boot is the validator for the non-hot-reloadable half — there is no dry-run subcommand, by decision on #530: boot config only takes effect through a restart, so the restart is where it is checked, and the docs say so. Closes #530. + - **The docs site now consumes the *published* `@wavehouse/sdk`, not the workspace one** (`docs/package.json`, `pnpm-workspace.yaml`, `Makefile`, `scripts/classify-paths.sh`). The landing page's live demo streams against a separately-deployed backend on its own release cadence, but took its SDK from the tree — so this release's wire change would have reached the deployed site the moment it merged, while the backend still spoke the old envelope: the panel keeps reporting "live" while every frame is dropped for want of a schema announcement, with no `error` callback ([#568](https://github.com/Wave-RF/WaveHouse/issues/568)). `docs` now pins `^0.1.1` from the registry, which takes `0.1.x` patches and stops short of `0.2.0`, so moving the site onto the new wire is a deliberate bump lined up with tagging the SDK release rather than a side effect of merging. `tests/e2e/sdk` deliberately keeps `workspace:*`. Depending on our own package from the registry also made `minimumReleaseAge` apply to it for the first time, and the exclude list named only `@wave-rf/*` (the plugin scope), so a freshly tagged SDK would have been uninstallable by the docs site for seven days — `@wavehouse/*` is now exempt too. The docs build no longer needs `build-ts`. - **Policy format v2: `policies.json` is role-first, and the two operations are separate permission types** (BREAKING; `internal/policy/policy.go`, `internal/policy/rowfilter.go`, `internal/settings/validate.go`, `internal/query/builder.go`, `internal/api/{ingest,structured_query}.go`, `internal/stream/hub.go`, `clients/ts/src/{types,index}.ts`, `deployments/compose/settings/policies.json`, `docs/src/content/docs/{access-control.mdx,settings-directory.mdx,architecture.md}`, `AGENTS.md`, `tests/e2e/sdk/`): a table entry was keyed `tables..select.`; it is now keyed `tables.
..select`. A role appears once per table and its grant carries two optional blocks, so a role that could both read and write no longer has to be written out twice, and "this role has no insert grant" is a missing block rather than an absence you have to notice in a second map. The blocks are now distinct types rather than one struct whose halves were inert per operation: `select` takes `allow_columns`, `deny_columns`, `filter`, `allowed_aggregations`, `denied_aggregations` and the four `max_*` limits; `insert` takes `allow_columns`, `deny_columns`, `check`. Field names and semantics are unchanged — only the nesting moves — but a field on the wrong side that the old layout *accepted* — the four `max_*` limits and the two aggregation rules — is now a validation error instead of being silently ignored. `filter` under an `insert` grant and `check` under a `select` one are rejected too, but that is not new here — [#541](https://github.com/Wave-RF/WaveHouse/pull/541), also unreleased, added the runtime check; the split types now refuse them one layer earlier, as unknown keys at the strict decode. Upgrading from **0.1.0**, though, none of the eight were enforced: a 0.1.0 policy could carry an insert-side `filter` that resolved into a `WHERE` the insert path never read, as well as an ignored limit. Converting to the role-first layout drops both. Internally `ResolvedPermissions` splits the same way (`.Select` / `.Insert`), and `IsColumnAllowed` takes the side to consult, which is what stops the read allowlist from ever answering a write question or vice versa. **There is no automatic conversion** — the settings files are the source of truth and WaveHouse has no write path back to them — so `policies.json` must be converted by hand; run `wavehouse validate` before restarting. A document still in the old layout is reported as one clear finding naming the table and operation and pointing at [the migration note](https://wavehouse.dev/access-control#migrating-from-the-operation-first-layout), instead of the confusing strict-decode "unknown field" error it would otherwise produce (or, for an empty operation block, silently decoding as a role named `select` with no grants — which fails the undeclared-role check when `roles.json` does not declare a role named `select` — the usual case, since `checkRoleRefs` errors and moves on before reaching the "grant sets neither select nor insert" warning. If such a role *is* declared, you get that warning instead and the document adopts). One shape is refused differently: a grant keyed by a role named after the *other* operation (`tables.t.select.insert`) reads as a different grant under each layout — different role, different operation, or both — so it gets its own error asking you to rename the role rather than the migration pointer. A role named after its *own* operation (`tables.t.select.select`) means the same thing either way and is accepted. diff --git a/config.yaml b/config.yaml index 51cea3fa..f1300837 100644 --- a/config.yaml +++ b/config.yaml @@ -1,5 +1,7 @@ -# This file is strict: a key the server doesn't declare (a typo, or a tunable -# that moved to the settings directory) refuses to boot and is named. +# This file is strict, and so is the environment: a key the server doesn't +# declare (a typo, or a tunable that moved to the settings directory) refuses +# to boot and is named — whether it arrives as a YAML key here or as a WH_* +# variable no key binds. # Root for embedded state. NATS lives at /nats; Pebble (when dedupe # is enabled) lives at /pebble. In a container, this MUST be on a # host-backed volume — the relative default is for local binary use only. diff --git a/deployments/Dockerfile b/deployments/Dockerfile index a29c7088..bb74ecd7 100644 --- a/deployments/Dockerfile +++ b/deployments/Dockerfile @@ -29,7 +29,7 @@ RUN --mount=type=cache,target=/go/pkg/mod \ # subdirectories itself when NATS/Pebble open their stores — pre-creating # them here would buy nothing and obscures intent. Named-volume copy-up runs # against `/app/data` regardless; bind mounts mask the image dir entirely -# and rely on host-side ownership matching UID 65532. +# and rely on the host directory being writable by UID 65532. # # /app/settings is the settings-directory mount point, pre-created but # deliberately EMPTY: the image ships no settings, same as it ships no diff --git a/deployments/Dockerfile.goreleaser b/deployments/Dockerfile.goreleaser index 3f681482..8d22201c 100644 --- a/deployments/Dockerfile.goreleaser +++ b/deployments/Dockerfile.goreleaser @@ -7,7 +7,7 @@ # Only the parents are pre-created — the binary mkdirs `nats/` and `pebble/` # subdirs itself when NATS/Pebble open their stores. Named-volume copy-up # runs against `/app/data`; bind mounts mask the image dir entirely and -# require host-side ownership matching UID 65532. +# require the host directory to be writable by UID 65532. # Pin to the builder's native arch via $BUILDPLATFORM. Without this, a # cross-arch build (e.g. linux/arm64 from an amd64 builder) would default diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index b0bf01ef..879038f5 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -108,7 +108,7 @@ The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https:/ ### `config/` — Configuration - **config.go** — Loads *boot* configuration from a YAML file with environment variable overrides (using [cleanenv](https://github.com/ilyakaznacheev/cleanenv)); every key has a `WH_`-prefixed env var. Boot config is only what can't change under a running process — resource sizing, listeners, observability exporters, the settings-directory path, and the secrets (`clickhouse.password`, `auth.jwt_secret`, `auth.operator_key`). Everything tenant-tunable lives in the settings directory (`settings/`). Both sources are strict: `rejectUnknownKeys` refuses to boot naming every YAML key the struct doesn't declare, and `rejectUnboundEnv` does the same for every `WH_*` environment variable no field binds, so a tunable that moved to the settings directory can't be read, ignored, and believed. Boot is the validator for this half — there is no dry-run command. See [Configuration Reference](/configuration). -- **check.go** — `UnboundEnv` walks the struct's `env` tags (plus the two process-level names, `WH_CONFIG` and `WH_LOG_LEVEL`) against the environment; `CheckDataDir` probes that `data_dir` is a writable directory, or creatable under a writable parent, by creating and removing one temp file — run by `main` right after `Load`, so a `data_dir` the process cannot write to refuses boot before anything dials out; a permission denial carries the UID-65532 hint, since a bind mount owned by root is the typical cause. An empty `data_dir` (reachable through `WH_DATA_DIR=`) is refused outright rather than probing the working directory. +- **check.go** — `unboundEnv` walks the struct's `env` tags (plus the two process-level names, `WH_CONFIG` and `WH_LOG_LEVEL`) against the environment; `CheckDataDir` probes `data_dir` — run by `main` right after `Load`, so an unusable `data_dir` refuses boot before anything dials out. It refuses an empty value (reachable through `WH_DATA_DIR=`) outright rather than probing the working directory; a path that exists and is not a directory; a dangling symlink at `data_dir` or any component above it (the walk to the nearest existing ancestor uses `Lstat`, so a failed mount is not skipped over as "does not exist"); and a directory the process cannot write to — or, when it does not exist, an unwritable nearest ancestor — probed by creating and removing one temp file. A permission denial, on the probe or on reaching the path through a parent without search permission, carries the UID-65532 hint, since a bind mount owned by root is the typical cause. ### `dedupe/` — Deduplication (Optional) diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx index 841062f9..0789496b 100644 --- a/docs/src/content/docs/configuration.mdx +++ b/docs/src/content/docs/configuration.mdx @@ -16,8 +16,8 @@ WaveHouse is configured via a YAML file with environment variable overrides. All 1. If a config file exists at the specified path (default: `config.yaml`), it is loaded first. 2. Environment variables override any values from the YAML file. 3. If no config file exists, all values are read from environment variables. Every key has a default except `settings.dir` (`WH_SETTINGS_DIR`), which must be set either way. -4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way; only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix. Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod, for each Service that existed before the pod started. A Service named `wh` or `wh-*` therefore produces `WH_SERVICE_HOST`, `WH_PORT`, and more, and the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else. The error says so. -5. Before anything dials out, `data_dir` is probed: it must be a writable directory, or absent with a writable parent so it can be created. A `data_dir` the process cannot write to refuses to boot before ClickHouse discovery rather than after it, and a permission denial carries the UID-65532 remediation, since a bind mount owned by root is the typical cause. An empty `data_dir` is refused outright. +4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way. Two variables have no YAML key and are exempt, because the process reads them before the file: `WH_CONFIG` (below), which locates the file, and `WH_LOG_LEVEL`. Only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix. Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod, for each Service that existed before the pod started. A Service named `wh` or `wh-*` therefore produces `WH_SERVICE_HOST`, `WH_PORT`, and more, and the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else. The error says so. +5. Before anything dials out, `data_dir` is probed, and boot refuses on any of these: the value is empty; the path exists but is not a directory; the path, or any component above it, is a dangling symlink (a mount that never came up); the directory exists but the process cannot write to it; the directory is absent and its nearest existing ancestor is not writable, so it could not be created. The probe runs before ClickHouse discovery, so the refusal lands at the top of the log, and a permission denial — on the write probe, or on reaching the path at all through a parent without search permission — carries the UID-65532 remediation, since a bind mount owned by root is the typical cause. Boot is the validator for this half of configuration: there is no dry run, and a refused boot with the offending key, variable, or path named in the error is the loud signal. The hot-reloadable half has a dry run — `wavehouse validate` — because it is edited under a running server; boot config only ever takes effect through a restart, so the restart is where it is checked. diff --git a/docs/src/content/docs/deployment.md b/docs/src/content/docs/deployment.md index d1e28c13..424e0745 100644 --- a/docs/src/content/docs/deployment.md +++ b/docs/src/content/docs/deployment.md @@ -186,7 +186,7 @@ On a first-ever run this is expected. On every subsequent run it should be silen ### Distroless Permission Traps (named volume vs bind mount) -WaveHouse images run as the distroless `nonroot` user (UID 65532). Bind mounts and named volumes interact with this differently, and the distroless image has no shell to `chown` things at runtime — so getting the host side wrong refuses boot in the first lines of the log with a named `data_dir` error and the `chown` remediation attached. +WaveHouse images run as the distroless `nonroot` user (UID 65532). Bind mounts and named volumes interact with this differently, and the distroless image has no shell to `chown` things at runtime — so getting the host side of `/app/data` wrong refuses boot in the first lines of the log with a named `data_dir` error and the `chown` remediation attached. `/app/settings` fails differently: the server only reads it, so a mount it cannot read, or that does not validate, is a settings-validation error rather than a `data_dir` one. **Named volumes** (the recommended pattern): @@ -340,10 +340,11 @@ This affects the streaming surface too, and more quietly. SSE gap-fill (`?since= On the worker side the outcome depends on the DLQ. **With the DLQ enabled for the table**, the message is parked on `dlq.{table}` with `X-DLQ-*` headers and is recoverable by hand — but re-ingest each parked envelope's inner `data` object as a fresh `POST /v1/ingest`; republishing the envelope as-is onto `ingest.{table}` fails the same `format` check and simply re-parks it. **With the DLQ switched off for the table, it is permanently lost**: acked and dropped with an `ERROR` log and a `wavehouse_ingest_poison_total` increment carrying `disposition="dropped"`, unrecoverable from either the ingest stream or the DLQ, because a message that can never insert must not redeliver forever. Draining first is cheaper than a manual replay, and it is the only option at all where the DLQ is off. -Two audits belong **before** the drain, because neither announces itself afterwards: +Three audits belong **before** the drain, because none of them announces itself afterwards: - **`Nullable(T) DEFAULT …` columns now store `NULL` where they took their default.** A positional row has one slot per insertable column and no way to say *absent*, so a key the record omits rides as an explicit `null`. `input_format_null_as_default=1` turns that back into the default for a **non-nullable** column, but ClickHouse stores `NULL` on a nullable one whatever the setting says — only an absent key ever took the default. Following this runbook exactly still changes what lands in those columns, silently. See [the ingest note](/ingest-pipeline#the-journey-of-one-event). - **Policy `check` blocks are now validated against the table.** A `check` naming a column the table lacks, one it computes (`MATERIALIZED`/`ALIAS`), or an `EPHEMERAL` one is a per-record `403` on *every* insert by that role. `wavehouse validate` cannot catch it — it never sees the ClickHouse schema — so audit them against their tables first. See [Access control → Insert checks](/access-control#insert-checks). +- **Every `WH_*` variable the binary no longer reads refuses boot.** The tunables that moved to the settings directory this release — the ClickHouse wiring other than the password, `dedupe.*`, `dlq.*`, `stream.*`, `mq.*`, `query.*`, `schema.*`, `cors.*`, `auth.role_claim`, and the old policy-file and pipes-directory paths — used to be `WH_*` variables. The old binary ignored a leftover; the new one names every variable it does not bind and exits before it opens the queue, so a pod spec or compose file that still carries one comes back from the upgrade as a container that will not start. Diff the environment against the [Configuration Reference](/configuration) first, and move each leftover to its [`config.json` key](/settings-directory#configjson-keys) or drop it. A Kubernetes Service named `wh` or `wh-*` counts too: its injected `WH_SERVICE_HOST` and `WH_PORT` link variables need `enableServiceLinks: false` on the pod spec. To drain before upgrading: diff --git a/docs/src/integrations/diagram-png.mjs b/docs/src/integrations/diagram-png.mjs index fc041b79..2ea57d04 100644 --- a/docs/src/integrations/diagram-png.mjs +++ b/docs/src/integrations/diagram-png.mjs @@ -109,8 +109,8 @@ export function diagramPng(options = {}) { name: "wh-diagram-png", hooks: { "astro:build:done": async ({ dir, pages, logger }) => { - if (process.env.WH_SKIP_DIAGRAM_PNG === "1") { - logger.info("skipped (WH_SKIP_DIAGRAM_PNG=1)"); + if (process.env.DOCS_SKIP_DIAGRAM_PNG === "1") { + logger.info("skipped (DOCS_SKIP_DIAGRAM_PNG=1)"); return; } try { diff --git a/internal/config/check.go b/internal/config/check.go index e67f9b52..279f2a6e 100644 --- a/internal/config/check.go +++ b/internal/config/check.go @@ -17,26 +17,26 @@ import ( const envPrefix = "WH_" // processEnv lists the WH_* names the binary reads outside the Config struct -// (cmd/wavehouse), so UnboundEnv doesn't flag them. +// (cmd/wavehouse), so unboundEnv doesn't flag them. var processEnv = []string{EnvConfig, EnvLogLevel} // rejectUnboundEnv is the environment half of rejectUnknownKeys: an error // naming every WH_* variable in environ that nothing reads. func rejectUnboundEnv(environ []string) error { - unbound := UnboundEnv(environ) + unbound := unboundEnv(environ) if len(unbound) == 0 { return nil } return fmt.Errorf("unbound environment variable(s): %s — a typo, or a key that moved to the settings directory (%s); unset it, or rename it to a key the Config struct declares. On Kubernetes, a Service named wh or wh-* injects WH_SERVICE_HOST, WH_PORT, … into every pod started after it: set enableServiceLinks: false on the pod spec, or rename the Service", strings.Join(unbound, ", "), EnvSettingsDir) } -// UnboundEnv returns, sorted, every WH_* name in environ (os.Environ() form, +// unboundEnv returns, sorted, every WH_* name in environ (os.Environ() form, // "KEY=value") that no Config field's env tag binds and the binary doesn't // read otherwise. Such a name is almost always a typo or a key that moved to // the settings directory — `WH_DEDUPE_ENABLED=true` left in a compose file // would otherwise be set, ignored, and believed. Only the WH_ prefix is // checked, since the environment is shared with whatever launched the process. -func UnboundEnv(environ []string) []string { +func unboundEnv(environ []string) []string { bound := map[string]bool{} for _, name := range processEnv { bound[name] = true @@ -77,13 +77,14 @@ func collectEnvTags(t reflect.Type, into map[string]bool) { // run, or a missing mount that WarnIfFreshDataDir calls out) must have a // writable nearest existing ancestor so that creation can succeed. Run before // anything dials out, so a data_dir the process cannot write to refuses boot -// before ClickHouse discovery rather than after it; a permission denial -// carries the UID-65532 hint, since a bind mount owned by root is the -// typical cause. Writability is probed by creating and removing one temp -// file: the only portable test that exercises the mount's ownership and -// mode. A blank dir — reachable through `WH_DATA_DIR=` — is refused -// outright: the ancestor walk would otherwise probe the working directory -// and pass, and NATS and Pebble state would land under it. +// before ClickHouse discovery rather than after it; a permission denial — +// on the write probe, or on reaching dir at all through a parent without +// search permission — carries the UID-65532 hint, since a bind mount owned +// by root is the typical cause. Writability is probed by creating and +// removing one temp file: the only portable test that exercises the mount's +// ownership and mode. A blank dir — reachable through `WH_DATA_DIR=` — is +// refused outright: the ancestor walk would otherwise probe the working +// directory and pass, and NATS and Pebble state would land under it. func CheckDataDir(dir string) error { if strings.TrimSpace(dir) == "" { return errors.New("data_dir (WH_DATA_DIR) is required: an empty value would scatter NATS and Pebble state under the working directory") @@ -92,6 +93,11 @@ func CheckDataDir(dir string) error { switch { case errors.Is(err, fs.ErrNotExist): // Boot creates it; check the ancestor below. + case errors.Is(err, fs.ErrPermission): + // A component above dir denies search permission (/srv/data at + // root:root 0700 with data_dir under it): Stat fails before the walk + // and the write probe ever run, so the hint has to attach here. + return fmt.Errorf("data_dir %s is not accessible; %s: %w", dir, permissionHint, err) case err != nil: return fmt.Errorf("data_dir %s: %w", dir, err) case !info.IsDir(): diff --git a/internal/config/check_test.go b/internal/config/check_test.go index 5decc737..df9951a6 100644 --- a/internal/config/check_test.go +++ b/internal/config/check_test.go @@ -27,8 +27,8 @@ func TestUnboundEnv(t *testing.T) { "WHATEVER=1", // prefix is WH_, not WH "NOT_WH_FOO=1", } - assert.Equal(t, []string{"WH_", "WH_DEDUPE_ENABLED", "WH_SERVER_PROT"}, UnboundEnv(environ)) - assert.Empty(t, UnboundEnv(nil)) + assert.Equal(t, []string{"WH_", "WH_DEDUPE_ENABLED", "WH_SERVER_PROT"}, unboundEnv(environ)) + assert.Empty(t, unboundEnv(nil)) } // Every variable cleanenv itself reads must count as bound — a name the @@ -50,7 +50,7 @@ func TestUnboundEnv_MatchesCleanenv(t *testing.T) { } require.True(t, seen[EnvSettingsDir]) require.True(t, seen["WH_OTEL_TRACES_SAMPLE_RATE"]) - assert.Empty(t, UnboundEnv(environ)) + assert.Empty(t, unboundEnv(environ)) } func TestCollectEnvTags_CommaList(t *testing.T) { @@ -147,6 +147,21 @@ func TestCheckDataDir(t *testing.T) { assert.Contains(t, err.Error(), "65532") }) + t.Run("unsearchable parent carries the UID hint", func(t *testing.T) { + // os.Stat fails with EACCES, not ErrNotExist, so this never reaches + // the write probe: the hint must ride on the Stat arm. + if os.Getuid() == 0 { + t.Skip("root searches anywhere") + } + parent := t.TempDir() + require.NoError(t, os.Chmod(parent, 0o600)) + t.Cleanup(func() { _ = os.Chmod(parent, 0o700) }) //nolint:gosec // G302: restore so TempDir cleanup works + err := CheckDataDir(filepath.Join(parent, "data")) + require.Error(t, err) + assert.Contains(t, err.Error(), "is not accessible") + assert.Contains(t, err.Error(), "65532") + }) + t.Run("missing directory under an unwritable parent", func(t *testing.T) { if os.Getuid() == 0 { t.Skip("root writes anywhere") diff --git a/internal/config/config.go b/internal/config/config.go index faf598c9..f02bb5b3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -35,7 +35,7 @@ const EnvSettingsDir = "WH_SETTINGS_DIR" // EnvConfig names the boot-config file (default: ./config.yaml) and // EnvLogLevel the minimum log level. Neither is a Config field — the first // locates the file the struct is read from, the second is read by main -// directly — so they are declared here as the process-level names UnboundEnv +// directly — so they are declared here as the process-level names unboundEnv // must not flag. const ( EnvConfig = "WH_CONFIG" From 26d90767117776414bc6db64bfd0dcf2a1aaf5a1 Mon Sep 17 00:00:00 2001 From: taitelee Date: Thu, 10 Sep 2026 09:52:54 -0400 Subject: [PATCH 6/9] docs(deployment): name the service-link variables a wh-* Service actually injects --- docs/src/content/docs/deployment.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/content/docs/deployment.md b/docs/src/content/docs/deployment.md index 424e0745..002dcd7b 100644 --- a/docs/src/content/docs/deployment.md +++ b/docs/src/content/docs/deployment.md @@ -344,7 +344,7 @@ Three audits belong **before** the drain, because none of them announces itself - **`Nullable(T) DEFAULT …` columns now store `NULL` where they took their default.** A positional row has one slot per insertable column and no way to say *absent*, so a key the record omits rides as an explicit `null`. `input_format_null_as_default=1` turns that back into the default for a **non-nullable** column, but ClickHouse stores `NULL` on a nullable one whatever the setting says — only an absent key ever took the default. Following this runbook exactly still changes what lands in those columns, silently. See [the ingest note](/ingest-pipeline#the-journey-of-one-event). - **Policy `check` blocks are now validated against the table.** A `check` naming a column the table lacks, one it computes (`MATERIALIZED`/`ALIAS`), or an `EPHEMERAL` one is a per-record `403` on *every* insert by that role. `wavehouse validate` cannot catch it — it never sees the ClickHouse schema — so audit them against their tables first. See [Access control → Insert checks](/access-control#insert-checks). -- **Every `WH_*` variable the binary no longer reads refuses boot.** The tunables that moved to the settings directory this release — the ClickHouse wiring other than the password, `dedupe.*`, `dlq.*`, `stream.*`, `mq.*`, `query.*`, `schema.*`, `cors.*`, `auth.role_claim`, and the old policy-file and pipes-directory paths — used to be `WH_*` variables. The old binary ignored a leftover; the new one names every variable it does not bind and exits before it opens the queue, so a pod spec or compose file that still carries one comes back from the upgrade as a container that will not start. Diff the environment against the [Configuration Reference](/configuration) first, and move each leftover to its [`config.json` key](/settings-directory#configjson-keys) or drop it. A Kubernetes Service named `wh` or `wh-*` counts too: its injected `WH_SERVICE_HOST` and `WH_PORT` link variables need `enableServiceLinks: false` on the pod spec. +- **Every `WH_*` variable the binary no longer reads refuses boot.** The tunables that moved to the settings directory this release — the ClickHouse wiring other than the password, `dedupe.*`, `dlq.*`, `stream.*`, `mq.*`, `query.*`, `schema.*`, `cors.*`, `auth.role_claim`, and the old policy-file and pipes-directory paths — used to be `WH_*` variables. The old binary ignored a leftover; the new one names every variable it does not bind and exits before it opens the queue, so a pod spec or compose file that still carries one comes back from the upgrade as a container that will not start. Diff the environment against the [Configuration Reference](/configuration) first, and move each leftover to its [`config.json` key](/settings-directory#configjson-keys) or drop it. A Kubernetes Service named `wh` or `wh-*` counts too: it injects link variables under the `WH_` prefix (`WH_SERVICE_HOST` and `WH_PORT` for `wh`, `WH_FOO_SERVICE_HOST` and `WH_FOO_PORT` for `wh-foo`), so set `enableServiceLinks: false` on the pod spec. To drain before upgrading: From 13296a73b78376cc80a7d9ad12acd61e7fbba4c3 Mon Sep 17 00:00:00 2001 From: taitelee Date: Thu, 10 Sep 2026 10:33:16 -0400 Subject: [PATCH 7/9] docs(config): state why WH_CONFIG and WH_LOG_LEVEL are exempt from the unbound check --- docs/src/content/docs/configuration.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx index 0789496b..08017c84 100644 --- a/docs/src/content/docs/configuration.mdx +++ b/docs/src/content/docs/configuration.mdx @@ -16,7 +16,7 @@ WaveHouse is configured via a YAML file with environment variable overrides. All 1. If a config file exists at the specified path (default: `config.yaml`), it is loaded first. 2. Environment variables override any values from the YAML file. 3. If no config file exists, all values are read from environment variables. Every key has a default except `settings.dir` (`WH_SETTINGS_DIR`), which must be set either way. -4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way. Two variables have no YAML key and are exempt, because the process reads them before the file: `WH_CONFIG` (below), which locates the file, and `WH_LOG_LEVEL`. Only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix. Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod, for each Service that existed before the pod started. A Service named `wh` or `wh-*` therefore produces `WH_SERVICE_HOST`, `WH_PORT`, and more, and the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else. The error says so. +4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way. Two variables have no YAML key and are exempt because they are not config keys at all but process-level settings `main` reads directly: `WH_CONFIG` (below), which locates the file, and `WH_LOG_LEVEL`. Only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix. Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod, for each Service that existed before the pod started. A Service named `wh` or `wh-*` therefore produces `WH_SERVICE_HOST`, `WH_PORT`, and more, and the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else. The error says so. 5. Before anything dials out, `data_dir` is probed, and boot refuses on any of these: the value is empty; the path exists but is not a directory; the path, or any component above it, is a dangling symlink (a mount that never came up); the directory exists but the process cannot write to it; the directory is absent and its nearest existing ancestor is not writable, so it could not be created. The probe runs before ClickHouse discovery, so the refusal lands at the top of the log, and a permission denial — on the write probe, or on reaching the path at all through a parent without search permission — carries the UID-65532 remediation, since a bind mount owned by root is the typical cause. Boot is the validator for this half of configuration: there is no dry run, and a refused boot with the offending key, variable, or path named in the error is the loud signal. The hot-reloadable half has a dry run — `wavehouse validate` — because it is edited under a running server; boot config only ever takes effect through a restart, so the restart is where it is checked. From 4d64cb25f317878f9f0c3371e86450eb9116d847 Mon Sep 17 00:00:00 2001 From: taitelee Date: Thu, 10 Sep 2026 11:33:26 -0400 Subject: [PATCH 8/9] docs(config): correct Kubernetes service-link wording, drop the old-variable list, map config files --- docs/src/content/docs/architecture.md | 6 ++++-- docs/src/content/docs/configuration.mdx | 2 +- docs/src/content/docs/deployment.md | 4 +++- internal/config/check.go | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index 879038f5..72c2e766 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -107,8 +107,10 @@ The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https:/ ### `config/` — Configuration -- **config.go** — Loads *boot* configuration from a YAML file with environment variable overrides (using [cleanenv](https://github.com/ilyakaznacheev/cleanenv)); every key has a `WH_`-prefixed env var. Boot config is only what can't change under a running process — resource sizing, listeners, observability exporters, the settings-directory path, and the secrets (`clickhouse.password`, `auth.jwt_secret`, `auth.operator_key`). Everything tenant-tunable lives in the settings directory (`settings/`). Both sources are strict: `rejectUnknownKeys` refuses to boot naming every YAML key the struct doesn't declare, and `rejectUnboundEnv` does the same for every `WH_*` environment variable no field binds, so a tunable that moved to the settings directory can't be read, ignored, and believed. Boot is the validator for this half — there is no dry-run command. See [Configuration Reference](/configuration). -- **check.go** — `unboundEnv` walks the struct's `env` tags (plus the two process-level names, `WH_CONFIG` and `WH_LOG_LEVEL`) against the environment; `CheckDataDir` probes `data_dir` — run by `main` right after `Load`, so an unusable `data_dir` refuses boot before anything dials out. It refuses an empty value (reachable through `WH_DATA_DIR=`) outright rather than probing the working directory; a path that exists and is not a directory; a dangling symlink at `data_dir` or any component above it (the walk to the nearest existing ancestor uses `Lstat`, so a failed mount is not skipped over as "does not exist"); and a directory the process cannot write to — or, when it does not exist, an unwritable nearest ancestor — probed by creating and removing one temp file. A permission denial, on the probe or on reaching the path through a parent without search permission, carries the UID-65532 hint, since a bind mount owned by root is the typical cause. +- **config.go** — Loads *boot* configuration from a YAML file with environment variable overrides (using [cleanenv](https://github.com/ilyakaznacheev/cleanenv)); every key has a `WH_`-prefixed env var. Boot config is only what can't change under a running process — resource sizing, listeners, observability exporters, the settings-directory path, and the secrets (`clickhouse.password`, `auth.jwt_secret`, `auth.operator_key`). Everything tenant-tunable lives in the settings directory (`settings/`). Both sources are strict: `Load` refuses to boot naming every YAML key the struct doesn't declare (`strict.go`) and every `WH_*` environment variable no field binds (`check.go`), so a tunable that moved to the settings directory can't be read, ignored, and believed. Boot is the validator for this half — there is no dry-run command. See [Configuration Reference](/configuration). +- **check.go** — `rejectUnboundEnv` is the environment half of the strict loader: `unboundEnv` walks the struct's `env` tags (plus the two process-level names, `WH_CONFIG` and `WH_LOG_LEVEL`) against the environment; `CheckDataDir` probes `data_dir` — run by `main` right after `Load`, so an unusable `data_dir` refuses boot before anything dials out. It refuses an empty value (reachable through `WH_DATA_DIR=`) outright rather than probing the working directory; a path that exists and is not a directory; a dangling symlink at `data_dir` or any component above it (the walk to the nearest existing ancestor uses `Lstat`, so a failed mount is not skipped over as "does not exist"); and a directory the process cannot write to — or, when it does not exist, an unwritable nearest ancestor — probed by creating and removing one temp file. A permission denial, on the probe or on reaching the path through a parent without search permission, carries the UID-65532 hint, since a bind mount owned by root is the typical cause. +- **strict.go** — `rejectUnknownKeys`, the YAML half: re-reads the file as a generic tree and walks it against the struct's `yaml` tags, listing every key the struct doesn't declare. cleanenv itself is lenient by design, which is exactly wrong for boot config once keys have moved to the settings directory. +- **persistence.go** — `WarnIfFreshDataDir` logs the startup `WARN` when `data_dir` is missing or empty (on a redeploy, the sign that the volume didn't persist); `LogStorageInitError` attaches the UID-65532 `permissionHint` to a NATS or Pebble open failure that looks like a permission denial — the same hint string `CheckDataDir` uses. ### `dedupe/` — Deduplication (Optional) diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx index 08017c84..791f15ba 100644 --- a/docs/src/content/docs/configuration.mdx +++ b/docs/src/content/docs/configuration.mdx @@ -16,7 +16,7 @@ WaveHouse is configured via a YAML file with environment variable overrides. All 1. If a config file exists at the specified path (default: `config.yaml`), it is loaded first. 2. Environment variables override any values from the YAML file. 3. If no config file exists, all values are read from environment variables. Every key has a default except `settings.dir` (`WH_SETTINGS_DIR`), which must be set either way. -4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way. Two variables have no YAML key and are exempt because they are not config keys at all but process-level settings `main` reads directly: `WH_CONFIG` (below), which locates the file, and `WH_LOG_LEVEL`. Only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix. Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod, for each Service that existed before the pod started. A Service named `wh` or `wh-*` therefore produces `WH_SERVICE_HOST`, `WH_PORT`, and more, and the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else. The error says so. +4. Both sources are **strict**. A YAML key this page doesn't list — a typo, or a tunable that has moved to the settings directory (`dlq.enabled`, `clickhouse.addr`, `stream.*`, a leftover `policy:` or `pipes:` block, …) — refuses to boot and names every offending key, so nothing is read, ignored, and believed. A `WH_*` environment variable that binds to no key on this page (`WH_DEDUPE_ENABLED`, `WH_CH_ADDR`, a misspelling) refuses to boot the same way. Two variables have no YAML key and are exempt because they are not config keys at all but process-level settings `main` reads directly: `WH_CONFIG` (below), which locates the file, and `WH_LOG_LEVEL`. Only the `WH_` prefix is checked, since the environment always carries names that aren't WaveHouse's. One outside source does share the prefix. Kubernetes injects `{SERVICE}_SERVICE_HOST`, `{SERVICE}_PORT`, and similar link variables into every pod in a Service's own namespace, for each Service with a cluster IP that existed before the pod started (a headless Service injects nothing, and a Service in another namespace is harmless). The name is uppercased with `-` mapped to `_`, so a Service named `wh` produces `WH_SERVICE_HOST` and `WH_PORT`, one named `wh-foo` produces `WH_FOO_SERVICE_HOST` and `WH_FOO_PORT`, and either way the pod refuses to boot on its next restart. Set `enableServiceLinks: false` on the pod spec, or name the Service something else. The error says so. 5. Before anything dials out, `data_dir` is probed, and boot refuses on any of these: the value is empty; the path exists but is not a directory; the path, or any component above it, is a dangling symlink (a mount that never came up); the directory exists but the process cannot write to it; the directory is absent and its nearest existing ancestor is not writable, so it could not be created. The probe runs before ClickHouse discovery, so the refusal lands at the top of the log, and a permission denial — on the write probe, or on reaching the path at all through a parent without search permission — carries the UID-65532 remediation, since a bind mount owned by root is the typical cause. Boot is the validator for this half of configuration: there is no dry run, and a refused boot with the offending key, variable, or path named in the error is the loud signal. The hot-reloadable half has a dry run — `wavehouse validate` — because it is edited under a running server; boot config only ever takes effect through a restart, so the restart is where it is checked. diff --git a/docs/src/content/docs/deployment.md b/docs/src/content/docs/deployment.md index 002dcd7b..1a478570 100644 --- a/docs/src/content/docs/deployment.md +++ b/docs/src/content/docs/deployment.md @@ -306,6 +306,8 @@ readinessProbe: httpGet: { path: /readyz, port: 8080 } ``` +Don't name WaveHouse's own Service `wh` or `wh-*`. kubelet injects `WH_SERVICE_HOST` and friends into every pod in the namespace started after such a Service exists, WaveHouse's own pods included, and the strict environment check refuses them — not at deploy time, but on the next restart. Any Service in the namespace named that way has the same effect; `enableServiceLinks: false` on the pod spec turns the injection off. See [Configuration → Loading Order](/configuration#loading-order). + Until `startupProbe` succeeds, kubelet doesn't run `livenessProbe` or `readinessProbe` against the pod — so a slow or temporarily-unreachable ClickHouse can't restart-loop the pod via the liveness path. Size `failureThreshold` to your expected worst-case CH boot time; the default 30 × 10s = 5min is generous and works for compose-on-NAS-style deployments where CH and WaveHouse can race during a host reboot. ## Behind a reverse proxy @@ -344,7 +346,7 @@ Three audits belong **before** the drain, because none of them announces itself - **`Nullable(T) DEFAULT …` columns now store `NULL` where they took their default.** A positional row has one slot per insertable column and no way to say *absent*, so a key the record omits rides as an explicit `null`. `input_format_null_as_default=1` turns that back into the default for a **non-nullable** column, but ClickHouse stores `NULL` on a nullable one whatever the setting says — only an absent key ever took the default. Following this runbook exactly still changes what lands in those columns, silently. See [the ingest note](/ingest-pipeline#the-journey-of-one-event). - **Policy `check` blocks are now validated against the table.** A `check` naming a column the table lacks, one it computes (`MATERIALIZED`/`ALIAS`), or an `EPHEMERAL` one is a per-record `403` on *every* insert by that role. `wavehouse validate` cannot catch it — it never sees the ClickHouse schema — so audit them against their tables first. See [Access control → Insert checks](/access-control#insert-checks). -- **Every `WH_*` variable the binary no longer reads refuses boot.** The tunables that moved to the settings directory this release — the ClickHouse wiring other than the password, `dedupe.*`, `dlq.*`, `stream.*`, `mq.*`, `query.*`, `schema.*`, `cors.*`, `auth.role_claim`, and the old policy-file and pipes-directory paths — used to be `WH_*` variables. The old binary ignored a leftover; the new one names every variable it does not bind and exits before it opens the queue, so a pod spec or compose file that still carries one comes back from the upgrade as a container that will not start. Diff the environment against the [Configuration Reference](/configuration) first, and move each leftover to its [`config.json` key](/settings-directory#configjson-keys) or drop it. A Kubernetes Service named `wh` or `wh-*` counts too: it injects link variables under the `WH_` prefix (`WH_SERVICE_HOST` and `WH_PORT` for `wh`, `WH_FOO_SERVICE_HOST` and `WH_FOO_PORT` for `wh-foo`), so set `enableServiceLinks: false` on the pod spec. +- **Every `WH_*` variable the binary does not bind refuses boot.** The old binary ignored a variable it did not read; the new one names every unbound one and exits before it opens the queue, so a pod spec or compose file that still carries one comes back from the upgrade as a container that will not start. Diff the environment against the [Configuration Reference](/configuration) first: a `WH_*` variable that is not in its tables is unbound, and whatever it used to configure now lives in the [settings directory](/settings-directory) or is gone. A Kubernetes Service in the pod's namespace named `wh` or `wh-*` counts too: it injects link variables under the `WH_` prefix (`WH_SERVICE_HOST` and `WH_PORT` for `wh`, `WH_FOO_SERVICE_HOST` and `WH_FOO_PORT` for `wh-foo`), so set `enableServiceLinks: false` on the pod spec. To drain before upgrading: diff --git a/internal/config/check.go b/internal/config/check.go index 279f2a6e..2436a9f2 100644 --- a/internal/config/check.go +++ b/internal/config/check.go @@ -27,7 +27,7 @@ func rejectUnboundEnv(environ []string) error { if len(unbound) == 0 { return nil } - return fmt.Errorf("unbound environment variable(s): %s — a typo, or a key that moved to the settings directory (%s); unset it, or rename it to a key the Config struct declares. On Kubernetes, a Service named wh or wh-* injects WH_SERVICE_HOST, WH_PORT, … into every pod started after it: set enableServiceLinks: false on the pod spec, or rename the Service", strings.Join(unbound, ", "), EnvSettingsDir) + return fmt.Errorf("unbound environment variable(s): %s — a typo, or a key that moved to the settings directory (%s); unset it, or rename it to a key the Config struct declares. On Kubernetes, a Service in the pod's namespace named wh or wh-* injects link variables under the WH_ prefix (WH_SERVICE_HOST and WH_PORT for wh, WH_FOO_SERVICE_HOST and WH_FOO_PORT for wh-foo, …) into every pod started after it: set enableServiceLinks: false on the pod spec, or rename the Service", strings.Join(unbound, ", "), EnvSettingsDir) } // unboundEnv returns, sorted, every WH_* name in environ (os.Environ() form, From 48ffc10468baf30a2655e6741ef6355b786b5df9 Mon Sep 17 00:00:00 2001 From: taitelee Date: Thu, 10 Sep 2026 12:13:36 -0400 Subject: [PATCH 9/9] fix(config): name the process-level variables in the unbound-env error --- internal/config/check.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/check.go b/internal/config/check.go index 2436a9f2..bf779530 100644 --- a/internal/config/check.go +++ b/internal/config/check.go @@ -27,7 +27,7 @@ func rejectUnboundEnv(environ []string) error { if len(unbound) == 0 { return nil } - return fmt.Errorf("unbound environment variable(s): %s — a typo, or a key that moved to the settings directory (%s); unset it, or rename it to a key the Config struct declares. On Kubernetes, a Service in the pod's namespace named wh or wh-* injects link variables under the WH_ prefix (WH_SERVICE_HOST and WH_PORT for wh, WH_FOO_SERVICE_HOST and WH_FOO_PORT for wh-foo, …) into every pod started after it: set enableServiceLinks: false on the pod spec, or rename the Service", strings.Join(unbound, ", "), EnvSettingsDir) + return fmt.Errorf("unbound environment variable(s): %s — a typo, or a key that moved to the settings directory (%s); unset it, or rename it to a variable the binary reads (a key the Config struct declares, or %s / %s). On Kubernetes, a Service in the pod's namespace named wh or wh-* injects link variables under the WH_ prefix (WH_SERVICE_HOST and WH_PORT for wh, WH_FOO_SERVICE_HOST and WH_FOO_PORT for wh-foo, …) into every pod started after it: set enableServiceLinks: false on the pod spec, or rename the Service", strings.Join(unbound, ", "), EnvSettingsDir, EnvConfig, EnvLogLevel) } // unboundEnv returns, sorted, every WH_* name in environ (os.Environ() form,