3 Commits

Author SHA1 Message Date
02b63a4e65 server: set ReadTimeout, WriteTimeout, and IdleTimeout (closes #99)
All checks were successful
check / check (push) Successful in 37s
The http.Server literal only set ReadHeaderTimeout. The other three
timeouts defaulted to zero, which in net/http means no limit: past the
header phase a peer could hold a connection open forever, responses had
no write deadline, and keep-alive connections were never reaped.
REPO_POLICIES.md requires all four before 1.0.

All four are now named constants in internal/server/server.go:

  ReadHeaderTimeout  10s  (unchanged)
  ReadTimeout        15s  whole request; every route is a bodyless GET
  WriteTimeout       75s  handler execution plus response flush
  IdleTimeout       120s  keep-alive reaping

WriteTimeout must exceed the 60s chimw.Timeout handler budget in
routes.go. net/http arms the write deadline once the request headers
have been read, so it covers handler execution as well as the response
write; a smaller value would sever the connection before a handler that
legitimately used its full budget could respond, making that budget
unreachable. The 15s difference is the response-flush allowance. The
comment on the const block states this relationship.

IdleTimeout sits above the common Prometheus scrape intervals so the
scraper reuses its connection instead of reconnecting each cycle, while
an abandoned connection is still reaped within two minutes.

The http.Server literal moved into newHTTPServer so the configuration
is testable without binding a socket. Tests assert all four fields are
non-zero, that WriteTimeout exceeds the handler budget, and that
ReadTimeout covers ReadHeaderTimeout; they compare configured values
only and measure no elapsed time, so they cannot flake.
2026-08-09 05:40:31 +00:00
9347a2838b build: update golangci-lint to v2.12.2 with org-standard v2 config (#96)
All checks were successful
check / check (push) Successful in 4s
Updates golangci-lint to v2.12.2 and sets `.golangci.yml` to the org-standard v2-schema config already deployed across the org's repos. The config change is owner-authorized (see #96 (comment) and #96 (comment)); the same file is being landed as canonical via prompts PR #24 (sneak/prompts#24).

## Changes

- **Commit-pinned installs**: golangci-lint pinned to commit `c0d3ddc9cf3faa61a4e378e879ece580256d76e5` (v2.12.2, released 2026-05-06) in `Dockerfile` and `script/bootstrap`.
- **`.golangci.yml` set to the org-standard v2 config** (sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`), byte-identical to the file used across the org's other repos. Settings live under `linters.settings`, so the `lll`/`funlen`/`cyclop`/`dupl` thresholds are actually applied (under the old hybrid file, v2 silently ignored the top-level `linters-settings` block).
- **Lint fixes** required by the now-active thresholds:
  - `goconst`: shared constants for repeated status/priority/DNS-fixture strings in `internal/watcher/watcher.go` and the notify, state, and watcher tests
  - `dupl`: consolidated duplicated ntfy/slack HTTP-error tests and SendNotification endpoint-error tests behind shared helpers in `internal/notify/delivery_test.go`
  - `lll`: wrapped long test table entries and comments in `internal/config/classify_test.go`, `internal/notify/history_test.go`, `internal/state/state_test.go`, `internal/watcher/watcher_test.go`; shortened one inline nolint justification in `internal/notify/retry.go`
- **`TODO.md`**: Completed Steps entry updated in the same commit.
- Rebased onto current `main` (`f79cd98`); the branch is one clean commit.

## Notes

- v2.12 deprecates the `gomodguard` linter in favor of `gomodguard_v2`. The org-standard config does not disable the deprecated linter, so golangci-lint may emit an informational deprecation warning; this is accepted by the owner and does not affect the exit status (this exact config+code combination was CI-green at `dea7e44`).

## Verification

- `make check` exits 0 (fmt-check, tests, lint)
- `make lint`: 0 issues; no deprecation warning surfaced in the runs performed
- sha256 of `.golangci.yml` at HEAD verified equal to `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`

Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #96
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-08-07 23:15:47 +02:00
f79cd98107 docs: document the no-DNS-mocking policy in README (closes #94) (#95)
All checks were successful
check / check (push) Successful in 5s
Adds a prominent "No DNS mocking. Ever." section near the top of `README.md`, per owner policy (sneak, 2026-08-07):

- DNS is never mocked in this project — no mock resolvers, fake DNS servers, or stubbed lookups, in tests or anywhere else.
- Tests exercise real iterative resolution against live nameservers by design.
- Flaky live tests are fixed with robustness (retries, multiple nameservers, timeouts) or explicit opt-in gating decided by the owner — never with mocks.
- Contributions introducing DNS mocks will be rejected.

Markdown-only change; matches the README's existing tone and hard-wrap style. `script/fmt` covers Go only, so no formatter output applies to this file. Verified via `script/cibuild` (docker build runs `make check` with the pinned toolchain) — green. A direct local `make check` shows 21 pre-existing `goconst` lint findings that come from a newer local `golangci-lint` (v2.12.2 vs the pinned v2.10.1) and are unrelated to this change.

Related: #93 is being reframed under this policy.
Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #95
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-08-07 22:31:48 +02:00
7 changed files with 255 additions and 14 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@v2.12.2 RUN go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5
# 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,6 +17,26 @@ 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)
@@ -162,6 +182,27 @@ 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,10 +25,22 @@ confirm make check still passes.
# Completed Steps # Completed Steps
- 2026-08-07: golangci-lint bumped to v2.12.2 (pins in `Dockerfile` and - 2026-08-09: `http.Server` now sets all four socket-level timeouts
`script/bootstrap`), `.golangci.yml` replaced with the canonical v2 (`ReadTimeout` 15s, `ReadHeaderTimeout` 10s, `WriteTimeout` 75s,
config (settings moved under `linters.settings` so thresholds now `IdleTimeout` 120s) as named constants in `internal/server/server.go`,
apply); fixed all resulting `goconst`, `dupl`, and `lll` findings closing the slowloris / unreaped-keep-alive exposure required by
`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

@@ -0,0 +1,19 @@
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,8 +33,52 @@ 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
// readHeaderTimeout is the max duration for reading request headers. // Socket-level timeouts for the HTTP server.
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 {
@@ -76,16 +120,29 @@ 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 = &http.Server{ s.httpServer = newHTTPServer(listenAddr, s)
Addr: listenAddr,
Handler: s,
ReadHeaderTimeout: readHeaderTimeout,
}
s.log.Info("http server starting", "addr", listenAddr) s.log.Info("http server starting", "addr", listenAddr)

View File

@@ -0,0 +1,112 @@
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 refs the Dockerfile uses (never "latest"). # pinned commits 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@v2.12.2" GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5"
# 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"