From b6d61318d07f5615a9e319be1f01aa83b56637e8 Mon Sep 17 00:00:00 2001
From: alesapin <alesapin@gmail.com>
Date: Sun, 18 Sep 2022 00:04:16 +0200
Subject: [PATCH] One more validation for broken config in keeper

---
 src/Coordination/KeeperStateManager.cpp       |  9 ++++++
 .../test_keeper_incorrect_config/test.py      | 32 +++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/src/Coordination/KeeperStateManager.cpp b/src/Coordination/KeeperStateManager.cpp
index 3d7f5f2fb34..9b6aab5533e 100644
--- a/src/Coordination/KeeperStateManager.cpp
+++ b/src/Coordination/KeeperStateManager.cpp
@@ -6,6 +6,7 @@
 #include <Common/Exception.h>
 #include <Common/isLocalAddress.h>
 #include <IO/ReadHelpers.h>
+#include <Common/getMultipleKeysFromConfig.h>
 
 namespace DB
 {
@@ -94,6 +95,14 @@ KeeperStateManager::parseServersConfiguration(const Poco::Util::AbstractConfigur
             continue;
 
         std::string full_prefix = config_prefix + ".raft_configuration." + server_key;
+
+        if (getMultipleValuesFromConfig(config, full_prefix, "id").size() > 1
+            || getMultipleValuesFromConfig(config, full_prefix, "hostname").size() > 1
+            || getMultipleValuesFromConfig(config, full_prefix, "port").size() > 1)
+        {
+            throw Exception(ErrorCodes::RAFT_ERROR, "Multiple <id> or <hostname> or <port> specified for a single <server>");
+        }
+
         int new_server_id = config.getInt(full_prefix + ".id");
         std::string hostname = config.getString(full_prefix + ".hostname");
         int port = config.getInt(full_prefix + ".port");
diff --git a/tests/integration/test_keeper_incorrect_config/test.py b/tests/integration/test_keeper_incorrect_config/test.py
index e0a28b00b4f..cedb195a6e0 100644
--- a/tests/integration/test_keeper_incorrect_config/test.py
+++ b/tests/integration/test_keeper_incorrect_config/test.py
@@ -172,6 +172,37 @@ NORMAL_CONFIG = """
 </clickhouse>
 """
 
+JUST_WRONG_CONFIG = """
+<clickhouse>
+    <keeper_server>
+        <tcp_port>9181</tcp_port>
+        <server_id>1</server_id>
+        <log_storage_path>/var/lib/clickhouse/coordination/log</log_storage_path>
+        <snapshot_storage_path>/var/lib/clickhouse/coordination/snapshots</snapshot_storage_path>
+
+        <coordination_settings>
+            <operation_timeout_ms>5000</operation_timeout_ms>
+            <session_timeout_ms>10000</session_timeout_ms>
+            <raft_logs_level>trace</raft_logs_level>
+        </coordination_settings>
+
+        <raft_configuration>
+            <server>
+                <id>1</id>
+                <hostname>node1</hostname>
+                <port>9234</port>
+                <id>2</id>
+                <hostname>node2</hostname>
+                <port>9234</port>
+                <id>3</id>
+                <hostname>node3</hostname>
+                <port>9234</port>
+            </server>
+        </raft_configuration>
+    </keeper_server>
+</clickhouse>
+"""
+
 
 def test_duplicate_endpoint(started_cluster):
     node1.stop_clickhouse()
@@ -187,6 +218,7 @@ def test_duplicate_endpoint(started_cluster):
     assert_config_fails(DUPLICATE_ID_CONFIG)
     assert_config_fails(LOCALHOST_WITH_REMOTE)
     assert_config_fails(MULTIPLE_LOCAL_WITH_REMOTE)
+    assert_config_fails(JUST_WRONG_CONFIG)
 
     node1.replace_config(
         "/etc/clickhouse-server/config.d/enable_keeper1.xml", NORMAL_CONFIG