[#2881] Fix encapsulating options from CB

diff --git a/src/bin/dhcp4/tests/config_backend_unittest.cc b/src/bin/dhcp4/tests/config_backend_unittest.cc
index 56c6a821a239f51797d774da228991b73172af57..510c2bffccf7859502504922786b3774a9c48b14 100644 (file)
--- a/src/bin/dhcp4/tests/config_backend_unittest.cc
+++ b/src/bin/dhcp4/tests/config_backend_unittest.cc
@@ -20,8 +20,8 @@
 #include <dhcpsrv/testutils/generic_backend_unittest.h>
 #include <dhcpsrv/testutils/test_config_backend_dhcp4.h>
 
-#include "dhcp4_test_utils.h"
-#include "get_config_unittest.h"
+#include <dhcp4/tests/dhcp4_test_utils.h>
+#include <dhcp4/tests/get_config_unittest.h>
 
 #include <boost/foreach.hpp>
 #include <boost/scoped_ptr.hpp>
@@ -385,6 +385,63 @@ TEST_F(Dhcp4CBTest, mergeOptions) {
     EXPECT_EQ("my-boot-file", found_opt.option_->toString());
 }
 
+// This test verifies that DHCP options fetched from the config backend
+// encapsulate their suboptions.
+TEST_F(Dhcp4CBTest, mergeOptionsWithSuboptions) {
+    string base_config =
+        "{ \n"
+        "    \"option-def\": [ { \n"
+        "        \"name\": \"vendor-suboption-1\", \n"
+        "        \"code\": 1, \n"
+        "        \"type\": \"string\", \n"
+        "        \"space\": \"vendor-encapsulated-options-space\" \n"
+        "    }], \n"
+        "    \"config-control\": { \n"
+        "       \"config-databases\": [ { \n"
+        "               \"type\": \"memfile\", \n"
+        "               \"host\": \"db1\" \n"
+        "           },{ \n"
+        "               \"type\": \"memfile\", \n"
+        "               \"host\": \"db2\" \n"
+        "           } \n"
+        "       ] \n"
+        "   } \n"
+        "} \n";
+
+    extractConfig(base_config);
+
+    // Create option 43 instance and store it in the database.
+    OptionDescriptorPtr opt;
+    opt.reset(new OptionDescriptor(
+              createEmptyOption(Option::V4, DHO_VENDOR_ENCAPSULATED_OPTIONS,
+                                true, false)));
+    opt->space_name_ = DHCP4_OPTION_SPACE;
+    db1_->createUpdateOption4(ServerSelector::ALL(), opt);
+
+    // Create option 43 suboption and store it in the database.
+    opt.reset(new OptionDescriptor(
+              createOption<OptionString>(Option::V4, 1, true, false, false,
+                                         "http://server:8080")
+          )
+    );
+    opt->space_name_ = VENDOR_ENCAPSULATED_OPTION_SPACE;
+    db1_->createUpdateOption4(ServerSelector::ALL(), opt);
+
+    // Fetch the configuration from the config backend.
+    ASSERT_NO_FATAL_FAILURE(configure(base_config, CONTROL_RESULT_SUCCESS, ""));
+
+    auto staging_cfg = CfgMgr::instance().getStagingCfg();
+
+    // Make sure that option 43 has been fetched.
+    auto found_opt_desc = staging_cfg->getCfgOption()->
+        get(DHCP4_OPTION_SPACE, DHO_VENDOR_ENCAPSULATED_OPTIONS);
+    ASSERT_TRUE(found_opt_desc.option_);
+
+    // Make sure that the option 43 contains its suboption.
+    auto found_subopt = found_opt_desc.option_->getOption(1);
+    EXPECT_TRUE(found_subopt);
+}
+
 // This test verifies that externally configured shared-networks are
 // merged correctly into staging configuration.
 TEST_F(Dhcp4CBTest, mergeSharedNetworks) {

diff --git a/src/bin/dhcp6/tests/config_backend_unittest.cc b/src/bin/dhcp6/tests/config_backend_unittest.cc
index 724b27163cb6f64d7b7baa1f0bd7e3673e85d254..6475ee832bcfa78f1deaec63134d1a75404310b3 100644 (file)
--- a/src/bin/dhcp6/tests/config_backend_unittest.cc
+++ b/src/bin/dhcp6/tests/config_backend_unittest.cc
@@ -22,8 +22,8 @@
 #include <dhcpsrv/testutils/generic_backend_unittest.h>
 #include <dhcpsrv/testutils/test_config_backend_dhcp6.h>
 
-#include "dhcp6_test_utils.h"
-#include "get_config_unittest.h"
+#include <dhcp6/tests/dhcp6_test_utils.h>
+#include <dhcp6/tests/get_config_unittest.h>
 
 #include <boost/foreach.hpp>
 #include <boost/scoped_ptr.hpp>
@@ -361,6 +361,68 @@ TEST_F(Dhcp6CBTest, mergeOptions) {
     EXPECT_EQ(500, opint->getValue());
 }
 
+// This test verifies that DHCP options fetched from the config backend
+// encapsulate their suboptions.
+TEST_F(Dhcp6CBTest, mergeOptionsWithSuboptions) {
+    string base_config =
+        "{ \n"
+        "    \"option-def\": [ { \n"
+        "        \"name\": \"option-1024\", \n"
+        "        \"code\": 1024, \n"
+        "        \"type\": \"empty\", \n"
+        "        \"space\": \"dhcp6\", \n"
+        "        \"encapsulate\": \"option-1024-space\" \n"
+        "    }, \n"
+        "    { \n"
+        "        \"name\": \"option-1025\", \n"
+        "        \"code\": 1025, \n"
+        "        \"type\": \"string\", \n"
+        "        \"space\": \"option-1024-space\" \n"
+        "    } ], \n"
+        "    \"config-control\": { \n"
+        "       \"config-databases\": [ { \n"
+        "               \"type\": \"memfile\", \n"
+        "               \"host\": \"db1\" \n"
+        "           },{ \n"
+        "               \"type\": \"memfile\", \n"
+        "               \"host\": \"db2\" \n"
+        "           } \n"
+        "       ] \n"
+        "   } \n"
+        "} \n";
+
+    extractConfig(base_config);
+
+    // Create option 1024 instance and store it in the database.
+    OptionDescriptorPtr opt;
+    opt.reset(new OptionDescriptor(
+              createEmptyOption(Option::V6, 1024, true, false)));
+    opt->space_name_ = DHCP6_OPTION_SPACE;
+    db1_->createUpdateOption6(ServerSelector::ALL(), opt);
+
+    // Create option 1024 suboption and store it in the database.
+    opt.reset(new OptionDescriptor(
+              createOption<OptionString>(Option::V6, 1025, true, false, false,
+                                         "http://server:8080")
+          )
+    );
+    opt->space_name_ = "option-1024-space";
+    db1_->createUpdateOption6(ServerSelector::ALL(), opt);
+
+    // Fetch the configuration from the config backend.
+    ASSERT_NO_FATAL_FAILURE(configure(base_config, CONTROL_RESULT_SUCCESS, ""));
+
+    auto staging_cfg = CfgMgr::instance().getStagingCfg();
+
+    // Make sure that option 1024 has been fetched.
+    auto found_opt_desc = staging_cfg->getCfgOption()->get(DHCP6_OPTION_SPACE, 1024);
+    ASSERT_TRUE(found_opt_desc.option_);
+
+    // Make sure that the option 1024 contains its suboption.
+    auto found_subopt = found_opt_desc.option_->getOption(1025);
+    EXPECT_TRUE(found_subopt);
+}
+
 // This test verifies that externally configured shared-networks are
 // merged correctly into staging configuration.
 TEST_F(Dhcp6CBTest, mergeSharedNetworks) {

diff --git a/src/lib/dhcpsrv/cfg_option.cc b/src/lib/dhcpsrv/cfg_option.cc
index e3c3e8b668be9b83f54e632de0341cdb5f1cc13f..3044530626f83d5f075983426130b394f409f043 100644 (file)
--- a/src/lib/dhcpsrv/cfg_option.cc
+++ b/src/lib/dhcpsrv/cfg_option.cc
@@ -148,6 +148,12 @@ CfgOption::merge(CfgOptionDefPtr cfg_def,  CfgOption& other) {
 
     // Next we copy "other" on top of ourself.
     other.copyTo(*this);
+
+    // If we copied new options we may need to populate the
+    // sub-options into the upper level options. The server
+    // expects that the top-level options have suitable
+    // suboptions appended.
+    encapsulate();
 }
 
 void
@@ -195,6 +201,11 @@ CfgOption::createDescriptorOption(CfgOptionDefPtr cfg_def, const std::string& sp
         def = cfg_def->get(space, code);
     }
 
+    // Finish with a last resort option definition.
+    if (!def) {
+        def = LibDHCP::getLastResortOptionDef(space, code);
+    }
+
     std::string& formatted_value = opt_desc.formatted_value_;
     if (!def) {
         if (!formatted_value.empty()) {