ci: add ShellCheck to CI (#458)

Issue #, if available: N/A *Description of changes:* Adds ShellCheck, a bash scripting linter, to CI as a non-critical step and resolves some of the trivial warnings/issues found by the tool. Another round of fixes is needed for some of the non-trivial warnings/issues. *Testing done:* - [x] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Austin Vazquez <macedonv@amazon.com>
runfinch · Aug 31, 2023 · b7f9e9a · b7f9e9a
1 parent 94b7497
commit b7f9e9a
Show file tree

Hide file tree

Showing 11 changed files with 66 additions and 54 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -63,6 +63,16 @@ jobs:
           # so we will just update it manually whenever it makes sense (e.g., a feature that we want is added).
           version: v1.53.3
           args: --fix=false --timeout=5m
+  shellcheck:
+    name: ShellCheck
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Run ShellCheck
+        uses: ludeeus/action-shellcheck@2.0.0
+        with:
+          version: v0.9.0 
+        continue-on-error: true
   go-mod-tidy-check:
     runs-on: ubuntu-latest
     steps:

diff --git a/Makefile b/Makefile
@@ -204,6 +204,8 @@ download-licenses:
 	curl https://raw.githubusercontent.com/golangci/golangci-lint-action/master/LICENSE --output "$(LICENSEDIR)/github.com/golangci/golangci-lint-action/LICENSE"
 	mkdir -p "$(LICENSEDIR)/github.com/avto-dev/markdown-lint"
 	curl https://raw.githubusercontent.com/avto-dev/markdown-lint/master/LICENSE --output "$(LICENSEDIR)/github.com/avto-dev/markdown-lint/LICENSE"
+	mkdir -p "$(LICENSEDIR)"/github.com/ludeeus/action-shellcheck"
+	curl https://raw.githubusercontent.com/ludeeus/action-shellcheck/blob/2.0.0/LICENSE --output "$(LICENSEDIR)/github.com/ludeeus/action-shellcheck/LICENSE"
 
     ### dependencies in ci.yaml - end ###
 

diff --git a/installer-builder/darwin/Resources/uninstall.sh b/installer-builder/darwin/Resources/uninstall.sh
@@ -8,7 +8,7 @@ fi
 
 echo "Finch-__VERSION__ will be REMOVED."
 while true; do
-    read -p "Do you wish to continue [Y/n]?" answer
+    read -r -p "Do you wish to continue [Y/n]?" answer
     [[ $answer == "y" || $answer == "Y" || $answer == "" ]] && break
     [[ $answer == "n" || $answer == "N" ]] && exit 0
     echo "Please answer with 'y' or 'n'"

diff --git a/installer-builder/darwin/scripts/postinstall b/installer-builder/darwin/scripts/postinstall
@@ -8,7 +8,7 @@ sudo pkill '^limactl'
 
 #change permissions in home directory
 echo "Change permissions for product home directory."
-cd /Applications/Finch
+cd /Applications/Finch || exit
 chmod -R 777 .
 chmod -R 755 /Applications/Finch/dependencies/lima-socket_vmnet/opt/finch
 [ -d /usr/local/bin ] || mkdir /usr/local/bin

diff --git a/installer-builder/tools/artifact-helper.sh b/installer-builder/tools/artifact-helper.sh
@@ -8,16 +8,16 @@ DELAY=60
 #$2: executable bucket
 #$3: pkg bucket
 cleanUpSigningArtifactInS3Buckets() {
-    aws s3 rm s3://${2}-${1//_/-}/pre-signed/package.tar.gz
-    aws s3 rm s3://${2}-${1//_/-}/signed/finch-executables-${1//_/-}.zip
-    aws s3 rm s3://${3}-${1//_/-}/pre-signed/package.tar.gz
-    aws s3 rm s3://${3}-${1//_/-}/signed/finch-pkg-${1//_/-}.zip
+    aws s3 rm "s3://${2}-${1//_/-}/pre-signed/package.tar.gz"
+    aws s3 rm "s3://${2}-${1//_/-}/signed/finch-executables-${1//_/-}.zip"
+    aws s3 rm "s3://${3}-${1//_/-}/pre-signed/package.tar.gz"
+    aws s3 rm "s3://${3}-${1//_/-}/signed/finch-pkg-${1//_/-}.zip"
 }
 
 #$1: arch: {x86_64, aarch64}
 #$2: executable bucket
 uploadUnsignedExecutables() {
-    aws s3 cp ./installer-builder/output/executables/unsigned/package.tar.gz s3://${2}-${1//_/-}/pre-signed/package.tar.gz --no-progress
+    aws s3 cp ./installer-builder/output/executables/unsigned/package.tar.gz "s3://${2}-${1//_/-}/pre-signed/package.tar.gz" --no-progress
 }
 
 #$1: arch: {x86_64, aarch64}
@@ -26,25 +26,25 @@ downloadSignedExecutables() {
     attempts=0
     while [ $attempts -lt $MAX_RETRY ]
     do
-        aws s3 cp s3://${2}-${1//_/-}/signed/finch-executables-${1//_/-}.zip ./installer-builder/output/executables/signed --no-progress && break
-        attempts=$(($attempts+1))
+        aws s3 cp "s3://${2}-${1//_/-}/signed/finch-executables-${1//_/-}.zip" ./installer-builder/output/executables/signed --no-progress && break
+        attempts=$((attempts+1))
         echo "Wait $DELAY seconds and retry..."
         sleep $DELAY
     done
 
     if [ $attempts -eq $MAX_RETRY ]
     then
-        echo "Download failed after $MAT_RETRY attempts."
+        echo "Download failed after $MAX_RETRY attempts."
     fi
 
-    tar xzvf ./installer-builder/output/executables/signed/finch-executables-${1//_/-}.zip -C ./installer-builder/output/executables/signed
-    aws s3 rm s3://${2}-${1//_/-}/pre-signed/package.tar.gz
+    tar xzvf "./installer-builder/output/executables/signed/finch-executables-${1//_/-}.zip" -C ./installer-builder/output/executables/signed
+    aws s3 "rm s3://${2}-${1//_/-}/pre-signed/package.tar.gz"
 }
 
 #$1: arch: {x86_64, aarch64}
 #$2: pkg bucket
 uploadUnsignedPkg() {
-    aws s3 cp ./installer-builder/output/installer/unsigned/package.tar.gz s3://${2}-${1//_/-}/pre-signed/package.tar.gz --no-progress
+    aws s3 cp ./installer-builder/output/installer/unsigned/package.tar.gz "s3://${2}-${1//_/-}/pre-signed/package.tar.gz" --no-progress
 }
 
 #$1: arch: {x86_64, aarch64}
@@ -53,27 +53,27 @@ downloadSignedPkg() {
     attempts=0
     while [ $attempts -lt $MAX_RETRY ]
     do
-        aws s3 cp s3://${2}-${1//_/-}/signed/finch-pkg-${1//_/-}.zip ./installer-builder/output/installer/signed --no-progress && break
-        attempts=$(($attempts+1))
+        aws s3 cp "s3://${2}-${1//_/-}/signed/finch-pkg-${1//_/-}.zip" ./installer-builder/output/installer/signed --no-progress && break
+        attempts=$((attempts+1))
         echo "Wait $DELAY seconds and retry..."
         sleep $DELAY
     done
 
     if [ $attempts -eq $MAX_RETRY ]
     then
-        echo "Download failed after $MAT_RETRY attempts."
+        echo "Download failed after $MAX_RETRY attempts."
     fi
 
-    tar xzvf ./installer-builder/output/installer/signed/finch-pkg-${1//_/-}.zip -C ./installer-builder/output/installer/signed
-    aws s3 rm s3://${2}-${1//_/-}/pre-signed/package.tar.gz
+    tar xzvf "./installer-builder/output/installer/signed/finch-pkg-${1//_/-}.zip" -C ./installer-builder/output/installer/signed
+    aws s3 rm "s3://${2}-${1//_/-}/pre-signed/package.tar.gz"
 }
 
 #$1: arch: {x86_64, aarch64}
 #$2: version
 #$3: private pkg bucket
 uploadNotarizedPkg() {
     #rename and upload the final installer
-    mv ./installer-builder/output/installer/signed/Payload/Finch.pkg ./installer-builder/output/installer/signed/Payload/Finch-${2}-${1}.pkg
-    aws s3 rm s3://${3}/Finch-${2}-${1}.pkg
-    aws s3 cp ./installer-builder/output/installer/signed/Payload/Finch-${2}-${1}.pkg s3://${3}/Finch-${2}-${1}.pkg --no-progress
+    mv ./installer-builder/output/installer/signed/Payload/Finch.pkg "./installer-builder/output/installer/signed/Payload/Finch-${2}-${1}.pkg"
+    aws s3 rm "s3://${3}/Finch-${2}-${1}.pkg"
+    aws s3 cp "./installer-builder/output/installer/signed/Payload/Finch-${2}-${1}.pkg" "s3://${3}/Finch-${2}-${1}.pkg" --no-progress
 }
diff --git a/installer-builder/tools/build-macos-pkg.sh b/installer-builder/tools/build-macos-pkg.sh
@@ -10,18 +10,18 @@ buildPkgInstaller() {
 
     #copy pkg resources and replace version placeholder
     cp -r ./installer-builder/darwin $INSTALLER_FULL_PATH/
-    sed -i '' -e 's/__VERSION__/'${VERSION}'/g' $INSTALLER_FULL_PATH/darwin/scripts/postinstall
-    sed -i '' -e 's/__VERSION__/'${VERSION}'/g' $INSTALLER_FULL_PATH/darwin/distribution.xml
+    sed -i '' -e 's/__VERSION__/'"${VERSION}"'/g' $INSTALLER_FULL_PATH/darwin/scripts/postinstall
+    sed -i '' -e 's/__VERSION__/'"${VERSION}"'/g' $INSTALLER_FULL_PATH/darwin/distribution.xml
 
     #distribution file requires different value for aarch64
-    if [ $ARCH == "x86_64" ]
+    if [ "$ARCH" == "x86_64" ]
     then
         supportedArch="x86_64"
     else
         supportedArch="arm64"
     fi
     sed -i '' -e 's/__ARCH__/'${supportedArch}'/g' $INSTALLER_FULL_PATH/darwin/distribution.xml
-    sed -i '' -e 's/__VERSION__/'${VERSION}'/g' $INSTALLER_FULL_PATH/darwin/Resources/*.html
+    sed -i '' -e 's/__VERSION__/'"${VERSION}"'/g' $INSTALLER_FULL_PATH/darwin/Resources/*.html
 
     #copy signed finch build to pkg resource
     mkdir -p $INSTALLER_FULL_PATH/darwinpkg
@@ -36,16 +36,16 @@ buildPkgInstaller() {
 
     #copy uninstall script and replace version placeholder
     cp ./installer-builder/darwin/Resources/uninstall.sh $INSTALLER_FULL_PATH/darwinpkg/Applications/Finch
-    sed -i '' -e 's/__VERSION__/'${VERSION}'/g' $INSTALLER_FULL_PATH/darwinpkg/Applications/Finch/uninstall.sh
+    sed -i '' -e 's/__VERSION__/'"${VERSION}"'/g' $INSTALLER_FULL_PATH/darwinpkg/Applications/Finch/uninstall.sh
 
     #construct pkg directory
     mkdir -p $INSTALLER_FULL_PATH/package
     mkdir -p $INSTALLER_FULL_PATH/signed
     mkdir -p $INSTALLER_FULL_PATH/unsigned/package/artifact
 
     #build pkg
-    pkgbuild --identifier org.Finch.${VERSION} \
-    --version $VERSION \
+    pkgbuild --identifier org.Finch."${VERSION}" \
+    --version "$VERSION" \
     --scripts $INSTALLER_FULL_PATH/darwin/scripts \
     --root $INSTALLER_FULL_PATH/darwinpkg \
     $INSTALLER_FULL_PATH/package/Finch.pkg > /dev/null 2>&1

diff --git a/installer-builder/tools/extract-executables.sh b/installer-builder/tools/extract-executables.sh
@@ -15,7 +15,7 @@ packageUnsignedExecutables() {
     extractExecutables ./installer-builder/output/origin/_output
 
     #prepare unsigned executable into .tar
-    cd ./installer-builder/output/executables/unsigned/package
+    cd ./installer-builder/output/executables/unsigned/package || exit
     tar -cvzf artifact.gz -C artifact .
     tar -cvzf  ../package.tar.gz manifest.yaml artifact.gz
 }
@@ -36,15 +36,15 @@ updateQEMUEntitlement() {
 
 #$1: the file object
 extractExecutables() {
-    for file in `ls -a $1`
+    for file in $(ls -a "$1")
     do
-        if [ -d $1/$file ];
+        if [ -d "$1/$file" ];
         then
             if [[ $file != '.' && $file != '..' ]];
             then
-                extractExecutables $1/$file
+                extractExecutables "$1/$file"
             fi
-        elif [[ -x $1/$file || ($file == *.dylib && !(-L $1/$file)) ]];
+        elif [[ -x $1/$file || ($file == *.dylib && ! (-L $1/$file)) ]];
         then
             #extract executables from all file directory to one folder
             #to have the ability to merge back, rename the executables with the file path
@@ -53,20 +53,20 @@ extractExecutables() {
             #1) ./a will be removed
             #2) '/' will be replaced by '__'
             #3) final executable name is 'b__c'
-            relativepath=$(echo $1/$file | sed 's|./installer-builder/output/origin/_output/||')
+            relativepath=$(echo "$1/$file" | sed 's|./installer-builder/output/origin/_output/||')
             newname=${relativepath//\//__}
 
             #copy executable to destination folder
-            cp -a $1/$file ./installer-builder/output/executables/unsigned/package/artifact/EXECUTABLES_TO_SIGN/$newname
-            codesign --remove-signature ./installer-builder/output/executables/unsigned/package/artifact/EXECUTABLES_TO_SIGN/$newname
+            cp -a "$1/$file" ./installer-builder/output/executables/unsigned/package/artifact/EXECUTABLES_TO_SIGN/"$newname"
+            codesign --remove-signature ./installer-builder/output/executables/unsigned/package/artifact/EXECUTABLES_TO_SIGN/"$newname"
 
             #qemu needs specific entitlement, handle it separately
             if [[ $file == "qemu-system-x86_64" || $file == "qemu-system-aarch64" ]];
             then
-                updateQEMUEntitlement $newname
+                updateQEMUEntitlement "$newname"
             elif [[ $file != "qemu-img" ]];
             then
-                updateEntitlement $newname
+                updateEntitlement "$newname"
             fi
         fi
     done

diff --git a/installer-builder/tools/merge-back-signed-executables.sh b/installer-builder/tools/merge-back-signed-executables.sh
@@ -1,7 +1,7 @@
 #!/bin/bash
 
 mergeBackSignedExecutables() {
-    for file in `ls -a ./installer-builder/output/executables/signed/Payload/EXECUTABLES_TO_SIGN`
+    for file in $(ls -a ./installer-builder/output/executables/signed/Payload/EXECUTABLES_TO_SIGN)
     do
         if [[ $file != '.' && $file != '..' ]]
         then
@@ -13,7 +13,7 @@ mergeBackSignedExecutables() {
             #3) final executable path is './a/b/c'
             originalPath=${file//__/\/}
             fullPath=./installer-builder/output/origin/_output/$originalPath
-            cp -f ./installer-builder/output/executables/signed/Payload/EXECUTABLES_TO_SIGN/$file $fullPath
+            cp -f ./installer-builder/output/executables/signed/Payload/EXECUTABLES_TO_SIGN/"$file" "$fullPath"
         fi
     done
 }

diff --git a/installer-builder/tools/notarize.sh b/installer-builder/tools/notarize.sh
@@ -2,6 +2,6 @@
 
 #$1: the account name
 #$2: the credential
-cd ./installer-builder/output/installer/signed/Payload
+cd ./installer-builder/output/installer/signed/Payload || exit
 ditto -c -k --sequesterRsrc --keepParent Finch.pkg Finch.zip
-xcrun notarytool submit Finch.zip --apple-id ${1} --password ${2} --team-id 94KV3E626L --wait
+xcrun notarytool submit Finch.zip --apple-id "${1}" --password "${2}" --team-id 94KV3E626L --wait
diff --git a/installer-builder/tools/pack-unsigned-pkg.sh b/installer-builder/tools/pack-unsigned-pkg.sh
@@ -3,7 +3,7 @@
 createUnsignedPkgTarball() {
     #prepare unsigned .pkg into .tar
     cp -a ./installer-builder/templates/manifest_pkg.yaml ./installer-builder/output/installer/unsigned/package/manifest.yaml
-    cd ./installer-builder/output/installer/unsigned/package
+    cd ./installer-builder/output/installer/unsigned/package || exit
     tar -cvzf artifact.gz -C artifact .
     tar -cvzf  ../package.tar.gz manifest.yaml artifact.gz
 }

diff --git a/installer-builder/tools/release-installer.sh b/installer-builder/tools/release-installer.sh
@@ -13,7 +13,7 @@ NOTARIZATION_CREDENTIAL=${7}
 releaseInstaller() {
     echo "Finch-$FINCH_VERSION-$ARCH.pkg Installer Generation Started..."
     echo "[1/12] Clean Old Signing Artifact in S3 Buckets"
-    cleanUpSigningArtifactInS3Buckets $ARCH $EXECUTABLE_BUCKET $PKG_BUCKET
+    cleanUpSigningArtifactInS3Buckets "$ARCH" "$EXECUTABLE_BUCKET" "$PKG_BUCKET"
     rm -rf "./installer-builder/output"
     mkdir -pv "./installer-builder/output"
 
@@ -22,34 +22,34 @@ releaseInstaller() {
     cp -RP ./_output "./installer-builder/output/origin"
 
     echo "[3/12] Extract Executables from Finch Build"
-    bash ./installer-builder/tools/extract-executables.sh $ARCH
+    bash ./installer-builder/tools/extract-executables.sh "$ARCH"
 
     echo "[4/12] Upload Unsigned Executables to S3 Buckets"
-    uploadUnsignedExecutables $ARCH $EXECUTABLE_BUCKET
+    uploadUnsignedExecutables "$ARCH" "$EXECUTABLE_BUCKET"
 
     echo "[5/12] Download Signed Executables from S3 Buckets"
-    downloadSignedExecutables $ARCH $EXECUTABLE_BUCKET
+    downloadSignedExecutables "$ARCH" "$EXECUTABLE_BUCKET"
 
     echo "[6/12] Merge Back Signed Executables to Finch Build"
-    bash ./installer-builder/tools/merge-back-signed-executables.sh $ARCH
+    bash ./installer-builder/tools/merge-back-signed-executables.sh "$ARCH"
 
     echo "[7/12] Build .pkg"
-    bash ./installer-builder/tools/build-macos-pkg.sh $ARCH $FINCH_VERSION
+    bash ./installer-builder/tools/build-macos-pkg.sh "$ARCH" "$FINCH_VERSION"
 
     echo "[8/12] Pack Unsigned .pkg"
-    bash ./installer-builder/tools/pack-unsigned-pkg.sh $ARCH
+    bash ./installer-builder/tools/pack-unsigned-pkg.sh "$ARCH"
 
     echo "[9/12] Upload Unsigned .pkg to S3 Buckets"
-    uploadUnsignedPkg $ARCH $PKG_BUCKET
+    uploadUnsignedPkg "$ARCH" "$PKG_BUCKET"
 
     echo "[10/12] Download Signed .pkg from S3 Buckets"
-    downloadSignedPkg $ARCH $PKG_BUCKET
+    downloadSignedPkg "$ARCH" "$PKG_BUCKET"
 
     echo "[11/12] App Store Notarization"
-    bash ./installer-builder/tools/notarize.sh $NOTARIZATION_ACCOUNT $NOTARIZATION_CREDENTIAL
+    bash ./installer-builder/tools/notarize.sh "$NOTARIZATION_ACCOUNT" "$NOTARIZATION_CREDENTIAL"
 
     echo "[12/12] Upload installer to S3 buckets"
-    uploadNotarizedPkg $ARCH $FINCH_VERSION $INSTALLER_PRIVATE_BUCKET_NAME
+    uploadNotarizedPkg "$ARCH" "$FINCH_VERSION" "$INSTALLER_PRIVATE_BUCKET_NAME"
 
     echo "Finch-$FINCH_VERSION-$ARCH.pkg Installer Generation Completed!"
 }