Skip to content

Commit 6630176

Browse files
committed
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 <[email protected]>
1 parent 482ea97 commit 6630176

File tree

6 files changed

+103
-55
lines changed

6 files changed

+103
-55
lines changed

unit_tests/falco/test_configuration_schema.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,14 @@ TEST(Configuration, schema_yaml_helper_validator)
111111
EXPECT_NO_THROW(conf.load_from_string(sample_yaml));
112112

113113
// We pass a string variable but not a schema
114-
std::string validation;
114+
std::vector<std::string> validation;
115115
EXPECT_NO_THROW(conf.load_from_string(sample_yaml, nlohmann::json{}, &validation));
116-
EXPECT_EQ(validation, yaml_helper::validation_none);
116+
EXPECT_EQ(validation[0], yaml_helper::validation_none);
117117

118118
// We pass a schema but not a string storage for the validation; no validation takes place
119119
EXPECT_NO_THROW(conf.load_from_string(sample_yaml, falco_config.m_config_schema, nullptr));
120120

121121
// We pass everything
122122
EXPECT_NO_THROW(conf.load_from_string(sample_yaml, falco_config.m_config_schema, &validation));
123-
EXPECT_EQ(validation, yaml_helper::validation_ok);
123+
EXPECT_EQ(validation[0], yaml_helper::validation_ok);
124124
}

userspace/engine/rule_loader.cpp

+56-12
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,7 @@ std::string rule_loader::context::snippet(const falco::load_result::rules_conten
283283

284284
rule_loader::result::result(const std::string &name)
285285
: name(name),
286-
success(true),
287-
schema_validation_str(yaml_helper::validation_none)
286+
success(true)
288287
{
289288
}
290289

@@ -300,7 +299,11 @@ bool rule_loader::result::has_warnings()
300299

301300
std::string rule_loader::result::schema_validation()
302301
{
303-
return schema_validation_str;
302+
if (schema_validation_status.empty())
303+
{
304+
return yaml_helper::validation_none;
305+
}
306+
return schema_validation_status[0];
304307
}
305308

306309
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
318321
warnings.push_back(warn);
319322
}
320323

321-
void rule_loader::result::set_schema_validation_status(const std::string& status)
324+
void rule_loader::result::set_schema_validation_status(const std::vector<std::string>& status)
322325
{
323-
schema_validation_str = status;
326+
schema_validation_status = status;
324327
}
325328

326329
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()
364367
}
365368

366369
// Only print schema validation info if any validation was requested
367-
if (schema_validation_str != yaml_helper::validation_none)
370+
if (!schema_validation_status.empty())
368371
{
369-
os << " | schema validation: " << schema_validation_str;
372+
bool schema_valid = schema_validation() == yaml_helper::validation_ok;
373+
// Only print info when there are validation warnings
374+
if (!schema_valid)
375+
{
376+
os << std::endl;
377+
378+
os << schema_validation_status.size() << " schema warnings: [";
379+
bool first = true;
380+
for(auto& status : schema_validation_status)
381+
{
382+
if(!first)
383+
{
384+
os << " ";
385+
}
386+
first = false;
387+
388+
os << status;
389+
}
390+
os << "]";
391+
}
370392
}
371393

372394
if(!errors.empty())
@@ -442,9 +464,28 @@ const std::string& rule_loader::result::as_verbose_string(const rules_contents_t
442464
}
443465

444466
// Only print schema validation info if any validation was requested
445-
if (schema_validation_str != yaml_helper::validation_none)
467+
if (!schema_validation_status.empty())
446468
{
447-
os << " | schema validation: " << schema_validation_str;
469+
bool schema_valid = schema_validation() == yaml_helper::validation_ok;
470+
// Only print info when there are validation warnings
471+
if (!schema_valid)
472+
{
473+
os << std::endl;
474+
475+
os << schema_validation_status.size() << " schema warnings: [";
476+
bool first = true;
477+
for(auto& status : schema_validation_status)
478+
{
479+
if(!first)
480+
{
481+
os << " ";
482+
}
483+
first = false;
484+
485+
os << status;
486+
}
487+
os << "]";
488+
}
448489
}
449490

450491
if (!errors.empty())
@@ -507,14 +548,17 @@ const nlohmann::json& rule_loader::result::as_json(const rules_contents_t& conte
507548
j["successful"] = success;
508549

509550
// Only print schema validation info if any validation was requested
510-
if (schema_validation_str != yaml_helper::validation_none)
551+
if (!schema_validation_status.empty())
511552
{
512-
bool schema_valid = schema_validation_str == yaml_helper::validation_ok;
553+
bool schema_valid = schema_validation() == yaml_helper::validation_ok;
513554
j["schema_valid"] = schema_valid;
514555
j["schema_warnings"] = nlohmann::json::array();
515556
if (!schema_valid)
516557
{
517-
j["schema_warnings"].push_back(schema_validation_str);
558+
for (const auto &schema_warning : schema_validation_status)
559+
{
560+
j["schema_warnings"].push_back(schema_warning);
561+
}
518562
}
519563
}
520564

userspace/engine/rule_loader.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -248,15 +248,15 @@ namespace rule_loader
248248
const std::string& msg,
249249
const context& ctx);
250250

