From: Razvan Becheriu Date: Thu, 16 Mar 2023 19:16:22 +0000 (+0200) Subject: [#1671] the -t parameter now loads and checks hook libraries config X-Git-Tag: Kea-2.3.6~63 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=35238f03aaf219bd1903cf5b43a58dc9fd71f5e8;p=thirdparty%2Fkea.git [#1671] the -t parameter now loads and checks hook libraries config --- diff --git a/doc/sphinx/arm/agent.rst b/doc/sphinx/arm/agent.rst index 1b827086d6..3dc0118d24 100644 --- a/doc/sphinx/arm/agent.rst +++ b/doc/sphinx/arm/agent.rst @@ -357,8 +357,56 @@ Since Kea 1.9.6, the ``kea-shell`` tool supports TLS. .. _agent-launch: -Starting the Control Agent -========================== +Starting and Stopping the Control Agent +======================================= + +``kea-ctrl-agent`` accepts the following command-line switches: + +- ``-c file`` - specifies the configuration file. + +- ``-d`` - specifies whether the agent logging should be switched to + debug/verbose mode. In verbose mode, the logging severity and + debuglevel specified in the configuration file are ignored and + "debug" severity and the maximum debuglevel (99) are assumed. The + flag is convenient for temporarily switching the server into maximum + verbosity, e.g. when debugging. + +- ``-t file`` - specifies the configuration file to be tested. + ``kea-netconf`` attempts to load it and conducts sanity checks; + certain checks are possible only while running the actual server. The + actual status is reported with exit code (0 = configuration appears valid, + 1 = error encountered). Kea prints out log messages to standard + output and error to standard error when testing the configuration. + +- ``-v`` - displays the version of ``kea-ctrl-agent`` and exits. + +- ``-V`` - displays the extended version information for ``kea-ctrl-agent`` + and exits. The listing includes the versions of the libraries + dynamically linked to Kea. + +- ``-W`` - displays the Kea configuration report and exits. The report + is a copy of the ``config.report`` file produced by ``./configure``; + it is embedded in the executable binary. + +The ``config.report`` file may also be accessed directly, via the +following command. The binary ``path`` may be found in the install +directory or in the ``.libs`` subdirectory in the source tree. For +example: ``kea/src/lib/process/.libs/``. + +:: + + strings path/libkea-process.so | sed -n 's/;;;; //p' + +:: + + strings path/libkea-process.a | sed -n 's/;;;; //p' + +The libcfgrpt.a library can also be used from the source tree with path: +``src/lib/process/cfgrpt/.libs/``. + +:: + + strings path/libcfgrpt.a | sed -n 's/;;;; //p' The CA is started by running its binary and specifying the configuration file it should use. For example: diff --git a/doc/sphinx/arm/ddns.rst b/doc/sphinx/arm/ddns.rst index 214cdf9c35..8e2bd844ae 100644 --- a/doc/sphinx/arm/ddns.rst +++ b/doc/sphinx/arm/ddns.rst @@ -141,11 +141,22 @@ directly. It accepts the following command-line switches: The ``config.report`` file may also be accessed directly, via the following command. The binary ``path`` may be found in the install directory or in the ``.libs`` subdirectory in the source tree. For -example: ``kea/src/bin/d2/.libs/kea-dhcp-ddns``. +example: ``kea/src/lib/process/.libs/``. :: - strings path/kea-dhcp-ddns | sed -n 's/;;;; //p' + strings path/libkea-process.so | sed -n 's/;;;; //p' + +:: + + strings path/libkea-process.a | sed -n 's/;;;; //p' + +The libcfgrpt.a library can also be used from the source tree with path: +``src/lib/process/cfgrpt/.libs/``. + +:: + + strings path/libcfgrpt.a | sed -n 's/;;;; //p' Upon startup, the module loads its configuration and begins listening for NCRs based on that configuration. diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst index 7723cd71ed..ee74e38b39 100644 --- a/doc/sphinx/arm/dhcp4-srv.rst +++ b/doc/sphinx/arm/dhcp4-srv.rst @@ -42,6 +42,14 @@ the following command-line switches: comprehensive; certain checks are possible only when running the server. +- ``-T file`` - specifies a configuration file to be tested. ``kea-dhcp4`` + loads it, checks it, and exits. It performs extra checks beside ``-t`` is + doing, like establising database connections (lease db, host db, CB db, + forensic logging db), hook libraries loading and configuration parsing, etc. + It does not open unix or TCP/UDP sockets, neither does it open or rotate + files, as all these actions could interfere with a running process on the + same machine. + - ``-v`` - displays the Kea version and exits. - ``-V`` - displays the Kea extended version with additional parameters @@ -52,6 +60,26 @@ the following command-line switches: is a copy of the ``config.report`` file produced by ``./configure``; it is embedded in the executable binary. +The ``config.report`` file may also be accessed directly, via the +following command. The binary ``path`` may be found in the install +directory or in the ``.libs`` subdirectory in the source tree. For +example: ``kea/src/lib/process/.libs/``. + +:: + + strings path/libkea-process.so | sed -n 's/;;;; //p' + +:: + + strings path/libkea-process.a | sed -n 's/;;;; //p' + +The libcfgrpt.a library can also be used from the source tree with path: +``src/lib/process/cfgrpt/.libs/``. + +:: + + strings path/libcfgrpt.a | sed -n 's/;;;; //p' + On startup, the server detects available network interfaces and attempts to open UDP sockets on all interfaces listed in the configuration file. Since the DHCPv4 server opens privileged ports, it diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst index e21535ecea..d8ca1203a8 100644 --- a/doc/sphinx/arm/dhcp6-srv.rst +++ b/doc/sphinx/arm/dhcp6-srv.rst @@ -42,6 +42,14 @@ the following command-line switches: comprehensive; certain checks are possible only when running the server. +- ``-T file`` - specifies a configuration file to be tested. ``kea-dhcp4`` + loads it, checks it, and exits. It performs extra checks beside ``-t`` is + doing, like establising database connections (lease db, host db, CB db, + forensic logging db), hook libraries loading and configuration parsing, etc. + It does not open unix or TCP/UDP sockets, neither does it open or rotate + files, as all these actions could interfere with a running process on the + same machine. + - ``-v`` - displays the Kea version and exits. - ``-V`` - displays the Kea extended version with additional parameters @@ -52,6 +60,26 @@ the following command-line switches: is a copy of the ``config.report`` file produced by ``./configure``; it is embedded in the executable binary. +The ``config.report`` file may also be accessed directly, via the +following command. The binary ``path`` may be found in the install +directory or in the ``.libs`` subdirectory in the source tree. For +example: ``kea/src/lib/process/.libs/``. + +:: + + strings path/libkea-process.so | sed -n 's/;;;; //p' + +:: + + strings path/libkea-process.a | sed -n 's/;;;; //p' + +The libcfgrpt.a library can also be used from the source tree with path: +``src/lib/process/cfgrpt/.libs/``. + +:: + + strings path/libcfgrpt.a | sed -n 's/;;;; //p' + On startup, the server detects available network interfaces and attempts to open UDP sockets on all interfaces listed in the configuration file. Since the DHCPv6 server opens privileged ports, it diff --git a/doc/sphinx/arm/ext-netconf.rst b/doc/sphinx/arm/ext-netconf.rst index a1c9ce3822..f4bc9922cf 100644 --- a/doc/sphinx/arm/ext-netconf.rst +++ b/doc/sphinx/arm/ext-netconf.rst @@ -686,6 +686,26 @@ Starting and Stopping the NETCONF Agent is a copy of the ``config.report`` file produced by ``./configure``; it is embedded in the executable binary. +The ``config.report`` file may also be accessed directly, via the +following command. The binary ``path`` may be found in the install +directory or in the ``.libs`` subdirectory in the source tree. For +example: ``kea/src/lib/process/.libs/``. + +:: + + strings path/libkea-process.so | sed -n 's/;;;; //p' + +:: + + strings path/libkea-process.a | sed -n 's/;;;; //p' + +The libcfgrpt.a library can also be used from the source tree with path: +``src/lib/process/cfgrpt/.libs/``. + +:: + + strings path/libcfgrpt.a | sed -n 's/;;;; //p' + .. _operation-example: A Step-by-Step NETCONF Agent Operation Example diff --git a/doc/sphinx/man/kea-dhcp4.8.rst b/doc/sphinx/man/kea-dhcp4.8.rst index be59b0f3d1..1061720dd0 100644 --- a/doc/sphinx/man/kea-dhcp4.8.rst +++ b/doc/sphinx/man/kea-dhcp4.8.rst @@ -49,6 +49,14 @@ The arguments are as follows: service and control channel sockets are not opened, and hook libraries are not loaded. +``-T config-file`` + Checks the configuration file and reports the first error, if any. + It performs extra checks beside ``-t`` is doing, like establising database + connections (lease db, host db, CB db, forensic logging db), hook libraries + loading and configuration parsing, etc. It does not open unix or TCP/UDP + sockets, neither does it open or rotate files, as all these actions could + interfere with a running process on the same machine. + ``-p server-port-number`` Specifies the server port number (1-65535) on which the server listens. This is useful for testing purposes only. diff --git a/doc/sphinx/man/kea-dhcp6.8.rst b/doc/sphinx/man/kea-dhcp6.8.rst index 19ec5e8816..86e2dc1d4f 100644 --- a/doc/sphinx/man/kea-dhcp6.8.rst +++ b/doc/sphinx/man/kea-dhcp6.8.rst @@ -49,6 +49,14 @@ The arguments are as follows: service and control channel sockets are not opened, and hook libraries are not loaded. +``-T config-file`` + Checks the configuration file and reports the first error, if any. + It performs extra checks beside ``-t`` is doing, like establising database + connections (lease db, host db, CB db, forensic logging db), hook libraries + loading and configuration parsing, etc. It does not open unix or TCP/UDP + sockets, neither does it open or rotate files, as all these actions could + interfere with a running process on the same machine. + ``-p server-port-number`` Specifies the server port number (1-65535) on which the server listens. This is useful for testing purposes only. diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index e5f6261887..66a369b118 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -1025,6 +1025,29 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { // exception free. LibDHCP::commitRuntimeOptionDefs(); + auto notify_libraries = ControlledDhcpv4Srv::finishConfigHookLibraries(config); + if (notify_libraries) { + return (notify_libraries); + } + + // Apply multi threading settings. + // @note These settings are applied/updated only if no errors occur while + // applying the new configuration. + // @todo This should be fixed. + try { + CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading()); + } catch (const std::exception& ex) { + err << "Error applying multi threading settings: " + << ex.what(); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); + } + + return (answer); +} + +isc::data::ConstElementPtr +ControlledDhcpv4Srv::finishConfigHookLibraries(isc::data::ConstElementPtr config) { + ControlledDhcpv4Srv* srv = ControlledDhcpv4Srv::getInstance(); // This hook point notifies hooks libraries that the configuration of the // DHCPv4 server has completed. It provides the hook library with the pointer // to the common IO service object, new server configuration in the JSON @@ -1053,27 +1076,11 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { } } - // Apply multi threading settings. - // @note These settings are applied/updated only if no errors occur while - // applying the new configuration. - // @todo This should be fixed. - try { - CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading()); - } catch (const std::exception& ex) { - err << "Error applying multi threading settings: " - << ex.what(); - return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); - } - - return (answer); + return (ConstElementPtr()); } isc::data::ConstElementPtr ControlledDhcpv4Srv::checkConfig(isc::data::ConstElementPtr config) { - - LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_RECEIVED) - .arg(redactConfig(config)->str()); - ControlledDhcpv4Srv* srv = ControlledDhcpv4Srv::getInstance(); if (!srv) { @@ -1082,6 +1089,9 @@ ControlledDhcpv4Srv::checkConfig(isc::data::ConstElementPtr config) { return (no_srv); } + LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_RECEIVED) + .arg(srv->redactConfig(config)->str()); + return (configureDhcp4Server(*srv, config, true)); } diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 2c34610b96..83b339f73e 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -105,21 +105,32 @@ public: /// As pointer to this method is used a callback in ASIO used in /// ModuleCCSession, it has to be static. /// - /// @param new_config textual representation of the new configuration + /// @param config textual representation of the new configuration /// /// @return status of the config update static isc::data::ConstElementPtr - processConfig(isc::data::ConstElementPtr new_config); + processConfig(isc::data::ConstElementPtr config); /// @brief Configuration checker /// /// This is a method for checking incoming configuration. /// - /// @param new_config JSON representation of the new configuration + /// @param config JSON representation of the new configuration /// /// @return status of the config check - isc::data::ConstElementPtr - checkConfig(isc::data::ConstElementPtr new_config); + static isc::data::ConstElementPtr + checkConfig(isc::data::ConstElementPtr config); + + /// @brief Configuration checker for hook libraries + /// + /// This is a method for checking incoming configuration in the hooks + /// libraries. It calls dhcp4_srv_configured hook point for all hooks. + /// + /// @param config JSON representation of the new configuration + /// + /// @return status of the config check + static isc::data::ConstElementPtr + finishConfigHookLibraries(isc::data::ConstElementPtr config); /// @brief Returns pointer to the sole instance of Dhcpv4Srv /// diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 3d6c48ce41..b60f0d033c 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -738,7 +739,7 @@ processDhcp4Config(isc::data::ConstElementPtr config_set) { isc::data::ConstElementPtr configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, - bool check_only) { + bool check_only, bool extra_checks) { if (!config_set) { ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "Can't parse NULL config"); @@ -748,21 +749,14 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, 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; + int status_code = CONTROL_RESULT_SUCCESS; isc::config::parseAnswer(status_code, answer); - if (status_code != CONTROL_RESULT_SUCCESS) { - rollback = true; - } SrvConfigPtr srv_config; - if (!rollback) { + if (status_code == CONTROL_RESULT_SUCCESS) { if (!check_only) { string parameter_name; ElementPtr mutable_cfg; @@ -803,20 +797,33 @@ 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; + status_code = CONTROL_RESULT_ERROR; } 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; + status_code = CONTROL_RESULT_ERROR; } } else { - rollback = true; + if (extra_checks) { + // Re-open lease and host database with new parameters. + try { + // Get the staging configuration. + srv_config = CfgMgr::instance().getStagingCfg(); + + CfgDbAccessPtr cfg_db = CfgMgr::instance().getStagingCfg()->getCfgDbAccess(); + string params = "universe=4 persist=false"; + if (cfg_db->getExtendedInfoTablesEnabled()) { + params += " extended-info-tables=true"; + } + cfg_db->setAppendedParameters(params); + cfg_db->createManagers(); + } catch (const std::exception& ex) { + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); + status_code = CONTROL_RESULT_ERROR; + } + } } } @@ -824,12 +831,26 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, // configuration. This will add created subnets and option values into // the server's configuration. // This operation should be exception safe but let's make sure. - if (!rollback) { + if (status_code == CONTROL_RESULT_SUCCESS && !check_only) { try { // Setup the command channel. configureCommandChannel(); + } catch (const isc::Exception& ex) { + LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what()); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); + status_code = CONTROL_RESULT_ERROR; + } catch (...) { + // For things like bad_cast in boost::lexical_cast + LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_EXCEPTION); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" + " parsing error"); + status_code = CONTROL_RESULT_ERROR; + } + } + if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) { + try { // No need to commit interface names as this is handled by the // CfgMgr::commit() function. @@ -837,7 +858,21 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, D2ClientConfigPtr cfg; cfg = CfgMgr::instance().getStagingCfg()->getD2ClientConfig(); CfgMgr::instance().setD2ClientConfig(cfg); + } catch (const isc::Exception& ex) { + LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what()); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); + status_code = CONTROL_RESULT_ERROR; + } catch (...) { + // For things like bad_cast in boost::lexical_cast + LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_EXCEPTION); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" + " parsing error"); + status_code = CONTROL_RESULT_ERROR; + } + } + if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) { + try { // This occurs last as if it succeeds, there is no easy way to // revert it. As a result, the failure to commit a subsequent // change causes problems when trying to roll back. @@ -849,35 +884,32 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, } catch (const isc::Exception& ex) { LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what()); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); - - // An error occurred, so make sure to restore the original data. - rollback = true; + status_code = CONTROL_RESULT_ERROR; } catch (...) { // For things like bad_cast in boost::lexical_cast LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_EXCEPTION); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" " parsing error"); - - // An error occurred, so make sure to restore the original data. - rollback = true; + status_code = CONTROL_RESULT_ERROR; } } // Moved from the commit block to add the config backend indication. - if (!rollback) { + if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) { try { - - // If there are config backends, fetch and merge into staging config - server.getCBControl()->databaseConfigFetch(srv_config, - CBControlDHCPv4::FetchMode::FETCH_ALL); + if (extra_checks) { + server.getCBControl()->databaseConfigConnect(srv_config); + } else { + // If there are config backends, fetch and merge into staging config + server.getCBControl()->databaseConfigFetch(srv_config, + CBControlDHCPv4::FetchMode::FETCH_ALL); + } } catch (const isc::Exception& ex) { std::ostringstream err; err << "during update from config backend database: " << ex.what(); LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(err.str()); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str()); - - // An error occurred, so make sure to restore the original data. - rollback = true; + status_code = CONTROL_RESULT_ERROR; } catch (...) { // For things like bad_cast in boost::lexical_cast std::ostringstream err; @@ -885,17 +917,23 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, << "undefined configuration parsing error"; LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(err.str()); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str()); - - // An error occurred, so make sure to restore the original data. - rollback = true; + status_code = CONTROL_RESULT_ERROR; } } // Rollback changes as the configuration parsing failed. - if (rollback) { + if (check_only || status_code != CONTROL_RESULT_SUCCESS) { // Revert to original configuration of runtime option definitions // in the libdhcp++. LibDHCP::revertRuntimeOptionDefs(); + + if (status_code == CONTROL_RESULT_SUCCESS && extra_checks) { + auto notify_libraries = ControlledDhcpv4Srv::finishConfigHookLibraries(config_set); + if (notify_libraries) { + return (notify_libraries); + } + } + return (answer); } diff --git a/src/bin/dhcp4/json_config_parser.h b/src/bin/dhcp4/json_config_parser.h index 34b1419700..fba72d1e48 100644 --- a/src/bin/dhcp4/json_config_parser.h +++ b/src/bin/dhcp4/json_config_parser.h @@ -8,15 +8,13 @@ #define DHCP4_CONFIG_PARSER_H #include -#include #include #include -#include #include /// @todo: This header file and its .cc counterpart are very similar between -/// DHCPv4 and DHCPv6. They should be merged. A ticket #2355. +/// DHCPv4 and DHCPv6. They should be merged. namespace isc { namespace dhcp { @@ -41,9 +39,9 @@ class Dhcpv4Srv; /// extra parameter is a reference to DHCPv4 server component. It is currently /// not used and CfgMgr::instance() is accessed instead. /// -/// Test-only mode added. If check_only flag is set to true, the configuration -/// is parsed, but the actual change is not applied. The goal is to have -/// the ability to test configuration. +/// Test-only mode is supported. If check_only flag is set to true, the +/// configuration is parsed, but the actual change is not applied. The goal is +/// to have the ability to test configuration. /// /// This method does not throw. It catches all exceptions and returns them as /// reconfiguration statuses. It may return the following response codes: @@ -52,13 +50,16 @@ class Dhcpv4Srv; /// 2 - commit failed (parsing was successful, but failed to store the /// values in to server's configuration) /// -/// @param server the server object -/// @param config_set a new configuration (JSON) for DHCPv4 server -/// @param check_only whether this configuration is for testing only -/// @return answer that contains result of reconfiguration +/// @param server DHCPv4 server object. +/// @param config_set a new configuration (JSON) for DHCPv4 server. +/// @param check_only whether this configuration is for testing only. +/// @param extra_checks load hooks and perform extra checks if this flag is true +/// and check_only is also true, otherwise perform simple check if +/// check_only is true. +/// @return answer that contains result of the reconfiguration. isc::data::ConstElementPtr configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, - bool check_only = false); + bool check_only = false, bool extra_checks = false); } // namespace dhcp } // namespace isc diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc index dfafb96772..7979a04666 100644 --- a/src/bin/dhcp4/main.cc +++ b/src/bin/dhcp4/main.cc @@ -52,13 +52,15 @@ usage() { << endl; cerr << endl; cerr << "Usage: " << DHCP4_NAME - << " -[v|V|W] [-d] [-{c|t} cfgfile] [-p number] [-P number]" << endl; + << " -[v|V|W] [-d] [-{c|t|T} cfgfile] [-p number] [-P number]" << endl; cerr << " -v: print version number and exit" << endl; cerr << " -V: print extended version and exit" << endl; cerr << " -W: display the configuration report and exit" << endl; cerr << " -d: debug mode with extra verbosity (former -v)" << endl; cerr << " -c file: specify configuration file" << endl; cerr << " -t file: check the configuration file syntax and exit" << endl; + cerr << " -T file: check the configuration file doing hooks load and extra " + << "checks and exit" << endl; cerr << " -p number: specify non-standard server port number 1-65535 " << "(useful for testing only)" << endl; cerr << " -P number: specify non-standard client port number 1-65535 " @@ -76,11 +78,12 @@ main(int argc, char* argv[]) { int client_port_number = 0; bool verbose_mode = false; // Should server be verbose? bool check_mode = false; // Check syntax + bool load_hooks = false; // Check hooks config // The standard config file std::string config_file(""); - while ((ch = getopt(argc, argv, "dvVWc:p:P:t:")) != -1) { + while ((ch = getopt(argc, argv, "dvVWc:p:P:t:T:")) != -1) { switch (ch) { case 'd': verbose_mode = true; @@ -98,9 +101,16 @@ main(int argc, char* argv[]) { cout << isc::detail::getConfigReport() << endl; return (EXIT_SUCCESS); + case 'T': + load_hooks = true; + check_mode = true; + config_file = optarg; + break; + case 't': check_mode = true; - // falls through + config_file = optarg; + break; case 'c': // config file config_file = optarg; @@ -184,9 +194,11 @@ main(int argc, char* argv[]) { ControlledDhcpv4Srv server(0); ConstElementPtr answer; + server.setProcName(DHCP4_NAME); + // Now we pass the Dhcp4 configuration to the server, but // tell it to check the configuration only (check_only = true) - answer = configureDhcp4Server(server, dhcp4, true); + answer = configureDhcp4Server(server, dhcp4, true, load_hooks); int status_code = 0; answer = isc::config::parseAnswer(status_code, answer); diff --git a/src/bin/dhcp4/tests/callout_library_1.cc b/src/bin/dhcp4/tests/callout_library_1.cc index 73e3a17f90..1199527622 100644 --- a/src/bin/dhcp4/tests/callout_library_1.cc +++ b/src/bin/dhcp4/tests/callout_library_1.cc @@ -20,6 +20,10 @@ static const int LIBRARY_NUMBER = 1; // issues related to namespaces. extern "C" { +int (*do_load)(isc::hooks::LibraryHandle& handle); + +int (*do_unload)(isc::hooks::LibraryHandle& handle); + /// @brief This function is called to retrieve the multi-threading compatibility. /// /// @return 1 which means compatible with multi-threading. diff --git a/src/bin/dhcp4/tests/callout_library_2.cc b/src/bin/dhcp4/tests/callout_library_2.cc index 7b1ad4b14d..cdb090f9f0 100644 --- a/src/bin/dhcp4/tests/callout_library_2.cc +++ b/src/bin/dhcp4/tests/callout_library_2.cc @@ -14,3 +14,11 @@ static const int LIBRARY_NUMBER = 2; #include #include + +extern "C" { + +int (*do_load)(isc::hooks::LibraryHandle& handle); + +int (*do_unload)(isc::hooks::LibraryHandle& handle); + +} diff --git a/src/bin/dhcp4/tests/callout_library_3.cc b/src/bin/dhcp4/tests/callout_library_3.cc index 494b25801d..a00ce10fc7 100644 --- a/src/bin/dhcp4/tests/callout_library_3.cc +++ b/src/bin/dhcp4/tests/callout_library_3.cc @@ -25,6 +25,23 @@ using namespace isc::hooks; // issues related to namespaces. extern "C" { +int +do_load_impl(LibraryHandle& handle) { + // Determine if this callout is configured to fail. + isc::dhcp::SrvConfigPtr config; + isc::data::ConstElementPtr const& parameters(handle.getParameters()); + isc::data::ConstElementPtr mode_element(parameters ? parameters->get("mode") : 0); + std::string mode(mode_element ? mode_element->stringValue() : ""); + if (mode == "fail-on-load") { + return (1); + } + return (0); +} + +int (*do_load)(isc::hooks::LibraryHandle& handle) = do_load_impl; + +int (*do_unload)(isc::hooks::LibraryHandle& handle); + /// @brief Callout which appends library number and provided arguments to /// the marker file for dhcp4_srv_configured callout. /// diff --git a/src/bin/dhcp4/tests/callout_library_common.h b/src/bin/dhcp4/tests/callout_library_common.h index 568746a0f9..b40465c2a3 100644 --- a/src/bin/dhcp4/tests/callout_library_common.h +++ b/src/bin/dhcp4/tests/callout_library_common.h @@ -33,6 +33,10 @@ extern "C" { +extern int (*do_load)(isc::hooks::LibraryHandle& handle); + +extern int (*do_unload)(isc::hooks::LibraryHandle& handle); + /// @brief Append digit to marker file /// /// If the marker file does not exist, create it. Then append the single @@ -84,13 +88,23 @@ version() { } int -load(isc::hooks::LibraryHandle&) { - return (appendDigit(LOAD_MARKER_FILE)); +load(isc::hooks::LibraryHandle& handle) { + int result = 0; + result = appendDigit(LOAD_MARKER_FILE); + if (result == 0 && do_load) { + result = do_load(handle); + } + return (result); } int -unload() { - return (appendDigit(UNLOAD_MARKER_FILE)); +unload(isc::hooks::LibraryHandle& handle) { + int result = 0; + result = appendDigit(UNLOAD_MARKER_FILE); + if (result == 0 && do_unload) { + result = do_unload(handle); + } + return (result); } }; diff --git a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in index 8cc8d93b6d..82f250299f 100644 --- a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in +++ b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in @@ -24,6 +24,8 @@ LOG_FILE="@abs_top_builddir@/src/bin/dhcp4/tests/test.log" LEASE_FILE="@abs_top_builddir@/src/bin/dhcp4/tests/test_leases.csv" # Path to the Kea LFC application export KEA_LFC_EXECUTABLE="@abs_top_builddir@/src/bin/lfc/kea-lfc" +# Path to test hooks library +HOOK_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco3.so" # Kea configuration to be stored in the configuration file. CONFIG="{ \"Dhcp4\": @@ -46,7 +48,7 @@ CONFIG="{ \"subnet\": \"10.0.0.0/8\", \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ] } ], - \"dhcp-ddns\": { + \"dhcp-ddns\": { \"enable-updates\": true, \"qualifying-suffix\": \"\" }, @@ -63,6 +65,7 @@ CONFIG="{ ] } }" + # Invalid configuration (syntax error) to check that Kea can check syntax. # This config has following errors: # - it should be interfaces-config/interfaces, not interfaces @@ -168,6 +171,108 @@ CONFIG_INVALID="{ } }" +# Invalid configuration (hook explicitly fails to load) to check that performing +# extra configuration checks detects the error. +INVALID_CONFIG_HOOKS_LOAD="{ + \"Dhcp4\": + { + \"interfaces-config\": { + \"interfaces\": [ ] + }, + \"multi-threading\": { + \"enable-multi-threading\": false + }, + \"valid-lifetime\": 4000, + \"renew-timer\": 1000, + \"rebind-timer\": 2000, + \"lease-database\": + { + \"type\": \"memfile\", + \"name\": \"$LEASE_FILE\", + \"persist\": false, + \"lfc-interval\": 0 + }, + \"subnet4\": [ + { + \"subnet\": \"10.0.0.0/8\", + \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ] + } ], + \"dhcp-ddns\": { + \"enable-updates\": true, + \"qualifying-suffix\": \"\" + }, + \"hooks-libraries\": [ + { + \"library\": \"$HOOK_PATH\", + \"parameters\": { + \"mode\": \"fail-on-load\" + } + } ], + \"loggers\": [ + { + \"name\": \"kea-dhcp4\", + \"output_options\": [ + { + \"output\": \"$LOG_FILE\" + } + ], + \"severity\": \"INFO\" + } + ] + } +}" + +# Invalid configuration (hook point returns error) to check that performing +# extra configuration checks detects the error. +INVALID_CONFIG_HOOKS_CALLOUT_FAIL="{ + \"Dhcp4\": + { + \"interfaces-config\": { + \"interfaces\": [ ] + }, + \"multi-threading\": { + \"enable-multi-threading\": false + }, + \"valid-lifetime\": 4000, + \"renew-timer\": 1000, + \"rebind-timer\": 2000, + \"lease-database\": + { + \"type\": \"memfile\", + \"name\": \"$LEASE_FILE\", + \"persist\": false, + \"lfc-interval\": 0 + }, + \"subnet4\": [ + { + \"subnet\": \"10.0.0.0/8\", + \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ] + } ], + \"dhcp-ddns\": { + \"enable-updates\": true, + \"qualifying-suffix\": \"\" + }, + \"hooks-libraries\": [ + { + \"library\": \"$HOOK_PATH\", + \"parameters\": { + \"mode\": \"fail-without-error\" + } + } ], + \"loggers\": [ + { + \"name\": \"kea-dhcp4\", + \"output_options\": [ + { + \"output\": \"$LOG_FILE\" + } + ], + \"severity\": \"INFO\" + } + ] + } +}" + # Set the location of the executable. bin="kea-dhcp4" bin_path="@abs_top_builddir@/src/bin/dhcp4" @@ -184,15 +289,22 @@ syntax_check_test() { local test_name="${1}" local config="${2}" local expected_code="${3}" + local extra_check="${4}" # Log the start of the test and print test name. test_start "${test_name}" # Create correct configuration file. create_config "${config}" # Check it - printf "Running command %s.\n" "\"${bin_path}/${bin} -t ${CFG_FILE}\"" - run_command \ - "${bin_path}/${bin}" -t "${CFG_FILE}" + if [ "${extra_check}" -eq 1 ]; then + printf "Running command %s.\n" "\"${bin_path}/${bin} -T ${CFG_FILE}\"" + run_command \ + "${bin_path}/${bin}" -T "${CFG_FILE}" + else + printf "Running command %s.\n" "\"${bin_path}/${bin} -t ${CFG_FILE}\"" + run_command \ + "${bin_path}/${bin}" -t "${CFG_FILE}" + fi if [ "${EXIT_CODE}" -ne "${expected_code}" ]; then printf 'ERROR: expected exit code %s, got %s\n' "${expected_code}" "${EXIT_CODE}" clean_exit 1 @@ -471,7 +583,9 @@ shutdown_test "dhcpv4.sigint_test" 2 version_test "dhcpv4.version" logger_vars_test "dhcpv4.variables" lfc_timer_test -syntax_check_test "dhcpv4.syntax_check_success" "${CONFIG}" 0 -syntax_check_test "dhcpv4.syntax_check_bad_syntax" "${CONFIG_BAD_SYNTAX}" 1 -syntax_check_test "dhcpv4.syntax_check_bad_values" "${CONFIG_BAD_VALUES}" 1 +syntax_check_test "dhcpv4.syntax_check_success" "${CONFIG}" 0 0 +syntax_check_test "dhcpv4.syntax_check_bad_syntax" "${CONFIG_BAD_SYNTAX}" 1 0 +syntax_check_test "dhcpv4.syntax_check_bad_values" "${CONFIG_BAD_VALUES}" 1 0 +syntax_check_test "dhcpv4.syntax_check_hooks_load_fail" "${INVALID_CONFIG_HOOKS_LOAD}" 1 1 +syntax_check_test "dhcpv4.syntax_check_hooks_callout_fail" "${INVALID_CONFIG_HOOKS_CALLOUT_FAIL}" 1 1 password_redact_test "dhcpv4.password_redact_test" "$(kea_dhcp_config 4)" 0 diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 62c37e2754..55c3046dbf 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -880,7 +880,6 @@ ControlledDhcpv6Srv::processCommand(const string& command, isc::data::ConstElementPtr ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { - ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance(); // Single stream instance used in all error clauses @@ -1046,6 +1045,29 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { // exception free. LibDHCP::commitRuntimeOptionDefs(); + auto notify_libraries = ControlledDhcpv6Srv::finishConfigHookLibraries(config); + if (notify_libraries) { + return (notify_libraries); + } + + // Apply multi threading settings. + // @note These settings are applied/updated only if no errors occur while + // applying the new configuration. + // @todo This should be fixed. + try { + CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading()); + } catch (const std::exception& ex) { + err << "Error applying multi threading settings: " + << ex.what(); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); + } + + return (answer); +} + +isc::data::ConstElementPtr +ControlledDhcpv6Srv::finishConfigHookLibraries(isc::data::ConstElementPtr config) { + ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance(); // This hook point notifies hooks libraries that the configuration of the // DHCPv6 server has completed. It provides the hook library with the pointer // to the common IO service object, new server configuration in the JSON @@ -1074,27 +1096,11 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { } } - // Apply multi threading settings. - // @note These settings are applied/updated only if no errors occur while - // applying the new configuration. - // @todo This should be fixed. - try { - CfgMultiThreading::apply(CfgMgr::instance().getStagingCfg()->getDHCPMultiThreading()); - } catch (const std::exception& ex) { - err << "Error applying multi threading settings: " - << ex.what(); - return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); - } - - return (answer); + return (ConstElementPtr()); } isc::data::ConstElementPtr ControlledDhcpv6Srv::checkConfig(isc::data::ConstElementPtr config) { - - LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_RECEIVED) - .arg(redactConfig(config)->str()); - ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance(); if (!srv) { @@ -1103,6 +1109,9 @@ ControlledDhcpv6Srv::checkConfig(isc::data::ConstElementPtr config) { return (no_srv); } + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_RECEIVED) + .arg(srv->redactConfig(config)->str()); + return (configureDhcp6Server(*srv, config, true)); } diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index adff127783..41c2a223e3 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -105,21 +105,32 @@ public: /// As pointer to this method is used a callback in ASIO used in /// ModuleCCSession, it has to be static. /// - /// @param new_config textual representation of the new configuration + /// @param config textual representation of the new configuration /// /// @return status of the config update static isc::data::ConstElementPtr - processConfig(isc::data::ConstElementPtr new_config); + processConfig(isc::data::ConstElementPtr config); /// @brief Configuration checker /// /// This is a method for checking incoming configuration. /// - /// @param new_config JSON representation of the new configuration + /// @param config JSON representation of the new configuration /// /// @return status of the config check - isc::data::ConstElementPtr - checkConfig(isc::data::ConstElementPtr new_config); + static isc::data::ConstElementPtr + checkConfig(isc::data::ConstElementPtr config); + + /// @brief Configuration checker for hook libraries + /// + /// This is a method for checking incoming configuration in the hooks + /// libraries. It calls dhcp4_srv_configured hook point for all hooks. + /// + /// @param config JSON representation of the new configuration + /// + /// @return status of the config check + static isc::data::ConstElementPtr + finishConfigHookLibraries(isc::data::ConstElementPtr config); /// @brief Returns pointer to the sole instance of Dhcpv6Srv /// diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 58e691c0a8..96cb4630d8 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -11,9 +11,10 @@ #include #include #include -#include +#include #include #include +#include #include #include #include @@ -867,7 +868,7 @@ processDhcp6Config(isc::data::ConstElementPtr config_set) { isc::data::ConstElementPtr configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, - bool check_only) { + bool check_only, bool extra_checks) { if (!config_set) { ConstElementPtr answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "Can't parse NULL config"); @@ -877,21 +878,14 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, 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; + int status_code = CONTROL_RESULT_SUCCESS; isc::config::parseAnswer(status_code, answer); - if (status_code != CONTROL_RESULT_SUCCESS) { - rollback = true; - } SrvConfigPtr srv_config; - if (!rollback) { + if (status_code == CONTROL_RESULT_SUCCESS) { if (!check_only) { string parameter_name; ElementPtr mutable_cfg; @@ -932,20 +926,33 @@ 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; + status_code = CONTROL_RESULT_ERROR; } 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; + status_code = CONTROL_RESULT_ERROR; } } else { - rollback = true; + if (extra_checks) { + // Re-open lease and host database with new parameters. + try { + // Get the staging configuration. + srv_config = CfgMgr::instance().getStagingCfg(); + + CfgDbAccessPtr cfg_db = CfgMgr::instance().getStagingCfg()->getCfgDbAccess(); + string params = "universe=6 persist=false"; + if (cfg_db->getExtendedInfoTablesEnabled()) { + params += " extended-info-tables=true"; + } + cfg_db->setAppendedParameters(params); + cfg_db->createManagers(); + } catch (const std::exception& ex) { + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); + status_code = CONTROL_RESULT_ERROR; + } + } } } @@ -953,12 +960,26 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, // configuration. This will add created subnets and option values into // the server's configuration. // This operation should be exception safe but let's make sure. - if (!rollback) { + if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) { try { // Setup the command channel. configureCommandChannel(); + } catch (const isc::Exception& ex) { + LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_FAIL).arg(ex.what()); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); + status_code = CONTROL_RESULT_ERROR; + } catch (...) { + // For things like bad_cast in boost::lexical_cast + LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_EXCEPTION); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" + " parsing error"); + status_code = CONTROL_RESULT_ERROR; + } + } + if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) { + try { // No need to commit interface names as this is handled by the // CfgMgr::commit() function. @@ -966,7 +987,21 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, D2ClientConfigPtr cfg; cfg = CfgMgr::instance().getStagingCfg()->getD2ClientConfig(); CfgMgr::instance().setD2ClientConfig(cfg); + } catch (const isc::Exception& ex) { + LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_FAIL).arg(ex.what()); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); + status_code = CONTROL_RESULT_ERROR; + } catch (...) { + // For things like bad_cast in boost::lexical_cast + LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_EXCEPTION); + answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" + " parsing error"); + status_code = CONTROL_RESULT_ERROR; + } + } + if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) { + try { // This occurs last as if it succeeds, there is no easy way to // revert it. As a result, the failure to commit a subsequent // change causes problems when trying to roll back. @@ -978,35 +1013,32 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, } catch (const isc::Exception& ex) { LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_FAIL).arg(ex.what()); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, ex.what()); - - // An error occurred, so make sure to restore the original data. - rollback = true; + status_code = CONTROL_RESULT_ERROR; } catch (...) { // For things like bad_cast in boost::lexical_cast LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_EXCEPTION); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, "undefined configuration" " parsing error"); - - // An error occurred, so make sure to restore the original data. - rollback = true; + status_code = CONTROL_RESULT_ERROR; } } // Moved from the commit block to add the config backend indication. - if (!rollback) { + if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) { try { - - // If there are config backends, fetch and merge into staging config - server.getCBControl()->databaseConfigFetch(srv_config, - CBControlDHCPv6::FetchMode::FETCH_ALL); + if (extra_checks) { + server.getCBControl()->databaseConfigConnect(srv_config); + } else { + // If there are config backends, fetch and merge into staging config + server.getCBControl()->databaseConfigFetch(srv_config, + CBControlDHCPv6::FetchMode::FETCH_ALL); + } } catch (const isc::Exception& ex) { std::ostringstream err; err << "during update from config backend database: " << ex.what(); LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_FAIL).arg(err.str()); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str()); - - // An error occurred, so make sure to restore the original data. - rollback = true; + status_code = CONTROL_RESULT_ERROR; } catch (...) { // For things like bad_cast in boost::lexical_cast std::ostringstream err; @@ -1014,17 +1046,23 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, << "undefined configuration parsing error"; LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_FAIL).arg(err.str()); answer = isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str()); - - // An error occurred, so make sure to restore the original data. - rollback = true; + status_code = CONTROL_RESULT_ERROR; } } // Rollback changes as the configuration parsing failed. - if (rollback) { + if (check_only || status_code != CONTROL_RESULT_SUCCESS) { // Revert to original configuration of runtime option definitions // in the libdhcp++. LibDHCP::revertRuntimeOptionDefs(); + + if (status_code == CONTROL_RESULT_SUCCESS && extra_checks) { + auto notify_libraries = ControlledDhcpv6Srv::finishConfigHookLibraries(config_set); + if (notify_libraries) { + return (notify_libraries); + } + } + return (answer); } diff --git a/src/bin/dhcp6/json_config_parser.h b/src/bin/dhcp6/json_config_parser.h index 8230d3550e..883339bef9 100644 --- a/src/bin/dhcp6/json_config_parser.h +++ b/src/bin/dhcp6/json_config_parser.h @@ -13,12 +13,27 @@ #include +/// @todo: This header file and its .cc counterpart are very similar between +/// DHCPv4 and DHCPv6. They should be merged. + namespace isc { namespace dhcp { class Dhcpv6Srv; -/// @brief Configures DHCPv6 server +/// @brief Configure DHCPv6 server (@c Dhcpv6Srv) with a set of configuration +/// values. +/// +/// This function parses configuration information stored in @c config_set +/// and configures the @c server by applying the configuration to it. +/// It provides the strong exception guarantee as long as the underlying +/// derived class implementations of @c DhcpConfigParser meet the assumption, +/// that is, it ensures that either configuration is fully applied or the +/// state of the server is intact. +/// +/// If a syntax or semantics level error happens during the configuration +/// (such as malformed configuration or invalid configuration parameter), +/// this function returns appropriate error code. /// /// This function is called every time a new configuration is received. The /// extra parameter is a reference to DHCPv6 server component. It is currently @@ -32,17 +47,19 @@ class Dhcpv6Srv; /// reconfiguration statuses. It may return the following response codes: /// 0 - configuration successful /// 1 - malformed configuration (parsing failed) -/// 2 - commit failed (parsing was successful, but the values could not be -/// stored in the configuration). +/// 2 - commit failed (parsing was successful, but failed to store the +/// values in to server's configuration) /// /// @param server DHCPv6 server object. -/// @param config_set a new configuration for DHCPv6 server. -/// @param check_only whether this configuration is for testing only +/// @param config_set a new configuration (JSON) for DHCPv6 server. +/// @param check_only whether this configuration is for testing only. +/// @param extra_checks load hooks and perform extra checks if this flag is true +/// and check_only is also true, otherwise perform simple check if +/// check_only is true. /// @return answer that contains result of the reconfiguration. -/// @throw Dhcp6ConfigError if trying to create a parser for NULL config. isc::data::ConstElementPtr configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, - bool check_only = false); + bool check_only = false, bool extra_checks = false); } // namespace dhcp } // namespace isc diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index e1f4cd50db..fcfff7df53 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -52,13 +52,15 @@ usage() { << endl; cerr << endl; cerr << "Usage: " << DHCP6_NAME - << " -[v|V|W] [-d] [-{c|t} cfgfile] [-p number] [-P number]" << endl; + << " -[v|V|W] [-d] [-{c|t|T} cfgfile] [-p number] [-P number]" << endl; cerr << " -v: print version number and exit" << endl; cerr << " -V: print extended version and exit" << endl; cerr << " -W: display the configuration report and exit" << endl; cerr << " -d: debug mode with extra verbosity (former -v)" << endl; cerr << " -c file: specify configuration file" << endl; cerr << " -t file: check the configuration file syntax and exit" << endl; + cerr << " -T file: check the configuration file doing hooks load and extra " + << "checks and exit" << endl; cerr << " -p number: specify non-standard server port number 1-65535 " << "(useful for testing only)" << endl; cerr << " -P number: specify non-standard client port number 1-65535 " @@ -76,11 +78,12 @@ main(int argc, char* argv[]) { int client_port_number = 0; bool verbose_mode = false; // Should server be verbose? bool check_mode = false; // Check syntax + bool load_hooks = false; // Check hooks config // The standard config file std::string config_file(""); - while ((ch = getopt(argc, argv, "dvVWc:p:P:t:")) != -1) { + while ((ch = getopt(argc, argv, "dvVWc:p:P:t:T:")) != -1) { switch (ch) { case 'd': verbose_mode = true; @@ -98,9 +101,16 @@ main(int argc, char* argv[]) { cout << isc::detail::getConfigReport() << endl; return (EXIT_SUCCESS); + case 'T': + load_hooks = true; + check_mode = true; + config_file = optarg; + break; + case 't': check_mode = true; - // falls through + config_file = optarg; + break; case 'c': // config file config_file = optarg; @@ -184,9 +194,11 @@ main(int argc, char* argv[]) { ControlledDhcpv6Srv server(0); ConstElementPtr answer; + server.setProcName(DHCP6_NAME); + // Now we pass the Dhcp6 configuration to the server, but // tell it to check the configuration only (check_only = true) - answer = configureDhcp6Server(server, dhcp6, true); + answer = configureDhcp6Server(server, dhcp6, true, load_hooks); int status_code = 0; answer = isc::config::parseAnswer(status_code, answer); diff --git a/src/bin/dhcp6/tests/callout_library_1.cc b/src/bin/dhcp6/tests/callout_library_1.cc index 95bd37cb49..ca6ff1b45a 100644 --- a/src/bin/dhcp6/tests/callout_library_1.cc +++ b/src/bin/dhcp6/tests/callout_library_1.cc @@ -20,6 +20,10 @@ static const int LIBRARY_NUMBER = 1; // issues related to namespaces. extern "C" { +int (*do_load)(isc::hooks::LibraryHandle& handle); + +int (*do_unload)(isc::hooks::LibraryHandle& handle); + /// @brief This function is called to retrieve the multi-threading compatibility. /// /// @return 1 which means compatible with multi-threading. diff --git a/src/bin/dhcp6/tests/callout_library_2.cc b/src/bin/dhcp6/tests/callout_library_2.cc index 5dc72c52d1..c76b2dc550 100644 --- a/src/bin/dhcp6/tests/callout_library_2.cc +++ b/src/bin/dhcp6/tests/callout_library_2.cc @@ -14,3 +14,11 @@ static const int LIBRARY_NUMBER = 2; #include #include + +extern "C" { + +int (*do_load)(isc::hooks::LibraryHandle& handle); + +int (*do_unload)(isc::hooks::LibraryHandle& handle); + +} diff --git a/src/bin/dhcp6/tests/callout_library_3.cc b/src/bin/dhcp6/tests/callout_library_3.cc index 5e5d9cb2e1..df4c44634c 100644 --- a/src/bin/dhcp6/tests/callout_library_3.cc +++ b/src/bin/dhcp6/tests/callout_library_3.cc @@ -24,6 +24,23 @@ using namespace isc::hooks; // issues related to namespaces. extern "C" { +int +do_load_impl(LibraryHandle& handle) { + // Determine if this callout is configured to fail. + isc::dhcp::SrvConfigPtr config; + isc::data::ConstElementPtr const& parameters(handle.getParameters()); + isc::data::ConstElementPtr mode_element(parameters ? parameters->get("mode") : 0); + std::string mode(mode_element ? mode_element->stringValue() : ""); + if (mode == "fail-on-load") { + return (1); + } + return (0); +} + +int (*do_load)(isc::hooks::LibraryHandle& handle) = do_load_impl; + +int (*do_unload)(isc::hooks::LibraryHandle& handle); + /// @brief Callout which appends library number and provided arguments to /// the marker file for dhcp6_srv_configured callout. /// diff --git a/src/bin/dhcp6/tests/callout_library_common.h b/src/bin/dhcp6/tests/callout_library_common.h index 36f459ab2e..8ae600a089 100644 --- a/src/bin/dhcp6/tests/callout_library_common.h +++ b/src/bin/dhcp6/tests/callout_library_common.h @@ -33,6 +33,10 @@ extern "C" { +extern int (*do_load)(isc::hooks::LibraryHandle& handle); + +extern int (*do_unload)(isc::hooks::LibraryHandle& handle); + /// @brief Append digit to marker file /// /// If the marker file does not exist, create it. Then append the single @@ -84,13 +88,23 @@ version() { } int -load(isc::hooks::LibraryHandle&) { - return (appendDigit(LOAD_MARKER_FILE)); +load(isc::hooks::LibraryHandle& handle) { + int result = 0; + result = appendDigit(LOAD_MARKER_FILE); + if (result == 0 && do_load) { + result = do_load(handle); + } + return (result); } int -unload() { - return (appendDigit(UNLOAD_MARKER_FILE)); +unload(isc::hooks::LibraryHandle& handle) { + int result = 0; + result = appendDigit(UNLOAD_MARKER_FILE); + if (result == 0 && do_unload) { + result = do_unload(handle); + } + return (result); } }; diff --git a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in index 0265ec62a8..9cd2bc6bc8 100644 --- a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in +++ b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in @@ -24,10 +24,13 @@ LOG_FILE="@abs_top_builddir@/src/bin/dhcp6/tests/test.log" LEASE_FILE="@abs_top_builddir@/src/bin/dhcp6/tests/test_leases.csv" # Path to the Kea LFC application export KEA_LFC_EXECUTABLE="@abs_top_builddir@/src/bin/lfc/kea-lfc" +# Path to test hooks library +HOOK_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco3.so" # Kea configuration to be stored in the configuration file. CONFIG="{ \"Dhcp6\": - { \"interfaces-config\": { + { + \"interfaces-config\": { \"interfaces\": [ ] }, \"server-id\": { @@ -67,6 +70,7 @@ CONFIG="{ ] } }" + # Invalid configuration (syntax error) to check that Kea can check syntax. # This config has following errors: # - it should be interfaces-config/interfaces, not interfaces @@ -102,6 +106,7 @@ CONFIG_BAD_SYNTAX="{ ] } }" + # Invalid configuration (negative preferred-lifetime) to check that Kea # gracefully handles reconfiguration errors. CONFIG_INVALID="{ @@ -142,7 +147,8 @@ CONFIG_INVALID="{ # it is defined in. Syntactically the config is correct, though. CONFIG_BAD_VALUES="{ \"Dhcp6\": - { \"interfaces-config\": { + { + \"interfaces-config\": { \"interfaces\": [ ] }, \"server-id\": { @@ -172,6 +178,117 @@ CONFIG_BAD_VALUES="{ } }" +# Invalid configuration (hook explicitly fails to load) to check that performing +# extra configuration checks detects the error. +INVALID_CONFIG_HOOKS_LOAD="{ + \"Dhcp6\": + { + \"interfaces-config\": { + \"interfaces\": [ ] + }, + \"multi-threading\": { + \"enable-multi-threading\": false + }, + \"server-id\": { + \"type\": \"LLT\", + \"persist\": false + }, + \"preferred-lifetime\": 3000, + \"valid-lifetime\": 4000, + \"renew-timer\": 1000, + \"rebind-timer\": 2000, + \"lease-database\": + { + \"type\": \"memfile\", + \"name\": \"$LEASE_FILE\", + \"persist\": false, + \"lfc-interval\": 0 + }, + \"subnet6\": [ + { + \"subnet\": \"2001:db8:1::/64\", + \"pools\": [ { \"pool\": \"2001:db8:1::10-2001:db8:1::100\" } ] + } ], + \"dhcp-ddns\": { + \"enable-updates\": true, + \"qualifying-suffix\": \"\" + }, + \"hooks-libraries\": [ + { + \"library\": \"$HOOK_PATH\", + \"parameters\": { + \"mode\": \"fail-on-load\" + } + } ], + \"loggers\": [ + { + \"name\": \"kea-dhcp6\", + \"output_options\": [ + { + \"output\": \"$LOG_FILE\" + } + ], + \"severity\": \"INFO\" + } + ] + } +}" + +# Invalid configuration (hook point returns error) to check that performing +# extra configuration checks detects the error. +INVALID_CONFIG_HOOKS_CALLOUT_FAIL="{ + \"Dhcp6\": + { + \"interfaces-config\": { + \"interfaces\": [ ] + }, + \"multi-threading\": { + \"enable-multi-threading\": false + }, + \"server-id\": { + \"type\": \"LLT\", + \"persist\": false + }, + \"preferred-lifetime\": 3000, + \"valid-lifetime\": 4000, + \"renew-timer\": 1000, + \"rebind-timer\": 2000, + \"lease-database\": + { + \"type\": \"memfile\", + \"name\": \"$LEASE_FILE\", + \"persist\": false, + \"lfc-interval\": 0 + }, + \"subnet6\": [ + { + \"subnet\": \"2001:db8:1::/64\", + \"pools\": [ { \"pool\": \"2001:db8:1::10-2001:db8:1::100\" } ] + } ], + \"dhcp-ddns\": { + \"enable-updates\": true, + \"qualifying-suffix\": \"\" + }, + \"hooks-libraries\": [ + { + \"library\": \"$HOOK_PATH\", + \"parameters\": { + \"mode\": \"fail-without-error\" + } + } ], + \"loggers\": [ + { + \"name\": \"kea-dhcp6\", + \"output_options\": [ + { + \"output\": \"$LOG_FILE\" + } + ], + \"severity\": \"INFO\" + } + ] + } +}" # Set the location of the executable. bin="kea-dhcp6" @@ -189,15 +306,22 @@ syntax_check_test() { local test_name="${1}" local config="${2}" local expected_code="${3}" + local extra_check="${4}" # Log the start of the test and print test name. test_start "${test_name}" # Create correct configuration file. create_config "${config}" # Check it - printf "Running command %s.\n" "\"${bin_path}/${bin} -t ${CFG_FILE}\"" - run_command \ - "${bin_path}/${bin}" -t "${CFG_FILE}" + if [ "${extra_check}" -eq 1 ]; then + printf "Running command %s.\n" "\"${bin_path}/${bin} -T ${CFG_FILE}\"" + run_command \ + "${bin_path}/${bin}" -T "${CFG_FILE}" + else + printf "Running command %s.\n" "\"${bin_path}/${bin} -t ${CFG_FILE}\"" + run_command \ + "${bin_path}/${bin}" -t "${CFG_FILE}" + fi if [ "${EXIT_CODE}" -ne "${expected_code}" ]; then printf 'ERROR: expected exit code %s, got %s\n' "${expected_code}" "${EXIT_CODE}" clean_exit 1 @@ -479,7 +603,9 @@ shutdown_test "dhcpv6.sigint_test" 2 version_test "dhcpv6.version" logger_vars_test "dhcpv6.variables" lfc_timer_test -syntax_check_test "dhcpv6.syntax_check_success" "${CONFIG}" 0 -syntax_check_test "dhcpv6.syntax_check_bad_syntax" "${CONFIG_BAD_SYNTAX}" 1 -syntax_check_test "dhcpv6.syntax_check_bad_values" "${CONFIG_BAD_VALUES}" 1 +syntax_check_test "dhcpv6.syntax_check_success" "${CONFIG}" 0 0 +syntax_check_test "dhcpv6.syntax_check_bad_syntax" "${CONFIG_BAD_SYNTAX}" 1 0 +syntax_check_test "dhcpv6.syntax_check_bad_values" "${CONFIG_BAD_VALUES}" 1 0 +syntax_check_test "dhcpv6.syntax_check_hooks_load_fail" "${INVALID_CONFIG_HOOKS_LOAD}" 1 1 +syntax_check_test "dhcpv6.syntax_check_hooks_callout_fail" "${INVALID_CONFIG_HOOKS_CALLOUT_FAIL}" 1 1 password_redact_test "dhcpv6.password_redact_test" "$(kea_dhcp_config 6)" 0 diff --git a/src/hooks/dhcp/high_availability/ha_messages.mes b/src/hooks/dhcp/high_availability/ha_messages.mes index 40825c407e..be1006c4ad 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.mes +++ b/src/hooks/dhcp/high_availability/ha_messages.mes @@ -186,7 +186,7 @@ is internal server error and a bug report should be created. % HA_DHCP6_START_SERVICE_FAILED failed to start DHCPv4 HA service in dhcp6_srv_configured callout: %1 This error message is issued when an attempt to start High Availability service -for the DHCPv4 server failed in the dhcp4_srv_configured callout. This +for the DHCPv6 server failed in the dhcp6_srv_configured callout. This is internal server error and a bug report should be created. % HA_DHCP_DISABLE_COMMUNICATIONS_FAILED failed to send request to disable DHCP service of %1: %2 @@ -242,7 +242,7 @@ servers to resume the HA service. % HA_INIT_OK loading High Availability hooks library successful This informational message indicates that the High Availability hooks library -has been loaded successfully. +has been loaded successfully. Enjoy! % HA_INVALID_PARTNER_STATE_COMMUNICATION_RECOVERY partner is in the communication-recovery state unexpectedly This warning message is issued when a partner is in the communication-recovery diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_messages.mes b/src/hooks/dhcp/mysql_cb/mysql_cb_messages.mes index 3aa526c5ba..77fc298342 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_messages.mes +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_messages.mes @@ -545,7 +545,7 @@ Debug message issued when triggered an action to retrieve type % MYSQL_CB_INIT_OK loading MYSQL CB hooks library successful This informational message indicates that the MySQL Configuration Backend hooks -library has been loaded successfully. +library has been loaded successfully. Enjoy! % MYSQL_CB_NO_TLS TLS was required but is not used This error message is issued when TLS for the connection was required but diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_messages.mes b/src/hooks/dhcp/pgsql_cb/pgsql_cb_messages.mes index 247ce0877b..0cd379a278 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_messages.mes +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_messages.mes @@ -545,7 +545,7 @@ Debug message issued when triggered an action to retrieve type % PGSQL_CB_INIT_OK loading Postgres CB hooks library successful This informational message indicates that the Postgres Configuration Backend hooks -library has been loaded successfully. +library has been loaded successfully. Enjoy! % PGSQL_CB_NO_TLS_SUPPORT Attempt to configure TLS (unsupported for PostgreSQL): %1 This error message is printed when TLS support was required in the Kea diff --git a/src/lib/testutils/dhcp_test_lib.sh.in b/src/lib/testutils/dhcp_test_lib.sh.in index d2c7ae81f3..c8b65fd41b 100644 --- a/src/lib/testutils/dhcp_test_lib.sh.in +++ b/src/lib/testutils/dhcp_test_lib.sh.in @@ -41,6 +41,12 @@ EXPECTED_VERSION="@PACKAGE_VERSION@" export KEA_LFC_EXECUTABLE="@abs_top_builddir@/src/bin/lfc/kea-lfc" export KEA_LOCKFILE_DIR="@abs_top_builddir@/test_lockfile_dir" export KEA_PIDFILE_DIR="@abs_top_builddir@/test_pidfile_dir" +KEA_DHCP4_LOAD_MARKER_FILE="@abs_top_builddir@/src/bin/dhcp4/tests/load_marker.txt" +KEA_DHCP4_UNLOAD_MARKER_FILE="@abs_top_builddir@/src/bin/dhcp4/tests/unload_marker.txt" +KEA_DHCP4_SRV_CONFIG_MARKER_FILE="@abs_top_builddir@/src/bin/dhcp4/tests/srv_config_marker_file.txt" +KEA_DHCP6_LOAD_MARKER_FILE="@abs_top_builddir@/src/bin/dhcp6/tests/load_marker.txt" +KEA_DHCP6_UNLOAD_MARKER_FILE="@abs_top_builddir@/src/bin/dhcp6/tests/unload_marker.txt" +KEA_DHCP6_SRV_CONFIG_MARKER_FILE="@abs_top_builddir@/src/bin/dhcp6/tests/srv_config_marker_file.txt" # A list of Kea processes, mainly used by the cleanup functions. KEA_PROCS="kea-dhcp4 kea-dhcp6 kea-dhcp-ddns kea-ctrl-agent" @@ -656,7 +662,13 @@ cleanup() { "${CFG_FILE-}" \ "${D2_CFG_FILE-}" \ "${DHCP4_CFG_FILE-}" \ + "${KEA_DHCP4_LOAD_MARKER_FILE-}" \ + "${KEA_DHCP4_UNLOAD_MARKER_FILE-}" \ + "${KEA_DHCP4_SRV_CONFIG_MARKER_FILE-}" \ "${DHCP6_CFG_FILE-}" \ + "${KEA_DHCP6_LOAD_MARKER_FILE-}" \ + "${KEA_DHCP6_UNLOAD_MARKER_FILE-}" \ + "${KEA_DHCP6_SRV_CONFIG_MARKER_FILE-}" \ "${KEACTRL_CFG_FILE-}" \ "${KEA_LOCKFILE_DIR-}" \ "${KEA_PIDFILE_DIR-}" \