From: Tomek Mrugalski Date: Tue, 24 Jan 2017 22:53:07 +0000 (+0100) Subject: [5019] Global v4/v6 parsers migrated to SimpleParser. X-Git-Tag: trac3389_base~12 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=96cdf5b01f909b2dab05a86cce087cf2dad3e9e5;p=thirdparty%2Fkea.git [5019] Global v4/v6 parsers migrated to SimpleParser. --- diff --git a/src/Makefile.am b/src/Makefile.am index 3323f4ab91..2cde2d5275 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -3,5 +3,4 @@ SUBDIRS = share lib bin hooks EXTRA_DIST = \ cppcheck-suppress.lst \ valgrind-suppressions \ - valgrind-suppressions.revisit \ - defaults.h + valgrind-suppressions.revisit diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 3fc311199f..2b4a2c3e3f 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -25,7 +25,6 @@ #include #include #include -#include #include #include @@ -395,77 +394,71 @@ namespace dhcp { DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id, ConstElementPtr element) { DhcpConfigParser* parser = NULL; - if ((config_id.compare("valid-lifetime") == 0) || - (config_id.compare("renew-timer") == 0) || - (config_id.compare("rebind-timer") == 0) || - (config_id.compare("decline-probation-period") == 0) || - (config_id.compare("dhcp4o6-port") == 0) ) { - parser = new Uint32Parser(config_id, - globalContext()->uint32_values_); + // valife-lifetime, renew-timer, rebind-timer, decline-probation-period + // have been migrated to SimpleParser already. // subnet4 has been migrated to SimpleParser already. // interface-config has been migrated to SimpleParser already. // option-data and option-def have been converted to SimpleParser already. - } else if ((config_id.compare("next-server") == 0)) { - parser = new StringParser(config_id, - globalContext()->string_values_); - // hooks-libraries are now migrated to SimpleParser. - // lease-database and hosts-database have been converted to SimpleParser already. - } else if (config_id.compare("echo-client-id") == 0) { - parser = new BooleanParser(config_id, globalContext()->boolean_values_); + + // next-server migrated + // echo-client-id migrated + // lease-database migrated + // hosts-database migrated + // dhcp-ddns has been converted to SimpleParser. - } else if (config_id.compare("match-client-id") == 0) { - parser = new BooleanParser(config_id, globalContext()->boolean_values_); + // match-client-id has been migrated to SimpleParser already. // control-socket has been converted to SimpleParser already. // expired-leases-processing has been converted to SimpleParser already. // client-classes has been converted to SimpleParser already. // host-reservation-identifiers have been converted to SimpleParser already. - } else { - isc_throw(DhcpConfigError, - "unsupported global configuration parameter: " - << config_id << " (" << element->getPosition() << ")"); - } + isc_throw(DhcpConfigError, + "unsupported global configuration parameter: " + << config_id << " (" << element->getPosition() << ")"); return (parser); } /// @brief Sets global parameters in staging configuration /// +/// @param global global configuration scope +/// /// Currently this method sets the following global parameters: /// /// - echo-client-id /// - decline-probation-period /// - dhcp4o6-port -void setGlobalParameters4() { - // Although the function is modest for now, it is certain that the number - // of global switches will increase over time, hence the name. - +void setGlobalParameters4(ConstElementPtr global) { // Set whether v4 server is supposed to echo back client-id (yes = RFC6842 // compatible, no = backward compatibility) - try { - bool echo_client_id = globalContext()->boolean_values_->getParam("echo-client-id"); - CfgMgr::instance().echoClientId(echo_client_id); - } catch (...) { - // Ignore errors. This flag is optional - } - - // Set the probation period for decline handling. - try { - uint32_t probation_period = globalContext()->uint32_values_ - ->getOptionalParam("decline-probation-period", - DEFAULT_DECLINE_PROBATION_PERIOD); - CfgMgr::instance().getStagingCfg()->setDeclinePeriod(probation_period); - } catch (...) { - // That's not really needed. + bool echo_client_id = SimpleParser::getBoolean(global, "echo-client-id"); + CfgMgr::instance().echoClientId(echo_client_id); + + /// @todo: Use Francis' template from SimpleParser once 5097 is merged + /// This will be done as part of #5116. + int64_t probation_period = SimpleParser::getInteger(global, + "decline-probation-period"); + if (probation_period < std::numeric_limits::min() || + probation_period > std::numeric_limits::max()) { + isc_throw(DhcpConfigError, "Invalid decline-probation-period value: " + << probation_period << ", allowed range: " + << std::numeric_limits::min() << ".." + << std::numeric_limits::max()); } + CfgMgr::instance().getStagingCfg()->setDeclinePeriod(probation_period); // Set the DHCPv4-over-DHCPv6 interserver port. - try { - uint32_t dhcp4o6_port = globalContext()->uint32_values_ - ->getOptionalParam("dhcp4o6-port", 0); - CfgMgr::instance().getStagingCfg()->setDhcp4o6Port(dhcp4o6_port); - } catch (...) { - // Ignore errors. This flag is optional + + /// @todo: Use Francis' template from SimpleParser once 5097 is merged + /// This will be done as part of #5116. + uint32_t dhcp4o6_port = SimpleParser::getInteger(global, "dhcp4o6-port"); + if (dhcp4o6_port < std::numeric_limits::min() || + dhcp4o6_port > std::numeric_limits::max()) { + isc_throw(DhcpConfigError, "Invalid dhcp4o6-port value: " + << dhcp4o6_port << ", allowed range: " + << std::numeric_limits::min() << ".." + << std::numeric_limits::max()); } + CfgMgr::instance().getStagingCfg()->setDhcp4o6Port(dhcp4o6_port); } /// @brief Initialize the command channel based on the staging configuration @@ -687,6 +680,24 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { continue; } + // Timers are not used in the global scope. Their values are derived + // to specific subnets (see SimpleParser6::deriveParameters). + // decline-probation-period, dhcp4o6-port, echo-client-id are + // handlded in the setGlobalParameters4. + // match-client-id is derived to subnet scope level. + if ( (config_pair.first == "renew-timer") || + (config_pair.first == "rebind-timer") || + (config_pair.first == "valid-lifetime") || + (config_pair.first == "decline-probation-period") || + (config_pair.first == "dhcp4o6-port") || + (config_pair.first == "echo-client-id") || + (config_pair.first == "match-client-id") || + (config_pair.first == "next-server")) { + continue; + } + + /// @todo: 5116 ticket: remove this chunk below once all parser + /// tickets are done. ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED) @@ -732,9 +743,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { // No need to commit interface names as this is handled by the // CfgMgr::commit() function. - // Apply global options in the staging config. - setGlobalParameters4(); - // This occurs last as if it succeeds, there is no easy way // revert it. As a result, the failure to commit a subsequent // change causes problems when trying to roll back. diff --git a/src/bin/dhcp4/simple_parser4.cc b/src/bin/dhcp4/simple_parser4.cc index 808bfab6a8..63febebfd0 100644 --- a/src/bin/dhcp4/simple_parser4.cc +++ b/src/bin/dhcp4/simple_parser4.cc @@ -55,9 +55,14 @@ const SimpleDefaults SimpleParser4::OPTION4_DEFAULTS = { /// in Dhcp4) are optional. If not defined, the following values will be /// used. const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = { - { "renew-timer", Element::integer, "900" }, - { "rebind-timer", Element::integer, "1800" }, - { "valid-lifetime", Element::integer, "7200" } + { "renew-timer", Element::integer, "900" }, + { "rebind-timer", Element::integer, "1800" }, + { "valid-lifetime", Element::integer, "7200" }, + { "decline-probation-period", Element::integer, "86400" }, // 24h + { "dhcp4o6-port", Element::integer, "0" }, + { "echo-client-id", Element::boolean, "true" }, + { "match-client-id", Element::boolean, "true" }, + { "next-server", Element::string, "0.0.0.0" } }; /// @brief List of parameters that can be inherited from the global to subnet4 scope. @@ -69,7 +74,9 @@ const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = { const ParamsList SimpleParser4::INHERIT_GLOBAL_TO_SUBNET4 = { "renew-timer", "rebind-timer", - "valid-lifetime" + "valid-lifetime", + "match-client-id", + "next-server" }; /// @} diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 0fc0d380fc..2448976d89 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -26,7 +26,6 @@ #include #include #include -#include #include "marker_file.h" #include "test_libraries.h" @@ -654,10 +653,6 @@ TEST_F(Dhcp4ParserTest, emptySubnet) { // returned value should be 0 (success) checkResult(status, 0); - - checkGlobalUint32("rebind-timer", 2000); - checkGlobalUint32("renew-timer", 1000); - checkGlobalUint32("valid-lifetime", 4000); } /// Check that the renew-timer doesn't have to be specified, in which case @@ -679,8 +674,6 @@ TEST_F(Dhcp4ParserTest, unspecifiedRenewTimer) { // returned value should be 0 (success) checkResult(status, 0); - checkGlobalUint32("rebind-timer", 2000); - checkGlobalUint32("valid-lifetime", 4000); Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); @@ -715,8 +708,6 @@ TEST_F(Dhcp4ParserTest, unspecifiedRebindTimer) { // returned value should be 0 (success) checkResult(status, 0); - checkGlobalUint32("renew-timer", 1000); - checkGlobalUint32("valid-lifetime", 4000); Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); @@ -4127,11 +4118,34 @@ TEST_F(Dhcp4ParserTest, declineTimerDefault) { checkResult(status, 0); // The value of decline-probation-period must be equal to the - // default value. - EXPECT_EQ(DEFAULT_DECLINE_PROBATION_PERIOD, - CfgMgr::instance().getStagingCfg()->getDeclinePeriod()); + // default value (86400). The default value is defined in GLOBAL6_DEFAULTS in + // simple_parser6.cc. + EXPECT_EQ(86400, CfgMgr::instance().getStagingCfg()->getDeclinePeriod()); +} + +/// Check that the dhcp4o6-port default value has a default value if not +/// specified explicitly. +TEST_F(Dhcp4ParserTest, dhcp4o6portDefault) { + + string config_txt = "{ " + genIfaceConfig() + "," + "\"subnet4\": [ ] " + "}"; + ConstElementPtr config; + ASSERT_NO_THROW(config = parseDHCP4(config_txt)); + + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, config)); + + // returned value should be 0 (success) + checkResult(status, 0); + + // The value of decline-probation-period must be equal to the + // default value (0). The default value is defined in GLOBAL4_DEFAULTS in + // simple_parser4.cc. + EXPECT_EQ(0, CfgMgr::instance().getStagingCfg()->getDhcp4o6Port()); } + /// Check that the decline-probation-period value can be set properly. TEST_F(Dhcp4ParserTest, declineTimer) { ConstElementPtr status; diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 764290d826..56de441226 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -33,7 +33,6 @@ #include #include #include -#include #include #include @@ -634,24 +633,23 @@ namespace dhcp { DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id, ConstElementPtr element) { DhcpConfigParser* parser = NULL; - if ((config_id.compare("preferred-lifetime") == 0) || - (config_id.compare("valid-lifetime") == 0) || - (config_id.compare("renew-timer") == 0) || - (config_id.compare("rebind-timer") == 0) || - (config_id.compare("decline-probation-period") == 0) || - (config_id.compare("dhcp4o6-port") == 0) ) { - parser = new Uint32Parser(config_id, - globalContext()->uint32_values_); + // preferred-lifetime, valid-lifetime, renew-timer, rebind-timer, + // decline-probation-period and dhcp4o6-port are now migrated to + // SimpleParser. // subnet6 has been converted to SimpleParser. // option-data and option-def are no longer needed here. They're now // converted to SimpleParser and are handled in configureDhcp6Server. // interfaces-config has been converted to SimpleParser. // version was removed - it was a leftover from bindctrl. + + // lease-database migrated + // hosts-database migrated + // hooks-libraries is now converted to SimpleParser. // lease-database and hosts-database have been converted to SimpleParser already. // mac-source has been converted to SimpleParser. // dhcp-ddns has been converted to SimpleParser - } else if (config_id.compare("relay-supplied-options") == 0) { + if (config_id.compare("relay-supplied-options") == 0) { parser = new RSOOListConfigParser(config_id); // control-socket has been converted to SimpleParser. // expired-leases-processing has been converted to SimpleParser. @@ -669,30 +667,43 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id, /// @brief Sets global parameters in the staging configuration /// +/// @param global global configuration scope +/// /// Currently this method sets the following global parameters: /// /// - decline-probation-period /// - dhcp4o6-port -void setGlobalParameters6() { +/// +/// @throw DhcpConfigError if parameters are missing or having incorrect values. +void setGlobalParameters6(ConstElementPtr global) { // Set the probation period for decline handling. - try { - uint32_t probation_period = globalContext()->uint32_values_ - ->getOptionalParam("decline-probation-period", - DEFAULT_DECLINE_PROBATION_PERIOD); - CfgMgr::instance().getStagingCfg()->setDeclinePeriod(probation_period); - } catch (...) { - // That's not really needed. + /// @todo: Use Francis' template from SimpleParser once 5097 is merged + /// This will be done as part of #5116. + int64_t probation_period = SimpleParser::getInteger(global, + "decline-probation-period"); + if (probation_period < std::numeric_limits::min() || + probation_period > std::numeric_limits::max()) { + isc_throw(DhcpConfigError, "Invalid decline-probation-period value: " + << probation_period << ", allowed range: " + << std::numeric_limits::min() << ".." + << std::numeric_limits::max()); } + CfgMgr::instance().getStagingCfg()->setDeclinePeriod(probation_period); // Set the DHCPv4-over-DHCPv6 interserver port. - try { - uint32_t dhcp4o6_port = globalContext()->uint32_values_ - ->getOptionalParam("dhcp4o6-port", 0); - CfgMgr::instance().getStagingCfg()->setDhcp4o6Port(dhcp4o6_port); - } catch (...) { - // Ignore errors. This flag is optional + + /// @todo: Use Francis' template from SimpleParser once 5097 is merged + /// This will be done as part of #5116. + uint32_t dhcp4o6_port = SimpleParser::getInteger(global, "dhcp4o6-port"); + if (dhcp4o6_port < std::numeric_limits::min() || + dhcp4o6_port > std::numeric_limits::max()) { + isc_throw(DhcpConfigError, "Invalid dhcp4o6-port value: " + << dhcp4o6_port << ", allowed range: " + << std::numeric_limits::min() << ".." + << std::numeric_limits::max()); } + CfgMgr::instance().getStagingCfg()->setDhcp4o6Port(dhcp4o6_port); } /// @brief Initialize the command channel based on the staging configuration @@ -788,6 +799,11 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { // so as we can rollback changes when an error occurs. ParserContext original_context(*globalContext()); + // 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. + ElementPtr mutable_cfg = boost::const_pointer_cast(config_set); + // answer will hold the result. ConstElementPtr answer; // rollback informs whether error occurred and original data @@ -801,11 +817,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { SrvConfigPtr 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. - ElementPtr mutable_cfg = boost::const_pointer_cast(config_set); - // Set all default values if not specified by the user. SimpleParser6::setAllDefaults(mutable_cfg); @@ -930,6 +941,21 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { continue; } + // Timers are not used in the global scope. Their values are derived + // to specific subnets (see SimpleParser6::deriveParameters). + // decline-probation-period and dhcp4o6-port are handled in the + // setGlobalParameters6. + if ( (config_pair.first == "renew-timer") || + (config_pair.first == "rebind-timer") || + (config_pair.first == "preferred-lifetime") || + (config_pair.first == "valid-lifetime") || + (config_pair.first == "decline-probation-period") || + (config_pair.first == "dhcp4o6-port")) { + continue; + } + + /// @todo: 5116 ticket: remove this chunk below once all parser + /// tickets are done. ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PARSER_CREATED) @@ -948,6 +974,9 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { // Setup the command channel. configureCommandChannel(); + // Apply global options in the staging config. + setGlobalParameters6(mutable_cfg); + } catch (const isc::Exception& ex) { LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL) .arg(config_pair.first).arg(ex.what()); @@ -971,9 +1000,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { if (!rollback) { try { - // Apply global options in the staging config. - setGlobalParameters6(); - // No need to commit interface names as this is handled by the // CfgMgr::commit() function. diff --git a/src/bin/dhcp6/simple_parser6.cc b/src/bin/dhcp6/simple_parser6.cc index 7c8fc96929..f30c19741f 100644 --- a/src/bin/dhcp6/simple_parser6.cc +++ b/src/bin/dhcp6/simple_parser6.cc @@ -55,10 +55,12 @@ const SimpleDefaults SimpleParser6::OPTION6_DEFAULTS = { /// in Dhcp6) are optional. If not defined, the following values will be /// used. const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = { - { "renew-timer", Element::integer, "900" }, - { "rebind-timer", Element::integer, "1800" }, - { "preferred-lifetime", Element::integer, "3600" }, - { "valid-lifetime", Element::integer, "7200" } + { "renew-timer", Element::integer, "900" }, + { "rebind-timer", Element::integer, "1800" }, + { "preferred-lifetime", Element::integer, "3600" }, + { "valid-lifetime", Element::integer, "7200" }, + { "decline-probation-period", Element::integer, "86400" }, // 24h + { "dhcp4o6-port", Element::integer, "0" } }; /// @brief List of parameters that can be inherited from the global to subnet6 scope. diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 382fc07d8b..e844014be1 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -25,7 +25,6 @@ #include #include #include -#include #include "test_data_files_config.h" #include "test_libraries.h" @@ -4749,7 +4748,8 @@ TEST_F(Dhcp6ParserTest, rsooBogusName) { EXPECT_TRUE(errorContainsPosition(status, "")); } -/// Check that the decline-probation-period value can be set properly. +/// Check that the decline-probation-period value has a default value if not +/// specified explicitly. TEST_F(Dhcp6ParserTest, declineTimerDefault) { string config_txt = "{ " + genIfaceConfig() + "," @@ -4765,9 +4765,31 @@ TEST_F(Dhcp6ParserTest, declineTimerDefault) { checkResult(status, 0); // The value of decline-probation-period must be equal to the - // default value. - EXPECT_EQ(DEFAULT_DECLINE_PROBATION_PERIOD, - CfgMgr::instance().getStagingCfg()->getDeclinePeriod()); + // default value (86400). The default value is defined in GLOBAL6_DEFAULTS in + // simple_parser6.cc. + EXPECT_EQ(86400, CfgMgr::instance().getStagingCfg()->getDeclinePeriod()); +} + +/// Check that the dhcp4o6-port default value has a default value if not +/// specified explicitly. +TEST_F(Dhcp6ParserTest, dhcp4o6portDefault) { + + string config_txt = "{ " + genIfaceConfig() + "," + "\"subnet6\": [ ] " + "}"; + ConstElementPtr config; + ASSERT_NO_THROW(config = parseDHCP6(config_txt)); + + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, config)); + + // returned value should be 0 (success) + checkResult(status, 0); + + // The value of decline-probation-period must be equal to the + // default value (0). The default value is defined in GLOBAL6_DEFAULTS in + // simple_parser6.cc. + EXPECT_EQ(0, CfgMgr::instance().getStagingCfg()->getDhcp4o6Port()); } /// Check that the decline-probation-period value can be set properly. diff --git a/src/defaults.h b/src/defaults.h deleted file mode 100644 index 0695baf72c..0000000000 --- a/src/defaults.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") -// -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -/// @file defaults.h -/// -/// @brief Contains the default values for various parameters. -/// -/// While the content of this file is currently small, it is envisaged that it -/// will grow over time. - -#ifndef DEFAULTS_H -#define DEFAULTS_H - -#include - -namespace isc { -namespace dhcp { - -/// @brief Number of seconds after declined lease recovers -/// -/// This define specifies the default value for decline probation period. -/// Once a lease is declined, it will spend this amount of seconds as -/// being unavailable. This is only the default value. Specific value may -/// be defined in the configuration file. The default is 1 day. -static const uint32_t DEFAULT_DECLINE_PROBATION_PERIOD = 24*3600; - -}; -}; - -#endif