[master] Merge branch 'trac5134_rebase'

# Conflicts:
#	src/bin/agent/Makefile.am
#	src/bin/agent/ca_cfg_mgr.cc
#	src/bin/agent/tests/.gitignore
#	src/bin/agent/tests/Makefile.am
#	src/bin/agent/tests/ca_cfg_mgr_unittests.cc
#	src/lib/process/tests/d_cfg_mgr_unittests.cc

configure.ac

@@ -1638,6 +1638,7 @@ AC_CONFIG_FILES([compatcheck/Makefile
                  src/bin/admin/tests/mysql_tests.sh
                  src/bin/admin/tests/pgsql_tests.sh
                  src/bin/admin/tests/cql_tests.sh
+                 src/bin/agent/tests/test_libraries.h
                  src/hooks/Makefile
                  src/hooks/dhcp/Makefile
                  src/hooks/dhcp/user_chk/Makefile

src/bin/agent/Makefile.am

@@ -51,6 +51,7 @@ libagent_la_SOURCES += ca_log.cc ca_log.h
 libagent_la_SOURCES += ca_process.cc ca_process.h
 libagent_la_SOURCES += ca_response_creator.cc ca_response_creator.h
 libagent_la_SOURCES += ca_response_creator_factory.h
+libagent_la_SOURCES += simple_parser.cc simple_parser.h
 libagent_la_SOURCES += parser_context.cc parser_context.h parser_context_decl.h
 libagent_la_SOURCES += agent_lexer.ll location.hh position.hh stack.hh
 

src/bin/agent/ca_cfg_mgr.cc

@@ -6,13 +6,35 @@
 
 #include <config.h>
 #include <agent/ca_cfg_mgr.h>
+#include <agent/ca_log.h>
+#include <agent/simple_parser.h>
+#include <cc/simple_parser.h>
+#include <cc/command_interpreter.h>
 
 using namespace isc::dhcp;
 using namespace isc::process;
+using namespace isc::data;
 
 namespace isc {
 namespace agent {
 
+CtrlAgentCfgContext::CtrlAgentCfgContext()
+    :http_host_(""), http_port_(0) {
+}
+
+CtrlAgentCfgContext::CtrlAgentCfgContext(const CtrlAgentCfgContext& orig)
+    : DCfgContextBase(),http_host_(orig.http_host_), http_port_(orig.http_port_),
+      libraries_(orig.libraries_) {
+
+    // We're copying pointers here only. The underlying data will be shared by
+    // old and new context. That's how shared pointers work and I see no reason
+    // why it would be different in this particular here.
+    ctrl_sockets_[TYPE_D2] = orig.ctrl_sockets_[TYPE_D2];
+    ctrl_sockets_[TYPE_DHCP4] = orig.ctrl_sockets_[TYPE_DHCP4];
+    ctrl_sockets_[TYPE_DHCP6] = orig.ctrl_sockets_[TYPE_DHCP6];
+}
+
+
 CtrlAgentCfgMgr::CtrlAgentCfgMgr()
     : DCfgMgrBase(DCfgContextBasePtr(new CtrlAgentCfgContext())) {
 }
@@ -22,16 +44,49 @@ CtrlAgentCfgMgr::~CtrlAgentCfgMgr() {
 
 std::string
 CtrlAgentCfgMgr::getConfigSummary(const uint32_t /*selection*/) {
-    return ("Control Agent is currently not configurable.");
+
+    CtrlAgentCfgContextPtr ctx = getCtrlAgentCfgContext();
+
+    // First print the http stuff.
+    std::ostringstream s;
+    s << "listening on " << ctx->getHttpHost() << ", port "
+      << ctx->getHttpPort() << ", control sockets: ";
+
+    // Then print the control-sockets
+    bool socks = false;
+    if (ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2)) {
+        s << "d2 ";
+        socks = true;
+    }
+    if (ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4)) {
+        s << "dhcp4 ";
+        socks = true;
+    }
+    if (ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6)) {
+        s << "dhcp6 ";
+        socks = true;
+    }
+    if (!socks) {
+        // That's uncommon, but correct scenario. CA can respond to some
+        // commands on its own. Further down the road we will possibly get the
+        // capability to tell CA to start other servers.
+        s << "none";
+    }
+
+    // Finally, print the hook libraries names
+    const hooks::HookLibsCollection libs = ctx->getLibraries();
+    s << ", " << libs.size() << " lib(s):";
+    for (auto lib = libs.begin(); lib != libs.end(); ++lib) {
+        s << lib->first << " ";
+    }
+
+    return (s.str());
 }
 
 isc::dhcp::ParserPtr
-CtrlAgentCfgMgr::createConfigParser(const std::string& element_id,
+CtrlAgentCfgMgr::createConfigParser(const std::string& /*element_id*/,
                                     const isc::data::Element::Position& /*pos*/) {
-    // Create dummy parser, so as we don't return null pointer.
-    isc::dhcp::ParserPtr parser;
-    parser.reset(new Uint32Parser(element_id, getContext()->getUint32Storage()));
-    return (parser);
+    isc_throw(NotImplemented, "We don't use parser pointers anymore");
 }
 
 DCfgContextBasePtr
@@ -39,5 +94,70 @@ CtrlAgentCfgMgr::createNewContext() {
     return (DCfgContextBasePtr(new CtrlAgentCfgContext()));
 }
 
+isc::data::ConstElementPtr
+CtrlAgentCfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) {
+    // Do a sanity check first.
+    if (!config_set) {
+        isc_throw(DhcpConfigError, "Mandatory config parameter not provided");
+    }
+
+    CtrlAgentCfgContextPtr ctx = getCtrlAgentCfgContext();
+
+    // Set the defaults
+    ElementPtr cfg = boost::const_pointer_cast<Element>(config_set);
+    AgentSimpleParser::setAllDefaults(cfg);
+
+    // And parse the configuration.
+    ConstElementPtr answer;
+    std::string excuse;
+    try {
+        // Do the actual parsing
+        AgentSimpleParser parser;
+        parser.parse(ctx, cfg, check_only);
+    } catch (const isc::Exception& ex) {
+        excuse = ex.what();
+        answer = isc::config::createAnswer(2, excuse);
+    } catch (...) {
+        excuse = "undefined configuration parsing error";
+        answer = isc::config::createAnswer(2, excuse);
+    }
+
+    // At this stage the answer was created only in case of exception.
+    if (answer) {
+        if (check_only) {
+            LOG_ERROR(agent_logger, CTRL_AGENT_CONFIG_CHECK_FAIL).arg(excuse);
+        } else {
+            LOG_ERROR(agent_logger, CTRL_AGENT_CONFIG_FAIL).arg(excuse);
+        }
+        return (answer);
+    }
+
+    if (check_only) {
+        answer = isc::config::createAnswer(0, "Configuration check successful");
+    } else {
+        answer = isc::config::createAnswer(0, "Configuration applied successfully.");
+    }
+
+    return (answer);
+}
+
+const data::ConstElementPtr
+CtrlAgentCfgContext::getControlSocketInfo(ServerType type) const {
+    if (type > MAX_TYPE_SUPPORTED) {
+        isc_throw(BadValue, "Invalid server type");
+    }
+    return (ctrl_sockets_[static_cast<uint8_t>(type)]);
+}
+
+void
+CtrlAgentCfgContext::setControlSocketInfo(const isc::data::ConstElementPtr& control_socket,
+                                          ServerType type) {
+    if (type > MAX_TYPE_SUPPORTED) {
+        isc_throw(BadValue, "Invalid server type");
+    }
+    ctrl_sockets_[static_cast<uint8_t>(type)] = control_socket;
+}
+
+
 } // namespace isc::agent
 } // namespace isc

src/bin/agent/ca_cfg_mgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-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
@@ -7,8 +7,10 @@
 #ifndef CTRL_AGENT_CFG_MGR_H
 #define CTRL_AGENT_CFG_MGR_H
 
+#include <cc/data.h>
 #include <process/d_cfg_mgr.h>
 #include <boost/pointer_cast.hpp>
