Merge pull request #9895 from Icinga/targeted-api-filter

FilterUtility::GetFilterTargets(): don't run filter for specific object(s) for all objects

lib/base/dictionary.cpp

@@ -67,6 +67,20 @@ bool Dictionary::Get(const String& key, Value *result) const
 	return true;
 }
 
+/**
+ * Retrieves a value's address from a dictionary.
+ *
+ * @param key The key whose value's address should be retrieved.
+ * @returns nullptr if the key was not found.
+ */
+const Value * Dictionary::GetRef(const String& key) const
+{
+	std::shared_lock<std::shared_timed_mutex> lock (m_DataMutex);
+	auto it (m_Data.find(key));
+
+	return it == m_Data.end() ? nullptr : &it->second;
+}
+
 /**
  * Sets a value in the dictionary.
  *

lib/base/dictionary.hpp

@@ -42,6 +42,7 @@ class Dictionary final : public Object
 
 	Value Get(const String& key) const;
 	bool Get(const String& key, Value *result) const;
+	const Value * GetRef(const String& key) const;
 	void Set(const String& key, Value value, bool overrideFrozen = false);
 	bool Contains(const String& key) const;
 

lib/config/applyrule-targeted.cpp

@@ -96,16 +96,16 @@ bool ApplyRule::AddTargetedRule(const ApplyRule::Ptr& rule, const String& target
  *
  * @returns Whether the given assign filter is like above.
  */
