swappable falco engine cleanups. this passes mosts tests

userspace/falco/swappable_falco_engine.cpp

@@ -38,31 +38,26 @@ bool swappable_falco_engine::config::contains_event_source(const std::string &so
 }
 
 bool swappable_falco_engine::open_files(std::list<std::string> &filenames,
-					std::list<swappable_falco_engine::rulesfile> &rulesfiles,
+					std::list<falco_engine::rulesfile> &rulesfiles,
 					std::string &errstr)
 {
 	rulesfiles.clear();
 
-	for(const auto &file : filenames)
+	for(const auto &filename : filenames)
 	{
-		std::ifstream is;
+		std::string errstr;
 
-		is.open(file);
-		if (!is.is_open())
+		rulesfiles.emplace_back();
+
+		falco_engine::rulesfile &rf = rulesfiles.back();
+
+		if (!rf.load(filename, errstr))
 		{
-			errstr = "Could not open rules filename " +
-				file + " " + "for reading";
 			return false;
 		}
-
-		std::string content((istreambuf_iterator<char>(is)),
-				    istreambuf_iterator<char>());
-
-		rulesfile rf{file, content, 0};
-
-		rulesfiles.emplace_back(rf);
 	}
 
+	errstr = "";
 	return true;
 }
 
@@ -81,7 +76,7 @@ bool swappable_falco_engine::init(swappable_falco_engine::config &cfg, sinsp *in
 	m_inspector = inspector;
 
 	// Initialize some engine with no rules
-	std::list<swappable_falco_engine::rulesfile> empty;
+	std::list<falco_engine::rulesfile> empty;
 	return replace(empty, errstr);
 }
 
@@ -107,7 +102,8 @@ filter_check_list &swappable_falco_engine::plugin_filter_checks()
 	return m_plugin_filter_checks;
 }
 
-bool swappable_falco_engine::replace(const std::list<swappable_falco_engine::rulesfile> &rulesfiles, std::string &errstr)
+bool swappable_falco_engine::replace(std::list<falco_engine::rulesfile> &rulesfiles,
+				     std::string &errstr)
 {
 	std::shared_ptr<falco_engine> new_engine;
 
@@ -123,7 +119,8 @@ bool swappable_falco_engine::replace(const std::list<swappable_falco_engine::rul
 	return true;
 }
 
-bool swappable_falco_engine::validate(const std::list<swappable_falco_engine::rulesfile> &rulesfiles, std::string &errstr)
+bool swappable_falco_engine::validate(std::list<falco_engine::rulesfile> &rulesfiles,
+				      std::string &errstr)
 {
 	std::shared_ptr<falco_engine> new_engine;
 
@@ -132,11 +129,13 @@ bool swappable_falco_engine::validate(const std::list<swappable_falco_engine::ru
 	return (new_engine != NULL);
 }
 
-std::shared_ptr<falco_engine> swappable_falco_engine::create_new(const std::list<swappable_falco_engine::rulesfile> &rulesfiles,
+std::shared_ptr<falco_engine> swappable_falco_engine::create_new(std::list<falco_engine::rulesfile> &rulesfiles,
 								 std::string &errstr)
 {
 	std::shared_ptr<falco_engine> ret = make_shared<falco_engine>();
 
+	errstr = "";
+
 	if(!m_inspector)
 	{
 		errstr = "No inspector provided yet";
@@ -186,24 +185,43 @@ std::shared_ptr<falco_engine> swappable_falco_engine::create_new(const std::list
 		ret->add_source(source, filter_factory, formatter_factory);
 	}
 
+	// Note that we load all rules files, even if one of them has an error.
+	bool successful = true;
+
+	// We include filenames if there is more than one file
+	bool include_filenames = (rulesfiles.size() > 1);
+
+	// We include warnings if verbose
+	bool include_warnings = m_config.verbose;
+
+	std::ostringstream os;
 	for(auto &rf : rulesfiles)
 	{
-		// XXX/mstemm all_events is actually unused, remove them.
+		falco_engine::load_result res(rf);
+
+		// XXX/mstemm all_events is actually unused, remove it.
 		bool all_events = false;
 
-		uint64_t required;
+		ret->load_rules(rf,
+				m_config.verbose, all_events,
+				res);
 
-		try {
-			ret->load_rules(rf.content, m_config.verbose, all_events, required);
-		}
-		catch(falco_exception &e)
+		os << res.as_string(include_filenames, include_warnings);
+
+		if(!res.successful)
 		{
-			errstr  = "Could not load rules file " + rf.name + ": " + e.what();
-			ret = NULL;
-			return ret;
+			successful = false;
 		}
 	}
 
+	errstr = os.str();
+
+	if(!successful)
+	{
+		ret = NULL;
+		return ret;
+	}
+
 	// Ensure that all plugins are compatible with the loaded set of rules
 	for(auto &info : m_config.plugin_infos)
 	{

userspace/falco/swappable_falco_engine.h

@@ -53,18 +53,9 @@ public:
 		std::set<std::string> enabled_rule_tags;
 	};
 
-	// Represents a rules file passed to replace() or validate().
-	// The required_engine_version will be filled in upon a
-	// successful call to replace() or validate().
-	struct rulesfile {
-		std::string name;
-		std::string content;
-		uint64_t required_engine_version;
-	};
-
 	// Helper to load a set of files from filenames
 	static bool open_files(std::list<std::string> &filenames,
-			       std::list<rulesfile> &rulesfiles,
+			       std::list<falco_engine::rulesfile> &rulesfiles,
 			       std::string &errstr);
 
 	swappable_falco_engine();
@@ -83,9 +74,11 @@ public:
 	// This can be called from a different thread than the one
 	// calling engine().
 	//
-        // Returns true on success, returns false and fills in
-        // errstr otherwise.
-	bool replace(const std::list<rulesfile> &rulesfiles, std::string &errstr);
+        // Returns true on success, false otherwise. Regardless,
+        // errstr will be filled in with details on any
+        // errors (and warnings, if config.verbose is true)
+	bool replace(std::list<falco_engine::rulesfile> &rulesfiles,
+		     std::string &errstr);
 
 	// Create a new engine, configure it, load the provided set of
 	// rules files, but do *not* queue it to replace the current
@@ -94,16 +87,20 @@ public:
 	// This can be called from a different thread than the one
 	// calling engine().
 	//
-        // Returns true if all rules were valid. Returns false and fills in
-        // errstr otherwise.
-	bool validate(const std::list<rulesfile> &rulesfiles, std::string &errstr);
+        // Returns true on success, false otherwise. Regardless,
+        // errstr will be filled in with details on any
+        // errors (and warnings, if config.verbose is true)
+	bool validate(std::list<falco_engine::rulesfile> &rulesfiles,
+		      std::string &errstr);
 
 private:
 
 	// Does everything but enqueue the new engine. Returns a
-	// shared_ptr to a new falco_engine on success. On error the
-	// shared_ptr will be empty and errstr will contain an error.
-	std::shared_ptr<falco_engine> create_new(const std::list<rulesfile> &rulesfiles, std::string &errstr);
+	// shared_ptr to a new falco_engine on success. Regardless,
+        // errstr will be filled in with details on any
+        // errors (and warnings, if config.verbose is true)
+	std::shared_ptr<falco_engine> create_new(std::list<falco_engine::rulesfile> &rulesfiles,
+						 std::string &errstr);
 
 	sinsp *m_inspector;
 	config m_config;