sneak
4e6f6ce1d3
All checks were successful
check / check (push) Successful in 3m41s
The plaintext listener bound `:PORT`, so it answered on every interface with no way to say otherwise. That published the admin UI and the unauthenticated receiver in cleartext beside whatever TLS proxy was in front of them, reachable from any host that could route to the machine. BIND_ADDRESS now selects the address. The binary defaults to 127.0.0.1, which is the safe answer for a bare host: reaching webhooker from elsewhere becomes a deliberate act. The image sets 0.0.0.0, which is the correct answer inside a container, where the network namespace is already the boundary and exposure is decided by the publish flag instead — so `-p 127.0.0.1:8080:8080` is what the README shows. Existing container deployments are unaffected. Only IP address literals are accepted: hostnames, host:port and CIDR blocks abort startup naming the variable and the value, and a literal that is not an address of this host fails at listen and exits non-zero. The http.Server is now built in New rather than in the serving goroutine, and sentryEnabled is atomic. Both fields were written by the serving goroutine and read by the fx stop hook with nothing ordering them, and the OnStart hook returns before that goroutine has necessarily run: cleanShutdown could dereference a nil httpServer on an early SIGTERM, and both reads raced. No test started and stopped the server, so nothing observed it. Closes #226. README gains a "Deployment behind a reverse proxy" section: a working nginx server block, and the five things that are silent when wrong — bind or firewall the app port, WEBHOOKER_ENVIRONMENT=prod, TRUSTED_PROXIES, Host as $http_host rather than $host, and keeping the proxy's access log because webhooker's own records only the proxy.
92 lines
3.2 KiB
Go
92 lines
3.2 KiB
Go
package server_test
|
|
|
|
import (
|
|
"context"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
"go.uber.org/fx"
|
|
"sneak.berlin/go/webhooker/internal/globals"
|
|
"sneak.berlin/go/webhooker/internal/server"
|
|
)
|
|
|
|
// earlyStopIterations is how many start/stop cycles the race test
|
|
// runs. The window it aims at is the gap between the OnStart hook
|
|
// returning and the serving goroutine reaching its first field
|
|
// access, which is microseconds wide. The race detector reports an
|
|
// unsynchronised pair whenever it observes one, but it has to observe
|
|
// one, so a single cycle can miss purely on scheduling. Repetition
|
|
// makes the observation reliable; the collaborators are built once,
|
|
// so the cycles themselves are cheap.
|
|
const earlyStopIterations = 25
|
|
|
|
// TestEarlyShutdown_NoPanicAndNoRace stops the application
|
|
// immediately after starting it, before the serving goroutine has
|
|
// necessarily run at all.
|
|
//
|
|
// Two defects live in that window. The OnStart hook returns as soon
|
|
// as it has spawned the serving goroutine, so fx runs the stop
|
|
// sequence against a Server whose serving goroutine may not have
|
|
// executed a single line. cleanShutdown called Shutdown on an
|
|
// httpServer that goroutine was supposed to assign, which was a nil
|
|
// dereference on an early SIGTERM; and it read httpServer and
|
|
// sentryEnabled with nothing ordering those reads against the
|
|
// goroutine's writes, which is a data race that only surfaces once
|
|
// something both starts and stops the server. Nothing did before this
|
|
// test: the listen-failure test never binds, and the router tests
|
|
// bypass the lifecycle entirely.
|
|
//
|
|
// httpServer is now built in New, on the constructing goroutine, so
|
|
// it is written before any hook exists and can never be nil.
|
|
// sentryEnabled is atomic. This test is what catches either one
|
|
// coming back — under -race, which is how the suite runs.
|
|
func TestEarlyShutdown_NoPanicAndNoRace(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Built once: the collaborators are not what is under test, and
|
|
// standing up a database per iteration would make repetition too
|
|
// expensive to be worth having.
|
|
env := newTestEnv(t)
|
|
env.cfg.BindAddress = loopbackV4
|
|
|
|
for range earlyStopIterations {
|
|
requireStartStopIsClean(t, env)
|
|
}
|
|
}
|
|
|
|
// requireStartStopIsClean runs one start/stop cycle with no wait in
|
|
// between, failing the test if either half errors.
|
|
//
|
|
// Each cycle gets a fresh fx app, so the Server under test is
|
|
// constructed anew every time — that construction is where the
|
|
// httpServer write now happens, and reusing one Server would test it
|
|
// only once.
|
|
func requireStartStopIsClean(t *testing.T, env *testEnv) {
|
|
t.Helper()
|
|
|
|
env.cfg.Port = freePort(t)
|
|
|
|
app := fx.New(
|
|
fx.NopLogger,
|
|
fx.Supply(env.log, env.cfg, env.mw, env.hnd),
|
|
fx.Provide(globals.New, server.New),
|
|
fx.Invoke(func(*server.Server) {}),
|
|
)
|
|
|
|
startCtx, cancelStart := context.WithTimeout(
|
|
context.Background(), lifecycleTimeout,
|
|
)
|
|
defer cancelStart()
|
|
|
|
require.NoError(t, app.Start(startCtx))
|
|
|
|
// No sleep and no readiness wait: stopping while the serving
|
|
// goroutine is still in flight is the whole point.
|
|
stopCtx, cancelStop := context.WithTimeout(
|
|
context.Background(), lifecycleTimeout,
|
|
)
|
|
defer cancelStop()
|
|
|
|
require.NoError(t, app.Stop(stopCtx))
|
|
}
|