Skip to content

Commit 949d983

Browse files
Al2KlimovAl2Klimov
authored
Merge pull request #9895 from Icinga/targeted-api-filter
FilterUtility::GetFilterTargets(): don't run filter for specific object(s) for all objects
2 parents fa07cd4 + 4424d57 commit 949d983

8 files changed

+414
-41
lines changed

‎lib/base/dictionary.cpp

+14
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,20 @@ bool Dictionary::Get(const String& key, Value *result) const
6767
return true;
6868
}
6969

70+
/**
71+
* Retrieves a value's address from a dictionary.
72+
*
73+
* @param key The key whose value's address should be retrieved.
74+
* @returns nullptr if the key was not found.
75+
*/
76+
const Value * Dictionary::GetRef(const String& key) const
77+
{
78+
std::shared_lock<std::shared_timed_mutex> lock (m_DataMutex);
79+
auto it (m_Data.find(key));
80+
81+
return it == m_Data.end() ? nullptr : &it->second;
82+
}
83+
7084
/**
7185
* Sets a value in the dictionary.
7286
*

‎lib/base/dictionary.hpp

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class Dictionary final : public Object
4242

4343
Value Get(const String& key) const;
4444
bool Get(const String& key, Value *result) const;
45+
const Value * GetRef(const String& key) const;
4546
void Set(const String& key, Value value, bool overrideFrozen = false);
4647
bool Contains(const String& key) const;
4748

‎lib/config/applyrule-targeted.cpp

+39-27
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,16 @@ bool ApplyRule::AddTargetedRule(const ApplyRule::Ptr& rule, const String& target
9696
*
9797
* @returns Whether the given assign filter is like above.
9898
*/
99-
bool ApplyRule::GetTargetHosts(Expression* assignFilter, std::vector<const String *>& hosts)
99+
bool ApplyRule::GetTargetHosts(Expression* assignFilter, std::vector<const String *>& hosts, const Dictionary::Ptr& constants)
100100
{
101101
auto lor (dynamic_cast<LogicalOrExpression*>(assignFilter));
102102

103103
if (lor) {
104-
return GetTargetHosts(lor->GetOperand1().get(), hosts)
105-
&& GetTargetHosts(lor->GetOperand2().get(), hosts);
104+
return GetTargetHosts(lor->GetOperand1().get(), hosts, constants)
105+
&& GetTargetHosts(lor->GetOperand2().get(), hosts, constants);
106106
}
107107

108-
auto name (GetComparedName(assignFilter, "host"));
108+
auto name (GetComparedName(assignFilter, "host", constants));
109109

110110
if (name) {
111111
hosts.emplace_back(name);
@@ -124,16 +124,16 @@ bool ApplyRule::GetTargetHosts(Expression* assignFilter, std::vector<const Strin
124124
*
125125
* @returns Whether the given assign filter is like above.
126126
*/
127-
bool ApplyRule::GetTargetServices(Expression* assignFilter, std::vector<std::pair<const String *, const String *>>& services)
127+
bool ApplyRule::GetTargetServices(Expression* assignFilter, std::vector<std::pair<const String *, const String *>>& services, const Dictionary::Ptr& constants)
128128
{
129129
auto lor (dynamic_cast<LogicalOrExpression*>(assignFilter));
130130

131131
if (lor) {
132-
return GetTargetServices(lor->GetOperand1().get(), services)
133-
&& GetTargetServices(lor->GetOperand2().get(), services);
132+
return GetTargetServices(lor->GetOperand1().get(), services, constants)
133+
&& GetTargetServices(lor->GetOperand2().get(), services, constants);
134134
}
135135

136-
auto service (GetTargetService(assignFilter));
136+
auto service (GetTargetService(assignFilter, constants));
137137

138138
if (service.first) {
139139
services.emplace_back(service);
@@ -152,7 +152,7 @@ bool ApplyRule::GetTargetServices(Expression* assignFilter, std::vector<std::pai
152152
*
153153
* @returns {host, service} on success and {nullptr, nullptr} on failure.
154154
*/
155-
std::pair<const String *, const String *> ApplyRule::GetTargetService(Expression* assignFilter)
155+
std::pair<const String *, const String *> ApplyRule::GetTargetService(Expression* assignFilter, const Dictionary::Ptr& constants)
156156
{
157157
auto land (dynamic_cast<LogicalAndExpression*>(assignFilter));
158158

@@ -162,15 +162,15 @@ std::pair<const String *, const String *> ApplyRule::GetTargetService(Expression
162162

163163
auto op1 (land->GetOperand1().get());
164164
auto op2 (land->GetOperand2().get());
165-
auto host (GetComparedName(op1, "host"));
165+
auto host (GetComparedName(op1, "host", constants));
166166

167167
if (!host) {
168168
std::swap(op1, op2);
169-
host = GetComparedName(op1, "host");
169+
host = GetComparedName(op1, "host", constants);
170170
}
171171

172172
if (host) {
173-
auto service (GetComparedName(op2, "service"));
173+
auto service (GetComparedName(op2, "service", constants));
174174

175175
if (service) {
176176
return {host, service};
@@ -189,7 +189,7 @@ std::pair<const String *, const String *> ApplyRule::GetTargetService(Expression
189189
*
190190
* @returns The object name on success and nullptr on failure.
191191
*/
192-
const String * ApplyRule::GetComparedName(Expression* assignFilter, const char * lcType)
192+
const String * ApplyRule::GetComparedName(Expression* assignFilter, const char * lcType, const Dictionary::Ptr& constants)
193193
{
194194
auto eq (dynamic_cast<EqualExpression*>(assignFilter));
195195

@@ -200,12 +200,12 @@ const String * ApplyRule::GetComparedName(Expression* assignFilter, const char *
200200
auto op1 (eq->GetOperand1().get());
201201
auto op2 (eq->GetOperand2().get());
202202

203-
if (IsNameIndexer(op1, lcType)) {
204-
return GetLiteralStringValue(op2);
203+
if (IsNameIndexer(op1, lcType, constants)) {
204+
return GetConstString(op2, constants);
205205
}
206206

207-
if (IsNameIndexer(op2, lcType)) {
208-
return GetLiteralStringValue(op1);
207+
if (IsNameIndexer(op2, lcType, constants)) {
208+
return GetConstString(op1, constants);
209209
}
210210

211211
return nullptr;
@@ -214,7 +214,7 @@ const String * ApplyRule::GetComparedName(Expression* assignFilter, const char *
214214
/**
215215
* @returns Whether the given expression is like $lcType$.name.
216216
*/
217-
bool ApplyRule::IsNameIndexer(Expression* exp, const char * lcType)
217+
bool ApplyRule::IsNameIndexer(Expression* exp, const char * lcType, const Dictionary::Ptr& constants)
218218
{
219219
auto ixr (dynamic_cast<IndexerExpression*>(exp));
220220

@@ -228,27 +228,39 @@ bool ApplyRule::IsNameIndexer(Expression* exp, const char * lcType)
228228
return false;
229229
}
230230

231-
auto val (GetLiteralStringValue(ixr->GetOperand2().get()));
231+
auto val (GetConstString(ixr->GetOperand2().get(), constants));
232232

233233
return val && *val == "name";
234234
}
235235

236236
/**
237-
* @returns If the given expression is a string literal, the string. nullptr on failure.
237+
* @returns If the given expression is a constant string, its address. nullptr on failure.
238238
*/
239-
const String * ApplyRule::GetLiteralStringValue(Expression* exp)
239+
const String * ApplyRule::GetConstString(Expression* exp, const Dictionary::Ptr& constants)
240+
{
241+
auto cnst (GetConst(exp, constants));
242+
243+
return cnst && cnst->IsString() ? &cnst->Get<String>() : nullptr;
244+
}
245+
246+
/**
247+
* @returns If the given expression is a constant, its address. nullptr on failure.
248+
*/
249+
const Value * ApplyRule::GetConst(Expression* exp, const Dictionary::Ptr& constants)
240250
{
241251
auto lit (dynamic_cast<LiteralExpression*>(exp));
242252

243-
if (!lit) {
244-
return nullptr;
253+
if (lit) {
254+
return &lit->GetValue();
245255
}
246256

247-
auto& val (lit->GetValue());
257+
if (constants) {
258+
auto var (dynamic_cast<VariableExpression*>(exp));
248259

249-
if (!val.IsString()) {
250-
return nullptr;
260+
if (var) {
261+
return constants->GetRef(var->GetVariable());
262+
}
251263
}
252264

253-
return &val.Get<String>();
265+
return nullptr;
254266
}

‎lib/config/applyrule.hpp

+7-6
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ class ApplyRule : public SharedObject
8282
static const std::vector<ApplyRule::Ptr>& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType);
8383
static const std::set<ApplyRule::Ptr>& GetTargetedHostRules(const Type::Ptr& sourceType, const String& host);
8484
static const std::set<ApplyRule::Ptr>& GetTargetedServiceRules(const Type::Ptr& sourceType, const String& host, const String& service);
85+
static bool GetTargetHosts(Expression* assignFilter, std::vector<const String *>& hosts, const Dictionary::Ptr& constants = nullptr);
86+
static bool GetTargetServices(Expression* assignFilter, std::vector<std::pair<const String *, const String *>>& services, const Dictionary::Ptr& constants = nullptr);
8587

8688
static void RegisterType(const String& sourceType, const std::vector<String>& targetTypes);
8789
static bool IsValidSourceType(const String& sourceType);
@@ -108,12 +110,11 @@ class ApplyRule : public SharedObject
108110
static RuleMap m_Rules;
109111

110112
static bool AddTargetedRule(const ApplyRule::Ptr& rule, const String& targetType, PerSourceType& rules);
111-
static bool GetTargetHosts(Expression* assignFilter, std::vector<const String *>& hosts);
112-
static bool GetTargetServices(Expression* assignFilter, std::vector<std::pair<const String *, const String *>>& services);
113-
static std::pair<const String *, const String *> GetTargetService(Expression* assignFilter);
114-
static const String * GetComparedName(Expression* assignFilter, const char * lcType);
115-
static bool IsNameIndexer(Expression* exp, const char * lcType);
116-
static const String * GetLiteralStringValue(Expression* exp);
113+
static std::pair<const String *, const String *> GetTargetService(Expression* assignFilter, const Dictionary::Ptr& constants);
114+
static const String * GetComparedName(Expression* assignFilter, const char * lcType, const Dictionary::Ptr& constants);
115+
static bool IsNameIndexer(Expression* exp, const char * lcType, const Dictionary::Ptr& constants);
116+
static const String * GetConstString(Expression* exp, const Dictionary::Ptr& constants);
117+
static const Value * GetConst(Expression* exp, const Dictionary::Ptr& constants);
117118

118119
ApplyRule(String name, Expression::Ptr expression,
119120
Expression::Ptr filter, String package, String fkvar, String fvvar, Expression::Ptr fterm,

‎lib/config/expression.hpp

+5
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,11 @@ class DictExpression final : public DebuggableExpression
622622

623623
void MakeInline();
624624

625+
inline const std::vector<std::unique_ptr<Expression>>& GetExpressions() const noexcept
626+
{
627+
return m_Expressions;
628+
}
629+
625630
protected:
626631
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
627632

‎lib/remote/filterutility.cpp

+64-8
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include "remote/filterutility.hpp"
44
#include "remote/httputility.hpp"
5+
#include "config/applyrule.hpp"
56
#include "config/configcompiler.hpp"
67
#include "config/expression.hpp"
78
#include "base/namespace.hpp"
@@ -271,18 +272,73 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
271272
if (query->Contains("filter")) {
272273
String filter = HttpUtility::GetLastParameter(query, "filter");
273274
std::unique_ptr<Expression> ufilter = ConfigCompiler::CompileText("<API query>", filter);
274-
275275
Dictionary::Ptr filter_vars = query->Get("filter_vars");
276-
if (filter_vars) {
277-
ObjectLock olock(filter_vars);
278-
for (const Dictionary::Pair& kv : filter_vars) {
279-
frameNS->Set(kv.first, kv.second);
276+
bool targeted = false;
277+
std::vector<ConfigObject::Ptr> targets;
278+
279+
if (dynamic_cast<ConfigObjectTargetProvider*>(provider.get())) {
280+
auto dict (dynamic_cast<DictExpression*>(ufilter.get()));
281+
282+
if (dict) {
283+
auto& subex (dict->GetExpressions());
284+
285+
if (subex.size() == 1u) {
286+
if (type == "Host") {
287+
std::vector<const String *> targetNames;
288+
289+
if (ApplyRule::GetTargetHosts(subex.at(0).get(), targetNames, filter_vars)) {
290+
static const auto typeHost (Type::GetByName("Host"));
291+
static const auto ctypeHost (dynamic_cast<ConfigType*>(typeHost.get()));
292+
targeted = true;
293+
294+
for (auto name : targetNames) {
295+
auto target (ctypeHost->GetObject(*name));
296+
297+
if (target) {
298+
targets.emplace_back(target);
299+
}
300+
}
301+
}
302+
} else if (type == "Service") {
303+
std::vector<std::pair<const String *, const String *>> targetNames;
304+
305+
if (ApplyRule::GetTargetServices(subex.at(0).get(), targetNames, filter_vars)) {
306+
static const auto typeService (Type::GetByName("Service"));
307+
static const auto ctypeService (dynamic_cast<ConfigType*>(typeService.get()));
308+
targeted = true;
309+
310+
for (auto name : targetNames) {
311+
auto target (ctypeService->GetObject(*name.first + "!" + *name.second));
312+
313+
if (target) {
314+
targets.emplace_back(target);
315+
}
316+
}
317+
}
318+
}
319+
}
280320
}
281321
}
282322

283-
provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) {
284-
FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target);
285-
});
323+
if (targeted) {
324+
for (auto& target : targets) {
325+
if (FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), target, variableName)) {
326+
result.emplace_back(std::move(target));
327+
}
328+
}
329+
} else {
330+
if (filter_vars) {
331+
ObjectLock olock (filter_vars);
332+
333+
for (auto& kv : filter_vars) {
334+
frameNS->Set(kv.first, kv.second);
335+
}
336+
}
337+
338+
provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) {
339+
FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target);
340+
});
341+
}
286342
} else {
287343
/* Ensure to pass a nullptr as filter expression.
288344
* GCC 8.1.1 on F28 causes problems, see GH #6533.

‎test/CMakeLists.txt

+33
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ set(base_test_SOURCES
2424
base-type.cpp
2525
base-utility.cpp
2626
base-value.cpp
27+
config-apply.cpp
2728
config-ops.cpp
2829
icinga-checkresult.cpp
2930
icinga-dependencies.cpp
@@ -130,6 +131,38 @@ add_boost_test(base
130131
base_value/scalar
131132
base_value/convert
132133
base_value/format
134+
config_apply/gettargethosts_literal
135+
config_apply/gettargethosts_const
136+
config_apply/gettargethosts_swapped
137+
config_apply/gettargethosts_two
138+
config_apply/gettargethosts_three
139+
config_apply/gettargethosts_mixed
140+
config_apply/gettargethosts_redundant
141+
config_apply/gettargethosts_badconst
142+
config_apply/gettargethosts_notliteral
143+
config_apply/gettargethosts_wrongop
144+
config_apply/gettargethosts_wrongattr
145+
config_apply/gettargethosts_wrongvar
146+
config_apply/gettargethosts_noindexer
147+
config_apply/gettargetservices_literal
148+
config_apply/gettargetservices_const
149+
config_apply/gettargetservices_swapped_outer
150+
config_apply/gettargetservices_swapped_inner
151+
config_apply/gettargetservices_two
152+
config_apply/gettargetservices_three
153+
config_apply/gettargetservices_mixed
154+
config_apply/gettargetservices_redundant
155+
config_apply/gettargetservices_badconst
156+
config_apply/gettargetservices_notliteral
157+
config_apply/gettargetservices_wrongop_outer
158+
config_apply/gettargetservices_wrongop_host
159+
config_apply/gettargetservices_wrongop_service
160+
config_apply/gettargetservices_wrongattr_host
161+
config_apply/gettargetservices_wrongattr_service
162+
config_apply/gettargetservices_wrongvar_host
163+
config_apply/gettargetservices_wrongvar_service
164+
config_apply/gettargetservices_noindexer_host
165+
config_apply/gettargetservices_noindexer_service
133166
config_ops/simple
134167
config_ops/advanced
135168
icinga_checkresult/host_1attempt

0 commit comments

Comments
 (0)