[5134] HooksLibrariesParser moved to lib/hooks

src/bin/dhcp4/json_config_parser.cc

@@ -22,6 +22,7 @@
 #include <dhcpsrv/parsers/host_reservations_list_parser.h>
 #include <dhcpsrv/parsers/ifaces_config_parser.h>
 #include <dhcpsrv/timer_mgr.h>
+#include <hooks/hooks_parser.h>
 #include <config/command_mgr.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
@@ -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;

src/bin/dhcp6/json_config_parser.cc

@@ -30,6 +30,7 @@
 #include <dhcpsrv/parsers/host_reservation_parser.h>
 #include <dhcpsrv/parsers/host_reservations_list_parser.h>
 #include <dhcpsrv/parsers/ifaces_config_parser.h>
+#include <hooks/hooks_parser.h>
 #include <log/logger_support.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
@@ -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

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

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -12,7 +12,6 @@
 #include <dhcpsrv/cfg_option.h>
 #include <dhcpsrv/parsers/dhcp_parsers.h>
 #include <dhcpsrv/cfg_mac_source.h>
-#include <hooks/hooks_manager.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
 
@@ -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<string> current_libraries = HooksManager::getLibraryNames();
-    if (current_libraries.empty() && libraries_.empty()) {
-        return;
-    }
-
-    // Library list has changed, validate each of the libraries specified.
-    vector<string> lib_names = isc::hooks::extractNames(libraries_);
-    vector<string> 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) {

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -20,7 +20,6 @@
 #include <dhcpsrv/srv_config.h>
 #include <dhcpsrv/parsers/dhcp_config_parser.h>
 #include <cc/simple_parser.h>
-#include <hooks/libinfo.h>
 #include <exceptions/exceptions.h>
 #include <util/optional_value.h>
 
@@ -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.
 ///

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -18,6 +18,7 @@
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/cfg_mac_source.h>
 #include <dhcpsrv/parsers/dhcp_parsers.h>
+#include <hooks/hooks_parser.h>
 #include <dhcpsrv/tests/test_libraries.h>
 #include <dhcpsrv/testutils/config_result_check.h>
 #include <exceptions/exceptions.h>

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

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 <config.h>
+
+#include <cc/data.h>
+#include <cc/dhcp_config_error.h>
+#include <hooks/hooks_parser.h>
+#include <hooks/hooks_manager.h>
+#include <boost/algorithm/string.hpp>
+#include <boost/foreach.hpp>
+#include <util/strutil.h>
+#include <vector>
+
+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<string> current_libraries = HooksManager::getLibraryNames();
+    if (current_libraries.empty() && libraries_.empty()) {
+        return;
+    }
+
+    // Library list has changed, validate each of the libraries specified.
+    vector<string> lib_names = isc::hooks::extractNames(libraries_);
+    vector<string> 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_;
+}

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 <cc/data.h>
+#include <cc/simple_parser.h>
+#include <hooks/libinfo.h>
+
+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