[checkmk-commits] Windows agent: make readConfigFile testable

Module: check_mk Branch: master Commit: 6ef77e94a85155b72b033b130b8afcdf48e99376 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=6ef77e94a85155… Author: Jukka Aro &lt;ja(a)mathias-kettner.de&gt; Date: Thu Feb 15 16:01:58 2018 +0100 Windows agent: make readConfigFile testable * Turn configFileName into standalone function that can be tested separately without dependencies to the internals of a Configuration instance. * Pass std::istream to readConfigFile instead of filename. * Move (static member function) configFileName out of Configuration class. * Move helper functions out of Configuration into an anonymous namespace. --- agents/windows/Configuration.cc | 67 ++++++++++++++++--------------- agents/windows/Configuration.h | 45 ++++++++++++--------- agents/windows/sections/SectionCheckMK.cc | 4 +- 3 files changed, 64 insertions(+), 52 deletions(-) diff --git a/agents/windows/Configuration.cc b/agents/windows/Configuration.cc index 44472a2..a6fd2f5 100644 --- a/agents/windows/Configuration.cc +++ b/agents/windows/Configuration.cc @@ -31,12 +31,24 @@ #include <fstream> #include <regex> #include "Configurable.h" -#include "Environment.h" #include "PerfCounter.h" #include "stringutil.h" #define __STDC_FORMAT_MACROS +namespace { + +bool checkHostRestriction(const std::string &hostname, + const std::string &input) { + const auto patterns = tokenize(input, "\\s+"); + return std::any_of(patterns.cbegin(), patterns.cend(), + [&hostname](const auto &p) { + return globmatch(p.c_str(), hostname.c_str()); + }); +} + +} // namespace + void Configuration::readSettings() { for (bool local : {false, true}) { for (const auto &cfg : _configurables) { @@ -44,8 +56,15 @@ void Configuration::readSettings() { entry->startFile(); } } - - readConfigFile(configFileName(local, _environment)); + const auto filename = configFileName(local, _environment); + try { + std::ifstream ifs(filename); + readConfigFile(ifs, _environment.hostname(), _configurables); + } catch (const ParseError &e) { + std::cerr << e.what() << " line " << e.getLineNo() << " in " + << filename << std::endl; + exit(1); + } } } @@ -55,20 +74,6 @@ void Configuration::reg(const char *section, const char *key, std::unique_ptr<ConfigurableBase>(cfg)); } -std::string Configuration::configFileName(bool local, const Environment &env) { - return std::string(env.agentDirectory()) + "\\" + "check_mk" + - (local ? "_local" : "") + ".ini"; -} - -bool Configuration::checkHostRestriction(const std::string &input) { - std::string hostname = _environment.hostname(); - const auto patterns = tokenize(input, "\\s+"); - return std::any_of(patterns.cbegin(), patterns.cend(), - [&hostname](const auto &p) { - return globmatch(p.c_str(), hostname.c_str()); - }); -} - void Configuration::outputConfigurables(std::ostream &out) { using ConfigMap = std::map<std::string, std::reference_wrapper<ConfigurableBase>>; @@ -95,9 +100,9 @@ void Configuration::outputConfigurables(std::ostream &out) { } } -void Configuration::readConfigFile(const std::string &filename) { - std::ifstream ifs(filename); - if (!ifs) { +void readConfigFile(std::istream &is, const std::string &hostname, + ConfigurableMap &configurables) { + if (!is) { return; } @@ -105,7 +110,7 @@ void Configuration::readConfigFile(const std::string &filename) { bool is_active = true; // false in sections with host restrictions std::string section; - for (unsigned lineno = 1; std::getline(ifs, line); ++lineno) { + for (unsigned lineno = 1; std::getline(is, line); ++lineno) { ltrim(line); rtrim(line); if (line.empty() || line.front() == '#' || line.front() == ';') @@ -120,9 +125,7 @@ void Configuration::readConfigFile(const std::string &filename) { // split up line at = sign const auto tokens = tokenize(line, "="); if (tokens.size() != 2) { - std::cerr << "Invalid line " << lineno << " in " << filename - << "." << std::endl; - exit(1); + throw ParseError("Invalid", lineno); } std::string variable{tokens[0]}; rtrim(variable); @@ -133,7 +136,8 @@ void Configuration::readConfigFile(const std::string &filename) { rtrim(value); // handle host restriction - if (variable == "host") is_active = checkHostRestriction(value); + if (variable == "host") + is_active = checkHostRestriction(hostname, value); // skip all other variables for non-relevant hosts else if (!is_active) @@ -146,9 +150,9 @@ void Configuration::readConfigFile(const std::string &filename) { else { bool found = false; size_t key_len = strcspn(variable.c_str(), " \n"); - auto map_iter = _configurables.find( - config_key(section, std::string(variable, 0, key_len))); - if (map_iter != _configurables.end()) { + auto map_iter = configurables.find( + ConfigKey(section, std::string(variable, 0, key_len))); + if (map_iter != configurables.end()) { for (auto &cfg : map_iter->second) { try { cfg->feed(variable, value); @@ -160,10 +164,9 @@ void Configuration::readConfigFile(const std::string &filename) { } } if (!found) { - std::cerr << "Invalid entry (" << section << ":" << variable - << ") in " << filename << " line " << lineno - << std::endl; - exit(1); + throw ParseError( + "Invalid entry (" + section + ":" + variable + ")", + lineno); } } } diff --git a/agents/windows/Configuration.h b/agents/windows/Configuration.h index edccb02..aff7cf6 100644 --- a/agents/windows/Configuration.h +++ b/agents/windows/Configuration.h @@ -27,11 +27,35 @@ #include <map> #include <memory> +#include "Environment.h" #include "SettingsCollector.h" class ConfigurableBase; class Environment; +class ParseError : public std::runtime_error { +public: + explicit ParseError(const std::string &what, unsigned lineno) + : std::runtime_error(what), _lineno(lineno) {} + + unsigned getLineNo() const { return _lineno; } + +private: + unsigned _lineno{0}; +}; + +using ConfigKey = std::pair<std::string, std::string>; +using ConfigurableMap = + std::map<ConfigKey, std::vector<std::unique_ptr<ConfigurableBase>>>; + +void readConfigFile(std::istream &is, const std::string &hostname, + ConfigurableMap &configurables); + +inline std::string configFileName(bool local, const Environment &env) { + return env.agentDirectory() + "\\" + "check_mk" + (local ? "_local" : "") + + ".ini"; +} + /* Example configuration file: [global] @@ -62,18 +86,10 @@ check = DISK_C: mrpe/check_disk -w C: check = MEM mrpe/check_mem -w 10 -c 20 */ class Configuration { - using ConfigKey = std::pair<std::string, std::string>; - using ConfigurableKey = std::pair<std::string, std::string>; - using ConfigurableMap = - std::map<ConfigurableKey, - std::vector<std::unique_ptr<ConfigurableBase>>>; - - ConfigKey config_key(const std::string &section, const std::string &key) { - return std::make_pair(section, key); - } - public: - Configuration(const Environment &env) : _environment(env) {} + explicit Configuration(const Environment &env) : _environment(env) {} + Configuration(Configuration &) = delete; + Configuration &operator=(const Configuration &) = delete; void outputConfigurables(std::ostream &out); void readSettings(); @@ -81,13 +97,6 @@ public: void reg(const char *section, const char *key, ConfigurableBase *cfg); inline const Environment &getEnvironment() const { return _environment; } - static std::string configFileName(bool local, const Environment &env); - -private: - void readConfigFile(const std::string &filename); - - bool checkHostRestriction(const std::string &input); - private: ConfigurableMap _configurables; diff --git a/agents/windows/sections/SectionCheckMK.cc b/agents/windows/sections/SectionCheckMK.cc index c10dab1..92ee0bd 100644 --- a/agents/windows/sections/SectionCheckMK.cc +++ b/agents/windows/sections/SectionCheckMK.cc @@ -56,8 +56,8 @@ std::vector<KVPair> SectionCheckMK::createInfoFields() const { {"Hostname", _env.hostname()}, {"Architecture", arch}, {"WorkingDirectory", _env.currentDirectory()}, - {"ConfigFile", Configuration::configFileName(false, _env)}, - {"LocalConfigFile", Configuration::configFileName(true, _env)}, + {"ConfigFile", configFileName(false, _env)}, + {"LocalConfigFile", configFileName(true, _env)}, {"AgentDirectory", _env.agentDirectory()}, {"PluginsDirectory", _env.pluginsDirectory()}, {"StateDirectory", _env.stateDirectory()},