Browse Source

[master] Finished merge of trac5150a (config-test command)

Francis Dupont 8 years ago
parent
commit
5f6cf226e8

+ 6 - 0
ChangeLog

@@ -1,3 +1,9 @@
+1229.	[func]		fdupont
+	A new command: config-test has been implemented in DHCPv4, DHCPv6
+	and control agent servers. It allows checking whether new
+	configuration looks correct.
+	(Trac #5150, git xxx)
+
 1228.	[bug]		fdupont
 1228.	[bug]		fdupont
 	Logging on syslog now uses correctly the given facility. If none
 	Logging on syslog now uses correctly the given facility. If none
 	is given the default facility is local0, and with an unrecognized
 	is given the default facility is local0, and with an unrecognized

+ 57 - 2
doc/guide/ctrl-channel.xml

@@ -154,7 +154,62 @@ will be sent to Kea and the responses received from Kea printed to standard outp
 }
 }
 </screen>
 </screen>
         </para>
         </para>
-      </section>
+      </section> <!-- end of command-config-get -->
+
+      <section id="command-config-test">
+        <title>config-test</title>
+
+      <para>
+     The <emphasis>config-test</emphasis> command instructs the server to check
+     whether the new configuration supplied in the command's arguments can
+     be loaded. The supplied configuration is expected to be the full
+     configuration for the target server along with an optional Logger
+     configuration. As for the <command>-t</command> command some sanity checks
+     are not performed so it is possible a configuration which successfully
+     passes this command will still fail in <command>set-config</command>
+     command or at launch time.
+     The structure of the command is as follows:
+      </para>
+<screen>
+{
+    "command": "config-test",
+    "arguments":  {
+        "&#60;server&#62;": {
+        },
+        "Logging": {
+        }
+     }
+}
+</screen>
+      <para>
+    where &#60;server&#62; is the configuration element name for a given server
+    such as "Dhcp4" or "Dhcp6".  For example:
+      </para>
+<screen>
+{
+    "command": "config-test",
+    "arguments":  {
+        "Dhcp6": {
+            :
+        },
+        "Logging": {
+            :
+        }
+     }
+}
+</screen>
+      <para>
+        The server's response will contain a numeric code, "result" (0 for success,
+    non-zero on failure), and  a string, "text", describing the outcome:
+<screen>
+    {"result": 0, "text": "Configuration seems sane..." }
+
+    or
+
+    {"result": 1, "text": "unsupported parameter: BOGUS (&#60;string&#62;:16:26)" }
+</screen>
+      </para>
+    </section> <!-- end of command-config-test -->
 
 
       <section id="command-config-write">
       <section id="command-config-write">
         <title>config-write</title>
         <title>config-write</title>
@@ -178,7 +233,7 @@ will be sent to Kea and the responses received from Kea printed to standard outp
 }
 }
 </screen>
 </screen>
         </para>
         </para>
-      </section>
+      </section> <!-- end of command-config-write -->
 
 
       <section id="command-leases-reclaim">
       <section id="command-leases-reclaim">
         <title>leases-reclaim</title>
         <title>leases-reclaim</title>

+ 1 - 0
doc/guide/dhcp4-srv.xml

@@ -3725,6 +3725,7 @@ src/lib/dhcpsrv/cfg_host_operations.cc -->
         <itemizedlist>
         <itemizedlist>
             <listitem>build-report</listitem>
             <listitem>build-report</listitem>
             <listitem>config-get</listitem>
             <listitem>config-get</listitem>
+            <listitem>config-test</listitem>
             <listitem>config-write</listitem>
             <listitem>config-write</listitem>
             <listitem>leases-reclaim</listitem>
             <listitem>leases-reclaim</listitem>
             <listitem>list-commands</listitem>
             <listitem>list-commands</listitem>

+ 1 - 0
doc/guide/dhcp6-srv.xml

@@ -4133,6 +4133,7 @@ If not specified, the default value is:
         <itemizedlist>
         <itemizedlist>
             <listitem>build-report</listitem>
             <listitem>build-report</listitem>
             <listitem>config-get</listitem>
             <listitem>config-get</listitem>
+            <listitem>config-test</listitem>
             <listitem>config-write</listitem>
             <listitem>config-write</listitem>
             <listitem>leases-reclaim</listitem>
             <listitem>leases-reclaim</listitem>
             <listitem>list-commands</listitem>
             <listitem>list-commands</listitem>

+ 16 - 4
src/bin/agent/ca_controller.cc

