[#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 99e64893db9a4a23b82093a9e15d15a2118da6e6..fd86f6dd76fbfaa356058e92801e492e3ffa490e 100644 (file)
--- 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<SubnetSubnetIdIndexTag>();
 
@@ -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 79fa3664d394b120635200d48a4e027238af9c1a..27890f6a28864b9a9b0f684b8f875c40ba453364 100644 (file)
--- 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 7a3bf88501a311b3640bfd6624b72007cc96fd4a..07137f3feb4c24f3dc8f5090070e1d740eb24100 100644 (file)
--- 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 31d6680ca75a72d38ca4dd33659e3b3902987bab..c2c16ff28e865e584ff72cce9999ae882e616b20 100644 (file)
--- 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 c5206458ae1a092863559e66b5e8182c29d6030d..3921dd4851687013bad4cf499bb02afaf199f887 100644 (file)
--- 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