diff --git a/TODO.md b/TODO.md index 596779e..f37f769 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-01: a certificate within the expiry warning period is warned about on + every TLS check, where some checks used to skip it at random (closes #204). - 2026-10-01: README has Getting Started, Rationale and TODO sections, and its Architecture section is now Design, in the order policy sets (closes #173). - 2026-10-01: a zone's server that answers SERVFAIL or a referral leading no diff --git a/internal/watcher/export_test.go b/internal/watcher/export_test.go index 557e787..59f0319 100644 --- a/internal/watcher/export_test.go +++ b/internal/watcher/export_test.go @@ -20,15 +20,14 @@ func NewForTest( n Notifier, ) *Watcher { return &Watcher{ - log: slog.Default(), - config: cfg, - state: st, - resolver: res, - portCheck: pc, - tlsCheck: tc, - notify: n, - firstRun: true, - expiryNotified: make(map[string]time.Time), + log: slog.Default(), + config: cfg, + state: st, + resolver: res, + portCheck: pc, + tlsCheck: tc, + notify: n, + firstRun: true, } } @@ -72,6 +71,11 @@ func (w *Watcher) CheckAllPorts(ctx context.Context) { w.checkAllPorts(ctx) } +// RunTLSChecks exports runTLSChecks for testing. +func (w *Watcher) RunTLSChecks(ctx context.Context) { + w.runTLSChecks(ctx) +} + // BuildHostnameState exports buildHostnameState for testing. func BuildHostnameState( results map[string]*resolver.NameserverResponse, diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 2df33f4..bcee203 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -7,7 +7,6 @@ import ( "slices" "sort" "strings" - "sync" "time" "go.uber.org/fx" @@ -49,18 +48,16 @@ type Params struct { // Watcher orchestrates all monitoring checks on a schedule. type Watcher struct { - log *slog.Logger - config *config.Config - state *state.State - resolver DNSResolver - portCheck PortChecker - tlsCheck TLSChecker - notify Notifier - cancel context.CancelFunc - done chan struct{} // closed when Run returns - firstRun bool - expiryNotifiedMu sync.Mutex - expiryNotified map[string]time.Time + log *slog.Logger + config *config.Config + state *state.State + resolver DNSResolver + portCheck PortChecker + tlsCheck TLSChecker + notify Notifier + cancel context.CancelFunc + done chan struct{} // closed when Run returns + firstRun bool } // New creates a new Watcher instance wired into the fx lifecycle. @@ -69,15 +66,14 @@ func New( params Params, ) (*Watcher, error) { w := &Watcher{ - log: params.Logger.Get(), - config: params.Config, - state: params.State, - resolver: params.Resolver, - portCheck: params.PortCheck, - tlsCheck: params.TLSCheck, - notify: params.Notify, - firstRun: true, - expiryNotified: make(map[string]time.Time), + log: params.Logger.Get(), + config: params.Config, + state: params.State, + resolver: params.Resolver, + portCheck: params.PortCheck, + tlsCheck: params.TLSCheck, + notify: params.Notify, + firstRun: true, } lifecycle.Append(fx.Hook{ @@ -1028,22 +1024,6 @@ func (w *Watcher) checkTLSExpiry( return } - // Deduplicate expiry warnings: don't re-notify for the same - // hostname within the TLS check interval. - dedupKey := fmt.Sprintf("expiry:%s:%s", hostname, ip) - - w.expiryNotifiedMu.Lock() - - lastNotified, seen := w.expiryNotified[dedupKey] - if seen && time.Since(lastNotified) < w.config.TLSInterval { - w.expiryNotifiedMu.Unlock() - - return - } - - w.expiryNotified[dedupKey] = time.Now() - w.expiryNotifiedMu.Unlock() - msg := fmt.Sprintf( "Host: %s\nIP: %s\nCN: %s\n"+ "Expires: %s (%.0f days)", diff --git a/internal/watcher/watcher_test.go b/internal/watcher/watcher_test.go index c166a3e..9918fc8 100644 --- a/internal/watcher/watcher_test.go +++ b/internal/watcher/watcher_test.go @@ -615,33 +615,54 @@ func TestTLSExpiryWarning(t *testing.T) { assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning") } -func TestTLSExpiryWarningDedup(t *testing.T) { +// TestTLSExpiryWarningEachCheck runs the TLS checks three times in a +// row on hostname and port state built here, for a certificate that +// expires within the warning period. Each check warns once, whether the +// TLS interval is a nanosecond, shorter than the time between two +// checks, or a day, longer than it. +func TestTLSExpiryWarningEachCheck(t *testing.T) { t.Parallel() - cfg := defaultTestConfig(t) - cfg.Hostnames = []string{testHost} - cfg.TLSInterval = 24 * time.Hour + title := "TLS Expiry Warning: " + host - title := "TLS Expiry Warning: " + testHost + for _, interval := range []time.Duration{time.Nanosecond, 24 * time.Hour} { + t.Run(interval.String(), func(t *testing.T) { + t.Parallel() - // The second check comes within the TLS interval of the first, - // so it must not warn again. - var warnings int + cfg := defaultTestConfig(t) + cfg.Hostnames = []string{host} + cfg.TLSInterval = interval - deps := runChecks(t, cfg, expiresInThreeDays, func(deps *testDeps) { - warnings = countNotifications(deps, title) - }) + // The TLS checks read the saved hostname and port state and + // look nothing up, so the watcher has no resolver. + deps := newTestDeps(t, cfg) + w := watcher.NewForTest( + cfg, deps.state, nil, + deps.portChecker, deps.tlsChecker, deps.notifier, + ) - if warnings == 0 { - t.Fatal("expected expiry warnings from the first check") - } + expiresInThreeDays(deps) + deps.state.SetHostnameState(host, saved( + map[string]*state.NameserverRecordState{ + nsA: answered(map[string][]string{"A": {ip1}}), + }, + )) + deps.state.SetPortState(ip1+":443", &state.PortState{ + Open: true, Hostnames: []string{host}, + }) - got := countNotifications(deps, title) - if got != warnings { - t.Errorf( - "expected %d expiry warnings (dedup), got %d", - warnings, got, - ) + for check := 1; check <= 3; check++ { + w.RunTLSChecks(t.Context()) + + got := countNotifications(deps, title) + if got != check { + t.Fatalf( + "after check %d: %d expiry warnings, want %d", + check, got, check, + ) + } + } + }) } }