diff --git a/README.md b/README.md index 5e1437c..b748270 100644 --- a/README.md +++ b/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 -file loads, so it overrides both the environment the process was started with -and the file's own key. A variable's value is parsed as the same text in the -file would be. The three lists take comma-separated entries, with the spaces -around each trimmed; an empty variable is an empty list. A value that does not -parse or is invalid aborts startup, naming the variable. A variable whose name -starts with `PIXA_` but is not in the table below, such as a misspelled one or -`PIXA_PORT`, aborts startup naming it, as an unknown config key does. The one -other accepted name is `PIXA_CONFIG_PATH`, the config file's path (like -`--config`). The variables set by the file's `env:` section are checked the same +exception is a variable set in the file's `env:` section. pixa sets that +variable while loading the file, so the variable wins over the environment pixa +started with. It also wins over the file's key for the same setting. A +variable's value is parsed as the same text in the file would be. A setting that +holds a list takes comma-separated entries, with the spaces around each trimmed. +An empty variable is an empty list. A value that does not parse or is invalid +aborts startup, naming the variable. pixa refuses to start when a variable +starts with `PIXA_` but is not in the list below, such as a misspelled one or +`PIXA_PORT`. The error names the variable, as it names an unknown config key. +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`), -otherwise the one `PIXA_CONFIG_PATH` names, otherwise the first of these that -pixa finds: `/etc/pixa/config.yml`, `/etc/pixa/config.yaml`, -`~/.config/pixa/config.yml`, `~/.config/pixa/config.yaml`, then `config.yml` and -`config.yaml` in the working directory. A named file that does not exist, cannot -be read or does not parse aborts startup. Of the files pixa looks for on its -own, only one that does not exist is passed over, without a message. One that -pixa cannot read or parse aborts startup, naming the file. So does one in a -directory pixa may not enter, whether or not it is there, since pixa cannot -tell. With no file, pixa uses the environment and the defaults. +pixa reads at most one config file. It reads the file given with `--config` (or +`-c`), or else the one `PIXA_CONFIG_PATH` names. pixa refuses to start when that +file does not exist, cannot be read or does not parse. Without `--config` or +`PIXA_CONFIG_PATH`, pixa looks in `/etc/pixa/`, then in `~/.config/pixa/`, then +in the working directory. In each directory it looks for `config.yml`, then +`config.yaml`. It reads the first file it finds. It skips a missing file without +a message. pixa refuses to start when a file it finds cannot be read or does not +parse, and the error names the file. It also refuses to start when it may not +enter one of these directories, whether or not the file is there, since it +cannot tell. With no file, pixa uses the environment and the defaults. -| Variable | Config key | Meaning | -| ------------------------------------ | ------------------------------- | ---------------------------------------------------------------------------- | -| `PIXA_SIGNING_KEY` | `signing_key` | Required: secret for signed and encrypted URLs and login, 32+ characters | -| `PORT` | `port` | Port to listen on; default `8080` | -| `PIXA_STATE_DIR` | `state_dir` | Directory for the database and the disk cache; default `/var/lib/pixa` | -| `PIXA_DB_URL` | `db_url` | SQLite database URL; default `state.sqlite3` in the state directory | -| `PIXA_CACHE_MAX_BYTES` | `cache_max_bytes` | Disk cache limit in bytes; `0` disables it; default 75% of (free + cached) | -| `PIXA_ALLOWLIST_HOSTS` | `allowlist_hosts` | Upstream hosts served without a signature | -| `PIXA_REFERER_BLOCKLIST` | `referer_blocklist` | Hosts whose pages the image routes refuse with 403, by `Referer` | -| `PIXA_BLOCKED_NETWORKS` | `blocked_networks` | CIDR ranges never fetched from, on top of the built-in ones | -| `PIXA_TRUSTED_PROXIES` | `trusted_proxies` | CIDR ranges of proxies whose `X-Forwarded-For` is believed; default RFC 1918 | -| `PIXA_ALLOW_HTTP` | `allow_http` | Allow plain-HTTP upstreams, for testing only; default `false` | -| `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20` | -| `PIXA_UPSTREAM_CONNECTIONS` | `upstream_connections` | Concurrent connections to all upstream hosts together; default `64` | -| `PIXA_MAX_CONCURRENT_PROCESSING` | `max_concurrent_processing` | Images processed at once; default half the CPUs, at least 1 | -| `PIXA_IMAGE_REQUESTS_PER_MINUTE` | `image_requests_per_minute` | Image requests per client per minute; `0` turns it off; default `600` | -| `PIXA_UPSTREAM_FETCH_TIMEOUT` | `upstream_fetch_timeout` | Time allowed for one fetch from an upstream host; default `30s` | -| `PIXA_UPSTREAM_MAX_RESPONSE_SIZE` | `upstream_max_response_size` | Largest upstream response accepted, in bytes; default 50 MiB | -| `PIXA_DOWNSTREAM_TIMEOUT` | `downstream_timeout` | Time allowed for answering one client request; default `60s` | -| `PIXA_ACCESS_CONTROL_ALLOW_ORIGIN` | `access_control_allow_origin` | CORS origin allowed to read image responses: `*` or one origin; default `*` | -| `PIXA_METRICS_USERNAME` | `metrics.username` | Username for `/metrics`, which is served only when both are set | -| `PIXA_METRICS_PASSWORD` | `metrics.password` | Password for `/metrics`; set together with the username | -| `PIXA_SENTRY_DSN` | `sentry_dsn` | Sentry DSN for error reporting; empty disables it | -| `PIXA_DEBUG` | `debug` | Debug logging and plain-HTTP local development; default `false` | -| `PIXA_MAINTENANCE_MODE` | `maintenance_mode` | Answer image requests with 503; the health check stays 200; default `false` | +Each item below is one setting, named by its config key: + +- `signing_key`: variable `PIXA_SIGNING_KEY`, required. The secret for signed + URLs, encrypted URLs and login. It must be 32 characters or longer. +- `port`: variable `PORT`, default `8080`. The port to listen on. +- `state_dir`: variable `PIXA_STATE_DIR`, default `/var/lib/pixa`. The directory + for the database and the disk cache. +- `db_url`: variable `PIXA_DB_URL`, default `state.sqlite3` in the state + directory. The URL of the SQLite database. +- `cache_max_bytes`: variable `PIXA_CACHE_MAX_BYTES`, default 75% of the sum of + the free space and what the cache holds. The disk cache limit in bytes. `0` + turns the disk cache off. +- `allowlist_hosts`: variable `PIXA_ALLOWLIST_HOSTS`, default none. The upstream + hosts whose images pixa serves without a signature. +- `referer_blocklist`: variable `PIXA_REFERER_BLOCKLIST`, default none. The + image routes refuse a request with 403 when its `Referer` header names one of + these hosts. +- `blocked_networks`: variable `PIXA_BLOCKED_NETWORKS`, default none. The CIDR + ranges pixa never fetches from, on top of the built-in ones. +- `trusted_proxies`: variable `PIXA_TRUSTED_PROXIES`, default the RFC 1918 + ranges. The CIDR ranges of the proxies whose `X-Forwarded-For` header pixa + believes. +- `allow_http`: variable `PIXA_ALLOW_HTTP`, default `false`. Allows plain-HTTP + upstream hosts. It is for testing only. +- `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: diff --git a/TODO.md b/TODO.md index 39e0f30..f0cba91 100644 --- a/TODO.md +++ b/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. diff --git a/internal/config/config.go b/internal/config/config.go index 3f484b7..2f433c7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -76,34 +76,6 @@ func DefaultCacheMaxBytes(freeBytes uint64, heldBytes int64) int64 { return max(int64(computed), DefaultCacheMaxBytesFloor) } -// Configuration key names. -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" -) - // placeholderSigningKey is the dummy signing_key shipped in // configs/config.example.yml. It is 45 characters, so it passes the length // check, but it is public in this repository and must be rejected at @@ -123,7 +95,7 @@ var ( errNotAStringList = errors.New("not a list of strings") errNotAValidCIDR = errors.New("not a valid CIDR network") errMappedTooShort = errors.New("in the IPv4-mapped form but shorter than /96") - errNotAMetricsMap = errors.New("not a map of metrics settings") + errNotAMap = errors.New("not a map") errEmptyListEntry = errors.New("list contains an empty entry") errEmptyEntry = errors.New("contains an empty entry") errNotAValidURL = errors.New("not a valid URL") @@ -284,7 +256,7 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) { // newFromSmartConfig constructs a Config from the environment and a // loaded smartconfig instance, and validates it. A nil sc means no -// config file was found, in which case every option the environment +// config file was found, in which case every setting the environment // does not set takes its default value. A key that is present but // unparseable or invalid is an error: defaults apply only to omitted // keys, never to invalid explicit values. @@ -294,93 +266,171 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { return nil, err } - err = validateAllowlistHostsValue(sc) - if err != nil { - return nil, err - } + c := &Config{} - blockedNetworks, err := parseCIDRList(sc, keyBlockedNetworks) - if err != nil { - return nil, err - } - - trustedProxies, err := parseCIDRList(sc, keyTrustedProxies) - if err != nil { - return nil, err - } - - // parseCIDRList returns a nil slice only when the key is absent; an - // explicitly empty list ([]) comes back non-nil and empty. An omitted - // key takes the RFC 1918 default, while an explicit empty list is left - // as trust-nothing. - if trustedProxies == nil { - trustedProxies = defaultTrustedProxies() - } - - loader := &strictLoader{sc: sc} - c := &Config{ - Debug: loader.boolVal(keyDebug, false), - MaintenanceMode: loader.boolVal(keyMaintenanceMode, false), - Port: loader.intVal(keyPort, DefaultPort), - StateDir: loader.stringVal(keyStateDir, DefaultStateDir), - SentryDSN: loader.stringVal(keySentryDSN, ""), - MetricsUsername: loader.stringVal(keyMetricsUsername, ""), - MetricsPassword: loader.stringVal(keyMetricsPassword, ""), - SigningKey: loader.stringVal(keySigningKey, ""), - AllowlistHosts: getStringSlice(sc), - AllowHTTP: loader.boolVal(keyAllowHTTP, false), - UpstreamConnectionsPerHost: loader.intVal( - keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost), - UpstreamConnections: loader.intVal( - keyUpstreamConnections, DefaultUpstreamConnections), - MaxConcurrentProcessing: loader.intVal( - keyMaxConcurrentProcessing, DefaultMaxConcurrentProcessing()), - UpstreamFetchTimeout: loader.durationVal( - keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout), - UpstreamMaxResponseSize: loader.int64Val( - keyUpstreamMaxResponseSize, DefaultUpstreamMaxResponseSize), - AccessControlAllowOrigin: loader.stringVal( - keyAccessControlAllowOrigin, DefaultAccessControlAllowOrigin), - DownstreamTimeout: loader.durationVal( - keyDownstreamTimeout, DefaultDownstreamTimeout), - ImageRequestsPerMinute: loader.intVal( - keyImageRequestsPerMinute, DefaultImageRequestsPerMinute), - CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0), - BlockedNetworks: blockedNetworks, - TrustedProxies: trustedProxies, - RefererBlocklist: loader.hostListVal(keyRefererBlocklist), - } - - // The default for an omitted cache_max_bytes is worked out when - // pixad builds the cache; here we only record whether the operator - // set the key explicitly. - _, c.CacheMaxBytesExplicit = lookupValue(sc, keyCacheMaxBytes) - - // Build DBURL from StateDir if not explicitly set. The derived URL - // is a default: it applies only when db_url is omitted, never to an - // explicitly empty value. - c.DBURL = loader.stringVal(keyDBURL, "") - if c.DBURL == "" && loader.err == nil { - if _, present := lookupValue(sc, keyDBURL); present { - return nil, fmt.Errorf("%s: %w; omit it to derive it from state_dir", - settingName(keyDBURL), errValueEmpty) + for _, s := range c.settings() { + err := s.load(lookupValue(sc, s)) + if err != nil { + return nil, err } - - c.DBURL = defaultDBURL(c.StateDir) } - if loader.err != nil { - return nil, loader.err - } - - err = c.validate() - if err != nil { - return nil, err + // The metrics username and password are checked together, once both + // have loaded. + if (c.MetricsUsername == "") != (c.MetricsPassword == "") { + return nil, fmt.Errorf("%s and %s %w", + settingName("metrics.username"), settingName("metrics.password"), + errMustBeSetTogether) } return c, nil } +// A setting is one row of the settings table. key is its key in the +// config file, and envVar is the environment variable that also sets it. +// load reads the value set for it into its Config field. +type setting struct { + key string + envVar string + load func(v value) error +} + +// A value is what the environment or the config file sets for one +// setting. present is false when neither sets it. +type value struct { + key string + raw any + present bool +} + +// settings returns the settings table: every setting pixa reads, once +// each, in the order they load into c. Most rows fill their Config field +// with field, whose arguments give the setting's type, its default and +// its check. Code that needs only the keys and the variables reads the +// table of a new Config. +func (c *Config) settings() []setting { + return []setting{ + {"signing_key", "PIXA_SIGNING_KEY", + field(&c.SigningKey, getString, "", validateSigningKey)}, + {"port", "PORT", field(&c.Port, getInt, DefaultPort, validatePort)}, + {"state_dir", "PIXA_STATE_DIR", + field(&c.StateDir, getString, DefaultStateDir, validateNotEmpty)}, + {"db_url", "PIXA_DB_URL", c.loadDBURL}, + {"cache_max_bytes", "PIXA_CACHE_MAX_BYTES", c.loadCacheMaxBytes}, + {"allowlist_hosts", "PIXA_ALLOWLIST_HOSTS", + field(&c.AllowlistHosts, parseHostList, nil, nil)}, + {"referer_blocklist", "PIXA_REFERER_BLOCKLIST", + field(&c.RefererBlocklist, parseHostList, nil, nil)}, + {"blocked_networks", "PIXA_BLOCKED_NETWORKS", + field(&c.BlockedNetworks, parseCIDRList, nil, nil)}, + {"trusted_proxies", "PIXA_TRUSTED_PROXIES", + field(&c.TrustedProxies, parseCIDRList, defaultTrustedProxies(), nil)}, + {"allow_http", "PIXA_ALLOW_HTTP", field(&c.AllowHTTP, getBool, false, nil)}, + {"upstream_connections_per_host", "PIXA_UPSTREAM_CONNECTIONS_PER_HOST", + field(&c.UpstreamConnectionsPerHost, getInt, + DefaultUpstreamConnectionsPerHost, validateAtLeastOne)}, + {"upstream_connections", "PIXA_UPSTREAM_CONNECTIONS", + field(&c.UpstreamConnections, getInt, + DefaultUpstreamConnections, validateAtLeastOne)}, + {"max_concurrent_processing", "PIXA_MAX_CONCURRENT_PROCESSING", + field(&c.MaxConcurrentProcessing, getInt, + DefaultMaxConcurrentProcessing(), validateAtLeastOne)}, + {"image_requests_per_minute", "PIXA_IMAGE_REQUESTS_PER_MINUTE", + field(&c.ImageRequestsPerMinute, getInt, + DefaultImageRequestsPerMinute, validateNotNegative)}, + {"upstream_fetch_timeout", "PIXA_UPSTREAM_FETCH_TIMEOUT", + field(&c.UpstreamFetchTimeout, getDuration, + DefaultUpstreamFetchTimeout, nil)}, + {"upstream_max_response_size", "PIXA_UPSTREAM_MAX_RESPONSE_SIZE", + field(&c.UpstreamMaxResponseSize, getInt64, + DefaultUpstreamMaxResponseSize, validateUpstreamMaxResponseSize)}, + {"downstream_timeout", "PIXA_DOWNSTREAM_TIMEOUT", + field(&c.DownstreamTimeout, getDuration, + DefaultDownstreamTimeout, nil)}, + {"access_control_allow_origin", "PIXA_ACCESS_CONTROL_ALLOW_ORIGIN", + field(&c.AccessControlAllowOrigin, getString, + DefaultAccessControlAllowOrigin, validateAccessControlAllowOrigin)}, + {"metrics.username", "PIXA_METRICS_USERNAME", + field(&c.MetricsUsername, getString, "", nil)}, + {"metrics.password", "PIXA_METRICS_PASSWORD", + field(&c.MetricsPassword, getString, "", nil)}, + {"sentry_dsn", "PIXA_SENTRY_DSN", + field(&c.SentryDSN, getString, "", validateSentryDSN)}, + {"debug", "PIXA_DEBUG", field(&c.Debug, getBool, false, nil)}, + {"maintenance_mode", "PIXA_MAINTENANCE_MODE", + field(&c.MaintenanceMode, getBool, false, nil)}, + } +} + +// field returns the load function of a setting that fills *dst. read +// reads a value that is set, and so gives the setting's type. A setting +// that is not set takes defaultValue. The value, a default too, must +// then pass check, unless check is nil. +func field[T any]( + dst *T, + read func(v value) (T, error), + defaultValue T, + check func(key string, val T) error, +) func(v value) error { + return func(v value) error { + val := defaultValue + + if v.present { + var err error + + val, err = read(v) + if err != nil { + return err + } + } + + if check != nil { + err := check(v.key, val) + if err != nil { + return err + } + } + + *dst = val + + return nil + } +} + +// loadDBURL loads db_url. An omitted db_url is derived from state_dir, +// so db_url comes after state_dir in the table. An empty db_url is +// refused, since defaults apply only to omitted keys. +func (c *Config) loadDBURL(v value) error { + if !v.present { + c.DBURL = defaultDBURL(c.StateDir) + + return nil + } + + dbURL, err := getString(v) + if err != nil { + return err + } + + if dbURL == "" { + return fmt.Errorf("%s: %w; omit it to derive it from state_dir", + settingName(v.key), errValueEmpty) + } + + c.DBURL = dbURL + + return nil +} + +// loadCacheMaxBytes loads cache_max_bytes, and records whether it is set. +// For an omitted cache_max_bytes pixad works out DefaultCacheMaxBytes +// when it builds the cache. +func (c *Config) loadCacheMaxBytes(v value) error { + c.CacheMaxBytesExplicit = v.present + + return field(&c.CacheMaxBytes, getInt64, 0, validateNotNegative)(v) +} + // defaultDBURL returns the db_url used when db_url is omitted: the file // state.sqlite3 in stateDir, in WAL mode, which the driver sets only // through a _pragma parameter. String() percent-encodes any %, ? or # in @@ -409,50 +459,65 @@ func defaultDBURL(stateDir string) string { // application does not understand, so typos fail at startup instead of // being silently ignored, and rejects keys that are explicitly set to // null: a null is a SET value, never an omission, so it must not -// silently take the default. The env section is permitted because -// smartconfig consumes it for environment variable injection. A nil sc -// means no config file, which has no keys to check. +// silently take the default. A setting whose key has a dot, such as +// metrics.username, is written inside the map of the key's first part. +// The env section is permitted because smartconfig consumes it for +// environment variable injection. A nil sc means no config file, which +// has no keys to check. func validateKnownKeys(sc *smartconfig.Config) error { if sc == nil { return nil } + topLevelKeys := map[string]bool{"env": true} + mapKeys := map[string]bool{} + keysInMaps := map[string]bool{} + + for _, s := range new(Config).settings() { + mapKey, _, inMap := strings.Cut(s.key, ".") + topLevelKeys[mapKey] = true + + if inMap { + mapKeys[mapKey] = true + keysInMaps[s.key] = true + } + } + var unknown, nullKeys []string - for key, value := range sc.Data() { - if !isKnownConfigKey(key) { + for key, val := range sc.Data() { + switch { + case !topLevelKeys[key]: unknown = append(unknown, key) - - continue - } - - if value == nil { + case val == nil: nullKeys = append(nullKeys, key) - - continue - } - - if key == keyMetrics { - metricsMap, ok := value.(map[string]any) + case mapKeys[key]: + settingsMap, ok := val.(map[string]any) if !ok { - return fmt.Errorf("config key %q: value %v is %w", - keyMetrics, value, errNotAMetricsMap) + return fmt.Errorf("config key %q: value %v is %w of %s settings", + key, val, errNotAMap, key) } - for subkey, subvalue := range metricsMap { - if subkey != "username" && subkey != "password" { - unknown = append(unknown, keyMetrics+"."+subkey) + for subkey, subvalue := range settingsMap { + fullKey := key + "." + subkey - continue - } - - if subvalue == nil { - nullKeys = append(nullKeys, keyMetrics+"."+subkey) + switch { + case !keysInMaps[fullKey]: + unknown = append(unknown, fullKey) + case subvalue == nil: + nullKeys = append(nullKeys, fullKey) } } } } + return unknownOrNullKeysError(unknown, nullKeys) +} + +// unknownOrNullKeysError reports the unknown keys validateKnownKeys +// found, or else the keys set to null. It returns nil when there are +// neither. +func unknownOrNullKeysError(unknown, nullKeys []string) error { if len(unknown) > 0 { sort.Strings(unknown) @@ -480,55 +545,6 @@ func errNullConfigValue(key string) error { return fmt.Errorf("config key %q: %w", key, errValueNull) } -// isKnownConfigKey reports whether key is a permitted top-level -// configuration key. -func isKnownConfigKey(key string) bool { - switch key { - case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN, - keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP, - keyUpstreamConnectionsPerHost, keyUpstreamConnections, - keyMaxConcurrentProcessing, keyCacheMaxBytes, keyBlockedNetworks, - keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout, - keyUpstreamMaxResponseSize, keyDownstreamTimeout, keyRefererBlocklist, - keyImageRequestsPerMinute, "env": - return true - } - - return false -} - -// envVarNames returns, for each configuration key, the environment -// variable that also sets it: PIXA_ plus the key in upper case, with "." -// written as "_", except the port, which REPO_POLICIES.md requires to be -// PORT. metrics is set through its two subkeys; env has no variable. -func envVarNames() map[string]string { - return map[string]string{ //nolint:gosec // G101: variable names, not secrets - keyDebug: "PIXA_DEBUG", - keyMaintenanceMode: "PIXA_MAINTENANCE_MODE", - keyPort: "PORT", - keyStateDir: "PIXA_STATE_DIR", - keySentryDSN: "PIXA_SENTRY_DSN", - keyDBURL: "PIXA_DB_URL", - keyMetricsUsername: "PIXA_METRICS_USERNAME", - keyMetricsPassword: "PIXA_METRICS_PASSWORD", - keySigningKey: "PIXA_SIGNING_KEY", - keyAllowlistHosts: "PIXA_ALLOWLIST_HOSTS", - keyRefererBlocklist: "PIXA_REFERER_BLOCKLIST", - keyAllowHTTP: "PIXA_ALLOW_HTTP", - keyUpstreamConnectionsPerHost: "PIXA_UPSTREAM_CONNECTIONS_PER_HOST", - keyUpstreamConnections: "PIXA_UPSTREAM_CONNECTIONS", - keyMaxConcurrentProcessing: "PIXA_MAX_CONCURRENT_PROCESSING", - keyCacheMaxBytes: "PIXA_CACHE_MAX_BYTES", - keyBlockedNetworks: "PIXA_BLOCKED_NETWORKS", - keyTrustedProxies: "PIXA_TRUSTED_PROXIES", - keyAccessControlAllowOrigin: "PIXA_ACCESS_CONTROL_ALLOW_ORIGIN", - keyUpstreamFetchTimeout: "PIXA_UPSTREAM_FETCH_TIMEOUT", - keyUpstreamMaxResponseSize: "PIXA_UPSTREAM_MAX_RESPONSE_SIZE", - keyDownstreamTimeout: "PIXA_DOWNSTREAM_TIMEOUT", - keyImageRequestsPerMinute: "PIXA_IMAGE_REQUESTS_PER_MINUTE", - } -} - // validateKnownEnvVars rejects environment variables whose names start // with PIXA_ but that are neither a setting's variable nor // PIXA_CONFIG_PATH, so a misspelled variable fails at startup instead of @@ -538,8 +554,8 @@ func envVarNames() map[string]string { func validateKnownEnvVars() error { known := map[string]bool{"PIXA_CONFIG_PATH": true} - for _, name := range envVarNames() { - known[name] = true + for _, s := range new(Config).settings() { + known[s.envVar] = true } var unknown []string @@ -566,28 +582,37 @@ func validateKnownEnvVars() error { return nil } -// lookupValue returns the value set for key and whether one is set. The -// key's environment variable wins when it is present, even when empty; -// its value is a string, read exactly as the same text quoted in the -// config file would be. Otherwise the config file's value is used. -func lookupValue(sc *smartconfig.Config, key string) (any, bool) { - value, present := os.LookupEnv(envVarNames()[key]) +// lookupValue returns the value set for s. Its environment variable wins +// when it is present, even when empty; its value is a string, read +// exactly as the same text quoted in the config file would be. Otherwise +// the config file's value is used. +func lookupValue(sc *smartconfig.Config, s setting) value { + envValue, present := os.LookupEnv(s.envVar) if present { - return value, true + return value{key: s.key, raw: envValue, present: true} } if sc == nil { - return nil, false + return value{key: s.key} } - return sc.Get(key) + fileValue, present := sc.Get(s.key) + + return value{key: s.key, raw: fileValue, present: present} } -// settingName names key in an error message together with its -// environment variable, since either one may have set the value. +// settingName names the setting key in an error message together with +// its environment variable, since either one may have set the value. func settingName(key string) string { - return fmt.Sprintf("config key %q (environment variable %s)", - key, envVarNames()[key]) + var envVar string + + for _, s := range new(Config).settings() { + if s.key == key { + envVar = s.envVar + } + } + + return fmt.Sprintf("config key %q (environment variable %s)", key, envVar) } // ensureStateDirWritable verifies at startup that StateDir can be @@ -596,16 +621,18 @@ func settingName(key string) string { func (c *Config) ensureStateDirWritable() error { const stateDirPerms = 0o750 + name := settingName("state_dir") + err := os.MkdirAll(c.StateDir, stateDirPerms) if err != nil { return fmt.Errorf("%s: cannot create directory %q: %w", - settingName(keyStateDir), c.StateDir, err) + name, c.StateDir, err) } probe, err := os.CreateTemp(c.StateDir, ".startup-write-probe-*") if err != nil { return fmt.Errorf("%s: directory %q is not writable: %w", - settingName(keyStateDir), c.StateDir, err) + name, c.StateDir, err) } probePath := probe.Name() @@ -613,13 +640,13 @@ func (c *Config) ensureStateDirWritable() error { err = probe.Close() if err != nil { return fmt.Errorf("%s: cannot close probe file %q: %w", - settingName(keyStateDir), probePath, err) + name, probePath, err) } err = os.Remove(probePath) if err != nil { return fmt.Errorf("%s: cannot remove probe file %q: %w", - settingName(keyStateDir), probePath, err) + name, probePath, err) } return nil @@ -628,101 +655,79 @@ func (c *Config) ensureStateDirWritable() error { // validateSigningKey checks that the signing key is present, long // enough, and not the public placeholder from configs/config.example.yml. // The key value itself is never echoed in error messages. -func (c *Config) validateSigningKey() error { - if c.SigningKey == "" { - return fmt.Errorf("%s: %w", settingName(keySigningKey), errValueRequired) +func validateSigningKey(key, signingKey string) error { + if signingKey == "" { + return fmt.Errorf("%s: %w", settingName(key), errValueRequired) } // Minimum key length for security (32 bytes = 256 bits) const minKeyLength = 32 - if len(c.SigningKey) < minKeyLength { + if len(signingKey) < minKeyLength { return fmt.Errorf("%s: %w: must be at least %d characters, got %d", - settingName(keySigningKey), errValueTooShort, minKeyLength, - len(c.SigningKey)) + settingName(key), errValueTooShort, minKeyLength, len(signingKey)) } - if c.SigningKey == placeholderSigningKey { - return fmt.Errorf("%s: %w", settingName(keySigningKey), errPlaceholderKey) + if signingKey == placeholderSigningKey { + return fmt.Errorf("%s: %w", settingName(key), errPlaceholderKey) } return nil } -// validate checks that all required configuration values are set and -// that every value is within its valid range. -func (c *Config) validate() error { - err := c.validateSigningKey() - if err != nil { - return err - } - +// validatePort checks that a port is from 1 to 65535. +func validatePort(key string, port int) error { const maxPort = 65535 - if c.Port < 1 || c.Port > maxPort { + if port < 1 || port > maxPort { return fmt.Errorf("%s: value %d is %w 1-%d", - settingName(keyPort), c.Port, errPortOutOfRange, maxPort) + settingName(key), port, errPortOutOfRange, maxPort) } - err = c.validateConcurrencyLimits() - if err != nil { - return err + return nil +} + +// validateNotEmpty checks that a value is not empty. +func validateNotEmpty(key, val string) error { + if val == "" { + return fmt.Errorf("%s: %w", settingName(key), errValueEmpty) } - if c.StateDir == "" { - return fmt.Errorf("%s: %w", settingName(keyStateDir), errValueEmpty) - } + return nil +} - // Zero is valid (it disables the disk cache); only negative - // values are rejected. No floor applies to explicit values. - if c.CacheMaxBytes < 0 { +// validateAtLeastOne checks that a limit, such as the most images +// processed at once, is at least 1. +func validateAtLeastOne(key string, limit int) error { + if limit < 1 { return fmt.Errorf("%s: value %d %w", - settingName(keyCacheMaxBytes), c.CacheMaxBytes, errMustNotBeNegative) + settingName(key), limit, errMustBeAtLeastOne) } - // Zero is valid (it turns the limit off); only negative values are - // rejected. - if c.ImageRequestsPerMinute < 0 { + return nil +} + +// validateNotNegative checks that a number is not negative. Zero is valid +// for the two settings that use it: it turns the disk cache or the limit +// on image requests off. +func validateNotNegative[T int | int64](key string, number T) error { + if number < 0 { return fmt.Errorf("%s: value %d %w", - settingName(keyImageRequestsPerMinute), c.ImageRequestsPerMinute, - errMustNotBeNegative) + settingName(key), number, errMustNotBeNegative) } - err = c.validateUpstreamMaxResponseSize() - if err != nil { - return err - } - - for _, host := range c.AllowlistHosts { - err := validateHostPattern(keyAllowlistHosts, host) - if err != nil { - return err - } - } - - err = c.validateSentryDSN() - if err != nil { - return err - } - - if (c.MetricsUsername == "") != (c.MetricsPassword == "") { - return fmt.Errorf("%s and %s %w", - settingName(keyMetricsUsername), settingName(keyMetricsPassword), - errMustBeSetTogether) - } - - return c.validateAccessControlAllowOrigin() + return nil } // validateSentryDSN checks that sentry_dsn, when set, is a URL with a scheme // and a host. -func (c *Config) validateSentryDSN() error { - if c.SentryDSN == "" { +func validateSentryDSN(key, dsn string) error { + if dsn == "" { return nil } - parsed, err := url.Parse(c.SentryDSN) + parsed, err := url.Parse(dsn) if err != nil || parsed.Scheme == "" || parsed.Host == "" { return fmt.Errorf("%s: value %q is %w", - settingName(keySentryDSN), c.SentryDSN, errNotAValidURL) + settingName(key), dsn, errNotAValidURL) } return nil @@ -732,13 +737,11 @@ func (c *Config) validateSentryDSN() error { // is from 1 byte to 1 GiB. An upstream response is read whole into // memory, and the image processor reads one byte past this limit, which // must not overflow. -func (c *Config) validateUpstreamMaxResponseSize() error { +func validateUpstreamMaxResponseSize(key string, size int64) error { const maxUpstreamMaxResponseSize = 1 << 30 // 1 GiB - if c.UpstreamMaxResponseSize < 1 || - c.UpstreamMaxResponseSize > maxUpstreamMaxResponseSize { + if size < 1 || size > maxUpstreamMaxResponseSize { return fmt.Errorf("%s: value %d is %w 1-%d", - settingName(keyUpstreamMaxResponseSize), c.UpstreamMaxResponseSize, - errSizeOutOfRange, maxUpstreamMaxResponseSize) + settingName(key), size, errSizeOutOfRange, maxUpstreamMaxResponseSize) } return nil @@ -747,14 +750,13 @@ func (c *Config) validateUpstreamMaxResponseSize() error { // validateAccessControlAllowOrigin accepts "*" or an origin exactly as a browser // sends it: http or https, an IP address as netip writes it or a lowercase name // with a letter in its last part, and an optional port 1-65535, not the default. -func (c *Config) validateAccessControlAllowOrigin() error { - origin := c.AccessControlAllowOrigin +func validateAccessControlAllowOrigin(key, origin string) error { if origin == "*" { return nil } errOrigin := fmt.Errorf("%s: value %q is %w", - settingName(keyAccessControlAllowOrigin), origin, errNotAnOrigin) + settingName(key), origin, errNotAnOrigin) parsed, err := url.Parse(origin) if err != nil { @@ -804,30 +806,6 @@ func (c *Config) validateAccessControlAllowOrigin() error { return nil } -// validateConcurrencyLimits checks that the two upstream connection limits -// and the image processing limit are at least 1. -func (c *Config) validateConcurrencyLimits() error { - if c.UpstreamConnectionsPerHost < 1 { - return fmt.Errorf("%s: value %d %w", - settingName(keyUpstreamConnectionsPerHost), - c.UpstreamConnectionsPerHost, errMustBeAtLeastOne) - } - - if c.UpstreamConnections < 1 { - return fmt.Errorf("%s: value %d %w", - settingName(keyUpstreamConnections), - c.UpstreamConnections, errMustBeAtLeastOne) - } - - if c.MaxConcurrentProcessing < 1 { - return fmt.Errorf("%s: value %d %w", - settingName(keyMaxConcurrentProcessing), - c.MaxConcurrentProcessing, errMustBeAtLeastOne) - } - - return nil -} - // hostNamePattern matches a host name: letters, digits, hyphens, underscores // and dots, optionally after one leading dot. var hostNamePattern = regexp.MustCompile(`^\.?[A-Za-z0-9_-][A-Za-z0-9_.-]*$`) @@ -903,128 +881,31 @@ func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, erro return nil, nil //nolint:nilnil // nil config is valid (use defaults) } -// strictLoader accumulates the first error encountered while reading -// typed values out of a smartconfig instance, so Config construction -// can stay a single struct literal. -type strictLoader struct { - sc *smartconfig.Config - err error -} - -func (l *strictLoader) stringVal(key, defaultVal string) string { - if l.err != nil { - return "" +// getString returns the string v holds. A value that is not a string, or +// is explicitly null, is an error. +func getString(v value) (string, error) { + if v.raw == nil { + return "", errNullConfigValue(v.key) } - val, err := getString(l.sc, key, defaultVal) - if err != nil { - l.err = err - } - - return val -} - -func (l *strictLoader) intVal(key string, defaultVal int) int { - if l.err != nil { - return 0 - } - - val, err := getInt(l.sc, key, defaultVal) - if err != nil { - l.err = err - } - - return val -} - -func (l *strictLoader) int64Val(key string, defaultVal int64) int64 { - if l.err != nil { - return 0 - } - - val, err := getInt64(l.sc, key, defaultVal) - if err != nil { - l.err = err - } - - return val -} - -func (l *strictLoader) durationVal(key string, defaultVal time.Duration) time.Duration { - if l.err != nil { - return 0 - } - - val, err := getDuration(l.sc, key, defaultVal) - if err != nil { - l.err = err - } - - return val -} - -func (l *strictLoader) boolVal(key string, defaultVal bool) bool { - if l.err != nil { - return false - } - - val, err := getBool(l.sc, key, defaultVal) - if err != nil { - l.err = err - } - - return val -} - -func (l *strictLoader) hostListVal(key string) []string { - if l.err != nil { - return nil - } - - val, err := parseHostList(l.sc, key) - if err != nil { - l.err = err - } - - return val -} - -// getString returns the string value for key, or defaultVal if the key -// is omitted. A present value that is not a string, or is explicitly -// null, is an error. -func getString(sc *smartconfig.Config, key, defaultVal string) (string, error) { - raw, ok := lookupValue(sc, key) - if !ok { - return defaultVal, nil - } - - if raw == nil { - return "", errNullConfigValue(key) - } - - str, ok := raw.(string) + str, ok := v.raw.(string) if !ok { return "", fmt.Errorf("config key %q: value %v (%T) is %w", - key, raw, raw, errNotAString) + v.key, v.raw, v.raw, errNotAString) } return str, nil } -// getInt returns the integer value for key, or defaultVal if the key is -// omitted. A present value that is not a whole number, or is explicitly -// null, is an error; fractional values are never truncated. -func getInt(sc *smartconfig.Config, key string, defaultVal int) (int, error) { - raw, ok := lookupValue(sc, key) - if !ok { - return defaultVal, nil +// getInt returns the integer v holds. A value that is not a whole +// number, or is explicitly null, is an error; fractional values are +// never truncated. +func getInt(v value) (int, error) { + if v.raw == nil { + return 0, errNullConfigValue(v.key) } - if raw == nil { - return 0, errNullConfigValue(key) - } - - switch val := raw.(type) { + switch val := v.raw.(type) { case int: return val, nil case int64: @@ -1032,7 +913,7 @@ func getInt(sc *smartconfig.Config, key string, defaultVal int) (int, error) { case float64: if val != math.Trunc(val) { return 0, fmt.Errorf("config key %q: value %v is %w", - key, val, errNotAnInteger) + v.key, val, errNotAnInteger) } return int(val), nil @@ -1040,31 +921,25 @@ func getInt(sc *smartconfig.Config, key string, defaultVal int) (int, error) { parsed, err := strconv.Atoi(strings.TrimSpace(val)) if err != nil { return 0, fmt.Errorf("%s: value %q is %w", - settingName(key), val, errNotAnInteger) + settingName(v.key), val, errNotAnInteger) } return parsed, nil default: return 0, fmt.Errorf("config key %q: value %v (%T) is %w", - key, raw, raw, errNotAnInteger) + v.key, v.raw, v.raw, errNotAnInteger) } } -// getInt64 returns the 64-bit integer value for key, or defaultVal if -// the key is omitted. A present value that is not a whole number, or -// is explicitly null, is an error; fractional values are never -// truncated and out-of-range values are never clamped. -func getInt64(sc *smartconfig.Config, key string, defaultVal int64) (int64, error) { - raw, ok := lookupValue(sc, key) - if !ok { - return defaultVal, nil +// getInt64 returns the 64-bit integer v holds. A value that is not a +// whole number, or is explicitly null, is an error; fractional values +// are never truncated and out-of-range values are never clamped. +func getInt64(v value) (int64, error) { + if v.raw == nil { + return 0, errNullConfigValue(v.key) } - if raw == nil { - return 0, errNullConfigValue(key) - } - - switch val := raw.(type) { + switch val := v.raw.(type) { case int: return int64(val), nil case int64: @@ -1072,14 +947,14 @@ func getInt64(sc *smartconfig.Config, key string, defaultVal int64) (int64, erro case uint64: if val > math.MaxInt64 { return 0, fmt.Errorf("config key %q: value %d %w", - key, val, errOverflowsInt64) + v.key, val, errOverflowsInt64) } return int64(val), nil case float64: if val != math.Trunc(val) { return 0, fmt.Errorf("config key %q: value %v is %w", - key, val, errNotAnInteger) + v.key, val, errNotAnInteger) } return int64(val), nil @@ -1087,174 +962,70 @@ func getInt64(sc *smartconfig.Config, key string, defaultVal int64) (int64, erro parsed, err := strconv.ParseInt(strings.TrimSpace(val), 10, 64) if err != nil { return 0, fmt.Errorf("%s: value %q is %w", - settingName(key), val, errNotAnInteger) + settingName(v.key), val, errNotAnInteger) } return parsed, nil default: return 0, fmt.Errorf("config key %q: value %v (%T) is %w", - key, raw, raw, errNotAnInteger) + v.key, v.raw, v.raw, errNotAnInteger) } } -// getDuration returns the duration value for key, or defaultVal if the -// key is omitted. A present value must be a positive Go duration string -// such as "30s" or "2m", read with time.ParseDuration; a bare number has -// no unit and is an error, as is an explicit null. -func getDuration( - sc *smartconfig.Config, key string, defaultVal time.Duration, -) (time.Duration, error) { - raw, ok := lookupValue(sc, key) - if !ok { - return defaultVal, nil +// getDuration returns the duration v holds. The value must be a positive +// Go duration string such as "30s" or "2m", read with +// time.ParseDuration; a bare number has no unit and is an error, as is +// an explicit null. +func getDuration(v value) (time.Duration, error) { + if v.raw == nil { + return 0, errNullConfigValue(v.key) } - if raw == nil { - return 0, errNullConfigValue(key) - } - - str, ok := raw.(string) + str, ok := v.raw.(string) if !ok { return 0, fmt.Errorf("config key %q: value %v (%T) is %w", - key, raw, raw, errNotADuration) + v.key, v.raw, v.raw, errNotADuration) } parsed, err := time.ParseDuration(strings.TrimSpace(str)) if err != nil { return 0, fmt.Errorf("%s: value %q is %w", - settingName(key), str, errNotADuration) + settingName(v.key), str, errNotADuration) } if parsed <= 0 { return 0, fmt.Errorf("%s: value %q %w", - settingName(key), str, errMustBePositive) + settingName(v.key), str, errMustBePositive) } return parsed, nil } -// getBool returns the boolean value for key, or defaultVal if the key -// is omitted. A present value that is not a boolean (or a ParseBool-able -// string), or is explicitly null, is an error; numbers are not accepted -// as booleans. -func getBool(sc *smartconfig.Config, key string, defaultVal bool) (bool, error) { - raw, ok := lookupValue(sc, key) - if !ok { - return defaultVal, nil +// getBool returns the boolean v holds. A value that is not a boolean (or +// a ParseBool-able string), or is explicitly null, is an error; numbers +// are not accepted as booleans. +func getBool(v value) (bool, error) { + if v.raw == nil { + return false, errNullConfigValue(v.key) } - if raw == nil { - return false, errNullConfigValue(key) - } - - switch val := raw.(type) { + switch val := v.raw.(type) { case bool: return val, nil case string: parsed, err := strconv.ParseBool(strings.TrimSpace(val)) if err != nil { return false, fmt.Errorf("%s: value %q is %w", - settingName(key), val, errNotABoolean) + settingName(v.key), val, errNotABoolean) } return parsed, nil default: return false, fmt.Errorf("config key %q: value %v (%T) is %w", - key, raw, raw, errNotABoolean) + v.key, v.raw, v.raw, errNotABoolean) } } -// validateAllowlistHostsValue checks the raw shape of the -// allowlist_hosts value before the lenient extraction in getStringSlice -// runs: an explicitly null value, a value that is not a list of strings -// (or a comma-separated string), a non-string entry, or an empty entry -// is an error, never silently skipped. -func validateAllowlistHostsValue(sc *smartconfig.Config) error { - raw, ok := lookupValue(sc, keyAllowlistHosts) - if !ok { - return nil - } - - if raw == nil { - return errNullConfigValue(keyAllowlistHosts) - } - - switch val := raw.(type) { - case []any: - for _, item := range val { - str, ok := item.(string) - if !ok { - return fmt.Errorf("config key %q: list entry %v (%T) is %w", - keyAllowlistHosts, item, item, errNotAString) - } - - if strings.TrimSpace(str) == "" { - return fmt.Errorf("config key %q: %w", - keyAllowlistHosts, errEmptyListEntry) - } - } - case string: - if strings.TrimSpace(val) == "" { - return nil - } - - for part := range strings.SplitSeq(val, ",") { - if strings.TrimSpace(part) == "" { - return fmt.Errorf("%s: value %q %w", - settingName(keyAllowlistHosts), val, errEmptyEntry) - } - } - default: - return fmt.Errorf("config key %q: value %v (%T) is %w", - keyAllowlistHosts, raw, raw, errNotAStringList) - } - - return nil -} - -// getStringSlice returns the allowlist_hosts list of strings, or nil if -// the key is omitted. It accepts a YAML list of strings or a -// comma-separated string (backwards compatibility). Malformed entries -// are rejected beforehand by validateAllowlistHostsValue. -func getStringSlice(sc *smartconfig.Config) []string { - val, ok := lookupValue(sc, keyAllowlistHosts) - if !ok || val == nil { - return nil - } - - // Handle YAML list format - if slice, ok := val.([]any); ok { - result := make([]string, 0, len(slice)) - for _, item := range slice { - if str, ok := item.(string); ok { - trimmed := strings.TrimSpace(str) - if trimmed != "" { - result = append(result, trimmed) - } - } - } - - return result - } - - // Fall back to comma-separated string for backwards compatibility - if str, ok := val.(string); ok && str != "" { - parts := strings.Split(str, ",") - result := make([]string, 0, len(parts)) - - for _, part := range parts { - trimmed := strings.TrimSpace(part) - if trimmed != "" { - result = append(result, trimmed) - } - } - - return result - } - - return nil -} - // defaultTrustedProxies returns the trusted_proxies default: the three RFC // 1918 private ranges. pixa is always deployed behind a TLS-terminating // reverse proxy, which in practice sits on a private network, so its @@ -1270,12 +1041,10 @@ func defaultTrustedProxies() []netip.Prefix { } } -// parseCIDRList parses the value of the named config key into CIDR -// prefixes, or returns nil if the key is omitted. It accepts a YAML list -// of strings or a comma-separated string. An explicitly null value, a -// wrong type, an empty entry, a non-string entry, or an unparseable CIDR -// aborts startup naming the key and the offending value; the default -// (an empty list) applies only to an omitted key. +// parseCIDRList parses the list v holds into CIDR prefixes. It accepts a +// YAML list of strings or a comma-separated string. An explicitly null +// value, a wrong type, an empty entry, a non-string entry, or an +// unparseable CIDR aborts startup naming the key and the offending value. // // An entry written in the IPv4-mapped form (::ffff:a.b.c.d) is kept as the // IPv4 range it covers: ::ffff:10.0.0.0/104 is kept as 10.0.0.0/8. Both @@ -1283,17 +1052,12 @@ func defaultTrustedProxies() []netip.Prefix { // address as the IPv4 address it holds, so the entry as written would match // nothing. Such an entry shorter than /96 covers more than the IPv4 // addresses and aborts startup. -func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { - raw, ok := lookupValue(sc, key) - if !ok { - return nil, nil +func parseCIDRList(v value) ([]netip.Prefix, error) { + if v.raw == nil { + return nil, errNullConfigValue(v.key) } - if raw == nil { - return nil, errNullConfigValue(key) - } - - entries, err := listEntries(raw, key) + entries, err := listEntries(v.raw, v.key) if err != nil { return nil, err } @@ -1308,12 +1072,12 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { prefix, err := netip.ParsePrefix(entry) if err != nil { return nil, fmt.Errorf("%s: value %q is %w", - settingName(key), entry, errNotAValidCIDR) + settingName(v.key), entry, errNotAValidCIDR) } if prefix.Addr().Is4In6() && prefix.Bits() < mappedPrefixBits { return nil, fmt.Errorf("%s: value %q is %w", - settingName(key), entry, errMappedTooShort) + settingName(v.key), entry, errMappedTooShort) } if prefix.Addr().Is4In6() { @@ -1327,28 +1091,23 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { return prefixes, nil } -// parseHostList parses the value of the named config key into host patterns, -// or returns nil if the key is omitted. It accepts a YAML list of strings or a -// comma-separated string. An explicitly null value, a wrong type, an empty -// entry, a non-string entry, or an entry validateHostPattern rejects aborts -// startup naming the key and the offending value. -func parseHostList(sc *smartconfig.Config, key string) ([]string, error) { - raw, ok := lookupValue(sc, key) - if !ok { - return nil, nil +// parseHostList parses the list v holds into host patterns. It accepts a +// YAML list of strings or a comma-separated string. An explicitly null +// value, a wrong type, an empty entry, a non-string entry, or an entry +// validateHostPattern rejects aborts startup naming the key and the +// offending value. +func parseHostList(v value) ([]string, error) { + if v.raw == nil { + return nil, errNullConfigValue(v.key) } - if raw == nil { - return nil, errNullConfigValue(key) - } - - entries, err := listEntries(raw, key) + entries, err := listEntries(v.raw, v.key) if err != nil { return nil, err } for _, entry := range entries { - err := validateHostPattern(key, entry) + err := validateHostPattern(v.key, entry) if err != nil { return nil, err } @@ -1359,8 +1118,8 @@ func parseHostList(sc *smartconfig.Config, key string) ([]string, error) { // listEntries extracts the raw entries of the named list key as trimmed, // non-empty strings, from either a YAML list of strings or a comma-separated -// string; an empty string is an empty list, as for allowlist_hosts. Any other -// shape is a configuration error. +// string; an empty string is an empty list. Any other shape is a +// configuration error. func listEntries(raw any, key string) ([]string, error) { switch val := raw.(type) { case []any: diff --git a/internal/config/config_internal_test.go b/internal/config/config_internal_test.go index b655b35..bae94c7 100644 --- a/internal/config/config_internal_test.go +++ b/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 diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index 1324050..1aaac63 100644 --- a/internal/config/config_validation_internal_test.go +++ b/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 diff --git a/internal/config/documented_settings_internal_test.go b/internal/config/documented_settings_internal_test.go new file mode 100644 index 0000000..6670252 --- /dev/null +++ b/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) + } +} diff --git a/internal/config/env_internal_test.go b/internal/config/env_internal_test.go index 4b23302..435c825 100644 --- a/internal/config/env_internal_test.go +++ b/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() { - t.Setenv(name, "") + for _, s := range new(Config).settings() { + t.Setenv(s.envVar, "") } err := validateKnownEnvVars()