Merge pull request #101163 from cockroachdb/blathers/backport-release…

…-23.1-100921

release-23.1: multitenant: add can_check_consistency capability

pkg/ccl/logictestccl/testdata/logic_test/tenant_capability

@@ -35,6 +35,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            true
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -59,6 +60,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            true
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -76,6 +78,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            false
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -100,6 +103,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            true
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -124,6 +128,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            true
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -148,6 +153,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            true
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         true
 can_view_tsdb_metrics      false
@@ -165,6 +171,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            false
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -182,6 +189,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            false
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -199,6 +207,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            false
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -236,6 +245,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            true
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false
@@ -264,6 +274,7 @@ can_admin_relocate_range   false
 can_admin_scatter          true
 can_admin_split            true
 can_admin_unsplit          false
+can_check_consistency      false
 can_use_nodelocal_storage  false
 can_view_node_info         false
 can_view_tsdb_metrics      false

pkg/multitenant/tenantcapabilities/capabilities.go

@@ -64,6 +64,9 @@ const (
 	// want to hide from customer tenants in CockroachCloud.
 	CanViewNodeInfo // can_view_node_info
 
+	// CanCheckConsistency allows the tenant to check range consistency.
+	CanCheckConsistency // can_check_consistency
+
 	// CanViewTSDBMetrics describes the ability of a tenant to read the
 	// timeseries from the storage cluster. These operations need a
 	// capability because excessive TS queries can overwhelm the storage
@@ -116,6 +119,7 @@ var capabilities = [MaxCapabilityID + 1]Capability{
 	CanAdminScatter:        boolCapability(CanAdminScatter),
 	CanAdminSplit:          boolCapability(CanAdminSplit),
 	CanAdminUnsplit:        boolCapability(CanAdminUnsplit),
+	CanCheckConsistency:    boolCapability(CanCheckConsistency),
 	CanUseNodelocalStorage: boolCapability(CanUseNodelocalStorage),
 	CanViewNodeInfo:        boolCapability(CanViewNodeInfo),
 	CanViewTSDBMetrics:     boolCapability(CanViewTSDBMetrics),

pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.go

@@ -159,12 +159,12 @@ var reqMethodToCap = map[kvpb.Method]tenantcapabilities.ID{
 	kvpb.AdminUnsplit:        tenantcapabilities.CanAdminUnsplit,
 	kvpb.AdminRelocateRange:  tenantcapabilities.CanAdminRelocateRange,
 	kvpb.AdminTransferLease:  tenantcapabilities.CanAdminRelocateRange,
+	kvpb.CheckConsistency:    tenantcapabilities.CanCheckConsistency,
 
 	// TODO(knz,arul): Verify with the relevant teams whether secondary
 	// tenants have legitimate access to any of those.
 	kvpb.AdminMerge:                    onlySystemTenant,
 	kvpb.AdminVerifyProtectedTimestamp: onlySystemTenant,
-	kvpb.CheckConsistency:              onlySystemTenant,
 	kvpb.ComputeChecksum:               onlySystemTenant,
 	kvpb.GC:                            onlySystemTenant,
 	kvpb.Merge:                         onlySystemTenant,

pkg/multitenant/tenantcapabilities/tenantcapabilitiespb/capabilities.proto

@@ -88,6 +88,10 @@ message TenantCapabilities {
   // CanUseNodelocalStorage if set to true, grants the tenant the ability
   // to use nodelocal storage  on any KV node.
   bool can_use_nodelocal_storage = 9;
+
+  // CanCheckConsistency if set to true, grants the tenant the ability to run
+  // range consistency checks.
+  bool can_check_consistency = 10;
 };
 
 // SpanConfigBound is used to constrain the possible values a SpanConfig may

pkg/multitenant/tenantcapabilities/values.go

@@ -44,6 +44,8 @@ func GetValueByID(t *tenantcapabilitiespb.TenantCapabilities, id ID) (Value, err
 		return (*invertedBoolValue)(&t.DisableAdminSplit), nil
 	case CanAdminUnsplit:
 		return (*boolValue)(&t.CanAdminUnsplit), nil
+	case CanCheckConsistency:
+		return (*boolValue)(&t.CanCheckConsistency), nil
 	case CanUseNodelocalStorage:
 		return (*boolValue)(&t.CanUseNodelocalStorage), nil
 	case CanViewNodeInfo:

pkg/sql/faketreeeval/BUILD.bazel

@@ -24,6 +24,7 @@ go_library(
         "//pkg/util/duration",
         "//pkg/util/errorutil/unimplemented",
         "//pkg/util/mon",
+        "//pkg/util/rangedesc",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_lib_pq//oid",
     ],

pkg/sql/faketreeeval/evalctx.go

@@ -32,6 +32,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/duration"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
+	"github.com/cockroachdb/cockroach/pkg/util/rangedesc"
 	"github.com/cockroachdb/errors"
 	"github.com/lib/pq/oid"
 )
@@ -468,6 +469,13 @@ func (ep *DummyEvalPlanner) EnforceHomeRegion() bool {
 	return false
 }
 
+// GetRangeDescIterator is part of the eval.Planner interface.
+func (ep *DummyEvalPlanner) GetRangeDescIterator(
+	context.Context, roachpb.Span,
+) (_ rangedesc.Iterator, _ error) {
+	return
+}
+
 // GetRangeDescByID is part of the eval.Planner interface.
 func (ep *DummyEvalPlanner) GetRangeDescByID(
 	context.Context, roachpb.RangeID,

pkg/sql/logictest/BUILD.bazel

@@ -35,6 +35,7 @@ go_library(
         "//pkg/kv/kvclient/rangefeed",
         "//pkg/kv/kvserver",
         "//pkg/kv/kvserver/kvserverbase",
+        "//pkg/multitenant/tenantcapabilities",
         "//pkg/security/username",
         "//pkg/server",
         "//pkg/settings/cluster",

pkg/sql/logictest/logic.go

@@ -48,6 +48,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
+	"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
@@ -1536,20 +1537,33 @@ func (t *logicTest) newCluster(
 			}
 		}
 
-		tenantID := serverutils.TestTenantID().ToUint64()
+		tenantID := serverutils.TestTenantID()
 		for name, value := range clusterSettings {
 			query := fmt.Sprintf("ALTER TENANT [$1] SET CLUSTER SETTING %s = $2", name)
-			if _, err := conn.Exec(query, tenantID, value); err != nil {
+			if _, err := conn.Exec(query, tenantID.ToUint64(), value); err != nil {
 				t.Fatal(err)
 			}
 		}
 
-		for name, value := range toa.capabilities {
+		capabilities := toa.capabilities
+		for name, value := range capabilities {
 			query := fmt.Sprintf("ALTER TENANT [$1] GRANT CAPABILITY %s = $2", name)
-			if _, err := conn.Exec(query, tenantID, value); err != nil {
+			if _, err := conn.Exec(query, tenantID.ToUint64(), value); err != nil {
 				t.Fatal(err)
 			}
 		}
+		numCapabilities := len(capabilities)
+		if numCapabilities > 0 {
+			capabilityMap := make(map[tenantcapabilities.ID]string, numCapabilities)
+			for k, v := range capabilities {
+				capability, ok := tenantcapabilities.FromName(k)
+				if !ok {
+					t.Fatalf("cannot get capability from name %q", k)
+				}
+				capabilityMap[capability.ID()] = v
+			}
+			t.cluster.WaitForTenantCapabilities(t.t(), tenantID, capabilityMap)
+		}
 	}
 
 	var randomWorkmem int

pkg/sql/logictest/testdata/logic_test/builtin_function

@@ -1,3 +1,5 @@
+# tenant-capability-override-opt: can_check_consistency=true
+
 statement ok
 CREATE TABLE foo (a int)
 
@@ -3869,7 +3871,8 @@ SELECT ‹'yes'› IN (‹'no'›, ‹'maybe'›, ‹'yes'›)
 query T
 SELECT crdb_internal.redactable_sql_constants(e'\r')
 ----
-‹›
+‹
+›
 
 query T
 SELECT crdb_internal.redactable_sql_constants('SELECT crdb_internal.redactable_sql_constants('''')')
@@ -3973,3 +3976,31 @@ query T
 SELECT crdb_internal.redact(crdb_internal.redactable_sql_constants(ARRAY['SELECT 1', NULL, 'SELECT ''hello''', '']))
 ----
 {"SELECT ‹×›",NULL,"SELECT ‹×›",‹×›}
+
+subtest check_consistency
+
+# Sanity-check crdb_internal.check_consistency.
+
+statement error start key must be >=
+SELECT crdb_internal.check_consistency(true, '\x00', crdb_internal.tenant_span()[2])
+
+statement error end key must be <=
+SELECT crdb_internal.check_consistency(true, crdb_internal.tenant_span()[1], '\xffff00')
+
+statement error start key must be less than end key
+SELECT crdb_internal.check_consistency(true, crdb_internal.tenant_span()[2], crdb_internal.tenant_span()[2])
+
+query TT
+SELECT status, regexp_replace(detail, '[0-9]+', '', 'g')
+FROM crdb_internal.check_consistency(true, crdb_internal.tenant_span()[1], crdb_internal.tenant_span()[2])
+ORDER BY range_id
+LIMIT 1
+----
+RANGE_CONSISTENT  stats: {ContainsEstimates: LastUpdateNanos: IntentAge: GCBytesAge: LiveBytes: LiveCount: KeyBytes: KeyCount: ValBytes: ValCount: IntentBytes: IntentCount: SeparatedIntentCount: RangeKeyCount: RangeKeyBytes: RangeValCount: RangeValBytes: SysBytes: SysCount: AbortSpanBytes:}
+
+# Fill a table with consistency check results. This used to panic.
+# See: https://github.com/cockroachdb/cockroach/issues/88222
+statement ok
+CREATE TABLE conscheckresult AS (SELECT * FROM crdb_internal.check_consistency(false, '', ''));
+
+subtest end

pkg/sql/logictest/testdata/logic_test/builtin_function_notenant

@@ -2,25 +2,6 @@
 
 subtest check_consistency
 
-# Sanity-check crdb_internal.check_consistency.
-
-statement error start key must be > "\\x02"
-SELECT crdb_internal.check_consistency(true, '\x02', '\xffff')
-
-statement error end key must be < "\\xff\\xff"
-SELECT crdb_internal.check_consistency(true, '\x0200', '\xffff00')
-
-statement error start key must be less than end key
-SELECT crdb_internal.check_consistency(true, '\x03', '\x03')
-
-statement error start key must be less than end key
-SELECT crdb_internal.check_consistency(true, '\x04', '\x03')
-
-query ITT
-SELECT range_id, status, regexp_replace(detail, '[0-9]+', '', 'g') FROM crdb_internal.check_consistency(true, '\x03', '\xffff') WHERE range_id = 1
-----
-1  RANGE_CONSISTENT  stats: {ContainsEstimates: LastUpdateNanos: IntentAge: GCBytesAge: LiveBytes: LiveCount: KeyBytes: KeyCount: ValBytes: ValCount: IntentBytes: IntentCount: SeparatedIntentCount: RangeKeyCount: RangeKeyBytes: RangeValCount: RangeValBytes: SysBytes: SysCount: AbortSpanBytes:}
-
 # Without explicit keys, scans all ranges (we don't test this too precisely to
 # avoid flaking the test when the range count changes, just want to know that
 # we're touching multiple ranges).
@@ -51,17 +32,12 @@ SELECT count(*) = 1 FROM crdb_internal.check_consistency(true, '\xff', '')
 ----
 true
 
-# Run a full consistency check across all ranges. 
+# Run a full consistency check across all ranges.
 query B
 SELECT count(*) > 10 FROM crdb_internal.check_consistency(false, '', '')
 ----
 true
 
-# Fill a table with consistency check results. This used to panic.
-# See: https://github.com/cockroachdb/cockroach/issues/88222
-statement ok
-CREATE TABLE conscheckresult AS (SELECT * FROM crdb_internal.check_consistency(false, '', ''));
-
 # Test crdb_internal commands which execute as root, but
 # only checks for permissions afterwards.
 subtest crdb_internal_privileged_only
@@ -158,7 +134,7 @@ SELECT crdb_internal.get_zone_config(crdb_internal.get_namespace_id(0, 'root_tes
 query T
 SELECT ($t_id)::regclass
 ----
-109
+108
 
 # reset state to default
 user root