+#include <hooks/libinfo.h>
 
 namespace isc {
 namespace agent {
@@ -26,16 +28,120 @@ typedef boost::shared_ptr<CtrlAgentCfgContext> CtrlAgentCfgContextPtr;
 /// It is derived from the context base class, DCfgContextBase.
 class CtrlAgentCfgContext : public process::DCfgContextBase {
 public:
+
+    /// @brief Default constructor
+    CtrlAgentCfgContext();
+
+    /// @brief Specifies type of the server being controlled.
+    enum ServerType {
+        TYPE_DHCP4 = 0, ///< kea-dhcp4
+        TYPE_DHCP6 = 1, ///< kea-dhcp6
+        TYPE_D2 = 2     ///< kea-dhcp-ddns
+    };
+
+    /// @brief Used check that specified ServerType is within valid range.
+    static const uint32_t MAX_TYPE_SUPPORTED = TYPE_D2;
+
     /// @brief Creates a clone of this context object.
     ///
+    /// Note this method does not do deep copy the information about control sockets.
+    /// That data is stored as ConstElementPtr (a shared pointer) to the actual data.
+    ///
     /// @return A pointer to the new clone.
     virtual process::DCfgContextBasePtr clone() {
         return (process::DCfgContextBasePtr(new CtrlAgentCfgContext(*this)));
     }
 
+    /// @brief Returns information about control socket
+    ///
+    /// This method returns Element tree structure that describes the control
+    /// socket (or null pointer if the socket is not defined for a particular
+    /// server type). This information is expected to be compatible with
+    /// data passed to @ref isc::config::CommandMgr::openCommandSocket.
+    ///
+    /// @param type type of the server being controlled
+    /// @return pointer to the Element that holds control-socket map (or NULL)
+    const data::ConstElementPtr getControlSocketInfo(ServerType type) const;
+
+    /// @brief Sets information about the control socket
+    ///
+    /// This method stores Element tree structure that describes the control
+    /// socket. This information is expected to be compatible with
+    /// data passed to @ref isc::config::CommandMgr::openCommandSocket.
+    ///
+    /// @param control_socket Element that holds control-socket map
+    /// @param type type of the server being controlled
+    void setControlSocketInfo(const isc::data::ConstElementPtr& control_socket,
+                              ServerType type);
+
+    /// @brief Sets http-host parameter
+    ///
+    /// @param host Hostname or IP address where the agent's HTTP service
+    /// will be available.
+    void setHttpHost(const std::string& host) {
+        http_host_ = host;
+    }
+
+    /// @brief Returns http-host parameter
+    ///
+    /// @return Hostname or IP address where the agent's HTTP service is
+    /// available.
+    std::string getHttpHost() const {
+        return (http_host_);
+    }
+
+    /// @brief Sets http port
+    ///
+    /// @param port sets the TCP port the HTTP server will listen on
+    void setHttpPort(const uint16_t port) {
+        http_port_ = port;
+    }
+
+    /// @brief Returns the TCP post the HTTP server will listen on
+    uint16_t getHttpPort() const {
+        return (http_port_);
+    }
+
+    /// @brief Returns a list of hook libraries
+    /// @return a list of hook libraries
+    const hooks::HookLibsCollection& getLibraries() const {
+        return (libraries_);
+    }
+
+    /// @brief Sets the list of hook libraries
+    ///
+    /// @params libs a coolection of libraries to remember.
+    void setLibraries(const hooks::HookLibsCollection& libs) {
+        libraries_ = libs;
+    }
+
+
 private:
+
+    /// @brief Private copy constructor
+    ///
+    /// It is private to forbid anyone outside of this class to make copies.
+    /// The only legal way to copy a context is to call @ref clone().
+    ///
+    /// @param orig the original context to copy from
+    CtrlAgentCfgContext(const CtrlAgentCfgContext& orig);
+
     /// @brief Private assignment operator to avoid potential for slicing.
+    ///
+    /// @param rhs Context to be assigned.
     CtrlAgentCfgContext& operator=(const CtrlAgentCfgContext& rhs);
+
+    /// Socket information will be stored here (for all supported servers)
+    data::ConstElementPtr ctrl_sockets_[MAX_TYPE_SUPPORTED + 1];
+
+    /// Hostname the CA should listen on.
+    std::string http_host_;
+
+    /// TCP port the CA should listen on.
+    uint16_t http_port_;
+
+    /// List of hook libraries.
+    hooks::HookLibsCollection libraries_;
 };
 
 /// @brief Ctrl Agent Configuration Manager.
@@ -69,20 +175,22 @@ public:
 
 protected:
 
-    /// @brief  Create a parser instance based on an element id.
-    ///
-    /// Given an element_id returns an instance of the appropriate parser.
+    /// @brief Parses configuration of the Control Agent.
     ///
-    /// @param element_id is the string name of the element as it will appear
-    /// in the configuration set.
-    /// @param pos position within the configuration text (or file) of element
-    /// to be parsed.  This is passed for error messaging.
+    /// @param config Pointer to a configuration specified for the agent.
+    /// @param check_only Boolean flag indicating if this method should
+    /// only verify correctness of the provided conifiguration.
+    /// @return Pointer to a result of configuration parsing.
+    virtual isc::data::ConstElementPtr
+    parse(isc::data::ConstElementPtr config, bool check_only);
+
+    /// @brief This is no longer used.
     ///
-    /// @return returns a ParserPtr to the parser instance.
+    /// @throw NotImplemented
+    /// @return nothing, always throws
     virtual isc::dhcp::ParserPtr
-    createConfigParser(const std::string& element_id,
-                       const isc::data::Element::Position& pos
-                       = isc::data::Element::ZERO_POSITION());
+    createConfigParser(const std::string&,
+                       const isc::data::Element::Position& pos);
 
     /// @brief Creates a new, blank CtrlAgentCfgContext context.
     ///

src/bin/agent/ca_messages.mes

@@ -23,3 +23,13 @@ event loop.
 This informational message indicates that the DHCP-DDNS server has
 processed all configuration information and is ready to begin processing.
 The version is also printed.
+
+% CTRL_AGENT_CONFIG_FAIL Control Agent configuration failed: %1
+This error message indicates that the CA had failed configuration
+attempt. Details are provided. Additional details may be available
+in earlier log entries, possibly on lower levels.
+
+% CTRL_AGENT_CONFIG_CHECK_FAIL Control Agent configuration check failed: %1
+This error message indicates that the CA had failed configuration
+check. Details are provided. Additional details may be available
+in earlier log entries, possibly on lower levels.

src/bin/agent/ca_process.cc

@@ -93,9 +93,11 @@ CtrlAgentProcess::shutdown(isc::data::ConstElementPtr /*args*/) {
 }
 
 isc::data::ConstElementPtr
-CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set) {
+CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
+                            bool check_only) {
     int rcode = 0;
-    isc::data::ConstElementPtr answer = getCfgMgr()->parseConfig(config_set);
+    isc::data::ConstElementPtr answer = getCfgMgr()->simpleParseConfig(config_set,
+                                                                       check_only);
     config::parseAnswer(rcode, answer);
     return (answer);
 }

src/bin/agent/ca_process.h

@@ -79,11 +79,12 @@ public:
     /// below.
     ///
     /// @param config_set a new configuration (JSON) for the process
+    /// @param check_only true if configuration is to be verified only, not applied
     /// @return an Element that contains the results of configuration composed
     /// of an integer status value (0 means successful, non-zero means failure),
     /// and a string explanation of the outcome.
     virtual isc::data::ConstElementPtr
-    configure(isc::data::ConstElementPtr config_set);
+    configure(isc::data::ConstElementPtr config_set, bool check_only);
 
     /// @brief Processes the given command.
     ///

src/bin/agent/simple_parser.cc

@@ -0,0 +1,130 @@
+// 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 <agent/simple_parser.h>
+#include <cc/data.h>
+#include <cc/dhcp_config_error.h>
+#include <hooks/hooks_parser.h>
+#include <boost/foreach.hpp>
+
+using namespace isc::data;
+
+namespace isc {
+namespace agent {
+/// @brief This sets of arrays define the default values in various scopes
+///        of the Control Agent Configuration.
+///
+/// Each of those is documented in @file agent/simple_parser.cc. This
+/// is different than most other comments in Kea code. The reason
+/// for placing those in .cc rather than .h file is that it
+/// is expected to be one centralized place to look at for
+/// the default values. This is expected to be looked at also by
+/// people who are not skilled in C or C++, so they may be
+/// confused with the differences between declaration and definition.
+/// As such, there's one file to look at that hopefully is readable
+/// without any C or C++ skills.
+///
+/// @{
+
+/// @brief This table defines default values for global options.
+///
+/// These are global Control Agent parameters.
+const SimpleDefaults AgentSimpleParser::AGENT_DEFAULTS = {
+    { "http-host",    Element::string,  "localhost"},
+    { "http-port",    Element::integer,  "8000"}
+};
+
+/// @brief This table defines default values for control sockets.
+///
+const SimpleDefaults AgentSimpleParser::SOCKET_DEFAULTS = {
+    { "socket-type",  Element::string,  "unix"}
+};
+
+/// @}
+
+/// ---------------------------------------------------------------------------
+/// --- end of default values -------------------------------------------------
+/// ---------------------------------------------------------------------------
+
+size_t AgentSimpleParser::setAllDefaults(const isc::data::ElementPtr& global) {
+    size_t cnt = 0;
+
+    // Set global defaults first.
+    cnt = setDefaults(global, AGENT_DEFAULTS);
+
+    // Now set the defaults for control-sockets, if any.
+    ConstElementPtr sockets = global->get("control-sockets");
+    if (sockets) {
+        ElementPtr d2 = boost::const_pointer_cast<Element>(sockets->get("d2-server"));
+        if (d2) {
+            cnt += SimpleParser::setDefaults(d2, SOCKET_DEFAULTS);
+        }
+
+        ElementPtr d4 = boost::const_pointer_cast<Element>(sockets->get("dhcp4-server"));
+        if (d4) {
+            cnt += SimpleParser::setDefaults(d4, SOCKET_DEFAULTS);
+        }
+
+        ElementPtr d6 = boost::const_pointer_cast<Element>(sockets->get("dhcp6-server"));
+        if (d6) {
+            cnt += SimpleParser::setDefaults(d6, SOCKET_DEFAULTS);
+        }
+    }
+
+    return (cnt);
+}
+
+void
+AgentSimpleParser::parse(const CtrlAgentCfgContextPtr& ctx,
+                         const isc::data::ConstElementPtr& config,
+                         bool check_only) {
+
+    // Let's get the HTTP parameters first.
+    ctx->setHttpHost(SimpleParser::getString(config, "http-host"));
+    ctx->setHttpPort(SimpleParser::getIntType<uint16_t>(config, "http-port"));
+
+    // Control sockets are second.
+    ConstElementPtr ctrl_sockets = config->get("control-sockets");
+    if (ctrl_sockets) {
+        ConstElementPtr d2_socket = ctrl_sockets->get("d2-server");
+        ConstElementPtr d4_socket = ctrl_sockets->get("dhcp4-server");
+        ConstElementPtr d6_socket = ctrl_sockets->get("dhcp6-server");
+
+        if (d2_socket) {
+            ctx->setControlSocketInfo(d2_socket, CtrlAgentCfgContext::TYPE_D2);
+        }
+
+        if (d4_socket) {
+            ctx->setControlSocketInfo(d4_socket, CtrlAgentCfgContext::TYPE_DHCP4);
+        }
+
+        if (d6_socket) {
+            ctx->setControlSocketInfo(d6_socket, CtrlAgentCfgContext::TYPE_DHCP6);
+        }
+    }
+
+    // Finally, let's get the hook libs!
+    hooks::HooksLibrariesParser hooks_parser;
+    ConstElementPtr hooks = config->get("hooks-libraries");
+    if (hooks) {
+        hooks_parser.parse(hooks);
+        hooks_parser.verifyLibraries();
+
+        hooks::HookLibsCollection libs;
+        hooks_parser.getLibraries(libs);
+        ctx->setLibraries(libs);
+    }
+
+    if (!check_only) {
+        // This occurs last as if it succeeds, there is no easy way
+        // revert it.  As a result, the failure to commit a subsequent
+        // change causes problems when trying to roll back.
+        hooks_parser.loadLibraries();
+    }
+}
+
+};
+};

src/bin/agent/simple_parser.h

@@ -0,0 +1,51 @@
+// 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 AGENT_SIMPLE_PARSER_H
+#define AGENT_SIMPLE_PARSER_H
+
+#include <cc/simple_parser.h>
+#include <agent/ca_cfg_mgr.h>
+
+namespace isc {
+namespace agent {
+
+/// @brief SimpleParser specialized for Control Agent
+///
+/// This class is a @ref isc::data::SimpleParser dedicated to Control Agent.
+/// In particular, it contains all the default values for the whole
+/// agent and for the socket defaults.
+///
+/// For the actual values, see @file agent/simple_parser.cc
+class AgentSimpleParser : public isc::data::SimpleParser {
+public:
+    /// @brief Sets all defaults for Control Agent configuration
+    ///
+    /// This method sets global, option data and option definitions defaults.
+    ///
+    /// @param global scope to be filled in with defaults.
+    /// @return number of default values added
+    static size_t setAllDefaults(const isc::data::ElementPtr& global);
+
+    /// @brief Parses the control agent configuration
+    ///
+    /// @param ctx - parsed information will be stored here
+    /// @param config - Element tree structure that holds configuration
+    /// @param check_only - if true the configuration is verified only, not applied
+    ///
+    /// @throw ConfigError if any issues are encountered.
+    void parse(const CtrlAgentCfgContextPtr& ctx,
+               const isc::data::ConstElementPtr& config,
+               bool check_only);
+
+    // see simple_parser.cc for comments for those parameters
+    static const isc::data::SimpleDefaults AGENT_DEFAULTS;
+    static const isc::data::SimpleDefaults SOCKET_DEFAULTS;
+};
+
+};
+};
+#endif

src/bin/agent/tests/.gitignore

@@ -1,2 +1,3 @@
 /ca_unittests
 /ca_process_tests.sh
+/test_libraries.h

src/bin/agent/tests/Makefile.am

@@ -4,8 +4,8 @@ SHTESTS =
 SHTESTS += ca_process_tests.sh
 
 noinst_SCRIPTS = ca_process_tests.sh
-
 EXTRA_DIST  = ca_process_tests.sh.in
+noinst_LTLIBRARIES = libbasic.la
 
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
@@ -78,6 +78,17 @@ ca_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 ca_unittests_LDADD += $(LOG4CPLUS_LIBS) $(CRYPTO_LIBS)
 ca_unittests_LDADD += $(BOOST_LIBS) $(GTEST_LDADD)
 
+# The basic callout library - contains standard callouts
+libbasic_la_SOURCES  = basic_library.cc
+libbasic_la_CXXFLAGS = $(AM_CXXFLAGS)
+libbasic_la_CPPFLAGS = $(AM_CPPFLAGS)
+libbasic_la_LIBADD   = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
+libbasic_la_LIBADD  += $(top_builddir)/src/lib/hooks/libkea-hooks.la
+libbasic_la_LIBADD  += $(top_builddir)/src/lib/log/libkea-log.la
+libbasic_la_LDFLAGS  = -avoid-version -export-dynamic -module -rpath /nowhere
+
+nodist_run_unittests_SOURCES = test_libraries.h
+
 endif
 
 noinst_PROGRAMS = $(TESTS)

src/bin/agent/tests/basic_library.cc

@@ -0,0 +1,72 @@
+// 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/.
+
+/// @file
+/// @brief Basic callout library
+///
+/// This is source of a test library for Control Agent.
+///
+/// - Only the "version" framework function is supplied.
+///
+/// - hookpt_one callout is supplied.
+
+#include <config.h>
+#include <hooks/hooks.h>
+
+using namespace isc::hooks;
+using namespace std;
+
+namespace {
+
+extern "C" {
+
+// Callouts.  All return their result through the "result" argument.
+
+int
+context_create(CalloutHandle& handle) {
+    handle.setContext("result", static_cast<int>(10));
+    handle.setArgument("result", static_cast<int>(10));
+    return (0);
+}
+
+// First callout adds the passed "integer" argument to the initialized context
+// value of 10. (Note that the value set by context_create is accessed through
+// context and not the argument, so checking that context is correctly passed
+// between callouts in the same library.)
+
+int
+hookpt_one(CalloutHandle& handle) {
+    int data;
+    handle.getArgument("integer", data);
+
+    int result;
+    handle.getArgument("result", result);
+
+    result += data;
+    handle.setArgument("result", result);
+
+    return (0);
+}
+
+// Framework functions.
+
+int
+version() {
+    return (KEA_HOOKS_VERSION);
+}
+
+// load() initializes the user library if the main image was statically linked.
+int
+load(isc::hooks::LibraryHandle&) {
+#ifdef USE_STATIC_LINK
+    hooksStaticLinkInit();
+#endif
+    return (0);
+}
+
+}
+}
+

src/bin/agent/tests/ca_cfg_mgr_unittests.cc

@@ -6,14 +6,27 @@
 
 #include <config.h>
 #include <agent/ca_cfg_mgr.h>
+#include <agent/parser_context.h>
 #include <process/testutils/d_test_stubs.h>
+#include <process/d_cfg_mgr.h>
+#include <agent/tests/test_libraries.h>
+#include <hooks/libinfo.h>
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
 
 using namespace isc::agent;
+using namespace isc::data;
+using namespace isc::hooks;
+using namespace isc::process;
 
 namespace  {
 
+/// @brief Almost regular agent CfgMgr with internal parse method exposed.
+class NakedAgentCfgMgr : public CtrlAgentCfgMgr {
+public:
+    using CtrlAgentCfgMgr::parse;
+};
+
 // Tests construction of CtrlAgentCfgMgr class.
 TEST(CtrlAgentCfgMgr, construction) {
     boost::scoped_ptr<CtrlAgentCfgMgr> cfg_mgr;
@@ -30,4 +43,358 @@ TEST(CtrlAgentCfgMgr, construction) {
     EXPECT_NO_THROW(cfg_mgr.reset());
 }
 
+// Tests if getContext can be retrieved.
+TEST(CtrlAgentCfgMgr, getContext) {
+    CtrlAgentCfgMgr cfg_mgr;
+
+    CtrlAgentCfgContextPtr ctx;
+    ASSERT_NO_THROW(ctx = cfg_mgr.getCtrlAgentCfgContext());
+    ASSERT_TRUE(ctx);
+}
+
+// Tests if context can store and retrieve HTTP parameters
+TEST(CtrlAgentCfgMgr, contextHttpParams) {
+    CtrlAgentCfgContext ctx;
+
+    // Check http parameters
+    ctx.setHttpPort(12345);
+    EXPECT_EQ(12345, ctx.getHttpPort());
+
+    ctx.setHttpHost("alnitak");
+    EXPECT_EQ("alnitak", ctx.getHttpHost());
+}
+
+// Tests if context can store and retrieve control socket information.
+TEST(CtrlAgentCfgMgr, contextSocketInfo) {
+
+    CtrlAgentCfgContext ctx;
+
+    // Check control socket parameters
+    // By default, there are no control sockets stored.
+    EXPECT_FALSE(ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2));
+    EXPECT_FALSE(ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4));
+    EXPECT_FALSE(ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6));
+
+    ConstElementPtr socket1 = Element::fromJSON("{ \"socket-type\": \"unix\",\n"
+                                                "  \"socket-name\": \"socket1\" }");
+    ConstElementPtr socket2 = Element::fromJSON("{ \"socket-type\": \"unix\",\n"
+                                                "  \"socket-name\": \"socket2\" }");
+    ConstElementPtr socket3 = Element::fromJSON("{ \"socket-type\": \"unix\",\n"
+                                                "  \"socket-name\": \"socket3\" }");
+    // Ok, now set the control socket for D2
+    EXPECT_NO_THROW(ctx.setControlSocketInfo(socket1, CtrlAgentCfgContext::TYPE_D2));
+
+    // Now check the values returned
+    EXPECT_EQ(socket1, ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2));
+    EXPECT_FALSE(ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4));
+    EXPECT_FALSE(ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6));
+
+    // Now set the v6 socket and sanity check again
+    EXPECT_NO_THROW(ctx.setControlSocketInfo(socket2, CtrlAgentCfgContext::TYPE_DHCP6));
+
+    // Should be possible to retrieve two sockets.
+    EXPECT_EQ(socket1, ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2));
+    EXPECT_EQ(socket2, ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6));
+    EXPECT_FALSE(ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4));
+
+    // Finally, set the third control socket.
+    EXPECT_NO_THROW(ctx.setControlSocketInfo(socket3, CtrlAgentCfgContext::TYPE_DHCP4));
+    EXPECT_EQ(socket1, ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2));
+    EXPECT_EQ(socket2, ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6));
+    EXPECT_EQ(socket3, ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4));
+}
+
+// Tests if copied context retains all parameters.
+TEST(CtrlAgentCfgMgr, contextSocketInfoCopy) {
+
+    CtrlAgentCfgContext ctx;
+
+    ConstElementPtr socket1 = Element::fromJSON("{ \"socket-type\": \"unix\",\n"
+                                                "  \"socket-name\": \"socket1\" }");
+    ConstElementPtr socket2 = Element::fromJSON("{ \"socket-type\": \"unix\",\n"
+                                                "  \"socket-name\": \"socket2\" }");
+    ConstElementPtr socket3 = Element::fromJSON("{ \"socket-type\": \"unix\",\n"
+                                                "  \"socket-name\": \"socket3\" }");
+    // Ok, now set the control sockets
+    EXPECT_NO_THROW(ctx.setControlSocketInfo(socket1, CtrlAgentCfgContext::TYPE_D2));
+    EXPECT_NO_THROW(ctx.setControlSocketInfo(socket2, CtrlAgentCfgContext::TYPE_DHCP4));
+    EXPECT_NO_THROW(ctx.setControlSocketInfo(socket3, CtrlAgentCfgContext::TYPE_DHCP6));
+
+    EXPECT_NO_THROW(ctx.setHttpPort(12345));
+    EXPECT_NO_THROW(ctx.setHttpHost("bellatrix"));
+
+    HookLibsCollection libs;
+    string exp_name("testlib1.so");
+    ConstElementPtr exp_param(new StringElement("myparam"));
+    libs.push_back(make_pair(exp_name, exp_param));
+    ctx.setLibraries(libs);
+
+    // Make a copy.
+    DCfgContextBasePtr copy_base(ctx.clone());
+    CtrlAgentCfgContextPtr copy = boost::dynamic_pointer_cast<CtrlAgentCfgContext>(copy_base);
+    ASSERT_TRUE(copy);
+
+    // Now check the values returned
+    EXPECT_EQ(12345, copy->getHttpPort());
+    EXPECT_EQ("bellatrix", copy->getHttpHost());
+
+    // Check socket info
+    ASSERT_TRUE(copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2));
+    ASSERT_TRUE(copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4));
+    ASSERT_TRUE(copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6));
+    EXPECT_EQ(socket1->str(), copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2)->str());
+    EXPECT_EQ(socket2->str(), copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4)->str());
+    EXPECT_EQ(socket3->str(), copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6)->str());
+
+    // Check hook libs
+    HookLibsCollection libs2 = copy->getLibraries();
+    ASSERT_EQ(1, libs2.size());
+    EXPECT_EQ(exp_name, libs2[0].first);
+    ASSERT_TRUE(libs2[0].second);
+    EXPECT_EQ(exp_param->str(), libs2[0].second->str());
+}
+
+
+// Tests if the context can store and retrieve hook libs information.
+TEST(CtrlAgentCfgMgr, contextHookParams) {
+    CtrlAgentCfgContext ctx;
+
+    // By default there should be no hooks.
+    HookLibsCollection libs = ctx.getLibraries();
+    EXPECT_TRUE(libs.empty());
+
+    libs.push_back(std::make_pair("libone.so", ConstElementPtr()));
+    libs.push_back(std::make_pair("libtwo.so", Element::fromJSON("{\"foo\": true}")));
+    libs.push_back(std::make_pair("libthree.so", Element::fromJSON("{\"bar\": 42}")));
+
+    ctx.setLibraries(libs);
+
+    HookLibsCollection stored_libs = ctx.getLibraries();
+    EXPECT_EQ(3, stored_libs.size());
+
+    EXPECT_EQ(libs, stored_libs);
+}
+
+/// Control Agent configurations used in tests.
+const char* AGENT_CONFIGS[] = {
+
+    // configuration 0: empty (nothing specified)
+    "{ }",
+
+    // Configuration 1: http parameters only (no control sockets, not hooks)
+    "{  \"http-host\": \"betelguese\",\n"
+    "    \"http-port\": 8001\n"
+    "}",
+
+    // Configuration 2: http and 1 socket
+    "{\n"
+    "    \"http-host\": \"betelguese\",\n"
+    "    \"http-port\": 8001,\n"
+    "    \"control-sockets\": {\n"
+    "        \"dhcp4-server\": {\n"
+    "            \"socket-name\": \"/tmp/socket-v4\"\n"
+    "        }\n"
+    "    }\n"
+    "}",
+
+    // Configuration 3: http and all 3 sockets
+    "{\n"
+    "    \"http-host\": \"betelguese\",\n"
+    "    \"http-port\": 8001,\n"
+    "    \"control-sockets\": {\n"
+    "        \"dhcp4-server\": {\n"
+    "            \"socket-name\": \"/tmp/socket-v4\"\n"
+    "        },\n"
+    "        \"dhcp6-server\": {\n"
+    "            \"socket-name\": \"/tmp/socket-v6\"\n"
+    "        },\n"
+    "        \"d2-server\": {\n"
+    "            \"socket-name\": \"/tmp/socket-d2\"\n"
+    "        }\n"
+    "   }\n"
+    "}",
+
+    // Configuration 4: http, 1 socket and hooks
+    // CA is able to load hook libraries that augment its operation.
+    // The primary functionality is the ability to add new commands.
+    "{\n"
+    "    \"http-host\": \"betelguese\",\n"
+    "    \"http-port\": 8001,\n"
+    "    \"control-sockets\": {\n"
+    "        \"dhcp4-server\": {\n"
+    "            \"socket-name\": \"/tmp/socket-v4\"\n"
+    "        }\n"
+    "   },\n"
+    "    \"hooks-libraries\": ["
+    "        {"
+    "          \"library\": \"%LIBRARY%\","
+    "              \"parameters\": {\n"
+    "              \"param1\": \"foo\"\n"
+    "            }\n"
+    "        }\n"
+    "     ]\n"
+    "}",
+
+    // Configuration 5: http and 1 socket (d2 only)
+    "{\n"
+    "    \"http-host\": \"betelguese\",\n"
+    "    \"http-port\": 8001,\n"
+    "    \"control-sockets\": {\n"
+    "        \"d2-server\": {\n"
+    "            \"socket-name\": \"/tmp/socket-d2\"\n"
+    "        }\n"
+    "    }\n"
+    "}",
+
+    // Configuration 6: http and 1 socket (dhcp6 only)
+    "{\n"
+    "    \"http-host\": \"betelguese\",\n"
+    "    \"http-port\": 8001,\n"
+    "    \"control-sockets\": {\n"
+    "        \"dhcp6-server\": {\n"
+    "            \"socket-name\": \"/tmp/socket-v6\"\n"
+    "        }\n"
+    "    }\n"
+    "}"
+};
+
+/// @brief Class used for testing CfgMgr
+class AgentParserTest : public isc::process::ConfigParseTest {
+public:
+
+    /// @brief Tries to load input text as a configuration
+    ///
+    /// @param config text containing input configuration
+    /// @param expected_answer expected result of configuration (0 = success)
+    void configParse(const char* config, int expected_answer) {
+        isc::agent::ParserContext parser;
+        ConstElementPtr json = parser.parseString(config, ParserContext::PARSER_SUB_AGENT);
+
+        EXPECT_NO_THROW(answer_ = cfg_mgr_.parse(json, false));
+        EXPECT_TRUE(checkAnswer(expected_answer));
+    }
+
+    /// @brief Replaces %LIBRARY% with specified library name
+    ///
+    /// @param config input config text (should contain "%LIBRARY%" string)
+    /// @param lib_name %LIBRARY% will be replaced with that name
+    /// @return configuration text with library name replaced
+    std::string pathReplacer(const char* config, const char* lib_name) {
+        string txt(config);
+        txt.replace(txt.find("%LIBRARY%"), strlen("%LIBRARY%"), string(lib_name));
+        return (txt);
+    }
+
+    /// Configuration Manager (used in tests)
+    NakedAgentCfgMgr cfg_mgr_;
+};
+
+// This test verifies if an empty config is handled properly. In practice such
+// a config makes little sense, but perhaps it's ok for a default deployment.
+// Sadly, our bison parser requires at last one parameter to be present.
+// Until we determine whether we want the empty config to be allowed or not,
+// this test remains disabled.
+TEST_F(AgentParserTest, DISABLED_configParseEmpty) {
+    configParse(AGENT_CONFIGS[0], 0);
+}
+
+// This test checks if a config with only HTTP parameters is parsed properly.
+TEST_F(AgentParserTest, configParseHttpOnly) {
+    configParse(AGENT_CONFIGS[1], 0);
+
+    CtrlAgentCfgContextPtr ctx = cfg_mgr_.getCtrlAgentCfgContext();
+    ASSERT_TRUE(ctx);
+    EXPECT_EQ("betelguese", ctx->getHttpHost());
+    EXPECT_EQ(8001, ctx->getHttpPort());
+}
+
+// Tests if a single socket can be configured. BTW this test also checks
+// if default value for socket-type is specified (the config doesn't have it,
+// so the default value should be filed in).
+TEST_F(AgentParserTest, configParseSocketDhcp4) {
+    configParse(AGENT_CONFIGS[2], 0);
+
+    CtrlAgentCfgContextPtr ctx = cfg_mgr_.getCtrlAgentCfgContext();
+    ASSERT_TRUE(ctx);
+    ConstElementPtr socket = ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4);
+    ASSERT_TRUE(socket);
+    EXPECT_EQ("{ \"socket-name\": \"/tmp/socket-v4\", \"socket-type\": \"unix\" }",
+              socket->str());
+    EXPECT_FALSE(ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6));
+    EXPECT_FALSE(ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2));
 }
