diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/BUILD.bazel b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/BUILD.bazel index 59a04956b03c..9923ad6f5f49 100644 --- a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/BUILD.bazel +++ b/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", diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go index 48f211dc7519..b728bc31b5cc 100644 --- a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go +++ b/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 { diff --git a/pkg/migration/migrations/migrate_span_configs_test.go b/pkg/migration/migrations/migrate_span_configs_test.go index a62a5c8a1a71..956052920fa7 100644 --- a/pkg/migration/migrations/migrate_span_configs_test.go +++ b/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") diff --git a/pkg/migration/migrations/seed_tenant_span_configs.go b/pkg/migration/migrations/seed_tenant_span_configs.go index c0b903c7cd9e..3010ac30bacf 100644 --- a/pkg/migration/migrations/seed_tenant_span_configs.go +++ b/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. diff --git a/pkg/roachpb/span_config.go b/pkg/roachpb/span_config.go index ad3ebc198a27..9c3cc253da90 100644 --- a/pkg/roachpb/span_config.go +++ b/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{}, + }, + } +} diff --git a/pkg/roachpb/span_config.proto b/pkg/roachpb/span_config.proto index b326da31cc2f..5f15ff040904 100644 --- a/pkg/roachpb/span_config.proto +++ b/pkg/roachpb/span_config.proto @@ -175,6 +175,19 @@ 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; @@ -182,17 +195,46 @@ message SystemSpanConfigTarget { // 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. diff --git a/pkg/rpc/auth_tenant.go b/pkg/rpc/auth_tenant.go index 819f20841a3e..201bfaf95fd8 100644 --- a/pkg/rpc/auth_tenant.go +++ b/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", ) diff --git a/pkg/rpc/auth_test.go b/pkg/rpc/auth_test.go index 613e741d41b2..9bb313bdaaa9 100644 --- a/pkg/rpc/auth_test.go +++ b/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`, }, }, diff --git a/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go b/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go index 581499c9f823..a1f725bc8bb2 100644 --- a/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go +++ b/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 } diff --git a/pkg/spanconfig/spanconfigkvaccessor/kvaccessor_test.go b/pkg/spanconfig/spanconfigkvaccessor/kvaccessor_test.go index 55d8dc610ad4..83721614f8d4 100644 --- a/pkg/spanconfig/spanconfigkvaccessor/kvaccessor_test.go +++ b/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)() diff --git a/pkg/spanconfig/spanconfigkvaccessor/testdata/system_span_configs b/pkg/spanconfig/spanconfigkvaccessor/testdata/system_span_configs index 42a6e592e75f..4e18e76149f2 100644 --- a/pkg/spanconfig/spanconfigkvaccessor/testdata/system_span_configs +++ b/pkg/spanconfig/spanconfigkvaccessor/testdata/system_span_configs @@ -2,7 +2,7 @@ # Test with an empty slate. kvaccessor-get -system-target {cluster} +system-target {entire-keyspace} system-target {source=1,target=1} system-target {source=1,target=10} system-target {source=20,target=20} @@ -10,13 +10,13 @@ system-target {source=20,target=20} # Try deleting a system span configuration that doesn't exist. kvaccessor-update -delete {cluster} +delete {entire-keyspace} ---- err: expected to delete 1 row(s), deleted 0 # Basic tests that set all possible combinations of system targets. kvaccessor-update -upsert {cluster}:A +upsert {entire-keyspace}:A upsert {source=1,target=1}:B upsert {source=1,target=10}:C upsert {source=20,target=20}:D @@ -24,19 +24,19 @@ upsert {source=20,target=20}:D ok kvaccessor-get -system-target {cluster} +system-target {entire-keyspace} system-target {source=1,target=1} system-target {source=1,target=10} system-target {source=20,target=20} ---- -{cluster}:A +{entire-keyspace}:A {source=1,target=1}:B {source=1,target=10}:C {source=20,target=20}:D # Update some of the span configurations that we added and ensure that works. kvaccessor-update -upsert {cluster}:F +upsert {entire-keyspace}:F upsert {source=1,target=1}:G upsert {source=1,target=10}:H upsert {source=20,target=20}:I @@ -44,20 +44,27 @@ upsert {source=20,target=20}:I ok kvaccessor-get -system-target {cluster} +system-target {entire-keyspace} system-target {source=1,target=1} system-target {source=1,target=10} system-target {source=20,target=20} ---- -{cluster}:F +{entire-keyspace}:F {source=1,target=1}:G {source=1,target=10}:H {source=20,target=20}:I +kvaccessor-get +system-target {source=1,all-tenant-keyspace-targets-set} +---- +{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 -delete {cluster} +delete {entire-keyspace} delete {source=1,target=1} delete {source=1,target=10} delete {source=20,target=20} @@ -65,17 +72,21 @@ delete {source=20,target=20} ok kvaccessor-get -system-target {cluster} +system-target {entire-keyspace} system-target {source=1,target=1} system-target {source=1,target=10} system-target {source=20,target=20} ---- +kvaccessor-get +system-target {source=1,all-tenant-keyspace-targets-set} +---- + # 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. kvaccessor-update -upsert {cluster}:Z +upsert {entire-keyspace}:Z upsert {source=1,target=10}:A upsert {source=1,target=20}:B upsert {source=1,target=30}:C @@ -84,14 +95,26 @@ upsert {source=10,target=10}:G ok kvaccessor-get -system-target {cluster} +system-target {entire-keyspace} system-target {source=1,target=10} system-target {source=1,target=20} system-target {source=1,target=30} system-target {source=10,target=10} ---- -{cluster}:Z +{entire-keyspace}:Z {source=1,target=10}:A {source=1,target=20}:B {source=1,target=30}:C {source=10,target=10}:G + +kvaccessor-get +system-target {source=1,all-tenant-keyspace-targets-set} +---- +{source=1,target=10}:A +{source=1,target=20}:B +{source=1,target=30}:C + +kvaccessor-get +system-target {source=10,all-tenant-keyspace-targets-set} +---- +{source=10,target=10}:G diff --git a/pkg/spanconfig/spanconfigkvaccessor/validation_test.go b/pkg/spanconfig/spanconfigkvaccessor/validation_test.go index a8215bbdda53..040f899aa404 100644 --- a/pkg/spanconfig/spanconfigkvaccessor/validation_test.go +++ b/pkg/spanconfig/spanconfigkvaccessor/validation_test.go @@ -25,13 +25,12 @@ import ( func TestValidateUpdateArgs(t *testing.T) { defer leaktest.AfterTest(t)() - clusterTarget := spanconfig.MakeTargetFromSystemTarget(spanconfig.SystemTarget{ - SourceTenantID: roachpb.SystemTenantID, - TargetTenantID: nil, - }) + entireKeyspaceTarget := spanconfig.MakeTargetFromSystemTarget( + spanconfig.MakeEntireKeyspaceTarget(), + ) makeTenantTarget := func(id uint64) spanconfig.Target { - target, err := spanconfig.MakeTenantTarget(roachpb.MakeTenantID(id), roachpb.MakeTenantID(id)) + target, err := spanconfig.MakeTenantKeyspaceTarget(roachpb.MakeTenantID(id), roachpb.MakeTenantID(id)) require.NoError(t, err) return spanconfig.MakeTargetFromSystemTarget(target) } @@ -145,9 +144,9 @@ func TestValidateUpdateArgs(t *testing.T) { // Duplicate in toDelete with some span targets. toDelete: []spanconfig.Target{ spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}), - clusterTarget, + entireKeyspaceTarget, spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("e"), EndKey: roachpb.Key("f")}), - clusterTarget, + entireKeyspaceTarget, spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("g"), EndKey: roachpb.Key("h")}), }, expErr: "duplicate system targets .* in the same list", @@ -156,9 +155,9 @@ func TestValidateUpdateArgs(t *testing.T) { // Duplicate in toDelete with some span targets. toDelete: []spanconfig.Target{ spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}), - clusterTarget, + entireKeyspaceTarget, spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("e"), EndKey: roachpb.Key("f")}), - clusterTarget, + entireKeyspaceTarget, spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("g"), EndKey: roachpb.Key("h")}), }, expErr: "duplicate system targets .* in the same list", @@ -170,7 +169,7 @@ func TestValidateUpdateArgs(t *testing.T) { spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}), makeTenantTarget(20), spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("e"), EndKey: roachpb.Key("f")}), - clusterTarget, + entireKeyspaceTarget, spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("g"), EndKey: roachpb.Key("h")}), }, toUpsert: []spanconfig.Record{ @@ -183,7 +182,38 @@ func TestValidateUpdateArgs(t *testing.T) { }, expErr: "", }, + { + // Read only targets are not valid delete args. + toDelete: []spanconfig.Target{ + spanconfig.MakeTargetFromSystemTarget( + spanconfig.MakeAllTenantKeyspaceTargetsSet(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.MakeAllTenantKeyspaceTargetsSet(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.MakeAllTenantKeyspaceTargetsSet(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) } } diff --git a/pkg/spanconfig/spanconfigtestutils/utils.go b/pkg/spanconfig/spanconfigtestutils/utils.go index 5ae42e7a398e..a8f485baf75d 100644 --- a/pkg/spanconfig/spanconfigtestutils/utils.go +++ b/pkg/spanconfig/spanconfigtestutils/utils.go @@ -33,8 +33,10 @@ import ( var spanRe = regexp.MustCompile(`^\[(\w+),\s??(\w+)\)$`) // systemTargetRe matches strings of the form -// "{source=(|system),target=(|system)}". -var systemTargetRe = regexp.MustCompile(`^{(cluster)|(source=(\d*),\s??target=(\d*))}$`) +// "{entire-keyspace|source=,(target=|all-tenant-keyspace-targets-set)}". +var systemTargetRe = regexp.MustCompile( + `^{(entire-keyspace)|(source=(\d*),\s??((target=(\d*))|all-tenant-keyspace-targets-set))}$`, +) // configRe matches a single word. It's a shorthand for declaring a unique // config. @@ -63,15 +65,18 @@ func parseSystemTarget(t *testing.T, systemTarget string) spanconfig.SystemTarge } matches := systemTargetRe.FindStringSubmatch(systemTarget) - if matches[1] == "cluster" { - return spanconfig.MakeClusterTarget() + if matches[1] == "entire-keyspace" { + return spanconfig.MakeEntireKeyspaceTarget() } sourceID, err := strconv.Atoi(matches[3]) require.NoError(t, err) - targetID, err := strconv.Atoi(matches[4]) + if matches[4] == "all-tenant-keyspace-targets-set" { + return spanconfig.MakeAllTenantKeyspaceTargetsSet(roachpb.MakeTenantID(uint64(sourceID))) + } + targetID, err := strconv.Atoi(matches[6]) require.NoError(t, err) - target, err := spanconfig.MakeTenantTarget( + target, err := spanconfig.MakeTenantKeyspaceTarget( roachpb.MakeTenantID(uint64(sourceID)), roachpb.MakeTenantID(uint64(targetID)), ) require.NoError(t, err) diff --git a/pkg/spanconfig/systemtarget.go b/pkg/spanconfig/systemtarget.go index be77cfcd808e..e8d8bb0df364 100644 --- a/pkg/spanconfig/systemtarget.go +++ b/pkg/spanconfig/systemtarget.go @@ -22,159 +22,265 @@ import ( // SystemTarget specifies the target of a system span configuration. type SystemTarget struct { - // SourceTenantID is the ID of the tenant that specified the system span + // sourceTenantID is the ID of the tenant that specified the system span // configuration. - // SourceTenantID is the ID of the tenant that specified the system span - // configuration. - SourceTenantID roachpb.TenantID + sourceTenantID roachpb.TenantID - // TargetTenantID is the ID of the tenant that the associated system span - // configuration applies to. - // - // If the host tenant is the source and the TargetTenantID is unspecified then - // the associated system span configuration applies over all ranges in the - // system (including those belonging to secondary tenants). + // targetTenantID is the ID of the tenant whose kesypace the associated system + // span configuration applies. This field can only be set in conjunction with + // the type being SystemTargetTypeSpecificTenantKeyspace; it must be left + // unset for all other system target types. // - // Secondary tenants are only allowed to target themselves. The host tenant - // may use this field to target a specific secondary tenant. We validate this - // when constructing new system targets. - TargetTenantID *roachpb.TenantID + // Secondary tenants are only allowed to target their own keyspace. The host + // tenant may use this field to target a specific secondary tenant. + targetTenantID *roachpb.TenantID + + // systemTargetType indicates the type of the system target. targetTenantID + // can only be set if the system target is specific. + systemTargetType systemTargetType } -// MakeTenantTarget constructs, validates, and returns a new SystemTarget that -// targets the physical keyspace of the targetTenantID. -func MakeTenantTarget( +// systemTargetType indicates the type of SystemTarget. +type systemTargetType int + +const ( + _ systemTargetType = iota + // SystemTargetTypeSpecificTenantKeyspace indicates that the system target is + // targeting a specific tenant's keyspace. + SystemTargetTypeSpecificTenantKeyspace + // SystemTargetTypeEntireKeyspace indicates that the system target is + // targeting the entire keyspace. Only the host tenant is allowed to do so. + SystemTargetTypeEntireKeyspace + // SystemTargetTypeAllTenantKeyspaceTargetsSet represents a system target that + // encompasses all system targets that have been set by the source tenant over + // specific tenant's keyspace. + // + // This is a read-only system target type as it may translate to more than one + // system targets that may have been persisted. This target type is useful in + // fetching all system span configurations a tenant may have set on tenant + // keyspaces without knowing the tenant ID of all other tenants in the system. + // This is only ever significant for the host tenant as it can set system span + // configurations that target other tenant's keyspaces. + SystemTargetTypeAllTenantKeyspaceTargetsSet +) + +// MakeTenantKeyspaceTarget constructs, validates, and returns a new +// SystemTarget that targets the keyspace of the target tenant. +func MakeTenantKeyspaceTarget( sourceTenantID roachpb.TenantID, targetTenantID roachpb.TenantID, ) (SystemTarget, error) { t := SystemTarget{ - SourceTenantID: sourceTenantID, - TargetTenantID: &targetTenantID, + sourceTenantID: sourceTenantID, + targetTenantID: &targetTenantID, + systemTargetType: SystemTargetTypeSpecificTenantKeyspace, } return t, t.validate() } -// MakeSystemTargetFromProto constructs a SystemTarget from a +// makeSystemTargetFromProto constructs a SystemTarget from a // roachpb.SystemSpanConfigTarget and validates it. -func MakeSystemTargetFromProto(proto *roachpb.SystemSpanConfigTarget) (SystemTarget, error) { - if proto.SourceTenantID == roachpb.SystemTenantID && proto.TargetTenantID == nil { - return MakeClusterTarget(), nil - } else if proto.TargetTenantID == nil { - return SystemTarget{}, - errors.Newf("secondary tenant %s cannot target the entire cluster", proto.SourceTenantID) +func makeSystemTargetFromProto(proto *roachpb.SystemSpanConfigTarget) (SystemTarget, error) { + var t SystemTarget + switch { + case proto.IsSpecificTenantKeyspaceTarget(): + t = SystemTarget{ + sourceTenantID: proto.SourceTenantID, + targetTenantID: &proto.Type.GetSpecificTenantKeyspace().TenantID, + systemTargetType: SystemTargetTypeSpecificTenantKeyspace, + } + case proto.IsEntireKeyspaceTarget(): + t = SystemTarget{ + sourceTenantID: proto.SourceTenantID, + targetTenantID: nil, + systemTargetType: SystemTargetTypeEntireKeyspace, + } + case proto.IsAllTenantKeyspaceTargetsSetTarget(): + t = SystemTarget{ + sourceTenantID: proto.SourceTenantID, + targetTenantID: nil, + systemTargetType: SystemTargetTypeAllTenantKeyspaceTargetsSet, + } + default: + return SystemTarget{}, errors.AssertionFailedf("unknown system target type") } + return t, t.validate() +} - return MakeTenantTarget(proto.SourceTenantID, *proto.TargetTenantID) +func (st SystemTarget) toProto() *roachpb.SystemSpanConfigTarget { + var systemTargetType *roachpb.SystemSpanConfigTarget_Type + switch st.systemTargetType { + case SystemTargetTypeEntireKeyspace: + systemTargetType = roachpb.NewEntireKeyspaceTargetType() + case SystemTargetTypeAllTenantKeyspaceTargetsSet: + systemTargetType = roachpb.NewAllTenantKeyspaceTargetsSetTargetType() + case SystemTargetTypeSpecificTenantKeyspace: + systemTargetType = roachpb.NewSpecificTenantKeyspaceTargetType(*st.targetTenantID) + default: + panic("unknown system target type") + } + return &roachpb.SystemSpanConfigTarget{ + SourceTenantID: st.sourceTenantID, + Type: systemTargetType, + } } -// MakeClusterTarget returns a new system target that targets the entire cluster. -// Only the host tenant is allowed to target the entire cluster. -func MakeClusterTarget() SystemTarget { +// MakeEntireKeyspaceTarget returns a new system target that targets the entire +// keyspace. Only the host tenant is allowed to target the entire keyspace. +func MakeEntireKeyspaceTarget() SystemTarget { return SystemTarget{ - SourceTenantID: roachpb.SystemTenantID, + sourceTenantID: roachpb.SystemTenantID, + systemTargetType: SystemTargetTypeEntireKeyspace, } } -// targetsEntireCluster returns true if the target applies to all ranges in the +// MakeAllTenantKeyspaceTargetsSet returns a new SystemTarget that +// represents all system span configurations installed by the given tenant ID +// on specific tenant's keyspace (including itself and other tenants). +func MakeAllTenantKeyspaceTargetsSet(sourceID roachpb.TenantID) SystemTarget { + return SystemTarget{ + sourceTenantID: sourceID, + systemTargetType: SystemTargetTypeAllTenantKeyspaceTargetsSet, + } +} + +// targetsEntireKeyspace returns true if the target applies to all ranges in the // system (including those belonging to secondary tenants). -func (st SystemTarget) targetsEntireCluster() bool { - return st.SourceTenantID == roachpb.SystemTenantID && st.TargetTenantID == nil +func (st SystemTarget) targetsEntireKeyspace() bool { + return st.systemTargetType == SystemTargetTypeEntireKeyspace +} + +// IsReadOnly returns true if the system target is read-only. Read only targets +// should not be persisted. +func (st SystemTarget) IsReadOnly() bool { + return st.systemTargetType == SystemTargetTypeAllTenantKeyspaceTargetsSet } // encode returns an encoded span associated with the receiver which is suitable -// for persistence in system.span_configurations. +// for interaction with system.span_configurations table. func (st SystemTarget) encode() roachpb.Span { var k roachpb.Key - if st.SourceTenantID == roachpb.SystemTenantID && - st.TargetTenantID == nil { + switch st.systemTargetType { + case SystemTargetTypeEntireKeyspace: k = keys.SystemSpanConfigEntireKeyspace - } else if st.SourceTenantID == roachpb.SystemTenantID { - k = encoding.EncodeUvarintAscending( - keys.SystemSpanConfigHostOnTenantKeyspace, st.TargetTenantID.ToUint64(), - ) - } else { - k = encoding.EncodeUvarintAscending( - keys.SystemSpanConfigSecondaryTenantOnEntireKeyspace, st.SourceTenantID.ToUint64(), - ) + case SystemTargetTypeSpecificTenantKeyspace: + if st.sourceTenantID == roachpb.SystemTenantID { + k = encoding.EncodeUvarintAscending( + keys.SystemSpanConfigHostOnTenantKeyspace, st.targetTenantID.ToUint64(), + ) + } else { + k = encoding.EncodeUvarintAscending( + keys.SystemSpanConfigSecondaryTenantOnEntireKeyspace, st.sourceTenantID.ToUint64(), + ) + } + case SystemTargetTypeAllTenantKeyspaceTargetsSet: + if st.sourceTenantID == roachpb.SystemTenantID { + k = keys.SystemSpanConfigHostOnTenantKeyspace + } else { + k = encoding.EncodeUvarintAscending( + keys.SystemSpanConfigSecondaryTenantOnEntireKeyspace, st.sourceTenantID.ToUint64(), + ) + } } return roachpb.Span{Key: k, EndKey: k.PrefixEnd()} } // validate ensures that the receiver is well-formed. func (st SystemTarget) validate() error { - if st.SourceTenantID == roachpb.SystemTenantID { - // The system tenant can target itself, other secondary tenants, and the - // entire cluster (including secondary tenant ranges). - return nil - } - if st.TargetTenantID == nil { - return errors.AssertionFailedf( - "secondary tenant %s cannot have unspecified target tenant ID", st.SourceTenantID, - ) - } - if st.SourceTenantID != *st.TargetTenantID { - return errors.AssertionFailedf( - "secondary tenant %s cannot target another tenant with ID %s", - st.SourceTenantID, - st.TargetTenantID, - ) + switch st.systemTargetType { + case SystemTargetTypeAllTenantKeyspaceTargetsSet: + if st.targetTenantID != nil { + return errors.AssertionFailedf( + "targetTenantID must be unset when targeting everything installed on tenants", + ) + } + case SystemTargetTypeEntireKeyspace: + if st.sourceTenantID != roachpb.SystemTenantID { + return errors.AssertionFailedf("only the host tenant is allowed to target the entire keyspace") + } + if st.targetTenantID != nil { + return errors.AssertionFailedf("malformed system target for entire keyspace; targetTenantID set") + } + case SystemTargetTypeSpecificTenantKeyspace: + if st.targetTenantID == nil { + return errors.AssertionFailedf( + "malformed system target for specific tenant keyspace; targetTenantID unset", + ) + } + if st.sourceTenantID != roachpb.SystemTenantID && st.sourceTenantID != *st.targetTenantID { + return errors.AssertionFailedf( + "secondary tenant %s cannot target another tenant with ID %s", + st.sourceTenantID, + st.targetTenantID, + ) + } + default: + return errors.AssertionFailedf("invalid system target type") } return nil } // isEmpty returns true if the receiver is empty. func (st SystemTarget) isEmpty() bool { - return st.SourceTenantID.Equal(roachpb.TenantID{}) && st.TargetTenantID.Equal(nil) + return st.sourceTenantID.Equal(roachpb.TenantID{}) && st.targetTenantID.Equal(nil) } // less returns true if the receiver is considered less than the supplied // target. The semantics are defined as follows: -// - host installed targets that target the entire cluster come first. -// - host installed targets that target a tenant come next (ordered by target -// tenant ID). -// - secondary tenant installed targets come next, ordered by secondary tenant -// ID. +// - read only targets come first, ordered by tenant ID. +// - targets that target the entire keyspace come next. +// - targets that target a specific tenant's keyspace come last, sorted by +// source tenant ID; target tenant ID is used as a tiebreaker if two targets +// have the same source. func (st SystemTarget) less(ot SystemTarget) bool { - if st.SourceTenantID == roachpb.SystemTenantID && - ot.SourceTenantID == roachpb.SystemTenantID { - if st.targetsEntireCluster() { - return true - } else if ot.targetsEntireCluster() { - return false - } + if st.IsReadOnly() && ot.IsReadOnly() { + return st.sourceTenantID.ToUint64() < ot.sourceTenantID.ToUint64() + } - return st.TargetTenantID.ToUint64() < ot.TargetTenantID.ToUint64() + if st.IsReadOnly() { + return true + } else if ot.IsReadOnly() { + return false } - if st.SourceTenantID == roachpb.SystemTenantID { + if st.targetsEntireKeyspace() { return true - } else if ot.SourceTenantID == roachpb.SystemTenantID { + } else if ot.targetsEntireKeyspace() { return false } - return st.SourceTenantID.ToUint64() < ot.SourceTenantID.ToUint64() + if st.sourceTenantID.ToUint64() == ot.sourceTenantID.ToUint64() { + return st.targetTenantID.ToUint64() < ot.targetTenantID.ToUint64() + } + + return st.sourceTenantID.ToUint64() < ot.sourceTenantID.ToUint64() } // equal returns true iff the receiver is equal to the supplied system target. func (st SystemTarget) equal(ot SystemTarget) bool { - return st.SourceTenantID.Equal(ot.SourceTenantID) && st.TargetTenantID.Equal(ot.TargetTenantID) + return st.sourceTenantID.Equal(ot.sourceTenantID) && st.targetTenantID.Equal(ot.targetTenantID) } // String returns a pretty printed version of a system target. func (st SystemTarget) String() string { - if st.targetsEntireCluster() { - return "{cluster}" + switch st.systemTargetType { + case SystemTargetTypeEntireKeyspace: + return "{entire-keyspace}" + case SystemTargetTypeAllTenantKeyspaceTargetsSet: + return fmt.Sprintf("{source=%d, all-tenant-keyspace-targets-set}", st.sourceTenantID) + case SystemTargetTypeSpecificTenantKeyspace: + return fmt.Sprintf( + "{source=%d,target=%d}", + st.sourceTenantID.ToUint64(), + st.targetTenantID.ToUint64(), + ) + default: + panic("unreachable") } - return fmt.Sprintf( - "{source=%d,target=%d}", - st.SourceTenantID.ToUint64(), - st.TargetTenantID.ToUint64(), - ) } // decodeSystemTarget converts the given span into a SystemTarget. An error is -// returned if the supplied span does not conform to a system target's -// encoding. +// returned if the supplied span does not conform to a system target's encoding. func decodeSystemTarget(span roachpb.Span) (SystemTarget, error) { // Validate the end key is well-formed. if !span.EndKey.Equal(span.Key.PrefixEnd()) { @@ -182,7 +288,7 @@ func decodeSystemTarget(span roachpb.Span) (SystemTarget, error) { } switch { case bytes.Equal(span.Key, keys.SystemSpanConfigEntireKeyspace): - return MakeClusterTarget(), nil + return MakeEntireKeyspaceTarget(), nil case bytes.HasPrefix(span.Key, keys.SystemSpanConfigHostOnTenantKeyspace): // System span config was applied by the host tenant over a secondary // tenant's entire keyspace. @@ -192,7 +298,7 @@ func decodeSystemTarget(span roachpb.Span) (SystemTarget, error) { return SystemTarget{}, err } tenID := roachpb.MakeTenantID(tenIDRaw) - return MakeTenantTarget(roachpb.SystemTenantID, tenID) + return MakeTenantKeyspaceTarget(roachpb.SystemTenantID, tenID) case bytes.HasPrefix(span.Key, keys.SystemSpanConfigSecondaryTenantOnEntireKeyspace): // System span config was applied by a secondary tenant over its entire // keyspace. @@ -202,7 +308,7 @@ func decodeSystemTarget(span roachpb.Span) (SystemTarget, error) { return SystemTarget{}, err } tenID := roachpb.MakeTenantID(tenIDRaw) - return MakeTenantTarget(tenID, tenID) + return MakeTenantKeyspaceTarget(tenID, tenID) default: return SystemTarget{}, errors.AssertionFailedf("span %s did not conform to SystemTarget encoding", span) diff --git a/pkg/spanconfig/target.go b/pkg/spanconfig/target.go index cd69dd604f6d..d0c1703826d0 100644 --- a/pkg/spanconfig/target.go +++ b/pkg/spanconfig/target.go @@ -11,6 +11,7 @@ package spanconfig import ( + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/errors" ) @@ -26,9 +27,9 @@ type Target struct { func MakeTarget(t roachpb.SpanConfigTarget) (Target, error) { switch t.Union.(type) { case *roachpb.SpanConfigTarget_Span: - return MakeTargetFromSpan(*t.GetSpan()), nil + return MakeSpanTargetFromProto(t) case *roachpb.SpanConfigTarget_SystemSpanConfigTarget: - systemTarget, err := MakeSystemTargetFromProto(t.GetSystemSpanConfigTarget()) + systemTarget, err := makeSystemTargetFromProto(t.GetSystemSpanConfigTarget()) if err != nil { return Target{}, err } @@ -38,8 +39,28 @@ func MakeTarget(t roachpb.SpanConfigTarget) (Target, error) { } } -// MakeTargetFromSpan constructs and returns a span target. +// MakeSpanTargetFromProto returns a new Target backed by an underlying span. +// An error is returned if the proto does not contain a span or if the span +// overlaps with the reserved system span config keyspace. +func MakeSpanTargetFromProto(spanTarget roachpb.SpanConfigTarget) (Target, error) { + if spanTarget.GetSpan() == nil { + return Target{}, errors.AssertionFailedf("span config target did not contain a span") + } + if keys.SystemSpanConfigSpan.Overlaps(*spanTarget.GetSpan()) { + return Target{}, errors.AssertionFailedf( + "cannot target spans in reserved system span config keyspace", + ) + } + return MakeTargetFromSpan(*spanTarget.GetSpan()), nil +} + +// MakeTargetFromSpan constructs and returns a span target. Callers are not +// allowed to target the reserved system span config keyspace (or part of it) +// directly; system targets should be used instead. func MakeTargetFromSpan(span roachpb.Span) Target { + if keys.SystemSpanConfigSpan.Overlaps(span) { + panic("cannot target spans in reserved system span config keyspace") + } return Target{span: span} } @@ -76,8 +97,8 @@ func (t Target) GetSystemTarget() SystemTarget { return t.systemTarget } -// Encode returns an encoded span suitable for persistence in -// system.span_configurations. +// Encode returns an encoded span suitable for interaction with the +// system.span_configurations table. func (t Target) Encode() roachpb.Span { switch { case t.IsSpanTarget(): @@ -155,10 +176,7 @@ func (t Target) ToProto() roachpb.SpanConfigTarget { case t.IsSystemTarget(): return roachpb.SpanConfigTarget{ Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{ - SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{ - SourceTenantID: t.GetSystemTarget().SourceTenantID, - TargetTenantID: t.GetSystemTarget().TargetTenantID, - }, + SystemSpanConfigTarget: t.GetSystemTarget().toProto(), }, } default: @@ -248,3 +266,15 @@ func TargetsFromProtos(protoTargets []roachpb.SpanConfigTarget) ([]Target, error } return targets, nil } + +// TestingEntireSpanConfigurationStateTargets returns a list of targets which +// can be used to read the entire span configuration state. This includes all +// span configurations installed by all tenants and all system span +// configurations, including those installed by secondary tenants. +func TestingEntireSpanConfigurationStateTargets() []Target { + return Targets{ + Target{ + span: keys.EverythingSpan, + }, + } +} diff --git a/pkg/spanconfig/target_test.go b/pkg/spanconfig/target_test.go index c31e8c25ff00..7d5a63598e71 100644 --- a/pkg/spanconfig/target_test.go +++ b/pkg/spanconfig/target_test.go @@ -26,13 +26,13 @@ import ( func TestEncodeDecodeSystemTarget(t *testing.T) { for _, testTarget := range []SystemTarget{ // Tenant targeting its logical cluster. - makeTenantTargetOrFatal(t, roachpb.MakeTenantID(10), roachpb.MakeTenantID(10)), + makeTenantKeyspaceTargetOrFatal(t, roachpb.MakeTenantID(10), roachpb.MakeTenantID(10)), // System tenant targeting its logical cluster. - makeTenantTargetOrFatal(t, roachpb.SystemTenantID, roachpb.SystemTenantID), + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.SystemTenantID), // System tenant targeting a secondary tenant. - makeTenantTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(10)), - // System tenant targeting the entire cluster. - MakeClusterTarget(), + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(10)), + // System tenant targeting the entire keyspace. + MakeEntireKeyspaceTarget(), } { systemTarget, err := decodeSystemTarget(testTarget.encode()) require.NoError(t, err) @@ -45,6 +45,37 @@ func TestEncodeDecodeSystemTarget(t *testing.T) { } } +// TestTargetToFromProto ensures that converting system targets to protos and +// from protos is roundtripable. +func TestTargetToFromProto(t *testing.T) { + for _, testSystemTarget := range []SystemTarget{ + // Tenant targeting its logical cluster. + makeTenantKeyspaceTargetOrFatal(t, roachpb.MakeTenantID(10), roachpb.MakeTenantID(10)), + // System tenant targeting its logical cluster. + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.SystemTenantID), + // System tenant targeting a secondary tenant. + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(10)), + // System tenant targeting the entire keyspace. + MakeEntireKeyspaceTarget(), + // System tenant's read-only target to fetch all system span configurations + // it has set over secondary tenant keyspaces. + MakeAllTenantKeyspaceTargetsSet(roachpb.SystemTenantID), + // A secondary tenant's read-only target to fetch all system span + // configurations it has set over secondary tenant keyspaces. + MakeAllTenantKeyspaceTargetsSet(roachpb.MakeTenantID(10)), + } { + systemTarget, err := makeSystemTargetFromProto(testSystemTarget.toProto()) + require.NoError(t, err) + require.Equal(t, testSystemTarget, systemTarget) + + // For good measure, let's also test at the level of a spanconfig.Target. + testTarget := MakeTargetFromSystemTarget(testSystemTarget) + target, err := MakeTarget(testTarget.ToProto()) + require.NoError(t, err) + require.Equal(t, target, testTarget) + } +} + // TestDecodeInvalidSpanAsSystemTarget ensures that decoding an invalid span // as a system target fails. func TestDecodeInvalidSpanAsSystemTarget(t *testing.T) { @@ -87,45 +118,128 @@ func TestDecodeInvalidSpanAsSystemTarget(t *testing.T) { // TestSystemTargetValidation ensures target.validate() works as expected. func TestSystemTargetValidation(t *testing.T) { + tenant10 := roachpb.MakeTenantID(10) + tenant20 := roachpb.MakeTenantID(20) for _, tc := range []struct { sourceTenantID roachpb.TenantID - targetTenantID roachpb.TenantID + targetTenantID *roachpb.TenantID + targetType systemTargetType expErr string }{ { // Secondary tenants cannot target the system tenant. - sourceTenantID: roachpb.MakeTenantID(10), - targetTenantID: roachpb.SystemTenantID, - expErr: "secondary tenant 10 cannot target another tenant with ID", + sourceTenantID: tenant10, + targetTenantID: &roachpb.SystemTenantID, + targetType: SystemTargetTypeSpecificTenantKeyspace, + expErr: "secondary tenant 10 cannot target another tenant with ID system", }, { // Secondary tenants cannot target other secondary tenants. - sourceTenantID: roachpb.MakeTenantID(10), - targetTenantID: roachpb.MakeTenantID(20), - expErr: "secondary tenant 10 cannot target another tenant with ID", + sourceTenantID: tenant10, + targetTenantID: &tenant20, + targetType: SystemTargetTypeSpecificTenantKeyspace, + expErr: "secondary tenant 10 cannot target another tenant with ID 20", + }, + { + // Secondary tenants cannot target the entire keyspace. + sourceTenantID: tenant10, + targetTenantID: nil, + targetType: SystemTargetTypeEntireKeyspace, + expErr: "only the host tenant is allowed to target the entire keyspace", + }, + { + // Ensure secondary tenants can't target the entire keyspace even if they + // set targetTenantID to themselves. + sourceTenantID: tenant10, + targetTenantID: &tenant10, + targetType: SystemTargetTypeEntireKeyspace, + expErr: "only the host tenant is allowed to target the entire keyspace", + }, + { + // System tenant can't set both targetTenantID and target everything + // installed on tenants. + sourceTenantID: roachpb.SystemTenantID, + targetTenantID: &tenant10, + targetType: SystemTargetTypeAllTenantKeyspaceTargetsSet, + expErr: "targetTenantID must be unset when targeting everything installed", + }, + { + // System tenant must fill in a targetTenantID when targeting a specific + // tenant. + sourceTenantID: roachpb.SystemTenantID, + targetTenantID: nil, + targetType: SystemTargetTypeSpecificTenantKeyspace, + expErr: "malformed system target for specific tenant keyspace; targetTenantID unset", + }, + { + // System tenant can't set both targetTenantID and target the entire + // keyspace. + sourceTenantID: roachpb.SystemTenantID, + targetTenantID: &tenant10, + targetType: SystemTargetTypeEntireKeyspace, + expErr: "malformed system target for entire keyspace; targetTenantID set", + }, + { + // secondary tenant can't set both targetTenantID and target everything + // installed on tenants. + sourceTenantID: tenant10, + targetTenantID: &tenant10, + targetType: SystemTargetTypeAllTenantKeyspaceTargetsSet, + expErr: "targetTenantID must be unset when targeting everything installed", }, // Test some valid targets. { // System tenant targeting secondary tenant is allowed. sourceTenantID: roachpb.SystemTenantID, - targetTenantID: roachpb.MakeTenantID(20), + targetTenantID: &tenant20, + targetType: SystemTargetTypeSpecificTenantKeyspace, + }, + { + // System tenant targeting the entire keyspace is allowed. + sourceTenantID: roachpb.SystemTenantID, + targetTenantID: nil, + targetType: SystemTargetTypeEntireKeyspace, }, { // System tenant targeting itself is allowed. sourceTenantID: roachpb.SystemTenantID, - targetTenantID: roachpb.SystemTenantID, + targetTenantID: &roachpb.SystemTenantID, + targetType: SystemTargetTypeSpecificTenantKeyspace, }, { // Secondary tenant targeting itself is allowed. - sourceTenantID: roachpb.MakeTenantID(10), - targetTenantID: roachpb.MakeTenantID(10), + sourceTenantID: tenant10, + targetTenantID: &tenant10, + targetType: SystemTargetTypeSpecificTenantKeyspace, + }, + { + // Secondary tenant targeting everything installed on tenants by it is + // allowed. + sourceTenantID: tenant10, + targetTenantID: nil, + targetType: SystemTargetTypeAllTenantKeyspaceTargetsSet, + }, + { + // System tenant targeting everything installed on tenants by it is + // allowed. + sourceTenantID: roachpb.SystemTenantID, + targetTenantID: nil, + targetType: SystemTargetTypeAllTenantKeyspaceTargetsSet, }, } { target := SystemTarget{ - SourceTenantID: tc.sourceTenantID, - TargetTenantID: &tc.targetTenantID, + sourceTenantID: tc.sourceTenantID, + targetTenantID: tc.targetTenantID, + systemTargetType: tc.targetType, } - require.True(t, testutils.IsError(target.validate(), tc.expErr)) + err := target.validate() + require.True( + t, + testutils.IsError(err, tc.expErr), + "expected: %s got: %s ", + tc.expErr, + err, + ) } } @@ -133,12 +247,24 @@ func TestSystemTargetValidation(t *testing.T) { func TestTargetSortingRandomized(t *testing.T) { // Construct a set of sorted targets. sortedTargets := Targets{ - MakeTargetFromSystemTarget(MakeClusterTarget()), - MakeTargetFromSystemTarget(makeTenantTargetOrFatal(t, roachpb.SystemTenantID, roachpb.SystemTenantID)), - MakeTargetFromSystemTarget(makeTenantTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(10))), - MakeTargetFromSystemTarget(makeTenantTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(20))), - MakeTargetFromSystemTarget(makeTenantTargetOrFatal(t, roachpb.MakeTenantID(5), roachpb.MakeTenantID(5))), - MakeTargetFromSystemTarget(makeTenantTargetOrFatal(t, roachpb.MakeTenantID(10), roachpb.MakeTenantID(10))), + MakeTargetFromSystemTarget(MakeAllTenantKeyspaceTargetsSet(roachpb.SystemTenantID)), + MakeTargetFromSystemTarget(MakeAllTenantKeyspaceTargetsSet(roachpb.MakeTenantID(10))), + MakeTargetFromSystemTarget(MakeEntireKeyspaceTarget()), + MakeTargetFromSystemTarget( + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.SystemTenantID), + ), + MakeTargetFromSystemTarget( + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(10)), + ), + MakeTargetFromSystemTarget( + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(20)), + ), + MakeTargetFromSystemTarget( + makeTenantKeyspaceTargetOrFatal(t, roachpb.MakeTenantID(5), roachpb.MakeTenantID(5)), + ), + MakeTargetFromSystemTarget( + makeTenantKeyspaceTargetOrFatal(t, roachpb.MakeTenantID(10), roachpb.MakeTenantID(10)), + ), MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("b")}), MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("d")}), MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("y"), EndKey: roachpb.Key("z")}), @@ -158,10 +284,38 @@ func TestTargetSortingRandomized(t *testing.T) { } } -func makeTenantTargetOrFatal( +// TestSpanTargetsConstructedInSystemSpanConfigKeyspace ensures that +// constructing span targets +func TestSpanTargetsConstructedInSystemSpanConfigKeyspace(t *testing.T) { + for _, tc := range []roachpb.Span{ + MakeEntireKeyspaceTarget().encode(), + makeTenantKeyspaceTargetOrFatal(t, roachpb.MakeTenantID(10), roachpb.MakeTenantID(10)).encode(), + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.SystemTenantID).encode(), + makeTenantKeyspaceTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(10)).encode(), + { + // Extends into from the left + Key: keys.TimeseriesKeyMax, + EndKey: keys.SystemSpanConfigPrefix.Next(), // End Key isn't inclusive. + }, + { + // Entirely contained. + Key: keys.SystemSpanConfigPrefix.Next(), + EndKey: keys.SystemSpanConfigPrefix.Next().PrefixEnd(), + }, + { + // Extends beyond on the right. + Key: keys.SystemSpanConfigPrefix.Next().PrefixEnd(), + EndKey: keys.SystemSpanConfigKeyMax.Next().Next(), + }, + } { + require.Panics(t, func() { MakeTargetFromSpan(tc) }) + } +} + +func makeTenantKeyspaceTargetOrFatal( t *testing.T, sourceID roachpb.TenantID, targetID roachpb.TenantID, ) SystemTarget { - target, err := MakeTenantTarget(sourceID, targetID) + target, err := MakeTenantKeyspaceTarget(sourceID, targetID) require.NoError(t, err) return target }