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.proto

@@ -175,24 +175,59 @@ 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. This is required during reconciliation
+// when querying the state stored in KV to compute upsert/delete requests.
+// 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"];
+  // TenantKeyspaceTarget targets the keyspace of a specific tenant.
+  message TenantKeyspaceTarget {
+    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"];
+  };
+
+  message SystemTargetType {
+    option (gogoproto.equal) = true;
+
+    oneof type {
+      TenantKeyspaceTarget specific_tenant_keyspace_target=1;
+      // EntireKeyspaceTarget targets the entire keyspace (all ranges, including
+      // those belonging to secondary tenants). Only the host tenant is allowed
+      // to target the entire keyspace.
+      bool entire_keyspace_target=2;
+      // AllTenantKeyspaceTargetsSet is a read-only system target that
+      // encompasses all system targets that have been installed by the source
+      // tenant on specific tenant's keyspaces.
+      bool all_tenant_keyspace_targets_set=3;
+    }
+  }
+
+  // SystemTargetType represents the type of the system target.
+  SystemTargetType system_target_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.SystemTargetType.GetEntireKeyspaceTarget() {
+			return authErrorf("secondary tenants cannot target the entire keyspace")
 		}
 
-		if target.SourceTenantID != *target.TargetTenantID {
+		if target.SystemTargetType.GetSpecificTenantKeyspaceTarget() != nil &&
+			target.SystemTargetType.GetSpecificTenantKeyspaceTarget().TenantID != target.SourceTenantID {
 			return authErrorf(
 				"secondary tenants cannot interact with system span configurations of other tenants",
 			)

pkg/rpc/auth_test.go

@@ -187,12 +187,17 @@ 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,
+					SystemTargetType: &roachpb.SystemSpanConfigTarget_SystemTargetType{
+						Type: &roachpb.SystemSpanConfigTarget_SystemTargetType_SpecificTenantKeyspaceTarget{
+							SpecificTenantKeyspaceTarget: &roachpb.SystemSpanConfigTarget_TenantKeyspaceTarget{
+								TenantID: roachpb.MakeTenantID(target),
+							},
+						},
+					},
 				},
 			},
 		}
@@ -536,11 +541,30 @@ func TestTenantAuthRequest(t *testing.T) {
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
 							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							SystemTargetType: &roachpb.SystemSpanConfigTarget_SystemTargetType{
+								Type: &roachpb.SystemSpanConfigTarget_SystemTargetType_EntireKeyspaceTarget{
+									EntireKeyspaceTarget: true,
+								},
+							},
 						},
 					},
 				}),
-				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),
+							SystemTargetType: &roachpb.SystemSpanConfigTarget_SystemTargetType{
+								Type: &roachpb.SystemSpanConfigTarget_SystemTargetType_EntireKeyspaceTarget{
+									EntireKeyspaceTarget: true,
+								},
+							},
+						},
+					},
+				}),
+				expErr: `malformed source tenant field`,
 			},
 		},
 		"/cockroach.roachpb.Internal/UpdateSpanConfigs": {
@@ -651,11 +675,15 @@ func TestTenantAuthRequest(t *testing.T) {
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
 							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							SystemTargetType: &roachpb.SystemSpanConfigTarget_SystemTargetType{
+								Type: &roachpb.SystemSpanConfigTarget_SystemTargetType_EntireKeyspaceTarget{
+									EntireKeyspaceTarget: true,
+								},
+							},
 						},
 					},
 				}, false),
-				expErr: `secondary tenants must explicitly target themselves`,
+				expErr: `secondary tenants cannot target the entire keyspace`,
 			},
 			{
 				req:    makeUpdateSpanConfigsReq(makeSystemSpanConfigTarget(10, 10), true),
@@ -676,11 +704,15 @@ func TestTenantAuthRequest(t *testing.T) {
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
 							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							SystemTargetType: &roachpb.SystemSpanConfigTarget_SystemTargetType{
+								Type: &roachpb.SystemSpanConfigTarget_SystemTargetType_EntireKeyspaceTarget{
+									EntireKeyspaceTarget: true,
+								},
+							},
 						},
 					},
 				}, 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,everything-installed-on-tenants}
 //      ----
 //
 // 		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)()