diff options
author | Razvan Becheriu <razvan@isc.org> | 2023-02-26 16:23:40 +0100 |
---|---|---|
committer | Razvan Becheriu <razvan@isc.org> | 2023-03-16 20:26:11 +0100 |
commit | db352cc04cf99e5b877d02f87eaa8dbe2bc82f10 (patch) | |
tree | f711d6316cebe9b2e0c37bd46ecf7efea0488c6f | |
parent | [#2722] always perform config check before config set (diff) | |
download | kea-db352cc04cf99e5b877d02f87eaa8dbe2bc82f10.tar.xz kea-db352cc04cf99e5b877d02f87eaa8dbe2bc82f10.zip |
[#2722] config set follows the same path as config test and can rollback
-rw-r--r-- | src/bin/agent/ca_process.cc | 3 | ||||
-rw-r--r-- | src/bin/d2/d2_process.cc | 13 | ||||
-rw-r--r-- | src/bin/d2/tests/d2_command_unittest.cc | 4 | ||||
-rw-r--r-- | src/bin/dhcp4/ctrl_dhcp4_srv.cc | 66 | ||||
-rw-r--r-- | src/bin/dhcp4/json_config_parser.cc | 120 | ||||
-rw-r--r-- | src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc | 4 | ||||
-rw-r--r-- | src/bin/dhcp6/ctrl_dhcp6_srv.cc | 58 | ||||
-rw-r--r-- | src/bin/dhcp6/json_config_parser.cc | 120 | ||||
-rw-r--r-- | src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 4 | ||||
-rw-r--r-- | src/bin/netconf/netconf_process.cc | 2 | ||||
-rw-r--r-- | src/lib/cc/command_interpreter.cc | 2 | ||||
-rw-r--r-- | src/lib/cc/tests/command_interpreter_unittests.cc | 8 | ||||
-rw-r--r-- | src/lib/config/hooked_command_mgr.cc | 2 | ||||
-rw-r--r-- | src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc | 8 | ||||
-rw-r--r-- | src/lib/process/d_cfg_mgr.cc | 9 | ||||
-rw-r--r-- | src/lib/process/testutils/d_test_stubs.cc | 10 |
16 files changed, 251 insertions, 182 deletions
diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc index 6e6825eb79..03287d4445 100644 --- a/src/bin/agent/ca_process.cc +++ b/src/bin/agent/ca_process.cc @@ -98,7 +98,8 @@ CtrlAgentProcess::runIO() { isc::data::ConstElementPtr CtrlAgentProcess::shutdown(isc::data::ConstElementPtr /*args*/) { setShutdownFlag(true); - return (isc::config::createAnswer(0, "Control Agent is shutting down")); + return (isc::config::createAnswer(CONTROL_RESULT_SUCCESS, + "Control Agent is shutting down")); } isc::data::ConstElementPtr diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index c06e0bea9a..dc7dff8915 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -17,6 +17,7 @@ #include <hooks/hooks.h> #include <hooks/hooks_manager.h> +using namespace isc::config; using namespace isc::hooks; using namespace isc::process; @@ -221,16 +222,18 @@ D2Process::shutdown(isc::data::ConstElementPtr args) { shutdown_type_ = SD_NOW; } else { setShutdownFlag(false); - return (isc::config::createAnswer(1, "Invalid Shutdown type: " - + type_str)); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, + "Invalid Shutdown type: " + + type_str)); } } } // Set the base class's shutdown flag. setShutdownFlag(true); - return (isc::config::createAnswer(0, "Shutdown initiated, type is: " - + type_str)); + return (isc::config::createAnswer(CONTROL_RESULT_SUCCESS, + "Shutdown initiated, type is: " +\ + type_str)); } isc::data::ConstElementPtr @@ -295,7 +298,7 @@ D2Process::configure(isc::data::ConstElementPtr config_set, bool check_only) { LOG_ERROR(d2_logger, DHCP_DDNS_CONFIGURED_CALLOUT_DROP) .arg(error); reconf_queue_flag_ = false; - answer = isc::config::createAnswer(1, error); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, error); return (answer); } } diff --git a/src/bin/d2/tests/d2_command_unittest.cc b/src/bin/d2/tests/d2_command_unittest.cc index 8f956e79ab..cf4fb2aa49 100644 --- a/src/bin/d2/tests/d2_command_unittest.cc +++ b/src/bin/d2/tests/d2_command_unittest.cc @@ -370,7 +370,7 @@ public: // both structures are built using the same order. EXPECT_EQ(Element::fromJSON(expected_command)->str(), entire_command->str()); - return (createAnswer(0, "long command received ok")); + return (createAnswer(CONTROL_RESULT_SUCCESS, "long command received ok")); } /// @brief Command handler which generates long response. @@ -386,7 +386,7 @@ public: s << std::setw(5) << i; arguments->add(Element::create(s.str())); } - return (createAnswer(0, arguments)); + return (createAnswer(CONTROL_RESULT_SUCCESS, arguments)); } }; diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 6e51e81c56..e5f6261887 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -246,12 +246,12 @@ ControlledDhcpv4Srv::commandLibReloadHandler(const string&, ConstElementPtr) { } } catch (const std::exception& ex) { LOG_ERROR(dhcp4_logger, DHCP4_HOOKS_LIBS_RELOAD_FAIL); - ConstElementPtr answer = isc::config::createAnswer(1, ex.what()); + ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); return (answer); } - ConstElementPtr answer = isc::config::createAnswer(0, - "Hooks libraries successfully reloaded" - " (WARNING: libreload is deprecated)."); + ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, + "Hooks libraries successfully reloaded " + "(WARNING: libreload is deprecated)."); return (answer); } @@ -281,7 +281,7 @@ ControlledDhcpv4Srv::commandConfigGetHandler(const string&, ConstElementPtr /*args*/) { ConstElementPtr config = CfgMgr::instance().getCurrentCfg()->toElement(); - return (createAnswer(0, config)); + return (createAnswer(CONTROL_RESULT_SUCCESS, config)); } ConstElementPtr @@ -391,25 +391,6 @@ ControlledDhcpv4Srv::commandConfigSetHandler(const string&, // configuration attempts. CfgMgr::instance().rollback(); - // Let's first check the config - ConstElementPtr result = checkConfig(dhcp4); - - int rcode = 0; - isc::config::parseAnswer(rcode, result); - if (rcode != CONTROL_RESULT_SUCCESS) { - return (result); - } - - // disable multi-threading (it will be applied by new configuration) - // this must be done in order to properly handle MT to ST transition - // when 'multi-threading' structure is missing from new config - MultiThreadingMgr::instance().apply(false, 0, 0); - - // We are starting the configuration process so we should remove any - // staging configuration that has been created during previous - // configuration attempts. - CfgMgr::instance().rollback(); - // Parse the logger configuration explicitly into the staging config. // Note this does not alter the current loggers, they remain in // effect until we apply the logging config below. If no logging @@ -421,11 +402,12 @@ ControlledDhcpv4Srv::commandConfigSetHandler(const string&, CfgMgr::instance().getStagingCfg()->applyLoggingCfg(); // Now we configure the server proper. - result = processConfig(dhcp4); + ConstElementPtr result = processConfig(dhcp4); // If the configuration parsed successfully, apply the new logger // configuration and the commit the new configuration. We apply // the logging first in case there's a configuration failure. + int rcode = 0; isc::config::parseAnswer(rcode, result); if (rcode == CONTROL_RESULT_SUCCESS) { CfgMgr::instance().getStagingCfg()->applyLoggingCfg(); @@ -645,7 +627,7 @@ ControlledDhcpv4Srv::commandVersionGetHandler(const string&, ConstElementPtr) { ElementPtr extended = Element::create(Dhcpv4Srv::getVersion(true)); ElementPtr arguments = Element::createMap(); arguments->set("extended", extended); - ConstElementPtr answer = isc::config::createAnswer(0, + ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, Dhcpv4Srv::getVersion(false), arguments); return (answer); @@ -655,7 +637,7 @@ ConstElementPtr ControlledDhcpv4Srv::commandBuildReportHandler(const string&, ConstElementPtr) { ConstElementPtr answer = - isc::config::createAnswer(0, isc::detail::getConfigReport()); + isc::config::createAnswer(CONTROL_RESULT_SUCCESS, isc::detail::getConfigReport()); return (answer); } @@ -791,7 +773,7 @@ ControlledDhcpv4Srv::commandStatusGetHandler(const string&, } status->set("sockets", sockets); - return (createAnswer(0, status)); + return (createAnswer(CONTROL_RESULT_SUCCESS, status)); } ConstElementPtr @@ -830,7 +812,7 @@ ControlledDhcpv4Srv::processCommand(const string& command, ControlledDhcpv4Srv* srv = ControlledDhcpv4Srv::getInstance(); if (!srv) { - ConstElementPtr no_srv = isc::config::createAnswer(1, + ConstElementPtr no_srv = isc::config::createAnswer(CONTROL_RESULT_ERROR, "Server object not initialized, so can't process command '" + command + "', arguments: '" + txt + "'."); return (no_srv); @@ -883,11 +865,11 @@ ControlledDhcpv4Srv::processCommand(const string& command, return (srv->commandStatusGetHandler(command, args)); } - return (isc::config::createAnswer(1, "Unrecognized command:" + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, "Unrecognized command:" + command)); } catch (const isc::Exception& ex) { - return (isc::config::createAnswer(1, "Error while processing command '" + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, "Error while processing command '" + command + "': " + ex.what() + ", params: '" + txt + "'")); } @@ -902,7 +884,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { if (!srv) { err << "Server object not initialized, can't process config."; - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_RECEIVED) @@ -920,7 +902,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { } } catch (const std::exception& ex) { err << "Failed to process configuration:" << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Re-open lease and host database with new parameters. @@ -945,7 +927,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { srv->getNetworkState()->reset(NetworkState::Origin::DB_CONNECTION); } catch (const std::exception& ex) { err << "Unable to open database: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Server will start DDNS communications if its enabled. @@ -954,7 +936,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { } catch (const std::exception& ex) { err << "Error starting DHCP_DDNS client after server reconfiguration: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Setup DHCPv4-over-DHCPv6 IPC @@ -963,7 +945,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { } catch (const std::exception& ex) { err << "error starting DHCPv4-over-DHCPv6 IPC " " after server reconfiguration: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Configure DHCP packet queueing @@ -978,7 +960,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { } catch (const std::exception& ex) { err << "Error setting packet queue controls after server reconfiguration: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Configure a callback to shut down the server when the bind socket @@ -1008,7 +990,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { err << "unable to setup timers for periodically running the" " reclamation of the expired leases: " << ex.what() << "."; - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Setup config backend polling, if configured for it. @@ -1094,12 +1076,10 @@ ControlledDhcpv4Srv::checkConfig(isc::data::ConstElementPtr config) { ControlledDhcpv4Srv* srv = ControlledDhcpv4Srv::getInstance(); - // Single stream instance used in all error clauses - std::ostringstream err; - if (!srv) { - err << "Server object not initialized, can't process config."; - return (isc::config::createAnswer(1, err.str())); + ConstElementPtr no_srv = isc::config::createAnswer(CONTROL_RESULT_ERROR, + "Server object not initialized, can't process config."); + return (no_srv); } return (configureDhcp4Server(*srv, config, true)); diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index c2244d5da1..a602c7c98b 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -321,29 +321,11 @@ void configureCommandChannel() { } isc::data::ConstElementPtr -configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, - bool check_only) { - if (!config_set) { - ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, - string("Can't parse NULL config")); - return (answer); - } - - LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_START) - .arg(server.redactConfig(config_set)->str()); - +processDhcp4Config(isc::data::ConstElementPtr config_set) { // Before starting any subnet operations, let's reset the subnet-id counter, // so newly recreated configuration starts with first subnet-id equal 1. Subnet::resetSubnetID(); - // Close DHCP sockets and remove any existing timers. - if (!check_only) { - IfaceMgr::instance().closeSockets(); - TimerMgr::instance()->unregisterTimers(); - server.discardPackets(); - server.getCBControl()->reset(); - } - // Revert any runtime option definitions configured so far and not committed. LibDHCP::revertRuntimeOptionDefs(); // Let's set empty container in case a user hasn't specified any configuration @@ -355,9 +337,7 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, // Answer will hold the result. ConstElementPtr answer; - // Rollback informs whether error occurred and original data - // have to be restored to global storages. - bool rollback = false; + // Global parameter name in case of an error. string parameter_name; ElementPtr mutable_cfg; @@ -459,7 +439,7 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, ConstElementPtr ifaces_config = mutable_cfg->get("interfaces-config"); if (ifaces_config) { parameter_name = "interfaces-config"; - IfacesConfigParser parser(AF_INET, check_only); + IfacesConfigParser parser(AF_INET, true); CfgIfacePtr cfg_iface = srv_config->getCfgIface(); parser.parse(cfg_iface, ifaces_config); } @@ -740,25 +720,95 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, LOG_ERROR(dhcp4_logger, DHCP4_PARSER_FAIL) .arg(parameter_name).arg(ex.what()); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); - - // An error occurred, so make sure that we restore original data. - rollback = true; } catch (...) { // For things like bad_cast in boost::lexical_cast LOG_ERROR(dhcp4_logger, DHCP4_PARSER_EXCEPTION).arg(parameter_name); - answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" - " processing error"); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration " + "processing error"); + } - // An error occurred, so make sure that we restore original data. - rollback = true; + if (!answer) { + answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, "Configuration seems sane. " + "Control-socket, hook-libraries, and D2 configuration " + "were sanity checked, but not applied."); } - if (check_only) { + return (answer); +} + +isc::data::ConstElementPtr +configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, + bool check_only) { + if (!config_set) { + ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, + "Can't parse NULL config"); + return (answer); + } + + LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_START) + .arg(server.redactConfig(config_set)->str()); + + // Rollback informs whether error occurred and original data + // have to be restored to global storages. + bool rollback = false; + + auto answer = processDhcp4Config(config_set); + + int status_code = 0; + isc::config::parseAnswer(status_code, answer); + if (status_code != CONTROL_RESULT_SUCCESS) { rollback = true; - if (!answer) { - answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, - "Configuration seems sane. Control-socket, hook-libraries, and D2 " - "configuration were sanity checked, but not applied."); + } + + SrvConfigPtr srv_config; + + if (!rollback) { + if (!check_only) { + string parameter_name; + ElementPtr mutable_cfg; + + // Close DHCP sockets and remove any existing timers. + IfaceMgr::instance().closeSockets(); + TimerMgr::instance()->unregisterTimers(); + server.discardPackets(); + server.getCBControl()->reset(); + + try { + + // Get the staging configuration. + srv_config = CfgMgr::instance().getStagingCfg(); + + // This is a way to convert ConstElementPtr to ElementPtr. + // We need a config that can be edited, because we will insert + // default values and will insert derived values as well. + mutable_cfg = boost::const_pointer_cast<Element>(config_set); + + ConstElementPtr ifaces_config = mutable_cfg->get("interfaces-config"); + if (ifaces_config) { + parameter_name = "interfaces-config"; + IfacesConfigParser parser(AF_INET, false); + CfgIfacePtr cfg_iface = srv_config->getCfgIface(); + cfg_iface->reset(); + parser.parse(cfg_iface, ifaces_config); + } + } catch (const isc::Exception& ex) { + LOG_ERROR(dhcp4_logger, DHCP4_PARSER_FAIL) + .arg(parameter_name).arg(ex.what()); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); + + // An error occurred, so make sure that we restore original data. + rollback = true; + } catch (...) { + // For things like bad_cast in boost::lexical_cast + LOG_ERROR(dhcp4_logger, DHCP4_PARSER_EXCEPTION).arg(parameter_name); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" + " processing error"); + + // An error occurred, so make sure that we restore original data. + rollback = true; + } + } else { + rollback = true; } } diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 87ea2ea0d3..f827f0ebff 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -380,7 +380,7 @@ public: // both structures are built using the same order. EXPECT_EQ(Element::fromJSON(expected_command)->str(), entire_command->str()); - return (createAnswer(0, "long command received ok")); + return (createAnswer(CONTROL_RESULT_SUCCESS, "long command received ok")); } /// @brief Command handler which generates long response @@ -396,7 +396,7 @@ public: s << std::setw(5) << i; arguments->add(Element::create(s.str())); } - return (createAnswer(0, arguments)); + return (createAnswer(CONTROL_RESULT_SUCCESS, arguments)); } }; diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index be4475d746..62c37e2754 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -249,10 +249,10 @@ ControlledDhcpv6Srv::commandLibReloadHandler(const string&, ConstElementPtr) { } } catch (const std::exception& ex) { LOG_ERROR(dhcp6_logger, DHCP6_HOOKS_LIBS_RELOAD_FAIL); - ConstElementPtr answer = isc::config::createAnswer(1, ex.what()); + ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); return (answer); } - ConstElementPtr answer = isc::config::createAnswer(0, + ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, "Hooks libraries successfully reloaded " "(WARNING: libreload is deprecated)."); return (answer); @@ -284,7 +284,7 @@ ControlledDhcpv6Srv::commandConfigGetHandler(const string&, ConstElementPtr /*args*/) { ConstElementPtr config = CfgMgr::instance().getCurrentCfg()->toElement(); - return (createAnswer(0, config)); + return (createAnswer(CONTROL_RESULT_SUCCESS, config)); } ConstElementPtr @@ -394,25 +394,6 @@ ControlledDhcpv6Srv::commandConfigSetHandler(const string&, // configuration attempts. CfgMgr::instance().rollback(); - // Let's first check the config - ConstElementPtr result = checkConfig(dhcp6); - - int rcode = 0; - isc::config::parseAnswer(rcode, result); - if (rcode != CONTROL_RESULT_SUCCESS) { - return (result); - } - - // disable multi-threading (it will be applied by new configuration) - // this must be done in order to properly handle MT to ST transition - // when 'multi-threading' structure is missing from new config - MultiThreadingMgr::instance().apply(false, 0, 0); - - // We are starting the configuration process so we should remove any - // staging configuration that has been created during previous - // configuration attempts. - CfgMgr::instance().rollback(); - // Parse the logger configuration explicitly into the staging config. // Note this does not alter the current loggers, they remain in // effect until we apply the logging config below. If no logging @@ -424,11 +405,12 @@ ControlledDhcpv6Srv::commandConfigSetHandler(const string&, CfgMgr::instance().getStagingCfg()->applyLoggingCfg(); // Now we configure the server proper. - result = processConfig(dhcp6); + ConstElementPtr result = processConfig(dhcp6); // If the configuration parsed successfully, apply the new logger // configuration and the commit the new configuration. We apply // the logging first in case there's a configuration failure. + int rcode = 0; isc::config::parseAnswer(rcode, result); if (rcode == CONTROL_RESULT_SUCCESS) { CfgMgr::instance().getStagingCfg()->applyLoggingCfg(); @@ -648,7 +630,7 @@ ControlledDhcpv6Srv::commandVersionGetHandler(const string&, ConstElementPtr) { ElementPtr extended = Element::create(Dhcpv6Srv::getVersion(true)); ElementPtr arguments = Element::createMap(); arguments->set("extended", extended); - ConstElementPtr answer = isc::config::createAnswer(0, + ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, Dhcpv6Srv::getVersion(false), arguments); return (answer); @@ -658,7 +640,7 @@ ConstElementPtr ControlledDhcpv6Srv::commandBuildReportHandler(const string&, ConstElementPtr) { ConstElementPtr answer = - isc::config::createAnswer(0, isc::detail::getConfigReport()); + isc::config::createAnswer(CONTROL_RESULT_SUCCESS, isc::detail::getConfigReport()); return (answer); } @@ -794,7 +776,7 @@ ControlledDhcpv6Srv::commandStatusGetHandler(const string&, } status->set("sockets", sockets); - return (createAnswer(0, status)); + return (createAnswer(CONTROL_RESULT_SUCCESS, status)); } ConstElementPtr @@ -833,7 +815,7 @@ ControlledDhcpv6Srv::processCommand(const string& command, ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance(); if (!srv) { - ConstElementPtr no_srv = isc::config::createAnswer(1, + ConstElementPtr no_srv = isc::config::createAnswer(CONTROL_RESULT_ERROR, "Server object not initialized, so can't process command '" + command + "', arguments: '" + txt + "'."); return (no_srv); @@ -886,11 +868,11 @@ ControlledDhcpv6Srv::processCommand(const string& command, return (srv->commandStatusGetHandler(command, args)); } - return (isc::config::createAnswer(1, "Unrecognized command:" + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, "Unrecognized command:" + command)); } catch (const isc::Exception& ex) { - return (isc::config::createAnswer(1, "Error while processing command '" + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, "Error while processing command '" + command + "': " + ex.what() + ", params: '" + txt + "'")); } @@ -906,7 +888,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { if (!srv) { err << "Server object not initialized, can't process config."; - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_RECEIVED) @@ -924,7 +906,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { } } catch (const std::exception& ex) { err << "Failed to process configuration:" << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Re-open lease and host database with new parameters. @@ -949,7 +931,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { srv->getNetworkState()->reset(NetworkState::Origin::DB_CONNECTION); } catch (const std::exception& ex) { err << "Unable to open database: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Regenerate server identifier if needed. @@ -967,7 +949,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { } catch (const std::exception& ex) { err << "unable to configure server identifier: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Server will start DDNS communications if its enabled. @@ -976,7 +958,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { } catch (const std::exception& ex) { err << "Error starting DHCP_DDNS client after server reconfiguration: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Setup DHCPv4-over-DHCPv6 IPC @@ -985,7 +967,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { } catch (const std::exception& ex) { err << "error starting DHCPv4-over-DHCPv6 IPC " " after server reconfiguration: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Configure DHCP packet queueing @@ -1000,7 +982,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { } catch (const std::exception& ex) { err << "Error setting packet queue controls after server reconfiguration: " << ex.what(); - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Configure a callback to shut down the server when the bind socket @@ -1029,7 +1011,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { err << "unable to setup timers for periodically running the" " reclamation of the expired leases: " << ex.what() << "."; - return (isc::config::createAnswer(1, err.str())); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); } // Setup config backend polling, if configured for it. @@ -1116,7 +1098,7 @@ ControlledDhcpv6Srv::checkConfig(isc::data::ConstElementPtr config) { ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance(); if (!srv) { - ConstElementPtr no_srv = isc::config::createAnswer(1, + ConstElementPtr no_srv = isc::config::createAnswer(CONTROL_RESULT_ERROR, "Server object not initialized, can't process config."); return (no_srv); } diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 91a3d68873..92cc046da7 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -424,29 +424,11 @@ void configureCommandChannel() { } isc::data::ConstElementPtr -configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, - bool check_only) { - if (!config_set) { - ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, - string("Can't parse NULL config")); - return (answer); - } - - LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_START) - .arg(server.redactConfig(config_set)->str()); - +processDhcp6Config(isc::data::ConstElementPtr config_set) { // Before starting any subnet operations, let's reset the subnet-id counter, // so newly recreated configuration starts with first subnet-id equal 1. Subnet::resetSubnetID(); - // Close DHCP sockets and remove any existing timers. - if (!check_only) { - IfaceMgr::instance().closeSockets(); - TimerMgr::instance()->unregisterTimers(); - server.discardPackets(); - server.getCBControl()->reset(); - } - // Revert any runtime option definitions configured so far and not committed. LibDHCP::revertRuntimeOptionDefs(); // Let's set empty container in case a user hasn't specified any configuration @@ -458,9 +440,7 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, // Answer will hold the result. ConstElementPtr answer; - // Rollback informs whether error occurred and original data - // have to be restored to global storages. - bool rollback = false; + // Global parameter name in case of an error. string parameter_name; ElementPtr mutable_cfg; @@ -585,7 +565,7 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, ConstElementPtr ifaces_config = mutable_cfg->get("interfaces-config"); if (ifaces_config) { parameter_name = "interfaces-config"; - IfacesConfigParser parser(AF_INET6, check_only); + IfacesConfigParser parser(AF_INET6, true); CfgIfacePtr cfg_iface = srv_config->getCfgIface(); parser.parse(cfg_iface, ifaces_config); } @@ -869,25 +849,95 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL) .arg(parameter_name).arg(ex.what()); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); - - // An error occurred, so make sure that we restore original data. - rollback = true; } catch (...) { // For things like bad_cast in boost::lexical_cast LOG_ERROR(dhcp6_logger, DHCP6_PARSER_EXCEPTION).arg(parameter_name); - answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" - " processing error"); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration " + "processing error"); + } - // An error occurred, so make sure that we restore original data. - rollback = true; + if (!answer) { + answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, "Configuration seems sane. " + "Control-socket, hook-libraries, and D2 configuration " + "were sanity checked, but not applied."); } - if (check_only) { + return (answer); +} + +isc::data::ConstElementPtr +configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, + bool check_only) { + if (!config_set) { + ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, + "Can't parse NULL config"); + return (answer); + } + + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_START) + .arg(server.redactConfig(config_set)->str()); + + // Rollback informs whether error occurred and original data + // have to be restored to global storages. + bool rollback = false; + + auto answer = processDhcp6Config(config_set); + + int status_code = 0; + isc::config::parseAnswer(status_code, answer); + if (status_code != CONTROL_RESULT_SUCCESS) { rollback = true; - if (!answer) { - answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, - "Configuration seems sane. Control-socket, hook-libraries, and D2 " - "configuration were sanity checked, but not applied."); + } + + SrvConfigPtr srv_config; + + if (!rollback) { + if (!check_only) { + string parameter_name; + ElementPtr mutable_cfg; + + // Close DHCP sockets and remove any existing timers. + IfaceMgr::instance().closeSockets(); + TimerMgr::instance()->unregisterTimers(); + server.discardPackets(); + server.getCBControl()->reset(); + + try { + + // Get the staging configuration. + srv_config = CfgMgr::instance().getStagingCfg(); + + // This is a way to convert ConstElementPtr to ElementPtr. + // We need a config that can be edited, because we will insert + // default values and will insert derived values as well. + mutable_cfg = boost::const_pointer_cast<Element>(config_set); + + ConstElementPtr ifaces_config = mutable_cfg->get("interfaces-config"); + if (ifaces_config) { + parameter_name = "interfaces-config"; + IfacesConfigParser parser(AF_INET6, false); + CfgIfacePtr cfg_iface = srv_config->getCfgIface(); + cfg_iface->reset(); + parser.parse(cfg_iface, ifaces_config); + } + } catch (const isc::Exception& ex) { + LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL) + .arg(parameter_name).arg(ex.what()); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); + + // An error occurred, so make sure that we restore original data. + rollback = true; + } catch (...) { + // For things like bad_cast in boost::lexical_cast + LOG_ERROR(dhcp6_logger, DHCP6_PARSER_EXCEPTION).arg(parameter_name); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" + " processing error"); + + // An error occurred, so make sure that we restore original data. + rollback = true; + } + } else { + rollback = true; } } diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index d6a3465543..a583751421 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -398,7 +398,7 @@ public: // both structures are built using the same order. EXPECT_EQ(Element::fromJSON(expected_command)->str(), entire_command->str()); - return (createAnswer(0, "long command received ok")); + return (createAnswer(CONTROL_RESULT_SUCCESS, "long command received ok")); } /// @brief Command handler which generates long response @@ -414,7 +414,7 @@ public: s << std::setw(5) << i; arguments->add(Element::create(s.str())); } - return (createAnswer(0, arguments)); + return (createAnswer(CONTROL_RESULT_SUCCESS, arguments)); } }; diff --git a/src/bin/netconf/netconf_process.cc b/src/bin/netconf/netconf_process.cc index eafc036108..c4a6b756e3 100644 --- a/src/bin/netconf/netconf_process.cc +++ b/src/bin/netconf/netconf_process.cc @@ -75,7 +75,7 @@ NetconfProcess::runIO() { isc::data::ConstElementPtr NetconfProcess::shutdown(isc::data::ConstElementPtr /*args*/) { setShutdownFlag(true); - return (isc::config::createAnswer(0, "Netconf is shutting down")); + return (isc::config::createAnswer(CONTROL_RESULT_SUCCESS, "Netconf is shutting down")); } isc::data::ConstElementPtr diff --git a/src/lib/cc/command_interpreter.cc b/src/lib/cc/command_interpreter.cc index 68f880cfc1..c3c2a7a233 100644 --- a/src/lib/cc/command_interpreter.cc +++ b/src/lib/cc/command_interpreter.cc @@ -52,7 +52,7 @@ createAnswer(const int status_code, const std::string& text, ConstElementPtr createAnswer() { - return (createAnswer(0, string(""), ConstElementPtr())); + return (createAnswer(CONTROL_RESULT_SUCCESS, string(""), ConstElementPtr())); } ConstElementPtr diff --git a/src/lib/cc/tests/command_interpreter_unittests.cc b/src/lib/cc/tests/command_interpreter_unittests.cc index 12b3eab658..e393eeb473 100644 --- a/src/lib/cc/tests/command_interpreter_unittests.cc +++ b/src/lib/cc/tests/command_interpreter_unittests.cc @@ -42,17 +42,17 @@ TEST(CommandInterpreterTest, createAnswer) { EXPECT_EQ("{ \"result\": 0 }", answer->str()); // Let's check if we can generate an error. - answer = createAnswer(1, "error"); + answer = createAnswer(CONTROL_RESULT_ERROR, "error"); EXPECT_EQ("{ \"result\": 1, \"text\": \"error\" }", answer->str()); // This is expected to throw. When status code is non-zero (indicating error), // textual explanation is mandatory. - EXPECT_THROW(createAnswer(1, ElementPtr()), CtrlChannelError); - EXPECT_THROW(createAnswer(1, Element::create(1)), CtrlChannelError); + EXPECT_THROW(createAnswer(CONTROL_RESULT_ERROR, ElementPtr()), CtrlChannelError); + EXPECT_THROW(createAnswer(CONTROL_RESULT_ERROR, Element::create(1)), CtrlChannelError); // Let's check if answer can be generate with some data in it. ConstElementPtr arg = el("[ \"just\", \"some\", \"data\" ]"); - answer = createAnswer(0, arg); + answer = createAnswer(CONTROL_RESULT_SUCCESS, arg); EXPECT_EQ("{ \"arguments\": [ \"just\", \"some\", \"data\" ], \"result\": 0 }", answer->str()); } diff --git a/src/lib/config/hooked_command_mgr.cc b/src/lib/config/hooked_command_mgr.cc index 3562e71ff3..8ec31c75f4 100644 --- a/src/lib/config/hooked_command_mgr.cc +++ b/src/lib/config/hooked_command_mgr.cc @@ -120,7 +120,7 @@ HookedCommandMgr::handleCommand(const std::string& cmd_name, // registered // for it, combine the lists of commands. if (!hooks_commands->empty()) { - response = combineCommandsLists(response, createAnswer(0, hooks_commands)); + response = combineCommandsLists(response, createAnswer(CONTROL_RESULT_SUCCESS, hooks_commands)); } } } diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 038314500f..2f052804f1 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -191,7 +191,7 @@ public: // Answer will hold the result. ConstElementPtr answer; if (!config_set) { - answer = isc::config::createAnswer(1, + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, string("Can't parse NULL config")); return (answer); } @@ -302,13 +302,13 @@ public: } // Everything was fine. Configuration is successful. - answer = isc::config::createAnswer(0, "Configuration committed."); + answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, "Configuration committed."); } catch (const isc::Exception& ex) { - answer = isc::config::createAnswer(1, + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, string("Configuration parsing failed: ") + ex.what()); } catch (...) { - answer = isc::config::createAnswer(1, + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, string("Configuration parsing failed")); } diff --git a/src/lib/process/d_cfg_mgr.cc b/src/lib/process/d_cfg_mgr.cc index 4a67183128..5ee15ec7fb 100644 --- a/src/lib/process/d_cfg_mgr.cc +++ b/src/lib/process/d_cfg_mgr.cc @@ -26,6 +26,7 @@ using namespace std; using namespace isc; +using namespace isc::config; using namespace isc::dhcp; using namespace isc::data; using namespace isc::asiolink; @@ -76,8 +77,8 @@ DCfgMgrBase::simpleParseConfig(isc::data::ConstElementPtr config_set, bool check_only, const std::function<void()>& post_config_cb) { if (!config_set) { - return (isc::config::createAnswer(1, - std::string("Can't parse NULL config"))); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, + std::string("Can't parse NULL config"))); } LOG_DEBUG(dctl_logger, isc::log::DBGLVL_COMMAND, DCTL_CONFIG_START) .arg(redactConfig(config_set)->str()); @@ -122,7 +123,7 @@ DCfgMgrBase::simpleParseConfig(isc::data::ConstElementPtr config_set, } // Use the answer provided. - //answer = isc::config::createAnswer(0, "Configuration committed."); + //answer = isc::config::createAnswer(CONTROL_RESULT_SUCCESS, "Configuration committed."); } else { LOG_INFO(dctl_logger, DCTL_CONFIG_CHECK_COMPLETE) .arg(getConfigSummary(0)) @@ -131,7 +132,7 @@ DCfgMgrBase::simpleParseConfig(isc::data::ConstElementPtr config_set, } catch (const std::exception& ex) { LOG_ERROR(dctl_logger, DCTL_PARSER_FAIL).arg(ex.what()); - answer = isc::config::createAnswer(1, ex.what()); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); rollback = true; } diff --git a/src/lib/process/testutils/d_test_stubs.cc b/src/lib/process/testutils/d_test_stubs.cc index 1186729e06..78b73d6ecf 100644 --- a/src/lib/process/testutils/d_test_stubs.cc +++ b/src/lib/process/testutils/d_test_stubs.cc @@ -61,15 +61,16 @@ DStubProcess::shutdown(isc::data::ConstElementPtr /* args */) { setShutdownFlag(true); stopIOService(); - return (isc::config::createAnswer(0, "Shutdown initiated.")); + return (isc::config::createAnswer(isc::config::CONTROL_RESULT_SUCCESS, + "Shutdown initiated.")); } isc::data::ConstElementPtr DStubProcess::configure(isc::data::ConstElementPtr config_set, bool check_only) { if (SimFailure::shouldFailOn(SimFailure::ftProcessConfigure)) { // Simulates a process configure failure. - return (isc::config::createAnswer(1, - "Simulated process configuration error.")); + return (isc::config::createAnswer(isc::config::CONTROL_RESULT_ERROR, + "Simulated process configuration error.")); } return (getCfgMgr()->simpleParseConfig(config_set, check_only)); @@ -329,7 +330,8 @@ DStubCfgMgr::createNewContext() { isc::data::ConstElementPtr DStubCfgMgr::parse(isc::data::ConstElementPtr /*config*/, bool /*check_only*/) { - return (isc::config::createAnswer(0, "It all went fine. I promise")); + return (isc::config::createAnswer(isc::config::CONTROL_RESULT_SUCCESS, + "It all went fine. I promise")); } } // namespace isc::process |