chore(unit_tests,userspace/falco): throw an exception when included config file is not present.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

falco.yaml

@@ -125,11 +125,8 @@
 # exactly in the order they are specified.
 # Therefore, loaded config files *can* override values from main config file.
 # Also, nested include is not allowed, ie: included config files won't be able to include other config files.
-# When relative path is used, the config file is assumed to be in the same folder as the main config file.
-# Otherwise, absolute path must be used.
 #
-# Like for rules (below), specifying a folder will load all the configs files present in it
+# Like for 'rules_file', specifying a folder will load all the configs files present in it in a lexicographical order.
-# in a lexicographical order.
 configs_files:
   - /etc/falco/config.d
 

unit_tests/falco/test_configuration.cpp

@@ -38,7 +38,6 @@ static std::string sample_yaml =
 	"    - elem3\n";
 
 static std::vector<std::string> loaded_conf_files;
-static std::vector<std::string> loaded_conf_warnings;
 
 TEST(Configuration, configuration_exceptions)
 {
@@ -139,7 +138,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, loaded_conf_warnings));
+	ASSERT_ANY_THROW(conf.load_from_file("main.yaml", loaded_conf_files));
 
 	std::filesystem::remove("main.yaml");
 	std::filesystem::remove("conf_2.yaml");
@@ -186,11 +185,10 @@ TEST(Configuration, configuration_config_files_ok)
 	outfile.close();
 
 	yaml_helper conf;
-	ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings));
+	ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files));
 
 	// main + conf_2 + conf_3
 	ASSERT_EQ(loaded_conf_files.size(), 3);
-	ASSERT_EQ(loaded_conf_warnings.size(), 0);
 
 	ASSERT_TRUE(conf.is_defined("foo"));
 	ASSERT_EQ(conf.get_scalar<std::string>("foo", ""), "bar");
@@ -220,9 +218,8 @@ TEST(Configuration, configuration_config_files_ok)
 TEST(Configuration, configuration_config_files_relative_main)
 {
 	/*
-	 * Test that when main config file is in a different folder,
+	 * Test that relative path are treated as relative to cwd and not to main config folder,
-	 * other config files are searched relative to its folder,
+	 * and that absolute includes are ok too.
-	 * when they are specified as relative paths
 	 */
 	const auto temp_main = std::filesystem::temp_directory_path() / "main.yaml";
 	// So, conf_2 will be looked up in the same folder as main config file,
@@ -259,13 +256,10 @@ 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, loaded_conf_warnings));
+	ASSERT_NO_THROW(conf.load_from_file(temp_main.string(), loaded_conf_files));
 
-	// main + conf_3
+	// main + conf_2 + conf_3
-	ASSERT_EQ(loaded_conf_files.size(), 2);
+	ASSERT_EQ(loaded_conf_files.size(), 3);
-
-	// conf_2 gives warning
-	ASSERT_EQ(loaded_conf_warnings.size(), 1);
 
 	ASSERT_TRUE(conf.is_defined("foo"));
 	ASSERT_EQ(conf.get_scalar<std::string>("foo", ""), "bar");
@@ -273,9 +267,11 @@ TEST(Configuration, configuration_config_files_relative_main)
 	ASSERT_EQ(conf.get_scalar<int>("base_value.id", 0), 1);
 	ASSERT_TRUE(conf.is_defined("base_value.name"));
 	ASSERT_EQ(conf.get_scalar<std::string>("base_value.name", ""), "foo");
-	ASSERT_FALSE(conf.is_defined("foo2"));		 // failed to include the config file since it is relative and is not under /tmp
+	ASSERT_TRUE(conf.is_defined("foo2"));
-	ASSERT_FALSE(conf.is_defined("base_value_2"));	 // failed to include the config file since it is relative and is not under /tmp
+	ASSERT_EQ(conf.get_scalar<std::string>("foo2", ""), "bar2");
-	ASSERT_TRUE(conf.is_defined("base_value_3.id")); // conf_3 was correctly included since it is absolute
+	ASSERT_TRUE(conf.is_defined("base_value_2"));
+	ASSERT_EQ(conf.get_scalar<int>("base_value_2.id", 0), 2);
+	ASSERT_TRUE(conf.is_defined("base_value_3.id"));
 	ASSERT_EQ(conf.get_scalar<int>("base_value_3.id", 0), 3);
 
 	std::filesystem::remove(temp_main.string());
