From cd155ed6f58f197b8db048f342f0f59257cc65be Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Wed, 22 Feb 2023 15:47:54 +0000 Subject: [PATCH] refactor(userspace/falco): update actions to use new hot restarter utility with dry-run safetyc checks Signed-off-by: Jason Dellaluce --- userspace/falco/app/actions/actions.h | 1 - .../app/actions/create_signal_handlers.cpp | 156 ++++++++---------- .../falco/app/actions/process_events.cpp | 3 +- userspace/falco/app/app.cpp | 9 +- userspace/falco/app/app.h | 3 + userspace/falco/app/state.h | 16 ++ 6 files changed, 93 insertions(+), 95 deletions(-) diff --git a/userspace/falco/app/actions/actions.h b/userspace/falco/app/actions/actions.h index a746d1fb..0a2fcd7e 100644 --- a/userspace/falco/app/actions/actions.h +++ b/userspace/falco/app/actions/actions.h @@ -23,7 +23,6 @@ namespace falco { namespace app { namespace actions { -falco::app::run_result attach_inotify_signals(falco::app::state& s); falco::app::run_result configure_interesting_sets(falco::app::state& s); falco::app::run_result configure_syscall_buffer_size(falco::app::state& s); falco::app::run_result create_requested_paths(falco::app::state& s); diff --git a/userspace/falco/app/actions/create_signal_handlers.cpp b/userspace/falco/app/actions/create_signal_handlers.cpp index 88392813..0a43cec5 100644 --- a/userspace/falco/app/actions/create_signal_handlers.cpp +++ b/userspace/falco/app/actions/create_signal_handlers.cpp @@ -16,23 +16,15 @@ limitations under the License. #include -#include -#include -#include -#include #include "actions.h" +#include "../app.h" #include "../signals.h" using namespace falco::app; using namespace falco::app::actions; -// This is initially set to a dummy application. When -// create_signal_handlers is called, it will be rebound to the -// provided application, and in unregister_signal_handlers it will be -// rebound back to the dummy application. - -static int inot_fd; +static std::shared_ptr s_restarter; static void terminate_signal_handler(int signal) { @@ -46,7 +38,10 @@ static void reopen_outputs_signal_handler(int signal) static void restart_signal_handler(int signal) { - falco::app::g_restart_signal.trigger(); + if (s_restarter != nullptr) + { + s_restarter->trigger(); + } } bool create_handler(int sig, void (*func)(int), run_result &ret) @@ -94,89 +89,65 @@ falco::app::run_result falco::app::actions::create_signal_handlers(falco::app::s ! create_handler(SIGUSR1, ::reopen_outputs_signal_handler, ret) || ! create_handler(SIGHUP, ::restart_signal_handler, ret)) { - // we use the if just to make sure we return at the first failed statement + return ret; } + falco::app::restart_handler::watch_list_t files_to_watch; + falco::app::restart_handler::watch_list_t dirs_to_watch; + if (s.config->m_watch_config_files) + { + files_to_watch.push_back(s.options.conf_filename); + files_to_watch.insert( + files_to_watch.end(), + s.config->m_loaded_rules_filenames.begin(), + s.config->m_loaded_rules_filenames.end()); + dirs_to_watch.insert( + dirs_to_watch.end(), + s.config->m_loaded_rules_folders.begin(), + s.config->m_loaded_rules_folders.end()); + } + + s.restarter = std::make_shared([&s]{ + bool tmp = false; + bool success = false; + std::string err; + falco::app::state tmp_state(s.cmdline, s.options); + tmp_state.options.dry_run = true; + try + { + success = falco::app::run(tmp_state, tmp, err); + } + catch (std::exception& e) + { + err = e.what(); + } + catch (...) + { + err = "unknown error"; + } + + if (!success && s.outputs != nullptr) + { + std::string rule = "Falco internal: hot restart failure"; + std::string msg = rule + ": " + err; + std::map o = {}; + auto now = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + s.outputs->handle_msg(now, falco_common::PRIORITY_CRITICAL, msg, rule, o); + } + + return success; + }, files_to_watch, dirs_to_watch); + + ret = run_result::ok(); + ret.success = s.restarter->start(ret.errstr); + ret.proceed = ret.success; + if (ret.success) + { + s_restarter = s.restarter; + } return ret; } -falco::app::run_result falco::app::actions::attach_inotify_signals(falco::app::state& s) -{ - if (s.options.dry_run) - { - falco_logger::log(LOG_DEBUG, "Skipping attaching inotify signals in dry-run\n"); - return run_result::ok(); - } - - if (s.config->m_watch_config_files) - { - inot_fd = inotify_init(); - if (inot_fd == -1) - { - return run_result::fatal("Could not create inotify handler"); - } - - struct sigaction sa; - sigemptyset(&sa.sa_mask); - sa.sa_flags = SA_RESTART; - sa.sa_handler = restart_signal_handler; - if (sigaction(SIGIO, &sa, NULL) == -1) - { - return run_result::fatal("Failed to link SIGIO to inotify handler"); - } - - /* Set owner process that is to receive "I/O possible" signal */ - if (fcntl(inot_fd, F_SETOWN, getpid()) == -1) - { - return run_result::fatal("Failed to setting owner on inotify handler"); - } - - /* - * Enable "I/O possible" signaling and make I/O nonblocking - * for file descriptor - */ - int flags = fcntl(inot_fd, F_GETFL); - if (fcntl(inot_fd, F_SETFL, flags | O_ASYNC | O_NONBLOCK) == -1) - { - return run_result::fatal("Failed to setting flags on inotify handler"); - } - - // Watch conf file - int wd = inotify_add_watch(inot_fd, s.options.conf_filename.c_str(), IN_CLOSE_WRITE); - if (wd == -1) - { - return run_result::fatal("Failed to watch conf file"); - } - falco_logger::log(LOG_DEBUG, "Watching " + s.options.conf_filename +"\n"); - - // Watch rules files - for (const auto &rule : s.config->m_loaded_rules_filenames) - { - wd = inotify_add_watch(inot_fd, rule.c_str(), IN_CLOSE_WRITE | IN_ONESHOT); - if (wd == -1) - { - return run_result::fatal("Failed to watch rule file: " + rule); - } - falco_logger::log(LOG_DEBUG, "Watching " + rule +"\n"); - } - - // Watch specified rules folders, if any: - // any newly created/removed file within the folder - // will trigger a Falco restart. - for (const auto &fld : s.config->m_loaded_rules_folders) - { - // For folders, we watch if any file is created or destroyed within - wd = inotify_add_watch(inot_fd, fld.c_str(), IN_CREATE | IN_DELETE | IN_ONESHOT); - if (wd == -1) - { - return run_result::fatal("Failed to watch rule folder: " + fld); - } - falco_logger::log(LOG_DEBUG, "Watching " + fld +" folder\n"); - } - } - return run_result::ok(); -} - falco::app::run_result falco::app::actions::unregister_signal_handlers(falco::app::state& s) { if (s.options.dry_run) @@ -185,8 +156,13 @@ falco::app::run_result falco::app::actions::unregister_signal_handlers(falco::ap return run_result::ok(); } + s_restarter = nullptr; + if (s.restarter != nullptr) + { + s.restarter->stop(); + } + run_result ret; - close(inot_fd); if(! create_handler(SIGINT, SIG_DFL, ret) || ! create_handler(SIGTERM, SIG_DFL, ret) || ! create_handler(SIGUSR1, SIG_DFL, ret) || diff --git a/userspace/falco/app/actions/process_events.cpp b/userspace/falco/app/actions/process_events.cpp index 357e0de4..86ebf3e3 100644 --- a/userspace/falco/app/actions/process_events.cpp +++ b/userspace/falco/app/actions/process_events.cpp @@ -208,8 +208,9 @@ static falco::app::run_result do_inspect( } else if(falco::app::g_restart_signal.triggered()) { - falco::app::g_restart_signal.handle([&](){ + falco::app::g_restart_signal.handle([&s](){ falco_logger::log(LOG_INFO, "SIGHUP received, restarting...\n"); + s.restart.store(true); }); break; } diff --git a/userspace/falco/app/app.cpp b/userspace/falco/app/app.cpp index 215f27b7..53e7aa82 100644 --- a/userspace/falco/app/app.cpp +++ b/userspace/falco/app/app.cpp @@ -40,7 +40,11 @@ bool falco::app::run(int argc, char** argv, bool& restart, std::string& errstr) } s.cmdline += *arg; } + return falco::app::run(s, restart, errstr); +} +bool falco::app::run(falco::app::state& s, bool& restart, std::string& errstr) +{ // The order here is the order in which the methods will be // called. Before changing the order, ensure that all // dependencies are honored (e.g. don't process events before @@ -64,11 +68,10 @@ bool falco::app::run(int argc, char** argv, bool& restart, std::string& errstr) falco::app::actions::validate_rules_files, falco::app::actions::load_rules_files, falco::app::actions::print_support, + falco::app::actions::init_outputs, falco::app::actions::create_signal_handlers, - falco::app::actions::attach_inotify_signals, falco::app::actions::create_requested_paths, falco::app::actions::daemonize, - falco::app::actions::init_outputs, falco::app::actions::init_clients, falco::app::actions::configure_interesting_sets, falco::app::actions::configure_syscall_buffer_size, @@ -104,7 +107,7 @@ bool falco::app::run(int argc, char** argv, bool& restart, std::string& errstr) errstr = res.errstr; } - restart = falco::app::g_restart_signal.triggered(); + restart = s.restart; return res.success; } diff --git a/userspace/falco/app/app.h b/userspace/falco/app/app.h index 999c4209..d5a54b8c 100644 --- a/userspace/falco/app/app.h +++ b/userspace/falco/app/app.h @@ -16,12 +16,15 @@ limitations under the License. #pragma once +#include "state.h" + #include namespace falco { namespace app { bool run(int argc, char** argv, bool& restart, std::string& errstr); +bool run(falco::app::state& s, bool& restart, std::string& errstr); }; // namespace app }; // namespace falco diff --git a/userspace/falco/app/state.h b/userspace/falco/app/state.h index 8ba9b19f..8a2612e1 100644 --- a/userspace/falco/app/state.h +++ b/userspace/falco/app/state.h @@ -19,6 +19,7 @@ limitations under the License. #include "indexed_vector.h" #include "options.h" +#include "restart_handler.h" #include "../configuration.h" #include "../stats_writer.h" #ifndef MINIMAL_BUILD @@ -30,6 +31,7 @@ limitations under the License. #include #include +#include #include namespace falco { @@ -59,6 +61,7 @@ struct state }; state(): + restart(false), loaded_sources(), enabled_sources(), source_infos(), @@ -71,7 +74,15 @@ struct state engine = std::make_shared(); offline_inspector = std::make_shared(); outputs = nullptr; + restarter = nullptr; } + + state(const std::string& cmd, const falco::app::options& opts): state() + { + cmdline = cmd; + options = opts; + } + ~state() = default; state(state&&) = default; state& operator = (state&&) = default; @@ -80,6 +91,8 @@ struct state std::string cmdline; falco::app::options options; + std::atomic restart; + std::shared_ptr config; std::shared_ptr outputs; @@ -114,6 +127,9 @@ struct state // Dimension of the syscall buffer in bytes. uint64_t syscall_buffer_bytes_size; + // Helper responsible for watching of handling hot application restarts + std::shared_ptr restarter; + #ifndef MINIMAL_BUILD falco::grpc::server grpc_server; std::thread grpc_server_thread;