From: Marcin Siodelski Date: Tue, 7 Nov 2017 09:26:14 +0000 (+0100) Subject: [5416] Allow null or 0 values in unique indexes within hosts table. X-Git-Tag: trac5400_base~1^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=19aaa0e0cc255c6e20997aa69b84df8135be8971;p=thirdparty%2Fkea.git [5416] Allow null or 0 values in unique indexes within hosts table. Also, updated MySQL backend to convert 0 values to null to make sure that multiple reservations are possible when IPv4 address is unspecified. --- diff --git a/configure.ac b/configure.ac index 22045c6dce..2fa156d5b5 100644 --- a/configure.ac +++ b/configure.ac @@ -1331,6 +1331,7 @@ AC_CONFIG_FILES([Makefile src/share/database/scripts/pgsql/upgrade_1.0_to_2.0.sh src/share/database/scripts/pgsql/upgrade_2.0_to_3.0.sh src/share/database/scripts/pgsql/upgrade_3.0_to_3.1.sh + src/share/database/scripts/pgsql/upgrade_3.1_to_3.2.sh tools/Makefile tools/path_replacer.sh ]) diff --git a/src/bin/admin/tests/pgsql_tests.sh.in b/src/bin/admin/tests/pgsql_tests.sh.in index da91db5fcf..41b7437e02 100644 --- a/src/bin/admin/tests/pgsql_tests.sh.in +++ b/src/bin/admin/tests/pgsql_tests.sh.in @@ -89,7 +89,7 @@ pgsql_lease_version_test() { # Verify that kea-admin lease-version returns the correct version version=$(${keaadmin} lease-version pgsql -u $db_user -p $db_password -n $db_name) - assert_str_eq "3.1" ${version} "Expected kea-admin to return %s, returned value was %s" + assert_str_eq "3.2" ${version} "Expected kea-admin to return %s, returned value was %s" # Let's wipe the whole database pgsql_wipe diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 82fd38d6a5..b8709bf5d4 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -83,6 +83,26 @@ const uint8_t MAX_IDENTIFIER_TYPE = static_cast(Host::LAST_IDENTIFIER_T /// options associated with a host to minimize impact on performance. Other /// classes derived from @ref MySqlHostExchange should be used to retrieve /// information about IPv6 reservations and options. +/// +/// Database schema contains several unique indexes to guard against adding +/// multiple hosts for the same client identifier in a single subnet and for +/// adding multiple hosts with a reservation for the same IPv4 address in a +/// single subnet. The exceptions that has to be taken into account are +/// listed below: +/// - zero or null IPv4 address indicates that there is no reservation for the +/// IPv4 address for the host, +/// - zero or null subnet identifier (either IPv4 or IPv6) indicates that +/// this subnet identifier must be ignored. Specifically, this is the case +/// when host reservation is created DHCPv4 server, the IPv6 subnet id should +/// be ignored. Conversely, when host reservation is created for DHCPv6 server, +/// the IPv4 subnet id should be ignored. +/// +/// To exclude those special case values from the unique indexes, the MySQL +/// backend relies on the property of the unique indexes in MySQL, i.e. null +/// values are excluded from unique indexes. That means that there might be +/// multiple null values in a given column on which unique index is applied. +/// Therefore, the MySQL backend converts subnet identifiers and IPv4 addresses +/// of 0 to null before inserting a host to the database. class MySqlHostExchange { private: @@ -279,17 +299,21 @@ public: // Can't take an address of intermediate object, so let's store it // in dhcp4_subnet_id_ dhcp4_subnet_id_ = host->getIPv4SubnetID(); + dhcp4_subnet_id_null_ = host->getIPv4SubnetID() == 0 ? MLM_TRUE : MLM_FALSE; bind_[3].buffer_type = MYSQL_TYPE_LONG; bind_[3].buffer = reinterpret_cast(&dhcp4_subnet_id_); bind_[3].is_unsigned = MLM_TRUE; + bind_[3].is_null = &dhcp4_subnet_id_null_; // dhcp6_subnet_id : INT UNSIGNED NULL // Can't take an address of intermediate object, so let's store it // in dhcp6_subnet_id_ dhcp6_subnet_id_ = host->getIPv6SubnetID(); + dhcp6_subnet_id_null_ = host->getIPv6SubnetID() == 0 ? MLM_TRUE : MLM_FALSE; bind_[4].buffer_type = MYSQL_TYPE_LONG; bind_[4].buffer = reinterpret_cast(&dhcp6_subnet_id_); bind_[4].is_unsigned = MLM_TRUE; + bind_[4].is_null = &dhcp6_subnet_id_null_; // ipv4_address : INT UNSIGNED NULL // The address in the Host structure is an IOAddress object. Convert diff --git a/src/lib/dhcpsrv/pgsql_connection.h b/src/lib/dhcpsrv/pgsql_connection.h index 9788972535..9aab9bfa35 100644 --- a/src/lib/dhcpsrv/pgsql_connection.h +++ b/src/lib/dhcpsrv/pgsql_connection.h @@ -17,9 +17,9 @@ namespace isc { namespace dhcp { -/// @brief Define PostgreSQL backend version: 3.0 +/// @brief Define PostgreSQL backend version: 3.2 const uint32_t PG_SCHEMA_VERSION_MAJOR = 3; -const uint32_t PG_SCHEMA_VERSION_MINOR = 1; +const uint32_t PG_SCHEMA_VERSION_MINOR = 2; // Maximum number of parameters that can be used a statement // @todo This allows us to use an initializer list (since we can't diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index ef0703f5bc..beee96c06f 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -57,6 +57,23 @@ const size_t DHCP_IDENTIFIER_MAX_LEN = 128; /// options associated with a host to minimize impact on performance. Other /// classes derived from @ref PgSqlHostExchange should be used to retrieve /// information about IPv6 reservations and options. +/// +/// Database schema contains several unique indexes to guard against adding +/// multiple hosts for the same client identifier in a single subnet and for +/// adding multiple hosts with a reservation for the same IPv4 address in a +/// single subnet. The exceptions that has to be taken into account are +/// listed below: +/// - zero or null IPv4 address indicates that there is no reservation for the +/// IPv4 address for the host, +/// - zero or null subnet identifier (either IPv4 or IPv6) indicates that +/// this subnet identifier must be ignored. Specifically, this is the case +/// when host reservation is created DHCPv4 server, the IPv6 subnet id should +/// be ignored. Conversely, when host reservation is created for DHCPv6 server, +/// the IPv4 subnet id should be ignored. +/// +/// To exclude those special case values, the Postgres backend uses partial +/// indexes, i.e. the only values that are included in the index are those that +/// are non-zero and non-null. class PgSqlHostExchange : public PgSqlExchange { private: diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index 75e46c14b4..71f9f46f7e 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -784,10 +784,10 @@ GenericHostDataSourceTest::testMultipleSubnets(int subnets, ASSERT_TRUE(hdsptr_); HostPtr host = initializeHost4("192.0.2.1", id); + host->setIPv6SubnetID(0); for (int i = 0; i < subnets; ++i) { host->setIPv4SubnetID(i + 1000); - host->setIPv6SubnetID(i + 1000); // Check that the same host can have reservations in multiple subnets. EXPECT_NO_THROW(hdsptr_->add(host)); @@ -1655,6 +1655,54 @@ void GenericHostDataSourceTest::testDeleteById6Options() { EXPECT_EQ(0, countDBReservations6()); } +void +GenericHostDataSourceTest::testMultipleHostsNoAddress4() { + // Make sure we have a pointer to the host data source. + ASSERT_TRUE(hdsptr_); + + // Create a host with zero IPv4 address. + HostPtr host1 = initializeHost4("0.0.0.0", Host::IDENT_HWADDR); + host1->setIPv4SubnetID(1); + host1->setIPv6SubnetID(0); + // Add the host to the database. + ASSERT_NO_THROW(hdsptr_->add(host1)); + + // An attempt to add this host again should fail due to client identifier + // duplication. + ASSERT_THROW(hdsptr_->add(host1), DuplicateEntry); + + // Create another host with zero IPv4 address. Adding this host to the + // database should be successful because zero addresses are not counted + // in the unique index. + HostPtr host2 = initializeHost4("0.0.0.0", Host::IDENT_HWADDR); + host2->setIPv4SubnetID(1); + host2->setIPv6SubnetID(0); + ASSERT_NO_THROW(hdsptr_->add(host2)); +} + +void +GenericHostDataSourceTest::testMultipleHosts6() { + // Make sure we have a pointer to the host data source. + ASSERT_TRUE(hdsptr_); + + // Create first host. + HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_DUID, false); + host1->setIPv4SubnetID(0); + host1->setIPv6SubnetID(1); + // Add the host to the database. + ASSERT_NO_THROW(hdsptr_->add(host1)); + + // An attempt to add this host again should fail due to client identifier + // duplication. + ASSERT_THROW(hdsptr_->add(host1), DuplicateEntry); + + HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_DUID, false); + host2->setIPv4SubnetID(0); + host2->setIPv6SubnetID(1); + // Add the host to the database. + ASSERT_NO_THROW(hdsptr_->add(host2)); +} + }; // namespace test }; // namespace dhcp }; // namespace isc diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h index c5bef73346..d41d539de1 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h @@ -580,6 +580,17 @@ public: /// Uses gtest macros to report failures. void testDeleteById6Options(); + /// @brief Tests that multiple reservations without IPv4 addresses can be + /// specified within a subnet. + /// + /// Uses gtest macros to report failures. + void testMultipleHostsNoAddress4(); + + /// @brief Tests that multiple hosts can be specified within an IPv6 subnet. + /// + /// Uses gtest macros to report failures. + void testMultipleHosts6(); + /// @brief Returns DUID with identical content as specified HW address /// /// This method does not have any sense in real life and is only useful diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index 2ca2b0f67a..da76ca0474 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -605,4 +605,15 @@ TEST_F(MySqlHostDataSourceTest, deleteById6Options) { testDeleteById6Options(); } +// Tests that multiple reservations without IPv4 addresses can be +// specified within a subnet. +TEST_F(MySqlHostDataSourceTest, testMultipleHostsNoAddress4) { + testMultipleHostsNoAddress4(); +} + +// Tests that multiple hosts can be specified within an IPv6 subnet. +TEST_F(MySqlHostDataSourceTest, testMultipleHosts6) { + testMultipleHosts6(); +} + }; // Of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc index f9aae9d93d..d0dbba5394 100644 --- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc @@ -562,4 +562,15 @@ TEST_F(PgSqlHostDataSourceTest, deleteById6Options) { testDeleteById6Options(); } +// Tests that multiple reservations without IPv4 addresses can be +// specified within a subnet. +TEST_F(PgSqlHostDataSourceTest, testMultipleHostsNoAddress4) { + testMultipleHostsNoAddress4(); +} + +// Tests that multiple hosts can be specified within an IPv6 subnet. +TEST_F(PgSqlHostDataSourceTest, testMultipleHosts6) { + testMultipleHosts6(); +} + }; // Of anonymous namespace diff --git a/src/share/database/scripts/pgsql/.gitignore b/src/share/database/scripts/pgsql/.gitignore index ba8b8e7958..2bf6d12948 100644 --- a/src/share/database/scripts/pgsql/.gitignore +++ b/src/share/database/scripts/pgsql/.gitignore @@ -1,3 +1,4 @@ upgrade_1.0_to_2.0.sh upgrade_2.0_to_3.0.sh upgrade_3.0_to_3.1.sh +upgrade_3.1_to_3.2.sh diff --git a/src/share/database/scripts/pgsql/Makefile.am b/src/share/database/scripts/pgsql/Makefile.am index d8df955df2..3c845dec77 100644 --- a/src/share/database/scripts/pgsql/Makefile.am +++ b/src/share/database/scripts/pgsql/Makefile.am @@ -6,9 +6,11 @@ sqlscripts_DATA += dhcpdb_drop.pgsql sqlscripts_DATA += upgrade_1.0_to_2.0.sh sqlscripts_DATA += upgrade_2.0_to_3.0.sh sqlscripts_DATA += upgrade_3.0_to_3.1.sh +sqlscripts_DATA += upgrade_3.1_to_3.2.sh DISTCLEANFILES = upgrade_1.0_to_2.0.sh DISTCLEANFILES += upgrade_2.0_to_3.0.sh -DISTCLEANFILES += upgrade_3.0_to_3.1.s +DISTCLEANFILES += upgrade_3.0_to_3.1.sh +DISTCLEANFILES += upgrade_3.1_to_3.2.sh EXTRA_DIST = ${sqlscripts_DATA} diff --git a/src/share/database/scripts/pgsql/dhcpdb_create.pgsql b/src/share/database/scripts/pgsql/dhcpdb_create.pgsql index 29bdcf608f..3597b73340 100644 --- a/src/share/database/scripts/pgsql/dhcpdb_create.pgsql +++ b/src/share/database/scripts/pgsql/dhcpdb_create.pgsql @@ -493,6 +493,41 @@ INSERT INTO host_identifier_type VALUES (4, 'flex-id'); UPDATE schema_version SET version = '3', minor = '1'; +-- Set 3.2 schema version. + +-- Remove constraints which perform too restrictive checks on the inserted +-- host reservations. We want to be able to insert host reservations which +-- include no specific IPv4 address or those that have repeating subnet +-- identifiers, e.g. IPv4 reservations would typically include 0 (or null) +-- IPv6 subnet identifiers. +ALTER TABLE hosts DROP CONSTRAINT key_dhcp4_ipv4_address_subnet_id; +ALTER TABLE hosts DROP CONSTRAINT key_dhcp4_identifier_subnet_id; +ALTER TABLE hosts DROP CONSTRAINT key_dhcp6_identifier_subnet_id; + +-- Create partial indexes instead of the constraints that we have removed. + +-- IPv4 address/IPv4 subnet identifier pair is unique if subnet identifier is +-- not null and not 0. +CREATE UNIQUE INDEX key_dhcp4_ipv4_address_subnet_id ON hosts + (ipv4_address ASC, dhcp4_subnet_id ASC) + WHERE ipv4_address IS NOT NULL AND ipv4_address <> 0; + +-- Client identifier is unique within an IPv4 subnet when subnet identifier is +-- not null and not 0. +CREATE UNIQUE INDEX key_dhcp4_identifier_subnet_id ON hosts + (dhcp_identifier ASC, dhcp_identifier_type ASC, dhcp4_subnet_id ASC) + WHERE (dhcp4_subnet_id IS NOT NULL AND dhcp4_subnet_id <> 0); + +-- Client identifier is unique within an IPv6 subnet when subnet identifier is +-- not null and not 0. +CREATE UNIQUE INDEX key_dhcp6_identifier_subnet_id ON hosts + (dhcp_identifier ASC, dhcp_identifier_type ASC, dhcp6_subnet_id ASC) + WHERE (dhcp6_subnet_id IS NOT NULL AND dhcp6_subnet_id <> 0); + +-- Set 3.2 schema version. +UPDATE schema_version + SET version = '3', minor = '2'; + -- Commit the script transaction. COMMIT; diff --git a/src/share/database/scripts/pgsql/upgrade_3.1_to_3.2.sh.in b/src/share/database/scripts/pgsql/upgrade_3.1_to_3.2.sh.in new file mode 100644 index 0000000000..3e4c44c66a --- /dev/null +++ b/src/share/database/scripts/pgsql/upgrade_3.1_to_3.2.sh.in @@ -0,0 +1,63 @@ +#!/bin/sh + +# Include utilities. Use installed version if available and +# use build version if it isn't. +if [ -e @datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh ]; then + . @datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh +else + . @abs_top_builddir@/src/bin/admin/admin-utils.sh +fi + +VERSION=`pgsql_version "$@"` + +if [ "$VERSION" != "3.0" ]; then + printf "This script upgrades 3.0 to 3.1. Reported version is $VERSION. Skipping upgrade.\n" + exit 0 +fi + +psql "$@" >/dev/null < 0; + +-- Client identifier is unique within an IPv4 subnet when subnet identifier is +-- not null and not 0. +CREATE UNIQUE INDEX hosts_dhcp4_identifier_subnet_id ON hosts + (dhcp_identifier ASC, dhcp_identifier_type ASC, dhcp4_subnet_id ASC) + WHERE (dhcp4_subnet_id IS NOT NULL AND dhcp4_subnet_id <> 0); + +-- Client identifier is unique within an IPv6 subnet when subnet identifier is +-- not null and not 0. +CREATE UNIQUE INDEX hosts_dhcp6_identifier_subnet_id ON hosts + (dhcp_identifier ASC, dhcp_identifier_type ASC, dhcp6_subnet_id ASC) + WHERE (dhcp6_subnet_id IS NOT NULL AND dhcp6_subnet_id <> 0); + +-- Set 3.2 schema version. +UPDATE schema_version + SET version = '3', minor = '2'; + +-- Schema 3.2 specification ends here. + +-- Commit the script transaction +COMMIT; + +EOF + +exit $RESULT +