watcher: a removed nameserver is an NS Change, not an NS Failure (closes #262)

A nameserver that left a domain's delegation was reported twice: as the
domain's NS Change, and by the check of the domain's own records as an
error-level NS Failure saying it disappeared. The check of a name's
records now sends one NS Change at warning level, starting Hostname: or
Domain:, listing the nameservers it was asked at before and not now.
For a domain it leaves out those of its previous delegation, which the
domain's own NS Change lists, so a domain's records are now checked
before its new delegation is saved. Nothing is sent for a nameserver
that joins. The README says what each case gets.

A nameserver that was failing before it left is now listed too; the
NS Failure skipped it.

Model: opus-5-5

README.md

@@ -78,9 +78,8 @@ notification endpoint set, changes show only on the dashboard; see
   answer NXDOMAIN, that it does not exist, has no nameservers and is shown as
   not existing (see Web Dashboard and HTTP API). A domain that exists but has no
   delegation of its own, such as `octocat.github.io`, has no nameservers either.
-  When the parent zone's servers do not answer, or the lookup of the domain's
-  own records (below) fails, the check fails and the set from the previous check
-  is kept.
+  When the parent zone's servers do not answer, the check fails and the set from
+  the previous check is kept.
 - Any change triggers a notification:
     - NS added to or removed from that set. A domain that had nameservers on the
       previous check and no longer exists gets one with all of them removed.
@@ -223,14 +222,8 @@ notification endpoint set, changes show only on the dashboard; see
     - Certificate CN, issuer, or SANs changed (replacement detected, reports old
       and new CN and issuer).
     - TLS connection failure to a previously-reachable IP:443 (handshake error,
-      timeout, or connection refused when port 443 closed after the port check
-      that found it open).
+      timeout, connection refused after previously succeeding).
     - TLS recovery: a previously-failing IP:443 now completes a handshake again.
-- A port check that finds port 443 closed removes the certificate entries for
-  that IP address, without a notification of its own: the port change
-  notification reports the closed port. When the port opens again, the next TLS
-  check saves the certificate as new, without a certificate change or recovery
-  notification.
 
 ### Notifications
 
@@ -300,10 +293,8 @@ clears them.
   port entry left with no name goes, so the dashboard and `/api/v1/status` no
   longer list or count it. The first check's port checks remove the port entries
   of addresses no configured name has.
-- Each port check also removes, without a notification, the certificate entries
-  for an address whose port 443 it found closed, and those for an address their
-  name no longer resolves to, except while none of the name's nameservers
-  answer.
+- Each port check also removes the certificate entries for an address their name
+  no longer resolves to, except while none of the name's nameservers answer.
 
 ### Web Dashboard
 
@@ -443,7 +434,7 @@ following precedence (highest to lowest):
 
 | Variable                               | Description                                                                                                                           | Default               |
 | -------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | --------------------- |
-| `PORT`                                 | HTTP listen port; a port that cannot be bound, such as one already in use, stops startup                                              | `8080`                |
+| `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)                                                                                   | `""`                  |
@@ -457,7 +448,7 @@ following precedence (highest to lowest):
 | `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; required when `DNSWATCHER_METRICS_USERNAME` is set, and startup stops without it                    | `""`                  |
