Compare commits
1 Commits
fix/99-ser
...
fix/121-li
| Author | SHA1 | Date | |
|---|---|---|---|
| 520ce79709 |
@@ -1,5 +1,6 @@
|
|||||||
.git/
|
.git/
|
||||||
bin/
|
bin/
|
||||||
|
.lint-cache/
|
||||||
*.md
|
*.md
|
||||||
LICENSE
|
LICENSE
|
||||||
.editorconfig
|
.editorconfig
|
||||||
|
|||||||
1
.gitignore
vendored
1
.gitignore
vendored
@@ -1,6 +1,7 @@
|
|||||||
bin/
|
bin/
|
||||||
vendor/
|
vendor/
|
||||||
data/
|
data/
|
||||||
|
.lint-cache/
|
||||||
.env
|
.env
|
||||||
*.exe
|
*.exe
|
||||||
/dnswatcher
|
/dnswatcher
|
||||||
|
|||||||
26
README.md
26
README.md
@@ -182,27 +182,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
|
||||||
@@ -408,7 +387,10 @@ them. We provide:
|
|||||||
- `script/projectname` — print the project name (used for the Docker
|
- `script/projectname` — print the project name (used for the Docker
|
||||||
image tag)
|
image tag)
|
||||||
- `script/test` — run the test suite (race detector, coverage)
|
- `script/test` — run the test suite (race detector, coverage)
|
||||||
- `script/lint` — run golangci-lint
|
- `script/lint` — run golangci-lint, with its cache and its lock file
|
||||||
|
isolated to this checkout (under the git-ignored `.lint-cache/`) so
|
||||||
|
concurrent checkouts on one host cannot share cache entries or
|
||||||
|
contend on a single lock
|
||||||
- `script/fmt` — format all code (gofmt -s, goimports)
|
- `script/fmt` — format all code (gofmt -s, goimports)
|
||||||
- `script/fmt-check` — check formatting (read-only)
|
- `script/fmt-check` — check formatting (read-only)
|
||||||
- `script/check` — run test, lint, and fmt-check
|
- `script/check` — run test, lint, and fmt-check
|
||||||
|
|||||||
17
TODO.md
17
TODO.md
@@ -25,14 +25,15 @@ confirm make check still passes.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
- 2026-08-09: `http.Server` now sets all four socket-level timeouts
|
- 2026-08-09: `script/lint` now isolates golangci-lint's per-user global
|
||||||
(`ReadTimeout` 15s, `ReadHeaderTimeout` 10s, `WriteTimeout` 75s,
|
state to the checkout (#121): `GOLANGCI_LINT_CACHE` and `TMPDIR` are
|
||||||
`IdleTimeout` 120s) as named constants in `internal/server/server.go`,
|
both pointed at the git-ignored, Docker-ignored `.lint-cache/`. The
|
||||||
closing the slowloris / unreaped-keep-alive exposure required by
|
cache fixes cross-contamination; `TMPDIR` is what moves the lock,
|
||||||
`REPO_POLICIES.md` before 1.0; `WriteTimeout` is deliberately greater
|
which lives at `$TMPDIR/golangci-lint.lock` and not in the cache
|
||||||
than the 60s `chimw.Timeout` handler budget so that budget stays
|
directory. Reproduced both failure modes on the unfixed script (10 of
|
||||||
reachable, and tests in `internal/server` pin both the non-zero
|
12 concurrent runs void with `parallel golangci-lint is running`; 11
|
||||||
values and that relationship (#99)
|
of 12 reporting another checkout's paths) and both are gone at 20-way
|
||||||
|
concurrency after the fix
|
||||||
- 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
|
- 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
|
||||||
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
|
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
|
||||||
org-standard v2-schema config used across the org's repos
|
org-standard v2-schema config used across the org's repos
|
||||||
|
|||||||
@@ -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
|
|
||||||
@@ -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)
|
||||||
|
|
||||||
|
|||||||
@@ -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")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
29
script/lint
29
script/lint
@@ -1,11 +1,40 @@
|
|||||||
#!/bin/sh
|
#!/bin/sh
|
||||||
# script/lint: run the linter.
|
# script/lint: run the linter.
|
||||||
|
#
|
||||||
|
# golangci-lint keeps two pieces of per-user global state, and both of
|
||||||
|
# them break when several checkouts on one host lint concurrently:
|
||||||
|
#
|
||||||
|
# 1. Its analysis cache (GOLANGCI_LINT_CACHE, default
|
||||||
|
# ~/.cache/golangci-lint). Entries are keyed by content, not by
|
||||||
|
# checkout, so a hit written by another checkout is replayed
|
||||||
|
# verbatim - including that checkout's file paths. The run then
|
||||||
|
# reports findings for files it never linted.
|
||||||
|
#
|
||||||
|
# 2. Its "one runner at a time" lock, which does NOT live in the
|
||||||
|
# cache directory: golangci-lint locks
|
||||||
|
# $(os.TempDir())/golangci-lint.lock, i.e.
|
||||||
|
# "$TMPDIR"/golangci-lint.lock (pkg/commands/run.go,
|
||||||
|
# acquireFileLock). It waits 5s, then aborts with "parallel
|
||||||
|
# golangci-lint is running" - a non-result that looks like a lint
|
||||||
|
# failure. Setting GOLANGCI_LINT_CACHE alone does not move it.
|
||||||
|
#
|
||||||
|
# So both are pinned under the checkout root. The cache is never shared,
|
||||||
|
# and TMPDIR makes the lock file per-checkout, which keeps the lock
|
||||||
|
# doing its actual job (serialising runs that share one cache) at the
|
||||||
|
# right scope. .lint-cache/ is git-ignored and Docker-ignored, and
|
||||||
|
# caching still works: it persists across runs in this checkout.
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
|
|
||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
|
|
||||||
|
GOLANGCI_LINT_CACHE="$ROOT/.lint-cache/cache"
|
||||||
|
TMPDIR="$ROOT/.lint-cache/tmp"
|
||||||
|
export GOLANGCI_LINT_CACHE TMPDIR
|
||||||
|
mkdir -p "$GOLANGCI_LINT_CACHE" "$TMPDIR"
|
||||||
|
|
||||||
golangci-lint run --config .golangci.yml ./...
|
golangci-lint run --config .golangci.yml ./...
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user