SERVER-50827 Nodes with IP addresses in split horizons must be able to start

6 files changed, 119 insertions, 8 deletions

diff --git a/jstests/replsets/split_horizon_hostname_startup.js b/jstests/replsets/split_horizon_hostname_startup.js
new file mode 100644
index 00000000000..d698a329b70
--- /dev/null
+++ b/jstests/replsets/split_horizon_hostname_startup.js
@@ -0,0 +1,65 @@
+/**
+ * IPs cannot be used as hostnames for split horizon configurations; replSetInitiate will not work
+ * correctly. If previously configured using IP, mongod will still be allowed to start
+ * @tags: [ incompatible_with_eft, requires_persistence ]
+ */
+
+(function() {
+"use strict";
+
+// Start and configure mongod with invalid split horizons using override setting
+
+let startupConfig = {
+    replSet: "test",
+    startClean: true,
+    dbpath: MongoRunner.dataPath + 'split-hz-test',
+    setParameter: {disableSplitHorizonIPCheck: true}
+};
+
+let mongod = MongoRunner.runMongod(startupConfig);
+assert(mongod);
+
+const replConfig = {
+    _id: "test",
+    members: [{_id: 0, host: "127.0.0.1:" + mongod.port, horizons: {horizon_name: "127.0.0.0/20"}}]
+};
+
+// Make sure replSetInitiate works
+
+let output = mongod.adminCommand({replSetInitiate: replConfig});
+jsTestLog("Command result: " + tojson(output));
+assert.commandWorked(output);
+
+let rsConfig1 = mongod.adminCommand({replSetGetConfig: 1});
+jsTestLog("rsConfig1: " + tojson(rsConfig1));
+assert.commandWorked(rsConfig1);
+
+MongoRunner.stopMongod(mongod);
+
+// Restart mongod without override setting and see that it will still work because it was previously
+// configured
+
+startupConfig.startClean = false;
+startupConfig.restart = true;
+startupConfig.noCleanData = true;
+startupConfig.setParameter.disableSplitHorizonIPCheck = false;
+startupConfig.port = mongod.port;
+
+mongod = MongoRunner.runMongod(startupConfig);
+assert(mongod);
+
+let rsConfig2 = mongod.adminCommand({replSetGetConfig: 1});
+jsTestLog("rsConfig2: " + tojson(rsConfig2));
+assert.commandWorked(rsConfig2);
+assert.eq(tojson(rsConfig2.config.members), tojson(rsConfig1.config.members));
+
+// Make sure that configuration can not be applied manually as it is invalid and no override is
+// specified
+
+output = mongod.adminCommand({replSetReconfig: rsConfig1.config});
+jsTestLog("replSetReconfig output: " + tojson(output));
+assert.commandFailed(output);
+assert(output.errmsg.includes("Found split horizon configuration using IP"));
+
+MongoRunner.stopMongod(mongod);
+}());
\ No newline at end of file
diff --git a/src/mongo/db/repl/repl_set_config.cpp b/src/mongo/db/repl/repl_set_config.cpp
index c40b7428203..85284321c74 100644
--- a/src/mongo/db/repl/repl_set_config.cpp
+++ b/src/mongo/db/repl/repl_set_config.cpp
@@ -190,6 +190,14 @@ Status ReplSetConfig::_initialize(bool forInitiate,
 }
 
 Status ReplSetConfig::validate() const {
+    return _validate(false);
+}
+
+Status ReplSetConfig::validateAllowingSplitHorizonIP() const {
+    return _validate(true);
+}
+
+Status ReplSetConfig::_validate(bool allowSplitHorizonIP) const {
     if (getMembers().size() > kMaxMembers || getMembers().empty()) {
         return Status(ErrorCodes::BadValue,
                       str::stream() << "Replica set configuration contains " << getMembers().size()
@@ -220,7 +228,7 @@ Status ReplSetConfig::validate() const {
         const MemberConfig& memberI = getMembers()[i];
 
         // Check that no horizon mappings contain IP addresses
-        if (!disableSplitHorizonIPCheck) {
+        if (!disableSplitHorizonIPCheck && !allowSplitHorizonIP) {
             for (auto&& mapping : memberI.getHorizonMappings()) {
                 // Ignore the default horizon -- this can be an IP
                 if (mapping.first == SplitHorizon::kDefaultHorizon) {
diff --git a/src/mongo/db/repl/repl_set_config.h b/src/mongo/db/repl/repl_set_config.h
index 5148d0fa8ea..df9a0481d26 100644
--- a/src/mongo/db/repl/repl_set_config.h
+++ b/src/mongo/db/repl/repl_set_config.h
@@ -252,6 +252,12 @@ public:
     Status validate() const;
 
     /**
+     * Performs basic consistency checks on the replica set configuration, but does not fail on
+     * IP addresses in split horizon configuration
+     */
+    Status validateAllowingSplitHorizonIP() const;
+
+    /**
      * Checks if this configuration can satisfy the given write concern.
      *
      * Things that are taken into consideration include:
@@ -550,6 +556,11 @@ private:
     void _setRequiredFields();
 
     /**
+     * Performs basic consistency checks on the replica set configuration.
+     */
+    Status _validate(bool allowSplitHorizonIP) const;
+
+    /**
      * Common code used by constructors
      */
     Status _initialize(bool forInitiate,
diff --git a/src/mongo/db/repl/repl_set_config_checks.cpp b/src/mongo/db/repl/repl_set_config_checks.cpp
index e4da3276793..6f51fc82b38 100644
--- a/src/mongo/db/repl/repl_set_config_checks.cpp
+++ b/src/mongo/db/repl/repl_set_config_checks.cpp
@@ -375,7 +375,7 @@ StatusWith<int> findSelfInConfigIfElectable(ReplicationCoordinatorExternalState*
 StatusWith<int> validateConfigForStartUp(ReplicationCoordinatorExternalState* externalState,
                                          const ReplSetConfig& newConfig,
                                          ServiceContext* ctx) {
-    Status status = newConfig.validate();
+    Status status = newConfig.validateAllowingSplitHorizonIP();
     if (!status.isOK()) {
         return StatusWith<int>(status);
     }
@@ -426,10 +426,12 @@ StatusWith<int> validateConfigForInitiate(ReplicationCoordinatorExternalState* e
     return findSelfInConfigIfElectable(externalState, newConfig, ctx);
 }
 
-Status validateConfigForReconfig(const ReplSetConfig& oldConfig,
-                                 const ReplSetConfig& newConfig,
-                                 bool force) {
-    Status status = newConfig.validate();
+Status _validateConfigForReconfig(const ReplSetConfig& oldConfig,
+                                  const ReplSetConfig& newConfig,
+                                  bool force,
+                                  bool allowSplitHorizonIP) {
+    Status status =
+        allowSplitHorizonIP ? newConfig.validateAllowingSplitHorizonIP() : newConfig.validate();
     if (!status.isOK()) {
         return status;
     }
@@ -468,11 +470,23 @@ Status validateConfigForReconfig(const ReplSetConfig& oldConfig,
     return Status::OK();
 }
 
+Status validateConfigForReconfig(const ReplSetConfig& oldConfig,
+                                 const ReplSetConfig& newConfig,
+                                 bool force) {
+    return _validateConfigForReconfig(oldConfig, newConfig, force, false);
+}
+
+Status validateConfigForOplogReconfig(const ReplSetConfig& oldConfig,
+                                      const ReplSetConfig& newConfig) {
+    return _validateConfigForReconfig(oldConfig, newConfig, true, true);
+}
+
+
 StatusWith<int> validateConfigForHeartbeatReconfig(
     ReplicationCoordinatorExternalState* externalState,
     const ReplSetConfig& newConfig,
     ServiceContext* ctx) {
-    Status status = newConfig.validate();
+    Status status = newConfig.validateAllowingSplitHorizonIP();
     if (!status.isOK()) {
         return StatusWith<int>(status);
     }
diff --git a/src/mongo/db/repl/repl_set_config_checks.h b/src/mongo/db/repl/repl_set_config_checks.h
index 7b96c73f2ee..16d84d2e2ec 100644
--- a/src/mongo/db/repl/repl_set_config_checks.h
+++ b/src/mongo/db/repl/repl_set_config_checks.h
@@ -100,6 +100,16 @@ Status validateConfigForReconfig(const ReplSetConfig& oldConfig,
                                  bool force);
 
 /**
+ * Validates that "newConfig" is a legal successor configuration to "oldConfig" that can be
+ * initiated by the current node (identified via "externalState"). Ignores an error condition
+ * when an IP address exists in split horizon configuration
+ *
+ * Returns an indicative error on validation failure.
+ */
+Status validateConfigForOplogReconfig(const ReplSetConfig& oldConfig,
+                                      const ReplSetConfig& newConfig);
+
+/**
  * Validates that "newConfig" is an acceptable configuration when received in a heartbeat
  * reasponse.
  *
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index a7edca785fa..a7d83fa9558 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -3431,7 +3431,10 @@ Status ReplicationCoordinatorImpl::doReplSetReconfig(OperationContext* opCtx,
     BSONObj newConfigObj = newConfig.toBSON();
     audit::logReplSetReconfig(opCtx->getClient(), &oldConfigObj, &newConfigObj);
 
-    Status validateStatus = validateConfigForReconfig(oldConfig, newConfig, force);
+    bool isManualReconfig = opCtx->getClient()->hasRemote();
+    Status validateStatus = isManualReconfig
+        ? validateConfigForReconfig(oldConfig, newConfig, force)
+        : validateConfigForOplogReconfig(oldConfig, newConfig);
     if (!validateStatus.isOK()) {
         LOGV2_ERROR(21420,
                     "replSetReconfig got {error} while validating {newConfig}",