[5134] Unit-tests implemented.

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

@@ -82,6 +82,7 @@ CtrlAgentCfgMgr::createNewContext() {
 
 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");
     }
@@ -92,6 +93,7 @@ CtrlAgentCfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) {
     ElementPtr cfg = boost::const_pointer_cast<Element>(config_set);
     AgentSimpleParser::setAllDefaults(cfg);
 
+    // And parse the configuration.
     ConstElementPtr answer;
     std::string excuse;
     try {
@@ -106,6 +108,7 @@ CtrlAgentCfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) {
         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);
@@ -117,12 +120,8 @@ CtrlAgentCfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) {
 
     if (check_only) {
         answer = isc::config::createAnswer(0, "Configuration check successful");
-        LOG_INFO(agent_logger, CTRL_AGENT_CONFIG_CHECK_COMPLETE)
-            .arg(getConfigSummary(0));
     } else {
         answer = isc::config::createAnswer(0, "Configuration applied successfully.");
-        LOG_INFO(agent_logger, CTRL_AGENT_CONFIG_COMPLETE)
-            .arg(getConfigSummary(0));
     }
 
     return (answer);

src/bin/agent/ctrl_agent_controller.cc

@@ -16,7 +16,7 @@ namespace agent {
 
 /// @brief Defines the application name, this is passed into base class
 /// it may be used to locate configuration data and appears in log statement.
-const char* CtrlAgentController::agent_app_name_ = "CtrlAgent";
+const char* CtrlAgentController::agent_app_name_ = "Control-agent";
 
 /// @brief Defines the executable name. This is passed into the base class
 const char* CtrlAgentController::agent_bin_name_ = "kea-ctrl-agent";

src/bin/agent/ctrl_agent_messages.mes

@@ -19,19 +19,11 @@ 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_COMPLETE Control Agent configuration complete: %1
-This informational message indicates that the CA had completed its
-configuration.
-
 % 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_COMPLETE Control Agent configuration check complete: %1
-This informationnal message indicates that the CA had completed the
-configuration check. The outcome of this check is part of the message.
-
 % 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

src/bin/agent/simple_parser.cc

@@ -33,7 +33,7 @@ namespace agent {
 ///
 /// These are global Control Agent parameters.
 const SimpleDefaults AgentSimpleParser::AGENT_DEFAULTS = {
-    { "http-post",    Element::string,  "localhost"},
+    { "http-host",    Element::string,  "localhost"},
     { "http-port",    Element::integer,  "8000"}
 };
 
@@ -81,32 +81,32 @@ void
 AgentSimpleParser::parse(CtrlAgentCfgContextPtr ctx, isc::data::ConstElementPtr config,
                          bool check_only) {
 
+    // Let's get the HTTP parameters first.
     ctx->setHost(SimpleParser::getString(config, "http-host"));
     ctx->setPort(SimpleParser::getIntType<uint16_t>(config, "http-port"));
 
+    // Control sockets are second.
     ConstElementPtr ctrl_sockets = config->get("control-sockets");
-    if (!ctrl_sockets) {
-        isc_throw(ConfigError, "Missing mandatory parameter 'control-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 (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 (d2_socket) {
+            ctx->setControlSocketInfo(d2_socket, CtrlAgentCfgContext::TYPE_D2);
+        }
 
-    if (d4_socket) {
-        ctx->setControlSocketInfo(d4_socket, CtrlAgentCfgContext::TYPE_DHCP4);
-    }
+        if (d4_socket) {
+            ctx->setControlSocketInfo(d4_socket, CtrlAgentCfgContext::TYPE_DHCP4);
+        }
 
-    if (d6_socket) {
-        ctx->setControlSocketInfo(d6_socket, CtrlAgentCfgContext::TYPE_DHCP6);
+        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);

src/bin/agent/tests/Makefile.am

@@ -86,6 +86,8 @@ 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/ctrl_agent_cfg_mgr_unittest.cc

@@ -6,14 +6,24 @@
 
 #include <config.h>
 #include <agent/ctrl_agent_cfg_mgr.h>
+#include <agent/parser_context.h>
 #include <process/testutils/d_test_stubs.h>
+#include <agent/tests/test_libraries.h>
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
 
 using namespace isc::agent;
+using namespace isc::data;
+using namespace isc::hooks;
 
 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 +40,252 @@ 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.setPort(12345);
+    EXPECT_EQ(12345, ctx.getPort());
+
+    ctx.setHost("alnitak");
+    EXPECT_EQ("alnitak", ctx.getHost());
+}
+
+// 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 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"
+    "}"
+};
+
+/// @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 Reeplaces %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->getHost());
+    EXPECT_EQ(8001, ctx->getPort());
+}
+
+// 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, configParse1Socket) {
+    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));
+}
+
+// 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/ctrl_agent_process_tests.sh.in

@@ -14,7 +14,7 @@ EXPECTED_VERSION="@PACKAGE_VERSION@"
 # Control Agent configuration to be stored in the configuration file.
 # todo: use actual configuration once we support it.
 CONFIG="{
-    \"CtrlAgent\":
+    \"Control-agent\":
     {
         \"dummy-param\": 123
     },
@@ -75,7 +75,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