From: Marcin Siodelski Date: Wed, 26 Jun 2019 15:43:56 +0000 (+0200) Subject: [#642,!373] Delete global parameters as a result of deleting the servers. X-Git-Tag: Kea-1.6.0-beta2~162 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=77742cdc9d019dcc4c9dbfa2430b2db32b47b8b2;p=thirdparty%2Fkea.git [#642,!373] Delete global parameters as a result of deleting the servers. --- diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index 37ba0d069b..cd4f931941 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -104,6 +104,7 @@ public: UPDATE_SERVER4, DELETE_GLOBAL_PARAMETER4, DELETE_ALL_GLOBAL_PARAMETERS4, + DELETE_ALL_GLOBAL_PARAMETERS4_UNASSIGNED, DELETE_SUBNET4_ID, DELETE_SUBNET4_PREFIX, DELETE_ALL_SUBNETS4, @@ -1893,6 +1894,86 @@ public: in_bindings)); } + /// @brief Attempts to delete a server having a given tag. + /// + /// @param server_tag Tag of the server to be deleted. + /// @return Number of deleted servers. + /// @throw isc::InvalidOperation when trying to delete the logical + /// server 'all'. + uint64_t deleteServer4(const data::ServerTag& server_tag) { + // It is not allowed to delete 'all' logical server. + if (server_tag.amAll()) { + isc_throw(InvalidOperation, "'all' is a name reserved for the server tag which" + " associates the configuration elements with all servers connecting" + " to the database and can't be deleted"); + } + + MySqlTransaction transaction(conn_); + + // Create scoped audit revision. As long as this instance exists + // no new audit revisions are created in any subsequent calls. + ScopedAuditRevision + audit_revision(this, MySqlConfigBackendDHCPv4Impl::CREATE_AUDIT_REVISION, + ServerSelector::ALL(), "deleting a server", false); + + // Specify which server should be deleted. + MySqlBindingCollection in_bindings = { + MySqlBinding::createString(server_tag.get()) + }; + + // Attempt to delete the server. + auto count = conn_.updateDeleteQuery(MySqlConfigBackendDHCPv4Impl::DELETE_SERVER4, + in_bindings); + + // If we have deleted any servers we have to remove any dangling global + // parameters. + if (count > 0) { + conn_.updateDeleteQuery(MySqlConfigBackendDHCPv4Impl:: + DELETE_ALL_GLOBAL_PARAMETERS4_UNASSIGNED, + MySqlBindingCollection()); + /// @todo delete dangling options and option definitions. + } + + transaction.commit(); + + return (count); + } + + /// @brief Attempts to delete all servers. + /// + /// This method deletes all servers added by the user. It does not + /// delete the logical server 'all'. + /// + /// @return Number of deleted servers. + uint64_t deleteAllServers4() { + MySqlTransaction transaction(conn_); + + // Create scoped audit revision. As long as this instance exists + // no new audit revisions are created in any subsequent calls. + ScopedAuditRevision + audit_revision(this, MySqlConfigBackendDHCPv4Impl::CREATE_AUDIT_REVISION, + ServerSelector::ALL(), "deleting all servers", + false); + + MySqlBindingCollection in_bindings; + + // Attempt to delete the servers. + auto count = conn_.updateDeleteQuery(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_SERVERS4, + in_bindings); + + // If we have deleted any servers we have to remove any dangling global + // parameters. + if (count > 0) { + conn_.updateDeleteQuery(MySqlConfigBackendDHCPv4Impl:: + DELETE_ALL_GLOBAL_PARAMETERS4_UNASSIGNED, + MySqlBindingCollection()); + /// @todo delete dangling options and option definitions. + } + + transaction.commit(); + + return (count); + } }; namespace { @@ -2260,6 +2341,11 @@ TaggedStatementArray tagged_statements = { { MYSQL_DELETE_GLOBAL_PARAMETER(dhcp4) }, + // Delete all global parameters which are unassigned to any servers. + { MySqlConfigBackendDHCPv4Impl::DELETE_ALL_GLOBAL_PARAMETERS4_UNASSIGNED, + MYSQL_DELETE_GLOBAL_PARAMETER_UNASSIGNED(dhcp4) + }, + // Delete subnet by id. { MySqlConfigBackendDHCPv4Impl::DELETE_SUBNET4_ID, MYSQL_DELETE_SUBNET(dhcp4, AND s.subnet_id = ?) @@ -2844,9 +2930,7 @@ uint64_t MySqlConfigBackendDHCPv4::deleteServer4(const ServerTag& server_tag) { LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_SERVER4) .arg(server_tag.get()); - uint64_t result = impl_->deleteServer(MySqlConfigBackendDHCPv4Impl::CREATE_AUDIT_REVISION, - MySqlConfigBackendDHCPv4Impl::DELETE_SERVER4, - server_tag); + uint64_t result = impl_->deleteServer4(server_tag); LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_SERVER4_RESULT) .arg(result); return (result); @@ -2855,8 +2939,7 @@ MySqlConfigBackendDHCPv4::deleteServer4(const ServerTag& server_tag) { uint64_t MySqlConfigBackendDHCPv4::deleteAllServers4() { LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_ALL_SERVERS4); - uint64_t result = impl_->deleteAllServers(MySqlConfigBackendDHCPv4Impl::CREATE_AUDIT_REVISION, - MySqlConfigBackendDHCPv4Impl::DELETE_ALL_SERVERS4); + uint64_t result = impl_->deleteAllServers4(); LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_ALL_SERVERS4_RESULT) .arg(result); return (result); diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc index 67ffe7006f..2acacb0bf2 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc @@ -109,6 +109,7 @@ public: UPDATE_SERVER6, DELETE_GLOBAL_PARAMETER6, DELETE_ALL_GLOBAL_PARAMETERS6, + DELETE_ALL_GLOBAL_PARAMETERS6_UNASSIGNED, DELETE_SUBNET6_ID, DELETE_SUBNET6_PREFIX, DELETE_ALL_SUBNETS6, @@ -2206,6 +2207,87 @@ public: true, in_bindings)); } + + /// @brief Attempts to delete a server having a given tag. + /// + /// @param server_tag Tag of the server to be deleted. + /// @return Number of deleted servers. + /// @throw isc::InvalidOperation when trying to delete the logical + /// server 'all'. + uint64_t deleteServer6(const data::ServerTag& server_tag) { + // It is not allowed to delete 'all' logical server. + if (server_tag.amAll()) { + isc_throw(InvalidOperation, "'all' is a name reserved for the server tag which" + " associates the configuration elements with all servers connecting" + " to the database and can't be deleted"); + } + + MySqlTransaction transaction(conn_); + + // Create scoped audit revision. As long as this instance exists + // no new audit revisions are created in any subsequent calls. + ScopedAuditRevision + audit_revision(this, MySqlConfigBackendDHCPv6Impl::CREATE_AUDIT_REVISION, + ServerSelector::ALL(), "deleting a server", false); + + // Specify which server should be deleted. + MySqlBindingCollection in_bindings = { + MySqlBinding::createString(server_tag.get()) + }; + + // Attempt to delete the server. + auto count = conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl::DELETE_SERVER6, + in_bindings); + + // If we have deleted any servers we have to remove any dangling global + // parameters. + if (count > 0) { + conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl:: + DELETE_ALL_GLOBAL_PARAMETERS6_UNASSIGNED, + MySqlBindingCollection()); + /// @todo delete dangling options and option definitions. + } + + transaction.commit(); + + return (count); + } + + /// @brief Attempts to delete all servers. + /// + /// This method deletes all servers added by the user. It does not + /// delete the logical server 'all'. + /// + /// @return Number of deleted servers. + uint64_t deleteAllServers6() { + MySqlTransaction transaction(conn_); + + // Create scoped audit revision. As long as this instance exists + // no new audit revisions are created in any subsequent calls. + ScopedAuditRevision + audit_revision(this, MySqlConfigBackendDHCPv6Impl::CREATE_AUDIT_REVISION, + ServerSelector::ALL(), "deleting all servers", + false); + + MySqlBindingCollection in_bindings; + + // Attempt to delete the servers. + auto count = conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl::DELETE_ALL_SERVERS6, + in_bindings); + + // If we have deleted any servers we have to remove any dangling global + // parameters. + if (count > 0) { + conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl:: + DELETE_ALL_GLOBAL_PARAMETERS6_UNASSIGNED, + MySqlBindingCollection()); + /// @todo delete dangling options and option definitions. + } + + transaction.commit(); + + return (count); + } }; namespace { @@ -2236,6 +2318,11 @@ TaggedStatementArray tagged_statements = { { MYSQL_GET_GLOBAL_PARAMETER(dhcp6, AND g.modification_ts > ?) }, + // Delete all global parameters which are unassigned to any servers. + { MySqlConfigBackendDHCPv6Impl::DELETE_ALL_GLOBAL_PARAMETERS6_UNASSIGNED, + MYSQL_DELETE_GLOBAL_PARAMETER_UNASSIGNED(dhcp6) + }, + // Select subnet by id. { MySqlConfigBackendDHCPv6Impl::GET_SUBNET6_ID, MYSQL_GET_SUBNET6(AND s.subnet_id = ?) @@ -3231,9 +3318,7 @@ uint64_t MySqlConfigBackendDHCPv6::deleteServer6(const ServerTag& server_tag) { LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_SERVER6) .arg(server_tag.get()); - uint64_t result = impl_->deleteServer(MySqlConfigBackendDHCPv6Impl::CREATE_AUDIT_REVISION, - MySqlConfigBackendDHCPv6Impl::DELETE_SERVER6, - server_tag); + uint64_t result = impl_->deleteServer6(server_tag); LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_SERVER6_RESULT) .arg(result); return (result); @@ -3242,8 +3327,7 @@ MySqlConfigBackendDHCPv6::deleteServer6(const ServerTag& server_tag) { uint64_t MySqlConfigBackendDHCPv6::deleteAllServers6() { LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_ALL_SERVERS6); - uint64_t result = impl_->deleteAllServers(MySqlConfigBackendDHCPv6Impl::CREATE_AUDIT_REVISION, - MySqlConfigBackendDHCPv6Impl::DELETE_ALL_SERVERS6); + uint64_t result = impl_->deleteAllServers6(); LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_ALL_SERVERS6_RESULT) .arg(result); return (result); diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index 8e761e2e20..ae69445b85 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -968,59 +968,6 @@ MySqlConfigBackendImpl::createUpdateServer(const int& create_audit_revision, transaction.commit(); } -uint64_t -MySqlConfigBackendImpl::deleteServer(const int& create_audit_revision, - const int& delete_index, - const ServerTag& server_tag) { - - // It is not allowed to delete 'all' logical server. - if (server_tag.amAll()) { - isc_throw(InvalidOperation, "'all' is a name reserved for the server tag which" - " associates the configuration elements with all servers connecting" - " to the database and can't be deleted"); - } - - MySqlTransaction transaction(conn_); - - // Create scoped audit revision. As long as this instance exists - // no new audit revisions are created in any subsequent calls. - ScopedAuditRevision - audit_revision(this, create_audit_revision, - ServerSelector::ALL(), "deleting a server", false); - - // Specify which server should be deleted. - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(server_tag.get()) - }; - - // Attempt to delete the server. - auto count = conn_.updateDeleteQuery(delete_index, in_bindings); - transaction.commit(); - - return (count); -} - -uint64_t -MySqlConfigBackendImpl::deleteAllServers(const int& create_audit_revision, - const int& delete_index) { - - MySqlTransaction transaction(conn_); - - // Create scoped audit revision. As long as this instance exists - // no new audit revisions are created in any subsequent calls. - ScopedAuditRevision - audit_revision(this, create_audit_revision, - ServerSelector::ALL(), "deleting all servers", false); - - MySqlBindingCollection in_bindings; - - // Attempt to delete the servers. - auto count = conn_.updateDeleteQuery(delete_index, in_bindings); - transaction.commit(); - - return (count); -} - std::string MySqlConfigBackendImpl::getType() const { return ("mysql"); diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h index ad584b82c4..456ee26d4d 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h @@ -650,30 +650,6 @@ public: const int& update_index, const db::ServerPtr& server); - /// @brief Attempts to delete a server having a given tag. - /// - /// @param create_audit_revision Index of the query inserting audit - /// revision. - /// @param create_index Index of the DELETE query to be executed. - /// @param server_tag Tag of the server to be deleted. - /// @return Number of deleted servers. - /// @throw isc::InvalidOperation when trying to delete the logical - /// server 'all'. - uint64_t deleteServer(const int& create_audit_revision, const int& index, - const data::ServerTag& server_tag); - - /// @brief Attempts to delete all servers. - /// - /// This method deletes all servers added by the user. It does not - /// delete the logical server 'all'. - /// - /// @param create_audit_revision Index of the query inserting audit - /// revision. - /// @param server_tag Tag of the server to be deleted. - /// @return Number of deleted servers. - uint64_t deleteAllServers(const int& create_audit_revision, - const int& index); - /// @brief Returns backend type in the textual format. /// /// @return "mysql". 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 4f0180d521..1f7c9efb23 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h +++ b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h @@ -607,6 +607,14 @@ namespace { "WHERE s.tag = ? " #__VA_ARGS__ #endif +#ifndef MYSQL_DELETE_GLOBAL_PARAMETER_UNASSIGNED +#define MYSQL_DELETE_GLOBAL_PARAMETER_UNASSIGNED(table_prefix, ...) \ + "DELETE g FROM " #table_prefix "_global_parameter AS g " \ + "LEFT JOIN " #table_prefix "_global_parameter_server AS a " \ + " ON g.id = a.parameter_id " \ + "WHERE a.parameter_id IS NULL " #__VA_ARGS__ +#endif + #ifndef MYSQL_DELETE_SUBNET #define MYSQL_DELETE_SUBNET(table_prefix, ...) \ "DELETE s FROM " #table_prefix "_subnet AS s " \ 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 8175ad75ae..e0f0dc883e 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 @@ -336,10 +336,12 @@ public: /// @brief Logs audit entries in the @c audit_entries_ member. /// /// This function is called in case of an error. - std::string logExistingAuditEntries() { + /// + /// @param server_tag Server tag for which the audit entries should be logged. + std::string logExistingAuditEntries(const std::string& server_tag) { std::ostringstream s; - auto& mod_time_idx = audit_entries_.get(); + auto& mod_time_idx = audit_entries_[server_tag].get(); for (auto audit_entry_it = mod_time_idx.begin(); audit_entry_it != mod_time_idx.end(); @@ -366,31 +368,58 @@ public: /// @param exp_object_type Expected object type. /// @param exp_modification_time Expected modification time. /// @param exp_log_message Expected log message. + /// @param server_selector Server selector to be used for next query. /// @param new_entries_num Number of the new entries expected to be inserted. + /// @param max_tested_entries Maximum number of entries tested. void testNewAuditEntry(const std::string& exp_object_type, const AuditEntry::ModificationType& exp_modification_type, const std::string& exp_log_message, - const size_t new_entries_num = 1) { - auto audit_entries_size_save = audit_entries_.size(); - audit_entries_ = cbptr_->getRecentAuditEntries(ServerSelector::ALL(), - timestamps_["two days ago"]); - ASSERT_EQ(audit_entries_size_save + new_entries_num, audit_entries_.size()) - << logExistingAuditEntries(); + const ServerSelector& server_selector = ServerSelector::ALL(), + const size_t new_entries_num = 1, + const size_t max_tested_entries = 65535) { + + // Get the server tag for which the entries are fetched. + std::string tag; + if (server_selector.getType() == ServerSelector::Type::ALL) { + // Server tag is 'all'. + tag = "all"; + + } else { + auto tags = server_selector.getTags(); + // This test is not meant to handle multiple server tags all at once. + if (tags.size() > 1) { + ADD_FAILURE() << "Test error: do not use multiple server tags"; + + } else if (tags.size() == 1) { + // Get the server tag for which we run the current test. + tag = *tags.begin(); + } + } - auto& mod_time_idx = audit_entries_.get(); + auto audit_entries_size_save = audit_entries_[tag].size(); + + // Audit entries for different server tags are stored in separate + // containers. + audit_entries_[tag] = cbptr_->getRecentAuditEntries(server_selector, + timestamps_["two days ago"]); + ASSERT_EQ(audit_entries_size_save + new_entries_num, audit_entries_[tag].size()) + << logExistingAuditEntries(tag); + + auto& mod_time_idx = audit_entries_[tag].get(); // Iterate over specified number of entries starting from the most recent // one and check they have correct values. for (auto audit_entry_it = mod_time_idx.rbegin(); - std::distance(mod_time_idx.rbegin(), audit_entry_it) < new_entries_num; + ((std::distance(mod_time_idx.rbegin(), audit_entry_it) < new_entries_num) && + (std::distance(mod_time_idx.rbegin(), audit_entry_it) < max_tested_entries)); ++audit_entry_it) { auto audit_entry = *audit_entry_it; EXPECT_EQ(exp_object_type, audit_entry->getObjectType()) - << logExistingAuditEntries(); + << logExistingAuditEntries(tag); EXPECT_EQ(exp_modification_type, audit_entry->getModificationType()) - << logExistingAuditEntries(); + << logExistingAuditEntries(tag); EXPECT_EQ(exp_log_message, audit_entry->getLogMessage()) - << logExistingAuditEntries(); + << logExistingAuditEntries(tag); } } @@ -416,7 +445,7 @@ public: boost::shared_ptr cbptr_; /// @brief Holds the most recent audit entries. - AuditEntryCollection audit_entries_; + std::map audit_entries_; }; // This test verifies that the expected backend type is returned. @@ -669,19 +698,64 @@ TEST_F(MySqlConfigBackendDHCPv4Test, globalParameters4WithServerTags) { // Create two servers. EXPECT_NO_THROW(cbptr_->createUpdateServer4(test_servers_[1])); + { + SCOPED_TRACE("server1 is created"); + testNewAuditEntry("dhcp4_server", + AuditEntry::ModificationType::CREATE, + "server set"); + } + EXPECT_NO_THROW(cbptr_->createUpdateServer4(test_servers_[2])); + { + SCOPED_TRACE("server2 is created"); + testNewAuditEntry("dhcp4_server", + AuditEntry::ModificationType::CREATE, + "server set"); + } // This time inserting the global parameters for the server1 and server2 should // be successful. EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ONE("server1"), global_parameter1)); + { + SCOPED_TRACE("Global parameter for server1 is set"); + // The value of 3 means there should be 3 audit entries available for the + // server1, two that indicate creation of the servers and one that we + // validate, which sets the global value. + testNewAuditEntry("dhcp4_global_parameter", + AuditEntry::ModificationType::CREATE, + "global parameter set", + ServerSelector::ONE("server1"), + 3, 1); + } + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ONE("server2"), global_parameter2)); + { + SCOPED_TRACE("Global parameter for server2 is set"); + // Same as in case of the server2, there should be 3 audit entries of + // which one we validate. + testNewAuditEntry("dhcp4_global_parameter", + AuditEntry::ModificationType::CREATE, + "global parameter set", + ServerSelector::ONE("server2"), + 3, 1); + } // The last parameter is associated with all servers. EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ALL(), global_parameter3)); + { + SCOPED_TRACE("Global parameter for all servers is set"); + // There should be one new audit entry for all servers. It indicates + // the insertion of the global value. + testNewAuditEntry("dhcp4_global_parameter", + AuditEntry::ModificationType::CREATE, + "global parameter set", + ServerSelector::ALL(), + 1, 1); + } StampedValuePtr returned_global; @@ -755,7 +829,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, globalParameters4WithServerTags) { EXPECT_EQ("all", returned_global->getServerTags()[0].get()); // Delete the server1. It should remove associations of this server with the - // global parameters. + // global parameter and the global parameter itself. EXPECT_NO_THROW(cbptr_->deleteServer4(ServerTag("server1"))); EXPECT_NO_THROW( returned_globals = cbptr_->getAllGlobalParameters4(ServerSelector::ONE("server1")) @@ -769,6 +843,17 @@ TEST_F(MySqlConfigBackendDHCPv4Test, globalParameters4WithServerTags) { ASSERT_EQ(1, returned_global->getServerTags().size()); EXPECT_EQ("all", returned_global->getServerTags()[0].get()); + { + SCOPED_TRACE("DELETE audit entry for the global parameter after server deletion"); + // We expect two new audit entries for the server1, one indicating that the + // server has been deleted and another one indicating that the corresponding + // global value has been deleted. We check the latter entry. + testNewAuditEntry("dhcp4_global_parameter", + AuditEntry::ModificationType::DELETE, + "deleting a server", ServerSelector::ONE("server1"), + 2, 1); + } + // Attempt to delete global parameter for server1. uint64_t deleted_num = 0; EXPECT_NO_THROW(deleted_num = cbptr_->deleteGlobalParameter4(ServerSelector::ONE("server1"), @@ -782,6 +867,10 @@ TEST_F(MySqlConfigBackendDHCPv4Test, globalParameters4WithServerTags) { "global")); EXPECT_EQ(1, deleted_num); + // Create it again to test that deletion of all server removes this too. + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ONE("server2"), + global_parameter2)); + // Delete all servers, except 'all'. EXPECT_NO_THROW(deleted_num = cbptr_->deleteAllServers4()); EXPECT_NO_THROW( @@ -795,6 +884,18 @@ TEST_F(MySqlConfigBackendDHCPv4Test, globalParameters4WithServerTags) { EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); ASSERT_EQ(1, returned_global->getServerTags().size()); EXPECT_EQ("all", returned_global->getServerTags()[0].get()); + + { + SCOPED_TRACE("DELETE audit entry for the global parameter after deletion of" + " all servers"); + // There should be 4 new audit entries. One for deleting the global, one for + // re-creating it, one for deleting the server2 and one for deleting the + // global again as a result of deleting the server2. + testNewAuditEntry("dhcp4_global_parameter", + AuditEntry::ModificationType::DELETE, + "deleting all servers", ServerSelector::ONE("server2"), + 4, 1); + } } // This test verifies that all global parameters can be retrieved and deleted. @@ -1428,7 +1529,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSharedNetworks4) { // That shared network overrides the first one so the audit entry should // indicate an update. - if ((network->getName() == "level1") && (!audit_entries_.empty())) { + if ((network->getName() == "level1") && (!audit_entries_["all"].empty())) { SCOPED_TRACE("UPDATE audit entry for the shared network " + network->getName()); testNewAuditEntry("dhcp4_shared_network", @@ -1478,7 +1579,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSharedNetworks4) { SCOPED_TRACE("CREATE audit entry for subnets"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::CREATE, - "subnet set", 3); + "subnet set", ServerSelector::ALL(), 3); } // Deleting non-existing shared network should return 0. @@ -1536,7 +1637,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSharedNetworks4) { // The last parameter indicates that we expect two new audit entries. testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::DELETE, - "deleted all shared networks", 2); + "deleted all shared networks", ServerSelector::ALL(), 2); } // Check that subnets are still there but detached. @@ -1756,7 +1857,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllOptionDefs4) { // The last parameter indicates that we expect two new audit entries. testNewAuditEntry("dhcp4_option_def", AuditEntry::ModificationType::DELETE, - "deleted all option definitions", 2); + "deleted all option definitions", ServerSelector::ALL(), 2); } } 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 78ac3b5012..b07a5a6d06 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 @@ -377,10 +377,12 @@ public: /// @brief Logs audit entries in the @c audit_entries_ member. /// /// This function is called in case of an error. - std::string logExistingAuditEntries() { + /// + /// @param server_tag Server tag for which the audit entries should be logged. + std::string logExistingAuditEntries(const std::string& server_tag) { std::ostringstream s; - auto& mod_time_idx = audit_entries_.get(); + auto& mod_time_idx = audit_entries_[server_tag].get(); for (auto audit_entry_it = mod_time_idx.begin(); audit_entry_it != mod_time_idx.end(); @@ -407,31 +409,57 @@ public: /// @param exp_object_type Expected object type. /// @param exp_modification_time Expected modification time. /// @param exp_log_message Expected log message. + /// @param server_selector Server selector to be used for next query. /// @param new_entries_num Number of the new entries expected to be inserted. + /// @param max_tested_entries Maximum number of entries tested. void testNewAuditEntry(const std::string& exp_object_type, const AuditEntry::ModificationType& exp_modification_type, const std::string& exp_log_message, - const size_t new_entries_num = 1) { - auto audit_entries_size_save = audit_entries_.size(); - audit_entries_ = cbptr_->getRecentAuditEntries(ServerSelector::ALL(), - timestamps_["two days ago"]); - ASSERT_EQ(audit_entries_size_save + new_entries_num, audit_entries_.size()) - << logExistingAuditEntries(); + const ServerSelector& server_selector = ServerSelector::ALL(), + const size_t new_entries_num = 1, + const size_t max_tested_entries = 65535) { + // Get the server tag for which the entries are fetched. + std::string tag; + if (server_selector.getType() == ServerSelector::Type::ALL) { + // Server tag is 'all'. + tag = "all"; + + } else { + auto tags = server_selector.getTags(); + // This test is not meant to handle multiple server tags all at once. + if (tags.size() > 1) { + ADD_FAILURE() << "Test error: do not use multiple server tags"; + + } else if (tags.size() == 1) { + // Get the server tag for which we run the current test. + tag = *tags.begin(); + } + } + + auto audit_entries_size_save = audit_entries_[tag].size(); - auto& mod_time_idx = audit_entries_.get(); + // Audit entries for different server tags are stored in separate + // containers. + audit_entries_[tag] = cbptr_->getRecentAuditEntries(server_selector, + timestamps_["two days ago"]); + ASSERT_EQ(audit_entries_size_save + new_entries_num, audit_entries_[tag].size()) + << logExistingAuditEntries(tag); + + auto& mod_time_idx = audit_entries_[tag].get(); // Iterate over specified number of entries starting from the most recent // one and check they have correct values. for (auto audit_entry_it = mod_time_idx.rbegin(); - std::distance(mod_time_idx.rbegin(), audit_entry_it) < new_entries_num; + ((std::distance(mod_time_idx.rbegin(), audit_entry_it) < new_entries_num) && + (std::distance(mod_time_idx.rbegin(), audit_entry_it) < max_tested_entries)); ++audit_entry_it) { auto audit_entry = *audit_entry_it; EXPECT_EQ(exp_object_type, audit_entry->getObjectType()) - << logExistingAuditEntries(); + << logExistingAuditEntries(tag); EXPECT_EQ(exp_modification_type, audit_entry->getModificationType()) - << logExistingAuditEntries(); + << logExistingAuditEntries(tag); EXPECT_EQ(exp_log_message, audit_entry->getLogMessage()) - << logExistingAuditEntries(); + << logExistingAuditEntries(tag); } } @@ -457,7 +485,7 @@ public: boost::shared_ptr cbptr_; /// @brief Holds the most recent audit entries. - AuditEntryCollection audit_entries_; + std::map audit_entries_; }; // This test verifies that the expected backend type is returned. @@ -700,27 +728,66 @@ TEST_F(MySqlConfigBackendDHCPv6Test, globalParameters6WithServerTags) { 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])); + { + SCOPED_TRACE("server1 is created"); + testNewAuditEntry("dhcp6_server", + AuditEntry::ModificationType::CREATE, + "server set"); + } + EXPECT_NO_THROW(cbptr_->createUpdateServer6(test_servers_[2])); + { + SCOPED_TRACE("server2 is created"); + testNewAuditEntry("dhcp6_server", + AuditEntry::ModificationType::CREATE, + "server set"); + } // This time inserting the global parameters for the server1 and server2 should // be successful. EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter6(ServerSelector::ONE("server1"), global_parameter1)); + { + SCOPED_TRACE("Global parameter for server1 is set"); + // The value of 3 means there should be 3 audit entries available for the + // server1, two that indicate creation of the servers and one that we + // validate, which sets the global value. + testNewAuditEntry("dhcp6_global_parameter", + AuditEntry::ModificationType::CREATE, + "global parameter set", + ServerSelector::ONE("server1"), + 3, 1); + } + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter6(ServerSelector::ONE("server2"), global_parameter2)); + { + SCOPED_TRACE("Global parameter for server2 is set"); + // Same as in case of the server2, there should be 3 audit entries of + // which one we validate. + testNewAuditEntry("dhcp6_global_parameter", + AuditEntry::ModificationType::CREATE, + "global parameter set", + ServerSelector::ONE("server2"), + 3, 1); + } // The last parameter is associated with all servers. EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter6(ServerSelector::ALL(), global_parameter3)); + { + SCOPED_TRACE("Global parameter for all servers is set"); + // There should be one new audit entry for all servers. It indicates + // the insertion of the global value. + testNewAuditEntry("dhcp6_global_parameter", + AuditEntry::ModificationType::CREATE, + "global parameter set", + ServerSelector::ALL(), + 1, 1); + } StampedValuePtr returned_global; @@ -742,6 +809,7 @@ TEST_F(MySqlConfigBackendDHCPv6Test, globalParameters6WithServerTags) { ); ASSERT_TRUE(returned_global); EXPECT_EQ(global_parameter1->getValue(), returned_global->getValue()); + ASSERT_EQ(1, returned_global->getServerTags().size()); EXPECT_EQ("server1", returned_global->getServerTags()[0].get()); @@ -763,7 +831,7 @@ TEST_F(MySqlConfigBackendDHCPv6Test, globalParameters6WithServerTags) { EXPECT_NO_THROW( returned_globals = cbptr_->getAllGlobalParameters6(ServerSelector:: MULTIPLE({ "server1", "server2", - "server3"})); + "server3" })); ); ASSERT_EQ(3, returned_globals.size()); @@ -793,7 +861,7 @@ TEST_F(MySqlConfigBackendDHCPv6Test, globalParameters6WithServerTags) { EXPECT_EQ("all", returned_global->getServerTags()[0].get()); // Delete the server1. It should remove associations of this server with the - // global parameters. + // global parameter and the global parameter itself. EXPECT_NO_THROW(cbptr_->deleteServer6(ServerTag("server1"))); EXPECT_NO_THROW( returned_globals = cbptr_->getAllGlobalParameters6(ServerSelector::ONE("server1")) @@ -807,6 +875,17 @@ TEST_F(MySqlConfigBackendDHCPv6Test, globalParameters6WithServerTags) { ASSERT_EQ(1, returned_global->getServerTags().size()); EXPECT_EQ("all", returned_global->getServerTags()[0].get()); + { + SCOPED_TRACE("DELETE audit entry for the global parameter after server deletion"); + // We expect two new audit entries for the server1, one indicating that the + // server has been deleted and another one indicating that the corresponding + // global value has been deleted. We check the latter entry. + testNewAuditEntry("dhcp6_global_parameter", + AuditEntry::ModificationType::DELETE, + "deleting a server", ServerSelector::ONE("server1"), + 2, 1); + } + // Attempt to delete global parameter for server1. uint64_t deleted_num = 0; EXPECT_NO_THROW(deleted_num = cbptr_->deleteGlobalParameter6(ServerSelector::ONE("server1"), @@ -820,8 +899,12 @@ TEST_F(MySqlConfigBackendDHCPv6Test, globalParameters6WithServerTags) { "global")); EXPECT_EQ(1, deleted_num); + // Create it again to test that deletion of all server removes this too. + EXPECT_NO_THROW(cbptr_->createUpdateGlobalParameter6(ServerSelector::ONE("server2"), + global_parameter2)); + // Delete all servers, except 'all'. - EXPECT_NO_THROW(cbptr_->deleteAllServers6()); + EXPECT_NO_THROW(deleted_num = cbptr_->deleteAllServers6()); EXPECT_NO_THROW( returned_globals = cbptr_->getAllGlobalParameters6(ServerSelector::ALL()) ); @@ -833,6 +916,18 @@ TEST_F(MySqlConfigBackendDHCPv6Test, globalParameters6WithServerTags) { EXPECT_EQ(global_parameter3->getValue(), returned_global->getValue()); ASSERT_EQ(1, returned_global->getServerTags().size()); EXPECT_EQ("all", returned_global->getServerTags()[0].get()); + + { + SCOPED_TRACE("DELETE audit entry for the global parameter after deletion of" + " all servers"); + // There should be 4 new audit entries. One for deleting the global, one for + // re-creating it, one for deleting the server2 and one for deleting the + // global again as a result of deleting the server2. + testNewAuditEntry("dhcp6_global_parameter", + AuditEntry::ModificationType::DELETE, + "deleting all servers", ServerSelector::ONE("server2"), + 4, 1); + } } // This test verifies that all global parameters can be retrieved and deleted. @@ -1449,7 +1544,7 @@ TEST_F(MySqlConfigBackendDHCPv6Test, getAllSharedNetworks6) { // That shared network overrides the first one so the audit entry should // indicate an update. - if ((network->getName() == "level1") && (!audit_entries_.empty())) { + if ((network->getName() == "level1") && (!audit_entries_["all"].empty())) { SCOPED_TRACE("UPDATE audit entry for the shared network " + network->getName()); testNewAuditEntry("dhcp6_shared_network", @@ -1501,7 +1596,7 @@ TEST_F(MySqlConfigBackendDHCPv6Test, getAllSharedNetworks6) { SCOPED_TRACE("CREATE audit entry for subnets"); testNewAuditEntry("dhcp6_subnet", AuditEntry::ModificationType::CREATE, - "subnet set", 3); + "subnet set", ServerSelector::ALL(), 3); } // Deleting non-existing shared network should return 0. @@ -1559,7 +1654,8 @@ TEST_F(MySqlConfigBackendDHCPv6Test, getAllSharedNetworks6) { // The last parameter indicates that we expect two new audit entries. testNewAuditEntry("dhcp6_shared_network", AuditEntry::ModificationType::DELETE, - "deleted all shared networks", 2); + "deleted all shared networks", + ServerSelector::ALL(), 2); } // Check that subnets are still there but detached. @@ -1781,7 +1877,8 @@ TEST_F(MySqlConfigBackendDHCPv6Test, getAllOptionDefs6) { // The last parameter indicates that we expect two new audit entries. testNewAuditEntry("dhcp6_option_def", AuditEntry::ModificationType::DELETE, - "deleted all option definitions", 2); + "deleted all option definitions", + ServerSelector::ALL(), 2); } }