chore(userspace,unit_tests): properly report all schema validation warnings from yaml_helper::validate_node().

`-V` option will print all warnings, while normal run will only print foremost warning.

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

unit_tests/falco/test_configuration_schema.cpp

@@ -111,14 +111,14 @@ TEST(Configuration, schema_yaml_helper_validator)
 	EXPECT_NO_THROW(conf.load_from_string(sample_yaml));
 
 	// We pass a string variable but not a schema
-	std::string validation;
+	std::vector<std::string> validation;
 	EXPECT_NO_THROW(conf.load_from_string(sample_yaml, nlohmann::json{}, &validation));
-	EXPECT_EQ(validation, yaml_helper::validation_none);
+	EXPECT_EQ(validation[0], yaml_helper::validation_none);
 
 	// We pass a schema but not a string storage for the validation; no validation takes place
 	EXPECT_NO_THROW(conf.load_from_string(sample_yaml, falco_config.m_config_schema, nullptr));
 
 	// We pass everything
 	EXPECT_NO_THROW(conf.load_from_string(sample_yaml, falco_config.m_config_schema, &validation));
-	EXPECT_EQ(validation, yaml_helper::validation_ok);
+	EXPECT_EQ(validation[0], yaml_helper::validation_ok);
 }

userspace/engine/rule_loader.cpp

@@ -283,8 +283,7 @@ std::string rule_loader::context::snippet(const falco::load_result::rules_conten
 
 rule_loader::result::result(const std::string &name)
 	: name(name),
-	  success(true),
+	  success(true)
-	  schema_validation_str(yaml_helper::validation_none)
 {
 }
 
@@ -300,7 +299,11 @@ bool rule_loader::result::has_warnings()
 
 std::string rule_loader::result::schema_validation()
 {
-	return schema_validation_str;
+	if (schema_validation_status.empty())
+	{
+		return yaml_helper::validation_none;
+	}
+	return schema_validation_status[0];
 }
 
 void rule_loader::result::add_error(load_result::error_code ec, const std::string& msg, const context& ctx)
@@ -318,9 +321,9 @@ void rule_loader::result::add_warning(load_result::warning_code wc, const std::s
 	warnings.push_back(warn);
 }
 
-void rule_loader::result::set_schema_validation_status(const std::string& status)
+void rule_loader::result::set_schema_validation_status(const std::vector<std::string>& status)
 {
-	schema_validation_str = status;
+	schema_validation_status = status;
 }
 
 const std::string& rule_loader::result::as_string(bool verbose, const rules_contents_t& contents)
@@ -364,9 +367,28 @@ const std::string& rule_loader::result::as_summary_string()
 	}
 
 	// Only print schema validation info if any validation was requested
-	if (schema_validation_str != yaml_helper::validation_none)
+	if (!schema_validation_status.empty())
 	{
-		os << " | schema validation: " << schema_validation_str;
+		bool schema_valid = schema_validation() == yaml_helper::validation_ok;
+		// Only print info when there are validation warnings
+		if (!schema_valid)
+		{
+			os << std::endl;
+
+			os << schema_validation_status.size() << " schema warnings: [";
+			bool first = true;
+			for(auto& status : schema_validation_status)
+			{
+				if(!first)
+				{
+					os << " ";
+				}
+				first = false;
+
+				os << status;
+			}
+			os << "]";
+		}
 	}
 
 	if(!errors.empty())
@@ -442,9 +464,28 @@ const std::string& rule_loader::result::as_verbose_string(const rules_contents_t
 	}
 
 	// Only print schema validation info if any validation was requested
-	if (schema_validation_str != yaml_helper::validation_none)
+	if (!schema_validation_status.empty())
 	{
-		os << " | schema validation: " << schema_validation_str;
+		bool schema_valid = schema_validation() == yaml_helper::validation_ok;
+		// Only print info when there are validation warnings
+		if (!schema_valid)
+		{
+			os << std::endl;
+
+			os << schema_validation_status.size() << " schema warnings: [";
+			bool first = true;
+			for(auto& status : schema_validation_status)
+			{
+				if(!first)
+				{
+					os << " ";
+				}
+				first = false;
+
+				os << status;
+			}
+			os << "]";
+		}
 	}
 
 	if (!errors.empty())
@@ -507,14 +548,17 @@ const nlohmann::json& rule_loader::result::as_json(const rules_contents_t& conte
 	j["successful"] = success;
 
 	// Only print schema validation info if any validation was requested
-	if (schema_validation_str != yaml_helper::validation_none)
+	if (!schema_validation_status.empty())
 	{
-		bool schema_valid = schema_validation_str == yaml_helper::validation_ok;
+		bool schema_valid = schema_validation() == yaml_helper::validation_ok;
 		j["schema_valid"] = schema_valid;
 		j["schema_warnings"] = nlohmann::json::array();
 		if (!schema_valid)
 		{
-			j["schema_warnings"].push_back(schema_validation_str);
+			for (const auto &schema_warning : schema_validation_status)
+			{
+				j["schema_warnings"].push_back(schema_warning);
+			}
 		}
 	}
 

userspace/engine/rule_loader.h

@@ -248,7 +248,7 @@ namespace rule_loader
 				 const std::string& msg,
 				 const context& ctx);
 
