diff --git a/pkg/rpc/auth_tenant.go b/pkg/rpc/auth_tenant.go index 6d47a0becd3b..30edfef3ef69 100644 --- a/pkg/rpc/auth_tenant.go +++ b/pkg/rpc/auth_tenant.go @@ -121,26 +121,10 @@ func (a tenantAuth) authBatch(tenID roachpb.TenantID, args *roachpb.BatchRequest return authError(err.Error()) } tenSpan := tenantPrefix(tenID) - if tenSpan.ContainsKeyRange(rSpan.Key, rSpan.EndKey) { - return nil - } - for _, allow := range batchSpanAllowlist { - if rSpan.Equal(allow) { - return nil - } + if !tenSpan.ContainsKeyRange(rSpan.Key, rSpan.EndKey) { + return authErrorf("requested key span %s not fully contained in tenant keyspace %s", rSpan, tenSpan) } - return authErrorf("requested key span %s not fully contained in tenant keyspace %s", rSpan, tenSpan) -} - -// batchSpanAllowlist contains spans outside of a tenant's keyspace that Batch -// RPC invocations are allowed to touch. -var batchSpanAllowlist = []roachpb.RSpan{ - // TODO(nvanbenschoten): Explore whether we can get rid of this by no longer - // reading this key in sqlServer.start. - { - Key: roachpb.RKey(keys.BootstrapVersionKey), - EndKey: roachpb.RKey(keys.BootstrapVersionKey.Next()), - }, + return nil } // authRangeLookup authorizes the provided tenant to invoke the RangeLookup RPC @@ -149,23 +133,10 @@ func (a tenantAuth) authRangeLookup( tenID roachpb.TenantID, args *roachpb.RangeLookupRequest, ) error { tenSpan := tenantPrefix(tenID) - if tenSpan.ContainsKey(args.Key) { - return nil - } - for _, allow := range rangeLookupKeyAllowlist { - if args.Key.Equal(allow) { - return nil - } + if !tenSpan.ContainsKey(args.Key) { + return authErrorf("requested key %s not fully contained in tenant keyspace %s", args.Key, tenSpan) } - return authErrorf("requested key %s not fully contained in tenant keyspace %s", args.Key, tenSpan) -} - -// rangeLookupKeyAllowlist contains keys outside of a tenant's keyspace that -// RangeLookup RPC invocations are allowed to touch. -var rangeLookupKeyAllowlist = []roachpb.Key{ - // TODO(nvanbenschoten): Explore whether we can get rid of this by no longer - // reading this key in sqlServer.start. - keys.BootstrapVersionKey, + return nil } // authRangeFeed authorizes the provided tenant to invoke the RangeFeed RPC with diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index 3ccd1f67e5a5..adb73cad55f2 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -21,6 +21,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/blobs" "github.com/cockroachdb/cockroach/pkg/blobs/blobspb" + "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/config" "github.com/cockroachdb/cockroach/pkg/gossip" "github.com/cockroachdb/cockroach/pkg/jobs" @@ -670,11 +671,31 @@ func (s *sqlServer) start( } var bootstrapVersion roachpb.Version - if err := s.execCfg.DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { - return txn.GetProto(ctx, keys.BootstrapVersionKey, &bootstrapVersion) - }); err != nil { - return err + if s.execCfg.Codec.ForSystemTenant() { + if err := s.execCfg.DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + return txn.GetProto(ctx, keys.BootstrapVersionKey, &bootstrapVersion) + }); err != nil { + return err + } + } else { + // We don't currently track the bootstrap version of each secondary tenant. + // For this to be meaningful, we'd need to record the binary version of the + // SQL gateway that processed the crdb_internal.create_tenant function which + // created the tenant, as this is what dictates the MetadataSchema that was + // in effect when the secondary tenant was constructed. This binary version + // very well may differ from the cluster-wide bootstrap version at which the + // system tenant was bootstrapped. + // + // Since we don't record this version anywhere, we do the next-best thing + // and pass a lower-bound on the bootstrap version. We know that no tenants + // could have been created before the start of the v20.2 dev cycle, so we + // pass VersionStart20_2. bootstrapVersion is only used to avoid performing + // superfluous but necessarily idempotent SQL migrations, so at worst, we're + // doing more work than strictly necessary during the first time that the + // migrations are run. + bootstrapVersion = clusterversion.VersionByKey(clusterversion.VersionStart20_2) } + // Run startup migrations (note: these depend on jobs subsystem running). if err := migMgr.EnsureMigrations(ctx, bootstrapVersion); err != nil { return errors.Wrap(err, "ensuring SQL migrations")