From 74034213a266751d62192158b42c7732d28cb1fa Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Mon, 8 Apr 2024 15:23:37 +0200 Subject: [PATCH] chore(unit_test,userspace): better log management. Also, warnings are now returned so that yaml_helper class does not need to log anything. Signed-off-by: Federico Di Pierro --- unit_tests/falco/test_configuration.cpp | 19 ++++++++++--------- userspace/falco/app/actions/load_config.cpp | 17 +++++++++++++++-- userspace/falco/configuration.cpp | 14 +++----------- userspace/falco/configuration.h | 2 +- userspace/falco/yaml_helper.h | 10 ++++++---- 5 files changed, 35 insertions(+), 27 deletions(-) diff --git a/unit_tests/falco/test_configuration.cpp b/unit_tests/falco/test_configuration.cpp index 04132918..0e9189b2 100644 --- a/unit_tests/falco/test_configuration.cpp +++ b/unit_tests/falco/test_configuration.cpp @@ -38,6 +38,7 @@ static std::string sample_yaml = " - elem3\n"; static std::vector loaded_conf_files; +static std::vector loaded_conf_warnings; TEST(Configuration, configuration_exceptions) { @@ -138,7 +139,7 @@ TEST(Configuration, configuration_config_files_secondary_fail) outfile.close(); yaml_helper conf; - ASSERT_ANY_THROW(conf.load_from_file("main.yaml", loaded_conf_files)); + ASSERT_ANY_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings)); std::filesystem::remove("main.yaml"); std::filesystem::remove("conf_2.yaml"); @@ -185,7 +186,7 @@ TEST(Configuration, configuration_config_files_ok) outfile.close(); yaml_helper conf; - ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files)); + ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings)); // main + conf_2 + conf_3 ASSERT_EQ(loaded_conf_files.size(), 3); @@ -257,7 +258,7 @@ TEST(Configuration, configuration_config_files_relative_main) outfile.close(); yaml_helper conf; - ASSERT_NO_THROW(conf.load_from_file(temp_main.string(), loaded_conf_files)); + ASSERT_NO_THROW(conf.load_from_file(temp_main.string(), loaded_conf_files, loaded_conf_warnings)); // main + conf_3 ASSERT_EQ(loaded_conf_files.size(), 2); @@ -310,7 +311,7 @@ TEST(Configuration, configuration_config_files_override) outfile.close(); yaml_helper conf; - ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files)); + ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings)); // main + conf_2 + conf_3 ASSERT_EQ(loaded_conf_files.size(), 3); @@ -346,7 +347,7 @@ TEST(Configuration, configuration_config_files_unexistent) outfile.close(); yaml_helper conf; - ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files)); + ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings)); // main ASSERT_EQ(loaded_conf_files.size(), 1); @@ -382,7 +383,7 @@ TEST(Configuration, configuration_config_files_scalar_configs_files) outfile.close(); yaml_helper conf; - ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files)); + ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings)); // main + conf_2 ASSERT_EQ(loaded_conf_files.size(), 2); @@ -417,7 +418,7 @@ TEST(Configuration, configuration_config_files_empty_configs_files) outfile.close(); yaml_helper conf; - ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files)); + ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings)); // main ASSERT_EQ(loaded_conf_files.size(), 1); @@ -447,7 +448,7 @@ TEST(Configuration, configuration_config_files_self) outfile.close(); yaml_helper conf; - ASSERT_ANY_THROW(conf.load_from_file("main.yaml", loaded_conf_files)); + ASSERT_ANY_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings)); std::filesystem::remove("main.yaml"); } @@ -499,7 +500,7 @@ TEST(Configuration, configuration_config_files_directory) outfile.close(); yaml_helper conf; - ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files)); + ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings)); // main + conf_2 + conf_3 ASSERT_EQ(loaded_conf_files.size(), 3); diff --git a/userspace/falco/app/actions/load_config.cpp b/userspace/falco/app/actions/load_config.cpp index c510ce7e..e8fa12c0 100644 --- a/userspace/falco/app/actions/load_config.cpp +++ b/userspace/falco/app/actions/load_config.cpp @@ -29,11 +29,15 @@ static falco::app::run_result apply_deprecated_options(const falco::app::state& falco::app::run_result falco::app::actions::load_config(const falco::app::state& s) { + // List of loaded conf files, ie: s.options.conf_filename + // plus all the `configs_files` expanded list of configs. + std::vector loaded_conf_files; + std::vector loaded_conf_warnings; try { if (!s.options.conf_filename.empty()) { - s.config->init(s.options.conf_filename, s.options.cmdline_config_options); + s.config->init(s.options.conf_filename, loaded_conf_files, loaded_conf_warnings, s.options.cmdline_config_options); } else { @@ -57,7 +61,16 @@ falco::app::run_result falco::app::actions::load_config(const falco::app::state& } if (!s.options.conf_filename.empty()) { - falco_logger::log(falco_logger::level::INFO, "Falco initialized with configuration file: " + s.options.conf_filename + "\n"); + falco_logger::log(falco_logger::level::INFO, "Falco initialized with configuration files:\n"); + for (const auto& path : loaded_conf_files) + { + falco_logger::log(falco_logger::level::INFO, std::string(" ") + path + "\n"); + } + + for (const auto &warn : loaded_conf_warnings) + { + falco_logger::log(falco_logger::level::WARNING, warn + "\n"); + } } s.config->m_buffered_outputs = !s.options.unbuffered_outputs; diff --git a/userspace/falco/configuration.cpp b/userspace/falco/configuration.cpp index ece5d6dd..50c25b07 100644 --- a/userspace/falco/configuration.cpp +++ b/userspace/falco/configuration.cpp @@ -91,29 +91,21 @@ void falco_configuration::init(const std::vector& cmdline_options) load_yaml("default", config); } -void falco_configuration::init(const std::string& conf_filename, const std::vector &cmdline_options) +void falco_configuration::init(const std::string& conf_filename, std::vector& loaded_conf_files, + std::vector& loaded_conf_warnings, const std::vector &cmdline_options) { yaml_helper config; - std::vector loaded_files; try { - config.load_from_file(conf_filename, loaded_files); + config.load_from_file(conf_filename, loaded_conf_files, loaded_conf_warnings); } catch(const std::exception& e) { std::cerr << "Cannot read config file (" + conf_filename + "): " + e.what() + "\n"; throw e; } - init_cmdline_options(config, cmdline_options); load_yaml(conf_filename, config); - - // Here we have already set up correct logging level - falco_logger::log(falco_logger::level::DEBUG, "Loaded config filenames:\n"); - for (const auto& path : loaded_files) - { - falco_logger::log(falco_logger::level::DEBUG, std::string(" ") + path + "\n"); - } } void falco_configuration::load_engine_config(const std::string& config_name, const yaml_helper& config) diff --git a/userspace/falco/configuration.h b/userspace/falco/configuration.h index 95cae626..8185eb8e 100644 --- a/userspace/falco/configuration.h +++ b/userspace/falco/configuration.h @@ -86,7 +86,7 @@ public: falco_configuration(); virtual ~falco_configuration() = default; - void init(const std::string& conf_filename, const std::vector& cmdline_options); + void init(const std::string& conf_filename, std::vector& loaded_conf_files, std::vector& loaded_conf_warnings, const std::vector& cmdline_options); void init(const std::vector& cmdline_options); static void read_rules_file_directory(const std::string& path, std::list& rules_filenames, std::list &rules_folders); diff --git a/userspace/falco/yaml_helper.h b/userspace/falco/yaml_helper.h index b1b56e5e..82088896 100644 --- a/userspace/falco/yaml_helper.h +++ b/userspace/falco/yaml_helper.h @@ -92,9 +92,11 @@ public: /** * Load the YAML document from the given file path. */ - void load_from_file(const std::string& path, std::vector& loaded_config_files) + void load_from_file(const std::string& path, std::vector& loaded_config_files, std::vector& loaded_config_warnings) { loaded_config_files.clear(); + loaded_config_warnings.clear(); + m_root = load_from_file_int(path, loaded_config_files); const auto ppath = std::filesystem::path(path); @@ -137,7 +139,7 @@ public: // We don't support nested directories else { - falco_logger::log(falco_logger::level::WARNING, "Included config file has wrong type: " + dir_entry.path().string()); + loaded_config_warnings.push_back("Included config file has wrong type: " + dir_entry.path().string()); } } std::sort(v.begin(), v.end()); @@ -148,12 +150,12 @@ public: } else { - falco_logger::log(falco_logger::level::WARNING, "Included config entry has wrong type: " + include_file_path.string()); + loaded_config_warnings.push_back("Included config entry has wrong type: " + include_file_path.string()); } } else { - falco_logger::log(falco_logger::level::WARNING, "Included config entry unexistent: " + include_file_path.string()); + loaded_config_warnings.push_back("Included config entry unexistent: " + include_file_path.string()); } } }