From 9a57cdf454a91ab78247c845773fa2d453f1a80c Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 04:13:23 +0000 Subject: [PATCH 1/2] Test that maintenance mode refuses image requests (closes #71) 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 --- .../server/login_rate_limit_internal_test.go | 10 +- internal/server/maintenance_internal_test.go | 169 ++++++++++++++++++ 2 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 internal/server/maintenance_internal_test.go diff --git a/internal/server/login_rate_limit_internal_test.go b/internal/server/login_rate_limit_internal_test.go index c37d86e..f5d8c81 100644 --- a/internal/server/login_rate_limit_internal_test.go +++ b/internal/server/login_rate_limit_internal_test.go @@ -18,6 +18,7 @@ import ( "sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/globals" "sneak.berlin/go/pixa/internal/handlers" + "sneak.berlin/go/pixa/internal/healthcheck" "sneak.berlin/go/pixa/internal/logger" "sneak.berlin/go/pixa/internal/middleware" ) @@ -71,8 +72,15 @@ func newTestServer(t *testing.T) *Server { t.Fatalf("database.New() error = %v", err) } + hc, err := healthcheck.New(lc, healthcheck.Params{ + Globals: &globals.Globals{}, Config: cfg, Logger: log, Database: db, + }) + if err != nil { + t.Fatalf("healthcheck.New() error = %v", err) + } + h, err := handlers.New(lc, handlers.Params{ - Logger: log, Database: db, Config: cfg, + Logger: log, Healthcheck: hc, Database: db, Config: cfg, }) if err != nil { t.Fatalf("handlers.New() error = %v", err) diff --git a/internal/server/maintenance_internal_test.go b/internal/server/maintenance_internal_test.go new file mode 100644 index 0000000..96b7272 --- /dev/null +++ b/internal/server/maintenance_internal_test.go @@ -0,0 +1,169 @@ +package server + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strconv" + "testing" + + "sneak.berlin/go/pixa/internal/healthcheck" +) + +// unsignedImagePath is an image URL that carries no signature. +const unsignedImagePath = "/v1/image/cdn.example.com/cat.jpg/100x100.jpeg" + +// TestMaintenanceModeRefusesImageRequests verifies that while maintenance +// mode is on, both image routes answer 503 Service Unavailable with a +// Retry-After header and the JSON error body the image handlers send. +func TestMaintenanceModeRefusesImageRequests(t *testing.T) { + t.Parallel() + + s := newTestServer(t) + s.config.MaintenanceMode = true + + requests := []struct { + method string + path string + }{ + {http.MethodGet, unsignedImagePath}, + {http.MethodHead, unsignedImagePath}, + {http.MethodGet, "/v1/e/token/cat.jpg"}, + } + + for _, tc := range requests { + t.Run(tc.method+" "+tc.path, func(t *testing.T) { + t.Parallel() + + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), tc.method, tc.path, nil)) + t.Logf("status %d, body %s", rec.Code, rec.Body.String()) + + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status = %d, want %d", + rec.Code, http.StatusServiceUnavailable) + } + + retryAfter := rec.Header().Get("Retry-After") + + seconds, err := strconv.Atoi(retryAfter) + if err != nil || seconds <= 0 { + t.Errorf("Retry-After = %q, want a positive number of seconds", + retryAfter) + } + + // A HEAD response carries no body. + if tc.method == http.MethodHead { + return + } + + var body struct { + Error string `json:"error"` + Status int `json:"status"` + Timestamp string `json:"timestamp"` + } + + err = json.NewDecoder(rec.Body).Decode(&body) + if err != nil { + t.Fatalf("body is not JSON: %v", err) + } + + if body.Error == "" || body.Status != http.StatusServiceUnavailable || + body.Timestamp == "" { + t.Errorf("body = %+v, want an error, status %d and a timestamp", + body, http.StatusServiceUnavailable) + } + }) + } +} + +// TestImageRequestsServedWithoutMaintenanceMode verifies that while +// maintenance mode is off, image requests reach the image handlers instead +// of the 503. The handlers refuse an unsigned image URL with 401 and a token +// they cannot decrypt with 400, so either status shows a request got through. +func TestImageRequestsServedWithoutMaintenanceMode(t *testing.T) { + t.Parallel() + + s := newTestServer(t) + s.config.MaintenanceMode = false + + requests := []struct { + method string + path string + want int + }{ + {http.MethodGet, unsignedImagePath, http.StatusUnauthorized}, + {http.MethodHead, unsignedImagePath, http.StatusUnauthorized}, + {http.MethodGet, "/v1/e/token/cat.jpg", http.StatusBadRequest}, + } + + for _, tc := range requests { + t.Run(tc.method+" "+tc.path, func(t *testing.T) { + t.Parallel() + + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), tc.method, tc.path, nil)) + t.Logf("status %d, body %s", rec.Code, rec.Body.String()) + + if rec.Code != tc.want { + t.Errorf("status = %d, want %d from the image handler", + rec.Code, tc.want) + } + }) + } +} + +// TestMaintenanceModeKeepsOtherRoutes verifies that while maintenance mode +// is on, the health check still answers 200 and reports it, and the login +// page and /metrics still answer 200. The image's Docker HEALTHCHECK +// requests the health check: a 503 there would make the container +// unhealthy, and upaas marks a deploy failed when its container is +// unhealthy. +func TestMaintenanceModeKeepsOtherRoutes(t *testing.T) { + t.Parallel() + + s := newTestServer(t) + s.config.MaintenanceMode = true + + // /metrics is routed only when its username is set. + s.config.MetricsUsername = "metrics" + s.config.MetricsPassword = "metrics-password" + s.SetupRoutes() + + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext(t.Context(), + http.MethodGet, "/.well-known/healthcheck.json", nil)) + t.Logf("health check status %d, body %s", rec.Code, rec.Body.String()) + + if rec.Code != http.StatusOK { + t.Fatalf("health check status = %d, want %d", rec.Code, http.StatusOK) + } + + var health healthcheck.Response + + err := json.NewDecoder(rec.Body).Decode(&health) + if err != nil || !health.Maintenance { + t.Errorf("health check maintenance_mode = %v (error %v), want true", + health.Maintenance, err) + } + + rec = httptest.NewRecorder() + s.ServeHTTP(rec, clientRequest(t, http.MethodGet, nil, firstClient, "")) + + if rec.Code != http.StatusOK { + t.Errorf("login page status = %d, want %d", rec.Code, http.StatusOK) + } + + req := httptest.NewRequestWithContext(t.Context(), + http.MethodGet, "/metrics", nil) + req.SetBasicAuth(s.config.MetricsUsername, s.config.MetricsPassword) + + rec = httptest.NewRecorder() + s.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("/metrics status = %d, want %d", rec.Code, http.StatusOK) + } +} -- 2.54.0 From c9e24be2e28e6db8bacca35f21670ac9b157c8bc Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 04:19:25 +0000 Subject: [PATCH 2/2] Answer image requests with 503 in maintenance mode (closes #71) 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 --- README.md | 9 ++++++- TODO.md | 8 ++++++ config.example.yml | 6 +++++ internal/server/routes.go | 57 +++++++++++++++++++++++++++++++++------ 4 files changed, 71 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 001d863..808d192 100644 --- a/README.md +++ b/README.md @@ -245,7 +245,7 @@ variables set by the file's `env:` section are checked the same way. | `PIXA_METRICS_PASSWORD` | `metrics.password` | Password for `/metrics`; set together with the username | | `PIXA_SENTRY_DSN` | `sentry_dsn` | Sentry DSN for error reporting; empty disables it | | `PIXA_DEBUG` | `debug` | Debug logging and plain-HTTP local development; default `false` | -| `PIXA_MAINTENANCE_MODE` | `maintenance_mode` | Maintenance flag reported by the health check; default `false` | +| `PIXA_MAINTENANCE_MODE` | `maintenance_mode` | Answer image requests with 503; the health check stays 200; default `false` | Key settings in more detail: @@ -306,6 +306,13 @@ Key settings in more detail: container's CPU limit. A request that finds all of them in use waits up to 10 seconds for one to free up; if none does, and `downstream_timeout` has not ended first, it is answered 503 the same way +- `maintenance_mode` — while `true`, the image routes (`/v1/image/` and + `/v1/e/`) answer every request with 503, a `Retry-After` header and a JSON + error body. The health check (`/.well-known/healthcheck.json`) still answers + 200 and reports `"maintenance_mode": true`. It stays 200 because 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 See `config.example.yml` for all options with defaults. diff --git a/TODO.md b/TODO.md index 8276df8..4674052 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,14 @@ P2: security: referer blacklist # Completed Steps +- 2026-09-29 maintenance mode refuses image requests (closes #71): while + `maintenance_mode` is on, `/v1/image/` and `/v1/e/` answer 503 with a + `Retry-After` header and the JSON error body, from one middleware in + `internal/server/routes.go`; the health check stays 200 and reports + `maintenance_mode`, as the image's Docker `HEALTHCHECK` requests it 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`. - 2026-09-29 bound concurrent image processing and upstream fetches (closes #64): `max_concurrent_processing` (default the number of CPUs pixa can use) limits the images decoded and encoded at once, and `upstream_connections` diff --git a/config.example.yml b/config.example.yml index d64d77e..ebbc833 100644 --- a/config.example.yml +++ b/config.example.yml @@ -16,6 +16,12 @@ # Server settings port: 8080 debug: false + +# While true, the image routes (/v1/image/ and /v1/e/) answer every request +# with 503 and a Retry-After header. The health check keeps answering 200 and +# reports maintenance_mode as true. It stays 200 because 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. maintenance_mode: false # Data directory for SQLite database and cache files diff --git a/internal/server/routes.go b/internal/server/routes.go index 4623b75..ade30ee 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -1,7 +1,9 @@ package server import ( + "encoding/json" "net/http" + "strconv" "time" sentryhttp "github.com/getsentry/sentry-go/http" @@ -17,6 +19,10 @@ import ( // make per minute; the next is refused with 429 Too Many Requests. const LoginAttemptsPerMinute = 5 +// MaintenanceRetryAfterSeconds is the Retry-After, in seconds, sent with +// the 503 that the image routes answer while maintenance mode is on. +const MaintenanceRetryAfterSeconds = 300 + // SetupRoutes configures all HTTP routes. func (s *Server) SetupRoutes() { s.router = chi.NewRouter() @@ -68,15 +74,23 @@ func (s *Server) SetupRoutes() { s.router.Get("/logout", s.h.HandleLogout()) - // Main image proxy route - // /v1/image///x. - s.router.Get("/v1/image/*", s.h.HandleImage()) - s.router.Head("/v1/image/*", s.h.HandleImage()) + // Image routes, refused while maintenance mode is on. Only these: the + // image's Docker HEALTHCHECK requests the health check, a 503 there + // would make the container unhealthy, and upaas marks a deploy failed + // when its container is unhealthy. + s.router.Group(func(r chi.Router) { + r.Use(s.refuseDuringMaintenance) - // Encrypted image URL route - // The trailing filename (e.g., /img.jpg) is ignored but helps - // browsers with content type - s.router.Get("/v1/e/{token}/*", s.h.HandleImageEnc()) + // Main image proxy route + // /v1/image///x. + r.Get("/v1/image/*", s.h.HandleImage()) + r.Head("/v1/image/*", s.h.HandleImage()) + + // Encrypted image URL route + // The trailing filename (e.g., /img.jpg) is ignored but helps + // browsers with content type + r.Get("/v1/e/{token}/*", s.h.HandleImageEnc()) + }) // Metrics endpoint with auth if s.config.MetricsUsername != "" { @@ -86,3 +100,30 @@ func (s *Server) SetupRoutes() { }) } } + +// refuseDuringMaintenance answers a request with 503 Service Unavailable, +// a Retry-After header and a JSON error body while maintenance mode is on, +// and passes it on otherwise. The body has the fields of the JSON errors +// the image handlers send. +func (s *Server) refuseDuringMaintenance(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !s.MaintenanceMode() { + next.ServeHTTP(w, r) + + return + } + + w.Header().Set("Retry-After", strconv.Itoa(MaintenanceRetryAfterSeconds)) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusServiceUnavailable) + + err := json.NewEncoder(w).Encode(map[string]any{ + "error": "down for maintenance, try again later", + "status": http.StatusServiceUnavailable, + "timestamp": time.Now().UTC().Format(time.RFC3339), + }) + if err != nil { + s.log.Error("json encode error", "error", err) + } + }) +} -- 2.54.0