ci: Add shellcheck step to lint job (#2650)

.github/workflows/build-deb-pkg.sh

@@ -4,34 +4,34 @@ set -euo pipefail
 
 DEBIAN_BASE_DIR=$PKG_ROOT/debian
 AVALANCHE_BUILD_BIN_DIR=$DEBIAN_BASE_DIR/usr/local/bin
-TEMPLATE=.github/workflows/debian/template 
+TEMPLATE=.github/workflows/debian/template
 DEBIAN_CONF=$DEBIAN_BASE_DIR/DEBIAN
 
-mkdir -p $DEBIAN_BASE_DIR
-mkdir -p $DEBIAN_CONF
-mkdir -p $AVALANCHE_BUILD_BIN_DIR
+mkdir -p "$DEBIAN_BASE_DIR"
+mkdir -p "$DEBIAN_CONF"
+mkdir -p "$AVALANCHE_BUILD_BIN_DIR"
 
 # Assume binaries are at default locations
-OK=`cp ./build/avalanchego $AVALANCHE_BUILD_BIN_DIR`
+OK=$(cp ./build/avalanchego "$AVALANCHE_BUILD_BIN_DIR")
 if [[ $OK -ne 0 ]]; then
-  exit $OK;
+  exit "$OK";
 fi
 
-OK=`cp $TEMPLATE/control $DEBIAN_CONF/control`
+OK=$(cp $TEMPLATE/control "$DEBIAN_CONF"/control)
 if [[ $OK -ne 0 ]]; then
-  exit $OK;
+  exit "$OK";
 fi
 
 echo "Build debian package..."
-cd $PKG_ROOT
+cd "$PKG_ROOT"
 echo "Tag: $TAG"
 VER=$TAG
 if [[ $TAG =~ ^v ]]; then
-  VER=$(echo $TAG | tr -d 'v')
+  VER=$(echo "$TAG" | tr -d 'v')
 fi
 NEW_VERSION_STRING="Version: $VER"
 NEW_ARCH_STRING="Architecture: $ARCH"
 sed -i "s/Version.*/$NEW_VERSION_STRING/g" debian/DEBIAN/control
 sed -i "s/Architecture.*/$NEW_ARCH_STRING/g" debian/DEBIAN/control
-dpkg-deb --build debian avalanchego-$TAG-$ARCH.deb
-aws s3 cp avalanchego-$TAG-$ARCH.deb s3://${BUCKET}/linux/debs/ubuntu/$RELEASE/$ARCH/
+dpkg-deb --build debian "avalanchego-$TAG-$ARCH.deb"
+aws s3 cp "avalanchego-$TAG-$ARCH.deb" "s3://${BUCKET}/linux/debs/ubuntu/$RELEASE/$ARCH/"

.github/workflows/build-tgz-pkg.sh

@@ -4,16 +4,16 @@ set -euo pipefail
 
 AVALANCHE_ROOT=$PKG_ROOT/avalanchego-$TAG
 
-mkdir -p $AVALANCHE_ROOT
+mkdir -p "$AVALANCHE_ROOT"
 
-OK=`cp ./build/avalanchego $AVALANCHE_ROOT`
+OK=$(cp ./build/avalanchego "$AVALANCHE_ROOT")
 if [[ $OK -ne 0 ]]; then
-  exit $OK;
+  exit "$OK";
 fi
 
 
 echo "Build tgz package..."
-cd $PKG_ROOT
+cd "$PKG_ROOT"
 echo "Tag: $TAG"
-tar -czvf "avalanchego-linux-$ARCH-$TAG.tar.gz" avalanchego-$TAG
-aws s3 cp avalanchego-linux-$ARCH-$TAG.tar.gz s3://$BUCKET/linux/binaries/ubuntu/$RELEASE/$ARCH/
+tar -czvf "avalanchego-linux-$ARCH-$TAG.tar.gz" "avalanchego-$TAG"
+aws s3 cp "avalanchego-linux-$ARCH-$TAG.tar.gz" "s3://$BUCKET/linux/binaries/ubuntu/$RELEASE/$ARCH/"

.github/workflows/ci.yml

@@ -130,6 +130,9 @@ jobs:
       - name: Run static analysis tests
         shell: bash
         run: scripts/lint.sh
+      - name: Run shellcheck
+        shell: bash
+        run: scripts/shellcheck.sh
   buf-lint:
     name: Protobuf Lint
     runs-on: ubuntu-latest

.github/workflows/cleanup-net-outage-sim.sh

@@ -5,8 +5,9 @@ set -euo pipefail
 ###
 # cleanup removes the docker instance and the network
 echo "Cleaning up..."
+# shellcheck disable=SC2046
 docker rm $(sudo docker stop $(sudo docker ps -a -q --filter ancestor=avaplatform/avalanchego:latest --format="{{.ID}}"))  #if the filter returns nothing the command fails, so ignore errors
-docker network rm controlled-net 
-rm /opt/mainnet-db-daily* 2>/dev/null 
-rm -rf /var/lib/avalanchego 2>/dev/null 
+docker network rm controlled-net
+rm /opt/mainnet-db-daily* 2>/dev/null
+rm -rf /var/lib/avalanchego 2>/dev/null
 echo "Done cleaning up"

.github/workflows/publish_image.sh

@@ -18,12 +18,12 @@ source "$AVALANCHE_PATH"/scripts/build_image.sh -r
 
 if [[ $current_branch == "master" ]]; then
   echo "Tagging current avalanchego image as $avalanchego_dockerhub_repo:latest"
-  docker tag $avalanchego_dockerhub_repo:$current_branch $avalanchego_dockerhub_repo:latest
+  docker tag "$avalanchego_dockerhub_repo:$current_branch" "$avalanchego_dockerhub_repo:latest"
 fi
 
 echo "Pushing: $avalanchego_dockerhub_repo:$current_branch"
 
 echo "$DOCKER_PASS" | docker login --username "$DOCKER_USERNAME" --password-stdin
 
 ## pushing image with tags
-docker image push -a $avalanchego_dockerhub_repo
+docker image push -a "$avalanchego_dockerhub_repo"

.github/workflows/run-net-outage-sim.sh

@@ -14,20 +14,20 @@ wait_until_healthy () {
   # store the response code here
   response=0
   # while the endpoint doesn't return 200
-  while [ $response -ne 200 ]
+  while [ "$response" -ne 200 ]
   do
     echo "Checking if local node is healthy..."
     # Ignore error in case of ephemeral failure to hit node's API
     response=$(curl --write-out '%{http_code}' --silent --output /dev/null localhost:9650/ext/health)
     echo "got status code $response from health endpoint"
     # check that 3 hours haven't passed
     now=$(date +%s)
-    if [ $now -ge $stop ];
-    then 
+    if [ "$now" -ge "$stop" ];
+    then
       # timeout: exit
       SUCCESS=1
       return
-    fi  
+    fi
     # no timeout yet, wait 30s until retry
     sleep 30
   done
@@ -44,20 +44,20 @@ echo "done existing database files"
 
 #download latest mainnet DB backup
 FILENAME="mainnet-db-daily-"
-DATE=`date +'%m-%d-%Y'`
+DATE=$(date +'%m-%d-%Y')
 DB_FILE="$FILENAME$DATE"
 echo "Copying database file $DB_FILE from S3 to local..."
-aws s3 cp s3://avalanche-db-daily/ /opt/ --no-progress --recursive --exclude "*" --include "$DB_FILE*" 
+aws s3 cp s3://avalanche-db-daily/ /opt/ --no-progress --recursive --exclude "*" --include "$DB_FILE*"
 echo "Done downloading database"
 
 # extract DB
 echo "Extracting database..."
-mkdir -p /var/lib/avalanchego/db 
-tar -zxf /opt/$DB_FILE*-tar.gz -C /var/lib/avalanchego/db 
+mkdir -p /var/lib/avalanchego/db
+tar -zxf /opt/"$DB_FILE"*-tar.gz -C /var/lib/avalanchego/db
 echo "Done extracting database"
 
 echo "Creating Docker network..."
-docker network create controlled-net 
+docker network create controlled-net
 
 echo "Starting Docker container..."
 containerID=$(docker run --name="net_outage_simulation" --memory="12g" --memory-reservation="11g" --cpus="6.0" --net=controlled-net -p 9650:9650 -p 9651:9651 -v /var/lib/avalanchego/db:/db -d avaplatform/avalanchego:latest /avalanchego/build/avalanchego --db-dir /db --http-host=0.0.0.0)
@@ -69,16 +69,16 @@ wait_until_healthy
 if [ $SUCCESS -eq 1 ];
 then
   echo "Timed out waiting for node to become healthy; exiting."
-  exit 1 
+  exit 1
 fi
 
-# To simulate internet outage, we will disable the docker network connection 
+# To simulate internet outage, we will disable the docker network connection
 echo "Disconnecting node from internet..."
-docker network disconnect controlled-net $containerID
+docker network disconnect controlled-net "$containerID"
 echo "Sleeping 60 minutes..."
-sleep 3600 
+sleep 3600
 echo "Reconnecting node to internet..."
-docker network connect controlled-net $containerID
+docker network connect controlled-net "$containerID"
 echo "Reconnected to internet. Waiting until healthy..."
 
 # now repeatedly check the node's health until it returns healthy
@@ -88,12 +88,12 @@ wait_until_healthy
 if [ $SUCCESS -eq 1 ];
 then
   echo "Timed out waiting for node to become healthy after outage; exiting."
-  exit 1 
+  exit 1
 fi
 
 # The node returned healthy, print how long it took
 end=$(date +%s)
 
-DELAY=$(($end - $start))
+DELAY=$((end - start))
 echo "Node became healthy again after complete outage after $DELAY seconds."
 echo "Test completed"

scripts/build_fuzz.sh

@@ -18,18 +18,17 @@ source "$AVALANCHE_PATH"/scripts/constants.sh
 fuzzTime=${1:-1}
 fuzzDir=${2:-.}
 
-files=$(grep -r --include='**_test.go' --files-with-matches 'func Fuzz' $fuzzDir)
+files=$(grep -r --include='**_test.go' --files-with-matches 'func Fuzz' "$fuzzDir")
 failed=false
 for file in ${files}
 do
-    funcs=$(grep -oP 'func \K(Fuzz\w*)' $file)
+    funcs=$(grep -oP 'func \K(Fuzz\w*)' "$file")
     for func in ${funcs}
     do
         echo "Fuzzing $func in $file"
-        parentDir=$(dirname $file)
-        go test $parentDir -run=$func -fuzz=$func -fuzztime=${fuzzTime}s
+        parentDir=$(dirname "$file")
         # If any of the fuzz tests fail, return exit code 1
-        if [ $? -ne 0 ]; then
+        if ! go test "$parentDir" -run="$func" -fuzz="$func" -fuzztime="${fuzzTime}"s; then
             failed=true
         fi
     done

scripts/build_test.sh

@@ -8,4 +8,5 @@ AVALANCHE_PATH=$( cd "$( dirname "${BASH_SOURCE[0]}" )"; cd .. && pwd )
 source "$AVALANCHE_PATH"/scripts/constants.sh
 
 # Ensure execution of fixture unit tests under tests/ but exclude ginkgo tests in tests/e2e and tests/upgrade
-go test -shuffle=on -race -timeout=${TIMEOUT:-"120s"} -coverprofile="coverage.out" -covermode="atomic" $(go list ./... | grep -v /mocks | grep -v proto | grep -v tests/e2e | grep -v tests/upgrade)
+# shellcheck disable=SC2046
+go test -shuffle=on -race -timeout="${TIMEOUT:-120s}" -coverprofile="coverage.out" -covermode="atomic" $(go list ./... | grep -v /mocks | grep -v proto | grep -v tests/e2e | grep -v tests/upgrade)

scripts/constants.sh

@@ -1,5 +1,10 @@
 #!/usr/bin/env bash
-#
+
+# Ignore warnings about variables appearing unused since this file is not the consumer of the variables it defines.
+# shellcheck disable=SC2034
+
+set -euo pipefail
+
 # Use lower_case variables in the scripts and UPPER_CASE variables for override
 # Use the constants.sh for env overrides
 

scripts/lint.sh

@@ -45,6 +45,7 @@ function test_license_header {
   local files=()
   while IFS= read -r line; do files+=("$line"); done < <(find . -type f -name '*.go' ! -name '*.pb.go' ! -name 'mock_*.go')
 
+  # shellcheck disable=SC2086
   go-license \
   --config=./header.yml \
   ${_addlicense_flags} \

scripts/mock.gen.sh

@@ -18,39 +18,39 @@ outputted_files=()
 input="scripts/mocks.mockgen.txt"
 while IFS= read -r line
 do
-  IFS='=' read src_import_path interface_name output_path <<< "${line}"
-  package_name=$(basename $(dirname $output_path))
+  IFS='=' read -r src_import_path interface_name output_path <<< "${line}"
+  package_name="$(basename "$(dirname "$output_path")")"
   echo "Generating ${output_path}..."
-  outputted_files+=(${output_path})
-  mockgen -package=${package_name} -destination=${output_path} ${src_import_path} ${interface_name}
-  
+  outputted_files+=("${output_path}")
+  mockgen -package="${package_name}" -destination="${output_path}" "${src_import_path}" "${interface_name}"
+
 done < "$input"
 
 # tuples of (source import path, comma-separated interface names to exclude, output file path)
 input="scripts/mocks.mockgen.source.txt"
 while IFS= read -r line
 do
-  IFS='=' read source_path exclude_interfaces output_path <<< "${line}"
-  package_name=$(basename $(dirname $output_path))
-  outputted_files+=(${output_path})
+  IFS='=' read -r source_path exclude_interfaces output_path <<< "${line}"
+  package_name=$(basename "$(dirname "$output_path")")
+  outputted_files+=("${output_path}")
   echo "Generating ${output_path}..."
 
   mockgen \
-    -source=${source_path} \
-    -destination=${output_path} \
-    -package=${package_name} \
-    -exclude_interfaces=${exclude_interfaces}
-  
+    -source="${source_path}" \
+    -destination="${output_path}" \
+    -package="${package_name}" \
+    -exclude_interfaces="${exclude_interfaces}"
+
 done < "$input"
 
-all_generated_files=( $(grep -Rl 'Code generated by MockGen. DO NOT EDIT.') )
+mapfile -t all_generated_files < <(grep -Rl 'Code generated by MockGen. DO NOT EDIT.')
 
 # Exclude certain files
 outputted_files+=('scripts/mock.gen.sh') # This file
-outputted_files+=('vms/components/avax/mock_transferable_out.go') # Embedded verify.IsState 
+outputted_files+=('vms/components/avax/mock_transferable_out.go') # Embedded verify.IsState
 outputted_files+=('vms/platformvm/fx/mock_fx.go') # Embedded verify.IsNotState
 
-diff_files=(`echo ${all_generated_files[@]} ${outputted_files[@]} | tr ' ' '\n' | sort | uniq -u`)
+mapfile -t diff_files < <(echo "${all_generated_files[@]}" "${outputted_files[@]}" | tr ' ' '\n' | sort | uniq -u)
 
 if (( ${#diff_files[@]} )); then
   printf "\nFAILURE\n"

scripts/protobuf_codegen.sh

@@ -46,23 +46,19 @@ if [ -n "${1:-}" ]; then
 fi
 
 # move to api directory
-cd $TARGET
+cd "$TARGET"
 
 echo "Running protobuf fmt..."
 buf format -w
 
 echo "Running protobuf lint check..."
-buf lint
-
-if [[ $? -ne 0 ]];  then
+if ! buf lint;  then
     echo "ERROR: protobuf linter failed"
     exit 1
 fi
 
 echo "Re-generating protobuf..."
-buf generate
-
-if [[ $? -ne 0 ]];  then
+if ! buf generate;  then
     echo "ERROR: protobuf generation failed"
     exit 1
 fi

scripts/shellcheck.sh

@@ -0,0 +1,39 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+VERSION="v0.9.0"
+
+function get_version {
+  local target_path=$1
+  if command -v "${target_path}" > /dev/null; then
+    echo "v$("${target_path}" --version | grep version: | awk '{print $2}')"
+  fi
+}
+
+REPO_ROOT=$( cd "$( dirname "${BASH_SOURCE[0]}" )"; cd .. && pwd )
+
+SYSTEM_VERSION="$(get_version shellcheck)"
+if [[ "${SYSTEM_VERSION}" == "${VERSION}" ]]; then
+  SHELLCHECK=shellcheck
+else
+  SHELLCHECK="${REPO_ROOT}/build/shellcheck"
+  # Try to install a local version
+  LOCAL_VERSION="$(get_version "${SHELLCHECK}")"
+  if [[ -z "${LOCAL_VERSION}" || "${LOCAL_VERSION}" != "${VERSION}" ]]; then
+    if which sw_vers &> /dev/null; then
+      echo "on macos, only x86_64 binaries are available so rosetta is required"
+      echo "to avoid using rosetta, install via homebrew: brew install shellcheck"
+      DIST=darwin.x86_64
+    else
+      # Linux - binaries for common arches *should* be available
+      arch="$(uname -i)"
+      DIST="linux.${arch}"
+    fi
+    curl -s -L "https://github.com/koalaman/shellcheck/releases/download/${VERSION}/shellcheck-${VERSION}.${DIST}.tar.xz" | tar Jxv -C /tmp > /dev/null
+    mkdir -p "${REPO_ROOT}"/build
+    cp /tmp/shellcheck-"${VERSION}"/shellcheck "${SHELLCHECK}"
+  fi
+fi
+
+find "${REPO_ROOT}" -name "*.sh" -type f -print0 | xargs -0 "${SHELLCHECK}"

scripts/tests.e2e.existing.sh

@@ -22,7 +22,8 @@ fi
 
 # Ensure an absolute path to avoid dependency on the working directory
 # of script execution.
-export AVALANCHEGO_PATH="$(realpath ${AVALANCHEGO_PATH:-./build/avalanchego})"
+AVALANCHEGO_PATH="$(realpath "${AVALANCHEGO_PATH:-./build/avalanchego}")"
+export AVALANCHEGO_PATH
 
 # Provide visual separation between testing and setup/teardown
 function print_separator {
@@ -47,7 +48,8 @@ print_separator
 # Determine the network configuration path from the latest symlink
 LATEST_SYMLINK_PATH="${HOME}/.tmpnet/networks/latest"
 if [[ -h "${LATEST_SYMLINK_PATH}" ]]; then
-  export TMPNET_NETWORK_DIR="$(realpath ${LATEST_SYMLINK_PATH})"
+  TMPNET_NETWORK_DIR="$(realpath "${LATEST_SYMLINK_PATH}")"
+  export TMPNET_NETWORK_DIR
 else
   echo "failed to find configuration path: ${LATEST_SYMLINK_PATH} symlink not found"
   exit 255