Merge #69174

69174: sql: add version gate for default privileges r=ajwerner a=RichardJCai 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 Co-authored-by: richardjcai <[email protected]>
cockroachdb · Aug 20, 2021 · 7897f24 · 7897f24
2 parents c87aea4 + 0eb2f7a
commit 7897f24
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 15 deletions.
diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt
@@ -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>'
diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html
@@ -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>
diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go
@@ -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.
 )
 
@@ -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.
 }
 

diff --git a/pkg/clusterversion/key_string.go b/pkg/clusterversion/key_string.go
diff --git a/pkg/sql/alter_default_privileges.go b/pkg/sql/alter_default_privileges.go
@@ -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"
@@ -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()

diff --git a/pkg/sql/grant_revoke.go b/pkg/sql/grant_revoke.go
@@ -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"
@@ -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
+			}
 		}
 	}