251-
void set_schema_validation_status(const std::string& status);
251+
void set_schema_validation_status(const std::vector<std::string>& status);
252252
std::string schema_validation();
253253
protected:
254254

255255
const std::string& as_summary_string();
256256
const std::string& as_verbose_string(const falco::load_result::rules_contents_t& contents);
257257
std::string name;
258258
bool success;
259-
std::string schema_validation_str;
259+
std::vector<std::string> schema_validation_status;
260260

261261
std::vector<error> errors;
262262
std::vector<warning> warnings;

userspace/engine/rule_loader_reader.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -788,11 +788,11 @@ bool rule_loader::reader::read(rule_loader::configuration& cfg, collector& colle
788788
{
789789
std::vector<YAML::Node> docs;
790790
yaml_helper reader;
791-
std::string schema_validation;
791+
std::vector<std::string> schema_warnings;
792792
rule_loader::context ctx(cfg.name);
793793
try
794794
{
795-
docs = reader.loadall_from_string(cfg.content, schema, &schema_validation);
795+
docs = reader.loadall_from_string(cfg.content, schema, &schema_warnings);
796796
}
797797
catch (YAML::ParserException& e)
798798
{
@@ -810,7 +810,7 @@ bool rule_loader::reader::read(rule_loader::configuration& cfg, collector& colle
810810
cfg.res->add_error(falco::load_result::LOAD_ERR_YAML_PARSE, "unknown YAML parsing error", ctx);
811811
return false;
812812
}
813-
cfg.res->set_schema_validation_status(schema_validation);
813+
cfg.res->set_schema_validation_status(schema_warnings);
814814
for (auto doc = docs.begin(); doc != docs.end(); doc++)
815815
{
816816
if (doc->IsDefined() && !doc->IsNull())

userspace/engine/yaml_helper.h

+28-28
Original file line numberDiff line numberDiff line change
@@ -90,27 +90,23 @@ class yaml_helper
9090
* Load all the YAML document represented by the input string.
9191
* Since this is used by rule loader, does not process env vars.
9292
*/
93-
std::vector<YAML::Node> loadall_from_string(const std::string& input, const nlohmann::json& schema={}, std::string *validation=nullptr)
93+
std::vector<YAML::Node> loadall_from_string(const std::string& input, const nlohmann::json& schema={}, std::vector<std::string> *schema_warnings=nullptr)
9494
{
9595
auto nodes = YAML::LoadAll(input);
96-
if (validation)
96+
if (schema_warnings)
9797
{
98+
schema_warnings->clear();
9899
if(!schema.empty())
99100
{
100101
// Validate each node.
101-
for (const auto& node : nodes)
102+
for(const auto& node : nodes)
102103
{
103-
*validation = validate_node(node, schema);
104-
if (*validation != validation_ok)
105-
{
106-
// Return first error
107-
break;
108-
}
104+
validate_node(node, schema, schema_warnings);
109105
}
110106
}
111107
else
112108
{
113-
*validation = validation_none;
109+
schema_warnings->push_back(validation_none);
114110
}
115111
}
116112
return nodes;
@@ -119,35 +115,36 @@ class yaml_helper
119115
/**
120116
* Load the YAML document represented by the input string.
121117
*/
122-
void load_from_string(const std::string& input, const nlohmann::json& schema={}, std::string *validation=nullptr)
118+
void load_from_string(const std::string& input, const nlohmann::json& schema={}, std::vector<std::string> *schema_warnings=nullptr)
123119
{
124120
m_root = YAML::Load(input);
125121
pre_process_env_vars(m_root);
126122

127-
if (validation)
123+
if (schema_warnings)
128124
{
125+
schema_warnings->clear();
129126
if(!schema.empty())
130127
{
131-
*validation = validate_node(m_root, schema);
128+
validate_node(m_root, schema, schema_warnings);
132129
}
133130
else
134131
{
135-
*validation = validation_none;
132+
schema_warnings->push_back(validation_none);
136133
}
137134
}
138135
}
139136

140137
/**
141138
* Load the YAML document from the given file path.
142139
*/
143-
void load_from_file(const std::string& path, const nlohmann::json& schema={}, std::string *validation=nullptr)
140+
void load_from_file(const std::string& path, const nlohmann::json& schema={}, std::vector<std::string> *schema_warnings=nullptr)
144141
{
145-
m_root = load_from_file_int(path, schema, validation);
142+
m_root = load_from_file_int(path, schema, schema_warnings);
146143
}
147144

148-
void include_config_file(const std::string& include_file_path, const nlohmann::json& schema={}, std::string *validation=nullptr)
145+
void include_config_file(const std::string& include_file_path, const nlohmann::json& schema={}, std::vector<std::string> *schema_warnings=nullptr)
149146
{
150-
auto loaded_nodes = load_from_file_int(include_file_path, schema, validation);
147+
auto loaded_nodes = load_from_file_int(include_file_path, schema, schema_warnings);
151148
for(auto n : loaded_nodes)
152149
{
153150
/*
@@ -243,26 +240,27 @@ class yaml_helper
243240
private:
244241
YAML::Node m_root;
245242

246-
YAML::Node load_from_file_int(const std::string& path, const nlohmann::json& schema={}, std::string *validation=nullptr)
243+
YAML::Node load_from_file_int(const std::string& path, const nlohmann::json& schema, std::vector<std::string> *schema_warnings)
247244
{
248245
auto root = YAML::LoadFile(path);
249246
pre_process_env_vars(root);
250247

251-
if (validation)
248+
if (schema_warnings)
252249
{
250+
schema_warnings->clear();
253251
if(!schema.empty())
254252
{
255-
*validation = validate_node(root, schema);
253+
validate_node(root, schema, schema_warnings);
256254
}
257255
else
258256
{
259-
*validation = validation_none;
257+
schema_warnings->push_back(validation_none);
260258
}
261259
}
262260
return root;
263261
}
264262

265-
std::string validate_node(const YAML::Node &node, const nlohmann::json& schema={})
263+
void validate_node(const YAML::Node &node, const nlohmann::json& schema, std::vector<std::string> *schema_warnings)
266264
{
267265
// Validate the yaml against our json schema
268266
valijson::Schema schemaDef;
@@ -277,16 +275,18 @@ class yaml_helper
277275
{
278276
valijson::ValidationResults::Error error;
279277
// report only the top-most error
280-
if (validationResults.popError(error))
278+
while (validationResults.popError(error))
281279
{
282-
return std::string(validation_failed + " for ")
280+
schema_warnings->push_back(std::string(validation_failed + " for ")
283281
+ std::accumulate(error.context.begin(), error.context.end(), std::string(""))
284282
+ ": "
285-
+ error.description;
283+
+ error.description);
286284
}
287-
return validation_failed;
288285
}
289-
return validation_ok;
286+
else
287+
{
288+
schema_warnings->push_back(validation_ok);
289+
}
290290
}
291291

292292
/*

userspace/falco/configuration.cpp

+11-7
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,13 @@ falco_configuration::falco_configuration():
9393
config_loaded_res falco_configuration::init_from_content(const std::string& config_content, const std::vector<std::string>& cmdline_options, const std::string& filename)
9494
{
9595
config_loaded_res res;
96-
std::string validation_status;
96+
std::vector<std::string> validation_status;
9797

9898
m_config.load_from_string(config_content, m_config_schema, &validation_status);
9999
init_cmdline_options(cmdline_options);
100100

101-
res[filename] = validation_status;
101+
// Only report top most schema validation status
102+
res[filename] = validation_status[0];
102103

103104
load_yaml(filename);
104105
return res;
@@ -107,7 +108,7 @@ config_loaded_res falco_configuration::init_from_content(const std::string& conf
107108
config_loaded_res falco_configuration::init_from_file(const std::string& conf_filename, const std::vector<std::string> &cmdline_options)
108109
{
109110
config_loaded_res res;
110-
std::string validation_status;
111+
std::vector<std::string> validation_status;
111112
try
112113
{
113114
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
119120
}
120121
init_cmdline_options(cmdline_options);
121122

122-
res[conf_filename] = validation_status;
123+
// Only report top most schema validation status
124+
res[conf_filename] = validation_status[0];
123125

124126
merge_config_files(conf_filename, res);
125127
load_yaml(conf_filename);
@@ -138,7 +140,7 @@ std::string falco_configuration::dump()
138140
// filenames and folders specified in config (minus the skipped ones).
139141
void falco_configuration::merge_config_files(const std::string& config_name, config_loaded_res &res)
140142
{
141-
std::string validation_status;
143+
std::vector<std::string> validation_status;
142144
m_loaded_configs_filenames.push_back(config_name);
143145
const auto ppath = std::filesystem::path(config_name);
144146
// Parse files to be included
@@ -161,7 +163,8 @@ void falco_configuration::merge_config_files(const std::string& config_name, con
161163
{
162164
m_loaded_configs_filenames.push_back(include_file);
163165
m_config.include_config_file(include_file_path.string(), m_config_schema, &validation_status);
164-
res[include_file_path.string()] = validation_status;
166+
// Only report top most schema validation status
167+
res[include_file_path.string()] = validation_status[0];
165168
}
166169
else if (std::filesystem::is_directory(include_file_path))
167170
{
@@ -180,7 +183,8 @@ void falco_configuration::merge_config_files(const std::string& config_name, con
180183
for (const auto &f : v)
181184
{
182185
m_config.include_config_file(f, m_config_schema, &validation_status);
183-
res[f] = validation_status;
186+
// Only report top most schema validation status
187+
res[f] = validation_status[0];
184188
}
185189
}
186190
}

0 commit comments

Comments
 (0)