RED-159320 Conforming with shellcheck and slight changes to tests

Author: Peter-Sh

.github/actions/build-and-tag-locally/action.yml

@@ -212,16 +212,16 @@ runs:
         cd test && env
         PLATFORM=${{ steps.platform.outputs.display_name }}
         REDIS_IMG=${{ github.sha }}:${{ steps.platform.outputs.display_name }}
-        ./test.sh
-        -- --output-junit-xml=report.xml
+        ./run-entrypoint-tests.sh
+        -- --output-junit-xml=report-entrypoint.xml
 
     - name: Test Report
       uses: dorny/test-reporter@v2
       # run this step even if previous step failed, but not if it was skipped
       if: ${{ !cancelled() && steps.test_entrypoint.conclusion != 'skipped' }}
       with:
         name: Entrypoint Tests
-        path: test/report.xml
+        path: test/report-entrypoint.xml
         reporter: java-junit
 
     - name: Push image

alpine/docker-entrypoint.sh

@@ -20,11 +20,15 @@
 #
 ##
 
-# By default create files owned by root
+if [ -z "$REDIS_IMG" ]; then
+	echo "REDIS_IMG may not be empty"
+	exit 1
+fi
+# By default create files owned by root to avoid intersecting with container user
 HOST_UID=0
 HOST_GID=0
 if docker info 2>/dev/null | grep -qi rootless; then
-	# For rootless docker use current user
+	# For rootless docker we have to use current user
 	HOST_UID=$(id -u)
 	HOST_GID=$(id -g)
 fi
@@ -33,13 +37,13 @@ HOST_OWNER=$HOST_UID:$HOST_GID
 get_container_user_uid_gid_on_the_host() {
 	container_user="$1"
 	dir=$(mktemp -d -p .)
-	docker run --rm -v `pwd`/$dir:/w -w /w --entrypoint=/bin/sh $REDIS_IMG -c "chown $container_user ."
+	docker run --rm -v "$(pwd)/$dir":/w -w /w --entrypoint=/bin/sh "$REDIS_IMG" -c "chown $container_user ."
 	stat -c "%u %g" "$dir"
 	sudo rm -rf "$dir"
 }
 
 # Detect how redis user and group from the container are mapped to the host ones
-read REDIS_UID REDIS_GID <<< "$(get_container_user_uid_gid_on_the_host redis:redis)"
+read -r REDIS_UID _ <<< "$(get_container_user_uid_gid_on_the_host redis:redis)"
 
 if [ "$REDIS_UID" == "$HOST_UID" ]; then
 	echo "Cannot test ownership as redis user uid is the same as current user"
@@ -48,6 +52,7 @@ fi
 
 # Helper functions #
 
