From 6309b170696af39a55bc7cc4f85274f7e51ea8e6 Mon Sep 17 00:00:00 2001 From: James Westby Date: Fri, 4 Oct 2019 15:13:14 +0100 Subject: [PATCH] Add shellcheck to catch bad shell code --- .travis.yml | 4 ++-- buildone | 40 ++++++++++++++++++++-------------------- mkimage | 26 ++++++++++++++------------ pushall | 14 ++++++++------ shellcheck | 8 ++++++++ 5 files changed, 52 insertions(+), 40 deletions(-) create mode 100755 shellcheck diff --git a/.travis.yml b/.travis.yml index c3f7af4..4f1d485 100644 --- a/.travis.yml +++ b/.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 diff --git a/buildone b/buildone index c9fe0fb..1713ce5 100755 --- a/buildone +++ b/buildone @@ -47,11 +47,11 @@ 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= @@ -59,46 +59,46 @@ build() { 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" diff --git a/mkimage b/mkimage index 9bc864b..0eedc39 100755 --- a/mkimage +++ b/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 diff --git a/pushall b/pushall index 6e8de88..fe40a25 100755 --- a/pushall +++ b/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 diff --git a/shellcheck b/shellcheck new file mode 100755 index 0000000..48b0ffe --- /dev/null +++ b/shellcheck @@ -0,0 +1,8 @@ +#!/bin/bash + +set -eu +set -o pipefail + +SCRIPTS=(shellcheck mkimage buildone buildall pushall) + +shellcheck -s bash "${SCRIPTS[@]}"