Compare commits
1 Commits
clawbot/do
...
4e6f6ce1d3
| Author | SHA1 | Date | |
|---|---|---|---|
| 4e6f6ce1d3 |
12
Dockerfile
12
Dockerfile
@@ -109,6 +109,18 @@ USER webhooker
|
||||
|
||||
EXPOSE 8080
|
||||
|
||||
# The binary defaults BIND_ADDRESS to 127.0.0.1, which is right for a
|
||||
# bare host: the cleartext listener serves the admin UI and the
|
||||
# unauthenticated receiver, so it must not appear on every interface
|
||||
# of a machine that configured nothing. A container is the other case.
|
||||
# Its network namespace is already the isolation boundary, so binding
|
||||
# every address inside it exposes nothing; what decides exposure is
|
||||
# the publish flag, and `-p 127.0.0.1:8080:8080` is the operator's
|
||||
# control there. Shipping the image on loopback would buy no security
|
||||
# and would make the process unreachable through its own published
|
||||
# port.
|
||||
ENV BIND_ADDRESS=0.0.0.0
|
||||
|
||||
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
|
||||
CMD wget --no-verbose --tries=1 --spider http://localhost:8080/.well-known/healthcheck || exit 1
|
||||
|
||||
|
||||
271
README.md
271
README.md
@@ -119,6 +119,7 @@ TTY detection, and security headers are always applied.
|
||||
| ----------------------- | ----------------------------------- | -------- |
|
||||
| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod` | `dev` |
|
||||
| `PORT` | HTTP listen port | `8080` |
|
||||
| `BIND_ADDRESS` | IP address the HTTP listener binds. Loopback by default, so the cleartext listener is not published on every interface. The Docker image ships `0.0.0.0` instead. See [Bind address](#bind-address) | `127.0.0.1` (image: `0.0.0.0`) |
|
||||
| `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` |
|
||||
| `DEBUG` | Enable debug logging | `false` |
|
||||
| `MAINTENANCE_MODE` | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false` |
|
||||
@@ -240,6 +241,68 @@ A set but unparseable value aborts startup. When the list is non-empty
|
||||
webhooker logs it at startup, blocks and all, so the hole is visible in
|
||||
the log of any deployment that has one.
|
||||
|
||||
#### Bind address
|
||||
|
||||
`BIND_ADDRESS` is the IP address the HTTP listener binds. The binary
|
||||
defaults to `127.0.0.1`, so a bare webhooker is reachable only from the
|
||||
host it runs on. The Docker image ships `ENV BIND_ADDRESS=0.0.0.0`
|
||||
instead — see below for why the two differ.
|
||||
|
||||
That listener speaks **cleartext**, and it serves both the admin UI and
|
||||
the unauthenticated webhook receiver. webhooker terminates no TLS
|
||||
itself; a production deployment puts a reverse proxy in front of it
|
||||
(see
|
||||
[Deployment behind a reverse proxy](#deployment-behind-a-reverse-proxy)),
|
||||
and the proxy reaches it over loopback. A default that bound every
|
||||
interface would leave that cleartext port answering the internet
|
||||
alongside the proxy — the admin login form and the receiver, in the
|
||||
clear, on a port nobody chose to publish. Reaching webhooker from
|
||||
another host is therefore something you configure, not something you
|
||||
get by default.
|
||||
|
||||
**In a container the answer is `0.0.0.0`, which is why the image ships
|
||||
that.** A container's network namespace is already the boundary the
|
||||
loopback default is reaching for: nothing outside the container gets to
|
||||
`0.0.0.0:8080` because of the namespace, whatever the process bound.
|
||||
Exposure is decided at the publish flag instead — `-p
|
||||
127.0.0.1:8080:8080` rather than `-p 8080:8080` — which is the
|
||||
operator's to choose and is what
|
||||
[Running with Docker](#running-with-docker) shows. A loopback bind
|
||||
inside a container buys nothing and makes the process unreachable
|
||||
through its own published port.
|
||||
|
||||
The value must be an IP address literal:
|
||||
|
||||
- `127.0.0.1` — loopback only (the binary's default). Use this with a
|
||||
reverse proxy on the same host.
|
||||
- `0.0.0.0` — every IPv4 address. The image's default; on a bare host,
|
||||
only behind a firewall on the port.
|
||||
- `::` — every address, IPv6 and (on Linux, with the default
|
||||
`net.ipv6.bindv6only=0`) IPv4 as well.
|
||||
- A specific address such as `10.0.0.5` — that interface only.
|
||||
|
||||
An **empty** value is treated as unset, as everywhere else here, and
|
||||
takes the default. In a container that matters: `BIND_ADDRESS=` throws
|
||||
away the image's `0.0.0.0` and falls back to the binary's
|
||||
`127.0.0.1`, which is the one quiet failure this setting has — see
|
||||
[Running with Docker](#running-with-docker).
|
||||
|
||||
Hostnames are **not** accepted. `localhost` aborts startup rather than
|
||||
being resolved: which of `127.0.0.1` and `::1` it means differs by
|
||||
host, a name can resolve to several addresses of which only one could
|
||||
be bound, and the answer can change under a running process. A value
|
||||
carrying a port (`127.0.0.1:8080`) is likewise rejected — the port is
|
||||
`PORT`'s business. Any unparseable value aborts startup; see
|
||||
[Invalid values abort startup](#invalid-values-abort-startup).
|
||||
|
||||
An address that parses but is not assigned to this host — say
|
||||
`10.0.0.5` on a machine that has no such interface — is a valid
|
||||
literal, so it reaches the listener and fails there. The process logs
|
||||
the bind error and exits non-zero rather than staying up with nothing
|
||||
listening. The effective value is in the `bindAddress` field of the
|
||||
startup log line, which is the way to check what a running deployment
|
||||
actually bound.
|
||||
|
||||
#### Metrics credentials
|
||||
|
||||
`METRICS_USERNAME` and `METRICS_PASSWORD` are set together or not at
|
||||
@@ -397,8 +460,11 @@ additionally be a number in the range 1–65535,
|
||||
`RECEIVER_RATE_LIMIT` must be at least 1,
|
||||
`RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
|
||||
period, so `0s` or a negative value would crash the reaper after
|
||||
startup), and every entry in `TRUSTED_PROXIES` and
|
||||
`ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address.
|
||||
startup), every entry in `TRUSTED_PROXIES` and
|
||||
`ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address, and
|
||||
`BIND_ADDRESS` must be an IP address literal — `localhost`,
|
||||
`127.0.0.1:8080` and `10.0.0.0/8` are each rejected rather than
|
||||
resolved, split, or narrowed to something they do not say.
|
||||
`SESSION_IDLE_TIMEOUT` is the exception: a
|
||||
non-positive value there means idle expiry is disabled, not invalid.
|
||||
|
||||
@@ -543,12 +609,54 @@ decision:
|
||||
|
||||
```bash
|
||||
docker run -d \
|
||||
-p 8080:8080 \
|
||||
-p 127.0.0.1:8080:8080 \
|
||||
-v /path/to/data:/var/lib/webhooker \
|
||||
-e WEBHOOKER_ENVIRONMENT=prod \
|
||||
-e BIND_ADDRESS=0.0.0.0 \
|
||||
webhooker:latest
|
||||
```
|
||||
|
||||
**The image and the bare binary default `BIND_ADDRESS` differently, on
|
||||
purpose.** The binary defaults to `127.0.0.1`; the image ships
|
||||
`ENV BIND_ADDRESS=0.0.0.0`, so the `-e BIND_ADDRESS=0.0.0.0` above is
|
||||
belt-and-braces and the command works without it.
|
||||
|
||||
The two cases are not the same question. On a bare host, `0.0.0.0`
|
||||
puts the cleartext admin UI and the unauthenticated receiver on every
|
||||
interface of the machine, which is what the loopback default exists to
|
||||
prevent. In a container, the network namespace is already that
|
||||
boundary: nothing outside reaches `0.0.0.0:8080` because of the
|
||||
namespace, not because of the bind. What decides exposure there is the
|
||||
**publish flag**, and that is the line to get right.
|
||||
|
||||
So publish to `127.0.0.1:8080` rather than `8080`. A bare
|
||||
`-p 8080:8080` opens the port on every interface of the host — through
|
||||
firewall rules too, since Docker's forwarding rules are inserted ahead
|
||||
of most host firewalls. Publish to the host address your reverse proxy
|
||||
connects from, and nothing wider.
|
||||
|
||||
**An empty `BIND_ADDRESS` is treated as unset**, like every other
|
||||
variable here, so `-e BIND_ADDRESS=` does not mean "keep the image
|
||||
default" — it discards the image's `0.0.0.0` and falls back to the
|
||||
_binary's_ `127.0.0.1`. In a container that is the failure below, and
|
||||
nothing in the logs names the variable. A templated Compose file or a
|
||||
`.env` line with an empty value is the usual way in. Either set a
|
||||
literal or leave the variable out entirely.
|
||||
|
||||
Overriding `BIND_ADDRESS` to a loopback address in a container — by
|
||||
that route or deliberately — makes the container unreachable from
|
||||
outside its namespace even with `-p`. The published port answers
|
||||
nothing, and the health check fails too: it requests
|
||||
`http://localhost:8080`, `localhost` resolves to `::1` first, and a
|
||||
`127.0.0.1` bind is not listening there. The container then goes
|
||||
`unhealthy` about **65 seconds** after start — from `HEALTHCHECK
|
||||
--start-period=5s --interval=30s --retries=3`, so failing probes at
|
||||
5s, 35s and 65s, and `unhealthy` on the third. (Docker's probe cadence
|
||||
during the start period has changed between versions; re-derive from
|
||||
those three values rather than trusting the figure. Measured at 65s on
|
||||
Docker 29.7.2.) A container `unhealthy` with `connection refused` in
|
||||
its health log, or a published port that resets connections, is this.
|
||||
|
||||
The container runs as a non-root user (`webhooker`, UID 1000), exposes
|
||||
port 8080, and includes a health check against
|
||||
`/.well-known/healthcheck`. The `/var/lib/webhooker` volume holds all
|
||||
@@ -558,6 +666,151 @@ databases written by `database` targets (`archive-{uuid}.db`). Mount
|
||||
this as a persistent volume to preserve data across container
|
||||
restarts.
|
||||
|
||||
## Deployment behind a reverse proxy
|
||||
|
||||
webhooker terminates no TLS of its own. It serves plaintext HTTP and
|
||||
expects a reverse proxy in front of it, which is the deployment it is
|
||||
built for: the proxy holds the certificate, and webhooker binds
|
||||
loopback where only the proxy can reach it.
|
||||
|
||||
Five things have to be right. Each one is silent when it is wrong —
|
||||
the service comes up, serves pages, and is broken in a way nothing
|
||||
reports.
|
||||
|
||||
1. **Bind or firewall the app port.** The binary binds `127.0.0.1` by
|
||||
default, so the cleartext listener is not published beside the
|
||||
proxy. The image binds `0.0.0.0` inside its own network namespace
|
||||
and relies on the publish address instead —
|
||||
`-p 127.0.0.1:8080:8080`. Either way the port must reach the proxy
|
||||
and nothing else; widen it only with a firewall or a publish
|
||||
address in front of it. A cleartext port answering the internet
|
||||
serves the admin login form and the unauthenticated receiver with
|
||||
no TLS at all, and the proxy in front of it changes nothing about
|
||||
that.
|
||||
2. **Set `WEBHOOKER_ENVIRONMENT=prod`, and make sure the proxy sends
|
||||
`X-Forwarded-Proto`.** These are two requirements, not one. The
|
||||
environment setting decides CORS and nothing else: the default
|
||||
`dev` answers every origin with `Access-Control-Allow-Origin: *`
|
||||
(without credentials), which a server-rendered production
|
||||
deployment has no use for. Cookie `Secure` and the strict
|
||||
Origin/Referer mode are **not** tied to it — they are decided per
|
||||
request from the transport, which behind a proxy means the
|
||||
`X-Forwarded-Proto` header. The block below sets it; without it
|
||||
every request is read as plaintext and cookies ship without
|
||||
`Secure`. See [Configuration](#configuration).
|
||||
3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate
|
||||
limiter keys on the connecting peer, which behind a proxy is the
|
||||
proxy on every request: all clients collapse into one global bucket
|
||||
per limit and the receiver's per-IP limits become service-wide
|
||||
ceilings. See [Trusted proxies](#trusted-proxies). List the proxy
|
||||
and nothing else.
|
||||
4. **Send `Host` as `$http_host`, not `$host`.** `$host` strips the
|
||||
port. webhooker's Origin/Referer check compares against the host it
|
||||
was given, so on any port other than 443 `$host` makes every form
|
||||
POST — including login — fail with `403 origin invalid`, with
|
||||
nothing in the error naming the cause.
|
||||
5. **Keep the proxy's access log.** webhooker's own access log records
|
||||
the peer address, which behind a proxy is always the proxy. The
|
||||
proxy's log is the only record of which client sent what. nginx's
|
||||
default `combined` format already logs `$remote_addr`; do not
|
||||
replace it with one that drops the client address, and retain those
|
||||
logs as long as you would want to answer a question about traffic.
|
||||
|
||||
### nginx
|
||||
|
||||
Complete server block. Replace the `server_name` and the two
|
||||
certificate paths.
|
||||
|
||||
```nginx
|
||||
server {
|
||||
listen 443 ssl;
|
||||
listen [::]:443 ssl;
|
||||
http2 on; # nginx 1.25.1+; older: listen 443 ssl http2;
|
||||
|
||||
server_name webhooker.example.com;
|
||||
|
||||
ssl_certificate /etc/ssl/certs/webhooker.example.com.crt;
|
||||
ssl_certificate_key /etc/ssl/private/webhooker.example.com.key;
|
||||
ssl_protocols TLSv1.2 TLSv1.3;
|
||||
|
||||
# webhooker caps form POST bodies at 1 MB. nginx's default happens
|
||||
# to match, so leaving this out breaks nothing today — but if you
|
||||
# ever raise webhooker's cap, this is the limit you will still be
|
||||
# hitting, and the rejection is nginx's HTML page rather than
|
||||
# webhooker's message.
|
||||
client_max_body_size 1m;
|
||||
|
||||
# $remote_addr is the client. webhooker's own log records this
|
||||
# proxy and nothing else, so this file is the only place the
|
||||
# client's address is written down.
|
||||
access_log /var/log/nginx/webhooker.access.log combined;
|
||||
|
||||
location / {
|
||||
# A literal address, not localhost: with BIND_ADDRESS at its
|
||||
# 127.0.0.1 default, a localhost that resolves to ::1 first
|
||||
# gets connection refused.
|
||||
proxy_pass http://127.0.0.1:8080;
|
||||
|
||||
# $http_host, NOT $host. $host drops the port and every form
|
||||
# POST fails with 403 origin invalid on any port but 443.
|
||||
proxy_set_header Host $http_host;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $scheme;
|
||||
|
||||
# Above webhooker's own 60s request timeout, so its 503
|
||||
# reaches the client instead of nginx cutting the connection
|
||||
# first and answering 504.
|
||||
proxy_read_timeout 70s;
|
||||
}
|
||||
}
|
||||
|
||||
server {
|
||||
listen 80;
|
||||
listen [::]:80;
|
||||
server_name webhooker.example.com;
|
||||
return 308 https://$host$request_uri;
|
||||
}
|
||||
```
|
||||
|
||||
`X-Forwarded-For` must be **appended** to, which
|
||||
`$proxy_add_x_forwarded_for` does. webhooker reads no other forwarded
|
||||
client header: `X-Real-IP` and `True-Client-IP` are ignored from every
|
||||
peer, so setting them has no effect. See
|
||||
[Trusted proxies](#trusted-proxies) for how the chain is walked.
|
||||
|
||||
`X-Forwarded-Proto: https` is what tells webhooker the request arrived
|
||||
over TLS, which decides the `Secure` flag on both the session and CSRF
|
||||
cookies and the strict Origin/Referer mode. Without it, requests are
|
||||
treated as plaintext and the cookies ship without `Secure`. Unlike
|
||||
`X-Forwarded-For`, this header is read from any peer and is not gated
|
||||
by `TRUSTED_PROXIES`, so the proxy must overwrite whatever a client
|
||||
sent — `$scheme` above does.
|
||||
|
||||
With that block, webhooker's environment is:
|
||||
|
||||
```sh
|
||||
WEBHOOKER_ENVIRONMENT=prod
|
||||
BIND_ADDRESS=127.0.0.1 # the default; stated here to be explicit
|
||||
TRUSTED_PROXIES=127.0.0.1
|
||||
```
|
||||
|
||||
If nginx runs on another host, `BIND_ADDRESS` becomes the address it
|
||||
connects to, `TRUSTED_PROXIES` becomes nginx's address, and the port
|
||||
must be firewalled to that address — the traffic between them is
|
||||
cleartext.
|
||||
|
||||
### HSTS is always sent, and is not configurable
|
||||
|
||||
Every response carries
|
||||
`Strict-Transport-Security: max-age=63072000; includeSubDomains; preload`.
|
||||
Two years, every subdomain, and a `preload` token. There is no setting
|
||||
that changes or suppresses it.
|
||||
|
||||
This is worth knowing before the first request reaches a browser: a
|
||||
client that sees it once will refuse plaintext HTTP to that hostname —
|
||||
and to every subdomain of it — for two years, whatever else is served
|
||||
there. Terminate TLS on a hostname you are prepared to keep on HTTPS.
|
||||
|
||||
## Backup, Restore, and Upgrades
|
||||
|
||||
### What to back up
|
||||
@@ -696,6 +949,18 @@ Upgrade procedure:
|
||||
`curl -s http://host:8080/.well-known/healthcheck` reports the
|
||||
version it was stamped with (see [Version stamping](#version-stamping)).
|
||||
|
||||
**Upgrading past the introduction of `BIND_ADDRESS`:** earlier versions
|
||||
always bound every interface. **Container deployments are unaffected**
|
||||
— the image ships `ENV BIND_ADDRESS=0.0.0.0`, so a `docker run` or
|
||||
Compose service that worked before still works with nothing changed.
|
||||
|
||||
A **bare binary** is the case that changes: the listener now binds
|
||||
`127.0.0.1` unless `BIND_ADDRESS` says otherwise, so a deployment that
|
||||
relied on reaching it from another host becomes unreachable until it
|
||||
sets the address the proxy connects to. Check the `bindAddress` field
|
||||
of the startup log to see what a running process bound. See
|
||||
[Bind address](#bind-address).
|
||||
|
||||
**Downgrade is unsupported.** Once a newer binary has migrated the files
|
||||
there is no way to move them back. `AutoMigrate` is additive — it adds
|
||||
tables, columns and indexes and never drops or rewrites them — so an
|
||||
|
||||
@@ -33,6 +33,34 @@ const (
|
||||
// defaultPort is the default HTTP listen port.
|
||||
defaultPort = 8080
|
||||
|
||||
// defaultBindAddress is the interface the plaintext HTTP
|
||||
// listener claims when BIND_ADDRESS is unset.
|
||||
//
|
||||
// Loopback, because the listener speaks cleartext and serves
|
||||
// both the admin UI and the unauthenticated receiver: a
|
||||
// wildcard default publishes them on every interface of every
|
||||
// host that never configured anything, which is the failure
|
||||
// this default exists to prevent. Reaching webhooker from off
|
||||
// the host is then a deliberate act — a reverse proxy in front
|
||||
// of it, or an explicit BIND_ADDRESS.
|
||||
//
|
||||
// This is the binary's default only. The Dockerfile ships
|
||||
// ENV BIND_ADDRESS=0.0.0.0, so a container deployment needs
|
||||
// nothing set and is unaffected by this constant. The two
|
||||
// differ because they answer different questions: a container's
|
||||
// network namespace is already the boundary this default is
|
||||
// reaching for, so binding every address inside it exposes
|
||||
// nothing, and what decides exposure there is the publish flag
|
||||
// (-p 127.0.0.1:8080:8080). A loopback bind inside a container
|
||||
// buys no security and makes the process unreachable through
|
||||
// its own published port.
|
||||
//
|
||||
// The split is expressed as two explicit defaults rather than
|
||||
// container auto-detection, because a heuristic that guesses
|
||||
// wrong opens the cleartext port exactly where nobody is
|
||||
// looking.
|
||||
defaultBindAddress = "127.0.0.1"
|
||||
|
||||
// defaultRetentionSweepInterval is how often the retention
|
||||
// reaper deletes events older than each webhook's RetentionDays.
|
||||
defaultRetentionSweepInterval = time.Hour
|
||||
@@ -75,6 +103,10 @@ var ErrInvalidPort = errors.New("invalid port")
|
||||
// nor a bare IP address.
|
||||
var ErrInvalidCIDR = errors.New("invalid CIDR")
|
||||
|
||||
// ErrInvalidBindAddress is returned when BIND_ADDRESS is set to
|
||||
// something that is not an IP address literal.
|
||||
var ErrInvalidBindAddress = errors.New("invalid bind address")
|
||||
|
||||
// ErrIncompleteMetricsAuth is returned when exactly one of
|
||||
// METRICS_USERNAME and METRICS_PASSWORD carries a value. Neither
|
||||
// fallback is acceptable: serving /metrics on the username alone
|
||||
@@ -105,6 +137,13 @@ type Config struct {
|
||||
Port int
|
||||
SentryDSN string
|
||||
|
||||
// BindAddress is the IP address the plaintext HTTP listener
|
||||
// binds, as an address literal. It defaults to
|
||||
// defaultBindAddress and is never empty: an empty string would
|
||||
// mean the wildcard to net.Listen, which is the opposite of the
|
||||
// default this ships.
|
||||
BindAddress string
|
||||
|
||||
// RetentionSweepInterval is how often the retention reaper runs.
|
||||
// Always positive: it becomes a time.NewTicker period.
|
||||
RetentionSweepInterval time.Duration
|
||||
@@ -387,6 +426,42 @@ func envPrefixList(key string) ([]netip.Prefix, error) {
|
||||
return prefixes, nil
|
||||
}
|
||||
|
||||
// envBindAddress returns the value of the named environment variable
|
||||
// parsed as an IP address literal. An unset (or empty, or
|
||||
// whitespace-only) value yields defaultValue.
|
||||
//
|
||||
// Only literals are accepted: no hostname is resolved, so `localhost`
|
||||
// is an error rather than a DNS lookup at startup whose answer could
|
||||
// be either loopback family, could change under the process, and
|
||||
// could return several addresses of which only one would be bound. A
|
||||
// value with a port in it (`127.0.0.1:8080`) is likewise an error —
|
||||
// the port is PORT's business, and silently accepting it would bind
|
||||
// something other than what was asked for.
|
||||
//
|
||||
// A set value that is not a literal is a hard error naming the key
|
||||
// and the bad value, so startup fails loudly rather than falling back
|
||||
// to a default the operator plainly did not want. A literal that is
|
||||
// not an address of this host parses here and fails at listen time
|
||||
// instead, which ends the process non-zero.
|
||||
func envBindAddress(key, defaultValue string) (string, error) {
|
||||
v := strings.TrimSpace(os.Getenv(key))
|
||||
if v == "" {
|
||||
return defaultValue, nil
|
||||
}
|
||||
|
||||
addr, err := netip.ParseAddr(v)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf(
|
||||
"%w: %s: %q must be an IP address literal such as "+
|
||||
"127.0.0.1, 0.0.0.0 or ::, not a hostname and not "+
|
||||
"host:port: %w",
|
||||
ErrInvalidBindAddress, key, v, err,
|
||||
)
|
||||
}
|
||||
|
||||
return addr.String(), nil
|
||||
}
|
||||
|
||||
// resolveMetricsAuth reads the /metrics basic-auth credentials and
|
||||
// rejects a half-set pair, naming both variables either way. The
|
||||
// error carries neither value: the password is a secret.
|
||||
@@ -431,6 +506,27 @@ func resolveEnvironment() (string, error) {
|
||||
return environment, nil
|
||||
}
|
||||
|
||||
// resolveListener reads the two variables that describe the HTTP
|
||||
// listener: which port it claims and which address it claims it on.
|
||||
// They are read together because neither is meaningful alone, and
|
||||
// because a validation failure in either has to abort startup before
|
||||
// anything binds.
|
||||
func resolveListener() (int, string, error) {
|
||||
port, err := envPort("PORT", defaultPort)
|
||||
if err != nil {
|
||||
return 0, "", err
|
||||
}
|
||||
|
||||
bindAddress, err := envBindAddress(
|
||||
"BIND_ADDRESS", defaultBindAddress,
|
||||
)
|
||||
if err != nil {
|
||||
return 0, "", err
|
||||
}
|
||||
|
||||
return port, bindAddress, nil
|
||||
}
|
||||
|
||||
// loadFromEnv builds a Config from the environment. Every value that
|
||||
// needs parsing fails loudly when it is set but unparseable: the
|
||||
// documented defaults apply only to variables that are unset (or
|
||||
@@ -442,7 +538,7 @@ func loadFromEnv() (*Config, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
port, err := envPort("PORT", defaultPort)
|
||||
port, bindAddress, err := resolveListener()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -506,6 +602,7 @@ func loadFromEnv() (*Config, error) {
|
||||
MetricsUsername: metricsUsername,
|
||||
MetricsPassword: metricsPassword,
|
||||
Port: port,
|
||||
BindAddress: bindAddress,
|
||||
SentryDSN: envString("SENTRY_DSN"),
|
||||
RetentionSweepInterval: retentionSweepInterval,
|
||||
SessionIdleTimeout: sessionIdleTimeout,
|
||||
@@ -625,6 +722,11 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
|
||||
log.Info("Configuration loaded",
|
||||
"environment", s.Environment,
|
||||
"port", s.Port,
|
||||
// Logged because which interfaces the cleartext listener
|
||||
// answers on is not otherwise observable from inside a
|
||||
// container, and it decides whether anything but the local
|
||||
// host can reach the admin UI.
|
||||
"bindAddress", s.BindAddress,
|
||||
"debug", s.Debug,
|
||||
"maintenanceMode", s.MaintenanceMode,
|
||||
"dataDir", s.DataDir,
|
||||
|
||||
@@ -21,6 +21,22 @@ const (
|
||||
envKeyPort = "PORT"
|
||||
envKeyDebug = "DEBUG"
|
||||
envKeyMaintenanceMode = "MAINTENANCE_MODE"
|
||||
envKeyBindAddress = "BIND_ADDRESS"
|
||||
)
|
||||
|
||||
// Sample BIND_ADDRESS values used by the tables below.
|
||||
const (
|
||||
// bindAddressDefault is the shipped default. It is asserted
|
||||
// against the package's own constant in
|
||||
// TestNewUsesDefaultsWhenUnset, so the two cannot drift.
|
||||
bindAddressDefault = "127.0.0.1"
|
||||
|
||||
// bindAddressWildcard is the value a container deployment sets.
|
||||
bindAddressWildcard = "0.0.0.0"
|
||||
|
||||
// bindAddressSample is an arbitrary specific address, standing
|
||||
// for "one interface of several".
|
||||
bindAddressSample = "10.1.2.3"
|
||||
)
|
||||
|
||||
// envBoolCase is one row of the envBool table.
|
||||
@@ -291,6 +307,160 @@ func TestEnvPort(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestEnvBindAddress covers BIND_ADDRESS parsing.
|
||||
//
|
||||
// Only IP address literals are accepted. Every rejection below is a
|
||||
// value an operator plausibly writes — a hostname, a host:port, a
|
||||
// CIDR block — and each has to abort startup rather than fall back to
|
||||
// the default, because falling back would bind an address other than
|
||||
// the one asked for and, in the wildcard-default case this setting
|
||||
// exists to end, publish cleartext on every interface.
|
||||
func TestEnvBindAddress(t *testing.T) {
|
||||
for _, tt := range envBindAddressCases() {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Cannot use t.Parallel() here because t.Setenv
|
||||
// is incompatible with parallel subtests.
|
||||
if tt.set {
|
||||
t.Setenv(testEnvKey, tt.value)
|
||||
} else {
|
||||
require.NoError(t, os.Unsetenv(testEnvKey))
|
||||
}
|
||||
|
||||
got, err := config.EnvBindAddressForTest(
|
||||
testEnvKey, bindAddressDefault,
|
||||
)
|
||||
|
||||
if tt.expectError {
|
||||
require.Error(t, err)
|
||||
require.ErrorIs(t, err, config.ErrInvalidBindAddress)
|
||||
assert.Contains(t, err.Error(), testEnvKey)
|
||||
assert.Contains(t, err.Error(), tt.value)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tt.expected, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// envBindAddressCase is one row of the envBindAddress table.
|
||||
type envBindAddressCase struct {
|
||||
name string
|
||||
set bool
|
||||
value string
|
||||
expectError bool
|
||||
expected string
|
||||
}
|
||||
|
||||
// envBindAddressCases is the envBindAddress table, kept out of the
|
||||
// test body so the test itself stays readable.
|
||||
func envBindAddressCases() []envBindAddressCase {
|
||||
return append(
|
||||
envBindAddressAcceptedCases(),
|
||||
envBindAddressRejectedCases()...,
|
||||
)
|
||||
}
|
||||
|
||||
// envBindAddressAcceptedCases are the values that parse: the three
|
||||
// spellings of "unset" that take the default, and the literals.
|
||||
func envBindAddressAcceptedCases() []envBindAddressCase {
|
||||
return []envBindAddressCase{
|
||||
{
|
||||
name: "unset returns the default",
|
||||
expected: bindAddressDefault,
|
||||
},
|
||||
{
|
||||
name: "empty returns the default",
|
||||
set: true,
|
||||
value: "",
|
||||
expected: bindAddressDefault,
|
||||
},
|
||||
{
|
||||
name: "whitespace returns the default",
|
||||
set: true,
|
||||
value: " ",
|
||||
expected: bindAddressDefault,
|
||||
},
|
||||
{
|
||||
name: "ipv4 wildcard is parsed",
|
||||
set: true,
|
||||
value: bindAddressWildcard,
|
||||
expected: bindAddressWildcard,
|
||||
},
|
||||
{
|
||||
name: "ipv4 literal is parsed",
|
||||
set: true,
|
||||
value: bindAddressSample,
|
||||
expected: bindAddressSample,
|
||||
},
|
||||
{
|
||||
name: "surrounding whitespace is trimmed",
|
||||
set: true,
|
||||
value: " " + bindAddressSample + " ",
|
||||
expected: bindAddressSample,
|
||||
},
|
||||
{
|
||||
name: "ipv6 wildcard is parsed",
|
||||
set: true,
|
||||
value: "::",
|
||||
expected: "::",
|
||||
},
|
||||
{
|
||||
name: "ipv6 literal is parsed",
|
||||
set: true,
|
||||
value: "2001:db8::5",
|
||||
expected: "2001:db8::5",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// envBindAddressRejectedCases are the values that abort startup.
|
||||
// Each is something an operator plausibly writes, and none may fall
|
||||
// back to the default: the default is loopback, so a silent fallback
|
||||
// would bind somewhere other than what was asked for.
|
||||
func envBindAddressRejectedCases() []envBindAddressCase {
|
||||
return []envBindAddressCase{
|
||||
{
|
||||
name: "garbage is rejected",
|
||||
set: true,
|
||||
value: "not-an-address",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "hostname is rejected",
|
||||
set: true,
|
||||
value: "localhost",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "unresolvable hostname is rejected",
|
||||
set: true,
|
||||
value: "no-such-host.invalid",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "host and port is rejected",
|
||||
set: true,
|
||||
value: bindAddressDefault + ":8080",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "bracketed ipv6 is rejected",
|
||||
set: true,
|
||||
value: "[::1]",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "CIDR block is rejected",
|
||||
set: true,
|
||||
value: "10.0.0.0/8",
|
||||
expectError: true,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// buildConfig constructs a Config through fx exactly as the
|
||||
// application does, returning the config and any construction error.
|
||||
func buildConfig(t *testing.T) (*config.Config, error) {
|
||||
@@ -312,13 +482,45 @@ func buildConfig(t *testing.T) (*config.Config, error) {
|
||||
}
|
||||
|
||||
func TestNewRejectsBadEnvValues(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
key string
|
||||
value string
|
||||
expectError bool
|
||||
check func(t *testing.T, cfg *config.Config)
|
||||
}{
|
||||
for _, tt := range badEnvValueCases() {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Cannot use t.Parallel() here because t.Setenv
|
||||
// is incompatible with parallel subtests.
|
||||
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
|
||||
t.Setenv(tt.key, tt.value)
|
||||
|
||||
cfg, err := buildConfig(t)
|
||||
|
||||
if tt.expectError {
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tt.key)
|
||||
assert.Contains(t, err.Error(), tt.value)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, cfg)
|
||||
tt.check(t, cfg)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// badEnvValueCase is one row of the config.New table: a variable, the
|
||||
// value it is set to, and either the assertion that startup fails
|
||||
// naming both, or a check on the Config that resulted.
|
||||
type badEnvValueCase struct {
|
||||
name string
|
||||
key string
|
||||
value string
|
||||
expectError bool
|
||||
check func(t *testing.T, cfg *config.Config)
|
||||
}
|
||||
|
||||
// badEnvValueCases is the config.New table, kept out of the test body
|
||||
// so the test itself stays readable.
|
||||
func badEnvValueCases() []badEnvValueCase {
|
||||
return []badEnvValueCase{
|
||||
{
|
||||
name: "valid PORT is used",
|
||||
key: envKeyPort,
|
||||
@@ -361,29 +563,35 @@ func TestNewRejectsBadEnvValues(t *testing.T) {
|
||||
value: "sometimes",
|
||||
expectError: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Cannot use t.Parallel() here because t.Setenv
|
||||
// is incompatible with parallel subtests.
|
||||
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
|
||||
t.Setenv(tt.key, tt.value)
|
||||
|
||||
cfg, err := buildConfig(t)
|
||||
|
||||
if tt.expectError {
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tt.key)
|
||||
assert.Contains(t, err.Error(), tt.value)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, cfg)
|
||||
tt.check(t, cfg)
|
||||
})
|
||||
{
|
||||
name: "valid BIND_ADDRESS is used",
|
||||
key: envKeyBindAddress,
|
||||
value: bindAddressWildcard,
|
||||
check: func(t *testing.T, cfg *config.Config) {
|
||||
t.Helper()
|
||||
assert.Equal(
|
||||
t, bindAddressWildcard, cfg.BindAddress,
|
||||
)
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "unparseable BIND_ADDRESS aborts startup",
|
||||
key: envKeyBindAddress,
|
||||
value: "not-an-address",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "hostname BIND_ADDRESS aborts startup",
|
||||
key: envKeyBindAddress,
|
||||
value: "localhost",
|
||||
expectError: true,
|
||||
},
|
||||
{
|
||||
name: "BIND_ADDRESS with a port aborts startup",
|
||||
key: envKeyBindAddress,
|
||||
value: bindAddressDefault + ":8080",
|
||||
expectError: true,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -395,6 +603,7 @@ func TestNewUsesDefaultsWhenUnset(t *testing.T) {
|
||||
|
||||
for _, key := range []string{
|
||||
envKeyPort, envKeyDebug, envKeyMaintenanceMode,
|
||||
envKeyBindAddress,
|
||||
} {
|
||||
require.NoError(t, os.Unsetenv(key))
|
||||
}
|
||||
@@ -406,4 +615,15 @@ func TestNewUsesDefaultsWhenUnset(t *testing.T) {
|
||||
assert.Equal(t, 8080, cfg.Port)
|
||||
assert.False(t, cfg.Debug)
|
||||
assert.False(t, cfg.MaintenanceMode)
|
||||
|
||||
// Loopback, not the wildcard: the default must not publish the
|
||||
// cleartext admin UI and the unauthenticated receiver on every
|
||||
// interface of a host that configured nothing. The value is read
|
||||
// from the package rather than repeated, so the README's
|
||||
// documented default and the compiled-in one are pinned to the
|
||||
// same constant.
|
||||
assert.Equal(
|
||||
t, config.DefaultBindAddressForTest, cfg.BindAddress,
|
||||
)
|
||||
assert.Equal(t, bindAddressDefault, cfg.BindAddress)
|
||||
}
|
||||
|
||||
@@ -50,3 +50,13 @@ func EnvPositiveIntForTest(key string, defaultValue int) (int, error) {
|
||||
func EnvPortForTest(key string, defaultValue int) (int, error) {
|
||||
return envPort(key, defaultValue)
|
||||
}
|
||||
|
||||
// EnvBindAddressForTest exposes envBindAddress.
|
||||
func EnvBindAddressForTest(key, defaultValue string) (string, error) {
|
||||
return envBindAddress(key, defaultValue)
|
||||
}
|
||||
|
||||
// DefaultBindAddressForTest exposes the compiled-in BIND_ADDRESS
|
||||
// default, so a test pins the documented value rather than repeating
|
||||
// a literal that could drift from it.
|
||||
const DefaultBindAddressForTest = defaultBindAddress
|
||||
|
||||
336
internal/server/bind_address_test.go
Normal file
336
internal/server/bind_address_test.go
Normal file
@@ -0,0 +1,336 @@
|
||||
package server_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/fx"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/globals"
|
||||
"sneak.berlin/go/webhooker/internal/server"
|
||||
)
|
||||
|
||||
const (
|
||||
// loopbackV4 is the shipped BIND_ADDRESS default.
|
||||
loopbackV4 = "127.0.0.1"
|
||||
|
||||
// wildcardV4 is the value a container deployment must set,
|
||||
// where a loopback-bound process is unreachable from outside
|
||||
// its network namespace even with a published port.
|
||||
wildcardV4 = "0.0.0.0"
|
||||
|
||||
// unavailableAddr is a TEST-NET-1 address (RFC 5737). It is a
|
||||
// well-formed literal that no host is assigned, so binding it
|
||||
// fails with EADDRNOTAVAIL rather than succeeding somewhere
|
||||
// unexpected.
|
||||
unavailableAddr = "192.0.2.1"
|
||||
|
||||
// listenReadyTimeout bounds the wait for the listener to accept
|
||||
// connections. The bind itself is immediate; this only covers
|
||||
// goroutine scheduling.
|
||||
listenReadyTimeout = 3 * time.Second
|
||||
|
||||
// listenPollInterval is how often the readiness wait retries.
|
||||
listenPollInterval = 10 * time.Millisecond
|
||||
|
||||
// dialTimeout bounds a single connection attempt in these
|
||||
// tests. Everything dialled here is on this host, so a dial
|
||||
// that is not answered immediately is a failure, not slowness.
|
||||
dialTimeout = time.Second
|
||||
)
|
||||
|
||||
// freePort returns a TCP port that is free on every local address at
|
||||
// the moment it returns, by taking one on the wildcard and releasing
|
||||
// it. The window between release and re-bind is the standard one
|
||||
// every "pick a free port" helper carries.
|
||||
func freePort(t *testing.T) int {
|
||||
t.Helper()
|
||||
|
||||
var listenCfg net.ListenConfig
|
||||
|
||||
l, err := listenCfg.Listen(t.Context(), "tcp", "0.0.0.0:0")
|
||||
require.NoError(t, err)
|
||||
|
||||
addr, ok := l.Addr().(*net.TCPAddr)
|
||||
require.True(t, ok, "listener is not TCP")
|
||||
require.NoError(t, l.Close())
|
||||
|
||||
return addr.Port
|
||||
}
|
||||
|
||||
// otherLocalAddr returns a local IPv4 address that is not
|
||||
// loopbackV4, or skips the test when the host has none.
|
||||
//
|
||||
// The bind-address tests need a second address of this host to stand
|
||||
// in for "another interface": what a wildcard bind claims and a
|
||||
// loopback bind does not. 127.0.0.2 is that address on Linux, where
|
||||
// the whole 127.0.0.0/8 is local; elsewhere an interface address is
|
||||
// used instead. Each candidate is proven bindable before it is
|
||||
// returned, so a host that offers neither skips rather than fails on
|
||||
// something that was never about the code under test.
|
||||
func otherLocalAddr(t *testing.T) string {
|
||||
t.Helper()
|
||||
|
||||
candidates := []string{"127.0.0.2"}
|
||||
|
||||
ifaceAddrs, err := net.InterfaceAddrs()
|
||||
require.NoError(t, err)
|
||||
|
||||
for _, a := range ifaceAddrs {
|
||||
ipNet, ok := a.(*net.IPNet)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
|
||||
ip4 := ipNet.IP.To4()
|
||||
if ip4 == nil || ip4.String() == loopbackV4 {
|
||||
continue
|
||||
}
|
||||
|
||||
candidates = append(candidates, ip4.String())
|
||||
}
|
||||
|
||||
var listenCfg net.ListenConfig
|
||||
|
||||
for _, candidate := range candidates {
|
||||
l, listenErr := listenCfg.Listen(
|
||||
t.Context(), "tcp", net.JoinHostPort(candidate, "0"),
|
||||
)
|
||||
if listenErr != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
require.NoError(t, l.Close())
|
||||
|
||||
return candidate
|
||||
}
|
||||
|
||||
t.Skip("host has no second local IPv4 address to bind")
|
||||
|
||||
return ""
|
||||
}
|
||||
|
||||
// startBoundServer starts the wired app with the given bind address
|
||||
// on a free port and returns that port. The app is stopped on
|
||||
// cleanup.
|
||||
func startBoundServer(t *testing.T, bindAddress string) int {
|
||||
t.Helper()
|
||||
|
||||
port := freePort(t)
|
||||
|
||||
env := newTestEnv(t)
|
||||
env.cfg.BindAddress = bindAddress
|
||||
env.cfg.Port = port
|
||||
|
||||
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))
|
||||
|
||||
t.Cleanup(func() {
|
||||
stopCtx, cancelStop := context.WithTimeout(
|
||||
context.Background(), lifecycleTimeout,
|
||||
)
|
||||
defer cancelStop()
|
||||
|
||||
require.NoError(t, app.Stop(stopCtx))
|
||||
})
|
||||
|
||||
return port
|
||||
}
|
||||
|
||||
// dialable reports whether a TCP connection to addr succeeds.
|
||||
func dialable(ctx context.Context, addr string) bool {
|
||||
dialer := net.Dialer{Timeout: dialTimeout}
|
||||
|
||||
conn, err := dialer.DialContext(ctx, "tcp", addr)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
|
||||
_ = conn.Close()
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
// requireDialable waits for addr to accept connections, failing the
|
||||
// test if it never does.
|
||||
func requireDialable(t *testing.T, addr string) {
|
||||
t.Helper()
|
||||
|
||||
deadline := time.Now().Add(listenReadyTimeout)
|
||||
for time.Now().Before(deadline) {
|
||||
if dialable(t.Context(), addr) {
|
||||
return
|
||||
}
|
||||
|
||||
time.Sleep(listenPollInterval)
|
||||
}
|
||||
|
||||
t.Fatalf("nothing accepted connections on %s", addr)
|
||||
}
|
||||
|
||||
// TestListenAddr pins how BindAddress and Port are rendered into the
|
||||
// listen address.
|
||||
//
|
||||
// The defect this covers was a bare fmt.Sprintf(":%d", port), which
|
||||
// binds every interface with no way to say otherwise. The IPv6 rows
|
||||
// are here because an unbracketed IPv6 host would produce an address
|
||||
// net.Listen rejects, turning a valid configuration into a startup
|
||||
// failure.
|
||||
func TestListenAddr(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
bindAddress string
|
||||
port int
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "loopback default",
|
||||
bindAddress: loopbackV4,
|
||||
port: 8080,
|
||||
expected: "127.0.0.1:8080",
|
||||
},
|
||||
{
|
||||
name: "ipv4 wildcard",
|
||||
bindAddress: wildcardV4,
|
||||
port: 8080,
|
||||
expected: "0.0.0.0:8080",
|
||||
},
|
||||
{
|
||||
name: "ipv6 wildcard is bracketed",
|
||||
bindAddress: "::",
|
||||
port: 8080,
|
||||
expected: "[::]:8080",
|
||||
},
|
||||
{
|
||||
name: "ipv6 literal is bracketed",
|
||||
bindAddress: "2001:db8::5",
|
||||
port: 9001,
|
||||
expected: "[2001:db8::5]:9001",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(t, tt.expected, server.ListenAddrForTest(
|
||||
&config.Config{
|
||||
BindAddress: tt.bindAddress,
|
||||
Port: tt.port,
|
||||
},
|
||||
))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestBindAddress_LoopbackIsNotOnOtherAddresses proves the fix end to
|
||||
// end: with BIND_ADDRESS at its loopback default, the cleartext
|
||||
// listener answers on loopback and has not claimed any other address
|
||||
// of this host.
|
||||
//
|
||||
// The second address is proven free by binding it on the same port
|
||||
// while the server runs. That is the assertion that fails against the
|
||||
// old wildcard bind — a wildcard listener owns the port on every
|
||||
// address, so this bind would return EADDRINUSE. Dialling from
|
||||
// another machine is what the operator cares about, and this is the
|
||||
// in-process form of it: the socket the remote host would connect to
|
||||
// does not exist.
|
||||
func TestBindAddress_LoopbackIsNotOnOtherAddresses(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
other := otherLocalAddr(t)
|
||||
port := startBoundServer(t, loopbackV4)
|
||||
|
||||
// Positive control: the service really is up and serving.
|
||||
requireDialable(t, net.JoinHostPort(loopbackV4, strconv.Itoa(port)))
|
||||
|
||||
var listenCfg net.ListenConfig
|
||||
|
||||
l, err := listenCfg.Listen(
|
||||
t.Context(), "tcp",
|
||||
net.JoinHostPort(other, strconv.Itoa(port)),
|
||||
)
|
||||
require.NoError(
|
||||
t, err,
|
||||
"port %d on %s is taken while bound to %s: the listener "+
|
||||
"claimed more than its configured address",
|
||||
port, other, loopbackV4,
|
||||
)
|
||||
|
||||
require.NoError(t, l.Close())
|
||||
}
|
||||
|
||||
// TestBindAddress_WildcardReachesOtherAddresses is the counterpart:
|
||||
// the value a container deployment sets does reach the addresses the
|
||||
// default withholds. Without this, a loopback-only bind would pass
|
||||
// the test above by never listening at all.
|
||||
func TestBindAddress_WildcardReachesOtherAddresses(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
other := otherLocalAddr(t)
|
||||
port := startBoundServer(t, wildcardV4)
|
||||
|
||||
requireDialable(t, net.JoinHostPort(other, strconv.Itoa(port)))
|
||||
}
|
||||
|
||||
// TestBindAddress_ServesRequestsOnConfiguredAddress proves the bound
|
||||
// listener serves the application rather than merely accepting TCP,
|
||||
// so a bind address that is honoured cannot be mistaken for one that
|
||||
// is honoured and broken.
|
||||
func TestBindAddress_ServesRequestsOnConfiguredAddress(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
port := startBoundServer(t, loopbackV4)
|
||||
addr := net.JoinHostPort(loopbackV4, strconv.Itoa(port))
|
||||
requireDialable(t, addr)
|
||||
|
||||
req, err := http.NewRequestWithContext(
|
||||
t.Context(), http.MethodGet,
|
||||
"http://"+addr+"/.well-known/healthcheck", nil,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
client := &http.Client{Timeout: dialTimeout}
|
||||
|
||||
resp, err := client.Do(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
assert.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
}
|
||||
|
||||
// TestBindAddress_UnavailableAddressShutsDownTheApp covers the half
|
||||
// of the fail-loud rule that configuration parsing cannot reach. A
|
||||
// syntactically valid address that is not assigned to this host
|
||||
// parses fine and fails at bind time, after fx has already reported
|
||||
// RUNNING. It must end the process non-zero rather than leave it
|
||||
// alive with nothing listening.
|
||||
func TestBindAddress_UnavailableAddressShutsDownTheApp(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := newTestEnv(t)
|
||||
env.cfg.BindAddress = unavailableAddr
|
||||
env.cfg.Port = freePort(t)
|
||||
|
||||
requireListenFailureExit(t, env)
|
||||
}
|
||||
91
internal/server/early_shutdown_test.go
Normal file
91
internal/server/early_shutdown_test.go
Normal file
@@ -0,0 +1,91 @@
|
||||
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))
|
||||
}
|
||||
@@ -55,6 +55,16 @@ func NewRouterForTest(
|
||||
return s.router
|
||||
}
|
||||
|
||||
// ListenAddrForTest exposes the address the HTTP listener binds for
|
||||
// a given Config, so the rendering of host and port — IPv6
|
||||
// bracketing above all — can be pinned without standing up a
|
||||
// listener.
|
||||
func ListenAddrForTest(cfg *config.Config) string {
|
||||
s := &Server{params: ServerParams{Config: cfg}}
|
||||
|
||||
return s.listenAddr()
|
||||
}
|
||||
|
||||
// ProbePattern is the route NewRouterWithProbeForTest adds to the
|
||||
// production route tree.
|
||||
const ProbePattern = "/probe"
|
||||
@@ -80,12 +90,12 @@ func NewRouterWithProbeForTest(
|
||||
probe http.HandlerFunc,
|
||||
) http.Handler {
|
||||
s := &Server{
|
||||
log: log,
|
||||
mw: mw,
|
||||
h: h,
|
||||
params: ServerParams{Config: cfg},
|
||||
sentryEnabled: sentryEnabled,
|
||||
log: log,
|
||||
mw: mw,
|
||||
h: h,
|
||||
params: ServerParams{Config: cfg},
|
||||
}
|
||||
s.sentryEnabled.Store(sentryEnabled)
|
||||
s.SetupRoutes()
|
||||
s.router.Handle(ProbePattern, probe)
|
||||
|
||||
|
||||
@@ -2,8 +2,9 @@ package server
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -24,21 +25,47 @@ const (
|
||||
httpMaxHeaderBytes = 1 << 20
|
||||
)
|
||||
|
||||
func (s *Server) serveUntilShutdown() {
|
||||
listenAddr := fmt.Sprintf(":%d", s.params.Config.Port)
|
||||
s.httpServer = &http.Server{
|
||||
Addr: listenAddr,
|
||||
// listenAddr renders the address the HTTP listener binds.
|
||||
//
|
||||
// The host half is always present: an empty host would be the
|
||||
// wildcard, and the whole point of BIND_ADDRESS is that binding every
|
||||
// interface is a choice the operator makes rather than one the
|
||||
// process makes for them. Config guarantees a literal, so
|
||||
// JoinHostPort's bracketing is enough to make IPv6 well formed.
|
||||
func (s *Server) listenAddr() string {
|
||||
return net.JoinHostPort(
|
||||
s.params.Config.BindAddress,
|
||||
strconv.Itoa(s.params.Config.Port),
|
||||
)
|
||||
}
|
||||
|
||||
// newHTTPServer builds the HTTP server for this Server's
|
||||
// configuration.
|
||||
//
|
||||
// It is called from New, on the constructing goroutine, rather than
|
||||
// from the serving goroutine that used to assign s.httpServer
|
||||
// directly. Two goroutines reach that field — the serving goroutine
|
||||
// and the fx stop hook, which calls Shutdown on it — with nothing
|
||||
// ordering them. Constructing it during New puts the write before
|
||||
// every hook fx will later run, which both removes the race and rules
|
||||
// out the nil dereference a stop that arrived before the serving
|
||||
// goroutine had run would have caused.
|
||||
func (s *Server) newHTTPServer() *http.Server {
|
||||
return &http.Server{
|
||||
Addr: s.listenAddr(),
|
||||
ReadTimeout: httpReadTimeout,
|
||||
WriteTimeout: httpWriteTimeout,
|
||||
MaxHeaderBytes: httpMaxHeaderBytes,
|
||||
Handler: s,
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Server) serveUntilShutdown() {
|
||||
// add routes
|
||||
// this does any necessary setup in each handler
|
||||
s.SetupRoutes()
|
||||
|
||||
s.log.Info("http begin listen", "listenaddr", listenAddr)
|
||||
s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
|
||||
|
||||
err := s.httpServer.ListenAndServe()
|
||||
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||
|
||||
@@ -36,9 +36,9 @@ const lifecycleTimeout = 15 * time.Second
|
||||
//
|
||||
// The port is occupied by a listener this test holds open, on a
|
||||
// kernel-chosen port, so the failure is the real EADDRINUSE the
|
||||
// operator hits when a second instance starts. Loopback is enough to
|
||||
// collide with the server's wildcard bind: a listening socket on a
|
||||
// specific address blocks the wildcard from claiming the same port.
|
||||
// operator hits when a second instance starts. The server is pointed
|
||||
// at the same loopback address, so the collision is a direct one on
|
||||
// the exact address it asks the kernel for.
|
||||
func TestListenFailure_ShutsDownTheApp(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -55,11 +55,27 @@ func TestListenFailure_ShutsDownTheApp(t *testing.T) {
|
||||
require.True(t, ok, "listener is not TCP")
|
||||
|
||||
// The collaborators come from the wired graph rather than stubs,
|
||||
// so the Server under test is the one that ships. Only the port
|
||||
// is test-specific.
|
||||
// so the Server under test is the one that ships. Only the
|
||||
// listen address is test-specific.
|
||||
env := newTestEnv(t)
|
||||
env.cfg.BindAddress = loopbackV4
|
||||
env.cfg.Port = addr.Port
|
||||
|
||||
requireListenFailureExit(t, env)
|
||||
}
|
||||
|
||||
// requireListenFailureExit starts the wired app over env and asserts
|
||||
// that it gives up on its own with the listen-failure status, then
|
||||
// completes its stop sequence.
|
||||
//
|
||||
// Two different listen failures share it — a port already in use and
|
||||
// an address that is not on this host — because what has to hold for
|
||||
// both is the same: the failure is discovered after fx has already
|
||||
// reported RUNNING, so the only thing that can turn it into a visible
|
||||
// exit is the shutdown path under test.
|
||||
func requireListenFailureExit(t *testing.T, env *testEnv) {
|
||||
t.Helper()
|
||||
|
||||
app := fx.New(
|
||||
fx.NopLogger,
|
||||
fx.Supply(env.log, env.cfg, env.mw, env.hnd),
|
||||
|
||||
@@ -81,7 +81,7 @@ func (s *Server) setupGlobalMiddleware() {
|
||||
// Sentry error reporting (if SENTRY_DSN is set). Repanic is
|
||||
// true so panics still bubble up to the Recoverer middleware
|
||||
// registered immediately above.
|
||||
if s.sentryEnabled {
|
||||
if s.sentryEnabled.Load() {
|
||||
sentryHandler := sentryhttp.New(sentryhttp.Options{
|
||||
Repanic: true,
|
||||
})
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"net/http"
|
||||
"os"
|
||||
"os/signal"
|
||||
"sync/atomic"
|
||||
"syscall"
|
||||
"time"
|
||||
|
||||
@@ -88,8 +89,15 @@ type ServerParams struct {
|
||||
// Server is the main HTTP server that wires up routes and manages
|
||||
// graceful shutdown.
|
||||
type Server struct {
|
||||
startupTime time.Time
|
||||
sentryEnabled bool
|
||||
startupTime time.Time
|
||||
|
||||
// sentryEnabled is written by the serving goroutine, in
|
||||
// enableSentry, and read by the fx stop hook in cleanShutdown.
|
||||
// Nothing orders those two: the OnStart hook returns as soon as
|
||||
// the goroutine is spawned, so a stop can be running while
|
||||
// enableSentry is still deciding. It is atomic to supply the
|
||||
// edge the goroutines do not.
|
||||
sentryEnabled atomic.Bool
|
||||
log *slog.Logger
|
||||
cancelFunc context.CancelFunc
|
||||
httpServer *http.Server
|
||||
@@ -107,6 +115,7 @@ func New(lc fx.Lifecycle, params ServerParams) (*Server, error) {
|
||||
s.mw = params.Middleware
|
||||
s.h = params.Handlers
|
||||
s.log = params.Logger.Get()
|
||||
s.httpServer = s.newHTTPServer()
|
||||
|
||||
lc.Append(fx.Hook{
|
||||
OnStart: func(_ context.Context) error {
|
||||
@@ -142,7 +151,7 @@ func (s *Server) MaintenanceMode() bool {
|
||||
}
|
||||
|
||||
func (s *Server) enableSentry() {
|
||||
s.sentryEnabled = false
|
||||
s.sentryEnabled.Store(false)
|
||||
|
||||
if s.params.Config.SentryDSN == "" {
|
||||
return
|
||||
@@ -163,7 +172,7 @@ func (s *Server) enableSentry() {
|
||||
}
|
||||
|
||||
s.log.Info("sentry error reporting activated")
|
||||
s.sentryEnabled = true
|
||||
s.sentryEnabled.Store(true)
|
||||
}
|
||||
|
||||
// serve installs the signal watcher, starts the listener and blocks
|
||||
@@ -242,7 +251,7 @@ func (s *Server) cleanShutdown(ctx context.Context) {
|
||||
|
||||
s.cleanupForExit()
|
||||
|
||||
if s.sentryEnabled {
|
||||
if s.sentryEnabled.Load() {
|
||||
s.flushSentry(ctx)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user