[5145b] Rebased keeping host_parser in hooks

src/bin/agent/ca_cfg_mgr.cc

@@ -24,7 +24,7 @@ CtrlAgentCfgContext::CtrlAgentCfgContext()
 
 CtrlAgentCfgContext::CtrlAgentCfgContext(const CtrlAgentCfgContext& orig)
     : DCfgContextBase(),http_host_(orig.http_host_), http_port_(orig.http_port_),
-      libraries_(orig.libraries_) {
+      hooks_config_(orig.hooks_config_) {
 
     // 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
@@ -74,7 +74,7 @@ CtrlAgentCfgMgr::getConfigSummary(const uint32_t /*selection*/) {
     }
 
     // Finally, print the hook libraries names
-    const hooks::HookLibsCollection libs = ctx->getLibraries();
+    const isc::hooks::HookLibsCollection libs = ctx->getHooksConfig().get();
     s << ", " << libs.size() << " lib(s):";
     for (auto lib = libs.begin(); lib != libs.end(); ++lib) {
         s << lib->first << " ";

src/bin/agent/ca_cfg_mgr.h

@@ -8,9 +8,9 @@
 #define CTRL_AGENT_CFG_MGR_H
 
 #include <cc/data.h>
+#include <hooks/hooks_config.h>
 #include <process/d_cfg_mgr.h>
 #include <boost/pointer_cast.hpp>
-#include <hooks/libinfo.h>
 
 namespace isc {
 namespace agent {
@@ -61,7 +61,7 @@ public:
     ///
     /// @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;
+    const isc::data::ConstElementPtr getControlSocketInfo(ServerType type) const;
 
     /// @brief Sets information about the control socket
     ///
@@ -102,20 +102,20 @@ public:
         return (http_port_);
     }
 
-    /// @brief Returns a list of hook libraries
-    /// @return a list of hook libraries
-    const hooks::HookLibsCollection& getLibraries() const {
-        return (libraries_);
+    /// @brief Returns non-const reference to configured hooks libraries.
+    ///
+    /// @return non-const reference to configured hooks libraries.
+    isc::hooks::HooksConfig& getHooksConfig() {
+        return (hooks_config_);
     }
 
-    /// @brief Sets the list of hook libraries
+    /// @brief Returns const reference to configured hooks libraries.
     ///
-    /// @params libs a coolection of libraries to remember.
-    void setLibraries(const hooks::HookLibsCollection& libs) {
-        libraries_ = libs;
+    /// @return const reference to configured hooks libraries.
+    const isc::hooks::HooksConfig& getHooksConfig() const {
+        return (hooks_config_);
     }
 
-
 private:
 
     /// @brief Private copy constructor
@@ -132,7 +132,7 @@ private:
     CtrlAgentCfgContext& operator=(const CtrlAgentCfgContext& rhs);
 
     /// Socket information will be stored here (for all supported servers)
-    data::ConstElementPtr ctrl_sockets_[MAX_TYPE_SUPPORTED + 1];
+    isc::data::ConstElementPtr ctrl_sockets_[MAX_TYPE_SUPPORTED + 1];
 
     /// Hostname the CA should listen on.
     std::string http_host_;
@@ -140,8 +140,8 @@ private:
     /// TCP port the CA should listen on.
     uint16_t http_port_;
 
-    /// List of hook libraries.
-    hooks::HookLibsCollection libraries_;
+    /// @brief Configured hooks libraries.
+    isc::hooks::HooksConfig hooks_config_;
 };
 
 /// @brief Ctrl Agent Configuration Manager.

src/bin/agent/simple_parser.cc

@@ -107,22 +107,21 @@ AgentSimpleParser::parse(const CtrlAgentCfgContextPtr& ctx,
     }
 
     // Finally, let's get the hook libs!
-    hooks::HooksLibrariesParser hooks_parser;
+    
+    using namespace isc::hooks;
+    HooksConfig& libraries = ctx->getHooksConfig();
     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);
+        HooksLibrariesParser hooks_parser;
+        hooks_parser.parse(libraries, hooks);
+        libraries.verifyLibraries(hooks->getPosition());
     }
 
     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();
+        libraries.loadLibraries();
     }
 }
 

src/bin/agent/tests/ca_cfg_mgr_unittests.cc

@@ -10,12 +10,12 @@
 #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::dhcp;
 using namespace isc::hooks;
 using namespace isc::process;
 
@@ -123,11 +123,10 @@ TEST(CtrlAgentCfgMgr, contextSocketInfoCopy) {
     EXPECT_NO_THROW(ctx.setHttpPort(12345));
     EXPECT_NO_THROW(ctx.setHttpHost("bellatrix"));
 
-    HookLibsCollection libs;
+    HooksConfig& libs = ctx.getHooksConfig();
     string exp_name("testlib1.so");
     ConstElementPtr exp_param(new StringElement("myparam"));
-    libs.push_back(make_pair(exp_name, exp_param));
-    ctx.setLibraries(libs);
+    libs.add(exp_name, exp_param);
 
     // Make a copy.
     DCfgContextBasePtr copy_base(ctx.clone());
@@ -147,7 +146,7 @@ TEST(CtrlAgentCfgMgr, contextSocketInfoCopy) {
     EXPECT_EQ(socket3->str(), copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6)->str());
 
     // Check hook libs
-    HookLibsCollection libs2 = copy->getLibraries();
+    const HookLibsCollection& libs2 = copy->getHooksConfig().get();
     ASSERT_EQ(1, libs2.size());
     EXPECT_EQ(exp_name, libs2[0].first);
     ASSERT_TRUE(libs2[0].second);
@@ -160,19 +159,18 @@ TEST(CtrlAgentCfgMgr, contextHookParams) {
     CtrlAgentCfgContext ctx;
 
     // By default there should be no hooks.
-    HookLibsCollection libs = ctx.getLibraries();
-    EXPECT_TRUE(libs.empty());
+    HooksConfig& libs = ctx.getHooksConfig();
+    EXPECT_TRUE(libs.get().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}")));
+    libs.add("libone.so", ConstElementPtr());
+    libs.add("libtwo.so", Element::fromJSON("{\"foo\": true}"));
+    libs.add("libthree.so", Element::fromJSON("{\"bar\": 42}"));
 
-    ctx.setLibraries(libs);
+    const HooksConfig& stored_libs = ctx.getHooksConfig();
+    EXPECT_EQ(3, stored_libs.get().size());
 
-    HookLibsCollection stored_libs = ctx.getLibraries();
-    EXPECT_EQ(3, stored_libs.size());
-
-    EXPECT_EQ(libs, stored_libs);
+    // @todo add a == operator to HooksConfig
+    EXPECT_EQ(libs.get(), stored_libs.get());
 }
 
 /// Control Agent configurations used in tests.
@@ -389,7 +387,7 @@ TEST_F(AgentParserTest, configParseHooks) {
 
     // The context now should have the library specified.
     CtrlAgentCfgContextPtr ctx = cfg_mgr_.getCtrlAgentCfgContext();
-    HookLibsCollection libs = ctx->getLibraries();
+    const HookLibsCollection libs = ctx->getHooksConfig().get();
     ASSERT_EQ(1, libs.size());
     EXPECT_EQ(string(BASIC_CALLOUT_LIBRARY), libs[0].first);
     ASSERT_TRUE(libs[0].second);

src/bin/dhcp4/json_config_parser.cc

@@ -42,6 +42,7 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::data;
 using namespace isc::asiolink;
+using namespace isc::hooks;
 
 namespace {
 
@@ -431,11 +432,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set,
     // for option definitions. This is equivalent to commiting empty container.
     LibDHCP::setRuntimeOptionDefs(OptionDefSpaceContainer());
 
-    // 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.)
-    hooks::HooksLibrariesParser hooks_parser;
-
     // Answer will hold the result.
     ConstElementPtr answer;
     // Rollback informs whether error occurred and original data
@@ -515,8 +511,10 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set,
             }
 
             if (config_pair.first == "hooks-libraries") {
-                hooks_parser.parse(config_pair.second);
-                hooks_parser.verifyLibraries();
+                HooksLibrariesParser hooks_parser;
+                HooksConfig& libraries = srv_cfg->getHooksConfig();
+                hooks_parser.parse(libraries, config_pair.second);
+                libraries.verifyLibraries(config_pair.second->getPosition());
                 continue;
             }
 
@@ -637,7 +635,9 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set,
             // 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();
+            const HooksConfig& libraries =
+                CfgMgr::instance().getStagingCfg()->getHooksConfig();
+            libraries.loadLibraries();
         }
         catch (const isc::Exception& ex) {
             LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what());

src/bin/dhcp6/json_config_parser.cc

@@ -54,6 +54,7 @@ using namespace isc;
 using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
+using namespace isc::hooks;
 
 namespace {
 
@@ -638,11 +639,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set,
     // for option definitions. This is equivalent to commiting empty container.
     LibDHCP::setRuntimeOptionDefs(OptionDefSpaceContainer());
 
-    // 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.)
-    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
     // default values and will insert derived values as well.
@@ -738,8 +734,10 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set,
             }
 
             if (config_pair.first == "hooks-libraries") {
-                hooks_parser.parse(config_pair.second);
-                hooks_parser.verifyLibraries();
+                HooksLibrariesParser hooks_parser;
+                HooksConfig& libraries = srv_config->getHooksConfig();
+                hooks_parser.parse(libraries, config_pair.second);
+                libraries.verifyLibraries(config_pair.second->getPosition());
                 continue;
             }
 
@@ -860,7 +858,9 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set,
             // This occurs last as if it succeeds, there is no easy way to
             // revert it.  As a result, the failure to commit a subsequent
             // change causes problems when trying to roll back.
-            hooks_parser.loadLibraries();
+            const HooksConfig& libraries =
+                CfgMgr::instance().getStagingCfg()->getHooksConfig();
+            libraries.loadLibraries();
         }
         catch (const isc::Exception& ex) {
             LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_FAIL).arg(ex.what());

src/lib/Makefile.am

 # The following build order must be maintained.
-SUBDIRS = exceptions util log cryptolink dns cc hooks asiolink testutils dhcp config \
+SUBDIRS = exceptions util log cryptolink dns cc asiolink testutils hooks dhcp config \
 	      stats asiodns dhcp_ddns eval dhcpsrv cfgrpt process http

src/lib/dhcpsrv/srv_config.cc

@@ -110,6 +110,14 @@ SrvConfig::copy(SrvConfig& new_config) const {
     new_config.class_dictionary_.reset(new ClientClassDictionary(*class_dictionary_));
     // Replace the D2 client configuration
     new_config.setD2ClientConfig(getD2ClientConfig());
+    // Replace configured hooks libraries.
+    new_config.hooks_config_.clear();
+    using namespace isc::hooks;
+    for (HookLibsCollection::const_iterator it =
+           hooks_config_.get().begin();
+         it != hooks_config_.get().end(); ++it) {
+        new_config.hooks_config_.add(it->first, it->second);
+    }
 }
 
 void
@@ -151,11 +159,49 @@ SrvConfig::equals(const SrvConfig& other) const {
         }
     }
     // Logging information is equal between objects, so check other values.
