1 Commits

Author SHA1 Message Date
dea7e441b9 build: update golangci-lint to v2.12.2 with canonical config
All checks were successful
check / check (push) Successful in 1m12s
Replace the commit-pinned golangci-lint install refs in Dockerfile and
script/bootstrap with the v2.12.2 version pin, and replace .golangci.yml
with the canonical v2-schema config (lint settings now live under
linters.settings, so the lll/funlen/cyclop/dupl thresholds are actually
applied).

Fix all findings surfaced by the newly-applied config:

- goconst: introduce shared constants for repeated status, priority,
  and DNS fixture strings in watcher.go and the notify, state, and
  watcher tests
- dupl: consolidate duplicated ntfy/slack HTTP-error tests and
  SendNotification endpoint-error tests behind shared helpers
- lll: wrap long test table entries and comments; shorten one inline
  nolint justification
2026-08-07 17:08:04 +00:00
7 changed files with 14 additions and 255 deletions

View File

@@ -5,7 +5,7 @@ FROM golang@sha256:f6751d823c26342f9506c03797d2527668d095b0a15f1862cddb4d927a7a4
RUN apk add --no-cache git make gcc musl-dev binutils-gold RUN apk add --no-cache git make gcc musl-dev binutils-gold
# golangci-lint v2.12.2, 2026-08-07 # golangci-lint v2.12.2, 2026-08-07
RUN go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5 RUN go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2
# goimports v0.42.0 # goimports v0.42.0
RUN go install golang.org/x/tools/cmd/goimports@009367f5c17a8d4c45a961a3a509277190a9a6f0 RUN go install golang.org/x/tools/cmd/goimports@009367f5c17a8d4c45a961a3a509277190a9a6f0

View File

@@ -17,26 +17,6 @@ without requiring an external database.
--- ---
## No DNS mocking. Ever.
**DNS is never mocked in this project — not in tests, not anywhere else.**
No mock resolvers, no fake DNS servers, no stubbed lookups.
dnswatcher's entire purpose is correct behavior against the real DNS.
Tests exercise real iterative resolution against live nameservers by
design; a test suite that passes against a mock proves nothing about the
one thing this program exists to do.
When live tests are flaky, that is a robustness problem, and it gets
fixed with robustness: retries with backoff, querying multiple
independent nameservers, longer timeouts — or explicit opt-in gating
decided by the project owner. Never with mocks.
Contributions that introduce mocked, faked, or stubbed DNS will be
rejected.
---
## Features ## Features
### DNS Domain Monitoring (Apex Domains) ### DNS Domain Monitoring (Apex Domains)
@@ -182,27 +162,6 @@ dnswatcher exposes a lightweight HTTP API for operational visibility:
| `GET /api/v1/status` | Current monitoring state | | `GET /api/v1/status` | Current monitoring state |
| `GET /metrics` | Prometheus metrics (optional) | | `GET /metrics` | Prometheus metrics (optional) |
#### Server timeouts
The HTTP server sets all four socket-level timeouts. These are compile-time
constants in `internal/server/server.go`, not configurable via environment
variables.
| Timeout | Value | Purpose |
|---------------------|-------|-----------------------------------------------|
| `ReadHeaderTimeout` | 10s | Bounds the request header read (slowloris) |
| `ReadTimeout` | 15s | Bounds the whole request read, headers + body |
| `WriteTimeout` | 75s | Bounds handler execution plus response flush |
| `IdleTimeout` | 120s | Reaps idle keep-alive connections |
These are distinct from the 60s per-request handler budget applied by
`chimw.Timeout` in `internal/server/routes.go`, which cancels the request
context but does not touch the socket. `WriteTimeout` is deliberately
larger than that budget: the write deadline is armed once request headers
are read, so a smaller value would sever the connection before a handler
using its full budget could respond. `IdleTimeout` exceeds common
Prometheus scrape intervals so the scraper reuses its connection.
--- ---
## Architecture ## Architecture

20
TODO.md
View File

