Lint shell scripts on CI (SQPIT-279)

.shellcheckrc

@@ -0,0 +1 @@
+disable=SC2002

Makefile

@@ -118,6 +118,12 @@ formatf:
 formatc:
 	./tools/ormolu.sh -c
 
+# lint all shell scripts with ShellCheck
+SHELL_FILES_TO_LINT=$(shell find -not -path "./dist-newstyle/*" -not -path "./services/nginz/third_party/*" -type f -iname '*.sh')
+.PHONY: shellcheck
+shellcheck:
+	shellcheck -x $(SHELL_FILES_TO_LINT)
+
 # For any Haskell or Rust file, update or add a license header if necessary.
 # Headers should be added according to Ormolu's formatting rules, but please check just in case.
 .PHONY: add-license

changelog.d/5-internal/add-shellcheck-make-target

@@ -0,0 +1 @@
+Add a target to the Makefile to run ShellCheck. I.e. to run a linter on all shell scripts. This will be used in the CI.

changelog.d/mk-changelog.sh

@@ -6,7 +6,7 @@ shopt -s nullglob
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 
 getPRNumber() {
-    git log --reverse --format=%s -- $1 | sed -rn '1 { /\((#.*)\)$/ s|^.*\((#.*)\)$|\1|p; }' | grep "" ||
+    git log --reverse --format=%s -- "$1" | sed -rn '1 { /\((#.*)\)$/ s|^.*\((#.*)\)$|\1|p; }' | grep "" ||
       echo "#PR_NOT_FOUND"
 }
 
