fix(fleet): remove duplicate route registration to prevent chi panic
Some checks are pending
CI Benchmark - Fusion Loop Timing / Fusion Loop Timing Benchmark (push) Waiting to run
Some checks are pending
CI Benchmark - Fusion Loop Timing / Fusion Loop Timing Benchmark (push) Waiting to run
- Remove duplicate node-specific routes (role, label, locate, delete) from
FleetHandler.RegisterRoutes to avoid chi panic on duplicate registration
- Keep only unique FleetHandler routes: /api/fleet/health, /api/fleet/history,
/api/fleet/optimise, /api/fleet/simulate
- Add startup smoke test TestRouteRegistrationNoPanic to verify both Handler
and FleetHandler can be registered on same router without panic
main.go registers both fleet.NewHandler and fleet.NewFleetHandler on the
same router, which previously caused chi to panic due to duplicate routes:
POST /api/nodes/{mac}/role
PATCH /api/nodes/{mac}/label
POST /api/nodes/{mac}/locate
DELETE /api/nodes/{mac}
The Handler has comprehensive node/room/mode endpoints while FleetHandler
focuses on health/optimization/simulation, so duplicates are removed from
FleetHandler.
Closes: bf-3o15x
This commit is contained in:
parent
b7bffd1388
commit
2c8bbcf646
2 changed files with 56 additions and 13 deletions
|
|
@ -34,29 +34,20 @@ func (h *FleetHandler) SetUnpairedProvider(p UnpairedProvider) {
|
|||
h.unpairedProvider = p
|
||||
}
|
||||
|
||||
// RegisterRoutes mounts fleet endpoints on r.
|
||||
// RegisterRoutes mounts fleet health/optimisation endpoints on r.
|
||||
//
|
||||
// GET /api/fleet — all provisioned nodes with full details
|
||||
// GET /api/fleet/health — current fleet health status
|
||||
// GET /api/fleet/history — recent optimisation history
|
||||
// POST /api/fleet/optimise — trigger manual re-optimisation
|
||||
// GET /api/fleet/simulate — simulate node removal impact
|
||||
// PATCH /api/nodes/{mac}/label — update node label
|
||||
// POST /api/nodes/{mac}/locate — send identify command
|
||||
// POST /api/nodes/{mac}/role — assign new role
|
||||
// DELETE /api/nodes/{mac} — remove from fleet
|
||||
//
|
||||
// Node-specific routes (role, label, locate, delete) are handled by fleet.Handler
|
||||
// to avoid duplicate route registration.
|
||||
func (h *FleetHandler) RegisterRoutes(r chi.Router) {
|
||||
r.Get("/api/fleet", h.getFleet)
|
||||
r.Get("/api/fleet/health", h.getFleetHealth)
|
||||
r.Get("/api/fleet/history", h.getFleetHistory)
|
||||
r.Post("/api/fleet/optimise", h.triggerOptimise)
|
||||
r.Get("/api/fleet/simulate", h.simulateNodeRemoval)
|
||||
|
||||
// Node-specific routes
|
||||
r.Patch("/api/nodes/{mac}/label", h.setNodeLabel)
|
||||
r.Post("/api/nodes/{mac}/locate", h.locateNode)
|
||||
r.Post("/api/nodes/{mac}/role", h.setNodeRole)
|
||||
r.Delete("/api/nodes/{mac}", h.removeNode)
|
||||
}
|
||||
|
||||
// fleetHealthResponse is the wire format for /api/fleet/health
|
||||
|
|
|
|||
|
|
@ -2510,3 +2510,55 @@ func TestHandlerDisableEnableRoundTrip(t *testing.T) {
|
|||
t.Errorf("Expected role_before_disable to be cleared after enable, got %s", savedRole)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRouteRegistrationNoPanic verifies that registering both Handler and FleetHandler
|
||||
// on the same router does not cause chi to panic with duplicate route registration.
|
||||
// This is a startup smoke test to catch accidental route conflicts early.
|
||||
func TestRouteRegistrationNoPanic(t *testing.T) {
|
||||
reg := newTestRegistry(t)
|
||||
mgr := NewManager(reg)
|
||||
selfHealMgr := NewSelfHealManager(reg, NewRoleOptimiser(DefaultOptimisationConfig()), DefaultSelfHealConfig())
|
||||
|
||||
// Create both handlers as main.go does
|
||||
h := NewHandler(mgr)
|
||||
fleetHealthHandler := NewFleetHandler(selfHealMgr, reg)
|
||||
|
||||
// This should not panic - if there are duplicate routes, chi will panic here
|
||||
r := chi.NewRouter()
|
||||
h.RegisterRoutes(r)
|
||||
fleetHealthHandler.RegisterRoutes(r)
|
||||
|
||||
// Verify expected routes are registered
|
||||
// From Handler: /api/nodes, /api/fleet (list), /api/nodes/{mac}/role, etc.
|
||||
// From FleetHandler: /api/fleet/health, /api/fleet/history, /api/fleet/optimise, /api/fleet/simulate
|
||||
// The key is no panic on duplicate routes like POST /api/nodes/{mac}/role
|
||||
|
||||
// Try to walk the routes to ensure they're registered
|
||||
routes := chi.Routes(r)
|
||||
if len(routes) == 0 {
|
||||
t.Fatal("No routes registered")
|
||||
}
|
||||
|
||||
// Verify some expected routes exist
|
||||
expectedRoutes := []string{
|
||||
"/api/nodes",
|
||||
"/api/fleet",
|
||||
"/api/fleet/health",
|
||||
"/api/fleet/history",
|
||||
"/api/fleet/optimise",
|
||||
"/api/fleet/simulate",
|
||||
}
|
||||
|
||||
for _, expected := range expectedRoutes {
|
||||
found := false
|
||||
for _, route := range routes {
|
||||
if route.Pattern == expected {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Errorf("Expected route %s not found", expected)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue