From 700850a9705d814e3799d87d7ce6978b169eb0c9 Mon Sep 17 00:00:00 2001
From: Nicolae Vartolomei <nv@cloudflare.com>
Date: Mon, 24 May 2021 13:34:39 +0100
Subject: [PATCH] Add more tests and handle correctly missing include

---
 src/Common/Config/ConfigProcessor.cpp           |  5 +++++
 .../configs/config_env.xml                      |  3 +++
 .../configs/config_incl.xml                     |  5 ++++-
 .../configs/config_include_from_env.xml         |  2 ++
 .../configs/include_from_source.xml             | 17 +++++++++++++++++
 .../configs/max_query_size.xml                  |  3 ---
 .../test_config_substitutions/test.py           | 13 ++++++++++---
 7 files changed, 41 insertions(+), 7 deletions(-)
 create mode 100644 tests/integration/test_config_substitutions/configs/include_from_source.xml
 delete mode 100644 tests/integration/test_config_substitutions/configs/max_query_size.xml

diff --git a/src/Common/Config/ConfigProcessor.cpp b/src/Common/Config/ConfigProcessor.cpp
index 851d8d9d250..5af13d8a9df 100644
--- a/src/Common/Config/ConfigProcessor.cpp
+++ b/src/Common/Config/ConfigProcessor.cpp
@@ -318,7 +318,12 @@ void ConfigProcessor::doIncludesRecursive(
             else if (throw_on_bad_incl)
                 throw Poco::Exception(error_msg + name);
             else
+            {
+                if (node->nodeName() == "include")
+                    node->parentNode()->removeChild(node);
+
                 LOG_WARNING(log, "{}{}", error_msg, name);
+            }
         }
         else
         {
diff --git a/tests/integration/test_config_substitutions/configs/config_env.xml b/tests/integration/test_config_substitutions/configs/config_env.xml
index 712855c47c0..2d63b9c688d 100644
--- a/tests/integration/test_config_substitutions/configs/config_env.xml
+++ b/tests/integration/test_config_substitutions/configs/config_env.xml
@@ -10,5 +10,8 @@
           <profile>default</profile>
           <quota>default</quota>
       </default>
+
+      <include incl="users_1" />
+      <include incl="users_2" />
   </users>
 </yandex>
diff --git a/tests/integration/test_config_substitutions/configs/config_incl.xml b/tests/integration/test_config_substitutions/configs/config_incl.xml
index 383a23af1ff..43ec78ff8ef 100644
--- a/tests/integration/test_config_substitutions/configs/config_incl.xml
+++ b/tests/integration/test_config_substitutions/configs/config_incl.xml
@@ -1,5 +1,5 @@
 <yandex>
-  <include_from>/etc/clickhouse-server/config.d/max_query_size.xml</include_from>
+  <include_from>/etc/clickhouse-server/config.d/include_from_source.xml</include_from>
   <profiles>
     <default>
         <max_query_size incl="mqs" />
@@ -11,5 +11,8 @@
           <profile>default</profile>
           <quota>default</quota>
       </default>
+
+      <include incl="users_1" />
+      <include incl="users_2" />
   </users>
 </yandex>
diff --git a/tests/integration/test_config_substitutions/configs/config_include_from_env.xml b/tests/integration/test_config_substitutions/configs/config_include_from_env.xml
index 71e11235749..79b650f3d9e 100644
--- a/tests/integration/test_config_substitutions/configs/config_include_from_env.xml
+++ b/tests/integration/test_config_substitutions/configs/config_include_from_env.xml
@@ -11,5 +11,7 @@
           <profile>default</profile>
           <quota>default</quota>
       </default>
+
+      <include incl="node_does_not_exist" />
   </users>
 </yandex>
diff --git a/tests/integration/test_config_substitutions/configs/include_from_source.xml b/tests/integration/test_config_substitutions/configs/include_from_source.xml
new file mode 100644
index 00000000000..6095180bb59
--- /dev/null
+++ b/tests/integration/test_config_substitutions/configs/include_from_source.xml
@@ -0,0 +1,17 @@
+<yandex>
+    <mqs>99999</mqs>
+
+    <users_1>
+        <user_1>
+            <password></password>
+            <profile>default</profile>
+        </user_1>
+    </users_1>
+
+    <users_2>
+        <user_2>
+            <password></password>
+            <profile>default</profile>
+        </user_2>
+    </users_2>
+</yandex>
diff --git a/tests/integration/test_config_substitutions/configs/max_query_size.xml b/tests/integration/test_config_substitutions/configs/max_query_size.xml
deleted file mode 100644
index 9ec61368be9..00000000000
--- a/tests/integration/test_config_substitutions/configs/max_query_size.xml
+++ /dev/null
@@ -1,3 +0,0 @@
-<yandex>
-  <mqs>99999</mqs>
-</yandex>
diff --git a/tests/integration/test_config_substitutions/test.py b/tests/integration/test_config_substitutions/test.py
index 47154efec36..aec3f1d3635 100644
--- a/tests/integration/test_config_substitutions/test.py
+++ b/tests/integration/test_config_substitutions/test.py
@@ -8,11 +8,11 @@ node2 = cluster.add_instance('node2', user_configs=['configs/config_env.xml'],
                              env_variables={"MAX_QUERY_SIZE": "55555"})
 node3 = cluster.add_instance('node3', user_configs=['configs/config_zk.xml'], with_zookeeper=True)
 node4 = cluster.add_instance('node4', user_configs=['configs/config_incl.xml'],
-                             main_configs=['configs/max_query_size.xml'])  # include value 77777
+                             main_configs=['configs/include_from_source.xml'])  # include value 77777
 node5 = cluster.add_instance('node5', user_configs=['configs/config_allow_databases.xml'])
 node6 = cluster.add_instance('node6', user_configs=['configs/config_include_from_env.xml'],
-                             env_variables={"INCLUDE_FROM_ENV": "/etc/clickhouse-server/config.d/max_query_size.xml"},
-                             main_configs=['configs/max_query_size.xml'])
+                             env_variables={"INCLUDE_FROM_ENV": "/etc/clickhouse-server/config.d/include_from_source.xml"},
+                             main_configs=['configs/include_from_source.xml'])
 
 
 @pytest.fixture(scope="module")
@@ -40,6 +40,13 @@ def test_config(start_cluster):
 
 
 def test_include_config(start_cluster):
+    # <include incl="source tag" />
+    assert node4.query("select 1")
+    assert node4.query("select 1", user="user_1")
+    assert node4.query("select 1", user="user_2")
+
+    # <include from_zk="zk path />
+    assert node3.query("select 1")
     assert node3.query("select 1", user="user_1")
     assert node3.query("select 1", user="user_2")