chore: conform post-merge config validation code to v2.12.2 lint config
All checks were successful
check / check (push) Successful in 1m44s
All checks were successful
check / check (push) Successful in 1m44s
The stricter canonical .golangci.yml surfaced 81 findings in the config validation code merged from main (#53). Fix them all with no behavior change: static sentinel errors wrapped with %w preserving the existing messages (err113), config key name constants (goconst), t.Parallel() throughout except the Setenv/Chdir test (paralleltest), white-box test renamed to config_validation_internal_test.go (testpackage), case tables extracted into builder functions plus a shared runAbortCases helper (funlen/dupl/gochecknoglobals), plain error assignments (noinlineerr), any instead of interface{} and strings.SplitSeq (modernize), slog.DiscardHandler (sloglint), 88-col wrapping (lll), and removal of two stale nolint:gosec directives (nolintlint).
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"math"
|
||||
@@ -25,9 +26,54 @@ const (
|
||||
DefaultUpstreamConnectionsPerHost = 20
|
||||
)
|
||||
|
||||
// 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"
|
||||
keyAllowHTTP = "allow_http"
|
||||
keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
|
||||
)
|
||||
|
||||
// Static validation errors. Each use site attaches the offending key
|
||||
// and value by wrapping these with fmt.Errorf and %w.
|
||||
var (
|
||||
errValueRequired = errors.New("a value is required")
|
||||
errValueEmpty = errors.New("value must not be empty")
|
||||
errUnknownConfigKeys = errors.New("unknown config keys")
|
||||
errNotAString = errors.New("not a string")
|
||||
errNotAnInteger = errors.New("not an integer")
|
||||
errNotABoolean = errors.New("not a boolean")
|
||||
errNotAStringList = errors.New("not a list of strings")
|
||||
errNotAMetricsMap = errors.New("not a map of metrics settings")
|
||||
errEmptyListEntry = errors.New("list contains an empty entry")
|
||||
errEmptyEntry = errors.New("contains an empty entry")
|
||||
errNotAValidURL = errors.New("not a valid URL")
|
||||
errPortOutOfRange = errors.New("outside the valid port range")
|
||||
errTooFewConnections = errors.New("must be at least 1")
|
||||
errValueTooShort = errors.New("value too short")
|
||||
errMustBeSetTogether = errors.New("must be set together")
|
||||
errValueNull = errors.New(
|
||||
"value is null; omit the key entirely to use the default")
|
||||
errValuesNull = errors.New(
|
||||
"value is null; omit a key entirely to use its default")
|
||||
errNotBareHostname = errors.New(
|
||||
"must be a bare hostname without scheme, path, or whitespace")
|
||||
errNoHostnameLabels = errors.New("contains no hostname labels")
|
||||
)
|
||||
|
||||
// Params defines dependencies for Config.
|
||||
type Params struct {
|
||||
fx.In
|
||||
|
||||
Globals *globals.Globals
|
||||
Logger *logger.Logger
|
||||
}
|
||||
@@ -69,7 +115,8 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := c.ensureStateDirWritable(); err != nil {
|
||||
err = c.ensureStateDirWritable()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -87,11 +134,13 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
|
||||
// to omitted keys, never to invalid explicit values.
|
||||
func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
||||
if sc != nil {
|
||||
if err := validateKnownKeys(sc); err != nil {
|
||||
err := validateKnownKeys(sc)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := validateAllowlistHostsValue(sc); err != nil {
|
||||
err = validateAllowlistHostsValue(sc)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
@@ -99,30 +148,30 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
||||
loader := &strictLoader{sc: sc}
|
||||
|
||||
c := &Config{
|
||||
Debug: loader.boolVal("debug", false),
|
||||
MaintenanceMode: loader.boolVal("maintenance_mode", false),
|
||||
Port: loader.intVal("port", DefaultPort),
|
||||
StateDir: loader.stringVal("state_dir", DefaultStateDir),
|
||||
SentryDSN: loader.stringVal("sentry_dsn", ""),
|
||||
MetricsUsername: loader.stringVal("metrics.username", ""),
|
||||
MetricsPassword: loader.stringVal("metrics.password", ""),
|
||||
SigningKey: loader.stringVal("signing_key", ""),
|
||||
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("allow_http", false),
|
||||
AllowHTTP: loader.boolVal(keyAllowHTTP, false),
|
||||
UpstreamConnectionsPerHost: loader.intVal(
|
||||
"upstream_connections_per_host", DefaultUpstreamConnectionsPerHost),
|
||||
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
|
||||
}
|
||||
|
||||
// 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("db_url", "")
|
||||
c.DBURL = loader.stringVal(keyDBURL, "")
|
||||
if c.DBURL == "" && loader.err == nil {
|
||||
if sc != nil {
|
||||
if _, present := sc.Get("db_url"); present {
|
||||
if _, present := sc.Get(keyDBURL); present {
|
||||
return nil, fmt.Errorf(
|
||||
"config key %q: value must not be empty; omit the key to derive it from state_dir",
|
||||
"db_url")
|
||||
"config key %q: %w; omit the key to derive it from state_dir",
|
||||
keyDBURL, errValueEmpty)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -133,7 +182,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
||||
return nil, loader.err
|
||||
}
|
||||
|
||||
if err := c.validate(); err != nil {
|
||||
err := c.validate()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -162,23 +212,22 @@ func validateKnownKeys(sc *smartconfig.Config) error {
|
||||
continue
|
||||
}
|
||||
|
||||
if key == "metrics" {
|
||||
metricsMap, ok := value.(map[string]interface{})
|
||||
if key == keyMetrics {
|
||||
metricsMap, ok := value.(map[string]any)
|
||||
if !ok {
|
||||
return fmt.Errorf(
|
||||
"config key %q: value %v is not a map of metrics settings",
|
||||
"metrics", value)
|
||||
return fmt.Errorf("config key %q: value %v is %w",
|
||||
keyMetrics, value, errNotAMetricsMap)
|
||||
}
|
||||
|
||||
for subkey, subvalue := range metricsMap {
|
||||
if subkey != "username" && subkey != "password" {
|
||||
unknown = append(unknown, "metrics."+subkey)
|
||||
unknown = append(unknown, keyMetrics+"."+subkey)
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
if subvalue == nil {
|
||||
nullKeys = append(nullKeys, "metrics."+subkey)
|
||||
nullKeys = append(nullKeys, keyMetrics+"."+subkey)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -187,7 +236,7 @@ func validateKnownKeys(sc *smartconfig.Config) error {
|
||||
if len(unknown) > 0 {
|
||||
sort.Strings(unknown)
|
||||
|
||||
return fmt.Errorf("unknown config keys: %s", strings.Join(unknown, ", "))
|
||||
return fmt.Errorf("%w: %s", errUnknownConfigKeys, strings.Join(unknown, ", "))
|
||||
}
|
||||
|
||||
if len(nullKeys) > 0 {
|
||||
@@ -197,9 +246,8 @@ func validateKnownKeys(sc *smartconfig.Config) error {
|
||||
return errNullConfigValue(nullKeys[0])
|
||||
}
|
||||
|
||||
return fmt.Errorf(
|
||||
"config keys %s: value is null; omit a key entirely to use its default",
|
||||
strings.Join(nullKeys, ", "))
|
||||
return fmt.Errorf("config keys %s: %w",
|
||||
strings.Join(nullKeys, ", "), errValuesNull)
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -209,17 +257,16 @@ func validateKnownKeys(sc *smartconfig.Config) error {
|
||||
// null (including the bare "key:" form and the "~" alias). Silently
|
||||
// applying the default would mask a truncated or typo'd config entry.
|
||||
func errNullConfigValue(key string) error {
|
||||
return fmt.Errorf(
|
||||
"config key %q: value is null; omit the key entirely to use the default", key)
|
||||
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 "debug", "maintenance_mode", "port", "state_dir", "sentry_dsn",
|
||||
"db_url", "metrics", "signing_key", "allowlist_hosts", "allow_http",
|
||||
"upstream_connections_per_host", "env":
|
||||
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
|
||||
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
|
||||
keyUpstreamConnectionsPerHost, "env":
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -232,28 +279,30 @@ func isKnownConfigKey(key string) bool {
|
||||
func (c *Config) ensureStateDirWritable() error {
|
||||
const stateDirPerms = 0o750
|
||||
|
||||
if err := os.MkdirAll(c.StateDir, stateDirPerms); err != nil {
|
||||
err := os.MkdirAll(c.StateDir, stateDirPerms)
|
||||
if err != nil {
|
||||
return fmt.Errorf("config key %q: cannot create directory %q: %w",
|
||||
"state_dir", c.StateDir, err)
|
||||
keyStateDir, c.StateDir, err)
|
||||
}
|
||||
|
||||
probe, err := os.CreateTemp(c.StateDir, ".startup-write-probe-*")
|
||||
if err != nil {
|
||||
return fmt.Errorf("config key %q: directory %q is not writable: %w",
|
||||
"state_dir", c.StateDir, err)
|
||||
keyStateDir, c.StateDir, err)
|
||||
}
|
||||
|
||||
probePath := probe.Name()
|
||||
|
||||
if err := probe.Close(); err != nil {
|
||||
err = probe.Close()
|
||||
if err != nil {
|
||||
return fmt.Errorf("config key %q: cannot close probe file %q: %w",
|
||||
"state_dir", probePath, err)
|
||||
keyStateDir, probePath, err)
|
||||
}
|
||||
|
||||
//nolint:gosec // G703: probePath comes from os.CreateTemp inside the just-validated StateDir
|
||||
if err := os.Remove(probePath); err != nil {
|
||||
err = os.Remove(probePath)
|
||||
if err != nil {
|
||||
return fmt.Errorf("config key %q: cannot remove probe file %q: %w",
|
||||
"state_dir", probePath, err)
|
||||
keyStateDir, probePath, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -264,33 +313,35 @@ func (c *Config) ensureStateDirWritable() error {
|
||||
func (c *Config) validate() error {
|
||||
// The signing key value is never echoed in error messages.
|
||||
if c.SigningKey == "" {
|
||||
return fmt.Errorf("config key %q: a value is required", "signing_key")
|
||||
return fmt.Errorf("config key %q: %w", keySigningKey, errValueRequired)
|
||||
}
|
||||
|
||||
// Minimum key length for security (32 bytes = 256 bits)
|
||||
const minKeyLength = 32
|
||||
if len(c.SigningKey) < minKeyLength {
|
||||
return fmt.Errorf("config key %q: value must be at least %d characters, got %d",
|
||||
"signing_key", minKeyLength, len(c.SigningKey))
|
||||
return fmt.Errorf("config key %q: %w: must be at least %d characters, got %d",
|
||||
keySigningKey, errValueTooShort, minKeyLength, len(c.SigningKey))
|
||||
}
|
||||
|
||||
const maxPort = 65535
|
||||
if c.Port < 1 || c.Port > maxPort {
|
||||
return fmt.Errorf("config key %q: value %d is outside the valid port range 1-%d",
|
||||
"port", c.Port, maxPort)
|
||||
return fmt.Errorf("config key %q: value %d is %w 1-%d",
|
||||
keyPort, c.Port, errPortOutOfRange, maxPort)
|
||||
}
|
||||
|
||||
if c.UpstreamConnectionsPerHost < 1 {
|
||||
return fmt.Errorf("config key %q: value %d must be at least 1",
|
||||
"upstream_connections_per_host", c.UpstreamConnectionsPerHost)
|
||||
return fmt.Errorf("config key %q: value %d %w",
|
||||
keyUpstreamConnectionsPerHost, c.UpstreamConnectionsPerHost,
|
||||
errTooFewConnections)
|
||||
}
|
||||
|
||||
if c.StateDir == "" {
|
||||
return fmt.Errorf("config key %q: value must not be empty", "state_dir")
|
||||
return fmt.Errorf("config key %q: %w", keyStateDir, errValueEmpty)
|
||||
}
|
||||
|
||||
for _, host := range c.AllowlistHosts {
|
||||
if err := validateAllowlistHost(host); err != nil {
|
||||
err := validateAllowlistHost(host)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
@@ -298,14 +349,14 @@ func (c *Config) validate() error {
|
||||
if c.SentryDSN != "" {
|
||||
parsed, err := url.Parse(c.SentryDSN)
|
||||
if err != nil || parsed.Scheme == "" || parsed.Host == "" {
|
||||
return fmt.Errorf("config key %q: value %q is not a valid URL",
|
||||
"sentry_dsn", c.SentryDSN)
|
||||
return fmt.Errorf("config key %q: value %q is %w",
|
||||
keySentryDSN, c.SentryDSN, errNotAValidURL)
|
||||
}
|
||||
}
|
||||
|
||||
if (c.MetricsUsername == "") != (c.MetricsPassword == "") {
|
||||
return fmt.Errorf("config keys %q and %q must be set together",
|
||||
"metrics.username", "metrics.password")
|
||||
return fmt.Errorf("config keys %q and %q %w",
|
||||
keyMetricsUsername, keyMetricsPassword, errMustBeSetTogether)
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -320,21 +371,20 @@ func (c *Config) validate() error {
|
||||
// disable URL signing.
|
||||
func validateAllowlistHost(host string) error {
|
||||
if strings.Contains(host, "://") || strings.ContainsAny(host, "/ \t") {
|
||||
return fmt.Errorf(
|
||||
"config key %q: entry %q must be a bare hostname without scheme, path, or whitespace",
|
||||
"allowlist_hosts", host)
|
||||
return fmt.Errorf("config key %q: entry %q %w",
|
||||
keyAllowlistHosts, host, errNotBareHostname)
|
||||
}
|
||||
|
||||
if strings.Trim(host, ".") == "" {
|
||||
return fmt.Errorf(
|
||||
"config key %q: entry %q contains no hostname labels",
|
||||
"allowlist_hosts", host)
|
||||
return fmt.Errorf("config key %q: entry %q %w",
|
||||
keyAllowlistHosts, host, errNoHostnameLabels)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// loadConfigFile loads configuration from PIXA_CONFIG_PATH env var or standard locations.
|
||||
// loadConfigFile loads configuration from the PIXA_CONFIG_PATH env var
|
||||
// or standard locations.
|
||||
func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, error) {
|
||||
// Check for explicit config path from environment
|
||||
if envPath := os.Getenv("PIXA_CONFIG_PATH"); envPath != "" {
|
||||
@@ -360,8 +410,9 @@ func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, erro
|
||||
|
||||
for _, path := range configPaths {
|
||||
cleanPath := filepath.Clean(path)
|
||||
//nolint:gosec // G703: paths are hardcoded config locations
|
||||
if _, statErr := os.Stat(cleanPath); statErr == nil {
|
||||
|
||||
_, statErr := os.Stat(cleanPath)
|
||||
if statErr == nil {
|
||||
// A config file that exists but does not parse is a fatal
|
||||
// startup error, never something to skip over.
|
||||
sc, err := smartconfig.NewFromConfigPath(path)
|
||||
@@ -444,8 +495,8 @@ func getString(sc *smartconfig.Config, key, defaultVal string) (string, error) {
|
||||
|
||||
str, ok := raw.(string)
|
||||
if !ok {
|
||||
return "", fmt.Errorf("config key %q: value %v (%T) is not a string",
|
||||
key, raw, raw)
|
||||
return "", fmt.Errorf("config key %q: value %v (%T) is %w",
|
||||
key, raw, raw, errNotAString)
|
||||
}
|
||||
|
||||
return str, nil
|
||||
@@ -475,20 +526,22 @@ func getInt(sc *smartconfig.Config, key string, defaultVal int) (int, error) {
|
||||
return int(val), nil
|
||||
case float64:
|
||||
if val != math.Trunc(val) {
|
||||
return 0, fmt.Errorf("config key %q: value %v is not an integer", key, val)
|
||||
return 0, fmt.Errorf("config key %q: value %v is %w",
|
||||
key, val, errNotAnInteger)
|
||||
}
|
||||
|
||||
return int(val), nil
|
||||
case string:
|
||||
parsed, err := strconv.Atoi(strings.TrimSpace(val))
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("config key %q: value %q is not an integer", key, val)
|
||||
return 0, fmt.Errorf("config key %q: value %q is %w",
|
||||
key, val, errNotAnInteger)
|
||||
}
|
||||
|
||||
return parsed, nil
|
||||
default:
|
||||
return 0, fmt.Errorf("config key %q: value %v (%T) is not an integer",
|
||||
key, raw, raw)
|
||||
return 0, fmt.Errorf("config key %q: value %v (%T) is %w",
|
||||
key, raw, raw, errNotAnInteger)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -516,13 +569,14 @@ func getBool(sc *smartconfig.Config, key string, defaultVal bool) (bool, error)
|
||||
case string:
|
||||
parsed, err := strconv.ParseBool(strings.TrimSpace(val))
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("config key %q: value %q is not a boolean", key, val)
|
||||
return false, fmt.Errorf("config key %q: value %q is %w",
|
||||
key, val, errNotABoolean)
|
||||
}
|
||||
|
||||
return parsed, nil
|
||||
default:
|
||||
return false, fmt.Errorf("config key %q: value %v (%T) is not a boolean",
|
||||
key, raw, raw)
|
||||
return false, fmt.Errorf("config key %q: value %v (%T) is %w",
|
||||
key, raw, raw, errNotABoolean)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -532,28 +586,27 @@ func getBool(sc *smartconfig.Config, key string, defaultVal bool) (bool, error)
|
||||
// (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 {
|
||||
const key = "allowlist_hosts"
|
||||
|
||||
raw, ok := sc.Get(key)
|
||||
raw, ok := sc.Get(keyAllowlistHosts)
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
|
||||
if raw == nil {
|
||||
return errNullConfigValue(key)
|
||||
return errNullConfigValue(keyAllowlistHosts)
|
||||
}
|
||||
|
||||
switch val := raw.(type) {
|
||||
case []interface{}:
|
||||
case []any:
|
||||
for _, item := range val {
|
||||
str, ok := item.(string)
|
||||
if !ok {
|
||||
return fmt.Errorf(
|
||||
"config key %q: list entry %v (%T) is not a string", key, item, item)
|
||||
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: list contains an empty entry", key)
|
||||
return fmt.Errorf("config key %q: %w",
|
||||
keyAllowlistHosts, errEmptyListEntry)
|
||||
}
|
||||
}
|
||||
case string:
|
||||
@@ -561,15 +614,15 @@ func validateAllowlistHostsValue(sc *smartconfig.Config) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
for _, part := range strings.Split(val, ",") {
|
||||
for part := range strings.SplitSeq(val, ",") {
|
||||
if strings.TrimSpace(part) == "" {
|
||||
return fmt.Errorf(
|
||||
"config key %q: value %q contains an empty entry", key, val)
|
||||
return fmt.Errorf("config key %q: value %q %w",
|
||||
keyAllowlistHosts, val, errEmptyEntry)
|
||||
}
|
||||
}
|
||||
default:
|
||||
return fmt.Errorf("config key %q: value %v (%T) is not a list of strings",
|
||||
key, raw, raw)
|
||||
return fmt.Errorf("config key %q: value %v (%T) is %w",
|
||||
keyAllowlistHosts, raw, raw, errNotAStringList)
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -584,13 +637,13 @@ func getStringSlice(sc *smartconfig.Config) []string {
|
||||
return nil
|
||||
}
|
||||
|
||||
val, ok := sc.Get("allowlist_hosts")
|
||||
val, ok := sc.Get(keyAllowlistHosts)
|
||||
if !ok || val == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Handle YAML list format
|
||||
if slice, ok := val.([]interface{}); ok {
|
||||
if slice, ok := val.([]any); ok {
|
||||
result := make([]string, 0, len(slice))
|
||||
for _, item := range slice {
|
||||
if str, ok := item.(string); ok {
|
||||
|
||||
Reference in New Issue
Block a user