Run shellcheck in CI

.github/buildomat/common.sh

@@ -1,5 +1,8 @@
 #!/bin/bash
 
+# This script is sourced by others for its variables, so they are actually used.
+# shellcheck disable=SC2034
+
 # The tofino2 has 20 stages, and the current sidecar.p4 needs all 20 of them.
 # Specifying the number of stages isn't strictly necessary, but it allows us to
 # track when we exceed the current ceiling. The underlying intention is to grow
@@ -15,9 +18,13 @@ SDE_COMMIT=2a6b33211c9675996dcb99fe939045506667ae94
 SDE_PKG_SHA256=d32739c368d1666b98dd74e25e22f83c209982e2c6670de6db5d6fdf49b5e275
 SDE_DEB_SHA256=3ecbf7c677bb722b351d5af74cee44fab70c1bb5eadc6ab2558ba714a8c3978b
 
-[ `uname -s` == "SunOS" ] && SERIES=illumos
-[ `uname -s` == "SunOS" ] || SERIES=linux
-SDE_DIR=https://buildomat.eng.oxide.computer/public/file/oxidecomputer/tofino-sde/$SERIES/$SDE_COMMIT
+os=$(uname -s)
+if [[ ${os} == "SunOS" ]]; then
+    SERIES=illumos
+else
+    SERIES=linux
+fi
+SDE_DIR="https://buildomat.eng.oxide.computer/public/file/oxidecomputer/tofino-sde/${SERIES}/${SDE_COMMIT}"
 
 # Install the active Rust toolchain from rust-toolchain.toml. We need this
 # because `rustup` version 1.28 made it where the toolchain is not installed by

.github/buildomat/illumos.sh

@@ -1,3 +1,5 @@
+# shellcheck shell=bash
+
 # Download the SDE from CI, verify its integrity, and install it
 banner "sde setup"
 
@@ -6,16 +8,20 @@ banner "sde setup"
 export PKG_SUCCESS_ON_NOP=1
 
 export PKG=tofino_sde.p5p
-curl -OL $SDE_DIR/$PKG
-SDE_CALC=`digest -a sha256 $PKG`
-if [ $SDE_CALC != $SDE_PKG_SHA256 ]; then
-	echo "downloaded tofino_sde has a bad checksum"
-	exit 1
+
+# shellcheck disable=SC2154 # SDE_DIR comes from common.sh
+curl -OL "${SDE_DIR}/${PKG}"
+SDE_CALC=$(digest -a sha256 "${PKG}")
+
+# shellcheck disable=SC2154 # SDE_PKG_SHA256 comes from common.sh
+if [[ "${SDE_CALC}" != "${SDE_PKG_SHA256}" ]]; then
+    echo "downloaded tofino_sde has a bad checksum"
+    exit 1
 fi
-pfexec pkg install -g $PKG tofino_sde
+pfexec pkg install -g "${PKG}" tofino_sde
 
 export SDE=/opt/oxide/tofino_sde
-export LD_LIBRARY_PATH="$SDE/lib:$LD_LIBRARY_PATH"
+export LD_LIBRARY_PATH="${SDE}/lib:${LD_LIBRARY_PATH}"
 
 # Install a couple of non-standard packages needed to build dendrite
 banner "packages"

.github/buildomat/jobs/image.sh

@@ -92,16 +92,16 @@ source .github/buildomat/illumos.sh
 # buildomat can retrieve for archiving.
 # usage: archive <source stem> <dest stem> <suffix>
 function archive {
-    mv out/$1$3 /out/$2$3
-    digest -a sha256 /out/$2$3 > /out/$2.sha256.txt
+    mv "out/$1$3" "/out/$2$3"
+    digest -a sha256 "/out/$2$3" > "/out/$2.sha256.txt"
 }
 
 pfexec mkdir -p /out
-pfexec chown "$UID" /out
+pfexec chown "${UID}" /out
 
 banner "P4 Codegen"
 # Add gcc-14 so the p4 compiler can find cpp
-PATH=/opt/gcc-14/bin:$PATH cargo xtask codegen --stages $TOFINO_STAGES
+PATH=/opt/gcc-14/bin:"${PATH}" cargo xtask codegen --stages "${TOFINO_STAGES}"
 
 # Preserve all the diagnostics spit out by the compiler
 mkdir -p /out/p4c-diags