+| `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
@@ -666,20 +657,10 @@ for A, AAAA and CNAME, the previous check's list is kept, or `null` when no
 earlier check saved one. A state file without it loads, and the first check
 after that saves it without a notification.
 
-A port entry's key is its address and port, and a certificate entry's key its
-address, port 443 and hostname, joined by colons. An IPv6 address is written in
-brackets in these keys, as in `[2001:db8::1]:443` and
-`[2001:db8::1]:443:www.example.com`. `/api/v1/status` and the dashboard show the
-same keys.
-
 A port entry's `hostnames` lists every name that resolves to its address,
 domains included. A port entry in the older format, with one `hostname` instead
 of the `hostnames` list, loads as a list of that one name.
 
-A state file that leaves out any of `domains`, `hostnames`, `ports` and
-`certificates`, or holds `null` for one, loads with that section empty. A file
-holding `{}` loads as empty state.
-
 ---
 
 ## Entrypoints

TODO.md

@@ -10,33 +10,17 @@
 
 # Status
 
-pre-1.0. Work lands on `next` by PR. Open work for 1.0 is tracked on the 1.0
-milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
+pre-1.0. No git tags. Work lands on `next` by PR. Open work for 1.0 is tracked
+on the 1.0 milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
 
 # Next Step
 
-1.0 tracking issue: https://git.eeqj.de/sneak/dnswatcher/issues/66
+trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
 
 # Completed Steps
 
-- 2026-10-13: an IPv6 address is written in brackets before its port, as in
-  `[2001:db8::1]:443`, in keys and Port Change notifications (closes #270).
-- 2026-10-13: `TODO.md` names only open issues as next and future steps, and its
-  Status says nothing about git tags (closes #281).
-- 2026-10-13: a port check that finds port 443 closed removes that address's
-  certificate entries, so none is left shown as `ok` (closes #265).
-- 2026-10-13: a state file holding `{}`, or leaving out a section, loads with
-  that section empty; the first check no longer crashes on it (closes #267).
-- 2026-10-13: a stop before the first scan completes sends no startup
-  notification; a scan that ran to the end still sends it (closes #271).
 - 2026-10-13: a nameserver leaving a domain's delegation sends only NS Change;
   one leaving a hostname's zone sends NS Change, not NS Failure (closes #262).
-- 2026-10-13: `DNSWATCHER_METRICS_USERNAME` set without
-  `DNSWATCHER_METRICS_PASSWORD` stops startup (closes #268).
-- 2026-10-13: a save syncs the state file to disk before renaming it into place,
-  so a crash cannot leave it empty and stop the next start (closes #264).
-- 2026-10-13: a `PORT` that cannot be bound stops startup with an error naming
-  the address, instead of running without the HTTP server (closes #266).
 - 2026-10-13: a bad `PORT`, `DNSWATCHER_TLS_EXPIRY_WARNING` or true/false
   setting stops startup instead of being read as 0 or false (closes #269).
 - 2026-10-13: a check saving the state while the dashboard or `/api/v1/status`
@@ -205,4 +189,6 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
 
 # Future Steps
 
-- DNSSEC validation, after 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/59
+- 1.0 readiness: run it with a real config and read the logs:
+  https://git.eeqj.de/sneak/dnswatcher/issues/66
+- review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144

internal/config/config.go

@@ -61,13 +61,6 @@ var ErrInvalidTLSExpiryWarning = errors.New(
 // unset and means the default.
 var ErrInvalidBool = errors.New("value must be true or false")
 
-// ErrNoMetricsPassword is returned when DNSWATCHER_METRICS_USERNAME is set and
-// DNSWATCHER_METRICS_PASSWORD is empty. Basic Auth on /metrics would otherwise
-// let in anyone who sends the username with an empty password.
-var ErrNoMetricsPassword = errors.New(
-	"DNSWATCHER_METRICS_PASSWORD must be set when DNSWATCHER_METRICS_USERNAME is set",
-)
-
 // Params contains dependencies for Config.
 type Params struct {
 	fx.In
@@ -115,10 +108,6 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
 		return nil, err
 	}
 
-	if cfg.MetricsUsername != "" && cfg.MetricsPassword == "" {
-		return nil, ErrNoMetricsPassword
-	}
-
 	configureDebugLogging(cfg, params)
 
 	return cfg, nil

internal/config/config_test.go

@@ -354,24 +354,6 @@ func TestNew_SkipRecordNotificationsNotInTargetsStopsStartup(t *testing.T) {
 	require.ErrorContains(t, err, `"www.example.net"`)
 }
 
-// TestNew_MetricsUsernameWithoutPasswordStopsStartup checks an unset and an
-// empty password; TestNew_EnvironmentOverrides checks a username with one.
-func TestNew_MetricsUsernameWithoutPasswordStopsStartup(t *testing.T) {
-	viper.Reset()
-	t.Setenv("DNSWATCHER_TARGETS", "example.com")
-	t.Setenv("DNSWATCHER_METRICS_USERNAME", "prom")
-
-	_, err := config.New(nil, newTestParams(t))
-	require.ErrorIs(t, err, config.ErrNoMetricsPassword, "password unset")
-	require.ErrorContains(t, err, "DNSWATCHER_METRICS_PASSWORD")
-
-	viper.Reset()
-	t.Setenv("DNSWATCHER_METRICS_PASSWORD", "")
-
-	_, err = config.New(nil, newTestParams(t))
-	require.ErrorIs(t, err, config.ErrNoMetricsPassword, "password empty")
-}
-
 func TestNew_CustomDNSIntervalDuration(t *testing.T) {
 	viper.Reset()
 	t.Setenv("DNSWATCHER_TARGETS", "example.com")

internal/server/export_test.go

@@ -13,8 +13,8 @@ import (
 const RequestTimeout time.Duration = requestTimeout
 
 // SetListenPort overrides the port Run binds. A test uses it to hand
-// Run an unbindable port, so that Run stores its http.Server and then
-// returns an error without listening.
+// Run an unbindable port so ListenAndServe fails immediately and Run
+// returns after storing its http.Server.
 func SetListenPort(s *Server, port int) {
 	s.port = port
 }

internal/server/server.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
-	"net"
 	"net/http"
 	"time"
 
@@ -113,7 +112,7 @@ func New(
 	}
 
 	lifecycle.Append(fx.Hook{
-		OnStart: func(ctx context.Context) error {
+		OnStart: func(_ context.Context) error {
 			srv.startupTime = time.Now()
 
 			err := srv.enableSentry()
@@ -121,7 +120,9 @@ func New(
 				return err
 			}
 
-			return srv.Run(ctx)
+			go srv.Run()
+
+			return nil
 		},
 		OnStop: func(ctx context.Context) error {
 			return srv.Shutdown(ctx)
@@ -148,10 +149,8 @@ func newHTTPServer(
 	}
 }
 
-// Run binds the listen port, then serves HTTP on it in a new goroutine
-// and returns. A port that cannot be bound is returned as an error, so
-// that startup stops instead of running without the HTTP server.
-func (s *Server) Run(ctx context.Context) error {
+// Run starts the HTTP server.
+func (s *Server) Run() {
 	s.SetupRoutes()
 
 	listenAddr := fmt.Sprintf(":%d", s.port)
@@ -159,21 +158,10 @@ func (s *Server) Run(ctx context.Context) error {
 
 	s.log.Info("http server starting", "addr", listenAddr)
 
-	lc := &net.ListenConfig{}
-
-	listener, err := lc.Listen(ctx, "tcp", listenAddr)
-	if err != nil {
-		return fmt.Errorf("starting http server: %w", err)
+	err := s.httpServer.ListenAndServe()
+	if err != nil && !errors.Is(err, http.ErrServerClosed) {
+		s.log.Error("http server error", "error", err)
 	}
-
-	go func() {
-		err := s.httpServer.Serve(listener)
-		if err != nil && !errors.Is(err, http.ErrServerClosed) {
-			s.log.Error("http server error", "error", err)
-		}
-	}()
-
-	return nil
 }
 
 // Shutdown gracefully shuts down the server, then sends the error

internal/server/server_test.go

@@ -1,8 +1,6 @@
 package server_test
 
 import (
-	"net"
-	"strings"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -60,13 +58,15 @@ func buildServer(t *testing.T) *server.Server {
 }
 
 // TestRunWiresSocketTimeouts pins that the http.Server the running
-// server actually serves — the one Run builds and serves on the port
-// it binds — carries every socket-level timeout, plus the two
+// server actually serves — the one Run builds and hands to
+// ListenAndServe — carries every socket-level timeout, plus the two
 // relationships the values must satisfy.
 //
-// Run is given an unbindable port: it builds and stores s.httpServer,
-// then the bind fails and Run returns the error without ever
-// listening. Nothing here measures elapsed time.
+// Run is driven to completion with an unbindable port: it builds and
+// stores s.httpServer, then ListenAndServe fails at once and Run
+// returns without ever listening. The assertions run in the same
+// goroutine after Run returns, so reading s.httpServer is free of any
+// data race. Nothing here measures elapsed time.
 //
 // ReadTimeout must be at least ReadHeaderTimeout. net/http reads the
 // headers under ReadHeaderTimeout, then sets the read deadline for the
@@ -84,10 +84,7 @@ func TestRunWiresSocketTimeouts(t *testing.T) {
 	srv := buildServer(t)
 	server.SetListenPort(srv, -1)
 
-	err := srv.Run(t.Context())
-	if err == nil {
-		t.Fatal("Run bound port -1")
-	}
+	srv.Run()
 
 	hs := server.HTTPServerOf(srv)
 	if hs == nil {
@@ -136,38 +133,3 @@ func TestRunWiresSocketTimeouts(t *testing.T) {
 		)
 	}
 }
-
-func TestPortInUseStopsStartup(t *testing.T) {
-	// Bound on every address, as Run binds, so that Run's bind fails.
-	lc := &net.ListenConfig{}
-
-	taken, err := lc.Listen(t.Context(), "tcp", ":0")
-	if err != nil {
-		t.Fatalf("binding a port for the test: %v", err)
-	}
-
-	defer func() { _ = taken.Close() }()
-
-	_, port, err := net.SplitHostPort(taken.Addr().String())
-	if err != nil {
-		t.Fatalf("reading the bound port: %v", err)
-	}
-
-	viper.Reset()
-	t.Setenv("DNSWATCHER_TARGETS", "example.com")
-	t.Setenv("DNSWATCHER_DATA_DIR", t.TempDir())
-	t.Setenv("PORT", port)
-
-	app := newServerApp(fx.Invoke(func(*server.Server) {}))
-
-	err = app.Start(t.Context())
-	if err == nil {
-		_ = app.Stop(t.Context())
-
-		t.Fatal("startup succeeded on a port that is already bound")
-	}
-
-	if !strings.Contains(err.Error(), ":"+port) {
-		t.Errorf("startup error does not name port %s: %v", port, err)
-	}
-}

internal/state/state.go

@@ -4,7 +4,6 @@ package state
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"log/slog"
 	"maps"
@@ -216,24 +215,6 @@ func (s *State) Load() error {
 		return fmt.Errorf("parsing state file: %w", err)
 	}
 
-	// A section the file leaves out, or holds as null, loads empty: the
-	// setters write into these maps, and writing into a nil map panics.
-	if snapshot.Domains == nil {
-		snapshot.Domains = make(map[string]*DomainState)
-	}
-
-	if snapshot.Hostnames == nil {
-		snapshot.Hostnames = make(map[string]*HostnameState)
-	}
-
-	if snapshot.Ports == nil {
-		snapshot.Ports = make(map[string]*PortState)
-	}
-
-	if snapshot.Certificates == nil {
-		snapshot.Certificates = make(map[string]*CertificateState)
-	}
-
 	// A state file saved before each record value was stored once can
 	// hold a hostname's CNAME once for every record type asked for.
 	// Each value is kept once, so the first check does not see a
@@ -272,20 +253,10 @@ func (s *State) Save() error {
 		return fmt.Errorf("creating data directory: %w", err)
 	}
 
-	// Atomic write: write to temp file, then rename. The temp file is
-	// synced before the rename and the directory after it, or a crash
-	// can leave the renamed state file empty.
+	// Atomic write: write to temp file, then rename
 	tmpPath := path + ".tmp"
 
-	//nolint:gosec // path is from trusted config
-	tmpFile, err := os.OpenFile(
-		tmpPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, filePermissions,
-	)
-	if err == nil {
-		_, err = tmpFile.Write(data)
-		err = errors.Join(err, tmpFile.Sync(), tmpFile.Close())
-	}
-
+	err = os.WriteFile(tmpPath, data, filePermissions)
 	if err != nil {
 		return fmt.Errorf("writing temp state file: %w", err)
 	}
@@ -295,15 +266,6 @@ func (s *State) Save() error {
 		return fmt.Errorf("renaming state file: %w", err)
 	}
 
-	dir, err := os.Open(filepath.Dir(path))
-	if err == nil {
-		err = errors.Join(dir.Sync(), dir.Close())
-	}
-
-	if err != nil {
-		return fmt.Errorf("syncing data directory: %w", err)
-	}
-
 	s.log.Debug("state saved to disk", "path", path)
 
 	return nil

internal/state/state_test.go

@@ -618,53 +618,6 @@ func TestSaveAtomicWrite(t *testing.T) {
 	}
 }
 
-// TestSaveOverLeftoverTempFile verifies that a temp file left by a
-// crash, longer than the state Save writes, leaves nothing of itself in
-// the state file, and that no temp file remains after Save.
-func TestSaveOverLeftoverTempFile(t *testing.T) {
-	t.Parallel()
-
-	dir := t.TempDir()
-	statePath := filepath.Join(dir, "state.json")
-	tmpPath := statePath + ".tmp"
-
-	err := os.WriteFile(tmpPath, []byte(strings.Repeat("x", 64*1024)), 0o600)
-	if err != nil {
-		t.Fatalf("writing leftover temp file: %v", err)
-	}
-
-	s := state.NewForTestWithDataDir(dir)
-
-	populateState(t, s)
-
-	err = s.Save()
-	if err != nil {
-		t.Fatalf("Save() error: %v", err)
-	}
-
-	//nolint:gosec // test file with known path
-	data, err := os.ReadFile(statePath)
-	if err != nil {
-		t.Fatalf("reading state file: %v", err)
-	}
-
-	var snap state.Snapshot
-
-	err = json.Unmarshal(data, &snap)
-	if err != nil {
-		t.Fatalf("state file is not complete JSON: %v", err)
-	}
-
-	if len(snap.Domains) != 2 {
-		t.Errorf("state file has %d domains, want 2", len(snap.Domains))
-	}
-
-	_, err = os.Stat(tmpPath)
-	if !os.IsNotExist(err) {
-		t.Error("temp file should not exist after save")
-	}
-}
-
 // TestLoadMissingFile verifies that loading a nonexistent file is not an error.
 func TestLoadMissingFile(t *testing.T) {
 	t.Parallel()
@@ -695,58 +648,6 @@ func TestLoadMissingFile(t *testing.T) {
 	}
 }
 
-// TestLoadFileWithoutSections loads a state file holding {}, and files
-// that each leave out one section. Every section loads as an empty map.
-// TestFirstCheckAfterLoadingFileWithoutSections in the watcher tests
-// runs the first check after such a load.
-func TestLoadFileWithoutSections(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name string
-		file string
-	}{
-		{"no sections", `{}`},
-		{"no domains", `{"hostnames": {}, "ports": {}, "certificates": {}}`},
-		{"no hostnames", `{"domains": {}, "ports": {}, "certificates": {}}`},
-		{"no ports", `{"domains": {}, "hostnames": {}, "certificates": {}}`},
-		{"no certificates", `{"domains": {}, "hostnames": {}, "ports": {}}`},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			dir := t.TempDir()
-
-			err := os.WriteFile(
-				filepath.Join(dir, "state.json"), []byte(tt.file), 0o600,
-			)
-			if err != nil {
-				t.Fatalf("writing state file: %v", err)
-			}
-
-			s := state.NewForTestWithDataDir(dir)
-
-			err = s.Load()
-			if err != nil {
-				t.Fatalf("Load() error: %v", err)
-			}
-
-			want := state.Snapshot{
-				Domains:      map[string]*state.DomainState{},
-				Hostnames:    map[string]*state.HostnameState{},
-				Ports:        map[string]*state.PortState{},
-				Certificates: map[string]*state.CertificateState{},
-			}
-
-			if got := s.GetSnapshot(); !reflect.DeepEqual(got, want) {
-				t.Errorf("loaded %#v, want %#v", got, want)
-			}
-		})
-	}
-}
-
 // TestLoadCorruptFile verifies that a corrupt state file returns an error.
 func TestLoadCorruptFile(t *testing.T) {
 	t.Parallel()

internal/watcher/cancelled_test.go

@@ -83,30 +83,6 @@ func TestCancelledCheckSavesNothing(t *testing.T) {
 	}
 }
 
-// TestStopBeforeFirstScanSendsNoStartupNotification runs a watcher with
-// the startup notification on and its context already cancelled, so the
-// first scan is cut short before its lookup is sent. The scan did not
-// complete, so no startup notification may be sent.
-func TestStopBeforeFirstScanSendsNoStartupNotification(t *testing.T) {
-	t.Parallel()
-
-	cfg := defaultTestConfig(t)
-	cfg.Hostnames = []string{testHost}
-	cfg.SendTestNotification = true
-
-	w, deps := newTestWatcher(t, cfg)
-
-	ctx, cancel := context.WithCancel(t.Context())
-	cancel()
-
-	w.Run(ctx)
-
-	notifications := deps.notifier.getNotifications()
-	if len(notifications) != 0 {
-		t.Errorf("sent %v, want no notifications", notifications)
-	}
-}
-
 // newLoggingWatcher returns a watcher for a domain and a hostname, with
 // the real resolver, that writes what it logs at warning level or above
 // into the returned buffer.

internal/watcher/ipv6_test.go

@@ -1,83 +0,0 @@
-package watcher_test
-
-import (
-	"maps"
-	"slices"
-	"testing"
-
-	"sneak.berlin/go/dnswatcher/internal/state"
-	"sneak.berlin/go/dnswatcher/internal/watcher"
-)
-
-// TestIPv6AddressInBrackets runs the port checks, not the watcher's
-// first, on hostname state built here for a hostname at an IPv6 address
-// whose port 443 the previous check found closed. It is open now. Then
-// it runs the TLS checks and the port checks again. The port and
-// certificate keys and the Port Change notification write the address
-// in brackets, so its last group cannot be read as the port, and the
-// second port checks keep the certificate entry, whose address they read
-// back from its key. When a last port check finds port 443 closed, the
-// certificate entry is removed. Nothing is looked up.
-func TestIPv6AddressInBrackets(t *testing.T) {
-	t.Parallel()
-
-	const aaaaType = "AAAA"
-
-	cfg := defaultTestConfig(t)
-	cfg.Hostnames = []string{host}
-
-	deps := newTestDeps(t, cfg)
-	w := watcher.NewForTest(
-		cfg, deps.state, nil,
-		deps.portChecker, deps.tlsChecker, deps.notifier,
-	)
-	w.SetFirstRun(false)
-
-	deps.state.SetHostnameState(host, saved(
-		map[string]*state.NameserverRecordState{
-			nsA: answered(map[string][]string{aaaaType: {"2001:db8::1"}}),
-		},
-	))
-	deps.state.SetPortState("[2001:db8::1]:443", &state.PortState{
-		Hostnames: []string{host},
-	})
-
-	w.CheckAllPorts(t.Context())
-	w.RunTLSChecks(t.Context())
-	w.CheckAllPorts(t.Context())
-
-	snap := deps.state.GetSnapshot()
-
-	gotPorts := slices.Sorted(maps.Keys(snap.Ports))
-	wantPorts := []string{"[2001:db8::1]:443", "[2001:db8::1]:80"}
-
-	if !slices.Equal(gotPorts, wantPorts) {
-		t.Errorf("port entries %v, want %v", gotPorts, wantPorts)
-	}
-
-	gotCerts := slices.Sorted(maps.Keys(snap.Certificates))
-	wantCerts := []string{"[2001:db8::1]:443:www.example.net"}
-
-	if !slices.Equal(gotCerts, wantCerts) {
-		t.Errorf("certificate entries %v, want %v", gotCerts, wantCerts)
-	}
-
-	title := "Port Change: [2001:db8::1]:443"
-	want := `Hostnames: www.example.net
-Address: [2001:db8::1]:443
-Port now open`
-
-	got := deps.notifier.getNotifications()
-	if len(got) != 1 || got[0].Title != title || got[0].Message != want {
-		t.Errorf("sent %v, want one %q with message:\n%s", got, title, want)
-	}
-
-	deps.portChecker.closed = true
-
-	w.CheckAllPorts(t.Context())
-
-	gotCerts = slices.Sorted(maps.Keys(deps.state.GetSnapshot().Certificates))
-	if len(gotCerts) != 0 {
-		t.Errorf("certificate entries %v with port 443 closed, want none", gotCerts)
-	}
-}

internal/watcher/removed_test.go

@@ -2,9 +2,7 @@ package watcher_test
 
 import (
 	"maps"
-	"net"
 	"slices"
-	"strings"
 	"testing"
 
 	"sneak.berlin/go/dnswatcher/internal/state"
@@ -174,7 +172,7 @@ func TestCertificateStateForAnAddressGone(t *testing.T) {
 					"A": {ip1}, "AAAA": {ip6},
 				}),
 			}),
-			[]string{ip1 + ":443:" + host, "[" + ip6 + "]:443:" + host},
+			[]string{ip1 + ":443:" + host, ip6 + ":443:" + host},
 		},
 		{
 			"no nameserver answered",
@@ -184,7 +182,7 @@ func TestCertificateStateForAnAddressGone(t *testing.T) {
 			[]string{
 				ip1 + ":443:" + host,
 				ip2 + ":443:" + host,
-				"[" + ip6 + "]:443:" + host,
+				ip6 + ":443:" + host,
 			},
 		},
 	}
@@ -207,8 +205,7 @@ func TestCertificateStateForAnAddressGone(t *testing.T) {
 
 			for _, ip := range []string{ip1, ip2, ip6} {
 				deps.state.SetCertificateState(
-					net.JoinHostPort(ip, "443")+":"+host,
-					&state.CertificateState{Status: "ok"},
+					ip+":443:"+host, &state.CertificateState{Status: "ok"},
 				)
 			}
 
@@ -225,67 +222,3 @@ func TestCertificateStateForAnAddressGone(t *testing.T) {
 		})
 	}
 }
-
-// TestCertificateStateForAPortClosed runs the port checks on hostname
-// and port state built here for a configured hostname at ip1, whose
-// port 443 the previous check found open and whose certificate entry
-// is ok. The port is closed now: the certificate entry is removed, and
-// only the Port Change is notified. When the port opens again, the next
-// TLS check saves the certificate as new and notifies nothing about it.
-// Nothing is looked up.
-func TestCertificateStateForAPortClosed(t *testing.T) {
-	t.Parallel()
-
-	cfg := defaultTestConfig(t)
-	cfg.Hostnames = []string{host}
-
-	deps := newTestDeps(t, cfg)
-	w := watcher.NewForTest(
-		cfg, deps.state, nil,
-		deps.portChecker, deps.tlsChecker, deps.notifier,
-	)
-	w.SetFirstRun(false)
-
-	certKey := ip1 + ":443:" + host
-
-	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},
-	})
-	deps.state.SetCertificateState(certKey, &state.CertificateState{
-		CommonName: host, Status: "ok",
-	})
-	deps.portChecker.closed = true
-
-	w.CheckAllPorts(t.Context())
-
-	if _, ok := deps.state.GetCertificateState(certKey); ok {
-		t.Errorf("certificate entry %s kept with port 443 closed", certKey)
-	}
-
-	sent := deps.notifier.getNotifications()
-	if title := "Port Change: " + ip1 + ":443"; len(sent) != 1 ||
-		sent[0].Title != title {
-		t.Errorf("sent %v, want one %q", sent, title)
-	}
-
-	deps.portChecker.closed = false
-
-	w.CheckAllPorts(t.Context())
-	w.RunTLSChecks(t.Context())
-
-	cert, ok := deps.state.GetCertificateState(certKey)
-	if !ok || cert.Status != "ok" {
-		t.Errorf("certificate entry %s after port 443 opened: %+v", certKey, cert)
-	}
-
-	for _, n := range deps.notifier.getNotifications() {
-		if strings.HasPrefix(n.Title, "TLS") {
-			t.Errorf("sent %v, want no TLS notification", n)
-		}
-	}
-}

internal/watcher/watcher.go

@@ -6,10 +6,8 @@ import (
 	"fmt"
 	"log/slog"
 	"maps"
-	"net"
 	"slices"
 	"sort"
-	"strconv"
 	"strings"
 	"time"
 
@@ -135,12 +133,7 @@ func (w *Watcher) Run(ctx context.Context) {
 
 	w.cleanupRemovedTargets()
 	w.RunOnce(ctx)
-
-	// RunOnce returns as usual when shutdown cuts the scan short, and
-	// the startup notification says the scan completed.
-	if ctx.Err() == nil {
-		w.maybeSendTestNotification(ctx)
-	}
+	w.maybeSendTestNotification(ctx)
 
 	dnsTicker := time.NewTicker(w.config.DNSInterval)
 	tlsTicker := time.NewTicker(w.config.TLSInterval)
@@ -329,23 +322,22 @@ func (w *Watcher) checkDomain(
 		ctx, nameservers, prevAddresses,
 	)
 
+	if hasPrev && !w.firstRun {
+		w.detectNSChanges(ctx, domain, prev.Nameservers, nameservers)
+		w.detectNSAddressChanges(ctx, domain, prevAddresses, addresses)
+	}
+
 	// A domain that does not exist has no records of its own: none are
 	// asked for, and those saved by an earlier check are removed.
 	// Otherwise the apex domain's records are also checked and saved as
 	// a hostname's, so that the port and TLS checks find its addresses.
-	// Notifications about them name it as a domain (see nameLine). When
-	// their lookup fails, the check fails as when the NS lookup does, so
-	// the saved delegation and the saved records come from the same
-	// check, as detectNSDisappearances needs.
+	// Notifications about them name it as a domain (see nameLine). They
+	// are checked before the domain's new nameservers are saved:
+	// detectNSDisappearances reads those of the previous check.
 	if nxdomain {
 		w.state.DeleteHostnameState(domain)
-	} else if !w.checkHostname(ctx, domain) {
-		return
-	}
-
-	if hasPrev && !w.firstRun {
-		w.detectNSChanges(ctx, domain, prev.Nameservers, nameservers)
-		w.detectNSAddressChanges(ctx, domain, prevAddresses, addresses)
+	} else {
+		w.checkHostname(ctx, domain)
 	}
 
 	w.state.SetDomainState(domain, &state.DomainState{
@@ -464,12 +456,10 @@ func (w *Watcher) detectNSAddressChanges(
 	}
 }
 
-// checkHostname checks hostname's records and reports whether it saved
-// them; it does not when their lookup fails.
 func (w *Watcher) checkHostname(
 	ctx context.Context,
 	hostname string,
-) bool {
+) {
 	results, err := w.resolver.LookupAllRecords(ctx, hostname)
 	if err != nil {
 		w.logFailedLookup(
@@ -479,7 +469,7 @@ func (w *Watcher) checkHostname(
 			"error", err,
 		)
 
-		return false
+		return
 	}
 
 	prev, _ := w.state.GetHostnameState(hostname)
@@ -487,8 +477,6 @@ func (w *Watcher) checkHostname(
 	w.updateHostnameState(
 		ctx, hostname, buildHostnameState(results, prev, time.Now().UTC()),
 	)
-
-	return true
 }
 
 // updateHostnameState finishes a check of hostname from newState, built
@@ -787,10 +775,9 @@ func (w *Watcher) detectRecordChanges(
 // detectNSDisappearances sends one NS Change listing the nameservers of
 // the previous check that are no longer those of the zone hostname is
 // in. For a configured apex domain it leaves out those of the domain's
-// saved delegation, which detectNSChanges notifies when they are removed
-// from it. checkDomain saves a new delegation only after this runs, and
-// only together with the records, so the saved delegation and the
-// records in prev come from the same check.
+// delegation on the previous check, which detectNSChanges notifies when
+// they are removed from it; checkDomain saves the new delegation only
+// after this runs.
 func (w *Watcher) detectNSDisappearances(
 	ctx context.Context,
 	hostname string,
@@ -980,8 +967,7 @@ func (w *Watcher) checkAllPorts(ctx context.Context) {
 
 	// Phase 3: Remove port state entries that no longer have
 	// any hostname referencing them, and certificate entries for
-	// an address whose port 443 is closed or that their name no
-	// longer has.
+	// an address their name no longer has.
 	w.cleanupStalePorts(associations)
 	w.cleanupStaleCertificates()
 }
@@ -1002,7 +988,7 @@ func (w *Watcher) buildPortAssociations() map[string][]string {
 		ips := w.collectIPs(name)
 		for _, ip := range ips {
 			for _, port := range monitoredPorts {
-				key := net.JoinHostPort(ip, strconv.Itoa(port))
+				key := fmt.Sprintf("%s:%d", ip, port)
 				if assoc[key] == nil {
 					assoc[key] = make(map[string]bool)
 				}
@@ -1027,15 +1013,18 @@ func (w *Watcher) buildPortAssociations() map[string][]string {
 	return result
 }
 
-// parsePortKey splits an "ip:port" key, written by net.JoinHostPort,
-// into its components.
+// parsePortKey splits an "ip:port" key into its components.
 func parsePortKey(key string) (string, int) {
-	ip, port, err := net.SplitHostPort(key)
-	if err != nil {
+	lastColon := strings.LastIndex(key, ":")
+	if lastColon < 0 {
 		return key, 0
 	}
 
-	p, err := strconv.Atoi(port)
+	ip := key[:lastColon]
+
+	var p int
+
+	_, err := fmt.Sscanf(key[lastColon+1:], "%d", &p)
 	if err != nil {
 		return ip, 0
 	}
@@ -1065,21 +1054,13 @@ func (w *Watcher) cleanupStalePorts(
 }
 
 // cleanupStaleCertificates removes the certificate entries for an
-// address whose port 443 was found closed, and for an address their
-// name no longer resolves to. An entry saved for a name none of whose
-// nameservers answered is kept unless its port is closed: that name's
-// addresses are not known, not gone.
+// address their name no longer resolves to. An entry saved for a name
+// none of whose nameservers answered is kept: that name's addresses are
+// not known, not gone.
 func (w *Watcher) cleanupStaleCertificates() {
 	for _, key := range w.state.GetAllCertificateKeys() {
 		ip, hostname := parseCertKey(key)
 
-		ps, ok := w.state.GetPortState(net.JoinHostPort(ip, strconv.Itoa(tlsPort)))
-		if ok && !ps.Open {
-			w.state.DeleteCertificateState(key)
-
-			continue
-		}
-
 		if slices.Contains(w.collectIPs(hostname), ip) ||
 			w.noNameserverAnswered(hostname) {
 			continue
@@ -1188,7 +1169,7 @@ func (w *Watcher) checkSinglePort(
 		return
 	}
 
-	key := net.JoinHostPort(ip, strconv.Itoa(port))
+	key := fmt.Sprintf("%s:%d", ip, port)
 	now := time.Now().UTC()
 	prev, hasPrev := w.state.GetPortState(key)
 
@@ -1250,7 +1231,7 @@ func (w *Watcher) checkTLSForHostname(
 	ips := w.collectIPs(hostname)
 
 	for _, ip := range ips {
-		portKey := net.JoinHostPort(ip, strconv.Itoa(tlsPort))
+		portKey := fmt.Sprintf("%s:%d", ip, tlsPort)
 
 		ps, ok := w.state.GetPortState(portKey)
 		if !ok || !ps.Open {
@@ -1274,7 +1255,7 @@ func (w *Watcher) checkTLSCert(
 		return
 	}
 
-	certKey := net.JoinHostPort(ip, strconv.Itoa(tlsPort)) + ":" + hostname
+	certKey := fmt.Sprintf("%s:%d:%s", ip, tlsPort, hostname)
 	now := time.Now().UTC()
 	prev, hasPrev := w.state.GetCertificateState(certKey)
 

internal/watcher/watcher_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
-	"net"
 	"os"
 	"slices"
 	"strings"
@@ -140,22 +139,6 @@ func (m *mockNotifier) getNotifications() []notification {
 	return result
 }
 
-// cancelOnNotify is a mockNotifier that calls cancel at each
-// notification, as if shutdown cut the check short there.
-type cancelOnNotify struct {
-	*mockNotifier
-
-	cancel context.CancelFunc
-}
-
-func (n cancelOnNotify) SendNotification(
-	ctx context.Context,
-	title, message, priority string,
-) {
-	n.mockNotifier.SendNotification(ctx, title, message, priority)
-	n.cancel()
-}
-
 // --- Helpers to build a Watcher and run its checks against live DNS ---
 
 type testDeps struct {
@@ -478,54 +461,6 @@ func TestDomainPortAndTLSChecks(t *testing.T) {
 	}
 }
 
-// TestFirstCheckAfterLoadingFileWithoutSections loads a state file
-// holding {}, and one that leaves out the certificates section, and runs
-// the first check. It saves domain, hostname, port and certificate
-// entries; a section loaded as nil made it panic.
-func TestFirstCheckAfterLoadingFileWithoutSections(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name string
-		file string
-	}{
-		{"no sections", `{}`},
-		{"no certificates", `{"domains": {}, "hostnames": {}, "ports": {}}`},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			cfg := defaultTestConfig(t)
-			cfg.Domains = []string{testSmallDomain}
-
-			_, deps := runChecks(t, cfg, func(deps *testDeps) {
-				err := os.WriteFile(cfg.StatePath(), []byte(tt.file), 0o600)
-				if err != nil {
-					t.Fatalf("writing the state file: %v", err)
-				}
-
-				err = deps.state.Load()
-				if err != nil {
-					t.Fatalf("loading the state file: %v", err)
-				}
-			})
-
-			snap := deps.state.GetSnapshot()
-			if len(snap.Domains) == 0 || len(snap.Hostnames) == 0 ||
-				len(snap.Ports) == 0 || len(snap.Certificates) == 0 {
-				t.Errorf(
-					"saved %d domain, %d hostname, %d port and %d "+
-						"certificate entries, want at least one of each",
-					len(snap.Domains), len(snap.Hostnames),
-					len(snap.Ports), len(snap.Certificates),
-				)
-			}
-		})
-	}
-}
-
 func TestNSChangeDetection(t *testing.T) {
 	t.Parallel()
 
@@ -624,10 +559,8 @@ func TestDomainThatDoesNotExist(t *testing.T) {
 // codeberg.page servers, are saved. Those are testSmallDomain's two
 // nameservers; github.io, the zone of the README's example, has eight.
 // The previous check saved a delegation to oldNS1, now withdrawn, and
-// records asked at oldNS1. A first check is cut short at its first
-// notification, as shutdown would, and a second runs in full: across
-// them the NS Change of its delegation removes oldNS1, and no other NS
-// Change lists it again.
+// records asked at oldNS1: the NS Change of its delegation removes
+// oldNS1, and no other NS Change lists it again.
 func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 	t.Parallel()
 
@@ -639,19 +572,9 @@ func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 	var deps *testDeps
 
 	livednstest.Retry(t, "watcher checks", func(ctx context.Context) error {
-		deps = newTestDeps(t, cfg)
+		var w *watcher.Watcher
 
-		firstCtx, cancel := context.WithCancel(ctx)
-		defer cancel()
-
-		w := watcher.NewForTest(
-			cfg,
-			deps.state,
-			resolver.NewFromLogger(slog.Default()),
-			deps.portChecker,
-			deps.tlsChecker,
-			cancelOnNotify{mockNotifier: deps.notifier, cancel: cancel},
-		)
+		w, deps = newTestWatcher(t, cfg)
 
 		deps.state.SetDomainState(domain, &state.DomainState{
 			Nameservers: []string{oldNS1},
@@ -662,8 +585,6 @@ func TestDomainWithNoDelegationOfItsOwn(t *testing.T) {
 			},
 		))
 
-		w.RunOnce(firstCtx)
-
 		err := checkOnce(ctx, w, deps)
 
 		// A domain saved as not existing has no records to wait for;
@@ -825,8 +746,7 @@ func TestPortStateChange(t *testing.T) {
 
 	hs, _ := deps.state.GetHostnameState(testHost)
 	assertNotified(
-		t, deps, "Port Change: "+net.JoinHostPort(addresses(hs)[0], "443"),
-		"warning",
+		t, deps, "Port Change: "+addresses(hs)[0]+":443", "warning",
 	)
 }
 
@@ -1033,12 +953,11 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
 
 	// Port and TLS checks must use the addresses this check found.
 	for _, ip := range addresses(snap.Hostnames[testHost]) {
-		portKey := net.JoinHostPort(ip, "80")
-		if _, ok := snap.Ports[portKey]; !ok {
-			t.Error("port check used stale DNS: missing " + portKey)
+		if _, ok := snap.Ports[ip+":80"]; !ok {
+			t.Error("port check used stale DNS: missing " + ip + ":80")
 		}
 
-		certKey := net.JoinHostPort(ip, "443") + ":" + testHost
+		certKey := ip + ":443:" + testHost
 		if _, ok := snap.Certificates[certKey]; !ok {
 			t.Error("TLS check used stale DNS: missing " + certKey)
 		}