diff options
| -rw-r--r-- | src/bin/agent/ca_command_mgr.cc | 14 | ||||
| -rw-r--r-- | src/bin/agent/ca_command_mgr.h | 2 | ||||
| -rw-r--r-- | src/bin/agent/ca_messages.mes | 5 | ||||
| -rw-r--r-- | src/bin/shell/tests/.gitignore | 3 | ||||
| -rw-r--r-- | src/lib/config/config_messages.mes | 7 | ||||
| -rw-r--r-- | src/lib/config/hooked_command_mgr.cc | 119 | ||||
| -rw-r--r-- | src/lib/config/hooked_command_mgr.h | 42 | ||||
| -rw-r--r-- | src/lib/config/tests/command_mgr_unittests.cc | 216 | ||||
| -rw-r--r-- | src/lib/hooks/callout_manager.h | 4 | ||||
| -rw-r--r-- | src/lib/hooks/library_handle.cc | 2 | ||||
| -rw-r--r-- | src/lib/hooks/library_handle.h | 4 | ||||
| -rw-r--r-- | src/lib/hooks/server_hooks.cc | 12 | ||||
| -rw-r--r-- | src/lib/hooks/server_hooks.h | 13 | ||||
| -rw-r--r-- | src/lib/hooks/tests/callout_manager_unittest.cc | 8 | ||||
| -rw-r--r-- | src/lib/hooks/tests/full_callout_library.cc | 4 | ||||
| -rw-r--r-- | src/lib/hooks/tests/load_callout_library.cc | 4 | ||||
| -rw-r--r-- | src/lib/hooks/tests/server_hooks_unittest.cc | 13 |
17 files changed, 191 insertions, 281 deletions
diff --git a/src/bin/agent/ca_command_mgr.cc b/src/bin/agent/ca_command_mgr.cc index f2081e286f..85e998db61 100644 --- a/src/bin/agent/ca_command_mgr.cc +++ b/src/bin/agent/ca_command_mgr.cc @@ -97,18 +97,12 @@ CtrlAgentCommandMgr::handleCommandInternal(std::string cmd_name, ElementPtr answer_list = Element::createList(); - // Before the command is forwarded it should be processed by the hooks libraries. + // Before the command is forwarded we check if there are any hooks libraries + // which would process the command. 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); - } + // The command has been processed by hooks library. Return the result. + return (answer_list); } // We don't know whether the hooks libraries modified the value of the diff --git a/src/bin/agent/ca_command_mgr.h b/src/bin/agent/ca_command_mgr.h index 819fa3399c..fef155c75d 100644 --- a/src/bin/agent/ca_command_mgr.h +++ b/src/bin/agent/ca_command_mgr.h @@ -30,8 +30,6 @@ public: /// it is also intended to forward commands to the respective Kea servers /// when the command is not supported directly by the Control Agent. /// -/// @todo This Command Manager doesn't yet support forwarding commands. -/// /// The @ref CtrlAgentCommandMgr is implemented as a singleton. The commands /// are registered using @c CtrlAgentCommandMgr::instance().registerCommand(). /// The @ref CtrlAgentResponseCreator uses the sole instance of the Command diff --git a/src/bin/agent/ca_messages.mes b/src/bin/agent/ca_messages.mes index 181b4b4b91..ac042d3df8 100644 --- a/src/bin/agent/ca_messages.mes +++ b/src/bin/agent/ca_messages.mes @@ -19,11 +19,6 @@ 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_CHECK_FAIL Control Agent configuration check failed: %1 This error message indicates that the CA had failed configuration check. Details are provided. Additional details may be available diff --git a/src/bin/shell/tests/.gitignore b/src/bin/shell/tests/.gitignore index 13066d0906..ed32d08a60 100644 --- a/src/bin/shell/tests/.gitignore +++ b/src/bin/shell/tests/.gitignore @@ -1 +1,2 @@ -shell_process_tests.sh +/shell_process_tests.sh +/shell_unittest.py diff --git a/src/lib/config/config_messages.mes b/src/lib/config/config_messages.mes index 4262f64673..a40bd2b569 100644 --- a/src/lib/config/config_messages.mes +++ b/src/lib/config/config_messages.mes @@ -16,13 +16,6 @@ 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_HOOK_RECEIVE_SKIP command %1 has been handled by the hook library which returned the skip state -This debug message is issued when a hook library has processed the control -command and returned the skip status. The callout should have set the -'response' argument which contains the result of processing the command. -The Command Manager skips processing of this command and simply returns -the response generated by the hook library. - % COMMAND_PROCESS_ERROR1 Error while processing command: %1 This warning message indicates that the server encountered an error while processing received command. Additional information will be provided, if diff --git a/src/lib/config/hooked_command_mgr.cc b/src/lib/config/hooked_command_mgr.cc index 690a3595ff..9a8e091c6a 100644 --- a/src/lib/config/hooked_command_mgr.cc +++ b/src/lib/config/hooked_command_mgr.cc @@ -8,33 +8,13 @@ #include <config/hooked_command_mgr.h> #include <config/config_log.h> #include <hooks/hooks_manager.h> +#include <hooks/server_hooks.h> #include <boost/pointer_cast.hpp> +#include <vector> using namespace isc::data; using namespace isc::hooks; -namespace { - -/// @brief Structure that holds registered hook indexes. -struct CommandMgrHooks { - /// @brief Index for "control_command_receive" hook point. - int hook_index_control_command_receive_; - - /// @brief Constructor that registers hook points for HookedCommandMgr - CommandMgrHooks() { - hook_index_control_command_receive_ = - HooksManager::registerHook("control_command_receive"); - } -}; - -// Declare a hooks object. As this is outside any function or method, it -// will be instantiated (and the constructor run) when the module is loaded. -// As a result, the hook indexes will be defined before any method in this -// module is called. -CommandMgrHooks Hooks; - -} // end of anonymous namespace - namespace isc { namespace config { @@ -43,13 +23,13 @@ HookedCommandMgr::HookedCommandMgr() } bool -HookedCommandMgr::delegateCommandToHookLibrary(std::string& cmd_name, - ConstElementPtr& params, - ConstElementPtr& original_cmd, +HookedCommandMgr::delegateCommandToHookLibrary(const std::string& cmd_name, + const ConstElementPtr& params, + const ConstElementPtr& original_cmd, ElementPtr& answer) { ConstElementPtr hook_response; - if (HooksManager::calloutsPresent(Hooks.hook_index_control_command_receive_)) { + if (HooksManager::commandHandlersPresent(cmd_name)) { callout_handle_ = HooksManager::createCalloutHandle(); @@ -66,21 +46,11 @@ HookedCommandMgr::delegateCommandToHookLibrary(std::string& cmd_name, callout_handle_->setArgument("command", command); callout_handle_->setArgument("response", hook_response); - HooksManager::callCallouts(Hooks.hook_index_control_command_receive_, - *callout_handle_); + HooksManager::callCommandHandlers(cmd_name, *callout_handle_); // The callouts should set the response. callout_handle_->getArgument("response", 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); - cmd_name = parseCommand(params, hook_command); - original_cmd = hook_command; - answer = boost::const_pointer_cast<Element>(hook_response); return (true); @@ -98,33 +68,62 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name, "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); - + // The 'list-commands' is a special case. Hook libraries do not implement + // this command. We determine what commands are supported by the hook + // libraries by checking what hook points are present that have callouts + // registered. + if ((cmd_name != "list-commands")) { + ElementPtr hook_response; + // Check if there are any hooks libraries to process this command. + if (delegateCommandToHookLibrary(cmd_name, params, original_cmd, + hook_response)) { + // Hooks libraries processed this command so simply return a + // result. 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(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 ((mutable_cmd_name == "list-commands") && hook_response && response) { - response = combineCommandsLists(hook_response, response); + // If we're here it means that the callouts weren't called. We need + // to handle the command using local Command Mananger. + ConstElementPtr response = BaseCommandMgr::handleCommand(cmd_name, + params, + original_cmd); + + // If we're processing 'list-commands' command we may need to include + // commands supported by hooks libraries in the response. + if (cmd_name == "list-commands") { + // Hooks names can be used to decode what commands are supported. + const std::vector<std::string>& hooks = + ServerHooks::getServerHooksPtr()->getHookNames(); + + // Only update the response if there are any hooks present. + if (!hooks.empty()) { + ElementPtr hooks_commands = Element::createList(); + for (auto h = hooks.cbegin(); h != hooks.end(); ++h) { + // Try to convert hook name to command name. If non-empty + // string is returned it means that the hook point may have + // command hanlers associated with it. Otherwise, it means that + // existing hook points are not for command handlers but for + // regular callouts. + std::string command_name = ServerHooks::hookToCommandName(*h); + if (!command_name.empty()) { + // Final check: are command handlers registered for this + // hook point? If there are no command handlers associated, + // it means that the hook library was already unloaded. + if (HooksManager::commandHandlersPresent(command_name)) { + hooks_commands->add(Element::create(command_name)); + } + } + } + + // If there is at least one hook point with command handlers + // registered + // for it, combine the lists of commands. + if (!hooks_commands->empty()) { + response = combineCommandsLists(response, createAnswer(0, hooks_commands)); + } + } } return (response); diff --git a/src/lib/config/hooked_command_mgr.h b/src/lib/config/hooked_command_mgr.h index 4113bdf461..bfa2ada933 100644 --- a/src/lib/config/hooked_command_mgr.h +++ b/src/lib/config/hooked_command_mgr.h @@ -18,7 +18,25 @@ namespace config { /// /// This class extends @ref BaseCommandMgr with the logic to delegate the /// commands to a hook library if the hook library is installed and provides -/// callouts for the control API. +/// command handlers for the control API. +/// +/// The command handlers are registered by a hook library by calling +/// @ref isc::hooks::LibraryHandle::registerCommandCallout. This call +/// creates a hook point for this command (if one doesn't exist) and then +/// registeres the specified handler(s). When the @ref HookedCommandMgr +/// receives a command for processing it calls the +/// @ref isc::hooks::HooksManager::commandHandlersPresent to check if there +/// are handlers present for this command. If so, the @ref HookedCommandMgr +/// calls @ref isc::hooks::HooksManager::callCommandHandlers to process +/// the command in the hooks libraries. If command handlers are not installed +/// for this command, the @ref HookedCommandMgr will try to process the +/// command on its own. +/// +/// The @ref isc::hooks::CalloutHandle::CalloutNextStep flag setting by the +/// command handlers does NOT have any influence on the operation of the +/// @ref HookedCommandMgr, i.e. it will always skip processing command on +/// its own if the command handlers are present for the given command, even +/// if the handlers return an error code. class HookedCommandMgr : public BaseCommandMgr { public: @@ -39,32 +57,28 @@ protected: /// @brief Handles the command within the hooks libraries. /// /// 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 hooks library supports the given command it creates - /// a response and returns it in the @c answer argument. + /// command handlers for the specified command to be processed. If the + /// command handlers are present, this method calls them to create a response + /// and then passes the response back within 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 cmd_name Command name. + /// @param params Command arguments. + /// @param 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, + delegateCommandToHookLibrary(const std::string& cmd_name, + const isc::data::ConstElementPtr& params, + const 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. diff --git a/src/lib/config/tests/command_mgr_unittests.cc b/src/lib/config/tests/command_mgr_unittests.cc index 9bff3f9839..a927df5f86 100644 --- a/src/lib/config/tests/command_mgr_unittests.cc +++ b/src/lib/config/tests/command_mgr_unittests.cc @@ -33,9 +33,9 @@ public: CommandMgr::instance().setIOService(io_service_); - handler_name = ""; - handler_params = ElementPtr(); - handler_called = false; + handler_name_ = ""; + handler_params_ = ElementPtr(); + handler_called_ = false; CommandMgr::instance().deregisterAll(); CommandMgr::instance().closeCommandSocket(); @@ -48,8 +48,6 @@ public: CommandMgr::instance().deregisterAll(); CommandMgr::instance().closeCommandSocket(); resetCalloutIndicators(); - HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts( - "control_command_receive"); } /// @brief Returns socket path (using either hardcoded path or env variable) @@ -67,18 +65,27 @@ public: } /// @brief Resets indicators related to callout invocation. + /// + /// It also removes any registered callouts. static void resetCalloutIndicators() { - callout_name = ""; - callout_argument_names.clear(); + callout_name_ = ""; + callout_argument_names_.clear(); + + // Iterate over existing hook points and for each of them remove + // callouts registered. + std::vector<std::string> hooks = ServerHooks::getServerHooksPtr()->getHookNames(); + for (auto h = hooks.cbegin(); h != hooks.cend(); ++h) { + HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(*h); + } } /// @brief A simple command handler that always returns an eror static ConstElementPtr my_handler(const std::string& name, const ConstElementPtr& params) { - handler_name = name; - handler_params = params; - handler_called = true; + handler_name_ = name; + handler_params_ = params; + handler_called_ = true; return (createAnswer(123, "test error message")); } @@ -92,76 +99,14 @@ public: return (createAnswer(234, "text generated by hook handler")); } - /// @brief Test callback which stores callout name and passed arguments. - /// - /// This callout doesn't indicate that the command has been processed, - /// allowing the Command Manager to process it. - /// - /// @param callout_handle Handle passed by the hooks framework. - /// @return Always 0. - static int - control_command_receive_callout(CalloutHandle& callout_handle) { - callout_name = "control_command_receive"; - - ConstElementPtr response; - callout_handle.setArgument("response", response); - - callout_argument_names = callout_handle.getArgumentNames(); - // Sort arguments alphabetically, so as we can access them on - // expected positions and verify. - std::sort(callout_argument_names.begin(), callout_argument_names.end()); - return (0); - } - /// @brief Test callback which stores callout name and passed arguments and /// which handles the command. /// - /// This callout returns the skip status to indicate the the command has - /// been handled. - /// /// @param callout_handle Handle passed by the hooks framework. /// @return Always 0. static int control_command_receive_handle_callout(CalloutHandle& callout_handle) { - callout_name = "control_command_receive"; - - // Create a hooks specific command manager. - BaseCommandMgr callout_command_mgr; - callout_command_mgr.registerCommand("my-command", my_hook_handler); - - ConstElementPtr command; - callout_handle.getArgument("command", command); - - ConstElementPtr arg; - std::string command_name = parseCommand(arg, command); - - ConstElementPtr response = callout_command_mgr.processCommand(command); - callout_handle.setArgument("response", response); - - // Set 'skip' status to indicate that the command has been handled. - if (command_name != "list-commands") { - callout_handle.setStatus(CalloutHandle::NEXT_STEP_SKIP); - } - - callout_argument_names = callout_handle.getArgumentNames(); - // Sort arguments alphabetically, so as we can access them on - // expected positions and verify. - std::sort(callout_argument_names.begin(), callout_argument_names.end()); - return (0); - } - - /// @brief Test callback which modifies parameters of the command and - /// does not return skip status. - /// - /// This callout is used to test the case when the callout modifies the - /// received command and does not set next state SKIP to propagate the - /// command with modified parameters to the local command handler. - /// - /// @param callout_handle Handle passed by the hooks framework. - /// @return Always 0. - static int - control_command_receive_modify_callout(CalloutHandle& callout_handle) { - callout_name = "control_command_receive"; + callout_name_ = "control_command_receive_handle"; ConstElementPtr command; callout_handle.getArgument("command", command); @@ -169,16 +114,13 @@ public: ConstElementPtr arg; std::string command_name = parseCommand(arg, command); - ElementPtr new_arg = Element::createList(); - new_arg->add(Element::create("hook-param")); - command = createCommand(command_name, new_arg); + callout_handle.setArgument("response", + createAnswer(234, "text generated by hook handler")); - callout_handle.setArgument("command", command); - - callout_argument_names = callout_handle.getArgumentNames(); + callout_argument_names_ = callout_handle.getArgumentNames(); // Sort arguments alphabetically, so as we can access them on // expected positions and verify. - std::sort(callout_argument_names.begin(), callout_argument_names.end()); + std::sort(callout_argument_names_.begin(), callout_argument_names_.end()); return (0); } @@ -186,35 +128,35 @@ public: IOServicePtr io_service_; /// @brief Name of the command (used in my_handler) - static std::string handler_name; + static std::string handler_name_; /// @brief Parameters passed to the handler (used in my_handler) - static ConstElementPtr handler_params; + static ConstElementPtr handler_params_; /// @brief Indicates whether my_handler was called - static bool handler_called; + static bool handler_called_; /// @brief Holds invoked callout name. - static std::string callout_name; + static std::string callout_name_; /// @brief Holds a list of arguments passed to the callout. - static std::vector<std::string> callout_argument_names; + static std::vector<std::string> callout_argument_names_; }; /// Name passed to the handler (used in my_handler) -std::string CommandMgrTest::handler_name(""); +std::string CommandMgrTest::handler_name_(""); /// Parameters passed to the handler (used in my_handler) -ConstElementPtr CommandMgrTest::handler_params; +ConstElementPtr CommandMgrTest::handler_params_; /// Indicates whether my_handler was called -bool CommandMgrTest::handler_called(false); +bool CommandMgrTest::handler_called_(false); /// Holds invoked callout name. -std::string CommandMgrTest::callout_name(""); +std::string CommandMgrTest::callout_name_(""); /// Holds a list of arguments passed to the callout. -std::vector<std::string> CommandMgrTest::callout_argument_names; +std::vector<std::string> CommandMgrTest::callout_argument_names_; // Test checks whether the internal command 'list-commands' // is working properly. @@ -338,12 +280,6 @@ TEST_F(CommandMgrTest, deregisterAll) { // Test checks whether a command handler can be installed and then // runs through processCommand to check that it's indeed called. TEST_F(CommandMgrTest, processCommand) { - - // Register callout so as we can check that it is called before - // processing the command by the manager. - HooksManager::preCalloutsLibraryHandle().registerCallout( - "control_command_receive", control_command_receive_callout); - // Install my handler EXPECT_NO_THROW(CommandMgr::instance().registerCommand("my-command", my_handler)); @@ -366,26 +302,22 @@ TEST_F(CommandMgrTest, processCommand) { EXPECT_EQ(123, status_code); // Check that the parameters passed are correct. - EXPECT_EQ(true, handler_called); - EXPECT_EQ("my-command", handler_name); - ASSERT_TRUE(handler_params); - EXPECT_EQ("[ \"just\", \"some\", \"data\" ]", handler_params->str()); - - EXPECT_EQ("control_command_receive", callout_name); - - // Check that the appropriate arguments have been set. Include the - // 'response' which should have been set by the callout. - ASSERT_EQ(2, callout_argument_names.size()); - EXPECT_EQ("command", callout_argument_names[0]); - EXPECT_EQ("response", callout_argument_names[1]); + EXPECT_EQ(true, handler_called_); + EXPECT_EQ("my-command", handler_name_); + ASSERT_TRUE(handler_params_); + EXPECT_EQ("[ \"just\", \"some\", \"data\" ]", handler_params_->str()); + + // Command handlers not installed so expecting that callouts weren't + // called. + EXPECT_TRUE(callout_name_.empty()); } // Verify that processing a command can be delegated to a hook library. TEST_F(CommandMgrTest, delegateProcessCommand) { // Register callout so as we can check that it is called before // processing the command by the manager. - HooksManager::preCalloutsLibraryHandle().registerCallout( - "control_command_receive", control_command_receive_handle_callout); + HooksManager::preCalloutsLibraryHandle().registerCommandCallout( + "my-command", control_command_receive_handle_callout); // Install local handler EXPECT_NO_THROW(CommandMgr::instance().registerCommand("my-command", @@ -403,7 +335,7 @@ TEST_F(CommandMgrTest, delegateProcessCommand) { // Local handler shouldn't be called because the command is handled by the // hook library. - ASSERT_FALSE(handler_called); + ASSERT_FALSE(handler_called_); // Returned status should be unique for the hook library. ConstElementPtr answer_arg; @@ -411,13 +343,13 @@ TEST_F(CommandMgrTest, delegateProcessCommand) { ASSERT_NO_THROW(answer_arg = parseAnswer(status_code, answer)); EXPECT_EQ(234, status_code); - EXPECT_EQ("control_command_receive", callout_name); + EXPECT_EQ("control_command_receive_handle", callout_name_); // Check that the appropriate arguments have been set. Include the // 'response' which should have been set by the callout. - ASSERT_EQ(2, callout_argument_names.size()); - EXPECT_EQ("command", callout_argument_names[0]); - EXPECT_EQ("response", callout_argument_names[1]); + ASSERT_EQ(2, callout_argument_names_.size()); + EXPECT_EQ("command", callout_argument_names_[0]); + EXPECT_EQ("response", callout_argument_names_[1]); } // Verify that 'list-command' command returns combined list of supported @@ -425,8 +357,8 @@ TEST_F(CommandMgrTest, delegateProcessCommand) { TEST_F(CommandMgrTest, delegateListCommands) { // Register callout so as we can check that it is called before // processing the command by the manager. - HooksManager::preCalloutsLibraryHandle().registerCallout( - "control_command_receive", control_command_receive_handle_callout); + HooksManager::preCalloutsLibraryHandle().registerCommandCallout( + "my-command", control_command_receive_handle_callout); // Create my-command-bis which is unique for the local Command Manager, // i.e. not supported by the hook library. This command should also @@ -464,56 +396,6 @@ TEST_F(CommandMgrTest, delegateListCommands) { EXPECT_EQ("my-command-bis", command_names_list[2]); } -// This test verifies the scenario in which the hook library influences the -// command processing by the Kea server. In this test, the callout modifies -// the arguments of the command and passes the command on to the Command -// Manager for processing. -TEST_F(CommandMgrTest, modifyCommandArgsInHook) { - // Register callout so as we can check that it is called before - // processing the command by the manager. - HooksManager::preCalloutsLibraryHandle().registerCallout( - "control_command_receive", control_command_receive_modify_callout); - - // Install local handler - EXPECT_NO_THROW(CommandMgr::instance().registerCommand("my-command", - my_handler)); - - // Now tell CommandMgr to process a command 'my-command' with the - // specified parameter. - ElementPtr my_params = Element::fromJSON("[ \"just\", \"some\", \"data\" ]"); - ConstElementPtr command = createCommand("my-command", my_params); - ConstElementPtr answer; - ASSERT_NO_THROW(answer = CommandMgr::instance().processCommand(command)); - - // There should be an answer. - ASSERT_TRUE(answer); - - // Returned status should be unique for the my_handler. - ConstElementPtr answer_arg; - int status_code; - ASSERT_NO_THROW(answer_arg = parseAnswer(status_code, answer)); - EXPECT_EQ(123, status_code); - - // Local handler should have been called after the callout. - ASSERT_TRUE(handler_called); - EXPECT_EQ("my-command", handler_name); - ASSERT_TRUE(handler_params); - // Check that the local handler received the command with arguments - // set by the callout. - EXPECT_EQ("[ \"hook-param\" ]", handler_params->str()); - - - // Check that the callout has been called with appropriate parameters. - EXPECT_EQ("control_command_receive", callout_name); - - // Check that the appropriate arguments have been set. Include the - // 'response' which should have been set by the callout. - ASSERT_EQ(2, callout_argument_names.size()); - EXPECT_EQ("command", callout_argument_names[0]); - EXPECT_EQ("response", callout_argument_names[1]); - -} - // This test verifies that a Unix socket can be opened properly and that input // parameters (socket-type and socket-name) are verified. TEST_F(CommandMgrTest, unixCreate) { diff --git a/src/lib/hooks/callout_manager.h b/src/lib/hooks/callout_manager.h index 06e9dac39a..a4a7f8a641 100644 --- a/src/lib/hooks/callout_manager.h +++ b/src/lib/hooks/callout_manager.h @@ -119,11 +119,11 @@ public: /// /// The @ref CalloutManager::registerCommandHook has been added to allow for /// dynamically creating hook points for which command handlers are registered. -/// This method is called from the @ref LibraryHandle::registerCommandHandler +/// This method is called from the @ref LibraryHandle::registerCommandCallout /// as a result of registering the command handlers by the hook library in /// its @c load() function. If the hook point for the given command already /// exists, this function doesn't do anything. The -/// @ref LibraryHandle::registerCommandHandler can install callouts on this +/// @ref LibraryHandle::registerCommandCallout can install callouts on this /// hook point. /// /// Note that the callout functions do not access the CalloutManager: instead, diff --git a/src/lib/hooks/library_handle.cc b/src/lib/hooks/library_handle.cc index 1df6de5d61..5707dbae52 100644 --- a/src/lib/hooks/library_handle.cc +++ b/src/lib/hooks/library_handle.cc @@ -36,7 +36,7 @@ LibraryHandle::registerCallout(const std::string& name, CalloutPtr callout) { } void -LibraryHandle::registerCommandHandler(const std::string& command_name, +LibraryHandle::registerCommandCallout(const std::string& command_name, CalloutPtr callout) { // Register hook point for this command, if one doesn't exist. callout_manager_->registerCommandHook(command_name); diff --git a/src/lib/hooks/library_handle.h b/src/lib/hooks/library_handle.h index f04c95283e..5dfad2a909 100644 --- a/src/lib/hooks/library_handle.h +++ b/src/lib/hooks/library_handle.h @@ -48,7 +48,7 @@ extern "C" { /// handler similarly to this: /// @code /// int load(LibraryHandle& libhandle) { -/// libhandle.registerCommandHandler("foo-bar", foo_bar_handler); +/// libhandle.registerCommandCallout("foo-bar", foo_bar_handler); /// return (0); /// } /// @endcode @@ -104,7 +104,7 @@ public: /// /// @param command_name Command name for which handler should be installed. /// @param callout Pointer to the command handler implemented as a callout. - void registerCommandHandler(const std::string& command_name, CalloutPtr callout); + void registerCommandCallout(const std::string& command_name, CalloutPtr callout); /// @brief De-Register a callout on a hook /// diff --git a/src/lib/hooks/server_hooks.cc b/src/lib/hooks/server_hooks.cc index e8f12ead24..d7cfcdf901 100644 --- a/src/lib/hooks/server_hooks.cc +++ b/src/lib/hooks/server_hooks.cc @@ -182,7 +182,17 @@ ServerHooks::commandToHookName(const std::string& command_name) { return (hook_name); } +std::string +ServerHooks::hookToCommandName(const std::string& hook_name) { + if (!hook_name.empty() && hook_name.front() == '$') { + std::string command_name = hook_name.substr(1); + std::replace(command_name.begin(), command_name.end(), '_', '-'); + return (command_name); + } + return (""); +} + -} // namespace util +} // namespace hooks } // namespace isc diff --git a/src/lib/hooks/server_hooks.h b/src/lib/hooks/server_hooks.h index 43f7d7df14..bf100cdb15 100644 --- a/src/lib/hooks/server_hooks.h +++ b/src/lib/hooks/server_hooks.h @@ -167,8 +167,21 @@ public: /// /// @param command_name Command name for which the hook point name is /// to be generated. + /// + /// @return Hook point name, or an empty string if the command name + /// can't be converted to a hook name (e.g. when it lacks dollar sign). static std::string commandToHookName(const std::string& command_name); + /// @brief Returns command name for a specified hook name. + /// + /// This function removes leading dollar sign and replaces underscores + /// with hyphens. + /// + /// @param hook_name Hook name for which command name should be returned. + /// + /// @return Command name. + static std::string hookToCommandName(const std::string& hook_name); + private: /// @brief Constructor /// diff --git a/src/lib/hooks/tests/callout_manager_unittest.cc b/src/lib/hooks/tests/callout_manager_unittest.cc index 1b96585b42..03da266ab2 100644 --- a/src/lib/hooks/tests/callout_manager_unittest.cc +++ b/src/lib/hooks/tests/callout_manager_unittest.cc @@ -880,14 +880,14 @@ TEST_F(CalloutManagerTest, LibraryHandleRegisterCommandHandler) { // 'command-two' should also be called. getCalloutManager()->setLibraryIndex(0); - getCalloutManager()->getLibraryHandle().registerCommandHandler("command-one", + getCalloutManager()->getLibraryHandle().registerCommandCallout("command-one", callout_one); - getCalloutManager()->getLibraryHandle().registerCommandHandler("command-one", + getCalloutManager()->getLibraryHandle().registerCommandCallout("command-one", callout_four); getCalloutManager()->setLibraryIndex(1); - getCalloutManager()->getLibraryHandle().registerCommandHandler("command-one", + getCalloutManager()->getLibraryHandle().registerCommandCallout("command-one", callout_two); - getCalloutManager()->getLibraryHandle().registerCommandHandler("command-two", + getCalloutManager()->getLibraryHandle().registerCommandCallout("command-two", callout_three); // Command handlers are installed for commands: 'command-one' and 'command-two'. diff --git a/src/lib/hooks/tests/full_callout_library.cc b/src/lib/hooks/tests/full_callout_library.cc index 50a34296c1..1846592e15 100644 --- a/src/lib/hooks/tests/full_callout_library.cc +++ b/src/lib/hooks/tests/full_callout_library.cc @@ -151,8 +151,8 @@ load(LibraryHandle& handle) { handle.registerCallout("hookpt_three", hook_nonstandard_three); // Register command_handler_one as control command handler. - handle.registerCommandHandler("command-one", command_handler_one); - handle.registerCommandHandler("command-two", command_handler_two); + handle.registerCommandCallout("command-one", command_handler_one); + handle.registerCommandCallout("command-two", command_handler_two); return (0); } diff --git a/src/lib/hooks/tests/load_callout_library.cc b/src/lib/hooks/tests/load_callout_library.cc index 40606f7466..613ec2c79f 100644 --- a/src/lib/hooks/tests/load_callout_library.cc +++ b/src/lib/hooks/tests/load_callout_library.cc @@ -142,8 +142,8 @@ int load(LibraryHandle& handle) { handle.registerCallout("hookpt_three", hook_nonstandard_three); // Register command_handler_one as control command handler. - handle.registerCommandHandler("command-one", command_handler_one); - handle.registerCommandHandler("command-two", command_handler_two); + handle.registerCommandCallout("command-one", command_handler_one); + handle.registerCommandCallout("command-two", command_handler_two); return (0); } diff --git a/src/lib/hooks/tests/server_hooks_unittest.cc b/src/lib/hooks/tests/server_hooks_unittest.cc index 78d8e13788..6c012fa41f 100644 --- a/src/lib/hooks/tests/server_hooks_unittest.cc +++ b/src/lib/hooks/tests/server_hooks_unittest.cc @@ -198,7 +198,8 @@ TEST(ServerHooksTest, HookCount) { EXPECT_EQ(6, hooks.getCount()); } -// Check that the hook name is correctly generated for a control command name. +// Check that the hook name is correctly generated for a control command name +// and vice versa. TEST(ServerHooksTest, CommandToHookName) { EXPECT_EQ("$x_y_z", ServerHooks::commandToHookName("x-y-z")); @@ -206,4 +207,14 @@ TEST(ServerHooksTest, CommandToHookName) { EXPECT_EQ("$", ServerHooks::commandToHookName("")); } +TEST(ServerHooksTest, HookToCommandName) { + // Underscores replaced by hyphens. + EXPECT_EQ("x-y-z", ServerHooks::hookToCommandName("$x_y_z")); + EXPECT_EQ("foo-bar-foo", ServerHooks::hookToCommandName("$foo_bar-foo")); + // Single dollar is converted to empty string. + EXPECT_TRUE(ServerHooks::hookToCommandName("$").empty()); + // If no dollar, it is not a hook name. Return empty string. + EXPECT_TRUE(ServerHooks::hookToCommandName("abc").empty()); +} + } // Anonymous namespace |