-    return ((*cfg_iface_ == *other.cfg_iface_) &&
-            (*cfg_option_def_ == *other.cfg_option_def_) &&
-            (*cfg_option_ == *other.cfg_option_) &&
-            (*class_dictionary_ == *other.class_dictionary_) &&
-            (*d2_client_config_ == *other.d2_client_config_));
+    if ((*cfg_iface_ != *other.cfg_iface_) ||
+        (*cfg_option_def_ != *other.cfg_option_def_) ||
+        (*cfg_option_ != *other.cfg_option_) ||
+        (*class_dictionary_ != *other.class_dictionary_) ||
+        (*d2_client_config_ != *other.d2_client_config_)) {
+        return (false);
+    }
+    // Now only configured hooks libraries can differ.
+    // If number of configured hooks libraries are different, then
+    // configurations aren't equal.
+    if (hooks_config_.get().size() != other.hooks_config_.get().size()) {
+        return (false);
+    }
+    // Pass through all configured hooks libraries.
+    using namespace isc::hooks;
+    for (isc::hooks::HookLibsCollection::const_iterator this_it =
+             hooks_config_.get().begin();
+         this_it != hooks_config_.get().end(); ++this_it) {
+        bool match = false;
+        for (isc::hooks::HookLibsCollection::const_iterator other_it =
+                 other.hooks_config_.get().begin();
+             other_it != other.hooks_config_.get().end(); ++other_it) {
+            if (this_it->first != other_it->first) {
+                continue;
+            }
+            if (isNull(this_it->second) && isNull(other_it->second)) {
+                match = true;
+                break;
+            }
+            if (isNull(this_it->second) || isNull(other_it->second)) {
+                continue;
+            }
+            if (this_it->second->equals(*other_it->second)) {
+                match = true;
+                break;
+            }
+        }
+        // No match found for the particular hooks library so return false.
+        if (!match) {
+            return (false);
+        }
+    }
+    return (true);
 }
 
 void

