From a43e79190df05595b3c6bfac943f4ea3ebd02576 Mon Sep 17 00:00:00 2001
From: Daniel Bast <2790401+dbast@users.noreply.github.com>
Date: Fri, 29 Nov 2024 15:51:05 +0100
Subject: [PATCH] Fix/handle all shellcheck findings

Signed-off-by: Daniel Bast <2790401+dbast@users.noreply.github.com>
---
 cancel/test/test.sh                            |  1 -
 cancel/test/trigger.sh                         |  1 +
 get_obj/test/test.sh                           | 16 ++++++++--------
 kubefwd/run-kubefwd-internal.sh                |  2 +-
 kubefwd/sudo-kubefwd.sh                        |  1 +
 ngrok/ngrok-reconcile-uiresource.sh            |  1 +
 pypiserver/src/build.sh                        |  3 +++
 restart_process/test/Dockerfile.test           |  3 ++-
 restart_process/test/start.sh                  |  4 +++-
 .../test/trigger_custom_deploy_live_update.sh  |  5 ++---
 syncback/krsync-callback.sh                    |  1 +
 syncback/krsync.sh                             | 17 +++++++++++------
 syncback/test/test.sh                          | 18 +++++++++---------
 test.py                                        |  6 +++++-
 test.sh                                        |  2 +-
 15 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/cancel/test/test.sh b/cancel/test/test.sh
index 73eb5cfa5..1e36f2892 100755
--- a/cancel/test/test.sh
+++ b/cancel/test/test.sh
@@ -4,7 +4,6 @@ cd "$(dirname "$0")" || exit
 
 set -uxo pipefail
 tilt ci > tilt.log &
-TILT_PID=$!
 
 sleep 1
 timeout 30 tail -f tilt.log | grep -q "uibutton.tilt.dev/sleep:update:cancel created"
diff --git a/cancel/test/trigger.sh b/cancel/test/trigger.sh
index f82913e6e..43013807d 100755
--- a/cancel/test/trigger.sh
+++ b/cancel/test/trigger.sh
@@ -3,6 +3,7 @@
 
 YAML=$(tilt get uibutton "sleep:update:cancel" -o yaml)
 TIME=$(date '+%FT%T.000000Z')
+# shellcheck disable=SC2001
 NEW_YAML=$(echo "$YAML" | sed "s/lastClickedAt.*/lastClickedAt: $TIME/g")
 
 # Currently, kubectl doesn't support subresource APIs.
diff --git a/get_obj/test/test.sh b/get_obj/test/test.sh
index 8a0bdea9f..2cf3bd1e0 100755
--- a/get_obj/test/test.sh
+++ b/get_obj/test/test.sh
@@ -1,6 +1,6 @@
 #!/bin/bash
 
-cd "$(dirname "$0")"
+cd "$(dirname "$0")" || exit
 
 # Spinning up initial test deployment
 tilt ci -f setup.Tiltfile
@@ -17,7 +17,7 @@ RESULT=$?
 if [[ $RESULT -ne 0 ]];then
   echo "ERROR: App failed to start"
   cat tilt.log
-  exit $RESULT
+  exit "$RESULT"
 fi
 
 echo "Verifying remote development is using the correct image built from Dockerfile.dev"
@@ -36,7 +36,7 @@ RESULT=$?
 if [[ $RESULT -ne 0 ]];then
   echo "ERROR: File sync doesn't work"
   cat tilt.log
-  exit $RESULT
+  exit "$RESULT"
 fi
 
 echo "Verifying that port forwarding was enabled"
@@ -45,23 +45,23 @@ RESULT=$?
 if [[ $RESULT -ne 0 ]];then
   echo "ERROR: Port forwarding wasn't enabled"
   cat tilt.log
-  exit $RESULT
+  exit "$RESULT"
 fi
 
 echo "Verifying that livenessProbe was removed"
-RESULT=$(kubectl get deployment/example-nodejs -o yaml | grep "livenessProbe" | wc -l)
+RESULT=$(kubectl get deployment/example-nodejs -o yaml | grep -c "livenessProbe")
 if [[ $RESULT -ne 0 ]];then
   echo "ERROR: livenessProbe wasn't removed"
   kubectl get deployment/example-nodejs -o yaml
-  exit $RESULT
+  exit "$RESULT"
 fi
 
 echo "Verifying that readinessProbe was removed"
-RESULT=$(kubectl get deployment/example-nodejs -o yaml | grep "readinessProbe" | wc -l)
+RESULT=$(kubectl get deployment/example-nodejs -o yaml | grep -c "readinessProbe")
 if [[ $RESULT -ne 0 ]];then
   echo "ERROR: readinessProbe wasn't removed"
   kubectl get deployment/example-nodejs -o yaml
-  exit $RESULT
+  exit "$RESULT"
 fi
 
 cat tilt.log
diff --git a/kubefwd/run-kubefwd-internal.sh b/kubefwd/run-kubefwd-internal.sh
index 7b9582c2b..4f56e97a8 100755
--- a/kubefwd/run-kubefwd-internal.sh
+++ b/kubefwd/run-kubefwd-internal.sh
@@ -6,7 +6,7 @@
 set -euo pipefail
 
 flags=()
