canonical-ubuntu-qa team mailing list archive

[Paride Legovini <mp+445422@xxxxxxxxxxxxxxxxxx>]

Paride Legovini has proposed merging ~paride/autopkgtest-cloud:lint-shellscripts into autopkgtest-cloud:master.

Requested reviews:
  Canonical's Ubuntu QA (canonical-ubuntu-qa)

For more details, see:
https://code.launchpad.net/~paride/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/445422
-- 
Your team Canonical's Ubuntu QA is requested to review the proposed merge of ~paride/autopkgtest-cloud:lint-shellscripts into autopkgtest-cloud:master.

diff --git a/.launchpad.yaml b/.launchpad.yaml
index b08e2c3..e90a256 100755
--- a/.launchpad.yaml
+++ b/.launchpad.yaml
@@ -1,8 +1,15 @@
 pipeline:
+  - pre_commit
   - build_charms
   - lint_test
 
 jobs:
+  pre_commit:
+    series: jammy
+    architectures: amd64
+    packages:
+      - pre-commit
+    run: pre-commit run --all-files --show-diff-on-failure
   build_charms:
     series: focal
     architectures: amd64
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
new file mode 100644
index 0000000..3189066
--- /dev/null
+++ b/.pre-commit-config.yaml
@@ -0,0 +1,5 @@
+repos:
+  - repo: https://github.com/shellcheck-py/shellcheck-py
+    rev: v0.9.0.5
+    hooks:
+    - id: shellcheck
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image
index be6b4d3..a0ad87a 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image
@@ -1,10 +1,11 @@
 #!/bin/bash
 # Build adt cloud images with create-nova-image-new-release for the given
 # cloud, release and arch
+# shellcheck disable=SC1090
 
 set -eu
 
-IFS="[- ]" read -r RELEASE REGION ARCH bootstrap <<< "$@"
+IFS="[- ]" read -r RELEASE REGION ARCH _bootstrap <<< "$@"
 
 if [ -z "${RELEASE}" ] || [ -z "${REGION}" ] || [ -z "${ARCH}" ]; then
         echo "Usage: $0 RELEASE REGION ARCH" >&2
@@ -12,8 +13,8 @@ if [ -z "${RELEASE}" ] || [ -z "${REGION}" ] || [ -z "${ARCH}" ]; then
 fi
 
 if [ -z "${MIRROR:-}" ]; then
-        if [ -e ~/mirror-${REGION}.rc ]; then
-                . ~/mirror-${REGION}.rc
+        if [ -e ~/mirror-"${REGION}".rc ]; then
+                . ~/mirror-"${REGION}".rc
         else
                 . ~/mirror.rc
         fi
@@ -24,10 +25,10 @@ export MIRROR
 export NET_NAME
 
 if [ -z "${USE_CLOUD_CONFIG_FROM_ENV:-}" ]; then
-        if [ -e ~/cloudrcs/${REGION}-${ARCH}.rc ]; then
-                . ~/cloudrcs/${REGION}-${ARCH}.rc
+        if [ -e ~/cloudrcs/"${REGION}"-"${ARCH}".rc ]; then
+                . ~/cloudrcs/"${REGION}"-"${ARCH}".rc
         else
-                . ~/cloudrcs/${REGION}.rc
+                . ~/cloudrcs/"${REGION}".rc
         fi
 fi
 
@@ -73,11 +74,11 @@ fi
 
 echo "$REGION-$ARCH: using image $IMG"
 KEYNAME=${KEYNAME:-testbed-$(hostname)}
-$(dirname $0)/create-nova-image-new-release $RELEASE $ARCH $IMG "${KEYNAME}" "$IMAGE_NAME"
+"$(dirname "${0}")/create-nova-image-new-release" "${RELEASE}" "${ARCH}" "${IMG}" "${KEYNAME}" "${IMAGE_NAME}"
 # clean old images
-openstack image list --private -f value | grep --color=none -v "$IMAGE_NAME" | while read id img state; do
-        if $(echo ${img} | grep -qs "adt/ubuntu-${RELEASE}-${ARCH}") && [ ${state} = active ]; then
+openstack image list --private -f value | grep --color=none -v "$IMAGE_NAME" | while read -r id img state; do
+        if echo "${img}" | grep -qs "adt/ubuntu-${RELEASE}-${ARCH}" && [ "${state}" = active ]; then
                 echo "Cleaning up old image $img ($id)"
