From df14c7593e6988d50cbc09e942c0357335bfc318 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 3 Jun 2022 09:43:24 -0400 Subject: [#2299] Create subnet audit entry when network is deleted Update subnets in shared-network BDEL trigger rather than relying on foreign key update action new files: src/share/database/scripts/mysql/upgrade_013_to_014.sh.in src/share/database/scripts/pgsql/upgrade_011_to_012.sh.in configure.ac added: src/share/database/scripts/mysql/upgrade_013_to_014.sh src/share/database/scripts/pgsql/upgrade_011_to_012.sh src/bin/admin/tests/mysql_tests.sh.in added 13 to 14 checks src/bin/admin/tests/pgsql_tests.sh.in added 11 to 12 checks src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc enabled disabled tests src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc GenericConfigBackendDHCPv4Test::getAllSharedNetworks4Test() - updated expected audit entry order src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc GenericConfigBackendDHCPv6Test::getAllSharedNetworks6Test() - updated expected audit entry order src/lib/mysql/mysql_constants.h Updated schema version to 14 src/lib/pgsql/pgsql_connection.h Updated schema version to 12 src/share/database/scripts/mysql/.gitignore src/share/database/scripts/mysql/Makefile.am added upgrade_013_to_014.sh src/share/database/scripts/mysql/dhcpdb_create.mysql subnet rows are now updated directly in shared-network BEFORE delete triggers (v4 and v6) src/share/database/scripts/pgsql/Makefile.am added upgrade_011_to_012.sh src/share/database/scripts/pgsql/dhcpdb_create.pgsql subnet rows are now updated directly in shared-network BEFORE delete triggers (v4 and v6) --- configure.ac | 4 + src/bin/admin/tests/mysql_tests.sh.in | 29 +++++- src/bin/admin/tests/pgsql_tests.sh.in | 31 +++++- .../dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc | 3 +- .../dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc | 3 +- .../dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc | 8 +- .../dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc | 8 +- src/lib/mysql/mysql_constants.h | 2 +- src/lib/pgsql/pgsql_connection.h | 4 +- src/share/database/scripts/mysql/.gitignore | 1 + src/share/database/scripts/mysql/Makefile.am | 1 + .../database/scripts/mysql/dhcpdb_create.mysql | 64 ++++++++++++ .../scripts/mysql/upgrade_013_to_014.sh.in | 116 +++++++++++++++++++++ src/share/database/scripts/pgsql/.gitignore | 1 + src/share/database/scripts/pgsql/Makefile.am | 1 + .../database/scripts/pgsql/dhcpdb_create.pgsql | 60 +++++++++++ .../scripts/pgsql/upgrade_011_to_012.sh.in | 101 ++++++++++++++++++ 17 files changed, 416 insertions(+), 21 deletions(-) create mode 100644 src/share/database/scripts/mysql/upgrade_013_to_014.sh.in create mode 100644 src/share/database/scripts/pgsql/upgrade_011_to_012.sh.in diff --git a/configure.ac b/configure.ac index 7d79c7524b..01c648683e 100644 --- a/configure.ac +++ b/configure.ac @@ -1631,6 +1631,8 @@ AC_CONFIG_FILES([src/share/database/scripts/mysql/upgrade_011_to_012.sh], [chmod +x src/share/database/scripts/mysql/upgrade_011_to_012.sh]) AC_CONFIG_FILES([src/share/database/scripts/mysql/upgrade_012_to_013.sh], [chmod +x src/share/database/scripts/mysql/upgrade_012_to_013.sh]) +AC_CONFIG_FILES([src/share/database/scripts/mysql/upgrade_013_to_014.sh], + [chmod +x src/share/database/scripts/mysql/upgrade_013_to_014.sh]) AC_CONFIG_FILES([src/share/database/scripts/mysql/wipe_data.sh], [chmod +x src/share/database/scripts/mysql/wipe_data.sh]) AC_CONFIG_FILES([src/share/database/scripts/pgsql/Makefile]) @@ -1666,6 +1668,8 @@ AC_CONFIG_FILES([src/share/database/scripts/pgsql/upgrade_009_to_010.sh], [chmod +x src/share/database/scripts/pgsql/upgrade_009_to_010.sh]) AC_CONFIG_FILES([src/share/database/scripts/pgsql/upgrade_010_to_011.sh], [chmod +x src/share/database/scripts/pgsql/upgrade_010_to_011.sh]) +AC_CONFIG_FILES([src/share/database/scripts/pgsql/upgrade_011_to_012.sh], + [chmod +x src/share/database/scripts/pgsql/upgrade_011_to_012.sh]) AC_CONFIG_FILES([src/share/database/scripts/pgsql/wipe_data.sh], [chmod +x src/share/database/scripts/pgsql/wipe_data.sh]) AC_CONFIG_FILES([src/share/yang/Makefile]) diff --git a/src/bin/admin/tests/mysql_tests.sh.in b/src/bin/admin/tests/mysql_tests.sh.in index 2402bce5eb..65e955c805 100644 --- a/src/bin/admin/tests/mysql_tests.sh.in +++ b/src/bin/admin/tests/mysql_tests.sh.in @@ -375,6 +375,26 @@ mysql_upgrade_12_to_13_test() { assert_str_eq '' "${OUTPUT}" } +mysql_upgrade_13_to_14_test() { + # Check function source code + run_command \ + mysql_execute "select action_statement from information_schema.TRIGGERS where trigger_schema = '${db_name}' and trigger_name = 'dhcp4_shared_network_BDEL'"; + + assert_eq 0 "${EXIT_CODE}" "function func_dhcp4_shared_network_BDEL() broken or missing. (expected status code %d, returned %d)" + + count=$(echo "${OUTPUT}" | grep -Eci 'UPDATE dhcp4_subnet SET shared_network_name = NULL') || true + assert_eq 1 "${count}" "function func_dhcp4_shared_network_BDEL() is missing changed line. (expected count %d, returned %d)" + + # Check function source code + run_command \ + mysql_execute "select action_statement from information_schema.TRIGGERS where trigger_schema = '${db_name}' and trigger_name = 'dhcp6_shared_network_BDEL'"; + + assert_eq 0 "${EXIT_CODE}" "function func_dhcp6_shared_network_BDEL() broken or missing. (expected status code %d, returned %d)" + + count=$(echo "${OUTPUT}" | grep -Eci 'UPDATE dhcp6_subnet SET shared_network_name = NULL') || true + assert_eq 1 "${count}" "function func_dhcp6_shared_network_BDEL() is missing changed line. (expected count %d, returned %d)" +} + mysql_upgrade_test() { test_start "mysql.upgrade" @@ -389,14 +409,14 @@ mysql_upgrade_test() { version=$("${kea_admin}" db-version mysql -u "${db_user}" -p "${db_password}" -n "${db_name}" -d "${db_scripts_dir}") assert_str_eq "1.0" "${version}" "Expected kea-admin to return %s, returned value was %s" - # Ok, we have a 1.0 database. Let's upgrade it to 13.0. + # Ok, we have a 1.0 database. Let's upgrade it to 14.0. run_command \ "${kea_admin}" db-upgrade mysql -u "${db_user}" -p "${db_password}" -n "${db_name}" -d "${db_scripts_dir}" assert_eq 0 "${EXIT_CODE}" "kea-admin db-upgrade mysql failed, expected %d, returned non-zero status code %d\n" - # Verify upgraded schema reports version 13.0. + # Verify upgraded schema reports version 14.0. version=$("${kea_admin}" db-version mysql -u "${db_user}" -p "${db_password}" -n "${db_name}" -d "${db_scripts_dir}") - assert_str_eq "13.0" "${version}" "Expected kea-admin to return %s, returned value was %s" + assert_str_eq "14.0" "${version}" "Expected kea-admin to return %s, returned value was %s" # Let's check that the new tables are indeed there. @@ -1020,6 +1040,9 @@ SET @disable_audit = 0;" # Check upgrade from 12.0 to 13.0. mysql_upgrade_12_to_13_test + # Check upgrade from 13.0 to 14.0. + mysql_upgrade_13_to_14_test + # Let's wipe the whole database mysql_wipe diff --git a/src/bin/admin/tests/pgsql_tests.sh.in b/src/bin/admin/tests/pgsql_tests.sh.in index 25268bb1ff..40baa83ef8 100644 --- a/src/bin/admin/tests/pgsql_tests.sh.in +++ b/src/bin/admin/tests/pgsql_tests.sh.in @@ -143,7 +143,7 @@ pgsql_db_version_test() { run_command \ "${kea_admin}" db-version pgsql -u "${db_user}" -p "${db_password}" -n "${db_name}" version="${OUTPUT}" - assert_str_eq "11.0" "${version}" "Expected kea-admin to return %s, returned value was %s" + assert_str_eq "12.0" "${version}" "Expected kea-admin to return %s, returned value was %s" # Let's wipe the whole database pgsql_wipe @@ -460,6 +460,28 @@ pgsql_upgrade_10_0_to_11_0() { assert_eq 1 "${count}" "function createOptionAuditDHCP6() is missing changed line. (expected count %d, returned %d)" } +pgsql_upgrade_11_0_to_12_0() { + run_command \ + pgsql_execute "$session_sql" + + # Check function source code + run_command \ + pgsql_execute "select proname,prosrc from pg_proc where proname='func_dhcp4_shared_network_bdel'" + + assert_eq 0 "${EXIT_CODE}" "function func_dhcp4_shared_network_BDEL() broken or missing. (expected status code %d, returned %d)" + + count=$(echo "${OUTPUT}" | grep -Eci 'UPDATE dhcp4_subnet SET shared_network_name = NULL') || true + assert_eq 1 "${count}" "function func_dhcp4_shared_network_BDEL() is missing changed line. (expected count %d, returned %d)" + + # Check function source code + run_command \ + pgsql_execute "select proname,prosrc from pg_proc where proname='func_dhcp6_shared_network_bdel'" + + assert_eq 0 "${EXIT_CODE}" "function func_dhcp6_shared_network_BDEL() broken or missing. (expected status code %d, returned %d)" + + count=$(echo "${OUTPUT}" | grep -Eci 'UPDATE dhcp6_subnet SET shared_network_name = NULL') || true + assert_eq 1 "${count}" "function func_dhcp6_shared_network_BDEL() is missing changed line. (expected count %d, returned %d)" +} pgsql_upgrade_test() { test_start "pgsql.upgrade-test" @@ -476,9 +498,9 @@ pgsql_upgrade_test() { "${kea_admin}" db-upgrade pgsql -u "${db_user}" -p "${db_password}" -n "${db_name}" -d "${db_scripts_dir}" assert_eq 0 "${EXIT_CODE}" "db-upgrade failed, expected exit code: %d, actual: %d" - # Verify upgraded schema reports version 11.0. + # Verify upgraded schema reports version 12.0. version=$("${kea_admin}" db-version pgsql -u "${db_user}" -p "${db_password}" -n "${db_name}" -d "${db_scripts_dir}") - assert_str_eq "11.0" "${version}" 'Expected kea-admin to return %s, returned value was %s' + assert_str_eq "12.0" "${version}" 'Expected kea-admin to return %s, returned value was %s' # Check 1.0 to 2.0 upgrade pgsql_upgrade_1_0_to_2_0 @@ -507,6 +529,9 @@ pgsql_upgrade_test() { # Check 10.0 to 11.0 upgrade pgsql_upgrade_10_0_to_11_0 + # Check 11.0 to 12.0 upgrade + pgsql_upgrade_11_0_to_12_0 + # Let's wipe the whole database pgsql_wipe 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 af7093c06e..63d1f10599 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 @@ -267,8 +267,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, deleteSharedNetworkSubnets4Test) { deleteSharedNetworkSubnets4Test(); } -/// @todo This test is disabled pending resolution of #2299. -TEST_F(MySqlConfigBackendDHCPv4Test, DISABLED_getAllSharedNetworks4Test) { +TEST_F(MySqlConfigBackendDHCPv4Test, getAllSharedNetworks4Test) { getAllSharedNetworks4Test(); } 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 cc05ed25c0..73d889194c 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 @@ -267,8 +267,7 @@ TEST_F(MySqlConfigBackendDHCPv6Test, deleteSharedNetworkSubnets6Test) { deleteSharedNetworkSubnets6Test(); } -/// @todo This test is disabled pending resolution of #2299. -TEST_F(MySqlConfigBackendDHCPv6Test, DISABLED_getAllSharedNetworks6Test) { +TEST_F(MySqlConfigBackendDHCPv6Test, getAllSharedNetworks6Test) { getAllSharedNetworks6Test(); } diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc index 5ab0265656..587a06bd84 100644 --- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc @@ -2359,10 +2359,6 @@ GenericConfigBackendDHCPv4Test::getAllSharedNetworks4Test() { // The last parameter indicates that we expect four new audit entries, // two for deleted shared networks and two for updated subnets std::vector exp_entries({ - { - "dhcp4_shared_network", - AuditEntry::ModificationType::DELETE, "deleted all shared networks" - }, { "dhcp4_shared_network", AuditEntry::ModificationType::DELETE, "deleted all shared networks" @@ -2374,6 +2370,10 @@ GenericConfigBackendDHCPv4Test::getAllSharedNetworks4Test() { { "dhcp4_subnet", AuditEntry::ModificationType::UPDATE, "deleted all shared networks" + }, + { + "dhcp4_shared_network", + AuditEntry::ModificationType::DELETE, "deleted all shared networks" } }); diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc index e2c782938f..eae2097d70 100644 --- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc @@ -2386,10 +2386,6 @@ GenericConfigBackendDHCPv6Test::getAllSharedNetworks6Test() { // The last parameter indicates that we expect four new audit entries, // two for deleted shared networks and two for updated subnets std::vector exp_entries({ - { - "dhcp6_shared_network", - AuditEntry::ModificationType::DELETE, "deleted all shared networks" - }, { "dhcp6_shared_network", AuditEntry::ModificationType::DELETE, "deleted all shared networks" @@ -2401,6 +2397,10 @@ GenericConfigBackendDHCPv6Test::getAllSharedNetworks6Test() { { "dhcp6_subnet", AuditEntry::ModificationType::UPDATE, "deleted all shared networks" + }, + { + "dhcp6_shared_network", + AuditEntry::ModificationType::DELETE, "deleted all shared networks" } }); diff --git a/src/lib/mysql/mysql_constants.h b/src/lib/mysql/mysql_constants.h index f556d061a9..f7f38b9e94 100644 --- a/src/lib/mysql/mysql_constants.h +++ b/src/lib/mysql/mysql_constants.h @@ -52,7 +52,7 @@ const int MLM_MYSQL_FETCH_FAILURE = 0; /// @name Current database schema version values. //@{ -const uint32_t MYSQL_SCHEMA_VERSION_MAJOR = 13; +const uint32_t MYSQL_SCHEMA_VERSION_MAJOR = 14; const uint32_t MYSQL_SCHEMA_VERSION_MINOR = 0; //@} diff --git a/src/lib/pgsql/pgsql_connection.h b/src/lib/pgsql/pgsql_connection.h index f0b63e97d3..edd1d8b54d 100644 --- a/src/lib/pgsql/pgsql_connection.h +++ b/src/lib/pgsql/pgsql_connection.h @@ -17,8 +17,8 @@ namespace isc { namespace db { -/// @brief Define PostgreSQL backend version: 11.0 -const uint32_t PGSQL_SCHEMA_VERSION_MAJOR = 11; +/// @brief Define PostgreSQL backend version: 12.0 +const uint32_t PGSQL_SCHEMA_VERSION_MAJOR = 12; const uint32_t PGSQL_SCHEMA_VERSION_MINOR = 0; // Maximum number of parameters that can be used a statement diff --git a/src/share/database/scripts/mysql/.gitignore b/src/share/database/scripts/mysql/.gitignore index 8603a6e342..511aa89d33 100644 --- a/src/share/database/scripts/mysql/.gitignore +++ b/src/share/database/scripts/mysql/.gitignore @@ -21,4 +21,5 @@ /upgrade_010_to_011.sh /upgrade_011_to_012.sh /upgrade_012_to_013.sh +/upgrade_013_to_014.sh /wipe_data.sh diff --git a/src/share/database/scripts/mysql/Makefile.am b/src/share/database/scripts/mysql/Makefile.am index 14af57feae..29ba228faf 100644 --- a/src/share/database/scripts/mysql/Makefile.am +++ b/src/share/database/scripts/mysql/Makefile.am @@ -32,6 +32,7 @@ mysql_SCRIPTS += upgrade_009.6_to_010.0.sh mysql_SCRIPTS += upgrade_010_to_011.sh mysql_SCRIPTS += upgrade_011_to_012.sh mysql_SCRIPTS += upgrade_012_to_013.sh +mysql_SCRIPTS += upgrade_013_to_014.sh mysql_SCRIPTS += wipe_data.sh DISTCLEANFILES = ${mysql_SCRIPTS} diff --git a/src/share/database/scripts/mysql/dhcpdb_create.mysql b/src/share/database/scripts/mysql/dhcpdb_create.mysql index c63c6b438c..f8038f9b2d 100644 --- a/src/share/database/scripts/mysql/dhcpdb_create.mysql +++ b/src/share/database/scripts/mysql/dhcpdb_create.mysql @@ -4286,6 +4286,70 @@ UPDATE schema_version -- This line concludes database upgrade to version 13. +-- Modify shared-network-name foreign key contraint on dhcp4_subnet to not perform +-- the update when the network is deleted the cascaded update will not execute +-- dhcp4_subnet update trigger leaving the updated subnets without audit_entries. +ALTER TABLE dhcp4_subnet + DROP FOREIGN KEY fk_dhcp4_subnet_shared_network; + +ALTER TABLE dhcp4_subnet + ADD CONSTRAINT fk_dhcp4_subnet_shared_network FOREIGN KEY (shared_network_name) + REFERENCES dhcp4_shared_network (name) + ON DELETE NO ACTION ON UPDATE NO ACTION; + +-- Modify BEFORE delete trigger on dhcp4_shared_network to explicitly +-- update dhcp4_subnets. This ensures there are audit entries for updated +-- subnets. +DROP TRIGGER dhcp4_shared_network_BDEL; + +DELIMITER $$ +CREATE TRIGGER dhcp4_shared_network_BDEL BEFORE DELETE ON dhcp4_shared_network + FOR EACH ROW + BEGIN + CALL createAuditEntryDHCP4('dhcp4_shared_network', OLD.id, "delete"); + -- In MySQL Foreign key constraint triggered updates will not cascade, so we explicitly + -- update subnets first which should ensure they get audit entries. + UPDATE dhcp4_subnet SET shared_network_name = NULL WHERE shared_network_name = OLD.name; + DELETE FROM dhcp4_options WHERE shared_network_name = OLD.name; + END $$ +DELIMITER ; + +-- Modify shared-network-name foreign key contraint on dhcp6_subnet to not perform +-- the update when the network is deleted the cascaded update will not execute +-- dhcp6_subnet update trigger leaving the updated subnets without audit_entries. +ALTER TABLE dhcp6_subnet + DROP FOREIGN KEY fk_dhcp6_subnet_shared_network; + +ALTER TABLE dhcp6_subnet + ADD CONSTRAINT fk_dhcp6_subnet_shared_network FOREIGN KEY (shared_network_name) + REFERENCES dhcp6_shared_network (name) + ON DELETE NO ACTION ON UPDATE NO ACTION; + +-- Modify BEFORE delete trigger on dhcp6_shared_network to explicitly +-- update dhcp6_subnets. This ensures there are audit entries for updated +-- subnets. +DROP TRIGGER dhcp6_shared_network_BDEL; + +DELIMITER $$ +CREATE TRIGGER dhcp6_shared_network_BDEL BEFORE DELETE ON dhcp6_shared_network + FOR EACH ROW + BEGIN + CALL createAuditEntryDHCP6('dhcp6_shared_network', OLD.id, "delete"); + -- In MySQL Foreign key constraint triggered updates will not cascade, so we explicitly + -- update subnets first which should ensure they get audit entries. + UPDATE dhcp6_subnet SET shared_network_name = NULL WHERE shared_network_name = OLD.name; + DELETE FROM dhcp6_options WHERE shared_network_name = OLD.name; + END $$ +DELIMITER ; + +-- Update the schema version number. +UPDATE schema_version + SET version = '14', minor = '0'; + + + +-- This line concludes database upgrade to version 14. + # Notes: # # Indexes diff --git a/src/share/database/scripts/mysql/upgrade_013_to_014.sh.in b/src/share/database/scripts/mysql/upgrade_013_to_014.sh.in new file mode 100644 index 0000000000..924b1bc566 --- /dev/null +++ b/src/share/database/scripts/mysql/upgrade_013_to_014.sh.in @@ -0,0 +1,116 @@ +#!/bin/sh + +# Copyright (C) 2022 Internet Systems Consortium, Inc. ("ISC") +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +# shellcheck disable=SC1091 +# SC1091: Not following: ... was not specified as input (see shellcheck -x). + +# Exit with error if commands exit with non-zero and if undefined variables are +# used. +set -eu + +# shellcheck disable=SC2034 +# SC2034: ... appears unused. Verify use (or export if used externally). +prefix="@prefix@" + +# Include utilities. Use installed version if available and +# use build version if it isn't. +if [ -e @datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh ]; then + . "@datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh" +else + . "@abs_top_builddir@/src/bin/admin/admin-utils.sh" +fi + +# Check version. +version=$(mysql_version "${@}") +if test "${version}" != "13.0"; then + printf 'This script upgrades 13.0 to 14.0. ' + printf 'Reported version is %s. Skipping upgrade.\n' "${version}" + exit 0 +fi + +# Get the schema name from database argument. We need this to +# query information_schema for the right database. +for arg in "${@}" +do + if ! printf '%s' "${arg}" | grep -Eq '^\-\-' + then + schema="$arg" + break + fi +done + +# Make sure we have the schema. +if [ -z "$schema" ] +then + printf "Could not find database schema name in cmd line args: %s\n" "${*}" + exit 255 +fi + +mysql "$@" </dev/null <