]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#3485] Backported fix #3481 to Kea 2.6.1
authorMarcin Siodelski <marcin@isc.org>
Tue, 16 Jul 2024 16:22:44 +0000 (18:22 +0200)
committerMarcin Siodelski <marcin@isc.org>
Thu, 18 Jul 2024 13:54:51 +0000 (15:54 +0200)
The fix removes an issue with not returning some suboptions specified in
the Kea config backend to the DHCP clients.

16 files changed:
src/lib/dhcpsrv/cb_ctl_dhcp4.cc
src/lib/dhcpsrv/cb_ctl_dhcp6.cc
src/lib/dhcpsrv/cfg_option.cc
src/lib/dhcpsrv/cfg_shared_networks.h
src/lib/dhcpsrv/cfg_subnets4.cc
src/lib/dhcpsrv/cfg_subnets6.cc
src/lib/dhcpsrv/cfgmgr.cc
src/lib/dhcpsrv/client_class_def.cc
src/lib/dhcpsrv/client_class_def.h
src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc
src/lib/dhcpsrv/tests/cfg_option_unittest.cc
src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc
src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc
src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc
src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc
src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

index 211681bce4b9773a22ab388ea10fb472aae8f86f..741f122acae66b7be890ca51891b5cc544164177 100644 (file)
@@ -215,6 +215,7 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector,
 
         // Class options also need to be created when returned from the config backend.
         client_classes.createOptions(external_cfg->getCfgOptionDef());
+        client_classes.encapsulateOptions();
 
         external_cfg->setClientClassDictionary(boost::make_shared<ClientClassDictionary>(client_classes));
     }
index 1ed48208100ca86cdb3213af318170910b2358b1..577b23ceee7ac6cf58bf9f29c35a24caa22a7541 100644 (file)
@@ -214,6 +214,7 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector
 
         // Class options also need to be created when returned from the config backend.
         client_classes.createOptions(external_cfg->getCfgOptionDef());
+        client_classes.encapsulateOptions();
 
         external_cfg->setClientClassDictionary(boost::make_shared<ClientClassDictionary>(client_classes));
     }
index 27ae10b14e234d3ed1ca9e27b82063a668803ed6..61137ed5eb8d5ccf51c2de8cdc9c360317b102b7 100644 (file)
@@ -256,6 +256,7 @@ CfgOption::copyTo(CfgOption& other) const {
     other.options_.clearItems();
     other.vendor_options_.clearItems();
     mergeTo(other);
+    other.encapsulated_ = isEncapsulated();
 }
 
 void
index 7123426e8c74ac8486114aa73a905487f5435b87..7a0a57148ffce8da5a6b52686f3d8ff9e79157d6 100644 (file)
@@ -200,7 +200,11 @@ public:
             // Create the network's options based on the given definitions.
             other_network->getCfgOption()->createOptions(cfg_def);
 
-            // Add the new/updated nework.
+            // Encapsulate options, so that the DHCP server can effectively return
+            // them to the clients without having to encapsulate them for each request.
+            other_network->getCfgOption()->encapsulate();
+
+            // Add the new/updated network.
             static_cast<void>(networks_.push_back(other_network));
         }
     }
index 681f4c36f06283c607d46270bfe4d08cbff92eaf..d462fca501a4970f34b26c265f880ba3539209fc 100644 (file)
@@ -154,9 +154,14 @@ CfgSubnets4::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4Ptr networks,
         // Create the subnet's options based on the given definitions.
         other_subnet->getCfgOption()->createOptions(cfg_def);
 
+        // Encapsulate options, so that the DHCP server can effectively return
+        // them to the clients without having to encapsulate them for each request.
+        other_subnet->getCfgOption()->encapsulate();
+
         // Create the options for pool based on the given definitions.
         for (auto const& pool : other_subnet->getPoolsWritable(Lease::TYPE_V4)) {
             pool->getCfgOption()->createOptions(cfg_def);
+            pool->getCfgOption()->encapsulate();
         }
 
         // Add the "other" subnet to the our collection of subnets.
index 704cd42cb92f7ba0e6f75f565d672fc0c3657739..faf20bbd803bd2cbdcfc253dd9ac635537ec68d3 100644 (file)
@@ -151,14 +151,20 @@ CfgSubnets6::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks6Ptr networks,
         // Create the subnet's options based on the given definitions.
         other_subnet->getCfgOption()->createOptions(cfg_def);
 
