spanconfig: introduce new read-only system target

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

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

@@ -183,16 +183,38 @@ message SystemSpanConfigTarget {
   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).
+  // configuration applies to. This field can only be set in conjunction with
+  // the type targeting a SpecificTenant; it must be left unset for all other
+  // system target types.
   //
   // 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"];
+
+
+  // SystemTargetType indicates the type of the SystemTarget.
+  enum SystemTargetType {
+	  // Unset indicates that the type hasn't been set; should never be the case
+	  // for well formed SystemTargets.
+    Unset = 0;
+    // SpecificTenant indicates that the system target is targeting a specific
+    // tenant.
+    SpecificTenant = 1;
+    // EntireCluster indicates that the system target is targeting the entire
+    // cluster, i.e, all ranges in the system. Only the system tenant is allowed
+    // to target the entire cluster.
+    EntireCluster = 2;
+    // EverythingTargetingTenants indicates that the system target is
+    // specifying all system span configurations installed by the source tenant
+    // that target specific tenants. This field is only meaningful in the
+    // context of fetching system span configurations, not installing them; it
+    // should not be used as a target in UpdateSystemSpanConfigsRequest.
+    EverythingTargetingTenants = 3;
+  }
+
+  // SystemTargetType represents the type of the system target.
+  SystemTargetType system_target_type = 3;
 }
 
 // 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 == roachpb.SystemSpanConfigTarget_EntireCluster {
+			return authErrorf("secondary tenants cannot target the entire cluster")
 		}
 
-		if target.SourceTenantID != *target.TargetTenantID {
+		if target.SystemTargetType == roachpb.SystemSpanConfigTarget_SpecificTenant &&
+			target.SourceTenantID != *target.TargetTenantID {
 			return authErrorf(
 				"secondary tenants cannot interact with system span configurations of other tenants",
 			)

pkg/rpc/auth_test.go

@@ -191,8 +191,9 @@ func TestTenantAuthRequest(t *testing.T) {
 		return roachpb.SpanConfigTarget{
 			Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 				SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
-					SourceTenantID: roachpb.MakeTenantID(source),
-					TargetTenantID: &targetID,
+					SourceTenantID:   roachpb.MakeTenantID(source),
+					TargetTenantID:   &targetID,
+					SystemTargetType: roachpb.SystemSpanConfigTarget_SpecificTenant,
 				},
 			},
 		}
@@ -535,12 +536,25 @@ func TestTenantAuthRequest(t *testing.T) {
 				req: makeGetSpanConfigsReq(roachpb.SpanConfigTarget{
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
-							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							SourceTenantID:   roachpb.MakeTenantID(10),
+							TargetTenantID:   nil,
+							SystemTargetType: roachpb.SystemSpanConfigTarget_EntireCluster,
 						},
 					},
 				}),
-				expErr: `secondary tenants must explicitly target themselves`,
+				expErr: `secondary tenants cannot target the entire cluster`,
+			},
+			{
+				req: makeGetSpanConfigsReq(roachpb.SpanConfigTarget{
+					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
+						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
+							SourceTenantID:   roachpb.MakeTenantID(20),
+							TargetTenantID:   nil,
+							SystemTargetType: roachpb.SystemSpanConfigTarget_EverythingTargetingTenants,
+						},
+					},
+				}),
+				expErr: `malformed source tenant field`,
 			},
 		},
 		"/cockroach.roachpb.Internal/UpdateSpanConfigs": {
@@ -650,12 +664,13 @@ func TestTenantAuthRequest(t *testing.T) {
 				req: makeUpdateSpanConfigsReq(roachpb.SpanConfigTarget{
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
-							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							SourceTenantID:   roachpb.MakeTenantID(10),
+							TargetTenantID:   nil,
+							SystemTargetType: roachpb.SystemSpanConfigTarget_EntireCluster,
 						},
 					},
 				}, false),
