From: Marcin Siodelski <marcin@isc.org>
Date: Fri, 24 May 2019 14:53:35 +0000 (+0200)
Subject: [#492,!252] Applied the subnet id merge fix into v6.
X-Git-Tag: Kea-1.6.0-beta~69
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=72bd8d8695111ec8db1a28f683c9501c7c16129c;p=thirdparty%2Fkea.git

[#492,!252] Applied the subnet id merge fix into v6.
---

diff --git a/src/lib/dhcpsrv/cfg_subnets6.cc b/src/lib/dhcpsrv/cfg_subnets6.cc
index d30b212fe5..17ce409f42 100644
--- a/src/lib/dhcpsrv/cfg_subnets6.cc
+++ b/src/lib/dhcpsrv/cfg_subnets6.cc
@@ -87,7 +87,8 @@ CfgSubnets6::del(const SubnetID& subnet_id) {
 void
 CfgSubnets6::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks6Ptr networks,
                    CfgSubnets6& other) {
-    auto& index = subnets_.get<SubnetSubnetIdIndexTag>();
+    auto& index_id = subnets_.get<SubnetSubnetIdIndexTag>();
+    auto& index_prefix = subnets_.get<SubnetPrefixIndexTag>();
 
     // Iterate over the subnets to be merged. They will replace the existing
     // subnets with the same id. All new subnets will be inserted into the
@@ -98,8 +99,8 @@ CfgSubnets6::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks6Ptr networks,
          ++other_subnet) {
 
         // Check if there is a subnet with the same ID.
-        auto subnet_it = index.find((*other_subnet)->getID());
-        if (subnet_it != index.end()) {
+        auto subnet_it = index_id.find((*other_subnet)->getID());
+        if (subnet_it != index_id.end()) {
 
             // Subnet found.
             auto existing_subnet = *subnet_it;
@@ -120,7 +121,31 @@ CfgSubnets6::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks6Ptr networks,
             }
 
             // Now we remove the existing subnet.
-            index.erase(subnet_it);
+            index_id.erase(subnet_it);
+        }
+
+        // Check if there is a subnet with the same prefix.
+        auto subnet_prefix_it = index_prefix.find((*other_subnet)->toText());
+        if (subnet_prefix_it != index_prefix.end()) {
+
+            // Subnet found.
+            auto existing_subnet = *subnet_prefix_it;
+
+            // Updating the id can lead to problems... e.g. reservation
+            // for the previous subnet ID.
+            // @todo: check reservations
+
+            // We're going to replace the existing subnet with the other
+            // version. If it belongs to a shared network, we need
+            // remove it from that network.
+            SharedNetwork6Ptr network;
+            existing_subnet->getSharedNetwork(network);
+            if (network) {
+                network->del(existing_subnet->getID());
+            }
+
+            // Now we remove the existing subnet.
+            index_prefix.erase(subnet_prefix_it);
         }
 
         // Create the subnet's options based on the given definitions.
diff --git a/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc b/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc
index 118b734f12..c37a79f899 100644
--- a/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc
@@ -873,8 +873,8 @@ TEST(CfgSubnets6Test, mergeSubnets) {
                                               SubnetID(4), 100, shared_network2));
 
     // Fill cfg_from configuration with subnets.
-    // subnet 1b updates subnet 1 but leaves it in network 1
-    Subnet6Ptr subnet1b(new Subnet6(IOAddress("2001:1::"),
+    // subnet 1b updates subnet 1 but leaves it in network 1 with the same ID.
+    Subnet6Ptr subnet1b(new Subnet6(IOAddress("2001:10::"),
                                    64, 2, 3, 100, 400, SubnetID(1)));
     subnet1b->setSharedNetworkName("shared-network1");
 
@@ -884,9 +884,10 @@ TEST(CfgSubnets6Test, mergeSubnets) {
     option->setData(value.begin(), value.end());
     ASSERT_NO_THROW(subnet1b->getCfgOption()->add(option, false, "isc"));
 
-    // subnet 3b updates subnet 3 and removes it from network 2
+    // subnet 3b updates subnet 3 with different UD and removes it
+    // from network 2
     Subnet6Ptr subnet3b(new Subnet6(IOAddress("2001:3::"),
-                                   64, 3, 4, 100, 500, SubnetID(3)));
+                                   64, 3, 4, 100, 500, SubnetID(30)));
 
     // Now Add generic option 1 to subnet 3b.
     value = "Team!";
@@ -932,11 +933,11 @@ TEST(CfgSubnets6Test, mergeSubnets) {
     ASSERT_EQ(5, cfg_to.getAll()->size());
 
     // The subnet1 should be replaced by subnet1b.
-    ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, "2001:1::/64",
+    ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, "2001:10::/64",
                                               SubnetID(1), 400, shared_network1));
 
     // Let's verify that our option is there and populated correctly.
-    auto subnet = cfg_to.getByPrefix("2001:1::/64");
+    auto subnet = cfg_to.getByPrefix("2001:10::/64");
     auto desc = subnet->getCfgOption()->get("isc", 1);
     ASSERT_TRUE(desc.option_);
     OptionStringPtr opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
@@ -949,7 +950,7 @@ TEST(CfgSubnets6Test, mergeSubnets) {
 
     // subnet3 should be replaced by subnet3b and no longer assigned to a network.
     ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, "2001:3::/64",
-                                              SubnetID(3), 500, no_network));
+                                              SubnetID(30), 500, no_network));
     // Let's verify that our option is there and populated correctly.
     subnet = cfg_to.getByPrefix("2001:3::/64");
     desc = subnet->getCfgOption()->get("isc", 1);