config: stop startup on an invalid DNS or TLS interval (closes #177)
check / check (push) Successful in 1m3s

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 or empty 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
This commit was merged in pull request #178.
This commit is contained in:
2026-10-01 22:19:04 +02:00
parent ed0f56f144
commit fcd4f7e2c2
4 changed files with 69 additions and 34 deletions
+10 -2
View File
@@ -320,8 +320,8 @@ the following precedence (highest to lowest):
| `DNSWATCHER_SLACK_WEBHOOK` | Slack incoming webhook URL | `""` | | `DNSWATCHER_SLACK_WEBHOOK` | Slack incoming webhook URL | `""` |
| `DNSWATCHER_MATTERMOST_WEBHOOK` | Mattermost incoming webhook URL | `""` | | `DNSWATCHER_MATTERMOST_WEBHOOK` | Mattermost incoming webhook URL | `""` |
| `DNSWATCHER_NTFY_TOPIC` | ntfy topic URL | `""` | | `DNSWATCHER_NTFY_TOPIC` | ntfy topic URL | `""` |
| `DNSWATCHER_DNS_INTERVAL` | DNS check interval | `1h` | | `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 | `12h` | | `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_TLS_EXPIRY_WARNING` | Days before expiry to warn | `7` |
| `DNSWATCHER_SENTRY_DSN` | Sentry DSN for error reporting | `""` | | `DNSWATCHER_SENTRY_DSN` | Sentry DSN for error reporting | `""` |
| `DNSWATCHER_MAINTENANCE_MODE` | Enable maintenance mode | `false` | | `DNSWATCHER_MAINTENANCE_MODE` | Enable maintenance mode | `false` |
@@ -344,6 +344,14 @@ the client address is taken from the `X-Real-IP` or `X-Forwarded-For` header
the proxy sets; a proxy that sets neither makes all its clients share one the proxy sets; a proxy that sets neither makes all its clients share one
allowance. allowance.
**`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` ### Example `.env`
```sh ```sh
+2 -2
View File
@@ -20,6 +20,8 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
# Completed Steps # Completed Steps
- 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is
not a positive duration stops startup; empty means the default (closes #177).
- 2026-10-01: `/metrics` allows each client address 30 requests a minute, - 2026-10-01: `/metrics` allows each client address 30 requests a minute,
counted before Basic Auth, and answers 429 beyond that (closes #101). counted before Basic Auth, and answers 429 beyond that (closes #101).
- 2026-10-01: the image built by `make docker` reports the `git describe` - 2026-10-01: the image built by `make docker` reports the `git describe`
@@ -97,8 +99,6 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
- nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105 - nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105
- `DNSWATCHER_SENTRY_DSN` does nothing: - `DNSWATCHER_SENTRY_DSN` does nothing:
https://git.eeqj.de/sneak/dnswatcher/issues/107 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
- trial run of the finished image: - trial run of the finished image:
https://git.eeqj.de/sneak/dnswatcher/issues/149 https://git.eeqj.de/sneak/dnswatcher/issues/149
- 1.0 readiness: run it with a real config and read the logs: - 1.0 readiness: run it with a real config and read the logs:
+27 -8
View File
@@ -28,6 +28,13 @@ var ErrNoTargets = errors.New(
"no monitoring targets configured: set DNSWATCHER_TARGETS environment variable", "no monitoring targets configured: set DNSWATCHER_TARGETS environment variable",
) )
// ErrInvalidInterval is returned when DNSWATCHER_DNS_INTERVAL or
// DNSWATCHER_TLS_INTERVAL is set but is not a positive duration. An empty
// value counts as unset and means the default.
var ErrInvalidInterval = errors.New(
"interval must be a positive duration such as 30m or 1h",
)
// Params contains dependencies for Config. // Params contains dependencies for Config.
type Params struct { type Params struct {
fx.In fx.In
@@ -125,18 +132,14 @@ func buildConfig(
} }
} }
dnsInterval, err := time.ParseDuration( dnsInterval, err := parseInterval("DNS_INTERVAL")
viper.GetString("DNS_INTERVAL"),
)
if err != nil { if err != nil {
dnsInterval = defaultDNSInterval return nil, err
} }
tlsInterval, err := time.ParseDuration( tlsInterval, err := parseInterval("TLS_INTERVAL")
viper.GetString("TLS_INTERVAL"),
)
if err != nil { if err != nil {
tlsInterval = defaultTLSInterval return nil, err
} }
domains, hostnames, err := parseAndValidateTargets() domains, hostnames, err := parseAndValidateTargets()
@@ -168,6 +171,22 @@ func buildConfig(
return cfg, nil 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) { func parseAndValidateTargets() ([]string, []string, error) {
domains, hostnames, err := ClassifyTargets( domains, hostnames, err := ClassifyTargets(
parseCSV(viper.GetString("TARGETS")), parseCSV(viper.GetString("TARGETS")),
+28 -20
View File
@@ -1,6 +1,7 @@
package config_test package config_test
import ( import (
"strconv"
"testing" "testing"
"time" "time"
@@ -113,33 +114,40 @@ func TestNew_OnlyEmptyCSVSegments(t *testing.T) {
assert.ErrorIs(t, err, config.ErrNoTargets) assert.ErrorIs(t, err, config.ErrNoTargets)
} }
func TestNew_InvalidDNSInterval_FallsBackToDefault(t *testing.T) { // 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
}
for _, variable := range variables {
for _, value := range values {
t.Run(variable+"="+value, func(t *testing.T) {
viper.Reset() viper.Reset()
t.Setenv("DNSWATCHER_TARGETS", "example.com") t.Setenv("DNSWATCHER_TARGETS", "example.com")
t.Setenv("DNSWATCHER_DNS_INTERVAL", "banana") t.Setenv(variable, value)
cfg, err := config.New(nil, newTestParams(t)) _, err := config.New(nil, newTestParams(t))
require.NoError(t, err) require.ErrorIs(t, err, config.ErrInvalidInterval)
assert.Equal(t, time.Hour, cfg.DNSInterval, require.ErrorContains(t, err, variable)
"invalid DNS interval should fall back to 1h default") require.ErrorContains(t, err, strconv.Quote(value))
})
}
}
} }
func TestNew_InvalidTLSInterval_FallsBackToDefault(t *testing.T) { func TestNew_EmptyIntervalMeansDefault(t *testing.T) {
viper.Reset() viper.Reset()
t.Setenv("DNSWATCHER_TARGETS", "example.com") t.Setenv("DNSWATCHER_TARGETS", "example.com")
t.Setenv("DNSWATCHER_TLS_INTERVAL", "notaduration") t.Setenv("DNSWATCHER_DNS_INTERVAL", "")
t.Setenv("DNSWATCHER_TLS_INTERVAL", "")
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)) cfg, err := config.New(nil, newTestParams(t))
require.NoError(t, err) require.NoError(t, err)