diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-tenants b/pkg/ccl/backupccl/testdata/backup-restore/restore-tenants index 2624353591c2..3312d2b92fc5 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/restore-tenants +++ b/pkg/ccl/backupccl/testdata/backup-restore/restore-tenants @@ -21,9 +21,9 @@ ALTER TENANT [5] STOP SERVICE; DROP TENANT [5] query-sql SELECT id,name,data_state,service_mode,active,crdb_internal.pb_to_json('cockroach.multitenant.ProtoInfo', info, true) FROM system.tenants; ---- -1 system 1 2 true {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -5 2 0 false {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "DROP", "deprecatedId": "5", "droppedName": "tenant-5", "tenantReplicationJobId": "0"} -6 tenant-6 1 1 true {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "6", "droppedName": "", "tenantReplicationJobId": "0"} +1 system 1 2 true {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +5 2 0 false {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "DROP", "deprecatedId": "5", "droppedName": "tenant-5", "tenantReplicationJobId": "0"} +6 tenant-6 1 1 true {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "6", "droppedName": "", "tenantReplicationJobId": "0"} exec-sql BACKUP INTO 'nodelocal://1/cluster' @@ -59,8 +59,8 @@ job paused at pausepoint query-sql SELECT id,active,crdb_internal.pb_to_json('cockroach.multitenant.ProtoInfo', info, true) FROM system.tenants; ---- -1 true {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -6 false {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "ADD", "deprecatedId": "6", "droppedName": "", "tenantReplicationJobId": "0"} +1 true {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +6 false {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "ADD", "deprecatedId": "6", "droppedName": "", "tenantReplicationJobId": "0"} exec-sql SET CLUSTER SETTING jobs.debug.pausepoints = '' @@ -80,8 +80,8 @@ USE defaultdb; query-sql SELECT id,name,data_state,service_mode,active,crdb_internal.pb_to_json('cockroach.multitenant.ProtoInfo', info, true) FROM system.tenants; ---- -1 system 1 2 true {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -6 tenant-6 1 1 true {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "6", "droppedName": "", "tenantReplicationJobId": "0"} +1 system 1 2 true {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +6 tenant-6 1 1 true {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "6", "droppedName": "", "tenantReplicationJobId": "0"} exec-sql expect-error-regex=(tenant 6 already exists) RESTORE TENANT 6 FROM LATEST IN 'nodelocal://1/tenant6'; @@ -105,9 +105,9 @@ RESTORE TENANT 6 FROM LATEST IN 'nodelocal://1/tenant6' WITH tenant_name = 'newn query-sql SELECT id,name,data_state,service_mode,active,crdb_internal.pb_to_json('cockroach.multitenant.ProtoInfo', info, true) FROM system.tenants; ---- -1 system 1 2 true {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -2 newname 1 1 true {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} -6 tenant-6 1 1 true {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "6", "droppedName": "", "tenantReplicationJobId": "0"} +1 system 1 2 true {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +2 newname 1 1 true {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} +6 tenant-6 1 1 true {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "6", "droppedName": "", "tenantReplicationJobId": "0"} # Check that another service mode is also preserved. exec-sql diff --git a/pkg/ccl/kvccl/kvtenantccl/connector.go b/pkg/ccl/kvccl/kvtenantccl/connector.go index 5f2c53426fa4..d78f2931a743 100644 --- a/pkg/ccl/kvccl/kvtenantccl/connector.go +++ b/pkg/ccl/kvccl/kvtenantccl/connector.go @@ -464,12 +464,12 @@ func (c *Connector) RangeLookup( return nil, nil, errors.Wrap(ctx.Err(), "range lookup") } -// NodesUI implements the serverpb.TenantStatusServer interface -func (c *Connector) NodesUI( +// Nodes implements the serverpb.TenantStatusServer interface +func (c *Connector) Nodes( ctx context.Context, req *serverpb.NodesRequest, -) (resp *serverpb.NodesResponseExternal, retErr error) { +) (resp *serverpb.NodesResponse, retErr error) { retErr = c.withClient(ctx, func(ctx context.Context, client *client) (err error) { - resp, err = client.NodesUI(ctx, req) + resp, err = client.Nodes(ctx, req) return }) return diff --git a/pkg/ccl/logictestccl/testdata/logic_test/tenant_capability b/pkg/ccl/logictestccl/testdata/logic_test/tenant_capability index 7d34feb6ba62..10746a27f916 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/tenant_capability +++ b/pkg/ccl/logictestccl/testdata/logic_test/tenant_capability @@ -24,6 +24,8 @@ SHOW TENANT 'five' WITH CAPABILITIES id name data_state service_mode capability_name capability_value 5 five ready none can_admin_split false 5 five ready none can_admin_unsplit false +5 five ready none can_view_node_info false +5 five ready none can_view_tsdb_metrics false statement ok ALTER TENANT [5] GRANT CAPABILITY can_admin_split=true @@ -38,6 +40,8 @@ SHOW TENANT 'five' WITH CAPABILITIES id name data_state service_mode capability_name capability_value 5 five ready none can_admin_split true 5 five ready none can_admin_unsplit false +5 five ready none can_view_node_info false +5 five ready none can_view_tsdb_metrics false statement ok ALTER TENANT [5] REVOKE CAPABILITY can_admin_split @@ -48,3 +52,5 @@ SHOW TENANT 'five' WITH CAPABILITIES id name data_state service_mode capability_name capability_value 5 five ready none can_admin_split false 5 five ready none can_admin_unsplit false +5 five ready none can_view_node_info false +5 five ready none can_view_tsdb_metrics false diff --git a/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_split b/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_split index 79a66b942973..b95186c692a3 100644 --- a/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_split +++ b/pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_admin_split @@ -3,6 +3,8 @@ SHOW TENANT [10] WITH CAPABILITIES ---- 10 tenant-10 ready none can_admin_split false 10 tenant-10 ready none can_admin_unsplit false +10 tenant-10 ready none can_view_node_info false +10 tenant-10 ready none can_view_tsdb_metrics false exec-sql-tenant CREATE TABLE t(a INT) diff --git a/pkg/ccl/serverccl/adminccl/BUILD.bazel b/pkg/ccl/serverccl/adminccl/BUILD.bazel index be633acba9bd..5e47062219da 100644 --- a/pkg/ccl/serverccl/adminccl/BUILD.bazel +++ b/pkg/ccl/serverccl/adminccl/BUILD.bazel @@ -17,12 +17,15 @@ go_test( "//pkg/server/serverpb", "//pkg/spanconfig", "//pkg/sql/tests", + "//pkg/testutils", "//pkg/testutils/serverutils", "//pkg/testutils/skip", "//pkg/testutils/testcluster", + "//pkg/ts/tspb", "//pkg/util/leaktest", "//pkg/util/log", "//pkg/util/randutil", + "//pkg/util/timeutil", "@com_github_stretchr_testify//require", ], ) diff --git a/pkg/ccl/serverccl/adminccl/tenant_admin_test.go b/pkg/ccl/serverccl/adminccl/tenant_admin_test.go index 22e33a6d946b..32c419c09242 100644 --- a/pkg/ccl/serverccl/adminccl/tenant_admin_test.go +++ b/pkg/ccl/serverccl/adminccl/tenant_admin_test.go @@ -10,6 +10,7 @@ package adminccl import ( "context" + "errors" "fmt" "testing" @@ -17,9 +18,12 @@ import ( "github.com/cockroachdb/cockroach/pkg/server/serverpb" "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/sql/tests" + "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" + "github.com/cockroachdb/cockroach/pkg/ts/tspb" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/stretchr/testify/require" ) @@ -52,6 +56,46 @@ func TestTenantAdminAPI(t *testing.T) { t.Run("tenant_metricmetadata", func(t *testing.T) { testMetricMetadataRPC(ctx, t, testHelper) }) + + t.Run("tenant_metrics_capability", func(t *testing.T) { + testTenantMetricsCapabilityRPC(ctx, t, testHelper) + }) +} + +func testTenantMetricsCapabilityRPC( + ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper, +) { + http := helper.TestCluster().TenantAdminHTTPClient(t, 1) + defer http.Close() + + query := tspb.TimeSeriesQueryRequest{ + StartNanos: 0, + EndNanos: timeutil.Now().UnixNano(), + Queries: []tspb.Query{ + { + Name: "cr.node.sql.select.count", + }, + }, + SampleNanos: 0, + } + queryResp := tspb.TimeSeriesQueryResponse{} + err := http.PostJSONChecked("/ts/query", &query, &queryResp) + require.Error(t, err) + + db := helper.HostCluster().ServerConn(0) + _, err = db.Exec("ALTER TENANT [10] GRANT CAPABILITY can_view_tsdb_metrics=true\n") + require.NoError(t, err) + + testutils.SucceedsSoon(t, func() error { + err := http.PostJSONChecked("/ts/query", &query, &queryResp) + if err != nil { + return err + } + if len(queryResp.Results) == 0 { + return errors.New("missing metrics data") + } + return nil + }) } func testMetricMetadataRPC(ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper) { diff --git a/pkg/ccl/serverccl/statusccl/tenant_status_test.go b/pkg/ccl/serverccl/statusccl/tenant_status_test.go index 8816133e98cd..78981ef2512b 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_status_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_status_test.go @@ -83,6 +83,12 @@ func TestTenantStatusAPI(t *testing.T) { defer testHelper.Cleanup(ctx, t) + // Speed up propagation of tenant capability changes. + db := testHelper.HostCluster().ServerConn(0) + tdb := sqlutils.MakeSQLRunner(db) + tdb.Exec(t, "SET CLUSTER SETTING kv.closed_timestamp.target_duration = '10ms'") + tdb.Exec(t, "SET CLUSTER SETTING kv.closed_timestamp.side_transport_interval = '10 ms'") + t.Run("reset_sql_stats", func(t *testing.T) { testResetSQLStatsRPCForTenant(ctx, t, testHelper) }) @@ -146,6 +152,10 @@ func TestTenantStatusAPI(t *testing.T) { t.Run("tenant_span_stats", func(t *testing.T) { testTenantSpanStats(ctx, t, testHelper) }) + + t.Run("tenant_nodes_capability", func(t *testing.T) { + testTenantNodesCapability(ctx, t, testHelper) + }) } func testTenantSpanStats(ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper) { @@ -211,6 +221,31 @@ func testTenantSpanStats(ctx context.Context, t *testing.T, helper serverccl.Ten require.Equal(t, controlStats.RangeCount+1, stats.RangeCount) require.Equal(t, controlStats.TotalStats.LiveCount+int64(len(incKeys)), stats.TotalStats.LiveCount) }) + +} + +func testTenantNodesCapability( + ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper, +) { + tenant := helper.TestCluster().TenantStatusSrv(0) + + _, err := tenant.NodesUI(ctx, &serverpb.NodesRequest{}) + require.Error(t, err) + + db := helper.HostCluster().ServerConn(0) + _, err = db.Exec("ALTER TENANT [10] GRANT CAPABILITY can_view_node_info=true\n") + require.NoError(t, err) + + testutils.SucceedsSoon(t, func() error { + resp, err := tenant.NodesUI(ctx, &serverpb.NodesRequest{}) + if err != nil { + return err + } + if len(resp.Nodes) == 0 || len(resp.LivenessByNodeID) == 0 { + return errors.New("missing nodes or liveness data") + } + return nil + }) } func testTenantLogs(ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper) { diff --git a/pkg/multitenant/tenantcapabilities/capabilities.go b/pkg/multitenant/tenantcapabilities/capabilities.go index 3722209bf032..cbc7b4e5d421 100644 --- a/pkg/multitenant/tenantcapabilities/capabilities.go +++ b/pkg/multitenant/tenantcapabilities/capabilities.go @@ -12,6 +12,7 @@ package tenantcapabilities import ( "context" + "fmt" "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb" "github.com/cockroachdb/cockroach/pkg/roachpb" @@ -59,6 +60,14 @@ type Authorizer interface { // implementation. Binding the Reader late allows us to break this dependency // cycle. BindReader(reader Reader) + + // HasNodeStatusCapability returns an error if a tenant, referenced by its ID, + // is not allowed to access cluster-level node metadata and liveness. + HasNodeStatusCapability(ctx context.Context, tenID roachpb.TenantID) error + + // HasTSDBQueryCapability returns an error if a tenant, referenced by its ID, + // is not allowed to query the TSDB for metrics. + HasTSDBQueryCapability(ctx context.Context, tenID roachpb.TenantID) error } // Entry ties together a tenantID with its capabilities. @@ -72,3 +81,14 @@ type Update struct { Entry Deleted bool // whether the entry was deleted or not } + +func (u Update) String() string { + if u.Deleted { + return fmt.Sprintf("delete: %+v", u.Entry) + } + return fmt.Sprintf("update: %+v", u.Entry) +} + +func (u Entry) String() string { + return fmt.Sprintf("ten=%s cap=%+v", u.TenantID, u.TenantCapabilities) +} diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go index 90a8c89bbaa7..02453f680080 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go @@ -85,3 +85,39 @@ func (a *Authorizer) HasCapabilityForBatch( func (a *Authorizer) BindReader(reader tenantcapabilities.Reader) { a.capabilitiesReader = reader } + +func (a *Authorizer) HasNodeStatusCapability(ctx context.Context, tenID roachpb.TenantID) error { + if tenID.IsSystem() { + return nil // the system tenant is allowed to do as it pleases + } + cp, found := a.capabilitiesReader.GetCapabilities(tenID) + if !found { + log.Infof( + ctx, + "no capability information for tenant %s; requests that require capabilities may be denied", + tenID, + ) + } + if !cp.CanViewNodeInfo { + return errors.Newf("tenant %s does not have capability to query cluster node metadata", tenID) + } + return nil +} + +func (a *Authorizer) HasTSDBQueryCapability(ctx context.Context, tenID roachpb.TenantID) error { + if tenID.IsSystem() { + return nil // the system tenant is allowed to do as it pleases + } + cp, found := a.capabilitiesReader.GetCapabilities(tenID) + if !found { + log.Infof( + ctx, + "no capability information for tenant %s; requests that require capabilities may be denied", + tenID, + ) + } + if !cp.CanViewTsdbMetrics { + return errors.Newf("tenant %s does not have capability to query timeseries data", tenID) + } + return nil +} diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer_test.go b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer_test.go index 2059ad323e6d..61c947fe41cf 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer_test.go +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer_test.go @@ -31,17 +31,18 @@ import ( // "update-state": updates the underlying global tenant capability state. // Example: // -// update-state -// upsert {ten=10}:{CanAdminSplit=true} -// delete {ten=15} -// ---- +// upsert ten=10 can_admin_split=true +// ---- +// ok +// +// delete ten=15 +// ---- +// ok // // "has-capability-for-batch": performs a capability check, given a tenant and // batch request declaration. Example: // -// has-capability-for-batch -// {ten=10} -// split +// has-capability-for-batch ten=10 cmds=(split) // ---- func TestDataDriven(t *testing.T) { defer leaktest.AfterTest(t)() @@ -52,19 +53,36 @@ func TestDataDriven(t *testing.T) { authorizer.BindReader(mockReader) datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string { + tenID := tenantcapabilitiestestutils.GetTenantID(t, d) switch d.Cmd { - case "update-state": - updates := tenantcapabilitiestestutils.ParseTenantCapabilityUpdateStateArguments(t, d.Input) - mockReader.updateState(updates) - + case "upsert": + update, err := tenantcapabilitiestestutils.ParseTenantCapabilityUpsert(t, d) + if err != nil { + return err.Error() + } + mockReader.updateState([]*tenantcapabilities.Update{update}) + case "delete": + update := tenantcapabilitiestestutils.ParseTenantCapabilityDelete(t, d) + mockReader.updateState([]*tenantcapabilities.Update{update}) case "has-capability-for-batch": - tenID, ba := tenantcapabilitiestestutils.ParseBatchRequestString(t, d.Input) + ba := tenantcapabilitiestestutils.ParseBatchRequests(t, d) err := authorizer.HasCapabilityForBatch(context.Background(), tenID, &ba) if err == nil { return "ok" } return err.Error() - + case "has-node-status-capability": + err := authorizer.HasNodeStatusCapability(context.Background(), tenID) + if err == nil { + return "ok" + } + return err.Error() + case "has-tsdb-query-capability": + err := authorizer.HasTSDBQueryCapability(context.Background(), tenID) + if err == nil { + return "ok" + } + return err.Error() default: return fmt.Sprintf("unknown command %s", d.Cmd) } @@ -75,7 +93,7 @@ func TestDataDriven(t *testing.T) { type mockReader map[roachpb.TenantID]tenantcapabilitiespb.TenantCapabilities -func (m mockReader) updateState(updates []tenantcapabilities.Update) { +func (m mockReader) updateState(updates []*tenantcapabilities.Update) { for _, update := range updates { if update.Deleted { delete(m, update.TenantID) diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/noop.go b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/noop.go index 0957aaf1528a..7b1f7844462f 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/noop.go +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/noop.go @@ -37,3 +37,15 @@ func (n *NoopAuthorizer) HasCapabilityForBatch( // BindReader implements the tenantcapabilities.Authorizer interface. func (n *NoopAuthorizer) BindReader(tenantcapabilities.Reader) { } + +// HasNodeStatusCapability implements the tenantcapabilities.Authorizer interface +func (n *NoopAuthorizer) HasNodeStatusCapability( + ctx context.Context, tenID roachpb.TenantID, +) error { + return nil +} + +// HasTSDBQueryCapability implements the tenantcapabilities.Authorizer interface +func (n *NoopAuthorizer) HasTSDBQueryCapability(ctx context.Context, tenID roachpb.TenantID) error { + return nil +} diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/basic b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/basic index a92d061409bc..9a6306f28d5c 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/basic +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/basic @@ -1,72 +1,66 @@ -update-state -upsert {ten=10}:{CanAdminSplit=true} -upsert {ten=11}:{CanAdminSplit=false} +upsert ten=10 can_admin_split=true can_view_node_info=true can_view_tsdb_metrics=true +---- +ok + +upsert ten=11 can_admin_split=false can_view_node_info=false can_view_tsdb_metrics=false ---- ok # Tenant 10 should be able to issue splits, given it has the capability to do # so. -has-capability-for-batch -{ten=10} -split -scan -cput +has-capability-for-batch ten=10 cmds=(split, scan, cput) ---- ok # Tenant 11 shouldn't be able to issue splits. -has-capability-for-batch -{ten=11} -split -scan -cput +has-capability-for-batch ten=11 cmds=(split, scan, cput) ---- tenant 11 does not have admin split capability # Test that the order of the split request doesn't have any effect. -has-capability-for-batch -{ten=11} -scan -cput +has-capability-for-batch ten=11 cmds=(scan, cput) ---- ok # However, a batch request which doesn't include a split (by tenant 11) should # work as you'd expect. -has-capability-for-batch -{ten=11} -scan -cput +has-capability-for-batch ten=11 cmds=(scan, cput) ---- ok # Ditto for tenant 10. -has-capability-for-batch -{ten=10} -scan -cput +has-capability-for-batch ten=10 cmds=(scan, cput) ---- ok # Lastly, flip tenant 10's capability for splits; ensure it can no longer issue # splits as a result. -update-state -upsert {ten=10}:{CanAdminSplit=false} +upsert ten=10 can_admin_split=false can_view_node_info=true can_view_tsdb_metrics=true ---- ok -has-capability-for-batch -{ten=10} -split -scan -cput +has-capability-for-batch ten=10 cmds=(split, scan, cput) ---- tenant 10 does not have admin split capability # However, this has no effect on batch requests that don't contain splits. -has-capability-for-batch -{ten=10} -scan -cput +has-capability-for-batch ten=10 cmds=(scan, cput) +---- +ok + +has-node-status-capability ten=10 +---- +ok + +has-node-status-capability ten=11 +---- +tenant 11 does not have capability to query cluster node metadata + + +has-tsdb-query-capability ten=10 ---- ok + +has-tsdb-query-capability ten=11 +---- +tenant 11 does not have capability to query timeseries data diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/no_capabilities b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/no_capabilities index fe775b70b10a..42dc329d91f4 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/no_capabilities +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/no_capabilities @@ -1,47 +1,30 @@ # Ensure if no capability exists for a tenant (or if an entry existed, but is # deleted), split requests can't be issued. -has-capability-for-batch -{ten=10} -split -scan -cput +has-capability-for-batch ten=10 cmds=(split, scan, cput) ---- tenant 10 does not have admin split capability # However, if there was no split in the batch, the batch should be allowed to # go through. -has-capability-for-batch -{ten=10} -scan -cput +has-capability-for-batch ten=10 cmds=(scan, cput) ---- ok # Update the capability state to give tenant 10 the capability to run splits. -update-state -upsert {ten=10}:{CanAdminSplit=true} +upsert ten=10 can_admin_split=true ---- ok -has-capability-for-batch -{ten=10} -split -scan -cput +has-capability-for-batch ten=10 cmds=(split, scan, cput) ---- ok # Remove the capability. -update-state -delete {ten=10} +delete ten=10 ---- ok -has-capability-for-batch -{ten=10} -split -scan -cput +has-capability-for-batch ten=10 cmds=(split, scan, cput) ---- tenant 10 does not have admin split capability diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/system_tenant b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/system_tenant index 1a93d81c0241..e45812612a6a 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/system_tenant +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/testdata/system_tenant @@ -1,28 +1,18 @@ # Ensure the system tenant can issue any request, regardless of capabilities. -has-capability-for-batch -{ten=system} -split -scan -cput +has-capability-for-batch ten=system cmds=(split, scan, cput) ---- ok -has-capability-for-batch -{ten=system} -scan -cput +has-capability-for-batch ten=system cmds=(scan, cput) ---- ok -has-capability-for-batch -{ten=system} -split +has-capability-for-batch ten=system cmds=(split) ---- ok -update-state -upsert {ten=system}:{CanAdminSplit=false} +upsert ten=system can_admin_split=false ---- ok @@ -31,8 +21,6 @@ ok # things properly. For now, we test this case explicitly to ensure nothing # breaks. -has-capability-for-batch -{ten=system} -split +has-capability-for-batch ten=system cmds=(split) ---- ok diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto b/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto index 592891c44c41..fd326cf27b2e 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto @@ -22,10 +22,27 @@ import "gogoproto/gogo.proto"; // [1] Certain requests in the system are considered "privileged", and as such, // tenants are only allowed to perform them if they have the appropriate // capability. For example, performing an AdminSplit. +// +// When adding new capabilities to this list, please make sure to add support +// for the new values to `show_tenant.go` and `tenant_capability.go` in order to +// integrate them with the SQL infrastructure for managing capabilities. message TenantCapabilities { option (gogoproto.equal) = true; - // CanAdminSplit, if set to true, grants grants the tenant the ability to + // CanAdminSplit, if set to true, grants the tenant the ability to // successfully perform `AdminSplit` requests. bool can_admin_split = 1; + + // CanViewNodeObservability, if set to true, grants the tenant the ability + // retrieve node-level observability data at endpoints such as `_status/nodes` + // and in the DB Console overview page. + bool can_view_node_info = 2; + + // CanViewTSDBMetrics, if set to true, grants the tenant the ability to + // make arbitrary queries of the TSDB of the entire cluster. Currently, + // we do not store per-tenant metrics so this will surface system metrics + // to the tenant. + // TODO(davidh): Revise this once tenant-scoped metrics are implemented in + // https://github.com/cockroachdb/cockroach/issues/96438 + bool can_view_tsdb_metrics = 3; }; diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils/BUILD.bazel b/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils/BUILD.bazel index 611d9eb1ef5a..73f61b9e374a 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils/BUILD.bazel +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils/BUILD.bazel @@ -10,6 +10,7 @@ go_library( "//pkg/multitenant/tenantcapabilities", "//pkg/multitenant/tenantcapabilities/tenantcapabilitiespb", "//pkg/roachpb", + "@com_github_cockroachdb_datadriven//:datadriven", "@com_github_stretchr_testify//require", ], ) diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils/testutils.go b/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils/testutils.go index ed6fc1da89c2..578b67ef1435 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils/testutils.go +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils/testutils.go @@ -11,157 +11,99 @@ package tenantcapabilitiestestutils import ( - "fmt" - "regexp" "strconv" - "strings" "testing" "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities" "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/datadriven" "github.com/stretchr/testify/require" ) -var tenIDRe = regexp.MustCompile(`^{ten=((\d*)|(system))}$`) -var capabilityRe = regexp.MustCompile(`^{(CanAdminSplit=(true|false))}$`) - // ParseBatchRequestString is a helper function to parse datadriven input that // declares (empty) batch requests of supported types, for a particular tenant. -// Both the constructed batch request and the requesting tenant ID are returned. -// The input is of the following form: +// The constructed batch request is returned. The cmds are of the following +// form: // -// {ten=10} -// split -// scan -// cput -func ParseBatchRequestString( - t *testing.T, input string, -) (tenID roachpb.TenantID, ba roachpb.BatchRequest) { - for i, line := range strings.Split(input, "\n") { - if i == 0 { // first line describes the tenant ID. - tenID = ParseTenantID(t, line) - continue - } - switch line { - case "split": - ba.Add(&roachpb.AdminSplitRequest{}) - case "scan": - ba.Add(&roachpb.ScanRequest{}) - case "cput": - ba.Add(&roachpb.ConditionalPutRequest{}) - default: - t.Fatalf("unsupported request type: %s", line) +// cmds=(split, scan, cput) +func ParseBatchRequests(t *testing.T, d *datadriven.TestData) (ba roachpb.BatchRequest) { + for _, cmd := range d.CmdArgs { + if cmd.Key == "cmds" { + for _, z := range cmd.Vals { + switch z { + case "split": + ba.Add(&roachpb.AdminSplitRequest{}) + case "scan": + ba.Add(&roachpb.ScanRequest{}) + case "cput": + ba.Add(&roachpb.ConditionalPutRequest{}) + default: + t.Fatalf("unsupported request type: %s", z) + } + } } } - return tenID, ba + return ba } -// ParseTenantCapabilityUpdateStateArguments is a helper function to parse -// datadriven input to update global tenant capability state. The input is of -// the following form: -// -// upsert {ten=10}:{CanAdminSplit=true} -// delete {ten=20} -func ParseTenantCapabilityUpdateStateArguments( - t *testing.T, input string, -) (updates []tenantcapabilities.Update) { - for _, line := range strings.Split(input, "\n") { - const upsertPrefix, deletePrefix = "upsert ", "delete " - switch { - case strings.HasPrefix(line, deletePrefix): - line = strings.TrimPrefix(line, line[:len(deletePrefix)]) - updates = append(updates, tenantcapabilities.Update{ - Entry: tenantcapabilities.Entry{ - TenantID: ParseTenantID(t, line), - }, - Deleted: true, - }) - case strings.HasPrefix(line, upsertPrefix): - line = strings.TrimPrefix(line, line[:len(upsertPrefix)]) - updates = append(updates, tenantcapabilities.Update{ - Entry: parseTenantCapabilityEntry(t, line), - Deleted: false, - }) - default: - t.Fatalf("malformed line %q, expected to find prefix %q or %q", - line, upsertPrefix, deletePrefix) +func ParseTenantCapabilityUpsert( + t *testing.T, d *datadriven.TestData, +) (*tenantcapabilities.Update, error) { + tID := GetTenantID(t, d) + cap := tenantcapabilitiespb.TenantCapabilities{} + for _, arg := range d.CmdArgs { + if arg.Key == "can_admin_split" { + b, err := strconv.ParseBool(arg.Vals[0]) + if err != nil { + return nil, err + } + cap.CanAdminSplit = b + } + if arg.Key == "can_view_node_info" { + b, err := strconv.ParseBool(arg.Vals[0]) + if err != nil { + return nil, err + } + cap.CanViewNodeInfo = b + } + if arg.Key == "can_view_tsdb_metrics" { + b, err := strconv.ParseBool(arg.Vals[0]) + if err != nil { + return nil, err + } + cap.CanViewTsdbMetrics = b } } - return updates -} - -// PrintTenantCapabilityUpdate is a helper function that prints out a -// tenantcapabilities.Update, allowing data-driven tests to assert on the -// output. -func PrintTenantCapabilityUpdate(update tenantcapabilities.Update) string { - if update.Deleted { - return fmt.Sprintf("deleted %s", printTenantID(update.TenantID)) + update := tenantcapabilities.Update{ + Entry: tenantcapabilities.Entry{ + TenantID: tID, + TenantCapabilities: cap, + }, } - - return fmt.Sprintf("updated %s", PrintTenantCapabilityEntry(update.Entry)) + return &update, nil } -// PrintTenantCapabilityEntry is a helper function that prints out a -// tenantcapabilities.Entry, allowing data-driven tests to assert on the -// output. -func PrintTenantCapabilityEntry(entry tenantcapabilities.Entry) string { - return fmt.Sprintf( - "%s:%s", - printTenantID(entry.TenantID), - PrintTenantCapability(entry.TenantCapabilities), - ) -} - -func parseTenantCapabilityEntry(t *testing.T, input string) tenantcapabilities.Entry { - parts := strings.Split(input, ":") - require.Equal(t, 2, len(parts)) - return tenantcapabilities.Entry{ - TenantID: ParseTenantID(t, parts[0]), - TenantCapabilities: parseTenantCapability(t, parts[1]), +func ParseTenantCapabilityDelete(t *testing.T, d *datadriven.TestData) *tenantcapabilities.Update { + tID := GetTenantID(t, d) + update := tenantcapabilities.Update{ + Entry: tenantcapabilities.Entry{ + TenantID: tID, + }, + Deleted: true, } + return &update } -func parseTenantCapability(t *testing.T, input string) tenantcapabilitiespb.TenantCapabilities { - if !capabilityRe.MatchString(input) { - t.Fatalf("expected %s to match capability ID regex", input) - } - matches := capabilityRe.FindStringSubmatch(input) - var capabilities tenantcapabilitiespb.TenantCapabilities - if matches[2] == "true" { - capabilities.CanAdminSplit = true +func GetTenantID(t *testing.T, d *datadriven.TestData) roachpb.TenantID { + var tenantID string + if d.HasArg("ten") { + d.ScanArgs(t, "ten", &tenantID) } - return capabilities -} - -func ParseTenantID(t *testing.T, input string) roachpb.TenantID { - if !tenIDRe.MatchString(input) { - t.Fatalf("expected %s to match tenant ID regex", input) - } - matches := tenIDRe.FindStringSubmatch(input) - - if matches[3] == "system" { + if roachpb.IsSystemTenantName(roachpb.TenantName(tenantID)) { return roachpb.SystemTenantID } - - tenID, err := strconv.Atoi(matches[2]) + tID, err := roachpb.TenantIDFromString(tenantID) require.NoError(t, err) - return roachpb.MustMakeTenantID(uint64(tenID)) -} - -func printTenantID(id roachpb.TenantID) string { - return fmt.Sprintf("{ten=%s}", id) -} - -func PrintTenantCapability(cap tenantcapabilitiespb.TenantCapabilities) string { - var s strings.Builder - s.WriteString("{") - s.WriteString("CanAdminSplit=") - if cap.CanAdminSplit { - s.WriteString("true") - } else { - s.WriteString("false") - } - s.WriteString("}") - return s.String() + return tID } diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/basic b/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/basic index 51f209f2b760..f24d41df86f6 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/basic +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/basic @@ -7,57 +7,53 @@ ok updates ---- -update-state -upsert {ten=10}:{CanAdminSplit=true} -upsert {ten=11}:{CanAdminSplit=false} +upsert ten=10 can_admin_split=true +---- +ok + +upsert ten=11 can_admin_split=false ---- ok updates ---- Incremental Update -updated {ten=10}:{CanAdminSplit=true} -updated {ten=11}:{CanAdminSplit=false} +update: ten=10 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} +update: ten=11 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} flush-state ---- -{ten=10}:{CanAdminSplit=true} -{ten=11}:{CanAdminSplit=false} +ten=10 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} +ten=11 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} -# Perform incremental updates. -update-state -upsert {ten=11}:{CanAdminSplit=true} +upsert ten=11 can_admin_split=true ---- ok updates ---- Incremental Update -updated {ten=11}:{CanAdminSplit=true} +update: ten=11 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=11} +get-capabilities ten=11 ---- -{CanAdminSplit=true} +{CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} -update-state -delete {ten=10} +delete ten=10 ---- ok updates ---- Incremental Update -deleted {ten=10} +delete: ten=10 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=10} +get-capabilities ten=10 ---- not-found # No-op update. -update-state -delete {ten=15} +delete ten=15 ---- ok @@ -68,59 +64,66 @@ updates # what we'd expect. flush-state ---- -{ten=11}:{CanAdminSplit=true} +ten=11 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} -update-state -upsert {ten=15}:{CanAdminSplit=true} +upsert ten=15 can_admin_split=true ---- ok updates ---- Incremental Update -updated {ten=15}:{CanAdminSplit=true} +update: ten=15 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} # Ensure only the last update is applied, even when there are multiple updates # to a single key. -update-state -upsert {ten=11}:{CanAdminSplit=false} -upsert {ten=11}:{CanAdminSplit=true} -delete {ten=11} +upsert ten=11 can_admin_split=false +---- +ok + +upsert ten=11 can_admin_split=true +---- +ok + +delete ten=11 ---- ok updates ---- Incremental Update -deleted {ten=11} +delete: ten=11 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=11} +get-capabilities ten=11 ---- not-found flush-state ---- -{ten=15}:{CanAdminSplit=true} +ten=15 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} # Same thing, but this time instead of deleting the key, leave it behind. -update-state -delete {ten=15} -upsert {ten=15}:{CanAdminSplit=true} -upsert {ten=15}:{CanAdminSplit=false} +delete ten=15 +---- +ok + +upsert ten=15 can_admin_split=true +---- +ok + +upsert ten=15 can_admin_split=false ---- ok updates ---- Incremental Update -updated {ten=15}:{CanAdminSplit=false} +update: ten=15 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} flush-state ---- -{ten=15}:{CanAdminSplit=false} +ten=15 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=15} +get-capabilities ten=15 ---- -{CanAdminSplit=false} +{CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/initial_scan b/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/initial_scan index af140385f1e8..842ef1788f07 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/initial_scan +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/initial_scan @@ -2,27 +2,32 @@ # We also ensure that initial scans see the most recent state when they're # started. -update-state -upsert {ten=10}:{CanAdminSplit=true} -upsert {ten=11}:{CanAdminSplit=false} -upsert {ten=15}:{CanAdminSplit=false} +upsert ten=10 can_admin_split=true ---- ok -update-state -delete {ten=10} -upsert {ten=15}:{CanAdminSplit=true} +upsert ten=11 can_admin_split=false +---- +ok + +upsert ten=15 can_admin_split=false +---- +ok + +delete ten=10 +---- +ok + +upsert ten=15 can_admin_split=true ---- ok # Try reading capabilities before the Watcher is started. -get-capabilities -{ten=15} +get-capabilities ten=15 ---- not-found -get-capabilities -{ten=10} +get-capabilities ten=10 ---- not-found @@ -33,20 +38,18 @@ ok updates ---- Complete Update -updated {ten=11}:{CanAdminSplit=false} -updated {ten=15}:{CanAdminSplit=true} +update: ten=11 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} +update: ten=15 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} flush-state ---- -{ten=11}:{CanAdminSplit=false} -{ten=15}:{CanAdminSplit=true} +ten=11 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} +ten=15 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=10} +get-capabilities ten=10 ---- not-found -get-capabilities -{ten=15} +get-capabilities ten=15 ---- -{CanAdminSplit=true} +{CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/rangefeed_errors b/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/rangefeed_errors index 6360b19cda43..d3fbf85caae8 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/rangefeed_errors +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/testdata/rangefeed_errors @@ -10,19 +10,24 @@ ok updates ---- -update-state -upsert {ten=10}:{CanAdminSplit=true} -upsert {ten=11}:{CanAdminSplit=false} -upsert {ten=12}:{CanAdminSplit=false} +upsert ten=10 can_admin_split=true +---- +ok + +upsert ten=11 can_admin_split=false +---- +ok + +upsert ten=12 can_admin_split=false ---- ok updates ---- Incremental Update -updated {ten=10}:{CanAdminSplit=true} -updated {ten=11}:{CanAdminSplit=false} -updated {ten=12}:{CanAdminSplit=false} +update: ten=10 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} +update: ten=11 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} +update: ten=12 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} inject-error ---- @@ -31,10 +36,15 @@ big-yikes # Update some more state. However, because of the injected error, we shouldn't # observe any updates. -update-state -upsert {ten=12}:{CanAdminSplit=true} -delete {ten=10} -upsert {ten=50}:{CanAdminSplit=false} +upsert ten=12 can_admin_split=true +---- +ok + +delete ten=10 +---- +ok + +upsert ten=50 can_admin_split=false ---- ok @@ -46,24 +56,21 @@ updates flush-state ---- -{ten=10}:{CanAdminSplit=true} -{ten=11}:{CanAdminSplit=false} -{ten=12}:{CanAdminSplit=false} +ten=10 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} +ten=11 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} +ten=12 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=50} +get-capabilities ten=50 ---- not-found -get-capabilities -{ten=12} +get-capabilities ten=12 ---- -{CanAdminSplit=false} +{CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=10} +get-capabilities ten=10 ---- -{CanAdminSplit=true} +{CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} # Let the Watcher attempt to restart. restart-after-injected-error @@ -75,27 +82,24 @@ ok updates ---- Complete Update -updated {ten=11}:{CanAdminSplit=false} -updated {ten=12}:{CanAdminSplit=true} -updated {ten=50}:{CanAdminSplit=false} +update: ten=11 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} +update: ten=12 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} +update: ten=50 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} flush-state ---- -{ten=11}:{CanAdminSplit=false} -{ten=12}:{CanAdminSplit=true} -{ten=50}:{CanAdminSplit=false} +ten=11 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} +ten=12 cap={CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} +ten=50 cap={CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=50} +get-capabilities ten=50 ---- -{CanAdminSplit=false} +{CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=12} +get-capabilities ten=12 ---- -{CanAdminSplit=true} +{CanAdminSplit:true CanViewNodeInfo:false CanViewTsdbMetrics:false} -get-capabilities -{ten=10} +get-capabilities ten=10 ---- not-found diff --git a/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/watcher_test.go b/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/watcher_test.go index 443223184770..4df3e3ecfb13 100644 --- a/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/watcher_test.go +++ b/pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/watcher_test.go @@ -42,13 +42,16 @@ import ( // // "start": starts the Watcher. // -// "update-state": updates the underlying global tenant capability state. +// "upsert" and "delete": updates the underlying global tenant capability state. // Example: // -// update-state -// upsert {ten=10}:{CanAdminSplit=true} -// delete {ten=15} -// ---- +// upsert ten=10 can_admin_split=true +// ---- +// ok +// +// delete ten=15 +// ---- +// ok // // "updates": lists out updates observed by the watcher after the underlying // tenant capability state has been updated. @@ -193,53 +196,48 @@ func TestDataDriven(t *testing.T) { if i+1 != len(receivedUpdates) && receivedUpdates[i+1].TenantID.Equal(receivedUpdates[i].TenantID) { continue // de-duplicate } - output.WriteString( - fmt.Sprintf( - "%s\n", tenantcapabilitiestestutils.PrintTenantCapabilityUpdate(receivedUpdates[i]), - ), - ) + output.WriteString(fmt.Sprintf("%+v\n", receivedUpdates[i])) } return output.String() - case "update-state": - updates := tenantcapabilitiestestutils.ParseTenantCapabilityUpdateStateArguments(t, d.Input) - for _, update := range updates { - if !update.Deleted { - info := mtinfopb.ProtoInfo{ - Capabilities: update.TenantCapabilities, - } - buf, err := protoutil.Marshal(&info) - require.NoError(t, err) - tdb.Exec( - t, - fmt.Sprintf("UPSERT INTO %s (id, active, info) VALUES ($1, $2, $3)", dummyTableName), - update.TenantID.ToUint64(), - true, /* active */ - buf, - ) - } else { - tdb.Exec( - t, - fmt.Sprintf("DELETE FROM %s WHERE id = $1", dummyTableName), - update.TenantID.ToUint64(), - ) - } + case "upsert": + update, err := tenantcapabilitiestestutils.ParseTenantCapabilityUpsert(t, d) + require.NoError(t, err) + info := mtinfopb.ProtoInfo{ + Capabilities: update.TenantCapabilities, } + buf, err := protoutil.Marshal(&info) + require.NoError(t, err) + tdb.Exec( + t, + fmt.Sprintf("UPSERT INTO %s (id, active, info) VALUES ($1, $2, $3)", dummyTableName), + update.TenantID.ToUint64(), + true, /* active */ + buf, + ) + lastUpdateTS = ts.Clock().Now() + case "delete": + delete := tenantcapabilitiestestutils.ParseTenantCapabilityDelete(t, d) + tdb.Exec( + t, + fmt.Sprintf("DELETE FROM %s WHERE id = $1", dummyTableName), + delete.TenantID.ToUint64(), + ) lastUpdateTS = ts.Clock().Now() case "get-capabilities": - tenID := tenantcapabilitiestestutils.ParseTenantID(t, d.Input) - cp, found := watcher.GetCapabilities(tenID) + tID := tenantcapabilitiestestutils.GetTenantID(t, d) + cp, found := watcher.GetCapabilities(tID) if !found { return "not-found" } - return tenantcapabilitiestestutils.PrintTenantCapability(cp) + return fmt.Sprintf("%+v", cp) case "flush-state": var output strings.Builder entries := watcher.TestingFlushCapabilitiesState() for _, entry := range entries { - output.WriteString(fmt.Sprintf("%s\n", tenantcapabilitiestestutils.PrintTenantCapabilityEntry(entry))) + output.WriteString(fmt.Sprintf("%+v\n", entry)) } return output.String() case "inject-error": diff --git a/pkg/rpc/auth_tenant.go b/pkg/rpc/auth_tenant.go index 9219aa3664ad..0679366e2bda 100644 --- a/pkg/rpc/auth_tenant.go +++ b/pkg/rpc/auth_tenant.go @@ -141,14 +141,14 @@ func (a tenantAuthorizer) authorize( case "/cockroach.server.serverpb.Status/HotRangesV2": return a.authHotRangesV2(tenID) - case "/cockroach.server.serverpb.Status/NodesUI": - return a.authCapability(tenID) + case "/cockroach.server.serverpb.Status/Nodes": + return a.capabilitiesAuthorizer.HasNodeStatusCapability(ctx, tenID) case "/cockroach.server.serverpb.Admin/Liveness": - return a.authCapability(tenID) + return a.capabilitiesAuthorizer.HasNodeStatusCapability(ctx, tenID) case "/cockroach.ts.tspb.TimeSeries/Query": - return a.authCapability(tenID) + return a.capabilitiesAuthorizer.HasTSDBQueryCapability(ctx, tenID) default: return authErrorf("unknown method %q", fullMethod) @@ -291,12 +291,6 @@ func (a tenantAuthorizer) authTenant(id roachpb.TenantID) error { return nil } -// authCapability checks if the current tenant has the requested capability. -func (a tenantAuthorizer) authCapability(id roachpb.TenantID) error { - // TODO(davidh): add capability-specific checks here that correspond to specific requests. - return nil -} - // gossipSubscriptionPatternAllowlist contains keys outside of a tenant's // keyspace that GossipSubscription RPC invocations are allowed to touch. // WIP: can't import gossip directly. diff --git a/pkg/rpc/auth_test.go b/pkg/rpc/auth_test.go index 6914704ba912..7be00f3b3ea5 100644 --- a/pkg/rpc/auth_test.go +++ b/pkg/rpc/auth_test.go @@ -940,7 +940,9 @@ func TestTenantAuthCapabilityChecks(t *testing.T) { } type mockAuthorizer struct { - hasCapabilityForBatch bool + hasCapabilityForBatch bool + hasNodestatusCapability bool + hasTSDBQueryCapability bool } var _ tenantcapabilities.Authorizer = &mockAuthorizer{} @@ -959,3 +961,17 @@ func (m mockAuthorizer) HasCapabilityForBatch( func (m mockAuthorizer) BindReader(tenantcapabilities.Reader) { panic("unimplemented") } + +func (m mockAuthorizer) HasNodeStatusCapability(ctx context.Context, tenID roachpb.TenantID) error { + if m.hasNodestatusCapability { + return nil + } + return errors.New("tenant does not have capability") +} + +func (m mockAuthorizer) HasTSDBQueryCapability(ctx context.Context, tenID roachpb.TenantID) error { + if m.hasTSDBQueryCapability { + return nil + } + return errors.New("tenant does not have capability") +} diff --git a/pkg/server/admin.go b/pkg/server/admin.go index 9b194c7fd617..ad808e98e69d 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -2185,6 +2185,11 @@ func getLivenessResponse( }, nil } +// Liveness is implemented on the tenant-facing admin server +// as a request through the tenant connector. Since at the +// time of writing, the request contains no additional SQL +// permission checks, the tenant capability gate is all +// that is required. This is handled by the connector. func (s *adminServer) Liveness( ctx context.Context, req *serverpb.LivenessRequest, ) (*serverpb.LivenessResponse, error) { diff --git a/pkg/server/serverpb/status.go b/pkg/server/serverpb/status.go index 91ec925dec24..26bf71d8f86e 100644 --- a/pkg/server/serverpb/status.go +++ b/pkg/server/serverpb/status.go @@ -46,6 +46,7 @@ type SQLStatusServer interface { LogFilesList(context.Context, *LogFilesListRequest) (*LogFilesListResponse, error) LogFile(context.Context, *LogFileRequest) (*LogEntriesResponse, error) Logs(context.Context, *LogsRequest) (*LogEntriesResponse, error) + NodesUI(context.Context, *NodesRequest) (*NodesResponseExternal, error) } // OptionalNodesStatusServer is a StatusServer that is only optionally present @@ -83,11 +84,9 @@ type TenantStatusServer interface { Regions(context.Context, *RegionsRequest) (*RegionsResponse, error) HotRangesV2(context.Context, *HotRangesRequest) (*HotRangesResponseV2, error) - // NodesUI is used by DB Console. - NodesUI(context.Context, *NodesRequest) (*NodesResponseExternal, error) - // SpanStats is used to access MVCC stats from KV SpanStats(context.Context, *roachpb.SpanStatsRequest) (*roachpb.SpanStatsResponse, error) + Nodes(context.Context, *NodesRequest) (*NodesResponse, error) } // OptionalNodesStatusServer returns the wrapped NodesStatusServer, if it is diff --git a/pkg/server/status.go b/pkg/server/status.go index f5d5c1054216..9511d536500e 100644 --- a/pkg/server/status.go +++ b/pkg/server/status.go @@ -1574,13 +1574,40 @@ func (s *systemStatusServer) Nodes( return resp, nil } +// NodesUI on the tenant, delegates to the storage layer's endpoint after +// checking local SQL permissions. The tenant connector will require +// additional tenant capabilities in order to let the request through, +// but the `NodesTenant` endpoint will do no additional permissioning on +// the system-tenant side. func (s *statusServer) NodesUI( ctx context.Context, req *serverpb.NodesRequest, ) (*serverpb.NodesResponseExternal, error) { - ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - return s.sqlServer.tenantConnect.NodesUI(ctx, req) + hasViewClusterMetadata := false + err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx) + if err != nil { + if !grpcutil.IsAuthError(err) { + return nil, err + } + } else { + hasViewClusterMetadata = true + } + + internalResp, err := s.sqlServer.tenantConnect.Nodes(ctx, req) + if err != nil { + return nil, serverError(ctx, err) + } + + resp := &serverpb.NodesResponseExternal{ + Nodes: make([]serverpb.NodeResponse, len(internalResp.Nodes)), + LivenessByNodeID: internalResp.LivenessByNodeID, + } + for i, nodeStatus := range internalResp.Nodes { + resp.Nodes[i] = nodeStatusToResp(&nodeStatus, hasViewClusterMetadata) + } + + return resp, nil } func (s *systemStatusServer) NodesUI( diff --git a/pkg/sql/gcjob_test/gc_job_test.go b/pkg/sql/gcjob_test/gc_job_test.go index 71abf4780b36..c3287225d076 100644 --- a/pkg/sql/gcjob_test/gc_job_test.go +++ b/pkg/sql/gcjob_test/gc_job_test.go @@ -490,7 +490,8 @@ func TestGCTenant(t *testing.T) { t, gcClosure(dropTenID, progress), `GC state for tenant is DELETED yet the tenant row still exists: `+ - `{ProtoInfo:{DeprecatedID:11 DeprecatedDataState:DROP DroppedName: TenantReplicationJobID:0 Capabilities:{CanAdminSplit:false}} `+ + `{ProtoInfo:{DeprecatedID:11 DeprecatedDataState:DROP DroppedName: TenantReplicationJobID:0 `+ + `Capabilities:{CanAdminSplit:false CanViewNodeInfo:false CanViewTsdbMetrics:false}} `+ `SQLInfo:{ID:11 Name:tenant-11 DataState:drop ServiceMode:none}}`, ) }) diff --git a/pkg/sql/logictest/testdata/logic_test/tenant b/pkg/sql/logictest/testdata/logic_test/tenant index 63ebe900f00c..15be047f9f58 100644 --- a/pkg/sql/logictest/testdata/logic_test/tenant +++ b/pkg/sql/logictest/testdata/logic_test/tenant @@ -38,10 +38,10 @@ FROM system.tenants ORDER BY id ---- id active name crdb_internal.pb_to_json -1 true system {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -2 true tenant-one {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} -3 true two {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "3", "droppedName": "", "tenantReplicationJobId": "0"} -4 true three {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "4", "droppedName": "", "tenantReplicationJobId": "0"} +1 true system {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +2 true tenant-one {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} +3 true two {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "3", "droppedName": "", "tenantReplicationJobId": "0"} +4 true three {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "4", "droppedName": "", "tenantReplicationJobId": "0"} query ITTT colnames SHOW TENANT system @@ -158,7 +158,7 @@ FROM system.tenants WHERE name = 'four' ORDER BY id ---- id active name crdb_internal.pb_to_json -5 true four {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "5", "droppedName": "", "tenantReplicationJobId": "0"} +5 true four {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "5", "droppedName": "", "tenantReplicationJobId": "0"} statement ok DROP TENANT four @@ -232,14 +232,14 @@ FROM system.tenants ORDER BY id ---- id active name crdb_internal.pb_to_json -1 true system {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -2 true tenant-one {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} -3 true two {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "3", "droppedName": "", "tenantReplicationJobId": "0"} -4 true three {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "4", "droppedName": "", "tenantReplicationJobId": "0"} -5 false NULL {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "DROP", "deprecatedId": "5", "droppedName": "four", "tenantReplicationJobId": "0"} -6 false NULL {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "DROP", "deprecatedId": "6", "droppedName": "five-requiring-quotes", "tenantReplicationJobId": "0"} -7 false NULL {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "DROP", "deprecatedId": "7", "droppedName": "to-be-reclaimed", "tenantReplicationJobId": "0"} -8 true to-be-reclaimed {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "8", "droppedName": "", "tenantReplicationJobId": "0"} +1 true system {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +2 true tenant-one {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} +3 true two {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "3", "droppedName": "", "tenantReplicationJobId": "0"} +4 true three {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "4", "droppedName": "", "tenantReplicationJobId": "0"} +5 false NULL {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "DROP", "deprecatedId": "5", "droppedName": "four", "tenantReplicationJobId": "0"} +6 false NULL {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "DROP", "deprecatedId": "6", "droppedName": "five-requiring-quotes", "tenantReplicationJobId": "0"} +7 false NULL {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "DROP", "deprecatedId": "7", "droppedName": "to-be-reclaimed", "tenantReplicationJobId": "0"} +8 true to-be-reclaimed {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "8", "droppedName": "", "tenantReplicationJobId": "0"} # More valid tenant names. statement ok diff --git a/pkg/sql/logictest/testdata/logic_test/tenant_builtins b/pkg/sql/logictest/testdata/logic_test/tenant_builtins index e5fcd981b6a0..5b786fbf9c5e 100644 --- a/pkg/sql/logictest/testdata/logic_test/tenant_builtins +++ b/pkg/sql/logictest/testdata/logic_test/tenant_builtins @@ -43,10 +43,10 @@ FROM system.tenants ORDER BY id ---- id active name data_state service_mode crdb_internal.pb_to_json -1 true system 1 2 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -2 true tenant-number-eleven 1 0 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} -5 true tenant-5 1 0 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "5", "droppedName": "", "tenantReplicationJobId": "0"} -10 true tenant-number-ten 1 0 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "10", "droppedName": "", "tenantReplicationJobId": "0"} +1 true system 1 2 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +2 true tenant-number-eleven 1 0 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} +5 true tenant-5 1 0 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "5", "droppedName": "", "tenantReplicationJobId": "0"} +10 true tenant-number-ten 1 0 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "10", "droppedName": "", "tenantReplicationJobId": "0"} # Check we can add a name where none existed before. statement ok @@ -114,10 +114,10 @@ FROM system.tenants ORDER BY id ---- id active name data_state service_mode crdb_internal.pb_to_json -1 true system 1 2 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -2 true tenant-number-eleven 1 0 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} -5 false NULL 2 0 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "DROP", "deprecatedId": "5", "droppedName": "my-new-tenant-name", "tenantReplicationJobId": "0"} -10 true tenant-number-ten 1 0 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "10", "droppedName": "", "tenantReplicationJobId": "0"} +1 true system 1 2 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +2 true tenant-number-eleven 1 0 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} +5 false NULL 2 0 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "DROP", "deprecatedId": "5", "droppedName": "my-new-tenant-name", "tenantReplicationJobId": "0"} +10 true tenant-number-ten 1 0 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "10", "droppedName": "", "tenantReplicationJobId": "0"} # Try to recreate an existing tenant. @@ -225,9 +225,9 @@ FROM system.tenants ORDER BY id ---- id active name data_state service_mode crdb_internal.pb_to_json -1 true system 1 2 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} -2 true tenant-number-eleven 1 0 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} -10 true tenant-number-ten 1 0 {"capabilities": {"canAdminSplit": false}, "deprecatedDataState": "READY", "deprecatedId": "10", "droppedName": "", "tenantReplicationJobId": "0"} +1 true system 1 2 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "1", "droppedName": "", "tenantReplicationJobId": "0"} +2 true tenant-number-eleven 1 0 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "2", "droppedName": "", "tenantReplicationJobId": "0"} +10 true tenant-number-ten 1 0 {"capabilities": {"canAdminSplit": false, "canViewNodeInfo": false, "canViewTsdbMetrics": false}, "deprecatedDataState": "READY", "deprecatedId": "10", "droppedName": "", "tenantReplicationJobId": "0"} query error tenant resource limits require a CCL binary SELECT crdb_internal.update_tenant_resource_limits(10, 1000, 100, 0, now(), 0) diff --git a/pkg/sql/show_tenant.go b/pkg/sql/show_tenant.go index 48d83aa9b341..db7bef1e3214 100644 --- a/pkg/sql/show_tenant.go +++ b/pkg/sql/show_tenant.go @@ -121,6 +121,14 @@ func (n *showTenantNode) getTenantValues( // TODO(sql-sessions): handle this capability. value: strconv.FormatBool(false), }, + { + name: canViewNodeInfoCapabilityName, + value: strconv.FormatBool(capabilities.CanViewNodeInfo), + }, + { + name: canViewTsdbMetricsCapabilityName, + value: strconv.FormatBool(capabilities.CanViewTsdbMetrics), + }, } } diff --git a/pkg/sql/tenant_capability.go b/pkg/sql/tenant_capability.go index cf4ae473f078..953baf97119a 100644 --- a/pkg/sql/tenant_capability.go +++ b/pkg/sql/tenant_capability.go @@ -25,10 +25,14 @@ import ( const canAdminSplitCapabilityName = "can_admin_split" const canAdminUnsplitCapabilityName = "can_admin_unsplit" +const canViewNodeInfoCapabilityName = "can_view_node_info" +const canViewTsdbMetricsCapabilityName = "can_view_tsdb_metrics" var capabilityTypes = map[string]*types.T{ - canAdminSplitCapabilityName: types.Bool, - canAdminUnsplitCapabilityName: types.Bool, + canAdminSplitCapabilityName: types.Bool, + canAdminUnsplitCapabilityName: types.Bool, + canViewNodeInfoCapabilityName: types.Bool, + canViewTsdbMetricsCapabilityName: types.Bool, } const alterTenantCapabilityOp = "ALTER TENANT CAPABILITY" @@ -131,6 +135,28 @@ func (n *alterTenantCapabilityNode) startExec(params runParams) error { // TODO(sql-sessions): handle this capability. return unimplemented.Newf("cap-unsplit", "update capability %q", cap.Name) + case canViewNodeInfoCapabilityName: + if n.n.IsRevoke { + dst.CanViewNodeInfo = false + } else { + b, err := paramparse.DatumAsBool(ctx, p.EvalContext(), cap.Name, n.typedExprs[i]) + if err != nil { + return err + } + dst.CanViewNodeInfo = b + } + + case canViewTsdbMetricsCapabilityName: + if n.n.IsRevoke { + dst.CanViewTsdbMetrics = false + } else { + b, err := paramparse.DatumAsBool(ctx, p.EvalContext(), cap.Name, n.typedExprs[i]) + if err != nil { + return err + } + dst.CanViewTsdbMetrics = b + } + default: return errors.AssertionFailedf("unhandled: %q", cap.Name) } diff --git a/pkg/ts/server.go b/pkg/ts/server.go index e429a53d5dbb..d216a5fbfbd3 100644 --- a/pkg/ts/server.go +++ b/pkg/ts/server.go @@ -109,6 +109,10 @@ type TenantServer struct { var _ tspb.TenantTimeSeriesServer = &TenantServer{} +// Query delegates to the tenant connector to query +// the tsdb on the system tenant. The only authorization +// necessary is the tenant capability check on the +// connector. func (t *TenantServer) Query( ctx context.Context, req *tspb.TimeSeriesQueryRequest, ) (*tspb.TimeSeriesQueryResponse, error) {