update(userspace/engine): address jasondellaluce comments

- avoiding inspector to be allocated for each rule
- use two boolean values for expecting macros and lists
- move items of lists alongside name, under info
- use snake case for json output, like we do for e.g alerts
- correctly retrieve evt names
- consider two levels of lists for exception operators

Signed-off-by: Lorenzo Susini <susinilorenzo1@gmail.com>

userspace/engine/falco_engine.cpp

@@ -460,6 +460,7 @@ void falco_engine::describe_rule(std::string *rule, bool json) const
 		return;
 	}
 
+	std::unique_ptr<sinsp> insp(new sinsp());
 	Json::FastWriter writer;
 	std::string json_str;
 
@@ -475,7 +476,7 @@ void falco_engine::describe_rule(std::string *rule, bool json) const
 		{
 			auto ri = m_rule_collector.rules().at(r.name);
 			Json::Value rule;
-			get_json_details(r, *ri, rule);
+			get_json_details(r, *ri, insp.get(), rule);
 
 			// Append to rule array
 			rules_array.append(rule);
@@ -514,7 +515,7 @@ void falco_engine::describe_rule(std::string *rule, bool json) const
 		}
 		auto r = m_rules.at(ri->name);
 		Json::Value rule; 
-		get_json_details(*r, *ri, rule);
+		get_json_details(*r, *ri, insp.get(), rule);
 		json_str = writer.write(rule);
 	}
 