-while read namespace; do
+while read -r namespace; do
     if [[ "$namespace" == "" ]]; then
       continue
     fi
diff --git a/kubefwd/sudo-kubefwd.sh b/kubefwd/sudo-kubefwd.sh
index 8f8f5d24b..0d7684457 100755
--- a/kubefwd/sudo-kubefwd.sh
+++ b/kubefwd/sudo-kubefwd.sh
@@ -48,6 +48,7 @@ chmod a+rw "$TRIGGER"
 "$DIR/watch-namespaces.sh" "$TRIGGER" &
 WATCH_PID="$!"
 
+# shellcheck disable=SC2317
 function cleanup {
     set -x
     set +e
diff --git a/ngrok/ngrok-reconcile-uiresource.sh b/ngrok/ngrok-reconcile-uiresource.sh
index 0943d0df9..7e461b611 100755
--- a/ngrok/ngrok-reconcile-uiresource.sh
+++ b/ngrok/ngrok-reconcile-uiresource.sh
@@ -17,6 +17,7 @@ links="$(tilt get uiresource -o jsonpath='{range .status.endpointLinks[*]}{.url}
 for link in $links;
 do
     host="localhost"
+    # shellcheck disable=SC2001
     port="$(echo "$link" | sed -e "s|^http://$host:\([0-9]*\).*|\1|")"
     if [[ "$port" != "$link" ]]; then
         ports+=("$port")
diff --git a/pypiserver/src/build.sh b/pypiserver/src/build.sh
index 2b44243ed..5b026604f 100644
--- a/pypiserver/src/build.sh
+++ b/pypiserver/src/build.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
 if [[ $# -eq 1 ]]; then
   printf "Specify path to the package"
   exit 1
@@ -12,4 +14,5 @@ if [[ $# -eq 2 ]]; then
   cmd="${cmd} upload -r ${2}"
 fi
 
+# shellcheck disable=SC2086
 python3 ${cmd}
diff --git a/restart_process/test/Dockerfile.test b/restart_process/test/Dockerfile.test
index cfbeb9faf..87397f41a 100644
--- a/restart_process/test/Dockerfile.test
+++ b/restart_process/test/Dockerfile.test
@@ -1,4 +1,5 @@
-FROM alpine
+# distro with bash required as start.sh contains bash specific code
+FROM debian:bookworm-slim
 
 RUN echo 0 > restart_count.txt
 
diff --git a/restart_process/test/start.sh b/restart_process/test/start.sh
index c9bc98143..c22484567 100755
--- a/restart_process/test/start.sh
+++ b/restart_process/test/start.sh
@@ -1,4 +1,6 @@
-#!/bin/sh
+#!/bin/bash
+# bash shebang required as we are using bash specific features: pipefail + trap
+
 set -euo pipefail
 RESTART_COUNT="$(cat restart_count.txt)"
 echo "RESTART_COUNT: $RESTART_COUNT"
diff --git a/restart_process/test/trigger_custom_deploy_live_update.sh b/restart_process/test/trigger_custom_deploy_live_update.sh
index a7547a095..68807bfaa 100755
--- a/restart_process/test/trigger_custom_deploy_live_update.sh
+++ b/restart_process/test/trigger_custom_deploy_live_update.sh
@@ -8,9 +8,8 @@ set -u
 touch start.sh
 # seconds
 TIMEOUT=10
-for ((i=0;i<=$TIMEOUT;++i)) do
-  tilt logs | grep -v test_update | grep -q "RESTART_COUNT: 1"
-  if [[ $? -eq 0 ]]; then
+for ((i=0;i<=TIMEOUT;++i)) do
+  if tilt logs | grep -v test_update | grep -q "RESTART_COUNT: 1"; then
     echo restart detected
     exit 0
   fi
diff --git a/syncback/krsync-callback.sh b/syncback/krsync-callback.sh
index 0cb20a446..68dbbaa6e 100755
--- a/syncback/krsync-callback.sh
+++ b/syncback/krsync-callback.sh
@@ -16,5 +16,6 @@ fi
 
 # $K8S_OBJECT is intentionally not quoted here so -n/-c flags can be
 # parsed by kubectl
+# shellcheck disable=SC2086
 exec kubectl exec -i $K8S_OBJECT -- "$@"
 
diff --git a/syncback/krsync.sh b/syncback/krsync.sh
index f4a2ae339..32234df7b 100755
--- a/syncback/krsync.sh
+++ b/syncback/krsync.sh
@@ -28,27 +28,31 @@ install_rsync_tilt() {
     set +e # disable exit on error
     # $K8S_OBJECT is intentionally not quoted here so -n/-c flags can be
     # parsed by kubectl
+    # shellcheck disable=SC2086
     kubectl exec $K8S_OBJECT -- ls "$RSYNC_TILT_PATH" > /dev/null 2>&1
     exit_code="$?"
     set -e
     if [[ "$exit_code" != "0" ]]; then
-        # shellcheck disable=SC2002
         echo "installing tilt's rsync"
-        cat "$KRSYNC_TAR_PATH" | kubectl exec -i $K8S_OBJECT -- tar -xf - -C $(dirname $RSYNC_TILT_PATH)
+        # shellcheck disable=SC2086,SC2002
+        cat "$KRSYNC_TAR_PATH" | kubectl exec -i $K8S_OBJECT -- tar -xf - -C "$(dirname $RSYNC_TILT_PATH)"
     fi
 }
 
 find_rsync_path() {
     local rsync_path=--rsync-path=$RSYNC_TILT_PATH
     # see if we already have a compatible rsync on the container
+    # shellcheck disable=SC2086,SC2181
     rsync_output=$(kubectl exec $K8S_OBJECT -- rsync --version 2> /dev/null)
+    # shellcheck disable=SC2181
     if [ $? -eq 0 ]; then
         detected_rsync_version=$(echo "$rsync_output" | head -1 | awk '{ print $3 }')
-        if [[ "$detected_rsync_version" > 3 ]]; then
+        # bash cannot compare versions, like 3.3.0 with 3. So we use sort -V:
+        if [[ $(printf "%s\n%s\n" "$detected_rsync_version" "3" | sort -V | head -n 1) == "3" ]]; then
             rsync_path=
         fi
     fi
-    echo $rsync_path
+    echo "$rsync_path"
 }
 
 # Quote/set to the entire first argument, which could contain
@@ -58,6 +62,7 @@ export RSYNC_TILT_PATH="$2"
 export KRSYNC_TAR_PATH="${0%/*}/rsync.tilt.tar"
 shift
 shift
-export RSYNC_PATH=$(find_rsync_path)
+RSYNC_PATH=$(find_rsync_path)
+export RSYNC_PATH
 install_rsync_tilt
-exec rsync $RSYNC_PATH --blocking-io --rsh "$krsync_callback_path" "$@"
\ No newline at end of file
+exec rsync "$RSYNC_PATH" --blocking-io --rsh "$krsync_callback_path" "$@"
\ No newline at end of file
diff --git a/syncback/test/test.sh b/syncback/test/test.sh
index 2cf45204d..24a3e5399 100755
--- a/syncback/test/test.sh
+++ b/syncback/test/test.sh
@@ -1,16 +1,16 @@
 #!/bin/bash
 
-cd "$(dirname "$0")"
+cd "$(dirname "$0")" || exit
 
 set -ex
-tilt ci $@
+tilt ci "$@"
 syncback_dir=$(cat syncback-dir.txt)
-test -f $syncback_dir/python/main.py
-test ! -f $syncback_dir/python/example.txt
-test -f $syncback_dir/ruby/main.rb
-test ! -f $syncback_dir/ruby/example.txt
-test -f $syncback_dir/rsync/rsync.txt
-test ! -f $syncback_dir/rsync/example.txt
+test -f "$syncback_dir"/python/main.py
+test ! -f "$syncback_dir"/python/example.txt
+test -f "$syncback_dir"/ruby/main.rb
+test ! -f "$syncback_dir"/ruby/example.txt
+test -f "$syncback_dir"/rsync/rsync.txt
+test ! -f "$syncback_dir"/rsync/example.txt
 # Test that rsync was copied for containers that needed it
 kubectl exec -n syncback-test -c python deploy/syncback-containers -- sh -c "[ -f /bin/rsync.tilt ]"
 kubectl exec -n syncback-test -c ruby deploy/syncback-containers -- sh -c "[ -f /bin/rsync.tilt ]"
@@ -20,4 +20,4 @@ kubectl exec -n syncback-test -c rsync deploy/syncback-containers -- sh -c "[ !
 test -f syncback-button.txt -a "$(cat syncback-button.txt)" = syncback-containers
 
 tilt down --delete-namespaces
-rm -rf $syncback_dir syncback-dir.txt syncback-button.txt
+rm -rf "$syncback_dir" syncback-dir.txt syncback-button.txt
diff --git a/test.py b/test.py
index 479600c83..a1e658de6 100755
--- a/test.py
+++ b/test.py
@@ -63,6 +63,7 @@
 
 # `git ls-files` instead of `find` to ensure we skip stuff like .git and ./git_resource/test/.git-sources
 files = subprocess.check_output(["git", "ls-files", "**/test.sh"]).decode('utf-8').splitlines()
+tests = []
 for f in files:
 
   # don't re-execute itself
@@ -82,5 +83,8 @@
     print("dry run: %s" % f)
     continue
 
-  print("running %s" % f)
+  tests.append(f)
+
+for count, f in enumerate(tests):
+  print("running %d/%d: %s" % (count + 1, len(tests), f))
   subprocess.check_call([f])
diff --git a/test.sh b/test.sh
index 827805073..6e4c31196 100755
--- a/test.sh
+++ b/test.sh
@@ -3,5 +3,5 @@
 # find all scripts named 'test.sh' and run them
 # fail immediately if any fail
 
-cd "$(dirname "$0")"
+cd "$(dirname "$0")" || exit
 exec python3 ./test.py