Skip to content

Commit

Permalink
sql: add version gate for default privileges
Browse files Browse the repository at this point in the history
Require user to be on DefaultPrivileges version to allow
using default privileges syntax and converting incompatible
pg database privileges to default privileges.

Release note: None
  • Loading branch information
RichardJCai committed Aug 20, 2021
1 parent 65457f9 commit 0eb2f7a
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 15 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,4 @@ trace.datadog.project string CockroachDB the project under which traces will be
trace.debug.enable boolean false if set, traces for recent requests can be seen at https://<ui>/debug/requests
trace.lightstep.token string if set, traces go to Lightstep using this token
trace.zipkin.collector string if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'). Only one tracer can be configured at a time.
version version 21.1-140 set the active cluster version in the format '<major>.<minor>'
version version 21.1-142 set the active cluster version in the format '<major>.<minor>'
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,6 @@
<tr><td><code>trace.debug.enable</code></td><td>boolean</td><td><code>false</code></td><td>if set, traces for recent requests can be seen at https://<ui>/debug/requests</td></tr>
<tr><td><code>trace.lightstep.token</code></td><td>string</td><td><code></code></td><td>if set, traces go to Lightstep using this token</td></tr>
<tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>if set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'). Only one tracer can be configured at a time.</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>21.1-140</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>21.1-142</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
</tbody>
</table>
7 changes: 6 additions & 1 deletion pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,8 @@ const (
// EnsureNoInterleavedTables interleaved tables no longer exist in
// this version.
EnsureNoInterleavedTables
// DefaultPrivileges default privileges are supported in this version.
DefaultPrivileges
// Step (1): Add new versions here.
)

Expand Down Expand Up @@ -494,7 +496,10 @@ var versionsSingleton = keyedVersions{
Key: EnsureNoInterleavedTables,
Version: roachpb.Version{Major: 21, Minor: 1, Internal: 140},
},

{
Key: DefaultPrivileges,
Version: roachpb.Version{Major: 21, Minor: 1, Internal: 142},
},
// Step (2): Add new versions here.
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/clusterversion/key_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/sql/alter_default_privileges.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package sql
import (
"context"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -44,6 +45,12 @@ func (n *alterDefaultPrivilegesNode) Close(context.Context) {}
func (p *planner) alterDefaultPrivileges(
ctx context.Context, n *tree.AlterDefaultPrivileges,
) (planNode, error) {
if !p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.DefaultPrivileges) {
return nil, pgerror.Newf(pgcode.FeatureNotSupported,
"version %v must be finalized to use default privileges",
clusterversion.DefaultPrivileges)
}

// ALTER DEFAULT PRIVILEGES without specifying a schema alters the privileges
// for the current database.
database := p.CurrentDatabase()
Expand Down
23 changes: 13 additions & 10 deletions pkg/sql/grant_revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"context"
"fmt"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
Expand Down Expand Up @@ -135,16 +136,18 @@ func (n *changePrivilegesNode) startExec(params runParams) error {
return nil
}

if n.grantOn == privilege.Database {
compatiblePrivileges, err := convertPGIncompatibleDatabasePrivilegesToDefaultPrivileges(ctx, p, n, params)
if err != nil {
return err
}
// When granting, we exclude the incompatible privileges.
// Note: we can't do this when revoking in case the privilege was granted
// before 21.2 where this conversion does not happen.
if n.isGrant {
n.desiredprivs = compatiblePrivileges
if p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.DefaultPrivileges) {
if n.grantOn == privilege.Database {
compatiblePrivileges, err := convertPGIncompatibleDatabasePrivilegesToDefaultPrivileges(ctx, p, n, params)
if err != nil {
return err
}
// When granting, we exclude the incompatible privileges.
// Note: we can't do this when revoking in case the privilege was granted
// before 21.2 where this conversion does not happen.
if n.isGrant {
n.desiredprivs = compatiblePrivileges
}
}
}

Expand Down

0 comments on commit 0eb2f7a

Please sign in to comment.