Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: don't query BootstrapVersionKey on tenant SQL startup #52595

Merged
merged 1 commit into from
Aug 11, 2020
Merged

sql: don't query BootstrapVersionKey on tenant SQL startup #52595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 6 additions & 35 deletions pkg/rpc/auth_tenant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
29 changes: 25 additions & 4 deletions pkg/server/server_sql.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand Down