From c593d4ff7b1fc37bb67bffaa1e0a896b136fdff6 Mon Sep 17 00:00:00 2001 From: Chris Dunlap Date: Fri, 4 Dec 2020 17:00:06 -0800 Subject: [PATCH 1/4] Sharness: Add munged_kill_daemon and munged_cleanup Add munged_kill_daemon() to kill an errant munged process left running in the background from a previous test, and munged_cleanup() which currently only calls munged_kill_daemon(). The situation of an errant munged process is most likely to occur when a munged test is expected to fail and instead erroneously succeeds since those tests do not include a corresponding munged_stop_daemon(). munged_cleanup() should be called at the end of any test script that starts a munged process. It is not necessary to call it or munged_kill_daemon() after every munged process is supposedly stopped since munged_kill_daemon() is now called by munged_start_daemon() to kill any previous munged process named in the $(MUNGE_PIDFILE} before starting a new one. By only checking for a leftover munged process named in the pidfile, munged_kill_daemon() will not interfere with munged processes belonging to other tests or system use. Tested: - Arch Linux - CentOS Stream 8, 8.3.2011, 7.9.2009, 6.10 - Debian sid, 10.8, 9.13, 8.11, 7.11, 6.0.10, 5.0.10, 4.0 - Fedora 33, 32, 31 - FreeBSD 12.2, 11.4 - NetBSD 9.1, 9.0, 8.1 - OpenBSD 6.8, 6.7, 6.6 - openSUSE 15.2, 15.1 - Raspberry Pi OS (Raspbian 10) [armv7l] - Ubuntu 20.10, 20.04.2 LTS, 18.04.5 LTS, 16.04.7 LTS, 14.04.6 LTS, 12.04.5 LTS Tested by calling munged_start_daemon() without a corresponding munged_stop_daemon() in order to leave the munged process running in the background. The test suite was run with debug=t and verbose=t to check for the test_debug() message for the killed munged pid. --- t/0010-basic.t | 4 +++ t/0011-munged-cmdline.t | 6 +++++ t/0012-munge-cmdline.t | 4 +++ t/0013-unmunge-cmdline.t | 4 +++ t/0021-munged-valgrind.t | 4 +++ t/0022-munge-valgrind.t | 4 +++ t/0023-unmunge-valgrind.t | 4 +++ t/0100-munged-lock.t | 8 +++++- t/0101-munged-security-socket.t | 6 +++-- t/0102-munged-security-keyfile.t | 6 +++++ t/0103-munged-security-logfile.t | 6 +++++ t/0104-munged-security-pidfile.t | 6 +++++ t/0105-munged-security-seedfile.t | 6 +++++ t/0110-munged-origin-addr.t | 6 +++++ t/sharness.d/03-munged.sh | 42 +++++++++++++++++++++++++++++-- 15 files changed, 111 insertions(+), 5 deletions(-) diff --git a/t/0010-basic.t b/t/0010-basic.t index 9294bab..1709c3f 100755 --- a/t/0010-basic.t +++ b/t/0010-basic.t @@ -84,4 +84,8 @@ test_expect_unstable 'check logfile for replay' ' grep "Replayed credential" "${MUNGE_LOGFILE}" ' +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0011-munged-cmdline.t b/t/0011-munged-cmdline.t index 95a1dc7..2566ce0 100755 --- a/t/0011-munged-cmdline.t +++ b/t/0011-munged-cmdline.t @@ -65,4 +65,10 @@ test_expect_failure 'finish writing tests' ' false ' +# Clean up after a munged process that may not have terminated. +## +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0012-munge-cmdline.t b/t/0012-munge-cmdline.t index 53d542a..57394f2 100755 --- a/t/0012-munge-cmdline.t +++ b/t/0012-munge-cmdline.t @@ -623,4 +623,8 @@ test_expect_success 'stop munged' ' munged_stop_daemon ' +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0013-unmunge-cmdline.t b/t/0013-unmunge-cmdline.t index c034109..c532123 100755 --- a/t/0013-unmunge-cmdline.t +++ b/t/0013-unmunge-cmdline.t @@ -245,4 +245,8 @@ test_expect_success 'stop munged' ' munged_stop_daemon ' +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0021-munged-valgrind.t b/t/0021-munged-valgrind.t index fb0dc56..071be97 100755 --- a/t/0021-munged-valgrind.t +++ b/t/0021-munged-valgrind.t @@ -40,4 +40,8 @@ test_expect_success 'check valgrind log for errors in munged' ' valgrind_check_log ' +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0022-munge-valgrind.t b/t/0022-munge-valgrind.t index 9e62bdb..ed9a7d2 100755 --- a/t/0022-munge-valgrind.t +++ b/t/0022-munge-valgrind.t @@ -32,4 +32,8 @@ test_expect_success 'check valgrind log for errors in munge' ' valgrind_check_log ' +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0023-unmunge-valgrind.t b/t/0023-unmunge-valgrind.t index e54fbbd..6788ee4 100755 --- a/t/0023-unmunge-valgrind.t +++ b/t/0023-unmunge-valgrind.t @@ -36,4 +36,8 @@ test_expect_success 'check valgrind log for errors in unmunge' ' valgrind_check_log ' +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0100-munged-lock.t b/t/0100-munged-lock.t index a1ab934..117e848 100755 --- a/t/0100-munged-lock.t +++ b/t/0100-munged-lock.t @@ -53,7 +53,7 @@ test_expect_success 'check lockfile permissions' ' # The lockfile should prevent this. ## test_expect_success 'start munged with in-use socket' ' - test_must_fail munged_start_daemon && + test_must_fail munged_start_daemon t-keep-process && egrep "Error:.* Failed to lock \"${MUNGE_LOCKFILE}\"" "${MUNGE_LOGFILE}" ' @@ -201,4 +201,10 @@ test_expect_success SUDO 'stop unprivileged munged as root' ' fi ' +# Clean up after a munged process that may not have terminated. +## +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0101-munged-security-socket.t b/t/0101-munged-security-socket.t index 560e4ac..532dc19 100755 --- a/t/0101-munged-security-socket.t +++ b/t/0101-munged-security-socket.t @@ -213,11 +213,13 @@ test_expect_success 'socket dir inaccessible by all override' ' "${MUNGE_LOGFILE}" ' -# Cleanup detritus from testing. +# Clean up detritus from testing. This may include an errant munged process +# that has not terminated. ## test_expect_success 'cleanup' ' rmdir "${MUNGE_SOCKETDIR}" && - if rmdir "$(dirname "${MUNGE_SOCKETDIR}")" 2>/dev/null; then :; fi + if rmdir "$(dirname "${MUNGE_SOCKETDIR}")" 2>/dev/null; then :; fi && + munged_cleanup ' test_done diff --git a/t/0102-munged-security-keyfile.t b/t/0102-munged-security-keyfile.t index 25e7ce2..5cc1808 100755 --- a/t/0102-munged-security-keyfile.t +++ b/t/0102-munged-security-keyfile.t @@ -358,4 +358,10 @@ test_expect_success 'keyfile dir writable by other with sticky bit' ' chmod 0755 "${MUNGE_KEYDIR}" ' +# Clean up after a munged process that may not have terminated. +## +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0103-munged-security-logfile.t b/t/0103-munged-security-logfile.t index c9887fd..fafd973 100755 --- a/t/0103-munged-security-logfile.t +++ b/t/0103-munged-security-logfile.t @@ -358,4 +358,10 @@ test_expect_success 'logfile failure writes single message to stderr' ' test "${NUM}" -eq 1 2>/dev/null ' +# Clean up after a munged process that may not have terminated. +## +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0104-munged-security-pidfile.t b/t/0104-munged-security-pidfile.t index dbe5825..0c2a505 100755 --- a/t/0104-munged-security-pidfile.t +++ b/t/0104-munged-security-pidfile.t @@ -42,4 +42,10 @@ test_expect_failure 'finish writing tests' ' false ' +# Clean up after a munged process that may not have terminated. +## +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0105-munged-security-seedfile.t b/t/0105-munged-security-seedfile.t index 31debc2..5008239 100755 --- a/t/0105-munged-security-seedfile.t +++ b/t/0105-munged-security-seedfile.t @@ -50,4 +50,10 @@ test_expect_failure 'finish writing tests' ' false ' +# Clean up after a munged process that may not have terminated. +## +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/0110-munged-origin-addr.t b/t/0110-munged-origin-addr.t index 3b4d369..7d0589c 100755 --- a/t/0110-munged-origin-addr.t +++ b/t/0110-munged-origin-addr.t @@ -139,4 +139,10 @@ test_expect_success 'munged --origin non-interface IP address metadata' ' egrep "^ENCODE_HOST:.* 192\.0\.0\.255\>" meta.$$ ' +# Clean up after a munged process that may not have terminated. +## +test_expect_success 'cleanup' ' + munged_cleanup +' + test_done diff --git a/t/sharness.d/03-munged.sh b/t/sharness.d/03-munged.sh index 0168326..6f6e975 100644 --- a/t/sharness.d/03-munged.sh +++ b/t/sharness.d/03-munged.sh @@ -73,19 +73,22 @@ munged_create_key() } ## -# Start munged, removing an existing logfile (from a previous run) if present. +# Start munged, removing an existing logfile or killing an errant munged +# process (from a previous run) if needed. # The following leading args are recognized: # t-exec=ARG - use ARG to exec munged. # t-keep-logfile - do not remove logfile before starting munged. +# t-keep-process - do not kill previous munged process. # Remaining args will be appended to the munged command-line. ## munged_start_daemon() { - local EXEC= KEEP_LOGFILE= && + local EXEC= KEEP_LOGFILE= KEEP_PROCESS= && while true; do case $1 in t-exec=*) EXEC=$(echo "$1" | sed 's/^[^=]*=//');; t-keep-logfile) KEEP_LOGFILE=1;; + t-keep-process) KEEP_PROCESS=1;; *) break;; esac shift @@ -93,6 +96,9 @@ munged_start_daemon() if test "${KEEP_LOGFILE}" != 1; then rm -f "${MUNGE_LOGFILE}" fi && + if test "${KEEP_PROCESS}" != 1; then + munged_kill_daemon + fi && test_debug "echo ${EXEC} \"${MUNGED}\" \ --socket=\"${MUNGE_SOCKET}\" \ --key-file=\"${MUNGE_KEYFILE}\" \ @@ -136,3 +142,35 @@ munged_stop_daemon() --stop \ "$@" } + +## +# Kill an errant munged process running in the background from a previous test. +# This situation is most likely to occur if a munged test is expected to fail +# and instead erroneously succeeds. +# Only check for the pid named in ${MUNGE_PIDFILE} to avoid intefering with +# munged processes belonging to other tests or system use. And check that +# the named pid is a munged process and not one recycled by the system for +# some other running process. +# A SIGTERM is used here instead of "munged --stop" in case the latter has a +# bug introduced that prevents cleanup from occurring. +# A SIGKILL would prevent the munged process from cleaning up which could cause +# other tests to inadvertently fail. +## +munged_kill_daemon() +{ + local PID + PID=$(cat "${MUNGE_PIDFILE}" 2>/dev/null) + if ps -p "${PID}" -ww 2>/dev/null | grep munged; then + kill "${PID}" + test_debug "echo \"Killed munged pid ${PID}\"" + fi +} + +## +# Perform any housekeeping to clean up after munged. This should be called +# at the end of any test script that starts a munged process. +## +munged_cleanup() +{ + munged_kill_daemon +} -- 2.30.0