-                openstack image delete $id
+                openstack image delete "${id}"
         fi
 done
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release
index bc51ff1..349c217 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release
@@ -1,6 +1,7 @@
 #!/bin/bash
 # create an autopkgtest nova image for a new release, based on a generic image
 # Author: Martin Pitt <martin.pitt@xxxxxxxxxx>
+# shellcheck disable=SC2154
 set -eu
 RELEASE="${1:-}"
 ARCH="${2:-}"
@@ -48,9 +49,9 @@ else
 fi
 
 # unbreak my server option :-(
-userdata=`mktemp`
-trap "rm $userdata" EXIT TERM INT QUIT PIPE
-cat <<EOF > $userdata
+userdata=$(mktemp)
+trap 'rm ${userdata}' EXIT TERM INT QUIT PIPE
+cat <<EOF > "${userdata}"
 #cloud-config
 
 manage_etc_hosts: true
@@ -67,13 +68,13 @@ EOF
 
 # create new instance
 INSTNAME="${BASEIMG}-adt-prepare"
-eval "$(openstack network show -f shell ${NET_NAME})"
+eval "$(openstack network show -f shell "${NET_NAME}")"
 
-NET_ID=${id}
+NET_ID="${id}"
 
 retries=20
 while true; do
-    eval "$(openstack server create -f shell --flavor autopkgtest --image $BASEIMG --user-data $userdata --key-name $KEYNAME --wait $INSTNAME --nic net-id=${NET_ID})"
+    eval "$(openstack server create -f shell --flavor autopkgtest --image "${BASEIMG}" --user-data "${userdata}" --key-name "${KEYNAME}" --wait "${INSTNAME}" --nic net-id="${NET_ID}")"
     if openstack server show "${id}" >/dev/null 2>/dev/null; then
         break
     fi
@@ -90,27 +91,25 @@ done
 
 SRVID="${id}"
 
-trap "openstack server delete ${SRVID}" EXIT TERM INT QUIT PIPE
+trap 'openstack server delete ${SRVID}' EXIT TERM INT QUIT PIPE
 
 # determine IP address
-eval "$(openstack server show -f shell ${SRVID})"
-ipaddr=$(echo ${addresses} | awk 'BEGIN { FS="=" } { print $2 }')
+eval "$(openstack server show -f shell "${SRVID}")"
+ipaddr=$(echo "${addresses}" | awk 'BEGIN { FS="=" } { print $2 }')
 
 SSH_CMD="ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no ubuntu@$ipaddr"
 echo "Waiting for ssh (may cause some error messages)..."
 timeout 300 sh -c "while ! $SSH_CMD true; do sleep 5; done"
 
 echo "Waiting until cloud-init is done..."
-timeout 25m $SSH_CMD 'while [ ! -e /var/lib/cloud/instance/boot-finished ]; do sleep 1; done'
+timeout 25m "${SSH_CMD}" 'while [ ! -e /var/lib/cloud/instance/boot-finished ]; do sleep 1; done'
 
 echo "Running setup script..."
-cat "${SETUP_TESTBED}" | $SSH_CMD "sudo env MIRROR='${MIRROR:-}' RELEASE='$RELEASE' sh -"
+"${SSH_CMD}" "sudo env MIRROR='${MIRROR:-}' RELEASE='$RELEASE' sh -" < "${SETUP_TESTBED}"
 
 echo "Running Canonical setup script..."
-CANONICAL_SCRIPT=$(dirname $(dirname $(readlink -f $0)))/worker-config-production/setup-canonical.sh
-cat "$CANONICAL_SCRIPT" | $SSH_CMD "sudo env MIRROR='${MIRROR:-}' RELEASE='$RELEASE' sh -"
-
-arch=$($SSH_CMD dpkg --print-architecture)
+CANONICAL_SCRIPT="$(dirname "$(dirname "$(readlink -f "${0}")")")"/worker-config-production/setup-canonical.sh
+"${SSH_CMD}" "sudo env MIRROR='${MIRROR:-}' RELEASE='$RELEASE' sh -" < "${CANONICAL_SCRIPT}"
 
 echo "Check that the upgraded image boots..."
 while true; do
