From: Francis Dupont Date: Thu, 26 Jan 2017 00:26:10 +0000 (+0100) Subject: [5019] Addressed some (but not all) comments/concerns X-Git-Tag: trac3389_base~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e1591d829b9dc1dacfbe5b366f04fd991f44ed5c;p=thirdparty%2Fkea.git [5019] Addressed some (but not all) comments/concerns --- diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 35b9ab1014..bf206bea45 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -125,10 +125,16 @@ public: parser.parse(pools_, pools); } - SubnetPtr generic = SubnetConfigParser::build(subnet); + SubnetPtr generic = SubnetConfigParser::parse(subnet); - Subnet4Ptr sub4ptr = boost::dynamic_pointer_cast(generic); - if (!sub4ptr) { + if (!generic) { + isc_throw(DhcpConfigError, + "Failed to create an IPv4 subnet (" << + subnet->getPosition() << ")"); + } + + Subnet4Ptr sn4ptr = boost::dynamic_pointer_cast(subnet_); + if (!sn4ptr) { // If we hit this, it is a programming error. isc_throw(Unexpected, "Invalid Subnet4 cast in Subnet4ConfigParser::parse"); @@ -136,7 +142,7 @@ public: // Set relay information if it was parsed if (relay_info_) { - sub4ptr->setRelayInfo(*relay_info_); + sn4ptr->setRelayInfo(*relay_info_); } // Parse Host Reservations for this subnet if any. @@ -146,7 +152,7 @@ public: parser.parse(subnet_->getID(), reservations); } - return (sub4ptr); + return (sn4ptr); } protected: @@ -378,6 +384,65 @@ public: } }; +class Dhcp4ConfigParser : public isc::data::SimpleParser { +public: + + /// @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 + /// + /// @throw DhcpConfigError if parameters are missing or + /// or having incorrect values. + void parse(ConstElementPtr global) { + + // Set whether v4 server is supposed to echo back client-id + // (yes = RFC6842 compatible, no = backward compatibility) + bool echo_client_id = getBoolean(global, "echo-client-id"); + CfgMgr::instance().echoClientId(echo_client_id); + + SrvConfigPtr srv_cfg = CfgMgr::instance().getStagingCfg(); + std::string name; + ConstElementPtr value; + try { + // Set the probation period for decline handling. + name = "decline-probation-period"; + value = global->get(name); + uint32_t probation_period = getUint32(name, value); + srv_cfg->setDeclinePeriod(probation_period); + + // Set the DHCPv4-over-DHCPv6 interserver port. + name = "dhcp4o6-port"; + value = global->get(name); + // @todo Change for uint16_t + uint32_t dhcp4o6_port = getUint32(name, value); + srv_cfg->setDhcp4o6Port(dhcp4o6_port); + } catch (const isc::data::TypeError& ex) { + isc_throw(DhcpConfigError, + "invalid value type specified for parameter '" << name + << "' (" << value->getPosition() << ")"); + } + } + +private: + + /// @brief Returns a value converted to uint32_t + /// + /// Instantiation of extractInt() to uint32_t + /// + /// @param value value of the parameter + /// @return an uint32_t value + uint32_t getUint32(const std::string& name, + isc::data::ConstElementPtr value) { + return (extractInt(name, value)); + } +}; + } // anonymous namespace namespace isc { @@ -420,49 +485,6 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id, 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(ConstElementPtr global) { - // Set whether v4 server is supposed to echo back client-id (yes = RFC6842 - // compatible, no = backward compatibility) - 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. - - /// @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 /// /// Only close the current channel, if the new channel configuration is @@ -685,7 +707,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { // 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. + // handled in global_parser.parse() which sets global parameters. // match-client-id is derived to subnet scope level. if ( (config_pair.first == "renew-timer") || (config_pair.first == "rebind-timer") || @@ -718,6 +740,19 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { // Setup the command channel. configureCommandChannel(); + // the leases database parser is the last to be run. + std::map::const_iterator leases_config = + values_map.find("lease-database"); + if (leases_config != values_map.end()) { + config_pair.first = "lease-database"; + leases_parser->build(leases_config->second); + leases_parser->commit(); + } + + // Apply global options in the staging config. + Dhcp4ConfigParser global_parser; + global_parser.parse(mutable_cfg); + } catch (const isc::Exception& ex) { LOG_ERROR(dhcp4_logger, DHCP4_PARSER_FAIL) .arg(config_pair.first).arg(ex.what()); @@ -791,7 +826,5 @@ ParserContextPtr& globalContext() { return (global_context_ptr); } - - }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 0b1546d40e..d32eaa7c8a 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -323,7 +323,7 @@ public: parser.parse(pools_, pd_pools); } - SubnetPtr generic = SubnetConfigParser::build(subnet); + SubnetPtr generic = SubnetConfigParser::parse(subnet); if (!generic) { isc_throw(DhcpConfigError, @@ -331,8 +331,8 @@ public: subnet->getPosition() << ")"); } - Subnet6Ptr sub6ptr = boost::dynamic_pointer_cast(subnet_); - if (!sub6ptr) { + Subnet6Ptr sn6ptr = boost::dynamic_pointer_cast(subnet_); + if (!sn6ptr) { // If we hit this, it is a programming error. isc_throw(Unexpected, "Invalid Subnet6 cast in Subnet6ConfigParser::parse"); @@ -340,7 +340,7 @@ public: // Set relay information if it was provided if (relay_info_) { - sub6ptr->setRelayInfo(*relay_info_); + sn6ptr->setRelayInfo(*relay_info_); } @@ -351,7 +351,7 @@ public: parser.parse(subnet_->getID(), reservations); } - return (sub6ptr); + return (sn6ptr); } protected: @@ -616,6 +616,58 @@ public: virtual void commit() {} }; +class Dhcp6ConfigParser : public isc::data::SimpleParser { +public: + + /// @brief Sets global parameters in staging configuration + /// + /// @param global global configuration scope + /// + /// Currently this method sets the following global parameters: + /// + /// - decline-probation-period + /// - dhcp4o6-port + /// + /// @throw DhcpConfigError if parameters are missing or + /// or having incorrect values. + void parse(ConstElementPtr global) { + + SrvConfigPtr srv_config = CfgMgr::instance().getStagingCfg(); + std::string name; + ConstElementPtr value; + try { + // Set the probation period for decline handling. + name = "decline-probation-period"; + value = global->get(name); + uint32_t probation_period = getUint32(name, value); + srv_config->setDeclinePeriod(probation_period); + + // Set the DHCPv4-over-DHCPv6 interserver port. + name = "dhcp4o6-port"; + value = global->get(name); + // @todo Change for uint16_t + uint32_t dhcp4o6_port = getUint32(name, value); + srv_config->setDhcp4o6Port(dhcp4o6_port); + } catch (const isc::data::TypeError& ex) { + isc_throw(DhcpConfigError, + "invalid value type specified for parameter '" << name + << "' (" << value->getPosition() << ")"); + } + } + +private: + + /// @brief Returns a value converted to uint32_t + /// + /// Instantiation of extractInt() to uint32_t + /// + /// @param value value of the parameter + /// @return an uint32_t value + uint32_t getUint32(const std::string& name, + isc::data::ConstElementPtr value) { + return (extractInt(name, value)); + } +}; } // anonymous namespace @@ -667,47 +719,6 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id, return (parser); } -/// @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 -/// -/// @throw DhcpConfigError if parameters are missing or having incorrect values. -void setGlobalParameters6(ConstElementPtr global) { - - // Set the probation period for decline handling. - /// @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. - - /// @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 /// /// Only close the current channel, if the new channel configuration is @@ -946,7 +957,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { // 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. + // global_parser.parse() which sets global parameters. if ( (config_pair.first == "renew-timer") || (config_pair.first == "rebind-timer") || (config_pair.first == "preferred-lifetime") || @@ -977,7 +988,8 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { configureCommandChannel(); // Apply global options in the staging config. - setGlobalParameters6(mutable_cfg); + Dhcp6ConfigParser global_parser; + global_parser.parse(mutable_cfg); } catch (const isc::Exception& ex) { LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL) diff --git a/src/lib/cc/simple_parser.h b/src/lib/cc/simple_parser.h index 1f7d872012..6d14cd0701 100644 --- a/src/lib/cc/simple_parser.h +++ b/src/lib/cc/simple_parser.h @@ -113,6 +113,8 @@ class SimpleParser { static const data::Element::Position& getPosition(const std::string& name, const data::ConstElementPtr parent); +protected: + /// @brief Returns a string parameter from a scope /// /// Unconditionally returns a parameter. If the parameter is not there or diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index e349794eed..7a41881dec 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -999,7 +999,7 @@ SubnetConfigParser::SubnetConfigParser(const std::string&, } SubnetPtr -SubnetConfigParser::build(ConstElementPtr subnet) { +SubnetConfigParser::parse(ConstElementPtr subnet) { BOOST_FOREACH(ConfigPair param, subnet->mapValue()) { // Pools has been converted to SimpleParser. if (param.first == "pools") { diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 835b2ce0fd..b347b91edd 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -831,7 +831,7 @@ protected: /// @return a pointer to newly created subnet /// /// @throw isc::DhcpConfigError if subnet configuration parsing failed. - virtual SubnetPtr build(isc::data::ConstElementPtr subnet); + SubnetPtr parse(isc::data::ConstElementPtr subnet); /// @brief creates parsers for entries in subnet definition /// @@ -840,8 +840,8 @@ protected: /// @return parser object for specified entry name /// @throw isc::dhcp::DhcpConfigError if trying to create a parser /// for unknown config element - virtual DhcpConfigParser* createSubnetConfigParser( - const std::string& config_id) = 0; + virtual DhcpConfigParser* + createSubnetConfigParser(const std::string& config_id) = 0; /// @brief Issues a server specific warning regarding duplicate subnet /// options. @@ -851,7 +851,7 @@ protected: /// @todo a means to know the correct logger and perhaps a common /// message would allow this method to be emitted by the base class. virtual void duplicate_option_warning(uint32_t code, - isc::asiolink::IOAddress& addr) = 0; + isc::asiolink::IOAddress& addr) = 0; /// @brief Instantiates the subnet based on a given IP prefix and prefix /// length.