From fb323ee215a14be4f2244ec21491124bb0efcb1b Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Thu, 29 Jan 2015 12:38:25 +0100 Subject: [PATCH] Evaluate apply filters in the for loop fixes #7610 --- doc/3-monitoring-basics.md | 6 ---- doc/4-configuring-icinga-2.md | 2 -- etc/icinga2/conf.d/services.conf | 4 --- lib/base/dictionary.cpp | 20 +++++++++++++ lib/base/dictionary.hpp | 3 ++ lib/config/config_parser.yy | 17 +++++++---- lib/icinga/dependency-apply.cpp | 39 +++++++++++++++++--------- lib/icinga/dependency.hpp | 2 +- lib/icinga/notification-apply.cpp | 34 ++++++++++++++-------- lib/icinga/notification.hpp | 2 +- lib/icinga/scheduleddowntime-apply.cpp | 39 +++++++++++++++++--------- lib/icinga/scheduleddowntime.hpp | 2 +- lib/icinga/service-apply.cpp | 39 +++++++++++++++++--------- lib/icinga/service.hpp | 2 +- 14 files changed, 136 insertions(+), 75 deletions(-) diff --git a/doc/3-monitoring-basics.md b/doc/3-monitoring-basics.md index 4df47d8a5..3d01e9c98 100644 --- a/doc/3-monitoring-basics.md +++ b/doc/3-monitoring-basics.md @@ -427,8 +427,6 @@ You can also specifiy the check command that way. notes = "Interface check for Port " + string(vars.port) + " in VLAN " + vars.vlan + " on Address " + vars.address + " QoS " + vars.qos notes_url = "http://foreman.company.com/hosts/" + host.name action_url = "http://snmp.checker.company.com/" + host.name + "if-" + if_name - - assign where host.vars.interfaces } Note that numbers must be explicitely casted to string when adding to strings. @@ -481,8 +479,6 @@ values for any object attribute specified in that apply rule. notes_url = "http://foreman.company.com/hosts/" + host.name action_url = "http://snmp.checker.company.com/" + host.name + "/" + vars.customer_id - - assign where host.vars.hosting } ### Groups @@ -1003,8 +999,6 @@ string values for passing multiple partitions to the `check_disk` check plugin. vars.disk_wfree = 10 vars.disk_cfree = 5 - - assign where host.vars.local_disks } diff --git a/doc/4-configuring-icinga-2.md b/doc/4-configuring-icinga-2.md index d0a06403c..543f9e22f 100644 --- a/doc/4-configuring-icinga-2.md +++ b/doc/4-configuring-icinga-2.md @@ -366,8 +366,6 @@ Configuration example: check_command = "disk" vars += config - - assign where host.vars.disks } A similar example is used for the `http` services. That way you can make your diff --git a/etc/icinga2/conf.d/services.conf b/etc/icinga2/conf.d/services.conf index 4f794b61d..014f07784 100644 --- a/etc/icinga2/conf.d/services.conf +++ b/etc/icinga2/conf.d/services.conf @@ -61,8 +61,6 @@ apply Service for (http_vhost => config in host.vars.http_vhosts) { check_command = "http" vars += config - - assign where host.vars.http_vhosts } apply Service for (disk => config in host.vars.disks) { @@ -71,8 +69,6 @@ apply Service for (disk => config in host.vars.disks) { check_command = "disk" vars += config - - assign where host.vars.disks } apply Service "icinga" { diff --git a/lib/base/dictionary.cpp b/lib/base/dictionary.cpp index 34c2368b3..509da9565 100644 --- a/lib/base/dictionary.cpp +++ b/lib/base/dictionary.cpp @@ -215,3 +215,23 @@ Dictionary::Ptr Dictionary::ShallowClone(void) const CopyTo(clone); return clone; } + +/** + * Returns an array containing all keys + * which are currently set in this directory. + * + * @returns an array of key names + */ +std::vector Dictionary::GetKeys(void) const +{ + ASSERT(!OwnsLock()); + ObjectLock olock(this); + + std::vector keys; + + BOOST_FOREACH(const Dictionary::Pair& kv, m_Data) { + keys.push_back(kv.first); + } + + return keys; +} diff --git a/lib/base/dictionary.hpp b/lib/base/dictionary.hpp index 10f278c50..d6fdefdfd 100644 --- a/lib/base/dictionary.hpp +++ b/lib/base/dictionary.hpp @@ -25,6 +25,7 @@ #include "base/value.hpp" #include #include +#include namespace icinga { @@ -64,6 +65,8 @@ public: void CopyTo(const Dictionary::Ptr& dest) const; Dictionary::Ptr ShallowClone(void) const; + std::vector GetKeys(void) const; + static Object::Ptr GetPrototype(void); private: diff --git a/lib/config/config_parser.yy b/lib/config/config_parser.yy index 2c1b27b80..acef893fa 100644 --- a/lib/config/config_parser.yy +++ b/lib/config/config_parser.yy @@ -996,16 +996,23 @@ apply: DictExpression *exprl = dynamic_cast($8); exprl->MakeInline(); - // assign && !ignore - if (!context->m_SeenAssign.top()) - BOOST_THROW_EXCEPTION(ScriptError("'apply' is missing 'assign'", DebugInfoRange(@2, @3))); - + bool seen_assign = context->m_SeenAssign.top(); context->m_SeenAssign.pop(); + // assign && !ignore + if (!seen_assign && !context->m_FTerm.top()) + BOOST_THROW_EXCEPTION(ScriptError("'apply' is missing 'assign'/'for'", DebugInfoRange(@2, @3))); + Expression *ignore = context->m_Ignore.top(); context->m_Ignore.pop(); - Expression *assign = context->m_Assign.top(); + Expression *assign; + + if (!seen_assign) + assign = MakeLiteral(true); + else + assign = context->m_Assign.top(); + context->m_Assign.pop(); Expression *filter; diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index b3e206507..a16c9c059 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -41,8 +41,11 @@ void Dependency::RegisterApplyRuleHandler(void) ApplyRule::RegisterType("Dependency", targets); } -void Dependency::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) +bool Dependency::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) { + if (!rule.EvaluateFilter(frame)) + return false; + DebugInfo di = rule.GetDebugInfo(); Log(LogDebug, "Dependency") @@ -72,6 +75,8 @@ void Dependency::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, cons ConfigItem::Ptr dependencyItem = builder->Compile(); dependencyItem->Commit(); + + return true; } bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule) @@ -93,27 +98,32 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR if (service) frame.Locals->Set("service", service); - if (!rule.EvaluateFilter(frame)) - return false; - Value vinstances; if (rule.GetFTerm()) { - vinstances = rule.GetFTerm()->Evaluate(frame); + try { + vinstances = rule.GetFTerm()->Evaluate(frame); + } catch (const std::exception&) { + /* Silently ignore errors here and assume there are no instances. */ + return false; + } } else { Array::Ptr instances = new Array(); instances->Add(""); vinstances = instances; } + bool match = false; + if (vinstances.IsObjectType()) { if (!rule.GetFVVar().IsEmpty()) BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); Array::Ptr arr = vinstances; + Array::Ptr arrclone = arr->ShallowClone(); - ObjectLock olock(arr); - BOOST_FOREACH(const String& instance, arr) { + ObjectLock olock(arrclone); + BOOST_FOREACH(const String& instance, arrclone) { String name = rule.GetName(); if (!rule.GetFKVar().IsEmpty()) { @@ -121,7 +131,8 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR name += instance; } - EvaluateApplyRuleInstance(checkable, name, frame, rule); + if (EvaluateApplyRuleInstance(checkable, name, frame, rule)) + match = true; } } else if (vinstances.IsObjectType()) { if (rule.GetFVVar().IsEmpty()) @@ -129,16 +140,16 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR Dictionary::Ptr dict = vinstances; - ObjectLock olock(dict); - BOOST_FOREACH(const Dictionary::Pair& kv, dict) { - frame.Locals->Set(rule.GetFKVar(), kv.first); - frame.Locals->Set(rule.GetFVVar(), kv.second); + BOOST_FOREACH(const String& key, dict->GetKeys()) { + frame.Locals->Set(rule.GetFKVar(), key); + frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule); + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule)) + match = true; } } - return true; + return match; } void Dependency::EvaluateApplyRules(const Host::Ptr& host) diff --git a/lib/icinga/dependency.hpp b/lib/icinga/dependency.hpp index fd2274293..6635cb302 100644 --- a/lib/icinga/dependency.hpp +++ b/lib/icinga/dependency.hpp @@ -65,7 +65,7 @@ private: Checkable::Ptr m_Parent; Checkable::Ptr m_Child; - static void EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); + static bool EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); static bool EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule); }; diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index 0fa71587c..b6fe251b4 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -41,8 +41,11 @@ void Notification::RegisterApplyRuleHandler(void) ApplyRule::RegisterType("Notification", targets); } -void Notification::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) +bool Notification::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) { + if (!rule.EvaluateFilter(frame)) + return false; + DebugInfo di = rule.GetDebugInfo(); Log(LogDebug, "Notification") @@ -71,6 +74,8 @@ void Notification::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, co ConfigItem::Ptr notificationItem = builder->Compile(); notificationItem->Commit(); + + return true; } bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule) @@ -92,19 +97,23 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl if (service) frame.Locals->Set("service", service); - if (!rule.EvaluateFilter(frame)) - return false; - Value vinstances; if (rule.GetFTerm()) { - vinstances = rule.GetFTerm()->Evaluate(frame); + try { + vinstances = rule.GetFTerm()->Evaluate(frame); + } catch (const std::exception&) { + /* Silently ignore errors here and assume there are no instances. */ + return false; + } } else { Array::Ptr instances = new Array(); instances->Add(""); vinstances = instances; } + bool match = false; + if (vinstances.IsObjectType()) { if (!rule.GetFVVar().IsEmpty()) BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); @@ -120,7 +129,8 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl name += instance; } - EvaluateApplyRuleInstance(checkable, name, frame, rule); + if (EvaluateApplyRuleInstance(checkable, name, frame, rule)) + match = true; } } else if (vinstances.IsObjectType()) { if (rule.GetFVVar().IsEmpty()) @@ -128,16 +138,16 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl Dictionary::Ptr dict = vinstances; - ObjectLock olock(dict); - BOOST_FOREACH(const Dictionary::Pair& kv, dict) { - frame.Locals->Set(rule.GetFKVar(), kv.first); - frame.Locals->Set(rule.GetFVVar(), kv.second); + BOOST_FOREACH(const String& key, dict->GetKeys()) { + frame.Locals->Set(rule.GetFKVar(), key); + frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule); + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule)) + match = true; } } - return true; + return match; } void Notification::EvaluateApplyRules(const Host::Ptr& host) diff --git a/lib/icinga/notification.hpp b/lib/icinga/notification.hpp index a56949d58..5bafdaf85 100644 --- a/lib/icinga/notification.hpp +++ b/lib/icinga/notification.hpp @@ -123,7 +123,7 @@ protected: private: void ExecuteNotificationHelper(NotificationType type, const User::Ptr& user, const CheckResult::Ptr& cr, bool force, const String& author = "", const String& text = ""); - static void EvaluateApplyRuleInstance(const intrusive_ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); + static bool EvaluateApplyRuleInstance(const intrusive_ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); static bool EvaluateApplyRule(const intrusive_ptr& checkable, const ApplyRule& rule); }; diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index d9ef1c1f6..33336e852 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -40,8 +40,11 @@ void ScheduledDowntime::RegisterApplyRuleHandler(void) ApplyRule::RegisterType("ScheduledDowntime", targets); } -void ScheduledDowntime::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) +bool ScheduledDowntime::EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule) { + if (!rule.EvaluateFilter(frame)) + return false; + DebugInfo di = rule.GetDebugInfo(); Log(LogDebug, "ScheduledDowntime") @@ -70,6 +73,8 @@ void ScheduledDowntime::EvaluateApplyRuleInstance(const Checkable::Ptr& checkabl ConfigItem::Ptr downtimeItem = builder->Compile(); downtimeItem->Commit(); + + return true; } bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule) @@ -91,27 +96,32 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const if (service) frame.Locals->Set("service", service); - if (!rule.EvaluateFilter(frame)) - return false; - Value vinstances; if (rule.GetFTerm()) { - vinstances = rule.GetFTerm()->Evaluate(frame); + try { + vinstances = rule.GetFTerm()->Evaluate(frame); + } catch (const std::exception&) { + /* Silently ignore errors here and assume there are no instances. */ + return false; + } } else { Array::Ptr instances = new Array(); instances->Add(""); vinstances = instances; } + bool match = false; + if (vinstances.IsObjectType()) { if (!rule.GetFVVar().IsEmpty()) BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); Array::Ptr arr = vinstances; + Array::Ptr arrclone = arr->ShallowClone(); - ObjectLock olock(arr); - BOOST_FOREACH(const String& instance, arr) { + ObjectLock olock(arrclone); + BOOST_FOREACH(const String& instance, arrclone) { String name = rule.GetName(); if (!rule.GetFKVar().IsEmpty()) { @@ -119,7 +129,8 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const name += instance; } - EvaluateApplyRuleInstance(checkable, name, frame, rule); + if (EvaluateApplyRuleInstance(checkable, name, frame, rule)) + match = true; } } else if (vinstances.IsObjectType()) { if (rule.GetFVVar().IsEmpty()) @@ -127,16 +138,16 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const Dictionary::Ptr dict = vinstances; - ObjectLock olock(dict); - BOOST_FOREACH(const Dictionary::Pair& kv, dict) { - frame.Locals->Set(rule.GetFKVar(), kv.first); - frame.Locals->Set(rule.GetFVVar(), kv.second); + BOOST_FOREACH(const String& key, dict->GetKeys()) { + frame.Locals->Set(rule.GetFKVar(), key); + frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule); + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule)) + match = true; } } - return true; + return match; } void ScheduledDowntime::EvaluateApplyRules(const Host::Ptr& host) diff --git a/lib/icinga/scheduleddowntime.hpp b/lib/icinga/scheduleddowntime.hpp index 723dfd779..22394c5a4 100644 --- a/lib/icinga/scheduleddowntime.hpp +++ b/lib/icinga/scheduleddowntime.hpp @@ -63,7 +63,7 @@ private: std::pair FindNextSegment(void); void CreateNextDowntime(void); - static void EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); + static bool EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule); static bool EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule); }; diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index 4c9124e4b..5c39e9cf0 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -39,8 +39,11 @@ void Service::RegisterApplyRuleHandler(void) ApplyRule::RegisterType("Service", targets); } -void Service::EvaluateApplyRuleInstance(const Host::Ptr& host, const String& name, ScriptFrame& frame, const ApplyRule& rule) +bool Service::EvaluateApplyRuleInstance(const Host::Ptr& host, const String& name, ScriptFrame& frame, const ApplyRule& rule) { + if (!rule.EvaluateFilter(frame)) + return false; + DebugInfo di = rule.GetDebugInfo(); Log(LogDebug, "Service") @@ -64,6 +67,8 @@ void Service::EvaluateApplyRuleInstance(const Host::Ptr& host, const String& nam ConfigItem::Ptr serviceItem = builder->Compile(); serviceItem->Commit(); + + return true; } bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule) @@ -79,27 +84,32 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule) rule.GetScope()->CopyTo(frame.Locals); frame.Locals->Set("host", host); - if (!rule.EvaluateFilter(frame)) - return false; - Value vinstances; if (rule.GetFTerm()) { - vinstances = rule.GetFTerm()->Evaluate(frame); + try { + vinstances = rule.GetFTerm()->Evaluate(frame); + } catch (const std::exception&) { + /* Silently ignore errors here and assume there are no instances. */ + return false; + } } else { Array::Ptr instances = new Array(); instances->Add(""); vinstances = instances; } + bool match = false; + if (vinstances.IsObjectType()) { if (!rule.GetFVVar().IsEmpty()) BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); Array::Ptr arr = vinstances; + Array::Ptr arrclone = arr->ShallowClone(); - ObjectLock olock(arr); - BOOST_FOREACH(const String& instance, arr) { + ObjectLock olock(arrclone); + BOOST_FOREACH(const String& instance, arrclone) { String name = rule.GetName(); if (!rule.GetFKVar().IsEmpty()) { @@ -107,7 +117,8 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule) name += instance; } - EvaluateApplyRuleInstance(host, name, frame, rule); + if (EvaluateApplyRuleInstance(host, name, frame, rule)) + match = true; } } else if (vinstances.IsObjectType()) { if (rule.GetFVVar().IsEmpty()) @@ -115,16 +126,16 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule) Dictionary::Ptr dict = vinstances; - ObjectLock olock(dict); - BOOST_FOREACH(const Dictionary::Pair& kv, dict) { - frame.Locals->Set(rule.GetFKVar(), kv.first); - frame.Locals->Set(rule.GetFVVar(), kv.second); + BOOST_FOREACH(const String& key, dict->GetKeys()) { + frame.Locals->Set(rule.GetFKVar(), key); + frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - EvaluateApplyRuleInstance(host, rule.GetName() + kv.first, frame, rule); + if (EvaluateApplyRuleInstance(host, rule.GetName() + key, frame, rule)) + match = true; } } - return true; + return match; } void Service::EvaluateApplyRules(const Host::Ptr& host) diff --git a/lib/icinga/service.hpp b/lib/icinga/service.hpp index 1c61fbfeb..548b64746 100644 --- a/lib/icinga/service.hpp +++ b/lib/icinga/service.hpp @@ -61,7 +61,7 @@ protected: private: Host::Ptr m_Host; - static void EvaluateApplyRuleInstance(const Host::Ptr& host, const String& name, ScriptFrame& frame, const ApplyRule& rule); + static bool EvaluateApplyRuleInstance(const Host::Ptr& host, const String& name, ScriptFrame& frame, const ApplyRule& rule); static bool EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule); };