From c8c3e8526d4e0a81f037f182186c5a1c64a5ff5f Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 7 Feb 2022 10:07:43 -0800
Subject: [PATCH] tests: fix checks for non-existent variables

Audit all checks for non-empty variables (i.e. ' -z ', ' -n ',
' != ""' and '= ""'), and fix those cases where a variable might be
unset. Those variables (that might not be set) are

 - RUNC_USE_SYSTEMD
 - BATS_RUN_TMPDIR
 - AUX_UID
 - AUX_DIR
 - SD_PARENT_NAME
 - REL_PARENT_PATH
 - ROOT
 - HAVE_CRIU
 - ROOTLESS_FEATURES
 - and a few test-specific or file-specific variables

This should allow us to enable set -u.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 tests/integration/cgroups.bats    |  2 +-
 tests/integration/checkpoint.bats |  2 +-
 tests/integration/cwd.bats        |  2 +-
 tests/integration/helpers.bash    | 62 ++++++++++++++++---------------
 tests/integration/hooks.bats      |  3 +-
 tests/integration/update.bats     |  2 +-
 tests/rootless.sh                 |  7 ++--
 7 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats
index ff7cf6d0b..89c63f398 100644
--- a/tests/integration/cgroups.bats
+++ b/tests/integration/cgroups.bats
@@ -45,7 +45,7 @@ function setup() {
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_permissions
 	[ "$status" -eq 0 ]
 	if [ "$CGROUP_UNIFIED" != "no" ]; then
-		if [ -n "${RUNC_USE_SYSTEMD}" ]; then
+		if [ -v RUNC_USE_SYSTEMD ]; then
 			if [ "$(id -u)" = "0" ]; then
 				check_cgroup_value "cgroup.controllers" "$(cat /sys/fs/cgroup/machine.slice/cgroup.controllers)"
 			else
diff --git a/tests/integration/checkpoint.bats b/tests/integration/checkpoint.bats
index 9bf999400..98b112dc6 100644
--- a/tests/integration/checkpoint.bats
+++ b/tests/integration/checkpoint.bats
@@ -252,7 +252,7 @@ function simple_cr() {
 	# in time when the last page is lazily transferred to the destination.
 	# Killing the CRIU on the checkpoint side will let the container
 	# continue to run if the migration failed at some point.
-	[ -n "$RUNC_USE_SYSTEMD" ] && set_cgroups_path
+	[ -v RUNC_USE_SYSTEMD ] && set_cgroups_path
 	runc_restore_with_pipes ./image-dir test_busybox_restore --lazy-pages
 
 	wait $cpt_pid
diff --git a/tests/integration/cwd.bats b/tests/integration/cwd.bats
index 114efb991..f1859623b 100644
--- a/tests/integration/cwd.bats
+++ b/tests/integration/cwd.bats
@@ -35,7 +35,7 @@ function teardown() {
 
 	# Some setup for this test (AUX_DIR and AUX_UID) is done
 	# by rootless.sh. Check that setup is done...
-	if [[ ! -d "$AUX_DIR" || -z "$AUX_UID" ]]; then
+	if [[ ! -v AUX_UID || ! -v AUX_DIR || ! -d "$AUX_DIR" ]]; then
 		skip "bad/unset AUX_DIR/AUX_UID"
 	fi
 	# ... and is correct, i.e. the current user
diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash
index bc6bf4b98..d76c5de80 100644
--- a/tests/integration/helpers.bash
+++ b/tests/integration/helpers.bash
@@ -1,7 +1,7 @@
 #!/bin/bash
 
-# bats-core v1.2.1 defines BATS_RUN_TMPDIR
-if [ -z "$BATS_RUN_TMPDIR" ]; then
+# bats-core v1.2.1 defines BATS_RUN_TMPDIR.
+if [ ! -v BATS_RUN_TMPDIR ]; then
 	echo "bats >= v1.2.1 is required. Aborting." >&2
 	exit 1
 fi
@@ -19,6 +19,10 @@ RECVTTY="${INTEGRATION_ROOT}/../../contrib/cmd/recvtty/recvtty"
 SD_HELPER="${INTEGRATION_ROOT}/../../contrib/cmd/sd-helper/sd-helper"
 SECCOMP_AGENT="${INTEGRATION_ROOT}/../../contrib/cmd/seccompagent/seccompagent"
 
+# Some variables may not always be set. Set those to empty value,
+# if unset, to avoid "unbound variable" error.
+: "${ROOTLESS_FEATURES:=}"
+
 # Test data path.
 # shellcheck disable=SC2034
 TESTDATA="${INTEGRATION_ROOT}/testdata"
@@ -66,7 +70,7 @@ function runc_spec() {
 	runc spec $rootless
 
 	# Always add additional mappings if we have idmaps.
-	if [[ "$ROOTLESS" -ne 0 ]] && [[ "$ROOTLESS_FEATURES" == *"idmap"* ]]; then
+	if [[ "$ROOTLESS" -ne 0 && "$ROOTLESS_FEATURES" == *"idmap"* ]]; then
 		runc_rootless_idmap
 	fi
 }
@@ -88,7 +92,7 @@ function runc_rootless_idmap() {
 
 # Returns systemd version as a number (-1 if systemd is not enabled/supported).
 function systemd_version() {
-	if [ -n "${RUNC_USE_SYSTEMD}" ]; then
+	if [ -v RUNC_USE_SYSTEMD ]; then
 		systemctl --version | awk '/^systemd / {print $2; exit}'
 		return
 	fi
@@ -98,7 +102,7 @@ function systemd_version() {
 
 function init_cgroup_paths() {
 	# init once
-	test -n "$CGROUP_UNIFIED" && return
+	[ -v CGROUP_UNIFIED ] && return
 
 	if stat -f -c %t /sys/fs/cgroup | grep -qFw 63677270; then
 		CGROUP_UNIFIED=yes
@@ -106,7 +110,7 @@ function init_cgroup_paths() {
 		# For rootless + systemd case, controllers delegation is required,
 		# so check the controllers that the current user has, not the top one.
 		# NOTE: delegation of cpuset requires systemd >= 244 (Fedora >= 32, Ubuntu >= 20.04).
-		if [[ "$ROOTLESS" -ne 0 && -n "$RUNC_USE_SYSTEMD" ]]; then
+		if [[ "$ROOTLESS" -ne 0 && -v RUNC_USE_SYSTEMD ]]; then
 			controllers="/sys/fs/cgroup/user.slice/user-$(id -u).slice/user@$(id -u).service/cgroup.controllers"
 		fi
 
@@ -141,11 +145,11 @@ function init_cgroup_paths() {
 }
 
 function create_parent() {
-	if [ -n "$RUNC_USE_SYSTEMD" ]; then
-		[ -z "$SD_PARENT_NAME" ] && return
+	if [ -v RUNC_USE_SYSTEMD ]; then
+		[ ! -v SD_PARENT_NAME ] && return
 		"$SD_HELPER" --parent machine.slice start "$SD_PARENT_NAME"
 	else
-		[ -z "$REL_PARENT_PATH" ] && return
+		[ ! -v REL_PARENT_PATH ] && return
 		if [ "$CGROUP_UNIFIED" == "yes" ]; then
 			mkdir "/sys/fs/cgroup$REL_PARENT_PATH"
 		else
@@ -161,11 +165,11 @@ function create_parent() {
 }
 
 function remove_parent() {
-	if [ -n "$RUNC_USE_SYSTEMD" ]; then
-		[ -z "$SD_PARENT_NAME" ] && return
+	if [ -v RUNC_USE_SYSTEMD ]; then
+		[ ! -v SD_PARENT_NAME ] && return
 		"$SD_HELPER" --parent machine.slice stop "$SD_PARENT_NAME"
 	else
-		[ -z "$REL_PARENT_PATH" ] && return
+		[ ! -v REL_PARENT_PATH ] && return
 		if [ "$CGROUP_UNIFIED" == "yes" ]; then
 			rmdir "/sys/fs/cgroup/$REL_PARENT_PATH"
 		else
@@ -180,8 +184,8 @@ function remove_parent() {
 }
 
 function set_parent_systemd_properties() {
-	[ -z "$SD_PARENT_NAME" ] && return
-	local user
+	[ ! -v SD_PARENT_NAME ] && return
+	local user=""
 	[ "$(id -u)" != "0" ] && user="--user"
 	systemctl set-property $user "$SD_PARENT_NAME" "$@"
 }
@@ -194,7 +198,7 @@ function set_parent_systemd_properties() {
 # refer to it.
 function set_cgroups_path() {
 	init_cgroup_paths
-	local pod dash_pod slash_pod pod_slice
+	local pod dash_pod="" slash_pod="" pod_slice=""
 	if [ "$#" -ne 0 ] && [ "$1" != "" ]; then
 		# Set up a parent/pod cgroup.
 		pod="$1"
@@ -205,7 +209,7 @@ function set_cgroups_path() {
 	fi
 
 	local rnd="$RANDOM"
-	if [ -n "${RUNC_USE_SYSTEMD}" ]; then
+	if [ -v RUNC_USE_SYSTEMD ]; then
 		SD_UNIT_NAME="runc-cgroups-integration-test-${rnd}.scope"
 		if [ "$(id -u)" = "0" ]; then
 			REL_PARENT_PATH="/machine.slice${pod_slice}"
@@ -227,7 +231,7 @@ function set_cgroups_path() {
 		CGROUP_PATH=${CGROUP_BASE_PATH}${REL_CGROUPS_PATH}
 	fi
 
-	[ -n "$pod" ] && create_parent
+	[ -v pod ] && create_parent
 
 	update_config '.linux.cgroupsPath |= "'"${OCI_CGROUPS_PATH}"'"'
 }
@@ -259,11 +263,11 @@ function check_cgroup_value() {
 
 # Helper to check a value in systemd.
 function check_systemd_value() {
-	[ -z "${RUNC_USE_SYSTEMD}" ] && return
+	[ ! -v RUNC_USE_SYSTEMD ] && return
 	local source="$1"
 	[ "$source" = "unsupported" ] && return
 	local expected="$2"
-	local expected2="$3"
+	local expected2="${3:-}"
 	local user=""
 	[ "$(id -u)" != "0" ] && user="--user"
 
@@ -339,7 +343,7 @@ function fail() {
 
 # Check whether rootless runc can use cgroups.
 function rootless_cgroup() {
-	[[ "$ROOTLESS_FEATURES" == *"cgroup"* || -n "$RUNC_USE_SYSTEMD" ]]
+	[[ "$ROOTLESS_FEATURES" == *"cgroup"* || -v RUNC_USE_SYSTEMD ]]
 }
 
 # Allows a test to specify what things it requires. If the environment can't
@@ -349,7 +353,7 @@ function requires() {
 		local skip_me
 		case $var in
 		criu)
-			if [ -n "$HAVE_CRIU" ]; then
+			if [ ! -v HAVE_CRIU ]; then
 				skip_me=1
 			fi
 			;;
@@ -379,7 +383,7 @@ function requires() {
 			fi
 			;;
 		rootless_no_features)
-			if [ "$ROOTLESS_FEATURES" != "" ]; then
+			if [ -n "$ROOTLESS_FEATURES" ]; then
 				skip_me=1
 			fi
 			;;
@@ -414,7 +418,7 @@ function requires() {
 			;;
 		cgroups_hybrid)
 			init_cgroup_paths
-			if [ "$CGROUP_HYBRID" != "yes" ]; then
+			if [ ! -v CGROUP_HYBRID ]; then
 				skip_me=1
 			fi
 			;;
@@ -433,12 +437,12 @@ function requires() {
 			fi
 			;;
 		systemd)
-			if [ -z "${RUNC_USE_SYSTEMD}" ]; then
+			if [ ! -v RUNC_USE_SYSTEMD ]; then
 				skip_me=1
 			fi
 			;;
 		no_systemd)
-			if [ -n "${RUNC_USE_SYSTEMD}" ]; then
+			if [ -v RUNC_USE_SYSTEMD ]; then
 				skip_me=1
 			fi
 			;;
@@ -451,7 +455,7 @@ function requires() {
 			fail "BUG: Invalid requires $var."
 			;;
 		esac
-		if [ -n "$skip_me" ]; then
+		if [ -v skip_me ]; then
 			skip "test requires $var"
 		fi
 	done
@@ -501,7 +505,7 @@ function testcontainer() {
 }
 
 function setup_recvtty() {
-	[ -z "$ROOT" ] && return 1 # must not be called without ROOT set
+	[ ! -v ROOT ] && return 1 # must not be called without ROOT set
 	local dir="$ROOT/tty"
 
 	mkdir "$dir"
@@ -512,7 +516,7 @@ function setup_recvtty() {
 }
 
 function teardown_recvtty() {
-	[ -z "$ROOT" ] && return 0 # nothing to teardown
+	[ ! -v ROOT ] && return 0 # nothing to teardown
 	local dir="$ROOT/tty"
 
 	# When we kill recvtty, the container will also be killed.
@@ -571,7 +575,7 @@ function setup_debian() {
 }
 
 function teardown_bundle() {
-	[ -z "$ROOT" ] && return 0 # nothing to teardown
+	[ ! -v ROOT ] && return 0 # nothing to teardown
 
 	cd "$INTEGRATION_ROOT" || return
 	teardown_recvtty
diff --git a/tests/integration/hooks.bats b/tests/integration/hooks.bats
index 25c08aa8b..1850f33e6 100644
--- a/tests/integration/hooks.bats
+++ b/tests/integration/hooks.bats
@@ -13,10 +13,11 @@ function setup() {
 }
 
 function teardown() {
-	if [ -n "$LIBPATH" ]; then
+	if [ -v LIBPATH ]; then
 		umount "$LIBPATH"/$HOOKLIBCR.1.0.0 &>/dev/null || true
 		umount "$LIBPATH"/$HOOKLIBCC.1.0.0 &>/dev/null || true
 		rm -f $HOOKLIBCR.1.0.0 $HOOKLIBCC.1.0.0
+		unset LIBPATH HOOKLIBCR HOOKLIBCC
 	fi
 	teardown_bundle
 }
diff --git a/tests/integration/update.bats b/tests/integration/update.bats
index 01c691529..f4f349aa9 100644
--- a/tests/integration/update.bats
+++ b/tests/integration/update.bats
@@ -399,7 +399,7 @@ EOF
 	set_cgroups_path "pod_${RANDOM}"
 
 	# Set parent/pod CPU quota limit to 50%.
-	if [ -n "${RUNC_USE_SYSTEMD}" ]; then
+	if [ -v RUNC_USE_SYSTEMD ]; then
 		set_parent_systemd_properties CPUQuota="50%"
 	else
 		echo 50000 >"/sys/fs/cgroup/cpu/$REL_PARENT_PATH/cpu.cfs_quota_us"
diff --git a/tests/rootless.sh b/tests/rootless.sh
index 952a6dd8d..f6872fad0 100755
--- a/tests/rootless.sh
+++ b/tests/rootless.sh
@@ -20,12 +20,11 @@
 # and add an enable_* and disable_* hook.
 
 set -e -u -o pipefail
-: "${RUNC_USE_SYSTEMD:=}"
 : "${ROOTLESS_TESTPATH:=}"
 
 ALL_FEATURES=("idmap" "cgroup")
-# cgroup is managed by systemd when RUNC_USE_SYSTEMD is set
-if [[ -n "${RUNC_USE_SYSTEMD}" ]]; then
+# cgroup is managed by systemd when RUNC_USE_SYSTEMD is set.
+if [ -v RUNC_USE_SYSTEMD ]; then
 	ALL_FEATURES=("idmap")
 fi
 ROOT="$(readlink -f "$(dirname "${BASH_SOURCE[0]}")/..")"
@@ -182,7 +181,7 @@ for enabled_features in $features_powerset; do
 	# Run the test suite!
 	echo "path: $PATH"
 	export ROOTLESS_FEATURES="$enabled_features"
-	if [[ -n "${RUNC_USE_SYSTEMD}" ]]; then
+	if [ -v RUNC_USE_SYSTEMD ]; then
 		# We use `ssh rootless@localhost` instead of `sudo -u rootless` for creating systemd user session.
 		# Alternatively we could use `machinectl shell`, but it is known not to work well on SELinux-enabled hosts as of April 2020:
 		# https://bugzilla.redhat.com/show_bug.cgi?id=1788616