@@ -50,21 +50,33 @@ CtrlAgentController::parseFile(const std::string& name) {
 
 
 void
 void
 CtrlAgentController::registerCommands() {
 CtrlAgentController::registerCommands() {
-    CtrlAgentCommandMgr::instance().registerCommand(VERSION_GET_COMMAND,
-        boost::bind(&DControllerBase::versionGetHandler, this, _1, _2));
-
     CtrlAgentCommandMgr::instance().registerCommand(BUILD_REPORT_COMMAND,
     CtrlAgentCommandMgr::instance().registerCommand(BUILD_REPORT_COMMAND,
         boost::bind(&DControllerBase::buildReportHandler, this, _1, _2));
         boost::bind(&DControllerBase::buildReportHandler, this, _1, _2));
 
 
+    CtrlAgentCommandMgr::instance().registerCommand(CONFIG_GET_COMMAND,
+        boost::bind(&DControllerBase::configGetHandler, this, _1, _2));
+
+    CtrlAgentCommandMgr::instance().registerCommand(CONFIG_TEST_COMMAND,
+        boost::bind(&DControllerBase::configTestHandler, this, _1, _2));
+
+    CtrlAgentCommandMgr::instance().registerCommand(CONFIG_WRITE_COMMAND,
+        boost::bind(&DControllerBase::configWriteHandler, this, _1, _2));
+
     CtrlAgentCommandMgr::instance().registerCommand(SHUT_DOWN_COMMAND,
     CtrlAgentCommandMgr::instance().registerCommand(SHUT_DOWN_COMMAND,
         boost::bind(&DControllerBase::shutdownHandler, this, _1, _2));
         boost::bind(&DControllerBase::shutdownHandler, this, _1, _2));
+
+    CtrlAgentCommandMgr::instance().registerCommand(VERSION_GET_COMMAND,
+        boost::bind(&DControllerBase::versionGetHandler, this, _1, _2));
 }
 }
 
 
 void
 void
 CtrlAgentController::deregisterCommands() {
 CtrlAgentController::deregisterCommands() {
-    CtrlAgentCommandMgr::instance().deregisterCommand(VERSION_GET_COMMAND);
     CtrlAgentCommandMgr::instance().deregisterCommand(BUILD_REPORT_COMMAND);
     CtrlAgentCommandMgr::instance().deregisterCommand(BUILD_REPORT_COMMAND);
+    CtrlAgentCommandMgr::instance().deregisterCommand(CONFIG_GET_COMMAND);
+    CtrlAgentCommandMgr::instance().deregisterCommand(CONFIG_TEST_COMMAND);
+    CtrlAgentCommandMgr::instance().deregisterCommand(CONFIG_WRITE_COMMAND);
     CtrlAgentCommandMgr::instance().deregisterCommand(SHUT_DOWN_COMMAND);
     CtrlAgentCommandMgr::instance().deregisterCommand(SHUT_DOWN_COMMAND);
+    CtrlAgentCommandMgr::instance().deregisterCommand(VERSION_GET_COMMAND);
 }
 }
 
 
 CtrlAgentController::CtrlAgentController()
 CtrlAgentController::CtrlAgentController()

+ 0 - 7
src/bin/agent/ca_controller.h

@@ -48,16 +48,9 @@ public:
     parseFile(const std::string& name);
     parseFile(const std::string& name);
 
 
     /// @brief Register commands.
     /// @brief Register commands.
-    ///
-    /// For all commands in the commands_ set at the exception of
-    /// list-commands register the command with the generic
-    /// @ref isc::process::DControllerBase::executeCommand() handler.
     void registerCommands();
     void registerCommands();
 
 
     /// @brief Deregister commands.
     /// @brief Deregister commands.
-    ///
-    /// For all commands in the commands_ set at the exception of
-    /// list-commands deregister the command.
     void deregisterCommands();
     void deregisterCommands();
 
 
 private:
 private:

+ 67 - 2
src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -6,6 +6,7 @@
 
 
 #include <config.h>
 #include <config.h>
 #include <cc/data.h>
 #include <cc/data.h>
+#include <cc/command_interpreter.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp4/dhcp4_log.h>
 #include <dhcp4/dhcp4_log.h>
 #include <dhcp4/dhcp4to6_ipc.h>
 #include <dhcp4/dhcp4to6_ipc.h>
@@ -148,7 +149,7 @@ ControlledDhcpv4Srv::commandConfigWriteHandler(const string&,
 ConstElementPtr
 ConstElementPtr
 ControlledDhcpv4Srv::commandSetConfigHandler(const string&,
 ControlledDhcpv4Srv::commandSetConfigHandler(const string&,
                                              ConstElementPtr args) {
                                              ConstElementPtr args) {
-    const int status_code = 1; // 1 indicates an error
+    const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
     ConstElementPtr dhcp4;
     ConstElementPtr dhcp4;
     string message;
     string message;
 
 
@@ -207,6 +208,44 @@ ControlledDhcpv4Srv::commandSetConfigHandler(const string&,
 }
 }
 
 
 ConstElementPtr
 ConstElementPtr
+ControlledDhcpv4Srv::commandConfigTestHandler(const string&,
+                                              ConstElementPtr args) {
+    const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
+    ConstElementPtr dhcp4;
+    string message;
+
+    // Command arguments are expected to be:
+    // { "Dhcp4": { ... }, "Logging": { ... } }
+    // The Logging component is technically optional. If it's not supplied
+    // logging will revert to default logging.
+    if (!args) {
+        message = "Missing mandatory 'arguments' parameter.";
+    } else {
+        dhcp4 = args->get("Dhcp4");
+        if (!dhcp4) {
+            message = "Missing mandatory 'Dhcp4' parameter.";
+        } else if (dhcp4->getType() != Element::map) {
+            message = "'Dhcp4' parameter expected to be a map.";
+        }
+    }
+
+    if (!message.empty()) {
+        // Something is amiss with arguments, return a failure response.
+        ConstElementPtr result = isc::config::createAnswer(status_code,
+                                                           message);
+        return (result);
+    }
+
+    // We are starting the configuration process so we should remove any
+    // staging configuration that has been created during previous
+    // configuration attempts.
+    CfgMgr::instance().rollback();
+
+    // Now we check the server proper.
+    return (checkConfig(dhcp4));
+}
+
+ConstElementPtr
 ControlledDhcpv4Srv::commandVersionGetHandler(const string&, ConstElementPtr) {
 ControlledDhcpv4Srv::commandVersionGetHandler(const string&, ConstElementPtr) {
     ElementPtr extended = Element::create(Dhcpv4Srv::getVersion(true));
     ElementPtr extended = Element::create(Dhcpv4Srv::getVersion(true));
     ElementPtr arguments = Element::createMap();
     ElementPtr arguments = Element::createMap();
@@ -228,7 +267,7 @@ ControlledDhcpv4Srv::commandBuildReportHandler(const string&,
 ConstElementPtr
 ConstElementPtr
 ControlledDhcpv4Srv::commandLeasesReclaimHandler(const string&,
 ControlledDhcpv4Srv::commandLeasesReclaimHandler(const string&,
                                                  ConstElementPtr args) {
                                                  ConstElementPtr args) {
-    int status_code = 1;
+    int status_code = CONTROL_RESULT_ERROR;
     string message;
     string message;
 
 
     // args must be { "remove": <bool> }
     // args must be { "remove": <bool> }
@@ -282,6 +321,9 @@ ControlledDhcpv4Srv::processCommand(const string& command,
         } else if (command == "config-get") {
         } else if (command == "config-get") {
             return (srv->commandConfigGetHandler(command, args));
             return (srv->commandConfigGetHandler(command, args));
 
 
+        } else if (command == "config-test") {
+            return (srv->commandConfigTestHandler(command, args));
+
         } else if (command == "version-get") {
         } else if (command == "version-get") {
             return (srv->commandVersionGetHandler(command, args));
             return (srv->commandVersionGetHandler(command, args));
 
 
@@ -414,6 +456,25 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
     return (answer);
     return (answer);
 }
 }
 
 
+isc::data::ConstElementPtr
+ControlledDhcpv4Srv::checkConfig(isc::data::ConstElementPtr config) {
+
+    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_RECEIVED)
+              .arg(config->str());
+
+    ControlledDhcpv4Srv* srv = ControlledDhcpv4Srv::getInstance();
+
+    // Single stream instance used in all error clauses
+    std::ostringstream err;
+
+    if (!srv) {
+        err << "Server object not initialized, can't process config.";
+        return (isc::config::createAnswer(1, err.str()));
+    }
+
+    return (configureDhcp4Server(*srv, config, true));
+}
+
 ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)
 ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)
     : Dhcpv4Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) {
     : Dhcpv4Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) {
     if (getInstance()) {
     if (getInstance()) {
@@ -432,6 +493,9 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)
 
 
     /// @todo: register config-reload (see CtrlDhcpv4Srv::commandConfigReloadHandler)
     /// @todo: register config-reload (see CtrlDhcpv4Srv::commandConfigReloadHandler)
 
 
+    CommandMgr::instance().registerCommand("config-test",
+        boost::bind(&ControlledDhcpv4Srv::commandConfigTestHandler, this, _1, _2));
+
     CommandMgr::instance().registerCommand("config-write",
     CommandMgr::instance().registerCommand("config-write",
         boost::bind(&ControlledDhcpv4Srv::commandConfigWriteHandler, this, _1, _2));
         boost::bind(&ControlledDhcpv4Srv::commandConfigWriteHandler, this, _1, _2));
 
 
@@ -491,6 +555,7 @@ ControlledDhcpv4Srv::~ControlledDhcpv4Srv() {
         // Deregister any registered commands (please keep in alphabetic order)
         // Deregister any registered commands (please keep in alphabetic order)
         CommandMgr::instance().deregisterCommand("build-report");
         CommandMgr::instance().deregisterCommand("build-report");
         CommandMgr::instance().deregisterCommand("config-get");
         CommandMgr::instance().deregisterCommand("config-get");
+        CommandMgr::instance().deregisterCommand("config-test");
         CommandMgr::instance().deregisterCommand("config-write");
         CommandMgr::instance().deregisterCommand("config-write");
         CommandMgr::instance().deregisterCommand("leases-reclaim");
         CommandMgr::instance().deregisterCommand("leases-reclaim");
         CommandMgr::instance().deregisterCommand("libreload");
         CommandMgr::instance().deregisterCommand("libreload");

+ 28 - 2
src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -60,10 +60,12 @@ public:
     /// in them.
     /// in them.
     ///
     ///
     /// Currently supported commands are:
     /// Currently supported commands are:
+    /// - config-reload
+    /// - config-test
     /// - shutdown
     /// - shutdown
     /// - libreload
     /// - libreload
-    /// - config-reload
     /// - leases-reclaim
     /// - leases-reclaim
+    /// ...
     ///
     ///
     /// @note It never throws.
     /// @note It never throws.
     ///
     ///
@@ -89,6 +91,16 @@ public:
     static isc::data::ConstElementPtr
     static isc::data::ConstElementPtr
     processConfig(isc::data::ConstElementPtr new_config);
     processConfig(isc::data::ConstElementPtr new_config);
 
 
+    /// @brief Configuration checker
+    ///
+    /// This is a method for checking incoming configuration.
+    ///
+    /// @param new_config JSON representation of the new configuration
+    ///
+    /// @return status of the config check
+    isc::data::ConstElementPtr
+    checkConfig(isc::data::ConstElementPtr new_config);
+
     /// @brief Returns pointer to the sole instance of Dhcpv4Srv
     /// @brief Returns pointer to the sole instance of Dhcpv4Srv
     ///
     ///
     /// @return server instance (may return NULL, if called before server is spawned)
     /// @return server instance (may return NULL, if called before server is spawned)
@@ -187,7 +199,21 @@ private:
     commandSetConfigHandler(const std::string& command,
     commandSetConfigHandler(const std::string& command,
                             isc::data::ConstElementPtr args);
                             isc::data::ConstElementPtr args);
 
 
-    /// @brief handler for processing 'version-get' command
+    /// @brief handler for processing 'config-test' command
+    ///
+    /// This handler processes config-test command, which checks
+    /// configuration specified in args parameter.
+    /// @param command (parameter ignored)
+    /// @param args configuration to be checked. Expected format:
+    /// map containing Dhcp4 map that contains DHCPv4 server configuration.
+    /// May also contain Logging map that specifies logging configuration.
+    ///
+    /// @return status of the command
+    isc::data::ConstElementPtr
+    commandConfigTestHandler(const std::string& command,
+                             isc::data::ConstElementPtr args);
+
+    /// @Brief handler for processing 'version-get' command
     ///
     ///
     /// This handler processes version-get command, which returns
     /// This handler processes version-get command, which returns
     /// over the control channel the -v and -V command line arguments.
     /// over the control channel the -v and -V command line arguments.

+ 151 - 4
src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -696,7 +696,8 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
 
 
     // Should fail with a syntax error
     // Should fail with a syntax error
     EXPECT_EQ("{ \"result\": 1, "
     EXPECT_EQ("{ \"result\": 1, "
-              "\"text\": \"subnet configuration failed: mandatory 'subnet' parameter is missing for a subnet being configured (<string>:20:17)\" }",
+              "\"text\": \"subnet configuration failed: mandatory 'subnet' "
+              "parameter is missing for a subnet being configured (<string>:20:17)\" }",
               response);
               response);
 
 
     // Check that the config was not lost
     // Check that the config was not lost
@@ -716,13 +717,13 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
         << "}\n"                      // close dhcp4
         << "}\n"                      // close dhcp4
         << "}}";
         << "}}";
 
 
-    /* Verify the control channel socket exists */
+    // Verify the control channel socket exists.
     ASSERT_TRUE(fileExists(socket_path_));
     ASSERT_TRUE(fileExists(socket_path_));
 
 
-    // Send the set-config command
+    // Send the set-config command.
     sendUnixCommand(os.str(), response);
     sendUnixCommand(os.str(), response);
 
 
-    /* Verify the control channel socket no longer exists */
+    // Verify the control channel socket no longer exists.
     EXPECT_FALSE(fileExists(socket_path_));
     EXPECT_FALSE(fileExists(socket_path_));
 
 
     // With no command channel, should still receive the response.
     // With no command channel, should still receive the response.
@@ -790,6 +791,152 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configGet) {
     EXPECT_TRUE(cfg->get("Dhcp4"));
     EXPECT_TRUE(cfg->get("Dhcp4"));
 }
 }
 
 
+// Verify that the "config-test" command will do what we expect.
+TEST_F(CtrlChannelDhcpv4SrvTest, configTest) {
+    createUnixChannelServer();
+
+    // Define strings to permutate the config arguments
+    // (Note the line feeds makes errors easy to find)
+    string set_config_txt = "{ \"command\": \"set-config\" \n";
+    string config_test_txt = "{ \"command\": \"config-test\" \n";
+    string args_txt = " \"arguments\": { \n";
+    string dhcp4_cfg_txt =
+        "    \"Dhcp4\": { \n"
+        "        \"interfaces-config\": { \n"
+        "            \"interfaces\": [\"*\"] \n"
+        "        },   \n"
+        "        \"valid-lifetime\": 4000, \n"
+        "        \"renew-timer\": 1000, \n"
+        "        \"rebind-timer\": 2000, \n"
+        "        \"lease-database\": { \n"
+        "           \"type\": \"memfile\", \n"
+        "           \"persist\":false, \n"
+        "           \"lfc-interval\": 0  \n"
+        "        }, \n"
+        "       \"expired-leases-processing\": { \n"
+        "            \"reclaim-timer-wait-time\": 0, \n"
+        "            \"hold-reclaimed-time\": 0, \n"
+        "            \"flush-reclaimed-timer-wait-time\": 0 \n"
+        "        },"
+        "        \"subnet4\": [ \n";
+    string subnet1 =
+        "               {\"subnet\": \"192.2.0.0/24\", \n"
+        "                \"pools\": [{ \"pool\": \"192.2.0.1-192.2.0.50\" }]}\n";
+    string subnet2 =
+        "               {\"subnet\": \"192.2.1.0/24\", \n"
+        "                \"pools\": [{ \"pool\": \"192.2.1.1-192.2.1.50\" }]}\n";
+    string bad_subnet =
+        "               {\"BOGUS\": \"192.2.2.0/24\", \n"
+        "                \"pools\": [{ \"pool\": \"192.2.2.1-192.2.2.50\" }]}\n";
+    string subnet_footer =
+        "          ] \n";
+    string control_socket_header =
+        "       ,\"control-socket\": { \n"
+        "       \"socket-type\": \"unix\", \n"
+        "       \"socket-name\": \"";
+    string control_socket_footer =
+        "\"   \n} \n";
+    string logger_txt =
+        "    \"Logging\": { \n"
+        "        \"loggers\": [ { \n"
+        "            \"name\": \"kea\", \n"
+        "            \"severity\": \"FATAL\", \n"
+        "            \"output_options\": [{ \n"
+        "                \"output\": \"/dev/null\" \n"
+        "            }] \n"
+        "        }] \n"
+        "    } \n";
+
+    std::ostringstream os;
+
+    // Create a valid config with all the parts should parse
+    os << set_config_txt << ","
+        << args_txt
+        << dhcp4_cfg_txt
+        << subnet1
+        << subnet_footer
+        << control_socket_header
+        << socket_path_
+        << control_socket_footer
+        << "}\n"                      // close dhcp4
+        << ","
+        << logger_txt
+        << "}}";
+
+    // Send the set-config command
+    std::string response;
+    sendUnixCommand(os.str(), response);
+
+    // Verify the configuration was successful.
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
+              response);
+
+    // Check that the config was indeed applied.
+    const Subnet4Collection* subnets =
+        CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Create a config with malformed subnet that should fail to parse.
+    os.str("");
+    os << config_test_txt << ","
+        << args_txt
+        << dhcp4_cfg_txt
+        << bad_subnet
+        << subnet_footer
+        << control_socket_header
+        << socket_path_
+        << control_socket_footer
+        << "}\n"                      // close dhcp4
+        "}}";
+
+    // Send the config-test command
+    sendUnixCommand(os.str(), response);
+
+    // Should fail with a syntax error
+    EXPECT_EQ("{ \"result\": 1, "
+              "\"text\": \"subnet configuration failed: mandatory 'subnet' "
+              "parameter is missing for a subnet being configured (<string>:20:17)\" }",
+              response);
+
+    // Check that the config was not lost
+    subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Create a valid config with two subnets and no command channel.
+    os.str("");
+    os << config_test_txt << ","
+        << args_txt
+        << dhcp4_cfg_txt
+        << subnet1
+        << ",\n"
+        << subnet2
+        << subnet_footer
+        << "}\n"                      // close dhcp4
+        << "}}";
+
+    // Verify the control channel socket exists.
+    ASSERT_TRUE(fileExists(socket_path_));
+
+    // Send the config-test command
+    sendUnixCommand(os.str(), response);
+
+    // Verify the control channel socket still exists.
+    EXPECT_TRUE(fileExists(socket_path_));
+
+    // Verify the configuration was successful.
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. "
+	      "Control-socket, hook-libraries, and D2 configuration were "
+	      "sanity checked, but not applied.\" }",
+              response);
+
+    // Check that the config was not applied
+    subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Clean up after the test.
+    CfgMgr::instance().clear();
+}
+                    
 // Tests if config-write can be called without any parameters.
 // Tests if config-write can be called without any parameters.
 TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigNoFilename) {
 TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigNoFilename) {
     createUnixChannelServer();
     createUnixChannelServer();

+ 70 - 4
src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -6,6 +6,7 @@
 
 
 #include <config.h>
 #include <config.h>
 #include <cc/data.h>
 #include <cc/data.h>
+#include <cc/command_interpreter.h>
 #include <config/command_mgr.h>
 #include <config/command_mgr.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/cfgmgr.h>
@@ -153,7 +154,7 @@ ControlledDhcpv6Srv::commandConfigWriteHandler(const string&, ConstElementPtr ar
 ConstElementPtr
 ConstElementPtr
 ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
 ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
                                              ConstElementPtr args) {
                                              ConstElementPtr args) {
-    const int status_code = 1; // 1 indicates an error
+    const int status_code = CONTROL_RESULT_ERROR;
     ConstElementPtr dhcp6;
     ConstElementPtr dhcp6;
     string message;
     string message;
 
 
@@ -201,7 +202,7 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
     // the logging first in case there's a configuration failure.
     // the logging first in case there's a configuration failure.
     int rcode = 0;
     int rcode = 0;
     isc::config::parseAnswer(rcode, result);
     isc::config::parseAnswer(rcode, result);
-    if (rcode == 0) {
+    if (rcode == CONTROL_RESULT_SUCCESS) {
         CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
         CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
 
 
         // Use new configuration.
         // Use new configuration.
@@ -211,6 +212,43 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
     return (result);
     return (result);
 }
 }
 
 
+ConstElementPtr
+ControlledDhcpv6Srv::commandConfigTestHandler(const string&,
+                                              ConstElementPtr args) {
+    const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
+    ConstElementPtr dhcp6;
+    string message;
+
+    // Command arguments are expected to be:
+    // { "Dhcp6": { ... }, "Logging": { ... } }
+    // The Logging component is technically optional. If it's not supplied
+    // logging will revert to default logging.
+    if (!args) {
+        message = "Missing mandatory 'arguments' parameter.";
+    } else {
+        dhcp6 = args->get("Dhcp6");
+        if (!dhcp6) {
+            message = "Missing mandatory 'Dhcp6' parameter.";
+        } else if (dhcp6->getType() != Element::map) {
+            message = "'Dhcp6' parameter expected to be a map.";
+        }
+    }
+
+    if (!message.empty()) {
+        // Something is amiss with arguments, return a failure response.
+        ConstElementPtr result = isc::config::createAnswer(status_code,
+                                                           message);
+        return (result);
+    }
+
+    // We are starting the configuration process so we should remove any
+    // staging configuration that has been created during previous
+    // configuration attempts.
+    CfgMgr::instance().rollback();
+
+    // Now we check the server proper.
+    return (checkConfig(dhcp6));
+}
 
 
 ConstElementPtr
 ConstElementPtr
 ControlledDhcpv6Srv::commandVersionGetHandler(const string&, ConstElementPtr) {
 ControlledDhcpv6Srv::commandVersionGetHandler(const string&, ConstElementPtr) {
@@ -287,6 +325,9 @@ ControlledDhcpv6Srv::processCommand(const std::string& command,
         } else if (command == "config-get") {
         } else if (command == "config-get") {
             return (srv->commandConfigGetHandler(command, args));
             return (srv->commandConfigGetHandler(command, args));
 
 
+        } else if (command == "config-test") {
+            return (srv->commandConfigTestHandler(command, args));
+
         } else if (command == "version-get") {
         } else if (command == "version-get") {
             return (srv->commandVersionGetHandler(command, args));
             return (srv->commandVersionGetHandler(command, args));
 
 
@@ -319,8 +360,9 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
     ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance();
     ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance();
 
 
     if (!srv) {
     if (!srv) {
-        ConstElementPtr no_srv = isc::config::createAnswer(1,
-          "Server object not initialized, can't process config.");
+        ConstElementPtr no_srv = isc::config::createAnswer(
+            CONTROL_RESULT_ERROR,
+            "Server object not initialized, can't process config.");
         return (no_srv);
         return (no_srv);
     }
     }
 
 
@@ -442,6 +484,26 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
     return (answer);
     return (answer);
 }
 }
 
 
+isc::data::ConstElementPtr
+ControlledDhcpv6Srv::checkConfig(isc::data::ConstElementPtr config) {
+ 
+    LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_RECEIVED)
+        .arg(config->str());
+ 
+    ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance();
+ 
+    // Single stream instance used in all error clauses
+    std::ostringstream err;
+ 
+    if (!srv) {
+        ConstElementPtr no_srv = isc::config::createAnswer(1,
+            "Server object not initialized, can't process config.");
+        return (no_srv);
+    }
+ 
+    return (configureDhcp6Server(*srv, config, true));
+}
+
 ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port)
 ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port)
     : Dhcpv6Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) {
     : Dhcpv6Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) {
     if (server_) {
     if (server_) {
@@ -460,6 +522,9 @@ ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port)
 
 
     /// @todo: register config-reload (see CtrlDhcpv6Srv::commandConfigReloadHandler)
     /// @todo: register config-reload (see CtrlDhcpv6Srv::commandConfigReloadHandler)
 
 
+    CommandMgr::instance().registerCommand("config-test",
+        boost::bind(&ControlledDhcpv6Srv::commandConfigTestHandler, this, _1, _2));
+
     CommandMgr::instance().registerCommand("config-write",
     CommandMgr::instance().registerCommand("config-write",
         boost::bind(&ControlledDhcpv6Srv::commandConfigWriteHandler, this, _1, _2));
         boost::bind(&ControlledDhcpv6Srv::commandConfigWriteHandler, this, _1, _2));
 
 
@@ -518,6 +583,7 @@ ControlledDhcpv6Srv::~ControlledDhcpv6Srv() {
         // Deregister any registered commands (please keep in alphabetic order)
         // Deregister any registered commands (please keep in alphabetic order)
         CommandMgr::instance().deregisterCommand("build-report");
         CommandMgr::instance().deregisterCommand("build-report");
         CommandMgr::instance().deregisterCommand("config-get");
         CommandMgr::instance().deregisterCommand("config-get");
+        CommandMgr::instance().deregisterCommand("config-test");
         CommandMgr::instance().deregisterCommand("config-write");
         CommandMgr::instance().deregisterCommand("config-write");
         CommandMgr::instance().deregisterCommand("leases-reclaim");
         CommandMgr::instance().deregisterCommand("leases-reclaim");
         CommandMgr::instance().deregisterCommand("libreload");
         CommandMgr::instance().deregisterCommand("libreload");

+ 29 - 3
src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -60,10 +60,12 @@ public:
     /// in them.
     /// in them.
     ///
     ///
     /// Currently supported commands are:
     /// Currently supported commands are:
-    /// - shutdown
-    /// - libreload
     /// - config-reload
     /// - config-reload
+    /// - config-test
     /// - leases-reclaim
     /// - leases-reclaim
+    /// - libreload    
+    /// - shutdown
+    /// ...
     ///
     ///
     /// @note It never throws.
     /// @note It never throws.
     ///
     ///
@@ -89,6 +91,16 @@ public:
     static isc::data::ConstElementPtr
     static isc::data::ConstElementPtr
     processConfig(isc::data::ConstElementPtr new_config);
     processConfig(isc::data::ConstElementPtr new_config);
 
 
+    /// @brief Configuration checker
+    ///
+    /// This is a method for checking incoming configuration.
+    ///
+    /// @param new_config JSON representation of the new configuration
+    ///
+    /// @return status of the config check
+    isc::data::ConstElementPtr
+    checkConfig(isc::data::ConstElementPtr new_config);
+
     /// @brief returns pointer to the sole instance of Dhcpv6Srv
     /// @brief returns pointer to the sole instance of Dhcpv6Srv
     ///
     ///
     /// @return server instance (may return NULL, if called before server is spawned)
     /// @return server instance (may return NULL, if called before server is spawned)
@@ -187,7 +199,21 @@ private:
     commandSetConfigHandler(const std::string& command,
     commandSetConfigHandler(const std::string& command,
                             isc::data::ConstElementPtr args);
                             isc::data::ConstElementPtr args);
 
 
-    /// @brief handler for processing 'version-get' command
+    /// @brief handler for processing 'config-test' command
+    ///
+    /// This handler processes config-test command, which checks
+    /// configuration specified in args parameter.
+    /// @param command (parameter ignored)
+    /// @param args configuration to be checked. Expected format:
+    /// map containing Dhcp6 map that contains DHCPv6 server configuration.
+    /// May also contain Logging map that specifies logging configuration.
+    ///
+    /// @return status of the command
+    isc::data::ConstElementPtr
+    commandConfigTestHandler(const std::string& command,
+                             isc::data::ConstElementPtr args);
+
+    /// @Brief handler for processing 'version-get' command
     ///
     ///
     /// This handler processes version-get command, which returns
     /// This handler processes version-get command, which returns
     /// over the control channel the -v and -V command line arguments.
     /// over the control channel the -v and -V command line arguments.

+ 153 - 4
src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc

@@ -430,7 +430,7 @@ TEST_F(CtrlDhcpv6SrvTest, configReload) {
 }
 }
 
 
 // Check that the "set-config" command will replace current configuration
 // Check that the "set-config" command will replace current configuration
-TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
+TEST_F(CtrlChannelDhcpv6SrvTest, configSet) {
     createUnixChannelServer();
     createUnixChannelServer();
 
 
     // Define strings to permutate the config arguments
     // Define strings to permutate the config arguments
@@ -552,13 +552,13 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
         << "}\n"                      // close dhcp6
         << "}\n"                      // close dhcp6
         << "}}";
         << "}}";
 
 
-    /* Verify the control channel socket exists */
+    // Verify the control channel socket exists.
     ASSERT_TRUE(fileExists(socket_path_));
     ASSERT_TRUE(fileExists(socket_path_));
 
 
-    // Send the set-config command
+    // Send the set-config command.
     sendUnixCommand(os.str(), response);
     sendUnixCommand(os.str(), response);
 
 
-    /* Verify the control channel socket no longer exists */
+    // Verify the control channel socket no longer exists.
     EXPECT_FALSE(fileExists(socket_path_));
     EXPECT_FALSE(fileExists(socket_path_));
 
 
     // With no command channel, should still receive the response.
     // With no command channel, should still receive the response.
@@ -573,6 +573,152 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
     CfgMgr::instance().clear();
     CfgMgr::instance().clear();
 }
 }
 
 
+  // Verify that the "config-test" command will do what we expect.
+TEST_F(CtrlChannelDhcpv6SrvTest, configTest) {
+    createUnixChannelServer();
+
+    // Define strings to permutate the config arguments
+    // (Note the line feeds makes errors easy to find)
+    string set_config_txt = "{ \"command\": \"set-config\" \n";
+    string config_test_txt = "{ \"command\": \"config-test\" \n";
+    string args_txt = " \"arguments\": { \n";
+    string dhcp6_cfg_txt =
+        "    \"Dhcp6\": { \n"
+        "        \"interfaces-config\": { \n"
+        "            \"interfaces\": [\"*\"] \n"
+        "        },   \n"
+        "        \"preferred-lifetime\": 3000, \n"
+        "        \"valid-lifetime\": 4000, \n"
+        "        \"renew-timer\": 1000, \n"
+        "        \"rebind-timer\": 2000, \n"
+        "        \"lease-database\": { \n"
+        "           \"type\": \"memfile\", \n"
+        "           \"persist\":false, \n"
+        "           \"lfc-interval\": 0  \n"
+        "        }, \n"
+        "        \"expired-leases-processing\": { \n"
+        "            \"reclaim-timer-wait-time\": 0, \n"
+        "            \"hold-reclaimed-time\": 0, \n"
+        "            \"flush-reclaimed-timer-wait-time\": 0 \n"
+        "        },"
+        "        \"subnet6\": [ \n";
+    string subnet1 =
+        "               {\"subnet\": \"3002::/64\", \n"
+        "                \"pools\": [{ \"pool\": \"3002::100-3002::200\" }]}\n";
+    string subnet2 =
+        "               {\"subnet\": \"3003::/64\", \n"
+        "                \"pools\": [{ \"pool\": \"3003::100-3003::200\" }]}\n";
+    string bad_subnet =
+        "               {\"BOGUS\": \"3005::/64\", \n"
+        "                \"pools\": [{ \"pool\": \"3005::100-3005::200\" }]}\n";
+    string subnet_footer =
+        "          ] \n";
+    string control_socket_header =
+        "       ,\"control-socket\": { \n"
+        "       \"socket-type\": \"unix\", \n"
+        "       \"socket-name\": \"";
+    string control_socket_footer =
+        "\"   \n} \n";
+    string logger_txt =
+        "    \"Logging\": { \n"
+        "        \"loggers\": [ { \n"
+        "            \"name\": \"kea\", \n"
+        "            \"severity\": \"FATAL\", \n"
+        "            \"output_options\": [{ \n"
+        "                \"output\": \"/dev/null\" \n"
+        "            }] \n"
+        "        }] \n"
+        "    } \n";
+
+    std::ostringstream os;
+
+    // Create a valid config with all the parts should parse
+    os << set_config_txt << ","
+        << args_txt
+        << dhcp6_cfg_txt
+        << subnet1
+        << subnet_footer
+        << control_socket_header
+        << socket_path_
+        << control_socket_footer
+        << "}\n"                      // close dhcp6
+        << ","
+        << logger_txt
+        << "}}";
+
+    // Send the set-config command
+    std::string response;
+    sendUnixCommand(os.str(), response);
+
+    // Verify the configuration was successful.
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
+              response);
+
+    // Check that the config was indeed applied.
+    const Subnet6Collection* subnets =
+        CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Create a config with malformed subnet that should fail to parse.
+    os.str("");
+    os << config_test_txt << ","
+        << args_txt
+        << dhcp6_cfg_txt
+        << bad_subnet
+        << subnet_footer
+        << control_socket_header
+        << socket_path_
+        << control_socket_footer
+        << "}\n"                      // close dhcp6
+        "}}";
+
+    // Send the config-test command
+    sendUnixCommand(os.str(), response);
+
+    // Should fail with a syntax error
+    EXPECT_EQ("{ \"result\": 1, "
+              "\"text\": \"subnet configuration failed: mandatory 'subnet' parameter "
+              "is missing for a subnet being configured (<string>:21:17)\" }",
+              response);
+
+    // Check that the config was not lost
+    subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Create a valid config with two subnets and no command channel.
+    os.str("");
+    os << config_test_txt << ","
+        << args_txt
+        << dhcp6_cfg_txt
+        << subnet1
+        << ",\n"
+        << subnet2
+        << subnet_footer
+        << "}\n"                      // close dhcp6
+        << "}}";
+
+    // Verify the control channel socket exists.
+    ASSERT_TRUE(fileExists(socket_path_));
+
+    // Send the config-test command.
+    sendUnixCommand(os.str(), response);
+
+    // Verify the control channel socket still exists.
+    EXPECT_TRUE(fileExists(socket_path_));
+
+    // Verify the configuration was successful.
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. "
+	      "Control-socket, hook-libraries, and D2 configuration were "
+	      "sanity checked, but not applied.\" }",
+              response);
+
+    // Check that the config was not applied.
+    subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Clean up after the test.
+    CfgMgr::instance().clear();
+}
 
 
 typedef std::map<std::string, isc::data::ConstElementPtr> ElementMap;
 typedef std::map<std::string, isc::data::ConstElementPtr> ElementMap;
 
 
