From: Thomas Markwalder Date: Tue, 4 Feb 2020 15:01:48 +0000 (-0500) Subject: [#1110] Reset sbu-component trackers during CB data fetches X-Git-Tag: Kea-1.6.2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=31ef2b60e0193b52ac3de52b46b5f66bf7d24b1a;p=thirdparty%2Fkea.git [#1110] Reset sbu-component trackers during CB data fetches Backports #1093 to v1_6_0 modified: ChangeLog src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc --- diff --git a/ChangeLog b/ChangeLog index afa734d36b..93434b2fcb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +1664. [bug] tmark + Corrected an issue in the MySQL CB hook library which could + cause subnet and shared-network options, properly added to + the CB database, to be discarded when fetched from the backend. + (Gitlab #1110,#1093) + 1663. [func] tmark Client supplied ciaddr is now sent back when responding to DHCPINFORM diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index bec9a1ccf0..771f3feb94 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018-2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2020 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 @@ -337,11 +337,12 @@ public: // row, it means that we have to construct new subnet object. if (!last_subnet || (last_subnet->getID() != out_bindings[0]->getInteger())) { - // Reset pool id, because current row defines new subnet. Subsequent - // rows will contain pool information. + // Reset per subnet component tracking and server tag because + // we're now starting to process a new subnet. last_pool_id = 0; - - // Reset last server tag as we're now starting to process new subnet. + last_pool_option_id = 0; + last_option_id = 0; + last_pool.reset(); last_tag.clear(); // subnet_id @@ -1221,8 +1222,9 @@ public: // row to create the new shared network instance. if (last_network_id != out_bindings[0]->getInteger()) { - // Reset last server tag as we're now starting to process new - // shared network. + // Reset per shared network subnet component tracking and server tag because + // we're now starting to process a new shared network. + last_option_id = 0; last_tag.clear(); last_network_id = out_bindings[0]->getInteger(); diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc index 14ffaf363c..5737b85695 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018-2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2020 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 @@ -375,12 +375,15 @@ public: // row, it means that we have to construct new subnet object. if (!last_subnet || (last_subnet->getID() != out_bindings[0]->getInteger())) { - // Reset pool ids, because current row defines new subnet. Subsequent - // rows will contain pool information. + // Reset per subnet component tracking and server tag because + // we're now starting to process a new subnet. last_pool_id = 0; last_pd_pool_id = 0; - - // Reset last server tag as we're now starting to process new subnet. + last_pool_option_id = 0; + last_pd_pool_option_id = 0; + last_option_id = 0; + last_pool.reset(); + last_pd_pool.reset(); last_tag.clear(); // subnet_id (0) @@ -1545,8 +1548,9 @@ public: // row to create the new shared network instance. if (last_network_id != out_bindings[0]->getInteger()) { - // Reset last server tag as we're now starting to process new - // shared network. + // Reset per shared network component tracking and server tag because + // we're now starting to process a new shared network. + last_option_id = 0; last_tag.clear(); last_network_id = out_bindings[0]->getInteger(); 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 55d9e6a227..82ab1c6728 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 @@ -1,4 +1,4 @@ -// Copyright (C) 2018-2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2020 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 @@ -3943,5 +3943,101 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSharedNetworkOption4) { EXPECT_EQ(0, countRows("dhcp4_options")); } +// This test verifies that option id values in one subnet do +// not impact options returned in subsequent subnets when +// fetching subnets from the backend. +TEST_F(MySqlConfigBackendDHCPv4Test, subnetOptionIdOrder) { + + // Add a subnet with two pools with one option each. + EXPECT_NO_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), test_subnets_[1])); + EXPECT_EQ(2, countRows("dhcp4_pool")); + EXPECT_EQ(2, countRows("dhcp4_options")); + + // Add a second subnet with a single option. The number of options in the database + // should now be 3. + EXPECT_NO_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), test_subnets_[2])); + EXPECT_EQ(2, countRows("dhcp4_pool")); + EXPECT_EQ(3, countRows("dhcp4_options")); + + // Now replace the first subnet with three options and two pools. This will cause + // the option id values for this subnet to be larger than those in the second + // subnet. + EXPECT_NO_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), test_subnets_[0])); + EXPECT_EQ(2, countRows("dhcp4_pool")); + EXPECT_EQ(4, countRows("dhcp4_options")); + + // Now fetch all subnets. + Subnet4Collection subnets; + EXPECT_NO_THROW(subnets = cbptr_->getAllSubnets4(ServerSelector::ALL())); + ASSERT_EQ(2, subnets.size()); + + // Verify that the subnets returned are as expected. + for (auto subnet : subnets) { + ASSERT_EQ(1, subnet->getServerTags().size()); + EXPECT_EQ("all", subnet->getServerTags().begin()->get()); + if (subnet->getID() == 1024) { + EXPECT_EQ(test_subnets_[0]->toElement()->str(), subnet->toElement()->str()); + } else if (subnet->getID() == 2048) { + EXPECT_EQ(test_subnets_[2]->toElement()->str(), subnet->toElement()->str()); + } else { + ADD_FAILURE() << "unexpected subnet id:" << subnet->getID(); + } + } +} + +// This test verifies that option id values in one shared network do +// not impact options returned in subsequent shared networks when +// fetching shared networks from the backend. +TEST_F(MySqlConfigBackendDHCPv4Test, sharedNetworkOptionIdOrder) { + auto level1_options = test_networks_[0]; + auto level1_no_options = test_networks_[1]; + auto level2 = test_networks_[2]; + + // Insert two shared networks. We insert level1 without options first, + // then level2. + EXPECT_NO_THROW(cbptr_->createUpdateSharedNetwork4(ServerSelector::ALL(), + level1_no_options)); + + EXPECT_NO_THROW(cbptr_->createUpdateSharedNetwork4(ServerSelector::ALL(), + level2)); + // Fetch all shared networks. + SharedNetwork4Collection networks = + cbptr_->getAllSharedNetworks4(ServerSelector::ALL()); + + ASSERT_EQ(2, networks.size()); + + // See if shared networks are returned ok. + for (auto i = 0; i < networks.size(); ++i) { + if (i == 0) { + // level1_no_options + EXPECT_EQ(level1_no_options->toElement()->str(), + networks[i]->toElement()->str()); + } else { + // bar + EXPECT_EQ(level2->toElement()->str(), + networks[i]->toElement()->str()); + } + } + + EXPECT_NO_THROW(cbptr_->createUpdateSharedNetwork4(ServerSelector::ALL(), + level1_options)); + + // Fetch all shared networks. + networks = cbptr_->getAllSharedNetworks4(ServerSelector::ALL()); + ASSERT_EQ(2, networks.size()); + + // See if shared networks are returned ok. + for (auto i = 0; i < networks.size(); ++i) { + if (i == 0) { + // level1_no_options + EXPECT_EQ(level1_options->toElement()->str(), + networks[i]->toElement()->str()); + } else { + // bar + EXPECT_EQ(level2->toElement()->str(), + networks[i]->toElement()->str()); + } + } +} } 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 b5a8e38358..a7c93d9498 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 @@ -1,4 +1,4 @@ -// Copyright (C) 2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2019-2020 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 @@ -4118,4 +4118,101 @@ TEST_F(MySqlConfigBackendDHCPv6Test, createUpdateDeleteSharedNetworkOption6) { EXPECT_EQ(0, countRows("dhcp6_options")); } +// This test verifies that option id values in one subnet do +// not impact options returned in subsequent subnets when +// fetching subnets from the backend. +TEST_F(MySqlConfigBackendDHCPv6Test, subnetOptionIdOrder) { + + // Add a network with two pools with two options each. + EXPECT_NO_THROW(cbptr_->createUpdateSubnet6(ServerSelector::ALL(), test_subnets_[1])); + EXPECT_EQ(2, countRows("dhcp6_pool")); + EXPECT_EQ(4, countRows("dhcp6_options")); + + // Add second subnet with a single option. The number of options in the database + // should now be 3. + EXPECT_NO_THROW(cbptr_->createUpdateSubnet6(ServerSelector::ALL(), test_subnets_[2])); + EXPECT_EQ(2, countRows("dhcp6_pool")); + EXPECT_EQ(5, countRows("dhcp6_options")); + + // Now replace the first subnet with three options and two pools. This will cause + // the option id values for this subnet to be larger than those in the second + // subnet. + EXPECT_NO_THROW(cbptr_->createUpdateSubnet6(ServerSelector::ALL(), test_subnets_[0])); + EXPECT_EQ(2, countRows("dhcp6_pool")); + EXPECT_EQ(4, countRows("dhcp6_options")); + + // Now fetch all subnets. + Subnet6Collection subnets; + EXPECT_NO_THROW(subnets = cbptr_->getAllSubnets6(ServerSelector::ALL())); + ASSERT_EQ(2, subnets.size()); + + // Verify that the subnets returned are as expected. + for (auto subnet : subnets) { + ASSERT_EQ(1, subnet->getServerTags().size()); + EXPECT_EQ("all", subnet->getServerTags().begin()->get()); + if (subnet->getID() == 1024) { + EXPECT_EQ(test_subnets_[0]->toElement()->str(), subnet->toElement()->str()); + } else if (subnet->getID() == 2048) { + EXPECT_EQ(test_subnets_[2]->toElement()->str(), subnet->toElement()->str()); + } else { + ADD_FAILURE() << "unexpected subnet id:" << subnet->getID(); + } + } +} + +// This test verifies that option id values in one shared network do +// not impact options returned in subsequent shared networks when +// fetching shared networks from the backend. +TEST_F(MySqlConfigBackendDHCPv6Test, sharedNetworkOptionIdOrder) { + auto level1_options = test_networks_[0]; + auto level1_no_options = test_networks_[1]; + auto level2 = test_networks_[2]; + + // Insert two shared networks. We insert level1 without options first, + // then level2. + EXPECT_NO_THROW(cbptr_->createUpdateSharedNetwork6(ServerSelector::ALL(), + level1_no_options)); + + EXPECT_NO_THROW(cbptr_->createUpdateSharedNetwork6(ServerSelector::ALL(), + level2)); + // Fetch all shared networks. + SharedNetwork6Collection networks = + cbptr_->getAllSharedNetworks6(ServerSelector::ALL()); + + ASSERT_EQ(2, networks.size()); + + // See if shared networks are returned ok. + for (auto i = 0; i < networks.size(); ++i) { + if (i == 0) { + // level1_no_options + EXPECT_EQ(level1_no_options->toElement()->str(), + networks[i]->toElement()->str()); + } else { + // bar + EXPECT_EQ(level2->toElement()->str(), + networks[i]->toElement()->str()); + } + } + + EXPECT_NO_THROW(cbptr_->createUpdateSharedNetwork6(ServerSelector::ALL(), + level1_options)); + + // Fetch all shared networks. + networks = cbptr_->getAllSharedNetworks6(ServerSelector::ALL()); + ASSERT_EQ(2, networks.size()); + + // See if shared networks are returned ok. + for (auto i = 0; i < networks.size(); ++i) { + if (i == 0) { + // level1_no_options + EXPECT_EQ(level1_options->toElement()->str(), + networks[i]->toElement()->str()); + } else { + // bar + EXPECT_EQ(level2->toElement()->str(), + networks[i]->toElement()->str()); + } + } +} + }