From: Marcin Siodelski Date: Tue, 20 Jun 2023 14:35:51 +0000 (+0200) Subject: [#2931] Fixed host queries X-Git-Tag: Kea-2.4.0~168 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f5e92c69dab4138cc9fa220fddc29169fa879b33;p=thirdparty%2Fkea.git [#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 3044530626..faa66dbd02 100644 --- 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 aff5b3dc3f..bc17cf2eaa 100644 --- 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 cf4d5ceff8..0c3d56e7f6 100644 --- 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(), host_id); diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index 77d1e3f3a4..71e245017c 100644 --- 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(), host_id); diff --git a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc index b14920a872..0333818725 100644 --- 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 441d2882b5..0be8217d5c 100644 --- 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 #include #include +#include #include #include #include @@ -169,6 +170,10 @@ GenericHostDataSourceTest::addTestOptions(const HostPtr& host, "3000::2", "3000::3"), "isc2"); + desc = createOption(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 00cd1df0bb..ecd5c28471 100644 --- 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 option_spaces = cfg2->getOptionSpaceNames(); std::list 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.