diff --git a/README.md b/README.md index f971096..b8ecfe5 100644 --- a/README.md +++ b/README.md @@ -320,8 +320,8 @@ the following precedence (highest to lowest): | `DNSWATCHER_SLACK_WEBHOOK` | Slack incoming webhook URL | `""` | | `DNSWATCHER_MATTERMOST_WEBHOOK` | Mattermost incoming webhook URL | `""` | | `DNSWATCHER_NTFY_TOPIC` | ntfy topic URL | `""` | -| `DNSWATCHER_DNS_INTERVAL` | DNS check interval | `1h` | -| `DNSWATCHER_TLS_INTERVAL` | TLS check interval | `12h` | +| `DNSWATCHER_DNS_INTERVAL` | DNS check interval, a positive duration such as `30m`; empty means the default, anything else stops startup | `1h` | +| `DNSWATCHER_TLS_INTERVAL` | TLS check interval, a positive duration such as `6h`; empty means the default, anything else stops startup | `12h` | | `DNSWATCHER_TLS_EXPIRY_WARNING` | Days before expiry to warn | `7` | | `DNSWATCHER_SENTRY_DSN` | Sentry DSN for error reporting | `""` | | `DNSWATCHER_MAINTENANCE_MODE` | Enable maintenance mode | `false` | @@ -335,6 +335,14 @@ is a misconfiguration, so dnswatcher fails fast with a clear error message rather than running silently. Set `DNSWATCHER_TARGETS` to a comma-separated list of DNS names before starting. +**`DNSWATCHER_DNS_INTERVAL` and `DNSWATCHER_TLS_INTERVAL`** take a positive +duration: a number followed by a unit such as `s`, `m` or `h`, for example +`90s`, `30m`, `1h` or `1h30m`. There is no unit for days; write `24h`. An +unset 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. + ### Example `.env` ```sh diff --git a/TODO.md b/TODO.md index 8af2f80..4fa8d93 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,8 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104 # Completed Steps +- 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is + not a positive duration stops startup instead of being ignored (closes #177). - 2026-10-01: `TODO.md` brought up to date: open issues listed by URL, every Completed Steps entry cut to at most two lines (closes #146). - 2026-10-01: wildcard CORS now applies only to the public routes, not to @@ -89,8 +91,6 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104 - nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105 - `DNSWATCHER_SENTRY_DSN` does nothing: https://git.eeqj.de/sneak/dnswatcher/issues/107 -- invalid DNS or TLS interval silently replaced by the default: - https://git.eeqj.de/sneak/dnswatcher/issues/177 - rate limit on `/metrics` Basic Auth: https://git.eeqj.de/sneak/dnswatcher/issues/101 - images report version `dev`: https://git.eeqj.de/sneak/dnswatcher/issues/109 diff --git a/internal/config/config.go b/internal/config/config.go index 264b90b..30366f7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,6 +28,12 @@ var ErrNoTargets = errors.New( "no monitoring targets configured: set DNSWATCHER_TARGETS environment variable", ) +// ErrInvalidInterval is returned when DNSWATCHER_DNS_INTERVAL or +// DNSWATCHER_TLS_INTERVAL is set to something other than a positive duration. +var ErrInvalidInterval = errors.New( + "interval must be a positive duration such as 30m or 1h", +) + // Params contains dependencies for Config. type Params struct { fx.In @@ -125,18 +131,14 @@ func buildConfig( } } - dnsInterval, err := time.ParseDuration( - viper.GetString("DNS_INTERVAL"), - ) + dnsInterval, err := parseInterval("DNS_INTERVAL") if err != nil { - dnsInterval = defaultDNSInterval + return nil, err } - tlsInterval, err := time.ParseDuration( - viper.GetString("TLS_INTERVAL"), - ) + tlsInterval, err := parseInterval("TLS_INTERVAL") if err != nil { - tlsInterval = defaultTLSInterval + return nil, err } domains, hostnames, err := parseAndValidateTargets() @@ -168,6 +170,22 @@ func buildConfig( return cfg, 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. +func parseInterval(key string) (time.Duration, error) { + value := viper.GetString(key) + + interval, err := time.ParseDuration(value) + if err != nil || interval <= 0 { + return 0, fmt.Errorf( + "invalid DNSWATCHER_%s %q: %w", key, value, ErrInvalidInterval, + ) + } + + return interval, nil +} + func parseAndValidateTargets() ([]string, []string, error) { domains, hostnames, err := ClassifyTargets( parseCSV(viper.GetString("TARGETS")), diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 2917818..c4471ff 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,6 +1,7 @@ package config_test import ( + "strconv" "testing" "time" @@ -113,33 +114,40 @@ func TestNew_OnlyEmptyCSVSegments(t *testing.T) { assert.ErrorIs(t, err, config.ErrNoTargets) } -func TestNew_InvalidDNSInterval_FallsBackToDefault(t *testing.T) { - viper.Reset() - t.Setenv("DNSWATCHER_TARGETS", "example.com") - t.Setenv("DNSWATCHER_DNS_INTERVAL", "banana") +// TestNew_InvalidIntervalStopsStartup checks values that must stop startup; +// TestNew_DefaultValues and TestNew_EmptyIntervalMeansDefault check that an +// unset or empty interval means the default. +func TestNew_InvalidIntervalStopsStartup(t *testing.T) { + variables := []string{"DNSWATCHER_DNS_INTERVAL", "DNSWATCHER_TLS_INTERVAL"} + values := []string{ + "banana", // not a duration + "5", // no unit + "1d", // days are not a unit time.ParseDuration knows + "0", // zero + "-1h", // negative + } - cfg, err := config.New(nil, newTestParams(t)) - require.NoError(t, err) - assert.Equal(t, time.Hour, cfg.DNSInterval, - "invalid DNS interval should fall back to 1h default") + for _, variable := range variables { + for _, value := range values { + t.Run(variable+"="+value, func(t *testing.T) { + viper.Reset() + t.Setenv("DNSWATCHER_TARGETS", "example.com") + t.Setenv(variable, value) + + _, err := config.New(nil, newTestParams(t)) + require.ErrorIs(t, err, config.ErrInvalidInterval) + require.ErrorContains(t, err, variable) + require.ErrorContains(t, err, strconv.Quote(value)) + }) + } + } } -func TestNew_InvalidTLSInterval_FallsBackToDefault(t *testing.T) { +func TestNew_EmptyIntervalMeansDefault(t *testing.T) { viper.Reset() t.Setenv("DNSWATCHER_TARGETS", "example.com") - t.Setenv("DNSWATCHER_TLS_INTERVAL", "notaduration") - - cfg, err := config.New(nil, newTestParams(t)) - require.NoError(t, err) - assert.Equal(t, 12*time.Hour, cfg.TLSInterval, - "invalid TLS interval should fall back to 12h default") -} - -func TestNew_BothIntervalsInvalid(t *testing.T) { - viper.Reset() - t.Setenv("DNSWATCHER_TARGETS", "example.com") - t.Setenv("DNSWATCHER_DNS_INTERVAL", "xyz") - t.Setenv("DNSWATCHER_TLS_INTERVAL", "abc") + t.Setenv("DNSWATCHER_DNS_INTERVAL", "") + t.Setenv("DNSWATCHER_TLS_INTERVAL", "") cfg, err := config.New(nil, newTestParams(t)) require.NoError(t, err)