@@ -18,10 +18,13 @@ for d in "$DIR"/*; do
     if [[ ${#entries[@]} -eq 0 ]]; then continue; fi
 
     echo -n "## "
+    # shellcheck disable=SC1003
     sed '$ a\' "$d/.title"
     echo ""
     for f in "${entries[@]}"; do
-        pr=$(getPRNumber $f)
+        pr=$(getPRNumber "$f")
+
+        # shellcheck disable=SC1003
         sed -r '
           # create a bullet point on the first line
           1 { s/^/\* /; }

deploy/dockerephemeral/init.sh

@@ -11,6 +11,7 @@ aws configure set aws_secret_access_key dummysecret
 aws configure set region eu-west-1
 
 # Potentially delete pre-existing tables
+# shellcheck disable=SC3037
 echo -n "waiting for dynamo: "
 while (! aws --endpoint-url=http://dynamodb:8000 --cli-connect-timeout=1 dynamodb list-tables); do
     sleep 1;

deploy/services-demo/create_team_members.sh

@@ -28,7 +28,7 @@ $ grep code out.log | grep -v email-exists
 
 If you are in a hurry, you may want to change the sleep(1) at the end
 of the invite loop to less than a second.  If you want to give up on
-the first error, add an exit(1) where we check the $INVIDATION_ID.
+the first error, add an exit(1) where we check the $INVITATION_ID.
 
 "
 

deploy/services-demo/create_test_team_admins.sh

@@ -46,9 +46,12 @@ fi
 
 # Generate users
 
+# shellcheck disable=SC2034
 for i in $(seq 1 "$COUNT")
 do
+    # shellcheck disable=2002
     EMAIL=$(cat /dev/urandom | env LC_CTYPE=C tr -dc a-zA-Z0-9 | head -c 8)"@example.com"
+    # shellcheck disable=2002
     PASSWORD=$(cat /dev/urandom | env LC_CTYPE=C tr -dc a-zA-Z0-9 | head -c 8)
 
     CURL_OUT=$(curl -i -s --show-error \

deploy/services-demo/create_test_team_members.sh

@@ -123,7 +123,7 @@ do
 
     if [ "$TEAM" != "$TEAM_UUID" ]; then
         echo "unexpected error: user got assigned to no / the wrong team?!"
-        echo ${CURL_OUT}
+        echo "${CURL_OUT}"
         exit 1
     fi
 

deploy/services-demo/create_test_team_scim.sh

@@ -61,17 +61,17 @@ BEARER=$(curl -X POST \
               --header 'Content-Type: application/json' \
               --header 'Accept: application/json' \
               -d '{"email":"'"$ADMIN_EMAIL"'","password":"'"$ADMIN_PASSWORD"'"}' \
-              $BRIG_HOST/login'?persist=false' | jq -r .access_token)
+              "$BRIG_HOST/login?persist=false" | jq -r .access_token)
 
 SCIM_TOKEN_FULL=$(curl -X POST \
                        --header "Authorization: Bearer $BEARER" \
                        --header 'Content-Type: application/json;charset=utf-8' \
                        --header 'Z-User: '"$ADMIN_UUID" \
-                       -d '{ "description": "test '"`date`"'", "password": "'"$ADMIN_PASSWORD"'" }' \
-                       $SPAR_HOST/scim/auth-tokens)
+                       -d '{ "description": "test '"$(date)"'", "password": "'"$ADMIN_PASSWORD"'" }' \
+                       "$SPAR_HOST/scim/auth-tokens")
 
-SCIM_TOKEN=$(echo $SCIM_TOKEN_FULL | jq -r .token)
-SCIM_TOKEN_ID=$(echo $SCIM_TOKEN_FULL | jq -r .info.id)
+SCIM_TOKEN=$(echo "$SCIM_TOKEN_FULL" | jq -r .token)
+SCIM_TOKEN_ID=$(echo "$SCIM_TOKEN_FULL" | jq -r .info.id)
 
 
 # Create regular user via team invitation
@@ -156,7 +156,7 @@ CURL_OUT_SCIM_POST=$(curl --location --request POST "$SPAR_HOST/scim/v2/Users" \
     --header "Authorization: Bearer $SCIM_TOKEN" \
     -d "$SCIM_USER")
 
-SCIM_USER_UUID=$(echo $CURL_OUT_SCIM_POST | jq -r .id)
+SCIM_USER_UUID=$(echo "$CURL_OUT_SCIM_POST" | jq -r .id)
 
 SCIM_USER_INVITATION_ID=$(curl --location -G "$BRIG_HOST/i/teams/invitations/by-email?" \
     --header 'Content-Type: application/json' \
@@ -186,13 +186,13 @@ EOF
 CURL_OUT=$(curl \
             -XPOST "$BRIG_HOST/i/users" \
             -H'Content-type: application/json' \
-            -d'{"email":"'"$scimUserEmail"'","password":"'"$scimUserPassword"'","name":"'"$scimUserDisplayName"'","team_code":"'"$SCIM_USER_INVITATION_CODE"'"}')
+            -d"$REGISTER_ACCEPT")
 
 SCIM_USER_REGISTER_TEAM=$(echo "$CURL_OUT" | jq -r .team)
 
 if [ "$SCIM_USER_REGISTER_TEAM" != "$TEAM_UUID" ]; then
     echo "unexpected error: user got assigned to no / the wrong team?!"
-    echo ${CURL_OUT}
+    echo "${CURL_OUT}"
     exit 1
 fi
 

deploy/services-demo/create_test_user.sh

@@ -45,20 +45,21 @@ fi;
 
 # Generate users
 
-for i in `seq 1 $COUNT`
+# shellcheck disable=SC2034
+for i in $(seq 1 "$COUNT")
 do
     EMAIL=$(cat /dev/urandom | env LC_CTYPE=C tr -dc a-zA-Z0-9 | head -c 8)"@example.com"
     PASSWORD=$(cat /dev/urandom | env LC_CTYPE=C tr -dc a-zA-Z0-9 | head -c 8)
 
     CURL_OUT=$(curl -i -s --show-error \
         -XPOST "$BRIG_HOST/i/users" \
         -H'Content-type: application/json' \
-        -d'{"email":"'$EMAIL'","password":"'$PASSWORD'","name":"demo"}')
+        -d'{"email":"'"$EMAIL"'","password":"'"$PASSWORD"'","name":"demo"}')
 
     UUID=$(echo "$CURL_OUT" | tail -1 | sed 's/.*\"id\":\"\([a-z0-9-]*\)\".*/\1/')
 
     if [ "$CSV" == "false" ]
-        then echo -e "Succesfully created a user with email: "$EMAIL" and password: "$PASSWORD
-        else echo -e $UUID","$EMAIL","$PASSWORD
+        then echo -e "Succesfully created a user with email: $EMAIL and password: $PASSWORD"
+        else echo -e "$UUID,$EMAIL,$PASSWORD"
     fi
 done

deploy/services-demo/demo.sh

@@ -4,6 +4,7 @@
 
 set -eo pipefail
 
+# shellcheck disable=SC2034
 USAGE="$0 [docker] [--run-backoffice]"
 docker_deployment="false"
 if [ "$1" = "docker" ] || [ "$2" = "docker" ] ; then
