diff options
| -rw-r--r-- | doc/guide/ctrl-channel.xml | 59 | ||||
| -rw-r--r-- | doc/guide/dhcp4-srv.xml | 1 | ||||
| -rw-r--r-- | doc/guide/dhcp6-srv.xml | 1 | ||||
| -rw-r--r-- | src/bin/agent/ca_controller.cc | 20 | ||||
| -rw-r--r-- | src/bin/agent/ca_controller.h | 7 | ||||
| -rw-r--r-- | src/bin/dhcp4/ctrl_dhcp4_srv.cc | 64 | ||||
| -rw-r--r-- | src/bin/dhcp4/ctrl_dhcp4_srv.h | 27 | ||||
| -rw-r--r-- | src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc | 143 | ||||
| -rw-r--r-- | src/bin/dhcp6/ctrl_dhcp6_srv.cc | 64 | ||||
| -rw-r--r-- | src/bin/dhcp6/ctrl_dhcp6_srv.h | 27 | ||||
| -rw-r--r-- | src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 147 | ||||
| -rw-r--r-- | src/bin/shell/tests/shell_process_tests.sh.in | 2 | ||||
| -rw-r--r-- | src/lib/process/d_controller.cc | 119 | ||||
| -rw-r--r-- | src/lib/process/d_controller.h | 41 | ||||
| -rw-r--r-- | src/lib/process/d_process.h | 9 | ||||
| -rw-r--r-- | src/lib/process/testutils/d_test_stubs.cc | 42 | ||||
| -rw-r--r-- | src/lib/process/testutils/d_test_stubs.h | 44 |
17 files changed, 715 insertions, 102 deletions
diff --git a/doc/guide/ctrl-channel.xml b/doc/guide/ctrl-channel.xml index abfefcdd4d..266282ff39 100644 --- a/doc/guide/ctrl-channel.xml +++ b/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> </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": { + "<server>": { + }, + "Logging": { + } + } +} +</screen> + <para> + where <server> 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 (<string>:16:26)" } +</screen> + </para> + </section> <!-- end of command-config-test --> <section id="command-config-write"> <title>config-write</title> @@ -178,7 +233,7 @@ will be sent to Kea and the responses received from Kea printed to standard outp } </screen> </para> - </section> + </section> <!-- end of command-config-write --> <section id="command-leases-reclaim"> <title>leases-reclaim</title> diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index 71a032a01e..70089c1fba 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -3725,6 +3725,7 @@ src/lib/dhcpsrv/cfg_host_operations.cc --> <itemizedlist> <listitem>build-report</listitem> <listitem>config-get</listitem> + <listitem>config-test</listitem> <listitem>config-write</listitem> <listitem>leases-reclaim</listitem> <listitem>list-commands</listitem> diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index e811d97b58..618d4092dc 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -4133,6 +4133,7 @@ If not specified, the default value is: <itemizedlist> <listitem>build-report</listitem> <listitem>config-get</listitem> + <listitem>config-test</listitem> <listitem>config-write</listitem> <listitem>leases-reclaim</listitem> <listitem>list-commands</listitem> diff --git a/src/bin/agent/ca_controller.cc b/src/bin/agent/ca_controller.cc index 38ae98388e..a5fce22fd1 100644 --- a/src/bin/agent/ca_controller.cc +++ b/src/bin/agent/ca_controller.cc @@ -50,21 +50,33 @@ CtrlAgentController::parseFile(const std::string& name) { void CtrlAgentController::registerCommands() { - CtrlAgentCommandMgr::instance().registerCommand(VERSION_GET_COMMAND, - boost::bind(&DControllerBase::versionGetHandler, this, _1, _2)); - CtrlAgentCommandMgr::instance().registerCommand(BUILD_REPORT_COMMAND, 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, boost::bind(&DControllerBase::shutdownHandler, this, _1, _2)); + + CtrlAgentCommandMgr::instance().registerCommand(VERSION_GET_COMMAND, + boost::bind(&DControllerBase::versionGetHandler, this, _1, _2)); } void CtrlAgentController::deregisterCommands() { - CtrlAgentCommandMgr::instance().deregisterCommand(VERSION_GET_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(VERSION_GET_COMMAND); } CtrlAgentController::CtrlAgentController() diff --git a/src/bin/agent/ca_controller.h b/src/bin/agent/ca_controller.h index 179d35d710..2cca081298 100644 --- a/src/bin/agent/ca_controller.h +++ b/src/bin/agent/ca_controller.h @@ -48,16 +48,9 @@ public: parseFile(const std::string& name); /// @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(); /// @brief Deregister commands. - /// - /// For all commands in the commands_ set at the exception of - /// list-commands deregister the command. void deregisterCommands(); private: diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 30872a653c..7134bc4b3d 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -207,6 +207,44 @@ ControlledDhcpv4Srv::commandSetConfigHandler(const string&, } ConstElementPtr +ControlledDhcpv4Srv::commandConfigTestHandler(const string&, + ConstElementPtr args) { + const int status_code = 1; // 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) { ElementPtr extended = Element::create(Dhcpv4Srv::getVersion(true)); ElementPtr arguments = Element::createMap(); @@ -282,6 +320,9 @@ ControlledDhcpv4Srv::processCommand(const string& command, } else if (command == "config-get") { return (srv->commandConfigGetHandler(command, args)); + } else if (command == "config-test") { + return (srv->commandConfigTestHandler(command, args)); + } else if (command == "version-get") { return (srv->commandVersionGetHandler(command, args)); @@ -414,6 +455,25 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { 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*/) : Dhcpv4Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) { if (getInstance()) { @@ -432,6 +492,9 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/) /// @todo: register config-reload (see CtrlDhcpv4Srv::commandConfigReloadHandler) + CommandMgr::instance().registerCommand("config-test", + boost::bind(&ControlledDhcpv4Srv::commandConfigTestHandler, this, _1, _2)); + CommandMgr::instance().registerCommand("config-write", boost::bind(&ControlledDhcpv4Srv::commandConfigWriteHandler, this, _1, _2)); @@ -491,6 +554,7 @@ ControlledDhcpv4Srv::~ControlledDhcpv4Srv() { // Deregister any registered commands (please keep in alphabetic order) CommandMgr::instance().deregisterCommand("build-report"); CommandMgr::instance().deregisterCommand("config-get"); + CommandMgr::instance().deregisterCommand("config-test"); CommandMgr::instance().deregisterCommand("config-write"); CommandMgr::instance().deregisterCommand("leases-reclaim"); CommandMgr::instance().deregisterCommand("libreload"); diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 163f167e5c..a245e02fe3 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -64,6 +64,7 @@ public: /// - libreload /// - config-reload /// - leases-reclaim + /// ... /// /// @note It never throws. /// @@ -89,6 +90,16 @@ public: static isc::data::ConstElementPtr processConfig(isc::data::ConstElementPtr new_config); + /// @brief Configuration checker + /// + /// This is a method for checking incoming configuration. + /// + /// @param new_config textual 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 /// /// @return server instance (may return NULL, if called before server is spawned) @@ -187,7 +198,21 @@ private: commandSetConfigHandler(const std::string& command, 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 /// over the control channel the -v and -V command line arguments. diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index e6554c2eec..496634182d 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -790,6 +790,149 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configGet) { 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. TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigNoFilename) { createUnixChannelServer(); diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 10363c30b8..8f5b1cffc8 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -211,6 +211,43 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&, return (result); } +ConstElementPtr +ControlledDhcpv6Srv::commandConfigTestHandler(const string&, + ConstElementPtr args) { + const int status_code = 1; // 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 ControlledDhcpv6Srv::commandVersionGetHandler(const string&, ConstElementPtr) { @@ -287,6 +324,9 @@ ControlledDhcpv6Srv::processCommand(const std::string& command, } else if (command == "config-get") { return (srv->commandConfigGetHandler(command, args)); + } else if (command == "config-test") { + return (srv->commandConfigTestHandler(command, args)); + } else if (command == "version-get") { return (srv->commandVersionGetHandler(command, args)); @@ -442,6 +482,26 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { 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) : Dhcpv6Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) { if (server_) { @@ -460,6 +520,9 @@ ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port) /// @todo: register config-reload (see CtrlDhcpv6Srv::commandConfigReloadHandler) + CommandMgr::instance().registerCommand("config-test", + boost::bind(&ControlledDhcpv6Srv::commandConfigTestHandler, this, _1, _2)); + CommandMgr::instance().registerCommand("config-write", boost::bind(&ControlledDhcpv6Srv::commandConfigWriteHandler, this, _1, _2)); @@ -518,6 +581,7 @@ ControlledDhcpv6Srv::~ControlledDhcpv6Srv() { // Deregister any registered commands (please keep in alphabetic order) CommandMgr::instance().deregisterCommand("build-report"); CommandMgr::instance().deregisterCommand("config-get"); + CommandMgr::instance().deregisterCommand("config-test"); CommandMgr::instance().deregisterCommand("config-write"); CommandMgr::instance().deregisterCommand("leases-reclaim"); CommandMgr::instance().deregisterCommand("libreload"); diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index 3749fbc580..6037048149 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -64,6 +64,7 @@ public: /// - libreload /// - config-reload /// - leases-reclaim + /// ... /// /// @note It never throws. /// @@ -89,6 +90,16 @@ public: static isc::data::ConstElementPtr processConfig(isc::data::ConstElementPtr new_config); + /// @brief Configuration checker + /// + /// This is a method for checking incoming configuration. + /// + /// @param new_config textual 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 /// /// @return server instance (may return NULL, if called before server is spawned) @@ -187,7 +198,21 @@ private: commandSetConfigHandler(const std::string& command, 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 /// over the control channel the -v and -V command line arguments. diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index c7efdcaf97..eebfa4ac5f 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -573,6 +573,150 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) { CfgMgr::instance().clear(); } + // Verify that the "config-test" command will do what we expect. +TEST_F(CtrlChannelDhcpv6SrvTest, config_test) { + 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; @@ -834,12 +978,15 @@ TEST_F(CtrlChannelDhcpv6SrvTest, commandsList) { EXPECT_NO_THROW(rsp = Element::fromJSON(response)); // We expect the server to report at least the following commands: + checkListCommands(rsp, "build-report"); checkListCommands(rsp, "config-get"); + checkListCommands(rsp, "config-test"); checkListCommands(rsp, "config-write"); checkListCommands(rsp, "list-commands"); checkListCommands(rsp, "leases-reclaim"); checkListCommands(rsp, "libreload"); checkListCommands(rsp, "set-config"); + checkListCommands(rsp, "version-get"); checkListCommands(rsp, "shutdown"); checkListCommands(rsp, "statistic-get"); checkListCommands(rsp, "statistic-get-all"); diff --git a/src/bin/shell/tests/shell_process_tests.sh.in b/src/bin/shell/tests/shell_process_tests.sh.in index a220640eec..746b2f5aae 100644 --- a/src/bin/shell/tests/shell_process_tests.sh.in +++ b/src/bin/shell/tests/shell_process_tests.sh.in @@ -178,6 +178,6 @@ version_test() { version_test "shell.version" 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" \ "[ { \"result\": 1, \"text\": \"'give-me-a-beer' command not supported.\" } ]" "" diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc index 974e178392..5b112d4331 100644 --- a/src/lib/process/d_controller.cc +++ b/src/lib/process/d_controller.cc @@ -406,6 +406,125 @@ DControllerBase::checkConfig(ConstElementPtr new_config) { } 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 = CONTROL_RESULT_SUCCESS; // 1 indicates an error + ConstElementPtr dhcp4; + std::string message; + + // Command arguments are expected to be: + // { "Dhcp4": { ... }, "Logging": { ... } } + // The Lnogging 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. + isc::dhcp::CfgMgr::instance().rollback(); + + // Now we check the server proper. + return (checkConfig(dhcp4)); +} + +ConstElementPtr DControllerBase::versionGetHandler(const std::string&, ConstElementPtr args) { ConstElementPtr answer; diff --git a/src/lib/process/d_controller.h b/src/lib/process/d_controller.h index f4b536202b..c7c3aa11fe 100644 --- a/src/lib/process/d_controller.h +++ b/src/lib/process/d_controller.h @@ -256,6 +256,47 @@ public: buildReportHandler(const std::string& command, 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 /// /// This method handles shutdown command. It initiates the shutdown procedure diff --git a/src/lib/process/d_process.h b/src/lib/process/d_process.h index 0ab13133d5..d7c58d9f02 100644 --- a/src/lib/process/d_process.h +++ b/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. 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. static const std::string SHUT_DOWN_COMMAND("shutdown"); diff --git a/src/lib/process/testutils/d_test_stubs.cc b/src/lib/process/testutils/d_test_stubs.cc index 8bff0f2acc..d265232161 100644 --- a/src/lib/process/testutils/d_test_stubs.cc +++ b/src/lib/process/testutils/d_test_stubs.cc @@ -19,9 +19,6 @@ namespace process { // Initialize the static failure flag. 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) : 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)); } -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() { }; //************************** DStubController ************************* -// Define custom controller command supported by DStubController. -const char* DStubController::stub_ctl_command_("spiffy"); - // Define custom command line option command supported by DStubController. const char* DStubController::stub_option_x_ = "x"; @@ -162,24 +138,6 @@ DProcessBase* DStubController::createProcess() { 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 { // Return the "list" of custom options supported by DStubController. return (std::string(stub_option_x_)); diff --git a/src/lib/process/testutils/d_test_stubs.h b/src/lib/process/testutils/d_test_stubs.h index 09c2298787..334adac7bf 100644 --- a/src/lib/process/testutils/d_test_stubs.h +++ b/src/lib/process/testutils/d_test_stubs.h @@ -45,8 +45,6 @@ public: ftCreateProcessNull, ftProcessInit, ftProcessConfigure, - ftControllerCommand, - ftProcessCommand, ftProcessShutdown, ftElementBuild, ftElementCommit, @@ -104,9 +102,6 @@ public: class DStubProcess : public DProcessBase { public: - /// @brief Static constant that defines a custom process command string. - static const char* stub_proc_command_; - /// @brief Constructor /// /// @param name name is a text label for the process. Generally used @@ -152,24 +147,6 @@ public: virtual isc::data::ConstElementPtr 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. /// /// @return Always an empty string. @@ -198,10 +175,6 @@ public: /// @return returns a pointer reference to the singleton 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 /// DStubController. static const char* stub_option_x_; @@ -266,23 +239,6 @@ protected: /// ftCreateProcessException. 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 /// supported by DStubController. DStubController supports one /// addition option, stub_option_x_. |