From: Marcin Siodelski Date: Wed, 23 Jan 2019 19:37:32 +0000 (+0100) Subject: [#396,!205] Numerous improvements to MySQL CB audit trail. X-Git-Tag: 429-Updated-StampedValue-to-support-reals_base~55 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ca43d90230db329cfcc5c2681f6261e79438990a;p=thirdparty%2Fkea.git [#396,!205] Numerous improvements to MySQL CB audit trail. --- diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index 864d20b44c..4d6f9cd4b6 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -216,11 +216,13 @@ public: MySqlTransaction transaction(conn_); - // The last parameter indicates that the parameter is not bound to any - // other object (e.g. addition of a subnet), so an audit entry should - // be created for the addition of the parameter. - initAuditRevision(MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, - "this is log message", true); + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + "global parameter set", false); // Try to update the existing row. if (conn_.updateDeleteQuery(MySqlConfigBackendDHCPv4Impl::UPDATE_GLOBAL_PARAMETER4, @@ -751,15 +753,15 @@ public: MySqlTransaction transaction(conn_); - try { + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + "subnet set", true); - // The change will involve multiple statements. The audit entry should - // be created for the parent object and should not be created for the - // DHCP options. The boolean value set to false indicates that the - // MySQL triggers should not create audit revision for the DHCP - // options associated with the subnet. - initAuditRevision(MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, - "this is log message", false); + try { // Try to insert subnet. If this duplicates primary key, i.e. this // subnet already exists it will throw DuplicateEntry exception in @@ -804,7 +806,7 @@ public: OptionDescriptorPtr desc_copy(new OptionDescriptor(*desc)); desc_copy->space_name_ = option_space; createUpdateOption4(server_selector, subnet->getID(), desc_copy, - false); + true); } } @@ -835,21 +837,48 @@ public: for (auto desc = options->begin(); desc != options->end(); ++desc) { OptionDescriptorPtr desc_copy(new OptionDescriptor(*desc)); desc_copy->space_name_ = option_space; - createUpdateOption4(server_selector, pool_id, desc_copy); + createUpdateOption4(server_selector, pool_id, desc_copy, true); } } } + template + uint64_t deleteTransactional(const int index, + const db::ServerSelector& server_selector, + const std::string& operation, + const std::string& log_message, + const bool cascade_delete, + Args&&... keys) { + + MySqlTransaction transaction(conn_); + + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + log_message, cascade_delete); + + auto count = deleteFromTable(index, server_selector, operation, keys...); + + transaction.commit(); + + return (count); + } + /// @brief Sends query to delete subnet by id. /// /// @param server_selector Server selector. /// @param subnet_id Identifier of the subnet to be deleted. /// @return Number of deleted subnets. uint64_t deleteSubnet4(const ServerSelector& server_selector, - const SubnetID& subnet_id) { - return (deleteFromTable(DELETE_SUBNET4_ID, server_selector, - "deleting a subnet", - subnet_id)); + const SubnetID& subnet_id) { + return (deleteTransactional(DELETE_SUBNET4_ID, server_selector, + "deleting a subnet", + "subnet deleted", + true, + static_cast(subnet_id))); } /// @brief Deletes pools belonging to a subnet from the database. @@ -1117,14 +1146,15 @@ public: MySqlTransaction transaction(conn_); + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + "shared network set", true); + try { - // The change will involve multiple statements. The audit entry should - // be created for the parent object and should not be created for the - // DHCP options. The boolean value set to false indicates that the - // MySQL triggers should not create audit revision for the DHCP - // options associated with the shared network. - initAuditRevision(MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, - "this is log message", false); // Try to insert shared network. The shared network name must be unique, // so if inserting fails with DuplicateEntry exception we'll need to @@ -1162,7 +1192,7 @@ public: OptionDescriptorPtr desc_copy(new OptionDescriptor(*desc)); desc_copy->space_name_ = option_space; createUpdateOption4(server_selector, shared_network->getName(), - desc_copy, false); + desc_copy, true); } } @@ -1231,11 +1261,13 @@ public: option->option_->getType(), option->space_name_); - // The last parameter indicates that the option is not bound to any - // other object (e.g. addition of a subnet), so an audit entry should - // be created for the addition of the option. - initAuditRevision(MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, - "this is log message", true); + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + "global option set", false); if (existing_option) { in_bindings.push_back(MySqlBinding::createString(tag)); @@ -1257,12 +1289,12 @@ public: /// @param server_selector Server selector. /// @param subnet_id Identifier of the subnet the option belongs to. /// @param option Pointer to the option descriptor encapsulating the option. - /// @param distinct_transaction Boolean value indicating whether setting - /// the option value should be enclosed in a separate transaction. + /// @param cascade_update Boolean value indicating whether the update is + /// performed as part of the ownining element, e.g. subnet. void createUpdateOption4(const ServerSelector& server_selector, const SubnetID& subnet_id, const OptionDescriptorPtr& option, - const bool distinct_transaction = false) { + const bool cascade_update) { if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" @@ -1292,7 +1324,7 @@ public: // Only start new transaction if specified to do so. This function may // be called from within an existing transaction in which case we // don't start the new one. - if (distinct_transaction) { + if (!cascade_update) { transaction.reset(new MySqlTransaction(conn_)); } @@ -1300,8 +1332,13 @@ public: option->option_->getType(), option->space_name_); - initAuditRevision(MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, - "this is log message", distinct_transaction); + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + "subnet specific option set", cascade_update); if (existing_option) { in_bindings.push_back(MySqlBinding::createString(tag)); @@ -1339,7 +1376,7 @@ public: << pool_end_address); } - createUpdateOption4(server_selector, pool_id, option, false); + createUpdateOption4(server_selector, pool_id, option, true); } @@ -1348,12 +1385,12 @@ public: /// @param selector Server selector. /// @param pool_id Identifier of the pool the option belongs to. /// @param option Pointer to the option descriptor encapsulating the option. - /// @param distinct_transaction Boolean value indicating whether setting - /// the option value should be enclosed in a separate transaction. + /// @param cascade_update Boolean value indicating whether the update is + /// performed as part of the ownining element, e.g. subnet. void createUpdateOption4(const ServerSelector& server_selector, const uint64_t pool_id, const OptionDescriptorPtr& option, - const bool distinct_transaction = false) { + const bool cascade_update) { if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" @@ -1382,7 +1419,7 @@ public: // Only start new transaction if specified to do so. This function may // be called from within an existing transaction in which case we // don't start the new one. - if (distinct_transaction) { + if (!cascade_update) { transaction.reset(new MySqlTransaction(conn_)); } @@ -1390,8 +1427,13 @@ public: option->option_->getType(), option->space_name_); - initAuditRevision(MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, - "this is log message", distinct_transaction); + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + "pool specific option set", cascade_update); if (existing_option) { in_bindings.push_back(MySqlBinding::createString(tag)); @@ -1416,12 +1458,12 @@ public: /// @param shared_network_name Name of the shared network the option /// belongs to. /// @param option Pointer to the option descriptor encapsulating the option. - /// @param distinct_transaction Boolean value indicating whether setting - /// the option value should be enclosed in a separate transaction.) + /// @param cascade_update Boolean value indicating whether the update is + /// performed as part of the ownining element, e.g. shared network. void createUpdateOption4(const ServerSelector& server_selector, const std::string& shared_network_name, const OptionDescriptorPtr& option, - const bool distinct_transaction = false) { + const bool cascade_update) { if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" @@ -1450,7 +1492,7 @@ public: // Only start new transaction if specified to do so. This function may // be called from within an existing transaction in which case we // don't start the new one. - if (distinct_transaction) { + if (!cascade_update) { transaction.reset(new MySqlTransaction(conn_)); } @@ -1458,8 +1500,14 @@ public: option->option_->getType(), option->space_name_); - initAuditRevision(MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, - "this is log message", distinct_transaction); + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + "shared network specific option set", + cascade_update); if (existing_option) { in_bindings.push_back(MySqlBinding::createString(tag)); @@ -1777,8 +1825,14 @@ public: option_def->getCode(), option_def->getOptionSpaceName()); - initAuditRevision(MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, - "this is log message", true); + // Create scoped audit revision. It initiates session variables in the + // database to be used for creating new audit revision. As long as this + // instance exists no new audit revisions are created as a result of + // any subsequent calls. + ScopedAuditRevision audit_revision(this, + MySqlConfigBackendDHCPv4Impl::INIT_AUDIT_REVISION, + "option definition set", + true); if (existing_definition) { // Need to add three more bindings for WHERE clause. @@ -1822,21 +1876,17 @@ public: const uint16_t code, const std::string& space) { - if (server_selector.amUnassigned()) { - isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); - } - - auto tag = getServerTag(server_selector, "deleting option definition"); - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(tag), MySqlBinding::createInteger(static_cast(code)), MySqlBinding::createString(space) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_OPTION_DEF4_CODE_NAME, in_bindings)); + return (deleteTransactional(DELETE_OPTION_DEF4_CODE_NAME, server_selector, + "deleting option definition", + "option definition deleted", + false, + in_bindings)); } /// @brief Deletes global option. @@ -1849,21 +1899,17 @@ public: const uint16_t code, const std::string& space) { - if (server_selector.amUnassigned()) { - isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); - } - - auto tag = getServerTag(server_selector, "deleting global option"); - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(tag), MySqlBinding::createInteger(code), MySqlBinding::createString(space) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_OPTION4, in_bindings)); + return (deleteTransactional(DELETE_OPTION4, server_selector, + "deleting global option", + "global option deleted", + false, + in_bindings)); } /// @brief Deletes subnet level option. @@ -1879,22 +1925,18 @@ public: const uint16_t code, const std::string& space) { - if (server_selector.amUnassigned()) { - isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); - } - - auto tag = getServerTag(server_selector, "deleting option for a subnet"); - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(tag), MySqlBinding::createInteger(static_cast(subnet_id)), MySqlBinding::createInteger(code), MySqlBinding::createString(space) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_OPTION4_SUBNET_ID, in_bindings)); + return (deleteTransactional(DELETE_OPTION4_SUBNET_ID, server_selector, + "deleting option for a subnet", + "subnet specific option deleted", + false, + in_bindings)); } /// @brief Deletes pool level option. @@ -1911,15 +1953,7 @@ public: const uint16_t code, const std::string& space) { - if (server_selector.amUnassigned()) { - isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); - } - - auto tag = getServerTag(server_selector, "deleting option for a pool"); - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(tag), MySqlBinding::createInteger(code), MySqlBinding::createString(space), MySqlBinding::createInteger(pool_start_address.toUint32()), @@ -1927,8 +1961,11 @@ public: }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_OPTION4_POOL_RANGE, - in_bindings)); + return (deleteTransactional(DELETE_OPTION4_POOL_RANGE, server_selector, + "deleting option for a pool", + "pool specific option deleted", + false, + in_bindings)); } /// @brief Deletes shared network level option. @@ -1944,23 +1981,18 @@ public: const uint16_t code, const std::string& space) { - if (server_selector.amUnassigned()) { - isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); - } - - auto tag = getServerTag(server_selector, "deleting option for a shared network"); - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(tag), MySqlBinding::createString(shared_network_name), MySqlBinding::createInteger(code), MySqlBinding::createString(space) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_OPTION4_SHARED_NETWORK, - in_bindings)); + return (deleteTransactional(DELETE_OPTION4_SHARED_NETWORK, server_selector, + "deleting option for a shared network", + "shared network specific option deleted", + false, + in_bindings)); } /// @brief Deletes options belonging to a subnet from the database. @@ -1972,21 +2004,16 @@ public: uint64_t deleteOptions4(const ServerSelector& server_selector, const Subnet4Ptr& subnet) { - if (server_selector.amUnassigned()) { - isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); - } - - auto tag = getServerTag(server_selector, "deleting options for a subnet"); - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(tag), MySqlBinding::createInteger(subnet->getID()) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_OPTIONS4_SUBNET_ID, - in_bindings)); + return (deleteTransactional(DELETE_OPTIONS4_SUBNET_ID, server_selector, + "deleting options for a subnet", + "subnet specific options deleted", + true, + in_bindings)); } /// @brief Deletes options belonging to a shared network from the database. @@ -1998,21 +2025,16 @@ public: uint64_t deleteOptions4(const ServerSelector& server_selector, const SharedNetwork4Ptr& shared_network) { - if (server_selector.amUnassigned()) { - isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); - } - - auto tag = getServerTag(server_selector, "deleting options for a shared network"); - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(tag), MySqlBinding::createString(shared_network->getName()) }; // Run DELETE. - return (conn_.updateDeleteQuery(DELETE_OPTIONS4_SHARED_NETWORK, - in_bindings)); + return (deleteTransactional(DELETE_OPTIONS4_SHARED_NETWORK, server_selector, + "deleting options for a shared network", + "shared network specific options deleted", + true, + in_bindings)); } }; @@ -2584,14 +2606,14 @@ void MySqlConfigBackendDHCPv4::createUpdateOption4(const db::ServerSelector& server_selector, const std::string& shared_network_name, const OptionDescriptorPtr& option) { - impl_->createUpdateOption4(server_selector, shared_network_name, option, true); + impl_->createUpdateOption4(server_selector, shared_network_name, option, false); } void MySqlConfigBackendDHCPv4::createUpdateOption4(const ServerSelector& server_selector, const SubnetID& subnet_id, const OptionDescriptorPtr& option) { - impl_->createUpdateOption4(server_selector, subnet_id, option, true); + impl_->createUpdateOption4(server_selector, subnet_id, option, false); } void @@ -2612,9 +2634,10 @@ MySqlConfigBackendDHCPv4::createUpdateGlobalParameter4(const ServerSelector& ser uint64_t MySqlConfigBackendDHCPv4::deleteSubnet4(const ServerSelector& server_selector, const std::string& subnet_prefix) { - return(impl_->deleteFromTable(MySqlConfigBackendDHCPv4Impl::DELETE_SUBNET4_PREFIX, - server_selector, "deleting a subnet by prefix", - subnet_prefix)); + return(impl_->deleteTransactional(MySqlConfigBackendDHCPv4Impl::DELETE_SUBNET4_PREFIX, + server_selector, "deleting a subnet by prefix", + "subnet deleted", true, + subnet_prefix)); } uint64_t @@ -2625,22 +2648,25 @@ MySqlConfigBackendDHCPv4::deleteSubnet4(const ServerSelector& server_selector, uint64_t MySqlConfigBackendDHCPv4::deleteAllSubnets4(const ServerSelector& server_selector) { - return (impl_->deleteFromTable(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_SUBNETS4, - server_selector, "deleting all subnets")); + return (impl_->deleteTransactional(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_SUBNETS4, + server_selector, "deleting all subnets", + "deleted all subnets", true)); } uint64_t MySqlConfigBackendDHCPv4::deleteSharedNetwork4(const ServerSelector& server_selector, const std::string& name) { - return (impl_->deleteFromTable(MySqlConfigBackendDHCPv4Impl::DELETE_SHARED_NETWORK4_NAME, - server_selector, "deleting a shared network", - name)); + return (impl_->deleteTransactional(MySqlConfigBackendDHCPv4Impl::DELETE_SHARED_NETWORK4_NAME, + server_selector, "deleting a shared network", + "shared network deleted", true, + name)); } uint64_t MySqlConfigBackendDHCPv4::deleteAllSharedNetworks4(const ServerSelector& server_selector) { - return (impl_->deleteFromTable(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_SHARED_NETWORKS4, - server_selector, "deleting all shared networks")); + return (impl_->deleteTransactional(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_SHARED_NETWORKS4, + server_selector, "deleting all shared networks", + "deleted all shared networks", true)); } uint64_t @@ -2652,8 +2678,9 @@ MySqlConfigBackendDHCPv4::deleteOptionDef4(const ServerSelector& server_selector uint64_t MySqlConfigBackendDHCPv4::deleteAllOptionDefs4(const ServerSelector& server_selector) { - return (impl_->deleteFromTable(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_OPTION_DEFS4, - server_selector, "deleting all option definitions")); + return (impl_->deleteTransactional(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_OPTION_DEFS4, + server_selector, "deleting all option definitions", + "deleted all option definitions", true)); } uint64_t @@ -2693,15 +2720,17 @@ MySqlConfigBackendDHCPv4::deleteOption4(const ServerSelector& server_selector, uint64_t MySqlConfigBackendDHCPv4::deleteGlobalParameter4(const ServerSelector& server_selector, const std::string& name) { - return (impl_->deleteFromTable(MySqlConfigBackendDHCPv4Impl::DELETE_GLOBAL_PARAMETER4, - server_selector, "deleting global parameter", - name)); + return (impl_->deleteTransactional(MySqlConfigBackendDHCPv4Impl::DELETE_GLOBAL_PARAMETER4, + server_selector, "deleting global parameter", + "global parameter deleted", false, + name)); } uint64_t MySqlConfigBackendDHCPv4::deleteAllGlobalParameters4(const ServerSelector& server_selector) { - return (impl_->deleteFromTable(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_GLOBAL_PARAMETERS4, - server_selector, "deleting all global parameters")); + return (impl_->deleteTransactional(MySqlConfigBackendDHCPv4Impl::DELETE_ALL_GLOBAL_PARAMETERS4, + server_selector, "deleting all global parameters", + "all global parameters deleted", true)); } std::string diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index 89bb93d26d..e19a9b0e5d 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -25,9 +25,23 @@ using namespace isc::util; namespace isc { namespace dhcp { +MySqlConfigBackendImpl:: +ScopedAuditRevision::ScopedAuditRevision(MySqlConfigBackendImpl* impl, + const int index, + const std::string& log_message, + bool cascade_transaction) + : impl_(impl) { + impl_->initAuditRevision(index, log_message, cascade_transaction); +} + +MySqlConfigBackendImpl:: +ScopedAuditRevision::~ScopedAuditRevision() { + impl_->clearAuditRevision(); +} + MySqlConfigBackendImpl:: MySqlConfigBackendImpl(const DatabaseConnection::ParameterMap& parameters) - : conn_(parameters) { + : conn_(parameters), audit_revision_created_(false) { // Open the database. conn_.openDatabase(); @@ -70,12 +84,23 @@ MySqlConfigBackendImpl::~MySqlConfigBackendImpl() { void MySqlConfigBackendImpl::initAuditRevision(const int index, const std::string& log_message, - const bool distinct_transaction) { + const bool cascade_transaction) { + // Do not touch existing audit revision in case of the cascade update. + if (audit_revision_created_) { + return; + } + MySqlBindingCollection in_bindings = { - MySqlBinding::createString("this is a log message"), - MySqlBinding::createInteger(static_cast(distinct_transaction)) + MySqlBinding::createString(log_message), + MySqlBinding::createInteger(static_cast(!cascade_transaction)) }; conn_.insertQuery(index, in_bindings); + audit_revision_created_ = true; +} + +void +MySqlConfigBackendImpl::clearAuditRevision() { + audit_revision_created_ = false; } void @@ -115,37 +140,12 @@ MySqlConfigBackendImpl::getRecentAuditEntries(const int index, }); } -uint64_t -MySqlConfigBackendImpl::deleteFromTable(const int index) { - MySqlBindingCollection in_bindings; - return (conn_.updateDeleteQuery(index, in_bindings)); -} - -uint64_t -MySqlConfigBackendImpl::deleteFromTable(const int index, const std::string& key) { - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(key) - }; - return (conn_.updateDeleteQuery(index, in_bindings)); -} - uint64_t MySqlConfigBackendImpl::deleteFromTable(const int index, const ServerSelector& server_selector, const std::string& operation) { - - if (server_selector.amUnassigned()) { - isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); - } - - auto tag = getServerTag(server_selector, operation); - - MySqlBindingCollection in_bindings = { - MySqlBinding::createString(tag) - }; - - return (conn_.updateDeleteQuery(index, in_bindings)); + MySqlBindingCollection in_bindings; + return (deleteFromTable(index, server_selector, operation, in_bindings)); } void diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h index f8908ed11e..fcf12fb0af 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -30,6 +31,21 @@ namespace dhcp { class MySqlConfigBackendImpl { public: + class ScopedAuditRevision { + public: + + ScopedAuditRevision(MySqlConfigBackendImpl* impl, + const int index, + const std::string& log_message, + bool cascade_transaction); + + ~ScopedAuditRevision(); + + private: + MySqlConfigBackendImpl* impl_; + + }; + /// @brief Constructor. /// /// @param parameters A data structure relating keywords and values @@ -111,18 +127,16 @@ public: /// /// @param index query index. /// @param log_message log message to be used for the audit revision. - /// @param distinct_transaction boolean value indicating if a single - /// change will be applied in the transaction (distinct transaction) - /// or a chain of transactions. The example of the former is when - /// an option is added to the existing subnet. The example of the - /// latter is when the subnet along with the options is added. This - /// consists of two changes (adding the subnet and options) as part - /// of the single transaction. The MySQL database needs to - /// distinguish between these two cases to bind audit revisions - /// to the appropriate objects. + /// @param cascade_transaction Boolean value indicating whether the + /// configuration modification is performed as part of the ownining + /// element modification, e.g. subnet is modified resulting in + /// modification of the DHCP options it owns. In that case only the + /// audit entry for the owning element should be created. void initAuditRevision(const int index, const std::string& log_message, - const bool distinct_transaction); + const bool cascade_transaction); + + void clearAuditRevision(); /// @brief Sends query to the database to retrieve most recent audit entries. /// @@ -138,17 +152,29 @@ public: /// @brief Sends query to delete rows from a table. /// /// @param index Index of the statement to be executed. - /// @return Number of deleted rows. - uint64_t deleteFromTable(const int index); - - /// @brief Sends query to delete rows from a table. - /// - /// @param index Index of the statement to be executed. - /// @param key String value to be used as input binding to the delete - /// statement + /// @param server_selector Server selector. + /// @param operation Operation which results in calling this function. This is + /// used for logging purposes. + /// @param in_bindings Reference to the MySQL input bindings. They are modified + /// as a result of this function - server tag is inserted into the beginning + /// of the bindings collection. /// @return Number of deleted rows. uint64_t deleteFromTable(const int index, - const std::string& key); + const db::ServerSelector& server_selector, + const std::string& operation, + db::MySqlBindingCollection& in_bindings) { + + if (server_selector.amUnassigned()) { + isc_throw(NotImplemented, "managing configuration for no particular server" + " (unassigned) is unsupported at the moment"); + } + + auto tag = getServerTag(server_selector, operation); + + in_bindings.insert(in_bindings.begin(), db::MySqlBinding::createString(tag)); + + return (conn_.updateDeleteQuery(index, in_bindings)); + } /// @brief Sends query to delete rows from a table. /// @@ -182,11 +208,7 @@ public: const db::ServerSelector& server_selector, const std::string& operation, KeyType key) { - auto tag = getServerTag(server_selector, operation); - - db::MySqlBindingCollection in_bindings = { - db::MySqlBinding::createString(tag) - }; + db::MySqlBindingCollection in_bindings; if (db::MySqlBindingTraits::column_type == MYSQL_TYPE_STRING) { in_bindings.push_back(db::MySqlBinding::createString(key)); @@ -195,7 +217,7 @@ public: in_bindings.push_back(db::MySqlBinding::createInteger(key)); } - return (conn_.updateDeleteQuery(index, in_bindings)); + return (deleteFromTable(index, server_selector, operation, in_bindings)); } /// @brief Sends query to the database to retrieve multiple option @@ -317,6 +339,10 @@ public: /// @brief Represents connection to the MySQL database. db::MySqlConnection conn_; + + /// @brief Boolean flag indicating if audit revision has been created + /// using @c ScopedAuditRevision object. + bool audit_revision_created_; }; } // end of namespace isc::dhcp 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 1177672d19..e39c87efcb 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 @@ -20,6 +20,7 @@ #include #include #include +#include using namespace isc::asiolink; using namespace isc::db; @@ -295,6 +296,26 @@ public: timestamps_["tomorrow"] = timestamps_["today"] + boost::posix_time::hours(24); } + std::string logExistingAuditEntries() { + std::ostringstream s; + + auto& mod_time_idx = audit_entries_.get(); + + for (auto audit_entry_it = mod_time_idx.begin(); + audit_entry_it != mod_time_idx.end(); + ++audit_entry_it) { + auto audit_entry = *audit_entry_it; + s << audit_entry->getObjectType() << ", " + << audit_entry->getObjectId() << ", " + << static_cast(audit_entry->getModificationType()) << ", " + << audit_entry->getModificationTime() << ", " + << audit_entry->getLogMessage() + << std::endl; + } + + return (s.str()); + } + /// @brief Tests that the new audit entry is added. /// /// This method retrieves a collection of the existing audit entries and @@ -313,7 +334,8 @@ public: auto audit_entries_size_save = audit_entries_.size(); audit_entries_ = cbptr_->getRecentAuditEntries4(ServerSelector::ALL(), timestamps_["two days ago"]); - ASSERT_EQ(audit_entries_size_save + new_entries_num, audit_entries_.size()); + ASSERT_EQ(audit_entries_size_save + new_entries_num, audit_entries_.size()) + << logExistingAuditEntries(); auto& mod_time_idx = audit_entries_.get(); @@ -323,9 +345,12 @@ public: std::distance(mod_time_idx.rbegin(), audit_entry_it) < new_entries_num; ++audit_entry_it) { auto audit_entry = *audit_entry_it; - EXPECT_EQ(exp_object_type, audit_entry->getObjectType()); - EXPECT_EQ(exp_modification_type, audit_entry->getModificationType()); - EXPECT_EQ(exp_log_message, audit_entry->getLogMessage()); + EXPECT_EQ(exp_object_type, audit_entry->getObjectType()) + << logExistingAuditEntries(); + EXPECT_EQ(exp_modification_type, audit_entry->getModificationType()) + << logExistingAuditEntries(); + EXPECT_EQ(exp_log_message, audit_entry->getLogMessage()) + << logExistingAuditEntries(); } } @@ -398,7 +423,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteGlobalParameter4) { SCOPED_TRACE("CREATE audit entry for global parameter"); testNewAuditEntry("dhcp4_global_parameter", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "global parameter set"); } // Verify returned parameter and the modification time. @@ -436,7 +461,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteGlobalParameter4) { SCOPED_TRACE("UPDATE audit entry for the global parameter"); testNewAuditEntry("dhcp4_global_parameter", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "global parameter set"); } // Should not delete parameter specified for all servers if explicit @@ -454,7 +479,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteGlobalParameter4) { SCOPED_TRACE("DELETE audit entry for the global parameter"); testNewAuditEntry("dhcp4_global_parameter", AuditEntry::ModificationType::DELETE, - "this is a log message"); + "global parameter deleted"); } } @@ -554,7 +579,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSubnet4) { SCOPED_TRACE("CREATE audit entry for the subnet"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "subnet set"); } // Update the subnet in the database (both use the same ID). @@ -576,7 +601,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSubnet4) { SCOPED_TRACE("UPDATE audit entry for the subnet"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "subnet set"); } } @@ -643,13 +668,13 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSubnets4) { SCOPED_TRACE("UPDATE audit entry for the subnet " + subnet->toText()); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "subnet set"); } else { SCOPED_TRACE("CREATE audit entry for the subnet " + subnet->toText()); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "subnet set"); } } @@ -694,7 +719,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSubnets4) { SCOPED_TRACE("DELETE first subnet audit entry"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::DELETE, - "this is a log message"); + "subnet deleted"); } subnets = cbptr_->getAllSubnets4(ServerSelector::ALL()); @@ -710,7 +735,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSubnets4) { SCOPED_TRACE("DELETE second subnet audit entry"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::DELETE, - "this is a log message"); + "subnet deleted"); } // Delete all. @@ -722,7 +747,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSubnets4) { SCOPED_TRACE("DELETE all subnets audit entry"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::DELETE, - "this is a log message"); + "deleted all subnets"); } } @@ -789,14 +814,14 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSharedNetwork4) { SCOPED_TRACE("CREATE audit entry for a shared network"); testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "shared network set"); } - + // Update shared network in the database. SharedNetwork4Ptr shared_network2 = test_networks_[1]; cbptr_->createUpdateSharedNetwork4(ServerSelector::ALL(), shared_network2); - // Fetch updated subnet and see if it matches. + // Fetch updated shared betwork and see if it matches. returned_network = cbptr_->getSharedNetwork4(ServerSelector::ALL(), test_networks_[1]->getName()); EXPECT_EQ(shared_network2->toElement()->str(), @@ -806,7 +831,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSharedNetwork4) { SCOPED_TRACE("UPDATE audit entry for a shared network"); testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "shared network set"); } // Fetching the shared network for an explicitly specified server tag should @@ -831,16 +856,15 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSharedNetworks4) { network->getName()); testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "shared network set"); } else { SCOPED_TRACE("CREATE audit entry for the shared network " + network->getName()); testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "shared network set"); } - } // Fetch all shared networks. @@ -883,7 +907,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSharedNetworks4) { SCOPED_TRACE("DELETE audit entry for the first shared network"); testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::DELETE, - "this is a log message"); + "shared network deleted"); } // Delete all. @@ -896,7 +920,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllSharedNetworks4) { // The last parameter indicates that we expect two new audit entries. testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::DELETE, - "this is a log message", 2); + "deleted all shared networks", 2); } } @@ -957,7 +981,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getOptionDef4) { SCOPED_TRACE("CREATE audit entry for an option definition"); testNewAuditEntry("dhcp4_option_def", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "option definition set"); } // Update the option definition in the database. @@ -981,7 +1005,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getOptionDef4) { SCOPED_TRACE("UPDATE audit entry for an option definition"); testNewAuditEntry("dhcp4_option_def", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "option definition set"); } } @@ -1000,14 +1024,14 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllOptionDefs4) { option_def->getName()); testNewAuditEntry("dhcp4_option_def", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "option definition set"); } else { SCOPED_TRACE("CREATE audit entry for the option defnition " + option_def->getName()); testNewAuditEntry("dhcp4_option_def", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "option definition set"); } } @@ -1059,7 +1083,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllOptionDefs4) { SCOPED_TRACE("DELETE audit entry for the first option definition"); testNewAuditEntry("dhcp4_option_def", AuditEntry::ModificationType::DELETE, - "this is a log message"); + "option definition deleted"); } // Delete all remaining option definitions. @@ -1072,7 +1096,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllOptionDefs4) { // The last parameter indicates that we expect two new audit entries. testNewAuditEntry("dhcp4_option_def", AuditEntry::ModificationType::DELETE, - "this is a log message", 2); + "deleted all option definitions", 2); } } @@ -1133,7 +1157,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteOption4) { SCOPED_TRACE("CREATE audit entry for an option"); testNewAuditEntry("dhcp4_options", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "global option set"); } // Modify option and update it in the database. @@ -1153,7 +1177,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteOption4) { SCOPED_TRACE("UPDATE audit entry for an option"); testNewAuditEntry("dhcp4_options", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "global option set"); } // Deleting an option with explicitly specified server tag should fail. @@ -1174,7 +1198,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteOption4) { SCOPED_TRACE("DELETE audit entry for an option"); testNewAuditEntry("dhcp4_options", AuditEntry::ModificationType::DELETE, - "this is a log message"); + "global option deleted"); } } @@ -1268,7 +1292,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSubnetOption4) { SCOPED_TRACE("CREATE audit entry for a new subnet"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "subnet set"); } OptionDescriptorPtr opt_boot_file_name = test_options_[0]; @@ -1292,7 +1316,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSubnetOption4) { // have means to retrieve only the newly added option. testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "subnet specific option set"); } opt_boot_file_name->persistent_ = !opt_boot_file_name->persistent_; @@ -1311,7 +1335,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSubnetOption4) { SCOPED_TRACE("UPDATE audit entry for an updated subnet option"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "subnet specific option set"); } // Deleting an option with explicitly specified server tag should fail. @@ -1335,7 +1359,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSubnetOption4) { SCOPED_TRACE("UPDATE audit entry for a deleted subnet option"); testNewAuditEntry("dhcp4_subnet", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "subnet specific option deleted"); } } @@ -1438,7 +1462,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSharedNetworkOption4) { SCOPED_TRACE("CREATE audit entry for the new shared network"); testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::CREATE, - "this is a log message"); + "shared network set"); } OptionDescriptorPtr opt_boot_file_name = test_options_[0]; @@ -1463,7 +1487,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSharedNetworkOption4) { // have means to retrieve only the newly added option. testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "shared network specific option set"); } opt_boot_file_name->persistent_ = !opt_boot_file_name->persistent_; @@ -1483,7 +1507,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSharedNetworkOption4) { SCOPED_TRACE("UPDATE audit entry for the updated shared network option"); testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "shared network specific option set"); } // Deleting an option with explicitly specified server tag should fail. @@ -1506,7 +1530,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSharedNetworkOption4) { SCOPED_TRACE("UPDATE audit entry for the deleted shared network option"); testNewAuditEntry("dhcp4_shared_network", AuditEntry::ModificationType::UPDATE, - "this is a log message"); + "shared network specific option deleted"); } }