From: Marcin Siodelski Date: Thu, 13 Jun 2019 14:54:29 +0000 (+0200) Subject: [#642,!373] Global parameters may be associated with server tags. X-Git-Tag: Kea-1.6.0-beta2~178 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3b6df1cffb3f35a18ff92e30c6b9cb24fce9bb4a;p=thirdparty%2Fkea.git [#642,!373] Global parameters may be associated with server tags. --- diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index f606a71ac9..2d86f301f9 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -250,21 +250,79 @@ MySqlConfigBackendImpl::getGlobalParameters(const int index, MySqlBinding::createString(SERVER_TAG_BUF_LENGTH) // server_tag }; + // This remembers id of the last entry processed. + uint64_t last_id = 0; + conn_.selectQuery(index, in_bindings, out_bindings, - [¶meters] (MySqlBindingCollection& out_bindings) { - if (!out_bindings[1]->getString().empty()) { - - // Convert value read as string from the database to the actual - // data type known from the database as binding #3. - StampedValuePtr stamped_value = - StampedValue::create(out_bindings[1]->getString(), - out_bindings[2]->getString(), - static_cast - (out_bindings[3]->getInteger())); - - stamped_value->setModificationTime(out_bindings[4]->getTimestamp()); - stamped_value->setServerTag(out_bindings[5]->getString()); - parameters.insert(stamped_value); + [&last_id, ¶meters] (MySqlBindingCollection& out_bindings) { + + // Store id of the current entry. + uint64_t id = out_bindings[0]->getInteger(); + + // If we're starting or if this is new parameter being processed... + if ((last_id == 0) || (last_id != id)) { + + // parameter name + std::string name = out_bindings[1]->getString(); + + if (!name.empty()) { + + // Remember last processed entry. + last_id = id; + + // The server may use one of the two values present in the + // database, i.e. the value specified for the particular + // server tag or the value specified for all servers. The + // former takes precedence. Therefore, we check here if the + // value already present in the parameters collection is + // specified for all servers or selected server. + auto& index = parameters.get(); + auto existing = index.find(name); + if (existing != index.end()) { + // The value of this parameter has been already seen. + // Let's check if the value we stored is for all + // servers or one server. + try { + ServerTag existing_tag((*existing)->getServerTag()); + ServerTag new_tag(out_bindings[5]->getString()); + if (!existing_tag.amAll() && new_tag.amAll()) { + // The stored value is for one server and the + // currently fetched value is for all servers, + // so let's drop the currently processed value. + return; + } + + } catch (...) { + // This shouldn't occur because the value comes + // from the database and should have been validated. + return; + } + } + + // Convert value read as string from the database to the actual + // data type known from the database as binding #3. + StampedValuePtr stamped_value = + StampedValue::create(out_bindings[1]->getString(), + out_bindings[2]->getString(), + static_cast + (out_bindings[3]->getInteger())); + + stamped_value->setId(id); + stamped_value->setModificationTime(out_bindings[4]->getTimestamp()); + stamped_value->setServerTag(out_bindings[5]->getString()); + + // If the parameter is already stored, it means that the + // stored value is for all servers and the one we're fetching + // is for a server tag. Let's replace the value. + if (existing != index.end()) { + parameters.replace(existing, stamped_value); + + } else { + // The parameter doesn't exist, so insert it whether + // it is for all or one server. + parameters.insert(stamped_value); + } + } } }); } 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 88dace1cc4..fc1a808b7d 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 @@ -638,6 +638,117 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteGlobalParameter4) { } } +// This test verifies that it is possible to differentiate between the +// global parameters by server tag and that the value specified for the +// particular server overrides the value specified for all servers. +TEST_F(MySqlConfigBackendDHCPv4Test, globalParameters4WithServerTags) { + // Create three global parameters having the same name. + StampedValuePtr global_parameter1 = StampedValue::create("global", "value1"); + StampedValuePtr global_parameter2 = StampedValue::create("global", "value2"); + StampedValuePtr global_parameter3 = StampedValue::create("global", "value3"); + + // Try to insert one of them and associate with non-existing server. + // This should fail because the server must be inserted first. + EXPECT_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ONE("server1"), + global_parameter1), + DbOperationError); + + // Create two servers. + EXPECT_NO_THROW(cbptr_->createUpdateServer4(test_servers_[1])); + EXPECT_NO_THROW(cbptr_->createUpdateServer4(test_servers_[2])); + + // This time inserting the global parameters for the server1 and server2 should + // be successful. + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ONE("server1"), + global_parameter1)); + + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ONE("server2"), + global_parameter2)); + + // The last parameter is associated with all servers. + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ALL(), + global_parameter3)); + + StampedValuePtr returned_global; + + // Try to fetch the value specified for all servers. + EXPECT_NO_THROW( + returned_global = cbptr_->getGlobalParameter4(ServerSelector::ALL(), + "global") + ); + ASSERT_TRUE(returned_global); + EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); + EXPECT_EQ("all", returned_global->getServerTag()); + + // Try to fetch the value specified for the server1. This should override the + // value specified for all servers. + EXPECT_NO_THROW( + returned_global = cbptr_->getGlobalParameter4(ServerSelector::ONE("server1"), + "global") + ); + ASSERT_TRUE(returned_global); + EXPECT_EQ(global_parameter1->getValue(), returned_global->getValue()); + EXPECT_EQ("server1", returned_global->getServerTag()); + + // The same in case of the server2. + EXPECT_NO_THROW( + returned_global = cbptr_->getGlobalParameter4(ServerSelector::ONE("server2"), + "global") + ); + ASSERT_TRUE(returned_global); + EXPECT_EQ(global_parameter2->getValue(), returned_global->getValue()); + EXPECT_EQ("server2", returned_global->getServerTag()); + + StampedValueCollection returned_globals; + + // Try to fetch the collection of globals for the server2. It should contain + // server specific values. + EXPECT_NO_THROW( + returned_globals = cbptr_->getAllGlobalParameters4(ServerSelector::ONE("server2")) + ); + ASSERT_EQ(1, returned_globals.size()); + returned_global = *returned_globals.begin(); + EXPECT_EQ(global_parameter2->getValue(), returned_global->getValue()); + EXPECT_EQ("server2", returned_global->getServerTag()); + + // Try to fetch the collection of global parameters specified for all servers. + // This excludes the values specific to server1 and server2. It returns only the + // common ones. + EXPECT_NO_THROW( + returned_globals = cbptr_->getAllGlobalParameters4(ServerSelector::ALL()) + ); + ASSERT_EQ(1, returned_globals.size()); + returned_global = *returned_globals.begin(); + EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); + EXPECT_EQ("all", returned_global->getServerTag()); + + // Delete the server1. It should remove associations of this server with the + // global parameters. + EXPECT_NO_THROW(cbptr_->deleteServer4(ServerTag("server1"))); + EXPECT_NO_THROW( + returned_globals = cbptr_->getAllGlobalParameters4(ServerSelector::ONE("server1")) + ); + ASSERT_EQ(1, returned_globals.size()); + returned_global = *returned_globals.begin(); + // As a result, the value fetched for the server1 should be the one available for + // all servers, rather than the one dedicated for server1. The association of + // the server1 specific value with the server1 should be gone. + EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); + EXPECT_EQ("all", returned_global->getServerTag()); + + // Delete all servers, except 'all'. + EXPECT_NO_THROW(cbptr_->deleteAllServers4()); + EXPECT_NO_THROW( + returned_globals = cbptr_->getAllGlobalParameters4(ServerSelector::ALL()) + ); + ASSERT_EQ(1, returned_globals.size()); + returned_global = *returned_globals.begin(); + // The common value for all servers should still be available because 'all' + // logical server should not be deleted. + EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); + EXPECT_EQ("all", returned_global->getServerTag()); +} + // This test verifies that all global parameters can be retrieved and deleted. TEST_F(MySqlConfigBackendDHCPv4Test, getAllGlobalParameters4) { // Create 3 parameters and put them into the database. 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 9c76c05050..e845a471bb 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 @@ -678,6 +678,117 @@ TEST_F(MySqlConfigBackendDHCPv6Test, createUpdateDeleteGlobalParameter6) { } } +// This test verifies that it is possible to differentiate between the +// global parameters by server tag and that the value specified for the +// particular server overrides the value specified for all servers. +TEST_F(MySqlConfigBackendDHCPv6Test, globalParameters6WithServerTags) { + // Create three global parameters having the same name. + StampedValuePtr global_parameter1 = StampedValue::create("global", "value1"); + StampedValuePtr global_parameter2 = StampedValue::create("global", "value2"); + StampedValuePtr global_parameter3 = StampedValue::create("global", "value3"); + + // Try to insert one of them and associate with non-existing server. + // This should fail because the server must be inserted first. + EXPECT_THROW(cbptr_->createUpdateGlobalParameter6(ServerSelector::ONE("server1"), + global_parameter1), + DbOperationError); + + // Create two servers. + EXPECT_NO_THROW(cbptr_->createUpdateServer6(test_servers_[1])); + EXPECT_NO_THROW(cbptr_->createUpdateServer6(test_servers_[2])); + + // This time inserting the global parameters for the server1 and server2 should + // be successful. + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter6(ServerSelector::ONE("server1"), + global_parameter1)); + + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter6(ServerSelector::ONE("server2"), + global_parameter2)); + + // The last parameter is associated with all servers. + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter6(ServerSelector::ALL(), + global_parameter3)); + + StampedValuePtr returned_global; + + // Try to fetch the value specified for all servers. + EXPECT_NO_THROW( + returned_global = cbptr_->getGlobalParameter6(ServerSelector::ALL(), + "global") + ); + ASSERT_TRUE(returned_global); + EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); + EXPECT_EQ("all", returned_global->getServerTag()); + + // Try to fetch the value specified for the server1. This should override the + // value specified for all servers. + EXPECT_NO_THROW( + returned_global = cbptr_->getGlobalParameter6(ServerSelector::ONE("server1"), + "global") + ); + ASSERT_TRUE(returned_global); + EXPECT_EQ(global_parameter1->getValue(), returned_global->getValue()); + EXPECT_EQ("server1", returned_global->getServerTag()); + + // The same in case of the server2. + EXPECT_NO_THROW( + returned_global = cbptr_->getGlobalParameter6(ServerSelector::ONE("server2"), + "global") + ); + ASSERT_TRUE(returned_global); + EXPECT_EQ(global_parameter2->getValue(), returned_global->getValue()); + EXPECT_EQ("server2", returned_global->getServerTag()); + + StampedValueCollection returned_globals; + + // Try to fetch the collection of globals for the server2. It should contain + // server specific values. + EXPECT_NO_THROW( + returned_globals = cbptr_->getAllGlobalParameters6(ServerSelector::ONE("server2")) + ); + ASSERT_EQ(1, returned_globals.size()); + returned_global = *returned_globals.begin(); + EXPECT_EQ(global_parameter2->getValue(), returned_global->getValue()); + EXPECT_EQ("server2", returned_global->getServerTag()); + + // Try to fetch the collection of global parameters specified for all servers. + // This excludes the values specific to server1 and server2. It returns only the + // common ones. + EXPECT_NO_THROW( + returned_globals = cbptr_->getAllGlobalParameters6(ServerSelector::ALL()) + ); + ASSERT_EQ(1, returned_globals.size()); + returned_global = *returned_globals.begin(); + EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); + EXPECT_EQ("all", returned_global->getServerTag()); + + // Delete the server1. It should remove associations of this server with the + // global parameters. + EXPECT_NO_THROW(cbptr_->deleteServer6(ServerTag("server1"))); + EXPECT_NO_THROW( + returned_globals = cbptr_->getAllGlobalParameters6(ServerSelector::ONE("server1")) + ); + ASSERT_EQ(1, returned_globals.size()); + returned_global = *returned_globals.begin(); + // As a result, the value fetched for the server1 should be the one available for + // all servers, rather than the one dedicated for server1. The association of + // the server1 specific value with the server1 should be gone. + EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); + EXPECT_EQ("all", returned_global->getServerTag()); + + // Delete all servers, except 'all'. + EXPECT_NO_THROW(cbptr_->deleteAllServers6()); + EXPECT_NO_THROW( + returned_globals = cbptr_->getAllGlobalParameters6(ServerSelector::ALL()) + ); + ASSERT_EQ(1, returned_globals.size()); + returned_global = *returned_globals.begin(); + // The common value for all servers should still be available because 'all' + // logical server should not be deleted. + EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); + EXPECT_EQ("all", returned_global->getServerTag()); +} + // This test verifies that all global parameters can be retrieved and deleted. TEST_F(MySqlConfigBackendDHCPv6Test, getAllGlobalParameters6) { // Create 3 parameters and put them into the database. diff --git a/src/lib/database/server_selector.h b/src/lib/database/server_selector.h index c53a235ae4..909044ca7a 100644 --- a/src/lib/database/server_selector.h +++ b/src/lib/database/server_selector.h @@ -53,30 +53,30 @@ public: }; /// @brief Factory returning "unassigned" server selector. - static ServerSelector& UNASSIGNED() { - static ServerSelector selector(Type::UNASSIGNED); + static ServerSelector UNASSIGNED() { + ServerSelector selector(Type::UNASSIGNED); return (selector); } /// @brief Factory returning "all servers" selector. - static ServerSelector& ALL() { - static ServerSelector selector(Type::ALL); + static ServerSelector ALL() { + ServerSelector selector(Type::ALL); return (selector); } /// @brief Factory returning selector of one server. /// /// @param server_tag tag of the single server to be selected. - static ServerSelector& ONE(const std::string& server_tag) { - static ServerSelector selector(server_tag); + static ServerSelector ONE(const std::string& server_tag) { + ServerSelector selector(server_tag); return (selector); } /// @brief Factory returning "multiple servers" selector. /// /// @param server_tags set of server tags to be selected. - static ServerSelector& MULTIPLE(const std::set& server_tags) { - static ServerSelector selector(server_tags); + static ServerSelector MULTIPLE(const std::set& server_tags) { + ServerSelector selector(server_tags); return (selector); } diff --git a/src/lib/process/cb_ctl_base.h b/src/lib/process/cb_ctl_base.h index 7602c894e0..e068efa158 100644 --- a/src/lib/process/cb_ctl_base.h +++ b/src/lib/process/cb_ctl_base.h @@ -183,7 +183,7 @@ public: // Use the server_tag if set, otherwise use ALL. std::string server_tag = srv_cfg->getServerTag(); - db::ServerSelector& server_selector = + db::ServerSelector server_selector = (server_tag.empty()? db::ServerSelector::ALL() : db::ServerSelector::ONE(server_tag)); // This collection will hold the audit entries since the last update if