+        // Encapsulate options, so that the DHCP server can effectively return
+        // them to the clients without having to encapsulate them for each request.
+        other_subnet->getCfgOption()->encapsulate();
+
         // Create the options for pool based on the given definitions.
         for (auto const& pool : other_subnet->getPoolsWritable(Lease::TYPE_NA)) {
             pool->getCfgOption()->createOptions(cfg_def);
+            pool->getCfgOption()->encapsulate();
         }
 
         // Create the options for pd pool based on the given definitions.
         for (auto const& pool : other_subnet->getPoolsWritable(Lease::TYPE_PD)) {
             pool->getCfgOption()->createOptions(cfg_def);
+            pool->getCfgOption()->encapsulate();
         }
 
         // Add the "other" subnet to the our collection of subnets.
index c457fb7bdecfdfdc596a68e6c32c6deca08fadfd..ef156dd22c7c0b886a7ad01e274a787a37e8f91e 100644 (file)
@@ -197,6 +197,7 @@ CfgMgr::mergeIntoCurrentCfg(const uint32_t seq) {
         // First we need to remove statistics.
         getCurrentCfg()->removeStatistics();
         mergeIntoCfg(getCurrentCfg(), seq);
+        LibDHCP::setRuntimeOptionDefs(getCurrentCfg()->getCfgOptionDef()->getContainer());
 
     } catch (...) {
         // Make sure the statistics is updated even if the merge failed.
index d9b69f14b80a393295f481358a01422b22bc542c..166d66e4b36b693f1e47dace6028b10b4961c309 100644 (file)
@@ -553,6 +553,16 @@ ClientClassDictionary::createOptions(const CfgOptionDefPtr& external_defs) {
     }
 }
 
+void
+ClientClassDictionary::encapsulateOptions() const {
+    for (auto const& c : *list_) {
+        CfgOptionPtr class_options = c->getCfgOption();
+        if (class_options) {
+            class_options->encapsulate();
+        }
+    }
+}
+
 ElementPtr
 ClientClassDictionary::toElement() const {
     ElementPtr result = Element::createList();
index da5a452e562891e3a284a567ab1e95489f32f862..c9158e1107eb0da708cb3319557b4147af6ea31c 100644 (file)
@@ -471,6 +471,10 @@ public:
     /// @param cfg_option_def set of option definitions to use.
     void createOptions(const CfgOptionDefPtr& cfg_option_def);
 
+    /// @brief Iterates over the classes in the dictionary and encapsulates
+    /// suboptions.
+    void encapsulateOptions() const;
+
     /// @brief Equality operator.
     ///
     /// @param other Other client class dictionary to compare to.
index ec0e7b9ef0e719aa4141a438d2f416baf643f858..3cdf63357778f90e284fc1ac891083f3a29387c3 100644 (file)
@@ -587,6 +587,7 @@ public:
             (getTimestamp("dhcp4_options") > lb_modification_time)) {
             ASSERT_TRUE(found_opt.option_);
             EXPECT_EQ("new.example.com", found_opt.option_->toString());
+            EXPECT_TRUE(options->isEncapsulated());
 
         } else {
             EXPECT_FALSE(found_opt.option_);
@@ -601,6 +602,7 @@ public:
             (getTimestamp("dhcp4_shared_network") > lb_modification_time)) {
             ASSERT_TRUE(found_network);
             EXPECT_TRUE(found_network->hasFetchGlobalsFn());
+            EXPECT_TRUE(found_network->getCfgOption()->isEncapsulated());
 
         } else {
             EXPECT_FALSE(found_network);
@@ -616,6 +618,7 @@ public:
             EXPECT_TRUE(found_subnet->hasFetchGlobalsFn());
             EXPECT_TRUE(boost::dynamic_pointer_cast<RandomAllocator>
                         (found_subnet->getAllocator(Lease::TYPE_V4)));
+            EXPECT_TRUE(found_subnet->getCfgOption()->isEncapsulated());
 
         } else {
             EXPECT_FALSE(found_subnet);
@@ -647,6 +650,9 @@ public:
             EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(),
                                    option_desc.formatted_value_.end()),
                       option->getData());
+
+            // Make sure that the options have been encapsulated.
+            EXPECT_TRUE(cfg_option_desc->isEncapsulated());
         } else {
             EXPECT_FALSE(found_class);
         }
@@ -1500,6 +1506,7 @@ public:
             (getTimestamp("dhcp6_options") > lb_modification_time)) {
             ASSERT_TRUE(found_opt.option_);
             EXPECT_EQ("some.bootfile", found_opt.option_->toString());
+            EXPECT_TRUE(options->isEncapsulated());
 
         } else {
             EXPECT_FALSE(found_opt.option_);
@@ -1514,6 +1521,7 @@ public:
             (getTimestamp("dhcp6_shared_network") > lb_modification_time)) {
             ASSERT_TRUE(found_network);
             EXPECT_TRUE(found_network->hasFetchGlobalsFn());
+            EXPECT_TRUE(found_network->getCfgOption()->isEncapsulated());
 
         } else {
             EXPECT_FALSE(found_network);
@@ -1531,6 +1539,7 @@ public:
             EXPECT_TRUE(boost::dynamic_pointer_cast<RandomAllocator>
                         (found_subnet->getAllocator(Lease::TYPE_PD)));
             EXPECT_TRUE(found_subnet->hasFetchGlobalsFn());
+            EXPECT_TRUE(found_subnet->getCfgOption()->isEncapsulated());
 
         } else {
             EXPECT_FALSE(found_subnet);
@@ -1554,6 +1563,8 @@ public:
             EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(),
                                    option_desc.formatted_value_.end()),
                       option->getData());
+            EXPECT_TRUE(found_class->getCfgOption()->isEncapsulated());
+
         } else {
             EXPECT_FALSE(found_class);
         }
index 573899b7ab6e815dc81967d95208f1e4224aaaf1..0fabbec3d1670755e0712905fd53e5f8c9faf8fc 100644 (file)
@@ -474,6 +474,14 @@ TEST_F(CfgOptionTest, copy) {
     container = cfg_dst.getAll("foo");
     ASSERT_TRUE(container);
     EXPECT_EQ(10, container->size());
+
+    // Source config wasn't encapsulated, so the destination shouldn't be too.
+    EXPECT_FALSE(cfg_dst.isEncapsulated());
+
+    // Now let's make sure that the encapsulation flag is correctly set.
+    cfg_src.encapsulate();
+    ASSERT_NO_THROW(cfg_src.copyTo(cfg_dst));
+    EXPECT_TRUE(cfg_dst.isEncapsulated());
 }
 
 // This test verifies that DHCP options from one configuration
