[#2931] Fixed host queries

Addresses two issues:
- The host query by IPv6 address has been fixed for the case when the
  duplicated addresses are allowed
- Corrected fetching vendor options from the host database

diff --git a/src/lib/dhcpsrv/cfg_option.cc b/src/lib/dhcpsrv/cfg_option.cc
index 3044530626f83d5f075983426130b394f409f043..faa66dbd02f60907cc6c2bdbc3e339ce2f2d6ee5 100644 (file)
--- a/src/lib/dhcpsrv/cfg_option.cc
+++ b/src/lib/dhcpsrv/cfg_option.cc
@@ -345,6 +345,15 @@ CfgOption::getAll(const uint32_t vendor_id) const {
     return (vendor_options_.getItems(vendor_id));
 }
 
+OptionContainerPtr
+CfgOption::getAllCombined(const std::string& option_space) const {
+    auto vendor_id = LibDHCP::optionSpaceToVendorId(option_space);
+    if (vendor_id > 0) {
+        return (getAll(vendor_id));
+    }
+    return (getAll(option_space));
+}
+
 size_t
 CfgOption::del(const std::string& option_space, const uint16_t option_code) {
     // Check for presence of options.

diff --git a/src/lib/dhcpsrv/cfg_option.h b/src/lib/dhcpsrv/cfg_option.h
index aff5b3dc3f333d5c27c0abd5a26dc232b8b53f3d..bc17cf2eaa6a84564533f82070bedeb79139f425 100644 (file)
--- a/src/lib/dhcpsrv/cfg_option.h
+++ b/src/lib/dhcpsrv/cfg_option.h
@@ -563,6 +563,19 @@ public:
     /// container is empty if no options have been found.
     OptionContainerPtr getAll(const uint32_t vendor_id) const;
 
+    /// @brief Returns all non-vendor or vendor options for the specified
+    /// option space.
+    ///
+    /// It combines the output of the @c getAll function variants. When
+    /// option space has the format of "vendor-X", it retrieves the vendor
+    /// options by vendor id, where X must be a 32-bit unsigned integer.
+    /// Otherwise, it fetches non-vendor options.
+    ///
+    /// @param option_space Name of the option space.
+    /// @return Pointer to the container holding returned options. This
+    /// container is empty if no options have been found.
+    OptionContainerPtr getAllCombined(const std::string& option_space) const;
+
     /// @brief Returns option for the specified key and option code.
     ///
     /// The key should be a string, in which case it specifies an option space

diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc
index cf4d5ceff872a85906baabadab4bc24799c20a19..0c3d56e7f6c04400fc17a7bd95485d8eec4838f7 100644 (file)
--- a/src/lib/dhcpsrv/mysql_host_data_source.cc
+++ b/src/lib/dhcpsrv/mysql_host_data_source.cc
@@ -1036,7 +1036,6 @@ private:
                               << "' is invalid JSON: " << ex.what());
                 }
             }
-
             cfg->add(desc, space);
         }
 
