Merge pull request #8253 from nvartolomei/nv/constraints-coverage

Improve settings constraints tests coverage

dbms/tests/integration/helpers/cluster.py

@@ -625,8 +625,21 @@ class ClickHouseInstance:
         return self.client.query_and_get_answer_with_error(sql, stdin, timeout, settings, user)
 
     # Connects to the instance via HTTP interface, sends a query and returns the answer
-    def http_query(self, sql, data=None):
-        return urllib.urlopen("http://" + self.ip_address + ":8123/?query=" + urllib.quote(sql, safe=''), data).read()
+    def http_query(self, sql, data=None, params=None, user=None):
+        if params is None:
+            params = {}
+        else:
+            params = params.copy()
+
+        params["query"] = sql
+
+        auth = ""
+        if user:
+            auth = "{}@".format(user)
+
+        url = "http://" + auth + self.ip_address + ":8123/?" + urllib.urlencode(params)
+
+        return urllib.urlopen(url, data).read()
 
     def restart_clickhouse(self, stop_start_wait_sec=5, kill=False):
         if not self.stay_alive:

dbms/tests/integration/test_settings_constraints/configs/users.xml

@@ -14,6 +14,14 @@
                </force_index_by_date>
             </constraints>
         </default>
+
+        <readonly_profile>
+            <readonly>1</readonly>
+        </readonly_profile>
+
+        <no_dll_profile>
+            <allow_ddl>0</allow_ddl>
+        </no_dll_profile>
     </profiles>
 
     <users>
@@ -25,6 +33,22 @@
             <profile>default</profile>
             <quota>default</quota>
         </default>
+        <readonly_user>
+            <password></password>
+            <networks incl="networks" replace="replace">
+                <ip>::/0</ip>
+            </networks>
+            <profile>readonly_profile</profile>
+            <quota>default</quota>
+        </readonly_user>
+        <no_dll_user>
+            <password></password>
+            <networks incl="networks" replace="replace">
+                <ip>::/0</ip>
+            </networks>
+            <profile>no_dll_profile</profile>
+            <quota>default</quota>
+        </no_dll_user>
     </users>
 
     <quotas>

dbms/tests/integration/test_settings_constraints/test.py

@@ -29,68 +29,112 @@ def test_system_settings(started_cluster):
            "readonly\t0\t\\N\t\\N\t0\n"
 
 
-def test_read_only_constraint(started_cluster):
-    # Change a setting for session with SET.
-    assert instance.query("SELECT value FROM system.settings WHERE name='force_index_by_date'") ==\
-           "0\n"
-
-    expected_error = "Setting force_index_by_date should not be changed"
-    assert expected_error in instance.query_and_get_error("SET force_index_by_date=1")
-
-    # Change a setting for query with SETTINGS.
-    assert instance.query("SELECT value FROM system.settings WHERE name='force_index_by_date'") ==\
-           "0\n"
+def test_system_constraints(started_cluster):
+    assert_query_settings(instance, "SELECT 1",
+                          settings={'readonly': 0},
+                          exception="Cannot modify 'readonly'",
+                          user="readonly_user")
 
-    assert expected_error in instance.query_and_get_error(
-           "SELECT value FROM system.settings WHERE name='force_index_by_date' "
-           "SETTINGS force_index_by_date=1")
+    assert_query_settings(instance, "SELECT 1",
+                          settings={'allow_ddl': 1},
+                          exception="Cannot modify 'allow_ddl'",
+                          user="no_dll_user")
 
 
-def test_min_constraint(started_cluster):
-    # Change a setting for session with SET.
-    assert instance.query("SELECT value FROM system.settings WHERE name='max_memory_usage'") ==\
-           "10000000000\n"
+def test_read_only_constraint(started_cluster):
+    # Default value
+    assert_query_settings(instance, "SELECT value FROM system.settings WHERE name='force_index_by_date'",
+                          settings={},
+                          result="0")
 
-    assert instance.query("SET max_memory_usage=5000000000;\n"
-                          "SELECT value FROM system.settings WHERE name='max_memory_usage'") ==\
-           "5000000000\n"
+    # Invalid value
+    assert_query_settings(instance, "SELECT value FROM system.settings WHERE name='force_index_by_date'",
+                          settings={'force_index_by_date': 1},
+                          result=None,
+                          exception="Setting force_index_by_date should not be changed")
 
-    expected_error = "Setting max_memory_usage shouldn't be less than 5000000000"
-    assert expected_error in instance.query_and_get_error("SET max_memory_usage=4999999999")
 