src/lib/dhcpsrv/srv_config.h

@@ -22,6 +22,7 @@
 #include <dhcpsrv/client_class_def.h>
 #include <dhcpsrv/d2_client_cfg.h>
 #include <dhcpsrv/logging_info.h>
+#include <hooks/hooks_config.h>
 #include <cc/data.h>
 #include <boost/shared_ptr.hpp>
 #include <vector>
@@ -365,6 +366,20 @@ public:
         class_dictionary_ = dictionary;
     }
 
+    /// @brief Returns non-const reference to configured hooks libraries.
+    ///
+    /// @return non-const reference to configured hooks libraries.
+    isc::hooks::HooksConfig& getHooksConfig() {
+        return (hooks_config_);
+    }
+
+    /// @brief Returns const reference to configured hooks libraries.
+    ///
+    /// @return const reference to configured hooks libraries.
+    const isc::hooks::HooksConfig& getHooksConfig() const {
+        return (hooks_config_);
+    }
+
     /// @brief Copies the current configuration to a new configuration.
     ///
     /// This method copies the parameters stored in the configuration to
@@ -593,6 +608,9 @@ private:
     /// @brief Pointer to the dictionary of global client class definitions
     ClientClassDictionaryPtr class_dictionary_;
 
+    /// @brief Configured hooks libraries.
+    isc::hooks::HooksConfig hooks_config_;
+
     /// @brief Decline Period time
     ///
     /// This timer specifies decline probation period, the time after a declined

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -18,10 +18,10 @@
 #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>