+
+// Tests if a single socket can be configured. BTW this test also checks
+// if default value for socket-type is specified (the config doesn't have it,
+// so the default value should be filed in).
+TEST_F(AgentParserTest, configParseSocketD2) {
+    configParse(AGENT_CONFIGS[5], 0);
+
+    CtrlAgentCfgContextPtr ctx = cfg_mgr_.getCtrlAgentCfgContext();
+    ASSERT_TRUE(ctx);
+    ConstElementPtr socket = ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2);
+    ASSERT_TRUE(socket);
+    EXPECT_EQ("{ \"socket-name\": \"/tmp/socket-d2\", \"socket-type\": \"unix\" }",
+              socket->str());
+
+    EXPECT_FALSE(ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4));
+    EXPECT_FALSE(ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6));
+}
+
+// Tests if a single socket can be configured. BTW this test also checks
+// if default value for socket-type is specified (the config doesn't have it,
+// so the default value should be filed in).
+TEST_F(AgentParserTest, configParseSocketDhcp6) {
+    configParse(AGENT_CONFIGS[6], 0);
+
+    CtrlAgentCfgContextPtr ctx = cfg_mgr_.getCtrlAgentCfgContext();
+    ASSERT_TRUE(ctx);
+    ConstElementPtr socket = ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6);
+    ASSERT_TRUE(socket);
+    EXPECT_EQ("{ \"socket-name\": \"/tmp/socket-v6\", \"socket-type\": \"unix\" }",
+              socket->str());
+    EXPECT_FALSE(ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4));
+    EXPECT_FALSE(ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2));
+}
+
+// This tests if all 3 sockets can be configured and makes sure the parser
+// doesn't confuse them.
+TEST_F(AgentParserTest, configParse3Sockets) {
+    configParse(AGENT_CONFIGS[3], 0);
+    CtrlAgentCfgContextPtr ctx = cfg_mgr_.getCtrlAgentCfgContext();
+    ASSERT_TRUE(ctx);
+    ConstElementPtr socket2 = ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2);
+    ConstElementPtr socket4 = ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4);
+    ConstElementPtr socket6 = ctx->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6);
+    ASSERT_TRUE(socket2);
+    EXPECT_EQ("{ \"socket-name\": \"/tmp/socket-d2\", \"socket-type\": \"unix\" }",
+              socket2->str());
+    ASSERT_TRUE(socket4);
+    EXPECT_EQ("{ \"socket-name\": \"/tmp/socket-v4\", \"socket-type\": \"unix\" }",
+              socket4->str());
+    ASSERT_TRUE(socket6);
+    EXPECT_EQ("{ \"socket-name\": \"/tmp/socket-v6\", \"socket-type\": \"unix\" }",
+              socket6->str());
+}
+
+// This test checks that the config file with hook library specified can be
+// loaded. This one is a bit tricky, because the parser sanity checks the lib
+// name. In particular, it checks if such a library exists. Therefore we
+// can't use AGENT_CONFIGS[4] as is, but need to run it through path replacer.
+TEST_F(AgentParserTest, configParseHooks) {
+    // Create the configuration with proper lib path.
+    std::string cfg = pathReplacer(AGENT_CONFIGS[4], BASIC_CALLOUT_LIBRARY);
+    // The configuration should be successful.
+    configParse(cfg.c_str(), 0);
+
+    // The context now should have the library specified.
+    CtrlAgentCfgContextPtr ctx = cfg_mgr_.getCtrlAgentCfgContext();
+    HookLibsCollection libs = ctx->getLibraries();
+    ASSERT_EQ(1, libs.size());
+    EXPECT_EQ(string(BASIC_CALLOUT_LIBRARY), libs[0].first);
+    ASSERT_TRUE(libs[0].second);
+    EXPECT_EQ("{ \"param1\": \"foo\" }", libs[0].second->str());
+}
+
+
+}; // end of anonymous namespace