@@ -315,11 +311,10 @@ TEST(Configuration, configuration_config_files_override)
 	outfile.close();
 
 	yaml_helper conf;
-	ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings));
+	ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files));
 
 	// main + conf_2 + conf_3
 	ASSERT_EQ(loaded_conf_files.size(), 3);
-	ASSERT_EQ(loaded_conf_warnings.size(), 0);
 
 	ASSERT_TRUE(conf.is_defined("foo"));
 	ASSERT_EQ(conf.get_scalar<std::string>("foo", ""), "bar");
@@ -339,7 +334,7 @@ TEST(Configuration, configuration_config_files_override)
 
 TEST(Configuration, configuration_config_files_unexistent)
 {
-	/* Test that including an unexistent file just skips it */
+	/* Test that including an unexistent file throws an exception */
 	const std::string main_conf_yaml =
 		yaml_helper::configs_key + ":\n"
 		"  - conf_5.yaml\n"
@@ -352,18 +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, loaded_conf_warnings));
+	ASSERT_ANY_THROW(conf.load_from_file("main.yaml", loaded_conf_files));
-
-	// main
-	ASSERT_EQ(loaded_conf_files.size(), 1);
-
-	// conf_5 unexistent
-	ASSERT_EQ(loaded_conf_warnings.size(), 1);
-
-	ASSERT_TRUE(conf.is_defined("base_value.id"));
-	ASSERT_EQ(conf.get_scalar<int>("base_value.id", 0), 1);
-	ASSERT_TRUE(conf.is_defined("base_value.name"));
-	ASSERT_EQ(conf.get_scalar<std::string>("base_value.name", ""), "foo");
 
 	std::filesystem::remove("main.yaml");
 }
@@ -391,11 +375,10 @@ 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, loaded_conf_warnings));
+	ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files));
 
 	// main + conf_2
 	ASSERT_EQ(loaded_conf_files.size(), 2);
-	ASSERT_EQ(loaded_conf_warnings.size(), 0);
 
 	ASSERT_TRUE(conf.is_defined("foo"));
 	ASSERT_EQ(conf.get_scalar<std::string>("foo", ""), "bar");
@@ -427,11 +410,10 @@ 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, loaded_conf_warnings));
+	ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files));
 
 	// main
 	ASSERT_EQ(loaded_conf_files.size(), 1);
-	ASSERT_EQ(loaded_conf_warnings.size(), 0);
 
 	ASSERT_TRUE(conf.is_defined("foo"));
 	ASSERT_EQ(conf.get_scalar<std::string>("foo", ""), "bar");
@@ -458,7 +440,7 @@ TEST(Configuration, configuration_config_files_self)
 	outfile.close();
 
 	yaml_helper conf;
-	ASSERT_ANY_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings));
+	ASSERT_ANY_THROW(conf.load_from_file("main.yaml", loaded_conf_files));
 
 	std::filesystem::remove("main.yaml");
 }
@@ -510,14 +492,12 @@ TEST(Configuration, configuration_config_files_directory)
 	outfile.close();
 
 	yaml_helper conf;
-	ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files, loaded_conf_warnings));
+	ASSERT_NO_THROW(conf.load_from_file("main.yaml", loaded_conf_files));
 
-	// main + conf_2 + conf_3
+	// main + conf_2 + conf_3.
+	// test/foo is not parsed.
 	ASSERT_EQ(loaded_conf_files.size(), 3);
 
-	// Found a folder (test/foo) that will be skipped
-	ASSERT_EQ(loaded_conf_warnings.size(), 1);
-
 	ASSERT_TRUE(conf.is_defined("foo"));
 	ASSERT_EQ(conf.get_scalar<std::string>("foo", ""), "bar");
 	ASSERT_TRUE(conf.is_defined("base_value.id"));

userspace/falco/app/actions/load_config.cpp