index bf0ba21bb307d73e3c708b704c0e36349f2c7dfc..1ed822d7a4713da490bcd0a11c43a2343582775b 100644 (file)
@@ -370,6 +370,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) {
     OptionStringPtr opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
     ASSERT_TRUE(opstr);
     EXPECT_EQ("Yay!", opstr->getValue());
+    EXPECT_TRUE(network->getCfgOption()->isEncapsulated());
 
     // No changes to network2.
     ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet<uint32_t>(200),
index 065829932b3b6d0b3e8775c90b0747f434aafa9f..ac684dd1f6e887a41f9470769efe3264ffee4e59 100644 (file)
@@ -378,6 +378,7 @@ TEST(CfgSharedNetworks6Test, mergeNetworks) {
     OptionStringPtr opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
     ASSERT_TRUE(opstr);
     EXPECT_EQ("Yay!", opstr->getValue());
+    EXPECT_TRUE(network->getCfgOption()->isEncapsulated());
 
     // No changes to network2.
     ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet<uint32_t>(200),
index 2e1540715921d0172d8d99d6c32f951e76f7b4b3..8f6e563be95ba41e5645ffca7193e0eb74af8ee9 100644 (file)
@@ -390,6 +390,7 @@ TEST(CfgSubnets4Test, mergeSubnets) {
     opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
     ASSERT_TRUE(opstr);
     EXPECT_EQ("Team!", opstr->getValue());
+    EXPECT_TRUE(subnet->getCfgOption()->isEncapsulated());
 
     // subnet4 should be replaced by subnet4b and moved to network1.
     ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, "192.0.4.0/26",
@@ -407,6 +408,7 @@ TEST(CfgSubnets4Test, mergeSubnets) {
     opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
     ASSERT_TRUE(opstr);
     EXPECT_EQ("POOLS", opstr->getValue());
+    EXPECT_TRUE(merged_pool->getCfgOption()->isEncapsulated());
 
     const PoolPtr merged_pool2 = subnet->getPool(Lease::TYPE_V4, IOAddress("192.0.5.30"));
     ASSERT_TRUE(merged_pool2);
@@ -414,6 +416,7 @@ TEST(CfgSubnets4Test, mergeSubnets) {
     opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
     ASSERT_TRUE(opstr);
     EXPECT_EQ("RULE!", opstr->getValue());
+    EXPECT_TRUE(merged_pool2->getCfgOption()->isEncapsulated());
 }
 
 // This test verifies that it is possible to retrieve a subnet using an
index 38f8b255e02f8b4e18323ecbe26918c276303de6..fdbfa124cea909de62ee5a97eb1611af43a1f0bc 100644 (file)
@@ -20,6 +20,7 @@
 #include <dhcpsrv/parsers/dhcp_parsers.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/subnet_id.h>
+
 #include <dhcpsrv/subnet_selector.h>
 #include <dhcpsrv/cfg_hosts.h>
 #include <stats/stats_mgr.h>
@@ -1076,6 +1077,7 @@ TEST(CfgSubnets6Test, mergeSubnets) {
     opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
     ASSERT_TRUE(opstr);
     EXPECT_EQ("Team!", opstr->getValue());
+    EXPECT_TRUE(subnet->getCfgOption()->isEncapsulated());
 
     // subnet4 should be replaced by subnet4b and moved to network1.
     ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, "2001:4::/64",
@@ -1093,6 +1095,7 @@ TEST(CfgSubnets6Test, mergeSubnets) {
     opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
     ASSERT_TRUE(opstr);
     EXPECT_EQ("POOLS", opstr->getValue());
+    EXPECT_TRUE(merged_pool->getCfgOption()->isEncapsulated());
 
     const PoolPtr merged_pool2 = subnet->getPool(Lease::TYPE_PD, IOAddress("2001:1::"));
     ASSERT_TRUE(merged_pool2);
@@ -1100,6 +1103,7 @@ TEST(CfgSubnets6Test, mergeSubnets) {
     opstr = boost::dynamic_pointer_cast<OptionString>(desc.option_);
     ASSERT_TRUE(opstr);
     EXPECT_EQ("RULE!", opstr->getValue());
+    EXPECT_TRUE(merged_pool2->getCfgOption()->isEncapsulated());
 }
 
 // This test verifies the Subnet6 parser's validation logic for id parameter.
index 1be8c6845624e615122ddc7e5d2e7f439f8411ef..13ec6a1147862c54cd9d2882c044abaad3fc4cd4 100644 (file)
@@ -8,6 +8,7 @@
 
 #include <exceptions/exceptions.h>
 #include <dhcp/dhcp6.h>
+#include <dhcp/libdhcp++.h>
 #include <dhcp/testutils/iface_mgr_test_config.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
@@ -1054,6 +1055,13 @@ TEST_F(CfgMgrTest, mergeIntoCurrentCfg) {
     // Those must be two separate instances.
     ASSERT_FALSE(ext_cfg1 == ext_cfg2);
 
+    // Add an option definition.
+    ext_cfg1->getCfgOptionDef()->add(OptionDefinition::create("option-foo",
+                                                              1,
+                                                              "isc",
+                                                              OPT_EMPTY_TYPE,
+                                                              ""));
+
     // Add a subnet which will be merged from first configuration.
     Subnet4Ptr subnet1(new Subnet4(IOAddress("192.1.2.0"), 24, 1, 2, 3, 123));
     ext_cfg1->getCfgSubnets4()->add(subnet1);
@@ -1073,6 +1081,10 @@ TEST_F(CfgMgrTest, mergeIntoCurrentCfg) {
     ASSERT_TRUE(cfg_mgr.getCurrentCfg()->getCfgSubnets4()->getBySubnetId(123));
     ASSERT_FALSE(cfg_mgr.getCurrentCfg()->getCfgSubnets4()->getBySubnetId(124));
 
+    // Ensure that the runtime option definitions have been updated.
+    auto runtime_def = LibDHCP::getRuntimeOptionDef("isc", "option-foo");
+    ASSERT_TRUE(runtime_def);
+
     // Create another configuration instance to check what sequence it would
     // pick. It should pick the first available one.
     SrvConfigPtr ext_cfg3;