From f9fc9e882ab55dc3907101de12d6e8a4345c29d5 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 1 Oct 2026 19:07:45 +0000 Subject: [PATCH] config: stop startup on an invalid DNS or TLS interval (closes #177) DNSWATCHER_DNS_INTERVAL and DNSWATCHER_TLS_INTERVAL were parsed with time.ParseDuration and silently replaced by the default when that failed, so a value like 5 or 1d gave hourly checks with no hint why, and zero or negative values were accepted. Both now go through parseInterval, which returns an error naming the variable and the value, and startup stops the same way it does for invalid targets. An unset variable still gets its default from setupViper. The three tests that pinned the old fallback are replaced. The README says what a valid value looks like. Model: opus-5-5 --- README.md | 11 +++++-- TODO.md | 2 ++ internal/config/config.go | 34 ++++++++++++++++----- internal/config/config_test.go | 55 ++++++++++++++++------------------ 4 files changed, 62 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 1e6294b..ab8d383 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`; anything else stops startup | `1h` | +| `DNSWATCHER_TLS_INTERVAL` | TLS check interval, a positive duration such as `6h`; 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,13 @@ 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`. 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. An unset variable means the default. + ### Example `.env` ```sh diff --git a/TODO.md b/TODO.md index 23a1f8f..9e95dfb 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ Rationale, Design, TODO, License, Author) if any are still missing. # 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: wildcard CORS now applies only to the public routes, not to `/metrics`, and allows only the methods they serve (closes #100). - 2026-10-01: `internal/state` and `internal/watcher` no longer export test-only 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..020c167 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,38 +114,32 @@ 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 checks that an unset 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) -func TestNew_InvalidTLSInterval_FallsBackToDefault(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") - - 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) + _, 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_DebugEnablesDebugLogging(t *testing.T) {