@@ -126,39 +126,39 @@ digest -a sha256 /out/swadm > /out/swadm.sha256.txt
 #   of `dpd`.
 function build() {
     local FEATURE="$1"
-    NAME="$(echo $FEATURE | sed "s/tofino_//")"
-    if [ "$2" = "--with-console" ]; then
+    NAME="${FEATURE//tofino_/}"
+    if [[ "$2" = "--with-console" ]]; then
         echo "building with tokio-console feature"
-        DPD_FEATURES="$FEATURE tokio-console"
-        LOCAL_ARCHIVE_SUFFIX="$NAME-console"
+        DPD_FEATURES="${FEATURE} tokio-console"
+        LOCAL_ARCHIVE_SUFFIX="${NAME}-console"
         GLOBAL_ARCHIVE_SUFFIX="global-console"
     else
-        DPD_FEATURES="$FEATURE"
-        LOCAL_ARCHIVE_SUFFIX="$NAME"
+        DPD_FEATURES="${FEATURE}"
+        LOCAL_ARCHIVE_SUFFIX="${NAME}"
         GLOBAL_ARCHIVE_SUFFIX="global"
     fi
-    banner build "$NAME"
-    ptime -m cargo build --release --verbose --features "$DPD_FEATURES" --bin dpd
+    banner build "${NAME}"
+    ptime -m cargo build --release --verbose --features "${DPD_FEATURES}" --bin dpd
 
-    banner package "$NAME"
-    ptime -m cargo xtask dist --format omicron --release --features "$FEATURE"
+    banner package "${NAME}"
+    ptime -m cargo xtask dist --format omicron --release --features "${FEATURE}"
 
-    if [ "$NAME" = "asic" ]; then
+    if [[ "${NAME}" = "asic" ]]; then
 	    echo "building helios tarball"
 	    ptime -m cargo xtask dist --format global --release
-        mv dendrite-global.tar.gz "out/dendrite-$GLOBAL_ARCHIVE_SUFFIX.tar.gz"
-	    archive "dendrite-$GLOBAL_ARCHIVE_SUFFIX" "dendrite-$GLOBAL_ARCHIVE_SUFFIX" .tar.gz
+        mv dendrite-global.tar.gz "out/dendrite-${GLOBAL_ARCHIVE_SUFFIX}.tar.gz"
+	    archive "dendrite-${GLOBAL_ARCHIVE_SUFFIX}" "dendrite-${GLOBAL_ARCHIVE_SUFFIX}" .tar.gz
     fi
 
-    banner archive "$NAME"
-    archive dendrite "dendrite-$LOCAL_ARCHIVE_SUFFIX" .tar.gz
+    banner archive "${NAME}"
+    archive dendrite "dendrite-${LOCAL_ARCHIVE_SUFFIX}" .tar.gz
 }
 
 for FEATURE in tofino_stub tofino_asic softnpu
 do
-    build "$FEATURE"
-    if [ "$FEATURE" = "tofino_asic" ]; then
-        build "$FEATURE" --with-console
+    build "${FEATURE}"
+    if [[ "${FEATURE}" = "tofino_asic" ]]; then
+        build "${FEATURE}" --with-console
     fi
 done
 

.github/buildomat/jobs/linux.sh

@@ -58,4 +58,4 @@ digest /work/swadm > /work/swadm.sha256.txt
 
 banner "Artifacts"
 sudo mkdir -p /out
-sudo chown "$UID" /out
+sudo chown "${UID}" /out

.github/buildomat/jobs/shellcheck.sh

@@ -0,0 +1,14 @@
+#!/bin/bash
+#:
+#: name = "shellcheck"
+#: variety = "basic"
+#: target = "ubuntu-24.04"
+
+banner "packages"
+sudo apt update
+sudo apt install -y shellcheck=0.9.0-*
+
+banner "shellcheck"
+
+shopt -s globstar
+shellcheck -x -o all .github/buildomat/**/*.sh

.github/buildomat/jobs/test.sh

@@ -22,7 +22,7 @@ banner "copyrights"
 banner "clippy"
 for feat in tofino_stub tofino_asic softnpu chaos
 do
