]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[5100] Addressed review comments.
authorMarcin Siodelski <marcin@isc.org>
Sat, 7 Jan 2017 22:23:59 +0000 (23:23 +0100)
committerMarcin Siodelski <marcin@isc.org>
Sat, 7 Jan 2017 22:23:59 +0000 (23:23 +0100)
Also, added a test for testing the hook library modifying a
received control command.

src/bin/dhcp4/dhcp4_hooks.dox
src/bin/dhcp6/dhcp6_hooks.dox
src/lib/config/hooked_command_mgr.cc
src/lib/config/tests/command_mgr_unittests.cc

index 5217d1c7bd8e39c9003d8d465bc1c2f4177aacec..1371cc3d2dc42771046b0df903819c3efad00d90 100644 (file)
@@ -295,7 +295,7 @@ to the end of this list.
 @subsection dhcpv4HooksControlCommandReceive control_command_receive
 
  - @b Arguments:
-   - name: @b command, type: isc::data::ConstElementPtr, direction: <b>in</b>
+   - name: @b command, type: isc::data::ConstElementPtr, direction: <b>in/out</b>
    - name: @b response, type: isc::data::ConstElementPtr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when DHCPv4 server receives a
@@ -313,6 +313,10 @@ to the end of this list.
    by the callouts with the list of commands supported by the server. If
    the callout sets the next step action to SKIP in this case, the server
    will only return the list of commands supported by the hook library.
+   The callout can modify the command arguments to influence the command
+   processing by the Command Manager. For example, it may freely modify
+   the configuration received in 'set-config' before it is processed by
+   the server. The SKIP action is not set in this case.
 
  - <b>Next step status</b>: if any callout sets the next step action to SKIP,
    the server will assume that the command has been handled by the callouts
index 44b07779ea6334ede667e9cebab59ffe03f61034..ae9bbe844a363e018589c6c3008ca8818039c106 100644 (file)
@@ -337,7 +337,7 @@ to the end of this list.
 @subsection dhcpv6HooksControlCommandReceive control_command_receive
 
  - @b Arguments:
-   - name: @b command, type: ConstElementPtr, direction: <b>in</b>
+   - name: @b command, type: ConstElementPtr, direction: <b>in/out</b>
    - name: @b response, type: ConstElementPtr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when DHCPv4 server receives a
@@ -355,6 +355,10 @@ to the end of this list.
    by the callouts with the list of commands supported by the server. If
    the callout sets the next step action to SKIP in this case, the server
    will only return the list of commands supported by the hook library.
+   The callout can modify the command arguments to influence the command
+   processing by the Command Manager. For example, it may freely modify
+   the configuration received in 'set-config' before it is processed by
+   the server. The SKIP action is not set in this case.
 
  - <b>Next step status</b>: if any callout sets the next step action to SKIP,
    the server will assume that the command has been handled by the callouts
index 8e865892020410a597705d1e715a2e3e3050dd07..be3c1cb04d4afa0104b03fdf1ea7511699b79c80 100644 (file)
@@ -4,9 +4,11 @@
 // 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 <cc/command_interpreter.h>
 #include <config/hooked_command_mgr.h>
 #include <config/config_log.h>
 #include <hooks/hooks_manager.h>
+#include <boost/pointer_cast.hpp>
 
 using namespace isc::data;
 using namespace isc::hooks;
@@ -48,6 +50,9 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name,
                   "Manager: this is a programming error");
     }
 
+    std::string final_cmd_name = cmd_name;
+    ConstElementPtr final_params = boost::const_pointer_cast<Element>(params);
+
     ConstElementPtr hook_response;
     if (HooksManager::calloutsPresent(Hooks.hook_index_control_command_receive_)) {
 
@@ -57,13 +62,10 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name,
         // 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.
-        ElementPtr original_command = Element::createMap();
-        original_command->set("command", Element::create(cmd_name));
-        original_command->set("arguments", params);
+        ConstElementPtr original_command = createCommand(cmd_name, params);
 
         // And pass it to the hook library.
-        callout_handle_->setArgument("command", boost::dynamic_pointer_cast<
-                                     const Element>(original_command));
+        callout_handle_->setArgument("command", original_command);
         callout_handle_->setArgument("response", hook_response);
 
         HooksManager::callCallouts(Hooks.hook_index_control_command_receive_,
@@ -74,23 +76,31 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name,
 
         // If the hook return 'skip' status, simply return the response.
         if (callout_handle_->getStatus() == CalloutHandle::NEXT_STEP_SKIP) {
-            return (hook_response);
-
-        } else {
             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);
     }
 
     // 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(cmd_name, params);
+    ConstElementPtr response = BaseCommandMgr::handleCommand(final_cmd_name,
+                                                             final_params);
 
     // 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 ((cmd_name == "list-commands") && hook_response && response) {
+    if ((final_cmd_name == "list-commands") && hook_response && response) {
         response = combineCommandsLists(hook_response, response);
     }
 
index 1787743630f6ac144451d02a75fd991c7e1a7451..29f839997348dd849aae824a606fceb9b05e2ad9 100644 (file)
@@ -93,7 +93,7 @@ public:
         return (0);
     }
 
-    /// @brief Text callback which stores callout name and passed arguments and
+    /// @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
@@ -130,6 +130,38 @@ public:
         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";
+
+        ConstElementPtr command;
+        callout_handle.getArgument("command", command);
+
+        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("command", command);
+
+        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 Name of the command (used in my_handler)
     static std::string handler_name;
 
@@ -408,3 +440,53 @@ TEST_F(CommandMgrTest, delegateListCommands) {
     EXPECT_EQ("my-command", command_names_list[1]);
     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]);
+
+}