From 30c75992107bf03e0c8207a195ddc8297506b401 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 6 Jun 2026 21:13:37 +0200 Subject: [PATCH] feat(service): add a deprecation signal on the legacy /setup and /mgmt paths (refs #451) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So the eventual 1.x removal of the legacy admin paths can be data-driven (cut a route only once it has gone quiet across real deployments), record usage of the pre-/api paths without changing their behavior. - New DeprecatedRouteMiddleware: after serving, counts the hit keyed by "METHOD " and logs a one-time warning per route pointing at the /api equivalent. Wired onto the legacy /setup and /mgmt mounts only — NOT the /api/* twins, NOT the externally-pinned OAuth callbacks, NOT the Stockholm setup-wizard catch-all. - Counts are exposed in the diagnostic export (deprecated_route_hits), so the shared bundles show whether the old paths are still in use. Legacy paths keep working unchanged. make test-http-client: 95 requests, 0 failed (the suite still exercises /mgmt directly and now emits the one-time warnings). go test + golangci-lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../deprecated_routes_test.go | 47 ++++++++++ cmd/soundtouch-service/main.go | 13 ++- pkg/service/handlers/deprecated_routes.go | 90 +++++++++++++++++++ pkg/service/handlers/handlers_export.go | 14 +-- pkg/service/handlers/server.go | 2 + 5 files changed, 158 insertions(+), 8 deletions(-) create mode 100644 cmd/soundtouch-service/deprecated_routes_test.go create mode 100644 pkg/service/handlers/deprecated_routes.go diff --git a/cmd/soundtouch-service/deprecated_routes_test.go b/cmd/soundtouch-service/deprecated_routes_test.go new file mode 100644 index 0000000..9dbcba4 --- /dev/null +++ b/cmd/soundtouch-service/deprecated_routes_test.go @@ -0,0 +1,47 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/service/datastore" + "github.com/gesellix/bose-soundtouch/pkg/service/handlers" +) + +// TestDeprecatedRouteSignal verifies the legacy admin paths are counted (and the +// new /api/* twins are not), so the diagnostic export can show whether the old +// paths are still in use before they are removed in a future major release. +func TestDeprecatedRouteSignal(t *testing.T) { + ds := datastore.NewDataStore(t.TempDir()) + _ = ds.Initialize() + + server := handlers.NewServer(ds, nil, "http://localhost:8000", true, false, false) + r := setupRouter(server, nil) + + ts := httptest.NewServer(r) + defer ts.Close() + + hit := func(path string) { + resp, err := http.Get(ts.URL + path) + if err != nil { + t.Fatalf("GET %s: %v", path, err) + } + + _ = resp.Body.Close() + } + + hit("/setup/version") // legacy — counted + hit("/setup/version") // legacy again — count increments + hit("/api/setup/version") // new canonical — must NOT be counted + + hits := server.DeprecatedRouteHits() + + if got := hits["GET /setup/version"]; got != 2 { + t.Errorf("legacy GET /setup/version hits = %d, want 2", got) + } + + if _, tracked := hits["GET /api/setup/version"]; tracked { + t.Errorf("/api/setup/version must not be tracked as deprecated; hits=%v", hits) + } +} diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index c8c228c..2c6efcd 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -1375,9 +1375,12 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler) * r.Get("/spotify/callback", server.HandleMgmtSpotifyCallback) r.Get("/amazon/callback", server.HandleMgmtAmazonCallback) - // All other management endpoints require Basic Auth. + // All other management endpoints require Basic Auth. On the legacy mount + // they also carry the deprecation signal (counts + one-time warning); the + // callbacks above are excluded (externally-pinned, not deprecated). r.Group(func(r chi.Router) { r.Use(server.BasicAuthMgmt()) + r.Use(server.DeprecatedRouteMiddleware) mountMgmtAuthed(r) }) }) @@ -1454,7 +1457,13 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler) * } r.Route("/setup", func(r chi.Router) { - mountSetupAPI(r) + // Legacy admin API: same handlers as /api/setup, plus the deprecation + // signal (counts + one-time warning). Scoped to the API routes only — the + // Stockholm wizard catch-all below is frontend, not a deprecated API path. + r.Group(func(r chi.Router) { + r.Use(server.DeprecatedRouteMiddleware) + mountSetupAPI(r) + }) // Serve Stockholm setup wizard pages for paths not matched by the // management API. The Stockholm frontend has a setup/ directory that must diff --git a/pkg/service/handlers/deprecated_routes.go b/pkg/service/handlers/deprecated_routes.go new file mode 100644 index 0000000..1e33b31 --- /dev/null +++ b/pkg/service/handlers/deprecated_routes.go @@ -0,0 +1,90 @@ +package handlers + +import ( + "log" + "net/http" + "sync" + + "github.com/go-chi/chi/v5" +) + +// deprecatedRouteTracker counts hits on legacy admin routes (the /setup and +// /mgmt paths that now also live under /api/*). The counts let the eventual 1.x +// removal be data-driven — a route is only cut once it has gone quiet across +// real deployments — and are surfaced in the diagnostic export. +type deprecatedRouteTracker struct { + mu sync.Mutex + counts map[string]int64 + logged map[string]bool +} + +func newDeprecatedRouteTracker() *deprecatedRouteTracker { + return &deprecatedRouteTracker{ + counts: map[string]int64{}, + logged: map[string]bool{}, + } +} + +// record increments the counter for key ("METHOD pattern") and reports whether +// this is the first time the key was seen, so the caller can log once per route +// per process rather than on every hit. +func (t *deprecatedRouteTracker) record(key string) (firstHit bool) { + t.mu.Lock() + defer t.mu.Unlock() + + t.counts[key]++ + + if !t.logged[key] { + t.logged[key] = true + + return true + } + + return false +} + +func (t *deprecatedRouteTracker) snapshot() map[string]int64 { + t.mu.Lock() + defer t.mu.Unlock() + + out := make(map[string]int64, len(t.counts)) + for k, v := range t.counts { + out[k] = v + } + + return out +} + +// DeprecatedRouteMiddleware records a hit on a legacy admin route and logs a +// one-time warning pointing at the /api equivalent. It never alters the +// response — the legacy paths keep working; this only produces the observability +// the 1.x removal needs. Wire it onto the legacy /setup and /mgmt mounts only, +// not their /api/* twins. +func (s *Server) DeprecatedRouteMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + next.ServeHTTP(w, r) + + rctx := chi.RouteContext(r.Context()) + if rctx == nil { + return + } + + pattern := rctx.RoutePattern() + if pattern == "" { + return + } + + key := r.Method + " " + pattern + if s.deprecatedRoutes.record(key) { + log.Printf("[deprecated-route] %s used by client=%s — use /api%s instead; "+ + "the legacy path still works but is slated for removal in a future major release", + sanitizeLog(key), sanitizeLog(clientHostFromRemoteAddr(r.RemoteAddr)), sanitizeLog(pattern)) + } + }) +} + +// DeprecatedRouteHits returns a snapshot of legacy-route hit counts keyed by +// "METHOD pattern", for the diagnostic export. +func (s *Server) DeprecatedRouteHits() map[string]int64 { + return s.deprecatedRoutes.snapshot() +} diff --git a/pkg/service/handlers/handlers_export.go b/pkg/service/handlers/handlers_export.go index fe2ba3c..c51eeda 100644 --- a/pkg/service/handlers/handlers_export.go +++ b/pkg/service/handlers/handlers_export.go @@ -31,10 +31,11 @@ import ( // alongside it so the maintainer can compare on-disk state with what the // service serves. type diagnosticReport struct { - GeneratedAt string `json:"generated_at"` - ServiceVersion map[string]string `json:"service_version"` - HealthChecks []health.CheckResult `json:"health_checks"` - Devices []deviceDiagnostic `json:"devices"` + GeneratedAt string `json:"generated_at"` + ServiceVersion map[string]string `json:"service_version"` + HealthChecks []health.CheckResult `json:"health_checks"` + Devices []deviceDiagnostic `json:"devices"` + DeprecatedRouteHits map[string]int64 `json:"deprecated_route_hits,omitempty"` } type deviceDiagnostic struct { @@ -768,8 +769,9 @@ func addTarBytes(tw *tar.Writer, name string, data []byte) error { func (s *Server) buildDiagnosticReport(redirectCfgs map[string]*redirectConfig) diagnosticReport { report := diagnosticReport{ - GeneratedAt: time.Now().UTC().Format(time.RFC3339), - ServiceVersion: buildVersionInfo(), + GeneratedAt: time.Now().UTC().Format(time.RFC3339), + ServiceVersion: buildVersionInfo(), + DeprecatedRouteHits: s.DeprecatedRouteHits(), } if s.healthRegistry != nil { diff --git a/pkg/service/handlers/server.go b/pkg/service/handlers/server.go index 58e1d9d..eb43280 100644 --- a/pkg/service/handlers/server.go +++ b/pkg/service/handlers/server.go @@ -55,6 +55,7 @@ type Server struct { dnsDiscovery *discovery.DNSDiscovery authProbes *authProbeRegistry authProbeTimeoutOverride time.Duration // zero means use defaultAuthProbeTimeout; injectable for tests + deprecatedRoutes *deprecatedRouteTracker Version string Commit string Date string @@ -133,6 +134,7 @@ func NewServer(ds *datastore.DataStore, sm *setup.Manager, serverURL string, red peerObserver: newPeerObserver(), healthRegistry: health.NewRegistry(), authProbes: newAuthProbeRegistry(defaultAuthProbeTTL), + deprecatedRoutes: newDeprecatedRouteTracker(), } health.RegisterSourcesXMLPresent(s.healthRegistry, ds)