From 389931f62249580ad448f0ff0cb5f51e4348ecab Mon Sep 17 00:00:00 2001 From: Petr Stefan Date: Thu, 5 Apr 2018 16:03:42 +0200 Subject: [PATCH] Copy files for sandbox to isolate dir and back --- CMakeLists.txt | 6 ++-- recodex-worker.spec | 4 +-- src/config/sandbox_config.h | 3 +- src/config/sandbox_limits.h | 2 -- src/config/task_metadata.h | 9 +++-- src/helpers/config.cpp | 3 ++ src/helpers/filesystem.cpp | 4 +-- src/helpers/format.h | 11 +++--- src/sandbox/isolate_sandbox.cpp | 62 ++++++++++++++++++++++++--------- src/sandbox/isolate_sandbox.h | 4 +++ src/sandbox/sandbox_base.h | 4 +-- src/tasks/external_task.cpp | 3 +- tests/CMakeLists.txt | 1 + tests/build_job_metadata.cpp | 4 +-- tests/isolate_sandbox.cpp | 28 +++++++-------- tests/job_config.cpp | 3 +- tests/worker_config.cpp | 2 +- 17 files changed, 94 insertions(+), 59 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a7a20d1a..b8c33081 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,7 +23,7 @@ endif() set (Boost_USE_MULTITHREADED ON) find_package(Boost 1.53.0 REQUIRED COMPONENTS filesystem system program_options) include_directories(${Boost_INCLUDE_DIRS}) - + # -- cURL find_package(CURL REQUIRED) include_directories(${CURL_INCLUDE_DIRS}) @@ -63,10 +63,10 @@ endif() # -- ZeroMQ on Windows -if(MSVC) +if(MSVC) find_path(ZEROMQ_INCLUDE NAMES zmq.h PATHS ${ZMQ_INCLUDE_DIR}) find_library(ZEROMQ_LIB NAMES libzmq PATHS ${ZMQ_LIBRARY_DIR}) - + if(ZEROMQ_LIB) include_directories(${ZEROMQ_INCLUDE}) else() diff --git a/recodex-worker.spec b/recodex-worker.spec index cd8f8bca..5cec2ed4 100644 --- a/recodex-worker.spec +++ b/recodex-worker.spec @@ -1,8 +1,8 @@ %define name recodex-worker %define short_name worker %define version 1.3.0 -%define unmangled_version ba766e3b1ae62ea23334f7b242f6c63764528790 -%define release 5 +%define unmangled_version 51f034c148874203a8230043e6c572805a044bd0 +%define release 2 %define spdlog_name spdlog %define spdlog_version 0.13.0 diff --git a/src/config/sandbox_config.h b/src/config/sandbox_config.h index cc130da8..8f468aa6 100644 --- a/src/config/sandbox_config.h +++ b/src/config/sandbox_config.h @@ -38,7 +38,7 @@ class sandbox_config * Change working directory to subdirectory inside the sandbox. * @note Path must be accessible from inside of sandbox. */ - std::string chdir; + std::string chdir = ""; /** * Associative array of loaded limits with textual index identifying its hw group. */ @@ -50,7 +50,6 @@ class sandbox_config */ sandbox_config() { - chdir = "${EVAL_DIR}"; } }; diff --git a/src/config/sandbox_limits.h b/src/config/sandbox_limits.h index b9f45454..4b2b0968 100644 --- a/src/config/sandbox_limits.h +++ b/src/config/sandbox_limits.h @@ -98,8 +98,6 @@ struct sandbox_limits { */ sandbox_limits() { - bound_dirs.push_back(std::tuple( - "${SOURCE_DIR}", "${EVAL_DIR}", dir_perm::RW)); } /** diff --git a/src/config/task_metadata.h b/src/config/task_metadata.h index 310915d8..dede61af 100644 --- a/src/config/task_metadata.h +++ b/src/config/task_metadata.h @@ -34,9 +34,10 @@ class task_metadata task_type type = task_type::INNER, std::string cmd = "", std::vector args = {}, - std::shared_ptr sandbox = nullptr) - : task_id(task_id), priority(priority), fatal_failure(fatal), dependencies(deps), type(type), binary(cmd), - cmd_args(args), sandbox(sandbox) + std::shared_ptr sandbox = nullptr, + std::string test_id = "") + : task_id(task_id), priority(priority), fatal_failure(fatal), dependencies(deps), test_id(test_id), type(type), + binary(cmd), cmd_args(args), sandbox(sandbox) { } @@ -48,6 +49,8 @@ class task_metadata bool fatal_failure; /** Dependent tasks which have to be executed before this one. */ std::vector dependencies; + /** Test id for external tasks */ + std::string test_id; /** Type of this task. */ task_type type; diff --git a/src/helpers/config.cpp b/src/helpers/config.cpp index a456e47c..b3187941 100644 --- a/src/helpers/config.cpp +++ b/src/helpers/config.cpp @@ -81,6 +81,9 @@ std::shared_ptr helpers::build_job_metadata(const YAML::Node &conf } else { throw config_exception("Configuration of one task has missing cmd"); } + if (ctask["test-id"] && ctask["test-id"].IsScalar()) { + task_meta->test_id = ctask["test-id"].as(); + } // load dependencies if (ctask["dependencies"] && ctask["dependencies"].IsSequence()) { diff --git a/src/helpers/filesystem.cpp b/src/helpers/filesystem.cpp index 66d4e28e..9c6c68d2 100644 --- a/src/helpers/filesystem.cpp +++ b/src/helpers/filesystem.cpp @@ -8,9 +8,7 @@ void helpers::copy_directory(const fs::path &src, const fs::path &dest) throw filesystem_exception("Source directory does not exist"); } else if (!fs::is_directory(src)) { throw filesystem_exception("Source directory is not a directory"); - } else if (fs::exists(dest)) { - throw filesystem_exception("Destination directory exists"); - } else if (!fs::create_directories(dest)) { + } else if (!fs::exists(dest) && !fs::create_directories(dest)) { throw filesystem_exception("Destination directory cannot be created"); } diff --git a/src/helpers/format.h b/src/helpers/format.h index 49276287..c96ffea7 100644 --- a/src/helpers/format.h +++ b/src/helpers/format.h @@ -3,11 +3,14 @@ #include -namespace helpers { - inline void format(std::ostringstream &) {} +namespace helpers +{ + inline void format(std::ostringstream &) + { + } - template - void format(std::ostringstream &oss, const ArgT &a, T...args) { + template void format(std::ostringstream &oss, const ArgT &a, T... args) + { oss << a; format(oss, args...); } diff --git a/src/sandbox/isolate_sandbox.cpp b/src/sandbox/isolate_sandbox.cpp index f60fced7..f6777192 100644 --- a/src/sandbox/isolate_sandbox.cpp +++ b/src/sandbox/isolate_sandbox.cpp @@ -16,25 +16,48 @@ #define BOOST_FILESYSTEM_NO_DEPRECATED #define BOOST_NO_CXX11_SCOPED_ENUMS #include +#include "../helpers/filesystem.h" namespace fs = boost::filesystem; +namespace +{ + void move_or_throw(std::shared_ptr logger, const std::string &from, const std::string &to) + { + try { + helpers::copy_directory(from, to); + } catch (fs::filesystem_error &e) { + log_and_throw(logger, "Failed moving ", from, " to ", to, ", error: ", e.what()); + } + + try { + fs::remove_all(from); + } catch (fs::filesystem_error &) { + } + } +} isolate_sandbox::isolate_sandbox(std::shared_ptr sandbox_config, sandbox_limits limits, size_t id, const std::string &temp_dir, + const std::string &data_dir, std::shared_ptr logger) - : sandbox_config_(sandbox_config), limits_(limits), logger_(logger), id_(id), isolate_binary_("isolate") + : sandbox_config_(sandbox_config), limits_(limits), logger_(logger), id_(id), isolate_binary_("isolate"), + data_dir_(data_dir) { - if (sandbox_config == nullptr) { - throw sandbox_exception("No sandbox configuration provided."); - } - if (logger_ == nullptr) { logger_ = helpers::create_null_logger(); } + if (sandbox_config_ == nullptr) { + log_and_throw(logger_, "No sandbox configuration provided."); + } + + if (data_dir_ == "") { + logger_->info("Empty data directory for moving to sandbox."); + } + // Set backup limit (for killing isolate if it hasn't finished yet) max_timeout_ = limits_.wall_time > limits_.cpu_time ? limits_.wall_time : limits_.cpu_time; max_timeout_ += 300; // plus 5 minutes (for short tasks) @@ -69,7 +92,19 @@ isolate_sandbox::~isolate_sandbox() sandbox_results isolate_sandbox::run(const std::string &binary, const std::vector &arguments) { + // move data to isolate directory + if (data_dir_ != "") { + move_or_throw(logger_, data_dir_, sandboxed_dir_); + } + + // run isolate isolate_run(binary, arguments); + + // move data from isolate directory back to data directory + if (data_dir_ != "") { + move_or_throw(logger_, sandboxed_dir_, data_dir_); + } + return process_meta_file(); } @@ -88,9 +123,7 @@ void isolate_sandbox::isolate_init() childpid = fork(); switch (childpid) { - case -1: - log_and_throw(logger_, "Fork failed: ", strerror(errno)); - break; + case -1: log_and_throw(logger_, "Fork failed: ", strerror(errno)); break; case 0: isolate_init_child(fd[0], fd[1]); break; default: //---Parent--- @@ -105,6 +138,7 @@ void isolate_sandbox::isolate_init() } sandboxed_dir_ += std::string(buf); } + sandboxed_dir_ += "/box"; if (ret == -1) { log_and_throw(logger_, "Read from pipe error."); } @@ -161,9 +195,7 @@ void isolate_sandbox::isolate_cleanup() childpid = fork(); switch (childpid) { - case -1: - log_and_throw(logger_, "Fork failed: ", strerror(errno)); - break; + case -1: log_and_throw(logger_, "Fork failed: ", strerror(errno)); break; case 0: //---Child--- // Redirect stderr to /dev/null file @@ -209,9 +241,7 @@ void isolate_sandbox::isolate_run(const std::string &binary, const std::vectordebug("Returned from the first fork as child"); @@ -245,9 +275,7 @@ void isolate_sandbox::isolate_run(const std::string &binary, const std::vectordebug("Running the second fork"); controlpid = fork(); switch (controlpid) { - case -1: - log_and_throw(logger_, "Fork failed: ", strerror(errno)); - break; + case -1: log_and_throw(logger_, "Fork failed: ", strerror(errno)); break; case 0: // Child--- { diff --git a/src/sandbox/isolate_sandbox.h b/src/sandbox/isolate_sandbox.h index d1e2c33b..bbcd515b 100644 --- a/src/sandbox/isolate_sandbox.h +++ b/src/sandbox/isolate_sandbox.h @@ -36,12 +36,14 @@ class isolate_sandbox : public sandbox_base * @param limits Limits for current command. * @param id Number of current worker. This must be unique for each worker on one machine! * @param temp_dir Directory to store temporary files (generated isolate's meta log) + * @param data_dit Directory containing sources which will be copied into sandbox * @param logger Set system logger (optional). */ isolate_sandbox(std::shared_ptr sandbox_config, sandbox_limits limits, size_t id, const std::string &temp_dir, + const std::string &data_dir, std::shared_ptr logger = nullptr); /** * Destructor. @@ -66,6 +68,8 @@ class isolate_sandbox : public sandbox_base std::string meta_file_; /** Maximum time to run separate isolate process */ int max_timeout_; + /** Path to the directory containing sources moved to sandbox and back */ + std::string data_dir_; /** Initialize isolate */ void isolate_init(); /** Actual code for isolate initialization inside a process. Called by isolate_init(). */ diff --git a/src/sandbox/sandbox_base.h b/src/sandbox/sandbox_base.h index acbd0816..dacaca53 100644 --- a/src/sandbox/sandbox_base.h +++ b/src/sandbox/sandbox_base.h @@ -88,8 +88,8 @@ class sandbox_exception : public std::exception }; -template -void log_and_throw(std::shared_ptr logger, T...args) { +template void log_and_throw(std::shared_ptr logger, T... args) +{ std::ostringstream oss; helpers::format(oss, args...); const auto message = oss.str(); diff --git a/src/tasks/external_task.cpp b/src/tasks/external_task.cpp index b9a58ae4..9dd3e1ca 100644 --- a/src/tasks/external_task.cpp +++ b/src/tasks/external_task.cpp @@ -53,8 +53,9 @@ void external_task::sandbox_init() { #ifndef _WIN32 if (task_meta_->sandbox->name == "isolate") { + auto data_dir = fs::path(source_dir_) / task_meta_->test_id; sandbox_ = std::make_shared( - sandbox_config_, *limits_, worker_config_->get_worker_id(), temp_dir_, logger_); + sandbox_config_, *limits_, worker_config_->get_worker_id(), temp_dir_, data_dir.string(), logger_); } #endif } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d65c1995..7245c628 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -203,6 +203,7 @@ add_test_suite(tool_isolate_sandbox isolate_sandbox.cpp ${SANDBOX_DIR}/isolate_sandbox.cpp ${HELPERS_DIR}/logger.cpp + ${HELPERS_DIR}/filesystem.cpp ) add_test_suite(tool_archivator diff --git a/tests/build_job_metadata.cpp b/tests/build_job_metadata.cpp index 62fea4d8..c5516da5 100644 --- a/tests/build_job_metadata.cpp +++ b/tests/build_job_metadata.cpp @@ -98,10 +98,10 @@ TEST(job_metadata, build_all_from_yaml) auto envs = std::pair{"ISOLATE_TMP", "/tmp"}; EXPECT_EQ(envs, limits->environ_vars[0]); - EXPECT_EQ(2u, limits->bound_dirs.size()); + EXPECT_EQ(1u, limits->bound_dirs.size()); auto dirs = std::tuple{ "path1/dir1", "path2/dir2", sandbox_limits::dir_perm::RW}; - EXPECT_EQ(dirs, limits->bound_dirs[1]); + EXPECT_EQ(dirs, limits->bound_dirs[0]); } TEST(job_metadata, queue_of_tasks) diff --git a/tests/isolate_sandbox.cpp b/tests/isolate_sandbox.cpp index 7d80b8aa..36692be1 100644 --- a/tests/isolate_sandbox.cpp +++ b/tests/isolate_sandbox.cpp @@ -16,10 +16,10 @@ TEST(IsolateSandbox, BasicCreation) { std::shared_ptr config = std::make_shared(); sandbox_limits limits; - EXPECT_NO_THROW(isolate_sandbox s(config, limits, 34, "/tmp")); - isolate_sandbox is(config, limits, 34, "/tmp"); - EXPECT_EQ(is.get_dir(), "/var/local/lib/isolate/34"); - EXPECT_THROW(isolate_sandbox s(config, limits, 2365, "/tmp"), sandbox_exception); + EXPECT_NO_THROW(isolate_sandbox s(config, limits, 34, "/tmp", "")); + isolate_sandbox is(config, limits, 34, "/tmp", ""); + EXPECT_EQ(is.get_dir(), "/var/local/lib/isolate/34/box"); + EXPECT_THROW(isolate_sandbox s(config, limits, 2365, "/tmp", ""), sandbox_exception); } TEST(IsolateSandbox, NormalCommand) @@ -42,8 +42,8 @@ TEST(IsolateSandbox, NormalCommand) limits.share_net = false; limits.bound_dirs.clear(); isolate_sandbox *is = nullptr; - EXPECT_NO_THROW(is = new isolate_sandbox(config, limits, 34, "/tmp")); - EXPECT_EQ(is->get_dir(), "/var/local/lib/isolate/34"); + EXPECT_NO_THROW(is = new isolate_sandbox(config, limits, 34, "/tmp", "")); + EXPECT_EQ(is->get_dir(), "/var/local/lib/isolate/34/box"); sandbox_results results; EXPECT_NO_THROW(results = is->run("/bin/ls", std::vector{"-a", "-l", "-i"})); EXPECT_TRUE(fs::is_regular_file("/var/local/lib/isolate/34/box/output.txt")); @@ -77,8 +77,8 @@ TEST(IsolateSandbox, TimeoutCommand) limits.share_net = false; limits.bound_dirs.clear(); isolate_sandbox *is = nullptr; - EXPECT_NO_THROW(is = new isolate_sandbox(config, limits, 34, "/tmp")); - EXPECT_EQ(is->get_dir(), "/var/local/lib/isolate/34"); + EXPECT_NO_THROW(is = new isolate_sandbox(config, limits, 34, "/tmp", "")); + EXPECT_EQ(is->get_dir(), "/var/local/lib/isolate/34/box"); sandbox_results results; EXPECT_NO_THROW(results = is->run("/bin/sleep", std::vector{"5"})); EXPECT_TRUE(fs::is_regular_file("/tmp/34/meta.log")); @@ -110,8 +110,8 @@ TEST(IsolateSandbox, NonzeroReturnCommand) limits.share_net = false; limits.bound_dirs.clear(); isolate_sandbox *is = nullptr; - EXPECT_NO_THROW(is = new isolate_sandbox(config, limits, 34, "/tmp")); - EXPECT_EQ(is->get_dir(), "/var/local/lib/isolate/34"); + EXPECT_NO_THROW(is = new isolate_sandbox(config, limits, 34, "/tmp", "")); + EXPECT_EQ(is->get_dir(), "/var/local/lib/isolate/34/box"); sandbox_results results; EXPECT_NO_THROW(results = is->run("/bin/false", std::vector{})); EXPECT_TRUE(fs::is_regular_file("/tmp/34/meta.log")); @@ -150,17 +150,15 @@ TEST(IsolateSandbox, NonzeroReturnCommand) TEST(IsolateSandbox, BindDirsExecuteGCC) { - using sp = sandbox_limits::dir_perm; auto tmp = fs::temp_directory_path(); std::shared_ptr config = std::make_shared(); - config->chdir = "evaluate"; + config->chdir = ""; sandbox_limits limits; limits.wall_time = 10; limits.cpu_time = 10; limits.extra_time = 1; limits.processes = 0; - limits.bound_dirs = { - std::tuple{(tmp / "recodex_35_test").string(), "evaluate", sp::RW}}; + limits.bound_dirs = {}; limits.environ_vars = {{"PATH", "/usr/bin"}}; fs::create_directories(tmp / "recodex_35_test"); @@ -171,7 +169,7 @@ TEST(IsolateSandbox, BindDirsExecuteGCC) } isolate_sandbox *is = nullptr; - EXPECT_NO_THROW(is = new isolate_sandbox(config, limits, 35, "/tmp")); + EXPECT_NO_THROW(is = new isolate_sandbox(config, limits, 35, tmp.string(), (tmp / "recodex_35_test").string())); sandbox_results results; EXPECT_NO_THROW(results = is->run("/usr/bin/gcc", std::vector{"-Wall", "-o", "test", "main.c"})); diff --git a/tests/job_config.cpp b/tests/job_config.cpp index fd9b2422..24c9f889 100644 --- a/tests/job_config.cpp +++ b/tests/job_config.cpp @@ -280,7 +280,6 @@ TEST(job_config_test, config_data) } std::vector> expected_bound_dirs = { - std::tuple{"${SOURCE_DIR}", "${EVAL_DIR}", sp::RW}, std::tuple{"/tmp/recodex", "/recodex/tmp", static_cast(sp::RW | sp::NOEXEC)}}; ASSERT_EQ(limit1->bound_dirs, expected_bound_dirs); @@ -294,5 +293,5 @@ TEST(job_config_test, config_data) ASSERT_EQ(limit2->disk_size, SIZE_MAX); ASSERT_EQ(limit2->disk_files, SIZE_MAX); ASSERT_EQ(limit2->environ_vars.size(), 0u); - ASSERT_EQ(limit2->bound_dirs.size(), 1u); + ASSERT_EQ(limit2->bound_dirs.size(), 0u); } diff --git a/tests/worker_config.cpp b/tests/worker_config.cpp index 09d3fb71..ace90e78 100644 --- a/tests/worker_config.cpp +++ b/tests/worker_config.cpp @@ -70,7 +70,7 @@ TEST(worker_config, load_yaml_basic) expected_limits.stack_size = 50000; expected_limits.disk_size = 50; expected_limits.disk_files = 7; - expected_limits.bound_dirs = {std::tuple{"${SOURCE_DIR}", "${EVAL_DIR}", sp::RW}, + expected_limits.bound_dirs = { std::tuple{"/usr/local/bin", "localbin", sp::RW}, std::tuple{"/usr/share", "share", sp::MAYBE}}; if (config.get_limits().environ_vars.at(0).first == "ISOLATE_TMP") {