config: a bad number or true/false setting stops startup (closes #269) #274

Merged
clawbot merged 1 commits from fix/269-strict-config-values into next 2026-10-13 15:27:34 +02:00
4 changed files with 220 additions and 16 deletions
+10
View File
@@ -463,6 +463,16 @@ or empty variable (`DNSWATCHER_DNS_INTERVAL=`) means the default. If either is
set to anything else, including a bare number or a zero or negative duration,
dnswatcher refuses to start with an error naming the variable and the value.
**`PORT` and `DNSWATCHER_TLS_EXPIRY_WARNING`** take a whole number: 1 to 65535
for `PORT`, and a number of days, 1 or more, for
`DNSWATCHER_TLS_EXPIRY_WARNING`. `DNSWATCHER_DEBUG`,
`DNSWATCHER_MAINTENANCE_MODE` and `DNSWATCHER_SEND_TEST_NOTIFICATION` take
`true` or `false`; `1`, `t`, `T`, `TRUE` and `True` also mean true, and `0`,
`f`, `F`, `FALSE` and `False` also mean false. An unset or empty variable means
the default. Any other value, such as `PORT=http`,
`DNSWATCHER_TLS_EXPIRY_WARNING=7d` or `DNSWATCHER_DEBUG=yes`, makes dnswatcher
refuse to start with an error naming the variable and the value.
**`DNSWATCHER_SENTRY_DSN` reports crashes in HTTP requests to Sentry.** When it
is set, a panic in an HTTP request handler is sent to Sentry, and the request
still gets a `500 Internal Server Error` answer. Nothing else is sent to Sentry:
+2
View File
@@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
# Completed Steps
- 2026-10-13: a bad `PORT`, `DNSWATCHER_TLS_EXPIRY_WARNING` or true/false
setting stops startup instead of being read as 0 or false (closes #269).
- 2026-10-13: a check saving the state while the dashboard or `/api/v1/status`
is read cannot crash the process (closes #261).
- 2026-10-06: canonical files re-fetched from `sneak/prompts` at `dd4027b`; lint
+115 -14
View File
@@ -6,6 +6,7 @@ import (
"fmt"
"log/slog"
"slices"
"strconv"
"strings"
"time"
@@ -24,6 +25,9 @@ const (
defaultTLSExpiryWarning = 7
)
// maxPort is the highest TCP port number.
const maxPort = 65535
// ErrNoTargets is returned when no monitoring targets are configured.
var ErrNoTargets = errors.New(
"no monitoring targets configured: set DNSWATCHER_TARGETS environment variable",
@@ -40,6 +44,23 @@ var ErrInvalidInterval = errors.New(
// lists a name that is not in DNSWATCHER_TARGETS.
var ErrNotInTargets = errors.New("name is not in DNSWATCHER_TARGETS")
// ErrInvalidPort is returned when PORT is set but is not a whole number
// from 1 to 65535. An empty value counts as unset and means the default.
var ErrInvalidPort = errors.New("port must be a whole number from 1 to 65535")
// ErrInvalidTLSExpiryWarning is returned when DNSWATCHER_TLS_EXPIRY_WARNING
// is set but is not a whole number, 1 or more. An empty value counts as unset
// and means the default.
var ErrInvalidTLSExpiryWarning = errors.New(
"TLS expiry warning must be a whole number of days, 1 or more",
)
// ErrInvalidBool is returned when DNSWATCHER_DEBUG,
// DNSWATCHER_MAINTENANCE_MODE or DNSWATCHER_SEND_TEST_NOTIFICATION is set
// but is not a value strconv.ParseBool accepts. An empty value counts as
// unset and means the default.
var ErrInvalidBool = errors.New("value must be true or false")
// Params contains dependencies for Config.
type Params struct {
fx.In
@@ -127,16 +148,9 @@ func buildConfig(
log *slog.Logger,
params *Params,
) (*Config, error) {
err := viper.ReadInConfig()
err := readConfigFile(log)
if err != nil {
var notFound viper.ConfigFileNotFoundError
if !errors.As(err, &notFound) {
log.Error("config file malformed", "error", err)
return nil, fmt.Errorf(
"config file malformed: %w", err,
)
}
return nil, err
}
dnsInterval, err := parseInterval("DNS_INTERVAL")
@@ -161,9 +175,34 @@ func buildConfig(
return nil, err
}
port, err := parsePort()
if err != nil {
return nil, err
}
tlsExpiryWarning, err := parseTLSExpiryWarning()
if err != nil {
return nil, err
}
debug, err := parseBool("DEBUG")
if err != nil {
return nil, err
}
maintenanceMode, err := parseBool("MAINTENANCE_MODE")
if err != nil {
return nil, err
}
sendTestNotification, err := parseBool("SEND_TEST_NOTIFICATION")
if err != nil {
return nil, err
}
cfg := &Config{
Port: viper.GetInt("PORT"),
Debug: viper.GetBool("DEBUG"),
Port: port,
Debug: debug,
DataDir: viper.GetString("DATA_DIR"),
Domains: domains,
Hostnames: hostnames,
@@ -173,12 +212,12 @@ func buildConfig(
NtfyTopic: viper.GetString("NTFY_TOPIC"),
DNSInterval: dnsInterval,
TLSInterval: tlsInterval,
TLSExpiryWarning: viper.GetInt("TLS_EXPIRY_WARNING"),
TLSExpiryWarning: tlsExpiryWarning,
SentryDSN: viper.GetString("SENTRY_DSN"),
MaintenanceMode: viper.GetBool("MAINTENANCE_MODE"),
MaintenanceMode: maintenanceMode,
MetricsUsername: viper.GetString("METRICS_USERNAME"),
MetricsPassword: viper.GetString("METRICS_PASSWORD"),
SendTestNotification: viper.GetBool("SEND_TEST_NOTIFICATION"),
SendTestNotification: sendTestNotification,
params: params,
log: log,
}
@@ -186,6 +225,21 @@ func buildConfig(
return cfg, nil
}
// readConfigFile reads the config file, if there is one.
func readConfigFile(log *slog.Logger) error {
err := viper.ReadInConfig()
if err != nil {
var notFound viper.ConfigFileNotFoundError
if !errors.As(err, &notFound) {
log.Error("config file malformed", "error", err)
return fmt.Errorf("config file malformed: %w", err)
}
}
return nil
}
// parseInterval reads the DNSWATCHER_-prefixed setting key as a duration. A
// value that does not parse, or is zero or negative, is an error naming the
// variable and the value; an unset variable has its default from setupViper.
@@ -202,6 +256,53 @@ func parseInterval(key string) (time.Duration, error) {
return interval, nil
}
// parsePort reads PORT, which has no DNSWATCHER_ prefix, as a port number. A
// value that is not a whole number from 1 to 65535 is an error naming the
// variable and the value; an unset variable has its default from setupViper.
func parsePort() (int, error) {
value := viper.GetString("PORT")
port, err := strconv.Atoi(value)
if err != nil || port < 1 || port > maxPort {
return 0, fmt.Errorf("invalid PORT %q: %w", value, ErrInvalidPort)
}
return port, nil
}
// parseTLSExpiryWarning reads DNSWATCHER_TLS_EXPIRY_WARNING as a number of
// days. A value that is not a whole number, 1 or more, is an error naming the
// variable and the value; an unset variable has its default from setupViper.
func parseTLSExpiryWarning() (int, error) {
value := viper.GetString("TLS_EXPIRY_WARNING")
days, err := strconv.Atoi(value)
if err != nil || days < 1 {
return 0, fmt.Errorf(
"invalid DNSWATCHER_TLS_EXPIRY_WARNING %q: %w",
value, ErrInvalidTLSExpiryWarning,
)
}
return days, nil
}
// parseBool reads the DNSWATCHER_-prefixed setting key as true or false. A
// value strconv.ParseBool does not accept is an error naming the variable and
// the value; an unset variable has its default from setupViper.
func parseBool(key string) (bool, error) {
value := viper.GetString(key)
result, err := strconv.ParseBool(value)
if err != nil {
return false, fmt.Errorf(
"invalid DNSWATCHER_%s %q: %w", key, value, ErrInvalidBool,
)
}
return result, nil
}
func parseAndValidateTargets() ([]string, []string, error) {
domains, hostnames, err := ClassifyTargets(
parseCSV(viper.GetString("TARGETS")),
+93 -2
View File
@@ -116,7 +116,7 @@ func TestNew_OnlyEmptyCSVSegments(t *testing.T) {
}
// TestNew_InvalidIntervalStopsStartup checks values that must stop startup;
// TestNew_DefaultValues and TestNew_EmptyIntervalMeansDefault check that an
// TestNew_DefaultValues and TestNew_EmptyValueMeansDefault check that an
// unset or empty interval means the default.
func TestNew_InvalidIntervalStopsStartup(t *testing.T) {
variables := []string{"DNSWATCHER_DNS_INTERVAL", "DNSWATCHER_TLS_INTERVAL"}
@@ -144,16 +144,107 @@ func TestNew_InvalidIntervalStopsStartup(t *testing.T) {
}
}
func TestNew_EmptyIntervalMeansDefault(t *testing.T) {
func TestNew_EmptyValueMeansDefault(t *testing.T) {
viper.Reset()
t.Setenv("DNSWATCHER_TARGETS", "example.com")
t.Setenv("DNSWATCHER_DNS_INTERVAL", "")
t.Setenv("DNSWATCHER_TLS_INTERVAL", "")
t.Setenv("PORT", "")
t.Setenv("DNSWATCHER_TLS_EXPIRY_WARNING", "")
t.Setenv("DNSWATCHER_DEBUG", "")
t.Setenv("DNSWATCHER_MAINTENANCE_MODE", "")
t.Setenv("DNSWATCHER_SEND_TEST_NOTIFICATION", "")
cfg, err := config.New(nil, newTestParams(t))
require.NoError(t, err)
assert.Equal(t, time.Hour, cfg.DNSInterval)
assert.Equal(t, 12*time.Hour, cfg.TLSInterval)
assert.Equal(t, 8080, cfg.Port)
assert.Equal(t, 7, cfg.TLSExpiryWarning)
assert.False(t, cfg.Debug)
assert.False(t, cfg.MaintenanceMode)
assert.False(t, cfg.SendTestNotification)
}
// TestNew_InvalidNumberOrBoolStopsStartup checks values that must stop
// startup; TestNew_EnvironmentOverrides, the two tests after this one and
// TestNew_EmptyValueMeansDefault check values that must not.
func TestNew_InvalidNumberOrBoolStopsStartup(t *testing.T) {
notBools := []string{"yes", "on", "2", "tRUE"}
tests := []struct {
variable string
values []string
want error
}{
{
variable: "PORT",
values: []string{"notaport", "0", "-1", "65536", "8080.5"},
want: config.ErrInvalidPort,
},
{
variable: "DNSWATCHER_TLS_EXPIRY_WARNING",
values: []string{"abc", "7d", "0", "-5"},
want: config.ErrInvalidTLSExpiryWarning,
},
{
variable: "DNSWATCHER_DEBUG",
values: notBools,
want: config.ErrInvalidBool,
},
{
variable: "DNSWATCHER_MAINTENANCE_MODE",
values: notBools,
want: config.ErrInvalidBool,
},
{
variable: "DNSWATCHER_SEND_TEST_NOTIFICATION",
values: notBools,
want: config.ErrInvalidBool,
},
}
for _, tt := range tests {
for _, value := range tt.values {
t.Run(tt.variable+"="+value, func(t *testing.T) {
viper.Reset()
t.Setenv("DNSWATCHER_TARGETS", "example.com")
t.Setenv(tt.variable, value)
_, err := config.New(nil, newTestParams(t))
require.ErrorIs(t, err, tt.want)
require.ErrorContains(t, err, tt.variable)
require.ErrorContains(t, err, strconv.Quote(value))
})
}
}
}
func TestNew_LowestNumbersAndOtherSpellingsOfTrue(t *testing.T) {
viper.Reset()
t.Setenv("DNSWATCHER_TARGETS", "example.com")
t.Setenv("PORT", "1")
t.Setenv("DNSWATCHER_TLS_EXPIRY_WARNING", "1")
t.Setenv("DNSWATCHER_DEBUG", "1")
t.Setenv("DNSWATCHER_MAINTENANCE_MODE", "T")
t.Setenv("DNSWATCHER_SEND_TEST_NOTIFICATION", "True")
cfg, err := config.New(nil, newTestParams(t))
require.NoError(t, err)
assert.Equal(t, 1, cfg.Port)
assert.Equal(t, 1, cfg.TLSExpiryWarning)
assert.True(t, cfg.Debug)
assert.True(t, cfg.MaintenanceMode)
assert.True(t, cfg.SendTestNotification)
}
func TestNew_HighestPort(t *testing.T) {
viper.Reset()
t.Setenv("DNSWATCHER_TARGETS", "example.com")
t.Setenv("PORT", "65535")
cfg, err := config.New(nil, newTestParams(t))
require.NoError(t, err)
assert.Equal(t, 65535, cfg.Port)
}
func TestNew_DebugEnablesDebugLogging(t *testing.T) {