From: Thomas Markwalder Date: Fri, 15 Mar 2019 12:26:35 +0000 (-0400) Subject: [#401,!254] Options are now created when merging subnets X-Git-Tag: Kea-1.6.0-beta~362^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5406df22fc2807a98372aa585771900232c6deed;p=thirdparty%2Fkea.git [#401,!254] Options are now created when merging subnets src/lib/dhcpsrv/cfg_subnets4.* CfgSubnets4::merge() - added cfg_def, now creates options for each subnet and their pools. src/lib/dhcpsrv/srv_config.cc SrvConfig::merge4() - passes merged option defs into subnets merge. src/lib/dhcpsrv/subnet.h Subnet::getPoolsWritable() - changed to public. --- diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index 99e64893db..fd86f6dd76 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -56,7 +56,7 @@ CfgSubnets4::del(const ConstSubnet4Ptr& subnet) { } void -CfgSubnets4::merge(CfgSharedNetworks4Ptr networks, +CfgSubnets4::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4Ptr networks, CfgSubnets4& other) { auto& index = subnets_.get(); @@ -94,6 +94,12 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks, index.erase(subnet_it); } + // Create the subnet's options based on the given definitions. + (*other_subnet)->getCfgOption()->createOptions(cfg_def); + for (auto pool : (*other_subnet)->getPoolsWritable(Lease::TYPE_V4)) { + pool->getCfgOption()->createOptions(cfg_def); + } + // Add the "other" subnet to the our collection of subnets. subnets_.push_back(*other_subnet); diff --git a/src/lib/dhcpsrv/cfg_subnets4.h b/src/lib/dhcpsrv/cfg_subnets4.h index 79fa3664d3..27890f6a28 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.h +++ b/src/lib/dhcpsrv/cfg_subnets4.h @@ -70,6 +70,8 @@ public: /// -# If it belongs to a shared network, remove it from that network /// -# Remove the subnet from this configuration and discard it /// + /// - Create the subnet's option instance, as well as any options + /// that belong to any of the subnet's pools. /// - Add the subnet from @c other to this configuration. /// - If that subnet is associated to shared network, find that network /// in @ networks and add that subnet to it. @@ -80,12 +82,15 @@ public: /// not be modified after the call to @c merge because it may affect the /// merged configuration. /// + /// @param cfg_def set of of user-defined option definitions to use + /// when creating option instances. /// @param networks collection of shared networks that to which assignments /// should be added. In other words, the list of shared networks that belong /// to the same SrvConfig instance we are merging into. /// @param other the subnet configuration to be merged into this /// configuration. - void merge(CfgSharedNetworks4Ptr networks, CfgSubnets4& other); + void merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4Ptr networks, + CfgSubnets4& other); /// @brief Returns pointer to the collection of all IPv4 subnets. /// diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 7a3bf88501..07137f3feb 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -195,7 +195,8 @@ SrvConfig::merge4(SrvConfig& other) { cfg_shared_networks4_->merge(cfg_option_def_, *(other.getCfgSharedNetworks4())); // Merge subnets. - cfg_subnets4_->merge(getCfgSharedNetworks4(), *(other.getCfgSubnets4())); + cfg_subnets4_->merge(cfg_option_def_, getCfgSharedNetworks4(), + *(other.getCfgSubnets4())); /// @todo merge other parts of the configuration here. } diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 31d6680ca7..c2c16ff28e 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -289,7 +289,6 @@ public: shared_network_name_ = shared_network_name; } -protected: /// @brief Returns all pools (non-const variant) /// /// The reference is only valid as long as the object that returned it. @@ -298,6 +297,8 @@ protected: /// @return a collection of all pools PoolCollection& getPoolsWritable(Lease::Type type); +protected: + /// @brief Protected constructor // /// By making the constructor protected, we make sure that no one will diff --git a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc index c5206458ae..3921dd4851 100644 --- a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc @@ -38,11 +38,11 @@ void checkMergedSubnet(CfgSubnets4& cfg_subnets, const std::string& prefix, int exp_valid, SharedNetwork4Ptr exp_network) { - - // The subnet1 should be replaced by subnet4 but the shared network - // should not be affected. + // Look for the network by prefix. auto subnet = cfg_subnets.getByPrefix(prefix); ASSERT_TRUE(subnet) << "subnet: " << prefix << " not found"; + + // Make sure we have the one we expect. ASSERT_EQ(exp_subnet_id, subnet->getID()) << "subnet ID is wrong"; ASSERT_EQ(exp_valid, subnet->getValid()) << "subnet valid time is wrong"; @@ -139,6 +139,11 @@ TEST(CfgSubnets4Test, deleteSubnet) { // This test verifies that subnets configuration is properly merged. TEST(CfgSubnets4Test, mergeSubnets) { + // 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"); + Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.1.0"), 26, 1, 2, 100, SubnetID(1))); Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.0"), @@ -148,7 +153,6 @@ TEST(CfgSubnets4Test, mergeSubnets) { Subnet4Ptr subnet4(new Subnet4(IOAddress("192.0.4.0"), 26, 1, 2, 100, SubnetID(4))); - // Create the "existing" list of shared networks CfgSharedNetworks4Ptr networks(new CfgSharedNetworks4()); SharedNetwork4Ptr shared_network1(new SharedNetwork4("shared-network1")); @@ -171,16 +175,14 @@ TEST(CfgSubnets4Test, mergeSubnets) { ASSERT_NO_THROW(cfg_to.add(subnet3)); ASSERT_NO_THROW(cfg_to.add(subnet4)); - // Merge in an "empty" config. Should have the original config, // still intact. CfgSubnets4 cfg_from; - ASSERT_NO_THROW(cfg_to.merge(networks, cfg_from)); + ASSERT_NO_THROW(cfg_to.merge(cfg_def, networks, cfg_from)); // We should have all four subnets, with no changes. ASSERT_EQ(4, cfg_to.getAll()->size()); - // Should be no changes to the configuration. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(1), "192.0.1.0/26", 100, shared_network1)); @@ -197,20 +199,55 @@ TEST(CfgSubnets4Test, mergeSubnets) { 26, 2, 3, 400, SubnetID(1))); subnet1b->setSharedNetworkName("shared-network1"); + // Add generic option 1 to subnet 1b. + std::string value("Yay!"); + OptionPtr option(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(subnet1b->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()); + // subnet 3b updates subnet 3 and removes it from network 2 Subnet4Ptr subnet3b(new Subnet4(IOAddress("192.0.3.0"), 26, 3, 4, 500, SubnetID(3))); + // Now Add generic option 1 to subnet 3b. + value = "Team!"; + option.reset(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(subnet3b->getCfgOption()->add(option, false, "isc")); + // Verify that our option is a generic option. + EXPECT_EQ("type=001, len=005: 54:65:61:6d:21", option->toText()); + // subnet 4b updates subnet 4 and moves it from network2 to network 1 Subnet4Ptr subnet4b(new Subnet4(IOAddress("192.0.4.0"), 26, 3, 4, 500, SubnetID(4))); subnet4b->setSharedNetworkName("shared-network1"); // subnet 5 is new and belongs to network 2 + // Has two pools both with an option 1 Subnet4Ptr subnet5(new Subnet4(IOAddress("192.0.5.0"), 26, 1, 2, 300, SubnetID(5))); subnet5->setSharedNetworkName("shared-network2"); + // Add pool 1 + Pool4Ptr pool(new Pool4(IOAddress("192.0.5.10"), IOAddress("192.0.5.20"))); + value = "POOLS"; + option.reset(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(pool->getCfgOption()->add(option, false, "isc")); + EXPECT_EQ(option->toText(), "type=001, len=005: 50:4f:4f:4c:53"); + subnet5->addPool(pool); + + // Add pool 2 + pool.reset(new Pool4(IOAddress("192.0.5.30"), IOAddress("192.0.5.40"))); + value ="RULE!"; + option.reset(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(pool->getCfgOption()->add(option, false, "isc")); + EXPECT_EQ(option->toText(), "type=001, len=005: 52:55:4c:45:21"); + subnet5->addPool(pool); + // Add subnets to the merge from config. ASSERT_NO_THROW(cfg_from.add(subnet1b)); ASSERT_NO_THROW(cfg_from.add(subnet3b)); @@ -218,13 +255,19 @@ TEST(CfgSubnets4Test, mergeSubnets) { ASSERT_NO_THROW(cfg_from.add(subnet5)); // Merge again. - ASSERT_NO_THROW(cfg_to.merge(networks, cfg_from)); + ASSERT_NO_THROW(cfg_to.merge(cfg_def, networks, cfg_from)); ASSERT_EQ(5, cfg_to.getAll()->size()); // The subnet1 should be replaced by subnet1b. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(1), "192.0.1.0/26", 400, shared_network1)); + // Let's verify that our option is there and populated correctly. + auto subnet = cfg_to.getByPrefix("192.0.1.0/26"); + auto desc = subnet->getCfgOption()->get("isc", 1); + ASSERT_TRUE(desc.option_); + EXPECT_EQ(desc.option_->toText(), "type=001, len=004: \"Yay!\" (string)"); + // The subnet2 should not be affected because it was not present. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(2), "192.0.2.0/26", 100, shared_network2)); @@ -232,6 +275,11 @@ TEST(CfgSubnets4Test, mergeSubnets) { // subnet3 should be replaced by subnet3b and no longer assigned to a network. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(3), "192.0.3.0/26", 500, no_network)); + // Let's verify that our option is there and populated correctly. + subnet = cfg_to.getByPrefix("192.0.3.0/26"); + desc = subnet->getCfgOption()->get("isc", 1); + ASSERT_TRUE(desc.option_); + EXPECT_EQ(desc.option_->toText(), "type=001, len=005: \"Team!\" (string)"); // subnet4 should be replaced by subnet4b and moved to network1. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(4), @@ -240,6 +288,20 @@ TEST(CfgSubnets4Test, mergeSubnets) { // subnet5 should have been added to configuration. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(5), "192.0.5.0/26", 300, shared_network2)); + + // Let's verify that both pools have the proper options. + subnet = cfg_to.getByPrefix("192.0.5.0/26"); + const PoolPtr merged_pool = subnet->getPool(Lease::TYPE_V4, IOAddress("192.0.5.10")); + ASSERT_TRUE(merged_pool); + desc = merged_pool->getCfgOption()->get("isc", 1); + ASSERT_TRUE(desc.option_); + EXPECT_EQ(desc.option_->toText(), "type=001, len=005: \"POOLS\" (string)"); + + const PoolPtr merged_pool2 = subnet->getPool(Lease::TYPE_V4, IOAddress("192.0.5.30")); + ASSERT_TRUE(merged_pool2); + desc = merged_pool2->getCfgOption()->get("isc", 1); + ASSERT_TRUE(desc.option_); + EXPECT_EQ(desc.option_->toText(), "type=001, len=005: \"RULE!\" (string)"); } // This test verifies that it is possible to retrieve a subnet using an