fix(health): split liveness from readiness and make reachability configurable - #40
Merged
Merged
Conversation
…igurable
Capstan is a separate process from Docker, but /health returned 503 whenever the
daemon was unreachable — a readiness signal on a liveness channel. Anything that
restarts on failed health checks would bounce Capstan for an outage a restart
cannot fix.
- /health is liveness: 200 whenever the process is serving, no Docker call. Body
keeps {"status":"healthy"} so existing monitors still match.
- /health/ready is readiness: reports dependencies, 503 naming what is degraded,
with a 2s timeout on the Docker probe so a hung daemon cannot pile up
goroutines across repeated probes. Uses Ping, one round-trip, rather than the
GetContainerList("") the old handler ran every 30 seconds.
- Handler moved out of main.go into internal/handlers/health.go, the last inline
route, with 12 tests covering every branch.
- Reachability is HEALTH_ALLOWED_NETWORKS, deliberately NOT TRUSTED_NETWORKS:
that value is Gin's trusted-proxy list and the AUTH_DISABLED bypass list, so
reusing it would force an operator to grant an uptime monitor
X-Forwarded-For spoofing and auth bypass just to read a health endpoint.
Empty means loopback only, so no upgrade widens exposure silently. The whole
loopback range is accepted, matching the net.IP.IsLoopback the old handler
used rather than narrowing to the literal 127.0.0.1.
- DockerService.Ping tolerates a nil receiver and nil client: main leaves
dockerService nil when the daemon was unreachable at startup, and a nil
*DockerService in an interface is a non-nil interface value.
The HEALTHCHECK was also fixed, and this was not in the issue. It used
`wget --spider`, which sends HEAD; no HEAD route is registered, so every HEAD
fell through to the SPA catch-all and returned 200. Measured on the pre-change
image with Docker unreachable: GET /health 503, HEAD /health 200, the healthcheck
command exit 0, and the container reporting healthy with FailingStreak=0 through
four intervals. So the outage the issue describes never marked the container
unhealthy in the first place — the check was a bare port probe. It now issues a
real GET, verified to exit 8 against a 503 and 0 against liveness.
Verified on a rebuilt image against a killable socat proxy in front of the real
socket, so the host daemon and its unrelated containers were never touched, with
the proxy's port confirmed closed before measuring:
Docker reachable /health 200 healthy /health/ready 200 ready
Docker unreachable /health 200 healthy /health/ready 503 degraded ["docker"]
container health healthy, streak 0, across 110s / >3 intervals
And over bridge networking, so the caller is genuinely non-loopback:
default 403 on both endpoints
HEALTH_ALLOWED_NETWORKS=172.16.0.0/12 200
Closes agent-os-69a
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes
agent-os-69a.What
/healthreturned 503 whenever the Docker daemon was unreachable. Capstan is aseparate process from Docker, so that is a readiness signal on a liveness
channel: anything that restarts on failed health checks would bounce Capstan for
an outage a restart cannot fix.
GET /health— liveness. 200 whenever the process is serving, no Dockercall. Body keeps
{"status":"healthy"}so existing monitors still match.GET /health/ready— readiness. Reports dependencies, 503 naming what isdegraded, 2s timeout on the Docker probe. Uses
Ping(one round-trip) insteadof the
GetContainerList("")the old handler ran every 30 seconds.main.go— the last inline route — intointernal/handlers/health.go, with 12 tests covering every branch.Two corrections to the issue's premises
1. The
HEALTHCHECKnever detected the outage in the first place. The issueassumes a Docker outage marks the container unhealthy. It does not. The check
used
wget --spider, which sends HEAD; no HEAD route is registered, so everyHEAD falls through to the SPA catch-all and returns 200. Measured on the
pre-change image with Docker confirmed unreachable:
So the shipped healthcheck was a bare port probe — it would not have caught a
wedged server returning 500s either. Fixed here (outside the issue's scope, but
it is the mechanism the issue's central claim rests on): it now issues a real
GET, verified to exit 8 against a 503 and 0 against liveness.
2. The
HEALTHCHECKURL does not change. The issue asks to point it atliveness; liveness stays at
/health, so only the command form changed. Theimage still needed rebuilding because the binary's behaviour changed.
Decision:
HEALTH_ALLOWED_NETWORKS, notTRUSTED_NETWORKSThe issue preferred reusing
TRUSTED_NETWORKS. That value is simultaneouslyGin's trusted-proxy list and the
AUTH_DISABLEDbypass list — the READMEdocuments that overloading at line 522. Reusing it would force an operator to
grant an uptime monitor
X-Forwarded-Forspoofing and authentication bypass justto let it read a health endpoint. A dedicated setting is least-privilege; only
the CIDR-matching helper is shared (
middleware.IsTrustedIP, now exported).Empty means loopback only, so no upgrade widens exposure silently. The whole
loopback range is accepted, matching the
net.IP.IsLoopbackthe old handler usedrather than narrowing to the literal
127.0.0.1thatIsTrustedIPmatches.Verification
Against a killable
socatproxy in front of the real socket, so the host daemonand its unrelated containers were never touched. The proxy's port was
confirmed closed before measuring — an earlier run assumed the kill worked
and measured a daemon that was still reachable, which produced a false "ready".
Reachability over bridge networking, so the caller is genuinely non-loopback
(under
--network hosteverything looks like loopback):That second run is also the proof the new env var is actually read end to end,
rather than being documented-and-defaulted like
PORT(agent-os-o6q).Each of the three behaviours that matter was confirmed by reverting it and
re-running: liveness consulting Docker (3 tests fail), the readiness timeout
removed (the hung-daemon test fails after 3s), and the network gate opened
(the default-denied and no-probe-when-denied tests fail).
Gates
No frontend changes.
gofmt -lflagsinternal/middleware/auth.gofor importordering, pre-existing on
main—agent-os-far's territory.Images removed and
docker builder prune -frun afterwards (2.9GB); no leftovercontainers or stray processes.
🤖 Generated with Claude Code