From: Tomek Mrugalski Date: Tue, 14 Feb 2017 14:06:28 +0000 (+0100) Subject: [5134] HooksLibrariesParser moved to lib/hooks X-Git-Tag: trac5137_base~3^2~14 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a998990f03c8bfbf5425d004e02a5839070168b2;p=thirdparty%2Fkea.git [5134] HooksLibrariesParser moved to lib/hooks --- diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 3e9f2a4a6f..5b388ee4db 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -433,7 +434,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set, // Some of the parsers alter the state of the system in a way that can't // easily be undone. (Or alter it in a way such that undoing the change has // the same risk of failure as doing the change.) - HooksLibrariesParser hooks_parser; + hooks::HooksLibrariesParser hooks_parser; // Answer will hold the result. ConstElementPtr answer; diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 52740668a6..8091c0ee04 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -640,7 +641,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set, // Some of the parsers alter state of the system that can't easily // be undone. (Or alter it in a way such that undoing the change // has the same risk of failure as doing the change.) - HooksLibrariesParser hooks_parser; + hooks::HooksLibrariesParser hooks_parser; // This is a way to convert ConstElementPtr to ElementPtr. // We need a config that can be edited, because we will insert diff --git a/src/lib/cc/dhcp_config_error.h b/src/lib/cc/dhcp_config_error.h index 272dc655f2..eabc98de04 100644 --- a/src/lib/cc/dhcp_config_error.h +++ b/src/lib/cc/dhcp_config_error.h @@ -21,19 +21,17 @@ class ParseError : public isc::Exception { isc::Exception(file, line, what) { }; }; -namespace dhcp { - /// An exception that is thrown if an error occurs while configuring -/// DHCP server. +/// any server. /// By convention when this exception is thrown there is a position /// between parentheses so the code style should be like this: /// /// try { /// ... -/// } catch (const DhcpConfigError&) { +/// } catch (const ConfigError&) { /// throw; /// } catch (const std::exception& ex) { -/// isc_throw(DhcpConfigError, "message" << ex.what() +/// isc_throw(ConfigError, "message" << ex.what() /// << " (" << getPosition(what) << ")"); /// } @@ -41,11 +39,25 @@ namespace dhcp { /// there is no dependency through DhcpConfigParser /// @todo: create an isc_throw like macro to add the /// position more easily. -/// @todo: rename the exception for instance into ConfigError +/// @todo: replace all references to DhcpConfigError with ConfigError, +/// then remove DhcpConfigError. +class ConfigError : public isc::Exception { +public: + + /// @brief constructor + /// + /// @param file name of the file, where exception occurred + /// @param line line of the file, where exception occurred + /// @param what text description of the issue that caused exception + ConfigError(const char* file, size_t line, const char* what) + : isc::Exception(file, line, what) {} +}; +namespace dhcp { + +/// @brief To be removed. Please use ConfigError instead. class DhcpConfigError : public isc::Exception { public: - /// @brief constructor /// /// @param file name of the file, where exception occurred diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index cafe509703..72921df067 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -28,7 +27,6 @@ using namespace std; using namespace isc::asiolink; using namespace isc::data; -using namespace isc::hooks; using namespace isc::util; namespace isc { @@ -166,139 +164,6 @@ void ControlSocketParser::parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr v srv_cfg.setControlSocketInfo(value); } -// ******************** HooksLibrariesParser ************************* -void -HooksLibrariesParser::parse(ConstElementPtr value) { - // Initialize. - libraries_.clear(); - - if (!value) { - isc_throw(DhcpConfigError, "Tried to parse null hooks libraries"); - } - - // Let's store - position_ = value->getPosition(); - - // This is the new syntax. Iterate through it and get each map. - BOOST_FOREACH(ConstElementPtr library_entry, value->listValue()) { - ConstElementPtr parameters; - - // Is it a map? - if (library_entry->getType() != Element::map) { - isc_throw(DhcpConfigError, "hooks library configuration error:" - " one or more entries in the hooks-libraries list is not" - " a map (" << library_entry->getPosition() << ")"); - } - - // Iterate through each element in the map. We check - // whether we have found a library element. - bool lib_found = false; - - string libname = ""; - - // Let's explicitly reset the parameters, so we won't cover old - // values from the previous loop round. - parameters.reset(); - - BOOST_FOREACH(ConfigPair entry_item, library_entry->mapValue()) { - if (entry_item.first == "library") { - if (entry_item.second->getType() != Element::string) { - isc_throw(DhcpConfigError, "hooks library configuration" - " error: value of 'library' element is not a string" - " giving the path to a hooks library (" << - entry_item.second->getPosition() << ")"); - } - - // Get the name of the library and add it to the list after - // removing quotes. - libname = (entry_item.second)->stringValue(); - - // Remove leading/trailing quotes and any leading/trailing - // spaces. - boost::erase_all(libname, "\""); - libname = isc::util::str::trim(libname); - if (libname.empty()) { - isc_throw(DhcpConfigError, "hooks library configuration" - " error: value of 'library' element must not be" - " blank (" << - entry_item.second->getPosition() << ")"); - } - - // Note we have found the library name. - lib_found = true; - continue; - } - - // If there are parameters, let's remember them. - if (entry_item.first == "parameters") { - parameters = entry_item.second; - continue; - } - - // For all other parameters we will throw. - isc_throw(DhcpConfigError, "unknown hooks library parameter: " - << entry_item.first << "(" - << library_entry->getPosition() << ")"); - } - - if (! lib_found) { - isc_throw(DhcpConfigError, "hooks library configuration error:" - " one or more hooks-libraries elements are missing the" - " name of the library" << - " (" << library_entry->getPosition() << ")"); - } - - libraries_.push_back(make_pair(libname, parameters)); - } -} - -void HooksLibrariesParser::verifyLibraries() { - // Check if the list of libraries has changed. If not, nothing is done - // - the command "DhcpN libreload" is required to reload the same - // libraries (this prevents needless reloads when anything else in the - // configuration is changed). - - // We no longer rely on this. Parameters can change. And even if the - // parameters stay the same, they could point to files that could - // change. - vector current_libraries = HooksManager::getLibraryNames(); - if (current_libraries.empty() && libraries_.empty()) { - return; - } - - // Library list has changed, validate each of the libraries specified. - vector lib_names = isc::hooks::extractNames(libraries_); - vector error_libs = HooksManager::validateLibraries(lib_names); - if (!error_libs.empty()) { - - // Construct the list of libraries in error for the message. - string error_list = error_libs[0]; - for (size_t i = 1; i < error_libs.size(); ++i) { - error_list += (string(", ") + error_libs[i]); - } - isc_throw(DhcpConfigError, "hooks libraries failed to validate - " - "library or libraries in error are: " << error_list - << "(" << position_ << ")"); - } -} - -void -HooksLibrariesParser::loadLibraries() { - /// Commits the list of libraries to the configuration manager storage if - /// the list of libraries has changed. - /// @todo: Delete any stored CalloutHandles before reloading the - /// libraries - if (!HooksManager::loadLibraries(libraries_)) { - isc_throw(DhcpConfigError, "One or more hook libraries failed to load"); - } -} - -// Method for testing -void -HooksLibrariesParser::getLibraries(isc::hooks::HookLibsCollection& libraries) { - libraries = libraries_; -} - // **************************** OptionDataParser ************************* OptionDataParser::OptionDataParser(const uint16_t address_family) : address_family_(address_family) { diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 67f253a5bf..1ef5667481 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -20,7 +20,6 @@ #include #include #include -#include #include #include @@ -342,106 +341,6 @@ public: void parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr value); }; -/// @brief Parser for hooks library list -/// -/// This parser handles the list of hooks libraries. This is an optional list, -/// which may be empty. -/// -/// However, the parser does more than just check the list of library names. -/// It does two other things: -/// -/// -# The problem faced with the hooks libraries is that we wish to avoid -/// reloading the libraries if they have not changed. (This would cause the -/// "unload" and "load" methods to run. Although libraries should be written -/// to cope with this, it is feasible that such an action may be costly in -/// terms of time and resources, or may cause side effects such as clearing -/// an internal cache.) To this end, the parser also checks the list against -/// the list of libraries current loaded and notes if there are changes. -/// -# If there are, the parser validates the libraries; it opens them and -/// checks that the "version" function exists and returns the correct value. -/// -/// Only if the library list has changed and the libraries are valid will the -/// change be applied. -class HooksLibrariesParser : public isc::data::SimpleParser { -public: - - /// @brief Parses parameters value - /// - /// Parses configuration entry (list of parameters) and adds each element - /// to the hooks libraries list. The method also checks whether the - /// list of libraries is the same as that already loaded. If not, it - /// checks each of the libraries in the list for validity (they exist and - /// have a "version" function that returns the correct value). - /// - /// The syntax for specifying hooks libraries allow for library-specific - /// parameters to be specified along with the library, e.g. - /// - /// @code - /// "hooks-libraries": [ - /// { - /// "library": "hook-lib-1.so", - /// "parameters": { - /// "alpha": "a string", - /// "beta": 42 - /// } - /// }, - /// : - /// ] - /// @endcode - /// - /// The parsing code only checks that: - /// - /// -# Each element in the hooks-libraries list is a map - /// -# The map contains an element "library" whose value is a not blank string - /// -# That there is an optional 'parameters' element. - /// -# That there are no other element. - /// - /// If you want to check whether the library is really present (if the file - /// is on disk, it is really a library and that it could be loaded), call - /// @ref verifyLibraries(). - /// - /// This method stores parsed libraries in libraries_. - /// - /// @param value pointer to the content of parsed values - void parse(isc::data::ConstElementPtr value); - - /// @brief Verifies that libraries stored in libraries_ are valid. - /// - /// This method is a smart wrapper around @ref - /// isc::hooks::HooksManager::validateLibraries(). - /// It tries to validate all the libraries stored in libraries_. - /// @throw DhcpConfigError if any issue is discovered. - void verifyLibraries(); - - /// @brief Commits hooks libraries data - /// - /// This method calls necessary methods in HooksManager that will unload - /// any libraries that may be currently loaded and will load the actual - /// libraries. Providing that the specified libraries are valid and are - /// different to those already loaded, this method loads the new set of - /// libraries (and unloads the existing set). - /// - /// @throw DhcpConfigError if the call to HooksManager fails. - void loadLibraries(); - - /// @brief Returns list of parsed libraries - /// - /// Principally for testing, this returns the list of libraries as well as - /// an indication as to whether the list is different from the list of - /// libraries already loaded. - /// - /// @param [out] libraries List of libraries that were specified in the - /// new configuration. - void getLibraries(isc::hooks::HookLibsCollection& libraries); - -private: - /// List of hooks libraries with their configuration parameters - isc::hooks::HookLibsCollection libraries_; - - /// Position of the original element is stored in case we need to report an - /// error later. - isc::data::Element::Position position_; -}; /// @brief Parser for option data value. /// diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index c7a729d5c9..15a40c633b 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include diff --git a/src/lib/hooks/Makefile.am b/src/lib/hooks/Makefile.am index 624e227205..b3169bea14 100644 --- a/src/lib/hooks/Makefile.am +++ b/src/lib/hooks/Makefile.am @@ -36,6 +36,7 @@ libkea_hooks_la_SOURCES += callout_manager.cc callout_manager.h libkea_hooks_la_SOURCES += hooks.h libkea_hooks_la_SOURCES += hooks_log.cc hooks_log.h libkea_hooks_la_SOURCES += hooks_manager.cc hooks_manager.h +libkea_hooks_la_SOURCES += hooks_parser.cc hooks_parser.h libkea_hooks_la_SOURCES += libinfo.cc libinfo.h libkea_hooks_la_SOURCES += library_handle.cc library_handle.h libkea_hooks_la_SOURCES += library_manager.cc library_manager.h diff --git a/src/lib/hooks/hooks_parser.cc b/src/lib/hooks/hooks_parser.cc new file mode 100644 index 0000000000..5c50d9997b --- /dev/null +++ b/src/lib/hooks/hooks_parser.cc @@ -0,0 +1,154 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace std; +using namespace isc::data; +using namespace isc::hooks; +using namespace isc::dhcp; + +// ******************** HooksLibrariesParser ************************* +void +HooksLibrariesParser::parse(ConstElementPtr value) { + // Initialize. + libraries_.clear(); + + if (!value) { + isc_throw(DhcpConfigError, "Tried to parse null hooks libraries"); + } + + // Let's store + position_ = value->getPosition(); + + // This is the new syntax. Iterate through it and get each map. + BOOST_FOREACH(ConstElementPtr library_entry, value->listValue()) { + ConstElementPtr parameters; + + // Is it a map? + if (library_entry->getType() != Element::map) { + isc_throw(DhcpConfigError, "hooks library configuration error:" + " one or more entries in the hooks-libraries list is not" + " a map (" << library_entry->getPosition() << ")"); + } + + // Iterate through each element in the map. We check + // whether we have found a library element. + bool lib_found = false; + + string libname = ""; + + // Let's explicitly reset the parameters, so we won't cover old + // values from the previous loop round. + parameters.reset(); + + BOOST_FOREACH(auto entry_item, library_entry->mapValue()) { + if (entry_item.first == "library") { + if (entry_item.second->getType() != Element::string) { + isc_throw(DhcpConfigError, "hooks library configuration" + " error: value of 'library' element is not a string" + " giving the path to a hooks library (" << + entry_item.second->getPosition() << ")"); + } + + // Get the name of the library and add it to the list after + // removing quotes. + libname = (entry_item.second)->stringValue(); + + // Remove leading/trailing quotes and any leading/trailing + // spaces. + boost::erase_all(libname, "\""); + libname = isc::util::str::trim(libname); + if (libname.empty()) { + isc_throw(DhcpConfigError, "hooks library configuration" + " error: value of 'library' element must not be" + " blank (" << + entry_item.second->getPosition() << ")"); + } + + // Note we have found the library name. + lib_found = true; + continue; + } + + // If there are parameters, let's remember them. + if (entry_item.first == "parameters") { + parameters = entry_item.second; + continue; + } + + // For all other parameters we will throw. + isc_throw(DhcpConfigError, "unknown hooks library parameter: " + << entry_item.first << "(" + << library_entry->getPosition() << ")"); + } + + if (! lib_found) { + isc_throw(DhcpConfigError, "hooks library configuration error:" + " one or more hooks-libraries elements are missing the" + " name of the library" << + " (" << library_entry->getPosition() << ")"); + } + + libraries_.push_back(make_pair(libname, parameters)); + } +} + +void HooksLibrariesParser::verifyLibraries() { + // Check if the list of libraries has changed. If not, nothing is done + // - the command "DhcpN libreload" is required to reload the same + // libraries (this prevents needless reloads when anything else in the + // configuration is changed). + + // We no longer rely on this. Parameters can change. And even if the + // parameters stay the same, they could point to files that could + // change. + vector current_libraries = HooksManager::getLibraryNames(); + if (current_libraries.empty() && libraries_.empty()) { + return; + } + + // Library list has changed, validate each of the libraries specified. + vector lib_names = isc::hooks::extractNames(libraries_); + vector error_libs = HooksManager::validateLibraries(lib_names); + if (!error_libs.empty()) { + + // Construct the list of libraries in error for the message. + string error_list = error_libs[0]; + for (size_t i = 1; i < error_libs.size(); ++i) { + error_list += (string(", ") + error_libs[i]); + } + isc_throw(DhcpConfigError, "hooks libraries failed to validate - " + "library or libraries in error are: " << error_list + << "(" << position_ << ")"); + } +} + +void +HooksLibrariesParser::loadLibraries() { + /// Commits the list of libraries to the configuration manager storage if + /// the list of libraries has changed. + /// @todo: Delete any stored CalloutHandles before reloading the + /// libraries + if (!HooksManager::loadLibraries(libraries_)) { + isc_throw(DhcpConfigError, "One or more hook libraries failed to load"); + } +} + +// Method for testing +void +HooksLibrariesParser::getLibraries(isc::hooks::HookLibsCollection& libraries) { + libraries = libraries_; +} diff --git a/src/lib/hooks/hooks_parser.h b/src/lib/hooks/hooks_parser.h new file mode 100644 index 0000000000..62d405c41a --- /dev/null +++ b/src/lib/hooks/hooks_parser.h @@ -0,0 +1,121 @@ +// Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef HOOKS_PARSER_H +#define HOOKS_PARSER_H + +#include +#include +#include + +namespace isc { +namespace hooks { + +/// @brief Parser for hooks library list +/// +/// This parser handles the list of hooks libraries. This is an optional list, +/// which may be empty. +/// +/// However, the parser does more than just check the list of library names. +/// It does two other things: +/// +/// -# The problem faced with the hooks libraries is that we wish to avoid +/// reloading the libraries if they have not changed. (This would cause the +/// "unload" and "load" methods to run. Although libraries should be written +/// to cope with this, it is feasible that such an action may be costly in +/// terms of time and resources, or may cause side effects such as clearing +/// an internal cache.) To this end, the parser also checks the list against +/// the list of libraries current loaded and notes if there are changes. +/// -# If there are, the parser validates the libraries; it opens them and +/// checks that the "version" function exists and returns the correct value. +/// +/// Only if the library list has changed and the libraries are valid will the +/// change be applied. +class HooksLibrariesParser : public isc::data::SimpleParser { +public: + + /// @brief Parses parameters value + /// + /// Parses configuration entry (list of parameters) and adds each element + /// to the hooks libraries list. The method also checks whether the + /// list of libraries is the same as that already loaded. If not, it + /// checks each of the libraries in the list for validity (they exist and + /// have a "version" function that returns the correct value). + /// + /// The syntax for specifying hooks libraries allow for library-specific + /// parameters to be specified along with the library, e.g. + /// + /// @code + /// "hooks-libraries": [ + /// { + /// "library": "hook-lib-1.so", + /// "parameters": { + /// "alpha": "a string", + /// "beta": 42 + /// } + /// }, + /// : + /// ] + /// @endcode + /// + /// The parsing code only checks that: + /// + /// -# Each element in the hooks-libraries list is a map + /// -# The map contains an element "library" whose value is a not blank string + /// -# That there is an optional 'parameters' element. + /// -# That there are no other element. + /// + /// If you want to check whether the library is really present (if the file + /// is on disk, it is really a library and that it could be loaded), call + /// @ref verifyLibraries(). + /// + /// This method stores parsed libraries in libraries_. + /// + /// @param value pointer to the content of parsed values + void parse(isc::data::ConstElementPtr value); + + /// @brief Verifies that libraries stored in libraries_ are valid. + /// + /// This method is a smart wrapper around @ref + /// isc::hooks::HooksManager::validateLibraries(). + /// It tries to validate all the libraries stored in libraries_. + /// @throw DhcpConfigError if any issue is discovered. + void verifyLibraries(); + + /// @brief Commits hooks libraries data + /// + /// This method calls necessary methods in HooksManager that will unload + /// any libraries that may be currently loaded and will load the actual + /// libraries. Providing that the specified libraries are valid and are + /// different to those already loaded, this method loads the new set of + /// libraries (and unloads the existing set). + /// + /// @throw DhcpConfigError if the call to HooksManager fails. + void loadLibraries(); + + /// @brief Returns list of parsed libraries + /// + /// Principally for testing, this returns the list of libraries as well as + /// an indication as to whether the list is different from the list of + /// libraries already loaded. + /// + /// @param [out] libraries List of libraries that were specified in the + /// new configuration. + void getLibraries(isc::hooks::HookLibsCollection& libraries); + +private: + /// List of hooks libraries with their configuration parameters + isc::hooks::HookLibsCollection libraries_; + + /// Position of the original element is stored in case we need to report an + /// error later. + isc::data::Element::Position position_; +}; + +}; // namespace isc::hooks +}; // namespace isc + +#endif