From e3031c78ec8e5cf4209544681fc727e35a3c1d3c Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Wed, 23 Aug 2023 22:20:16 +0000 Subject: [PATCH] feat(userspace): deprecate `-d` daemonize option Deprecate `-d` option (currently broken). Symptoms included the message queue filling up without popping any messages even though events were handled normally. Maintainers decided to deprecate not needed `-d` option while keeping the useful `pidfile` command args option. Signed-off-by: Melissa Kilby --- userspace/falco/CMakeLists.txt | 2 +- userspace/falco/app/actions/actions.h | 2 +- userspace/falco/app/actions/daemonize.cpp | 89 -------------------- userspace/falco/app/actions/init_outputs.cpp | 2 +- userspace/falco/app/actions/pidfile.cpp | 52 ++++++++++++ userspace/falco/app/app.cpp | 2 +- userspace/falco/app/options.cpp | 8 +- userspace/falco/app/options.h | 1 - userspace/falco/falco.cpp | 3 +- 9 files changed, 58 insertions(+), 103 deletions(-) delete mode 100644 userspace/falco/app/actions/daemonize.cpp create mode 100644 userspace/falco/app/actions/pidfile.cpp diff --git a/userspace/falco/CMakeLists.txt b/userspace/falco/CMakeLists.txt index 6ceb1400426..43d9be9b8c4 100644 --- a/userspace/falco/CMakeLists.txt +++ b/userspace/falco/CMakeLists.txt @@ -22,7 +22,7 @@ set( app/actions/helpers_inspector.cpp app/actions/configure_interesting_sets.cpp app/actions/create_signal_handlers.cpp - app/actions/daemonize.cpp + app/actions/pidfile.cpp app/actions/init_falco_engine.cpp app/actions/init_inspectors.cpp app/actions/init_clients.cpp diff --git a/userspace/falco/app/actions/actions.h b/userspace/falco/app/actions/actions.h index 45477b4bcd6..840b6d76fc6 100644 --- a/userspace/falco/app/actions/actions.h +++ b/userspace/falco/app/actions/actions.h @@ -28,7 +28,7 @@ falco::app::run_result configure_syscall_buffer_size(falco::app::state& s); falco::app::run_result configure_syscall_buffer_num(falco::app::state& s); falco::app::run_result create_requested_paths(falco::app::state& s); falco::app::run_result create_signal_handlers(falco::app::state& s); -falco::app::run_result daemonize(falco::app::state& s); +falco::app::run_result pidfile(falco::app::state& s); falco::app::run_result init_clients(falco::app::state& s); falco::app::run_result init_falco_engine(falco::app::state& s); falco::app::run_result init_inspectors(falco::app::state& s); diff --git a/userspace/falco/app/actions/daemonize.cpp b/userspace/falco/app/actions/daemonize.cpp deleted file mode 100644 index f2b82561a48..00000000000 --- a/userspace/falco/app/actions/daemonize.cpp +++ /dev/null @@ -1,89 +0,0 @@ -/* -Copyright (C) 2023 The Falco Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -#include -#include -#include - -#include "actions.h" - -using namespace falco::app; -using namespace falco::app::actions; - -static bool s_daemonized = false; - -falco::app::run_result falco::app::actions::daemonize(falco::app::state& s) -{ -#ifdef __linux__ - if (s.options.dry_run) - { - falco_logger::log(LOG_DEBUG, "Skipping daemonizing in dry-run\n"); - return run_result::ok(); - } - - // If daemonizing, do it here so any init errors will - // be returned in the foreground process. - if (s.options.daemon && !s_daemonized) { - pid_t pid, sid; - - pid = fork(); - if (pid < 0) { - // error - return run_result::fatal("Could not fork"); - } else if (pid > 0) { - // parent. Write child pid to pidfile and exit - std::ofstream pidfile; - pidfile.open(s.options.pidfilename); - - if (!pidfile.good()) - { - falco_logger::log(LOG_ERR, "Could not write pid to pid file " + s.options.pidfilename + ". Exiting.\n"); - exit(-1); - } - pidfile << pid; - pidfile.close(); - exit(0); - } - // if here, child. - - // Become own process group. - sid = setsid(); - if (sid < 0) { - return run_result::fatal("Could not set session id"); - } - - // Set umask so no files are world anything or group writable. - umask(027); - - // Change working directory to '/' - if ((chdir("/")) < 0) { - return run_result::fatal("Could not change working directory to '/'"); - } - - // Close stdin, stdout, stderr and reopen to /dev/null - close(0); - close(1); - close(2); - open("/dev/null", O_RDONLY); - open("/dev/null", O_RDWR); - open("/dev/null", O_RDWR); - - s_daemonized = true; - } -#endif // __linux__ - - return run_result::ok(); -} diff --git a/userspace/falco/app/actions/init_outputs.cpp b/userspace/falco/app/actions/init_outputs.cpp index b4a60751871..aebefe44da6 100644 --- a/userspace/falco/app/actions/init_outputs.cpp +++ b/userspace/falco/app/actions/init_outputs.cpp @@ -51,7 +51,7 @@ falco::app::run_result falco::app::actions::init_outputs(falco::app::state& s) if (s.options.dry_run) { - falco_logger::log(LOG_DEBUG, "Skipping daemonizing in dry-run\n"); + falco_logger::log(LOG_DEBUG, "Skipping outputs initialization in dry-run\n"); return run_result::ok(); } diff --git a/userspace/falco/app/actions/pidfile.cpp b/userspace/falco/app/actions/pidfile.cpp new file mode 100644 index 00000000000..2116d65f897 --- /dev/null +++ b/userspace/falco/app/actions/pidfile.cpp @@ -0,0 +1,52 @@ +/* +Copyright (C) 2023 The Falco Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include +#include +#include + +#include "actions.h" + +using namespace falco::app; +using namespace falco::app::actions; + +falco::app::run_result falco::app::actions::pidfile(falco::app::state& s) +{ + if (s.options.dry_run) + { + falco_logger::log(LOG_DEBUG, "Skipping pidfile creation in dry-run\n"); + return run_result::ok(); + } + + if (!s.options.pidfilename.empty()) + { + int64_t self_pid = getpid(); + + std::ofstream pidfile; + pidfile.open(s.options.pidfilename); + + if (!pidfile.good()) + { + falco_logger::log(LOG_ERR, "Could not write pid to pidfile " + s.options.pidfilename + ". Exiting.\n"); + exit(-1); + } + pidfile << self_pid; + pidfile.close(); + + } + + return run_result::ok(); +} diff --git a/userspace/falco/app/app.cpp b/userspace/falco/app/app.cpp index 6218078de60..b18000e5e98 100644 --- a/userspace/falco/app/app.cpp +++ b/userspace/falco/app/app.cpp @@ -80,7 +80,7 @@ bool falco::app::run(falco::app::state& s, bool& restart, std::string& errstr) falco::app::actions::init_outputs, falco::app::actions::create_signal_handlers, falco::app::actions::create_requested_paths, - falco::app::actions::daemonize, + falco::app::actions::pidfile, falco::app::actions::init_clients, falco::app::actions::configure_interesting_sets, falco::app::actions::configure_syscall_buffer_size, diff --git a/userspace/falco/app/options.cpp b/userspace/falco/app/options.cpp index 80d74e4a319..fecf7fe3a77 100644 --- a/userspace/falco/app/options.cpp +++ b/userspace/falco/app/options.cpp @@ -142,11 +142,6 @@ bool options::parse(int argc, char **argv, std::string &errstr) return false; } - if (daemon && pidfilename == "") { - errstr = std::string("If -d is provided, a pid file must also be provided"); - return false; - } - list_fields = m_cmdline_parsed.count("list") > 0 ? true : false; int open_modes = 0; @@ -183,7 +178,6 @@ void options::define(cxxopts::Options& opts) ("b,print-base64", "Print data buffers in base64. This is useful for encoding binary data that needs to be used over media designed to consume this format.") #if !defined(_WIN32) && !defined(__EMSCRIPTEN__) && !defined(MINIMAL_BUILD) ("cri", "Path to CRI socket for container metadata. Use the specified socket to fetch data from a CRI-compatible runtime. If not specified, uses the libs default. This option can be passed multiple times to specify socket to be tried until a successful one is found.", cxxopts::value(cri_socket_paths), "") - ("d,daemon", "Run as a daemon.", cxxopts::value(daemon)->default_value("false")) ("disable-cri-async", "Disable asynchronous CRI metadata fetching. This is useful to let the input event wait for the container metadata fetch to finish before moving forward. Async fetching, in some environments leads to empty fields for container metadata when the fetch is not fast enough to be completed asynchronously. This can have a performance penalty on your environment depending on the number of containers and the frequency at which they are created/started/stopped.", cxxopts::value(disable_cri_async)->default_value("false")) #endif ("disable-source", "Disable a specific event source. By default, all loaded sources get enabled. Available sources are 'syscall' and all sources defined by loaded plugins supporting the event sourcing capability. This option can be passed multiple times. This has no offect when reading events from a trace file. Can not disable all event sources. Can not be mixed with --enable-source.", cxxopts::value(disable_sources), "") @@ -217,7 +211,7 @@ void options::define(cxxopts::Options& opts) ("o,option", "Set the value of option to . Overrides values in configuration file. can be identified using its location in configuration file using dot notation. Elements which are entries of lists can be accessed via square brackets [].\n E.g. base.id = val\n base.subvalue.subvalue2 = val\n base.list[1]=val", cxxopts::value(cmdline_config_options), "=") ("plugin-info", "Print info for a single plugin and exit.\nThis includes all descriptivo info like name and author, along with the\nschema format for the init configuration and a list of suggested open parameters.\n can be the name of the plugin or its configured library_path.", cxxopts::value(print_plugin_info), "") ("p,print", "Print (or replace) additional information in rule's output.\nUse -pc or -pcontainer to append container details.\nUse -pk or -pkubernetes to add both container and Kubernetes details.\nIf using gVisor, choose -pcg or -pkg variants (or -pcontainer-gvisor and -pkubernetes-gvisor, respectively).\nIf a rule's output contains %container.info, it will be replaced with the corresponding details. Otherwise, these details will be directly appended to the rule's output.\nAlternatively, use -p \"...\" for a custom format. In this case, the given content will be appended to the rule's output without any replacement.", cxxopts::value(print_additional), "") - ("P,pidfile", "When run as a daemon, write pid to specified file", cxxopts::value(pidfilename)->default_value("/var/run/falco.pid"), "") + ("P,pidfile", "Write pid to specified file, by default no pidfile is created.", cxxopts::value(pidfilename)->default_value(""), "") ("r", "Rules file/directory (defaults to value set in configuration file, or /etc/falco_rules.yaml). This option can be passed multiple times to read from multiple files/directories.", cxxopts::value>(), "") ("S,snaplen", "Capture the first bytes of each I/O buffer. By default, the first 80 bytes are captured. Use this option with caution, it can have a strong performance impact.", cxxopts::value(snaplen)->default_value("0"), "") ("support", "Print support information including version, rules files used, etc. and exit.", cxxopts::value(print_support)->default_value("false")) diff --git a/userspace/falco/app/options.h b/userspace/falco/app/options.h index 04c6b6f8c57..e9183446fc7 100644 --- a/userspace/falco/app/options.h +++ b/userspace/falco/app/options.h @@ -42,7 +42,6 @@ class options { bool all_events; sinsp_evt::param_fmt event_buffer_format; std::vector cri_socket_paths; - bool daemon; bool disable_cri_async; std::vector disable_sources; std::vector disabled_rule_substrings; diff --git a/userspace/falco/falco.cpp b/userspace/falco/falco.cpp index 21420e30147..71d42b4000d 100644 --- a/userspace/falco/falco.cpp +++ b/userspace/falco/falco.cpp @@ -26,8 +26,7 @@ limitations under the License. static void display_fatal_err(const std::string &&msg) { /** - * If stderr logging is not enabled, also log to stderr. When - * daemonized this will simply write to /dev/null. + * If stderr logging is not enabled, also log to stderr. */ if (! falco_logger::log_stderr) {