SERVER-46924: Ensure arbiters are never given a 'newlyAdded' field

4 files changed, 85 insertions, 0 deletions

diff --git a/src/mongo/db/repl/member_config.cpp b/src/mongo/db/repl/member_config.cpp
index 47c7384caed..7b8d721866f 100644
--- a/src/mongo/db/repl/member_config.cpp
+++ b/src/mongo/db/repl/member_config.cpp
@@ -127,6 +127,10 @@ MemberConfig::MemberConfig(const BSONObj& mcfg, ReplSetTagConfig* tagConfig) {
         if (_tags.size() != 2) {
             uasserted(ErrorCodes::BadValue, "Cannot set tags on arbiters.");
         }
+
+        if (isNewlyAdded()) {
+            uasserted(ErrorCodes::BadValue, "Arbiter cannot have newlyAdded field set");
+        }
     }
 
     // Check for additional electable requirements, when priority is non zero.
diff --git a/src/mongo/db/repl/member_config_test.cpp b/src/mongo/db/repl/member_config_test.cpp
index df58900ae50..baff9f6075a 100644
--- a/src/mongo/db/repl/member_config_test.cpp
+++ b/src/mongo/db/repl/member_config_test.cpp
@@ -462,6 +462,22 @@ TEST(MemberConfig, NodeShouldStillHavePriorityAfterToBSON) {
     ASSERT_EQ(3, mc2.getPriority());
 }
 
+TEST(MemberConfig, ArbiterCannotHaveNewlyAddedFieldSet) {
+    // Set the flag to add the 'newlyAdded' field to MemberConfigs.
+    enableAutomaticReconfig = true;
+    // Set the flag back to false after this test exits.
+    ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; });
+
+    ReplSetTagConfig tagConfig;
+
+    // Verify that an exception is thrown when we try to create an arbiter with 'newlyAdded' field.
+    ASSERT_THROWS(MemberConfig(BSON("_id" << 0 << "host"
+                                          << "h"
+                                          << "arbiterOnly" << true << "newlyAdded" << true),
+                               &tagConfig),
+                  ExceptionFor<ErrorCodes::BadValue>);
+}
+
 TEST(MemberConfig, ParseHidden) {
     ReplSetTagConfig tagConfig;
     {
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index 2635494cbd4..93c652aea6f 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -3184,6 +3184,11 @@ Status ReplicationCoordinatorImpl::processReplSetReconfig(OperationContext* opCt
                         return Status(ErrorCodes::InvalidReplicaSetConfig, errmsg);
                     }
 
+                    // We should never set the 'newlyAdded' field for arbiters.
+                    if (newMem.isArbiter()) {
+                        continue;
+                    }
+
                     const int newMemId = newMem.getId().getData();
                     const auto oldMem = oldConfig.findMemberByID(newMemId);
 
diff --git a/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp
index 12126dcea16..6997d407b71 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp
@@ -1742,6 +1742,66 @@ TEST_F(ReplCoordReconfigTest, NodesWithNewlyAddedFieldSetHavePriorityZero) {
     ASSERT_EQUALS(0, secondNewMember->getPriority());
 }
 
+TEST_F(ReplCoordReconfigTest, ArbiterNodesShouldNeverHaveNewlyAddedField) {
+    // Set the flag to add the `newlyAdded` field to MemberConfigs.
+    enableAutomaticReconfig = true;
+    // Set the flag back to false after this test exits.
+    ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; });
+
+    setUpNewlyAddedFieldTest();
+
+    auto opCtx = makeOperationContext();
+    // Do a reconfig that adds a new arbiter.
+    auto members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1")
+                                                << BSON("_id" << 3 << "host"
+                                                              << "n3:1"
+                                                              << "arbiterOnly" << true));
+
+    startCapturingLogMessages();
+    ASSERT_OK(doSafeReconfig(opCtx.get(), 3, members, 1 /* quorumHbs */));
+    stopCapturingLogMessages();
+
+    const auto rsConfig = getReplCoord()->getReplicaSetConfig_forTest();
+    const auto arbiterNode = rsConfig.findMemberByID(3);
+
+    // Verify that the node did not have 'newlyAdded' set.
+    ASSERT_FALSE(arbiterNode->isNewlyAdded());
+
+    // Verify that the node is a voting member.
+    ASSERT_TRUE(arbiterNode->isVoter());
+
+    // Verify that a log message was not created for adding the 'newlyAdded' field.
+    ASSERT_EQUALS(0,
+                  countTextFormatLogLinesContaining(
+                      "Appended the 'newlyAdded' field to a node in the new config."));
+}
+
+TEST_F(ReplCoordReconfigTest, ForceReconfigShouldThrowIfArbiterNodesHaveNewlyAddedField) {
+    // Set the flag to add the `newlyAdded` field to MemberConfigs.
+    enableAutomaticReconfig = true;
+    // Set the flag back to false after this test exits.
+    ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; });
+
+    setUpNewlyAddedFieldTest();
+
+    auto opCtx = makeOperationContext();
+    BSONObjBuilder result;
+    ReplSetReconfigArgs args;
+    args.force = true;
+    // Do a force reconfig that tries to add an arbiter with 'newlyAdded: true'.
+    args.newConfigObj =
+        configWithMembers(2,
+                          0,
+                          BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1")
+                                                       << BSON("_id" << 3 << "host"
+                                                                     << "n3:1"
+                                                                     << "arbiterOnly" << true
+                                                                     << "newlyAdded" << true)));
+
+    ASSERT_EQUALS(ErrorCodes::InvalidReplicaSetConfig,
+                  getReplCoord()->processReplSetReconfig(opCtx.get(), args, &result));
+}
+
 }  // anonymous namespace
 }  // namespace repl
 }  // namespace mongo