-		void set_schema_validation_status(const std::string& status);
+		void set_schema_validation_status(const std::vector<std::string>& status);
 		std::string schema_validation();
 	protected:
 
@@ -256,7 +256,7 @@ namespace rule_loader
 		const std::string& as_verbose_string(const falco::load_result::rules_contents_t& contents);
 		std::string name;
 		bool success;
-		std::string schema_validation_str;
+		std::vector<std::string> schema_validation_status;
 
 		std::vector<error> errors;
 		std::vector<warning> warnings;

userspace/engine/rule_loader_reader.cpp

@@ -788,11 +788,11 @@ bool rule_loader::reader::read(rule_loader::configuration& cfg, collector& colle
 {
 	std::vector<YAML::Node> docs;
 	yaml_helper reader;
-	std::string schema_validation;
+	std::vector<std::string> schema_warnings;
 	rule_loader::context ctx(cfg.name);
 	try
 	{
-		docs = reader.loadall_from_string(cfg.content, schema, &schema_validation);
+		docs = reader.loadall_from_string(cfg.content, schema, &schema_warnings);
 	}
 	catch (YAML::ParserException& e)
 	{
@@ -810,7 +810,7 @@ bool rule_loader::reader::read(rule_loader::configuration& cfg, collector& colle
 		cfg.res->add_error(falco::load_result::LOAD_ERR_YAML_PARSE, "unknown YAML parsing error", ctx);
 		return false;
 	}
-	cfg.res->set_schema_validation_status(schema_validation);
+	cfg.res->set_schema_validation_status(schema_warnings);
 	for (auto doc = docs.begin(); doc != docs.end(); doc++)
 	{
 		if (doc->IsDefined() && !doc->IsNull())

userspace/engine/yaml_helper.h

@@ -90,27 +90,23 @@ public:
 	* Load all the YAML document represented by the input string.
 	* Since this is used by rule loader, does not process env vars.
 	*/
-	std::vector<YAML::Node> loadall_from_string(const std::string& input, const nlohmann::json& schema={}, std::string *validation=nullptr)
+	std::vector<YAML::Node> loadall_from_string(const std::string& input, const nlohmann::json& schema={}, std::vector<std::string> *schema_warnings=nullptr)
 	{
 		auto nodes = YAML::LoadAll(input);
-		if (validation)
+		if (schema_warnings)
 		{
+			schema_warnings->clear();
 			if(!schema.empty())
 			{
 				// Validate each node.
 				for(const auto& node : nodes)
 				{
-					*validation = validate_node(node, schema);
+					validate_node(node, schema, schema_warnings);
-					if (*validation != validation_ok)
-					{
-						// Return first error
-						break;
-					}
 				}
 			}
 			else
 			{
-				*validation = validation_none;
+				schema_warnings->push_back(validation_none);
 			}
 		}
 		return nodes;
@@ -119,20 +115,21 @@ public:
 	/**
 	* Load the YAML document represented by the input string.
 	*/
-	void load_from_string(const std::string& input, const nlohmann::json& schema={}, std::string *validation=nullptr)
+	void load_from_string(const std::string& input, const nlohmann::json& schema={}, std::vector<std::string> *schema_warnings=nullptr)
 	{
 		m_root = YAML::Load(input);
 		pre_process_env_vars(m_root);
 
-		if (validation)
+		if (schema_warnings)
 		{
+			schema_warnings->clear();
 			if(!schema.empty())
 			{
-				*validation = validate_node(m_root, schema);
+				validate_node(m_root, schema, schema_warnings);
 			}
 			else
 			{
-				*validation = validation_none;
+				schema_warnings->push_back(validation_none);
 			}
 		}
 	}
@@ -140,14 +137,14 @@ public:
 	/**
 	* Load the YAML document from the given file path.
 	*/
-	void load_from_file(const std::string& path, const nlohmann::json& schema={}, std::string *validation=nullptr)
+	void load_from_file(const std::string& path, const nlohmann::json& schema={}, std::vector<std::string> *schema_warnings=nullptr)
 	{
-		m_root = load_from_file_int(path, schema, validation);
+		m_root = load_from_file_int(path, schema, schema_warnings);
 	}
 
-	void include_config_file(const std::string& include_file_path, const nlohmann::json& schema={}, std::string *validation=nullptr)
+	void include_config_file(const std::string& include_file_path, const nlohmann::json& schema={}, std::vector<std::string> *schema_warnings=nullptr)
 	{
-		auto loaded_nodes = load_from_file_int(include_file_path, schema, validation);
+		auto loaded_nodes = load_from_file_int(include_file_path, schema, schema_warnings);
 		for(auto n : loaded_nodes)
 		{
 			/*
@@ -243,26 +240,27 @@ public:
 private:
 	YAML::Node m_root;
 
-	YAML::Node load_from_file_int(const std::string& path, const nlohmann::json& schema={}, std::string *validation=nullptr)
+	YAML::Node load_from_file_int(const std::string& path, const nlohmann::json& schema, std::vector<std::string> *schema_warnings)
 	{
 		auto root = YAML::LoadFile(path);
 		pre_process_env_vars(root);
 
-		if (validation)
+		if (schema_warnings)
 		{
+			schema_warnings->clear();
 			if(!schema.empty())
 			{
-				*validation = validate_node(root, schema);
+				validate_node(root, schema, schema_warnings);
 			}
 			else
 			{
-				*validation = validation_none;
+				schema_warnings->push_back(validation_none);
 			}
 		}
 		return root;
 	}
 
-	std::string validate_node(const YAML::Node &node, const nlohmann::json& schema={})
+	void validate_node(const YAML::Node &node, const nlohmann::json& schema, std::vector<std::string> *schema_warnings)
 	{
 		// Validate the yaml against our json schema
 		valijson::Schema schemaDef;
@@ -277,16 +275,18 @@ private:
 		{
 			valijson::ValidationResults::Error error;
 			// report only the top-most error
-			if (validationResults.popError(error))
+			while (validationResults.popError(error))
 			{
-				return std::string(validation_failed + " for ")
+				schema_warnings->push_back(std::string(validation_failed + " for ")
 				       + std::accumulate(error.context.begin(), error.context.end(), std::string(""))
 				       + ": "
-				       + error.description;
+				       + error.description);
 			}
-			return validation_failed;
 		}
-		return validation_ok;
+		else
+		{
+			schema_warnings->push_back(validation_ok);
+		}
 	}
 
 	/*

userspace/falco/configuration.cpp

@@ -93,12 +93,13 @@ falco_configuration::falco_configuration():
 config_loaded_res falco_configuration::init_from_content(const std::string& config_content, const std::vector<std::string>& cmdline_options, const std::string& filename)
 {
 	config_loaded_res res;
-	std::string validation_status;
+	std::vector<std::string> validation_status;
 
 	m_config.load_from_string(config_content, m_config_schema, &validation_status);
 	init_cmdline_options(cmdline_options);
 
-	res[filename] = validation_status;
+	// Only report top most schema validation status
+	res[filename] = validation_status[0];
 
 	load_yaml(filename);
 	return res;
@@ -107,7 +108,7 @@ config_loaded_res falco_configuration::init_from_content(const std::string& conf
 config_loaded_res falco_configuration::init_from_file(const std::string& conf_filename, const std::vector<std::string> &cmdline_options)
 {
 	config_loaded_res res;
-	std::string validation_status;
+	std::vector<std::string> validation_status;
 	try
 	{
 		m_config.load_from_file(conf_filename, m_config_schema, &validation_status);
@@ -119,7 +120,8 @@ config_loaded_res falco_configuration::init_from_file(const std::string& conf_fi
 	}
 	init_cmdline_options(cmdline_options);
 
-	res[conf_filename] = validation_status;
+	// Only report top most schema validation status
+	res[conf_filename] = validation_status[0];
 
 	merge_config_files(conf_filename, res);
 	load_yaml(conf_filename);
@@ -138,7 +140,7 @@ std::string falco_configuration::dump()
 // filenames and folders specified in config (minus the skipped ones).
 void falco_configuration::merge_config_files(const std::string& config_name, config_loaded_res &res)
 {
-	std::string validation_status;
+	std::vector<std::string> validation_status;
 	m_loaded_configs_filenames.push_back(config_name);
 	const auto ppath = std::filesystem::path(config_name);
 	// Parse files to be included
@@ -161,7 +163,8 @@ void falco_configuration::merge_config_files(const std::string& config_name, con
 		{
 			m_loaded_configs_filenames.push_back(include_file);
 			m_config.include_config_file(include_file_path.string(), m_config_schema, &validation_status);
-			res[include_file_path.string()] = validation_status;
+			// Only report top most schema validation status
+			res[include_file_path.string()] = validation_status[0];
 		}
 		else if (std::filesystem::is_directory(include_file_path))
 		{
@@ -180,7 +183,8 @@ void falco_configuration::merge_config_files(const std::string& config_name, con
 			for (const auto &f : v)
 			{
 				m_config.include_config_file(f, m_config_schema, &validation_status);
-				res[f] = validation_status;
+				// Only report top most schema validation status
+				res[f] = validation_status[0];
 			}
 		}
 	}