fix(config): keep an explicit false/0/"" from config.yaml

AGENTS.md

@@ -398,9 +398,9 @@ Internal-only backend changes (middleware refactors, observability internals, de
 
 ### Adding a new config option
 
-1. Add the field to the appropriate struct in `internal/config/config.go` with `yaml`, `env`, and `env-default` tags.
+1. Add the field to the appropriate struct in `internal/config/config.go` with `yaml` and `env` tags, and put a non-zero default in `defaults()` there. Never use cleanenv's `env-default` tag: it is applied after the YAML decode, so an explicit `false`/`0`/`""` in the file would be replaced by it (#631); `TestConfig_NoEnvDefaultTags` refuses it.
 2. Use the new config value in `internal/app/wire.go` or the relevant internal package.
-3. Document in `docs/src/content/docs/configuration.mdx`.
+3. Document in `docs/src/content/docs/configuration.mdx`, with a table row whose default matches `defaults()`; `TestDocs_DefaultsMatchCode` checks every field has one.
 
 ### Adding a new internal package
 

CHANGELOG.md

@@ -78,6 +78,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 ### Fixed
 
+- **An explicit `false`, `0` or `""` in `config.yaml` is no longer replaced by the key's default** (`internal/config/config.go`, `internal/config/defaults_test.go` (new), `docs/src/content/docs/configuration.mdx`, `AGENTS.md`): [#631](https://github.com/Wave-RF/WaveHouse/issues/631). Defaults lived in cleanenv `env-default` tags, which cleanenv applies after the YAML decode to any field still at its zero value, so it could not tell a key the file set to its zero value from one the file left out. `otel.traces.enabled: false`, `otel.metrics.enabled: false` and `otel.logs.enabled: false` came back `true`; `otel.traces.sample_rate: 0` and `otel.logs.sample_rate: 0` came back `1.0`; `server.shutdown_timeout: 0` came back `10`; `cache.l1_max_cost: 0`, `prometheus.path: ""` and `data_dir: ""` came back as their defaults; `server.port: 0` came back `8080`. All of it was silent. Defaults now live in one Go function, `defaults()`, which `Load` starts from before decoding the file and then applying `WH_*` variables, so the order is env > YAML > default and a key the file sets always wins. **Behaviour change if your file relied on the bug:** a zero you wrote now takes effect. A file that says `sample_rate: 0` now exports no traces (or no DEBUG/INFO logs), where it silently exported everything; a signal set `enabled: false` is now off; `shutdown_timeout: 0` now skips the drain. `cache.l1_max_cost: 0`, `server.port: 0`, and `data_dir: ""` now refuse boot (`cache init: MaxCost can't be zero`, `server.port 0 out of range`, `data_dir (WH_DATA_DIR) is required`) instead of running on the default; an empty `prometheus.path` refuses boot when `prometheus.enabled` is true. Delete the key to get the default back. Env vars are unchanged: they already honoured an explicit zero. New tests load through `config.Load` for every affected key (a YAML zero is kept, an absent key gets the default, env wins in both directions), refuse an `env-default` tag on any field, and pin each documented default in `configuration.mdx` to `defaults()`.
 - **An embedded queue store that cannot be created fails boot at once, naming the cause** (`internal/mq/embedded.go` (+ tests)): part of [#617](https://github.com/Wave-RF/WaveHouse/issues/617). A regular file at `<data_dir>/nats`, or a `nats` directory that could not be created there, failed JetStream in the background, so boot waited out the server's 5s readiness check and reported only `nats server not ready`. `NewEmbedded` now creates the directory first (at `0700`, as the server does) and refuses boot with the mkdir error. An existing but unwritable `nats` directory still takes the old path.
 - **An insert invalidates a table's cached results under every tenant the directory holds** (`internal/app/wire.go` (+ tests), `internal/settings/registry.go` (+ tests), `AGENTS.md`, `docs/src/content/docs/{deployment,architecture,ingest-pipeline}.md`): until [#583](https://github.com/Wave-RF/WaveHouse/issues/583) story 6 gives each tenant its own ClickHouse, every tenant reads the same tables, but the ingest worker — which writes every event as tenant `0`'s until story 5 — bumped only tenant `0`'s cache namespaces after an insert, so another tenant's cached query could answer stale rows for up to its TTL (an hour at most). The cache the worker invalidates through now fans each bumped namespace out to every tenant the registry knows (the new `Registry.Known`), the named one and a rejected one included — a rejected tenant comes back into service with the entries it has, so leaving it out would let a folder repaired inside a TTL serve pre-insert rows; reads are untouched, so a tenant is still never served another's cached rows. The residual, a folder removed and restored inside a TTL, is closed since #610: a tenant back on a pool after an absence has its structured-query results orphaned at once (`Cache.InvalidateTenant`). Raised by CodeRabbit on #602.
 - **The `?token=` strip no longer repairs a query string that does not parse** (`internal/auth/auth.go`): `bearerToken` removed a query-string token by parsing the query, deleting `token`, and re-encoding what was left — and `url.ParseQuery` skips a pair it cannot read, so the re-encoding erased that pair. A handler that parses the query strictly in order to refuse a malformed one would then see a clean query: `GET /v1/ops/pipes?tenant=acme;x=1&token=…` would have answered `200` with the default tenant's pipes. The token is read exactly as before and a query that parses is rewritten exactly as before; a query that does not parse now loses its token pairs and nothing else, byte for byte. Pinned through `api.NewRouter` with the real authenticator, since a handler-level test never runs the middleware that rewrote the URL.

docs/src/content/docs/configuration.mdx

@@ -14,7 +14,7 @@ WaveHouse is configured via a YAML file with environment variable overrides. All
 ## Loading Order
 
 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.
+2. Environment variables override any values from the YAML file. A key the file sets always wins over its default, including an explicit `false`, `0` or `""`: `otel.traces.enabled: false` turns traces off, and `otel.traces.sample_rate: 0` exports no traces. Only a key the file leaves out takes the default listed below.
 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 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.

internal/config/check.go

@@ -82,9 +82,10 @@ func collectEnvTags(t reflect.Type, into map[string]bool) {
 // 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.
+// ownership and mode. A blank dir — reachable through `WH_DATA_DIR=` or
+// `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")

internal/config/config.go

@@ -16,7 +16,7 @@ type Config struct {
 	// Subdirectory names are conventions, not config — one knob, one mount.
 	// In a container this MUST resolve to a host-backed volume; the relative
 	// `./data` default is fine for local binary use only.
-	DataDir    string     `yaml:"data_dir" env:"WH_DATA_DIR" env-default:"./data"`
+	DataDir    string     `yaml:"data_dir" env:"WH_DATA_DIR"`
 	Server     Server     `yaml:"server"`
 	ClickHouse ClickHouse `yaml:"clickhouse"`
 	Cache      Cache      `yaml:"cache"`
@@ -63,19 +63,19 @@ type Settings struct {
 // variables read by the OpenTelemetry SDK, not WaveHouse config. See
 // docs/src/content/docs/configuration.mdx.
 type OTel struct {
-	Enabled bool        `yaml:"enabled" env:"WH_OTEL_ENABLED" env-default:"false"`
+	Enabled bool        `yaml:"enabled" env:"WH_OTEL_ENABLED"`
 	Traces  OTelTraces  `yaml:"traces"`
 	Metrics OTelMetrics `yaml:"metrics"`
 	Logs    OTelLogs    `yaml:"logs"`
 }
 
 type OTelTraces struct {
-	Enabled    bool    `yaml:"enabled" env:"WH_OTEL_TRACES_ENABLED" env-default:"true"`
-	SampleRate float64 `yaml:"sample_rate" env:"WH_OTEL_TRACES_SAMPLE_RATE" env-default:"1.0"`
+	Enabled    bool    `yaml:"enabled" env:"WH_OTEL_TRACES_ENABLED"`
+	SampleRate float64 `yaml:"sample_rate" env:"WH_OTEL_TRACES_SAMPLE_RATE"`
 }
 
 type OTelMetrics struct {
-	Enabled bool `yaml:"enabled" env:"WH_OTEL_METRICS_ENABLED" env-default:"true"`
+	Enabled bool `yaml:"enabled" env:"WH_OTEL_METRICS_ENABLED"`
 }
 
 // Prometheus controls a Prometheus exposition endpoint served alongside (or
@@ -94,9 +94,9 @@ type OTelMetrics struct {
 // port spins up a dedicated HTTP listener — useful for firewalling metrics
 // off the public API surface in production.
 type Prometheus struct {
-	Enabled bool   `yaml:"enabled" env:"WH_PROMETHEUS_ENABLED" env-default:"false"`
-	Path    string `yaml:"path" env:"WH_PROMETHEUS_PATH" env-default:"/metrics"`
-	Port    int    `yaml:"port" env:"WH_PROMETHEUS_PORT" env-default:"0"`
+	Enabled bool   `yaml:"enabled" env:"WH_PROMETHEUS_ENABLED"`
+	Path    string `yaml:"path" env:"WH_PROMETHEUS_PATH"`
+	Port    int    `yaml:"port" env:"WH_PROMETHEUS_PORT"`
 }
 
 // OTelLogs sample rate applies to OTLP export of DEBUG/INFO only.
@@ -105,16 +105,16 @@ type Prometheus struct {
 // records regardless of this rate (sampling for scraped-log pipelines like
 // Loki/Promtail belongs at the scraper, not the application).
 type OTelLogs struct {
-	Enabled    bool    `yaml:"enabled" env:"WH_OTEL_LOGS_ENABLED" env-default:"true"`
-	SampleRate float64 `yaml:"sample_rate" env:"WH_OTEL_LOGS_SAMPLE_RATE" env-default:"1.0"`
+	Enabled    bool    `yaml:"enabled" env:"WH_OTEL_LOGS_ENABLED"`
+	SampleRate float64 `yaml:"sample_rate" env:"WH_OTEL_LOGS_SAMPLE_RATE"`
 }
 
 // Server holds listener wiring. The CORS allowlist is a tenant tunable and
 // lives in the settings directory's config.json (internal/settings), as do
 // the SSE keepalive and gap-window knobs (stream.*).
 type Server struct {
-	Port            int `yaml:"port" env:"WH_SERVER_PORT" env-default:"8080"`
-	ShutdownTimeout int `yaml:"shutdown_timeout" env:"WH_SERVER_SHUTDOWN_TIMEOUT" env-default:"10"`
+	Port            int `yaml:"port" env:"WH_SERVER_PORT"`
+	ShutdownTimeout int `yaml:"shutdown_timeout" env:"WH_SERVER_SHUTDOWN_TIMEOUT"`
 }
 
 // ClickHouse holds the password and the connection ceiling. The wiring —
@@ -129,14 +129,14 @@ type ClickHouse struct {
 	// MaxTotalConns caps the native connections the process may hold open
 	// across its pools: the settings directory's clickhouse.max_open_conns
 	// must not exceed it. 0, the default, is no ceiling.
-	MaxTotalConns int `yaml:"max_total_conns" env:"WH_CH_MAX_TOTAL_CONNS" env-default:"0"`
+	MaxTotalConns int `yaml:"max_total_conns" env:"WH_CH_MAX_TOTAL_CONNS"`
 }
 
 // Cache sizes the in-process L1 cache. The time-range bucket structured
 // queries normalize to is a settings-directory key
 // (query.timestamp_bucket_seconds) — query shaping, not process memory.
 type Cache struct {
-	L1MaxCost int64 `yaml:"l1_max_cost" env:"WH_CACHE_L1_MAX_COST" env-default:"67108864"`
+	L1MaxCost int64 `yaml:"l1_max_cost" env:"WH_CACHE_L1_MAX_COST"`
 }
 
 // Auth holds the authentication secrets. The verifier wiring — `jwks_url`,
@@ -159,6 +159,27 @@ type Auth struct {
 	OperatorKey string `yaml:"operator_key" env:"WH_AUTH_OPERATOR_KEY"`
 }
 
+// defaults is the one definition of every boot-config default: Load starts
+// from it, then decodes the YAML over it, then applies WH_* variables over
+// that. A key the file sets — to false, 0 or "" too — therefore wins over its
+// default, which an `env-default` tag cannot do: cleanenv applies those after
+// the decode, to any field still zero, so it can't tell an explicit zero from
+// an absent key (#631). A key absent here defaults to its zero value.
+// configuration.mdx documents these; a config test pins the two together.
+func defaults() Config {
+	return Config{
+		DataDir: "./data",
+		Server:  Server{Port: 8080, ShutdownTimeout: 10},
+		Cache:   Cache{L1MaxCost: 64 << 20},
+		OTel: OTel{
+			Traces:  OTelTraces{Enabled: true, SampleRate: 1.0},
+			Metrics: OTelMetrics{Enabled: true},
+			Logs:    OTelLogs{Enabled: true, SampleRate: 1.0},
+		},
+		Prometheus: Prometheus{Path: "/metrics"},
+	}
+}
+
 // Validate checks the loaded configuration for logical consistency.
 func (c *Config) Validate() error {
 	if c.Server.Port < 1 || c.Server.Port > 65535 {
@@ -239,7 +260,7 @@ func Load(path string) (*Config, error) {
 	if err := rejectUnboundEnv(os.Environ()); err != nil {
 		return nil, err
 	}
-	var cfg Config
+	cfg := defaults()
 	if _, err := os.Stat(path); err == nil {
 		if err := cleanenv.ReadConfig(path, &cfg); err != nil {
 			return nil, fmt.Errorf("read config: %w", err)