+# creates one entry of directory structure
 # used in combination with iterate_dir_structure_with
 create_entry() {
 	dir="$1"
@@ -63,15 +68,16 @@ create_entry() {
 	sudo chown "$initial_owner" "$dir/$entry"
 }
 
+# asserts ownership and permissions for one entry from directory structure
 # used in combination with iterate_dir_structure_with
 assert_entry() {
 	dir="$1"
 	msg="$2"
 	actual_uid=$(sudo stat -c %u "$dir/$entry")
 	actual_mode=0$(sudo stat -c '%a' "$dir/$entry")
-	actual_mask=$(printf "0%03o" $(( $actual_mode & $expected_mode_mask )))
-	assertEquals "$msg: Owner for $type '$entry'" $expected_owner $actual_uid
-	assertEquals "$msg: Mode mask for $type '$entry'"  $expected_mode_mask $actual_mask
+	actual_mask=$(printf "0%03o" $(( actual_mode & expected_mode_mask )))
+	assertEquals "$msg: Owner for $type '$entry'" "$expected_owner" "$actual_uid"
+	assertEquals "$msg: Mode mask for $type '$entry'"  "$expected_mode_mask" "$actual_mask"
 }
 
 # Iterates over directory structure assigning variables and executing command
@@ -151,7 +157,7 @@ run_docker_and_test_ownership() {
 		;;
 		esac
 	done
-	docker_cmd="$@"
+	docker_cmd="$*"
 
 	if [ -z "$mount_target" ]; then
 		fail "Mount target is empty"
@@ -160,9 +166,9 @@ run_docker_and_test_ownership() {
 
 	dir=$(mktemp -d -p .)
 
-	iterate_dir_structure_with create_entry "$dir" <<<$dir_structure
+	iterate_dir_structure_with create_entry "$dir" <<<"$dir_structure"
 
-	docker_run="docker run --rm -v `pwd`/$dir:$mount_target $docker_flags $REDIS_IMG $docker_cmd"
+	docker_run="docker run --rm -v "$(pwd)/$dir":$mount_target $docker_flags $REDIS_IMG $docker_cmd"
 	if [ "$TEST_VERBOSE" ]; then
 		echo -e "\n#### ownership test: $docker_cmd"
 		echo "running $docker_run"
@@ -179,7 +185,7 @@ run_docker_and_test_ownership() {
 		echo "$docker_output"
 	fi
 
-	iterate_dir_structure_with assert_entry "$dir" "$docker_cmd" <<<$dir_structure
+	iterate_dir_structure_with assert_entry "$dir" "$docker_cmd" <<<"$dir_structure"
 
 	if [ "$extra_assert" ]; then
 		$extra_assert
@@ -192,19 +198,19 @@ run_docker_and_test_ownership() {
 # -v option will make redis-server to either return version or fail (if config has been provided)
 # either one is OK for us
 run_docker_and_test_ownership_with_common_flags_for_server() {
-	run_docker_and_test_ownership "${common_flags[@]}" $@ -v
-	run_docker_and_test_ownership "${common_flags[@]}" redis-server $@ -v
-	run_docker_and_test_ownership "${common_flags[@]}" /usr/local/bin/redis-server $@ -v
+	run_docker_and_test_ownership "${common_flags[@]}" "$@" -v
+	run_docker_and_test_ownership "${common_flags[@]}" redis-server "$@" -v
+	run_docker_and_test_ownership "${common_flags[@]}" /usr/local/bin/redis-server "$@" -v
 }
 
 # running redis-sentinel using different forms and --dumb-option
 # expecting sentinel to fail, it's ok as we are only interested in entrypoint testing here
 run_docker_and_test_ownership_with_common_flags_for_sentinel() {
-	run_docker_and_test_ownership "${common_flags[@]}" $@ --sentinel --dumb-option
-	run_docker_and_test_ownership "${common_flags[@]}" redis-sentinel $@ --dumb-option
-	run_docker_and_test_ownership "${common_flags[@]}" /usr/local/bin/redis-sentinel $@ --dumb-option
-	run_docker_and_test_ownership "${common_flags[@]}" redis-server $@ --sentinel --dumb-option
-	run_docker_and_test_ownership "${common_flags[@]}" /usr/local/bin/redis-server $@ --sentinel --dumb-option
+	run_docker_and_test_ownership "${common_flags[@]}" "$@" --sentinel --dumb-option
+	run_docker_and_test_ownership "${common_flags[@]}" redis-sentinel "$@" --dumb-option
+	run_docker_and_test_ownership "${common_flags[@]}" /usr/local/bin/redis-sentinel "$@" --dumb-option
+	run_docker_and_test_ownership "${common_flags[@]}" redis-server "$@" --sentinel --dumb-option
+	run_docker_and_test_ownership "${common_flags[@]}" /usr/local/bin/redis-server "$@" --sentinel --dumb-option
 }
 
 # start redis server or sentinel and check process uid and gid
@@ -230,7 +236,7 @@ run_redis_docker_and_check_uid_gid() {
 			shift 2
 		;;
 			--docker-flags)
-			docker_flags="$2"
+			docker_flags=$2
 			shift 2
 		;;
 			--file-owner)
@@ -248,7 +254,7 @@ run_redis_docker_and_check_uid_gid() {
 	done
 
 	if echo "$expected_cmd" | grep -q "sentinel"; then
-		dir=$(readlink -f $(mktemp -d -p .))
+		dir="$(readlink -f "$(mktemp -d -p .)")"
 		touch "$dir/sentinel.conf"
 		if [ "$file_owner" ]; then
 			sudo chown -R "$file_owner" "$dir"
@@ -257,7 +263,8 @@ run_redis_docker_and_check_uid_gid() {
 	fi
 
 	docker_cmd="$*"
-	container=$(docker run $docker_flags -d $REDIS_IMG $docker_cmd)
+	# shellcheck disable=SC2086
+	container=$(docker run $docker_flags -d "$REDIS_IMG" $docker_cmd)
 	ret=$?
 
 	assertTrue "Container '$docker_flags $REDIS_IMG $docker_cmd' created" "[ $ret -eq 0 ]"
@@ -269,8 +276,8 @@ run_redis_docker_and_check_uid_gid() {
 	cmdline=$(docker exec "$container" cat /proc/1/cmdline|tr -d \\0)
 	assertContains "$docker_flags $docker_cmd, cmdline: $cmdline" "$cmdline" "$expected_cmd"
 
-	redis_user_uid=$(docker exec "$container" id -u $user)
-	redis_user_gid=$(docker exec "$container" id -g $group)
+	redis_user_uid=$(docker exec "$container" id -u "$user")
+	redis_user_gid=$(docker exec "$container" id -g "$group")
 
 	status=$(docker exec "$container" cat /proc/1/status)
 	process_uid=$(echo "$status" | grep Uid | cut -f2)
@@ -287,10 +294,11 @@ run_redis_docker_and_check_uid_gid() {
 
 run_redis_docker_and_check_modules() {
 	docker_cmd="$1"
-	container=$(docker run --rm -d $REDIS_IMG $docker_cmd)
+	# shellcheck disable=SC2086
+	container=$(docker run --rm -d "$REDIS_IMG" $docker_cmd)
 	info=$(docker exec "$container" redis-cli info)
 
-	[ "$PLATFORM" -a "$PLATFORM" != "amd64" ] && startSkipping
+	[ "$PLATFORM" ] && [ "$PLATFORM" != "amd64" ] && startSkipping
 	assertContains "$info" "module:name=timeseries"
 	assertContains "$info" "module:name=search"
 	assertContains "$info" "module:name=bf"
@@ -313,7 +321,7 @@ assert_redis_v8() {
 # Tests #
 
 test_redis_version() {
-	ret=$(docker run --rm $REDIS_IMG -v|tail -n 1)
+	ret=$(docker run --rm "$REDIS_IMG" -v|tail -n 1)
 	assert_redis_v8 "$ret"
 }
 
@@ -478,7 +486,9 @@ test_config_owner_and_perms_changed_by_sentinel_when_config_is_WO() {
 # test that entrypoint tries to start redis even when config is non existent dir
 test_redis_start_reached_when_config_dir_does_not_exist() {
 	assert_has_config_error() {
+		# shellcheck disable=SC2317
 		assertContains "$docker_output" "Fatal error, can't open config file"
+		# shellcheck disable=SC2317
 		assertContains "$docker_output" "No such file or directory"
 	}
 	common_flags=(--mount-target /etc/somewhere --extra-assert assert_has_config_error)
@@ -487,6 +497,7 @@ test_redis_start_reached_when_config_dir_does_not_exist() {
 
 test_redis_start_reached_when_chown_on_data_dir_is_denied() {
 	assert_internal() {
+		# shellcheck disable=SC2317
 		assert_redis_v8 "$docker_output"
 	}
 	dir_structure="
@@ -533,19 +544,22 @@ test_redis_server_persistence_with_bind_mount() {
 	# make data directory non writable
 	chmod 0444 "$dir"
 
-	container=$(docker run --rm -d -v `pwd`/$dir:/data $REDIS_IMG)
+	container=$(docker run --rm -d -v "$(pwd)/$dir":/data "$REDIS_IMG" --appendonly yes)
 
-	result=$(echo save | docker exec -i $container redis-cli)
+	result=$(echo save | docker exec -i "$container" redis-cli)
 	assertEquals "OK" "$result"
 
 	# save container hash as a value
-	result=$(echo "SET FOO $container" | docker exec -i $container redis-cli)
+	result=$(echo "SET FOO $container" | docker exec -i "$container" redis-cli)
 	assertEquals "OK" "$result"
 
 	docker stop "$container" >/dev/null
 
-	container2=$(docker run --rm -d -v `pwd`/$dir:/data $REDIS_IMG)
-	value=$(echo "GET FOO" | docker exec -i $container2 redis-cli)
+	# change the owner
+	sudo chown -R "$HOST_OWNER" "$dir"
+
+	container2=$(docker run --rm -d -v "$(pwd)/$dir":/data "$REDIS_IMG")
+	value=$(echo "GET FOO" | docker exec -i "$container2" redis-cli)
 	assertEquals "$container" "$value"
 
 	docker stop "$container2" >/dev/null
@@ -558,19 +572,25 @@ test_redis_server_persistence_with_volume() {
 
 	docker volume create test_redis >/dev/null
 
-	container=$(docker run --rm -d -v test_redis:/data $REDIS_IMG)
+	# change owner of the data volume
+	docker run --rm -v test_redis:/data --entrypoint=/bin/sh "$REDIS_IMG" -c 'chown -R 0:0 /data'
+
+	container=$(docker run --rm -d -v test_redis:/data "$REDIS_IMG" --appendonly yes)
 
-	result=$(echo save | docker exec -i $container redis-cli)
+	result=$(echo save | docker exec -i "$container" redis-cli)
 	assertEquals "OK" "$result"
 
 	# save container hash as a value
-	result=$(echo "SET FOO $container" | docker exec -i $container redis-cli)
+	result=$(echo "SET FOO $container" | docker exec -i "$container" redis-cli)
 	assertEquals "OK" "$result"
 
 	docker stop "$container" >/dev/null
 
-	container2=$(docker run --rm -d -v test_redis:/data $REDIS_IMG)
-	value=$(echo "GET FOO" | docker exec -i $container2 redis-cli)
+	# change owner and permissions of files in data volume
+	docker run --rm -v test_redis:/data --entrypoint=/bin/sh "$REDIS_IMG" -c 'chown -R 0:0 /data && chmod 0000 -R /data'
+
+	container2=$(docker run --rm -d -v test_redis:/data "$REDIS_IMG")
+	value=$(echo "GET FOO" | docker exec -i "$container2" redis-cli)
 	assertEquals "$container" "$value"
 
 	docker stop "$container2" >/dev/null
@@ -591,19 +611,19 @@ test_redis_process_uid_and_gid_are_redis() {
 }
 
 test_redis_process_uid_and_gid_respects_docker_user_arg() {
-	read daemon_user_uid _ <<< "$(get_container_user_uid_gid_on_the_host daemon:daemon)"
+	read -r daemon_user_uid _ <<< "$(get_container_user_uid_gid_on_the_host daemon:daemon)"
 
 	# disable persistence as directory data dir would not be writable
 	common_flags=(--user daemon --group daemon --docker-flags "--user daemon")
 	run_redis_docker_and_check_uid_gid "${common_flags[@]}" "" --save ""
 	run_redis_docker_and_check_uid_gid "${common_flags[@]}" redis-server --save ""
 	run_redis_docker_and_check_uid_gid "${common_flags[@]}" /usr/local/bin/redis-server --save ""
 
-	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner $daemon_user_uid --expected-cmd redis-sentinel redis-sentinel /etc/sentinel/sentinel.conf
-	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner $daemon_user_uid --expected-cmd redis-sentinel /usr/local/bin/redis-sentinel /etc/sentinel/sentinel.conf
-	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner $daemon_user_uid --expected-cmd "[sentinel]" /etc/sentinel/sentinel.conf --sentinel
-	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner $daemon_user_uid --expected-cmd "[sentinel]" redis-server /etc/sentinel/sentinel.conf --sentinel
-	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner $daemon_user_uid --expected-cmd "[sentinel]" /usr/local/bin/redis-server /etc/sentinel/sentinel.conf --sentinel
+	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner "$daemon_user_uid" --expected-cmd redis-sentinel redis-sentinel /etc/sentinel/sentinel.conf
+	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner "$daemon_user_uid" --expected-cmd redis-sentinel /usr/local/bin/redis-sentinel /etc/sentinel/sentinel.conf
+	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner "$daemon_user_uid" --expected-cmd "[sentinel]" /etc/sentinel/sentinel.conf --sentinel
+	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner "$daemon_user_uid" --expected-cmd "[sentinel]" redis-server /etc/sentinel/sentinel.conf --sentinel
+	run_redis_docker_and_check_uid_gid "${common_flags[@]}" --file-owner "$daemon_user_uid" --expected-cmd "[sentinel]" /usr/local/bin/redis-server /etc/sentinel/sentinel.conf --sentinel
 }
 
 test_redis_process_uid_and_gid_are_root_when_SKIP_DROP_PRIVS_is_used() {
@@ -625,4 +645,5 @@ test_redis_server_modules_are_loaded() {
 	run_redis_docker_and_check_modules /usr/local/bin/redis-server
 }
 
+# shellcheck disable=SC1091
 . ./shunit2