-				expErr: `secondary tenants must explicitly target themselves`,
+				expErr: `secondary tenants cannot target the entire cluster`,
 			},
 			{
 				req:    makeUpdateSpanConfigsReq(makeSystemSpanConfigTarget(10, 10), true),
@@ -675,12 +690,13 @@ func TestTenantAuthRequest(t *testing.T) {
 				req: makeUpdateSpanConfigsReq(roachpb.SpanConfigTarget{
 					Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{
 						SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{
-							SourceTenantID: roachpb.MakeTenantID(10),
-							TargetTenantID: nil,
+							SourceTenantID:   roachpb.MakeTenantID(10),
+							TargetTenantID:   nil,
+							SystemTargetType: roachpb.SystemSpanConfigTarget_EntireCluster,
 						},
 					},
 				}, true),
-				expErr: `secondary tenants must explicitly target themselves`,
+				expErr: `secondary tenants cannot target the entire cluster`,
 			},
 		},
 

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)()

pkg/spanconfig/spanconfigkvaccessor/testdata/system_span_configs

@@ -54,6 +54,13 @@ system-target {source=20,target=20}
 {source=1,target=10}:H
 {source=20,target=20}:I
 
+kvaccessor-get
+system-target {source=1,everything-installed-on-tenants}
+----
+{source=1,target=1}:G
+{source=1,target=10}:H
+
+
 # Delete all the system span configurations that we just added and ensure
 # they take effect.
 kvaccessor-update
@@ -71,6 +78,10 @@ system-target {source=1,target=10}
 system-target {source=20,target=20}
 ----
 
+kvaccessor-get
+system-target {source=1,everything-installed-on-tenants}
+----
+
 # Lastly, try adding multiple system targets set by the host tenant that apply
 # to distinct secondary tenants. We also add a system span configuration set by
 # one of these secondary tenant's on itself for kicks.
@@ -95,3 +106,15 @@ system-target {source=10,target=10}
 {source=1,target=20}:B
 {source=1,target=30}:C
 {source=10,target=10}:G
+
+kvaccessor-get
+system-target {source=1,everything-installed-on-tenants}
+----
+{source=1,target=10}:A
+{source=1,target=20}:B
+{source=1,target=30}:C
+
+kvaccessor-get
+system-target {source=10,everything-installed-on-tenants}
+----
+{source=10,target=10}:G

pkg/spanconfig/spanconfigkvaccessor/validation_test.go

@@ -25,10 +25,7 @@ import (
 func TestValidateUpdateArgs(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 
-	clusterTarget := spanconfig.MakeTargetFromSystemTarget(spanconfig.SystemTarget{
-		SourceTenantID: roachpb.SystemTenantID,
-		TargetTenantID: nil,
-	})
+	clusterTarget := spanconfig.MakeTargetFromSystemTarget(spanconfig.MakeClusterTarget())
 
 	makeTenantTarget := func(id uint64) spanconfig.Target {
 		target, err := spanconfig.MakeTenantTarget(roachpb.MakeTenantID(id), roachpb.MakeTenantID(id))
@@ -183,7 +180,38 @@ func TestValidateUpdateArgs(t *testing.T) {
 			},
 			expErr: "",
 		},
+		{
+			// Read only targets are not valid delete args.
+			toDelete: []spanconfig.Target{
+				spanconfig.MakeTargetFromSystemTarget(
+					spanconfig.MakeEverythingTargetingTenantsTarget(roachpb.SystemTenantID),
+				),
+			},
+			expErr: "cannot use read only system target .* as an update argument",
+		},
+		{
+			// Read only targets are not valid upsert args.
+			toUpsert: []spanconfig.Record{
+				{
+					Target: spanconfig.MakeTargetFromSystemTarget(
+						spanconfig.MakeEverythingTargetingTenantsTarget(roachpb.SystemTenantID),
+					),
+				},
+			},
+			expErr: "cannot use read only system target .* as an update argument",
+		},
+		{
+			// Read only target validation also applies when the source is a secondary
+			// tenant.
+			toDelete: []spanconfig.Target{
+				spanconfig.MakeTargetFromSystemTarget(
+					spanconfig.MakeEverythingTargetingTenantsTarget(roachpb.MakeTenantID(10)),
+				),
+			},
+			expErr: "cannot use read only system target .* as an update argument",
+		},
 	} {
-		require.True(t, testutils.IsError(validateUpdateArgs(tc.toDelete, tc.toUpsert), tc.expErr))
+		err := validateUpdateArgs(tc.toDelete, tc.toUpsert)
+		require.True(t, testutils.IsError(err, tc.expErr), "exp %s; got %s", tc.expErr, err)
 	}
 }