Compare commits
1 Commits
fix/99-ser
...
9054db8d86
| Author | SHA1 | Date | |
|---|---|---|---|
| 9054db8d86 |
21
README.md
21
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
|
||||||
|
|||||||
8
TODO.md
8
TODO.md
@@ -25,14 +25,6 @@ confirm make check still passes.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
- 2026-08-09: `http.Server` now sets all four socket-level timeouts
|
|
||||||
(`ReadTimeout` 15s, `ReadHeaderTimeout` 10s, `WriteTimeout` 75s,
|
|
||||||
`IdleTimeout` 120s) as named constants in `internal/server/server.go`,
|
|
||||||
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
|
- 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")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Reference in New Issue
Block a user