src/bin/agent/tests/ca_process_tests.sh.in

@@ -74,7 +74,7 @@ shutdown_test() {
     # It should be just once on startup.
     get_reconfigs
     if [ ${_GET_RECONFIGS} -ne 1 ]; then
-        printf "ERROR: server hasn't been configured.\n"
+        printf "ERROR: server been configured ${_GET_RECONFIGS} time(s), but exactly 1 was expected.\n"
         clean_exit 1
     else
         printf "Server successfully configured.\n"

src/bin/agent/tests/test_libraries.h.in

@@ -0,0 +1,24 @@
+// 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 AGENT_TEST_LIBRARIES_H
+#define AGENT_TEST_LIBRARIES_H
+
+#include <config.h>
+
+namespace {
+
+// Names of the libraries used in these tests.  These libraries are built using
+// libtool, so we need to look in the hidden ".libs" directory to locate the
+// .so file.  Note that we access the .so file - libtool creates this as a
+// like to the real shared library.
+
+// Basic library with context_create and three "standard" callouts.
+static const char* BASIC_CALLOUT_LIBRARY = "@abs_builddir@/.libs/libbasic.so";
+
+} // anonymous namespace
+
+#endif // TEST_LIBRARIES_H

src/bin/d2/d2_process.cc

@@ -191,10 +191,15 @@ D2Process::shutdown(isc::data::ConstElementPtr args) {
 }
 
 isc::data::ConstElementPtr
-D2Process::configure(isc::data::ConstElementPtr config_set) {
+D2Process::configure(isc::data::ConstElementPtr config_set, bool check_only) {
     LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC,
               DHCP_DDNS_CONFIGURE).arg(config_set->str());
 
+    /// @todo: Implement this eventually.
+    if (check_only) {
+        return (isc::config::createAnswer(0, "Configuration check is not supported by D2."));
+    }
+
     int rcode = 0;
     isc::data::ConstElementPtr comment;
     isc::data::ConstElementPtr answer = getCfgMgr()->parseConfig(config_set);;

src/bin/d2/d2_process.h

@@ -153,11 +153,12 @@ public:
     /// is retained and a failure response is returned as described below.
     ///
     /// @param config_set a new configuration (JSON) for the process
+    /// @param check_only true if configuration is to be verified only, not applied
     /// @return an Element that contains the results of configuration composed
     /// of an integer status value (0 means successful, non-zero means failure),
     /// and a string explanation of the outcome.
-    virtual isc::data::ConstElementPtr configure(isc::data::ConstElementPtr
-                                                 config_set);
+    virtual isc::data::ConstElementPtr
+    configure(isc::data::ConstElementPtr config_set, bool check_only);
 
     /// @brief Processes the given command.
     ///

src/bin/d2/tests/d2_process_unittests.cc

@@ -95,7 +95,7 @@ public:
             return res;
         }
 
-        isc::data::ConstElementPtr answer = configure(config_set_);
+        isc::data::ConstElementPtr answer = configure(config_set_, false);
         isc::data::ConstElementPtr comment;
         comment = isc::config::parseAnswer(rcode, answer);
 
@@ -182,7 +182,7 @@ TEST_F(D2ProcessTest, configure) {
     ASSERT_TRUE(fromJSON(valid_d2_config));
 
     // Invoke configure() with a valid D2 configuration.
-    isc::data::ConstElementPtr answer = configure(config_set_);
+    isc::data::ConstElementPtr answer = configure(config_set_, false);
 
     // Verify that configure result is success and reconfigure queue manager
     // flag is true.
@@ -200,7 +200,7 @@ TEST_F(D2ProcessTest, configure) {
     ASSERT_TRUE(fromJSON("{ \"bogus\": 1000 } "));
 
     // Invoke configure() with the invalid configuration.
-    answer = configure(config_set_);
+    answer = configure(config_set_, false);
 
     // Verify that configure result is failure, the reconfigure flag is
     // false, and that the queue manager is still running.
@@ -361,7 +361,7 @@ TEST_F(D2ProcessTest, badConfigureRecovery) {
 
     // Invoke configure() with a valid config that contains an unusable IP
     ASSERT_TRUE(fromJSON(bad_ip_d2_config));
-    isc::data::ConstElementPtr answer = configure(config_set_);
+    isc::data::ConstElementPtr answer = configure(config_set_, false);
 
     // Verify that configure result is success and reconfigure queue manager
     // flag is true.
@@ -378,7 +378,7 @@ TEST_F(D2ProcessTest, badConfigureRecovery) {
 
     // Verify we can recover given a valid config with an usable IP address.
     ASSERT_TRUE(fromJSON(valid_d2_config));
-    answer = configure(config_set_);
+    answer = configure(config_set_, false);
 
     // Verify that configure result is success and reconfigure queue manager
     // flag is true.

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/command_interpreter.cc

@@ -94,6 +94,43 @@ parseAnswer(int &rcode, const ConstElementPtr& msg) {
     return (msg->get(CONTROL_TEXT));
 }
 
+std::string
+answerToText(const ConstElementPtr& msg) {
+    if (!msg) {
+        isc_throw(CtrlChannelError, "No answer specified");
+    }
+    if (msg->getType() != Element::map) {
+        isc_throw(CtrlChannelError,
+                  "Invalid answer Element specified, expected map");
+    }
+    if (!msg->contains(CONTROL_RESULT)) {
+        isc_throw(CtrlChannelError,
+                  "Invalid answer specified, does not contain mandatory 'result'");
+    }
+
+    ConstElementPtr result = msg->get(CONTROL_RESULT);
+    if (result->getType() != Element::integer) {
+            isc_throw(CtrlChannelError,
+                      "Result element in answer message is not a string");
+    }
+
+    stringstream txt;
+    int rcode = result->intValue();
+    if (rcode == 0) {
+        txt << "success(0)";
+    } else {
+        txt << "failure(" << rcode << ")";
+    }
+
+    // Was any text provided? If yes, include it.
+    ConstElementPtr txt_elem = msg->get(CONTROL_TEXT);
+    if (txt_elem) {
+        txt << ", text=" << txt_elem->stringValue();
+    }
+
+    return (txt.str());
+}
+
 ConstElementPtr
 createCommand(const std::string& command) {
     return (createCommand(command, ElementPtr()));

src/lib/cc/command_interpreter.h

@@ -90,6 +90,12 @@ isc::data::ConstElementPtr createAnswer(const int status_code,
 isc::data::ConstElementPtr parseAnswer(int &status_code,
                                        const isc::data::ConstElementPtr& msg);
 
+/// @brief Converts answer to printable text
+///
+/// @param msg answer to be parsed
+/// @return printable string
+std::string answerToText(const isc::data::ConstElementPtr& msg);
+
 /// @brief Creates a standard config/command command message with no
 /// argument (of the form { "command": "my_command" })
 ///

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/cc/tests/command_interpreter_unittests.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2009-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2009-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
@@ -87,6 +87,19 @@ TEST(CommandInterpreterTest, parseAnswer) {
     EXPECT_EQ("[ \"just\", \"some\", \"data\" ]", arg->str());
 }
 
+// This checks whether we can convert an answer to easily printable form.
+TEST(CommandInterpreterTest, answerToText) {
+    ConstElementPtr answer;
+
+    // Doing jolly good here.
+    answer = el("{ \"result\": 0 }");
+    EXPECT_EQ("success(0)", answerToText(answer));
+
+    // Sometimes things don't go according to plan.
+    answer = el("{ \"result\": 1, \"text\": \"ho lee fuk sum ting wong\" }");
+    EXPECT_EQ("failure(1), text=ho lee fuk sum ting wong", answerToText(answer));
+}
+
 // This test checks whether createCommand function is able to create commands
 // with and without parameters.
 TEST(CommandInterpreterTest, createCommand) {

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,160 @@
+// 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;
+
+namespace isc {
+namespace hooks {
+
+// ******************** 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

src/lib/hooks/library_manager.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-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
@@ -33,7 +33,8 @@ namespace hooks {
 LibraryManager::LibraryManager(const std::string& name, int index,
                                const boost::shared_ptr<CalloutManager>& manager)
         : dl_handle_(NULL), index_(index), manager_(manager),
-          library_name_(name)
+          library_name_(name),
+          server_hooks_(ServerHooks::getServerHooksPtr())
 {
     if (!manager) {
         isc_throw(NoCalloutManager, "must specify a CalloutManager when "

src/lib/hooks/library_manager.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-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
@@ -8,7 +8,7 @@
 #define LIBRARY_MANAGER_H
 
 #include <exceptions/exceptions.h>
-
+#include <hooks/server_hooks.h>
 #include <boost/shared_ptr.hpp>
 
 #include <string>
@@ -219,6 +219,8 @@ private:
                                 ///< Callout manager for registration
     std::string library_name_;  ///< Name of the library
 
+    ServerHooksPtr server_hooks_; ///< Stores a pointer to ServerHooks.
+
 };
 
 } // namespace hooks

src/lib/hooks/server_hooks.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-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
@@ -142,7 +142,12 @@ ServerHooks::getHookNames() const {
 
 ServerHooks&
 ServerHooks::getServerHooks() {
-    static ServerHooks hooks;
+    return (*getServerHooksPtr());
+}
+
+ServerHooksPtr
+ServerHooks::getServerHooksPtr() {
+    static ServerHooksPtr hooks(new ServerHooks());
     return (hooks);
 }
 

src/lib/hooks/server_hooks.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-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
@@ -10,6 +10,7 @@
 #include <exceptions/exceptions.h>
 
 #include <boost/noncopyable.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include <map>
 #include <string>
@@ -37,6 +38,8 @@ public:
         isc::Exception(file, line, what) {}
 };
 
+class ServerHooks;
+typedef boost::shared_ptr<ServerHooks> ServerHooksPtr;
 
 /// @brief Server hook collection
 ///
@@ -133,6 +136,11 @@ public:
     /// @return Reference to the global ServerHooks object.
     static ServerHooks& getServerHooks();
 
+    /// @brief Returns pointer to ServerHooks object.
+    ///
+    /// @return Pointer to the global ServerHooks object.
+    static ServerHooksPtr getServerHooksPtr();
+
 private:
     /// @brief Constructor
     ///

src/lib/hooks/tests/.gitignore

@@ -1,4 +1,3 @@
 /marker_file.h
 /test_libraries.h
-
 /run_unittests

src/lib/process/d_cfg_mgr.cc

@@ -260,6 +260,61 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) {
     return (answer);
 }
 
+isc::data::ConstElementPtr
+DCfgMgrBase::simpleParseConfig(isc::data::ConstElementPtr config_set,
+                               bool check_only) {
+    if (!config_set) {
+        return (isc::config::createAnswer(1,
+                                    std::string("Can't parse NULL config")));
+    }
+    LOG_DEBUG(dctl_logger, DBGLVL_COMMAND,
+                DCTL_CONFIG_START).arg(config_set->str());
+
+    // The parsers implement data inheritance by directly accessing
+    // configuration context. For this reason the data parsers must store
+    // the parsed data into context immediately. This may cause data
+    // inconsistency if the parsing operation fails after the context has been
+    // modified. We need to preserve the original context here
+    // so as we can rollback changes when an error occurs.
+    DCfgContextBasePtr original_context = context_;
+    resetContext();
+
+    // Answer will hold the result returned to the caller.
+    ConstElementPtr answer;
+
+    try {
+        // Let's call the actual implementation
+        answer = parse(config_set, check_only);
+
+        // Everything was fine. Configuration set processed successfully.
+        if (!check_only) {
+            LOG_INFO(dctl_logger, DCTL_CONFIG_COMPLETE).arg(getConfigSummary(0));
+            answer = isc::config::createAnswer(0, "Configuration committed.");
+        } else {
+            LOG_INFO(dctl_logger, DCTL_CONFIG_CHECK_COMPLETE)
+                .arg(getConfigSummary(0))
+                .arg(config::answerToText(answer));
+        }
+
+    } catch (const std::exception& ex) {
+        LOG_ERROR(dctl_logger, DCTL_PARSER_FAIL).arg(ex.what());
+        answer = isc::config::createAnswer(1, ex.what());
+
+        // An error occurred, so make sure that we restore original context.
+        context_ = original_context;
+        return (answer);
+    }
+
+    if (check_only) {
+        // If this is a configuration check only, then don't actually apply
+        // the configuration and reverse to the previous one.
+        context_ = original_context;
+    }
+
+    return (answer);
+}
+
+
 void
 DCfgMgrBase::setCfgDefaults(isc::data::ElementPtr) {
 }
@@ -284,6 +339,10 @@ void DCfgMgrBase::buildAndCommit(std::string& element_id,
     parseElement(element_id, value);
 }
 
+isc::data::ConstElementPtr
+DCfgMgrBase::parse(isc::data::ConstElementPtr, bool) {
+    isc_throw(DCfgMgrBaseError, "This class does not implement simple parser paradigm yet");
+}
+
 }; // end of isc::dhcp namespace
 }; // end of isc namespace
-

src/lib/process/d_cfg_mgr.h

@@ -210,6 +210,10 @@ typedef std::vector<std::string> ElementIdList;
 /// class which is handed a set of configuration values to process by upper
 /// application management layers.
 ///
+/// This class allows two configuration methods:
+///
+/// 1. classic method
+///
 /// The class presents a public method for receiving new configurations,
 /// parseConfig.  This method coordinates the parsing effort as follows:
 ///
@@ -251,6 +255,25 @@ typedef std::vector<std::string> ElementIdList;
 ///
 /// In the event that an error occurs, parsing is halted and the
 /// configuration context is restored from backup.
+///
+/// See @ref isc::d2::D2CfgMgr and @ref isc::d2::D2Process for example use of
+/// this approach.
+///
+/// 2. simple configuration method
+///
+/// This approach assumes usage of @ref isc::data::SimpleParser paradigm. It
+/// does not use any intermediate storage, does not use parser pointers, does
+/// not enforce parsing order.
+///
+/// Here's the expected control flow order:
+/// 1. implementation calls simpleParseConfig from its configure method.
+/// 2. simpleParseConfig makes a configuration context
+/// 3. parse method from the derived class is called
+/// 4. if the configuration was unsuccessful of this is only a check, the
+///    old context is reinstantiated. If not, the configuration is kept.
+///
+/// See @ref isc::agent::CtrlAgentCfgMgr and @ref isc::agent::CtrlAgentProcess
+/// for example use of this approach.
 class DCfgMgrBase {
 public:
     /// @brief Constructor
@@ -267,7 +290,7 @@ public:
     /// @brief Acts as the receiver of new configurations and coordinates
     /// the parsing as described in the class brief.
     ///
-    /// @param config_set is a set of configuration elements to parsed.
+    /// @param config_set is a set of configuration elements to be parsed.
     ///
     /// @return an Element that contains the results of configuration composed
     /// of an integer status value (0 means successful, non-zero means failure),
@@ -275,6 +298,27 @@ public:
     isc::data::ConstElementPtr parseConfig(isc::data::ConstElementPtr
                                            config_set);
 
+
+    /// @brief Acts as the receiver of new configurations.
+    ///
+    /// This method is similar to what @ref parseConfig does, execept it employs
+    /// the simple parser paradigm: no intermediate storage, no parser pointers
+    /// no distinction between params_map and objects_map, parse order (if needed)
+    /// can be enforced in the actual implementation by calling specific
+    /// parsers first. See @ref isc::agent::CtrlAgentCfgMgr::parse for example.
+    ///
+    /// If check_only is true, the actual parsing is done to check if the configuration
+    /// is sane, but is then reverted.
+    ///
+    /// @param config set of configuration elements to be parsed
+    /// @param check_only true if the config is to be checked only, but not applied
+    /// @return an Element that contains the results of configuration composed
+    /// of an integer status value (0 means successful, non-zero means failure),
+    /// and a string explanation of the outcome.
+    isc::data::ConstElementPtr
+    simpleParseConfig(isc::data::ConstElementPtr config,
+                      bool check_only = false);
+
     /// @brief Adds a given element id to the end of the parse order list.
     ///
     /// The order in which object elements are retrieved from this is the
@@ -371,6 +415,24 @@ protected:
     /// @throw DCfgMgrBaseError if context is NULL.
     void setContext(DCfgContextBasePtr& context);
 
+    /// @brief Parses actual configuration.
+    ///
+    /// This method is expected to be implemented in derived classes that employ
+    /// SimpleParser paradigm (i.e. they call simpleParseConfig rather than
+    /// parseConfig from their configure method).
+    ///
+    /// Implementations that do not employ this method may provide dummy
+    /// implementation.
+    ///
+    /// @param config the Element tree structure that decribes the configuration.
+    /// @param check_only false for normal configuration, true when verifying only
+    ///
+    /// @return an Element that contains the results of configuration composed
+    /// of an integer status value (0 means successful, non-zero means failure),
+    /// and a string explanation of the outcome.
+    virtual isc::data::ConstElementPtr parse(isc::data::ConstElementPtr config,
+                                             bool check_only);
+
 private:
 
     /// @brief Parse a configuration element.

src/lib/process/d_controller.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-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
@@ -330,7 +330,7 @@ DControllerBase::runProcess() {
 // Instance method for handling new config
 isc::data::ConstElementPtr
 DControllerBase::updateConfig(isc::data::ConstElementPtr new_config) {
-    return (process_->configure(new_config));
+    return (process_->configure(new_config, false));
 }
 
 

src/lib/process/d_process.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-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
@@ -113,11 +113,12 @@ public:
     /// below.
     ///
     /// @param config_set a new configuration (JSON) for the process
+    /// @param check_only true if configuration is to be verified only, not applied
     /// @return an Element that contains the results of configuration composed
     /// of an integer status value (0 means successful, non-zero means failure),
     /// and a string explanation of the outcome.
-    virtual isc::data::ConstElementPtr configure(isc::data::ConstElementPtr
-                                                 config_set) = 0;
+    virtual isc::data::ConstElementPtr
+    configure(isc::data::ConstElementPtr config_set, bool check_only) = 0;
 
     /// @brief Processes the given command.
     ///

src/lib/process/process_messages.mes

@@ -28,6 +28,11 @@ to establish a session with the Kea control channel.
 A debug message listing the command (and possible arguments) received
 from the Kea control system by the controller.
 
+% DCTL_CONFIG_CHECK_COMPLETE server has completed configuration check: %1, result: %2
+This is an informational message announcing the successful processing of a
+new configuration check is complete. The result of that check is printed.
+This informational message is printed when configuration check is requested.
+
 % DCTL_CONFIG_COMPLETE server has completed configuration: %1
 This is an informational message announcing the successful processing of a
 new configuration. It is output during server startup, and when an updated

src/lib/process/tests/d_cfg_mgr_unittests.cc

@@ -459,4 +459,29 @@ TEST_F(DStubCfgMgrTest, paramPosition) {
     EXPECT_EQ(pos.file_, isc::data::Element::ZERO_POSITION().file_);
 }
 
+// This tests if some aspects of simpleParseConfig are behaving properly.
+// Thorough testing is only possible for specific implementations. This
+// is done for control agent (see CtrlAgentControllerTest tests in
+// src/bin/agent/tests/ctrl_agent_controller_unittest.cc for example).
+// Also, shell tests in src/bin/agent/ctrl_agent_process_tests.sh test
+// the whole CA process that uses simpleParseConfig. The alternative
+// would be to implement whole parser that would set the context
+// properly. The ROI for this is not worth the effort.
+TEST_F(DStubCfgMgrTest, simpleParseConfig) {
+    using namespace isc::data;
+
+    // Passing just null pointer should result in error return code
+    answer_ = cfg_mgr_->simpleParseConfig(ConstElementPtr(), false);
+    EXPECT_TRUE(checkAnswer(1));
+
+    // Ok, now try with a dummy, but valid json code
+    string config = "{ \"bool_test\": true , \n"
+                    "  \"uint32_test\": 77 , \n"
+                    "  \"string_test\": \"hmmm chewy\" }";
+    ASSERT_NO_THROW(fromJSON(config));
+
+    answer_ = cfg_mgr_->simpleParseConfig(config_set_, false);
+    EXPECT_TRUE(checkAnswer(0));
+}
+
 } // end of anonymous namespace

src/lib/process/testutils/d_test_stubs.cc

@@ -9,6 +9,7 @@
 #include <process/d_log.h>
 #include <process/spec_config.h>
 #include <process/testutils/d_test_stubs.h>
+#include <cc/command_interpreter.h>
 
 using namespace boost::asio;
 
@@ -66,7 +67,12 @@ DStubProcess::shutdown(isc::data::ConstElementPtr /* args */) {
 }
 
 isc::data::ConstElementPtr
-DStubProcess::configure(isc::data::ConstElementPtr config_set) {
+DStubProcess::configure(isc::data::ConstElementPtr config_set, bool check_only) {
+    if (check_only) {
+        return (isc::config::createAnswer(1,
+                "Configuration checking is not supported."));
+    }
+
     if (SimFailure::shouldFailOn(SimFailure::ftProcessConfigure)) {
         // Simulates a process configure failure.
         return (isc::config::createAnswer(1,
@@ -387,5 +393,10 @@ DStubCfgMgr::parseElement(const std::string& element_id,
     parsed_order_.push_back(element_id);
 }
 
+isc::data::ConstElementPtr
+DStubCfgMgr::parse(isc::data::ConstElementPtr /*config*/, bool /*check_only*/) {
+    return (isc::config::createAnswer(0, "It all went fine. I promise"));
+}
+
 }; // namespace isc::process
 }; // namespace isc

src/lib/process/testutils/d_test_stubs.h

@@ -145,11 +145,12 @@ public:
     /// of the inbound configuration.
     ///
     /// @param config_set a new configuration (JSON) for the process
+    /// @param check_only true if configuration is to be verified only, not applied
     /// @return an Element that contains the results of configuration composed
     /// of an integer status value (0 means successful, non-zero means failure),
     /// and a string explanation of the outcome.
-    virtual isc::data::ConstElementPtr configure(isc::data::ConstElementPtr
-                                                 config_set);
+    virtual isc::data::ConstElementPtr
+    configure(isc::data::ConstElementPtr config_set, bool check_only);
 
     /// @brief Executes the given command.
     ///
@@ -699,6 +700,19 @@ public:
     virtual void parseElement(const std::string& element_id,
                               isc::data::ConstElementPtr element);
 
+    /// @brief Pretends to parse the config
+    ///
+    /// This method pretends to parse the configuration specified on input
+    /// and returns a positive answer. The check_only flag is currently ignored.
+    ///
+    /// @param config configuration specified
+    /// @param check_only whether it's real configuration (false) or just
+    ///                configuration check (true)
+    /// @return always positive answer
+    ///
+    isc::data::ConstElementPtr
+    parse(isc::data::ConstElementPtr config, bool check_only);
+
     /// @brief Returns a summary of the configuration in the textual format.
     ///
     /// @return Always an empty string.