From 13ac7cc03fb9e7dab1dd9881e5ec4351c510db53 Mon Sep 17 00:00:00 2001 From: Jon Fox Date: Thu, 12 Feb 2026 13:13:30 -0800 Subject: [PATCH 1/2] Fix Caddy route creation when routes array is null Caddy returns a 500 error when appending a route to a server whose `routes` field is null (rather than an empty array). This fixes `devx caddy check --fix` by: - Discovering the port-80 server dynamically instead of hardcoding srv1 - Adding EnsureRoutesArray() to initialize null/missing routes before route creation batches - Making GetAllRoutes resilient to null bodies and 404 responses - Adding comprehensive unit tests with httptest mocks Co-Authored-By: Claude Opus 4.6 --- caddy/health.go | 5 + caddy/provisioning.go | 5 + caddy/routes.go | 101 +++++++++++++-- caddy/routes_test.go | 278 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 382 insertions(+), 7 deletions(-) diff --git a/caddy/health.go b/caddy/health.go index 70bafe7..5ea2c71 100644 --- a/caddy/health.go +++ b/caddy/health.go @@ -142,6 +142,11 @@ func RepairRoutes(result *HealthCheckResult, sessions map[string]*SessionInfo) e return fmt.Errorf("Caddy is not running") } + // Ensure routes array exists (Caddy can't append to null) + if err := client.EnsureRoutesArray(); err != nil { + return fmt.Errorf("failed to initialize routes array: %w", err) + } + // If catch-all is first, we need to reorder all routes if result.CatchAllFirst { fmt.Println("Fixing route order (catch-all route is blocking specific routes)...") diff --git a/caddy/provisioning.go b/caddy/provisioning.go index f45a0d6..c4b6213 100644 --- a/caddy/provisioning.go +++ b/caddy/provisioning.go @@ -90,6 +90,11 @@ func ProvisionSessionRoutesWithProject(sessionName string, services map[string]i return make(map[string]string), nil } + // Ensure routes array exists (Caddy can't append to null) + if err := client.EnsureRoutesArray(); err != nil { + return nil, fmt.Errorf("failed to initialize routes array: %w", err) + } + // Check if there are any catch-all routes that need to be moved to the end existingRoutes, err := client.GetAllRoutes() if err == nil { diff --git a/caddy/routes.go b/caddy/routes.go index e0fd574..59336bc 100644 --- a/caddy/routes.go +++ b/caddy/routes.go @@ -42,8 +42,9 @@ type RouteResponse struct { // CaddyClient manages communication with Caddy's admin API type CaddyClient struct { - client *resty.Client - baseURL string + client *resty.Client + baseURL string + serverName string } // NewCaddyClient creates a new Caddy API client @@ -56,10 +57,48 @@ func NewCaddyClient() *CaddyClient { client := resty.New() client.SetTimeout(10 * time.Second) - return &CaddyClient{ + c := &CaddyClient{ client: client, baseURL: caddyAPI, } + c.discoverServerName() + return c +} + +// discoverServerName finds the HTTP server listening on :80. +// Falls back to "srv1" on any failure. +func (c *CaddyClient) discoverServerName() { + c.serverName = "srv1" // default fallback + + resp, err := c.client.R().Get(c.baseURL + "/config/apps/http/servers") + if err != nil || resp.StatusCode() != http.StatusOK { + return + } + + var servers map[string]json.RawMessage + if err := json.Unmarshal(resp.Body(), &servers); err != nil { + return + } + + for name, raw := range servers { + var srv struct { + Listen []string `json:"listen"` + } + if err := json.Unmarshal(raw, &srv); err != nil { + continue + } + for _, addr := range srv.Listen { + if strings.Contains(addr, ":80") { + c.serverName = name + return + } + } + } +} + +// serverPath returns the Caddy config path for the discovered HTTP server. +func (c *CaddyClient) serverPath() string { + return "/config/apps/http/servers/" + c.serverName } // CreateRoute creates a route for a service @@ -116,7 +155,7 @@ func (c *CaddyClient) CreateRouteWithProject(sessionName, serviceName string, po resp, err := c.client.R(). SetHeader("Content-Type", "application/json"). SetBody(routeJSON). - Post(c.baseURL + "/config/apps/http/servers/srv1/routes/-") + Post(c.baseURL + c.serverPath() + "/routes/-") if err != nil { return "", fmt.Errorf("failed to create route: %w", err) @@ -183,15 +222,26 @@ func (c *CaddyClient) DeleteSessionRoutes(sessionName string) error { // GetAllRoutes retrieves all routes from Caddy func (c *CaddyClient) GetAllRoutes() ([]Route, error) { - resp, err := c.client.R().Get(c.baseURL + "/config/apps/http/servers/srv1/routes") + resp, err := c.client.R().Get(c.baseURL + c.serverPath() + "/routes") if err != nil { return nil, fmt.Errorf("failed to list routes: %w", err) } + // Routes path doesn't exist yet — treat as empty + if resp.StatusCode() == http.StatusNotFound { + return []Route{}, nil + } + if resp.StatusCode() != http.StatusOK { return nil, fmt.Errorf("caddy API returned status %d: %s", resp.StatusCode(), resp.String()) } + // Handle null or empty body + body := strings.TrimSpace(string(resp.Body())) + if body == "null" || body == "" { + return []Route{}, nil + } + // Parse routes response var routes []Route if err := json.Unmarshal(resp.Body(), &routes); err != nil { @@ -201,6 +251,43 @@ func (c *CaddyClient) GetAllRoutes() ([]Route, error) { return routes, nil } +// EnsureRoutesArray initializes the server's routes array if it is null or missing. +// Caddy cannot append a route to a null RouteList, so this must be called before +// any route-creation batch. +func (c *CaddyClient) EnsureRoutesArray() error { + resp, err := c.client.R().Get(c.baseURL + c.serverPath()) + if err != nil { + return fmt.Errorf("failed to read server config: %w", err) + } + if resp.StatusCode() != http.StatusOK { + return fmt.Errorf("caddy API returned status %d reading server config: %s", resp.StatusCode(), resp.String()) + } + + var serverCfg map[string]json.RawMessage + if err := json.Unmarshal(resp.Body(), &serverCfg); err != nil { + return fmt.Errorf("failed to parse server config: %w", err) + } + + raw, exists := serverCfg["routes"] + if exists && strings.TrimSpace(string(raw)) != "null" { + return nil // routes array already present + } + + // PATCH with an empty routes array — merges into existing config + resp, err = c.client.R(). + SetHeader("Content-Type", "application/json"). + SetBody([]byte(`{"routes":[]}`)). + Patch(c.baseURL + c.serverPath()) + if err != nil { + return fmt.Errorf("failed to initialize routes array: %w", err) + } + if resp.StatusCode() != http.StatusOK { + return fmt.Errorf("caddy API returned status %d initializing routes: %s", resp.StatusCode(), resp.String()) + } + + return nil +} + // CheckCaddyConnection verifies that Caddy is running and accessible func (c *CaddyClient) CheckCaddyConnection() error { resp, err := c.client.R().Get(c.baseURL + "/config/") @@ -240,7 +327,7 @@ func GetServiceMapping(portName string) string { // ReplaceAllRoutes deletes all current routes and creates new ones in the specified order func (c *CaddyClient) ReplaceAllRoutes(routes []Route) error { // First, delete all existing routes - resp, err := c.client.R().Delete(c.baseURL + "/config/apps/http/servers/srv1/routes") + resp, err := c.client.R().Delete(c.baseURL + c.serverPath() + "/routes") if err != nil { return fmt.Errorf("failed to delete existing routes: %w", err) } @@ -258,7 +345,7 @@ func (c *CaddyClient) ReplaceAllRoutes(routes []Route) error { resp, err = c.client.R(). SetHeader("Content-Type", "application/json"). SetBody(routesJSON). - Post(c.baseURL + "/config/apps/http/servers/srv1/routes") + Post(c.baseURL + c.serverPath() + "/routes") if err != nil { return fmt.Errorf("failed to create routes: %w", err) diff --git a/caddy/routes_test.go b/caddy/routes_test.go index 9d80170..c1b33db 100644 --- a/caddy/routes_test.go +++ b/caddy/routes_test.go @@ -2,7 +2,13 @@ package caddy import ( "encoding/json" + "net/http" + "net/http/httptest" + "sync" "testing" + "time" + + "github.com/go-resty/resty/v2" ) func TestRouteGeneration(t *testing.T) { @@ -155,3 +161,275 @@ func contains(s, substr string) bool { } return false } + +// newTestClient creates a CaddyClient wired to the given httptest.Server, +// bypassing NewCaddyClient (which uses viper and does live discovery). +func newTestClient(ts *httptest.Server, serverName string) *CaddyClient { + client := resty.New() + client.SetTimeout(5 * time.Second) + return &CaddyClient{ + client: client, + baseURL: ts.URL, + serverName: serverName, + } +} + +// --- discoverServerName tests --- + +func TestDiscoverServerName(t *testing.T) { + t.Run("finds srv1 with :80", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "srv0": map[string]any{"listen": []string{":443"}}, + "srv1": map[string]any{"listen": []string{":80"}}, + }) + })) + defer ts.Close() + + c := newTestClient(ts, "placeholder") + c.discoverServerName() + if c.serverName != "srv1" { + t.Errorf("expected srv1, got %s", c.serverName) + } + }) + + t.Run("finds srv0 with :80", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "srv0": map[string]any{"listen": []string{":80"}}, + "srv1": map[string]any{"listen": []string{":443"}}, + }) + })) + defer ts.Close() + + c := newTestClient(ts, "placeholder") + c.discoverServerName() + if c.serverName != "srv0" { + t.Errorf("expected srv0, got %s", c.serverName) + } + }) + + t.Run("falls back to srv1 when no :80 server", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "myserver": map[string]any{"listen": []string{":443"}}, + }) + })) + defer ts.Close() + + c := newTestClient(ts, "placeholder") + c.discoverServerName() + if c.serverName != "srv1" { + t.Errorf("expected fallback srv1, got %s", c.serverName) + } + }) + + t.Run("falls back to srv1 on API error", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer ts.Close() + + c := newTestClient(ts, "placeholder") + c.discoverServerName() + if c.serverName != "srv1" { + t.Errorf("expected fallback srv1, got %s", c.serverName) + } + }) +} + +// --- EnsureRoutesArray tests --- + +func TestEnsureRoutesArray(t *testing.T) { + t.Run("routes already exists — no PATCH", func(t *testing.T) { + patched := false + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + w.Write([]byte(`{"listen":[":80"],"routes":[]}`)) + return + } + if r.Method == http.MethodPatch { + patched = true + } + w.WriteHeader(http.StatusOK) + })) + defer ts.Close() + + c := newTestClient(ts, "srv1") + if err := c.EnsureRoutesArray(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if patched { + t.Error("expected no PATCH when routes already exists") + } + }) + + t.Run("routes is null — sends PATCH", func(t *testing.T) { + patched := false + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + w.Write([]byte(`{"listen":[":80"],"routes":null}`)) + return + } + if r.Method == http.MethodPatch { + patched = true + } + w.WriteHeader(http.StatusOK) + })) + defer ts.Close() + + c := newTestClient(ts, "srv1") + if err := c.EnsureRoutesArray(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !patched { + t.Error("expected PATCH when routes is null") + } + }) + + t.Run("routes key missing — sends PATCH", func(t *testing.T) { + patched := false + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + w.Write([]byte(`{"listen":[":80"]}`)) + return + } + if r.Method == http.MethodPatch { + patched = true + } + w.WriteHeader(http.StatusOK) + })) + defer ts.Close() + + c := newTestClient(ts, "srv1") + if err := c.EnsureRoutesArray(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !patched { + t.Error("expected PATCH when routes key is missing") + } + }) +} + +// --- GetAllRoutes null/404 handling tests --- + +func TestGetAllRoutesNullResponse(t *testing.T) { + t.Run("null body returns empty slice", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("null")) + })) + defer ts.Close() + + c := newTestClient(ts, "srv1") + routes, err := c.GetAllRoutes() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(routes) != 0 { + t.Errorf("expected empty slice, got %d routes", len(routes)) + } + }) + + t.Run("404 returns empty slice", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer ts.Close() + + c := newTestClient(ts, "srv1") + routes, err := c.GetAllRoutes() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(routes) != 0 { + t.Errorf("expected empty slice, got %d routes", len(routes)) + } + }) + + t.Run("valid routes are parsed", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode([]Route{ + {ID: "test-route"}, + }) + })) + defer ts.Close() + + c := newTestClient(ts, "srv1") + routes, err := c.GetAllRoutes() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(routes) != 1 || routes[0].ID != "test-route" { + t.Errorf("expected 1 route with ID test-route, got %v", routes) + } + }) +} + +// --- serverPath tests --- + +func TestServerPath(t *testing.T) { + c := &CaddyClient{serverName: "myserver"} + expected := "/config/apps/http/servers/myserver" + if got := c.serverPath(); got != expected { + t.Errorf("serverPath() = %q, want %q", got, expected) + } +} + +// --- Integration: routes use discovered server path --- + +func TestRoutesUseDiscoveredServer(t *testing.T) { + var mu sync.Mutex + var requestPaths []string + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + requestPaths = append(requestPaths, r.URL.Path) + mu.Unlock() + + // Discovery endpoint + if r.URL.Path == "/config/apps/http/servers" { + json.NewEncoder(w).Encode(map[string]any{ + "myhttp": map[string]any{"listen": []string{":80"}}, + }) + return + } + + // Routes GET + if r.Method == http.MethodGet { + w.Write([]byte("[]")) + return + } + + // Routes POST + w.WriteHeader(http.StatusOK) + })) + defer ts.Close() + + client := resty.New() + client.SetTimeout(5 * time.Second) + c := &CaddyClient{ + client: client, + baseURL: ts.URL, + } + c.discoverServerName() + + if c.serverName != "myhttp" { + t.Fatalf("expected myhttp, got %s", c.serverName) + } + + // GetAllRoutes should use /config/apps/http/servers/myhttp/routes + _, _ = c.GetAllRoutes() + + mu.Lock() + defer mu.Unlock() + found := false + for _, p := range requestPaths { + if p == "/config/apps/http/servers/myhttp/routes" { + found = true + break + } + } + if !found { + t.Errorf("expected request to /config/apps/http/servers/myhttp/routes, got paths: %v", requestPaths) + } +} From bd4ef5b84557de6856fba3d3e823a65c8f763451 Mon Sep 17 00:00:00 2001 From: Jon Fox Date: Thu, 12 Feb 2026 13:17:57 -0800 Subject: [PATCH 2/2] Fix unchecked error returns in test handlers Co-Authored-By: Claude Opus 4.6 --- caddy/routes_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/caddy/routes_test.go b/caddy/routes_test.go index c1b33db..c062697 100644 --- a/caddy/routes_test.go +++ b/caddy/routes_test.go @@ -179,7 +179,7 @@ func newTestClient(ts *httptest.Server, serverName string) *CaddyClient { func TestDiscoverServerName(t *testing.T) { t.Run("finds srv1 with :80", func(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewEncoder(w).Encode(map[string]any{ + _ = json.NewEncoder(w).Encode(map[string]any{ "srv0": map[string]any{"listen": []string{":443"}}, "srv1": map[string]any{"listen": []string{":80"}}, }) @@ -195,7 +195,7 @@ func TestDiscoverServerName(t *testing.T) { t.Run("finds srv0 with :80", func(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewEncoder(w).Encode(map[string]any{ + _ = json.NewEncoder(w).Encode(map[string]any{ "srv0": map[string]any{"listen": []string{":80"}}, "srv1": map[string]any{"listen": []string{":443"}}, }) @@ -211,7 +211,7 @@ func TestDiscoverServerName(t *testing.T) { t.Run("falls back to srv1 when no :80 server", func(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewEncoder(w).Encode(map[string]any{ + _ = json.NewEncoder(w).Encode(map[string]any{ "myserver": map[string]any{"listen": []string{":443"}}, }) })) @@ -245,7 +245,7 @@ func TestEnsureRoutesArray(t *testing.T) { patched := false ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet { - w.Write([]byte(`{"listen":[":80"],"routes":[]}`)) + _, _ = w.Write([]byte(`{"listen":[":80"],"routes":[]}`)) return } if r.Method == http.MethodPatch { @@ -268,7 +268,7 @@ func TestEnsureRoutesArray(t *testing.T) { patched := false ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet { - w.Write([]byte(`{"listen":[":80"],"routes":null}`)) + _, _ = w.Write([]byte(`{"listen":[":80"],"routes":null}`)) return } if r.Method == http.MethodPatch { @@ -291,7 +291,7 @@ func TestEnsureRoutesArray(t *testing.T) { patched := false ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet { - w.Write([]byte(`{"listen":[":80"]}`)) + _, _ = w.Write([]byte(`{"listen":[":80"]}`)) return } if r.Method == http.MethodPatch { @@ -316,7 +316,7 @@ func TestEnsureRoutesArray(t *testing.T) { func TestGetAllRoutesNullResponse(t *testing.T) { t.Run("null body returns empty slice", func(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("null")) + _, _ = w.Write([]byte("null")) })) defer ts.Close() @@ -348,7 +348,7 @@ func TestGetAllRoutesNullResponse(t *testing.T) { t.Run("valid routes are parsed", func(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewEncoder(w).Encode([]Route{ + _ = json.NewEncoder(w).Encode([]Route{ {ID: "test-route"}, }) })) @@ -388,7 +388,7 @@ func TestRoutesUseDiscoveredServer(t *testing.T) { // Discovery endpoint if r.URL.Path == "/config/apps/http/servers" { - json.NewEncoder(w).Encode(map[string]any{ + _ = json.NewEncoder(w).Encode(map[string]any{ "myhttp": map[string]any{"listen": []string{":80"}}, }) return @@ -396,7 +396,7 @@ func TestRoutesUseDiscoveredServer(t *testing.T) { // Routes GET if r.Method == http.MethodGet { - w.Write([]byte("[]")) + _, _ = w.Write([]byte("[]")) return }