-    # Change a setting for query with SETTINGS.
-    assert instance.query("SELECT value FROM system.settings WHERE name='max_memory_usage'") ==\
-           "10000000000\n"
+def test_min_constraint(started_cluster):
+    # Default value
+    assert_query_settings(instance, "SELECT value FROM system.settings WHERE name='max_memory_usage'",
+                          {},
+                          result="10000000000")
 
-    assert instance.query("SET max_memory_usage=5000000001;\n"
-                          "SELECT value FROM system.settings WHERE name='max_memory_usage'") ==\
-           "5000000001\n"
+    # Valid value
+    assert_query_settings(instance, "SELECT value FROM system.settings WHERE name='max_memory_usage'",
+                          settings={'max_memory_usage': 5000000000},
+                          result="5000000000")
 
-    assert expected_error in instance.query_and_get_error(
-           "SELECT value FROM system.settings WHERE name='max_memory_usage' "
-           "SETTINGS max_memory_usage=4999999999")
+    # Invalid value
+    assert_query_settings(instance, "SELECT value FROM system.settings WHERE name='max_memory_usage'",
+                          settings={'max_memory_usage': 4999999999},
+                          result=None,
+                          exception="Setting max_memory_usage shouldn't be less than 5000000000")
 
 
 def test_max_constraint(started_cluster):
-    # Change a setting for session with SET.
-    assert instance.query("SELECT value FROM system.settings WHERE name='max_memory_usage'") ==\
-           "10000000000\n"
-
-    assert instance.query("SET max_memory_usage=20000000000;\n"
-                          "SELECT value FROM system.settings WHERE name='max_memory_usage'") ==\
-           "20000000000\n"
-
-    expected_error = "Setting max_memory_usage shouldn't be greater than 20000000000"
-    assert expected_error in instance.query_and_get_error("SET max_memory_usage=20000000001")
-
-     # Change a setting for query with SETTINGS.
-    assert instance.query("SELECT value FROM system.settings WHERE name='max_memory_usage'") ==\
-           "10000000000\n"
-
-    assert instance.query("SELECT value FROM system.settings WHERE name='max_memory_usage' "
-                          "SETTINGS max_memory_usage=19999999999") == "19999999999\n"
-
-    assert expected_error in instance.query_and_get_error(
-           "SELECT value FROM system.settings WHERE name='max_memory_usage' "
-           "SETTINGS max_memory_usage=20000000001")
- 
\ No newline at end of file
+    # Default value
+    assert_query_settings(instance, "SELECT value FROM system.settings WHERE name='max_memory_usage'",
+                          {},
+                          result="10000000000")
+
+    # Valid value
+    assert_query_settings(instance, "SELECT value FROM system.settings WHERE name='max_memory_usage'",
+                          settings={'max_memory_usage': 20000000000},
+                          result="20000000000")
+
+    # Invalid value
+    assert_query_settings(instance, "SELECT value FROM system.settings WHERE name='max_memory_usage'",
+                          settings={'max_memory_usage': 20000000001},
+                          result=None,
+                          exception="Setting max_memory_usage shouldn't be greater than 20000000000")
+
+
+def assert_query_settings(instance, query, settings, result=None, exception=None, user=None):
+    """
+    Try and send the query with custom settings via all available methods:
+    1. TCP Protocol with settings packet
+    2. HTTP Protocol with settings params
+    3. TCP Protocol with session level settings
+    4. TCP Protocol with query level settings
+    """
+
+    if not settings:
+        settings = {}
+
+    # tcp level settings
+    if exception:
+        assert exception in instance.query_and_get_error(query, settings=settings, user=user)
+    else:
+        assert instance.query(query, settings=settings, user=user).strip() == result
+
+    # http level settings
+    if exception:
+        assert exception in instance.http_query(query, params=settings, user=user)
+    else:
+        assert instance.http_query(query, params=settings, user=user).strip() == result
+
+    # session level settings
+    queries = ""
+
+    for k, v in settings.items():
+        queries += "SET {}={};\n".format(k, v)
+
+    queries += query
+
+    if exception:
+        assert exception in instance.query_and_get_error(queries, user=user)
+    else:
+        assert instance.query(queries, user=user).strip() == result
+
+    if settings:
+        query += " SETTINGS "
+        for ix, (k, v) in enumerate(settings.items()):
+            query += "{} = {}".format(k, v)
+            if ix != len(settings) - 1:
+                query += ", "
+
+    if exception:
+        assert exception in instance.query_and_get_error(queries, user=user)
+    else:
+        assert instance.query(queries, user=user).strip() == result