@@ -138,10 +137,10 @@ $SSH_CMD sudo journalctl --rotate --vacuum-time=12h || true
 
 echo "Powering off to get a clean file system..."
 $SSH_CMD sudo poweroff || true
-eval "$(openstack server show -f shell ${SRVID})"
-while [ ${os_ext_sts_vm_state} != "stopped" ]; do
+eval "$(openstack server show -f shell "${SRVID}")"
+while [ "${os_ext_sts_vm_state}" != "stopped" ]; do
         sleep 1
-        eval "$(openstack server show -f shell ${SRVID})"
+        eval "$(openstack server show -f shell "${SRVID}")"
 done
 
 echo "Creating image $IMAGE_NAME ..."
@@ -155,8 +154,8 @@ while true; do
     while [ $inner_retries -gt 0 ]; do
         # server image create often loses its connection but it's actually
         # working - if the image is uploading, wait a bit for it to finish
-        eval $(openstack image show -f shell --prefix=image_ "${IMAGE_NAME}")
-        eval $(openstack server show -f shell --prefix=server_ "${SRVID}")
+        eval "$(openstack image show -f shell --prefix=image_ "${IMAGE_NAME}")"
+        eval "$(openstack server show -f shell --prefix=server_ "${SRVID}")"
         if [ "${server_os_ext_sts_task_state}" = "image_uploading" ] ||
            [ "${image_status}" = "saving" ]; then
             echo "image ${IMAGE_NAME} is uploading, waiting..." >&2
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances
index f0b7125..c057ce8 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances
@@ -13,5 +13,5 @@ IMAGE=$(openstack image list | grep "adt/ubuntu-$DEVEL-$ARCH" | cut -d' ' -f2)
 NET_ID=$(openstack network list | grep 'net_prod-proposed-migration' | cut -d' ' -f2)
 
 for i in $(seq 1 10); do
-    openstack server create --image $IMAGE --flavor cpu4-ram8-disk50 --nic net-id=$NET_ID -- "creation-test-$i"
+    openstack server create --image "${IMAGE}" --flavor cpu4-ram8-disk50 --nic net-id="${NET_ID}" -- "creation-test-${i}"
 done
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair
index be664d6..31b7d32 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair
@@ -2,12 +2,14 @@
 
 set -eu
 
+# shellcheck disable=SC2034
 IFS="[- ]" read -r RELEASE REGION ARCH bootstrap <<< "$@"
 
-if [ -e ~/cloudrcs/${REGION}-${ARCH}.rc ]; then
-        . ~/cloudrcs/${REGION}-${ARCH}.rc
+# shellcheck disable=SC1090
+if [ -e ~/cloudrcs/"${REGION}"-"${ARCH}".rc ]; then
+        . ~/cloudrcs/"${REGION}"-"${ARCH}".rc
 else
-        . ~/cloudrcs/${REGION}.rc
+        . ~/cloudrcs/"${REGION}".rc
 fi
 
 if ! [ -e "${HOME}/.ssh/id_rsa" ]; then
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region
index 0261108..2e78e83 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region
@@ -1,5 +1,6 @@
 #!/bin/sh
 # usage: exec-in-region <region name> <command> <argument>...
+# shellcheck disable=SC1090
 
 set -e
 
@@ -25,7 +26,7 @@ export REGION
 if [ "${REGION#lxd-}" != "$REGION" ]; then
 	LXD_ARCH=${REGION#*-}; LXD_ARCH=${LXD_ARCH%%-*}
 else
-	. ${HOME}/cloudrcs/${REGION}.rc
+	. "${HOME}"/cloudrcs/"${REGION}".rc
 fi
 
 exec "$@"
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh
index 93d48d8..7cd94c8 100644
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh
@@ -1,3 +1,4 @@
+#!/bin/sh
 # Canonical/Ubuntu specific testbed setup
 
 # Remove trailing dot from the machine fqdn.
@@ -70,6 +71,7 @@ if type iptables >/dev/null 2>&1; then
 iptables -w -t mangle -A FORWARD -p tcp --tcp-flags SYN,RST SYN -j TCPMSS --clamp-mss-to-pmtu || true
 EOF
     chmod 755 /etc/rc.local
+    # shellcheck disable=SC1091
     . /etc/rc.local
 fi
 
diff --git a/ci/lint_test b/ci/lint_test
index e52edc4..22cb834 100755
--- a/ci/lint_test
+++ b/ci/lint_test
@@ -94,14 +94,6 @@ if __name__=="__main__":
             "output": "",
             "code": 0
         },
