From de5b599455d1eff5004b1a6c3022103fb009b5cb Mon Sep 17 00:00:00 2001 From: Gabriel Radureau Date: Tue, 5 May 2026 10:35:03 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat(server):=20api.v2=5Fenabled=20?= =?UTF-8?q?hot-reload=20via=20middleware=20gate=20(ADR-0023=20Phase=204)?= =?UTF-8?q?=20(#56)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gabriel Radureau Co-committed-by: Gabriel Radureau --- adr/0023-config-hot-reloading.md | 2 +- adr/README.md | 2 +- pkg/server/server.go | 35 ++++++++++--- pkg/server/v2_gate_test.go | 84 ++++++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 pkg/server/v2_gate_test.go diff --git a/adr/0023-config-hot-reloading.md b/adr/0023-config-hot-reloading.md index f75923b..625548b 100644 --- a/adr/0023-config-hot-reloading.md +++ b/adr/0023-config-hot-reloading.md @@ -1,6 +1,6 @@ # Config Hot Reloading Strategy -**Status:** Phase 1+2+3 Implemented (2026-05-05). Hot-reloadable fields: `logging.level`, `auth.jwt.ttl`, `telemetry.sampler.type`, `telemetry.sampler.ratio`. Plumbing: `Config.WatchAndApply` in `pkg/config/config.go`, `ReconfigureTracerProvider` in `pkg/telemetry/telemetry.go`, sampler reconfigure callback wired in `pkg/server/server.go Run`. Phase 2 also fixed a pre-existing bug where the hardcoded 24h TTL ignored `auth.jwt.ttl` from config. Remaining field `api.v2_enabled` is **deferred**: hot-reloading routing requires either an always-register-with-middleware-gate refactor of the chi router or an atomic router swap — different complexity class, separate ADR if reopened. +**Status:** Implemented — all 4 phases shipped (2026-05-05). Hot-reloadable fields: `logging.level` (Phase 1), `auth.jwt.ttl` (Phase 2), `telemetry.sampler.type` + `telemetry.sampler.ratio` (Phase 3), `api.v2_enabled` (Phase 4). Plumbing: `Config.WatchAndApply` in `pkg/config/config.go` is the single entry point. Phase 2 fixed a pre-existing bug where hardcoded 24h TTL ignored `auth.jwt.ttl`. Phase 4 chose the **always-register-with-middleware-gate** approach: v2 routes are now ALWAYS registered, and `Server.v2EnabledGate` middleware reads the live config on every request (returns 404 + JSON body when disabled). No router rebuild needed for the flag flip. 3 unit tests in `pkg/server/v2_gate_test.go` cover blocked-when-disabled / passes-when-enabled / hot-reload-mid-life-of-same-Server. **Authors:** Gabriel Radureau, AI Agent **Date:** 2026-04-05 **Last Updated:** 2026-05-05 diff --git a/adr/README.md b/adr/README.md index 7cba2df..5461ecf 100644 --- a/adr/README.md +++ b/adr/README.md @@ -26,7 +26,7 @@ This directory contains the Architecture Decision Records (ADRs) for the dance-l | [0020](0020-docker-build-strategy.md) | Docker Build Strategy: Traditional vs Buildx | Accepted | | [0021](0021-jwt-secret-retention-policy.md) | JWT Secret Retention Policy | Implemented | | [0022](0022-rate-limiting-cache-strategy.md) | Rate Limiting and Cache Strategy | Implemented (Phase 1) | -| [0023](0023-config-hot-reloading.md) | Config Hot Reloading Strategy | Implemented (Phase 1+2+3) | +| [0023](0023-config-hot-reloading.md) | Config Hot Reloading Strategy | Implemented | | [0024](0024-bdd-test-organization-and-isolation.md) | BDD Test Organization and Isolation Strategy | Implemented | | [0025](0025-bdd-scenario-isolation-strategies.md) | BDD Scenario Isolation Strategies | Implemented | | [0026](0026-composite-info-endpoint.md) | Composite Info Endpoint vs Separate Calls | Implemented | diff --git a/pkg/server/server.go b/pkg/server/server.go index 5f65e0f..4d5b209 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -193,13 +193,15 @@ func (s *Server) setupRoutes() { r.Post("/cache/flush", s.handleAdminCacheFlush) }) - // Register v2 routes if enabled - if s.config.GetV2Enabled() { - s.router.Route("/api/v2", func(r chi.Router) { - r.Use(s.getAllMiddlewares()...) - s.registerApiV2Routes(r) - }) - } + // Register v2 routes ALWAYS (ADR-0023 Phase 4 hot-reload). The + // v2EnabledGate middleware checks the live config on every request + // and returns 404 when api.v2_enabled is false. This lets the flag + // be flipped via config hot-reload without a router rebuild. + s.router.Route("/api/v2", func(r chi.Router) { + r.Use(s.getAllMiddlewares()...) + r.Use(s.v2EnabledGate) + s.registerApiV2Routes(r) + }) // Add Swagger UI with embedded spec // Serve the embedded swagger.json file @@ -269,6 +271,25 @@ func (s *Server) registerApiV2Routes(r chi.Router) { }) } +// v2EnabledGate is the middleware that gates the /api/v2/* subtree on the +// live api.v2_enabled config value (ADR-0023 Phase 4 hot-reload). When +// disabled, returns 404 with the same body shape as a missing route would +// emit, so clients see "v2 doesn't exist" rather than "v2 is forbidden". +// +// Flipping the config at runtime via Config.WatchAndApply takes effect on +// the next request — no router rebuild, no restart. +func (s *Server) v2EnabledGate(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !s.config.GetV2Enabled() { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"not_found","message":"v2 API is currently disabled"}`)) + return + } + next.ServeHTTP(w, r) + }) +} + // getAllMiddlewares returns all middleware including OpenTelemetry if enabled func (s *Server) getAllMiddlewares() []func(http.Handler) http.Handler { middlewares := []func(http.Handler) http.Handler{ diff --git a/pkg/server/v2_gate_test.go b/pkg/server/v2_gate_test.go new file mode 100644 index 0000000..4eb7102 --- /dev/null +++ b/pkg/server/v2_gate_test.go @@ -0,0 +1,84 @@ +package server + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "dance-lessons-coach/pkg/config" + + "github.com/stretchr/testify/assert" +) + +// TestV2EnabledGate_BlocksWhenDisabled verifies the ADR-0023 Phase 4 +// hot-reload security property: when api.v2_enabled is false, ANY request +// to /api/v2/* returns 404 with a JSON body, not a 200, not a panic. +func TestV2EnabledGate_BlocksWhenDisabled(t *testing.T) { + cfg := &config.Config{} + cfg.API.V2Enabled = false // explicit, even though it is the zero value + s := NewServer(cfg, context.Background()) + + req := httptest.NewRequest(http.MethodPost, "/api/v2/greet", strings.NewReader(`{"name":"world"}`)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + + s.router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusNotFound, w.Code, "v2 disabled should 404") + assert.Contains(t, w.Body.String(), "v2 API is currently disabled", + "response should explain why") + assert.Equal(t, "application/json", w.Header().Get("Content-Type")) +} + +// TestV2EnabledGate_PassesWhenEnabled verifies the gate lets requests +// through to the actual v2 handler when api.v2_enabled is true. We use +// a v2 endpoint that exists and responds with a 2xx so we can assert +// "got past the gate, hit the handler". +func TestV2EnabledGate_PassesWhenEnabled(t *testing.T) { + cfg := &config.Config{} + cfg.API.V2Enabled = true + s := NewServer(cfg, context.Background()) + + req := httptest.NewRequest(http.MethodPost, "/api/v2/greet", strings.NewReader(`{"name":"world"}`)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + + s.router.ServeHTTP(w, req) + + // 200 = v2 handler executed. Anything other than 404 with the gate's + // message proves the gate let the request through. + assert.NotEqual(t, http.StatusNotFound, w.Code, "v2 enabled should not return 404 from gate") + assert.NotContains(t, w.Body.String(), "v2 API is currently disabled", + "gate message must NOT appear when enabled") +} + +// TestV2EnabledGate_HotReloadEffect simulates the ADR-0023 Phase 4 +// scenario: the same Server (same router) sees opposite responses +// before and after a config flip — proving the gate reads the live +// config rather than a snapshot from setupRoutes. +func TestV2EnabledGate_HotReloadEffect(t *testing.T) { + cfg := &config.Config{} + cfg.API.V2Enabled = false + s := NewServer(cfg, context.Background()) + + // Round 1: disabled + req1 := httptest.NewRequest(http.MethodPost, "/api/v2/greet", strings.NewReader(`{"name":"a"}`)) + req1.Header.Set("Content-Type", "application/json") + w1 := httptest.NewRecorder() + s.router.ServeHTTP(w1, req1) + assert.Equal(t, http.StatusNotFound, w1.Code, "round 1 (disabled) should 404") + + // Flip the config. In production, Config.WatchAndApply does this on + // file change; here we set the field directly to simulate the result. + cfg.API.V2Enabled = true + + // Round 2: enabled — same Server, same router, just the config flipped + req2 := httptest.NewRequest(http.MethodPost, "/api/v2/greet", strings.NewReader(`{"name":"b"}`)) + req2.Header.Set("Content-Type", "application/json") + w2 := httptest.NewRecorder() + s.router.ServeHTTP(w2, req2) + assert.NotEqual(t, http.StatusNotFound, w2.Code, "round 2 (enabled) should NOT 404") + assert.NotContains(t, w2.Body.String(), "v2 API is currently disabled") +}