From 9e2a0463e55b85af85b29cca6f534c9d7a15633a Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 7 Feb 2022 09:44:27 -0800
Subject: [PATCH 1/5] tests/int: fix runc_spec for set -u

Older bash versions treats variable as unset if nothing has been
assigned to it. Here is an example from CentOS 7 system:

	[kir@localhost ~]$ bash -u -c 'x() { local args=(); echo "${args[@]}"; }; x'
	bash: args[@]: unbound variable
	[kir@localhost ~]$ echo $BASH_VERSION
	4.2.46(2)-release

Rewrite to work around this.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 tests/integration/helpers.bash | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash
index de6bf7050..91cd9967c 100644
--- a/tests/integration/helpers.bash
+++ b/tests/integration/helpers.bash
@@ -59,12 +59,10 @@ function __runc() {
 
 # Wrapper for runc spec.
 function runc_spec() {
-	local args=()
-	if [ "$ROOTLESS" -ne 0 ]; then
-		args+=("--rootless")
-	fi
+	local rootless=""
+	[ "$ROOTLESS" -ne 0 ] && rootless="--rootless"
 
-	runc spec "${args[@]}"
+	runc spec $rootless
 
 	# Always add additional mappings if we have idmaps.
 	if [[ "$ROOTLESS" -ne 0 ]] && [[ "$ROOTLESS_FEATURES" == *"idmap"* ]]; then

From 7da77d802e5734b76b8bd7d9a76c51690de8e3b4 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Tue, 8 Feb 2022 16:09:31 -0800
Subject: [PATCH 2/5] tests/int: don't add --root if $ROOT is not set

Some tests (those in help.bats and version.bats) do not use setup_bundle
(as they do not need to start any containers), and thus they do not set
$ROOT. As a consequence, these tests now call "runc --root /state" which
is not nice.

Make adding --root conditional (only if $ROOT is set).

Amazingly, this change breaks help.bats tests under rootless, because
"sudo rootless" does not change the value of XDG_RUNTIME_DIR which still
points to root-owned directory, and as a result we have this:

> runc foo -h (status=1):
> the path in $XDG_RUNTIME_DIR must be writable by the user
> time="2022-02-08T07:04:57Z" level=error msg="mkdir /run/user/0/runc: permission denied"

This could be fixed by adding proper $ROOT, but it's easier just to skip
those tests under non-root.

NOTE that version.bats is not broken because -v is handled by urfave/cli
very early, so app.Before function is not run.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 tests/integration/help.bats    | 7 +++++++
 tests/integration/helpers.bash | 3 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tests/integration/help.bats b/tests/integration/help.bats
index c512d7a9e..04bbc80cf 100644
--- a/tests/integration/help.bats
+++ b/tests/integration/help.bats
@@ -2,6 +2,13 @@
 
 load helpers
 
+function setup() {
+	# It does not make sense to repeat these trivial tests for non-root.
+	# Also, they fail due to $ROOT not being set and XDG_RUNTIME_DIR
+	# pointing to another user's directory after sudo rootless.
+	requires root
+}
+
 @test "runc -h" {
 	runc -h
 	[ "$status" -eq 0 ]
diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash
index 91cd9967c..bc6bf4b98 100644
--- a/tests/integration/helpers.bash
+++ b/tests/integration/helpers.bash
@@ -54,7 +54,8 @@ function runc() {
 
 # Raw wrapper for runc.
 function __runc() {
-	"$RUNC" ${RUNC_USE_SYSTEMD+--systemd-cgroup} --root "$ROOT/state" "$@"
+	"$RUNC" ${RUNC_USE_SYSTEMD+--systemd-cgroup} \
+		${ROOT:+--root "$ROOT/state"} "$@"
 }
 
 # Wrapper for runc spec.

From 99d5c0231fff54657f890b3eb9ffe50ff3133db6 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 28 Feb 2022 18:33:43 -0800
Subject: [PATCH 3/5] tests/int/{root,list}.bats: ALT_ROOT fixups in teardown

1. Add "unset ALT_ROOT" since it should not be used after teardown is
   called.

2. Remove "rm -rf $ALT_ROOT". It is not needed, because ALT_ROOT is a
   subdirectory of ROOT, which is removed in teardown_bundle.

3. Checking for ALT_ROOT being non-empty is a leftover from the era when
   teardown() was called as the first step from setup(). Since commit
   41670e21f0f36390ee9cb this is no longer the case, so the condition
   is no longer needed (plus, the `set -u` which is about to be added
   should catch any possible use of unset ALT_ROOT).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 tests/integration/list.bats | 5 ++---
 tests/integration/root.bats | 6 ++----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/tests/integration/list.bats b/tests/integration/list.bats
index 9d25dfd36..a22dd19a4 100644
--- a/tests/integration/list.bats
+++ b/tests/integration/list.bats
@@ -9,9 +9,8 @@ function setup() {
 }
 
 function teardown() {
-	if [ -n "$ALT_ROOT" ]; then
-		ROOT="$ALT_ROOT" teardown_bundle
-	fi
+	ROOT="$ALT_ROOT" teardown_bundle
+	unset ALT_ROOT
 	teardown_bundle
 }
 
diff --git a/tests/integration/root.bats b/tests/integration/root.bats
index 53bd1ce56..68bdf5e76 100644
--- a/tests/integration/root.bats
+++ b/tests/integration/root.bats
@@ -9,10 +9,8 @@ function setup() {
 }
 
 function teardown() {
-	if [ -n "$ALT_ROOT" ]; then
-		ROOT=$ALT_ROOT __runc delete -f test_dotbox
-		rm -rf "$ALT_ROOT"
-	fi
+	ROOT=$ALT_ROOT __runc delete -f test_dotbox
+	unset ALT_ROOT
 	teardown_bundle
 }
 

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 4/5] 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

From 38c21694ba8b3873c9d3792a19e0e4fcd46a887c Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 7 Feb 2022 10:29:45 -0800
Subject: [PATCH 5/5] tests/integration/helpers: set -u

This is a way to prevent the code doing something really bad when a
variable it uses is not set. Good to have since it helps to catch some
logical errors etc.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 tests/integration/helpers.bash | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash
index d76c5de80..90f92bdd7 100644
--- a/tests/integration/helpers.bash
+++ b/tests/integration/helpers.bash
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+set -u
+
 # bats-core v1.2.1 defines BATS_RUN_TMPDIR.
 if [ ! -v BATS_RUN_TMPDIR ]; then
 	echo "bats >= v1.2.1 is required. Aborting." >&2