+#include <hooks/hooks_parser.h>
 #include <hooks/hooks_manager.h>
 
 #include <gtest/gtest.h>
@@ -377,10 +377,12 @@ public:
                 }
 
                 if (config_pair.first == "hooks-libraries") {
-                    hooks_libraries_parser_.reset(new HooksLibrariesParser());
-                    hooks_libraries_parser_->parse(config_pair.second);
-                    hooks_libraries_parser_->verifyLibraries();
-                    hooks_libraries_parser_->loadLibraries();
+                    HooksLibrariesParser hook_parser;
+                    HooksConfig&  libraries =
+                        CfgMgr::instance().getStagingCfg()->getHooksConfig();
+                    hook_parser.parse(libraries, config_pair.second);
+                    libraries.verifyLibraries(config_pair.second->getPosition());
+                    libraries.loadLibraries();
                     continue;
                 }
             }
@@ -610,10 +612,10 @@ public:
         CfgMgr::instance().setD2ClientConfig(tmp);
     }
 
-    /// @brief Parsers used in the parsing of the configuration
-    ///
-    /// Allows the tests to interrogate the state of the parsers (if required).
-    boost::shared_ptr<HooksLibrariesParser> hooks_libraries_parser_;
+    /// Allows the tests to interrogate the state of the libraries (if required).
+    const isc::hooks::HookLibsCollection& getLibraries() {
+        return (CfgMgr::instance().getStagingCfg()->getHooksConfig().get());
+    }
 
     /// @brief specifies IP protocol family (AF_INET or AF_INET6)
     uint16_t family_;
@@ -1306,8 +1308,7 @@ TEST_F(ParseConfigTest, noHooksLibraries) {
     ASSERT_TRUE(rcode == 0) << error_text_;
 
     // Check that the parser recorded nothing.
-    isc::hooks::HookLibsCollection libraries;
-    hooks_libraries_parser_->getLibraries(libraries);
+    isc::hooks::HookLibsCollection libraries = getLibraries();
     EXPECT_TRUE(libraries.empty());
 
     // Check that there are still no libraries loaded.
@@ -1329,8 +1330,7 @@ TEST_F(ParseConfigTest, oneHooksLibrary) {
     ASSERT_TRUE(rcode == 0) << error_text_;
 
     // Check that the parser recorded a single library.