-	cargo clippy --features $feat -- --deny warnings
+	cargo clippy --features "${feat}" -- --deny warnings
 done
 
 banner "fmt"
@@ -31,5 +31,5 @@ cargo fmt -- --check
 banner "test"
 for feat in tofino_stub tofino_asic softnpu chaos
 do
-    RUST_BACKTRACE=full cargo test --features $feat --no-fail-fast
+    RUST_BACKTRACE=full cargo test --features "${feat}" --no-fail-fast
 done

.github/buildomat/linux.sh

@@ -1,21 +1,29 @@
+# shellcheck shell=bash
+
 function digest {
-    shasum -a 256 "$1" | awk -F ' ' '{print $1}'
+    local out
+    out=$(shasum -a 256 "$1")
+    echo "${out%% *}"
 }
 
 # Download the SDE from CI, verify its integrity, and install it
 banner "sde setup"
 
 export PKG=tofino_sde.deb
-curl -OL $SDE_DIR/$PKG
-SDE_CALC=`digest $PKG`
-if [ $SDE_CALC != $SDE_DEB_SHA256 ]; then
+
+# shellcheck disable=SC2154 # SDE_DIR comes from common.sh
+curl -OL "${SDE_DIR}/${PKG}"
+SDE_CALC=$(digest "${PKG}")
+
+# shellcheck disable=SC2154 # SDE_DEB_SHA256 comes from common.sh
+if [[ "${SDE_CALC}" != "${SDE_DEB_SHA256}" ]]; then
 	echo "downloaded tofino_sde has a bad checksum"
 	exit 1
 fi
-sudo dpkg -i $PKG
+sudo dpkg -i "${PKG}"
 
 export SDE=/opt/oxide/tofino_sde
-export LD_LIBRARY_PATH="$SDE/lib:$LD_LIBRARY_PATH"
+export LD_LIBRARY_PATH="${SDE}/lib:${LD_LIBRARY_PATH}"
 
 cargo --version
 rustc --version

.github/buildomat/packet-test-common.sh

@@ -1,27 +1,37 @@
+#!/bin/bash
+
 export RUST_BACKTRACE=1
 
 source .github/buildomat/common.sh
 source .github/buildomat/linux.sh
 
-wd=`pwd`
-export WS=$wd
+wd=$(pwd)
+export WS="${wd}"
+
+DENDRITE_TEST_HOST=${DENDRITE_TEST_HOST:="[::1]"}
+DENDRITE_TEST_VERBOSITY=${DENDRITE_TEST_VERBOSITY:=3}
+JUST_TEST=${JUST_TEST:=0}
 MODEL_STARTUP_TIMEOUT=${MODEL_STARTUP_TIMEOUT:=5}
+MULTICAST=${MULTICAST:=""}
+NOBUILD=${NOBUILD:=0}
 STARTUP_TIMEOUT=${STARTUP_TIMEOUT:=120}
+testname=(${TESTNAME:+${TESTNAME}})
 
-if [ x$MULTICAST == x ]; then
-        BUILD_FEATURES=tofino_asic
-	CODEGEN_FEATURES=
-        SWADM_FEATURES=
-    else
-        BUILD_FEATURES=tofino_asic,multicast
-	CODEGEN_FEATURES=--multicast
-        SWADM_FEATURES=--features=multicast
+
+if [[ "${MULTICAST}" == "" ]]; then
+    BUILD_FEATURES=tofino_asic
+    CODEGEN_FEATURES=()
+    SWADM_FEATURES=()
+else
+    BUILD_FEATURES=tofino_asic,multicast
+    CODEGEN_FEATURES=(--multicast)
+    SWADM_FEATURES=(--features=multicast)
 fi
