SERVER-56919: Add validation for memberIndex to reconfigToPSASet() shell helper

(cherry picked from commit 3192123cbc96d4c8121bd9849e70fd8b79eb37a9)

2 files changed, 76 insertions, 9 deletions

diff --git a/jstests/noPassthrough/reconfig_for_psa_set_shell.js b/jstests/noPassthrough/reconfig_for_psa_set_shell.js
index a40704ffdaf..3ba5460a428 100644
--- a/jstests/noPassthrough/reconfig_for_psa_set_shell.js
+++ b/jstests/noPassthrough/reconfig_for_psa_set_shell.js
@@ -21,14 +21,19 @@ rst.initiateWithHighElectionTimeout();
 const primary = rst.getPrimary();
 assert.eq(primary, rst.nodes[0], "the primary should be the node at index 0");
 
-// Verify that a reconfig that directly gives the secondary 'votes: 1' and 'priority: 1' will fail.
-const config = rst.getReplSetConfigFromNode();
+// Store the old config so that we can reset to it after every successful reconfig.
+const originalConfig = rst.getReplSetConfigFromNode();
+
+// Verify that directly calling the standard 'rs.reconfig()' function to give the secondary 'votes:
+// 1' and 'priority: 1' will fail.
+let config = rst.getReplSetConfigFromNode();
 config.members[1].votes = 1;
 config.members[1].priority = 1;
 
-let reconfigScript = `assert.commandFailedWithCode(rs.reconfig(${
+jsTestLog("Testing standard rs.reconfig() function");
+const reconfigScript = `assert.commandFailedWithCode(rs.reconfig(${
     tojson(config)}), ErrorCodes.NewReplicaSetConfigurationIncompatible)`;
-let result = runMongoProgram('mongo', '--port', primary.port, '--eval', reconfigScript);
+const result = runMongoProgram('mongo', '--port', primary.port, '--eval', reconfigScript);
 assert.eq(0, result, `reconfig did not fail with expected error code`);
 
 const runReconfigForPSASet = (memberIndex, config, shouldSucceed, endPriority = 1) => {
@@ -88,9 +93,38 @@ config.members = filteredMembers;
 config.version += 1;
 assert.commandWorked(primary.adminCommand({replSetReconfig: config}));
 
-const replSetGetConfig = assert.commandWorked(primary.adminCommand({replSetGetConfig: 1})).config;
-assert.eq(1, replSetGetConfig.members[1].votes);
-assert.eq(1, replSetGetConfig.members[1].priority);
+// Attempt to add a node and assert that the reconfigToPSASet() call is successful.
+config.members = originalConfig.members;
+config.members[1].votes = 1;
+config.members[1].priority = 1;
+config.version += 1;
+runReconfigForPSASet(1, config, true /* shouldSucceed */);
+
+// Test that reconfigToPSASet() will succeed even if the priority of the node is 0 in the final
+// config.
+jsTestLog("Testing reconfigForPSASet() succeeded: priority of node is 0 in final config");
+config = rst.getReplSetConfigFromNode();
+config.members[1].votes = 1;
+config.members[1].priority = 0;
+runReconfigForPSASet(1, config, true /* shouldSucceed */, 0 /* endPriority */);
+
+// Test that reconfigToPSASet() will fail if the node at 'memberIndex' exists in the old config but
+// was a voter. It should fail because this is not the use case of the helper function.
+jsTestLog("Testing reconfigForPSASet() failed: node at memberIndex was a voter in old config");
+
+// First turn the secondary into a voting node but with priority 0.
+config = rst.getReplSetConfigFromNode();
+config.members[1].votes = 1;
+config.members[1].priority = 0;
+config.version += 1;
+assert.commandWorked(primary.adminCommand({replSetReconfig: config}));
+
+// Now attempt a reconfig to give the secondary priority 1 via the reconfigForPSASet() function.
+// This should fail because this function should not be used if the secondary is a voter in the old
+// config.
+config.members[1].priority = 1;
+config.version += 1;
+runReconfigForPSASet(1, config, false /* shouldSucceed */);
 
 rst.stopSet();
 })();
diff --git a/src/mongo/shell/utils.js b/src/mongo/shell/utils.js
index a65ad7cb933..4f75c8d3c04 100644
--- a/src/mongo/shell/utils.js
+++ b/src/mongo/shell/utils.js
@@ -1550,13 +1550,38 @@ rs.reconfig = function(cfg, options) {
     }
     return this._runCmd(cmd);
 };
+
+_validateMemberIndex = function(memberIndex, newConfig) {
+    const newMemberConfig = newConfig.members[memberIndex];
+    assert(newMemberConfig, `Node at index ${memberIndex} does not exist in the new config`);
+    assert.eq(1,
+              newMemberConfig.votes,
+              `Node at index ${memberIndex} must have {votes: 1} in the new config`);
+
+    // Use memberId to compare nodes across configs.
+    const memberId = newMemberConfig._id;
+    const oldConfig = rs.conf();
+    const oldMemberConfig = oldConfig.members.find(member => member._id === memberId);
+
+    // If the node doesn't exist in the old config, we are adding it as a new node. Skip validating
+    // the node in the old config.
+    if (!oldMemberConfig) {
+        return;
+    }
+
+    assert(!oldMemberConfig.votes,
+           `Node at index ${memberIndex} must have {votes: 0} in the old config`);
+};
+
 rs.reconfigForPSASet = function(memberIndex, cfg, options) {
+    _validateMemberIndex(memberIndex, cfg);
+
     const memberPriority = cfg.members[memberIndex].priority;
     print(
         `Running first reconfig to give member at index ${memberIndex} { votes: 1, priority: 0 }`);
     cfg.members[memberIndex].votes = 1;
     cfg.members[memberIndex].priority = 0;
-    const res = rs.reconfig(cfg, options);
+    let res = rs.reconfig(cfg, options);
     if (!res.ok) {
         return res;
     }
@@ -1564,7 +1589,15 @@ rs.reconfigForPSASet = function(memberIndex, cfg, options) {
     print(`Running second reconfig to give member at index ${memberIndex} { priority: ${
         memberPriority} }`);
     cfg.members[memberIndex].priority = memberPriority;
-    return rs.reconfig(cfg, options);
+
+    // If the first reconfig added a new node, the second config will not succeed until the
+    // automatic reconfig to remove the 'newlyAdded' field is completed. Retry the second reconfig
+    // until it succeeds in that case.
+    assert.soon(() => {
+        res = rs.reconfig(cfg, options);
+        return res.ok;
+    });
+    return res;
 };
 rs.add = function(hostport, arb) {
     var cfg = hostport;