From 159004ba12f4181e03d7307c656f9b8b806ddc23 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 27 Mar 2019 09:21:09 +0100 Subject: [PATCH] [#490,!284] Finished implementation of the inheritance for v4 and v6. --- src/lib/dhcpsrv/network.cc | 9 +- src/lib/dhcpsrv/network.h | 114 +++++++++- src/lib/dhcpsrv/tests/network_unittest.cc | 247 +++++++++++++++++++++- 3 files changed, 354 insertions(+), 16 deletions(-) diff --git a/src/lib/dhcpsrv/network.cc b/src/lib/dhcpsrv/network.cc index 59f599bf92..f84123b9d2 100644 --- a/src/lib/dhcpsrv/network.cc +++ b/src/lib/dhcpsrv/network.cc @@ -272,10 +272,9 @@ Network6::toElement() const { ElementPtr map = Network::toElement(); // Set preferred-lifetime - if (!getPreferred().unspecified()) { + if (!preferred_.unspecified()) { map->set("preferred-lifetime", - Element::create(static_cast - (getPreferred().get()))); + Element::create(static_cast(preferred_.get()))); } // Set interface-id @@ -291,8 +290,8 @@ Network6::toElement() const { } // Set rapid-commit - if (!getRapidCommit().unspecified()) { - map->set("rapid-commit", Element::create(getRapidCommit().get())); + if (!rapid_commit_.unspecified()) { + map->set("rapid-commit", Element::create(rapid_commit_.get())); } return (map); diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h index 583960745c..b2760681a9 100644 --- a/src/lib/dhcpsrv/network.h +++ b/src/lib/dhcpsrv/network.h @@ -25,6 +25,26 @@ #include #include +namespace isc { +namespace data { + +/// @brief The @c ElementExtractor specialization for IOAddress. +template<> +class data::ElementExtractor { +public: + + /// @brief Function operator extracting an @c Element value as + /// string. + /// + /// @param el Element holding a value to be extracted. + asiolink::IOAddress operator()(ConstElementPtr el) const { + return (asiolink::IOAddress(el->stringValue())); + } +}; + +} // end of namespace isc::data +} // end of namespace isc + namespace isc { namespace dhcp { @@ -435,32 +455,107 @@ public: protected: + /// @brief Returns a value associated with a network using inheritance. + /// + /// This template method provides a generic mechanism to retrieve a + /// network parameter using inheritance. It is called from public + /// accessor methods which return an @c OptionalValue or @c Triplet. + /// + /// @tparam BaseType Type of this instance, e.g. @c Network, @c Network4 + /// etc, which exposes a method to be called. + /// @tparam ReturnType Type of the returned value, e.g. + /// @c Optional. + /// + /// @param MethodPointer Pointer to the method of the base class which + /// 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 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 + /// global value for the given parameter is not supported and shouldn't + /// be fetched. + /// + /// @return Optional value fetched from this instance level, parent + /// network level or global level template 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. if (property.unspecified()) { + // Check if this instance has a parent network. auto parent = boost::dynamic_pointer_cast(parent_network_.lock()); if (parent) { + // Run the same method on the parent instance to fetch the + // parent level (typically shared network level) value. auto parent_property = ((*parent).*MethodPointer)(); + // If the value is found, return it. if (!parent_property.unspecified()) { return (parent_property); - } } + // The value is not specified on network level. If the value + // 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::ElementExtractor()(global_param)); } } } } + // We haven't found the value at any level, so return the unspecified. + return (property); + } + + /// @brief Returns option pointer associated with a network using inheritance. + /// + /// This template method provides a generic mechanism to retrieve a + /// network parameter using inheritance. It is called from public + /// accessor methods which return an @c OptionPtr. + /// + /// @tparam BaseType Type of this instance, e.g. @c Network, @c Network4 + /// etc, which exposes a method to be called. + /// + /// @param MethodPointer Pointer to the method of the base class which + /// 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. + /// + /// @return Option pointer fetched from this instance level or parent + /// network level. + template + OptionPtr + getOptionProperty(OptionPtr(BaseType::*MethodPointer)() const, + OptionPtr property) const { + // If the value is specified on this level, let's simply return it. + // The lower level value always takes precedence. + if (!property) { + // Check if this instance has a parent network. + auto parent = boost::dynamic_pointer_cast(parent_network_.lock()); + if (parent) { + // Run the same method on the parent instance to fetch the + // parent level (typically shared network level) value. + auto parent_property = ((*parent).*MethodPointer)(); + // If the value is found, return it. + if (parent_property) { + return (parent_property); + } + } + } + + // We haven't found the value at any level, so return the unspecified. return (property); } @@ -583,7 +678,8 @@ public: /// /// @return siaddr value util::Optional getSiaddr() const { - return (siaddr_); + return (getProperty(&Network4::getSiaddr, siaddr_, + "next-server")); } /// @brief Sets server hostname for the network. @@ -595,7 +691,8 @@ public: /// /// @return server hostname value util::Optional getSname() const { - return (sname_); + return (getProperty(&Network4::getSname, sname_, + "server-hostname")); } /// @brief Sets boot file name for the network. @@ -607,7 +704,8 @@ public: /// /// @return boot file name value util::Optional getFilename() const { - return (filename_); + return (getProperty(&Network4::getFilename, filename_, + "boot-file-name")); } /// @brief Unparses network object. @@ -653,7 +751,8 @@ public: /// /// @return a triplet with preferred lifetime Triplet getPreferred() const { - return (preferred_); + return (getProperty(&Network6::getPreferred, preferred_, + "preferred-lifetime")); } /// @brief Sets new preferred lifetime for a network. @@ -667,7 +766,7 @@ public: /// /// @return interface-id option (if defined) OptionPtr getInterfaceId() const { - return (interface_id_); + return (getOptionProperty(&Network6::getInterfaceId, interface_id_)); } /// @brief sets interface-id option (if defined) @@ -682,7 +781,8 @@ public: /// /// @return true if the Rapid Commit option is supported, false otherwise. util::Optional getRapidCommit() const { - return (rapid_commit_); + return (getProperty(&Network6::getRapidCommit, rapid_commit_, + "rapid-commit")); } /// @brief Enables or disables Rapid Commit option support for the subnet. diff --git a/src/lib/dhcpsrv/tests/network_unittest.cc b/src/lib/dhcpsrv/tests/network_unittest.cc index a9d9a609f3..b8dfef047b 100644 --- a/src/lib/dhcpsrv/tests/network_unittest.cc +++ b/src/lib/dhcpsrv/tests/network_unittest.cc @@ -5,61 +5,295 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include +#include +#include +#include #include #include #include #include +using namespace isc::asiolink; using namespace isc::data; using namespace isc::dhcp; +using namespace isc::util; namespace { class TestNetwork; + +/// @brief Shared pointer to the derivation of the @c Network class +/// used in tests. typedef boost::shared_ptr TestNetworkPtr; -class TestNetwork : public Network { +/// @brief Derivation of the @c Network class allowing to set +/// the parent @c Network object. +class TestNetwork : public virtual Network { public: + /// @brief Associates parent network with this network. + /// + /// @param parent Pointer to the instance of the parent network. void setParent(TestNetworkPtr parent) { parent_network_ = boost::dynamic_pointer_cast(parent); } - }; +/// @brief Derivation of the @c Network4 class allowing to set +/// the parent @c Network object. +class TestNetwork4 : public TestNetwork, public Network4 { }; + +/// @brief Derivation of the @c Network6 class allowing to set +/// the parent @c Network object. +class TestNetwork6 : public TestNetwork, public Network6 { }; + +/// @brief Test fixture class for testing @c Network class and +/// its derivations. class NetworkTest : public ::testing::Test { public: + /// @brief Constructor. NetworkTest() : globals_(Element::createMap()) { } + /// @brief Returns pointer to the function which returns configured + /// global parameters. FetchNetworkGlobalsFn getFetchGlobalsFn() { return (std::bind(&NetworkTest::fetchGlobalsFn, this)); } + /// @brief Returns configured global parameters. ConstElementPtr fetchGlobalsFn() { return (globals_); } - template - void testNetworkInheritance() { + /// @brief Test that inheritance mechanism is used for the particular + /// network parameter. + /// + /// @tparam BaseType1 Type of the network object to be tested, e.g. + /// @c TestNetwork, @c TestNetwork4 etc. + /// @tparam BaseType2 Type of the object to which the tested parameter + /// belongs, e.g. @c Network, @c Network4 etc. + /// @tparam ParameterType1 Type returned by the accessor of the parameter + /// under test, e.g. @c Optional. + /// @tparam ParameterType2 Type of the value accepted by the modifier + /// method which sets the value under test. It may be the same as + /// @c ParameterType1 but not necessarily. For example, the @c ParameterType1 + /// may be @c Optional while the @c ParameterType2 is + /// @c std::string. + /// + /// @param GetMethodPointer Pointer to the method of the class under test + /// which returns the particular parameter. + /// @param SetMethodPointer Pointer to the method of the class under test + /// used to set the particular parameter. + /// @param network_value Value of the parameter to be assigned to the + /// parent network. + /// @param global_value Global value of the parameter to be assigned. + /// @param test_global_value Boolean value indicating if the inheritance + /// of the global value should be tested. + template + void testNetworkInheritance(ParameterType1(BaseType2::*GetMethodPointer)() const, + void(BaseType2::*SetMethodPointer)(const ParameterType2&), + typename ParameterType1::ValueType network_value, + typename ParameterType1::ValueType global_value, + const bool test_global_value = true) { + + // Create child network object. The value retrieved from that + // object should be unspecified until we set the value for the + // parent network or a global value. + boost::shared_ptr net_child(new BaseType1()); + EXPECT_TRUE(((*net_child).*GetMethodPointer)().unspecified()); + + // Create parent network and set the value. + boost::shared_ptr net_parent(new BaseType1()); + ((*net_parent).*SetMethodPointer)(network_value); + EXPECT_EQ(network_value, ((*net_parent).*GetMethodPointer)().get()); + + // Assign callbacks that fetch global values to the networks. + net_child->setFetchGlobalsFn(getFetchGlobalsFn()); + net_parent->setFetchGlobalsFn(getFetchGlobalsFn()); + + // Not all parameters have the corresponding global values. + if (test_global_value) { + // If there is a global value it should now be returned. + EXPECT_FALSE(((*net_child).*GetMethodPointer)().unspecified()); + EXPECT_EQ(global_value, ((*net_child).*GetMethodPointer)().get()); + } + + // Associated the network with its parent. + ASSERT_NO_THROW(net_child->setParent(net_parent)); + + // This time the parent specific value should be returned. + EXPECT_FALSE(((*net_child).*GetMethodPointer)().unspecified()); + EXPECT_EQ(network_value, ((*net_child).*GetMethodPointer)().get()); } + /// @brief Holds the collection of configured globals. ElementPtr globals_; }; +// This test verifies that the inheritance is supported for certain +// network parameters. +TEST_F(NetworkTest, inheritanceSupport4) { + // Set global values for each parameter. + globals_->set("interface", Element::create("g")); + 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("calculate-tee-times", Element::create(false)); + globals_->set("t1-percent", Element::create(0.75)); + globals_->set("t2-percent", Element::create(0.6)); + globals_->set("match-client-id", Element::create(true)); + globals_->set("authoritative", Element::create(false)); + globals_->set("next-server", Element::create("192.0.2.3")); + globals_->set("server-hostname", Element::create("g")); + globals_->set("boot-file-name", Element::create("g")); + + // For each parameter for which inheritance is supported run + // the test that checks if the values are inherited properly. + + { + SCOPED_TRACE("interface"); + testNetworkInheritance(&Network::getIface, &Network::setIface, + "n", "g"); + } + { + SCOPED_TRACE("client_class"); + testNetworkInheritance(&Network::getClientClass, + &Network::allowClientClass, + "n", "g", false); + } + { + SCOPED_TRACE("valid-lifetime"); + testNetworkInheritance(&Network::getValid, &Network::setValid, + 60, 80); + } + { + SCOPED_TRACE("renew-timer"); + testNetworkInheritance(&Network::getT1, &Network::setT1, + 60, 80); + } + { + SCOPED_TRACE("rebind-timer"); + testNetworkInheritance(&Network::getT2, &Network::setT2, + 60, 80); + } + { + SCOPED_TRACE("reservation-mode"); + testNetworkInheritance(&Network::getHostReservationMode, + &Network::setHostReservationMode, + Network::HR_OUT_OF_POOL, + Network::HR_DISABLED); + } + { + SCOPED_TRACE("calculate-tee-times"); + testNetworkInheritance(&Network::getCalculateTeeTimes, + &Network::setCalculateTeeTimes, + true, false); + } + { + SCOPED_TRACE("t1-percent"); + testNetworkInheritance(&Network::getT1Percent, + &Network::setT1Percent, + 0.5, 0.75); + } + { + SCOPED_TRACE("t2-percent"); + testNetworkInheritance(&Network::getT2Percent, + &Network::setT2Percent, + 0.3, 0.6); + } + { + SCOPED_TRACE("match-client-id"); + testNetworkInheritance(&Network4::getMatchClientId, + &Network4::setMatchClientId, + false, true); + } + { + SCOPED_TRACE("authoritative"); + testNetworkInheritance(&Network4::getAuthoritative, + &Network4::setAuthoritative, + true, false); + } + { + SCOPED_TRACE("next-server"); + testNetworkInheritance(&Network4::getSiaddr, + &Network4::setSiaddr, + IOAddress("192.0.2.0"), + IOAddress("192.0.2.3")); + } + { + SCOPED_TRACE("server-hostname"); + testNetworkInheritance(&Network4::getSname, + &Network4::setSname, + "n", "g"); + } + { + SCOPED_TRACE("boot-file-name"); + testNetworkInheritance(&Network4::getFilename, + &Network4::setFilename, + "n", "g"); + } +} + +// This test verifies that the inheritance is supported for DHCPv6 +// specific network parameters. +TEST_F(NetworkTest, inheritanceSupport6) { + // Set global values for each parameter. + globals_->set("preferred-lifetime", Element::create(80)); + globals_->set("rapid-commit", Element::create(false)); + + // For each parameter for which inheritance is supported run + // the test that checks if the values are inherited properly. + + { + SCOPED_TRACE("preferred-lifetime"); + testNetworkInheritance(&Network6::getPreferred, + &Network6::setPreferred, + 60, 80); + } + { + SCOPED_TRACE("rapid-commit"); + testNetworkInheritance(&Network6::getRapidCommit, + &Network6::setRapidCommit, + true, false); + } + + // Interface-id requires special type of test. + boost::shared_ptr net_child(new TestNetwork6()); + EXPECT_TRUE(net_child->getIface().unspecified()); + + OptionPtr interface_id(new Option(Option::V6, D6O_INTERFACE_ID, + OptionBuffer(10, 0xFF))); + + boost::shared_ptr net_parent(new TestNetwork6()); + net_parent->setInterfaceId(interface_id); + + ASSERT_NO_THROW(net_child->setParent(net_parent)); + + EXPECT_TRUE(net_child->getInterfaceId()); + EXPECT_EQ(interface_id, net_child->getInterfaceId()); +} + +// Test that child network returns unspecified value if neither +// parent no global value exists. TEST_F(NetworkTest, getPropertyNoParentNoChild) { NetworkPtr net_child(new Network()); EXPECT_TRUE(net_child->getIface().unspecified()); } +// Test that child network returns specified value. TEST_F(NetworkTest, getPropertyNoParentChild) { NetworkPtr net_child(new Network()); net_child->setIface("child_iface"); EXPECT_EQ("child_iface", net_child->getIface().get()); } +// Test that parent specific value is returned when the value +// is not specified for the child network. TEST_F(NetworkTest, getPropertyParentNoChild) { TestNetworkPtr net_child(new TestNetwork()); EXPECT_TRUE(net_child->getIface().unspecified()); @@ -74,6 +308,8 @@ TEST_F(NetworkTest, getPropertyParentNoChild) { EXPECT_EQ("parent_iface", net_child->getIface().get()); } +// Test that value specified for the child network takes +// precedence over the value specified for the parent network. TEST_F(NetworkTest, getPropertyParentChild) { TestNetworkPtr net_child(new TestNetwork()); net_child->setIface("child_iface"); @@ -89,6 +325,8 @@ TEST_F(NetworkTest, getPropertyParentChild) { EXPECT_EQ("child_iface", net_child->getIface().get()); } +// Test that global value is inherited if there is no network +// specific value. TEST_F(NetworkTest, getPropertyGlobalNoParentNoChild) { TestNetworkPtr net_child(new TestNetwork()); @@ -97,6 +335,7 @@ TEST_F(NetworkTest, getPropertyGlobalNoParentNoChild) { net_child->setFetchGlobalsFn(getFetchGlobalsFn()); EXPECT_FALSE(net_child->getIface().unspecified()); + EXPECT_EQ("global_iface", net_child->getIface().get()); } } -- 2.47.2