From: Marcin Siodelski Date: Thu, 25 Jul 2019 09:49:39 +0000 (+0200) Subject: [#632] Properly update the subnet pools when the subnet is updated. X-Git-Tag: Kea-1.6.1~10^2~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dbd1e5c308ad3fe795264fd83bbbd95bcf853004;p=thirdparty%2Fkea.git [#632] Properly update the subnet pools when the subnet is updated. This corrects the problem of orphaned pools when the subnet identifier is updated with the subnet update. --- diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index e9f3bb6206..7bf1805164 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -123,7 +123,7 @@ public: DELETE_ALL_SUBNETS4_UNASSIGNED, DELETE_ALL_SUBNETS4_SHARED_NETWORK_NAME, DELETE_SUBNET4_SERVER, - DELETE_POOLS4_SUBNET_ID, + DELETE_POOLS4, DELETE_SHARED_NETWORK4_NAME_WITH_TAG, DELETE_SHARED_NETWORK4_NAME_ANY, DELETE_ALL_SHARED_NETWORKS4, @@ -1044,15 +1044,18 @@ public: /// @brief Deletes pools belonging to a subnet from the database. /// + /// The query deletes all pools associated with the subnet's + /// identifier or prefix. /// @param subnet Pointer to the subnet for which pools should be /// deleted. uint64_t deletePools4(const Subnet4Ptr& subnet) { MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(subnet->getID()) + MySqlBinding::createInteger(subnet->getID()), + MySqlBinding::createString(subnet->toText()) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_POOLS4_SUBNET_ID, in_bindings)); + return (conn_.updateDeleteQuery(DELETE_POOLS4, in_bindings)); } /// @brief Sends query to the database to retrieve multiple shared @@ -2491,7 +2494,7 @@ TaggedStatementArray tagged_statements = { { }, // Delete pools for a subnet. - { MySqlConfigBackendDHCPv4Impl::DELETE_POOLS4_SUBNET_ID, + { MySqlConfigBackendDHCPv4Impl::DELETE_POOLS4, MYSQL_DELETE_POOLS(dhcp4) }, diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc index b62a974976..e7c30b70b0 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc @@ -128,8 +128,8 @@ public: DELETE_ALL_SUBNETS6_UNASSIGNED, DELETE_ALL_SUBNETS6_SHARED_NETWORK_NAME, DELETE_SUBNET6_SERVER, - DELETE_POOLS6_SUBNET_ID, - DELETE_PD_POOLS_SUBNET_ID, + DELETE_POOLS6, + DELETE_PD_POOLS, DELETE_SHARED_NETWORK6_NAME_WITH_TAG, DELETE_SHARED_NETWORK6_NAME_ANY, DELETE_ALL_SHARED_NETWORKS6, @@ -1231,28 +1231,34 @@ public: /// @brief Deletes pools belonging to a subnet from the database. /// + /// The query deletes all pools associated with the subnet's + /// identifier or prefix. /// @param subnet Pointer to the subnet for which pools should be /// deleted. uint64_t deletePools6(const Subnet6Ptr& subnet) { MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(subnet->getID()) + MySqlBinding::createInteger(subnet->getID()), + MySqlBinding::createString(subnet->toText()) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_POOLS6_SUBNET_ID, in_bindings)); + return (conn_.updateDeleteQuery(DELETE_POOLS6, in_bindings)); } /// @brief Deletes pd pools belonging to a subnet from the database. /// + /// The query deletes all pools associated with the subnet's + /// identifier or prefix. /// @param subnet Pointer to the subnet for which pd pools should be /// deleted. uint64_t deletePdPools6(const Subnet6Ptr& subnet) { MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(subnet->getID()) + MySqlBinding::createInteger(subnet->getID()), + MySqlBinding::createString(subnet->toText()) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_PD_POOLS_SUBNET_ID, in_bindings)); + return (conn_.updateDeleteQuery(DELETE_PD_POOLS, in_bindings)); } /// @brief Sends query to the database to retrieve multiple shared @@ -2850,12 +2856,12 @@ TaggedStatementArray tagged_statements = { { }, // Delete pools for a subnet. - { MySqlConfigBackendDHCPv6Impl::DELETE_POOLS6_SUBNET_ID, + { MySqlConfigBackendDHCPv6Impl::DELETE_POOLS6, MYSQL_DELETE_POOLS(dhcp6) }, // Delete pd pools for a subnet. - { MySqlConfigBackendDHCPv6Impl::DELETE_PD_POOLS_SUBNET_ID, + { MySqlConfigBackendDHCPv6Impl::DELETE_PD_POOLS, MYSQL_DELETE_PD_POOLS() }, diff --git a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h index c3bd66380d..7eb8a8a8e0 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h +++ b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h @@ -743,13 +743,17 @@ namespace { #ifndef MYSQL_DELETE_POOLS #define MYSQL_DELETE_POOLS(table_prefix) \ "DELETE FROM " #table_prefix "_pool " \ - "WHERE subnet_id = ?" + "WHERE subnet_id = ? OR subnet_id = " \ + "(SELECT subnet_id FROM " #table_prefix "_subnet" \ + " WHERE subnet_prefix = ?)" #endif #ifndef MYSQL_DELETE_PD_POOLS #define MYSQL_DELETE_PD_POOLS() \ "DELETE FROM dhcp6_pd_pool " \ - "WHERE subnet_id = ?" + "WHERE subnet_id = ? OR subnet_id = " \ + "(SELECT subnet_id FROM dhcp6_subnet" \ + " WHERE subnet_prefix = ?)" #endif #ifndef MYSQL_DELETE_SHARED_NETWORK_COMMON diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc index cf104ceb70..8631eeca6b 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc @@ -1989,6 +1989,44 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSharedNetworkSubnets4) { EXPECT_TRUE(isEquivalent(returned_list, test_list)); } +// Test that pools are properly updated as a result a subnet update. +TEST_F(MySqlConfigBackendDHCPv4Test, subnetUpdatePools) { + + auto test_subnet_update = [this](const std::string& subnet_prefix, + const SubnetID& subnet_id) { + // Add the subnet with two pools. + EXPECT_NO_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), + test_subnets_[0])); + // Make sure that the pools have been added to the database. + EXPECT_EQ(2, countRows("dhcp4_pool")); + + // Create the subnet without options which updates the existing + // subnet. + Subnet4Ptr subnet(new Subnet4(IOAddress(subnet_prefix), 24, 30, 40, 60, + subnet_id)); + EXPECT_NO_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), subnet)); + // Check that options are gone. + EXPECT_EQ(0, countRows("dhcp4_pool")); + }; + + { + SCOPED_TRACE("update subnet, modify subnet id"); + // Create another subnet with the same prefix as the original subnet but + // different id. This is legal to update the subnet id if the prefix is + // stable. However, the new subnet has no address pools, so we need to + // check of the pools associated with the existing subnet instance are + // gone after the update. + test_subnet_update("192.0.2.0", 2048); + } + + { + SCOPED_TRACE("update subnet, modify prefix"); + // Create a subnet with the same subnet id but different prefix. + // The prefix should be updated. + test_subnet_update("192.0.3.0", 1024); + } +} + // Test that deleting a subnet triggers deletion of the options associated // with the subnet and pools. TEST_F(MySqlConfigBackendDHCPv4Test, subnetOptions) { diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc index 80975f3b86..b49f7520e4 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc @@ -2007,6 +2007,47 @@ TEST_F(MySqlConfigBackendDHCPv6Test, getSharedNetworkSubnets6) { EXPECT_TRUE(isEquivalent(returned_list, test_list)); } +// Test that pools are properly updated as a result a subnet update. +TEST_F(MySqlConfigBackendDHCPv6Test, subnetUpdatePools) { + + auto test_subnet_update = [this](const std::string& subnet_prefix, + const SubnetID& subnet_id) { + // Add the subnet with two address pools and two prefix delegation + // pools. + EXPECT_NO_THROW(cbptr_->createUpdateSubnet6(ServerSelector::ALL(), + test_subnets_[0])); + // Make sure that the pools have been added to the database. + EXPECT_EQ(2, countRows("dhcp6_pool")); + EXPECT_EQ(2, countRows("dhcp6_pd_pool")); + + // Create the subnet without options which updates the existing + // subnet. + Subnet6Ptr subnet(new Subnet6(IOAddress(subnet_prefix), 64, 30, 60, 50, 60, + subnet_id)); + EXPECT_NO_THROW(cbptr_->createUpdateSubnet6(ServerSelector::ALL(), subnet)); + // Check that options are gone. + EXPECT_EQ(0, countRows("dhcp6_pool")); + EXPECT_EQ(0, countRows("dhcp6_pd_pool")); + }; + + { + SCOPED_TRACE("update subnet, modify subnet id"); + // Create another subnet with the same prefix as the original subnet but + // different id. This is legal to update the subnet id if the prefix is + // stable. However, the new subnet has no address pools, so we need to + // check of the pools associated with the existing subnet instance are + // gone after the update. + test_subnet_update("2001:db8::", 2048); + } + + { + SCOPED_TRACE("update subnet, modify prefix"); + // Create a subnet with the same subnet id but different prefix. + // The prefix should be updated. + test_subnet_update("2001:db9::", 1024); + } +} + // Test that deleting a subnet triggers deletion of the options associated // with the subnet and pools. TEST_F(MySqlConfigBackendDHCPv6Test, subnetOptions) {