Compare commits

2 Commits

Author SHA1 Message Date
clawbot
cc5836d1f9 Run all linting in Docker via Dockerfile.lint (closes #109)
All checks were successful
check / check (push) Successful in 2m53s
golangci-lint no longer runs on the host. script/lint builds
Dockerfile.lint, which copies the repo into the digest-pinned
golangci-lint image and lints as a build step, so a successful build is
a clean lint. The host binary shared one cache and one lock with every
other checkout on the machine, which produced findings attributed to
unrelated worktrees as well as unearned passes.

Three properties the wrapper has to get right:

- --no-cache-filter=lint forces the lint stage to re-execute. Without
  it an unchanged tree replays the layer and the build exits 0 in under
  a second having linted nothing. The deps stage stays cacheable.
- docker silently ignores --no-cache-filter when the stage name does
  not match, so the flag alone is a convention, not a guarantee: a
  rename or a typo restores the cached false green with no warning.
  script/lint therefore tees the build output and fails unless
  golangci-lint's own summary line ("N issues." / "N issues:") appears
  in it. No summary, no lint, whatever the exit code says.
- Both lint steps use RUN --network=none. golangci-lint config verify
  is documented as fetching its JSON schema over HTTPS; the pinned
  image resolves it with no network, and --network=none enforces that
  rather than trusting it. Verify is kept because golangci-lint run
  silently ignores config keys it does not recognize.

The main Dockerfile's lint stage now invokes golangci-lint directly
instead of `make lint`, which would otherwise need a docker daemon
inside the build.

golangci-lint installation is removed from script/bootstrap. Its curl
guard and its script/fetch-assets call are untouched.
2026-08-17 22:13:50 +00:00
bef9986542 Set fx.StopTimeout inside the container stop grace (closes #134)
All checks were successful
check / check (push) Successful in 3m3s
fx defaults to a 15s stop timeout and the Dockerfile sets no grace
override, so Docker SIGKILLed at 10s and the bounded shutdown #130 built
— including the log line that tells an operator a component is wedged —
was unreachable in the image this repo produces.

Sets fx.StopTimeout to 5s, and lowers the HTTP drain to 3s so a
full-length drain no longer exhausts the whole sequence budget and skip
every later hook, database close included. The Sentry flush, which runs
in the same hook and honours no context, is clamped to the remaining
stop budget less a 2s tail reserve, so a stalled flush drops Sentry
events rather than the database close.

Also fixes a latent coin flip in the shared stop-hook waiter, which
reported "shutdown timed out" about half the time for a component that
drained cleanly against an already-expired context.

Independently reviewed three times. The final reviewer derived a
stronger invariant than the implementation claims — the server hook's
absolute end is bounded at stopTimeout minus the reserve regardless of
drain length or of time consumed by preceding hooks — and confirmed the
guard's 10ms sweep cannot step over the maximum, since both breakpoints
land on its grid. Both Sentry probe arms, the docker stop demo and every
mutation were reproduced independently.

Known residual, filed separately: the HTTP drain itself is not clamped
by the reserve, so slow preceding hooks can still jointly exhaust the
budget. Demonstrated with a 2.2s sweeper delay.
2026-08-18 00:12:51 +02:00
12 changed files with 549 additions and 91 deletions

View File

@@ -19,9 +19,14 @@ RUN go mod download
# .dockerignore. # .dockerignore.
COPY . . COPY . .
# Run formatting check and linter # Run formatting check and linter. golangci-lint is invoked directly rather
# than through `make lint`: this stage is already the pinned linter image, and
# script/lint is a wrapper that builds Dockerfile.lint, so calling it here
# would need a docker daemon inside the build. Keep these steps in step with
# Dockerfile.lint, including --network=none (see its header for why).
RUN make fmt-check RUN make fmt-check
RUN make lint RUN --network=none golangci-lint config verify --config .golangci.yml
RUN --network=none golangci-lint run --config .golangci.yml ./...
# Build stage # Build stage
# golang:1.26.1-bookworm (Debian-based), 2026-03-17 # golang:1.26.1-bookworm (Debian-based), 2026-03-17

37
Dockerfile.lint Normal file
View File

@@ -0,0 +1,37 @@
# Lint-only image, built by script/lint. golangci-lint is never installed on
# the host: the repo is COPYed into the pinned image and linted as a build
# step, so a successful build IS a clean lint. This works even when the docker
# daemon is remote and bind mounts are impossible.
#
# script/lint passes --no-cache-filter=lint. Without it an unchanged tree
# replays the lint stage from cache and the build succeeds in under a second
# having run no linter at all. Do not drop that flag.
#
# The lint steps run with --network=none. `golangci-lint config verify` is
# documented as fetching its JSON schema over HTTPS, which would make linting
# depend on an unpinned remote artifact; this pinned image resolves the schema
# without any network, and --network=none enforces that rather than trusting
# it. It also proves no linter reaches out at analysis time. If a future image
# bump makes either step need the network, this build fails loudly instead of
# quietly acquiring an unpinned dependency.
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
# Using Debian-based image because mattn/go-sqlite3 (CGO) does not
# compile on Alpine musl (off64_t is a glibc type).
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps
WORKDIR /src
# Copy go mod files first for better layer caching. This stage is cacheable;
# only the lint stage below is forced to re-execute.
COPY go.mod go.sum ./
RUN go mod download
FROM deps AS lint
COPY . .
# `run` silently ignores config keys it does not recognize, so a typo would
# disable a setting without a word. `config verify` is what catches that.
RUN --network=none golangci-lint config verify --config .golangci.yml
RUN --network=none golangci-lint run --config .golangci.yml ./...

143
README.md
View File

@@ -12,14 +12,16 @@ with retry support, logging, and observability. Category: infrastructure
### Prerequisites ### Prerequisites
- Go 1.26.1+ (the version in `go.mod`) - Go 1.26.1+ (the version in `go.mod`)
- golangci-lint v2.12.2 (the version pinned in `script/bootstrap` and - Docker (for linting, for the test stage of the CI gate, and for
in the `Dockerfile`'s lint stage; `make bootstrap` installs it) containerized deployment)
- Docker (for containerized deployment, and for the lint and test
stages of the CI gate)
- `curl`, used by `script/fetch-assets` to download the third-party - `curl`, used by `script/fetch-assets` to download the third-party
browser assets, which are not committed (`make bootstrap` installs browser assets, which are not committed (`make bootstrap` installs
it if missing) it if missing)
golangci-lint is not a prerequisite and must not be installed on the
host: `script/bootstrap` does not install it, and `make lint` runs the
digest-pinned linter image via `Dockerfile.lint`.
### Quick Start ### Quick Start
```bash ```bash
@@ -27,9 +29,9 @@ with retry support, logging, and observability. Category: infrastructure
git clone https://git.eeqj.de/sneak/webhooker.git git clone https://git.eeqj.de/sneak/webhooker.git
cd webhooker cd webhooker
# Install Go dependencies, the pinned linter, and the third-party # Install Go dependencies and the third-party browser assets.
# browser assets. `make deps` alone is not enough: it only runs # `make deps` alone is not enough: it only runs go mod download/tidy,
# go mod download/tidy, and the checks below need the fetched assets. # and the checks below need the fetched assets.
make bootstrap make bootstrap
# Run all checks (test, lint, format check) # Run all checks (test, lint, format check)
@@ -52,7 +54,7 @@ make setup # Bootstrap + install git pre-commit hook
make assets # Fetch + verify third-party browser assets make assets # Fetch + verify third-party browser assets
make fmt # Format code (gofmt + goimports) make fmt # Format code (gofmt + goimports)
make fmt-check # Fail if gofmt would change anything (writes nothing) make fmt-check # Fail if gofmt would change anything (writes nothing)
make lint # Run golangci-lint make lint # Run golangci-lint in Docker (Dockerfile.lint)
make test # Run tests with race detection make test # Run tests with race detection
make check # test + lint + fmt-check (CI gate) make check # test + lint + fmt-check (CI gate)
make build # Build binary to bin/webhooker make build # Build binary to bin/webhooker
@@ -275,7 +277,7 @@ are inline commands with no script behind them. We provide:
- `script/fetch-assets` — download the third-party browser assets into - `script/fetch-assets` — download the third-party browser assets into
`static/`, verifying each against its pinned sha256 `static/`, verifying each against its pinned sha256
- `script/test` — run the test suite - `script/test` — run the test suite
- `script/lint` — run golangci-lint - `script/lint` — run golangci-lint in Docker (see Linting below)
- `script/fmt` — format all code (writes) - `script/fmt` — format all code (writes)
- `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
@@ -1145,8 +1147,6 @@ webhooker/
│ │ ├── archive_sweeper.go # Periodic pruning of idle archives │ │ ├── archive_sweeper.go # Periodic pruning of idle archives
│ │ ├── url_mask.go # Strips credentials from *url.Error │ │ ├── url_mask.go # Strips credentials from *url.Error
│ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport) │ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport)
│ ├── lifecycle/
│ │ └── lifecycle.go # Shared fx start/stop hook helpers
│ ├── handlers/ │ ├── handlers/
│ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering │ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering
│ │ ├── auth.go # Login, logout handlers │ │ ├── auth.go # Login, logout handlers
@@ -1158,6 +1158,8 @@ webhooker/
│ │ └── webhook.go # Webhook receiver handler │ │ └── webhook.go # Webhook receiver handler
│ ├── healthcheck/ │ ├── healthcheck/
│ │ └── healthcheck.go # Health check service (uptime, version) │ │ └── healthcheck.go # Health check service (uptime, version)
│ ├── lifecycle/
│ │ └── lifecycle.go # Shared stop-hook waiter, bounded by the stop context
│ ├── logger/ │ ├── logger/
│ │ └── logger.go # slog setup with TTY detection │ │ └── logger.go # slog setup with TTY detection
│ ├── middleware/ │ ├── middleware/
@@ -1183,6 +1185,7 @@ webhooker/
├── templates/ # Go HTML templates (base, login, sources, etc.) ├── templates/ # Go HTML templates (base, login, sources, etc.)
├── script/ # Scripts to Rule Them All entrypoints ├── script/ # Scripts to Rule Them All entrypoints
├── Dockerfile # Three stages: lint, test+build, Alpine runtime ├── Dockerfile # Three stages: lint, test+build, Alpine runtime
├── Dockerfile.lint # Lint-only image built by script/lint
├── Makefile # 10 of 16 targets shim script/; 6 are inline ├── Makefile # 10 of 16 targets shim script/; 6 are inline
├── go.mod / go.sum ├── go.mod / go.sum
└── .golangci.yml # Linter configuration └── .golangci.yml # Linter configuration
@@ -1316,6 +1319,102 @@ rather than global: **LoginRateLimit** on `/pages/login`,
- GORM soft deletes on every entity that carries `BaseModel`, which is - GORM soft deletes on every entity that carries `BaseModel`, which is
all of them but `Setting` (data preserved for audit) all of them but `Setting` (data preserved for audit)
### Shutdown
On SIGINT or SIGTERM, fx runs the registered stop hooks in reverse
dependency order under a **5 second budget** (`fx.StopTimeout` in
`cmd/webhooker/main.go`). That budget covers the whole sequence, not
each hook. The order, read off the fx stop-hook log:
1. `ArchiveSweeper`
2. `RetentionReaper`
3. `server` — the HTTP drain, bounded separately by
`server.ShutdownTimeout` (**3 seconds**), then a Sentry flush if
`SENTRY_DSN` is set
4. `delivery.Engine`
5. `healthcheck`
6. `WebhookDBManager`
7. the database close
The two components that can realistically hold the budget run
first: a retention sweep or an archive prune caught mid-tick each
waits on its `WaitGroup` bounded by the stop context, so a wedge
there consumes the 5 seconds before the HTTP server hook is ever
entered. The hooks after the server are microsecond-scale in normal
operation.
The HTTP drain budget is deliberately **shorter** than the sequence
budget. Were the two equal, a drain that used its whole budget would
exhaust the sequence budget at the instant it finished, and every
later hook — the delivery engine, the healthcheck, the webhook DB
manager and the database close — would be skipped in exactly the
case where the drain mattered. 3 seconds leaves 2 seconds
(`server.TailHookReserve`) for the tail, which is far more than the
microseconds it needs.
That reserve belongs to the tail hooks, not to the server hook, and
the Sentry flush is what could take it: it runs after the drain
**inside the same hook**, and `sentry.Flush` takes a bare duration
and honours no context, so an unreachable Sentry endpoint would add
its own timeout on top of a full-length drain and consume the whole
sequence budget by itself. It is therefore clamped to whatever is
left on the stop context minus the reserve, and skipped when that
leaves too little to be worth attempting — so a full-length drain
means Sentry events are dropped rather than the database close being
skipped.
This does not make the database close unconditional: a wedged
`ArchiveSweeper` or `RetentionReaper` still runs first and can
consume the whole budget on its own.
The value is chosen to sit inside the container stop grace period.
Docker's default `docker stop` grace is 10 seconds and the Dockerfile
sets no `STOPSIGNAL` or grace override, so the process must be gone
before that. fx's own default is 15 seconds, which is past the grace:
the container would be SIGKILLed (exit 137) before the bound could
fire, and nothing that depends on it — including the
`shutdown timed out, goroutines still running` error log that tells
an operator a component is wedged — would ever be reached.
Two operational consequences follow from bounding the sequence:
- **A wedged component aborts the rest of the shutdown.** fx checks
the stop context before each remaining hook and returns outright
once it has expired, skipping the hooks it has not reached. If the
first-stopped component consumes the whole budget, the later hooks
never run — **the database close among them**. SQLite is crash-safe,
so this is not corruption, but it is not a clean close either.
- **Lowering the grace below 5 seconds reintroduces the silent
truncation.** `docker stop --time`, Compose's `stop_grace_period`,
or Kubernetes' `terminationGracePeriodSeconds` set under 5 seconds
put SIGKILL back in front of the bound, and the process dies with
no shutdown diagnostics at all. Keep the deployment's grace above
the stop timeout.
### Linting
golangci-lint never runs on the host. `script/lint` builds
`Dockerfile.lint`, which copies the repo into the digest-pinned
golangci-lint image and lints as a build step, so a successful build is
a clean lint. A host binary would share one cache and one lock with
every other checkout on the machine, which has produced both invented
findings attributed to other worktrees and unearned passes.
Two properties are load-bearing:
- `script/lint` passes `--no-cache-filter=lint`. Without it an unchanged
tree replays the lint layer from cache and the build exits 0 in under
a second having linted nothing. The `deps` stage stays cacheable, so
module downloads are not repeated. Invalidation is scoped to the one
stage; never prune the shared build cache.
- Both lint steps use `RUN --network=none`. `golangci-lint config
verify` is documented as fetching its JSON schema over HTTPS, which
would be an unpinned remote dependency; the pinned image resolves the
schema without network access, and `--network=none` enforces that
instead of trusting it. Verify is worth keeping because
`golangci-lint run` silently ignores config keys it does not
recognize, so a typo would disable a setting with no warning.
### Docker ### Docker
The Dockerfile uses a three-stage build. Each stage is pinned by The Dockerfile uses a three-stage build. Each stage is pinned by
@@ -1324,7 +1423,8 @@ version is fixed independently of the compiler's:
1. **Lint stage** (`golangci/golangci-lint:v2.12.2`, Debian-based) — 1. **Lint stage** (`golangci/golangci-lint:v2.12.2`, Debian-based) —
installs `make`, downloads dependencies, copies the source, and runs installs `make`, downloads dependencies, copies the source, and runs
`make fmt-check` then `make lint`. `make fmt-check`, then `golangci-lint config verify` and
`golangci-lint run`, both with `--network=none`.
2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint 2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint
stage passing (it copies a file from it), runs `script/fetch-assets` stage passing (it copies a file from it), runs `script/fetch-assets`
to download and verify the third-party browser assets, then runs to download and verify the third-party browser assets, then runs
@@ -1335,6 +1435,10 @@ version is fixed independently of the compiler's:
runs as the non-root `webhooker` user (UID 1000), exposes port 8080, runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
and includes a health check against `/.well-known/healthcheck`. and includes a health check against `/.well-known/healthcheck`.
The lint stage invokes `golangci-lint` directly rather than `make lint`:
it is already the pinned linter image, and `make lint` builds
`Dockerfile.lint`, which would need a docker daemon inside this build.
Both check stages use Debian rather than Alpine because Both check stages use Debian rather than Alpine because
`gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO `gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO
and does not compile against musl. Only the final binary is statically and does not compile against musl. Only the final binary is statically
@@ -1342,13 +1446,10 @@ linked, which is what lets it run on the Alpine runtime image.
`script/cibuild` — `docker build .` — is the CI gate: the four check `script/cibuild` — `docker build .` — is the CI gate: the four check
targets run inside the image, so a build that succeeds is a repo that targets run inside the image, so a build that succeeds is a repo that
is formatted, linted, tested and compiled. Only `script/cibuild` and is formatted, linted, tested and compiled. `script/lint` also uses
`script/docker` involve Docker. `script/lint`, and therefore Docker (`Dockerfile.lint`, see Linting above), so `make lint` and
`make lint` and `make check`, run whatever `golangci-lint` is on the `make check` run the same pinned linter version the gate does; only
host, which can be a different version from the pinned one — so the `script/test` and `script/fmt-check` run on the host.
container is the authoritative lint result
([issue #109](https://git.eeqj.de/sneak/webhooker/issues/109) tracks
routing local linting through it as well).
#### CI gate honesty #### CI gate honesty
@@ -1361,8 +1462,8 @@ the hash of the last commit that touched the build context, so:
- Any commit that changes code (including a squash merge whose tree - Any commit that changes code (including a squash merge whose tree
matches an already-built branch) gets a new fingerprint, invalidates matches an already-built branch) gets a new fingerprint, invalidates
the `COPY . .` layer of both check stages, and really runs the `COPY . .` layer of both check stages, and really runs
`make fmt-check`, `make lint`, `make test`, and `make build`. A run `make fmt-check`, `golangci-lint`, `make test`, and `make build`. A
that reports success ran them. run that reports success ran them.
- A docs-only commit leaves the fingerprint unchanged — `.dockerignore` - A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
excludes `*.md`, `LICENSE` and `.editorconfig` from the context excludes `*.md`, `LICENSE` and `.editorconfig` from the context
anyway — so the image replays from cache and costs seconds. anyway — so the image replays from cache and costs seconds.

View File

@@ -2,6 +2,8 @@
package main package main
import ( import (
"time"
"go.uber.org/fx" "go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
@@ -15,6 +17,33 @@ import (
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
// stopTimeout bounds the whole fx stop sequence, not each hook.
//
// fx defaults to 15s, which is longer than Docker's 10s default
// stop grace: the container would be SIGKILLed before the bound
// could fire, so nothing bounded by it would ever be observed.
// 5s leaves headroom inside that grace for signal delivery and
// process exit; the observed wedge case already exits at ~5.3s,
// so a larger bound would trade a rare skipped database close for
// a more common hard kill.
//
// The server's stop hook must fit inside it with room to spare: a
// hook that used the whole budget would exhaust it at that instant,
// and fx would skip every hook after the server — the delivery
// engine, the healthcheck, the webhook DB manager and the database
// close. That hook is the 3s HTTP drain plus the Sentry flush that
// follows it in the same hook, so the flush is clamped to the stop
// context's remaining time less server.TailHookReserve rather than
// running for its own fixed 2s; the reserve is what the tail hooks
// live on, and they are microsecond-scale in normal operation.
// TestStopTimeout_LeavesHeadroomForTailHooks pins the arithmetic
// across every drain length.
//
// This does not make the database close unconditional: the
// ArchiveSweeper and RetentionReaper hooks run before the server
// and can still consume the whole budget on their own.
const stopTimeout = 5 * time.Second
// Build-time variables set via -ldflags. // Build-time variables set via -ldflags.
// //
//nolint:gochecknoglobals // Build-time variables injected by the linker. //nolint:gochecknoglobals // Build-time variables injected by the linker.
@@ -27,7 +56,14 @@ func main() {
globals.Appname = appname globals.Appname = appname
globals.Version = version globals.Version = version
fx.New( newApp().Run()
}
// newApp builds the application graph. It is separate from main so
// a test can assert the options it carries.
func newApp() *fx.App {
return fx.New(
fx.StopTimeout(stopTimeout),
fx.Provide( fx.Provide(
globals.New, globals.New,
logger.New, logger.New,
@@ -60,5 +96,5 @@ func main() {
) { ) {
}, },
), ),
).Run() )
} }

View File

@@ -0,0 +1,75 @@
package main
import (
"testing"
"time"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/server"
)
// dockerStopGrace is Docker's default `docker stop` grace period.
// The Dockerfile sets no STOPSIGNAL or grace override, so this is
// the deadline the container is actually held to, and the fx stop
// timeout has to fit inside it with room for signal delivery and
// process exit.
const dockerStopGrace = 10 * time.Second
// TestNewApp_StopTimeout pins the fx stop timeout. Without the
// explicit fx.StopTimeout option the app reads fx's 15s
// DefaultTimeout, which exceeds dockerStopGrace: the container is
// SIGKILLed before the bound fires and every shutdown hook bounded
// by it — including the operator-facing timeout log — becomes
// unreachable in the image this repo produces.
//
// fx.New applies options before it executes invokes, so the timeout
// is set whether or not the graph itself can be constructed here.
func TestNewApp_StopTimeout(t *testing.T) {
t.Setenv("DATA_DIR", t.TempDir())
got := newApp().StopTimeout()
require.Equal(t, stopTimeout, got)
require.Less(t, got, dockerStopGrace)
}
// tailHeadroom is the slack the fx stop budget must keep beyond the
// server stop hook. The hooks that run after the server — the
// delivery engine, the healthcheck, the webhook DB manager and the
// database close — are microsecond-scale in normal operation, so
// this is generous for them.
const tailHeadroom = 2 * time.Second
// TestStopTimeout_LeavesHeadroomForTailHooks pins the relationship
// between the server's stop hook and the fx stop budget. fx bounds
// the whole stop sequence, and returns without running its
// remaining hooks once the stop context has expired. If the hook
// could use the entire budget, every later hook — the database close
// included — would be skipped in exactly the case where the drain
// mattered.
//
// The hook is not just the HTTP drain: a Sentry flush follows it in
// the same hook, and sentry.Flush honours no context, so both halves
// have to be counted. The sweep walks every drain length the hook
// can produce, since a shorter drain leaves the flush more room and
// the worst case is not necessarily at either extreme.
//
// Shrinking either budget, or unbounding the flush again, must fail
// here rather than silently recreating a hook that swallows the
// whole sequence.
func TestStopTimeout_LeavesHeadroomForTailHooks(t *testing.T) {
t.Parallel()
require.Less(t, server.ShutdownTimeout, stopTimeout)
const step = 10 * time.Millisecond
for drain := time.Duration(0); drain <= server.ShutdownTimeout; drain += step {
hook := drain + server.SentryFlushBudget(stopTimeout-drain)
require.LessOrEqual(
t, hook+tailHeadroom, stopTimeout,
"a %s drain leaves the tail hooks short", drain,
)
}
}

View File

@@ -0,0 +1,21 @@
package lifecycle
import (
"context"
"log/slog"
)
// WaitDone exposes waitDone to the external test package. Only the
// unexported waiter can be handed a channel that is already closed
// before the call, which is the state the preamble exists for;
// through WaitForShutdown the waiter goroutine may or may not have
// closed the channel yet, so the case is not reachable
// deterministically from outside.
func WaitDone(
ctx context.Context,
log *slog.Logger,
component string,
done <-chan struct{},
) error {
return waitDone(ctx, log, component, done)
}

View File

@@ -38,6 +38,29 @@ func WaitForShutdown(
wg.Wait() wg.Wait()
}() }()
return waitDone(ctx, log, component, done)
}
// waitDone waits for done to close, bounded by ctx.
//
// The non-blocking preamble is load-bearing. When the component has
// already drained and ctx has already expired, both cases of the
// bounded select are ready and Go picks between them uniformly at
// random, so a clean shutdown would be reported as a timeout about
// half the time. Draining wins: the goroutines are gone, and there
// is nothing left for the operator to act on.
func waitDone(
ctx context.Context,
log *slog.Logger,
component string,
done <-chan struct{},
) error {
select {
case <-done:
return nil
default:
}
select { select {
case <-done: case <-done:
return nil return nil

View File

@@ -37,6 +37,57 @@ func TestWaitForShutdown_DrainedGroup(t *testing.T) {
) )
} }
// racePasses is how many times the both-cases-ready race is run.
// Without the preamble each pass is an independent coin flip, so
// the probability of the whole loop passing by luck is 2^-N: at
// this N the test is deterministic in practice, and it involves no
// wall-clock waiting at all.
const racePasses = 1000
// TestWaitDone_DrainedBeforeExpiredContext covers the case where a
// component drained cleanly but the stop context had already
// expired. Both select cases are ready, and Go chooses among ready
// cases uniformly at random, so the drained case must be settled by
// the preamble before the bounded select ever runs.
func TestWaitDone_DrainedBeforeExpiredContext(t *testing.T) {
t.Parallel()
done := make(chan struct{})
close(done)
ctx, cancel := context.WithCancel(context.Background())
cancel()
for pass := range racePasses {
require.NoErrorf(
t,
lifecycle.WaitDone(
ctx, discardLogger(), "test component", done,
),
"pass %d reported a timeout for a drained component",
pass,
)
}
}
// TestWaitDone_ExpiredContext pins the other side of the preamble:
// an expired context with a component that has not drained is still
// a timeout.
func TestWaitDone_ExpiredContext(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithCancel(context.Background())
cancel()
err := lifecycle.WaitDone(
ctx, discardLogger(), "test component",
make(chan struct{}),
)
require.ErrorIs(t, err, context.Canceled)
require.ErrorContains(t, err, "test component")
}
func TestWaitForShutdown_ContextExpires(t *testing.T) { func TestWaitForShutdown_ContextExpires(t *testing.T) {
t.Parallel() t.Parallel()

View File

@@ -24,15 +24,48 @@ import (
) )
const ( const (
// shutdownTimeout is the maximum time to wait for the HTTP // ShutdownTimeout is the maximum time to wait for the HTTP
// server to finish in-flight requests during shutdown. // server to finish in-flight requests during shutdown.
shutdownTimeout = 5 * time.Second //
// It must stay strictly below the fx stop timeout in
// cmd/webhooker, which bounds the whole stop sequence: a drain
// that used the entire sequence budget would leave nothing for
// the hooks that run after the server, including the database
// close. It is exported so that relationship can be tested.
ShutdownTimeout = 3 * time.Second
// sentryFlushTimeout is the maximum time to wait for Sentry // TailHookReserve is the share of the fx stop budget this hook
// to flush pending events during shutdown. // refuses to spend, leaving it for the hooks that run after the
// server: the delivery engine, the healthcheck, the webhook DB
// manager and the database close.
TailHookReserve = 2 * time.Second
// sentryFlushTimeout is the longest wait for Sentry to flush
// pending events during shutdown, before the remaining stop
// budget is taken into account.
sentryFlushTimeout = 2 * time.Second sentryFlushTimeout = 2 * time.Second
// minSentryFlush is the shortest flush worth attempting. Below
// it the remaining budget goes to the tail hooks instead.
minSentryFlush = 250 * time.Millisecond
) )
// SentryFlushBudget reports how long the Sentry flush may run when
// remaining is the time left on the fx stop context after the HTTP
// drain. sentry.Flush takes a bare duration and honours no context,
// so this clamp is the only thing keeping a stalled flush from
// spending the tail hooks' share of the budget on top of a
// full-length drain. TailHookReserve is held back, and anything
// under minSentryFlush is skipped rather than attempted uselessly.
func SentryFlushBudget(remaining time.Duration) time.Duration {
budget := min(remaining-TailHookReserve, sentryFlushTimeout)
if budget < minSentryFlush {
return 0
}
return budget
}
//nolint:revive // ServerParams is a standard fx naming convention. //nolint:revive // ServerParams is a standard fx naming convention.
type ServerParams struct { type ServerParams struct {
fx.In fx.In
@@ -164,7 +197,7 @@ func (s *Server) cleanShutdown(ctx context.Context) {
s.exitCode = 0 s.exitCode = 0
ctxShutdown, shutdownCancel := context.WithTimeout( ctxShutdown, shutdownCancel := context.WithTimeout(
ctx, shutdownTimeout, ctx, ShutdownTimeout,
) )
defer shutdownCancel() defer shutdownCancel()
@@ -178,10 +211,31 @@ func (s *Server) cleanShutdown(ctx context.Context) {
s.cleanupForExit() s.cleanupForExit()
if s.sentryEnabled { if s.sentryEnabled {
sentry.Flush(sentryFlushTimeout) s.flushSentry(ctx)
} }
} }
// flushSentry drains Sentry's queue inside what is left of the fx
// stop budget. A context carrying no deadline — a caller outside the
// fx lifecycle — gets the full timeout.
func (s *Server) flushSentry(ctx context.Context) {
flush := sentryFlushTimeout
if deadline, ok := ctx.Deadline(); ok {
flush = SentryFlushBudget(time.Until(deadline))
}
if flush <= 0 {
s.log.Warn(
"skipping sentry flush, stop budget exhausted",
)
return
}
sentry.Flush(flush)
}
func (s *Server) configure() { func (s *Server) configure() {
// identify ourselves in the logs // identify ourselves in the logs
s.params.Logger.Identify() s.params.Logger.Identify()

View File

@@ -0,0 +1,59 @@
package server_test
import (
"testing"
"time"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/server"
)
// TestSentryFlushBudget covers the clamp that keeps the Sentry flush
// from spending the tail hooks' share of the fx stop budget.
// sentry.Flush ignores the stop context, so without the clamp a
// stalled flush adds its whole timeout on top of the HTTP drain.
func TestSentryFlushBudget(t *testing.T) {
t.Parallel()
tests := []struct {
name string
remaining time.Duration
want time.Duration
}{
{
name: "full drain leaves only the reserve",
remaining: server.TailHookReserve,
want: 0,
},
{
name: "expired budget",
remaining: -time.Second,
want: 0,
},
{
name: "sliver above the reserve is not worth it",
remaining: server.TailHookReserve + 10*time.Millisecond,
want: 0,
},
{
name: "partial flush when some room is left",
remaining: server.TailHookReserve + time.Second,
want: time.Second,
},
{
name: "capped at the nominal timeout",
remaining: time.Hour,
want: 2 * time.Second,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(
t, tt.want, server.SentryFlushBudget(tt.remaining),
)
})
}
}

View File

@@ -3,20 +3,14 @@
# this repo. Idempotent: every install is guarded by a check so already # this repo. Idempotent: every install is guarded by a check so already
# 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 (not git, # or apk (detected in that order); assumes NOTHING is present (not git,
# make, or go). golangci-lint is packaged in nix, brew, and apk; on apt # make, or go). golangci-lint is deliberately not installed: linting runs
# it is installed from a hash-verified GitHub release archive (never # only in docker, via script/lint and Dockerfile.lint. Finishes by running
# curl | sh). Finishes by running script/fetch-assets, which installs the # script/fetch-assets, which installs the hash-pinned third-party browser
# hash-pinned third-party browser assets the repo does not commit. # assets the repo does not commit.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Pinned versions, 2026-08-07. Never "latest"; exact versions only.
GOLANGCI_LINT_VERSION="2.12.2"
# sha256 of golangci-lint-2.12.2-linux-<arch>.tar.gz release archives
GOLANGCI_LINT_SHA256_AMD64="8df580d2670fed8fa984aac0507099af8df275e665215f5c7a2ae3943893a553"
GOLANGCI_LINT_SHA256_ARM64="44cd40a8c76c86755375adfeea52cfd3533cb43d7bd647771e0ae065e166df3a"
PKGMGR="" PKGMGR=""
SUDO="" SUDO=""
@@ -57,52 +51,6 @@ missing() {
! command -v "$1" >/dev/null 2>&1 ! command -v "$1" >/dev/null 2>&1
} }
# verify_sha256 <file> <expected-hash>
verify_sha256() {
if command -v sha256sum >/dev/null 2>&1; then
actual="$(sha256sum "$1" | cut -d' ' -f1)"
else
actual="$(shasum -a 256 "$1" | cut -d' ' -f1)"
fi
if [ "$actual" != "$2" ]; then
echo "bootstrap: sha256 mismatch for $1" >&2
echo " expected: $2" >&2
echo " actual: $actual" >&2
exit 1
fi
}
# apt has no golangci-lint package: install a pinned release archive
# from GitHub, verified by hardcoded sha256 (never curl | sh).
install_golangci_lint_release() {
case "$(uname -m)" in
x86_64) goarch="amd64"; sha="$GOLANGCI_LINT_SHA256_AMD64" ;;
aarch64|arm64) goarch="arm64"; sha="$GOLANGCI_LINT_SHA256_ARM64" ;;
*)
echo "bootstrap: unsupported architecture $(uname -m)" >&2
exit 1
;;
esac
if missing curl; then pkg_install curl curl curl curl; fi
name="golangci-lint-${GOLANGCI_LINT_VERSION}-linux-${goarch}"
tmp="$(mktemp -d)"
curl -fsSL -o "$tmp/$name.tar.gz" \
"https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/${name}.tar.gz"
verify_sha256 "$tmp/$name.tar.gz" "$sha"
tar -xzf "$tmp/$name.tar.gz" -C "$tmp"
$SUDO install -m 0755 "$tmp/$name/golangci-lint" /usr/local/bin/golangci-lint
rm -rf "$tmp"
}
ensure_golangci_lint() {
if ! missing golangci-lint; then return 0; fi
detect_pkgmgr
case "$PKGMGR" in
apt) install_golangci_lint_release ;;
*) pkg_install golangci-lint golangci-lint golangci-lint golangci-lint ;;
esac
}
main() { main() {
cd "$ROOT" cd "$ROOT"
@@ -110,9 +58,14 @@ main() {
if missing git; then pkg_install git git git git; fi if missing git; then pkg_install git git git git; fi
if missing make; then pkg_install gnumake make make make; fi if missing make; then pkg_install gnumake make make make; fi
# Go toolchain and linter # Go toolchain
if missing go; then pkg_install go golang go go; fi if missing go; then pkg_install go golang go go; fi
ensure_golangci_lint
# Not installed here: docker is platform-specific and out of scope for a
# package-manager bootstrap, but script/lint needs it.
if missing docker; then
echo "bootstrap: docker not found; script/lint requires it" >&2
fi
go mod download go mod download

View File

@@ -1,12 +1,55 @@
#!/bin/sh #!/bin/sh
# script/lint: run the linter. # script/lint: run the linter. golangci-lint is never installed locally: it
# runs via docker only, one way, everywhere — script/lint builds
# Dockerfile.lint, which COPYs the repo into the pinned golangci-lint image
# and lints as a build step. This works even when the docker daemon is remote
# and bind mounts are impossible, and it removes the host linter's shared
# cache, which has attributed other checkouts' findings to this one.
#
# --no-cache-filter=lint forces the lint stage to re-execute on every run; a
# cached lint stage exits 0 in under a second having linted nothing. The deps
# stage keeps its cache, so module downloads are not repeated.
# --progress=plain keeps the linter's own output visible on success, so a
# passing run shows the issue count rather than nothing.
# --output=type=cacheonly leaves no image behind to clean up.
#
# docker silently ignores --no-cache-filter for a stage name that does not
# match, so a rename or a typo would restore the cached false green with no
# warning and a fast exit 0. The flag is therefore not trusted: the build
# output is teed to a log and a run is only a pass if golangci-lint's own
# summary line ("N issues." / "N issues:") is in it. No summary, no lint,
# whatever the exit code says.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
golangci-lint run --config .golangci.yml ./...
log="$(mktemp -t webhooker-lint.XXXXXXXX)"
rcfile="$(mktemp -t webhooker-lint-rc.XXXXXXXX)"
trap 'rm -f "$log" "$rcfile"' EXIT INT TERM
# The pipeline's status is tee's, and POSIX sh has no pipefail, so the
# build's status travels via a file. Output still streams live.
{
docker build \
-f Dockerfile.lint \
--no-cache-filter=lint \
--progress=plain \
--output=type=cacheonly \
. 2>&1 && echo 0 >"$rcfile" || echo $? >"$rcfile"
} | tee "$log" >&2
rc="$(cat "$rcfile")"
[ "$rc" -eq 0 ] || exit "$rc"
if ! grep -qE '[0-9]+ issues[.:]' "$log"; then
echo "script/lint: golangci-lint printed no summary line; the linter" >&2
echo " did not run. Check that the stage named in --no-cache-filter" >&2
echo " still matches a stage in Dockerfile.lint." >&2
exit 1
fi
} }
main "$@" main "$@"