From: Marcin Siodelski Date: Wed, 29 Mar 2017 10:06:30 +0000 (+0200) Subject: [5078] Implemented commands forwarding in Control Agent. X-Git-Tag: trac5196_base~14^2~10 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0d0288e1bbbe71a6161d228dbaae685bcf3cb831;p=thirdparty%2Fkea.git [5078] Implemented commands forwarding in Control Agent. --- diff --git a/src/bin/agent/ca_cfg_mgr.cc b/src/bin/agent/ca_cfg_mgr.cc index 1ba886b20c..2028edc279 100644 --- a/src/bin/agent/ca_cfg_mgr.cc +++ b/src/bin/agent/ca_cfg_mgr.cc @@ -10,6 +10,7 @@ #include #include #include +#include using namespace isc::dhcp; using namespace isc::process; @@ -34,6 +35,20 @@ CtrlAgentCfgContext::CtrlAgentCfgContext(const CtrlAgentCfgContext& orig) ctrl_sockets_[TYPE_DHCP6] = orig.ctrl_sockets_[TYPE_DHCP6]; } +CtrlAgentCfgContext::ServerType +CtrlAgentCfgContext::toServerType(const std::string& service) { + if (service == "dhcp4") { + return (CtrlAgentCfgContext::TYPE_DHCP4); + + } else if (service == "dhcp6") { + return (CtrlAgentCfgContext::TYPE_DHCP6); + + } else if (service == "d2") { + return (CtrlAgentCfgContext::TYPE_D2); + } + + isc_throw(isc::BadValue, "invalid service value " << service); +} CtrlAgentCfgMgr::CtrlAgentCfgMgr() : DCfgMgrBase(DCfgContextBasePtr(new CtrlAgentCfgContext())) { diff --git a/src/bin/agent/ca_cfg_mgr.h b/src/bin/agent/ca_cfg_mgr.h index 655d2b5c39..4373d61a09 100644 --- a/src/bin/agent/ca_cfg_mgr.h +++ b/src/bin/agent/ca_cfg_mgr.h @@ -11,6 +11,7 @@ #include #include #include +#include namespace isc { namespace agent { @@ -42,6 +43,11 @@ public: /// @brief Used check that specified ServerType is within valid range. static const uint32_t MAX_TYPE_SUPPORTED = TYPE_D2; + /// @brief Converts service specified as a string to ServerType. + /// + /// @param service Service value as a string: 'dhcp4', 'dhcp6', 'd2'. + static ServerType toServerType(const std::string& service); + /// @brief Creates a clone of this context object. /// /// Note this method does not do deep copy the information about control sockets. diff --git a/src/bin/agent/ca_command_mgr.cc b/src/bin/agent/ca_command_mgr.cc index 5a36d8e954..449ec7fd50 100644 --- a/src/bin/agent/ca_command_mgr.cc +++ b/src/bin/agent/ca_command_mgr.cc @@ -4,11 +4,24 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. +#include #include +#include +#include +#include +#include +#include +#include +#include #include #include +#include +#include +using namespace isc::asiolink; +using namespace isc::config; using namespace isc::data; +using namespace isc::process; namespace isc { namespace agent { @@ -25,13 +38,191 @@ CtrlAgentCommandMgr::CtrlAgentCommandMgr() ConstElementPtr CtrlAgentCommandMgr::handleCommand(const std::string& cmd_name, - const isc::data::ConstElementPtr& params) { + const isc::data::ConstElementPtr& params, + const isc::data::ConstElementPtr& original_cmd) { + ConstElementPtr answer; + + try { + // list-commands is a special case. The Control Agent always supports this + // command but most of the time users don't want to list commands supported + // by the CA but by one of the Kea servers. The user would indicate that + // by specifying 'service' value. + if (cmd_name == "list-commands") { + if (original_cmd && original_cmd->contains("service")) { + ConstElementPtr services = original_cmd->get("service"); + if (services && !services->empty()) { + // The non-empty control command 'service' parameter exists which + // means we will forward this command to the Kea server. Let's + // cheat that Control Agent doesn't support this command to + // avoid it being handled by CA. + answer = createAnswer(CONTROL_RESULT_COMMAND_UNSUPPORTED, + "forwarding list-commands command"); + } + } + } + } catch (const std::exception& ex) { + answer = createAnswer(CONTROL_RESULT_ERROR, "invalid service parameter value: " + + std::string(ex.what())); + } + + if (!answer) { + // Try handling this command on our own. + answer = HookedCommandMgr::handleCommand(cmd_name, params, original_cmd); + } + + int rcode = 0; + static_cast(parseAnswer(rcode, answer)); + + // We have tried handling the command on our own but it seems that neither + // the Control Agent nor a hook library can handle this command. We need + // to try forwarding the command to one of the Kea servers. + if (original_cmd && (rcode == CONTROL_RESULT_COMMAND_UNSUPPORTED)) { + try { + answer = tryForwardCommand(cmd_name, original_cmd); + + } catch (const CommandForwardingError& ex) { + // This is apparently some configuration error or client's error. + // We have notify the client. + answer = createAnswer(CONTROL_RESULT_ERROR, ex.what()); + + } catch (const CommandForwardingSkip& ex) { + // Command is not intended to be forwarded so do nothing. + } + } + + // We have a response, so let's wrap it in the list. ElementPtr answer_list = Element::createList(); - answer_list->add(boost::const_pointer_cast< - Element>(HookedCommandMgr::handleCommand(cmd_name, params))); + answer_list->add(boost::const_pointer_cast(answer)); + return (answer_list); } +ConstElementPtr +CtrlAgentCommandMgr::tryForwardCommand(const std::string& cmd_name, + const isc::data::ConstElementPtr& command) { + // Context will hold the server configuration. + CtrlAgentCfgContextPtr ctx; + + // There is a hierarchy of the objects through which we need to pass to get + // the configuration context. We may simplify this at some point but since + // we're in the singleton we want to make sure that we're using most current + // configuration. + boost::shared_ptr controller = + boost::dynamic_pointer_cast(CtrlAgentController::instance()); + if (controller) { + CtrlAgentProcessPtr process = controller->getCtrlAgentProcess(); + if (process) { + CtrlAgentCfgMgrPtr cfgmgr = process->getCtrlAgentCfgMgr(); + if (cfgmgr) { + ctx = cfgmgr->getCtrlAgentCfgContext(); + } + } + } + + // This is highly unlikely but keep the checks just in case someone messes up + // in the code. + if (!ctx) { + isc_throw(CommandForwardingError, "internal server error: unable to retrieve" + " Control Agent configuration information"); + } + + // If the service is not specified it means that the Control Agent is the + // intended receiver of this message. This is not a fatal error, we simply + // skip forwarding the command and rely on the internal logic of the + // Control Agent to generate response. + ConstElementPtr service_elements = command->get("service"); + if (!service_elements) { + isc_throw(CommandForwardingSkip, "service parameter not specified"); + } + + // If the service exists it must be a list, even though we currently allow + // only one service. + std::vector service_vec; + try { + service_vec = service_elements->listValue(); + + } catch (const std::exception& ex) { + isc_throw(CommandForwardingError, "service parameter is not a list"); + } + + // service list may be empty in which case we treat it as it is not specified. + if (service_vec.empty()) { + isc_throw(CommandForwardingSkip, "service parameter is empty"); + } + + // Do not allow more than one service value. This will be allowed in the + // future. + if (service_vec.size() > 1) { + isc_throw(CommandForwardingError, "service parameter must contain 0 or 1" + " service value"); + } + + // Convert the service to the server type values. Make sure the client + // provided right value. + CtrlAgentCfgContext::ServerType server_type; + try { + server_type = CtrlAgentCfgContext::toServerType(service_vec.at(0)->stringValue()); + + } catch (const std::exception& ex) { + // Invalid value in service list. Can't proceed. + isc_throw(CommandForwardingError, ex.what()); + } + + // Now that we know what service it should be forwarded to, we should + // find a matching forwarding socket. If this socket is not configured, + // we have to communicate it to the client. + ConstElementPtr socket_info = ctx->getControlSocketInfo(server_type); + if (!socket_info) { + isc_throw(CommandForwardingError, "forwarding socket is not configured" + " for the server type " << service_vec.at(0)->stringValue()); + } + + // If the configuration does its job properly the socket-name must be + // specified and must be a string value. + std::string socket_name = socket_info->get("socket-name")->stringValue(); + + // Forward command and receive reply. + IOService io_service; + UnixDomainSocket unix_socket(io_service); + size_t receive_len; + try { + unix_socket.connect(socket_name); + std::string wire_command = command->toWire(); + unix_socket.write(&wire_command[0], wire_command.size()); + receive_len = unix_socket.receive(&receive_buf_[0], receive_buf_.size()); + + } catch (...) { + isc_throw(CommandForwardingError, "unable to forward command to the " + + service_vec.at(0)->stringValue() + " service. The server " + "is likely to be offline"); + } + + // This is really not possible right now, but when we migrate to the + // solution using timeouts it is possible that the response is not + // received. + if (receive_len == 0) { + isc_throw(CommandForwardingError, "internal server error: no answer" + " received from the server to the forwarded message"); + } + + std::string reply(&receive_buf_[0], receive_len); + + ConstElementPtr answer; + try { + answer = Element::fromJSON(reply); + + LOG_INFO(agent_logger, CTRL_AGENT_COMMAND_FORWARDED) + .arg(cmd_name) + .arg(service_vec.at(0)->stringValue()); + + } catch (const std::exception& ex) { + isc_throw(CommandForwardingError, "internal server error: unable to parse" + " server's answer to the forwarded message: " << ex.what()); + } + + return (answer); +} + } // end of namespace isc::agent } // end of namespace isc diff --git a/src/bin/agent/ca_command_mgr.h b/src/bin/agent/ca_command_mgr.h index 03de1b12a9..d8c598fb77 100644 --- a/src/bin/agent/ca_command_mgr.h +++ b/src/bin/agent/ca_command_mgr.h @@ -8,12 +8,29 @@ #define CTRL_AGENT_COMMAND_MGR_H #include +#include #include #include +#include namespace isc { namespace agent { +/// @brief Exception thrown when an error occurred during control command +/// forwarding. +class CommandForwardingError : public Exception { +public: + CommandForwardingError(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) { }; +}; + +/// @brief Exception thrown when command forwarding has been skipped. +class CommandForwardingSkip : public Exception { +public: + CommandForwardingSkip(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) { }; +}; + /// @brief Command Manager for Control Agent. /// /// This is an implementation of the Command Manager within Control Agent. @@ -45,20 +62,55 @@ public: /// /// @param cmd_name Command name. /// @param params Command arguments. + /// @param original_cmd Original command being processed. /// /// @return Pointer to the const data element representing response /// to a command. virtual isc::data::ConstElementPtr handleCommand(const std::string& cmd_name, - const isc::data::ConstElementPtr& params); + const isc::data::ConstElementPtr& params, + const isc::data::ConstElementPtr& orginal_cmd); private: + + /// @brief Tries to forward received control command to Kea servers. + /// + /// When the Control Agent was unable to process the control command + /// because it doesn't recognize it, the command should be forwarded to + /// the specific Kea services listed within a 'service' parameter. + /// + /// @todo Currently only one service per control command is supported. + /// Forwarding to multiple services should be allowed in the future. + /// + /// This method makes an attempt to forward the control command. If + /// the 'service' parameter is not specified or it is empty, the + /// command is not forwarded and the @ref CommandForwardingSkip exception + /// is thrown. The caller catching this exception should not treat + /// this situation as an error but this is normal situation when the + /// message is not intended to be forwarded. + /// + /// All other exceptions should be treated as an error. + /// + /// @param cmd_name Command name. + /// @param command Pointer to the object representing the forwarded command. + /// + /// @return Response to forwarded command. + /// @throw CommandForwardingError when an error occurred during forwarding. + /// @throw CommandForwardingSkip when 'service' parameter hasn't been + /// specified which means that the command should not be forwarded. + isc::data::ConstElementPtr + tryForwardCommand(const std::string& cmd_name, + const isc::data::ConstElementPtr& command); + /// @brief Private constructor. /// /// The instance should be created using @ref CtrlAgentCommandMgr::instance, /// thus the constructor is private. CtrlAgentCommandMgr(); + /// @brief Buffer into which responses to forwarded commands are stored. + std::array receive_buf_; + }; } // end of namespace isc::agent diff --git a/src/bin/agent/ca_controller.cc b/src/bin/agent/ca_controller.cc index a5fce22fd1..02ad66ac9d 100644 --- a/src/bin/agent/ca_controller.cc +++ b/src/bin/agent/ca_controller.cc @@ -86,5 +86,10 @@ CtrlAgentController::CtrlAgentController() CtrlAgentController::~CtrlAgentController() { } +CtrlAgentProcessPtr +CtrlAgentController::getCtrlAgentProcess() { + return (boost::dynamic_pointer_cast(getProcess())); +} + } // namespace isc::agent } // namespace isc diff --git a/src/bin/agent/ca_controller.h b/src/bin/agent/ca_controller.h index 2cca081298..fa1c1c3504 100644 --- a/src/bin/agent/ca_controller.h +++ b/src/bin/agent/ca_controller.h @@ -7,6 +7,7 @@ #ifndef CTRL_AGENT_CONTROLLER_H #define CTRL_AGENT_CONTROLLER_H +#include #include namespace isc { @@ -20,6 +21,8 @@ namespace agent { class CtrlAgentController : public process::DControllerBase { public: + using DControllerBase::getIOService; + /// @brief Static singleton instance method. /// /// This method returns the base class singleton instance member. @@ -32,6 +35,9 @@ public: /// @brief Destructor virtual ~CtrlAgentController(); + /// @brief Returns pointer to an instance of the underlying process object. + CtrlAgentProcessPtr getCtrlAgentProcess(); + /// @brief Defines the application name, this is passed into base class /// and appears in log statements. static const char* agent_app_name_; diff --git a/src/bin/agent/ca_messages.mes b/src/bin/agent/ca_messages.mes index 2bdc7371f7..1336dcba41 100644 --- a/src/bin/agent/ca_messages.mes +++ b/src/bin/agent/ca_messages.mes @@ -6,6 +6,10 @@ $NAMESPACE isc::agent +% CTRL_AGENT_COMMAND_FORWARDED command %1 successfully forwarded to the service %2 +This informational message is issued when the CA successfully forwards +the control message to the specified Kea service and receives a response. + % CTRL_AGENT_HTTP_SERVICE_STARTED HTTP service bound to address %1:%2 This informational message indicates that the server has started HTTP service on the specified address and port. All control commands should be sent to this diff --git a/src/bin/agent/tests/Makefile.am b/src/bin/agent/tests/Makefile.am index 19b1359146..94e8d941fc 100644 --- a/src/bin/agent/tests/Makefile.am +++ b/src/bin/agent/tests/Makefile.am @@ -69,6 +69,7 @@ ca_unittests_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la ca_unittests_LDADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la ca_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la ca_unittests_LDADD += $(top_builddir)/src/lib/http/libkea-http.la +ca_unittests_LDADD += $(top_builddir)/src/lib/asiolink/testutils/libasiolinktest.la ca_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la ca_unittests_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la ca_unittests_LDADD += $(top_builddir)/src/lib/dns/libkea-dns++.la diff --git a/src/bin/agent/tests/ca_cfg_mgr_unittests.cc b/src/bin/agent/tests/ca_cfg_mgr_unittests.cc index a3e6fcaafe..9f5e26c2f9 100644 --- a/src/bin/agent/tests/ca_cfg_mgr_unittests.cc +++ b/src/bin/agent/tests/ca_cfg_mgr_unittests.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -27,6 +28,18 @@ public: using CtrlAgentCfgMgr::parse; }; +// Tests conversion of the 'service' parameter to ServerType. +TEST(CtrlAgentCfgContextTest, toServerType) { + EXPECT_EQ(CtrlAgentCfgContext::TYPE_DHCP4, + CtrlAgentCfgContext::toServerType("dhcp4")); + EXPECT_EQ(CtrlAgentCfgContext::TYPE_DHCP6, + CtrlAgentCfgContext::toServerType("dhcp6")); + EXPECT_EQ(CtrlAgentCfgContext::TYPE_D2, + CtrlAgentCfgContext::toServerType("d2")); + EXPECT_THROW(CtrlAgentCfgContext::toServerType("other"), + isc::BadValue); +} + // Tests construction of CtrlAgentCfgMgr class. TEST(CtrlAgentCfgMgr, construction) { boost::scoped_ptr cfg_mgr; diff --git a/src/bin/agent/tests/ca_command_mgr_unittests.cc b/src/bin/agent/tests/ca_command_mgr_unittests.cc index 8061150adc..8a97f6f684 100644 --- a/src/bin/agent/tests/ca_command_mgr_unittests.cc +++ b/src/bin/agent/tests/ca_command_mgr_unittests.cc @@ -5,29 +5,52 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include +#include #include +#include +#include +#include +#include +#include +#include #include +#include +#include +#include +#include +#include #include using namespace isc::agent; +using namespace isc::asiolink; using namespace isc::data; +using namespace isc::process; namespace { +/// @brief Test unix socket file name. +const std::string TEST_SOCKET = "test-socket"; + +/// @brief Test timeout in ms. +const long TEST_TIMEOUT = 10000; + /// @brief Test fixture class for @ref CtrlAgentCommandMgr. /// /// @todo Add tests for various commands, including the cases when the /// commands are forwarded to other servers via unix sockets. /// Meanwhile, this is just a placeholder for the tests. -class CtrlAgentCommandMgrTest : public ::testing::Test { +class CtrlAgentCommandMgrTest : public DControllerTest { public: /// @brief Constructor. /// /// Deregisters all commands except 'list-commands'. CtrlAgentCommandMgrTest() - : mgr_(CtrlAgentCommandMgr::instance()) { + : DControllerTest(CtrlAgentController::instance), + mgr_(CtrlAgentCommandMgr::instance()) { mgr_.deregisterAll(); + removeUnixSocketFile(); + initProcess(); } /// @brief Destructor. @@ -35,6 +58,7 @@ public: /// Deregisters all commands except 'list-commands'. virtual ~CtrlAgentCommandMgrTest() { mgr_.deregisterAll(); + removeUnixSocketFile(); } /// @brief Verifies received answer @@ -58,8 +82,128 @@ public: } } + /// @brief Returns socket file path. + static std::string unixSocketFilePath() { + std::ostringstream s; + s << TEST_DATA_BUILDDIR << "/" << TEST_SOCKET; + return (s.str()); + } + + /// @brief Removes unix socket descriptor. + void removeUnixSocketFile() { + static_cast(remove(unixSocketFilePath().c_str())); + } + + /// @brief Returns pointer to CtrlAgentProcess instance. + CtrlAgentProcessPtr getCtrlAgentProcess() { + return (boost::dynamic_pointer_cast(getProcess())); + } + + /// @brief Returns pointer to CtrlAgentCfgMgr instance for a process. + CtrlAgentCfgMgrPtr getCtrlAgentCfgMgr() { + CtrlAgentCfgMgrPtr p; + if (getCtrlAgentProcess()) { + p = getCtrlAgentProcess()->getCtrlAgentCfgMgr(); + } + return (p); + } + + /// @brief Returns a pointer to the configuration context. + CtrlAgentCfgContextPtr getCtrlAgentCfgContext() { + CtrlAgentCfgContextPtr p; + if (getCtrlAgentCfgMgr()) { + p = getCtrlAgentCfgMgr()->getCtrlAgentCfgContext(); + } + return (p); + } + + /// @brief Adds configuration of the control socket. + /// + /// @param server_type Server type for which socket configuration is to + /// be added. + void + configureControlSocket(const CtrlAgentCfgContext::ServerType& server_type) { + CtrlAgentCfgContextPtr ctx = getCtrlAgentCfgContext(); + ASSERT_TRUE(ctx); + + ElementPtr control_socket = Element::createMap(); + control_socket->set("socket-name", + Element::create(unixSocketFilePath())); + ctx->setControlSocketInfo(control_socket, server_type); + } + + /// @brief Create and bind server side socket. + /// + /// @param response Stub response to be sent from the server socket to the + /// client. + void bindServerSocket(const std::string& response) { + server_socket_.reset(new test::TestServerUnixSocket(*getIOService(), + unixSocketFilePath(), + TEST_TIMEOUT, + response)); + server_socket_->bindServerSocket(); + } + + /// @brief Creates command with no arguments. + /// + /// @param command_name Command name. + /// @param service Service value to be added to the command. If this value + /// holds an empty string, the service parameter is not added. + /// + /// @return Pointer to the instance of the created command. + ConstElementPtr createCommand(const std::string& command_name, + const std::string& service) { + ElementPtr command = Element::createMap(); + + command->set("command", Element::create(command_name)); + + // Only add the 'service' parameter if non-empty. + if (!service.empty()) { + ElementPtr services = Element::createList(); + services->add(Element::create(service)); + command->set("service", services); + } + + command->set("arguments", Element::createMap()); + + return (command); + } + + /// @brief Test forwarding the command. + /// + /// @param server_type Server for which the client socket should be + /// configured. + /// @param service Service to be included in the command. + /// @param expected_result Expected result in response from the server. + /// @param server_response Stub response to be sent by the server. + void testForward(const CtrlAgentCfgContext::ServerType& server_type, + const std::string& service, + const int expected_result, + const std::string& server_response = "{ \"result\": 0 }") { + // Configure client side socket. + configureControlSocket(server_type); + // Create server side socket. + bindServerSocket(server_response); + + // The client side communication is synchronous. To be able to respond + // to this we need to run the server side socket at the same time. + // Running IO service in a thread guarantees that the server responds + // as soon as it receives the control command. + isc::util::thread::Thread(boost::bind(&IOService::run, + getIOService().get())); + + ConstElementPtr command = createCommand("foo", service); + ConstElementPtr answer = mgr_.handleCommand("foo", ConstElementPtr(), + command); + + checkAnswer(answer, expected_result); + } + /// @brief a convenience reference to control agent command manager CtrlAgentCommandMgr& mgr_; + + /// @brief Pointer to the test server unix socket. + test::TestServerUnixSocketPtr server_socket_; }; /// Just a basic test checking that non-existent command is handled @@ -67,17 +211,91 @@ public: TEST_F(CtrlAgentCommandMgrTest, bogus) { ConstElementPtr answer; EXPECT_NO_THROW(answer = mgr_.handleCommand("fish-and-chips-please", + ConstElementPtr(), ConstElementPtr())); - checkAnswer(answer, isc::config::CONTROL_RESULT_ERROR); + checkAnswer(answer, isc::config::CONTROL_RESULT_COMMAND_UNSUPPORTED); }; /// Just a basic test checking that 'list-commands' is supported. TEST_F(CtrlAgentCommandMgrTest, listCommands) { ConstElementPtr answer; EXPECT_NO_THROW(answer = mgr_.handleCommand("list-commands", + ConstElementPtr(), ConstElementPtr())); checkAnswer(answer, isc::config::CONTROL_RESULT_SUCCESS); }; +/// Check that control command is successfully forwarded to the DHCPv4 server. +TEST_F(CtrlAgentCommandMgrTest, forwardToDHCPv4Server) { + testForward(CtrlAgentCfgContext::TYPE_DHCP4, "dhcp4", + isc::config::CONTROL_RESULT_SUCCESS); +} + +/// Check that control command is successfully forwarded to the DHCPv6 server. +TEST_F(CtrlAgentCommandMgrTest, forwardToDHCPv6Server) { + testForward(CtrlAgentCfgContext::TYPE_DHCP6, "dhcp6", + isc::config::CONTROL_RESULT_SUCCESS); +} + +/// Check that control command is not forwarded if the service is not specified. +TEST_F(CtrlAgentCommandMgrTest, noService) { + testForward(CtrlAgentCfgContext::TYPE_DHCP6, "", + isc::config::CONTROL_RESULT_COMMAND_UNSUPPORTED); +} + +/// Check that error is returned to the client when the server to which the +/// command was forwarded sent an invalid message. +TEST_F(CtrlAgentCommandMgrTest, invalidAnswer) { + testForward(CtrlAgentCfgContext::TYPE_DHCP6, "dhcp6", + isc::config::CONTROL_RESULT_ERROR, + "{ \"result\": 0"); +} + +/// Check that error is returned to the client if the forwarding socket is +/// not configured for the given service. +TEST_F(CtrlAgentCommandMgrTest, noClientSocket) { + ConstElementPtr command = createCommand("foo", "dhcp4"); + ConstElementPtr answer = mgr_.handleCommand("foo", ConstElementPtr(), + command); + + checkAnswer(answer, isc::config::CONTROL_RESULT_ERROR); +} + +/// Check that error is returned to the client if the remote server to +/// which the control command is to be forwarded is not available. +TEST_F(CtrlAgentCommandMgrTest, noServerSocket) { + configureControlSocket(CtrlAgentCfgContext::TYPE_DHCP6); + + ConstElementPtr command = createCommand("foo", "dhcp6"); + ConstElementPtr answer = mgr_.handleCommand("foo", ConstElementPtr(), + command); + + checkAnswer(answer, isc::config::CONTROL_RESULT_ERROR); +} + +// Check that list-commands command is forwarded when the service +// value is specified. +TEST_F(CtrlAgentCommandMgrTest, forwardListCommands) { + // Configure client side socket. + configureControlSocket(CtrlAgentCfgContext::TYPE_DHCP4); + // Create server side socket. + bindServerSocket("{ \"result\" : 3 }"); + + // The client side communication is synchronous. To be able to respond + // to this we need to run the server side socket at the same time. + // Running IO service in a thread guarantees that the server responds + // as soon as it receives the control command. + isc::util::thread::Thread(boost::bind(&IOService::run, + getIOService().get())); + + ConstElementPtr command = createCommand("list-commands", "dhcp4"); + ConstElementPtr answer = mgr_.handleCommand("list-commands", ConstElementPtr(), + command); + + // Answer of 3 is specific to the stub response we send when the + // command is forwarded. So having this value returned means that + // the command was forwarded as expected. + checkAnswer(answer, 3); +} } diff --git a/src/lib/asiolink/tests/unix_domain_socket_unittest.cc b/src/lib/asiolink/tests/unix_domain_socket_unittest.cc index 2c7c57fc11..ee9e58e923 100644 --- a/src/lib/asiolink/tests/unix_domain_socket_unittest.cc +++ b/src/lib/asiolink/tests/unix_domain_socket_unittest.cc @@ -72,7 +72,7 @@ TEST_F(UnixDomainSocketTest, sendReceive) { // Setup client side. UnixDomainSocket socket(io_service_); - ASSERT_NO_THROW(socket.connect(TEST_SOCKET)); + ASSERT_NO_THROW(socket.connect(unixSocketFilePath())); // Send "foo". const std::string outbound_data = "foo"; @@ -100,7 +100,7 @@ TEST_F(UnixDomainSocketTest, sendReceive) { // is not available. TEST_F(UnixDomainSocketTest, clientErrors) { UnixDomainSocket socket(io_service_); - ASSERT_THROW(socket.connect(TEST_SOCKET), UnixDomainSocketError); + ASSERT_THROW(socket.connect(unixSocketFilePath()), UnixDomainSocketError); const std::string outbound_data = "foo"; ASSERT_THROW(socket.write(outbound_data.c_str(), outbound_data.size()), UnixDomainSocketError); @@ -117,7 +117,7 @@ TEST_F(UnixDomainSocketTest, getNative) { // Setup client side. UnixDomainSocket socket(io_service_); - ASSERT_NO_THROW(socket.connect(TEST_SOCKET)); + ASSERT_NO_THROW(socket.connect(unixSocketFilePath())); ASSERT_GE(socket.getNative(), 0); } diff --git a/src/lib/asiolink/testutils/test_server_unix_socket.cc b/src/lib/asiolink/testutils/test_server_unix_socket.cc index c4184df890..4959ca39f7 100644 --- a/src/lib/asiolink/testutils/test_server_unix_socket.cc +++ b/src/lib/asiolink/testutils/test_server_unix_socket.cc @@ -6,6 +6,7 @@ #include #include +#include namespace isc { namespace asiolink { @@ -13,12 +14,14 @@ namespace test { TestServerUnixSocket::TestServerUnixSocket(IOService& io_service, const std::string& socket_file_path, - const long test_timeout) + const long test_timeout, + const std::string& custom_response) : io_service_(io_service), server_endpoint_(socket_file_path), server_acceptor_(io_service_.get_io_service()), server_socket_(io_service_.get_io_service()), - test_timer_(io_service_) { + test_timer_(io_service_), + custom_response_(custom_response) { test_timer_.setup(boost::bind(&TestServerUnixSocket::timeoutHandler, this), test_timeout, IntervalTimer::ONE_SHOT); } @@ -35,9 +38,6 @@ TestServerUnixSocket::bindServerSocket() { void TestServerUnixSocket::acceptHandler(const boost::system::error_code& ec) { - if (ec) { - ADD_FAILURE() << ec.message(); - } server_socket_.async_read_some(boost::asio::buffer(&raw_buf_[0], raw_buf_.size()), boost::bind(&TestServerUnixSocket:: @@ -47,10 +47,15 @@ TestServerUnixSocket::acceptHandler(const boost::system::error_code& ec) { void TestServerUnixSocket::readHandler(const boost::system::error_code& ec, size_t bytes_transferred) { - std::string received(&raw_buf_[0], bytes_transferred); - std::string response("received " + received); - boost::asio::write(server_socket_, boost::asio::buffer(response.c_str(), - response.size())); + if (!custom_response_.empty()) { + boost::asio::write(server_socket_, boost::asio::buffer(custom_response_.c_str(), + custom_response_.size())); + } else { + std::string received(&raw_buf_[0], bytes_transferred); + std::string response("received " + received); + boost::asio::write(server_socket_, boost::asio::buffer(response.c_str(), + response.size())); + } io_service_.stop(); } diff --git a/src/lib/asiolink/testutils/test_server_unix_socket.h b/src/lib/asiolink/testutils/test_server_unix_socket.h index 1e201b06d6..77e556b130 100644 --- a/src/lib/asiolink/testutils/test_server_unix_socket.h +++ b/src/lib/asiolink/testutils/test_server_unix_socket.h @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include @@ -28,9 +28,11 @@ public: /// @param io_service IO service. /// @param socket_file_path Socket file path. /// @param test_timeout Test timeout in milliseconds. + /// @param custom_response Custom response to be sent to the client. TestServerUnixSocket(IOService& io_service, const std::string& socket_file_path, - const long test_timeout); + const long test_timeout, + const std::string& custom_response = ""); /// @brief Creates and binds server socket. void bindServerSocket(); @@ -59,7 +61,6 @@ private: /// @brief Server endpoint. boost::asio::local::stream_protocol::endpoint server_endpoint_; - /// @brief Server acceptor. boost::asio::local::stream_protocol::acceptor server_acceptor_; @@ -71,8 +72,14 @@ private: /// @brief Asynchronous timer service to detect timeouts. IntervalTimer test_timer_; + + /// @brief Holds custom response to be sent to the client. + std::string custom_response_; }; +/// @brief Pointer to the @ref TestServerUnixSocket. +typedef boost::shared_ptr TestServerUnixSocketPtr; + } // end of namespace isc::asiolink::test } // end of namespace isc::asiolink } // end of namespace isc diff --git a/src/lib/cc/command_interpreter.h b/src/lib/cc/command_interpreter.h index 3420086620..a0bad85d77 100644 --- a/src/lib/cc/command_interpreter.h +++ b/src/lib/cc/command_interpreter.h @@ -38,6 +38,9 @@ const int CONTROL_RESULT_SUCCESS = 0; /// @brief Status code indicating a general failure const int CONTROL_RESULT_ERROR = 1; +/// @brief Status code indicating that the specified command is not supported. +const int CONTROL_RESULT_COMMAND_UNSUPPORTED = 2; + /// @brief A standard control channel exception that is thrown if a function /// is there is a problem with one of the messages class CtrlChannelError : public isc::Exception { diff --git a/src/lib/config/base_command_mgr.cc b/src/lib/config/base_command_mgr.cc index c148dc5f3d..88b6a290d0 100644 --- a/src/lib/config/base_command_mgr.cc +++ b/src/lib/config/base_command_mgr.cc @@ -31,11 +31,33 @@ BaseCommandMgr::registerCommand(const std::string& cmd, CommandHandler handler) << "' is already installed."); } - handlers_.insert(make_pair(cmd, handler)); + HandlersPair handlers; + handlers.handler = handler; + handlers_.insert(make_pair(cmd, handlers)); LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_REGISTERED).arg(cmd); } +void +BaseCommandMgr::registerExtendedCommand(const std::string& cmd, + ExtendedCommandHandler handler) { + if (!handler) { + isc_throw(InvalidCommandHandler, "Specified command handler is NULL"); + } + + HandlerContainer::const_iterator it = handlers_.find(cmd); + if (it != handlers_.end()) { + isc_throw(InvalidCommandName, "Handler for command '" << cmd + << "' is already installed."); + } + + HandlersPair handlers; + handlers.extended_handler = handler; + handlers_.insert(make_pair(cmd, handlers)); + + LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_EXTENDED_REGISTERED).arg(cmd); +} + void BaseCommandMgr::deregisterCommand(const std::string& cmd) { if (cmd == "list-commands") { @@ -76,7 +98,7 @@ BaseCommandMgr::processCommand(const isc::data::ConstElementPtr& cmd) { LOG_INFO(command_logger, COMMAND_RECEIVED).arg(name); - return (handleCommand(name, arg)); + return (handleCommand(name, arg, cmd)); } catch (const Exception& e) { LOG_WARN(command_logger, COMMAND_PROCESS_ERROR2).arg(e.what()); @@ -88,16 +110,20 @@ BaseCommandMgr::processCommand(const isc::data::ConstElementPtr& cmd) { ConstElementPtr BaseCommandMgr::handleCommand(const std::string& cmd_name, - const ConstElementPtr& params) { + const ConstElementPtr& params, + const ConstElementPtr& original_cmd) { auto it = handlers_.find(cmd_name); if (it == handlers_.end()) { // Ok, there's no such command. - return (createAnswer(CONTROL_RESULT_ERROR, + return (createAnswer(CONTROL_RESULT_COMMAND_UNSUPPORTED, "'" + cmd_name + "' command not supported.")); } // Call the actual handler and return whatever it returned - return (it->second(cmd_name, params)); + if (it->second.handler) { + return (it->second.handler(cmd_name, params)); + } + return (it->second.extended_handler(cmd_name, params, original_cmd)); } isc::data::ConstElementPtr diff --git a/src/lib/config/base_command_mgr.h b/src/lib/config/base_command_mgr.h index 97321a5387..19bbae3248 100644 --- a/src/lib/config/base_command_mgr.h +++ b/src/lib/config/base_command_mgr.h @@ -81,6 +81,20 @@ public: typedef boost::function CommandHandler; + /// @brief Defines extended command handler type. + /// + /// This command handler includes third parameter which holds the + /// entire command control message. The handler can retrieve + /// additional information from this parameter, e.g. 'service'. + /// + /// @param name name of the commands + /// @param params parameters specific to the command + /// @param original original control command. + /// @return response (created with createAnswer()) + typedef boost::function ExtendedCommandHandler; + /// @brief Constructor. /// /// Registers "list-commands" command. @@ -105,6 +119,19 @@ public: /// @param handler Pointer to the method that will handle the command. void registerCommand(const std::string& cmd, CommandHandler handler); + /// @brief Registers specified command handler for a given command. + /// + /// This variant of the method uses extended command handler which, besides + /// command name and arguments, also has a third parameter 'original_cmd' + /// in its signature. Such handlers can retrieve additional parameters from + /// the command, e.g. 'service' indicating where the command should be + /// routed. + /// + /// @param cmd Name of the command to be handled. + /// @param handler Pointer to the method that will handle the command. + void registerExtendedCommand(const std::string& cmd, + ExtendedCommandHandler handler); + /// @brief Deregisters specified command handler. /// /// @param cmd Name of the command that's no longer handled. @@ -127,15 +154,24 @@ protected: /// /// @param cmd_name Command name. /// @param params Command arguments. + /// @param original_cmd Pointer to the entire command received. It may + /// be sometimes useful to retrieve additional parameters from this + /// command. /// /// @return Pointer to the const data element representing response /// to a command. virtual isc::data::ConstElementPtr handleCommand(const std::string& cmd_name, - const isc::data::ConstElementPtr& params); + const isc::data::ConstElementPtr& params, + const isc::data::ConstElementPtr& original_cmd); + + struct HandlersPair { + CommandHandler handler; + ExtendedCommandHandler extended_handler; + }; /// @brief Type of the container for command handlers. - typedef std::map HandlerContainer; + typedef std::map HandlerContainer; /// @brief Container for command handlers. HandlerContainer handlers_; diff --git a/src/lib/config/config_messages.mes b/src/lib/config/config_messages.mes index 215edf90b4..f76ef6cc74 100644 --- a/src/lib/config/config_messages.mes +++ b/src/lib/config/config_messages.mes @@ -6,6 +6,11 @@ $NAMESPACE isc::config +% COMMAND_EXTENDED_REGISTERED Command %1 registered +This debug message indicates that the daemon started supporting specified +command. The handler for the registered command includes a parameter holding +entire command to be processed. + % COMMAND_DEREGISTERED Command %1 deregistered This debug message indicates that the daemon stopped supporting specified command. This command can no longer be issued. If the command socket is diff --git a/src/lib/config/hooked_command_mgr.cc b/src/lib/config/hooked_command_mgr.cc index be3c1cb04d..2a32f10b38 100644 --- a/src/lib/config/hooked_command_mgr.cc +++ b/src/lib/config/hooked_command_mgr.cc @@ -44,7 +44,8 @@ HookedCommandMgr::HookedCommandMgr() ConstElementPtr HookedCommandMgr::handleCommand(const std::string& cmd_name, - const ConstElementPtr& params) { + const ConstElementPtr& params, + const ConstElementPtr& original_cmd) { if (!callout_handle_) { isc_throw(Unexpected, "callout handle not configured for the Command " "Manager: this is a programming error"); @@ -52,6 +53,7 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name, std::string final_cmd_name = cmd_name; ConstElementPtr final_params = boost::const_pointer_cast(params); + ConstElementPtr final_cmd = original_cmd; ConstElementPtr hook_response; if (HooksManager::calloutsPresent(Hooks.hook_index_control_command_receive_)) { @@ -59,13 +61,11 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name, // Delete previously set arguments. callout_handle_->deleteAllArguments(); - // Being in this function we don't have access to the original data - // object holding the whole command (name and arguments). Let's - // recreate it. - ConstElementPtr original_command = createCommand(cmd_name, params); + ConstElementPtr command = original_cmd ? original_cmd : + createCommand(cmd_name, params); // And pass it to the hook library. - callout_handle_->setArgument("command", original_command); + callout_handle_->setArgument("command", command); callout_handle_->setArgument("response", hook_response); HooksManager::callCallouts(Hooks.hook_index_control_command_receive_, @@ -89,6 +89,7 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name, ConstElementPtr hook_command; callout_handle_->getArgument("command", hook_command); final_cmd_name = parseCommand(final_params, hook_command); + final_cmd = hook_command; } // If we're here it means that the callouts weren't called or the 'skip' @@ -96,7 +97,8 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name, // is being processed. Anyhow, we need to handle the command using local // Command Mananger. ConstElementPtr response = BaseCommandMgr::handleCommand(final_cmd_name, - final_params); + final_params, + final_cmd); // For the 'list-commands' case we will have to combine commands supported // by the hook libraries with the commands that this Command Manager supports. diff --git a/src/lib/config/hooked_command_mgr.h b/src/lib/config/hooked_command_mgr.h index ab7cdebd04..7d2c36f93e 100644 --- a/src/lib/config/hooked_command_mgr.h +++ b/src/lib/config/hooked_command_mgr.h @@ -48,12 +48,14 @@ protected: /// /// @param cmd_name Command name. /// @param params Command arguments. + /// @param original_cmd Original command received. /// /// @return Pointer to the const data element representing response /// to a command. virtual isc::data::ConstElementPtr handleCommand(const std::string& cmd_name, - const isc::data::ConstElementPtr& params); + const isc::data::ConstElementPtr& params, + const isc::data::ConstElementPtr& original_cmd); /// @brief Pointer to a callout handle used by this class. isc::hooks::CalloutHandlePtr callout_handle_; diff --git a/src/lib/config/tests/command_mgr_unittests.cc b/src/lib/config/tests/command_mgr_unittests.cc index e73ebbb7bc..95e736379e 100644 --- a/src/lib/config/tests/command_mgr_unittests.cc +++ b/src/lib/config/tests/command_mgr_unittests.cc @@ -222,7 +222,7 @@ TEST_F(CommandMgrTest, bogusCommand) { ASSERT_TRUE(answer); int status_code; parseAnswer(status_code, answer); - EXPECT_EQ(CONTROL_RESULT_ERROR, status_code); + EXPECT_EQ(CONTROL_RESULT_COMMAND_UNSUPPORTED, status_code); } // Test checks whether handlers installation is sanitized. In particular,