@@ -523,6 +524,7 @@ void falco_engine::describe_rule(std::string *rule, bool json) const
 
 void falco_engine::get_json_details(const falco_rule &r,
 	const rule_loader::rule_info &ri,
+	sinsp *insp,
 	Json::Value &rule) const
 {
 	Json::Value rule_info;
@@ -551,17 +553,15 @@ void falco_engine::get_json_details(const falco_rule &r,
 	rule["details"] = json_details;
 
 	// Get fields from output string
-	auto insp = new sinsp;
 	sinsp_evt_formatter fmt(insp, r.output);
 	std::vector<std::string> out_fields;
 	fmt.get_field_names(out_fields);
-	delete insp;
 	Json::Value outputFields = Json::arrayValue;
 	for(const auto &of : out_fields)
 	{
 		outputFields.append(of);
 	}
-	rule["details"]["outputFields"] = outputFields;
+	rule["details"]["output_fields"] = outputFields;
 
 	// Get fields from exceptions
 	Json::Value exception_fields = Json::arrayValue;
@@ -569,7 +569,7 @@ void falco_engine::get_json_details(const falco_rule &r,
 	{
 		exception_fields.append(f);
 	}
-	rule["details"]["exceptionFields"] = exception_fields;
+	rule["details"]["exception_fields"] = exception_fields;
 
 	// Get operators from exceptions
 	Json::Value exception_operators = Json::arrayValue;
@@ -579,7 +579,18 @@ void falco_engine::get_json_details(const falco_rule &r,
 		{
 			for(const auto& c : e.comps.items)
 			{
-				exception_operators.append(c.item);
+				if(c.is_list)
+				{
+					// considering max two levels of lists
+					for(const auto& i : c.items)
+					{
+						exception_operators.append(i.item);
+					}
+				}
+				else
+				{
+					exception_operators.append(c.item);
+				}
 			}
 		}
 		else
@@ -587,7 +598,7 @@ void falco_engine::get_json_details(const falco_rule &r,
 			exception_operators.append(e.comps.item);
 		}	
 	}
-	rule["details"]["exceptionOperators"] = exception_operators;
+	rule["details"]["exception_operators"] = exception_operators;
 
 	if(ri.source == falco_common::syscall_source)
 	{
@@ -625,7 +636,6 @@ void falco_engine::get_json_details(const rule_loader::list_info& l,
 {
 	Json::Value list_info;
 	list_info["name"] = l.name;
-	list["info"] = list_info;
 
 	Json::Value items = Json::arrayValue;
 	Json::Value lists = Json::arrayValue;
@@ -639,7 +649,8 @@ void falco_engine::get_json_details(const rule_loader::list_info& l,
 		items.append(i);
 	}
 
-	list["details"]["items"] = items;
+	list_info["items"] = items;
+	list["info"] = list_info;
 	list["details"]["lists"] = lists;
 }
 
@@ -680,7 +691,7 @@ void falco_engine::get_json_details(libsinsp::filter::ast::expr* ast,
 	{
 		condition_fields.append(f);
 	}
-	output["conditionFields"] = condition_fields;
+	output["condition_fields"] = condition_fields;
 
 	Json::Value lists = Json::arrayValue;
 	for(const auto &l : details.lists)
@@ -696,18 +707,15 @@ void falco_engine::get_json_evt_types(libsinsp::filter::ast::expr* ast,
 					Json::Value& output) const
 {
 	output = Json::arrayValue;
-	auto evttypes = libsinsp::filter::ast::ppm_event_codes(ast);
+	auto evtcodes = libsinsp::filter::ast::ppm_event_codes(ast);
-	if(evttypes.size() != libsinsp::events::all_event_set().size())
+	if(evtcodes.size() != libsinsp::events::all_event_set().size())
 	{
-		std::unordered_set<std::string> evts;
+		auto syscodes = libsinsp::filter::ast::ppm_sc_codes(ast);
-		for(const auto &e : evttypes)
+		auto syscodes_to_evt_names = libsinsp::events::sc_set_to_event_names(syscodes);
+		auto evtcodes_to_evt_names = libsinsp::events::event_set_to_names(evtcodes, false);
+		for (const auto& n : unordered_set_union(syscodes_to_evt_names, evtcodes_to_evt_names))
 		{
-			auto evt_info = libsinsp::events::info(e);
+			output.append(n);
-			auto res = evts.insert(std::string(evt_info->name));
-			if(res.second)
-			{
-				output.append(evt_info->name);
-			}
 		}
 	}
 }

userspace/engine/falco_engine.h

@@ -302,6 +302,7 @@ private:
 	// Retrieve json details from rules, macros, lists
 	void get_json_details(const falco_rule& r,
 					const rule_loader::rule_info& ri,
+					sinsp* insp,
 					Json::Value& rule) const;
 	void get_json_details(const rule_loader::macro_info& m,
 					Json::Value& macro) const;

userspace/engine/filter_details_resolver.cpp

@@ -36,7 +36,9 @@ void filter_details_resolver::visitor::visit(ast::and_expr* e)
 {
 	for(size_t i = 0; i < e->children.size(); i++)
 	{
+		m_expect_macro = true;
 		e->children[i]->accept(this);
+		m_expect_macro = false;
 	}
 }
 
@@ -44,7 +46,9 @@ void filter_details_resolver::visitor::visit(ast::or_expr* e)
 {
 	for(size_t i = 0; i < e->children.size(); i++)
 	{
+		m_expect_macro = true;
 		e->children[i]->accept(this);
+		m_expect_macro = false;
 	}
 }
 
@@ -55,42 +59,45 @@ void filter_details_resolver::visitor::visit(ast::not_expr* e)
 
 void filter_details_resolver::visitor::visit(ast::list_expr* e)
 {
-
+	if(m_expect_list)
-}
-
-void filter_details_resolver::visitor::visit(ast::binary_check_expr* e)
-{
-	m_details.fields.insert(e->field);
-	m_details.operators.insert(e->op);
-	
-	auto list = dynamic_cast<ast::list_expr*>(e->value.get());
-	if(list == nullptr)
 	{
-		return;
+		for(const auto& item : e->values)
-	}
-
-	for(const auto& item : list->values)
-	{
-		if(m_details.known_lists.find(item) != m_details.known_lists.end())
 		{
-			m_details.lists.insert(item);
+			if(m_details.known_lists.find(item) != m_details.known_lists.end())
+			{
+				m_details.lists.insert(item);
+			}
 		}
 	}
 }
 
+void filter_details_resolver::visitor::visit(ast::binary_check_expr* e)
+{
+	m_expect_macro = false;
+	m_details.fields.insert(e->field);
+	m_details.operators.insert(e->op);
+	m_expect_list = true;
+	e->value->accept(this);
+	m_expect_list = false;
+}
+
 void filter_details_resolver::visitor::visit(ast::unary_check_expr* e)
 {
+	m_expect_macro = false;
 	m_details.fields.insert(e->field);
 	m_details.operators.insert(e->op);
 }
 
 void filter_details_resolver::visitor::visit(ast::value_expr* e)
 {
-	auto it = m_details.known_macros.find(e->value);
+	if(m_expect_macro)
-	if(it == m_details.known_macros.end())
 	{
-		return;
+		auto it = m_details.known_macros.find(e->value);
-	}
+		if(it == m_details.known_macros.end())
+		{
+			return;
+		}
 
-	m_details.macros.insert(e->value);
+		m_details.macros.insert(e->value);
+	}
 }

userspace/engine/filter_details_resolver.h

@@ -55,7 +55,10 @@ public:
 private:
 	struct visitor : public libsinsp::filter::ast::expr_visitor
 	{
-		visitor(filter_details& details) : m_details(details) {}
+		visitor(filter_details& details) : 
+			m_details(details),
+			m_expect_list(false),
+			m_expect_macro(false) {}
 		visitor(visitor&&) = default;
 		visitor& operator = (visitor&&) = default;
 		visitor(const visitor&) = delete;
@@ -70,5 +73,7 @@ private:
 		void visit(libsinsp::filter::ast::binary_check_expr* e) override;
 
 		filter_details& m_details;
+		bool m_expect_list;
+		bool m_expect_macro;
 	};
 };