Commit 8041d177 authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[master] Merge branch 'trac5330'

parents d4f9ab1a 24df9ecb
......@@ -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
......
......@@ -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
......
......@@ -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
......
shell_process_tests.sh
/shell_process_tests.sh
/shell_unittest.py
......@@ -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
......
......@@ -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);
......
......@@ -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.
......
......@@ -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;