From: Marcin Siodelski Date: Fri, 29 Mar 2019 10:29:35 +0000 (+0100) Subject: [#490,!293] Corrected an issue with global reservation-mode inheritance. X-Git-Tag: Kea-1.6.0-beta~293 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=61847b06cf67c57137a45e6b1e575463ce4d6c7f;p=thirdparty%2Fkea.git [#490,!293] Corrected an issue with global reservation-mode inheritance. --- diff --git a/src/lib/dhcpsrv/network.cc b/src/lib/dhcpsrv/network.cc index f84123b9d2..8b106aa303 100644 --- a/src/lib/dhcpsrv/network.cc +++ b/src/lib/dhcpsrv/network.cc @@ -99,6 +99,24 @@ Network::getRequiredClasses() const { return (required_classes_); } +Network::HRMode +Network::hrModeFromString(const std::string& hr_mode_name) { + if ( (hr_mode_name.compare("disabled") == 0) || + (hr_mode_name.compare("off") == 0) ) { + return (Network::HR_DISABLED); + } else if (hr_mode_name.compare("out-of-pool") == 0) { + return (Network::HR_OUT_OF_POOL); + } else if (hr_mode_name.compare("global") == 0) { + return (Network::HR_GLOBAL); + } else if (hr_mode_name.compare("all") == 0) { + return (Network::HR_ALL); + } else { + // Should never happen... + isc_throw(BadValue, "Can't convert '" << hr_mode_name + << "' into any valid reservation-mode values"); + } +} + ElementPtr Network::toElement() const { ElementPtr map = Element::createMap(); diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h index 1793baf81b..91611b9f91 100644 --- a/src/lib/dhcpsrv/network.h +++ b/src/lib/dhcpsrv/network.h @@ -388,12 +388,29 @@ public: /// not in the dynamic pool). HR may also be completely disabled for /// performance reasons. /// - /// @return whether in-pool host reservations are allowed. + /// @return Host reservation mode enabled. util::Optional getHostReservationMode() const { - return (getProperty(&Network::getHostReservationMode, - host_reservation_mode_, - "reservation-mode")); + // Inheritance for host reservations is a little different than for other + // parameters. The reservation at the global level is given as a string. + // Thus we call getProperty here without a global name to check if the + // host reservation mode is specified on network level only. + const util::Optional& hr_mode = getProperty(&Network::getHostReservationMode, + host_reservation_mode_); + // If HR mode is not specified at network level we need this special + // case code to handle conversion of the globally configured HR + // mode to an enum. + if (hr_mode.unspecified()) { + // Get global reservation mode. + util::Optional hr_mode_name; + hr_mode_name = getGlobalProperty(hr_mode_name, "reservation-mode"); + if (!hr_mode_name.unspecified()) { + // If the HR mode is globally configured, let's convert it from + // a string to enum. + return (hrModeFromString(hr_mode_name.get())); + } + } + return (hr_mode); } /// @brief Sets host reservation mode. @@ -405,6 +422,18 @@ public: host_reservation_mode_ = mode; } + /// @brief Attempts to convert text representation to HRMode enum. + /// + /// Allowed values are "disabled", "off" (alias for disabled), + /// "out-of-pool" and "all". See @c Network::HRMode for their exact meaning. + /// + /// @param hr_mode_name Host Reservation mode in the textual form. + /// + /// @throw BadValue if the text cannot be converted. + /// + /// @return one of allowed HRMode values + static HRMode hrModeFromString(const std::string& hr_mode_name); + /// @brief Returns pointer to the option data configuration for this network. CfgOptionPtr getCfgOption() { return (cfg_option_); @@ -465,6 +494,45 @@ public: protected: + /// @brief Returns a value of global configuration parameter with + /// a given name. + /// + /// If the @c ferch_globals_fn_ function is non-null, this method will + /// invoke this function to retrieve a global value having the given + /// name. Typically, this method is invoked by @c getProperty when + /// network specific value of the parameter is not found. In some cases + /// it may be called by other methods. One such example is the + /// @c getHostReservationMode which needs to call @c getGlobalProperty + /// explicitly to convert the global host reservation mode value from + /// a string to an enum. + /// + /// @tparam ReturnType Type of the returned value, e.g. + /// @c Optional. + /// + /// @param property Value to be returned when it is specified or when + /// no global value is found. + /// @param global_name Name of the global parameter which value should + /// be returned + /// + /// @return Optional value fetched from the global level or the value + /// of @c property. + template + ReturnType getGlobalProperty(ReturnType property, + const std::string& global_name) const { + if (!global_name.empty() && fetch_globals_fn_) { + data::ConstElementPtr globals = fetch_globals_fn_(); + if (globals && (globals->getType() == data::Element::map)) { + data::ConstElementPtr global_param = globals->get(global_name); + if (global_param) { + // If there is a global parameter, convert it to the + // optional value of the given type and return. + return (data::ElementValue()(global_param)); + } + } + } + return (property); + } + /// @brief Returns a value associated with a network using inheritance. /// /// This template method provides a generic mechanism to retrieve a @@ -480,6 +548,8 @@ protected: /// should be called on the parent network instance (typically on /// @c SharedNetwork4 or @c SharedNetwork6) to fetch the parent specific /// value if the value is unspecified for this instance. + /// @param property Value to be returned when it is specified or when + /// no explicit value is specified on upper inheritance levels. /// @param global_name Optional name of the global parameter which value /// should be returned if the given parameter is not specified on network /// level. This value is empty by default, which indicates that the @@ -489,9 +559,8 @@ protected: /// @return Optional value fetched from this instance level, parent /// network level or global level template - ReturnType - getProperty(ReturnType(BaseType::*MethodPointer)() const, - ReturnType property, + ReturnType getProperty(ReturnType(BaseType::*MethodPointer)() const, + ReturnType property, const std::string& global_name = "") const { // If the value is specified on this level, let's simply return it. // The lower level value always takes precedence. @@ -512,17 +581,7 @@ protected: // can be specified on global level and there is a callback // that returns the global values, try to find this parameter // at the global scope. - if (!global_name.empty() && fetch_globals_fn_) { - data::ConstElementPtr globals = fetch_globals_fn_(); - if (globals && (globals->getType() == data::Element::map)) { - data::ConstElementPtr global_param = globals->get(global_name); - if (global_param) { - // If there is a global parameter, convert it to the - // optional value of the given type and return. - return (data::ElementValue()(global_param)); - } - } - } + return (getGlobalProperty(property, global_name)); } // We haven't found the value at any level, so return the unspecified. diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 392aab545d..bcffcca7dc 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -518,24 +518,6 @@ SubnetConfigParser::parse(ConstElementPtr subnet) { return (subnet_); } -Network::HRMode -SubnetConfigParser::hrModeFromText(const std::string& txt) { - if ( (txt.compare("disabled") == 0) || - (txt.compare("off") == 0) ) { - return (Network::HR_DISABLED); - } else if (txt.compare("out-of-pool") == 0) { - return (Network::HR_OUT_OF_POOL); - } else if (txt.compare("global") == 0) { - return (Network::HR_GLOBAL); - } else if (txt.compare("all") == 0) { - return (Network::HR_ALL); - } else { - // Should never happen... - isc_throw(BadValue, "Can't convert '" << txt - << "' into any valid reservation-mode values"); - } -} - void SubnetConfigParser::createSubnet(ConstElementPtr params) { std::string subnet_txt; @@ -793,7 +775,7 @@ Subnet4ConfigParser::initSubnet(data::ConstElementPtr params, if (params->contains("reservation-mode")) { try { std::string hr_mode = getString(params, "reservation-mode"); - subnet4->setHostReservationMode(hrModeFromText(hr_mode)); + subnet4->setHostReservationMode(Network::hrModeFromString(hr_mode)); } catch (const BadValue& ex) { isc_throw(DhcpConfigError, "Failed to process specified value " " of reservation-mode parameter: " << ex.what() @@ -1219,7 +1201,7 @@ Subnet6ConfigParser::initSubnet(data::ConstElementPtr params, if (params->contains("reservation-mode")) { try { std::string hr_mode = getString(params, "reservation-mode"); - subnet6->setHostReservationMode(hrModeFromText(hr_mode)); + subnet6->setHostReservationMode(Network::hrModeFromString(hr_mode)); } catch (const BadValue& ex) { isc_throw(DhcpConfigError, "Failed to process specified value " " of reservation-mode parameter: " << ex.what() diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 234075621d..cdba49fa98 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -482,18 +482,6 @@ protected: virtual void initSubnet(isc::data::ConstElementPtr params, isc::asiolink::IOAddress addr, uint8_t len) = 0; - /// @brief Attempts to convert text representation to HRMode enum. - /// - /// Allowed values are "disabled", "off" (alias for disabled), - /// "out-of-pool" and "all". See Subnet::HRMode for their exact meaning. - /// - /// @param txt Host Reservation mode in the textual form. - /// - /// @throw BadValue if the text cannot be converted. - /// - /// @return one of allowed HRMode values - static Network::HRMode hrModeFromText(const std::string& txt); - private: /// @brief Create a new subnet using a data from child parsers. diff --git a/src/lib/dhcpsrv/tests/network_unittest.cc b/src/lib/dhcpsrv/tests/network_unittest.cc index b8dfef047b..dd13f9432a 100644 --- a/src/lib/dhcpsrv/tests/network_unittest.cc +++ b/src/lib/dhcpsrv/tests/network_unittest.cc @@ -134,6 +134,17 @@ public: ElementPtr globals_; }; +// This test verifies conversions of host reservation mode names to +// appropriate enum values. +TEST_F(NetworkTest, hrModeFromString) { + EXPECT_EQ(Network::HR_DISABLED, Network::hrModeFromString("off")); + EXPECT_EQ(Network::HR_DISABLED, Network::hrModeFromString("disabled")); + EXPECT_EQ(Network::HR_OUT_OF_POOL, Network::hrModeFromString("out-of-pool")); + EXPECT_EQ(Network::HR_GLOBAL, Network::hrModeFromString("global")); + EXPECT_EQ(Network::HR_GLOBAL, Network::hrModeFromString("all")); + EXPECT_THROW(Network::hrModeFromString("bogus"), isc::BadValue); +} + // This test verifies that the inheritance is supported for certain // network parameters. TEST_F(NetworkTest, inheritanceSupport4) { @@ -142,7 +153,7 @@ TEST_F(NetworkTest, inheritanceSupport4) { globals_->set("valid-lifetime", Element::create(80)); globals_->set("renew-timer", Element::create(80)); globals_->set("rebind-timer", Element::create(80)); - globals_->set("reservation-mode", Element::create(static_cast(Network::HR_DISABLED))); + globals_->set("reservation-mode", Element::create("disabled")); globals_->set("calculate-tee-times", Element::create(false)); globals_->set("t1-percent", Element::create(0.75)); globals_->set("t2-percent", Element::create(0.6));