-bool ApplyRule::GetTargetHosts(Expression* assignFilter, std::vector<const String *>& hosts)
+bool ApplyRule::GetTargetHosts(Expression* assignFilter, std::vector<const String *>& hosts, const Dictionary::Ptr& constants)
 {
 	auto lor (dynamic_cast<LogicalOrExpression*>(assignFilter));
 
 	if (lor) {
-		return GetTargetHosts(lor->GetOperand1().get(), hosts)
-			&& GetTargetHosts(lor->GetOperand2().get(), hosts);
+		return GetTargetHosts(lor->GetOperand1().get(), hosts, constants)
+			&& GetTargetHosts(lor->GetOperand2().get(), hosts, constants);
 	}
 
-	auto name (GetComparedName(assignFilter, "host"));
+	auto name (GetComparedName(assignFilter, "host", constants));
 
 	if (name) {
 		hosts.emplace_back(name);
@@ -124,16 +124,16 @@ bool ApplyRule::GetTargetHosts(Expression* assignFilter, std::vector<const Strin
  *
  * @returns Whether the given assign filter is like above.
  */
-bool ApplyRule::GetTargetServices(Expression* assignFilter, std::vector<std::pair<const String *, const String *>>& services)
+bool ApplyRule::GetTargetServices(Expression* assignFilter, std::vector<std::pair<const String *, const String *>>& services, const Dictionary::Ptr& constants)
 {
 	auto lor (dynamic_cast<LogicalOrExpression*>(assignFilter));
 
 	if (lor) {
-		return GetTargetServices(lor->GetOperand1().get(), services)
-			&& GetTargetServices(lor->GetOperand2().get(), services);
+		return GetTargetServices(lor->GetOperand1().get(), services, constants)
+			&& GetTargetServices(lor->GetOperand2().get(), services, constants);
 	}
 
-	auto service (GetTargetService(assignFilter));
+	auto service (GetTargetService(assignFilter, constants));
 
 	if (service.first) {
 		services.emplace_back(service);
@@ -152,7 +152,7 @@ bool ApplyRule::GetTargetServices(Expression* assignFilter, std::vector<std::pai
  *
  * @returns {host, service} on success and {nullptr, nullptr} on failure.
  */
-std::pair<const String *, const String *> ApplyRule::GetTargetService(Expression* assignFilter)
+std::pair<const String *, const String *> ApplyRule::GetTargetService(Expression* assignFilter, const Dictionary::Ptr& constants)
 {
 	auto land (dynamic_cast<LogicalAndExpression*>(assignFilter));
 
@@ -162,15 +162,15 @@ std::pair<const String *, const String *> ApplyRule::GetTargetService(Expression
 
 	auto op1 (land->GetOperand1().get());
 	auto op2 (land->GetOperand2().get());
-	auto host (GetComparedName(op1, "host"));
+	auto host (GetComparedName(op1, "host", constants));
 
 	if (!host) {
 		std::swap(op1, op2);
-		host = GetComparedName(op1, "host");
+		host = GetComparedName(op1, "host", constants);
 	}
 
 	if (host) {
-		auto service (GetComparedName(op2, "service"));
+		auto service (GetComparedName(op2, "service", constants));
 
 		if (service) {
 			return {host, service};
@@ -189,7 +189,7 @@ std::pair<const String *, const String *> ApplyRule::GetTargetService(Expression
  *
  * @returns The object name on success and nullptr on failure.
  */
-const String * ApplyRule::GetComparedName(Expression* assignFilter, const char * lcType)
+const String * ApplyRule::GetComparedName(Expression* assignFilter, const char * lcType, const Dictionary::Ptr& constants)
 {
 	auto eq (dynamic_cast<EqualExpression*>(assignFilter));
 
@@ -200,12 +200,12 @@ const String * ApplyRule::GetComparedName(Expression* assignFilter, const char *
 	auto op1 (eq->GetOperand1().get());
 	auto op2 (eq->GetOperand2().get());
 
-	if (IsNameIndexer(op1, lcType)) {
-		return GetLiteralStringValue(op2);
+	if (IsNameIndexer(op1, lcType, constants)) {
+		return GetConstString(op2, constants);
 	}
 
-	if (IsNameIndexer(op2, lcType)) {
-		return GetLiteralStringValue(op1);
+	if (IsNameIndexer(op2, lcType, constants)) {
+		return GetConstString(op1, constants);
 	}
 
 	return nullptr;
@@ -214,7 +214,7 @@ const String * ApplyRule::GetComparedName(Expression* assignFilter, const char *
 /**
  * @returns Whether the given expression is like $lcType$.name.
  */
-bool ApplyRule::IsNameIndexer(Expression* exp, const char * lcType)
+bool ApplyRule::IsNameIndexer(Expression* exp, const char * lcType, const Dictionary::Ptr& constants)
 {
 	auto ixr (dynamic_cast<IndexerExpression*>(exp));
 
@@ -228,27 +228,39 @@ bool ApplyRule::IsNameIndexer(Expression* exp, const char * lcType)
 		return false;
 	}
 
-	auto val (GetLiteralStringValue(ixr->GetOperand2().get()));
+	auto val (GetConstString(ixr->GetOperand2().get(), constants));
 
 	return val && *val == "name";
 }
 
 /**
- * @returns If the given expression is a string literal, the string. nullptr on failure.
+ * @returns If the given expression is a constant string, its address. nullptr on failure.
  */
-const String * ApplyRule::GetLiteralStringValue(Expression* exp)
+const String * ApplyRule::GetConstString(Expression* exp, const Dictionary::Ptr& constants)
+{
+	auto cnst (GetConst(exp, constants));
+
+	return cnst && cnst->IsString() ? &cnst->Get<String>() : nullptr;
+}
+
+/**
+ * @returns If the given expression is a constant, its address. nullptr on failure.
+ */
+const Value * ApplyRule::GetConst(Expression* exp, const Dictionary::Ptr& constants)
 {
 	auto lit (dynamic_cast<LiteralExpression*>(exp));
 
-	if (!lit) {
-		return nullptr;
+	if (lit) {
+		return &lit->GetValue();
 	}
 
-	auto& val (lit->GetValue());
+	if (constants) {
+		auto var (dynamic_cast<VariableExpression*>(exp));
 
-	if (!val.IsString()) {
-		return nullptr;
+		if (var) {
+			return constants->GetRef(var->GetVariable());
+		}
 	}
 
-	return &val.Get<String>();
+	return nullptr;
 }

lib/config/applyrule.hpp

@@ -82,6 +82,8 @@ class ApplyRule : public SharedObject
 	static const std::vector<ApplyRule::Ptr>& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType);
 	static const std::set<ApplyRule::Ptr>& GetTargetedHostRules(const Type::Ptr& sourceType, const String& host);
 	static const std::set<ApplyRule::Ptr>& GetTargetedServiceRules(const Type::Ptr& sourceType, const String& host, const String& service);
+	static bool GetTargetHosts(Expression* assignFilter, std::vector<const String *>& hosts, const Dictionary::Ptr& constants = nullptr);
+	static bool GetTargetServices(Expression* assignFilter, std::vector<std::pair<const String *, const String *>>& services, const Dictionary::Ptr& constants = nullptr);
 
 	static void RegisterType(const String& sourceType, const std::vector<String>& targetTypes);
 	static bool IsValidSourceType(const String& sourceType);
@@ -108,12 +110,11 @@ class ApplyRule : public SharedObject
 	static RuleMap m_Rules;
 
 	static bool AddTargetedRule(const ApplyRule::Ptr& rule, const String& targetType, PerSourceType& rules);
-	static bool GetTargetHosts(Expression* assignFilter, std::vector<const String *>& hosts);
-	static bool GetTargetServices(Expression* assignFilter, std::vector<std::pair<const String *, const String *>>& services);
-	static std::pair<const String *, const String *> GetTargetService(Expression* assignFilter);
-	static const String * GetComparedName(Expression* assignFilter, const char * lcType);
-	static bool IsNameIndexer(Expression* exp, const char * lcType);
-	static const String * GetLiteralStringValue(Expression* exp);
+	static std::pair<const String *, const String *> GetTargetService(Expression* assignFilter, const Dictionary::Ptr& constants);
+	static const String * GetComparedName(Expression* assignFilter, const char * lcType, const Dictionary::Ptr& constants);
+	static bool IsNameIndexer(Expression* exp, const char * lcType, const Dictionary::Ptr& constants);
+	static const String * GetConstString(Expression* exp, const Dictionary::Ptr& constants);
+	static const Value * GetConst(Expression* exp, const Dictionary::Ptr& constants);
 
 	ApplyRule(String name, Expression::Ptr expression,
 		Expression::Ptr filter, String package, String fkvar, String fvvar, Expression::Ptr fterm,

lib/config/expression.hpp

@@ -622,6 +622,11 @@ class DictExpression final : public DebuggableExpression
 
 	void MakeInline();
 
+	inline const std::vector<std::unique_ptr<Expression>>& GetExpressions() const noexcept
+	{
+		return m_Expressions;
+	}
+
 protected:
 	ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
 

lib/remote/filterutility.cpp

@@ -2,6 +2,7 @@
 
 #include "remote/filterutility.hpp"
 #include "remote/httputility.hpp"
+#include "config/applyrule.hpp"
 #include "config/configcompiler.hpp"
 #include "config/expression.hpp"
 #include "base/namespace.hpp"
@@ -271,18 +272,73 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
 		if (query->Contains("filter")) {
 			String filter = HttpUtility::GetLastParameter(query, "filter");
 			std::unique_ptr<Expression> ufilter = ConfigCompiler::CompileText("<API query>", filter);
-
 			Dictionary::Ptr filter_vars = query->Get("filter_vars");
-			if (filter_vars) {
-				ObjectLock olock(filter_vars);
-				for (const Dictionary::Pair& kv : filter_vars) {
-					frameNS->Set(kv.first, kv.second);
+			bool targeted = false;
+			std::vector<ConfigObject::Ptr> targets;
+
+			if (dynamic_cast<ConfigObjectTargetProvider*>(provider.get())) {
+				auto dict (dynamic_cast<DictExpression*>(ufilter.get()));
+
+				if (dict) {
+					auto& subex (dict->GetExpressions());
+
+					if (subex.size() == 1u) {
+						if (type == "Host") {
+							std::vector<const String *> targetNames;
+
+							if (ApplyRule::GetTargetHosts(subex.at(0).get(), targetNames, filter_vars)) {
+								static const auto typeHost (Type::GetByName("Host"));
+								static const auto ctypeHost (dynamic_cast<ConfigType*>(typeHost.get()));
+								targeted = true;
+
+								for (auto name : targetNames) {
+									auto target (ctypeHost->GetObject(*name));
+
+									if (target) {
+										targets.emplace_back(target);
+									}
+								}
+							}
+						} else if (type == "Service") {
+							std::vector<std::pair<const String *, const String *>> targetNames;
+
+							if (ApplyRule::GetTargetServices(subex.at(0).get(), targetNames, filter_vars)) {
+								static const auto typeService (Type::GetByName("Service"));
+								static const auto ctypeService (dynamic_cast<ConfigType*>(typeService.get()));
+								targeted = true;
+
+								for (auto name : targetNames) {
+									auto target (ctypeService->GetObject(*name.first + "!" + *name.second));
+
+									if (target) {
+										targets.emplace_back(target);
+									}
+								}
+							}
+						}
+					}
 				}
 			}
 
-			provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) {
-				FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target);
-			});
+			if (targeted) {
+				for (auto& target : targets) {
+					if (FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), target, variableName)) {
+						result.emplace_back(std::move(target));
+					}
+				}
+			} else {
+				if (filter_vars) {
+					ObjectLock olock (filter_vars);
+
+					for (auto& kv : filter_vars) {
+						frameNS->Set(kv.first, kv.second);
+					}
+				}
+
+				provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) {
+					FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target);
+				});
+			}
 		} else {
 			/* Ensure to pass a nullptr as filter expression.
 			 * GCC 8.1.1 on F28 causes problems, see GH #6533.

test/CMakeLists.txt

@@ -24,6 +24,7 @@ set(base_test_SOURCES
   base-type.cpp
   base-utility.cpp
   base-value.cpp
+  config-apply.cpp
   config-ops.cpp
   icinga-checkresult.cpp
   icinga-dependencies.cpp
@@ -130,6 +131,38 @@ add_boost_test(base
     base_value/scalar
     base_value/convert
     base_value/format
+    config_apply/gettargethosts_literal
+    config_apply/gettargethosts_const
+    config_apply/gettargethosts_swapped
+    config_apply/gettargethosts_two
+    config_apply/gettargethosts_three
+    config_apply/gettargethosts_mixed
+    config_apply/gettargethosts_redundant
+    config_apply/gettargethosts_badconst
+    config_apply/gettargethosts_notliteral
+    config_apply/gettargethosts_wrongop
+    config_apply/gettargethosts_wrongattr
+    config_apply/gettargethosts_wrongvar
+    config_apply/gettargethosts_noindexer
+    config_apply/gettargetservices_literal
+    config_apply/gettargetservices_const
+    config_apply/gettargetservices_swapped_outer
+    config_apply/gettargetservices_swapped_inner
+    config_apply/gettargetservices_two
+    config_apply/gettargetservices_three
+    config_apply/gettargetservices_mixed
+    config_apply/gettargetservices_redundant
+    config_apply/gettargetservices_badconst
+    config_apply/gettargetservices_notliteral
+    config_apply/gettargetservices_wrongop_outer
+    config_apply/gettargetservices_wrongop_host
+    config_apply/gettargetservices_wrongop_service
+    config_apply/gettargetservices_wrongattr_host
+    config_apply/gettargetservices_wrongattr_service
+    config_apply/gettargetservices_wrongvar_host
+    config_apply/gettargetservices_wrongvar_service
+    config_apply/gettargetservices_noindexer_host
+    config_apply/gettargetservices_noindexer_service
     config_ops/simple
     config_ops/advanced
     icinga_checkresult/host_1attempt