From: Thomas Markwalder Date: Fri, 29 Mar 2019 19:17:57 +0000 (-0400) Subject: [#413,!288] kea-dhcp6 now uses shared-networks config backends X-Git-Tag: Kea-1.6.0-beta~270^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ff367e273ed8763b354db272c5955a78203d865e;p=thirdparty%2Fkea.git [#413,!288] kea-dhcp6 now uses shared-networks config backends src/bin/dhcp6/tests/config_backend_unittest.cc TEST_F(Dhcp6CBTest, mergeSharedNetworks) - enabled test src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc TEST(CfgSharedNetworks6Test, mergeNetworks) - new test src/lib/dhcpsrv/cfg_shared_networks.* CfgSharedNetworks4::merge() CfgSharedNetworks4::getAll() - moved to CfgSharedNetworks template class --- diff --git a/src/bin/dhcp6/tests/config_backend_unittest.cc b/src/bin/dhcp6/tests/config_backend_unittest.cc index 799de93b4f..b7108e522e 100644 --- a/src/bin/dhcp6/tests/config_backend_unittest.cc +++ b/src/bin/dhcp6/tests/config_backend_unittest.cc @@ -364,7 +364,7 @@ TEST_F(Dhcp6CBTest, mergeOptions) { // This test verifies that externally configured shared-networks are // merged correctly into staging configuration. -TEST_F(Dhcp6CBTest, DISABLED_mergeSharedNetworks) { +TEST_F(Dhcp6CBTest, mergeSharedNetworks) { string base_config = "{ \n" " \"interfaces-config\": { \n" diff --git a/src/lib/dhcpsrv/cfg_shared_networks.cc b/src/lib/dhcpsrv/cfg_shared_networks.cc index 7f5c48041f..75d7059e51 100644 --- a/src/lib/dhcpsrv/cfg_shared_networks.cc +++ b/src/lib/dhcpsrv/cfg_shared_networks.cc @@ -19,56 +19,5 @@ CfgSharedNetworks4::hasNetworkWithServerId(const IOAddress& server_id) const { return (network_it != index.cend()); } - - -void -CfgSharedNetworks4::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4& other) { - auto& index = networks_.get(); - - // Iterate over the subnets to be merged. They will replace the existing - // subnets with the same id. All new subnets will be inserted into this - // configuration. - auto other_networks = other.getAll(); - for (auto other_network = other_networks->begin(); - other_network != other_networks->end(); ++other_network) { - - // In theory we should drop subnet assignments from "other". The - // idea being those that come from the CB should not have subnets_ - // populated. We will quietly throw them away, just in case. - (*other_network)->delAll(); - - // Check if the other network exists in this config. - auto existing_network = index.find((*other_network)->getName()); - if (existing_network != index.end()) { - - // Somehow the same instance is in both, skip it. - if (*existing_network == *other_network) { - continue; - } - - // Network exists, which means we're updating it. - // First we need to move its subnets to the new - // version of the network. - const Subnet4Collection* subnets = (*existing_network)->getAllSubnets(); - - Subnet4Collection copy_subnets(*subnets); - for (auto subnet = copy_subnets.cbegin(); subnet != copy_subnets.cend(); ++subnet) { - (*existing_network)->del((*subnet)->getID()); - (*other_network)->add(*subnet); - } - - // Now we discard the existing copy of the network. - index.erase(existing_network); - } - - // Create the network's options based on the given definitions. - (*other_network)->getCfgOption()->createOptions(cfg_def); - - // Add the new/updated nework. - networks_.push_back(*other_network); - } - -} - } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcpsrv/cfg_shared_networks.h b/src/lib/dhcpsrv/cfg_shared_networks.h index f97b4dbd69..8072306a24 100644 --- a/src/lib/dhcpsrv/cfg_shared_networks.h +++ b/src/lib/dhcpsrv/cfg_shared_networks.h @@ -33,6 +33,10 @@ namespace dhcp { template class CfgSharedNetworks : public data::CfgToElement { public: + /// @brief Returns pointer to all configured shared networks. + const SharedNetworkCollection* getAll() const { + return (&networks_); + } /// @brief Adds new shared network to the configuration. /// @@ -126,30 +130,6 @@ public: return (list); } -protected: - - /// @brief Multi index container holding shared networks. - SharedNetworkCollection networks_; -}; - -/// @brief Represents configuration of IPv4 shared networks. -class CfgSharedNetworks4 : public CfgSharedNetworks { -public: - - /// @brief Returns pointer to all configured shared networks. - const SharedNetwork4Collection* getAll() const { - return (&networks_); - } - - /// @brief Checks if specified server identifier has been specified for - /// any network. - /// - /// @param server_id Server identifier. - /// - /// @return true if there is a network with a specified server identifier. - bool hasNetworkWithServerId(const asiolink::IOAddress& server_id) const; - /// @brief Merges specified shared network configuration into this /// configuration. /// @@ -179,7 +159,70 @@ public: /// when creating option instances. /// @param other the shared network configuration to be merged into this /// configuration. - void merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4& other); + void merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks& other) { + auto& index = networks_.template get(); + + // Iterate over the subnets to be merged. They will replace the existing + // subnets with the same id. All new subnets will be inserted into this + // configuration. + auto other_networks = other.getAll(); + for (auto other_network = other_networks->begin(); + other_network != other_networks->end(); ++other_network) { + + // In theory we should drop subnet assignments from "other". The + // idea being those that come from the CB should not have subnets_ + // populated. We will quietly throw them away, just in case. + (*other_network)->delAll(); + + // Check if the other network exists in this config. + auto existing_network = index.find((*other_network)->getName()); + if (existing_network != index.end()) { + + // Somehow the same instance is in both, skip it. + if (*existing_network == *other_network) { + continue; + } + + // Network exists, which means we're updating it. + // First we need to move its subnets to the new + // version of the network. + const auto subnets = (*existing_network)->getAllSubnets(); + + auto copy_subnets(*subnets); + for (auto subnet = copy_subnets.cbegin(); subnet != copy_subnets.cend(); ++subnet) { + (*existing_network)->del((*subnet)->getID()); + (*other_network)->add(*subnet); + } + + // Now we discard the existing copy of the network. + index.erase(existing_network); + } + + // Create the network's options based on the given definitions. + (*other_network)->getCfgOption()->createOptions(cfg_def); + + // Add the new/updated nework. + networks_.push_back(*other_network); + } + } + +protected: + + /// @brief Multi index container holding shared networks. + SharedNetworkCollection networks_; +}; + +/// @brief Represents configuration of IPv4 shared networks. +class CfgSharedNetworks4 : public CfgSharedNetworks { +public: + /// @brief Checks if specified server identifier has been specified for + /// any network. + /// + /// @param server_id Server identifier. + /// + /// @return true if there is a network with a specified server identifier. + bool hasNetworkWithServerId(const asiolink::IOAddress& server_id) const; }; /// @brief Pointer to the configuration of IPv4 shared networks. @@ -188,12 +231,6 @@ typedef boost::shared_ptr CfgSharedNetworks4Ptr; /// @brief Represents configuration of IPv6 shared networks. class CfgSharedNetworks6 : public CfgSharedNetworks { -public: - - /// @brief Returns pointer to all configured shared networks. - const SharedNetwork6Collection* getAll() const { - return (&networks_); - } }; /// @brief Pointer to the configuration of IPv6 shared networks. diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index e42ae4c78a..37b8a12bdb 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -161,6 +161,21 @@ SrvConfig::merge(ConfigBase& other) { ConfigBase::merge(other); try { SrvConfig& other_srv_config = dynamic_cast(other); + // We merge objects in order of dependency (real or theoretical). + // First we merge the common stuff. + + // Merge globals. + mergeGlobals(other_srv_config); + + // Merge option defs. We need to do this next so we + // pass these into subsequent merges so option instances + // at each level can be created based on the merged + // definitions. + cfg_option_def_->merge((*other_srv_config.getCfgOptionDef())); + + // Merge options. + cfg_option_->merge(cfg_option_def_, (*other_srv_config.getCfgOption())); + if (CfgMgr::instance().getFamily() == AF_INET) { merge4(other_srv_config); } else { @@ -175,20 +190,6 @@ SrvConfig::merge(ConfigBase& other) { void SrvConfig::merge4(SrvConfig& other) { - // We merge objects in order of dependency (real or theoretical). - - // Merge globals. - mergeGlobals(other); - - // Merge option defs. We need to do this next so we - // pass these into subsequent merges so option instances - // at each level can be created based on the merged - // definitions. - cfg_option_def_->merge((*other.getCfgOptionDef())); - - // Merge options. - cfg_option_->merge(cfg_option_def_, (*other.getCfgOption())); - // Merge shared networks. cfg_shared_networks4_->merge(cfg_option_def_, *(other.getCfgSharedNetworks4())); @@ -201,24 +202,10 @@ SrvConfig::merge4(SrvConfig& other) { void SrvConfig::merge6(SrvConfig& other) { - // We merge objects in order of dependency (real or theoretical). - - // Merge globals. - mergeGlobals(other); - - // Merge option defs. We need to do this next so we - // pass these into subsequent merges so option instances - // at each level can be created based on the merged - // definitions. - cfg_option_def_->merge((*other.getCfgOptionDef())); - - // Merge options. - cfg_option_->merge(cfg_option_def_, (*other.getCfgOption())); - -#if 0 // Merge shared networks. cfg_shared_networks6_->merge(cfg_option_def_, *(other.getCfgSharedNetworks6())); +#if 0 // Merge subnets. cfg_subnets6_->merge(cfg_option_def_, getCfgSharedNetworks4(), *(other.getCfgSubnets6())); diff --git a/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc b/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc index 1332fe2199..8e9ea86a82 100644 --- a/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc @@ -18,6 +18,11 @@ using namespace asiolink; namespace { +/// Attempts to verify an expected network within a collection of networks +/// @param networks set of networks in which to look +/// @param name name of the expected network +/// @param exp_valid expected valid lifetime of the network +/// @param exp_subnets list of subnet IDs the network is expected to own void checkMergedNetwork(const CfgSharedNetworks4& networks, const std::string& name, const Triplet& exp_valid, const std::vector& exp_subnets) { @@ -288,9 +293,6 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { OptionPtr option(new Option(Option::V4, 1)); option->setData(value.begin(), value.end()); ASSERT_NO_THROW(network1b->getCfgOption()->add(option, false, "isc")); - // Verify that our option is a generic option. - EXPECT_EQ("type=001, len=004: 59:61:79:21", option->toText()); - ASSERT_NO_THROW(network1b->add(subnet4)); // Network2 we will not touch. @@ -318,8 +320,9 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { // Make sure we have option 1 and that it has been replaced with a string option. auto network = cfg_to.getByName("network1"); auto desc = network->getCfgOption()->get("isc", 1); - ASSERT_TRUE(desc.option_); - EXPECT_EQ("type=001, len=004: \"Yay!\" (string)", desc.option_->toText()); + OptionStringPtr opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("Yay!", opstr->getValue()); // No changes to network2. ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet(200), diff --git a/src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc b/src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc index c561c93f02..5add7d9859 100644 --- a/src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -17,6 +18,25 @@ using namespace asiolink; namespace { +/// Attempts to verify an expected network within a collection of networks +/// @param networks set of networks in which to look +/// @param name name of the expected network +/// @param exp_valid expected valid lifetime of the network +/// @param exp_subnets list of subnet IDs the network is expected to own +void checkMergedNetwork(const CfgSharedNetworks6& networks, const std::string& name, + const Triplet& exp_valid, + const std::vector& exp_subnets) { + auto network = networks.getByName(name); + ASSERT_TRUE(network) << "expected network: " << name << " not found"; + ASSERT_EQ(exp_valid, network->getValid()) << " network valid lifetime wrong"; + const Subnet6Collection* subnets = network->getAllSubnets(); + ASSERT_EQ(exp_subnets.size(), subnets->size()) << " wrong number of subnets"; + for (auto exp_id : exp_subnets) { + ASSERT_TRUE(network->getSubnet(exp_id)) + << " did not find expected subnet: " << exp_id; + } +} + // This test verifies that shared networks can be added to the configruation // and retrieved by name. TEST(CfgSharedNetworks6Test, getByName) { @@ -208,4 +228,106 @@ TEST(CfgSharedNetworks6Test, unparse) { test::runToElementTest(expected, cfg); } +// This test verifies that shared-network configurations are properly merged. +TEST(CfgSharedNetworks6Test, mergeNetworks) { + // Create custom options dictionary for testing merge. We're keeping it + // simple because they are more rigorous tests elsewhere. + CfgOptionDefPtr cfg_def(new CfgOptionDef()); + cfg_def->add((OptionDefinitionPtr(new OptionDefinition("one", 1, "string"))), "isc"); + + Subnet6Ptr subnet1(new Subnet6(IOAddress("2001:1::"), + 64, 60, 80, 100, 200, SubnetID(1))); + Subnet6Ptr subnet2(new Subnet6(IOAddress("2001:2::"), + 64, 60, 80, 100, 200, SubnetID(2))); + Subnet6Ptr subnet3(new Subnet6(IOAddress("2001:3::"), + 64, 60, 80, 100, 200, SubnetID(3))); + Subnet6Ptr subnet4(new Subnet6(IOAddress("2001:4::"), + 64, 60, 80, 100, 200, SubnetID(4))); + + // Create network1 and add two subnets to it + SharedNetwork6Ptr network1(new SharedNetwork6("network1")); + network1->setValid(Triplet(100)); + ASSERT_NO_THROW(network1->add(subnet1)); + ASSERT_NO_THROW(network1->add(subnet2)); + + // Create network2 with no subnets. + SharedNetwork6Ptr network2(new SharedNetwork6("network2")); + network2->setValid(Triplet(200)); + + // Create network3 with one subnet. + SharedNetwork6Ptr network3(new SharedNetwork6("network3")); + network3->setValid(Triplet(300)); + ASSERT_NO_THROW(network3->add(subnet3)); + + // Create our "existing" configured networks. + // Add all three networks to the existing config. + CfgSharedNetworks6 cfg_to; + ASSERT_NO_THROW(cfg_to.add(network1)); + ASSERT_NO_THROW(cfg_to.add(network2)); + ASSERT_NO_THROW(cfg_to.add(network3)); + + // Merge in an "empty" config. Should have the original config, still intact. + CfgSharedNetworks6 cfg_from; + ASSERT_NO_THROW(cfg_to.merge(cfg_def, cfg_from)); + + ASSERT_EQ(3, cfg_to.getAll()->size()); + ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network1", Triplet(100), + std::vector{SubnetID(1), SubnetID(2)})); + ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet(200), + std::vector())); + + ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network3", Triplet(300), + std::vector{SubnetID(3)})); + + // Create network1b, this is an "update" of network1 + // We'll double the valid time and add subnet4 to it + SharedNetwork6Ptr network1b(new SharedNetwork6("network1")); + network1b->setValid(Triplet(200)); + + // Now let's a add generic option 1 to network1b. + std::string value("Yay!"); + OptionPtr option(new Option(Option::V6, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(network1b->getCfgOption()->add(option, false, "isc")); + ASSERT_NO_THROW(network1b->add(subnet4)); + + // Network2 we will not touch. + + // Create network3b, this is an "update" of network3. + // We'll double it's valid time, but leave off the subnet. + SharedNetwork6Ptr network3b(new SharedNetwork6("network3")); + network3b->setValid(Triplet(600)); + + // Create our "existing" configured networks. + ASSERT_NO_THROW(cfg_from.add(network1b)); + ASSERT_NO_THROW(cfg_from.add(network3b)); + + ASSERT_NO_THROW(cfg_to.merge(cfg_def, cfg_from)); + + // Should still have 3 networks. + + // Network1 should have doubled its valid lifetime but still only have + // the orignal two subnets. Merge should discard assocations on CB + // subnets and preserve the associations from existing config. + ASSERT_EQ(3, cfg_to.getAll()->size()); + ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network1", Triplet(200), + std::vector{SubnetID(1), SubnetID(2)})); + + // Make sure we have option 1 and that it has been replaced with a string option. + auto network = cfg_to.getByName("network1"); + auto desc = network->getCfgOption()->get("isc", 1); + ASSERT_TRUE(desc.option_); + OptionStringPtr opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("Yay!", opstr->getValue()); + + // No changes to network2. + ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet(200), + std::vector())); + + // Network1 should have doubled its valid lifetime and still subnet3. + ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network3", Triplet(600), + std::vector{SubnetID(3)})); +} + } // end of anonymous namespace