@@ -25,22 +25,10 @@ confirm make check still passes.
# Completed Steps # Completed Steps
- 2026-08-09: `http.Server` now sets all four socket-level timeouts - 2026-08-07: golangci-lint bumped to v2.12.2 (pins in `Dockerfile` and
(`ReadTimeout` 15s, `ReadHeaderTimeout` 10s, `WriteTimeout` 75s, `script/bootstrap`), `.golangci.yml` replaced with the canonical v2
`IdleTimeout` 120s) as named constants in `internal/server/server.go`, config (settings moved under `linters.settings` so thresholds now
closing the slowloris / unreaped-keep-alive exposure required by apply); fixed all resulting `goconst`, `dupl`, and `lll` findings
`REPO_POLICIES.md` before 1.0; `WriteTimeout` is deliberately greater
than the 60s `chimw.Timeout` handler budget so that budget stays
reachable, and tests in `internal/server` pin both the non-zero
values and that relationship (#99)
- 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
org-standard v2-schema config used across the org's repos
(owner-authorized; same file is being landed as canonical via prompts
PR #24), with settings under `linters.settings` so the
lll/funlen/cyclop/dupl thresholds apply; fixed the resulting
`goconst`, `dupl`, and `lll` findings; the informational `gomodguard`
deprecation warning under this config is accepted
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
Makefile shims, README Entrypoints section Makefile shims, README Entrypoints section
- 2026-02-20: iterative DNS resolver implemented; tests made hermetic - 2026-02-20: iterative DNS resolver implemented; tests made hermetic

View File

@@ -1,19 +0,0 @@
package server
import (
"net/http"
"time"
)
// NewHTTPServer exports newHTTPServer for testing.
func NewHTTPServer(
listenAddr string,
handler http.Handler,
) *http.Server {
return newHTTPServer(listenAddr, handler)
}
// RequestTimeout exports the handler execution budget applied by
// chimw.Timeout in SetupRoutes, so tests can assert the relationship
// between it and the server's WriteTimeout.
const RequestTimeout time.Duration = requestTimeout

View File

@@ -33,52 +33,8 @@ type Params struct {
// shutdownTimeout is how long to wait for graceful shutdown. // shutdownTimeout is how long to wait for graceful shutdown.
const shutdownTimeout = 30 * time.Second const shutdownTimeout = 30 * time.Second
// Socket-level timeouts for the HTTP server. // readHeaderTimeout is the max duration for reading request headers.
// const readHeaderTimeout = 10 * time.Second
// These bound time spent on the connection itself and are a distinct
// control from the per-request handler budget enforced by
// chimw.Timeout(requestTimeout) in routes.go: that one cancels the
// request context after requestTimeout but never touches the socket,
// so without the values below a peer can hold a connection open
// forever (slowloris, unreaped keep-alives).
//
// The one hard constraint between the two controls is
// writeTimeout > requestTimeout. net/http arms the write deadline
// once the request headers have been read, so on a plaintext
// connection it covers handler execution AND the response flush. If
// writeTimeout were <= requestTimeout the server would sever the
// connection before a handler that legitimately consumed its full
// budget could emit anything, making the 60s budget unreachable in
// practice. The margin between them is the response-flush allowance.
//
// The only clients of this service are browsers loading the dashboard
// and a Prometheus scraper; the values are sized for those.
const (
// readHeaderTimeout is the max duration for reading request
// headers.
readHeaderTimeout = 10 * time.Second
// readTimeout bounds reading the entire request, headers plus
// body. Every route here is a GET with no body, so this only
// ever needs to cover headers; the extra 5s over
// readHeaderTimeout is slack, not a real allowance, and keeps a
// body dribbled one byte at a time from holding the read side
// open indefinitely.
readTimeout = 15 * time.Second
// writeTimeout must exceed the requestTimeout handler budget
// (60s) per the note above. The 15s difference is the allowance
// for flushing a completed response to a slow client.
writeTimeout = 75 * time.Second
// idleTimeout reaps keep-alive connections between requests. It
// is deliberately longer than the common Prometheus scrape
// intervals (15s/30s/60s) so the scraper reuses its connection
// rather than reconnecting every cycle, while a browser tab
// left open on the dashboard stops occupying a connection
// within two minutes of going quiet.
idleTimeout = 120 * time.Second
)
// Server is the HTTP server. // Server is the HTTP server.
type Server struct { type Server struct {
@@ -120,29 +76,16 @@ func New(
return srv, nil return srv, nil
} }
// newHTTPServer builds the listening http.Server with every
// socket-level timeout set. All four are set deliberately: a zero
// value in net/http means "no limit", not "some default".
func newHTTPServer(
listenAddr string,
handler http.Handler,
) *http.Server {
return &http.Server{
Addr: listenAddr,
Handler: handler,
ReadTimeout: readTimeout,
ReadHeaderTimeout: readHeaderTimeout,
WriteTimeout: writeTimeout,
IdleTimeout: idleTimeout,
}
}
// Run starts the HTTP server. // Run starts the HTTP server.
func (s *Server) Run() { func (s *Server) Run() {
s.SetupRoutes() s.SetupRoutes()
listenAddr := fmt.Sprintf(":%d", s.port) listenAddr := fmt.Sprintf(":%d", s.port)
s.httpServer = newHTTPServer(listenAddr, s) s.httpServer = &http.Server{
Addr: listenAddr,
Handler: s,
ReadHeaderTimeout: readHeaderTimeout,
}
s.log.Info("http server starting", "addr", listenAddr) s.log.Info("http server starting", "addr", listenAddr)

View File

@@ -1,112 +0,0 @@
package server_test
import (
"net/http"
"testing"
"sneak.berlin/go/dnswatcher/internal/server"
)
// noopHandler stands in for the router; newHTTPServer only stores it.
func noopHandler() http.Handler {
return http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
},
)
}
// TestHTTPServerTimeoutsAreSet asserts that every socket-level
// timeout is configured. A zero value in net/http means "no limit",
// so a refactor that silently drops one of these reintroduces the
// slowloris / unreaped-keep-alive exposure this guards against.
//
// The assertions are on the configured field values only; nothing
// here measures elapsed time, so the test cannot flake on timing.
func TestHTTPServerTimeoutsAreSet(t *testing.T) {
t.Parallel()
srv := server.NewHTTPServer(":8080", noopHandler())
if srv.ReadTimeout <= 0 {
t.Errorf(
"ReadTimeout must be non-zero, got %v",
srv.ReadTimeout,
)
}
if srv.ReadHeaderTimeout <= 0 {
t.Errorf(
"ReadHeaderTimeout must be non-zero, got %v",
srv.ReadHeaderTimeout,
)
}
if srv.WriteTimeout <= 0 {
t.Errorf(
"WriteTimeout must be non-zero, got %v",
srv.WriteTimeout,
)
}
if srv.IdleTimeout <= 0 {
t.Errorf(
"IdleTimeout must be non-zero, got %v",
srv.IdleTimeout,
)
}
}
// TestWriteTimeoutExceedsHandlerBudget pins the one relationship the
// values must satisfy. net/http arms the write deadline once request
// headers are read, so it covers handler execution plus the response
// flush. If WriteTimeout were not greater than the chimw.Timeout
// handler budget, the connection would be severed before a handler
// that used its full budget could respond, making that budget
// unreachable.
func TestWriteTimeoutExceedsHandlerBudget(t *testing.T) {
t.Parallel()
srv := server.NewHTTPServer(":8080", noopHandler())
if srv.WriteTimeout <= server.RequestTimeout {
t.Errorf(
"WriteTimeout (%v) must exceed handler budget (%v)",
srv.WriteTimeout,
server.RequestTimeout,
)
}
}
// TestReadTimeoutCoversHeaderTimeout asserts the read deadline for
// the whole request is at least as long as the header-only deadline;
// a smaller ReadTimeout would make ReadHeaderTimeout unreachable.
func TestReadTimeoutCoversHeaderTimeout(t *testing.T) {
t.Parallel()
srv := server.NewHTTPServer(":8080", noopHandler())
if srv.ReadTimeout < srv.ReadHeaderTimeout {
t.Errorf(
"ReadTimeout (%v) must be >= ReadHeaderTimeout (%v)",
srv.ReadTimeout,
srv.ReadHeaderTimeout,
)
}
}
// TestHTTPServerAddrAndHandler covers the rest of the constructor so
// a future edit cannot drop the listen address or the handler.
func TestHTTPServerAddrAndHandler(t *testing.T) {
t.Parallel()
srv := server.NewHTTPServer(":9999", noopHandler())
if srv.Addr != ":9999" {
t.Errorf("Addr = %q, want %q", srv.Addr, ":9999")
}
if srv.Handler == nil {
t.Error("Handler must not be nil")
}
}

View File

@@ -4,14 +4,14 @@
# installed tools are skipped. Base tooling comes from nix, apt, brew, # installed tools are skipped. Base tooling comes from nix, apt, brew,
# or apk (detected in that order); assumes nothing is present. # or apk (detected in that order); assumes nothing is present.
# golangci-lint and goimports are installed via `go install` at the same # golangci-lint and goimports are installed via `go install` at the same
# pinned commits the Dockerfile uses (never "latest"). # pinned refs the Dockerfile uses (never "latest").
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Pinned versions, 2026-08-07 (same pins as the Dockerfile) # Pinned versions, 2026-08-07 (same pins as the Dockerfile)
# golangci-lint v2.12.2 # golangci-lint v2.12.2
GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5" GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2"
# goimports v0.42.0 # goimports v0.42.0
GOIMPORTS_REF="golang.org/x/tools/cmd/goimports@009367f5c17a8d4c45a961a3a509277190a9a6f0" GOIMPORTS_REF="golang.org/x/tools/cmd/goimports@009367f5c17a8d4c45a961a3a509277190a9a6f0"