From 252ef7bd60c9841e42aecd504ef24fbda8aa3bad Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Fri, 25 Sep 2026 04:20:37 -0400 Subject: [PATCH] fix(config): keep an explicit false/0/"" from config.yaml cleanenv applies env-default after the YAML decode to any field still at its zero value, so an explicit `otel.traces.enabled: false` or `sample_rate: 0` came back as the default. Defaults now live in one Go function that Load starts from before the decode; env-default is gone. Tests load through config.Load for every affected key, refuse an env-default tag, and pin configuration.mdx's defaults to defaults(). Fixes #631. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01EJr5tY4WQUy2sc4MbW67vL --- AGENTS.md | 4 +- CHANGELOG.md | 2 + docs/src/content/docs/configuration.mdx | 2 +- internal/config/check.go | 7 +- internal/config/config.go | 51 +++-- internal/config/defaults_test.go | 279 ++++++++++++++++++++++++ 6 files changed, 324 insertions(+), 21 deletions(-) create mode 100644 internal/config/defaults_test.go diff --git a/AGENTS.md b/AGENTS.md index 59f08ef36..f3a88840e 100644 --- a/AGENTS.md +++ b/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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bf580196..1d574dbbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,8 @@ 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 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. diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx index a9c9de1db..96ba593e5 100644 --- a/docs/src/content/docs/configuration.mdx +++ b/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. diff --git a/internal/config/check.go b/internal/config/check.go index bf779530b..771d7908c 100644 --- a/internal/config/check.go +++ b/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") diff --git a/internal/config/config.go b/internal/config/config.go index 68b0314b6..cfb199f27 100644 --- a/internal/config/config.go +++ b/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) diff --git a/internal/config/defaults_test.go b/internal/config/defaults_test.go new file mode 100644 index 000000000..164e179e7 --- /dev/null +++ b/internal/config/defaults_test.go @@ -0,0 +1,279 @@ +package config + +import ( + "fmt" + "os" + "path/filepath" + "reflect" + "regexp" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +// zeroCase is one key whose default is not its zero value (#631's table). +type zeroCase struct { + key string // dotted YAML path + env string + zero any // the zero value, as written to YAML and as Load must return it + def any // defaults() value, returned when the key is absent + envVal string // a non-default, non-zero value set through env + fromEnv any // envVal as Load must return it + get func(*Config) any +} + +// server.port is not here: 0 fails Validate, pinned by TestLoad_YAMLZeroPortIsRefused. +var zeroCases = []zeroCase{ + {"otel.traces.enabled", "WH_OTEL_TRACES_ENABLED", false, true, "false", false, func(c *Config) any { return c.OTel.Traces.Enabled }}, + {"otel.metrics.enabled", "WH_OTEL_METRICS_ENABLED", false, true, "false", false, func(c *Config) any { return c.OTel.Metrics.Enabled }}, + {"otel.logs.enabled", "WH_OTEL_LOGS_ENABLED", false, true, "false", false, func(c *Config) any { return c.OTel.Logs.Enabled }}, + {"otel.traces.sample_rate", "WH_OTEL_TRACES_SAMPLE_RATE", 0.0, 1.0, "0.25", 0.25, func(c *Config) any { return c.OTel.Traces.SampleRate }}, + {"otel.logs.sample_rate", "WH_OTEL_LOGS_SAMPLE_RATE", 0.0, 1.0, "0.25", 0.25, func(c *Config) any { return c.OTel.Logs.SampleRate }}, + {"server.shutdown_timeout", "WH_SERVER_SHUTDOWN_TIMEOUT", 0, 10, "3", 3, func(c *Config) any { return c.Server.ShutdownTimeout }}, + {"cache.l1_max_cost", "WH_CACHE_L1_MAX_COST", int64(0), int64(64 << 20), "1024", int64(1024), func(c *Config) any { return c.Cache.L1MaxCost }}, + {"prometheus.path", "WH_PROMETHEUS_PATH", "", "/metrics", "/prom", "/prom", func(c *Config) any { return c.Prometheus.Path }}, + {"data_dir", "WH_DATA_DIR", "", "./data", "/var/lib/wh", "/var/lib/wh", func(c *Config) any { return c.DataDir }}, +} + +// yamlAt renders a file setting key to value, plus otel.enabled: true so +// the test can tell the file was read. +func yamlAt(t *testing.T, key string, value any) string { + t.Helper() + tree := map[string]any{"otel": map[string]any{"enabled": true}} + node := tree + parts := strings.Split(key, ".") + for _, p := range parts[:len(parts)-1] { + sub, ok := node[p].(map[string]any) + if !ok { + sub = map[string]any{} + node[p] = sub + } + node = sub + } + node[parts[len(parts)-1]] = value + out, err := yaml.Marshal(tree) + require.NoError(t, err) + return string(out) +} + +func writeYAML(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + return path +} + +// TestLoad_YAMLZeroIsKept is the #631 regression: an explicit false/0/"" in +// the file must survive Load. It fails if defaults are re-applied after the +// decode — by an env-default tag or by any fill-the-zero-fields pass. +func TestLoad_YAMLZeroIsKept(t *testing.T) { + t.Parallel() + for _, tc := range zeroCases { + t.Run(tc.key, func(t *testing.T) { + t.Parallel() + cfg, err := Load(writeYAML(t, yamlAt(t, tc.key, tc.zero))) + require.NoError(t, err) + assert.Equal(t, tc.zero, tc.get(cfg)) + assert.True(t, cfg.OTel.Enabled, "the file was read") + }) + } +} + +// The issue's repro file, loaded whole: every zero it sets comes back as set. +func TestLoad_IssueReproFile(t *testing.T) { + t.Parallel() + cfg, err := Load(writeYAML(t, ` +settings: + dir: ./settings +otel: + enabled: true + traces: { enabled: false, sample_rate: 0 } + metrics: { enabled: false } + logs: { enabled: false, sample_rate: 0 } +cache: + l1_max_cost: 0 +server: + shutdown_timeout: 0 +prometheus: + path: "" +data_dir: "" +`)) + require.NoError(t, err) + assert.True(t, cfg.OTel.Enabled) + assert.False(t, cfg.OTel.Traces.Enabled) + assert.Zero(t, cfg.OTel.Traces.SampleRate) + assert.False(t, cfg.OTel.Metrics.Enabled) + assert.False(t, cfg.OTel.Logs.Enabled) + assert.Zero(t, cfg.OTel.Logs.SampleRate) + assert.Zero(t, cfg.Cache.L1MaxCost) + assert.Zero(t, cfg.Server.ShutdownTimeout) + assert.Empty(t, cfg.Prometheus.Path) + assert.Empty(t, cfg.DataDir) + assert.Equal(t, 8080, cfg.Server.Port, "a key the file leaves out still gets its default") +} + +func TestLoad_YAMLZeroPortIsRefused(t *testing.T) { + t.Parallel() + _, err := Load(writeYAML(t, "server:\n port: 0\n")) + require.ErrorContains(t, err, "server.port 0 out of range", "0 reaches Validate instead of becoming 8080") +} + +// A file that exists but leaves a key out gets the default, like no file. +func TestLoad_AbsentKeyGetsDefault(t *testing.T) { + t.Parallel() + for _, tc := range zeroCases { + t.Run(tc.key, func(t *testing.T) { + t.Parallel() + cfg, err := Load(writeYAML(t, "server:\n port: 9090\n")) + require.NoError(t, err) + assert.Equal(t, tc.def, tc.get(cfg)) + assert.Equal(t, 9090, cfg.Server.Port) + }) + } +} + +// Precedence env > YAML > default, both ways round: env sets a value over a +// YAML zero, and a zero over the default with no file key. Not parallel: +// t.Setenv. +func TestLoad_EnvWinsOverYAMLZeroAndDefault(t *testing.T) { + for _, tc := range zeroCases { + t.Run(tc.key+"/over yaml zero", func(t *testing.T) { + t.Setenv(tc.env, tc.envVal) + cfg, err := Load(writeYAML(t, yamlAt(t, tc.key, tc.zero))) + require.NoError(t, err) + assert.Equal(t, tc.fromEnv, tc.get(cfg)) + }) + t.Run(tc.key+"/zero over yaml value", func(t *testing.T) { + t.Setenv(tc.env, fmt.Sprint(tc.zero)) + cfg, err := Load(writeYAML(t, yamlAt(t, tc.key, tc.fromEnv))) + require.NoError(t, err) + assert.Equal(t, tc.zero, tc.get(cfg)) + }) + t.Run(tc.key+"/zero over default, no file", func(t *testing.T) { + t.Setenv(tc.env, fmt.Sprint(tc.zero)) + cfg, err := Load(filepath.Join(t.TempDir(), "absent.yaml")) + require.NoError(t, err) + assert.Equal(t, tc.zero, tc.get(cfg)) + }) + } +} + +// Every non-zero default must be in zeroCases, so a new one gets the +// regression coverage above rather than silently skipping it. +func TestZeroCases_CoverEveryNonZeroDefault(t *testing.T) { + t.Parallel() + covered := map[string]bool{"server.port": true} + for _, tc := range zeroCases { + covered[tc.key] = true + } + for _, f := range configFields(t) { + if !f.def.IsZero() { + assert.True(t, covered[f.key], "%s has a non-zero default but no zeroCases entry", f.key) + } + } +} + +// cleanenv's env-default is applied after the YAML decode, to any field still +// zero, which is the #631 bug. Defaults belong in defaults(). +func TestConfig_NoEnvDefaultTags(t *testing.T) { + t.Parallel() + for _, f := range configFields(t) { + _, has := f.tag.Lookup("env-default") + assert.False(t, has, "%s: move its env-default into defaults()", f.key) + } +} + +type configField struct { + key string + tag reflect.StructTag + def reflect.Value +} + +// configFields walks defaults() and returns every leaf with its dotted YAML +// path — the same tree rejectUnknownKeys walks. +func configFields(t *testing.T) []configField { + t.Helper() + var out []configField + var walk func(prefix string, v reflect.Value) + walk = func(prefix string, v reflect.Value) { + for i := range v.NumField() { + f := v.Type().Field(i) + key := strings.Split(f.Tag.Get("yaml"), ",")[0] + if prefix != "" { + key = prefix + "." + key + } + if f.Type.Kind() == reflect.Struct { + walk(key, v.Field(i)) + continue + } + out = append(out, configField{key: key, tag: f.Tag, def: v.Field(i)}) + } + } + walk("", reflect.ValueOf(defaults())) + require.NotEmpty(t, out) + return out +} + +// TestDocs_DefaultsMatchCode ties configuration.mdx's reference tables to +// defaults() and the env tags: every field has exactly one row, the row names +// its env var, and the documented default parses to the value in code. +func TestDocs_DefaultsMatchCode(t *testing.T) { + t.Parallel() + doc, err := os.ReadFile("../../docs/src/content/docs/configuration.mdx") + require.NoError(t, err) + type row struct{ env, def string } + rows := map[string][]row{} + re := regexp.MustCompile("(?m)^\\| `([a-z0-9_.]+)` \\| `(WH_[A-Z0-9_]+)` \\| ([^|]+?) \\|") + for _, m := range re.FindAllStringSubmatch(string(doc), -1) { + rows[m[1]] = append(rows[m[1]], row{m[2], m[3]}) + } + fields := configFields(t) + keys := map[string]bool{} + for _, f := range fields { + keys[f.key] = true + got := rows[f.key] + if !assert.Len(t, got, 1, "%s: want exactly one row in configuration.mdx", f.key) { + continue + } + assert.Equal(t, f.tag.Get("env"), got[0].env, "%s: env var", f.key) + assert.Equal(t, f.def.Interface(), parseDocDefault(t, f.key, got[0].def, f.def.Interface()), "%s: documented default", f.key) + } + for k := range rows { + assert.True(t, keys[k], "configuration.mdx documents %s, which the Config struct does not declare", k) + } +} + +// parseDocDefault reads a table cell as the type of like. +func parseDocDefault(t *testing.T, key, cell string, like any) any { + t.Helper() + cell = strings.TrimSpace(cell) + if cell == "*(empty)*" || cell == "*(required)*" { + cell = "" + } else { + cell = strings.Trim(cell, "`") + } + var ( + v any + err error + ) + switch like.(type) { + case string: + v = cell + case bool: + v, err = strconv.ParseBool(cell) + case int: + v, err = strconv.Atoi(cell) + case int64: + v, err = strconv.ParseInt(cell, 10, 64) + case float64: + v, err = strconv.ParseFloat(cell, 64) + default: + t.Fatalf("%s: no doc parser for %T", key, like) + } + require.NoError(t, err, "%s: documented default %q", key, cell) + return v +}