SERVER-45078 Ignore term in configs provided by replSetReconfig command

6 files changed, 86 insertions, 19 deletions

diff --git a/src/mongo/db/repl/repl_set_config.cpp b/src/mongo/db/repl/repl_set_config.cpp
index ad70a54c3ee..c19addc5cda 100644
--- a/src/mongo/db/repl/repl_set_config.cpp
+++ b/src/mongo/db/repl/repl_set_config.cpp
@@ -96,15 +96,20 @@ const std::string kCatchUpTakeoverDelayFieldName = "catchUpTakeoverDelayMillis";
 
 }  // namespace
 
-Status ReplSetConfig::initialize(const BSONObj& cfg, OID defaultReplicaSetId) {
-    return _initialize(cfg, false, defaultReplicaSetId);
+Status ReplSetConfig::initialize(const BSONObj& cfg,
+                                 boost::optional<long long> forceTerm,
+                                 OID defaultReplicaSetId) {
+    return _initialize(cfg, false, forceTerm, defaultReplicaSetId);
 }
 
 Status ReplSetConfig::initializeForInitiate(const BSONObj& cfg) {
-    return _initialize(cfg, true, OID());
+    return _initialize(cfg, true, OpTime::kInitialTerm, OID());
 }
 
-Status ReplSetConfig::_initialize(const BSONObj& cfg, bool forInitiate, OID defaultReplicaSetId) {
+Status ReplSetConfig::_initialize(const BSONObj& cfg,
+                                  bool forInitiate,
+                                  boost::optional<long long> forceTerm,
+                                  OID defaultReplicaSetId) {
     _isInitialized = false;
     _members.clear();
 
@@ -132,18 +137,20 @@ Status ReplSetConfig::_initialize(const BSONObj& cfg, bool forInitiate, OID defa
         return status;
 
     //
-    // Parse term. Default to -1 if none is given. Initial configs always have the initial term.
+    // Set term
     //
-    if (forInitiate) {
-        _term = OpTime::kInitialTerm;
+    if (forceTerm != boost::none) {
+        // Set term to the value explicitly passed in.
+        _term = forceTerm.get();
     } else {
+        // Parse term if an explicit term was not passed in. If we cannot find a term to parse,
+        // default to -1.
         status = bsonExtractIntegerFieldWithDefault(
             cfg, kTermFieldName, OpTime::kUninitializedTerm, &_term);
         if (!status.isOK())
             return status;
     }
 
-
     //
     // Parse members
     //
diff --git a/src/mongo/db/repl/repl_set_config.h b/src/mongo/db/repl/repl_set_config.h
index 4bf779fe76f..66075c5c105 100644
--- a/src/mongo/db/repl/repl_set_config.h
+++ b/src/mongo/db/repl/repl_set_config.h
@@ -79,12 +79,17 @@ public:
     /**
      * Initializes this ReplSetConfig from the contents of "cfg".
      * Sets _replicaSetId to "defaultReplicaSetId" if a replica set ID is not specified in "cfg".
+     * If forceTerm is not boost::none, sets _term to the given term. Otherwise, parses term from
+     * config BSON.
      */
-    Status initialize(const BSONObj& cfg, OID defaultReplicaSetId = OID());
+    Status initialize(const BSONObj& cfg,
+                      boost::optional<long long> forceTerm = boost::none,
+                      OID defaultReplicaSetId = OID());
 
     /**
      * Same as the generic initialize() above except will default "configsvr" setting to the value
      * of serverGlobalParams.configsvr.
+     * Sets _term to kInitialTerm.
      */
     Status initializeForInitiate(const BSONObj& cfg);
 
@@ -419,8 +424,13 @@ private:
     /**
      * Sets replica set ID to 'defaultReplicaSetId' if forInitiate is false and 'cfg' does not
      * contain an ID.
+     * Sets _term to kInitialTerm for initiate.
+     * Sets _term to forceTerm if it is not boost::none. Otherwise, parses term from 'cfg'.
      */
-    Status _initialize(const BSONObj& cfg, bool forInitiate, OID defaultReplicaSetId);
+    Status _initialize(const BSONObj& cfg,
+                       bool forInitiate,
+                       boost::optional<long long> forceTerm,
+                       OID defaultReplicaSetId);
 
     bool _isInitialized = false;
     long long _version = 1;
diff --git a/src/mongo/db/repl/repl_set_config_checks_test.cpp b/src/mongo/db/repl/repl_set_config_checks_test.cpp
index de48c7c9f79..d48383c767a 100644
--- a/src/mongo/db/repl/repl_set_config_checks_test.cpp
+++ b/src/mongo/db/repl/repl_set_config_checks_test.cpp
@@ -49,12 +49,12 @@ TEST_F(ServiceContextTest, ValidateConfigForInitiate_VersionMustBe1) {
     rses.addSelf(HostAndPort("h1"));
 
     ReplSetConfig config;
-    ASSERT_OK(config.initializeForInitiate(BSON("_id"
-                                                << "rs0"
-                                                << "version" << 2 << "term" << OpTime::kInitialTerm
-                                                << "protocolVersion" << 1 << "members"
-                                                << BSON_ARRAY(BSON("_id" << 1 << "host"
-                                                                         << "h1")))));
+    ASSERT_OK(
+        config.initializeForInitiate(BSON("_id"
+                                          << "rs0"
+                                          << "version" << 2 << "protocolVersion" << 1 << "members"
+                                          << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                                                   << "h1")))));
     ASSERT_EQUALS(ErrorCodes::NewReplicaSetConfigurationIncompatible,
                   validateConfigForInitiate(&rses, config, getGlobalServiceContext()).getStatus());
 }
diff --git a/src/mongo/db/repl/repl_set_config_test.cpp b/src/mongo/db/repl/repl_set_config_test.cpp
index 90554226a5f..ddebd5f25f9 100644
--- a/src/mongo/db/repl/repl_set_config_test.cpp
+++ b/src/mongo/db/repl/repl_set_config_test.cpp
@@ -366,6 +366,18 @@ TEST(ReplSetConfig, ParseFailsWithBadOrMissingTermField) {
                                      << BSON_ARRAY(BSON("_id" << 0 << "host"
                                                               << "localhost:12345")))));
     ASSERT_EQUALS(ErrorCodes::BadValue, config.validate());
+
+    // If we provide an explicit term field, then we should not fail initialize since we overwrite
+    // the invalid term. This tests the case where a reconfig command passes in an invalid term.
+    ASSERT_OK(config.initialize(BSON("_id"
+                                     << "rs0"
+                                     << "version" << 1 << "term"
+                                     << "1"
+                                     << "protocolVersion" << 1 << "members"
+                                     << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                                              << "localhost:12345"))),
+                                1 /* explicit term */));
+    ASSERT_OK(config.validate());
 }
 
 TEST(ReplSetConfig, ParseFailsWithBadMembers) {
@@ -1672,6 +1684,7 @@ TEST(ReplSetConfig, ReplSetId) {
                                           << BSON_ARRAY(BSON("_id" << 0 << "host"
                                                                    << "localhost:12345"
                                                                    << "priority" << 1))),
+                                     boost::none,
                                      defaultReplicaSetId));
     ASSERT_OK(configLocal.validate());
     ASSERT_TRUE(configLocal.hasReplicaSetId());
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index 1d1fed3e8ff..adff8fcdcb3 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -2752,7 +2752,10 @@ Status ReplicationCoordinatorImpl::processReplSetReconfig(OperationContext* opCt
     BSONObj oldConfigObj = oldConfig.toBSON();
     audit::logReplSetReconfig(opCtx->getClient(), &oldConfigObj, &newConfigObj);
 
-    Status status = newConfig.initialize(newConfigObj, oldConfig.getReplicaSetId());
+    // When initializing a new config through the replSetReconfig command, ignore the term
+    // field passed in through its args. Instead, use this node's term.
+    Status status =
+        newConfig.initialize(newConfigObj, _topCoord->getTerm(), oldConfig.getReplicaSetId());
     if (!status.isOK()) {
         error() << "replSetReconfig got " << status << " while parsing " << newConfigObj;
         return Status(ErrorCodes::InvalidReplicaSetConfig, status.reason());
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 92ea7ea3aa8..5822e6c5506 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp
@@ -245,14 +245,16 @@ void doReplSetInitiate(ReplicationCoordinatorImpl* replCoord,
 
 void doReplSetReconfig(ReplicationCoordinatorImpl* replCoord,
                        Status* status,
-                       OperationContext* opCtx) {
+                       OperationContext* opCtx,
+                       long long term = OpTime::kInitialTerm) {
     BSONObjBuilder garbage;
     ReplSetReconfigArgs args;
     args.force = false;
     // Replica set id will be copied from existing configuration.
     args.newConfigObj = BSON("_id"
                              << "mySet"
-                             << "version" << 3 << "protocolVersion" << 1 << "members"
+                             << "version" << 3 << "term" << term << "protocolVersion" << 1
+                             << "members"
                              << BSON_ARRAY(BSON("_id" << 1 << "host"
                                                       << "node1:12345")
                                            << BSON("_id" << 2 << "host"
@@ -450,6 +452,38 @@ TEST_F(ReplCoordTest, PrimaryNodeAcceptsNewConfigWhenReceivingAReconfigWithAComp
     ASSERT_OK(status);
 }
 
+TEST_F(ReplCoordTest, OverrideReconfigBsonTermSoReconfigSucceeds) {
+    // start up, become primary, reconfig successfully
+    assertStartSuccess(BSON("_id"
+                            << "mySet"
+                            << "version" << 2 << "members"
+                            << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                                     << "node1:12345")
+                                          << BSON("_id" << 2 << "host"
+                                                        << "node2:12345"))
+                            << "settings" << BSON("replicaSetId" << OID::gen())),
+                       HostAndPort("node1", 12345));
+    ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY));
+    replCoordSetMyLastAppliedOpTime(OpTime(Timestamp(100, 1), 0), Date_t() + Seconds(100));
+    replCoordSetMyLastDurableOpTime(OpTime(Timestamp(100, 1), 0), Date_t() + Seconds(100));
+    simulateSuccessfulV1Election();  // Since we have simulated one election, term should be 1.
+
+    Status status(ErrorCodes::InternalError, "Not Set");
+    const auto opCtx = makeOperationContext();
+    // Term is 1, but pass an invalid term, 50, to the reconfig command. The reconfig should still
+    // succeed because we will override 50 with 1.
+    stdx::thread reconfigThread(
+        [&] { doReplSetReconfig(getReplCoord(), &status, opCtx.get(), 50 /* incorrect term */); });
+
+    replyToReceivedHeartbeatV1();
+    reconfigThread.join();
+    ASSERT_OK(status);
+
+    // After the reconfig, the config term should be 1, not 50.
+    const auto config = getReplCoord()->getConfig();
+    ASSERT_EQUALS(config.getConfigTerm(), 1);
+}
+
 TEST_F(
     ReplCoordTest,
     NodeReturnsConfigurationInProgressWhenReceivingAReconfigWhileInTheMidstOfAHeartbeatReconfig) {