From 51b6dd67b06d9cc0c3cc09b7de4f45aaff3a1aad Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Fri, 7 Dec 2018 04:40:35 +0300 Subject: [PATCH 01/11] Match the process' effective user id against the data owner at the server startup. --- .../__init__.py | 0 .../configs/config.xml | 11 ++++++ .../configs/no_such_directory.xml | 3 ++ .../configs/owner_mismatch.xml | 3 ++ .../configs/users.xml | 3 ++ .../test.py | 37 +++++++++++++++++++ libs/libdaemon/src/BaseDaemon.cpp | 34 +++++++++++++++++ 7 files changed, 91 insertions(+) create mode 100644 dbms/tests/integration/test_match_process_uid_against_data_owner/__init__.py create mode 100644 dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml create mode 100644 dbms/tests/integration/test_match_process_uid_against_data_owner/configs/no_such_directory.xml create mode 100644 dbms/tests/integration/test_match_process_uid_against_data_owner/configs/owner_mismatch.xml create mode 100644 dbms/tests/integration/test_match_process_uid_against_data_owner/configs/users.xml create mode 100644 dbms/tests/integration/test_match_process_uid_against_data_owner/test.py diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/__init__.py b/dbms/tests/integration/test_match_process_uid_against_data_owner/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml new file mode 100644 index 00000000000..48aa82349d3 --- /dev/null +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml @@ -0,0 +1,11 @@ + + + + /var/log/clickhouse-server/log.log + /var/log/clickhouse-server/clickhouse-server.err.log + /var/log/clickhouse-server/stderr.log + /var/log/clickhouse-server/stdout.log + + + users.xml + diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/no_such_directory.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/no_such_directory.xml new file mode 100644 index 00000000000..80ddf7c4722 --- /dev/null +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/no_such_directory.xml @@ -0,0 +1,3 @@ + + /no_such_directory/data/ + diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/owner_mismatch.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/owner_mismatch.xml new file mode 100644 index 00000000000..46d2dcc49ee --- /dev/null +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/owner_mismatch.xml @@ -0,0 +1,3 @@ + + /root/data/ + diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/users.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/users.xml new file mode 100644 index 00000000000..9aba4ac0914 --- /dev/null +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/users.xml @@ -0,0 +1,3 @@ + + + diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py b/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py new file mode 100644 index 00000000000..56c40530c36 --- /dev/null +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py @@ -0,0 +1,37 @@ +import os +import pwd +import pytest +import re + +from helpers.cluster import ClickHouseCluster + + +def expect_failure_with_message(config, expected_message): + cluster = ClickHouseCluster(__file__) + node = cluster.add_instance('node', main_configs=[config], with_zookeeper=False) + + with pytest.raises(Exception): + cluster.start() + + cluster.shutdown() # cleanup + + with open(os.path.join(node.path, 'logs/stderr.log')) as log: + last_message = log.readlines()[-1].strip() + if re.search(expected_message, last_message) is None: + pytest.fail('Expected the server to fail with a message "{}", but the last message is "{}"'.format(expected_message, last_message)) + + +def test_no_such_directory(): + expect_failure_with_message('configs/no_such_directory.xml', 'Failed to stat.*no_such_directory') + + +def test_different_user(): + current_user_id = os.getuid() + + if current_user_id != 0: + current_user_name = pwd.getpwuid(current_user_id).pw_name + + expect_failure_with_message( + 'configs/owner_mismatch.xml', + 'Effective user of the process \(({}|{})\) does not match the owner of the data \((0|root)\)'.format(current_user_id, current_user_name), + ) diff --git a/libs/libdaemon/src/BaseDaemon.cpp b/libs/libdaemon/src/BaseDaemon.cpp index bad38c78529..ff06735e04e 100644 --- a/libs/libdaemon/src/BaseDaemon.cpp +++ b/libs/libdaemon/src/BaseDaemon.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #if USE_UNWIND @@ -577,6 +578,17 @@ static bool tryCreateDirectories(Poco::Logger * logger, const std::string & path } +static std::string getUserName(uid_t userId) { + /// Try to convert user id into user name. + const struct passwd * result = getpwuid(userId); + if (errno) + throw Poco::SystemException("Failed to get user name for " + DB::toString(userId)); + else if (result) + return result->pw_name; + return DB::toString(userId); +} + + void BaseDaemon::reloadConfiguration() { /** If the program is not run in daemon mode and 'config-file' is not specified, @@ -898,6 +910,28 @@ void BaseDaemon::initialize(Application & self) umask(umask_num); } + std::string path = config().getString("path", DBMS_DEFAULT_PATH); + + /// Check that the process' user id matches the owner of the data. + const auto effectiveUserId = geteuid(); + struct stat statbuf; + if (stat(path.c_str(), &statbuf)) { + const auto parent = Poco::Path(path).parent().toString(); + if (stat(parent.c_str(), &statbuf)) { + throw Poco::SystemException("Failed to stat data path " + parent); + } + } + if (effectiveUserId != statbuf.st_uid) + { + const auto effectiveUser = getUserName(effectiveUserId); + const auto dataOwner = getUserName(statbuf.st_uid); + std::string message = "Effective user of the process (" + effectiveUser + + ") does not match the owner of the data (" + dataOwner + ")."; + if (effectiveUserId == 0) + message += " Run under 'sudo -u " + dataOwner + "'."; + throw Poco::SystemException(message); + } + DB::ConfigProcessor(config_path).savePreprocessedConfig(loaded_config, ""); /// Write core dump on crash. From 42de08f3b92671b1c9609f8620a4123201933a03 Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Mon, 10 Dec 2018 01:15:59 +0300 Subject: [PATCH 02/11] Move euid check back to Server.cpp. Use getpwnam_r instead of getpwnam. Fix style. --- dbms/programs/server/Server.cpp | 64 +++++++++++++++++++++++-------- dbms/src/Common/ErrorCodes.cpp | 3 ++ libs/libdaemon/src/BaseDaemon.cpp | 34 ---------------- 3 files changed, 52 insertions(+), 49 deletions(-) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index f965bf58eaa..c7e44b60f56 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -2,7 +2,11 @@ #include #include +#include +#include #include +#include +#include #include #include #include @@ -70,6 +74,9 @@ namespace ErrorCodes extern const int EXCESSIVE_ELEMENT_IN_CONFIG; extern const int INVALID_CONFIG_PARAMETER; extern const int SYSTEM_ERROR; + extern const int FAILED_TO_STAT_DATA; + extern const int FAILED_TO_GETPWUID; + extern const int MISMATCHING_USERS_FOR_PROCESS_AND_DATA; } @@ -83,6 +90,25 @@ static std::string getCanonicalPath(std::string && path) return std::move(path); } +static std::string getUserName(uid_t user_id) { + /// Try to convert user id into user name. + auto buffer_size = sysconf(_SC_GETPW_R_SIZE_MAX); + if (buffer_size <= 0) + buffer_size = 32; + std::string buffer; + buffer.reserve(buffer_size); + + struct passwd passwd_entry; + struct passwd * result = nullptr; + const auto error = getpwuid_r(user_id, &passwd_entry, buffer.data(), buffer_size, &result); + + if (error) + throwFromErrno("Failed to find user name for " + toString(user_id), ErrorCodes::FAILED_TO_GETPWUID, error); + else if (result) + return result->pw_name; + return toString(user_id); +} + void Server::uninitialize() { logger().information("shutting down"); @@ -166,6 +192,22 @@ int Server::main(const std::vector & /*args*/) std::string path = getCanonicalPath(config().getString("path", DBMS_DEFAULT_PATH)); std::string default_database = config().getString("default_database", "default"); + /// Check that the process' user id matches the owner of the data. + const auto effective_user_id = geteuid(); + struct stat statbuf; + const auto effective_user = getUserName(effective_user_id); + LOG_INFO(log, "effective_user = " + effective_user); + if (stat(path.c_str(), &statbuf) == 0 && effective_user_id != statbuf.st_uid) + { + const auto effective_user = getUserName(effective_user_id); + const auto data_owner = getUserName(statbuf.st_uid); + std::string message = "Effective user of the process (" + effective_user + + ") does not match the owner of the data (" + data_owner + ")."; + if (effective_user_id == 0) + message += " Run under 'sudo -u " + data_owner + "'."; + throw Exception(message, ErrorCodes::MISMATCHING_USERS_FOR_PROCESS_AND_DATA); + } + global_context->setPath(path); /// Create directories for 'path' and for default database, if not exist. @@ -376,21 +418,13 @@ int Server::main(const std::vector & /*args*/) format_schema_path.createDirectories(); LOG_INFO(log, "Loading metadata."); - try - { - loadMetadataSystem(*global_context); - /// After attaching system databases we can initialize system log. - global_context->initializeSystemLogs(); - /// After the system database is created, attach virtual system tables (in addition to query_log and part_log) - attachSystemTablesServer(*global_context->getDatabase("system"), has_zookeeper); - /// Then, load remaining databases - loadMetadata(*global_context); - } - catch (...) - { - tryLogCurrentException(log, "Caught exception while loading metadata"); - throw; - } + loadMetadataSystem(*global_context); + /// After attaching system databases we can initialize system log. + global_context->initializeSystemLogs(); + /// After the system database is created, attach virtual system tables (in addition to query_log and part_log) + attachSystemTablesServer(*global_context->getDatabase("system"), has_zookeeper); + /// Then, load remaining databases + loadMetadata(*global_context); LOG_DEBUG(log, "Loaded metadata."); global_context->setCurrentDatabase(default_database); diff --git a/dbms/src/Common/ErrorCodes.cpp b/dbms/src/Common/ErrorCodes.cpp index e5b6028594b..f8d6b58eef7 100644 --- a/dbms/src/Common/ErrorCodes.cpp +++ b/dbms/src/Common/ErrorCodes.cpp @@ -402,6 +402,9 @@ namespace ErrorCodes extern const int SYSTEM_ERROR = 425; extern const int NULL_POINTER_DEREFERENCE = 426; extern const int CANNOT_COMPILE_REGEXP = 427; + extern const int FAILED_TO_STAT_DATA = 428; + extern const int FAILED_TO_GETPWUID = 429; + extern const int MISMATCHING_USERS_FOR_PROCESS_AND_DATA = 430; extern const int KEEPER_EXCEPTION = 999; extern const int POCO_EXCEPTION = 1000; diff --git a/libs/libdaemon/src/BaseDaemon.cpp b/libs/libdaemon/src/BaseDaemon.cpp index ff06735e04e..bad38c78529 100644 --- a/libs/libdaemon/src/BaseDaemon.cpp +++ b/libs/libdaemon/src/BaseDaemon.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #if USE_UNWIND @@ -578,17 +577,6 @@ static bool tryCreateDirectories(Poco::Logger * logger, const std::string & path } -static std::string getUserName(uid_t userId) { - /// Try to convert user id into user name. - const struct passwd * result = getpwuid(userId); - if (errno) - throw Poco::SystemException("Failed to get user name for " + DB::toString(userId)); - else if (result) - return result->pw_name; - return DB::toString(userId); -} - - void BaseDaemon::reloadConfiguration() { /** If the program is not run in daemon mode and 'config-file' is not specified, @@ -910,28 +898,6 @@ void BaseDaemon::initialize(Application & self) umask(umask_num); } - std::string path = config().getString("path", DBMS_DEFAULT_PATH); - - /// Check that the process' user id matches the owner of the data. - const auto effectiveUserId = geteuid(); - struct stat statbuf; - if (stat(path.c_str(), &statbuf)) { - const auto parent = Poco::Path(path).parent().toString(); - if (stat(parent.c_str(), &statbuf)) { - throw Poco::SystemException("Failed to stat data path " + parent); - } - } - if (effectiveUserId != statbuf.st_uid) - { - const auto effectiveUser = getUserName(effectiveUserId); - const auto dataOwner = getUserName(statbuf.st_uid); - std::string message = "Effective user of the process (" + effectiveUser + - ") does not match the owner of the data (" + dataOwner + ")."; - if (effectiveUserId == 0) - message += " Run under 'sudo -u " + dataOwner + "'."; - throw Poco::SystemException(message); - } - DB::ConfigProcessor(config_path).savePreprocessedConfig(loaded_config, ""); /// Write core dump on crash. From 93df4306af6e0777b28aed04f8e94ebfa24a1450 Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Mon, 10 Dec 2018 20:42:33 +0300 Subject: [PATCH 03/11] Fix style. --- dbms/programs/server/Server.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index c7e44b60f56..01d792fb8d2 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -90,7 +90,8 @@ static std::string getCanonicalPath(std::string && path) return std::move(path); } -static std::string getUserName(uid_t user_id) { +static std::string getUserName(uid_t user_id) +{ /// Try to convert user id into user name. auto buffer_size = sysconf(_SC_GETPW_R_SIZE_MAX); if (buffer_size <= 0) From b56de46b7974a11c0a8129f3117146c9c9d2adc5 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Mon, 10 Dec 2018 22:35:20 +0300 Subject: [PATCH 04/11] Update Server.cpp --- dbms/programs/server/Server.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index c7e44b60f56..01d792fb8d2 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -90,7 +90,8 @@ static std::string getCanonicalPath(std::string && path) return std::move(path); } -static std::string getUserName(uid_t user_id) { +static std::string getUserName(uid_t user_id) +{ /// Try to convert user id into user name. auto buffer_size = sysconf(_SC_GETPW_R_SIZE_MAX); if (buffer_size <= 0) From df9833fd3f0cd4b35f73cc78c7e037728057b748 Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Tue, 11 Dec 2018 02:18:16 +0300 Subject: [PATCH 05/11] Remove no longer used error code. --- dbms/programs/server/Server.cpp | 1 - dbms/src/Common/ErrorCodes.cpp | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index 01d792fb8d2..cd107a4b5e0 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -74,7 +74,6 @@ namespace ErrorCodes extern const int EXCESSIVE_ELEMENT_IN_CONFIG; extern const int INVALID_CONFIG_PARAMETER; extern const int SYSTEM_ERROR; - extern const int FAILED_TO_STAT_DATA; extern const int FAILED_TO_GETPWUID; extern const int MISMATCHING_USERS_FOR_PROCESS_AND_DATA; } diff --git a/dbms/src/Common/ErrorCodes.cpp b/dbms/src/Common/ErrorCodes.cpp index f8d6b58eef7..0522382196a 100644 --- a/dbms/src/Common/ErrorCodes.cpp +++ b/dbms/src/Common/ErrorCodes.cpp @@ -402,9 +402,8 @@ namespace ErrorCodes extern const int SYSTEM_ERROR = 425; extern const int NULL_POINTER_DEREFERENCE = 426; extern const int CANNOT_COMPILE_REGEXP = 427; - extern const int FAILED_TO_STAT_DATA = 428; - extern const int FAILED_TO_GETPWUID = 429; - extern const int MISMATCHING_USERS_FOR_PROCESS_AND_DATA = 430; + extern const int FAILED_TO_GETPWUID = 428; + extern const int MISMATCHING_USERS_FOR_PROCESS_AND_DATA = 429; extern const int KEEPER_EXCEPTION = 999; extern const int POCO_EXCEPTION = 1000; From 30acd5000e6e22aa46f4fa302018f0afb05dd10b Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Tue, 11 Dec 2018 02:21:03 +0300 Subject: [PATCH 06/11] Fix bad merge. --- dbms/programs/server/Server.cpp | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index cd107a4b5e0..ad3de00cf46 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -418,13 +418,21 @@ int Server::main(const std::vector & /*args*/) format_schema_path.createDirectories(); LOG_INFO(log, "Loading metadata."); - loadMetadataSystem(*global_context); - /// After attaching system databases we can initialize system log. - global_context->initializeSystemLogs(); - /// After the system database is created, attach virtual system tables (in addition to query_log and part_log) - attachSystemTablesServer(*global_context->getDatabase("system"), has_zookeeper); - /// Then, load remaining databases - loadMetadata(*global_context); + try + { + loadMetadataSystem(*global_context); + /// After attaching system databases we can initialize system log. + global_context->initializeSystemLogs(); + /// After the system database is created, attach virtual system tables (in addition to query_log and part_log) + attachSystemTablesServer(*global_context->getDatabase("system"), has_zookeeper); + /// Then, load remaining databases + loadMetadata(*global_context); + } + catch (...) + { + tryLogCurrentException(log, "Caught exception while loading metadata"); + throw; + } LOG_DEBUG(log, "Loaded metadata."); global_context->setCurrentDatabase(default_database); From c9b984285d421b362b3cd3ab43f435b113e22022 Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Tue, 11 Dec 2018 02:32:21 +0300 Subject: [PATCH 07/11] Fail on user mismatch under root only. Just warn under non root user. --- dbms/programs/server/Server.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index ad3de00cf46..715e448c262 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -204,8 +204,14 @@ int Server::main(const std::vector & /*args*/) std::string message = "Effective user of the process (" + effective_user + ") does not match the owner of the data (" + data_owner + ")."; if (effective_user_id == 0) + { message += " Run under 'sudo -u " + data_owner + "'."; - throw Exception(message, ErrorCodes::MISMATCHING_USERS_FOR_PROCESS_AND_DATA); + throw Exception(message, ErrorCodes::MISMATCHING_USERS_FOR_PROCESS_AND_DATA); + } + else + { + LOG_WARNING(log, message); + } } global_context->setPath(path); From f9869b7d7ecb95c60aee4fffb7fc2ab30249a6cc Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Tue, 11 Dec 2018 02:36:20 +0300 Subject: [PATCH 08/11] Fix default buffer_size for getUserName(). Use real sysconf(_SC_GETPW_R_SIZE_MAX) value. --- dbms/programs/server/Server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index 715e448c262..ecc61e125f1 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -94,7 +94,7 @@ static std::string getUserName(uid_t user_id) /// Try to convert user id into user name. auto buffer_size = sysconf(_SC_GETPW_R_SIZE_MAX); if (buffer_size <= 0) - buffer_size = 32; + buffer_size = 1024; std::string buffer; buffer.reserve(buffer_size); From acb8cf1849b677753237e7faac9c5f41b78665a8 Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Wed, 19 Dec 2018 23:29:52 +0300 Subject: [PATCH 09/11] Fix test. --- .../configs/config.xml | 3 +++ .../configs/users.xml | 20 +++++++++++++++ .../test.py | 25 +++++++++++-------- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml index 48aa82349d3..032b8874af2 100644 --- a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml @@ -7,5 +7,8 @@ /var/log/clickhouse-server/stdout.log + /var/lib/clickhouse/ + + 5368709120 users.xml diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/users.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/users.xml index 9aba4ac0914..6061af8e33d 100644 --- a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/users.xml +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/users.xml @@ -1,3 +1,23 @@ + + + + + + + + + + ::/0 + + default + default + + + + + + + diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py b/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py index 56c40530c36..c96560230eb 100644 --- a/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py @@ -1,3 +1,4 @@ +import docker import os import pwd import pytest @@ -6,32 +7,36 @@ import re from helpers.cluster import ClickHouseCluster -def expect_failure_with_message(config, expected_message): +def expect_failure_with_message(expected_message): cluster = ClickHouseCluster(__file__) - node = cluster.add_instance('node', main_configs=[config], with_zookeeper=False) + node = cluster.add_instance('node', with_zookeeper=False) with pytest.raises(Exception): cluster.start() + current_user_id = os.getuid() + other_user_id = pwd.getpwnam('nobody').pw_uid + + docker_api = docker.from_env().api + container = node.get_docker_handle() + container.start() + container.exec_run('chown {0} /var/lib/clickhouse'.format(other_user_id), privileged=True) + container.exec_run('clickhouse server --config-file=/etc/clickhouse-server/config.xml --log-file=/var/log/clickhouse-server/clickhouse-server.log --errorlog-file=/var/log/clickhouse-server/clickhouse-server.err.log', privileged=True) + cluster.shutdown() # cleanup - with open(os.path.join(node.path, 'logs/stderr.log')) as log: + with open(os.path.join(node.path, 'logs/clickhouse-server.err.log')) as log: last_message = log.readlines()[-1].strip() if re.search(expected_message, last_message) is None: pytest.fail('Expected the server to fail with a message "{}", but the last message is "{}"'.format(expected_message, last_message)) -def test_no_such_directory(): - expect_failure_with_message('configs/no_such_directory.xml', 'Failed to stat.*no_such_directory') - - def test_different_user(): current_user_id = os.getuid() - if current_user_id != 0: + if current_user_id == 0: current_user_name = pwd.getpwuid(current_user_id).pw_name expect_failure_with_message( - 'configs/owner_mismatch.xml', - 'Effective user of the process \(({}|{})\) does not match the owner of the data \((0|root)\)'.format(current_user_id, current_user_name), + 'Effective user of the process \(.*\) does not match the owner of the data \(.*\)', ) From 81eeaec7fe76b2e054bb26de5cb8f7acb536a917 Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Thu, 20 Dec 2018 00:12:06 +0300 Subject: [PATCH 10/11] Cleanup test. --- .../configs/no_such_directory.xml | 3 --- .../configs/owner_mismatch.xml | 3 --- 2 files changed, 6 deletions(-) delete mode 100644 dbms/tests/integration/test_match_process_uid_against_data_owner/configs/no_such_directory.xml delete mode 100644 dbms/tests/integration/test_match_process_uid_against_data_owner/configs/owner_mismatch.xml diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/no_such_directory.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/no_such_directory.xml deleted file mode 100644 index 80ddf7c4722..00000000000 --- a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/no_such_directory.xml +++ /dev/null @@ -1,3 +0,0 @@ - - /no_such_directory/data/ - diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/owner_mismatch.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/owner_mismatch.xml deleted file mode 100644 index 46d2dcc49ee..00000000000 --- a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/owner_mismatch.xml +++ /dev/null @@ -1,3 +0,0 @@ - - /root/data/ - From fb5dbb7959697970342f6dddce2d64e82980aac3 Mon Sep 17 00:00:00 2001 From: "Sergey V. Galtsev" Date: Thu, 20 Dec 2018 01:47:42 +0300 Subject: [PATCH 11/11] Do not require the server to fail to start in the test. --- .../configs/config.xml | 3 +++ .../test_match_process_uid_against_data_owner/test.py | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml index 032b8874af2..1bd6dc45549 100644 --- a/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/configs/config.xml @@ -7,6 +7,9 @@ /var/log/clickhouse-server/stdout.log + 9000 + 127.0.0.1 + /var/lib/clickhouse/ 5368709120 diff --git a/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py b/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py index 080066f5b0a..77cbe25ff80 100644 --- a/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py +++ b/dbms/tests/integration/test_match_process_uid_against_data_owner/test.py @@ -13,16 +13,16 @@ def test_different_user(): if current_user_id != 0: return + other_user_id = pwd.getpwnam('nobody').pw_uid + cluster = ClickHouseCluster(__file__) node = cluster.add_instance('node') - with pytest.raises(Exception): - cluster.start() - - other_user_id = pwd.getpwnam('nobody').pw_uid + cluster.start() docker_api = docker.from_env().api container = node.get_docker_handle() + container.stop() container.start() container.exec_run('chown {} /var/lib/clickhouse'.format(other_user_id), privileged=True) container.exec_run(CLICKHOUSE_START_COMMAND) @@ -30,8 +30,8 @@ def test_different_user(): cluster.shutdown() # cleanup with open(os.path.join(node.path, 'logs/clickhouse-server.err.log')) as log: + expected_message = "Effective user of the process \(.*\) does not match the owner of the data \(.*\)\. Run under 'sudo -u .*'\." last_message = log.readlines()[-1].strip() - expected_message = 'Effective user of the process \(.*\) does not match the owner of the data \(.*\)' if re.search(expected_message, last_message) is None: pytest.fail('Expected the server to fail with a message "{}", but the last message is "{}"'.format(expected_message, last_message))