watcher: save and notify nothing for a cut-short port or TLS check (closes #185)
check / check (push) Successful in 1m38s

When shutdown cancels a check that is under way, the rest of the check
still runs with the cancelled context. The resolver already drops a
lookup the context cut short, but a cancelled connection attempt was
saved as a closed port or a failed certificate check and notified as
Port Change or TLS Failure. The watcher now drops a port or TLS check
result when its context was cancelled, the same way. The test runs a
check with the context already cancelled, using the real resolver and
the real port and TLS checkers; no query is sent and no connection is
made.

Model: opus-5-5
This commit is contained in:
2026-10-01 20:42:01 +00:00
parent a8f9a64600
commit a860dd62ed
4 changed files with 99 additions and 1 deletions
+2 -1
View File
@@ -618,7 +618,8 @@ repository's `Dockerfile` and runs it. The app needs:
abandoned, and the number abandoned is logged at warn level rather abandoned, and the number abandoned is logged at warn level rather
than dropped silently. Notifications generated after shutdown has than dropped silently. Notifications generated after shutdown has
begun are refused and logged, so a late burst cannot extend the begun are refused and logged, so a late burst cannot extend the
shutdown. shutdown. A DNS lookup, port check or TLS check that shutdown cuts
short saves nothing and sends no notification.
--- ---
+2
View File
@@ -19,6 +19,8 @@ nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105
# Completed Steps # Completed Steps
- 2026-10-01: a port or TLS check that shutdown cuts short saves nothing and
sends no notification, as a cut-short DNS lookup already did (closes #185).
- 2026-10-01: the client address from `X-Forwarded-For` is the last entry that - 2026-10-01: the client address from `X-Forwarded-For` is the last entry that
is not a trusted proxy, not the first, which the client sets (closes #181). is not a trusted proxy, not the first, which the client sets (closes #181).
- 2026-10-01: a nameserver that does not answer is saved as `error` with the - 2026-10-01: a nameserver that does not answer is saved as `error` with the
+81
View File
@@ -0,0 +1,81 @@
package watcher_test
import (
"context"
"log/slog"
"reflect"
"testing"
"sneak.berlin/go/dnswatcher/internal/portcheck"
"sneak.berlin/go/dnswatcher/internal/resolver"
"sneak.berlin/go/dnswatcher/internal/state"
"sneak.berlin/go/dnswatcher/internal/tlscheck"
"sneak.berlin/go/dnswatcher/internal/watcher"
)
// TestCancelledCheckSavesNothing runs a check with its context already
// cancelled, which is how the rest of a check runs once shutdown cuts it
// short. The real resolver drops the DNS lookup without sending a query,
// and the real port and TLS checkers fail without connecting. The port
// and certificate state the last check saved must stay as it was, and
// nothing may be notified.
func TestCancelledCheckSavesNothing(t *testing.T) {
t.Parallel()
cfg := defaultTestConfig(t)
cfg.Hostnames = []string{host}
// newTestWatcher's watcher has stand-in checkers. This one, on the
// same state and notifier, has the real ones.
_, deps := newTestWatcher(t, cfg)
w := watcher.NewForTest(
cfg,
deps.state,
resolver.NewFromLogger(slog.Default()),
portcheck.NewStandalone(),
tlscheck.NewStandalone(),
deps.notifier,
)
// The last check found host at a local address, with both ports
// open and a good certificate.
const localIP = "127.0.0.1"
deps.state.SetHostnameState(host, hostnameState(
map[string]map[string][]string{nsA: {"A": {localIP}}},
))
ports := map[string]*state.PortState{
localIP + ":80": {Open: true, Hostnames: []string{host}},
localIP + ":443": {Open: true, Hostnames: []string{host}},
}
for key, ps := range ports {
deps.state.SetPortState(key, ps)
}
certKey := localIP + ":443:" + host
cert := &state.CertificateState{CommonName: host, Status: "ok"}
deps.state.SetCertificateState(certKey, cert)
ctx, cancel := context.WithCancel(t.Context())
cancel()
w.RunOnce(ctx)
for key, want := range ports {
got, _ := deps.state.GetPortState(key)
if !reflect.DeepEqual(got, want) {
t.Errorf("port %s saved as %+v, want %+v", key, got, want)
}
}
got, _ := deps.state.GetCertificateState(certKey)
if !reflect.DeepEqual(got, cert) {
t.Errorf("certificate saved as %+v, want %+v", got, cert)
}
notifications := deps.notifier.getNotifications()
if len(notifications) != 0 {
t.Errorf("sent %v, want no notifications", notifications)
}
}
+14
View File
@@ -658,6 +658,13 @@ func (w *Watcher) checkSinglePort(
hostnames []string, hostnames []string,
) { ) {
result, err := w.portCheck.CheckPort(ctx, ip, port) result, err := w.portCheck.CheckPort(ctx, ip, port)
// A check the context cut short says nothing about the port, so it
// is neither saved nor notified.
if ctx.Err() != nil {
return
}
if err != nil { if err != nil {
w.log.Error( w.log.Error(
"port check failed", "port check failed",
@@ -733,6 +740,13 @@ func (w *Watcher) checkTLSCert(
hostname string, hostname string,
) { ) {
cert, err := w.tlsCheck.CheckCertificate(ctx, ip, hostname) cert, err := w.tlsCheck.CheckCertificate(ctx, ip, hostname)
// A check the context cut short says nothing about the certificate,
// so it is neither saved nor notified.
if ctx.Err() != nil {
return
}
certKey := fmt.Sprintf("%s:%d:%s", ip, tlsPort, hostname) certKey := fmt.Sprintf("%s:%d:%s", ip, tlsPort, hostname)
now := time.Now().UTC() now := time.Now().UTC()
prev, hasPrev := w.state.GetCertificateState(certKey) prev, hasPrev := w.state.GetCertificateState(certKey)