Load every setting from one table in internal/config (closes #320)

Config.settings lists each setting once: its key, its environment variable
and the function that loads it. Most rows load through field, whose
arguments give the type, the default and the check. db_url and
cache_max_bytes keep their own load functions. Loading, the unknown-key
and unknown-variable checks and the names in error messages all read the
table. allowlist_hosts now loads through parseHostList.
isKnownConfigKey, envVarNames, strictLoader, validate,
validateAllowlistHostsValue and getStringSlice are gone, with the
getStringSlice tests. The key constants move to the tests, their only
users. README.md lists the settings one item each instead of in a table,
with shorter sentences above the list. A new test checks that this list
and configs/config.example.yml name exactly the keys of the table.

Model: opus-5-5

README.md

@@ -133,7 +133,7 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
       filesystem's free space and what the cache holds, and at least 500 MiB, so
       the cache can grow to three quarters or more of the space they all have
       left, and to all of it when that is 500 MiB or less.
-    - the rest are in the table under Configuration below
+    - the rest are in the list under Configuration below
 - **Memory:** pixa can use up to `max_concurrent_processing` times 4.2 GiB for
   the images it processes at once, plus `upstream_connections` times
   `upstream_max_response_size` for the source images it has fetched that wait
@@ -678,54 +678,85 @@ refused with 400.
 Every setting can be given as an environment variable, in a YAML config file
 (`--config`), or both. A variable present in the environment wins over the file,
 even when it is empty, and the file wins over the built-in default. The one
-exception is a variable named in the file's `env:` section: it is set while the
+exception is a variable set in the file's `env:` section. pixa sets that
-file loads, so it overrides both the environment the process was started with
+variable while loading the file, so the variable wins over the environment pixa
-and the file's own key. A variable's value is parsed as the same text in the
+started with. It also wins over the file's key for the same setting. A
-file would be. The three lists take comma-separated entries, with the spaces
+variable's value is parsed as the same text in the file would be. A setting that
-around each trimmed; an empty variable is an empty list. A value that does not
+holds a list takes comma-separated entries, with the spaces around each trimmed.
-parse or is invalid aborts startup, naming the variable. A variable whose name
+An empty variable is an empty list. A value that does not parse or is invalid
-starts with `PIXA_` but is not in the table below, such as a misspelled one or
+aborts startup, naming the variable. pixa refuses to start when a variable
-`PIXA_PORT`, aborts startup naming it, as an unknown config key does. The one
+starts with `PIXA_` but is not in the list below, such as a misspelled one or
-other accepted name is `PIXA_CONFIG_PATH`, the config file's path (like
+`PIXA_PORT`. The error names the variable, as it names an unknown config key.
-`--config`). The variables set by the file's `env:` section are checked the same
+pixa also accepts `PIXA_CONFIG_PATH`, which names the config file, like
+`--config`. The variables set by the file's `env:` section are checked the same
 way.
 
-pixa reads at most one config file: the one given with `--config` (or `-c`),
+pixa reads at most one config file. It reads the file given with `--config` (or
-otherwise the one `PIXA_CONFIG_PATH` names, otherwise the first of these that
+`-c`), or else the one `PIXA_CONFIG_PATH` names. pixa refuses to start when that
-pixa finds: `/etc/pixa/config.yml`, `/etc/pixa/config.yaml`,
+file does not exist, cannot be read or does not parse. Without `--config` or
-`~/.config/pixa/config.yml`, `~/.config/pixa/config.yaml`, then `config.yml` and
+`PIXA_CONFIG_PATH`, pixa looks in `/etc/pixa/`, then in `~/.config/pixa/`, then
-`config.yaml` in the working directory. A named file that does not exist, cannot
+in the working directory. In each directory it looks for `config.yml`, then
-be read or does not parse aborts startup. Of the files pixa looks for on its
+`config.yaml`. It reads the first file it finds. It skips a missing file without
-own, only one that does not exist is passed over, without a message. One that
+a message. pixa refuses to start when a file it finds cannot be read or does not
-pixa cannot read or parse aborts startup, naming the file. So does one in a
+parse, and the error names the file. It also refuses to start when it may not
-directory pixa may not enter, whether or not it is there, since pixa cannot
+enter one of these directories, whether or not the file is there, since it
-tell. With no file, pixa uses the environment and the defaults.
+cannot tell. With no file, pixa uses the environment and the defaults.
 
