spanconfig: introduce a new read-only system target type

This patch is motivated by the host tenant's desire to fetch all system
span configurations it has installed over secondary tenants without
explicitly constructing targets for all tenant IDs in the system. This
is handy for the host tenant's reconciliation job, which populates an
in-memory view of all system span configurations to diff against the
state implied by the schema. This inturn allows the host tenant to issue
targeted updates/deletes.

We introduce a new system target type to achieve this which allows a
tenant to request all system span configurations it has installed that
target tenant keyspaces. This is only ever consequential in the context
of the host as only the host tenant can install system span
configurations on other tenants. We also make this target read-only,
in that, we disallow persisting its implied encoding in
`system.span_configurations`. We add validation to the KVAccessor to
this effect.

While here, we also disallow creating span targets that overlap with
the reserved system span configuration keyspace. We also improve the
concepts around these system targets to talk about "tenant keyspaces'
and "entire keyspace" as opposed to "tenant" and "cluster".

Release note: None

pkg/ccl/spanconfigccl/spanconfigreconcilerccl/BUILD.bazel

@@ -13,7 +13,6 @@ go_test(
         "//pkg/ccl/partitionccl",
         "//pkg/ccl/utilccl",
         "//pkg/jobs",
-        "//pkg/keys",
         "//pkg/roachpb",
         "//pkg/security",
         "//pkg/security/securitytest",

pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go

@@ -19,7 +19,6 @@ import (
 	_ "github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvtenantccl"
 	_ "github.com/cockroachdb/cockroach/pkg/ccl/partitionccl"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
-	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigtestutils"
@@ -192,7 +191,7 @@ func TestDataDriven(t *testing.T) {
 					return nil
 				})
 				records, err := kvAccessor.GetSpanConfigRecords(
-					ctx, []spanconfig.Target{spanconfig.MakeTargetFromSpan(keys.EverythingSpan)},
+					ctx, spanconfig.TestingEntireSpanConfigurationStateTargets(),
 				)
 				require.NoError(t, err)
 				sort.Slice(records, func(i, j int) bool {

pkg/migration/migrations/migrate_span_configs_test.go

@@ -17,7 +17,6 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
-	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed/rangefeedcache"
 	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
@@ -67,10 +66,11 @@ func TestEnsureSpanConfigReconciliation(t *testing.T) {
 	tdb.Exec(t, `SET CLUSTER SETTING spanconfig.reconciliation_job.enabled = true`)
 	tdb.Exec(t, `SET CLUSTER SETTING spanconfig.reconciliation_job.checkpoint_interval = '100ms'`)
 
-	{ // Ensure that no span config entries are found.
-		records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{
-			spanconfig.MakeTargetFromSpan(keys.EverythingSpan),
-		})
+	{ // Ensure that no span config records are found.
+		records, err := scKVAccessor.GetSpanConfigRecords(
+			ctx,
+			spanconfig.TestingEntireSpanConfigurationStateTargets(),
+		)
 		require.NoError(t, err)
 		require.Empty(t, records)
 	}
@@ -90,9 +90,10 @@ func TestEnsureSpanConfigReconciliation(t *testing.T) {
 	require.False(t, scReconciler.Checkpoint().IsEmpty())
 
 	{ // Ensure that the host tenant's span configs are installed.
-		records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{
-			spanconfig.MakeTargetFromSpan(keys.EverythingSpan),
-		})
+		records, err := scKVAccessor.GetSpanConfigRecords(
+			ctx,
+			spanconfig.TestingEntireSpanConfigurationStateTargets(),
+		)
 		require.NoError(t, err)
 		require.NotEmpty(t, records)
 	}
@@ -152,10 +153,11 @@ func TestEnsureSpanConfigReconciliationMultiNode(t *testing.T) {
 	tdb.Exec(t, `SET CLUSTER SETTING spanconfig.reconciliation_job.enabled = true`)
 	tdb.Exec(t, `SET CLUSTER SETTING spanconfig.reconciliation_job.checkpoint_interval = '100ms'`)
 
-	{ // Ensure that no span config entries are to be found.
-		records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{
-			spanconfig.MakeTargetFromSpan(keys.EverythingSpan),
-		})
+	{ // Ensure that no span config records are to be found.
+		records, err := scKVAccessor.GetSpanConfigRecords(
+			ctx,
+			spanconfig.TestingEntireSpanConfigurationStateTargets(),
+		)
 		require.NoError(t, err)
 		require.Empty(t, records)
 	}
@@ -175,9 +177,10 @@ func TestEnsureSpanConfigReconciliationMultiNode(t *testing.T) {
 	require.False(t, scReconciler.Checkpoint().IsEmpty())
 
 	{ // Ensure that the host tenant's span configs are installed.
-		records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{
-			spanconfig.MakeTargetFromSpan(keys.EverythingSpan),
-		})
+		records, err := scKVAccessor.GetSpanConfigRecords(
+			ctx,
+			spanconfig.TestingEntireSpanConfigurationStateTargets(),
+		)
 		require.NoError(t, err)
 		require.NotEmpty(t, records)
 	}
@@ -219,9 +222,10 @@ func TestEnsureSpanConfigSubscription(t *testing.T) {
 	tdb.Exec(t, `SET CLUSTER SETTING spanconfig.reconciliation_job.enabled = true`)
 
 	testutils.SucceedsSoon(t, func() error {
-		records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{
-			spanconfig.MakeTargetFromSpan(keys.EverythingSpan),
-		})
+		records, err := scKVAccessor.GetSpanConfigRecords(
+			ctx,
+			spanconfig.TestingEntireSpanConfigurationStateTargets(),
+		)
 		require.NoError(t, err)
 		if len(records) == 0 {
 			return fmt.Errorf("empty global span configuration state")

pkg/migration/migrations/seed_tenant_span_configs.go

@@ -78,7 +78,7 @@ func seedTenantSpanConfigsMigration(
 				return err
 			}
 			if len(scRecords) != 0 {
-				// This tenant already has span config entries. It was either
+				// This tenant already has span config records. It was either
 				// already migrated (migrations need to be idempotent) or it was
 				// created after PreSeedTenantSpanConfigs was activated. There's
 				// nothing left to do here.

pkg/roachpb/span_config.go

@@ -127,3 +127,55 @@ func TestingDatabaseSystemSpanConfig(host bool) SpanConfig {
 	config.GCPolicy.IgnoreStrictEnforcement = true
 	return config
 }
+
+// IsEntireKeyspaceTarget returns true if the receiver targets the entire
+// keyspace.
+func (st SystemSpanConfigTarget) IsEntireKeyspaceTarget() bool {
+	return st.Type.GetEntireKeyspace() != nil
+}
+
+// IsSpecificTenantKeyspaceTarget returns true if the receiver targets a
+// specific tenant's keyspace.
+func (st SystemSpanConfigTarget) IsSpecificTenantKeyspaceTarget() bool {
+	return st.Type.GetSpecificTenantKeyspace() != nil
+}
+
+// IsAllTenantKeyspaceTargetsSetTarget returns true if the receiver target
+// encompasses all targets that have been set on specific tenant keyspaces
+// by the system target source.
+func (st SystemSpanConfigTarget) IsAllTenantKeyspaceTargetsSetTarget() bool {
+	return st.Type.GetAllTenantKeyspaceTargetsSet() != nil
+}
+
+// NewEntireKeyspaceTargetType returns a system span config target type that
+// targets the entire keyspace.
+func NewEntireKeyspaceTargetType() *SystemSpanConfigTarget_Type {
+	return &SystemSpanConfigTarget_Type{
+		Type: &SystemSpanConfigTarget_Type_EntireKeyspace{
+			EntireKeyspace: &SystemSpanConfigTarget_EntireKeyspace{},
+		},
+	}
+}
+
+// NewSpecificTenantKeyspaceTargetType returns a system span config target type
+// that the given tenant ID's keyspace.
+func NewSpecificTenantKeyspaceTargetType(tenantID TenantID) *SystemSpanConfigTarget_Type {
+	return &SystemSpanConfigTarget_Type{
+		Type: &SystemSpanConfigTarget_Type_SpecificTenantKeyspace{
+			SpecificTenantKeyspace: &SystemSpanConfigTarget_TenantKeyspace{
+				TenantID: tenantID,
+			},
+		},
+	}
+}
+
+// NewAllTenantKeyspaceTargetsSetTargetType returns a read-only system span
+// config target  type that encompasses all targets that have been set on
+// specific tenant keyspaces.
+func NewAllTenantKeyspaceTargetsSetTargetType() *SystemSpanConfigTarget_Type {
+	return &SystemSpanConfigTarget_Type{
+		Type: &SystemSpanConfigTarget_Type_AllTenantKeyspaceTargetsSet{
+			AllTenantKeyspaceTargetsSet: &SystemSpanConfigTarget_AllTenantKeyspaceTargetsSet{},
+		},
+	}
+}

pkg/roachpb/span_config.proto

@@ -175,24 +175,66 @@ message SpanConfig {
 }
 
 // SystemSpanConfigTarget specifies the target of system span configurations.
+// System targets are designed for a few different kinds of interactions. We
+// want the ability to:
+// 1. Allow the host tenant to set a system span configuration on the entire
+// keyspace.
+// 2. Allow the host tenant to set a system span configuration on a particular
+// tenant's keyspace.
+// 3. Allow secondary tenants to set system span configurations on their
+// keyspace.
+//
+// Additionally, we also want each tenant to be able to fetch all system span
+// configurations that it has installed. Ideally, we want to be able to do this
+// without knowing the tenantID of all other tenants that exist. We provide a
+// read-only system span config target type to achieve exactly this.
 message SystemSpanConfigTarget {
   option (gogoproto.equal) = true;
 
   // SourceTenantID is the ID of the tenant that specified the system span
   // configuration.
   TenantID source_tenant_id = 1 [(gogoproto.nullable) = false, (gogoproto.customname) = "SourceTenantID"];
 
-  // TargetTenantID is the ID of the tenant that the associated system span
-  // configuration applies to.
-  //
-  // If the host tenant is the source and the target is unspecified then the
-  // associated system span configuration applies over all ranges in the system
-  // (including those belonging to secondary tenants).
-  //
-  // Secondary tenants are only allowed to target themselves and must fill in
-  // this field. The host tenant may use this field to target a specific
-  // secondary tenant.
-  TenantID target_tenant_id = 2 [(gogoproto.customname) = "TargetTenantID"];
+  // TenantKeyspace is a target type that targets the keyspace of a specific
+  // tenant.
+  message TenantKeyspace {
+    option (gogoproto.equal) = true;
+
+    // TenantID is the ID of the tenant whose keyspace the the associated
+    // system span configuration applies to.
+    //
+    // Secondary tenants are only allowed to target their keyspace. The host
+    // tenant may use this field to target a specific secondary tenant's
+    // keyspace.
+    TenantID tenant_id = 1 [(gogoproto.nullable) = false, (gogoproto.customname) = "TenantID"];
+  };
+
+   // EntireKeyspace is a target type that targets the entire keyspace (all
+   // ranges, including those belonging to secondary tenants). Only the host
+   // tenant is allowed to target the entire keyspace.
+  message EntireKeyspace{
+    option (gogoproto.equal) = true;
+  };
+
+  // AllTenantKeyspacesTargetsSet is is a read-only system target type that
+  // encompasses all system targets that have been set by the source tenant on
+  // specific tenant's keyspaces.
+  message AllTenantKeyspaceTargetsSet{
+    option (gogoproto.equal) = true;
+  };
+
+  message Type {
+    option (gogoproto.equal) = true;
+
+    oneof type {
+      TenantKeyspace specific_tenant_keyspace = 1;
+      EntireKeyspace entire_keyspace = 2;
+      AllTenantKeyspaceTargetsSet all_tenant_keyspace_targets_set = 3;
+    }
+  }
+
+  // Type of the system target.
+  Type type = 2;
 }
 
 // SpanConfigTarget specifies the target of an associated span configuration.

pkg/rpc/auth_tenant.go

@@ -315,11 +315,12 @@ func validateSpanConfigTarget(
 			return nil
 		}
 
-		if target.TargetTenantID == nil {
-			return authErrorf("secondary tenants must explicitly target themselves")
+		if target.IsEntireKeyspaceTarget() {
+			return authErrorf("secondary tenants cannot target the entire keyspace")
 		}
 
-		if target.SourceTenantID != *target.TargetTenantID {
+		if target.IsSpecificTenantKeyspaceTarget() &&
+			target.Type.GetSpecificTenantKeyspace().TenantID != target.SourceTenantID {
 			return authErrorf(
 				"secondary tenants cannot interact with system span configurations of other tenants",
 			)

pkg/rpc/auth_test.go

@@ -187,12 +187,11 @@ func TestTenantAuthRequest(t *testing.T) {
 		return ru
 	}
 	makeSystemSpanConfigTarget := func(source, target uint64) roachpb.SpanConfigTarget {
-		targetID := roachpb.MakeTenantID(target)
 		return roachpb.SpanConfigTarget{
 			Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 				SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
 					SourceTenantID: roachpb.MakeTenantID(source),
-					TargetTenantID: &targetID,
+					Type:           roachpb.NewSpecificTenantKeyspaceTargetType(roachpb.MakeTenantID(target)),
 				},
 			},
 		}
@@ -536,11 +535,22 @@ func TestTenantAuthRequest(t *testing.T) {
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
 							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							Type:           roachpb.NewEntireKeyspaceTargetType(),
 						},
 					},
 				}),
-				expErr: `secondary tenants must explicitly target themselves`,
+				expErr: `secondary tenants cannot target the entire keyspace`,
+			},
+			{
+				req: makeGetSpanConfigsReq(roachpb.SpanConfigTarget{
+					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
+						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
+							SourceTenantID: roachpb.MakeTenantID(20),
+							Type:           roachpb.NewEntireKeyspaceTargetType(),
+						},
+					},
+				}),
+				expErr: `malformed source tenant field`,
 			},
 		},
 		"/cockroach.roachpb.Internal/UpdateSpanConfigs": {
@@ -651,11 +661,11 @@ func TestTenantAuthRequest(t *testing.T) {
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
 							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							Type:           roachpb.NewEntireKeyspaceTargetType(),
 						},
 					},
 				}, false),
-				expErr: `secondary tenants must explicitly target themselves`,
+				expErr: `secondary tenants cannot target the entire keyspace`,
 			},
 			{
 				req:    makeUpdateSpanConfigsReq(makeSystemSpanConfigTarget(10, 10), true),
@@ -676,11 +686,11 @@ func TestTenantAuthRequest(t *testing.T) {
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
 							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							Type:           roachpb.NewEntireKeyspaceTargetType(),
 						},
 					},
 				}, true),
