From 7755d85ab9750d292384c6ab54219a05f75af6fc Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 5 Oct 2026 23:29:44 +0000 Subject: [PATCH] watcher, config: no Record Change or Inconsistency for listed names (closes #255) DNSWATCHER_SKIP_RECORD_NOTIFICATIONS takes a comma-separated list of names from DNSWATCHER_TARGETS, read as the targets are (letter case, trailing dot, repeats). For a listed name no Record Change and no Inconsistency notification is sent; its records are still checked and saved, and its other notifications are sent. A listed name that is not a target stops startup with an error naming it. The two detections return early for a listed name. Apex domains are covered too: their own records go through the same detection. Live DNS cannot be made to disagree on purpose, so the Inconsistency test feeds records to the change detection directly, as the existing inconsistency tests do; the live test covers Record Change. Model: opus-5-5 --- README.md | 40 +++--- TODO.md | 3 + internal/config/config.go | 117 ++++++++++++------ internal/config/config_test.go | 28 +++++ .../watcher/skiprecordnotifications_test.go | 107 ++++++++++++++++ internal/watcher/watcher.go | 13 +- 6 files changed, 253 insertions(+), 55 deletions(-) create mode 100644 internal/watcher/skiprecordnotifications_test.go diff --git a/README.md b/README.md index c1988ed..50b2e7c 100644 --- a/README.md +++ b/README.md @@ -139,7 +139,10 @@ notification endpoint set, changes show only on the dashboard; see - Any observable change in any nameserver's response triggers a notification. This includes: - **Record change**: A nameserver returns different records than it did on - the previous check (additions, removals, value changes). + the previous check (additions, removals, value changes). For a domain or + hostname listed in `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS`, neither a + record change nor an inconsistency is notified; its records are still + checked and saved, and its other notifications are sent. - **NS query failure**: A nameserver that previously responded becomes unreachable (timeout, SERVFAIL, REFUSED, network error). This is distinct from "responded with no records": a nameserver that answers NXDOMAIN or @@ -409,23 +412,24 @@ following precedence (highest to lowest): ### Environment Variables -| Variable | Description | Default | -| ----------------------------------- | ----------------------------------------------------------------------------------------------------------- | --------------------- | -| `PORT` | HTTP listen port | `8080` | -| `DNSWATCHER_DEBUG` | Enable debug logging | `false` | -| `DNSWATCHER_DATA_DIR` | Directory for state file | `/var/lib/dnswatcher` | -| `DNSWATCHER_TARGETS` | Comma-separated DNS names (auto-classified via PSL) | `""` | -| `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, 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` | Only sets `maintenanceMode` in the health check response; changes nothing else | `false` | -| `DNSWATCHER_METRICS_USERNAME` | Basic auth username for /metrics, which is served only when this is set | `""` | -| `DNSWATCHER_METRICS_PASSWORD` | Basic auth password for /metrics | `""` | -| `DNSWATCHER_SEND_TEST_NOTIFICATION` | Send a test notification after first scan completes | `false` | +| Variable | Description | Default | +| -------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | --------------------- | +| `PORT` | HTTP listen port | `8080` | +| `DNSWATCHER_DEBUG` | Enable debug logging | `false` | +| `DNSWATCHER_DATA_DIR` | Directory for state file | `/var/lib/dnswatcher` | +| `DNSWATCHER_TARGETS` | Comma-separated DNS names (auto-classified via PSL) | `""` | +| `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS` | Comma-separated names from `DNSWATCHER_TARGETS` for which no record change or inconsistency is notified; any other name stops startup | `""` | +| `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, 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` | Only sets `maintenanceMode` in the health check response; changes nothing else | `false` | +| `DNSWATCHER_METRICS_USERNAME` | Basic auth username for /metrics, which is served only when this is set | `""` | +| `DNSWATCHER_METRICS_PASSWORD` | Basic auth password for /metrics | `""` | +| `DNSWATCHER_SEND_TEST_NOTIFICATION` | Send a test notification after first scan completes | `false` | **`DNSWATCHER_TARGETS` is required.** dnswatcher will refuse to start if no monitoring targets are configured. A monitoring daemon with nothing to monitor diff --git a/TODO.md b/TODO.md index 0a5c99f..878dd2e 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,9 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-05: a name listed in `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS` gets no + Record Change or Inconsistency notification; a name listed there that is not + in `DNSWATCHER_TARGETS` stops startup (closes #255). - 2026-10-02: a nameserver whose query for one record type failed while the others answered with no records is `ok`, not `nodata` (closes #253). - 2026-10-02: a domain that does not exist is shown so, with no nameservers; no diff --git a/internal/config/config.go b/internal/config/config.go index 8d58433..2f82f40 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log/slog" + "slices" "strings" "time" @@ -35,6 +36,10 @@ var ErrInvalidInterval = errors.New( "interval must be a positive duration such as 30m or 1h", ) +// ErrNotInTargets is returned when DNSWATCHER_SKIP_RECORD_NOTIFICATIONS +// lists a name that is not in DNSWATCHER_TARGETS. +var ErrNotInTargets = errors.New("name is not in DNSWATCHER_TARGETS") + // Params contains dependencies for Config. type Params struct { fx.In @@ -45,24 +50,25 @@ type Params struct { // Config holds application configuration. type Config struct { - Port int - Debug bool - DataDir string - Domains []string - Hostnames []string - SlackWebhook string - MattermostWebhook string - NtfyTopic string - DNSInterval time.Duration - TLSInterval time.Duration - TLSExpiryWarning int - SentryDSN string - MaintenanceMode bool - MetricsUsername string - MetricsPassword string - SendTestNotification bool - params *Params - log *slog.Logger + Port int + Debug bool + DataDir string + Domains []string + Hostnames []string + SkipRecordNotifications []string + SlackWebhook string + MattermostWebhook string + NtfyTopic string + DNSInterval time.Duration + TLSInterval time.Duration + TLSExpiryWarning int + SentryDSN string + MaintenanceMode bool + MetricsUsername string + MetricsPassword string + SendTestNotification bool + params *Params + log *slog.Logger } // New creates a new Config instance from environment and config files. @@ -103,6 +109,7 @@ func setupViper(name string) { viper.SetDefault("DEBUG", false) viper.SetDefault("DATA_DIR", "/var/lib/"+name) viper.SetDefault("TARGETS", "") + viper.SetDefault("SKIP_RECORD_NOTIFICATIONS", "") viper.SetDefault("SLACK_WEBHOOK", "") viper.SetDefault("MATTERMOST_WEBHOOK", "") viper.SetDefault("NTFY_TOPIC", "") @@ -147,25 +154,33 @@ func buildConfig( return nil, err } + skipRecordNotifications, err := parseSkipRecordNotifications( + domains, hostnames, + ) + if err != nil { + return nil, err + } + cfg := &Config{ - Port: viper.GetInt("PORT"), - Debug: viper.GetBool("DEBUG"), - DataDir: viper.GetString("DATA_DIR"), - Domains: domains, - Hostnames: hostnames, - SlackWebhook: viper.GetString("SLACK_WEBHOOK"), - MattermostWebhook: viper.GetString("MATTERMOST_WEBHOOK"), - NtfyTopic: viper.GetString("NTFY_TOPIC"), - DNSInterval: dnsInterval, - TLSInterval: tlsInterval, - TLSExpiryWarning: viper.GetInt("TLS_EXPIRY_WARNING"), - SentryDSN: viper.GetString("SENTRY_DSN"), - MaintenanceMode: viper.GetBool("MAINTENANCE_MODE"), - MetricsUsername: viper.GetString("METRICS_USERNAME"), - MetricsPassword: viper.GetString("METRICS_PASSWORD"), - SendTestNotification: viper.GetBool("SEND_TEST_NOTIFICATION"), - params: params, - log: log, + Port: viper.GetInt("PORT"), + Debug: viper.GetBool("DEBUG"), + DataDir: viper.GetString("DATA_DIR"), + Domains: domains, + Hostnames: hostnames, + SkipRecordNotifications: skipRecordNotifications, + SlackWebhook: viper.GetString("SLACK_WEBHOOK"), + MattermostWebhook: viper.GetString("MATTERMOST_WEBHOOK"), + NtfyTopic: viper.GetString("NTFY_TOPIC"), + DNSInterval: dnsInterval, + TLSInterval: tlsInterval, + TLSExpiryWarning: viper.GetInt("TLS_EXPIRY_WARNING"), + SentryDSN: viper.GetString("SENTRY_DSN"), + MaintenanceMode: viper.GetBool("MAINTENANCE_MODE"), + MetricsUsername: viper.GetString("METRICS_USERNAME"), + MetricsPassword: viper.GetString("METRICS_PASSWORD"), + SendTestNotification: viper.GetBool("SEND_TEST_NOTIFICATION"), + params: params, + log: log, } return cfg, nil @@ -204,6 +219,36 @@ func parseAndValidateTargets() ([]string, []string, error) { return domains, hostnames, nil } +// parseSkipRecordNotifications reads DNSWATCHER_SKIP_RECORD_NOTIFICATIONS, +// a comma-separated list of names from the targets. Each name is written +// as ClassifyTargets writes a target, in lower case without a trailing +// dot, and a name listed more than once is kept once. A name that is +// not one of domains or hostnames is an error naming it. +func parseSkipRecordNotifications( + domains, hostnames []string, +) ([]string, error) { + value := viper.GetString("SKIP_RECORD_NOTIFICATIONS") + + var names []string + + for _, listed := range parseCSV(value) { + name := strings.ToLower(strings.TrimSuffix(listed, ".")) + + if !slices.Contains(domains, name) && !slices.Contains(hostnames, name) { + return nil, fmt.Errorf( + "invalid DNSWATCHER_SKIP_RECORD_NOTIFICATIONS %q: %w", + listed, ErrNotInTargets, + ) + } + + if !slices.Contains(names, name) { + names = append(names, name) + } + } + + return names, nil +} + func parseCSV(input string) []string { if input == "" { return nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c4471ff..ffb93ab 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -57,6 +57,7 @@ func TestNew_DefaultValues(t *testing.T) { assert.Empty(t, cfg.MetricsUsername) assert.Empty(t, cfg.MetricsPassword) assert.False(t, cfg.SendTestNotification) + assert.Empty(t, cfg.SkipRecordNotifications) } func TestNew_EnvironmentOverrides(t *testing.T) { @@ -235,6 +236,33 @@ func TestNew_TargetsWithTrailingComma(t *testing.T) { "trailing comma should be ignored") } +func TestNew_SkipRecordNotifications(t *testing.T) { + viper.Reset() + t.Setenv("DNSWATCHER_TARGETS", "example.net,www.example.net,example.org") + t.Setenv("DNSWATCHER_SKIP_RECORD_NOTIFICATIONS", + " WWW.Example.net. , example.net,www.example.net") + + cfg, err := config.New(nil, newTestParams(t)) + require.NoError(t, err) + assert.Equal(t, + []string{"www.example.net", "example.net"}, + cfg.SkipRecordNotifications, + "names are written as targets are, each once", + ) +} + +func TestNew_SkipRecordNotificationsNotInTargetsStopsStartup(t *testing.T) { + viper.Reset() + t.Setenv("DNSWATCHER_TARGETS", "example.net") + t.Setenv("DNSWATCHER_SKIP_RECORD_NOTIFICATIONS", + "example.net,www.example.net") + + _, err := config.New(nil, newTestParams(t)) + require.ErrorIs(t, err, config.ErrNotInTargets) + require.ErrorContains(t, err, "DNSWATCHER_SKIP_RECORD_NOTIFICATIONS") + require.ErrorContains(t, err, `"www.example.net"`) +} + func TestNew_CustomDNSIntervalDuration(t *testing.T) { viper.Reset() t.Setenv("DNSWATCHER_TARGETS", "example.com") diff --git a/internal/watcher/skiprecordnotifications_test.go b/internal/watcher/skiprecordnotifications_test.go new file mode 100644 index 0000000..b28afb5 --- /dev/null +++ b/internal/watcher/skiprecordnotifications_test.go @@ -0,0 +1,107 @@ +package watcher_test + +import ( + "slices" + "testing" + + "sneak.berlin/go/dnswatcher/internal/config" + "sneak.berlin/go/dnswatcher/internal/state" + "sneak.berlin/go/dnswatcher/internal/watcher" +) + +// TestSkipRecordNotificationsAlerts runs the hostname change detection +// for two names on the same check: nsA's address changes, so that nsA +// now differs from nsB, and nsC stops answering. The name in +// SkipRecordNotifications gets only the NS Failure; the other name also +// gets the Record Change and the Inconsistency. +func TestSkipRecordNotificationsAlerts(t *testing.T) { + t.Parallel() + + const skipped = "skipped.example.net" + + prev := saved(map[string]*state.NameserverRecordState{ + nsA: answered(map[string][]string{"A": {ip1}}), + nsB: answered(map[string][]string{"A": {ip1}}), + nsC: answered(map[string][]string{"A": {ip1}}), + }) + current := saved(map[string]*state.NameserverRecordState{ + nsA: answered(map[string][]string{"A": {ip2}}), + nsB: answered(map[string][]string{"A": {ip1}}), + nsC: failed(), + }) + + cfg := &config.Config{ + Hostnames: []string{host, skipped}, + SkipRecordNotifications: []string{skipped}, + } + + // The hostname change detection uses only the configuration and the + // notifier. + notifier := &mockNotifier{} + w := watcher.NewForTest(cfg, nil, nil, nil, nil, notifier) + + w.DetectHostnameChanges(t.Context(), host, prev, current) + w.DetectHostnameChanges(t.Context(), skipped, prev, current) + + sent := notifier.getNotifications() + titles := make([]string, 0, len(sent)) + + for _, n := range sent { + titles = append(titles, n.Title) + } + + slices.Sort(titles) + + want := []string{ + "Inconsistency: " + host, + "NS Failure: " + skipped, + "NS Failure: " + host, + "Record Change: " + host, + } + + if !slices.Equal(titles, want) { + t.Errorf("sent %v, want %v", titles, want) + } +} + +// TestSkipRecordNotificationsCheck checks testHost, which is in +// SkipRecordNotifications, from a saved state in which every nameserver +// live DNS lists answered with an address live DNS never returns. No +// Record Change and no Inconsistency is sent, and the check still saves +// what the nameservers answer. +func TestSkipRecordNotificationsCheck(t *testing.T) { + t.Parallel() + + cfg := defaultTestConfig(t) + cfg.Hostnames = []string{testHost} + cfg.SkipRecordNotifications = []string{testHost} + + nameservers := lookupNameservers(t, testHost) + + _, deps := runChecks(t, cfg, func(deps *testDeps) { + byNameserver := make(map[string]*state.NameserverRecordState) + for _, ns := range nameservers { + byNameserver[ns] = answered(map[string][]string{"A": {oldIP}}) + } + + deps.state.SetHostnameState(testHost, saved(byNameserver)) + }) + + for _, title := range []string{ + "Record Change: " + testHost, + "Inconsistency: " + testHost, + } { + if n := countNotifications(deps, title); n != 0 { + t.Errorf("sent %d %q, want 0", n, title) + } + } + + // A nameserver whose query for A failed keeps oldIP, so the check + // is that some address live DNS gave was saved. + hs, _ := deps.state.GetHostnameState(testHost) + fromLiveDNS := func(ip string) bool { return ip != oldIP } + + if !slices.ContainsFunc(addresses(hs), fromLiveDNS) { + t.Errorf("saved addresses %v, want those live DNS gave", addresses(hs)) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 7368dab..8c5b17b 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -730,12 +730,16 @@ func (w *Watcher) detectCNAMEAddressChanges( // failed on either check has no records to compare. The records kept // for a record type whose query failed are compared too, but not those // of a type in UnknownTypes on either check, which the message leaves -// out as well. +// out as well. Nothing is sent for a name in SkipRecordNotifications. func (w *Watcher) detectRecordChanges( ctx context.Context, hostname string, prev, current *state.HostnameState, ) { + if slices.Contains(w.config.SkipRecordNotifications, hostname) { + return + } + for ns, cur := range current.RecordsByNameserver { prevNS, ok := prev.RecordsByNameserver[ns] if !ok || prevNS.Status != statusOK || cur.Status != statusOK { @@ -835,11 +839,18 @@ func (w *Watcher) detectNSFailures( } } +// detectInconsistencies notifies each pair of nameservers that newly +// disagree (see newlyDisagreeingPairs). Nothing is sent for a name in +// SkipRecordNotifications. func (w *Watcher) detectInconsistencies( ctx context.Context, hostname string, prev, current *state.HostnameState, ) { + if slices.Contains(w.config.SkipRecordNotifications, hostname) { + return + } + for _, pair := range newlyDisagreeingPairs(prev, current) { ns1, ns2 := pair[0], pair[1] state1 := current.RecordsByNameserver[ns1]