@@ -32,12 +32,11 @@ falco::app::run_result falco::app::actions::load_config(const falco::app::state&
 	// List of loaded conf files, ie: s.options.conf_filename
 	// plus all the `configs_files` expanded list of configs.
 	std::vector<std::string> loaded_conf_files;
-	std::vector<std::string> loaded_conf_warnings;
 	try
 	{
 		if (!s.options.conf_filename.empty())
 		{
-			s.config->init(s.options.conf_filename, loaded_conf_files, loaded_conf_warnings, s.options.cmdline_config_options);
+			s.config->init(s.options.conf_filename, loaded_conf_files, s.options.cmdline_config_options);
 		}
 		else
 		{
@@ -66,11 +65,6 @@ falco::app::run_result falco::app::actions::load_config(const falco::app::state&
 		{
 			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;

userspace/falco/configuration.cpp

@@ -91,11 +91,11 @@ void falco_configuration::init(const std::vector<std::string>& cmdline_options)
 }
 
 void falco_configuration::init(const std::string& conf_filename, std::vector<std::string>& loaded_conf_files,
-			       std::vector<std::string>& loaded_conf_warnings, const std::vector<std::string> &cmdline_options)
+			       const std::vector<std::string> &cmdline_options)
 {
 	try
 	{
-		config.load_from_file(conf_filename, loaded_conf_files, loaded_conf_warnings);
+		config.load_from_file(conf_filename, loaded_conf_files);
 	}
 	catch(const std::exception& e)
 	{

userspace/falco/configuration.h

@@ -86,7 +86,7 @@ public:
 	falco_configuration();
 	virtual ~falco_configuration() = default;
 
-	void init(const std::string& conf_filename, std::vector<std::string>& loaded_conf_files, std::vector<std::string>& loaded_conf_warnings, const std::vector<std::string>& cmdline_options);
+	void init(const std::string& conf_filename, std::vector<std::string>& loaded_conf_files, const std::vector<std::string>& cmdline_options);
 	void init(const std::vector<std::string>& cmdline_options);
 
 	std::string dump();

userspace/falco/yaml_helper.h

@@ -92,35 +92,28 @@ public:
 	/**
 	* Load the YAML document from the given file path.
 	*/
-	void load_from_file(const std::string& path, std::vector<std::string>& loaded_config_files, std::vector<std::string>& loaded_config_warnings)
+	void load_from_file(const std::string& path, std::vector<std::string>& loaded_config_files)
 	{
 		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);
-		const auto config_folder = ppath.parent_path();
 		// Parse files to be included
 		std::vector<std::string> include_files;
 		get_sequence<std::vector<std::string>>(include_files, configs_key);
 		for(const std::string& include_file : include_files)
 		{
-			// If user specifies a relative include file,
-			// make it relative to main config file folder,
-			// instead of cwd.
 			auto include_file_path = std::filesystem::path(include_file);
-			if (!include_file_path.is_absolute())
-			{
-				include_file_path = config_folder / include_file;
-			}
 			if (include_file_path == ppath)
 			{
 				throw std::runtime_error(
 					"Config error: '" + configs_key + "' directive tried to recursively include main config file: " + path + ".");
 			}
-			if (std::filesystem::exists(include_file_path))
+			if (!std::filesystem::exists(include_file_path))
 			{
+				throw std::runtime_error("Included config entry not existent: " + include_file_path.string());
+			}
 			if (std::filesystem::is_regular_file(include_file_path))
 			{
 				include_config_file(include_file_path.string(), loaded_config_files);
@@ -136,11 +129,6 @@ public:
 					{
 						v.push_back(dir_entry.path().string());
 					}
-						// We don't support nested directories
-						else
-						{
-							loaded_config_warnings.push_back("Included config file has wrong type: " + dir_entry.path().string());
-						}
 				}
 				std::sort(v.begin(), v.end());
 				for (const auto &f : v)
@@ -148,15 +136,6 @@ public:
 					include_config_file(f, loaded_config_files);
 				}
 			}
-				else
-				{
-					loaded_config_warnings.push_back("Included config entry has wrong type: " + include_file_path.string());
-				}
-			}
-			else
-			{
-				loaded_config_warnings.push_back("Included config entry unexistent: " + include_file_path.string());
-			}
 		}
 	}