-				expErr: `secondary tenants must explicitly target themselves`,
+				expErr: `secondary tenants cannot target the entire keyspace`,
 			},
 		},
 

pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go

@@ -309,7 +309,7 @@ func (k *KVAccessor) constructDeleteStmtAndArgs(
 }
 
 // constructUpsertStmtAndArgs constructs the statement and query arguments
-// needed to upsert the given span config entries.
+// needed to upsert the given span config records.
 func (k *KVAccessor) constructUpsertStmtAndArgs(
 	toUpsert []spanconfig.Record,
 ) (string, []interface{}, error) {
@@ -429,6 +429,12 @@ func validateUpdateArgs(toDelete []spanconfig.Target, toUpsert []spanconfig.Reco
 		copy(targets, list)
 		sort.Sort(spanconfig.Targets(targets))
 		for i := range targets {
+			if targets[i].IsSystemTarget() && targets[i].GetSystemTarget().IsReadOnly() {
+				return errors.AssertionFailedf(
+					"cannot use read only system target %s as an update argument", targets[i],
+				)
+			}
+
 			if i == 0 {
 				continue
 			}

pkg/spanconfig/spanconfigkvaccessor/kvaccessor_test.go

@@ -34,9 +34,11 @@ import (
 // 		span [a,e)
 // 		span [a,b)
 // 		span [b,c)
+//		system-target {cluster}
 //		system-target {source=1,target=20}
 //		system-target {source=1,target=1}
 //		system-target {source=20,target=20}
+// 		system-target {source=1, all-tenant-keyspace-targets-set}
 //      ----
 //
 // 		kvaccessor-update
@@ -45,13 +47,14 @@ import (
 // 		upsert [d,e):D
 // 		delete {source=1,target=1}
 // 		upsert {source=1,target=1}:A
+// 		upsert {cluster}:F
 //      ----
 //
 // They tie into GetSpanConfigRecords and UpdateSpanConfigRecords
 // respectively. For kvaccessor-get, each listed target is added to the set of
 // targets being read. For kvaccessor-update, the lines prefixed with "delete"
 // count towards the targets being deleted, and for "upsert" they correspond to
-// the span config entries being upserted. See
+// the span config records being upserted. See
 // spanconfigtestutils.Parse{Span,Config,SpanConfigRecord} for more details.
 func TestDataDriven(t *testing.T) {
 	defer leaktest.AfterTest(t)()