diff --git a/pkg/ccl/oidcccl/authentication_oidc_test.go b/pkg/ccl/oidcccl/authentication_oidc_test.go index 1c040db90c4f..eadc88ac9c71 100644 --- a/pkg/ccl/oidcccl/authentication_oidc_test.go +++ b/pkg/ccl/oidcccl/authentication_oidc_test.go @@ -36,7 +36,7 @@ import ( "github.com/stretchr/testify/require" ) -func tenantOrSystemAdminURL(s serverutils.TestServerInterface) string { +func tenantOrSystemAdminURL(s serverutils.TestServerInterface) *serverutils.TestURL { if len(s.TestTenants()) > 0 { return s.TestTenants()[0].AdminURL() } @@ -71,7 +71,7 @@ func TestOIDCBadRequestIfDisabled(t *testing.T) { client, err := testCertsContext.GetHTTPClient() require.NoError(t, err) - resp, err := client.Get(tenantOrSystemAdminURL(s) + "/oidc/v1/login") + resp, err := client.Get(tenantOrSystemAdminURL(s).WithPath("/oidc/v1/login").String()) if err != nil { t.Fatalf("could not issue GET request to admin server: %s", err) } @@ -196,7 +196,7 @@ func TestOIDCEnabled(t *testing.T) { } t.Run("login redirect", func(t *testing.T) { - resp, err := client.Get(tenantOrSystemAdminURL(s) + "/oidc/v1/login") + resp, err := client.Get(tenantOrSystemAdminURL(s).WithPath("/oidc/v1/login").String()) if err != nil { t.Fatalf("could not issue GET request to admin server: %s", err) } diff --git a/pkg/ccl/serverccl/server_controller_test.go b/pkg/ccl/serverccl/server_controller_test.go index 9edaef5030fa..aeb6c8058cda 100644 --- a/pkg/ccl/serverccl/server_controller_test.go +++ b/pkg/ccl/serverccl/server_controller_test.go @@ -16,7 +16,6 @@ import ( "io" "net/http" "net/http/cookiejar" - "net/url" "testing" "time" @@ -253,7 +252,7 @@ VALUES($1, $2, $3, $4, $5, (SELECT user_id FROM system.users WHERE username = $3 } newreq := func() *http.Request { - req, err := http.NewRequest("GET", aurl+"/_status/sessions", nil) + req, err := http.NewRequest("GET", aurl.WithPath("/_status/sessions").String(), nil) require.NoError(t, err) return req } @@ -289,9 +288,7 @@ VALUES($1, $2, $3, $4, $5, (SELECT user_id FROM system.users WHERE username = $3 HttpOnly: true, Secure: true, } - purl, err := url.Parse(aurl) - require.NoError(t, err) - client.Jar.SetCookies(purl, []*http.Cookie{c}) + client.Jar.SetCookies(aurl.URL, []*http.Cookie{c}) req = newreq() body, err = get(req) @@ -303,7 +300,7 @@ VALUES($1, $2, $3, $4, $5, (SELECT user_id FROM system.users WHERE username = $3 t.Logf("retrieving session list from test tenant via cookie") c.Value = "hello" - client.Jar.SetCookies(purl, []*http.Cookie{c}) + client.Jar.SetCookies(aurl.URL, []*http.Cookie{c}) req = newreq() body, err = get(req) require.NoError(t, err) @@ -351,7 +348,7 @@ func TestServerControllerDefaultHTTPTenant(t *testing.T) { client, err := s.GetUnauthenticatedHTTPClient() require.NoError(t, err) - resp, err := client.Post(s.AdminURL()+"/login", + resp, err := client.Post(s.AdminURL().WithPath("/login").String(), "application/json", bytes.NewBuffer([]byte("{\"username\":\"foo\",\"password\":\"cockroach\"})")), ) @@ -392,7 +389,7 @@ func TestServerControllerBadHTTPCookies(t *testing.T) { Secure: true, } - req, err := http.NewRequest("GET", s.AdminURL()+"/", nil) + req, err := http.NewRequest("GET", s.AdminURL().WithPath("/").String(), nil) require.NoError(t, err) req.AddCookie(c) resp, err := client.Do(req) @@ -400,7 +397,7 @@ func TestServerControllerBadHTTPCookies(t *testing.T) { defer resp.Body.Close() require.Equal(t, 200, resp.StatusCode) - req, err = http.NewRequest("GET", s.AdminURL()+"/bundle.js", nil) + req, err = http.NewRequest("GET", s.AdminURL().WithPath("/bundle.js").String(), nil) require.NoError(t, err) req.AddCookie(c) resp, err = client.Do(req) @@ -569,7 +566,7 @@ func TestServerControllerLoginLogout(t *testing.T) { client, err := s.GetAuthenticatedHTTPClient(false, serverutils.SingleTenantSession) require.NoError(t, err) - resp, err := client.Post(s.AdminURL()+"/logout", "", nil) + resp, err := client.Post(s.AdminURL().WithPath("/logout").String(), "", nil) require.NoError(t, err) defer resp.Body.Close() @@ -590,7 +587,7 @@ func TestServerControllerLoginLogout(t *testing.T) { clientMT, err := s2.GetAuthenticatedHTTPClient(false, serverutils.MultiTenantSession) require.NoError(t, err) - respMT, err := clientMT.Get(s.AdminURL() + "/logout") + respMT, err := clientMT.Get(s.AdminURL().WithPath("/logout").String()) require.NoError(t, err) defer respMT.Body.Close() @@ -605,11 +602,9 @@ func TestServerControllerLoginLogout(t *testing.T) { require.ElementsMatch(t, []string{"", ""}, cookieValues) // Now using manual clients to simulate states that might be invalid - url, err := url.Parse(s2.AdminURL()) - require.NoError(t, err) cookieJar, err := cookiejar.New(nil) require.NoError(t, err) - cookieJar.SetCookies(url, []*http.Cookie{ + cookieJar.SetCookies(s2.AdminURL().URL, []*http.Cookie{ { Name: "multitenant-session", Value: "abc-123", @@ -617,7 +612,7 @@ func TestServerControllerLoginLogout(t *testing.T) { }) clientMT.Jar = cookieJar - respBadCookie, err := clientMT.Get(s.AdminURL() + "/logout") + respBadCookie, err := clientMT.Get(s.AdminURL().WithPath("/logout").String()) require.NoError(t, err) defer respBadCookie.Body.Close() diff --git a/pkg/ccl/serverccl/server_sql_test.go b/pkg/ccl/serverccl/server_sql_test.go index d9dbe83d5f6b..79213aab414a 100644 --- a/pkg/ccl/serverccl/server_sql_test.go +++ b/pkg/ccl/serverccl/server_sql_test.go @@ -169,7 +169,7 @@ func TestTenantHTTP(t *testing.T) { httpClient, err := tenant.GetUnauthenticatedHTTPClient() require.NoError(t, err) defer httpClient.CloseIdleConnections() - resp, err := httpClient.Get(tenant.AdminURL() + "/_status/vars") + resp, err := httpClient.Get(tenant.AdminURL().WithPath("/_status/vars").String()) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) @@ -180,7 +180,7 @@ func TestTenantHTTP(t *testing.T) { httpClient, err := tenant.GetAdminHTTPClient() require.NoError(t, err) defer httpClient.CloseIdleConnections() - resp, err := httpClient.Get(tenant.AdminURL() + "/debug/pprof/goroutine?debug=2") + resp, err := httpClient.Get(tenant.AdminURL().WithPath("/debug/pprof/goroutine?debug=2").String()) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) diff --git a/pkg/ccl/serverccl/statusccl/tenant_grpc_test.go b/pkg/ccl/serverccl/statusccl/tenant_grpc_test.go index 57cb9f6d0f7f..aee73a266e2d 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_grpc_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_grpc_test.go @@ -78,7 +78,7 @@ func TestTenantGRPCServices(t *testing.T) { defer httpClient.CloseIdleConnections() t.Run("gRPC Gateway is running", func(t *testing.T) { - resp, err := httpClient.Get(tenant.AdminURL() + "/_status/statements") + resp, err := httpClient.Get(tenant.AdminURL().WithPath("/_status/statements").String()) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) @@ -97,7 +97,7 @@ func TestTenantGRPCServices(t *testing.T) { defer connTenant2.Close() t.Run("statements endpoint fans out request to multiple pods", func(t *testing.T) { - resp, err := httpClient.Get(tenant2.AdminURL() + "/_status/statements") + resp, err := httpClient.Get(tenant2.AdminURL().WithPath("/_status/statements").String()) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) @@ -117,7 +117,7 @@ func TestTenantGRPCServices(t *testing.T) { require.NoError(t, err) defer httpClient3.CloseIdleConnections() - resp, err := httpClient3.Get(tenant3.AdminURL() + "/_status/statements") + resp, err := httpClient3.Get(tenant3.AdminURL().WithPath("/_status/statements").String()) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) @@ -155,7 +155,7 @@ func TestTenantGRPCServices(t *testing.T) { }) t.Run("sessions endpoint is available", func(t *testing.T) { - resp, err := httpClient.Get(tenant.AdminURL() + "/_status/sessions") + resp, err := httpClient.Get(tenant.AdminURL().WithPath("/_status/sessions").String()) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, 200, resp.StatusCode) diff --git a/pkg/ccl/serverccl/tenant_test_utils.go b/pkg/ccl/serverccl/tenant_test_utils.go index c32a8b5d32b2..41aa5ecd296a 100644 --- a/pkg/ccl/serverccl/tenant_test_utils.go +++ b/pkg/ccl/serverccl/tenant_test_utils.go @@ -250,7 +250,7 @@ func (c tenantCluster) TenantHTTPClient(t *testing.T, idx serverIdx, isAdmin boo client, err = c.Tenant(idx).GetTenant().GetAuthenticatedHTTPClient(false, serverutils.SingleTenantSession) } require.NoError(t, err) - return &httpClient{t: t, client: client, baseURL: c[idx].GetTenant().AdminURL()} + return &httpClient{t: t, client: client, baseURL: c[idx].GetTenant().AdminURL().String()} } func (c tenantCluster) TenantAdminHTTPClient(t *testing.T, idx serverIdx) *httpClient { diff --git a/pkg/ccl/serverccl/tenant_vars_test.go b/pkg/ccl/serverccl/tenant_vars_test.go index d7d6db4134a0..26087ae225a3 100644 --- a/pkg/ccl/serverccl/tenant_vars_test.go +++ b/pkg/ccl/serverccl/tenant_vars_test.go @@ -62,7 +62,7 @@ func TestTenantVars(t *testing.T) { } startNowNanos := timeutil.Now().UnixNano() - url := tenant.AdminURL() + "/_status/load" + url := tenant.AdminURL().WithPath("/_status/load").String() client := http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, diff --git a/pkg/cli/democluster/demo_cluster.go b/pkg/cli/democluster/demo_cluster.go index e7801589153b..4b833dcfa309 100644 --- a/pkg/cli/democluster/demo_cluster.go +++ b/pkg/cli/democluster/demo_cluster.go @@ -1899,26 +1899,21 @@ func (c *transientCluster) ListDemoNodes(w, ew io.Writer, justOne, verbose bool) } rpcAddr := c.tenantServers[i].RPCAddr() - tenantUiURLstr := c.tenantServers[i].AdminURL() - tenantUiURL, err := url.Parse(tenantUiURLstr) + tenantUiURL := c.tenantServers[i].AdminURL() + tenantSqlURL, err := c.getNetworkURLForServer(context.Background(), i, + false /* includeAppName */, forSecondaryTenant) if err != nil { fmt.Fprintln(ew, errors.Wrap(err, "retrieving network URL for tenant server")) } else { - tenantSqlURL, err := c.getNetworkURLForServer(context.Background(), i, - false /* includeAppName */, forSecondaryTenant) - if err != nil { - fmt.Fprintln(ew, errors.Wrap(err, "retrieving network URL for tenant server")) - } else { - // Only include a separate HTTP URL if there's no server - // controller. - includeHTTP := !c.demoCtx.Multitenant || c.demoCtx.DisableServerController + // Only include a separate HTTP URL if there's no server + // controller. + includeHTTP := !c.demoCtx.Multitenant || c.demoCtx.DisableServerController - socketDetails, err := c.sockForServer(i, forSecondaryTenant) - if err != nil { - fmt.Fprintln(ew, errors.Wrap(err, "retrieving socket URL for tenant server")) - } - c.printURLs(w, ew, tenantSqlURL, tenantUiURL, socketDetails, rpcAddr, verbose, includeHTTP) + socketDetails, err := c.sockForServer(i, forSecondaryTenant) + if err != nil { + fmt.Fprintln(ew, errors.Wrap(err, "retrieving socket URL for tenant server")) } + c.printURLs(w, ew, tenantSqlURL, tenantUiURL.URL, socketDetails, rpcAddr, verbose, includeHTTP) } fmt.Fprintln(w) if verbose { diff --git a/pkg/kv/kvserver/client_tenant_test.go b/pkg/kv/kvserver/client_tenant_test.go index 973ed252c38f..3ced0e2f285f 100644 --- a/pkg/kv/kvserver/client_tenant_test.go +++ b/pkg/kv/kvserver/client_tenant_test.go @@ -283,7 +283,7 @@ func TestTenantRateLimiter(t *testing.T) { httpClient, err := s.GetUnauthenticatedHTTPClient() require.NoError(t, err) getMetrics := func() string { - resp, err := httpClient.Get(s.AdminURL() + "/_status/vars") + resp, err := httpClient.Get(s.AdminURL().WithPath("/_status/vars").String()) require.NoError(t, err) read, err := io.ReadAll(resp.Body) require.NoError(t, err) diff --git a/pkg/security/certs_rotation_test.go b/pkg/security/certs_rotation_test.go index 8979464c055a..a1871b6125e3 100644 --- a/pkg/security/certs_rotation_test.go +++ b/pkg/security/certs_rotation_test.go @@ -73,7 +73,7 @@ func TestRotateCerts(t *testing.T) { // Client test function. clientTest := func(httpClient http.Client) error { - req, err := http.NewRequest("GET", s.AdminURL()+"/_status/metrics/local", nil) + req, err := http.NewRequest("GET", s.AdminURL().WithPath("/_status/metrics/local").String(), nil) if err != nil { return errors.Wrap(err, "could not create request") } diff --git a/pkg/security/certs_test.go b/pkg/security/certs_test.go index 9d79d180643a..0dd3a25f8d43 100644 --- a/pkg/security/certs_test.go +++ b/pkg/security/certs_test.go @@ -410,7 +410,7 @@ func TestUseCerts(t *testing.T) { if err != nil { t.Fatal(err) } - req, err := http.NewRequest("GET", s.AdminURL()+"/_status/metrics/local", nil) + req, err := http.NewRequest("GET", s.AdminURL().WithPath("/_status/metrics/local").String(), nil) if err != nil { t.Fatalf("could not create request: %v", err) } @@ -436,7 +436,7 @@ func TestUseCerts(t *testing.T) { if err != nil { t.Fatalf("Expected success, got %v", err) } - req, err = http.NewRequest("GET", s.AdminURL()+"/_status/metrics/local", nil) + req, err = http.NewRequest("GET", s.AdminURL().WithPath("/_status/metrics/local").String(), nil) if err != nil { t.Fatalf("could not create request: %v", err) } @@ -502,7 +502,7 @@ func TestUseSplitCACerts(t *testing.T) { if err != nil { t.Fatal(err) } - req, err := http.NewRequest("GET", s.AdminURL()+"/_status/metrics/local", nil) + req, err := http.NewRequest("GET", s.AdminURL().WithPath("/_status/metrics/local").String(), nil) if err != nil { t.Fatalf("could not create request: %v", err) } @@ -528,7 +528,7 @@ func TestUseSplitCACerts(t *testing.T) { if err != nil { t.Fatalf("Expected success, got %v", err) } - req, err = http.NewRequest("GET", s.AdminURL()+"/_status/metrics/local", nil) + req, err = http.NewRequest("GET", s.AdminURL().WithPath("/_status/metrics/local").String(), nil) if err != nil { t.Fatalf("could not create request: %v", err) } @@ -630,7 +630,7 @@ func TestUseWrongSplitCACerts(t *testing.T) { if err != nil { t.Fatal(err) } - req, err := http.NewRequest("GET", s.AdminURL()+"/_status/metrics/local", nil) + req, err := http.NewRequest("GET", s.AdminURL().WithPath("/_status/metrics/local").String(), nil) if err != nil { t.Fatalf("could not create request: %v", err) } @@ -656,7 +656,7 @@ func TestUseWrongSplitCACerts(t *testing.T) { if err != nil { t.Fatalf("Expected success, got %v", err) } - req, err = http.NewRequest("GET", s.AdminURL()+"/_status/metrics/local", nil) + req, err = http.NewRequest("GET", s.AdminURL().WithPath("/_status/metrics/local").String(), nil) if err != nil { t.Fatalf("could not create request: %v", err) } diff --git a/pkg/server/admin_cluster_test.go b/pkg/server/admin_cluster_test.go index dea4cd2addcc..6710212743e7 100644 --- a/pkg/server/admin_cluster_test.go +++ b/pkg/server/admin_cluster_test.go @@ -128,7 +128,7 @@ func TestAdminAPITableStats(t *testing.T) { } } - url := server0.AdminURL() + "/_admin/v1/databases/test test/tables/foo foo/stats" + url := server0.AdminURL().WithPath("/_admin/v1/databases/test test/tables/foo foo/stats").String() var tsResponse serverpb.TableStatsResponse // The new SQL table may not yet have split into its own range. Wait for diff --git a/pkg/server/admin_test.go b/pkg/server/admin_test.go index 53b70fd59070..f88d2ef60b5d 100644 --- a/pkg/server/admin_test.go +++ b/pkg/server/admin_test.go @@ -132,7 +132,7 @@ func getJSON(ts serverutils.TestServerInterface, url string) (interface{}, error // debugURL returns the root debug URL. func debugURL(s serverutils.TestServerInterface) string { - return s.AdminURL() + debug.Endpoint + return s.AdminURL().WithPath(debug.Endpoint).String() } // TestAdminDebugExpVar verifies that cmdline and memstats variables are diff --git a/pkg/server/api_v2_ranges_test.go b/pkg/server/api_v2_ranges_test.go index 3b89130fdea2..b8fde8bed994 100644 --- a/pkg/server/api_v2_ranges_test.go +++ b/pkg/server/api_v2_ranges_test.go @@ -36,7 +36,7 @@ func TestHotRangesV2(t *testing.T) { client, err := ts.GetAdminHTTPClient() require.NoError(t, err) - req, err := http.NewRequest("GET", ts.AdminURL()+apiV2Path+"ranges/hot/", nil) + req, err := http.NewRequest("GET", ts.AdminURL().WithPath(apiV2Path+"ranges/hot/").String(), nil) require.NoError(t, err) resp, err := client.Do(req) require.NoError(t, err) @@ -76,7 +76,7 @@ func TestNodeRangesV2(t *testing.T) { client, err := ts.GetAdminHTTPClient() require.NoError(t, err) - req, err := http.NewRequest("GET", ts.AdminURL()+apiV2Path+"nodes/local/ranges/", nil) + req, err := http.NewRequest("GET", ts.AdminURL().WithPath(apiV2Path+"nodes/local/ranges/").String(), nil) require.NoError(t, err) resp, err := client.Do(req) require.NoError(t, err) @@ -141,7 +141,7 @@ func TestNodesV2(t *testing.T) { client, err := ts1.GetAdminHTTPClient() require.NoError(t, err) - req, err := http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"nodes/", nil) + req, err := http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"nodes/").String(), nil) require.NoError(t, err) resp, err := client.Do(req) require.NoError(t, err) diff --git a/pkg/server/api_v2_sql_schema_test.go b/pkg/server/api_v2_sql_schema_test.go index a9c4cebf9ef3..cb9bdc1978fe 100644 --- a/pkg/server/api_v2_sql_schema_test.go +++ b/pkg/server/api_v2_sql_schema_test.go @@ -44,7 +44,7 @@ func TestUsersV2(t *testing.T) { client, err := ts1.GetAdminHTTPClient() require.NoError(t, err) - req, err := http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"users/", nil) + req, err := http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"users/").String(), nil) require.NoError(t, err) resp, err := client.Do(req) require.NoError(t, err) @@ -86,7 +86,7 @@ func TestDatabasesTablesV2(t *testing.T) { require.NoError(t, err) defer client.CloseIdleConnections() - req, err := http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"databases/", nil) + req, err := http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"databases/").String(), nil) require.NoError(t, err) resp, err := client.Do(req) require.NoError(t, err) @@ -99,7 +99,7 @@ func TestDatabasesTablesV2(t *testing.T) { require.Contains(t, dr.Databases, "testdb") - req, err = http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"databases/testdb/", nil) + req, err = http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"databases/testdb/").String(), nil) require.NoError(t, err) resp, err = client.Do(req) require.NoError(t, err) @@ -110,7 +110,7 @@ func TestDatabasesTablesV2(t *testing.T) { require.NoError(t, json.NewDecoder(resp.Body).Decode(&ddr)) require.NoError(t, resp.Body.Close()) - req, err = http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"databases/testdb/grants/", nil) + req, err = http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"databases/testdb/grants/").String(), nil) require.NoError(t, err) resp, err = client.Do(req) require.NoError(t, err) @@ -122,7 +122,7 @@ func TestDatabasesTablesV2(t *testing.T) { require.NoError(t, resp.Body.Close()) require.NotEmpty(t, dgr.Grants) - req, err = http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"databases/testdb/tables/", nil) + req, err = http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"databases/testdb/tables/").String(), nil) require.NoError(t, err) resp, err = client.Do(req) require.NoError(t, err) @@ -135,7 +135,7 @@ func TestDatabasesTablesV2(t *testing.T) { require.Contains(t, dtr.TableNames, "public.testtable") // Test that querying the wrong db name returns 404. - req, err = http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"databases/testdb2/tables/", nil) + req, err = http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"databases/testdb2/tables/").String(), nil) require.NoError(t, err) resp, err = client.Do(req) require.NoError(t, err) @@ -143,7 +143,7 @@ func TestDatabasesTablesV2(t *testing.T) { require.Equal(t, 404, resp.StatusCode) require.NoError(t, resp.Body.Close()) - req, err = http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"databases/testdb/tables/public.testtable/", nil) + req, err = http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"databases/testdb/tables/public.testtable/").String(), nil) require.NoError(t, err) resp, err = client.Do(req) require.NoError(t, err) @@ -181,7 +181,7 @@ func TestEventsV2(t *testing.T) { client, err := ts1.GetAdminHTTPClient() require.NoError(t, err) - req, err := http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"events/", nil) + req, err := http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"events/").String(), nil) require.NoError(t, err) resp, err := client.Do(req) require.NoError(t, err) diff --git a/pkg/server/api_v2_sql_test.go b/pkg/server/api_v2_sql_test.go index 230a0be0b761..70d032364be3 100644 --- a/pkg/server/api_v2_sql_test.go +++ b/pkg/server/api_v2_sql_test.go @@ -62,7 +62,7 @@ func TestExecSQL(t *testing.T) { } resp, err := client.Post( - server.AdminURL()+"/api/v2/sql/", "application/json", + server.AdminURL().WithPath("/api/v2/sql/").String(), "application/json", bytes.NewReader([]byte(d.Input)), ) require.NoError(t, err) diff --git a/pkg/server/api_v2_test.go b/pkg/server/api_v2_test.go index ac65bb86ae98..7dd3747bf37f 100644 --- a/pkg/server/api_v2_test.go +++ b/pkg/server/api_v2_test.go @@ -59,7 +59,7 @@ func TestListSessionsV2(t *testing.T) { }() doSessionsRequest := func(client http.Client, limit int, start string) listSessionsResponse { - req, err := http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"sessions/", nil) + req, err := http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"sessions/").String(), nil) require.NoError(t, err) query := req.URL.Query() query.Add("exclude_closed_sessions", "true") @@ -120,7 +120,7 @@ func TestListSessionsV2(t *testing.T) { // A non-admin user cannot see sessions at all. nonAdminClient, err := ts1.GetAuthenticatedHTTPClient(false, serverutils.SingleTenantSession) require.NoError(t, err) - req, err := http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"sessions/", nil) + req, err := http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"sessions/").String(), nil) require.NoError(t, err) resp, err := nonAdminClient.Do(req) require.NoError(t, err) @@ -145,7 +145,7 @@ func TestHealthV2(t *testing.T) { client, err := ts1.GetAdminHTTPClient() require.NoError(t, err) - req, err := http.NewRequest("GET", ts1.AdminURL()+apiV2Path+"health/", nil) + req, err := http.NewRequest("GET", ts1.AdminURL().WithPath(apiV2Path+"health/").String(), nil) require.NoError(t, err) resp, err := client.Do(req) require.NoError(t, err) @@ -173,7 +173,7 @@ func TestRulesV2(t *testing.T) { client, err := ts.GetUnauthenticatedHTTPClient() require.NoError(t, err) - req, err := http.NewRequest("GET", ts.AdminURL()+apiV2Path+"rules/", nil) + req, err := http.NewRequest("GET", ts.AdminURL().WithPath(apiV2Path+"rules/").String(), nil) require.NoError(t, err) resp, err := client.Do(req) require.NoError(t, err) @@ -246,7 +246,7 @@ func TestAuthV2(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - req, err := http.NewRequest("GET", ts.AdminURL()+apiV2Path+"sessions/", nil) + req, err := http.NewRequest("GET", ts.AdminURL().WithPath(apiV2Path+"sessions/").String(), nil) require.NoError(t, err) if tc.header != "" { req.Header.Set(apiV2AuthHeader, tc.header) diff --git a/pkg/server/authentication_test.go b/pkg/server/authentication_test.go index 4b9781c887b2..912b8bb55c77 100644 --- a/pkg/server/authentication_test.go +++ b/pkg/server/authentication_test.go @@ -520,7 +520,7 @@ func TestAuthenticationAPIUserLogin(t *testing.T) { } var resp serverpb.UserLoginResponse return httputil.PostJSONWithRequest( - httpClient, ts.AdminURL()+loginPath, &req, &resp, + httpClient, ts.AdminURL().WithPath(loginPath).String(), &req, &resp, ) } @@ -591,7 +591,7 @@ func TestLogoutClearsCookies(t *testing.T) { require.NoError(t, err) // Log out. - resp, err := authHTTPClient.Get(ts.AdminURL() + logoutPath) + resp, err := authHTTPClient.Get(ts.AdminURL().WithPath(logoutPath).String()) require.NoError(t, err) defer resp.Body.Close() @@ -621,7 +621,7 @@ func TestLogout(t *testing.T) { // Log out. var resp serverpb.UserLogoutResponse - if err := httputil.GetJSON(authHTTPClient, ts.AdminURL()+logoutPath, &resp); err != nil { + if err := httputil.GetJSON(authHTTPClient, ts.AdminURL().WithPath(logoutPath).String(), &resp); err != nil { t.Fatal("logout request failed:", err) } @@ -637,7 +637,7 @@ func TestLogout(t *testing.T) { t.Fatal("expected revoked at to not be empty; was empty") } - databasesURL := ts.AdminURL() + "/_admin/v1/databases" + databasesURL := ts.AdminURL().WithPath("/_admin/v1/databases").String() // Verify that we're unauthorized after logout. response, err := authHTTPClient.Get(databasesURL) @@ -651,10 +651,6 @@ func TestLogout(t *testing.T) { } // Try to use the revoked cookie; verify that it doesn't work. - parsedURL, err := url.Parse(s.AdminURL()) - if err != nil { - t.Fatal(err) - } encodedCookie, err := EncodeSessionCookie(cookie, false /* forHTTPSOnly */) if err != nil { t.Fatal(err) @@ -669,7 +665,7 @@ func TestLogout(t *testing.T) { t.Fatal(err) } invalidAuthClient.Jar = jar - invalidAuthClient.Jar.SetCookies(parsedURL, []*http.Cookie{encodedCookie}) + invalidAuthClient.Jar.SetCookies(s.AdminURL().URL, []*http.Cookie{encodedCookie}) invalidAuthResp, err := invalidAuthClient.Get(databasesURL) if err != nil { @@ -708,7 +704,7 @@ func TestAuthenticationMux(t *testing.T) { runRequest := func( client http.Client, method string, path string, body []byte, cookieHeader string, expected int, ) error { - req, err := http.NewRequest(method, tsrv.AdminURL()+path, bytes.NewBuffer(body)) + req, err := http.NewRequest(method, tsrv.AdminURL().WithPath(path).String(), bytes.NewBuffer(body)) if cookieHeader != "" { // The client still attaches its own cookies to this one. req.Header.Set("cookie", cookieHeader) diff --git a/pkg/server/node_http_router_test.go b/pkg/server/node_http_router_test.go index d45d681c51ce..c56ebdc642b9 100644 --- a/pkg/server/node_http_router_test.go +++ b/pkg/server/node_http_router_test.go @@ -16,7 +16,6 @@ import ( "io" "net/http" "net/http/cookiejar" - "net/url" "regexp" "testing" @@ -154,11 +153,9 @@ func TestRouteToNode(t *testing.T) { client.Jar, err = cookiejar.New(&cookiejar.Options{}) require.NoError(t, err) } - adminURL, err := url.Parse(s.AdminURL()) - require.NoError(t, err) - client.Jar.SetCookies(adminURL, []*http.Cookie{{Name: RemoteNodeID, Value: rt.nodeIDRequestedInCookie}}) + client.Jar.SetCookies(s.AdminURL().URL, []*http.Cookie{{Name: RemoteNodeID, Value: rt.nodeIDRequestedInCookie}}) - requestPath := s.AdminURL() + rt.path + requestPath := s.AdminURL().WithPath(rt.path).String() if rt.nodeIDRequestedInQueryParam != "" { requestPath += fmt.Sprintf("?%s=%s", RemoteNodeID, rt.nodeIDRequestedInQueryParam) } @@ -191,7 +188,7 @@ func TestRouteToNode(t *testing.T) { client, err := s.GetUnauthenticatedHTTPClient() require.NoError(t, err) - resp, err := client.Get(s.AdminURL() + fmt.Sprintf("/_status/vars?%s=%s", RemoteNodeID, "2")) + resp, err := client.Get(s.AdminURL().WithPath(fmt.Sprintf("/_status/vars?%s=%s", RemoteNodeID, "2")).String()) require.NoError(t, err) defer resp.Body.Close() // We expect some error here. It's difficult to know what diff --git a/pkg/server/server_http_test.go b/pkg/server/server_http_test.go index 03112d87894f..55f5f6eb76e9 100644 --- a/pkg/server/server_http_test.go +++ b/pkg/server/server_http_test.go @@ -40,7 +40,7 @@ func TestHSTS(t *testing.T) { urlsToTest := []string{"/", "/_status/vars", "/index.html"} - adminURLHTTPS := s.AdminURL() + adminURLHTTPS := s.AdminURL().String() adminURLHTTP := strings.Replace(adminURLHTTPS, "https", "http", 1) for _, u := range urlsToTest { diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 9870d9c34616..893f1efa63d6 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -99,7 +99,7 @@ func TestPanicRecovery(t *testing.T) { })) // Create a request. - req, err := http.NewRequest(http.MethodGet, ts.AdminURL()+"/panic", nil /* body */) + req, err := http.NewRequest(http.MethodGet, ts.AdminURL().WithPath("/panic").String(), nil /* body */) require.NoError(t, err) // Create a ResponseRecorder to record the response. @@ -261,7 +261,7 @@ func TestPlainHTTPServer(t *testing.T) { // They won't succeed because we're not jumping through // authentication hoops, but they verify that the server is using // the correct protocol. - url := s.AdminURL() + url := s.AdminURL().String() if !strings.HasPrefix(url, "http://") { t.Fatalf("expected insecure admin url to start with http://, but got %s", url) } @@ -372,7 +372,7 @@ func TestAcceptEncoding(t *testing.T) { } for _, d := range testData { func() { - req, err := http.NewRequest("GET", s.AdminURL()+statusPrefix+"metrics/local", nil) + req, err := http.NewRequest("GET", s.AdminURL().WithPath(statusPrefix+"metrics/local").String(), nil) if err != nil { t.Fatalf("could not create request: %s", err) } @@ -764,7 +764,7 @@ func TestServeIndexHTML(t *testing.T) { require.NoError(t, err) t.Run("short build", func(t *testing.T) { - resp, err := client.Get(s.AdminURL()) + resp, err := client.Get(s.AdminURL().String()) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, 200, resp.StatusCode) @@ -785,7 +785,7 @@ Binary built without web UI. t.Run("non-short build", func(t *testing.T) { linkInFakeUI() defer unlinkFakeUI() - resp, err := client.Get(s.AdminURL()) + resp, err := client.Get(s.AdminURL().String()) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, 200, resp.StatusCode) @@ -796,7 +796,7 @@ Binary built without web UI. respString := string(respBytes) require.Equal(t, htmlTemplate, respString) - resp, err = client.Get(s.AdminURL() + "/uiconfig") + resp, err = client.Get(s.AdminURL().WithPath("/uiconfig").String()) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, 200, resp.StatusCode) @@ -851,7 +851,7 @@ Binary built without web UI. for i, testCase := range cases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - req, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL(), nil) + req, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL().String(), nil) require.NoError(t, err) resp, err := testCase.client.Do(req) @@ -865,7 +865,7 @@ Binary built without web UI. respString := string(respBytes) require.Equal(t, htmlTemplate, respString) - req, err = http.NewRequestWithContext(ctx, "GET", s.AdminURL()+"/uiconfig", nil) + req, err = http.NewRequestWithContext(ctx, "GET", s.AdminURL().WithPath("/uiconfig").String(), nil) require.NoError(t, err) resp, err = testCase.client.Do(req) @@ -925,7 +925,7 @@ Binary built without web UI. for _, testCase := range cases { t.Run(fmt.Sprintf("bundle caching for %s", testCase.desc), func(t *testing.T) { // Request bundle.js without an If-None-Match header first, to simulate the initial load - uncachedReq, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL()+"/bundle.js", nil) + uncachedReq, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL().WithPath("/bundle.js").String(), nil) require.NoError(t, err) uncachedResp, err := testCase.client.Do(uncachedReq) @@ -937,7 +937,7 @@ Binary built without web UI. require.NotEmpty(t, etag, "Server must provide ETag response header with asset responses") // Use that ETag header on the next request to simulate a client reload - cachedReq, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL()+"/bundle.js", nil) + cachedReq, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL().WithPath("/bundle.js").String(), nil) require.NoError(t, err) cachedReq.Header.Add("If-None-Match", etag) diff --git a/pkg/server/status_test.go b/pkg/server/status_test.go index 66c269cdf73c..e83346751ba1 100644 --- a/pkg/server/status_test.go +++ b/pkg/server/status_test.go @@ -193,7 +193,7 @@ func TestHealthTelemetry(t *testing.T) { if err := serverutils.GetJSONProto(s, "/health", &details); err != nil { t.Fatal(err) } - if _, err := getText(s, s.AdminURL()+statusPrefix+"vars"); err != nil { + if _, err := getText(s, s.AdminURL().WithPath(statusPrefix+"vars").String()); err != nil { t.Fatal(err) } @@ -1000,7 +1000,7 @@ func TestMetricsEndpoint(t *testing.T) { s := startServer(t) defer s.Stopper().Stop(context.Background()) - if _, err := getText(s, s.AdminURL()+statusPrefix+"metrics/"+s.Gossip().NodeID.String()); err != nil { + if _, err := getText(s, s.AdminURL().WithPath(statusPrefix+"metrics/"+s.Gossip().NodeID.String()).String()); err != nil { t.Fatal(err) } } @@ -1359,12 +1359,12 @@ func TestStatusVars(t *testing.T) { s, _, _ := serverutils.StartServer(t, base.TestServerArgs{}) defer s.Stopper().Stop(context.Background()) - if body, err := getText(s, s.AdminURL()+statusPrefix+"vars"); err != nil { + if body, err := getText(s, s.AdminURL().WithPath(statusPrefix+"vars").String()); err != nil { t.Fatal(err) } else if !bytes.Contains(body, []byte("# TYPE sql_bytesout counter\nsql_bytesout")) { t.Errorf("expected sql_bytesout, got: %s", body) } - if body, err := getText(s, s.AdminURL()+statusPrefix+"load"); err != nil { + if body, err := getText(s, s.AdminURL().WithPath(statusPrefix+"load").String()); err != nil { t.Fatal(err) } else if !bytes.Contains(body, []byte("# TYPE sys_cpu_user_ns gauge\nsys_cpu_user_ns")) { t.Errorf("expected sys_cpu_user_ns, got: %s", body) @@ -1388,7 +1388,7 @@ func TestStatusVarsTxnMetrics(t *testing.T) { t.Fatal(err) } - body, err := getText(s, s.AdminURL()+statusPrefix+"vars") + body, err := getText(s, s.AdminURL().WithPath(statusPrefix+"vars").String()) if err != nil { t.Fatal(err) } @@ -1430,7 +1430,7 @@ func TestSpanStatsResponse(t *testing.T) { Spans: []roachpb.Span{span}, } - url := ts.AdminURL() + statusPrefix + "span" + url := ts.AdminURL().WithPath(statusPrefix + "span").String() if err := httputil.PostJSON(httpClient, url, &request, &response); err != nil { t.Fatal(err) } diff --git a/pkg/server/testserver_http.go b/pkg/server/testserver_http.go index d560de7318a7..0510ffe64554 100644 --- a/pkg/server/testserver_http.go +++ b/pkg/server/testserver_http.go @@ -64,14 +64,14 @@ func (t tenantHeaderDecorator) RoundTrip(req *http.Request) (*http.Response, err var _ http.RoundTripper = &tenantHeaderDecorator{} // AdminURL implements TestServerInterface. -func (ts *httpTestServer) AdminURL() string { +func (ts *httpTestServer) AdminURL() *serverutils.TestURL { u := ts.t.sqlServer.execCfg.RPCContext.Config.AdminURL() if ts.t.tenantName != "" { q := u.Query() q.Add(TenantNameParamInQueryURL, string(ts.t.tenantName)) u.RawQuery = q.Encode() } - return u.String() + return &serverutils.TestURL{URL: u} } // GetUnauthenticatedHTTPClient implements TestServerInterface. diff --git a/pkg/testutils/serverutils/test_server_shim.go b/pkg/testutils/serverutils/test_server_shim.go index 4617d453ba04..2db2904efea2 100644 --- a/pkg/testutils/serverutils/test_server_shim.go +++ b/pkg/testutils/serverutils/test_server_shim.go @@ -523,7 +523,7 @@ func GetJSONProtoWithAdminOption( if err != nil { return err } - fullURL := ts.AdminURL() + path + fullURL := ts.AdminURL().WithPath(path).String() log.Infof(context.Background(), "test retrieving protobuf over HTTP: %s", fullURL) return httputil.GetJSON(httpClient, fullURL, response) } @@ -544,7 +544,7 @@ func PostJSONProtoWithAdminOption( if err != nil { return err } - fullURL := ts.AdminURL() + path + fullURL := ts.AdminURL().WithPath(path).String() log.Infof(context.Background(), "test retrieving protobuf over HTTP: %s", fullURL) return httputil.PostJSON(httpClient, fullURL, request, response) } diff --git a/pkg/testutils/serverutils/test_tenant_shim.go b/pkg/testutils/serverutils/test_tenant_shim.go index 7d367f7ff2de..b92d984bc5ba 100644 --- a/pkg/testutils/serverutils/test_tenant_shim.go +++ b/pkg/testutils/serverutils/test_tenant_shim.go @@ -16,6 +16,7 @@ package serverutils import ( "context" "net/http" + "net/url" "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/config" @@ -38,6 +39,27 @@ const ( MultiTenantSession ) +type TestURL struct { + *url.URL +} + +func NewTestURL(path string) TestURL { + u, err := url.Parse(path) + if err != nil { + panic(err) + } + return TestURL{u} +} + +func (t *TestURL) WithPath(path string) *TestURL { + newPath, err := url.JoinPath(t.Path, path) + if err != nil { + panic(err) + } + t.Path = newPath + return t +} + // TestTenantInterface defines SQL-only tenant functionality that tests need; it // is implemented by server.Test{Tenant,Server}. Tests written against this // interface are effectively agnostic to the type of tenant (host or secondary) @@ -147,7 +169,7 @@ type TestTenantInterface interface { AmbientCtx() log.AmbientContext // AdminURL returns the URL for the admin UI. - AdminURL() string + AdminURL() *TestURL // GetUnauthenticatedHTTPClient returns an http client configured with the client TLS // config required by the TestServer's configuration. // Discourages implementer from using unauthenticated http connections diff --git a/pkg/testutils/testcluster/testcluster_test.go b/pkg/testutils/testcluster/testcluster_test.go index b4b381a34a8a..413b7cdecda1 100644 --- a/pkg/testutils/testcluster/testcluster_test.go +++ b/pkg/testutils/testcluster/testcluster_test.go @@ -203,7 +203,7 @@ func TestStopServer(t *testing.T) { if err != nil { t.Fatal(err) } - url := server1.AdminURL() + "/_status/metrics/local" + url := server1.AdminURL().WithPath("/_status/metrics/local").String() if err := httputil.GetJSON(httpClient1, url, &response); err != nil { t.Fatal(err) } @@ -265,7 +265,7 @@ func TestStopServer(t *testing.T) { if err != nil { t.Fatal(err) } - url = tc.Server(0).AdminURL() + "/_status/metrics/local" + url = tc.Server(0).AdminURL().WithPath("/_status/metrics/local").String() if err := httputil.GetJSON(httpClient1, url, &response); err != nil { t.Fatal(err) }