@@ -2477,9 +2476,9 @@ TaggedStatementArray tagged_statements = { {
                 "ON h.host_id = o.host_id "
             "LEFT JOIN ipv6_reservations AS r "
                 "ON h.host_id = r.host_id "
-            "WHERE h.dhcp6_subnet_id = ? AND h.host_id = "
-                "( SELECT host_id FROM ipv6_reservations "
-                    "WHERE address = ? ) "
+            "WHERE h.dhcp6_subnet_id = ? AND h.host_id IN "
+                "(SELECT host_id FROM ipv6_reservations "
+                    "WHERE address = ?) "
             "ORDER BY h.host_id, o.option_id, r.reservation_id"},
 
     // Retrieves host information along with the DHCPv4 options associated with
@@ -3078,7 +3077,7 @@ MySqlHostDataSourceImpl::addOptions(MySqlHostContextPtr& ctx,
     // For each option space retrieve all options and insert them into the
     // database.
     for (auto space = option_spaces.begin(); space != option_spaces.end(); ++space) {
-        OptionContainerPtr options = options_cfg->getAll(*space);
+        OptionContainerPtr options = options_cfg->getAllCombined(*space);
         if (options && !options->empty()) {
             for (auto opt = options->begin(); opt != options->end(); ++opt) {
                 addOption(ctx, stindex, *opt, *space, Optional<SubnetID>(), host_id);

diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc
index 77d1e3f3a4f5b1bcef57dc2536a8dcbe6399526a..71e245017c76a76ae8ceaeb5cb8f3bf70eece383 100644 (file)
--- a/src/lib/dhcpsrv/pgsql_host_data_source.cc
+++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc
@@ -1807,7 +1807,7 @@ TaggedStatementArray tagged_statements = { {
      "FROM hosts AS h "
      "LEFT JOIN dhcp6_options AS o ON h.host_id = o.host_id "
      "LEFT JOIN ipv6_reservations AS r ON h.host_id = r.host_id "
-     "WHERE h.dhcp6_subnet_id = $1 AND h.host_id = "
+     "WHERE h.dhcp6_subnet_id = $1 AND h.host_id IN "
      "  (SELECT host_id FROM ipv6_reservations "
      "   WHERE address = $2) "
      "ORDER BY h.host_id, o.option_id, r.reservation_id"
@@ -2508,7 +2508,7 @@ PgSqlHostDataSourceImpl::addOptions(PgSqlHostContextPtr& ctx,
     // For each option space retrieve all options and insert them into the
     // database.
     for (auto space = option_spaces.begin(); space != option_spaces.end(); ++space) {
-        OptionContainerPtr options = options_cfg->getAll(*space);
+        OptionContainerPtr options = options_cfg->getAllCombined(*space);
         if (options && !options->empty()) {
             for (auto opt = options->begin(); opt != options->end(); ++opt) {
                 addOption(ctx, stindex, *opt, *space, Optional<SubnetID>(), host_id);

diff --git a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc
index b14920a872d2df4851e56efb36ecf19bdb38f6eb..0333818725e9cc7a5ff80f5753ebbd5c246c69ab 100644 (file)
--- a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc
@@ -285,7 +285,8 @@ TEST_F(CfgOptionTest, add) {
         ++expected_code;
     }
 
-    options = cfg.getAll("isc");
+    // Try another function variant.
+    options = cfg.getAllCombined("isc");
     ASSERT_TRUE(options);
     ASSERT_EQ(7, options->size());
 
@@ -1174,6 +1175,11 @@ TEST_F(CfgOptionTest, addVendorOptions) {
     ASSERT_TRUE(options);
     ASSERT_EQ(10, options->size());
 
+    // Make sure we can get vendor options by option space.
+    options = cfg.getAllCombined("vendor-12345678");
+    ASSERT_TRUE(options);
+    ASSERT_EQ(10, options->size());
+
     // Validate codes of options added to dhcp6 option space.
     uint16_t expected_code = 100;
     for (OptionContainer::const_iterator option_desc = options->begin();
@@ -1200,6 +1206,11 @@ TEST_F(CfgOptionTest, addVendorOptions) {
     options = cfg.getAll(1111111);
     ASSERT_TRUE(options);
     EXPECT_TRUE(options->empty());
+
+    // Try another function variant.
+    options = cfg.getAll("vendor-1111111");
+    ASSERT_TRUE(options);
+    EXPECT_TRUE(options->empty());
 }
 
 // This test verifies that option space names for the vendor options are

diff --git a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
index 441d2882b511fd8449f041abbb92fd0b54b1bcac..0be8217d5c0c61cc88ae312b656d5a007ebade6a 100644 (file)
--- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
@@ -9,6 +9,7 @@
 #include <database/database_connection.h>
 #include <database/db_exceptions.h>
 #include <dhcp/dhcp6.h>
+#include <dhcp/docsis3_option_defs.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/option4_addrlst.h>
 #include <dhcp/option6_addrlst.h>
@@ -169,6 +170,10 @@ GenericHostDataSourceTest::addTestOptions(const HostPtr& host,
                                                       "3000::2", "3000::3"),
                   "isc2");
 
+        desc = createOption<OptionString>(Option::V6, DOCSIS3_V6_TFTP_SERVERS,
+                                          true, false, true, "3000:1::234");
+        opts->add(desc, "vendor-4491");
+
         // Add definitions for DHCPv6 non-standard options.
         defs.addItem(OptionDefinitionPtr(new OptionDefinition(
                          "option-1024", 1024, DHCP6_OPTION_SPACE,
@@ -1727,6 +1732,8 @@ GenericHostDataSourceTest::testGetBySubnetIPv6() {
     // Let's create a couple of hosts...
     HostPtr host1 = HostDataSourceUtils::initializeHost6("2001:db8:1::", Host::IDENT_DUID, true);
     addIPv6Address(host1, "2001:db8:1::10");
+    ASSERT_NO_THROW(addTestOptions(host1, true, DHCP6_ONLY));
+
     HostPtr host2 = HostDataSourceUtils::initializeHost6("2001:db8:2::", Host::IDENT_DUID, true);
     addIPv6Address(host2, "2001:db8:1::20");
     HostPtr host3 = HostDataSourceUtils::initializeHost6("2001:db8:3::", Host::IDENT_DUID, true);
@@ -1814,6 +1821,7 @@ GenericHostDataSourceTest::testAllowDuplicateIPv6() {
 
     // Create a host reservations.
     HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1", Host::IDENT_HWADDR, true, true);
+    addIPv6Address(host, "2001:db8::2");
     auto host_id = host->getHostId();
     auto subnet_id = host->getIPv6SubnetID();
 
@@ -1823,6 +1831,7 @@ GenericHostDataSourceTest::testAllowDuplicateIPv6() {
     // Then try to add it again, it should throw an exception because the
     // HWADDR is the same.
     host = HostDataSourceUtils::initializeHost6("2001:db8::1", Host::IDENT_HWADDR, true, false);
+    addIPv6Address(host, "2001:db8::2");
     host->setHostId(++host_id);
     host->setIPv6SubnetID(subnet_id);
     ASSERT_THROW(hdsptr_->add(host), DuplicateEntry);

diff --git a/src/lib/dhcpsrv/testutils/host_data_source_utils.cc b/src/lib/dhcpsrv/testutils/host_data_source_utils.cc
index 00cd1df0bbfe1812ba46672a37b31871c9bc526f..ecd5c28471e8fc2751d6dc51733ce48ae269a484 100644 (file)
--- a/src/lib/dhcpsrv/testutils/host_data_source_utils.cc
+++ b/src/lib/dhcpsrv/testutils/host_data_source_utils.cc
@@ -325,8 +325,6 @@ HostDataSourceUtils::compareOptions(const ConstCfgOptionPtr& cfg1,
     // Combine option space names with vendor space names in a single list.
     std::list<std::string> option_spaces = cfg2->getOptionSpaceNames();
     std::list<std::string> vendor_spaces = cfg2->getVendorIdsSpaceNames();
-    option_spaces.insert(option_spaces.end(), vendor_spaces.begin(),
-                         vendor_spaces.end());
 
     // Make sure that the number of option spaces is equal in both
     // configurations.