refactor(api): stabilize router settings and admin route wiring (#155)

This commit is contained in:
n30nex
2026-09-19 21:10:45 -04:00
committed by GitHub
parent 6aba17c75e
commit 951b79bf6e
12 changed files with 128 additions and 25 deletions
+18
View File
@@ -100,6 +100,24 @@ sqlc generate
## API changes
### Router configuration and optional admin endpoints
Keep `router.New`'s signature stable. Pass startup settings by name in
`router.Options`; a new optional setting must not require unrelated callers and
tests to supply another positional argument.
An optional admin feature owns its subrouter, dependencies and HTTP methods.
Register it once through `Options.AdminRoutes` in the startup wiring, using a
literal path such as `/accounts`. The parent admin router applies bearer
authentication to the entire subtree; the feature must not mount itself beside
that boundary. `/config` remains owned by the shared admin router. No global
registration or `init` side effects are needed.
When concurrent PRs touch the same startup wiring, stack them in an explicit
merge order and link each feature-only comparison. Rebase the stack together
after prerequisite merges, regenerate Swagger rather than hand-merging it, and
compile the combined result even when GitHub reports it as mergeable.
Any new or modified REST endpoint must have swagger annotations and regenerated
docs:
+5 -1
View File
@@ -303,7 +303,11 @@ func main() {
// Wrap after wiring cache invalidators and cleanup callbacks to the actual
// CachedReader. Only response projections receive the geographic annotation.
reader = api.WithLocalBorders(reader, localBorders)
r := router.New(h, reader, []*ingest.Worker{broker1, broker2}, resolved.MaxConnsPerIP, resolved.MaxConnectsPerMinute, cfg.CORS, cfg.Server, cfg.Auth, resolved.RateLimit)
r := router.New(h, reader, []*ingest.Worker{broker1, broker2}, router.Options{
MaxConnsPerIP: resolved.MaxConnsPerIP,
MaxConnectsPerMinute: resolved.MaxConnectsPerMinute,
CORS: cfg.CORS, Server: cfg.Server, Auth: cfg.Auth, RateLimit: resolved.RateLimit,
})
srv := &http.Server{
Addr: addr,
+4 -1
View File
@@ -17,10 +17,13 @@ import (
// AdminRouter mounts operator endpoints. Its caller must wrap the
// entire subrouter with BearerAuth, including unknown paths and methods.
func AdminRouter(runtime *mw.RuntimeConfig) http.Handler {
func AdminRouter(runtime *mw.RuntimeConfig, routes map[string]http.Handler) http.Handler {
r := chi.NewRouter()
r.Get("/config", getAdminConfig(runtime))
r.Put("/config", updateAdminConfig(runtime))
for path, handler := range routes {
r.Mount(path, handler)
}
return r
}
+1 -1
View File
@@ -32,7 +32,7 @@ func TestAdminConfig(t *testing.T) {
want = map[string]any{"allowed_origins": []any{"https://example.test"}, "allowed_methods": []any{"GET"},
"allowed_headers": []any{"Authorization", "X-Operator"}, "allow_credentials": true, "max_age": float64(600)}
}
handler := New(nil, nil, []*ingest.Worker{{}, {}}, 5, 1000, cfg, config.ServerConfig{}, config.AuthConfig{APIKey: key}, config.ResolvedRateLimitConfig{})
handler := New(nil, nil, []*ingest.Worker{{}, {}}, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, CORS: cfg, Auth: config.AuthConfig{APIKey: key}})
if custom {
cfg.AllowedOrigins[0] = "https://changed.test"
cfg.AllowedMethods[0] = "DELETE"
+54
View File
@@ -0,0 +1,54 @@
// Copyright 2026 Beacon Contributors
// SPDX-License-Identifier: AGPL-3.0-or-later
package router
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/MeshCore-Beacon/beacon-server/internal/config"
)
func TestAdditionalAdminRoutesStayAuthenticated(t *testing.T) {
for _, key := range []string{"", "fixture-key"} {
calls := 0
opts := Options{Auth: config.AuthConfig{APIKey: key}, AdminRoutes: map[string]http.Handler{
"/feature": http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
calls++
w.WriteHeader(http.StatusNoContent)
}),
}}
handler := New(nil, nil, nil, opts)
// Registration happens at startup; later edits to the caller's map must
// not replace the registered handler or create a new unprotected route.
delete(opts.AdminRoutes, "/feature")
for _, path := range []string{"/api/v1/admin/feature", "/api/v1/admin/feature/nested"} {
for _, method := range []string{"GET", "POST", "DELETE"} {
for _, token := range []string{"", "wrong", "fixture-key"} {
request := httptest.NewRequest(method, path, nil)
if token != "" {
request.Header.Set("Authorization", "Bearer "+token)
}
response := httptest.NewRecorder()
before := calls
handler.ServeHTTP(response, request)
want := http.StatusServiceUnavailable
if key != "" {
want = http.StatusUnauthorized
if token == key {
want = http.StatusNoContent
}
}
if response.Code != want || response.Header().Get("Cache-Control") != "no-store" {
t.Fatalf("%s %s: status=%d want=%d", method, path, response.Code, want)
}
if (calls != before) != (want == http.StatusNoContent) {
t.Fatal("authentication boundary bypassed")
}
}
}
}
}
}
+1 -1
View File
@@ -13,7 +13,7 @@ import (
func TestAdminAuthBoundary(t *testing.T) {
for _, key := range []string{"", "synthetic-test-key"} {
handler := New(nil, nil, nil, 5, 1000, config.CORSConfig{}, config.ServerConfig{}, config.AuthConfig{APIKey: key}, config.ResolvedRateLimitConfig{})
handler := New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, Auth: config.AuthConfig{APIKey: key}})
for _, path := range []string{"/api/v1/admin", "/api/v1/admin/", "/api/v1/admin/config", "/api/v1/admin//config"} {
for _, method := range []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"} {
for _, token := range []string{"", "wrong", "synthetic-test-key"} {
+25
View File
@@ -0,0 +1,25 @@
// Copyright 2026 Beacon Contributors
// SPDX-License-Identifier: AGPL-3.0-or-later
package router
import (
"net/http"
"github.com/MeshCore-Beacon/beacon-server/internal/config"
)
// Options names the startup settings used by the router. Adding an optional
// route does not change New's signature or require unrelated callers to change.
type Options struct {
MaxConnsPerIP int
MaxConnectsPerMinute int
CORS config.CORSConfig
Server config.ServerConfig
Auth config.AuthConfig
RateLimit config.ResolvedRateLimitConfig
// AdminRoutes mounts operator-only subrouters at literal paths such as
// "/accounts". Each feature owns its dependencies, methods and handlers.
// The router applies authentication to every path and method in this group.
AdminRoutes map[string]http.Handler
}
+6 -6
View File
@@ -22,7 +22,7 @@ import (
)
func TestDefaultAPIRateLimit(t *testing.T) {
handler := New(nil, nil, nil, 5, 1000, config.CORSConfig{}, config.ServerConfig{}, config.AuthConfig{}, config.Resolve(&config.Config{}).RateLimit)
handler := New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, RateLimit: config.Resolve(&config.Config{}).RateLimit})
for i := 0; i <= 300; i++ {
request := httptest.NewRequest(http.MethodGet, "/api/v1/brokers", nil)
request.RemoteAddr = "198.51.100.1:1234"
@@ -56,7 +56,7 @@ func TestAPIRateLimitContractAndLogging(t *testing.T) {
slog.SetDefault(slog.New(slog.NewJSONHandler(&logs, nil)))
t.Cleanup(func() { slog.SetDefault(previous); log.SetOutput(writer); log.SetFlags(flags) })
proxy := config.ServerConfig{TrustedProxies: []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")}}
handler := New(nil, nil, nil, 5, 1000, config.CORSConfig{}, proxy, config.AuthConfig{}, config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 2, Burst: 10})
handler := New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, Server: proxy, RateLimit: config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 2, Burst: 10}})
for _, path := range []string{"/api/v1/brokers", "/api/v1/packets?limit=0"} {
response := rateRequest(handler, path, "127.0.0.1:1234", "198.51.100.25")
if response.Code != http.StatusOK && response.Code != http.StatusBadRequest {
@@ -111,7 +111,7 @@ func TestAPIRateLimitClientIdentity(t *testing.T) {
if tc.trusted {
proxy.TrustedProxies = []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")}
}
handler := New(nil, nil, nil, 5, 1000, config.CORSConfig{}, proxy, config.AuthConfig{}, config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 1, Burst: 1})
handler := New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, Server: proxy, RateLimit: config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 1, Burst: 1}})
if response := rateRequest(handler, "/api/v1/brokers", tc.firstPeer, tc.firstHeader); response.Code != http.StatusOK {
t.Fatalf("first client: %d", response.Code)
}
@@ -128,7 +128,7 @@ func TestAPIRateLimitClientIdentity(t *testing.T) {
func TestAPIRateLimitWindowsAndExclusions(t *testing.T) {
synctest.Test(t, func(t *testing.T) {
handler := New(nil, nil, nil, 5, 1000, config.CORSConfig{}, config.ServerConfig{}, config.AuthConfig{}, config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 4, Burst: 2})
handler := New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, RateLimit: config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 4, Burst: 2}})
request := func() *httptest.ResponseRecorder {
return rateRequest(handler, "/api/v1/brokers", "198.51.100.1:1", "")
}
@@ -164,7 +164,7 @@ func TestAPIRateLimitWindowsAndExclusions(t *testing.T) {
t.Fatal("minute budget did not recover")
}
})
handler := New(nil, nil, nil, 5, 1000, config.CORSConfig{}, config.ServerConfig{}, config.AuthConfig{}, config.ResolvedRateLimitConfig{Enabled: false, RequestsPerMinute: 1, Burst: 1})
handler := New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, RateLimit: config.ResolvedRateLimitConfig{Enabled: false, RequestsPerMinute: 1, Burst: 1}})
for range 5 {
if response := rateRequest(handler, "/api/v1/brokers", "198.51.100.1:1", ""); response.Code != http.StatusOK || response.Header().Get("Retry-After") != "" {
t.Fatal("disabled limiter still applied")
@@ -173,7 +173,7 @@ func TestAPIRateLimitWindowsAndExclusions(t *testing.T) {
}
func TestAPIRateLimitConcurrentRequests(t *testing.T) {
handler := New(nil, nil, nil, 5, 1000, config.CORSConfig{}, config.ServerConfig{}, config.AuthConfig{}, config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 5, Burst: 5})
handler := New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, RateLimit: config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 5, Burst: 5}})
var accepted, rejected atomic.Int32
var group sync.WaitGroup
for range 20 {
+11 -12
View File
@@ -15,7 +15,6 @@ import (
"github.com/MeshCore-Beacon/beacon-server/internal/api"
"github.com/MeshCore-Beacon/beacon-server/internal/api/handlers"
mw "github.com/MeshCore-Beacon/beacon-server/internal/api/middleware"
"github.com/MeshCore-Beacon/beacon-server/internal/config"
"github.com/MeshCore-Beacon/beacon-server/internal/hub"
"github.com/MeshCore-Beacon/beacon-server/internal/ingest"
"github.com/MeshCore-Beacon/beacon-server/internal/ws"
@@ -38,35 +37,35 @@ import (
// /stats → stats subrouter
//
// Admin endpoints require a configured bearer key.
func New(h *hub.Hub, reader api.Reader, workers []*ingest.Worker, maxConnsPerIP, maxConnectsPerMinute int, corsCfg config.CORSConfig, serverCfg config.ServerConfig, authCfg config.AuthConfig, rateLimitCfg config.ResolvedRateLimitConfig) http.Handler {
func New(h *hub.Hub, reader api.Reader, workers []*ingest.Worker, opts Options) http.Handler {
r := chi.NewRouter()
// ── CORS ─────────────────────────────────────────────────────────────────
allowedOrigins := corsCfg.AllowedOrigins
allowedOrigins := opts.CORS.AllowedOrigins
if len(allowedOrigins) == 0 {
allowedOrigins = []string{"*"}
}
allowedMethods := corsCfg.AllowedMethods
allowedMethods := opts.CORS.AllowedMethods
if len(allowedMethods) == 0 {
allowedMethods = []string{"GET", "HEAD", "OPTIONS"}
}
allowedHeaders := corsCfg.AllowedHeaders
allowedHeaders := opts.CORS.AllowedHeaders
if len(allowedHeaders) == 0 {
allowedHeaders = []string{"Accept", "Authorization", "Content-Type"}
}
maxAge := corsCfg.MaxAge
maxAge := opts.CORS.MaxAge
if maxAge == 0 {
maxAge = 300
}
// Capture only values used by this router. Do not retain Config, credentials
// or caller-owned slices in the admin response.
adminConfig := api.AdminConfig{
Auth: api.AdminAuthConfig{Configured: authCfg.APIKey != ""},
Auth: api.AdminAuthConfig{Configured: opts.Auth.APIKey != ""},
CORS: api.AdminCORSConfig{
AllowedOrigins: append([]string{}, allowedOrigins...),
AllowedMethods: append([]string{}, allowedMethods...),
AllowedHeaders: append([]string{}, allowedHeaders...),
AllowCredentials: corsCfg.AllowCredentials,
AllowCredentials: opts.CORS.AllowCredentials,
MaxAge: maxAge,
},
Ingest: api.AdminIngestConfig{BrokerCount: len(workers)},
@@ -76,7 +75,7 @@ func New(h *hub.Hub, reader api.Reader, workers []*ingest.Worker, maxConnsPerIP,
// ── Global middleware ────────────────────────────────────────────────────
r.Use(middleware.RequestID)
r.Use(mw.TrustedProxyIP(serverCfg.TrustedProxies))
r.Use(mw.TrustedProxyIP(opts.Server.TrustedProxies))
r.Use(mw.RequestLogger)
r.Use(middleware.Recoverer)
r.Use(middleware.CleanPath)
@@ -92,11 +91,11 @@ func New(h *hub.Hub, reader api.Reader, workers []*ingest.Worker, maxConnsPerIP,
))
// ── WebSocket ────────────────────────────────────────────────────────────
r.Get("/ws", ws.Handler(h, reader, maxConnsPerIP, maxConnectsPerMinute))
r.Get("/ws", ws.Handler(h, reader, opts.MaxConnsPerIP, opts.MaxConnectsPerMinute))
// ── Public REST API (v1) ─────────────────────────────────────────────────
r.Route("/api/v1", func(r chi.Router) {
r.Use(mw.RateLimit(rateLimitCfg))
r.Use(mw.RateLimit(opts.RateLimit))
// Public group — no authentication required.
r.Group(func(r chi.Router) {
r.Mount("/packets", handlers.PacketsRouter(reader))
@@ -114,7 +113,7 @@ func New(h *hub.Hub, reader api.Reader, workers []*ingest.Worker, maxConnsPerIP,
})
// Protect the entire subtree, including its root and unknown paths.
r.Mount("/admin", mw.BearerAuth(authCfg.APIKey, handlers.AdminRouter(runtimeConfig)))
r.Mount("/admin", mw.BearerAuth(opts.Auth.APIKey, handlers.AdminRouter(runtimeConfig, opts.AdminRoutes)))
})
return r
+1 -1
View File
@@ -36,7 +36,7 @@ func TestWebSocketLimitUsesTrustedClientIP(t *testing.T) {
func checkWebSocketLimit(t *testing.T, cfg config.ServerConfig, secondIP string, wantShed bool) {
t.Helper()
server := httptest.NewServer(New(hub.New(), nil, nil, 1, 10, config.CORSConfig{}, cfg, config.AuthConfig{}, config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 1, Burst: 1}))
server := httptest.NewServer(New(hub.New(), nil, nil, Options{MaxConnsPerIP: 1, MaxConnectsPerMinute: 10, Server: cfg, RateLimit: config.ResolvedRateLimitConfig{Enabled: true, RequestsPerMinute: 1, Burst: 1}}))
defer server.Close()
// Exhaust this client's REST budget before checking its independent WS cap.
for _, want := range []int{http.StatusOK, http.StatusTooManyRequests} {
+1 -1
View File
@@ -16,7 +16,7 @@ import (
func TestRuntimeConfigUpdate(t *testing.T) {
newRouter := func() http.Handler {
return New(nil, nil, nil, 5, 1000, config.CORSConfig{AllowedOrigins: []string{"https://before.test"}}, config.ServerConfig{}, config.AuthConfig{APIKey: "test-key"}, config.ResolvedRateLimitConfig{})
return New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1000, CORS: config.CORSConfig{AllowedOrigins: []string{"https://before.test"}}, Auth: config.AuthConfig{APIKey: "test-key"}})
}
handler := newRouter()
request := func(method, path, body, token, media, origin string) *httptest.ResponseRecorder {
+1 -1
View File
@@ -29,7 +29,7 @@ func TestWebSocketAttemptClientIdentity(t *testing.T) {
if tc.trusted {
cfg.TrustedProxies = []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")}
}
handler := New(nil, nil, nil, 5, 1, config.CORSConfig{}, cfg, config.AuthConfig{}, config.ResolvedRateLimitConfig{})
handler := New(nil, nil, nil, Options{MaxConnsPerIP: 5, MaxConnectsPerMinute: 1, Server: cfg})
attempt := func(peer, header string) int {
request := httptest.NewRequest(http.MethodGet, "/ws", nil)
request.RemoteAddr = peer