-| Variable                             | Config key                      | Meaning                                                                      |
+Each item below is one setting, named by its config key:
-| ------------------------------------ | ------------------------------- | ---------------------------------------------------------------------------- |
+
-| `PIXA_SIGNING_KEY`                   | `signing_key`                   | Required: secret for signed and encrypted URLs and login, 32+ characters     |
+- `signing_key`: variable `PIXA_SIGNING_KEY`, required. The secret for signed
-| `PORT`                               | `port`                          | Port to listen on; default `8080`                                            |
+  URLs, encrypted URLs and login. It must be 32 characters or longer.
-| `PIXA_STATE_DIR`                     | `state_dir`                     | Directory for the database and the disk cache; default `/var/lib/pixa`       |
+- `port`: variable `PORT`, default `8080`. The port to listen on.
-| `PIXA_DB_URL`                        | `db_url`                        | SQLite database URL; default `state.sqlite3` in the state directory          |
+- `state_dir`: variable `PIXA_STATE_DIR`, default `/var/lib/pixa`. The directory
-| `PIXA_CACHE_MAX_BYTES`               | `cache_max_bytes`               | Disk cache limit in bytes; `0` disables it; default 75% of (free + cached)   |
+  for the database and the disk cache.
-| `PIXA_ALLOWLIST_HOSTS`               | `allowlist_hosts`               | Upstream hosts served without a signature                                    |
+- `db_url`: variable `PIXA_DB_URL`, default `state.sqlite3` in the state
-| `PIXA_REFERER_BLOCKLIST`             | `referer_blocklist`             | Hosts whose pages the image routes refuse with 403, by `Referer`             |
+  directory. The URL of the SQLite database.
-| `PIXA_BLOCKED_NETWORKS`              | `blocked_networks`              | CIDR ranges never fetched from, on top of the built-in ones                  |
+- `cache_max_bytes`: variable `PIXA_CACHE_MAX_BYTES`, default 75% of the sum of
-| `PIXA_TRUSTED_PROXIES`               | `trusted_proxies`               | CIDR ranges of proxies whose `X-Forwarded-For` is believed; default RFC 1918 |
+  the free space and what the cache holds. The disk cache limit in bytes. `0`
-| `PIXA_ALLOW_HTTP`                    | `allow_http`                    | Allow plain-HTTP upstreams, for testing only; default `false`                |
+  turns the disk cache off.
-| `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20`                       |
+- `allowlist_hosts`: variable `PIXA_ALLOWLIST_HOSTS`, default none. The upstream
-| `PIXA_UPSTREAM_CONNECTIONS`          | `upstream_connections`          | Concurrent connections to all upstream hosts together; default `64`          |
+  hosts whose images pixa serves without a signature.
-| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default half the CPUs, at least 1                  |
+- `referer_blocklist`: variable `PIXA_REFERER_BLOCKLIST`, default none. The
-| `PIXA_IMAGE_REQUESTS_PER_MINUTE`     | `image_requests_per_minute`     | Image requests per client per minute; `0` turns it off; default `600`        |
+  image routes refuse a request with 403 when its `Referer` header names one of
-| `PIXA_UPSTREAM_FETCH_TIMEOUT`        | `upstream_fetch_timeout`        | Time allowed for one fetch from an upstream host; default `30s`              |
+  these hosts.
-| `PIXA_UPSTREAM_MAX_RESPONSE_SIZE`    | `upstream_max_response_size`    | Largest upstream response accepted, in bytes; default 50 MiB                 |
+- `blocked_networks`: variable `PIXA_BLOCKED_NETWORKS`, default none. The CIDR
-| `PIXA_DOWNSTREAM_TIMEOUT`            | `downstream_timeout`            | Time allowed for answering one client request; default `60s`                 |
+  ranges pixa never fetches from, on top of the built-in ones.
-| `PIXA_ACCESS_CONTROL_ALLOW_ORIGIN`   | `access_control_allow_origin`   | CORS origin allowed to read image responses: `*` or one origin; default `*`  |
+- `trusted_proxies`: variable `PIXA_TRUSTED_PROXIES`, default the RFC 1918
-| `PIXA_METRICS_USERNAME`              | `metrics.username`              | Username for `/metrics`, which is served only when both are set              |
+  ranges. The CIDR ranges of the proxies whose `X-Forwarded-For` header pixa
-| `PIXA_METRICS_PASSWORD`              | `metrics.password`              | Password for `/metrics`; set together with the username                      |
+  believes.
-| `PIXA_SENTRY_DSN`                    | `sentry_dsn`                    | Sentry DSN for error reporting; empty disables it                            |
+- `allow_http`: variable `PIXA_ALLOW_HTTP`, default `false`. Allows plain-HTTP
-| `PIXA_DEBUG`                         | `debug`                         | Debug logging and plain-HTTP local development; default `false`              |
+  upstream hosts. It is for testing only.
-| `PIXA_MAINTENANCE_MODE`              | `maintenance_mode`              | Answer image requests with 503; the health check stays 200; default `false`  |
+- `upstream_connections_per_host`: variable
+  `PIXA_UPSTREAM_CONNECTIONS_PER_HOST`, default `20`. The most connections to
+  one upstream host at once.
+- `upstream_connections`: variable `PIXA_UPSTREAM_CONNECTIONS`, default `64`.
+  The most connections to all upstream hosts together at once.
+- `max_concurrent_processing`: variable `PIXA_MAX_CONCURRENT_PROCESSING`,
+  default half the CPUs, at least 1. The most images processed at once.
+- `image_requests_per_minute`: variable `PIXA_IMAGE_REQUESTS_PER_MINUTE`,
+  default `600`. How many image requests one client may send per minute. `0`
+  turns the limit off.
+- `upstream_fetch_timeout`: variable `PIXA_UPSTREAM_FETCH_TIMEOUT`, default
+  `30s`. The time allowed for one fetch from an upstream host.
+- `upstream_max_response_size`: variable `PIXA_UPSTREAM_MAX_RESPONSE_SIZE`,
+  default 50 MiB. The largest upstream response accepted, in bytes.
+- `downstream_timeout`: variable `PIXA_DOWNSTREAM_TIMEOUT`, default `60s`. The
+  time allowed for answering one client request.
+- `access_control_allow_origin`: variable `PIXA_ACCESS_CONTROL_ALLOW_ORIGIN`,
+  default `*`. The CORS origin allowed to read image responses. It is `*` or one
+  origin.
+- `metrics.username`: variable `PIXA_METRICS_USERNAME`, default empty. The
+  username for `/metrics`. pixa serves `/metrics` only when the username and the
+  password are both set.
+- `metrics.password`: variable `PIXA_METRICS_PASSWORD`, default empty. The
+  password for `/metrics`. Set it together with the username.
+- `sentry_dsn`: variable `PIXA_SENTRY_DSN`, default empty. The Sentry DSN for
+  error reporting. An empty DSN turns error reporting off.
+- `debug`: variable `PIXA_DEBUG`, default `false`. Turns on debug logging and
+  plain-HTTP local development.
+- `maintenance_mode`: variable `PIXA_MAINTENANCE_MODE`, default `false`. While
+  `true`, pixa answers image requests with 503. The health check still
+  answers 200.
 
 Key settings in more detail:
 