@@ -834,12 +980,15 @@ TEST_F(CtrlChannelDhcpv6SrvTest, commandsList) {
     EXPECT_NO_THROW(rsp = Element::fromJSON(response));
     EXPECT_NO_THROW(rsp = Element::fromJSON(response));
 
 
     // We expect the server to report at least the following commands:
     // We expect the server to report at least the following commands:
+    checkListCommands(rsp, "build-report");
     checkListCommands(rsp, "config-get");
     checkListCommands(rsp, "config-get");
+    checkListCommands(rsp, "config-test");
     checkListCommands(rsp, "config-write");
     checkListCommands(rsp, "config-write");
     checkListCommands(rsp, "list-commands");
     checkListCommands(rsp, "list-commands");
     checkListCommands(rsp, "leases-reclaim");
     checkListCommands(rsp, "leases-reclaim");
     checkListCommands(rsp, "libreload");
     checkListCommands(rsp, "libreload");
     checkListCommands(rsp, "set-config");
     checkListCommands(rsp, "set-config");
+    checkListCommands(rsp, "version-get");
     checkListCommands(rsp, "shutdown");
     checkListCommands(rsp, "shutdown");
     checkListCommands(rsp, "statistic-get");
     checkListCommands(rsp, "statistic-get");
     checkListCommands(rsp, "statistic-get-all");
     checkListCommands(rsp, "statistic-get-all");

+ 6 - 5
src/bin/shell/kea-shell.in

@@ -80,17 +80,18 @@ def shell_body():
     params.timeout = cmd_args.timeout
     params.timeout = cmd_args.timeout
     params.version = VERSION
     params.version = VERSION
 
 
-    params.generate_body()
-    params.generate_headers()
-
     # Load command processor
     # Load command processor
     # @todo - command specific processing will be added as part of
     # @todo - command specific processing will be added as part of
     # future work (either #5138 or #5139, whichever is implemented
     # future work (either #5138 or #5139, whichever is implemented
     # first)
     # first)
 
 
-    # Read parameters from stdin (they're optional for some commands)
+    # Read arguments from stdin (they're optional for some commands)
     for line in sys.stdin:
     for line in sys.stdin:
-        params.params += line
+        params.args += line
+
+    # Now we have the arguments so we can build the request
+    params.generate_body()
+    params.generate_headers()
 
 
     # Set the timeout timer. If the connection takes too long,
     # Set the timeout timer. If the connection takes too long,
     # it will send a signal to us.
     # it will send a signal to us.

+ 1 - 1
src/bin/shell/kea-shell.xml

@@ -64,7 +64,7 @@
       The <command>kea-shell</command> provides a REST client for the
       The <command>kea-shell</command> provides a REST client for the
       Kea Control Agent (CA). It takes command as a command-line parameter
       Kea Control Agent (CA). It takes command as a command-line parameter
       that is being sent to CA with proper JSON
       that is being sent to CA with proper JSON
-      encapsulation. Optional parameters may be specified on the
+      encapsulation. Optional arguments may be specified on the
       standard input. The request it sent of HTTP and a response is
       standard input. The request it sent of HTTP and a response is
       retrieved. That response is displayed out on the standard output.
       retrieved. That response is displayed out on the standard output.
     </para>
     </para>

+ 4 - 4
src/bin/shell/kea_conn.py

@@ -17,6 +17,7 @@ class CARequest:
      - http-port - TCP port of the CA
      - http-port - TCP port of the CA
      - command - specifies the command to send (e.g. list-commands)
      - command - specifies the command to send (e.g. list-commands)
      - timeout - timeout (in ms)
      - timeout - timeout (in ms)
+     - args - extra arguments my be added here
      - headers - extra HTTP headers may be added here
      - headers - extra HTTP headers may be added here
      - version - version to be reported in HTTP header
      - version - version to be reported in HTTP header
     """
     """
@@ -25,7 +26,7 @@ class CARequest:
     http_port = 0
     http_port = 0
     command = ''
     command = ''
     timeout = 0
     timeout = 0
-    params = ''
+    args = ''
     headers = {}
     headers = {}
     version = ""
     version = ""
     # This is a storage for generated command (input data to be sent over POST)
     # This is a storage for generated command (input data to be sent over POST)
@@ -35,12 +36,11 @@ class CARequest:
         """
         """
         Generates the content, out of specified command line
         Generates the content, out of specified command line
         and optional content.
         and optional content.
-        @todo: Add support for parameters
         this stores the output in self.content
         this stores the output in self.content
         """
         """
         self.content = '{ "command": "' + self.command + '"'
         self.content = '{ "command": "' + self.command + '"'
-        if len(self.params):
-            self.content += ', "parameters": { ' + self.params + ' }'
+        if len(self.args) > 1:
+            self.content += ', "arguments": { ' + self.args + ' }'
         self.content += ' }'
         self.content += ' }'
 
 
     def generate_headers(self):
     def generate_headers(self):

+ 0 - 1
src/bin/shell/tests/.gitignore

@@ -1,2 +1 @@
 shell_process_tests.sh
 shell_process_tests.sh
-shell_unittest.py

+ 16 - 5
src/bin/shell/tests/shell_process_tests.sh.in

@@ -111,7 +111,7 @@ shell_command_test() {
     shell_exit_code=$?
     shell_exit_code=$?
     if [ ${shell_exit_code} -ne 0 ]; then
     if [ ${shell_exit_code} -ne 0 ]; then
         echo "ERROR:" \
         echo "ERROR:" \
-	"kea-shell returned ${shell_exit_code} exit code,  expected 0."
+        "kea-shell returned ${shell_exit_code} exit code,  expected 0."
     else
     else
         echo "kea-shell returned ${shell_exit_code} exit code as expected."
         echo "kea-shell returned ${shell_exit_code} exit code as expected."
     fi
     fi
@@ -123,8 +123,8 @@ shell_command_test() {
     diff_code=$?
     diff_code=$?
     if [ ${diff_code} -ne 0 ]; then
     if [ ${diff_code} -ne 0 ]; then
         echo "ERROR:" \
         echo "ERROR:" \
-	"content returned is different than expected." \
-	"See ${tmpfile_path}/shell-*.txt"
+        "content returned is different than expected." \
+        "See ${tmpfile_path}/shell-*.txt"
         echo "EXPECTED:"
         echo "EXPECTED:"
         cat ${tmpfile_path}/shell-expected.txt
         cat ${tmpfile_path}/shell-expected.txt
         echo "ACTUAL RESULT:"
         echo "ACTUAL RESULT:"
@@ -171,13 +171,24 @@ version_test() {
         test_finish 0
         test_finish 0
     else
     else
         echo "ERROR:" \
         echo "ERROR:" \
-	"Expected version ${EXPECTED_VERSION}, got ${REPORTED_VERSION}"
+        "Expected version ${EXPECTED_VERSION}, got ${REPORTED_VERSION}"
         test_finish 1
         test_finish 1
     fi
     fi
 }
 }
 
 
 version_test "shell.version"
 version_test "shell.version"
 shell_command_test "shell.list-commands" "list-commands" \
 shell_command_test "shell.list-commands" "list-commands" \
-    "[ { \"arguments\": [ \"build-report\", \"list-commands\", \"shutdown\", \"version-get\" ], \"result\": 0 } ]" ""
+    "[ { \"arguments\": [ \"build-report\", \"config-get\", \"config-test\", \"config-write\", \"list-commands\", \"shutdown\", \"version-get\" ], \"result\": 0 } ]" ""
 shell_command_test "shell.bogus" "give-me-a-beer" \
 shell_command_test "shell.bogus" "give-me-a-beer" \
     "[ { \"result\": 1, \"text\": \"'give-me-a-beer' command not supported.\" } ]" ""
     "[ { \"result\": 1, \"text\": \"'give-me-a-beer' command not supported.\" } ]" ""
+shell_command_test "shell.empty-config-test" "config-test" \
+    "[ { \"result\": 1, \"text\": \"Missing mandatory 'arguments' parameter.\" } ]" ""
+shell_command_test "shell.no-app-config-test" "config-test" \
+    "[ { \"result\": 1, \"text\": \"Missing mandatory 'Control-agent' parameter.\" } ]" \
+    "\"FooBar\": { }"
+shell_command_test "shell.no-map-config-test" "config-test" \
+    "[ { \"result\": 1, \"text\": \"'Control-agent' parameter expected to be a map.\" } ]" \
+    "\"Control-agent\": [ ]"
+shell_command_test "shell.bad-value-config-test" "config-test" \
+    "[ { \"result\": 2, \"text\": \"out of range value (80000) specified for parameter 'http-port' (<string>:1:76)\" } ]" \
+    "\"Control-agent\": { \"http-port\": 80000 }"

+ 6 - 6
src/bin/shell/tests/shell_unittest.py.in

@@ -26,27 +26,27 @@ class CARequestUnitTest(unittest.TestCase):
         """
         """
         pass
         pass
 
 
-    def test_body_without_params(self):
+    def test_body_without_args(self):
         """
         """
         This test verifies if the CARequest object generates the request
         This test verifies if the CARequest object generates the request
-        content properly when there are no parameters.
+        content properly when there are no arguments.
         """
         """
         request = CARequest()
         request = CARequest()
         request.command = "foo"
         request.command = "foo"
         request.generate_body()
         request.generate_body()
         self.assertEqual(request.content, '{ "command": "foo" }')
         self.assertEqual(request.content, '{ "command": "foo" }')
 
 
-    def test_body_with_params(self):
+    def test_body_with_args(self):
         """
         """
         This test verifies if the CARequest object generates the request
         This test verifies if the CARequest object generates the request
-        content properly when there are parameters.
+        content properly when there are arguments.
         """
         """
         request = CARequest()
         request = CARequest()
         request.command = "foo"
         request.command = "foo"
-        request.params = '"bar": "baz"'
+        request.args = '"bar": "baz"'
         request.generate_body()
         request.generate_body()
         self.assertEqual(request.content,
         self.assertEqual(request.content,
-                         '{ "command": "foo", "parameters": { "bar": "baz" } }')
+                         '{ "command": "foo", "arguments": { "bar": "baz" } }')
 
 
     @staticmethod
     @staticmethod
     def check_header(headers, header_name, value):
     def check_header(headers, header_name, value):

+ 121 - 1
src/lib/process/d_controller.cc

@@ -409,7 +409,127 @@ DControllerBase::checkConfig(ConstElementPtr new_config) {
 }
 }
 
 
 ConstElementPtr
 ConstElementPtr
-DControllerBase::versionGetHandler(const std::string&, ConstElementPtr) {
+DControllerBase::configGetHandler(const std::string&,
+                                  ConstElementPtr /*args*/) {
+    ConstElementPtr config = process_->getCfgMgr()->getContext()->toElement();
+
+    return (createAnswer(COMMAND_SUCCESS, config));
+}
+
+ConstElementPtr
+DControllerBase::configWriteHandler(const std::string&,
+                                    ConstElementPtr args) {
+    std::string filename;
+
+    if (args) {
+        if (args->getType() != Element::map) {
+            return (createAnswer(COMMAND_ERROR, "Argument must be a map"));
+        }
+        ConstElementPtr filename_param = args->get("filename");
+        if (filename_param) {
+            if (filename_param->getType() != Element::string) {
+                return (createAnswer(COMMAND_ERROR,
+                                     "passed parameter 'filename' "
+                                     "is not a string"));
+            }
+            filename = filename_param->stringValue();
+        }
+    }
+
+    if (filename.empty()) {
+        // filename parameter was not specified, so let's use
+        // whatever we remember
+        filename = getConfigFile();
+        if (filename.empty()) {
+            return (createAnswer(COMMAND_ERROR,
+                                 "Unable to determine filename."
+                                 "Please specify filename explicitly."));
+        }
+    }
+
+    // Now do the sanity checks on the filename
+    if (filename.find("..") != std::string::npos) {
+        // Trying to escape the directory.. nope.
+        return (createAnswer(COMMAND_ERROR,
+                             "Using '..' in filename is not allowed."));
+    }
+
+    if (filename.find("\\") != std::string::npos) {
+        // Trying to inject escapes (possibly to inject quotes and something
+        // nasty afterward)
+        return (createAnswer(COMMAND_ERROR,
+                             "Using \\ in filename is not allowed."));
+    }
+
+    if (filename[0] == '/') {
+        // Absolute paths are not allowed.
+        return (createAnswer(COMMAND_ERROR,
+                             "Absolute path in filename is not allowed."));
+    }
+
+    // Ok, it's time to write the file.
+    size_t size = 0;
+    try {
+        size = writeConfigFile(filename);
+    } catch (const isc::Exception& ex) {
+        return (createAnswer(COMMAND_ERROR,
+                             std::string("Error during write-config:")
+                             + ex.what()));
+    }
+    if (size == 0) {
+        return (createAnswer(COMMAND_ERROR,
+                             "Error writing configuration to " + filename));
+    }
+
+    // Ok, it's time to return the successful response.
+    ElementPtr params = Element::createMap();
+    params->set("size", Element::create(static_cast<long long>(size)));
+    params->set("filename", Element::create(filename));
+
+    return (createAnswer(CONTROL_RESULT_SUCCESS, "Configuration written to "
+                         + filename + " successful", params));
+}
+
+ConstElementPtr
+DControllerBase::configTestHandler(const std::string&, ConstElementPtr args) {
+    const int status_code = COMMAND_ERROR; // 1 indicates an error
+    ConstElementPtr module_config;
+    std::string app_name = getAppName();
+    std::string message;
+
+    // Command arguments are expected to be:
+    // { "Module": { ... }, "Logging": { ... } }
+    // The Logging component is technically optional. If it's not supplied
+    // logging will revert to default logging.
+    if (!args) {
+        message = "Missing mandatory 'arguments' parameter.";
+    } else {
+      module_config = args->get(app_name);
+        if (!module_config) {
+            message = "Missing mandatory '" + app_name + "' parameter.";
+        } else if (module_config->getType() != Element::map) {
+            message = "'" + app_name + "' parameter expected to be a map.";
+        }
+    }
+
+    if (!message.empty()) {
+        // Something is amiss with arguments, return a failure response.
+        ConstElementPtr result = isc::config::createAnswer(status_code,
+                                                           message);
+        return (result);
+    }
+
+    // We are starting the configuration process so we should remove any
+    // staging configuration that has been created during previous
+    // configuration attempts.
+    isc::dhcp::CfgMgr::instance().rollback();
+
+    // Now we check the server proper.
+    return (checkConfig(module_config));
+}
+
+ConstElementPtr
+DControllerBase::versionGetHandler(const std::string&, ConstElementPtr args) {
     ConstElementPtr answer;
     ConstElementPtr answer;
 
 
     // For version-get put the extended version in arguments
     // For version-get put the extended version in arguments

+ 41 - 0
src/lib/process/d_controller.h

@@ -256,6 +256,47 @@ public:
     buildReportHandler(const std::string& command,
     buildReportHandler(const std::string& command,
                        isc::data::ConstElementPtr args);
                        isc::data::ConstElementPtr args);
 
 
+    /// @brief handler for config-get command
+    ///
+    /// This method handles the config-get command, which retrieves
+    /// the current configuration and returns it in response.
+    ///
+    /// @param command (ignored)
+    /// @param args (ignored)
+    /// @return current configuration wrapped in a response
+    isc::data::ConstElementPtr
+    configGetHandler(const std::string& command,
+                     isc::data::ConstElementPtr args);
+
+    /// @brief handler for config-write command
+    ///
+    /// This handle processes write-config comamnd, which writes the
+    /// current configuration to disk. This command takes one optional
+    /// parameter called filename. If specified, the current configuration
+    /// will be written to that file. If not specified, the file used during
+    /// Kea start-up will be used. To avoid any exploits, the path is
+    /// always relative and .. is not allowed in the filename. This is
+    /// a security measure against exploiting file writes remotely.
+    ///
+    /// @param command (ignored)
+    /// @param args may contain optional string argument filename
+    /// @return status of the configuration file write
+    isc::data::ConstElementPtr
+    configWriteHandler(const std::string& command,
+                       isc::data::ConstElementPtr args);
+
+    /// @brief handler for config-test command
+    ///
+    /// This method handles the config-test command, which checks
+    /// configuration specified in args parameter.
+    ///
+    /// @param command (ignored)
+    /// @param args configuration to be checked.
+    /// @return status of the command
+    isc::data::ConstElementPtr
+    configTestHandler(const std::string& command,
+                      isc::data::ConstElementPtr args);
+
     /// @brief handler for 'shutdown' command
     /// @brief handler for 'shutdown' command
     ///
     ///
     /// This method handles shutdown command. It initiates the shutdown procedure
     /// This method handles shutdown command. It initiates the shutdown procedure

+ 9 - 0
src/lib/process/d_process.h

@@ -31,6 +31,15 @@ static const std::string VERSION_GET_COMMAND("version-get");
 /// @brief String value for the build-report command.
 /// @brief String value for the build-report command.
 static const std::string BUILD_REPORT_COMMAND("build-report");
 static const std::string BUILD_REPORT_COMMAND("build-report");
 
 
+/// @brief String value for the config-get command.
+static const std::string CONFIG_GET_COMMAND("config-get");
+
+/// @brief String value for the config-write command.
+static const std::string CONFIG_WRITE_COMMAND("config-write");
+
+/// @brief String value for the config-test command.
+static const std::string CONFIG_TEST_COMMAND("config-test");
+
 /// @brief String value for the shutdown command.
 /// @brief String value for the shutdown command.
 static const std::string SHUT_DOWN_COMMAND("shutdown");
 static const std::string SHUT_DOWN_COMMAND("shutdown");
 
 

+ 0 - 42
src/lib/process/testutils/d_test_stubs.cc

@@ -19,9 +19,6 @@ namespace process {
 // Initialize the static failure flag.
 // Initialize the static failure flag.
 SimFailure::FailureType SimFailure::failure_type_ = SimFailure::ftNoFailure;
 SimFailure::FailureType SimFailure::failure_type_ = SimFailure::ftNoFailure;
 
 
-// Define custom process command supported by DStubProcess.
-const char*  DStubProcess::stub_proc_command_("cool_proc_cmd");
-
 DStubProcess::DStubProcess(const char* name, asiolink::IOServicePtr io_service)
 DStubProcess::DStubProcess(const char* name, asiolink::IOServicePtr io_service)
     : DProcessBase(name, io_service, DCfgMgrBasePtr(new DStubCfgMgr())) {
     : DProcessBase(name, io_service, DCfgMgrBasePtr(new DStubCfgMgr())) {
 };
 };
@@ -77,32 +74,11 @@ DStubProcess::configure(isc::data::ConstElementPtr config_set, bool check_only)
     return (getCfgMgr()->parseConfig(config_set, check_only));
     return (getCfgMgr()->parseConfig(config_set, check_only));
 }
 }
 
 
-isc::data::ConstElementPtr
-DStubProcess::command(const std::string& command,
-                      isc::data::ConstElementPtr /* args */) {
-    isc::data::ConstElementPtr answer;
-    if (SimFailure::shouldFailOn(SimFailure::ftProcessCommand)) {
-        // Simulates a process command execution failure.
-        answer = isc::config::createAnswer(COMMAND_ERROR,
-                                          "SimFailure::ftProcessCommand");
-    } else if (command.compare(stub_proc_command_) == 0) {
-        answer = isc::config::createAnswer(COMMAND_SUCCESS, "Command accepted");
-    } else {
-        answer = isc::config::createAnswer(COMMAND_INVALID,
-                                           "Unrecognized command:" + command);
-    }
-
-    return (answer);
-}
-
 DStubProcess::~DStubProcess() {
 DStubProcess::~DStubProcess() {
 };
 };
 
 
 //************************** DStubController *************************
 //************************** DStubController *************************
 
 
-// Define custom controller command supported by DStubController.
-const char* DStubController::stub_ctl_command_("spiffy");
-
 // Define custom command line option command supported by DStubController.
 // Define custom command line option command supported by DStubController.
 const char* DStubController::stub_option_x_ = "x";
 const char* DStubController::stub_option_x_ = "x";
 
 
@@ -162,24 +138,6 @@ DProcessBase* DStubController::createProcess() {
     return (new DStubProcess(getAppName().c_str(), getIOService()));
     return (new DStubProcess(getAppName().c_str(), getIOService()));
 }
 }
 
 
-isc::data::ConstElementPtr
-DStubController::customControllerCommand(const std::string& command,
-                                     isc::data::ConstElementPtr /* args */) {
-    isc::data::ConstElementPtr answer;
-    if (SimFailure::shouldFailOn(SimFailure::ftControllerCommand)) {
-        // Simulates command failing to execute.
-        answer = isc::config::createAnswer(COMMAND_ERROR,
-                                          "SimFailure::ftControllerCommand");
-    } else if (command.compare(stub_ctl_command_) == 0) {
-        answer = isc::config::createAnswer(COMMAND_SUCCESS, "Command accepted");
-    } else {
-        answer = isc::config::createAnswer(COMMAND_INVALID,
-                                           "Unrecognized command:" + command);
-    }
-
-    return (answer);
-}
-
 const std::string DStubController::getCustomOpts() const {
 const std::string DStubController::getCustomOpts() const {
     // Return the "list" of custom options supported by DStubController.
     // Return the "list" of custom options supported by DStubController.
     return (std::string(stub_option_x_));
     return (std::string(stub_option_x_));

+ 0 - 44
src/lib/process/testutils/d_test_stubs.h

@@ -45,8 +45,6 @@ public:
         ftCreateProcessNull,
         ftCreateProcessNull,
         ftProcessInit,
         ftProcessInit,
         ftProcessConfigure,
         ftProcessConfigure,
-        ftControllerCommand,
-        ftProcessCommand,
         ftProcessShutdown,
         ftProcessShutdown,
         ftElementBuild,
         ftElementBuild,
         ftElementCommit,
         ftElementCommit,
@@ -104,9 +102,6 @@ public:
 class DStubProcess : public DProcessBase {
 class DStubProcess : public DProcessBase {
 public:
 public:
 
 
-    /// @brief Static constant that defines a custom process command string.
-    static const char* stub_proc_command_;
-
     /// @brief Constructor
     /// @brief Constructor
     ///
     ///
     /// @param name name is a text label for the process. Generally used
     /// @param name name is a text label for the process. Generally used
@@ -152,24 +147,6 @@ public:
     virtual isc::data::ConstElementPtr
     virtual isc::data::ConstElementPtr
     configure(isc::data::ConstElementPtr config_set, bool check_only);
     configure(isc::data::ConstElementPtr config_set, bool check_only);
 
 
-    /// @brief Executes the given command.
-    ///
-    /// This implementation will recognizes one "custom" process command,
-    /// stub_proc_command_.  It will fail if SimFailure is set to
-    /// ftProcessCommand.
-    ///
-    /// @param command is a string label representing the command to execute.
-    /// @param args is a set of arguments (if any) required for the given
-    /// command.
-    /// @return an Element that contains the results of command composed
-    /// of an integer status value and a string explanation of the outcome.
-    /// The status value is:
-    /// COMMAND_SUCCESS if the command is recognized and executes successfully.
-    /// COMMAND_ERROR if the command is recognized but fails to execute.
-    /// COMMAND_INVALID if the command is not recognized.
-    virtual isc::data::ConstElementPtr command(const std::string& command,
-                                               isc::data::ConstElementPtr args);
-
     /// @brief Returns configuration summary in the textual format.
     /// @brief Returns configuration summary in the textual format.
     ///
     ///
     /// @return Always an empty string.
     /// @return Always an empty string.
@@ -198,10 +175,6 @@ public:
     /// @return returns a pointer reference to the singleton instance.
     /// @return returns a pointer reference to the singleton instance.
     static DControllerBasePtr& instance();
     static DControllerBasePtr& instance();
 
 
-    /// @brief Defines a custom controller command string. This is a
-    /// custom command supported by DStubController.
-    static const char* stub_ctl_command_;
-
     /// @brief Defines a custom command line option supported by
     /// @brief Defines a custom command line option supported by
     /// DStubController.
     /// DStubController.
     static const char* stub_option_x_;
     static const char* stub_option_x_;
@@ -266,23 +239,6 @@ protected:
     /// ftCreateProcessException.
     /// ftCreateProcessException.
     virtual DProcessBase* createProcess();
     virtual DProcessBase* createProcess();
 
 
-    /// @brief Executes custom controller commands are supported by
-    /// DStubController. This implementation supports one custom controller
-    /// command, stub_ctl_command_.  It will fail if SimFailure is set
-    /// to ftControllerCommand.
-    ///
-    /// @param command is a string label representing the command to execute.
-    /// @param args is a set of arguments (if any) required for the given
-    /// command.
-    /// @return an Element that contains the results of command composed
-    /// of an integer status value and a string explanation of the outcome.
-    /// The status value is:
-    /// COMMAND_SUCCESS if the command is recognized and executes successfully.
-    /// COMMAND_ERROR if the command is recognized but fails to execute.
-    /// COMMAND_INVALID if the command is not recognized.
-    virtual isc::data::ConstElementPtr customControllerCommand(
-            const std::string& command, isc::data::ConstElementPtr args);
-
     /// @brief Provides a string of the additional command line options
     /// @brief Provides a string of the additional command line options
     /// supported by DStubController.  DStubController supports one
     /// supported by DStubController.  DStubController supports one
     /// addition option, stub_option_x_.
     /// addition option, stub_option_x_.