Add shellcheck to catch bad shell code

.travis.yml

@@ -1,6 +1,6 @@
 language: bash
 sudo: required
-script: sudo bash buildall
+script: bash shellcheck && sudo bash buildall
 dist: xenial
 services:
   - docker
@@ -13,7 +13,7 @@ before_install:
       sleep 1
     done
   - sudo apt-get -qq update
-  - sudo apt-get install -y debian-archive-keyring debootstrap
+  - sudo apt-get install -y debian-archive-keyring debootstrap shellcheck
 deploy:
   provider: script
   script: bash pushall

buildone

@@ -47,58 +47,58 @@ log() {
 
 build() {
     DIST=$1
-    [ -f debootstrap/$DIST ] || (echo "buildall: Unknown distribution: $DIST" && exit 1)
+    [ -f "debootstrap/$DIST" ] || (echo "buildall: Unknown distribution: $DIST" && exit 1)
     current_ts="$(date -u +%Y-%m-%dT%H:%M:%S.%NZ)"
-    if docker pull $BASENAME:$DIST > /dev/null; then
-        target_ts="$(docker inspect $BASENAME:$DIST | jq --raw-output ".[0].Created")"
-        pulled_image_id="$(docker inspect $BASENAME:$DIST | jq --raw-output ".[0].Id")"
+    if docker pull "$BASENAME:$DIST" > /dev/null; then
+        target_ts="$(docker inspect "$BASENAME:$DIST" | jq --raw-output ".[0].Created")"
+        pulled_image_id="$(docker inspect "$BASENAME:$DIST" | jq --raw-output ".[0].Id")"
     else
         target_ts="$current_ts"
         pulled_image_id=
     fi
     log "============================================"
     log "Building $BASENAME:$DIST"
     log "============================================"
-    ./mkimage build/$DIST.tar $DIST
-    built_image_id=$(./import build/$DIST.tar "$target_ts")
+    ./mkimage "build/$DIST.tar" "$DIST"
+    built_image_id=$(./import "build/$DIST.tar" "$target_ts")
     log "============================================"
     log "Running tests for $BASENAME:$DIST"
     log "============================================"
-    ./test $built_image_id $DIST
+    ./test "$built_image_id" "$DIST"
     log "============================================"
     log "Rebuilding $BASENAME:$DIST to test reproducibility"
     log "============================================"
-    ./mkimage build/${DIST}-repro.tar $DIST
-    repro_image_id=$(./import build/${DIST}-repro.tar "$target_ts")
+    ./mkimage "build/${DIST}-repro.tar" "$DIST"
+    repro_image_id=$(./import "build/${DIST}-repro.tar" "$target_ts")
     if [ "$repro_image_id" != "$built_image_id" ]; then
         log "$BASENAME:$DIST differs after a rebuild. Examine $built_image_id and $repro_image_id"
         log "to find the differences and fix the build to be reproducible again."
         log "Changes (- first build, + second build):"
-        ./dockerdiff $built_image_id $repro_image_id || true
+        ./dockerdiff "$built_image_id" "$repro_image_id" || true
         exit 1
     fi
-    rm build/${DIST}-repro.tar
+    rm "build/${DIST}-repro.tar"
     if [ -n "$pulled_image_id" ]; then
         if [ "$built_image_id" != "$pulled_image_id" ]; then
             log "Image changed $built_image_id (new) != $pulled_image_id (old)"
             log "Changes (- old, + new):"
-            ./dockerdiff $pulled_image_id $built_image_id || true
+            ./dockerdiff "$pulled_image_id" "$built_image_id" || true
             # Re-import with the current timestamp so that the image shows
             # as new
-            built_image_id="$(./import build/$DIST.tar "$current_ts")"
+            built_image_id="$(./import "build/$DIST.tar" "$current_ts")"
         else
             log "Image didn't change"
             return
         fi
     fi
-    docker tag $built_image_id $BASENAME:$DIST
-    docker tag $built_image_id $QUAY_BASENAME:$DIST
-    docker tag $built_image_id $GCR_BASENAME:$DIST
+    docker tag "$built_image_id" "$BASENAME:$DIST"
+    docker tag "$built_image_id" "$QUAY_BASENAME:$DIST"
+    docker tag "$built_image_id" "$GCR_BASENAME:$DIST"
     log "Tagged $built_image_id as $BASENAME:$DIST $QUAY_BASENAME:$DIST $GCR_BASENAME:$DIST"
     if [ "$DIST" == "$LATEST" ]; then
-        docker tag $BASENAME:$DIST $BASENAME:latest
-        docker tag $QUAY_BASENAME:$DIST $QUAY_BASENAME:latest
-        docker tag $GCR_BASENAME:$DIST $GCR_BASENAME:latest
+        docker tag "$BASENAME:$DIST" "$BASENAME:latest"
+        docker tag "$QUAY_BASENAME:$DIST" "$QUAY_BASENAME:latest"
+        docker tag "$GCR_BASENAME:$DIST" "$GCR_BASENAME:latest"
     fi
 }
 
@@ -107,4 +107,4 @@ if [ -z "$1" ]; then
     exit 1
 fi
 
-build $1
+build "$1"

mkimage

@@ -3,26 +3,26 @@ set -e
 set -u
 set -o pipefail
 
-ROOT=$(cd $(dirname $0) && pwd)
+ROOT=$(cd "$(dirname "$0")" && pwd)
 
 TARGET=${1:?Specify the target filename}
 DIST=${2:-stable}
 
 LOGFILE=${TARGET}.log
 
->$LOGFILE
-exec >  >(tee -ia $LOGFILE)
-exec 2> >(tee -ia $LOGFILE >&2)
+:>"$LOGFILE"
+exec >  >(tee -ia "$LOGFILE")
+exec 2> >(tee -ia "$LOGFILE" >&2)
 
 DEBOOTSTRAP_DIR=$(mktemp -d)
-cp -a /usr/share/debootstrap/* $DEBOOTSTRAP_DIR
-cp -a /usr/share/keyrings/debian-archive-keyring.gpg $DEBOOTSTRAP_DIR
-cp -a $ROOT/debootstrap/* $DEBOOTSTRAP_DIR/scripts
+cp -a /usr/share/debootstrap/* "$DEBOOTSTRAP_DIR"
+cp -a /usr/share/keyrings/debian-archive-keyring.gpg "$DEBOOTSTRAP_DIR"
+cp -a "${ROOT}/debootstrap/"* "${DEBOOTSTRAP_DIR}/scripts"
 
 KEYRING=$DEBOOTSTRAP_DIR/debian-archive-keyring.gpg
 
-if [ -f $ROOT/keys/$DIST.gpg ]; then
-    gpg --no-default-keyring --keyring $KEYRING --import $ROOT/keys/$DIST.gpg
+if [ -f "${ROOT}/keys/${DIST}.gpg" ]; then
+    gpg --no-default-keyring --keyring "$KEYRING" --import "${ROOT}/keys/${DIST}.gpg"
 fi
 
 export DEBIAN_FRONTEND=noninteractive
@@ -37,7 +37,7 @@ DIRS_TO_TRIM="/usr/share/man
 
 rootfsDir=$(mktemp -d)
 echo "Building base in $rootfsDir"
-DEBOOTSTRAP_DIR=$DEBOOTSTRAP_DIR debootstrap --keyring $KEYRING --variant container --foreign ${DIST} "$rootfsDir"
+DEBOOTSTRAP_DIR="$DEBOOTSTRAP_DIR" debootstrap --keyring "$KEYRING" --variant container --foreign "${DIST}" "$rootfsDir"
 chroot "$rootfsDir" bash debootstrap/debootstrap --second-stage
 
 echo -e "deb http://deb.debian.org/debian $DIST main" > "$rootfsDir/etc/apt/sources.list"
@@ -212,11 +212,11 @@ echo "host" > "$rootfsDir/etc/hostname"
 # Capture the most recent date that a package in the image was changed.
 # We don't care about the particular date, or which package it comes from,
 # we just need a date that isn't very far in the past.
-BUILD_DATE="$(find $rootfsDir/usr/share/doc -name changelog.Debian.gz -exec dpkg-parsechangelog -SDate -l'{}' \; | xargs -l -i date --date="{}" +%s | sort -n | tail -n 1)"
+BUILD_DATE="$(find "$rootfsDir/usr/share/doc" -name changelog.Debian.gz -print0 | xargs -0 -n1 -I{} dpkg-parsechangelog -SDate -l'{}' | xargs -l -i date --date="{}" +%s | sort -n | tail -n 1)"
 
 echo "Trimming down"
 for DIR in $DIRS_TO_TRIM; do
-  rm -r "$rootfsDir/$DIR"/*
+  rm -r "${rootfsDir:?rootfsDir cannot be empty}/$DIR"/*
 done
 # Remove the aux-cache as it isn't reproducible. It doesn't seem to
 # cause any problems to remove it.
@@ -231,6 +231,8 @@ find "$rootfsDir" -depth -newermt "@$BUILD_DATE" -print0 | xargs -0r touch --no-
 echo "Total size"
 du -skh "$rootfsDir"
 echo "Package sizes"
+# these aren't shell variables, this is a template, so override sc thinking these are the wrong type of quotes
+# shellcheck disable=SC2016
 chroot "$rootfsDir" dpkg-query -W -f '${Package} ${Installed-Size}\n'
 echo "Largest dirs"
 du "$rootfsDir" | sort -n | tail -n 20

pushall

@@ -27,7 +27,7 @@ if [ -n "${GCR_KEY:-}" ]; then
 fi
 
 ENABLE_DOCKER_CONTENT_TRUST=0
-if [ -n "${DOCKER_CONTENT_TRUST_REPOSITORY_PASSPHRASE:-}" -a -n "${DOCKER_CONTENT_TRUST_REPOSITORY_KEY:-}" ]; then
+if [ -n "${DOCKER_CONTENT_TRUST_REPOSITORY_PASSPHRASE:-}" ] && [ -n "${DOCKER_CONTENT_TRUST_REPOSITORY_KEY:-}" ]; then
     tmpdir=$(mktemp -d)
     (cd "${tmpdir}" && bash -c 'echo -n "${DOCKER_CONTENT_TRUST_REPOSITORY_KEY}" | base64 -d > key')
     chmod 400 "${tmpdir}/key"
@@ -44,11 +44,13 @@ done
 
 # Create and merge a PR to update minideb-extras
 CIRCLE_CI_FUNCTIONS_URL=${CIRCLE_CI_FUNCTIONS_URL:-https://raw.githubusercontent.com/bitnami/test-infra/master/circle/functions}
-source <(curl -sSL $CIRCLE_CI_FUNCTIONS_URL)
+# sc can't follow source as it is a remote file
+# shellcheck disable=SC1090
+source <(curl -sSL "$CIRCLE_CI_FUNCTIONS_URL")
 for DIST in $DISTS; do
     # Use '.RepoDigests 0' for getting Dockerhub repo digest as it was the first pushed
-    DIST_REPO_DIGEST=$(docker image inspect --format '{{index .RepoDigests 0}}' ${BASENAME}:${DIST})
-    update_minideb_derived "https://github.com/bitnami/minideb-extras" $DIST $DIST_REPO_DIGEST
-    update_minideb_derived "https://github.com/bitnami/minideb-extras-base" $DIST $DIST_REPO_DIGEST
-    update_minideb_derived "https://github.com/bitnami/minideb-runtimes" $DIST $DIST_REPO_DIGEST
+    DIST_REPO_DIGEST=$(docker image inspect --format '{{index .RepoDigests 0}}' "${BASENAME}:${DIST}")
+    update_minideb_derived "https://github.com/bitnami/minideb-extras" "$DIST" "$DIST_REPO_DIGEST"
+    update_minideb_derived "https://github.com/bitnami/minideb-extras-base" "$DIST" "$DIST_REPO_DIGEST"
+    update_minideb_derived "https://github.com/bitnami/minideb-runtimes" "$DIST" "$DIST_REPO_DIGEST"
 done

shellcheck

@@ -0,0 +1,8 @@
+#!/bin/bash
+
+set -eu
+set -o pipefail
+
+SCRIPTS=(shellcheck mkimage buildone buildall pushall)
+
+shellcheck -s bash "${SCRIPTS[@]}"