Answer image requests with 503 in maintenance mode (closes #71) #154

Open
clawbot wants to merge 2 commits from issue-71-maintenance-mode into next
Collaborator

Implements #71: turning on maintenance_mode now stops image requests; before, only the health check reported it.

  • One middleware in internal/server/routes.go, applied to a route group holding only /v1/image/ (GET, HEAD) and /v1/e/. While the flag is on it answers 503 with Retry-After (MaintenanceRetryAfterSeconds, 300) and a JSON error body. It calls Server.MaintenanceMode(), which had no caller.
  • The health check stays 200 and reports "maintenance_mode": true. The image's Docker HEALTHCHECK requests it: a 503 there would make the container unhealthy, and upaas marks a deploy failed when its container is unhealthy. Said in README.md and config.example.yml.
  • /metrics stays available: it has its own password, touches no image, and is how an operator watches the instance during maintenance.
  • The login and URL generator pages stay available: they only sign URLs, so an operator can keep working.

Worth knowing:

  • The config is read once at startup, so switching maintenance mode on or off still takes a restart.
  • The tests are the first commit. Those with maintenance mode on fail without the second; one with it off checks image requests still reach the image handlers.

Disclosures:

  • Judgement call: the 503 body is written in routes.go with the fields of the handlers' JSON errors (error, status, timestamp), as their helper is private to the handlers package.
  • Judgement call: the test server (newTestServer) now also builds the health check, which the new tests request; no existing test's checks change.
  • Judgement call: TODO.md gets a Completed Steps entry only, as its Next Step is other work.

Model: opus-5-5

Implements https://git.eeqj.de/sneak/pixa/issues/71: turning on `maintenance_mode` now stops image requests; before, only the health check reported it. - One middleware in `internal/server/routes.go`, applied to a route group holding only `/v1/image/` (GET, HEAD) and `/v1/e/`. While the flag is on it answers 503 with `Retry-After` (`MaintenanceRetryAfterSeconds`, 300) and a JSON error body. It calls `Server.MaintenanceMode()`, which had no caller. - The health check stays 200 and reports `"maintenance_mode": true`. The image's Docker `HEALTHCHECK` requests it: a 503 there would make the container unhealthy, and upaas marks a deploy failed when its container is unhealthy. Said in `README.md` and `config.example.yml`. - `/metrics` stays available: it has its own password, touches no image, and is how an operator watches the instance during maintenance. - The login and URL generator pages stay available: they only sign URLs, so an operator can keep working. Worth knowing: - The config is read once at startup, so switching maintenance mode on or off still takes a restart. - The tests are the first commit. Those with maintenance mode on fail without the second; one with it off checks image requests still reach the image handlers. Disclosures: - Judgement call: the 503 body is written in `routes.go` with the fields of the handlers' JSON errors (`error`, `status`, `timestamp`), as their helper is private to the handlers package. - Judgement call: the test server (`newTestServer`) now also builds the health check, which the new tests request; no existing test's checks change. - Judgement call: `TODO.md` gets a Completed Steps entry only, as its Next Step is other work. Model: opus-5-5
clawbot added the needs-review label 2026-09-29 06:25:36 +02:00
clawbot self-assigned this 2026-09-29 06:25:36 +02:00
Author
Collaborator

FAIL

  1. internal/server/maintenance_internal_test.go: nothing tests that with maintenance_mode off the image routes are still served. Both new tests turn it on, and no other test sends an image request through the server's routes, so a middleware that ignored the flag and refused every image request would pass while taking every instance's images down. Acceptable: a test with the flag off that sends an image request through the server and checks it reaches the image handler rather than the 503.

  2. README.md (the maintenance_mode entry, lines 279-284), config.example.yml (lines 16-19), TODO.md (line 37) and the comments at internal/server/routes.go:77-79 and internal/server/maintenance_internal_test.go:78-81 say the image's Docker HEALTHCHECK and upaas both read the health check. upaas never requests it: it reads the container's Docker health, which the HEALTHCHECK sets from the health check's status code alone, as "Running under upaas" in README.md already says. config.example.yml also reads as if both look at the maintenance_mode value. Acceptable: the HEALTHCHECK requests the health check, a 503 there would make the container unhealthy, and upaas marks a deploy failed when its container is unhealthy.

Model: opus-5-5

FAIL 1. `internal/server/maintenance_internal_test.go`: nothing tests that with `maintenance_mode` off the image routes are still served. Both new tests turn it on, and no other test sends an image request through the server's routes, so a middleware that ignored the flag and refused every image request would pass while taking every instance's images down. Acceptable: a test with the flag off that sends an image request through the server and checks it reaches the image handler rather than the 503. 2. `README.md` (the `maintenance_mode` entry, lines 279-284), `config.example.yml` (lines 16-19), `TODO.md` (line 37) and the comments at `internal/server/routes.go:77-79` and `internal/server/maintenance_internal_test.go:78-81` say the image's Docker `HEALTHCHECK` and upaas both read the health check. upaas never requests it: it reads the container's Docker health, which the `HEALTHCHECK` sets from the health check's status code alone, as "Running under upaas" in `README.md` already says. `config.example.yml` also reads as if both look at the `maintenance_mode` value. Acceptable: the `HEALTHCHECK` requests the health check, a 503 there would make the container unhealthy, and upaas marks a deploy failed when its container is unhealthy. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 07:01:58 +02:00
clawbot added 2 commits 2026-09-29 07:14:31 +02:00
Tests ahead of the change: with maintenance_mode on, both image routes
must answer 503 with a Retry-After header and the JSON error body, while
the health check keeps answering 200 and reporting maintenance_mode, and
the login page and /metrics keep answering 200; these fail until the next
commit. With it off, image requests must still reach the image handlers.
The test server now also builds the health check, which these tests
request.

Model: opus-5-5
maintenance_mode was only reported by the health check; every request
was still served. One middleware in routes.go, applied to /v1/image/
and /v1/e/ only, now answers them with 503, a Retry-After of
MaintenanceRetryAfterSeconds and the JSON error body while it is on.
It calls Server.MaintenanceMode(), which had no caller.

The health check stays 200 and reports maintenance_mode: the image's
Docker HEALTHCHECK requests it, a 503 there would make the container
unhealthy, and upaas marks a deploy failed when its container is
unhealthy. The login and URL generator pages and /metrics keep working.
Documented in README.md and config.example.yml.

Model: opus-5-5
clawbot force-pushed issue-71-maintenance-mode from f70723ac4d to a7bb770e60 2026-09-29 07:14:31 +02:00 Compare
Author
Collaborator

Rework for the review at #154 (comment), with both commits rewritten and force-pushed:

  1. Added TestImageRequestsServedWithoutMaintenanceMode to the tests commit: with the flag off, both image routes must answer with the image handlers' own 401 or 400. The repeated image URL became the constant unsignedImagePath, as the linter required.
  2. Corrected in README.md, config.example.yml, TODO.md, the comments in routes.go and the test file, the feature commit's message and the PR body.

Model: opus-5-5

Rework for the review at https://git.eeqj.de/sneak/pixa/pulls/154#issuecomment-105914, with both commits rewritten and force-pushed: 1. Added `TestImageRequestsServedWithoutMaintenanceMode` to the tests commit: with the flag off, both image routes must answer with the image handlers' own 401 or 400. The repeated image URL became the constant `unsignedImagePath`, as the linter required. 2. Corrected in `README.md`, `config.example.yml`, `TODO.md`, the comments in `routes.go` and the test file, the feature commit's message and the PR body. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-29 07:14:54 +02:00
Author
Collaborator

PASS: while maintenance mode is on, both image routes are refused with 503 and every other route is unchanged; the health check wording is accurate.

Model: opus-5-5

PASS: while maintenance mode is on, both image routes are refused with 503 and every other route is unchanged; the health check wording is accurate. Model: opus-5-5
All checks were successful
check / check (push) Successful in 2m54s
This pull request has changes conflicting with the target branch.
  • TODO.md
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue-71-maintenance-mode:issue-71-maintenance-mode
git checkout issue-71-maintenance-mode
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#154