From: Marcin Siodelski Date: Mon, 3 Apr 2017 10:18:38 +0000 (+0200) Subject: [5078] Addressed review comments. X-Git-Tag: trac5196_base~14^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=41b560cc8a97dda7005957f5dc8ea02e8ca9e8f5;p=thirdparty%2Fkea.git [5078] Addressed review comments. --- diff --git a/src/bin/agent/ca_command_mgr.cc b/src/bin/agent/ca_command_mgr.cc index 449ec7fd50..cb61d3c30f 100644 --- a/src/bin/agent/ca_command_mgr.cc +++ b/src/bin/agent/ca_command_mgr.cc @@ -15,12 +15,14 @@ #include #include #include +#include #include #include using namespace isc::asiolink; using namespace isc::config; using namespace isc::data; +using namespace isc::hooks; using namespace isc::process; namespace isc { @@ -40,66 +42,99 @@ ConstElementPtr CtrlAgentCommandMgr::handleCommand(const std::string& cmd_name, const isc::data::ConstElementPtr& params, const isc::data::ConstElementPtr& original_cmd) { - ConstElementPtr answer; + ConstElementPtr answer = handleCommandInternal(cmd_name, params, original_cmd); - 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"); - } - } + if (answer->getType() == Element::list) { + return (answer); + } + + // In general, the handlers should return a list of answers rather than a + // single answer, but in some cases we rely on the generic handlers, + // e.g. 'list-commands', which may return a single answer not wrapped in + // the list. Such answers need to be wrapped in the list here. + ElementPtr answer_list = Element::createList(); + answer_list->add(boost::const_pointer_cast(answer)); + + return (answer_list); +} + + +ConstElementPtr +CtrlAgentCommandMgr::handleCommandInternal(std::string cmd_name, + isc::data::ConstElementPtr params, + isc::data::ConstElementPtr original_cmd) { + + ConstElementPtr services = Element::createList(); + + // Retrieve 'service' parameter to determine if we should forward the + // command or handle it on our own. + if (original_cmd && original_cmd->contains("service")) { + services = original_cmd->get("service"); + // If 'service' value is not a list, this is a fatal error. We don't want + // to try processing commands that don't adhere to the required format. + if (services->getType() != Element::list) { + return (createAnswer(CONTROL_RESULT_ERROR, "service value must be a list")); } - } 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); + // 'service' parameter hasn't been specified which indicates that the command + // is intended to be processed by the CA. The following command will try to + // process the command with hooks libraries (if available) or by one of the + // CA's native handlers. + if (services->empty()) { + return (HookedCommandMgr::handleCommand(cmd_name, params, original_cmd)); } - int rcode = 0; - static_cast(parseAnswer(rcode, answer)); + ElementPtr answer_list = Element::createList(); - // 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); + // Before the command is forwarded it should be processed by the hooks libraries. + if (HookedCommandMgr::delegateCommandToHookLibrary(cmd_name, params, original_cmd, + answer_list)) { + // If the hooks libraries set the 'skip' flag, they indicate that the + // commands have been processed. The answer_list should contain the list + // of answers with each answer pertaining to one service. + if (callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) { + LOG_DEBUG(agent_logger, isc::log::DBGLVL_COMMAND, + CTRL_AGENT_COMMAND_PROCESS_SKIP) + .arg(cmd_name); + return (answer_list); + } + } - } 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()); + // We don't know whether the hooks libraries modified the value of the + // answer list, so let's be safe and re-create the answer_list. + answer_list = Element::createList(); + + // For each value within 'service' we have to try forwarding the command. + for (unsigned i = 0; i < services->size(); ++i) { + if (original_cmd) { + ConstElementPtr answer; + try { + LOG_DEBUG(agent_logger, isc::log::DBGLVL_COMMAND, + CTRL_AGENT_COMMAND_FORWARD_BEGIN) + .arg(cmd_name).arg(services->get(i)->stringValue()); + + answer = forwardCommand(services->get(i)->stringValue(), + cmd_name, original_cmd); + + } catch (const CommandForwardingError& ex) { + LOG_DEBUG(agent_logger, isc::log::DBGLVL_COMMAND, + CTRL_AGENT_COMMAND_FORWARD_FAILED) + .arg(cmd_name).arg(ex.what()); + answer = createAnswer(CONTROL_RESULT_ERROR, ex.what()); + } - } catch (const CommandForwardingSkip& ex) { - // Command is not intended to be forwarded so do nothing. + answer_list->add(boost::const_pointer_cast(answer)); } } - // We have a response, so let's wrap it in the list. - ElementPtr answer_list = Element::createList(); - answer_list->add(boost::const_pointer_cast(answer)); - return (answer_list); } ConstElementPtr -CtrlAgentCommandMgr::tryForwardCommand(const std::string& cmd_name, - const isc::data::ConstElementPtr& command) { +CtrlAgentCommandMgr::forwardCommand(const std::string& service, + const std::string& cmd_name, + const isc::data::ConstElementPtr& command) { // Context will hold the server configuration. CtrlAgentCfgContextPtr ctx; @@ -126,42 +161,11 @@ CtrlAgentCommandMgr::tryForwardCommand(const std::string& cmd_name, " 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()); + server_type = CtrlAgentCfgContext::toServerType(service); } catch (const std::exception& ex) { // Invalid value in service list. Can't proceed. @@ -174,7 +178,7 @@ CtrlAgentCommandMgr::tryForwardCommand(const std::string& cmd_name, 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()); + " for the server type " << service); } // If the configuration does its job properly the socket-name must be @@ -191,9 +195,9 @@ CtrlAgentCommandMgr::tryForwardCommand(const std::string& cmd_name, unix_socket.write(&wire_command[0], wire_command.size()); receive_len = unix_socket.receive(&receive_buf_[0], receive_buf_.size()); - } catch (...) { + } catch (const std::exception& ex) { isc_throw(CommandForwardingError, "unable to forward command to the " - + service_vec.at(0)->stringValue() + " service. The server " + << service << " service: " << ex.what() << ". The server " "is likely to be offline"); } @@ -212,8 +216,7 @@ CtrlAgentCommandMgr::tryForwardCommand(const std::string& cmd_name, answer = Element::fromJSON(reply); LOG_INFO(agent_logger, CTRL_AGENT_COMMAND_FORWARDED) - .arg(cmd_name) - .arg(service_vec.at(0)->stringValue()); + .arg(cmd_name).arg(service); } catch (const std::exception& ex) { isc_throw(CommandForwardingError, "internal server error: unable to parse" diff --git a/src/bin/agent/ca_command_mgr.h b/src/bin/agent/ca_command_mgr.h index 28c504d419..1d34138f15 100644 --- a/src/bin/agent/ca_command_mgr.h +++ b/src/bin/agent/ca_command_mgr.h @@ -24,13 +24,6 @@ public: 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. @@ -54,18 +47,24 @@ public: /// @brief Handles the command having a given name and arguments. /// /// This method extends the base implementation with the ability to forward - /// commands to Kea servers if the Control Agent failed to handle it itself. + /// commands to Kea servers. /// - /// @todo Currently this method only wraps an answer within a list Element. - /// This will be later used to include multiple answers within this list. - /// For now it is just a single answer from the Control Agent. + /// If the received command doesn't include 'service' parameter or this + /// parameter is blank, the command is handled by the Control Agent or the + /// attached hooks libraries. + /// + /// If the non-blank 'service' parameter has been specified the callouts + /// are executed. If the callouts process the command the result is returned + /// to the controlling client. Otherwise, the command is forwarded to each + /// Kea server listed in the 'service' parameter. /// /// @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. + /// @return Pointer to the const data element representing a list of + /// responses to the command. If the command has been handled by the CA, + /// this list includes one response. virtual isc::data::ConstElementPtr handleCommand(const std::string& cmd_name, const isc::data::ConstElementPtr& params, @@ -73,34 +72,42 @@ public: private: - /// @brief Tries to forward received control command to Kea servers. + /// @brief Implements the logic for @ref CtrlAgentCommandMgr::handleCommand. /// - /// 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. + /// All parameters are passed by value because they may be modified within + /// the method. /// - /// @todo Currently only one service per control command is supported. - /// Forwarding to multiple services should be allowed in the future. + /// @param cmd_name Command name. + /// @param params Command arguments. + /// @param original_cmd Original command being processed. /// - /// 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. + /// @return Pointer to the const data element representing a list of responses + /// to the command or a single response (not wrapped in a list). The + /// @ref CtrlAgentCommandMgr::handleCommand will wrap non-list value returned + /// in a single element list. + isc::data::ConstElementPtr + handleCommandInternal(std::string cmd_name, + isc::data::ConstElementPtr params, + isc::data::ConstElementPtr original_cmd); + + /// @brief Tries to forward received control command to Kea servers. /// - /// All other exceptions should be treated as an error. + /// 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. This + /// method forwards the command to the specified Kea service. /// + /// @param service Contains name of the service where the command should be + /// forwarded. /// @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); + forwardCommand(const std::string& destination, + const std::string& cmd_name, + const isc::data::ConstElementPtr& command); /// @brief Private constructor. /// diff --git a/src/bin/agent/ca_controller.h b/src/bin/agent/ca_controller.h index fa1c1c3504..7d6b5aee3e 100644 --- a/src/bin/agent/ca_controller.h +++ b/src/bin/agent/ca_controller.h @@ -21,8 +21,6 @@ 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. diff --git a/src/bin/agent/ca_messages.mes b/src/bin/agent/ca_messages.mes index 1336dcba41..f276c8e20a 100644 --- a/src/bin/agent/ca_messages.mes +++ b/src/bin/agent/ca_messages.mes @@ -24,10 +24,24 @@ This is a debug message issued when the Control Agent exits its event loop. % CTRL_AGENT_STARTED Kea Control Agent version %1 started -This informational message indicates that the DHCP-DDNS server has +This informational message indicates that the Control Agent has processed all configuration information and is ready to begin processing. The version is also printed. +% CTRL_AGENT_COMMAND_FORWARD_BEGIN begin forwarding command %1 to service %2 +This debug message is issued when the Control Agent starts forwarding a +received command to one of the Kea servers. + +% CTRL_AGENT_COMMAND_FORWARD_FAILED failed forwarding command %1: %2 +This debug message is issued when the Control Agent failed forwarding a +received command to one of the Kea servers. The second argument provides +the details of the error. + +% CTRL_AGENT_COMMAND_PROCESS_SKIP command %1 already processed by hooks libraries, skipping +This debug message is issued when the Control Agent skips processing +received command because it has determined that the hooks libraries +already processed the command. + % CTRL_AGENT_CONFIG_FAIL Control Agent configuration failed: %1 This error message indicates that the CA had failed configuration attempt. Details are provided. Additional details may be available diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc index 65ac277dcc..f63e999e24 100644 --- a/src/bin/agent/ca_process.cc +++ b/src/bin/agent/ca_process.cc @@ -55,11 +55,12 @@ CtrlAgentProcess::run() { CtrlAgentController::instance()); controller->registerCommands(); - CtrlAgentCfgContextPtr ctx = - boost::dynamic_pointer_cast(base_ctx); // Let's process incoming data or expiring timers in a loop until // shutdown condition is detected. while (!shouldShutdown()) { + // Remove unused listeners within the main loop because new listeners + // are created in within a callback method. This avoids removal the + // listeners within a callback. garbageCollectListeners(); runIO(); } diff --git a/src/bin/agent/tests/ca_command_mgr_unittests.cc b/src/bin/agent/tests/ca_command_mgr_unittests.cc index 2de61cb3a9..b318bfaba4 100644 --- a/src/bin/agent/tests/ca_command_mgr_unittests.cc +++ b/src/bin/agent/tests/ca_command_mgr_unittests.cc @@ -21,6 +21,7 @@ #include #include #include +#include using namespace isc::agent; using namespace isc::asiolink; @@ -68,18 +69,40 @@ public: /// verification of the response parameters. /// /// @param answer answer to be verified - /// @param expected_code code expected to be returned in the answer - void checkAnswer(ConstElementPtr answer, int expected_code) { + /// @param expected_code0 code expected to be returned in first result within + /// the answer. + /// @param expected_code1 code expected to be returned in second result within + /// the answer. + /// @param expected_code2 code expected to be returned in third result within + /// the answer. + void checkAnswer(const ConstElementPtr& answer, const int expected_code0 = 0, + const int expected_code1 = -1, const int expected_code2 = -1) { + std::vector expected_codes; + if (expected_code0 >= 0) { + expected_codes.push_back(expected_code0); + } + + if (expected_code1 >= 0) { + expected_codes.push_back(expected_code1); + } + + if (expected_code2 >= 0) { + expected_codes.push_back(expected_code2); + } + int status_code; // There may be multiple answers returned within a list. std::vector answer_list = answer->listValue(); - // There must be at least one answer. - ASSERT_GE(answer_list.size(), 1); - // Check that all answers indicate success. + + ASSERT_EQ(expected_codes.size(), answer_list.size()); + // Check all answers. for (auto ans = answer_list.cbegin(); ans != answer_list.cend(); ++ans) { - ASSERT_NO_THROW(isc::config::parseAnswer(status_code, *ans)); - EXPECT_EQ(expected_code, status_code); + ConstElementPtr text; + ASSERT_NO_THROW(text = isc::config::parseAnswer(status_code, *ans)); + EXPECT_EQ(expected_codes[std::distance(answer_list.cbegin(), ans)], + status_code) + << "answer contains text: " << text->stringValue(); } } @@ -148,32 +171,34 @@ public: /// /// @param response Stub response to be sent from the server socket to the /// client. - void bindServerSocket(const std::string& response) { + /// @param stop_after_count Number of received messages received over the + /// server socket after which the IO service should be stopped. + void bindServerSocket(const std::string& response, + const unsigned int stop_after_count = 1) { server_socket_.reset(new test::TestServerUnixSocket(*getIOService(), unixSocketFilePath(), TEST_TIMEOUT, response)); - server_socket_->bindServerSocket(); + server_socket_->bindServerSocket(stop_after_count); } /// @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. + /// @param service Service value to be added to the command. This value is + /// specified as a list of comma separated values, e.g. "dhcp4, dhcp6". /// /// @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); + std::string s = boost::replace_all_copy(service, ",", "\",\""); + s = std::string("[ \"") + s + std::string("\" ]"); + command->set("service", Element::fromJSON(s)); } command->set("arguments", Element::createMap()); @@ -186,16 +211,23 @@ public: /// @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 expected_result0 Expected first result in response from the server. + /// @param expected_result1 Expected second result in response from the server. + /// @param expected_result2 Expected third result in response from the server. + /// @param stop_after_count Number of received messages received over the + /// server socket after which the IO service should be stopped. /// @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 int expected_result0, + const int expected_result1 = -1, + const int expected_result2 = -1, + const unsigned stop_after_count = 1, const std::string& server_response = "{ \"result\": 0 }") { // Configure client side socket. configureControlSocket(server_type); // Create server side socket. - bindServerSocket(server_response); + bindServerSocket(server_response, stop_after_count); // 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. @@ -208,7 +240,7 @@ public: ConstElementPtr answer = mgr_.handleCommand("foo", ConstElementPtr(), command); - checkAnswer(answer, expected_result); + checkAnswer(answer, expected_result0, expected_result1, expected_result2); } /// @brief a convenience reference to control agent command manager @@ -249,6 +281,24 @@ TEST_F(CtrlAgentCommandMgrTest, forwardToDHCPv6Server) { isc::config::CONTROL_RESULT_SUCCESS); } +/// Check that the same command is forwarded to multiple servers. +TEST_F(CtrlAgentCommandMgrTest, forwardToBothDHCPServers) { + configureControlSocket(CtrlAgentCfgContext::TYPE_DHCP6); + + testForward(CtrlAgentCfgContext::TYPE_DHCP4, "dhcp4,dhcp6", + isc::config::CONTROL_RESULT_SUCCESS, + isc::config::CONTROL_RESULT_SUCCESS, + -1, 2); +} + +/// Check that the command may forwarded to the second server even if +/// forwarding to a first server fails. +TEST_F(CtrlAgentCommandMgrTest, failForwardToServer) { + testForward(CtrlAgentCfgContext::TYPE_DHCP6, "dhcp4,dhcp6", + isc::config::CONTROL_RESULT_ERROR, + 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, "", @@ -259,7 +309,7 @@ TEST_F(CtrlAgentCommandMgrTest, noService) { /// command was forwarded sent an invalid message. TEST_F(CtrlAgentCommandMgrTest, invalidAnswer) { testForward(CtrlAgentCfgContext::TYPE_DHCP6, "dhcp6", - isc::config::CONTROL_RESULT_ERROR, + isc::config::CONTROL_RESULT_ERROR, -1, -1, 1, "{ \"result\": 0"); } diff --git a/src/lib/asiolink/testutils/test_server_unix_socket.cc b/src/lib/asiolink/testutils/test_server_unix_socket.cc index 7a11aabbf0..205f635fc0 100644 --- a/src/lib/asiolink/testutils/test_server_unix_socket.cc +++ b/src/lib/asiolink/testutils/test_server_unix_socket.cc @@ -21,19 +21,21 @@ TestServerUnixSocket::TestServerUnixSocket(IOService& io_service, server_acceptor_(io_service_.get_io_service()), server_socket_(io_service_.get_io_service()), test_timer_(io_service_), - custom_response_(custom_response) { + custom_response_(custom_response), + stop_after_count_(1), + read_count_(0) { test_timer_.setup(boost::bind(&TestServerUnixSocket::timeoutHandler, this), test_timeout, IntervalTimer::ONE_SHOT); } void -TestServerUnixSocket::bindServerSocket() { +TestServerUnixSocket::bindServerSocket(const unsigned int stop_after_count) { server_acceptor_.open(); server_acceptor_.bind(server_endpoint_); server_acceptor_.listen(); - server_acceptor_.async_accept(server_socket_, - boost::bind(&TestServerUnixSocket:: - acceptHandler, this, _1)); + accept(); + + stop_after_count_ = stop_after_count; } void @@ -44,19 +46,40 @@ TestServerUnixSocket::acceptHandler(const boost::system::error_code&) { readHandler, this, _1, _2)); } +void +TestServerUnixSocket::accept() { + server_acceptor_.async_accept(server_socket_, + boost::bind(&TestServerUnixSocket:: + acceptHandler, this, _1)); +} + + void TestServerUnixSocket::readHandler(const boost::system::error_code&, size_t bytes_transferred) { 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(); + + // Close the connection as we might be expecting another connection over the + // same socket. + server_socket_.close(); + + // Stop IO service if we have reached the maximum number of read messages. + if (++read_count_ >= stop_after_count_) { + io_service_.stop(); + + } else { + // Previous connection is done, so let's accept another connection. + accept(); + } } void diff --git a/src/lib/asiolink/testutils/test_server_unix_socket.h b/src/lib/asiolink/testutils/test_server_unix_socket.h index 77e556b130..5fc477f07f 100644 --- a/src/lib/asiolink/testutils/test_server_unix_socket.h +++ b/src/lib/asiolink/testutils/test_server_unix_socket.h @@ -32,10 +32,13 @@ public: TestServerUnixSocket(IOService& io_service, const std::string& socket_file_path, const long test_timeout, - const std::string& custom_response = ""); + const std::string& custom_respons_ = ""); /// @brief Creates and binds server socket. - void bindServerSocket(); + /// + /// @param stop_after_count Number of received messages after which the + /// IO service should be stopped. + void bindServerSocket(const unsigned int stop_after_count = 1); /// @brief Server acceptor handler. /// @@ -56,6 +59,9 @@ public: private: + /// @brief Asynchronously accept new connections. + void accept(); + /// @brief IO service used by the tests. IOService& io_service_; @@ -75,6 +81,12 @@ private: /// @brief Holds custom response to be sent to the client. std::string custom_response_; + + /// @brief Number of messages received after which IO service gets stopped. + unsigned int stop_after_count_; + + /// @brief Number of messages received so far. + unsigned int read_count_; }; /// @brief Pointer to the @ref TestServerUnixSocket. diff --git a/src/lib/asiolink/unix_domain_socket.h b/src/lib/asiolink/unix_domain_socket.h index 4d10136510..03b3c6201c 100644 --- a/src/lib/asiolink/unix_domain_socket.h +++ b/src/lib/asiolink/unix_domain_socket.h @@ -52,8 +52,6 @@ public: /// /// @param data Pointer to data to be written. /// @param length Number of bytes to be written. - /// @param [out] ec Error code returned as a result of an attempt to - /// write to socket. /// /// @return Number of bytes written. /// @throw UnixDomainSocketError if error occurs. @@ -64,8 +62,6 @@ public: /// @param [out] data Pointer to a location into which the read data should /// be stored. /// @param length Length of the buffer. - /// @param [out] ec Error code returned as a result of an attempt to - /// read from socket. /// /// @return Number of bytes read. /// @throw UnixDomainSocketError if error occurs. diff --git a/src/lib/config/hooked_command_mgr.cc b/src/lib/config/hooked_command_mgr.cc index 2a32f10b38..aa13f0cc54 100644 --- a/src/lib/config/hooked_command_mgr.cc +++ b/src/lib/config/hooked_command_mgr.cc @@ -42,18 +42,11 @@ HookedCommandMgr::HookedCommandMgr() : BaseCommandMgr(), callout_handle_(HooksManager::createCalloutHandle()) { } -ConstElementPtr -HookedCommandMgr::handleCommand(const std::string& cmd_name, - 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"); - } - - std::string final_cmd_name = cmd_name; - ConstElementPtr final_params = boost::const_pointer_cast(params); - ConstElementPtr final_cmd = original_cmd; +bool +HookedCommandMgr::delegateCommandToHookLibrary(std::string& cmd_name, + ConstElementPtr& params, + ConstElementPtr& original_cmd, + ElementPtr& answer) { ConstElementPtr hook_response; if (HooksManager::calloutsPresent(Hooks.hook_index_control_command_receive_)) { @@ -74,35 +67,58 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name, // The callouts should set the response. callout_handle_->getArgument("response", hook_response); - // If the hook return 'skip' status, simply return the response. - if (callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) { - LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_HOOK_RECEIVE_SKIP) - .arg(cmd_name); - - return (hook_response); - - } // The hook library can modify the command or arguments. Thus, we // retrieve the command returned by the callouts and use it as input // to the local command handler. ConstElementPtr hook_command; callout_handle_->getArgument("command", hook_command); - final_cmd_name = parseCommand(final_params, hook_command); - final_cmd = hook_command; + cmd_name = parseCommand(params, hook_command); + original_cmd = hook_command; + + answer = boost::const_pointer_cast(hook_response); + + return (true); + } + + return (false); +} + +ConstElementPtr +HookedCommandMgr::handleCommand(const std::string& cmd_name, + 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"); + } + + std::string mutable_cmd_name = cmd_name; + ConstElementPtr mutable_params = params; + ConstElementPtr mutable_cmd = original_cmd; + + ElementPtr hook_response; + if (delegateCommandToHookLibrary(mutable_cmd_name, mutable_params, + mutable_cmd, hook_response)) { + if (callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) { + LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_HOOK_RECEIVE_SKIP) + .arg(cmd_name); + + return (hook_response); + } } // If we're here it means that the callouts weren't called or the 'skip' // status wasn't returned. The latter is the case when the 'list-commands' // is being processed. Anyhow, we need to handle the command using local // Command Mananger. - ConstElementPtr response = BaseCommandMgr::handleCommand(final_cmd_name, - final_params, - final_cmd); + ConstElementPtr response = BaseCommandMgr::handleCommand(mutable_cmd_name, + mutable_params, + mutable_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. - if ((final_cmd_name == "list-commands") && hook_response && response) { + if ((mutable_cmd_name == "list-commands") && hook_response && response) { response = combineCommandsLists(hook_response, response); } diff --git a/src/lib/config/hooked_command_mgr.h b/src/lib/config/hooked_command_mgr.h index 7d2c36f93e..4113bdf461 100644 --- a/src/lib/config/hooked_command_mgr.h +++ b/src/lib/config/hooked_command_mgr.h @@ -36,15 +36,35 @@ protected: return (callout_handle_); } - /// @brief Handles the command having a given name and arguments. + /// @brief Handles the command within the hooks libraries. /// - /// This method checks if the hook library is installed which implements + /// This method checks if the hooks libraries are installed which implement /// callouts for the 'control_command_receive' hook point, and calls them - /// if they exist. If the hook library supports the given command it creates - /// a response and returns it in the 'response' argument of the - /// @ref isc::data::ConstElementPtr type. If the callout also sets the 'skip' - /// status, the response created by the callout is returned. Otherwise, the - /// @ref BaseCommandMgr::handleCommand is called. + /// if they exist. If the hooks library supports the given command it creates + /// a response and returns it in the @c answer argument. + /// + /// Values of all arguments can be modified by the hook library. + /// + /// @param [out] cmd_name Command name. + /// @param [out] params Command arguments. + /// @param [out] original_cmd Original command received. + /// @param [out] answer Command processing result returned by the hook. + /// + /// @return Boolean value indicating if any callouts have been executed. + bool + delegateCommandToHookLibrary(std::string& cmd_name, + isc::data::ConstElementPtr& params, + isc::data::ConstElementPtr& original_cmd, + isc::data::ElementPtr& answer); + + /// @brief Handles the command having a given name and arguments. + /// + /// This method calls @ref HookedCommandMgr::delegateCommandToHookLibrary to + /// try to process the command with the hook libraries, if they are installed. + /// If the returned @c skip value indicates that the callout set the 'skip' flag + /// the command is assumed to have been processed and the response is returned. + /// If the 'skip' flag is not set, the @ref BaseCommandMgr::handleCommand is + /// called. /// /// @param cmd_name Command name. /// @param params Command arguments.