@@ -27,7 +28,8 @@ function kill_all() {
 }
 
 function list_descendants () {
-  local children=$(pgrep -P "$1")
+  local children
+  children=$(pgrep -P "$1")
   for pid in $children
   do
     list_descendants "$pid"
@@ -38,62 +40,66 @@ function list_descendants () {
 function kill_gracefully() {
     pkill "gundeck|brig|galley|cargohold|cannon|spar|stern"
     sleep 1
-    kill $(list_descendants $PARENT_PID) &> /dev/null
+    kill "$(list_descendants $PARENT_PID)" &> /dev/null
 }
 
 function run_zauth() {
     if [ "$docker_deployment" = "false" ]; then
-        ${DIR}/../dist/zauth "$@"
+        "${DIR}/../dist/zauth" "$@"
     else
-        docker run --entrypoint "/usr/bin/zauth" ${docker_zauth_image:-quay.io/wire/zauth} $@
+        docker run --entrypoint "/usr/bin/zauth" "${docker_zauth_image:-quay.io/wire/zauth}" "$@"
     fi
 }
 
 trap "kill_gracefully; kill_all" INT TERM ERR
 
 function check_secrets() {
     if [ "$docker_deployment" = "false" ]; then
-        test -f ${DIR}/../dist/zauth || { echo "zauth is not compiled. How about you run 'cd ${TOP_LEVEL} && make services' first?"; exit 1; }
+        test -f "${DIR}/../dist/zauth" || { echo "zauth is not compiled. How about you run 'cd ${TOP_LEVEL} && make services' first?"; exit 1; }
     fi
 
     if [[ ! -f ${SCRIPT_DIR}/resources/turn/secret.txt ]]; then
         echo "Generate a secret for the TURN servers (must match the turn.secret key in brig's config)..."
-        openssl rand -base64 64 | env LC_CTYPE=C tr -dc a-zA-Z0-9 | head -c 42 > ${SCRIPT_DIR}/resources/turn/secret.txt
+        openssl rand -base64 64 | env LC_CTYPE=C tr -dc a-zA-Z0-9 | head -c 42 > "${SCRIPT_DIR}/resources/turn/secret.txt"
     else
         echo "re-using existing TURN secret"
     fi
     if [[ ! -f ${SCRIPT_DIR}/resources/zauth/privkeys.txt || ! -f ${SCRIPT_DIR}/resources/zauth/pubkeys.txt ]]; then
         echo "Generate private and public keys (used both by brig and nginz)..."
-        mkdir -p ${SCRIPT_DIR}/resources/zauth/
+        mkdir -p "${SCRIPT_DIR}/resources/zauth/"
         TMP_KEYS=$(mktemp "/tmp/demo.keys.XXXXXXXXXXX")
-        run_zauth -m gen-keypair -i 1 > $TMP_KEYS
-        cat $TMP_KEYS | sed -n 's/public: \(.*\)/\1/p' > ${SCRIPT_DIR}/resources/zauth/pubkeys.txt
-        cat $TMP_KEYS | sed -n 's/secret: \(.*\)/\1/p' > ${SCRIPT_DIR}/resources/zauth/privkeys.txt
+        run_zauth -m gen-keypair -i 1 > "$TMP_KEYS"
+        cat "$TMP_KEYS" | sed -n 's/public: \(.*\)/\1/p' > "${SCRIPT_DIR}/resources/zauth/pubkeys.txt"
+        cat "$TMP_KEYS" | sed -n 's/secret: \(.*\)/\1/p' > "${SCRIPT_DIR}/resources/zauth/privkeys.txt"
     else
         echo "re-using existing public/private keys"
     fi
 }
 
 function check_prerequisites() {
+
+    # shellcheck disable=SC2015
     nc -z 127.0.0.1 9042 \
         && nc -z 127.0.0.1 9200 \
         && nc -z 127.0.0.1 6379 \
         || { echo "Databases not up. Maybe run 'deploy/dockerephemeral/run.sh' in a separate terminal first?";  exit 1; }
     if [ "$docker_deployment" = "false" ]; then
-        test -f ${DIR}/../dist/brig \
-            && test -f ${DIR}/../dist/galley \
-            && test -f ${DIR}/../dist/cannon \
-            && test -f ${DIR}/../dist/gundeck \
-            && test -f ${DIR}/../dist/cargohold \
-            && test -f ${DIR}/../dist/proxy \
-            && test -f ${DIR}/../dist/spar \
-            && test -f ${DIR}/../dist/stern \
-            && ( test -f ${DIR}/../dist/nginx || which nix-build ) \
+        # shellcheck disable=SC2015
+        test -f "${DIR}/../dist/brig" \
+            && test -f "${DIR}/../dist/galley" \
+            && test -f "${DIR}/../dist/cannon" \
+            && test -f "${DIR}/../dist/gundeck" \
+            && test -f "${DIR}/../dist/cargohold" \
+            && test -f "${DIR}/../dist/proxy" \
+            && test -f "${DIR}/../dist/spar" \
+            && test -f "${DIR}/../dist/stern" \
+            && ( test -f "${DIR}/../dist/nginx" || which nix-build ) \
             || { echo "Not all services are compiled. How about you run 'cd ${TOP_LEVEL} && make services' first?"; exit 1; }
     fi
 }
 
 blue=6
+# shellcheck disable=SC2034
 white=7
 green=10
 orange=3
@@ -105,8 +111,9 @@ blueish=4
 function run_haskell_service() {
     service=$1
     colour=$2
-    (cd ${SCRIPT_DIR} && ${DIR}/../dist/${service} -c ${SCRIPT_DIR}/conf/${service}.demo.yaml || kill_all) \
-        | sed -e "s/^/$(tput setaf ${colour})[${service}] /" -e "s/$/$(tput sgr0)/" &
+    # shellcheck disable=SC2015
+    (cd "${SCRIPT_DIR}" && "${DIR}/../dist/${service}" -c "${SCRIPT_DIR}/conf/${service}.demo.yaml" || kill_all) \
+        | sed -e "s/^/$(tput setaf "${colour}")[${service}] /" -e "s/$/$(tput sgr0)/" &
 }
 
 function run_nginz() {
@@ -117,12 +124,14 @@ function run_nginz() {
     # nix-build will put a symlink to ./result with the nginx artifact
     if which nix-build; then
       nginz=$(nix-build "${DIR}/../nix" -A nginz --no-out-link )
-      (cd ${SCRIPT_DIR} && ${nginz}/bin/nginx -p ${SCRIPT_DIR} -c ${SCRIPT_DIR}/conf/nginz/nginx.conf -g 'daemon off;' || kill_all) \
-          | sed -e "s/^/$(tput setaf ${colour})[nginz] /" -e "s/$/$(tput sgr0)/" &
+      # shellcheck disable=SC2015
+      (cd "${SCRIPT_DIR}" && "${nginz}/bin/nginx" -p "${SCRIPT_DIR}" -c "${SCRIPT_DIR}/conf/nginz/nginx.conf" -g 'daemon off;' || kill_all) \
+          | sed -e "s/^/$(tput setaf "${colour}")[nginz] /" -e "s/$/$(tput sgr0)/" &
     else
       prefix=$([ -w /usr/local ] && echo /usr/local || echo "${HOME}/.wire-dev")
-      (cd ${SCRIPT_DIR} && LD_LIBRARY_PATH=$LD_LIBRARY_PATH:${prefix}/lib/ ${DIR}/../dist/nginx -p ${SCRIPT_DIR} -c ${SCRIPT_DIR}/conf/nginz/nginx.conf -g 'daemon off;' || kill_all) \
-          | sed -e "s/^/$(tput setaf ${colour})[nginz] /" -e "s/$/$(tput sgr0)/" &
+      # shellcheck disable=SC2015
+      (cd "${SCRIPT_DIR}" && LD_LIBRARY_PATH=$LD_LIBRARY_PATH:${prefix}/lib/ "${DIR}/../dist/nginx" -p "${SCRIPT_DIR}" -c "${SCRIPT_DIR}/conf/nginz/nginx.conf" -g 'daemon off;' || kill_all) \
+          | sed -e "s/^/$(tput setaf "${colour}")[nginz] /" -e "s/$/$(tput sgr0)/" &
     fi
 }
 

deploy/services-demo/register_idp_internal.sh

@@ -14,7 +14,7 @@ if [ ! -e "${metadata_file}" ]; then
 fi
 
 z_user=$2
-if [ ! -n "${z_user}" ]; then
+if [ -z "${z_user}" ]; then
     echo "*** no z_user uuid"
     exit 80
 fi

dev-packages.nix

@@ -180,6 +180,7 @@ in
   pkgs.jq
   pkgs.niv
   pkgs.ormolu
+  pkgs.shellcheck
   pkgs.wget
   pkgs.yq
   pkgs.rsync

docs/legacy/developer/linting.md

@@ -1,6 +1,6 @@
 # Linting
 
-# HLint
+## HLint
 
 To run [HLint](https://github.com/ndmitchell/hlint) you need it's binary, e.g.
 by executing:
@@ -21,7 +21,7 @@ To run it on a sub-project:
 hlint services/federator
 ```
 
-# Stan
+## Stan
 
 To run [Stan](https://github.com/kowainik/stan), you need it's binary compiled
 by the same GHC version as used in the project.
@@ -53,3 +53,11 @@ To analyze a sub-project with stan:
 cd services/cargohold
 stan
 ```
+
+## ShellCheck
+
+To lint shell scripts run [*ShellCheck*](https://github.com/koalaman/shellcheck):
+
+```sh
+make shellcheck
+```

hack/bin/cabal-install-artefacts.sh

@@ -19,4 +19,4 @@ cd "$TOP_LEVEL"
 cabal-plan list-bins "$pattern:exe:*" |
   awk '{print $2}' |
   xargs -i sh -c 'test -f {} && echo {} || true' |
-  xargs -P8 -i rsync -a {} "$DIST"
+  xargs -P8 -I{} rsync -a {} "$DIST"

hack/bin/cabal-run-tests.sh

@@ -14,8 +14,8 @@ fi
 for cabal in $(find "$TOP_LEVEL" -name "$pattern" | grep -v dist-newstyle); do
   # This is required because some tests (e.g. golden tests) must be run from
   # the package root.
-  cd "$(dirname $cabal)"
-  package="$(basename ${cabal%.*})"
+  cd "$(dirname "$cabal")"
+  package="$(basename "${cabal%.*}")"
   for test_suite in $(cabal-plan list-bins "$package:test:*" | awk '{print $2}'); do
     $test_suite "${@:2}"
   done

hack/bin/copy-charts.sh

@@ -14,14 +14,14 @@ CHART_DIST=$TOP_LEVEL/.local/charts
 # TODO sanity check folder must exist
 
 mkdir -p .local/charts
-rm -rf "$CHART_DIST/$CHART"
+rm -rf "${CHART_DIST:?}/$CHART"
 cp -r "$CHART_SOURCE/$CHART" "$CHART_DIST/"
 
 if [ -f "$CHART_SOURCE/$CHART/requirements.yaml" ]; then
   # very hacky bash, I'm sorry
   for subpath in $(grep "file://" "$CHART_SOURCE/$CHART/requirements.yaml" | awk '{ print $2 }' | xargs -n 1 | cut -c 8-)
   do
-    rm -rf "$CHART_DIST/$CHART/$subpath"
+    rm -rf "${CHART_DIST:?}/$CHART/$subpath"
     cp -r "$CHART_SOURCE/$CHART/$subpath" "$CHART_DIST/"
   done
 fi

hack/bin/diff-failure.sh

@@ -1,4 +1,6 @@
 #!/bin/bash
+
+# shellcheck disable=SC2162
 sed 's| =/= |\n|' | {
     IFS= read first
     IFS= read second

hack/bin/helm-template.sh

@@ -25,4 +25,4 @@ if [ -f "$certificatesfile" ]; then
 fi
 
 "$DIR/update.sh" "$CHARTS_DIR/$chart"
-helm template $"chart" "$CHARTS_DIR/$chart" ${options[*]}
+helm template "$chart" "$CHARTS_DIR/$chart" "${options[*]}"

hack/bin/integration-setup-federation.sh

@@ -8,8 +8,9 @@ export NAMESPACE=${NAMESPACE:-test-integration}
 HELMFILE_ENV=${HELMFILE_ENV:-default}
 CHARTS_DIR="${TOP_LEVEL}/.local/charts"
 
+# shellcheck disable=SC1091
 . "$DIR/helm_overrides.sh"
-${DIR}/integration-cleanup.sh
+"${DIR}/integration-cleanup.sh"
 
 # FUTUREWORK explore: have helmfile do the interpolation (and skip the "make charts" step) https://wearezeta.atlassian.net/browse/SQPIT-722
 #

hack/bin/integration-setup.sh

@@ -8,6 +8,7 @@ export NAMESPACE=${NAMESPACE:-test-integration}
 HELMFILE_ENV=${HELMFILE_ENV:-default}
 CHARTS_DIR="${TOP_LEVEL}/.local/charts"
 
+# shellcheck disable=SC1091
 . "$DIR/helm_overrides.sh"
 
 "${DIR}/integration-cleanup.sh"