TODO.md

@@ -30,6 +30,30 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-26 one table in `internal/config` lists every setting once (closes
+  #320). `Config.settings` gives each setting's key, its environment variable
+  and the function that loads it.
+    - Most rows load through `field`, whose arguments give the setting's type,
+      its default and its check.
+    - `db_url` and `cache_max_bytes` have their own load functions. The default
+      of `db_url` comes from `state_dir`, and `cache_max_bytes` records whether
+      it is set.
+    - Loading, the unknown-key check, the unknown-variable check and the names
+      in error messages all read the table.
+    - `allowlist_hosts` loads as `referer_blocklist` does, through
+      `parseHostList`.
+    - The metrics username and password are checked together once the table has
+      loaded.
+    - When a config holds more than one invalid value, the error names the one
+      that comes first in the table.
+    - `README.md` lists the settings one item each, where it had a table. A test
+      checks that this list and `configs/config.example.yml` name exactly the
+      keys of the table.
+    - The key constants moved to the tests, their only users.
+    - Removed:
+        - `isKnownConfigKey`, `envVarNames`, `strictLoader` and `validate`
+        - `validateAllowlistHostsValue` and `getStringSlice`, with the three
+          tests of `getStringSlice`
 - 2026-10-26 a fetched image's format is detected from its bytes once, as an
   `imagerequest.Format` (closes #319). The upstream `Content-Type` check in the
   fetcher stays as the filter before downloading.

internal/config/config_internal_test.go

@@ -1,97 +1,12 @@
 package config
 
 import (
-	"os"
 	"path/filepath"
 	"testing"
 
 	"git.eeqj.de/sneak/smartconfig"
 )
 
-// writeTestConfig writes yamlContent to a temp config file and returns
-// the file path.
-func writeTestConfig(t *testing.T, yamlContent string) string {
-	t.Helper()
-
-	tmpDir := t.TempDir()
-	configPath := filepath.Join(tmpDir, "config.yml")
-
-	err := os.WriteFile(configPath, []byte(yamlContent), 0o600)
-	if err != nil {
-		t.Fatalf("failed to write test config: %v", err)
-	}
-
-	return configPath
-}
-
-// checkAllowlistHosts loads the config at configPath and asserts that
-// getStringSlice returns the three expected hosts.
-func checkAllowlistHosts(t *testing.T, configPath string) {
-	t.Helper()
-
-	sc, err := loadTestConfig(configPath)
-	if err != nil {
-		t.Fatalf("failed to load config: %v", err)
-	}
-
-	hosts := getStringSlice(sc)
-
-	if len(hosts) != 3 {
-		t.Errorf("expected 3 hosts, got %d: %v", len(hosts), hosts)
-	}
-
-	expected := []string{"static.sneak.cloud", "sneak.berlin", testHostS3}
-	for i, want := range expected {
-		if i >= len(hosts) {
-			t.Errorf("missing host at index %d: want %q", i, want)
-
-			continue
-		}
-
-		if hosts[i] != want {
-			t.Errorf("host[%d] = %q, want %q", i, hosts[i], want)
-		}
-	}
-}
-
-func TestGetStringSlice_YAMLList(t *testing.T) {
-	t.Parallel()
-
-	yamlContent := `
-allowlist_hosts:
-  - static.sneak.cloud
-  - sneak.berlin
-  - s3.sneak.cloud
-`
-
-	checkAllowlistHosts(t, writeTestConfig(t, yamlContent))
-}
-
-func TestGetStringSlice_CommaSeparated(t *testing.T) {
-	t.Parallel()
-
-	// Backwards compatibility with comma-separated string values.
-	yamlContent := `allowlist_hosts: "static.sneak.cloud, sneak.berlin, s3.sneak.cloud"`
-
-	checkAllowlistHosts(t, writeTestConfig(t, yamlContent))
-}
-
-func TestGetStringSlice_Empty(t *testing.T) {
-	t.Parallel()
-
-	configPath := writeTestConfig(t, `port: 8080`)
-
-	sc, err := loadTestConfig(configPath)
-	if err != nil {
-		t.Fatalf("failed to load config: %v", err)
-	}
-
-	hosts := getStringSlice(sc)
-	if len(hosts) != 0 {
-		t.Errorf("expected nil or empty slice, got %v", hosts)
-	}
-}
-
 // TestExampleConfigAllowlistsNoHost loads configs/config.example.yml
 // with a real signing key, as Getting Started in README.md has it copied
 // and edited, and checks that it allowlists no host: every image on an

internal/config/config_validation_internal_test.go

@@ -15,8 +15,36 @@ import (
 	_ "modernc.org/sqlite" // SQLite driver registration
 )
 
+// The config keys the tests name in the errors they expect.
+const (
+	keyDebug                      = "debug"
+	keyMaintenanceMode            = "maintenance_mode"
+	keyPort                       = "port"
+	keyStateDir                   = "state_dir"
+	keySentryDSN                  = "sentry_dsn"
+	keyDBURL                      = "db_url"
+	keyMetrics                    = "metrics"
+	keyMetricsUsername            = "metrics.username"
+	keyMetricsPassword            = "metrics.password"
+	keySigningKey                 = "signing_key"
+	keyAllowlistHosts             = "allowlist_hosts"
+	keyRefererBlocklist           = "referer_blocklist"
+	keyAllowHTTP                  = "allow_http"
+	keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
+	keyUpstreamConnections        = "upstream_connections"
+	keyMaxConcurrentProcessing    = "max_concurrent_processing"
+	keyCacheMaxBytes              = "cache_max_bytes"
+	keyBlockedNetworks            = "blocked_networks"
+	keyTrustedProxies             = "trusted_proxies"
+	keyAccessControlAllowOrigin   = "access_control_allow_origin"
+	keyUpstreamFetchTimeout       = "upstream_fetch_timeout"
+	keyUpstreamMaxResponseSize    = "upstream_max_response_size"
+	keyDownstreamTimeout          = "downstream_timeout"
+	keyImageRequestsPerMinute     = "image_requests_per_minute"
+)
+
 // validTestSigningKey is a 32-character signing key that satisfies the
-// minimum length requirement in validate().
+// minimum length requirement in validateSigningKey.
 const validTestSigningKey = "0123456789abcdef0123456789abcdef"
 
 // signingKeyLine is a valid signing_key config line used as the base of

internal/config/documented_settings_internal_test.go

@@ -0,0 +1,127 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+)
+
+// TestReadmeNamesEverySetting checks that the list of settings in
+// README.md names exactly the keys of the settings table, one item each.
+// The list is the first one in the Configuration section, and it ends at
+// the first blank line. Each item starts with its key in backticks.
+func TestReadmeNamesEverySetting(t *testing.T) {
+	t.Parallel()
+
+	readme, err := os.ReadFile(filepath.Join("..", "..", "README.md"))
+	if err != nil {
+		t.Fatalf("failed to read README.md: %v", err)
+	}
+
+	_, section, found := strings.Cut(string(readme), "\n### Configuration\n")
+	if !found {
+		t.Fatal("README.md has no Configuration section")
+	}
+
+	// The section ends at the next heading of its level or above.
+	section, _, _ = strings.Cut(section, "\n## ")
+	section, _, _ = strings.Cut(section, "\n### ")
+
+	_, list, found := strings.Cut(section, "\n- ")
+	if !found {
+		t.Fatal("README.md's Configuration section has no list")
+	}
+
+	list, _, _ = strings.Cut("- "+list, "\n\n")
+
+	itemKey := regexp.MustCompile("^- `([^`]+)`")
+	named := map[string]bool{}
+
+	for line := range strings.SplitSeq(list, "\n") {
+		// A line that does not start with "- " goes on with the item above.
+		if !strings.HasPrefix(line, "- ") {
+			continue
+		}
+
+		match := itemKey.FindStringSubmatch(line)
+		if match == nil {
+			t.Errorf("README.md's list of settings has an item "+
+				"that does not start with a key in backticks: %s", line)
+
+			continue
+		}
+
+		if named[match[1]] {
+			t.Errorf("README.md's list of settings names %s twice", match[1])
+		}
+
+		named[match[1]] = true
+	}
+
+	for _, s := range new(Config).settings() {
+		if !named[s.key] {
+			t.Errorf("README.md's list of settings does not name %s", s.key)
+		}
+
+		delete(named, s.key)
+	}
+
+	for key := range named {
+		t.Errorf("README.md's list of settings names %s, "+
+			"which is not a setting", key)
+	}
+}
+
+// TestExampleConfigNamesEverySetting checks that
+// configs/config.example.yml names exactly the keys of the settings
+// table, each one set or commented out with "# ". A key line holds the
+// key, a colon and at most a one-word value, which tells it from a
+// comment line that starts with a word and a colon. An indented key is
+// inside the map of the key above it, as username is inside metrics.
+func TestExampleConfigNamesEverySetting(t *testing.T) {
+	t.Parallel()
+
+	const examplePath = "configs/config.example.yml"
+
+	example, err := os.ReadFile(filepath.Join("..", "..", examplePath))
+	if err != nil {
+		t.Fatalf("failed to read %s: %v", examplePath, err)
+	}
+
+	keyLine := regexp.MustCompile(`(?m)^(?:# )?( *)([a-z_]+):(?: \S+)?$`)
+	named := map[string]bool{}
+
+	var mapKey string
+
+	for _, match := range keyLine.FindAllStringSubmatch(string(example), -1) {
+		indent, key := match[1], match[2]
+		if indent == "" {
+			mapKey = key
+			named[key] = true
+
+			continue
+		}
+
+		// The key holding the map is not itself a setting.
+		delete(named, mapKey)
+
+		named[mapKey+"."+key] = true
+	}
+
+	// The env section sets environment variables; it is not a setting.
+	delete(named, "env")
+
+	for _, s := range new(Config).settings() {
+		if !named[s.key] {
+			t.Errorf("%s does not name %s", examplePath, s.key)
+		}
+
+		delete(named, s.key)
+	}
+
+	for key := range named {
+		t.Errorf("%s names %s, which is not a setting", examplePath, key)
+	}
+}

internal/config/env_internal_test.go

@@ -138,7 +138,8 @@ func TestPixaPortAbortsStartupPointingToPort(t *testing.T) {
 
 // TestSettingVariablesAndConfigPathAreAccepted checks that every
 // setting's variable and PIXA_CONFIG_PATH pass the check for unknown
-// PIXA_ variables. TestEnvironmentSetsEveryKey pins the names in the list.
+// PIXA_ variables. TestEnvironmentSetsEveryKey pins the names in the
+// settings table.
 func TestSettingVariablesAndConfigPathAreAccepted(t *testing.T) {
 	// A config file's env section loaded by another test can leave a
 	// PIXA_ variable set for the whole process, so every one is unset
@@ -159,8 +160,8 @@ func TestSettingVariablesAndConfigPathAreAccepted(t *testing.T) {
 
 	t.Setenv("PIXA_CONFIG_PATH", "/etc/pixa/config.yml")
 
-	for _, name := range envVarNames() {
+	for _, s := range new(Config).settings() {
-		t.Setenv(name, "")
+		t.Setenv(s.envVar, "")
 	}
 
 	err := validateKnownEnvVars()