clusterversion: add a gate for new system privileges

A 22.2/23.1 mixed version cluster cannot handle new system privileges
well. This commit gates their usage and adds a test.

Without this gate, the included test would fail and users would not be
able to log in to nodes running on the old binary.

Release note: None

docs/generated/settings/settings-for-tenants.txt

@@ -299,4 +299,4 @@ trace.opentelemetry.collector	string		address of an OpenTelemetry trace collecto
 trace.snapshot.rate	duration	0s	if non-zero, interval at which background trace snapshots are captured
 trace.span_registry.enabled	boolean	true	if set, ongoing traces can be seen at https://<ui>/#/debug/tracez
 trace.zipkin.collector	string		the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
-version	version	1000022.2-76	set the active cluster version in the format '<major>.<minor>'
+version	version	1000022.2-78	set the active cluster version in the format '<major>.<minor>'

docs/generated/settings/settings.html

@@ -240,6 +240,6 @@
 <tr><td><div id="setting-trace-snapshot-rate" class="anchored"><code>trace.snapshot.rate</code></div></td><td>duration</td><td><code>0s</code></td><td>if non-zero, interval at which background trace snapshots are captured</td></tr>
 <tr><td><div id="setting-trace-span-registry-enabled" class="anchored"><code>trace.span_registry.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://&lt;ui&gt;/#/debug/tracez</td></tr>
 <tr><td><div id="setting-trace-zipkin-collector" class="anchored"><code>trace.zipkin.collector</code></div></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as &lt;host&gt;:&lt;port&gt;. If no port is specified, 9411 will be used.</td></tr>
-<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000022.2-76</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td></tr>
+<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000022.2-78</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td></tr>
 </tbody>
 </table>

pkg/cli/testdata/declarative-rules/deprules

@@ -1,6 +1,6 @@
 dep
 ----
-debug declarative-print-rules 1000022.2-76 dep
+debug declarative-print-rules 1000022.2-78 dep
 deprules
 ----
 - name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: PUBLIC->VALIDATED'

pkg/cli/testdata/declarative-rules/oprules

@@ -1,6 +1,6 @@
 op
 ----
-debug declarative-print-rules 1000022.2-76 op
+debug declarative-print-rules 1000022.2-78 op
 rules
 ----
 []

pkg/clusterversion/cockroach_versions.go

@@ -482,6 +482,12 @@ const (
 	// has been backfilled.
 	V23_1ExternalConnectionsTableOwnerIDColumnBackfilled
 
+	// V23_1AllowNewSystemPrivileges is the version at which we allow the new
+	// MODIFYSQLCLUSTERSETTING abd VIEWJOB system privileges to be used.
+	// Note: After v23.1 is released, we won't need to version gate these anymore,
+	// since we've made mixed-version clusters tolerate new privileges.
+	V23_1AllowNewSystemPrivileges
+
 	// *************************************************
 	// Step (1): Add new versions here.
 	// Do not add new versions to a patch release.
@@ -835,6 +841,10 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     V23_1ExternalConnectionsTableOwnerIDColumnBackfilled,
 		Version: roachpb.Version{Major: 22, Minor: 2, Internal: 76},
 	},
+	{
+		Key:     V23_1AllowNewSystemPrivileges,
+		Version: roachpb.Version{Major: 22, Minor: 2, Internal: 78},
+	},
 
 	// *************************************************
 	// Step (2): Add new versions here.

pkg/sql/grant_revoke_system.go

@@ -21,6 +21,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catprivilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemadesc"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
@@ -38,6 +40,17 @@ func (n *changeNonDescriptorBackedPrivilegesNode) ReadingOwnWrites() {}
 func (n *changeNonDescriptorBackedPrivilegesNode) startExec(params runParams) error {
 	privilegesTableHasUserIDCol := params.p.ExecCfg().Settings.Version.IsActive(params.ctx,
 		clusterversion.V23_1SystemPrivilegesTableHasUserIDColumn)
+	if !params.p.ExecCfg().Settings.Version.IsActive(
+		params.ctx,
+		clusterversion.V23_1AllowNewSystemPrivileges,
+	) {
+		if n.desiredprivs.Contains(privilege.MODIFYSQLCLUSTERSETTING) {
+			return pgerror.New(pgcode.FeatureNotSupported, "upgrade must be finalized before using MODIFYSQLCLUSTERSETTING system privilege")
+		}
+		if n.desiredprivs.Contains(privilege.VIEWJOB) {
+			return pgerror.New(pgcode.FeatureNotSupported, "upgrade must be finalized before using VIEWJOB system privilege")
+		}
+	}
 
 	if err := params.p.preChangePrivilegesValidation(params.ctx, n.grantees, n.withGrantOption, n.isGrant); err != nil {
 		return err

pkg/sql/logictest/testdata/logic_test/mixed_version_new_system_privileges

@@ -0,0 +1,18 @@
+# LogicTest: cockroach-go-testserver-upgrade-to-master
+
+upgrade 0
+
+upgrade 1
+
+statement error upgrade must be finalized before using MODIFYSQLCLUSTERSETTING
+GRANT SYSTEM MODIFYSQLCLUSTERSETTING TO testuser
+
+statement error upgrade must be finalized before using VIEWJOB
+GRANT SYSTEM VIEWJOB TO testuser
+
+# Verify that a non-root user can login on the upgraded node.
+user testuser nodeidx=0
+
+# The non-root user should not be able to set cluster settings on the new node.
+statement error only users with the MODIFYCLUSTERSETTING or MODIFYSQLCLUSTERSETTING privilege are allowed to set cluster setting 'sql.defaults.default_int_size'
+SET CLUSTER SETTING sql.defaults.default_int_size = 8

pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/BUILD.bazel

@@ -11,7 +11,7 @@ go_test(
         "//pkg/cmd/cockroach-short",  # keep
         "//pkg/sql/logictest:testdata",  # keep
     ],
-    shard_count = 3,
+    shard_count = 4,
     tags = ["cpu:2"],
     deps = [
         "//pkg/build/bazel",