-        "shellcheck": {
-            "files": [],
-            "extensions": [".sh", ".bash"],
-            "shebangs": ["#!/bin/bash", "#!/bin/sh"],
-            "args": None,
-            "output": "",
-            "code": 0
-        },
         'yamllint': {
             "files": ["../"],
             "extensions": None,
@@ -137,4 +129,4 @@ if __name__=="__main__":
         if key == "yamllint" and item["code"] != 0:
             sys.exit(1)
         sys.exit(0)
-    sys.exit(0)
\ No newline at end of file
+    sys.exit(0)
diff --git a/lxc-slave-admin/cmd b/lxc-slave-admin/cmd
index ff991ff..0700964 100755
--- a/lxc-slave-admin/cmd
+++ b/lxc-slave-admin/cmd
@@ -1,6 +1,6 @@
 #!/bin/sh
 set -e
-MYDIR=`dirname $0`
+MYDIR=$(dirname "${0}")
 
 if [ -z "$1" ]; then
     echo "Usage: $0 <hosts> <commands or .commands file>" >&2
@@ -8,11 +8,11 @@ if [ -z "$1" ]; then
 fi
 
 if [ "$1" = "all" ]; then
-    for f in $MYDIR/*.hosts; do
+    for f in "${MYDIR}"/*.hosts; do
         hosts="$hosts -h $f";
     done
 else
-    if [ -e ${1} ]; then
+    if [ -e "${1}" ]; then
         hosts="-h ${1}"
     elif [ -e "${1}.hosts" ]; then
         hosts="-h ${1}.hosts"
@@ -29,8 +29,8 @@ if [ "${1%.commands}" != "$1" ]; then
         exit 1
     fi
     # command file
-    cat "$1" | parallel-ssh -x "-F $MYDIR/ssh_config" -OUserKnownHostsFile=/dev/null -OStrictHostKeyChecking=no -OIdentitiesOnly=yes $hosts -p8 -t 0 -i -I
+    parallel-ssh -x "-F ${MYDIR}/ssh_config" -OUserKnownHostsFile=/dev/null -OStrictHostKeyChecking=no -OIdentitiesOnly=yes "${hosts}" -p8 -t 0 -i -I < "${1}"
 else
     # command
-    parallel-ssh -x "-F $MYDIR/ssh_config" -OUserKnownHostsFile=/dev/null -OStrictHostKeyChecking=no -OIdentitiesOnly=yes $hosts -p8 -t 0 -i -- "$@"
+    parallel-ssh -x "-F ${MYDIR}/ssh_config" -OUserKnownHostsFile=/dev/null -OStrictHostKeyChecking=no -OIdentitiesOnly=yes "${hosts}" -p8 -t 0 -i -- "$@"
 fi
diff --git a/mojo/make-lxd-secgroup b/mojo/make-lxd-secgroup
index d41274c..187c6d4 100755
--- a/mojo/make-lxd-secgroup
+++ b/mojo/make-lxd-secgroup
@@ -1,5 +1,5 @@
 #!/bin/sh
-
+# shellcheck disable=SC1090
 set -eu
 
 # there's apparently no way to get this dynamically
@@ -25,4 +25,4 @@ done
 
 if [ -n "${ROUTER_IP:-}" ]; then
         nova secgroup-add-rule lxd tcp 8443 8443 "${ROUTER_IP}/32" 2>/dev/null || true  # perhaps it already existed
-fi
+fi
\ No newline at end of file
diff --git a/mojo/postdeploy b/mojo/postdeploy
index 0f857ae..79fe50a 100755
--- a/mojo/postdeploy
+++ b/mojo/postdeploy
@@ -11,5 +11,5 @@ if [ "${MOJO_STAGE_NAME}" == "staging" ]; then
 fi
 
 echo "Setting up the floating IP address of the front end..."
-$(dirname $0)/add-floating-ip haproxy
-$(dirname $0)/add-floating-ip rabbitmq-server
+"$(dirname "$0")/add-floating-ip" haproxy
+"$(dirname "$0")/add-floating-ip" rabbitmq-server