From e6b6f0183684d216141d4d15ce6d7ae87c9ae538 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 13 Apr 2026 22:52:43 +0200 Subject: [PATCH] Replace gorilla/mux with Go 1.22 stdlib ServeMux in test server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go 1.22 added method-based routing ("GET /items/{id}"), path wildcards ({id}), and rest wildcards ({path...}) to net/http.ServeMux, which directly replaces the features we used gorilla/mux for. This removes the gorilla/mux dependency from go.mod and NOTICE. Exact (non-wildcard) paths are dispatched via a map lookup before reaching ServeMux. This avoids a ServeMux limitation where registering e.g. "GET /exact/path" alongside "HEAD /prefix/{path...}" panics because Go's implicit GET→HEAD handling creates an unresolvable precedence conflict. When an exact path is registered for one method but a request arrives for a different method, it falls through to ServeMux where the wildcard handler serves it. Wildcard patterns in test stubs (test.toml) must use the same placeholder names as the default handlers they coexist with. ServeMux panics on structurally identical patterns with different names (e.g. {name} vs {full_name}). Two test stubs are updated accordingly. Co-authored-by: Isaac --- NOTICE | 4 - acceptance/bundle/invariant/test.toml | 2 +- .../synced_database_tables/basic/test.toml | 2 +- acceptance/internal/prepare_server.go | 7 +- go.mod | 1 - go.sum | 2 - libs/testserver/handlers.go | 14 +- libs/testserver/server.go | 239 ++++++++++++------ 8 files changed, 171 insertions(+), 100 deletions(-) diff --git a/NOTICE b/NOTICE index 883c24ab78..a6842a143f 100644 --- a/NOTICE +++ b/NOTICE @@ -79,10 +79,6 @@ Copyright (c) 2013 Dario Castañé. All rights reserved. Copyright (c) 2012 The Go Authors. All rights reserved. License - https://github.com/darccio/mergo/blob/master/LICENSE -gorilla/mux - https://github.com/gorilla/mux -Copyright (c) 2023 The Gorilla Authors. All rights reserved. -License - https://github.com/gorilla/mux/blob/main/LICENSE - palantir/pkg - https://github.com/palantir/pkg Copyright (c) 2016, Palantir Technologies, Inc. License - https://github.com/palantir/pkg/blob/master/LICENSE diff --git a/acceptance/bundle/invariant/test.toml b/acceptance/bundle/invariant/test.toml index a850fc91a1..2c1de8ae6b 100644 --- a/acceptance/bundle/invariant/test.toml +++ b/acceptance/bundle/invariant/test.toml @@ -73,5 +73,5 @@ Pattern = "POST /api/2.0/sql/statements/" Response.Body = '{"status": {"state": "SUCCEEDED"}, "manifest": {"schema": {"columns": []}}}' [[Server]] -Pattern = "DELETE /api/2.1/unity-catalog/tables/{name}" +Pattern = "DELETE /api/2.1/unity-catalog/tables/{full_name}" Response.Body = '{"status": "OK"}' diff --git a/acceptance/bundle/resources/synced_database_tables/basic/test.toml b/acceptance/bundle/resources/synced_database_tables/basic/test.toml index d41d9b917c..191670590b 100644 --- a/acceptance/bundle/resources/synced_database_tables/basic/test.toml +++ b/acceptance/bundle/resources/synced_database_tables/basic/test.toml @@ -20,5 +20,5 @@ Pattern = "POST /api/2.0/sql/statements/" Response.Body = '{"status": {"state": "SUCCEEDED"}, "manifest": {"schema": {"columns": []}}}' [[Server]] -Pattern = "DELETE /api/2.1/unity-catalog/tables/{name}" +Pattern = "DELETE /api/2.1/unity-catalog/tables/{full_name}" Response.Body = '{"status": "OK"}' diff --git a/acceptance/internal/prepare_server.go b/acceptance/internal/prepare_server.go index 2a4d02f8c4..a01ff02d58 100644 --- a/acceptance/internal/prepare_server.go +++ b/acceptance/internal/prepare_server.go @@ -188,8 +188,8 @@ func startLocalServer(t *testing.T, killCountersMu := &sync.Mutex{} for ind := range stubs { - // We want later stubs takes precedence, because then leaf configs take precedence over parent directory configs - // In gorilla/mux earlier handlers take precedence, so we need to reverse the order + // Later stubs take precedence over earlier ones (leaf configs override parent configs). + // The first handler registered for a given pattern wins, so we reverse the order. stub := stubs[len(stubs)-1-ind] require.NotEmpty(t, stub.Pattern) items := strings.Split(stub.Pattern, " ") @@ -226,7 +226,8 @@ func startLocalServer(t *testing.T, }) } - // The earliest handlers take precedence, add default handlers last + // The first handler registered for a given pattern wins, so default + // handlers registered last serve as fallbacks. testserver.AddDefaultHandlers(s) return s.URL } diff --git a/go.mod b/go.mod index b9877f9440..9c7c16a2e6 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,6 @@ require ( github.com/fatih/color v1.19.0 // MIT github.com/google/jsonschema-go v0.4.2 // MIT github.com/google/uuid v1.6.0 // BSD-3-Clause - github.com/gorilla/mux v1.8.1 // BSD-3-Clause github.com/gorilla/websocket v1.5.3 // BSD-2-Clause github.com/hashicorp/go-version v1.8.0 // MPL-2.0 github.com/hashicorp/hc-install v0.9.3 // MPL-2.0 diff --git a/go.sum b/go.sum index 836996cc90..2581eed1d6 100644 --- a/go.sum +++ b/go.sum @@ -121,8 +121,6 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.11 h1:vAe81Msw+8tKUxi2Dq github.com/googleapis/enterprise-certificate-proxy v0.3.11/go.mod h1:RFV7MUdlb7AgEq2v7FmMCfeSMCllAzWxFgRdusoGks8= github.com/googleapis/gax-go/v2 v2.17.0 h1:RksgfBpxqff0EZkDWYuz9q/uWsTVz+kf43LsZ1J6SMc= github.com/googleapis/gax-go/v2 v2.17.0/go.mod h1:mzaqghpQp4JDh3HvADwrat+6M3MOIDp5YKHhb9PAgDY= -github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= -github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= diff --git a/libs/testserver/handlers.go b/libs/testserver/handlers.go index b2a95b1902..374d0b17a5 100644 --- a/libs/testserver/handlers.go +++ b/libs/testserver/handlers.go @@ -109,7 +109,7 @@ func AddDefaultHandlers(server *Server) { return "" }) - server.Handle("POST", "/api/2.0/workspace-files/import-file/{path:.*}", func(req Request) any { + server.Handle("POST", "/api/2.0/workspace-files/import-file/{path...}", func(req Request) any { path := req.Vars["path"] overwrite := req.URL.Query().Get("overwrite") == "true" return req.Workspace.WorkspaceFilesImportFile(path, req.Body, overwrite) @@ -145,12 +145,12 @@ func AddDefaultHandlers(server *Server) { return req.Workspace.WorkspaceFilesImportFile(request.Path, decoded, request.Overwrite) }) - server.Handle("GET", "/api/2.0/workspace-files/{path:.*}", func(req Request) any { + server.Handle("GET", "/api/2.0/workspace-files/{path...}", func(req Request) any { path := req.Vars["path"] return req.Workspace.WorkspaceFilesExportFile(path) }) - server.Handle("HEAD", "/api/2.0/fs/directories/{path:.*}", func(req Request) any { + server.Handle("HEAD", "/api/2.0/fs/directories/{path...}", func(req Request) any { dirPath := req.Vars["path"] if !strings.HasPrefix(dirPath, "/") { dirPath = "/" + dirPath @@ -165,7 +165,7 @@ func AddDefaultHandlers(server *Server) { return Response{StatusCode: 404} }) - server.Handle("HEAD", "/api/2.0/fs/files/{path:.*}", func(req Request) any { + server.Handle("HEAD", "/api/2.0/fs/files/{path...}", func(req Request) any { path := req.Vars["path"] if req.Workspace.FileExists(path) { return Response{StatusCode: 200} @@ -173,7 +173,7 @@ func AddDefaultHandlers(server *Server) { return Response{StatusCode: 404} }) - server.Handle("PUT", "/api/2.0/fs/directories/{path:.*}", func(req Request) any { + server.Handle("PUT", "/api/2.0/fs/directories/{path...}", func(req Request) any { dirPath := req.Vars["path"] if !strings.HasPrefix(dirPath, "/") { dirPath = "/" + dirPath @@ -194,13 +194,13 @@ func AddDefaultHandlers(server *Server) { return Response{} }) - server.Handle("PUT", "/api/2.0/fs/files/{path:.*}", func(req Request) any { + server.Handle("PUT", "/api/2.0/fs/files/{path...}", func(req Request) any { path := req.Vars["path"] overwrite := req.URL.Query().Get("overwrite") == "true" return req.Workspace.WorkspaceFilesImportFile(path, req.Body, overwrite) }) - server.Handle("GET", "/api/2.0/fs/files/{path:.*}", func(req Request) any { + server.Handle("GET", "/api/2.0/fs/files/{path...}", func(req Request) any { path := req.Vars["path"] data := req.Workspace.WorkspaceFilesExportFile(path) if data == nil { diff --git a/libs/testserver/server.go b/libs/testserver/server.go index 2d7048dc8d..a88450495f 100644 --- a/libs/testserver/server.go +++ b/libs/testserver/server.go @@ -16,7 +16,6 @@ import ( "sync" "github.com/databricks/cli/internal/testutil" - "github.com/gorilla/mux" ) const testPidKey = "test-pid" @@ -38,10 +37,13 @@ func ExtractPidFromHeaders(headers http.Header) int { type Server struct { *httptest.Server - Router *mux.Router t testutil.TestingT + mux *http.ServeMux + wildcardMethods map[string]bool // "METHOD /pattern" -> registered + exactRoutes map[string]map[string]HandlerFunc // path -> method -> handler + fakeWorkspaces map[string]*FakeWorkspace fakeOidc *FakeOidc mu sync.Mutex @@ -83,7 +85,6 @@ func NewRequest(t testutil.TestingT, r *http.Request, fakeWorkspace *FakeWorkspa URL: r.URL, Headers: r.Header, Body: body, - Vars: mux.Vars(r), Workspace: fakeWorkspace, Context: r.Context(), } @@ -200,64 +201,52 @@ func getHeaders(value []byte) http.Header { } func New(t testutil.TestingT) *Server { - router := mux.NewRouter() - server := httptest.NewServer(router) - t.Cleanup(server.Close) + mux := http.NewServeMux() s := &Server{ - Server: server, - Router: router, - t: t, - fakeWorkspaces: map[string]*FakeWorkspace{}, - fakeOidc: &FakeOidc{url: server.URL}, + t: t, + mux: mux, + wildcardMethods: map[string]bool{}, + exactRoutes: map[string]map[string]HandlerFunc{}, + fakeWorkspaces: map[string]*FakeWorkspace{}, } - t.Cleanup(func() { - for _, ws := range s.fakeWorkspaces { - ws.Cleanup() + // Exact (non-wildcard) routes are kept out of ServeMux to avoid + // conflicts between method-specific exact paths and wildcard patterns + // (e.g., GET on an exact path vs HEAD on a wildcard that covers it). + // + // When an exact path is registered for one method but a request arrives + // for a different method, it intentionally falls through to ServeMux. + // This lets wildcard handlers serve methods not covered by the exact + // registration (e.g., a stub registers GET /exact, but HEAD /exact + // falls through to the wildcard HEAD handler). + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Clear RawPath so ServeMux matches decoded paths; without this, + // percent-encoded slashes (%2F) would not match literal slashes. + if r.URL.RawPath != "" { + r.URL.RawPath = "" } - }) - - // Set up the not found handler as fallback - notFoundFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - pattern := r.Method + " " + r.URL.Path - bodyBytes, err := io.ReadAll(r.Body) - var body string - if err != nil { - body = fmt.Sprintf("failed to read the body: %s", err) - } else { - body = fmt.Sprintf("[%d bytes] %s", len(bodyBytes), bodyBytes) - } - - t.Errorf(`No handler for URL: %s -Body: %s - -For acceptance tests, add this to test.toml: -[[Server]] -Pattern = %q -Response.Body = '' -# Response.StatusCode = -`, r.URL, body, pattern) - - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusNotImplemented) - - resp := map[string]string{ - "message": "No stub found for pattern: " + pattern, + if methods, ok := s.exactRoutes[r.URL.Path]; ok { + if handler, ok := methods[r.Method]; ok { + s.serve(w, r, handler, nil) + return + } } + mux.ServeHTTP(w, r) + })) + t.Cleanup(server.Close) - respBytes, err := json.Marshal(resp) - if err != nil { - t.Errorf("JSON encoding error: %s", err) - respBytes = []byte("{\"message\": \"JSON encoding error\"}") - } + s.Server = server + s.fakeOidc = &FakeOidc{url: server.URL} - if _, err := w.Write(respBytes); err != nil { - t.Errorf("Response write error: %s", err) + t.Cleanup(func() { + for _, ws := range s.fakeWorkspaces { + ws.Cleanup() } }) - router.NotFoundHandler = notFoundFunc - router.MethodNotAllowedHandler = notFoundFunc + + // Register a catch-all handler as fallback for unmatched routes. + mux.HandleFunc("/", s.handleNotFound) // Register a default handler for the SDK's host metadata discovery endpoint. // The SDK resolves this during config initialization (as of v0.126.0) to @@ -291,48 +280,136 @@ func (s *Server) getWorkspaceForToken(token string) *FakeWorkspace { type HandlerFunc func(req Request) any +// Handle registers a handler for the given method and path pattern. +// First registration wins: subsequent calls with the same method+path are +// ignored. Exact paths are stored in a map checked before ServeMux; +// wildcard paths are registered with ServeMux using method-specific patterns. func (s *Server) Handle(method, path string, handler HandlerFunc) { - s.Router.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { - // Each test uses unique DATABRICKS_TOKEN, we simulate each token having - // it's own fake fakeWorkspace to avoid interference between tests. - fakeWorkspace := s.getWorkspaceForToken(getToken(r)) - - request := NewRequest(s.t, r, fakeWorkspace) + if !strings.Contains(path, "{") { + s.handleExact(method, path, handler) + } else { + s.handleWildcard(method, path, handler) + } +} - if s.RequestCallback != nil { - s.RequestCallback(&request) - } +func (s *Server) handleExact(method, path string, handler HandlerFunc) { + if s.exactRoutes[path] == nil { + s.exactRoutes[path] = map[string]HandlerFunc{} + } + if _, exists := s.exactRoutes[path][method]; !exists { + s.exactRoutes[path][method] = handler + } +} - var resp EncodedResponse +func (s *Server) handleWildcard(method, path string, handler HandlerFunc) { + pattern := method + " " + path + if s.wildcardMethods[pattern] { + return + } + s.wildcardMethods[pattern] = true - if bytes.Contains(request.Body, []byte("INJECT_ERROR")) { - resp = EncodedResponse{ - StatusCode: 500, - Body: []byte("INJECTED"), - } - } else { - respAny := handler(request) - if respAny == nil && request.Context.Err() != nil { - return - } - resp = normalizeResponse(s.t, respAny) + names := wildcardNames(path) + s.mux.HandleFunc(pattern, func(w http.ResponseWriter, r *http.Request) { + vars := make(map[string]string, len(names)) + for _, name := range names { + vars[name] = r.PathValue(name) } + s.serve(w, r, handler, vars) + }) +} - for k, v := range resp.Headers { - w.Header()[k] = v +// wildcardNames extracts wildcard parameter names from a path pattern, +// e.g. "/api/{id}/files/{path...}" returns ["id", "path"]. +func wildcardNames(path string) []string { + var names []string + for _, part := range strings.Split(path, "/") { + if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") { + name := part[1 : len(part)-1] + name = strings.TrimSuffix(name, "...") + names = append(names, name) } + } + return names +} - w.WriteHeader(resp.StatusCode) +// serve is the common request handling logic for both exact and wildcard routes. +func (s *Server) serve(w http.ResponseWriter, r *http.Request, handler HandlerFunc, vars map[string]string) { + fakeWorkspace := s.getWorkspaceForToken(getToken(r)) - if s.ResponseCallback != nil { - s.ResponseCallback(&request, &resp) - } + request := NewRequest(s.t, r, fakeWorkspace) + request.Vars = vars - if _, err := w.Write(resp.Body); err != nil { - s.t.Errorf("Failed to write response: %s", err) + if s.RequestCallback != nil { + s.RequestCallback(&request) + } + + var resp EncodedResponse + + if bytes.Contains(request.Body, []byte("INJECT_ERROR")) { + resp = EncodedResponse{ + StatusCode: 500, + Body: []byte("INJECTED"), + } + } else { + respAny := handler(request) + if respAny == nil && request.Context.Err() != nil { return } - }).Methods(method) + resp = normalizeResponse(s.t, respAny) + } + + for k, v := range resp.Headers { + w.Header()[k] = v + } + + w.WriteHeader(resp.StatusCode) + + if s.ResponseCallback != nil { + s.ResponseCallback(&request, &resp) + } + + if _, err := w.Write(resp.Body); err != nil { + s.t.Errorf("Failed to write response: %s", err) + return + } +} + +func (s *Server) handleNotFound(w http.ResponseWriter, r *http.Request) { + pattern := r.Method + " " + r.URL.Path + bodyBytes, err := io.ReadAll(r.Body) + var body string + if err != nil { + body = fmt.Sprintf("failed to read the body: %s", err) + } else { + body = fmt.Sprintf("[%d bytes] %s", len(bodyBytes), bodyBytes) + } + + s.t.Errorf(`No handler for URL: %s +Body: %s + +For acceptance tests, add this to test.toml: +[[Server]] +Pattern = %q +Response.Body = '' +# Response.StatusCode = +`, r.URL, body, pattern) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotImplemented) + + resp := map[string]string{ + "message": "No stub found for pattern: " + pattern, + } + + respBytes, err := json.Marshal(resp) + if err != nil { + s.t.Errorf("JSON encoding error: %s", err) + respBytes = []byte("{\"message\": \"JSON encoding error\"}") + } + + if _, err := w.Write(respBytes); err != nil { + s.t.Errorf("Response write error: %s", err) + } } func getToken(r *http.Request) string {