SERVER-59922: Use FCV check with feature flag in replica set setClusterParameter

4 files changed, 113 insertions, 18 deletions

diff --git a/jstests/multiVersion/targetedTestsLastLtsFeatures/cluster_parameter_feature_flags.js b/jstests/multiVersion/targetedTestsLastLtsFeatures/cluster_parameter_feature_flags.js
new file mode 100644
index 00000000000..2c06dfc2411
--- /dev/null
+++ b/jstests/multiVersion/targetedTestsLastLtsFeatures/cluster_parameter_feature_flags.js
@@ -0,0 +1,89 @@
+/**
+ * Tests that cluster server parameter feature flags work correctly.
+ *
+ * TODO: Delete this test once we branch for 6.0.
+ *
+ * @tags: [requires_replication, requires_sharding]
+ */
+
+(function() {
+"use strict";
+
+load("jstests/multiVersion/libs/verify_versions.js");
+load("jstests/multiVersion/libs/multi_rs.js");       // For upgradeSecondaries and upgradeSet.
+load("jstests/multiVersion/libs/multi_cluster.js");  // For upgradeCluster.
+
+const kDowngradeVersion = "last-lts";
+const kUpgradeVersion = "latest";
+
+function assertSetClusterParameterFailsAndthenSucceedsAfterFCVUpgrade(db) {
+    // Despite an upgrade, the test shouldn't pass because the FCV has not been explicitly set.
+    assert.commandFailedWithCode(
+        db.runCommand({setClusterParameter: {testStrClusterParameter: {strData: "ok"}}}),
+        ErrorCodes.IllegalOperation);
+
+    // Set the FCV; the test should now pass.
+    assert.commandWorked(
+        db.runCommand({setFeatureCompatibilityVersion: binVersionToFCV(kUpgradeVersion)}));
+    assert.commandWorked(
+        db.runCommand({setClusterParameter: {testStrClusterParameter: {strData: "ok"}}}));
+}
+
+function assertSetClusterParameterFailsInDowngradedVersion(db) {
+    assert.commandFailedWithCode(
+        db.runCommand({setClusterParameter: {testStrClusterParameter: {strData: "ok"}}}),
+        ErrorCodes.CommandNotFound);
+}
+
+function replicaSetClusterParameterIsFCVGated() {
+    const dbName = "admin";
+
+    // Set up a replica-set in a 'downgraded' version.
+    const rst = new ReplSetTest({nodes: 2, nodeOptions: {binVersion: kDowngradeVersion}});
+    rst.startSet();
+    rst.initiate();
+
+    assertSetClusterParameterFailsInDowngradedVersion(rst.getPrimary().getDB(dbName));
+
+    // Upgrade the replica set.
+    rst.upgradeSet({binVersion: kUpgradeVersion});
+
+    // Verify that all nodes are in the latest version.
+    for (const node of rst.nodes) {
+        assert.binVersion(node, kUpgradeVersion);
+    }
+
+    rst.awaitNodesAgreeOnPrimary();
+
+    assertSetClusterParameterFailsAndthenSucceedsAfterFCVUpgrade(rst.getPrimary().getDB(dbName));
+
+    rst.stopSet();
+}
+
+function shardedClusterParameterIsFCVGated() {
+    const dbName = "admin";
+
+    // Set up a sharded cluster in a 'downgraded' version.
+    const st = new ShardingTest({
+        shards: 2,
+        rs: {nodes: 2, binVersion: kDowngradeVersion},
+        other: {
+            mongosOptions: {binVersion: kDowngradeVersion},
+            configOptions: {binVersion: kDowngradeVersion}
+        }
+    });
+
+    // Sanity check: setClusterParameter shouldn't pass in 'downgraded' version.
+    assertSetClusterParameterFailsInDowngradedVersion(st.s.getDB(dbName));
+
+    // Upgrade the cluster.
+    st.upgradeCluster(kUpgradeVersion, {waitUntilStable: true});
+
+    assertSetClusterParameterFailsAndthenSucceedsAfterFCVUpgrade(st.s.getDB(dbName));
+
+    st.stop();
+}
+
+replicaSetClusterParameterIsFCVGated();
+shardedClusterParameterIsFCVGated();
+})();
diff --git a/src/mongo/db/commands/set_cluster_parameter_command.cpp b/src/mongo/db/commands/set_cluster_parameter_command.cpp
index ce752e33266..31df926ce68 100644
--- a/src/mongo/db/commands/set_cluster_parameter_command.cpp
+++ b/src/mongo/db/commands/set_cluster_parameter_command.cpp
@@ -68,9 +68,10 @@ public:
         using InvocationBase::InvocationBase;
 
         void typedRun(OperationContext* opCtx) {
-            uassert(ErrorCodes::IllegalOperation,
-                    "Cannot set cluster parameter, gFeatureFlagClusterWideConfig is not enabled",
-                    gFeatureFlagClusterWideConfig.isEnabledAndIgnoreFCV());
+            uassert(
+                ErrorCodes::IllegalOperation,
+                "Cannot set cluster parameter, gFeatureFlagClusterWideConfig is not enabled",
+                gFeatureFlagClusterWideConfig.isEnabled(serverGlobalParams.featureCompatibility));
 
             uassert(ErrorCodes::ErrorCodes::NotImplemented,
                     "setClusterParameter can only run on mongos in sharded clusters",
diff --git a/src/mongo/db/s/config/sharding_catalog_manager_add_shard_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_add_shard_test.cpp
index 3c42d87e79f..b9e33d855e6 100644
--- a/src/mongo/db/s/config/sharding_catalog_manager_add_shard_test.cpp
+++ b/src/mongo/db/s/config/sharding_catalog_manager_add_shard_test.cpp
@@ -459,12 +459,12 @@ TEST_F(AddShardTest, StandaloneBasicSuccess) {
     // The shard receives the _addShard command
     expectAddShardCmdReturnSuccess(shardTarget, expectedShardName);
 
-    // The shard receives a delete op to clear any leftover clusterParameters doc.
-    expectRemoveSetClusterParameterDocs(shardTarget);
-
     // The shard receives a delete op to clear any leftover user_writes_critical_sections doc.
     expectRemoveUserWritesCriticalSectionsDocs(shardTarget);
 
+    // The shard receives a delete op to clear any leftover clusterParameters doc.
+    expectRemoveSetClusterParameterDocs(shardTarget);
+
     // The shard receives the setFeatureCompatibilityVersion command.
     expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1), expectWriteConcern.toBSON());
 
@@ -545,12 +545,12 @@ TEST_F(AddShardTest, StandaloneGenerateName) {
     // The shard receives the _addShard command
     expectAddShardCmdReturnSuccess(shardTarget, expectedShardName);
 
-    // The shard receives a delete op to clear any leftover clusterParameters doc.
-    expectRemoveSetClusterParameterDocs(shardTarget);
-
     // The shard receives a delete op to clear any leftover user_writes_critical_sections doc.
     expectRemoveUserWritesCriticalSectionsDocs(shardTarget);
 
+    // The shard receives a delete op to clear any leftover clusterParameters doc.
+    expectRemoveSetClusterParameterDocs(shardTarget);
+
     // The shard receives the setFeatureCompatibilityVersion command.
     expectSetFeatureCompatibilityVersion(
         shardTarget, BSON("ok" << 1), operationContext()->getWriteConcern().toBSON());
@@ -946,12 +946,12 @@ TEST_F(AddShardTest, SuccessfullyAddReplicaSet) {
     // The shard receives the _addShard command
     expectAddShardCmdReturnSuccess(shardTarget, expectedShardName);
 
-    // The shard receives a delete op to clear any leftover clusterParameters doc.
-    expectRemoveSetClusterParameterDocs(shardTarget);
-
     // The shard receives a delete op to clear any leftover user_writes_critical_sections doc.
     expectRemoveUserWritesCriticalSectionsDocs(shardTarget);
 
+    // The shard receives a delete op to clear any leftover clusterParameters doc.
+    expectRemoveSetClusterParameterDocs(shardTarget);
+
     // The shard receives the setFeatureCompatibilityVersion command.
     expectSetFeatureCompatibilityVersion(
         shardTarget, BSON("ok" << 1), operationContext()->getWriteConcern().toBSON());
@@ -1017,12 +1017,12 @@ TEST_F(AddShardTest, ReplicaSetExtraHostsDiscovered) {
     // The shard receives the _addShard command
     expectAddShardCmdReturnSuccess(shardTarget, expectedShardName);
 
-    // The shard receives a delete op to clear any leftover clusterParameters doc.
-    expectRemoveSetClusterParameterDocs(shardTarget);
-
     // The shard receives a delete op to clear any leftover user_writes_critical_sections doc.
     expectRemoveUserWritesCriticalSectionsDocs(shardTarget);
 
+    // The shard receives a delete op to clear any leftover clusterParameters doc.
+    expectRemoveSetClusterParameterDocs(shardTarget);
+
     // The shard receives the setFeatureCompatibilityVersion command.
     expectSetFeatureCompatibilityVersion(
         shardTarget, BSON("ok" << 1), operationContext()->getWriteConcern().toBSON());
@@ -1101,12 +1101,12 @@ TEST_F(AddShardTest, AddShardSucceedsEvenIfAddingDBsFromNewShardFails) {
     // The shard receives the _addShard command
     expectAddShardCmdReturnSuccess(shardTarget, expectedShardName);
 
-    // The shard receives a delete op to clear any leftover clusterParameters doc.
-    expectRemoveSetClusterParameterDocs(shardTarget);
-
     // The shard receives a delete op to clear any leftover user_writes_critical_sections doc.
     expectRemoveUserWritesCriticalSectionsDocs(shardTarget);
 
+    // The shard receives a delete op to clear any leftover clusterParameters doc.
+    expectRemoveSetClusterParameterDocs(shardTarget);
+
     // The shard receives the setFeatureCompatibilityVersion command.
     expectSetFeatureCompatibilityVersion(
         shardTarget, BSON("ok" << 1), operationContext()->getWriteConcern().toBSON());
diff --git a/src/mongo/idl/cluster_server_parameter.idl b/src/mongo/idl/cluster_server_parameter.idl
index 6e8b4b28756..9d99717cba5 100644
--- a/src/mongo/idl/cluster_server_parameter.idl
+++ b/src/mongo/idl/cluster_server_parameter.idl
@@ -90,6 +90,11 @@ feature_flags:
     featureFlagClusterWideConfig:
         description: Mechanism for cluster-wide configuration options
         cpp_varname: gFeatureFlagClusterWideConfig
+        default: true
+        version: 6.0
+    featureFlagClusterWideConfigM2:
+        description: Mechanism for cluster-wide configuration options, milestone 2
+        cpp_varname: gFeatureFlagClusterWideConfigM2
         default: false
 
 server_parameters: