chore(userspace,unit_tests): enable override of main config from secondary config files.

Moreover, do not trigger an exception when an included file is not present; just print a warning.
Finally, add more tests.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
This commit is contained in:
Federico Di Pierro 2024-01-19 09:23:18 +01:00 committed by poiana
parent 6e1f128851
commit 8112f6210b
2 changed files with 75 additions and 32 deletions

View File

@ -185,7 +185,38 @@ TEST(Configuration, configuration_include_files)
ASSERT_TRUE(conf.is_defined("base_value_3.name")); ASSERT_TRUE(conf.is_defined("base_value_3.name"));
ASSERT_EQ(conf.get_scalar<std::string>("base_value_3.name", ""), "foo3"); ASSERT_EQ(conf.get_scalar<std::string>("base_value_3.name", ""), "foo3");
/* Test that included config files are not able to override configs from main file */ /*
* Test that when main config file is in a different folder,
* other config files are searched relative to its folder,
* when they are specified as relative paths
*/
const auto temp_main = std::filesystem::temp_directory_path() / "main.yaml";
const std::string main_conf_yaml_relative_absolute =
"includes:\n"
" - conf_2.yaml\n"
" - " + std::filesystem::current_path().string() + "/conf_3.yaml\n"
"foo: bar\n"
"base_value:\n"
" id: 1\n"
" name: foo\n";
outfile.open(temp_main.string());
outfile << main_conf_yaml_relative_absolute;
outfile.close();
ASSERT_NO_THROW(conf.load_from_file(temp_main.string()));
ASSERT_TRUE(conf.is_defined("foo"));
ASSERT_EQ(conf.get_scalar<std::string>("foo", ""), "bar");
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");
ASSERT_FALSE(conf.is_defined("foo2")); // failed to include the config file since it is relative and is not under /tmp
ASSERT_FALSE(conf.is_defined("base_value_2")); // failed to include the config file since it is relative and is not under /tmp
ASSERT_TRUE(conf.is_defined("base_value_3.id")); // conf_3 was correctly included since it is absolute
ASSERT_EQ(conf.get_scalar<int>("base_value_3.id", 0), 3);
/* Test that included config files are able to override configs from main file */
const std::string conf_yaml_3_override = const std::string conf_yaml_3_override =
"base_value:\n" "base_value:\n"
" id: 3\n"; " id: 3\n";
@ -193,9 +224,19 @@ TEST(Configuration, configuration_include_files)
outfile << conf_yaml_3_override; outfile << conf_yaml_3_override;
outfile.close(); outfile.close();
ASSERT_ANY_THROW(conf.load_from_file("main.yaml")); ASSERT_NO_THROW(conf.load_from_file("main.yaml"));
ASSERT_TRUE(conf.is_defined("foo"));
ASSERT_EQ(conf.get_scalar<std::string>("foo", ""), "bar");
ASSERT_TRUE(conf.is_defined("base_value.id"));
ASSERT_EQ(conf.get_scalar<int>("base_value.id", 0), 3); // overridden!
ASSERT_FALSE(conf.is_defined("base_value.name")); // no more present since entire `base_value` block was overridden
ASSERT_TRUE(conf.is_defined("foo2"));
ASSERT_EQ(conf.get_scalar<std::string>("foo2", ""), "bar2");
ASSERT_TRUE(conf.is_defined("base_value_2.id"));
ASSERT_EQ(conf.get_scalar<int>("base_value_2.id", 0), 2);
ASSERT_FALSE(conf.is_defined("base_value_3.id")); // not defined
/* Test that including an unexistent file triggers an exception */ /* Test that including an unexistent file just skips it */
const std::string main_conf_unexistent_yaml = const std::string main_conf_unexistent_yaml =
"includes:\n" "includes:\n"
" - conf_5.yaml\n" " - conf_5.yaml\n"
@ -207,9 +248,13 @@ TEST(Configuration, configuration_include_files)
outfile << main_conf_unexistent_yaml; outfile << main_conf_unexistent_yaml;
outfile.close(); outfile.close();
ASSERT_ANY_THROW(conf.load_from_file("main.yaml")); ASSERT_NO_THROW(conf.load_from_file("main.yaml"));
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");
/* Test that a single file can be included as a scalar (thanks to get_sequence_from_node magic)*/ /* Test that a single file can be included as a scalar (thanks to get_sequence_from_node magic) */
const std::string main_conf_yaml_no_list = const std::string main_conf_yaml_no_list =
"includes: conf_2.yaml\n" "includes: conf_2.yaml\n"
"foo: bar\n" "foo: bar\n"
@ -232,12 +277,13 @@ TEST(Configuration, configuration_include_files)
ASSERT_EQ(conf.get_scalar<std::string>("foo2", ""), "bar2"); ASSERT_EQ(conf.get_scalar<std::string>("foo2", ""), "bar2");
ASSERT_TRUE(conf.is_defined("base_value_2.id")); ASSERT_TRUE(conf.is_defined("base_value_2.id"));
ASSERT_EQ(conf.get_scalar<int>("base_value_2.id", 0), 2); ASSERT_EQ(conf.get_scalar<int>("base_value_2.id", 0), 2);
// Cleanup everything // Cleanup everything
std::filesystem::remove("main.yaml"); std::filesystem::remove("main.yaml");
std::filesystem::remove("conf_2.yaml"); std::filesystem::remove("conf_2.yaml");
std::filesystem::remove("conf_3.yaml"); std::filesystem::remove("conf_3.yaml");
std::filesystem::remove("conf_4.yaml"); std::filesystem::remove("conf_4.yaml");
std::filesystem::remove(temp_main.string());
} }
TEST(Configuration, configuration_environment_variables) TEST(Configuration, configuration_environment_variables)

View File

@ -95,7 +95,7 @@ public:
m_root = load_from_file_int(path); m_root = load_from_file_int(path);
const auto ppath = std::filesystem::path(path); const auto ppath = std::filesystem::path(path);
const auto config_folder = ppath.std::filesystem::path::parent_path(); const auto config_folder = ppath.parent_path();
// Parse files to be included // Parse files to be included
std::vector<std::string> include_files; std::vector<std::string> include_files;
get_sequence<std::vector<std::string>>(include_files, "includes"); get_sequence<std::vector<std::string>>(include_files, "includes");
@ -107,36 +107,33 @@ public:
auto include_file_path = std::filesystem::path(include_file); auto include_file_path = std::filesystem::path(include_file);
if (!include_file_path.is_absolute()) if (!include_file_path.is_absolute())
{ {
include_file_path = config_folder; include_file_path = config_folder / include_file;
include_file_path += include_file;
} }
auto loaded_nodes = load_from_file_int(include_file_path.string()); if (std::filesystem::exists(include_file_path) && std::filesystem::is_regular_file(include_file_path))
for(auto n : loaded_nodes)
{ {
/* auto loaded_nodes = load_from_file_int(include_file_path.string());
* To avoid recursion hell, for(auto n : loaded_nodes)
* we don't support `includes` directives from included config files
* (that use load_from_file_int recursively).
*/
const auto &key = n.first.Scalar();
if (key == "includes")
{ {
throw std::runtime_error("Config error: 'includes' directive in included config file " + include_file + "."); /*
} * To avoid recursion hell,
* we don't support `includes` directives from included config files
YAML::Node node; * (that use load_from_file_int recursively).
get_node(node, key); */
if (!node.IsDefined()) const auto &key = n.first.Scalar();
{ if (key == "includes")
// There was no such node in root config file; proceed. {
node = n.second; throw std::runtime_error("Config error: 'includes' directive in included config file " + include_file + ".");
} }
else // We allow to override keys.
{ // We don't need to use `get_node()` here,
throw std::runtime_error("Config error: included config files cannot override root config nodes: " // since key is a top-level one.
+ include_file + " tried to override '" + key + "'."); m_root[key] = n.second;
} }
} }
else
{
falco_logger::log(falco_logger::level::WARNING, "Included config file unexistent or wrong type: " + include_file_path.string());
}
} }
} }