config: a bad number or true/false setting stops startup (closes #269)
check / check (push) Waiting to run
check / check (push) Waiting to run
PORT and DNSWATCHER_TLS_EXPIRY_WARNING were read with viper.GetInt, and DNSWATCHER_DEBUG, DNSWATCHER_MAINTENANCE_MODE and DNSWATCHER_SEND_TEST_NOTIFICATION with viper.GetBool, which turn a value that does not parse into 0 or false with no error. Each is now parsed the way parseInterval parses an interval: PORT must be 1 to 65535, the expiry warning 1 or more days, the true/false settings a value strconv.ParseBool accepts. Anything else stops startup with an error naming the variable and the value; empty still means the default. Reading the config file moved, unchanged, from buildConfig into readConfigFile, so buildConfig stays within the linter's length limit. Model: opus-5-5
This commit is contained in:
@@ -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,
|
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.
|
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
|
**`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
|
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:
|
still gets a `500 Internal Server Error` answer. Nothing else is sent to Sentry:
|
||||||
|
|||||||
@@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
|
|||||||
|
|
||||||
# Completed Steps
|
# 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`
|
- 2026-10-13: a check saving the state while the dashboard or `/api/v1/status`
|
||||||
is read cannot crash the process (closes #261).
|
is read cannot crash the process (closes #261).
|
||||||
- 2026-10-06: canonical files re-fetched from `sneak/prompts` at `dd4027b`; lint
|
- 2026-10-06: canonical files re-fetched from `sneak/prompts` at `dd4027b`; lint
|
||||||
|
|||||||
+115
-14
@@ -6,6 +6,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"slices"
|
"slices"
|
||||||
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -24,6 +25,9 @@ const (
|
|||||||
defaultTLSExpiryWarning = 7
|
defaultTLSExpiryWarning = 7
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// maxPort is the highest TCP port number.
|
||||||
|
const maxPort = 65535
|
||||||
|
|
||||||
// ErrNoTargets is returned when no monitoring targets are configured.
|
// ErrNoTargets is returned when no monitoring targets are configured.
|
||||||
var ErrNoTargets = errors.New(
|
var ErrNoTargets = errors.New(
|
||||||
"no monitoring targets configured: set DNSWATCHER_TARGETS environment variable",
|
"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.
|
// lists a name that is not in DNSWATCHER_TARGETS.
|
||||||
var ErrNotInTargets = errors.New("name 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.
|
// Params contains dependencies for Config.
|
||||||
type Params struct {
|
type Params struct {
|
||||||
fx.In
|
fx.In
|
||||||
@@ -127,16 +148,9 @@ func buildConfig(
|
|||||||
log *slog.Logger,
|
log *slog.Logger,
|
||||||
params *Params,
|
params *Params,
|
||||||
) (*Config, error) {
|
) (*Config, error) {
|
||||||
err := viper.ReadInConfig()
|
err := readConfigFile(log)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
var notFound viper.ConfigFileNotFoundError
|
return nil, err
|
||||||
if !errors.As(err, ¬Found) {
|
|
||||||
log.Error("config file malformed", "error", err)
|
|
||||||
|
|
||||||
return nil, fmt.Errorf(
|
|
||||||
"config file malformed: %w", err,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
dnsInterval, err := parseInterval("DNS_INTERVAL")
|
dnsInterval, err := parseInterval("DNS_INTERVAL")
|
||||||
@@ -161,9 +175,34 @@ func buildConfig(
|
|||||||
return nil, err
|
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{
|
cfg := &Config{
|
||||||
Port: viper.GetInt("PORT"),
|
Port: port,
|
||||||
Debug: viper.GetBool("DEBUG"),
|
Debug: debug,
|
||||||
DataDir: viper.GetString("DATA_DIR"),
|
DataDir: viper.GetString("DATA_DIR"),
|
||||||
Domains: domains,
|
Domains: domains,
|
||||||
Hostnames: hostnames,
|
Hostnames: hostnames,
|
||||||
@@ -173,12 +212,12 @@ func buildConfig(
|
|||||||
NtfyTopic: viper.GetString("NTFY_TOPIC"),
|
NtfyTopic: viper.GetString("NTFY_TOPIC"),
|
||||||
DNSInterval: dnsInterval,
|
DNSInterval: dnsInterval,
|
||||||
TLSInterval: tlsInterval,
|
TLSInterval: tlsInterval,
|
||||||
TLSExpiryWarning: viper.GetInt("TLS_EXPIRY_WARNING"),
|
TLSExpiryWarning: tlsExpiryWarning,
|
||||||
SentryDSN: viper.GetString("SENTRY_DSN"),
|
SentryDSN: viper.GetString("SENTRY_DSN"),
|
||||||
MaintenanceMode: viper.GetBool("MAINTENANCE_MODE"),
|
MaintenanceMode: maintenanceMode,
|
||||||
MetricsUsername: viper.GetString("METRICS_USERNAME"),
|
MetricsUsername: viper.GetString("METRICS_USERNAME"),
|
||||||
MetricsPassword: viper.GetString("METRICS_PASSWORD"),
|
MetricsPassword: viper.GetString("METRICS_PASSWORD"),
|
||||||
SendTestNotification: viper.GetBool("SEND_TEST_NOTIFICATION"),
|
SendTestNotification: sendTestNotification,
|
||||||
params: params,
|
params: params,
|
||||||
log: log,
|
log: log,
|
||||||
}
|
}
|
||||||
@@ -186,6 +225,21 @@ func buildConfig(
|
|||||||
return cfg, nil
|
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, ¬Found) {
|
||||||
|
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
|
// 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
|
// 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.
|
// 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
|
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) {
|
func parseAndValidateTargets() ([]string, []string, error) {
|
||||||
domains, hostnames, err := ClassifyTargets(
|
domains, hostnames, err := ClassifyTargets(
|
||||||
parseCSV(viper.GetString("TARGETS")),
|
parseCSV(viper.GetString("TARGETS")),
|
||||||
|
|||||||
@@ -116,7 +116,7 @@ func TestNew_OnlyEmptyCSVSegments(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// TestNew_InvalidIntervalStopsStartup checks values that must stop startup;
|
// 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.
|
// unset or empty interval means the default.
|
||||||
func TestNew_InvalidIntervalStopsStartup(t *testing.T) {
|
func TestNew_InvalidIntervalStopsStartup(t *testing.T) {
|
||||||
variables := []string{"DNSWATCHER_DNS_INTERVAL", "DNSWATCHER_TLS_INTERVAL"}
|
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()
|
viper.Reset()
|
||||||
t.Setenv("DNSWATCHER_TARGETS", "example.com")
|
t.Setenv("DNSWATCHER_TARGETS", "example.com")
|
||||||
t.Setenv("DNSWATCHER_DNS_INTERVAL", "")
|
t.Setenv("DNSWATCHER_DNS_INTERVAL", "")
|
||||||
t.Setenv("DNSWATCHER_TLS_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))
|
cfg, err := config.New(nil, newTestParams(t))
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
assert.Equal(t, time.Hour, cfg.DNSInterval)
|
assert.Equal(t, time.Hour, cfg.DNSInterval)
|
||||||
assert.Equal(t, 12*time.Hour, cfg.TLSInterval)
|
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) {
|
func TestNew_DebugEnablesDebugLogging(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user