From bbba4bfdcb10384eee6d968ee6ee4ec2c46ce71f 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 | 61 +++++---- 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, 266 insertions(+), 63 deletions(-) create mode 100644 internal/watcher/skiprecordnotifications_test.go diff --git a/README.md b/README.md index c1988ed..cfff881 100644 --- a/README.md +++ b/README.md @@ -99,10 +99,11 @@ notification endpoint set, changes show only on the dashboard; see records, stored per nameserver. Their changes are notified as a hostname's are, as a record change, NS query failure, NS recovery, inconsistency or CNAME address change, in a message that starts `Domain:` where a hostname's starts - `Hostname:`. A domain with no delegation of its own has these records asked at - the servers of the zone it is in, as a hostname has. A domain that does not - exist has none: they are not asked for, and those saved by an earlier check - are removed without a notification. + `Hostname:`. A domain listed in `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS` gets no + record change or inconsistency notification. A domain with no delegation of + its own has these records asked at the servers of the zone it is in, as a + hostname has. A domain that does not exist has none: they are not asked for, + and those saved by an earlier check are removed without a notification. ### DNS Hostname Monitoring (Subdomains) @@ -136,10 +137,14 @@ notification endpoint set, changes show only on the dashboard; see lower case, because names are case-insensitive and nameservers may answer in any letter case. TXT and CAA values keep their letter case; they are not lower-cased. -- Any observable change in any nameserver's response triggers a notification. - This includes: +- Any observable change in any nameserver's response triggers a notification, + except a record change or an inconsistency for a domain or hostname listed in + `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS`. 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 @@ -160,7 +165,8 @@ notification endpoint set, changes show only on the dashboard; see failed on it, and answers differently is reported on the check where it answers. So is a pair that differs in a record type whose query to either nameserver failed on the previous check. If a pair agrees again and later - disagrees, the alert is sent again. + disagrees, the alert is sent again. For a domain or hostname listed in + `DNSWATCHER_SKIP_RECORD_NOTIFICATIONS`, no inconsistency is notified. - **CNAME address change**: The addresses at the end of a name's CNAME chain differ from those of the previous check. They are found when its nameservers answer with a CNAME and no address; a name that answers with @@ -210,7 +216,9 @@ notification endpoint set, changes show only on the dashboard; see **Every observable state change produces a notification.** dnswatcher is designed as a real-time change feed — degradations, failures, recoveries, and -routine changes are all reported equally. +routine changes are all reported equally. A domain or hostname listed in +`DNSWATCHER_SKIP_RECORD_NOTIFICATIONS` gets no record change or inconsistency +notification. Supported notification backends: @@ -409,23 +417,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]