-    
+
 function cleanup {
     set +o errexit
     set +o pipefail
-    cd $wd
+    cd "${wd}"
     sudo -E pkill -9 dpd
     sudo -E pkill -9 tofino-model
     sudo -E ./tools/veth_teardown.sh
@@ -31,7 +41,7 @@ function cleanup {
 }
 trap cleanup EXIT
 
-if [[ $JUST_TEST -ne 1 ]]; then
+if [[ ${JUST_TEST} -ne 1 ]]; then
     # See what hugepages was before starting
     sysctl vm.nr_hugepages
     # Make sure huge pages is enabled. This is required for running the SDE on
@@ -60,59 +70,46 @@ fi
 export SDE=/opt/oxide/tofino_sde
 
 banner "Build"
-if [[ $NOBUILD -ne 1 ]]; then
-    cargo build --features=$BUILD_FEATURES --bin dpd --bin swadm
-    cargo xtask codegen --stages $TOFINO_STAGES $CODEGEN_FEATURES
+if [[ ${NOBUILD} -ne 1 ]]; then
+    cargo build --features="${BUILD_FEATURES}" --bin dpd --bin swadm
+    cargo xtask codegen --stages "${TOFINO_STAGES}" "${CODEGEN_FEATURES[@]}"
 fi
 
 banner "Test"
 sudo -E ./tools/veth_setup.sh
-id=`id -un`
-gr=`id -gn`
+id=$(id -un)
+gr=$(id -gn)
 sudo -E mkdir -p /work
-sudo -E chown $id:$gr /work
+sudo -E chown "${id}":"${gr}" /work
+
+# We don't want logs owned by root, which is what shellcheck advises.
+# shellcheck disable=SC2024
 sudo -E ./tools/run_tofino_model.sh &> /work/simulator.log &
-sleep $MODEL_STARTUP_TIMEOUT
+sleep "${MODEL_STARTUP_TIMEOUT}"
+# shellcheck disable=SC2024
 sudo -E ./tools/run_dpd.sh -m 127.0.0.1 &> /work/dpd.log &
+
 echo "waiting for dpd to come online"
 set +o errexit
 
 SLEEP_TIME=5
-iters=$(( $STARTUP_TIMEOUT / $SLEEP_TIME ))
-while [ 1 ] ; do
-	./target/debug/swadm --host '[::1]' build-info 2> /dev/null
-	if [ $? == 0 ]; then
-		break
-	fi
-	iters=$(($iters - 1))
-	if [ $iters = 0 ]; then
-		echo "dpd failed to come online in $STARTUP_TIMEOUT seconds"
-		exit 1
-	fi
-	sleep $SLEEP_TIME
+iters=$(( STARTUP_TIMEOUT / SLEEP_TIME ))
+while true ; do
+    if ./target/debug/swadm --host '[::1]' build-info 2> /dev/null; then
+        break
+    fi
+    iters=$((iters - 1))
+    if [[ ${iters} = 0 ]]; then
+        echo "dpd failed to come online in ${STARTUP_TIMEOUT} seconds"
+        exit 1
+    fi
+    sleep "${SLEEP_TIME}"
 done
-set -o errexit
 
 banner "Links"
 
 ./target/debug/swadm --host '[::1]' link ls || echo "failed to list links"
 
-banner "swadm Checks"
-
-pushd swadm
-
-DENDRITE_TEST_HOST='[::1]' \
-    DENDRITE_TEST_VERBOSITY=3 \
-    cargo test \
-    --no-fail-fast \
-    $SWADM_FEATURES \
-    --test \
-    counters \
-    -- \
-    --ignored
-
-popd
-
 banner "Packet Tests"
 
 set +o errexit
@@ -121,14 +118,32 @@ stty sane
 set -o errexit
 set -o pipefail
 
+export DENDRITE_TEST_HOST DENDRITE_TEST_VERBOSITY
+
 pushd dpd-client
 
-DENDRITE_TEST_HOST='[::1]' \
-    DENDRITE_TEST_VERBOSITY=3 \
-    cargo test \
-    --features $BUILD_FEATURES \
+cargo test \
+    --features "${BUILD_FEATURES}" \
     --no-fail-fast \
-    $TESTNAME \
+    "${testname[@]}" \
     -- \
     --ignored \
     --skip succeeds_when_table_fragmented
+
+popd
+
+banner "swadm checks"
+
+pushd swadm
+
+cargo test \
+    --no-fail-fast \
+    "${SWADM_FEATURES[@]}" \
+    -- \
+    --ignored
+
+cargo test \
+    --no-fail-fast \
+    "${SWADM_FEATURES[@]}"
+
+popd