From 66a216e62d7745cdafd1681df2af7682b10640e6 Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Thu, 31 Jul 2025 11:54:52 +0300 Subject: [PATCH] [3826] addressed review comments --- ChangeLog | 5 ++ src/bin/admin/tests/mysql_tests.sh.in | 4 +- src/bin/admin/tests/pgsql_tests.sh.in | 4 +- src/hooks/dhcp/lease_cmds/lease_cmds.cc | 4 ++ .../libloadtests/lease_cmds4_unittest.cc | 10 +++ .../libloadtests/lease_cmds6_unittest.cc | 15 +++- src/lib/dhcpsrv/memfile_lease_mgr.cc | 12 ++-- src/lib/dhcpsrv/memfile_lease_storage.h | 37 ++++------ src/lib/mysql/mysql_constants.h | 2 +- src/lib/pgsql/pgsql_connection.h | 2 +- .../scripts/mysql/dhcpdb_create.mysql | 11 +++ src/share/database/scripts/mysql/meson.build | 1 + .../scripts/mysql/upgrade_031_to_032.sh.in | 68 +++++++++++++++++++ .../scripts/pgsql/dhcpdb_create.pgsql | 11 +++ src/share/database/scripts/pgsql/meson.build | 1 + .../scripts/pgsql/upgrade_030_to_031.sh.in | 54 +++++++++++++++ 16 files changed, 204 insertions(+), 37 deletions(-) create mode 100755 src/share/database/scripts/mysql/upgrade_031_to_032.sh.in create mode 100755 src/share/database/scripts/pgsql/upgrade_030_to_031.sh.in diff --git a/ChangeLog b/ChangeLog index 5d7b9a003f..e8834390b5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2382. [build] razvan + Implemented the 'lease6-get-by-hw-address' command used to query + IPv6 leases by HW Address. + (Gitlab #3826) + Kea 3.1.0 (development) released on July 30, 2025 2381. [build] razvan diff --git a/src/bin/admin/tests/mysql_tests.sh.in b/src/bin/admin/tests/mysql_tests.sh.in index c4331707b1..4a48050ef0 100755 --- a/src/bin/admin/tests/mysql_tests.sh.in +++ b/src/bin/admin/tests/mysql_tests.sh.in @@ -157,7 +157,7 @@ mysql_db_version_test() { run_command \ "${kea_admin}" db-version mysql -u "${db_user}" -p "${db_password}" -n "${db_name}" version="${OUTPUT}" - assert_str_eq "31.0" "${version}" "Expected kea-admin to return %s, returned value was %s" + assert_str_eq "32.0" "${version}" "Expected kea-admin to return %s, returned value was %s" # Let's wipe the whole database mysql_wipe @@ -1200,7 +1200,7 @@ mysql_upgrade_test() { # Verify that the upgraded schema reports the latest version. version=$("${kea_admin}" db-version mysql -u "${db_user}" -p "${db_password}" -n "${db_name}" -d "${db_scripts_dir}") - assert_str_eq "31.0" "${version}" "Expected kea-admin to return %s, returned value was %s" + assert_str_eq "32.0" "${version}" "Expected kea-admin to return %s, returned value was %s" # Let's check that the new tables are indeed there. diff --git a/src/bin/admin/tests/pgsql_tests.sh.in b/src/bin/admin/tests/pgsql_tests.sh.in index e20b6ad025..b287c23353 100755 --- a/src/bin/admin/tests/pgsql_tests.sh.in +++ b/src/bin/admin/tests/pgsql_tests.sh.in @@ -158,7 +158,7 @@ pgsql_db_version_test() { run_command \ "${kea_admin}" db-version pgsql -u "${db_user}" -p "${db_password}" -n "${db_name}" version="${OUTPUT}" - assert_str_eq "30.0" "${version}" "Expected kea-admin to return %s, returned value was %s" + assert_str_eq "31.0" "${version}" "Expected kea-admin to return %s, returned value was %s" # Let's wipe the whole database pgsql_wipe @@ -1171,7 +1171,7 @@ pgsql_upgrade_test() { # Verify upgraded schema reports the latest version. version=$("${kea_admin}" db-version pgsql -u "${db_user}" -p "${db_password}" -n "${db_name}" -d "${db_scripts_dir}") - assert_str_eq "30.0" "${version}" 'Expected kea-admin to return %s, returned value was %s' + assert_str_eq "31.0" "${version}" 'Expected kea-admin to return %s, returned value was %s' # Check 1.0 to 2.0 upgrade pgsql_upgrade_1_0_to_2_0_test diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index b4a4f84aeb..c0fc5a7869 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -1597,6 +1597,10 @@ LeaseCmdsImpl::leaseGetByHwAddressHandler(CalloutHandle& handle) { isc_throw(BadValue, "'hw-address' parameter must be a string"); } + if (!v4 && hw_address->stringValue().empty()) { + isc_throw(BadValue, "'hw-address' parameter must not be empty"); + } + HWAddr hwaddr = HWAddr::fromText(hw_address->stringValue()); ElementPtr leases_json = Element::createList(); diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc index a4fb0a61d3..a970224bcb 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc @@ -1794,6 +1794,16 @@ void Lease4CmdsTest::testLease4GetByHwAddressFind0() { "}"; string exp_rsp = "0 IPv4 lease(s) found."; testCommand(cmd, CONTROL_RESULT_EMPTY, exp_rsp); + + // Empty HWAddr. + cmd = + "{\n" + " \"command\": \"lease4-get-by-hw-address\",\n" + " \"arguments\": {" + " \"hw-address\": \"\"\n" + " }\n" + "}"; + testCommand(cmd, CONTROL_RESULT_EMPTY, exp_rsp); } void Lease4CmdsTest::testLease4GetByHwAddressFind2() { diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc index 1b01ca69ac..cbcd6a3fc2 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc @@ -2014,6 +2014,17 @@ void Lease6CmdsTest::testLease6GetByHwAddressParams() { exp_rsp = "'hw-address' parameter must be a string"; testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + // Empty HWAddr. + cmd = + "{\n" + " \"command\": \"lease6-get-by-hw-address\",\n" + " \"arguments\": {" + " \"hw-address\": \"\"\n" + " }\n" + "}"; + exp_rsp = "'hw-address' parameter must not be empty"; + testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp); + // Simply bad value. cmd = "{\n" @@ -2071,10 +2082,10 @@ void Lease6CmdsTest::testLease6GetByHwAddressFind2() { // Let's check if the response makes any sense. ConstElementPtr lease = leases->get(0); ASSERT_TRUE(lease); - checkLease6(lease, "2001:db8:1::1", 0, 66, "42:42:42:42:42:42:42:42", "08:08:08:08:08:08"); + checkLease6(lease, "2001:db8:2::1", 0, 99, "42:42:42:42:42:42:42:42", "08:08:08:08:08:08"); lease = leases->get(1); ASSERT_TRUE(lease); - checkLease6(lease, "2001:db8:2::1", 0, 99, "42:42:42:42:42:42:42:42", "08:08:08:08:08:08"); + checkLease6(lease, "2001:db8:1::1", 0, 66, "42:42:42:42:42:42:42:42", "08:08:08:08:08:08"); } void Lease6CmdsTest::testLease6GetByDuidParams() { diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 84eef4842b..e6096e9564 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -1459,13 +1459,11 @@ Memfile_LeaseMgr::getLease6Internal(Lease::Type type, void Memfile_LeaseMgr::getLease6Internal(const HWAddr& hwaddr, Lease6Collection& collection) const { - // Using composite index by 'hw address' and 'subnet id'. It is - // ok to use it for searching by the 'hw address' only. - const Lease6StorageHWAddressSubnetIdIndex& idx = - storage6_.get(); - std::pair l - = idx.equal_range(boost::make_tuple(hwaddr.hwaddr_)); + const Lease6StorageHWAddressIndex& idx = + storage6_.get(); + std::pair l + = idx.equal_range(hwaddr.hwaddr_); BOOST_FOREACH(auto const& lease, l) { collection.push_back(Lease6Ptr(new Lease6(*lease))); diff --git a/src/lib/dhcpsrv/memfile_lease_storage.h b/src/lib/dhcpsrv/memfile_lease_storage.h index 006bafff2e..8907f4f521 100644 --- a/src/lib/dhcpsrv/memfile_lease_storage.h +++ b/src/lib/dhcpsrv/memfile_lease_storage.h @@ -37,6 +37,9 @@ struct ExpirationIndexTag { }; /// @brief Tag for indexes by HW address, subnet-id tuple. struct HWAddressSubnetIdIndexTag { }; +/// @brief Tag for indexes by HW address. +struct HWAddressIndexTag { }; + /// @brief Tag for indexes by client-id, subnet-id tuple. struct ClientIdSubnetIdIndexTag { }; @@ -176,25 +179,15 @@ typedef boost::multi_index_container< > >, - // Specification of the eight index starts here. - boost::multi_index::ordered_non_unique< - boost::multi_index::tag, - // This is a composite index that combines two attributes of the - // Lease6 object: hardware address and subnet id. - boost::multi_index::composite_key< - Lease6, - // The hardware address is held in the hwaddr_ member of the - // Lease4 object, which is a HWAddr object. Boost does not - // provide a key extractor for getting a member of a member, - // so we need a simple method for that. - boost::multi_index::const_mem_fun&, - &Lease::getHWAddrVector>, - // The subnet id is held in the subnet_id_ member of Lease6 - // class. Note that the subnet_id_ is defined in the base - // class (Lease) so we have to point to this class rather - // than derived class: Lease6. - boost::multi_index::member - > + // Specification of the eighth index starts here. + boost::multi_index::hashed_non_unique< + boost::multi_index::tag, + // The hardware address is held in the hwaddr_ member of the + // Lease6 object, which is a HWAddr object. Boost does not + // provide a key extractor for getting a member of a member, + // so we need a simple method for that. + boost::multi_index::const_mem_fun&, + &Lease::getHWAddrVector> > > > Lease6Storage; // Specify the type name of this container. @@ -364,9 +357,9 @@ typedef Lease6Storage::index::type Lease6StorageDuidIaidTy /// @brief DHCPv6 lease storage index by expiration time. typedef Lease6Storage::index::type Lease6StorageExpirationIndex; -/// @brief DHCPv6 lease storage index by HW address and subnet-id. -typedef Lease6Storage::index::type -Lease6StorageHWAddressSubnetIdIndex; +/// @brief DHCPv6 lease storage index by HW address. +typedef Lease6Storage::index::type +Lease6StorageHWAddressIndex; /// @brief DHCPv6 lease storage index by subnet-id. typedef Lease6Storage::index::type Lease6StorageSubnetIdIndex; diff --git a/src/lib/mysql/mysql_constants.h b/src/lib/mysql/mysql_constants.h index 1fad37a884..9197842fb6 100644 --- a/src/lib/mysql/mysql_constants.h +++ b/src/lib/mysql/mysql_constants.h @@ -52,7 +52,7 @@ const int MLM_MYSQL_FETCH_FAILURE = 0; /// @name Current database schema version values. //@{ -const uint32_t MYSQL_SCHEMA_VERSION_MAJOR = 31; +const uint32_t MYSQL_SCHEMA_VERSION_MAJOR = 32; const uint32_t MYSQL_SCHEMA_VERSION_MINOR = 0; //@} diff --git a/src/lib/pgsql/pgsql_connection.h b/src/lib/pgsql/pgsql_connection.h index c6c8ab3928..50984f7ace 100644 --- a/src/lib/pgsql/pgsql_connection.h +++ b/src/lib/pgsql/pgsql_connection.h @@ -18,7 +18,7 @@ namespace isc { namespace db { /// @brief Define the PostgreSQL backend version. -const uint32_t PGSQL_SCHEMA_VERSION_MAJOR = 30; +const uint32_t PGSQL_SCHEMA_VERSION_MAJOR = 31; const uint32_t PGSQL_SCHEMA_VERSION_MINOR = 0; // Maximum number of parameters that can be used a statement diff --git a/src/share/database/scripts/mysql/dhcpdb_create.mysql b/src/share/database/scripts/mysql/dhcpdb_create.mysql index c16b4c11be..1e1497ddba 100644 --- a/src/share/database/scripts/mysql/dhcpdb_create.mysql +++ b/src/share/database/scripts/mysql/dhcpdb_create.mysql @@ -6451,6 +6451,17 @@ UPDATE schema_version -- This line concludes the schema upgrade to version 31.0. +-- This line starts the schema upgrade to version 32.0. + +# Create index for searching leases by hwaddr. +CREATE INDEX lease6_by_hwaddr ON lease6 (hwaddr); + +-- Update the schema version number. +UPDATE schema_version + SET version = '32', minor = '0'; + +-- This line concludes the schema upgrade to version 32.0. + # Notes: # # Indexes diff --git a/src/share/database/scripts/mysql/meson.build b/src/share/database/scripts/mysql/meson.build index 647591aac6..61fce59c79 100644 --- a/src/share/database/scripts/mysql/meson.build +++ b/src/share/database/scripts/mysql/meson.build @@ -72,6 +72,7 @@ upgrade_scripts = [ 'upgrade_028_to_029.sh', 'upgrade_029_to_030.sh', 'upgrade_030_to_031.sh', + 'upgrade_031_to_032.sh', ] list = run_command( GRABBER, diff --git a/src/share/database/scripts/mysql/upgrade_031_to_032.sh.in b/src/share/database/scripts/mysql/upgrade_031_to_032.sh.in new file mode 100755 index 0000000000..521469640f --- /dev/null +++ b/src/share/database/scripts/mysql/upgrade_031_to_032.sh.in @@ -0,0 +1,68 @@ +#!/bin/sh + +# Copyright (C) 2025 Internet Systems Consortium, Inc. ("ISC") # +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +# Exit with error if commands exit with non-zero and if undefined variables are +# used. +set -eu + +# shellcheck disable=SC2034 +# SC2034: ... appears unused. Verify use (or export if used externally). +prefix="@prefix@" + +# Include utilities based on location of this script. Check for sources first, +# so that the unexpected situations with weird paths fall on the default +# case of installed. +script_path=$(cd "$(dirname "${0}")" && pwd) +if test "${script_path}" = "@abs_top_builddir@/src/share/database/scripts/mysql"; then + # shellcheck source=./src/bin/admin/admin-utils.sh.in + . "@abs_top_builddir@/src/bin/admin/admin-utils.sh" +else + # shellcheck source=./src/bin/admin/admin-utils.sh.in + . "@datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh" +fi + +# Check only major version to allow for intermediary backported schema changes. +version=$(mysql_version "${@}" | cut -d '.' -f 1) +if test "${version}" != '31'; then + printf 'This script upgrades 31.* to 32.0. ' + printf 'Reported version is %s. Skipping upgrade.\n' "${version}" + exit 0 +fi + +# Get the schema name from database argument. We need this to +# query information_schema for the right database. +for arg in "${@}" +do + if ! printf '%s' "${arg}" | grep -Eq -- '^--' + then + schema="$arg" + break + fi +done + +# Make sure we have the schema. +if [ -z "$schema" ] +then + printf "Could not find database schema name in cmd line args: %s\n" "${*}" + exit 255 +fi + +mysql "$@" </dev/null <