From: Gunnar Beutner Date: Tue, 28 Oct 2014 10:54:56 +0000 (+0100) Subject: Implement validation for "repository add" X-Git-Tag: v2.2.0~227 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3dc2f8234504b7e94a605955cd3cd0306696040d;p=icinga2 Implement validation for "repository add" fixes #7458 --- diff --git a/lib/cli/repositoryobjectcommand.cpp b/lib/cli/repositoryobjectcommand.cpp index c2a67521c..6924dccdf 100644 --- a/lib/cli/repositoryobjectcommand.cpp +++ b/lib/cli/repositoryobjectcommand.cpp @@ -175,13 +175,6 @@ int RepositoryObjectCommand::Run(const boost::program_options::variables_map& vm String name = attrs->Get("name"); - if (m_Type == "Service") { - if (!attrs->Contains("host_name")) { - Log(LogCritical, "cli", "Service objects require the 'host_name' attribute."); - return 1; - } - } - if (vm.count("import")) { Array::Ptr imports = make_shared(); @@ -196,13 +189,12 @@ int RepositoryObjectCommand::Run(const boost::program_options::variables_map& vm if (m_Command == RepositoryCommandAdd) { + Utility::LoadExtensionLibrary("icinga"); RepositoryUtility::AddObject(name, m_Type, attrs); - } - else if (m_Command == RepositoryCommandRemove) { + } else if (m_Command == RepositoryCommandRemove) { /* pass attrs for service->host_name requirement */ RepositoryUtility::RemoveObject(name, m_Type, attrs); - } - else if (m_Command == RepositoryCommandSet) { + } else if (m_Command == RepositoryCommandSet) { Log(LogWarning, "cli") << "Not supported yet. Please check the roadmap at https://dev.icinga.org\n"; return 1; diff --git a/lib/cli/repositoryutility.cpp b/lib/cli/repositoryutility.cpp index a77d05336..ffef6dc5e 100644 --- a/lib/cli/repositoryutility.cpp +++ b/lib/cli/repositoryutility.cpp @@ -19,6 +19,9 @@ #include "cli/repositoryutility.hpp" #include "cli/clicommand.hpp" +#include "config/configtype.hpp" +#include "config/configcompilercontext.hpp" +#include "config/configcompiler.hpp" #include "base/logger.hpp" #include "base/application.hpp" #include "base/convert.hpp" @@ -177,6 +180,15 @@ void RepositoryUtility::PrintChangeLog(std::ostream& fp) } } +class RepositoryTypeRuleUtilities : public TypeRuleUtilities +{ +public: + virtual bool ValidateName(const String& type, const String& name, String *hint) const + { + return true; + } +}; + /* modify objects and write changelog */ bool RepositoryUtility::AddObject(const String& name, const String& type, const Dictionary::Ptr& attrs) { @@ -191,6 +203,45 @@ bool RepositoryUtility::AddObject(const String& name, const String& type, const change->Set("command", "add"); change->Set("attrs", attrs); + ConfigCompilerContext::GetInstance()->Reset(); + + String fname, fragment; + BOOST_FOREACH(boost::tie(fname, fragment), ConfigFragmentRegistry::GetInstance()->GetItems()) { + ConfigCompiler::CompileText(fname, fragment); + } + + ConfigType::Ptr ctype = ConfigType::GetByName(type); + + if (!ctype) + Log(LogCritical, "cli") + << "No validation type available for '" << type << "'."; + else { + Dictionary::Ptr vattrs = attrs->ShallowClone(); + vattrs->Set("__name", vattrs->Get("name")); + vattrs->Remove("name"); + vattrs->Set("type", type); + + RepositoryTypeRuleUtilities utils; + ctype->ValidateItem(name, vattrs, DebugInfo(), &utils); + + int warnings = 0, errors = 0; + + BOOST_FOREACH(const ConfigCompilerMessage& message, ConfigCompilerContext::GetInstance()->GetMessages()) { + String logmsg = String("Config ") + (message.Error ? "error" : "warning") + ": " + message.Text; + + if (message.Error) { + Log(LogCritical, "config", logmsg); + errors++; + } else { + Log(LogWarning, "config", logmsg); + warnings++; + } + } + + if (errors > 0) + return false; + } + return WriteObjectToRepositoryChangeLog(path, change); } diff --git a/lib/config/configitem.cpp b/lib/config/configitem.cpp index 67b288ba8..06bf024a5 100644 --- a/lib/config/configitem.cpp +++ b/lib/config/configitem.cpp @@ -270,8 +270,10 @@ void ConfigItem::ValidateItem(void) return; } + TypeRuleUtilities utils; + try { - ctype->ValidateItem(GetSelf()); + ctype->ValidateItem(GetName(), GetProperties(), GetDebugInfo(), &utils); } catch (const ConfigError& ex) { const DebugInfo *di = boost::get_error_info(ex); ConfigCompilerContext::GetInstance()->AddMessage(true, ex.what(), di ? *di : DebugInfo()); diff --git a/lib/config/configtype.cpp b/lib/config/configtype.cpp index 3c15d09de..fb28c654e 100644 --- a/lib/config/configtype.cpp +++ b/lib/config/configtype.cpp @@ -74,33 +74,21 @@ void ConfigType::AddParentRules(std::vector& ruleLists, const } } -void ConfigType::ValidateItem(const ConfigItem::Ptr& item) +void ConfigType::ValidateItem(const String& name, const Dictionary::Ptr& attrs, const DebugInfo& debugInfo, const TypeRuleUtilities *utils) { - /* Don't validate abstract items. */ - if (item->IsAbstract()) - return; - - Dictionary::Ptr attrs; - DebugInfo debugInfo; - String type, name; - - { - ObjectLock olock(item); - - attrs = item->GetProperties(); - debugInfo = item->GetDebugInfo(); - type = item->GetType(); - name = item->GetName(); - } + String location = "Object '" + name + "' (Type: '" + GetName() + "')"; + if (!debugInfo.Path.IsEmpty()) + location += " at " + debugInfo.Path + ":" + Convert::ToString(debugInfo.FirstLine); + std::vector locations; - locations.push_back("Object '" + name + "' (Type: '" + type + "') at " + debugInfo.Path + ":" + Convert::ToString(debugInfo.FirstLine)); + locations.push_back(location); std::vector ruleLists; AddParentRules(ruleLists, GetSelf()); ruleLists.push_back(m_RuleList); - ValidateDictionary(attrs, ruleLists, locations); + ValidateDictionary(attrs, ruleLists, locations, utils); } String ConfigType::LocationToString(const std::vector& locations) @@ -120,7 +108,8 @@ String ConfigType::LocationToString(const std::vector& locations) } void ConfigType::ValidateDictionary(const Dictionary::Ptr& dictionary, - const std::vector& ruleLists, std::vector& locations) + const std::vector& ruleLists, std::vector& locations, + const TypeRuleUtilities *utils) { BOOST_FOREACH(const TypeRuleList::Ptr& ruleList, ruleLists) { BOOST_FOREACH(const String& require, ruleList->GetRequires()) { @@ -163,7 +152,7 @@ void ConfigType::ValidateDictionary(const Dictionary::Ptr& dictionary, BOOST_FOREACH(const TypeRuleList::Ptr& ruleList, ruleLists) { TypeRuleList::Ptr subRuleList; - TypeValidationResult result = ruleList->ValidateAttribute(kv.first, kv.second, &subRuleList, &hint); + TypeValidationResult result = ruleList->ValidateAttribute(kv.first, kv.second, &subRuleList, &hint, utils); if (subRuleList) subRuleLists.push_back(subRuleList); @@ -192,16 +181,17 @@ void ConfigType::ValidateDictionary(const Dictionary::Ptr& dictionary, } if (!subRuleLists.empty() && kv.second.IsObjectType()) - ValidateDictionary(kv.second, subRuleLists, locations); + ValidateDictionary(kv.second, subRuleLists, locations, utils); else if (!subRuleLists.empty() && kv.second.IsObjectType()) - ValidateArray(kv.second, subRuleLists, locations); + ValidateArray(kv.second, subRuleLists, locations, utils); locations.pop_back(); } } void ConfigType::ValidateArray(const Array::Ptr& array, - const std::vector& ruleLists, std::vector& locations) + const std::vector& ruleLists, std::vector& locations, + const TypeRuleUtilities *utils) { BOOST_FOREACH(const TypeRuleList::Ptr& ruleList, ruleLists) { BOOST_FOREACH(const String& require, ruleList->GetRequires()) { @@ -249,7 +239,7 @@ void ConfigType::ValidateArray(const Array::Ptr& array, BOOST_FOREACH(const TypeRuleList::Ptr& ruleList, ruleLists) { TypeRuleList::Ptr subRuleList; - TypeValidationResult result = ruleList->ValidateAttribute(key, value, &subRuleList, &hint); + TypeValidationResult result = ruleList->ValidateAttribute(key, value, &subRuleList, &hint, utils); if (subRuleList) subRuleLists.push_back(subRuleList); @@ -267,7 +257,7 @@ void ConfigType::ValidateArray(const Array::Ptr& array, } if (overallResult == ValidationUnknownField) - ConfigCompilerContext::GetInstance()->AddMessage(false, "Unknown attribute: " + LocationToString(locations)); + ConfigCompilerContext::GetInstance()->AddMessage(true, "Unknown attribute: " + LocationToString(locations)); else if (overallResult == ValidationInvalidType) { String message = "Invalid value for array index: " + LocationToString(locations); @@ -278,9 +268,9 @@ void ConfigType::ValidateArray(const Array::Ptr& array, } if (!subRuleLists.empty() && value.IsObjectType()) - ValidateDictionary(value, subRuleLists, locations); + ValidateDictionary(value, subRuleLists, locations, utils); else if (!subRuleLists.empty() && value.IsObjectType()) - ValidateArray(value, subRuleLists, locations); + ValidateArray(value, subRuleLists, locations, utils); locations.pop_back(); } @@ -310,4 +300,3 @@ ConfigTypeRegistry *ConfigTypeRegistry::GetInstance(void) { return Singleton::GetInstance(); } - diff --git a/lib/config/configtype.hpp b/lib/config/configtype.hpp index 3b62d96db..aa8b770ad 100644 --- a/lib/config/configtype.hpp +++ b/lib/config/configtype.hpp @@ -50,7 +50,8 @@ public: DebugInfo GetDebugInfo(void) const; - void ValidateItem(const ConfigItem::Ptr& object); + void ValidateItem(const String& name, const Dictionary::Ptr& attrs, + const DebugInfo& debugInfo, const TypeRuleUtilities *utils); void Register(void); static ConfigType::Ptr GetByName(const String& name); @@ -65,9 +66,11 @@ private: DebugInfo m_DebugInfo; /**< Debug information. */ static void ValidateDictionary(const Dictionary::Ptr& dictionary, - const std::vector& ruleLists, std::vector& locations); + const std::vector& ruleLists, std::vector& locations, + const TypeRuleUtilities *utils); static void ValidateArray(const Array::Ptr& array, - const std::vector& ruleLists, std::vector& locations); + const std::vector& ruleLists, std::vector& locations, + const TypeRuleUtilities *utils); static String LocationToString(const std::vector& locations); diff --git a/lib/config/typerule.cpp b/lib/config/typerule.cpp index fd33429a6..4a0c3940f 100644 --- a/lib/config/typerule.cpp +++ b/lib/config/typerule.cpp @@ -42,7 +42,7 @@ bool TypeRule::MatchName(const String& name) const return (Utility::Match(m_NamePattern, name)); } -bool TypeRule::MatchValue(const Value& value, String *hint) const +bool TypeRule::MatchValue(const Value& value, String *hint, const TypeRuleUtilities *utils) const { ConfigItem::Ptr item; @@ -77,21 +77,26 @@ bool TypeRule::MatchValue(const Value& value, String *hint) const if (!value.IsScalar()) return false; - item = ConfigItem::GetObject(m_NameType, value); + return utils->ValidateName(m_NameType, value, hint); - if (!item) { - *hint = "Object '" + value + "' of type '" + m_NameType + "' does not exist."; - return false; - } + default: + return false; + } +} - if (item->IsAbstract()) { - *hint = "Object '" + value + "' of type '" + m_NameType + "' must not be a template."; - return false; - } +bool TypeRuleUtilities::ValidateName(const String& type, const String& name, String *hint) const +{ + ConfigItem::Ptr item = ConfigItem::GetObject(type, name); - return true; + if (!item) { + *hint = "Object '" + name + "' of type '" + type + "' does not exist."; + return false; + } - default: - return false; + if (item->IsAbstract()) { + *hint = "Object '" + name + "' of type '" + type + "' must not be a template."; + return false; } + + return true; } diff --git a/lib/config/typerule.hpp b/lib/config/typerule.hpp index 2e6ece95a..355913239 100644 --- a/lib/config/typerule.hpp +++ b/lib/config/typerule.hpp @@ -27,6 +27,17 @@ namespace icinga { +/** + * Utilities for type rules. + * + * @ingroup config + */ +class TypeRuleUtilities +{ +public: + virtual bool ValidateName(const String& type, const String& name, String *hint) const; +}; + /** * The allowed type for a type rule. * @@ -58,7 +69,7 @@ public: TypeRuleList::Ptr GetSubRules(void) const; bool MatchName(const String& name) const; - bool MatchValue(const Value& value, String *hint) const; + bool MatchValue(const Value& value, String *hint, const TypeRuleUtilities *utils) const; private: TypeSpecifier m_Type; diff --git a/lib/config/typerulelist.cpp b/lib/config/typerulelist.cpp index 9f48fbf15..b9e9ab4be 100644 --- a/lib/config/typerulelist.cpp +++ b/lib/config/typerulelist.cpp @@ -117,7 +117,8 @@ size_t TypeRuleList::GetLength(void) const * @returns The validation result. */ TypeValidationResult TypeRuleList::ValidateAttribute(const String& name, - const Value& value, TypeRuleList::Ptr *subRules, String *hint) const + const Value& value, TypeRuleList::Ptr *subRules, String *hint, + const TypeRuleUtilities *utils) const { bool foundField = false; BOOST_FOREACH(const TypeRule& rule, m_Rules) { @@ -126,7 +127,7 @@ TypeValidationResult TypeRuleList::ValidateAttribute(const String& name, foundField = true; - if (rule.MatchValue(value, hint)) { + if (rule.MatchValue(value, hint, utils)) { *subRules = rule.GetSubRules(); return ValidationOK; } diff --git a/lib/config/typerulelist.hpp b/lib/config/typerulelist.hpp index 0c6a1504e..a131b4960 100644 --- a/lib/config/typerulelist.hpp +++ b/lib/config/typerulelist.hpp @@ -28,6 +28,7 @@ namespace icinga { struct TypeRule; +class TypeRuleUtilities; /** * @ingroup config @@ -59,7 +60,8 @@ public: void AddRule(const TypeRule& rule); void AddRules(const TypeRuleList::Ptr& ruleList); - TypeValidationResult ValidateAttribute(const String& name, const Value& value, TypeRuleList::Ptr *subRules, String *hint) const; + TypeValidationResult ValidateAttribute(const String& name, const Value& value, + TypeRuleList::Ptr *subRules, String *hint, const TypeRuleUtilities *utils) const; size_t GetLength(void) const;