diff --git a/pkg/ccl/logictestccl/testdata/logic_test/tenant_capability b/pkg/ccl/logictestccl/testdata/logic_test/tenant_capability index 790382695505..b4f9559a17bb 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/tenant_capability +++ b/pkg/ccl/logictestccl/testdata/logic_test/tenant_capability @@ -36,6 +36,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -61,6 +62,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -79,6 +81,7 @@ can_admin_scatter true can_admin_split false can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -104,6 +107,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -129,6 +133,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -154,6 +159,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info true can_view_tsdb_metrics false @@ -172,6 +178,7 @@ can_admin_scatter true can_admin_split false can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -190,6 +197,7 @@ can_admin_scatter true can_admin_split false can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -208,6 +216,7 @@ can_admin_scatter true can_admin_split false can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -227,6 +236,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit true can_check_consistency true +can_debug_process true can_use_nodelocal_storage true can_view_node_info true can_view_tsdb_metrics true @@ -268,6 +278,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -297,6 +308,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -331,6 +343,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -349,6 +362,7 @@ can_admin_scatter false can_admin_split false can_admin_unsplit false can_check_consistency false +can_debug_process false can_use_nodelocal_storage false can_view_node_info false can_view_tsdb_metrics false @@ -367,6 +381,7 @@ can_admin_scatter true can_admin_split true can_admin_unsplit true can_check_consistency true +can_debug_process true can_use_nodelocal_storage true can_view_node_info true can_view_tsdb_metrics true diff --git a/pkg/ccl/oidcccl/authentication_oidc_test.go b/pkg/ccl/oidcccl/authentication_oidc_test.go index ad07c8e3bbce..2d7c06170da5 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..2a193ec080ca 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" @@ -167,7 +166,6 @@ func TestServerControllerHTTP(t *testing.T) { // Retrieve a privileged HTTP client. NB: this also populates // system.web_sessions. - aurl := s.AdminURL() client, err := s.GetAdminHTTPClient() require.NoError(t, err) @@ -252,8 +250,10 @@ VALUES($1, $2, $3, $4, $5, (SELECT user_id FROM system.users WHERE username = $3 return &ls, err } + var aurl *serverutils.TestURL newreq := func() *http.Request { - req, err := http.NewRequest("GET", aurl+"/_status/sessions", nil) + aurl = s.AdminURL() + req, err := http.NewRequest("GET", aurl.WithPath("/_status/sessions").String(), nil) require.NoError(t, err) return req } @@ -289,9 +289,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 +301,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 +349,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 +390,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 +398,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 +567,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 +588,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 +603,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 +613,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..3e9ef4a785df 100644 --- a/pkg/ccl/serverccl/server_sql_test.go +++ b/pkg/ccl/serverccl/server_sql_test.go @@ -13,6 +13,7 @@ import ( "errors" "fmt" "io" + "net/http" "strings" "testing" "time" @@ -20,6 +21,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/ccl" "github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl" + "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/server/systemconfigwatcher/systemconfigwatchertest" @@ -169,7 +171,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 +182,11 @@ 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") + u := tenant.AdminURL().WithPath("/debug/pprof/goroutine") + q := u.Query() + q.Set("debug", "2") + u.RawQuery = q.Encode() + resp, err := httpClient.Get(u.String()) require.NoError(t, err) defer resp.Body.Close() body, err := io.ReadAll(resp.Body) @@ -190,6 +196,137 @@ func TestTenantHTTP(t *testing.T) { } +// TestTenantProcessDebugging verifies that in-process SQL tenant servers gate +// process debugging behind capabilities. +func TestTenantProcessDebugging(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + ctx := context.Background() + + tc := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ + DefaultTestTenant: base.TestTenantDisabled, + }}) + defer tc.Stopper().Stop(ctx) + db := tc.ServerConn(0) + defer db.Close() + + s := tc.Server(0) + + tenant, _, err := s.StartSharedProcessTenant(ctx, + base.TestSharedProcessTenantArgs{ + TenantID: serverutils.TestTenantID(), + TenantName: "processdebug", + }) + require.NoError(t, err) + defer tenant.Stopper().Stop(ctx) + + t.Run("system tenant pprof", func(t *testing.T) { + httpClient, err := s.GetAdminHTTPClient() + require.NoError(t, err) + defer httpClient.CloseIdleConnections() + + url := s.AdminURL().URL + url.Path = url.Path + "/debug/pprof/goroutine" + q := url.Query() + q.Add("debug", "2") + url.RawQuery = q.Encode() + + resp, err := httpClient.Get(url.String()) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Contains(t, string(body), "goroutine") + }) + + t.Run("pprof", func(t *testing.T) { + httpClient, err := tenant.GetAdminHTTPClient() + require.NoError(t, err) + defer httpClient.CloseIdleConnections() + + url := tenant.AdminURL().URL + url.Path = url.Path + "/debug/pprof/" + q := url.Query() + q.Add("debug", "2") + url.RawQuery = q.Encode() + + resp, err := httpClient.Get(url.String()) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + require.Contains(t, string(body), "tenant does not have capability to debug the running process") + + _, err = db.Exec(`ALTER TENANT processdebug GRANT CAPABILITY can_debug_process=true`) + require.NoError(t, err) + + tc.WaitForTenantCapabilities(t, serverutils.TestTenantID(), map[tenantcapabilities.ID]string{ + tenantcapabilities.CanDebugProcess: "true", + }) + + resp, err = httpClient.Get(url.String()) + require.NoError(t, err) + defer resp.Body.Close() + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Contains(t, string(body), "goroutine") + + _, err = db.Exec(`ALTER TENANT processdebug REVOKE CAPABILITY can_debug_process`) + require.NoError(t, err) + + tc.WaitForTenantCapabilities(t, serverutils.TestTenantID(), map[tenantcapabilities.ID]string{ + tenantcapabilities.CanDebugProcess: "false", + }) + }) + + t.Run("vmodule", func(t *testing.T) { + httpClient, err := tenant.GetAdminHTTPClient() + require.NoError(t, err) + defer httpClient.CloseIdleConnections() + + url := tenant.AdminURL().URL + url.Path = url.Path + "/debug/vmodule" + q := url.Query() + q.Add("duration", "-1s") + q.Add("vmodule", "exec_log=3") + url.RawQuery = q.Encode() + + resp, err := httpClient.Get(url.String()) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + require.Contains(t, string(body), "tenant does not have capability to debug the running process") + + _, err = db.Exec(`ALTER TENANT processdebug GRANT CAPABILITY can_debug_process=true`) + require.NoError(t, err) + + tc.WaitForTenantCapabilities(t, serverutils.TestTenantID(), map[tenantcapabilities.ID]string{ + tenantcapabilities.CanDebugProcess: "true", + }) + + resp, err = httpClient.Get(url.String()) + require.NoError(t, err) + defer resp.Body.Close() + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Contains(t, string(body), "previous vmodule configuration: \nnew vmodule configuration: exec_log=3\n") + + _, err = db.Exec(`ALTER TENANT processdebug REVOKE CAPABILITY can_debug_process`) + require.NoError(t, err) + + tc.WaitForTenantCapabilities(t, serverutils.TestTenantID(), map[tenantcapabilities.ID]string{ + tenantcapabilities.CanDebugProcess: "false", + }) + }) +} + func TestNonExistentTenant(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) 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/configprofiles/testdata/multitenant-app b/pkg/configprofiles/testdata/multitenant-app index a4e71ecbb9fe..d2425716bcd5 100644 --- a/pkg/configprofiles/testdata/multitenant-app +++ b/pkg/configprofiles/testdata/multitenant-app @@ -58,6 +58,7 @@ SHOW TENANTS WITH CAPABILITIES 1 system ready shared can_admin_split true 1 system ready shared can_admin_unsplit true 1 system ready shared can_check_consistency true +1 system ready shared can_debug_process true 1 system ready shared can_use_nodelocal_storage true 1 system ready shared can_view_node_info true 1 system ready shared can_view_tsdb_metrics true @@ -68,6 +69,7 @@ SHOW TENANTS WITH CAPABILITIES 2 template ready none can_admin_split true 2 template ready none can_admin_unsplit true 2 template ready none can_check_consistency true +2 template ready none can_debug_process true 2 template ready none can_use_nodelocal_storage true 2 template ready none can_view_node_info true 2 template ready none can_view_tsdb_metrics true @@ -78,6 +80,7 @@ SHOW TENANTS WITH CAPABILITIES 3 application ready shared can_admin_split true 3 application ready shared can_admin_unsplit true 3 application ready shared can_check_consistency true +3 application ready shared can_debug_process true 3 application ready shared can_use_nodelocal_storage true 3 application ready shared can_view_node_info true 3 application ready shared can_view_tsdb_metrics true diff --git a/pkg/configprofiles/testdata/multitenant-app-repl b/pkg/configprofiles/testdata/multitenant-app-repl index ae5e43c6f68c..bbfef13442e1 100644 --- a/pkg/configprofiles/testdata/multitenant-app-repl +++ b/pkg/configprofiles/testdata/multitenant-app-repl @@ -58,6 +58,7 @@ SHOW TENANTS WITH CAPABILITIES 1 system ready shared can_admin_split true 1 system ready shared can_admin_unsplit true 1 system ready shared can_check_consistency true +1 system ready shared can_debug_process true 1 system ready shared can_use_nodelocal_storage true 1 system ready shared can_view_node_info true 1 system ready shared can_view_tsdb_metrics true @@ -68,6 +69,7 @@ SHOW TENANTS WITH CAPABILITIES 2 template ready none can_admin_split true 2 template ready none can_admin_unsplit true 2 template ready none can_check_consistency true +2 template ready none can_debug_process true 2 template ready none can_use_nodelocal_storage true 2 template ready none can_view_node_info true 2 template ready none can_view_tsdb_metrics true @@ -78,6 +80,7 @@ SHOW TENANTS WITH CAPABILITIES 3 application ready shared can_admin_split true 3 application ready shared can_admin_unsplit true 3 application ready shared can_check_consistency true +3 application ready shared can_debug_process true 3 application ready shared can_use_nodelocal_storage true 3 application ready shared can_view_node_info true 3 application ready shared can_view_tsdb_metrics true diff --git a/pkg/configprofiles/testdata/multitenant-noapp b/pkg/configprofiles/testdata/multitenant-noapp index dc207e1a0b3d..1785b6138a68 100644 --- a/pkg/configprofiles/testdata/multitenant-noapp +++ b/pkg/configprofiles/testdata/multitenant-noapp @@ -51,6 +51,7 @@ SHOW TENANTS WITH CAPABILITIES 1 system ready shared can_admin_split true 1 system ready shared can_admin_unsplit true 1 system ready shared can_check_consistency true +1 system ready shared can_debug_process true 1 system ready shared can_use_nodelocal_storage true 1 system ready shared can_view_node_info true 1 system ready shared can_view_tsdb_metrics true @@ -61,6 +62,7 @@ SHOW TENANTS WITH CAPABILITIES 2 template ready none can_admin_split true 2 template ready none can_admin_unsplit true 2 template ready none can_check_consistency true +2 template ready none can_debug_process true 2 template ready none can_use_nodelocal_storage true 2 template ready none can_view_node_info true 2 template ready none can_view_tsdb_metrics true diff --git a/pkg/configprofiles/testdata/multitenant-noapp-repl b/pkg/configprofiles/testdata/multitenant-noapp-repl index e5af19772c99..52456f3f6f57 100644 --- a/pkg/configprofiles/testdata/multitenant-noapp-repl +++ b/pkg/configprofiles/testdata/multitenant-noapp-repl @@ -51,6 +51,7 @@ SHOW TENANTS WITH CAPABILITIES 1 system ready shared can_admin_split true 1 system ready shared can_admin_unsplit true 1 system ready shared can_check_consistency true +1 system ready shared can_debug_process true 1 system ready shared can_use_nodelocal_storage true 1 system ready shared can_view_node_info true 1 system ready shared can_view_tsdb_metrics true @@ -61,6 +62,7 @@ SHOW TENANTS WITH CAPABILITIES 2 template ready none can_admin_split true 2 template ready none can_admin_unsplit true 2 template ready none can_check_consistency true +2 template ready none can_debug_process true 2 template ready none can_use_nodelocal_storage true 2 template ready none can_view_node_info true 2 template ready none can_view_tsdb_metrics true 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/kv/kvserver/tenantrate/limiter_test.go b/pkg/kv/kvserver/tenantrate/limiter_test.go index 5efc33f62409..446fb1c17f36 100644 --- a/pkg/kv/kvserver/tenantrate/limiter_test.go +++ b/pkg/kv/kvserver/tenantrate/limiter_test.go @@ -667,6 +667,16 @@ func (ts *testState) HasCapabilityForBatch( func (ts *testState) BindReader(tenantcapabilities.Reader) {} +var _ tenantcapabilities.Authorizer = &testState{} + +func (ts *testState) HasProcessDebugCapability(ctx context.Context, tenID roachpb.TenantID) error { + if ts.capabilities[tenID].CanDebugProcess { + return nil + } else { + return errors.New("unauthorized") + } +} + func (ts *testState) HasNodeStatusCapability(_ context.Context, tenID roachpb.TenantID) error { if ts.capabilities[tenID].CanViewNodeInfo { return nil @@ -767,6 +777,8 @@ func parseStrings(t *testing.T, d *datadriven.TestData) []string { // is subject to rate limit checks. (For testing in this package.) type fakeAuthorizer struct{} +var _ tenantcapabilities.Authorizer = &fakeAuthorizer{} + func (fakeAuthorizer) HasNodeStatusCapability(_ context.Context, tenID roachpb.TenantID) error { return nil } @@ -787,3 +799,7 @@ func (fakeAuthorizer) HasCapabilityForBatch( return nil } func (fakeAuthorizer) BindReader(tenantcapabilities.Reader) {} + +func (fakeAuthorizer) HasProcessDebugCapability(ctx context.Context, tenID roachpb.TenantID) error { + return nil +} diff --git a/pkg/multitenant/tenantcapabilities/capabilities.go b/pkg/multitenant/tenantcapabilities/capabilities.go index e9869a79bb81..c6b59e442dd4 100644 --- a/pkg/multitenant/tenantcapabilities/capabilities.go +++ b/pkg/multitenant/tenantcapabilities/capabilities.go @@ -84,6 +84,11 @@ const ( // span configs. TenantSpanConfigBounds // span_config_bounds + // CanDebugProcess describes the ability of a tenant to set vmodule on the + // process and run pprof profiles and tools. This can reveal information + // across tenant boundaries. + CanDebugProcess // can_debug_process + MaxCapabilityID ID = iota - 1 ) @@ -114,6 +119,7 @@ var capabilities = [MaxCapabilityID + 1]Capability{ CanViewTSDBMetrics: boolCapability(CanViewTSDBMetrics), ExemptFromRateLimiting: boolCapability(ExemptFromRateLimiting), TenantSpanConfigBounds: spanConfigBoundsCapability(TenantSpanConfigBounds), + CanDebugProcess: boolCapability(CanDebugProcess), } // EnableAll enables maximum access to services. diff --git a/pkg/multitenant/tenantcapabilities/id_string.go b/pkg/multitenant/tenantcapabilities/id_string.go index 4ba2401e8697..da79f2376809 100644 --- a/pkg/multitenant/tenantcapabilities/id_string.go +++ b/pkg/multitenant/tenantcapabilities/id_string.go @@ -18,7 +18,8 @@ func _() { _ = x[CanViewTSDBMetrics-8] _ = x[ExemptFromRateLimiting-9] _ = x[TenantSpanConfigBounds-10] - _ = x[MaxCapabilityID-10] + _ = x[CanDebugProcess-11] + _ = x[MaxCapabilityID-11] } func (i ID) String() string { @@ -43,6 +44,8 @@ func (i ID) String() string { return "exempt_from_rate_limiting" case TenantSpanConfigBounds: return "span_config_bounds" + case CanDebugProcess: + return "can_debug_process" default: return "ID(" + strconv.FormatInt(int64(i), 10) + ")" } @@ -59,7 +62,8 @@ var stringToCapabilityIDMap = map[string]ID{ "can_view_tsdb_metrics": 8, "exempt_from_rate_limiting": 9, "span_config_bounds": 10, - "MaxCapabilityID": 10, + "can_debug_process": 11, + "MaxCapabilityID": 11, } var IDs = []ID{ @@ -68,6 +72,7 @@ var IDs = []ID{ CanAdminSplit, CanAdminUnsplit, CanCheckConsistency, + CanDebugProcess, CanUseNodelocalStorage, CanViewNodeInfo, CanViewTSDBMetrics, diff --git a/pkg/multitenant/tenantcapabilities/interfaces.go b/pkg/multitenant/tenantcapabilities/interfaces.go index 1dd4dfb911a4..498c9f1aaa0d 100644 --- a/pkg/multitenant/tenantcapabilities/interfaces.go +++ b/pkg/multitenant/tenantcapabilities/interfaces.go @@ -70,6 +70,10 @@ type Authorizer interface { // IsExemptFromRateLimiting returns true of the tenant should // not be subject to rate limiting. IsExemptFromRateLimiting(ctx context.Context, tenID roachpb.TenantID) bool + + // HasProcessDebugCapability returns an error if a tenant, referenced by its ID, + // is not allowed to debug the running process. + HasProcessDebugCapability(ctx context.Context, tenID roachpb.TenantID) error } // Entry ties together a tenantID with its capabilities. diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/allow_everything.go b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/allow_everything.go index 1426b7c58516..e6bc27658a3a 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/allow_everything.go +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/allow_everything.go @@ -66,3 +66,10 @@ func (n *AllowEverythingAuthorizer) IsExemptFromRateLimiting( ) bool { return true } + +// HasProcessDebugCapability implements the tenantcapabilities.Authorizer interface. +func (n *AllowEverythingAuthorizer) HasProcessDebugCapability( + ctx context.Context, tenID roachpb.TenantID, +) error { + return nil +} diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/allow_nothing.go b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/allow_nothing.go index f759f6ac0e76..02eb5a0597aa 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/allow_nothing.go +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/allow_nothing.go @@ -65,3 +65,10 @@ func (n *AllowNothingAuthorizer) HasNodelocalStorageCapability( func (n *AllowNothingAuthorizer) IsExemptFromRateLimiting(context.Context, roachpb.TenantID) bool { return false } + +// HasProcessDebugCapability implements the tenantcapabilities.Authorizer interface. +func (n *AllowNothingAuthorizer) HasProcessDebugCapability( + ctx context.Context, tenID roachpb.TenantID, +) error { + return errors.New("operation blocked") +} diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go index 2c4ae8f1b034..9443b77fc2ea 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go @@ -390,3 +390,32 @@ func (a *Authorizer) getMode( } return cp, selectedMode } + +func (a *Authorizer) HasProcessDebugCapability(ctx context.Context, tenID roachpb.TenantID) error { + if tenID.IsSystem() { + return nil + } + errFn := func() error { + return errors.New("client tenant does not have capability to debug the process") + } + cp, mode := a.getMode(ctx, tenID) + switch mode { + case authorizerModeOn: + break // fallthrough to the next check. + case authorizerModeAllowAll: + return nil + case authorizerModeV222: + return errFn() + default: + err := errors.AssertionFailedf("unknown authorizer mode: %d", mode) + logcrash.ReportOrPanic(ctx, &a.settings.SV, "%v", err) + return err + } + + if !tenantcapabilities.MustGetBoolByID( + cp, tenantcapabilities.CanDebugProcess, + ) { + return errFn() + } + return nil +} diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/authorizer_enabled b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/authorizer_enabled index b4e244d89491..0f345632857c 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/authorizer_enabled +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/authorizer_enabled @@ -8,7 +8,7 @@ client tenant does not have capability "can_admin_scatter" (*kvpb.AdminScatterRe has-capability-for-batch ten=10 cmds=(Merge) ---- -client tenant does not have capability "ID(12)" (*kvpb.MergeRequest) +client tenant does not have capability "ID(13)" (*kvpb.MergeRequest) has-node-status-capability ten=10 ---- @@ -34,7 +34,7 @@ ok has-capability-for-batch ten=10 cmds=(Merge) ---- -client tenant does not have capability "ID(12)" (*kvpb.MergeRequest) +client tenant does not have capability "ID(13)" (*kvpb.MergeRequest) has-node-status-capability ten=10 ---- diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto b/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto index 398c2c08458b..173d0dfda0e2 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto @@ -92,6 +92,10 @@ message TenantCapabilities { // CanCheckConsistency if set to true, grants the tenant the ability to run // range consistency checks. bool can_check_consistency = 10; + + // CanDebugProcess, if set to true, grants the tenant the ability to + // set vmodule on the process and run pprof profiles and tools. + bool can_debug_process = 11; }; // SpanConfigBound is used to constrain the possible values a SpanConfig may diff --git a/pkg/multitenant/tenantcapabilities/values.go b/pkg/multitenant/tenantcapabilities/values.go index 9f9d93f3af70..50dc8daab735 100644 --- a/pkg/multitenant/tenantcapabilities/values.go +++ b/pkg/multitenant/tenantcapabilities/values.go @@ -56,6 +56,8 @@ func GetValueByID(t *tenantcapabilitiespb.TenantCapabilities, id ID) (Value, err return (*boolValue)(&t.ExemptFromRateLimiting), nil case TenantSpanConfigBounds: return &spanConfigBoundsValue{b: &t.SpanConfigBounds}, nil + case CanDebugProcess: + return (*boolValue)(&t.CanDebugProcess), nil default: return nil, errors.AssertionFailedf("unknown capability: %q", id.String()) } diff --git a/pkg/rpc/auth_test.go b/pkg/rpc/auth_test.go index 7e7219bc81a5..b0a5d8f6cfbc 100644 --- a/pkg/rpc/auth_test.go +++ b/pkg/rpc/auth_test.go @@ -954,6 +954,12 @@ type mockAuthorizer struct { hasExemptFromRateLimiterCapability bool } +func (m mockAuthorizer) HasProcessDebugCapability( + ctx context.Context, tenID roachpb.TenantID, +) error { + return errors.New("tenant does not have capability") +} + var _ tenantcapabilities.Authorizer = &mockAuthorizer{} // HasCapabilityForBatch implements the tenantcapabilities.Authorizer interface. 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..4fce51af9edc 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().String() + "/_admin/v1/databases/test test/tables/foo foo/stats" 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..e2f40bddb141 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 @@ -382,14 +382,14 @@ func TestAdminAPIStatementDiagnosticsBundle(t *testing.T) { client, err := ts.GetAuthenticatedHTTPClient(false, serverutils.SingleTenantSession) require.NoError(t, err) - resp, err := client.Get(ts.AdminURL() + "/_admin/v1/stmtbundle/" + diagnosticRow) + resp, err := client.Get(ts.AdminURL().WithPath("/_admin/v1/stmtbundle/" + diagnosticRow).String()) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, 500, resp.StatusCode) adminClient, err := ts.GetAuthenticatedHTTPClient(true, serverutils.SingleTenantSession) require.NoError(t, err) - adminResp, err := adminClient.Get(ts.AdminURL() + "/_admin/v1/stmtbundle/" + diagnosticRow) + adminResp, err := adminClient.Get(ts.AdminURL().WithPath("/_admin/v1/stmtbundle/" + diagnosticRow).String()) require.NoError(t, err) defer adminResp.Body.Close() require.Equal(t, 200, adminResp.StatusCode) 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/config.go b/pkg/server/config.go index 096369fac8e2..0a2993eed8ed 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -30,6 +30,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed" "github.com/cockroachdb/cockroach/pkg/kv/kvpb" "github.com/cockroachdb/cockroach/pkg/kv/kvserver" + "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/rpc" "github.com/cockroachdb/cockroach/pkg/server/autoconfig/acprovider" @@ -536,6 +537,10 @@ type LocalKVServerInfo struct { InternalServer kvpb.InternalServer ServerInterceptors rpc.ServerInterceptorInfo Tracer *tracing.Tracer + + // SameProcessCapabilityAuthorizer is the tenant capability authorizer to + // use for servers running in the same process as the KV node. + SameProcessCapabilityAuthorizer tenantcapabilities.Authorizer } // MakeSQLConfig returns a SQLConfig with default values. diff --git a/pkg/server/debug/BUILD.bazel b/pkg/server/debug/BUILD.bazel index 90bc9c8c4609..3c40fcc2a6ea 100644 --- a/pkg/server/debug/BUILD.bazel +++ b/pkg/server/debug/BUILD.bazel @@ -17,6 +17,7 @@ go_library( "//pkg/kv/kvserver", "//pkg/kv/kvserver/closedts/sidetransport", "//pkg/kv/kvserver/kvstorage", + "//pkg/multitenant/tenantcapabilities", "//pkg/roachpb", "//pkg/server/debug/goroutineui", "//pkg/server/debug/pprofui", diff --git a/pkg/server/debug/server.go b/pkg/server/debug/server.go index 284b6cf3f8b5..f4caee78e89c 100644 --- a/pkg/server/debug/server.go +++ b/pkg/server/debug/server.go @@ -24,6 +24,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv/kvserver" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/sidetransport" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvstorage" + "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/server/debug/goroutineui" "github.com/cockroachdb/cockroach/pkg/server/debug/pprofui" @@ -72,41 +73,98 @@ type Server struct { type serverTickleFn = func(ctx context.Context, name roachpb.TenantName) error -// NewServer sets up a debug server. -func NewServer( - ambientContext log.AmbientContext, +func setupProcessWideRoutes( + mux *http.ServeMux, st *cluster.Settings, - hbaConfDebugFn http.HandlerFunc, + tenantID roachpb.TenantID, + authorizer tenantcapabilities.Authorizer, + vsrv *vmoduleServer, profiler pprofui.Profiler, - serverTickleFn serverTickleFn, -) *Server { - mux := http.NewServeMux() +) { + authzCheck := func(w http.ResponseWriter, r *http.Request) bool { + if err := authorizer.HasProcessDebugCapability(r.Context(), tenantID); err != nil { + http.Error(w, "tenant does not have capability to debug the running process", http.StatusForbidden) + return false + } + return true + } - // Install a redirect to the UI's collection of debug tools. - mux.HandleFunc(Endpoint, handleLanding) + authzFunc := func(origHandler http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if authzCheck(w, r) { + origHandler(w, r) + } + } + } // Cribbed straight from pprof's `init()` method. See: // https://golang.org/src/net/http/pprof/pprof.go - mux.HandleFunc("/debug/pprof/", pprof.Index) - mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) - mux.HandleFunc("/debug/pprof/profile", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/debug/pprof/", authzFunc(pprof.Index)) + mux.HandleFunc("/debug/pprof/cmdline", authzFunc(pprof.Cmdline)) + mux.HandleFunc("/debug/pprof/profile", authzFunc(func(w http.ResponseWriter, r *http.Request) { CPUProfileHandler(st, w, r) - }) - mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) - mux.HandleFunc("/debug/pprof/trace", pprof.Trace) + })) + mux.HandleFunc("/debug/pprof/symbol", authzFunc(pprof.Symbol)) + mux.HandleFunc("/debug/pprof/trace", authzFunc(pprof.Trace)) // Cribbed straight from trace's `init()` method. See: // https://github.com/golang/net/blob/master/trace/trace.go - mux.HandleFunc("/debug/requests", trace.Traces) - mux.HandleFunc("/debug/events", trace.Events) + mux.HandleFunc("/debug/requests", authzFunc(trace.Traces)) + mux.HandleFunc("/debug/events", authzFunc(trace.Events)) // This registers a superset of the variables exposed through the // /debug/vars endpoint onto the /debug/metrics endpoint. It includes all // expvars registered globally and all metrics registered on the // DefaultRegistry. - mux.Handle("/debug/metrics", exp.ExpHandler(metrics.DefaultRegistry)) + mux.HandleFunc("/debug/metrics", authzFunc(exp.ExpHandler(metrics.DefaultRegistry).ServeHTTP)) // Also register /debug/vars (even though /debug/metrics is better). - mux.Handle("/debug/vars", expvar.Handler()) + mux.HandleFunc("/debug/vars", authzFunc(expvar.Handler().ServeHTTP)) + + // Register the stopper endpoint, which lists all active tasks. + mux.HandleFunc("/debug/stopper", authzFunc(stop.HandleDebug)) + + // Set up the vmodule endpoint. + mux.HandleFunc("/debug/vmodule", authzFunc(vsrv.vmoduleHandleDebug)) + + ps := pprofui.NewServer(pprofui.NewMemStorage(pprofui.ProfileConcurrency, pprofui.ProfileExpiry), profiler) + mux.Handle("/debug/pprof/ui/", authzFunc(func(w http.ResponseWriter, r *http.Request) { + http.StripPrefix("/debug/pprof/ui", ps).ServeHTTP(w, r) + })) + + mux.HandleFunc("/debug/pprof/goroutineui/", authzFunc(func(w http.ResponseWriter, req *http.Request) { + dump := goroutineui.NewDump() + + _ = req.ParseForm() + switch req.Form.Get("sort") { + case "count": + dump.SortCountDesc() + case "wait": + dump.SortWaitDesc() + default: + } + _ = dump.HTML(w) + })) + +} + +// NewServer sets up a debug server. +func NewServer( + ambientContext log.AmbientContext, + st *cluster.Settings, + hbaConfDebugFn http.HandlerFunc, + profiler pprofui.Profiler, + serverTickleFn serverTickleFn, + tenantID roachpb.TenantID, + authorizer tenantcapabilities.Authorizer, +) *Server { + mux := http.NewServeMux() + + // Install a redirect to the UI's collection of debug tools. + mux.HandleFunc(Endpoint, handleLanding) + + // Debug routes that retrieve process-wide state. + vsrv := &vmoduleServer{} + setupProcessWideRoutes(mux, st, tenantID, authorizer, vsrv, profiler) if hbaConfDebugFn != nil { // Expose the processed HBA configuration through the debug @@ -114,9 +172,6 @@ func NewServer( mux.HandleFunc("/debug/hba_conf", hbaConfDebugFn) } - // Register the stopper endpoint, which lists all active tasks. - mux.HandleFunc("/debug/stopper", stop.HandleDebug) - if serverTickleFn != nil { // Register the server tickling function. // @@ -126,10 +181,6 @@ func NewServer( mux.Handle("/debug/tickle", handleTickle(serverTickleFn)) } - // Set up the vmodule endpoint. - vsrv := &vmoduleServer{} - mux.HandleFunc("/debug/vmodule", vsrv.vmoduleHandleDebug) - // Set up the log spy, a tool that allows inspecting filtered logs at high // verbosity. We require the tenant ID from the ambientCtx to set the logSpy // tenant filter. @@ -149,23 +200,6 @@ func NewServer( mux.HandleFunc("/debug/logspy", spy.handleDebugLogSpy) - ps := pprofui.NewServer(pprofui.NewMemStorage(pprofui.ProfileConcurrency, pprofui.ProfileExpiry), profiler) - mux.Handle("/debug/pprof/ui/", http.StripPrefix("/debug/pprof/ui", ps)) - - mux.HandleFunc("/debug/pprof/goroutineui/", func(w http.ResponseWriter, req *http.Request) { - dump := goroutineui.NewDump() - - _ = req.ParseForm() - switch req.Form.Get("sort") { - case "count": - dump.SortCountDesc() - case "wait": - dump.SortWaitDesc() - default: - } - _ = dump.HTML(w) - }) - return &Server{ ambientCtx: ambientContext, st: st, 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.go b/pkg/server/server.go index 4d939a804d06..2e230eacb4e2 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -1242,6 +1242,8 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) { } return errors.Newf("server found with type %T", d) }, + roachpb.SystemTenantID, + authorizer, ) recoveryServer := loqrecovery.NewServer( diff --git a/pkg/server/server_controller_new_server.go b/pkg/server/server_controller_new_server.go index e00c64cbae89..7c712a6cddfd 100644 --- a/pkg/server/server_controller_new_server.go +++ b/pkg/server/server_controller_new_server.go @@ -155,8 +155,9 @@ func (s *Server) makeSharedProcessTenantConfig( // Create a configuration for the new tenant. parentCfg := s.cfg localServerInfo := LocalKVServerInfo{ - InternalServer: s.node, - ServerInterceptors: s.grpc.serverInterceptorsInfo, + InternalServer: s.node, + ServerInterceptors: s.grpc.serverInterceptorsInfo, + SameProcessCapabilityAuthorizer: s.rpcContext.TenantRPCAuthorizer, } baseCfg, sqlCfg, err := makeSharedProcessTenantServerConfig(ctx, tenantID, index, parentCfg, localServerInfo, stopper, s.recorder) if err != nil { 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 97ba5a690b8a..d009095ad26e 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 f8a46711ebaa..1a3e23508612 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) } @@ -999,7 +999,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) } } @@ -1358,12 +1358,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) @@ -1387,7 +1387,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) } @@ -1429,7 +1429,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/tenant.go b/pkg/server/tenant.go index 2eff7dd1a8da..a42a1dca5a7e 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -411,6 +411,11 @@ func newTenantServer( sStatus.baseStatusServer.sqlServer = sqlServer sAdmin.sqlServer = sqlServer + var processCapAuthz tenantcapabilities.Authorizer = &tenantcapabilitiesauthorizer.AllowEverythingAuthorizer{} + if lsi := sqlCfg.LocalKVServerInfo; lsi != nil { + processCapAuthz = lsi.SameProcessCapabilityAuthorizer + } + // Create the debug API server. debugServer := debug.NewServer( baseCfg.AmbientCtx, @@ -418,6 +423,8 @@ func newTenantServer( sqlServer.pgServer.HBADebugFn(), sqlServer.execCfg.SQLStatusServer, nil, /* serverTickleFn */ + sqlCfg.TenantID, + processCapAuthz, ) return &SQLServerWrapper{ diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 80ec0922cafd..8c22328fbb75 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -921,6 +921,8 @@ func (ts *TestServer) StartSharedProcessTenant( hts := &httpTestServer{} hts.t.authentication = sqlServerWrapper.authentication hts.t.sqlServer = sqlServer + hts.t.tenantName = args.TenantName + testTenant := &TestTenant{ SQLServer: sqlServer, Cfg: sqlServer.cfg, 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..6cf38f984ef7 100644 --- a/pkg/testutils/serverutils/test_server_shim.go +++ b/pkg/testutils/serverutils/test_server_shim.go @@ -523,7 +523,8 @@ func GetJSONProtoWithAdminOption( if err != nil { return err } - fullURL := ts.AdminURL() + path + u := ts.AdminURL().String() + fullURL := u + path log.Infof(context.Background(), "test retrieving protobuf over HTTP: %s", fullURL) return httputil.GetJSON(httpClient, fullURL, response) } @@ -544,7 +545,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..d52dea126ac4 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,32 @@ 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} +} + +// WithPath is a helper that allows the user of the `TestURL` to easily +// append paths to use for testing. Please be aware that your path will +// automatically be escaped for you, but if it includes any invalid hex +// escapes (eg: `%s`) it will fail silently and you'll get back a blank +// URL. +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 +174,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 3c303d894dda..44af28ef8634 100644 --- a/pkg/testutils/testcluster/testcluster_test.go +++ b/pkg/testutils/testcluster/testcluster_test.go @@ -202,7 +202,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) } @@ -264,7 +264,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) }