ci: add workflow that lints shell scripts with ShellCheck (#147)

* Add workflow that lints shell scripts with ShellCheck Signed-off-by: Jack Baldry <[email protected]> * Replace inline script with script file Signed-off-by: Jack Baldry <[email protected]> * Lint for style I've not bothered saving this patch for reapplication if upstream changes because we had already deviated from that before these changes. - Use consistent variable syntax - Prefer `-n` over `! -z` - Use appropriate quoting for words that have no variable expansion Signed-off-by: Jack Baldry <[email protected]> * Move to lint-shared-workflows action Signed-off-by: Jack Baldry <[email protected]> * Lint translate-secrets script - Remove useless echo subshell - Set readonly variables for to clarify when the script is no longer going to modify those variables. Signed-off-by: Jack Baldry <[email protected]> * Lint README for Grafana Labs style https://grafana.com/docs/writers-toolkit/ - Simplify some language for improved readability - Prefer [semantic line breaks](https://sembr.org/) for better line based diffing in the GitHub UI. Signed-off-by: Jack Baldry <[email protected]> * Document preference for separate shell scripts Signed-off-by: Jack Baldry <[email protected]> * Revert "Lint README for Grafana Labs style" This reverts commit 4278510. * Restore documentation dropped in conflict resolution Signed-off-by: Jack Baldry <[email protected]> * Clean up whitespace Signed-off-by: Jack Baldry <[email protected]> --------- Signed-off-by: Jack Baldry <[email protected]> Co-authored-by: Dimitris Sotirakis <[email protected]>
grafana · Oct 16, 2024 · 570898e · 570898e
1 parent 6426ecd
commit 570898e
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 28 deletions.
diff --git a/.github/workflows/lint-shared-workflows.yaml b/.github/workflows/lint-shared-workflows.yaml
@@ -36,6 +36,9 @@ jobs:
       - name: Lint workflow files
         uses: raven-actions/actionlint@01fce4f43a270a612932cb1c64d40505a029f821 # v2.0.0
 
+      - name: Run ShellCheck on scripts
+        uses: ludeeus/action-shellcheck@00cae500b08a931fb5698e11e79bfbd38e612a38 # v2.0.0
+
   # A separate job so we can run in the `yq` container
   lint-action-yaml:
     name: Lint action YAMLs

diff --git a/README.md b/README.md
@@ -68,6 +68,27 @@ will ensure actions in this repo are always used at the same commit. To do this:
     some-input: some-value
 ```
 
+### Use separate files for shell scripts so they're linted
+
+Instead of embedding a shell script in the `run` string, write a separate script and refer to that.
+
+For example, don't use the step:
+
+```yaml
+id: echo-success
+shell: bash
+run: |
+  echo "Success!"
+```
+
+Instead, create the file `echo-success.bash` in the same directory and use the step:
+
+```yaml
+id: echo-success
+shell: bash
+run: ./echo-success.bash
+```
+
 ### Releasing a version of a component in shared-workflows
 
 When working with `shared-workflows`, it's essential to avoid breaking backwards compatibility. To ensure this, we must provide releasable actions for engineers to review incoming changes. This also helps automated update tools like `dependabot` and `renovate` to work effectively.

diff --git a/actions/aws-auth/action.yaml b/actions/aws-auth/action.yaml
@@ -57,29 +57,9 @@ runs:
         chain-pass-claims: "${{ inputs.pass-claims }}"
         chain-set-in-environment: "${{ inputs.set-creds-in-environment }}"
 
-    - id: aws_region # Pulled from catnekaise/cognito-idpool-auth/action.yml
+    - id: aws_region
       shell: bash
       env:
         AWS_REGION: "${{ inputs.aws-region }}"
         AWS_DEFAULT_REGION: "${{ inputs.aws-region }}"
-      run: |
-        value=""
-
-        if [ ! -z "${AWS_REGION}" ] && [ ! -z "${AWS_DEFAULT_REGION}" ]; then
-          value="$AWS_REGION"
-        fi
-
-        if [ -z "$value" ]; then
-          echo "Unable to resolve what AWS Region to use"
-          exit 1
-        fi
-
-        # Some-effort validation of aws region
-        if echo "$value" | grep -Eqv "^[a-z]{2}-[a-z]{4,9}-[0-9]$"; then
-          echo "Resolved value for AWS Region is invalid"
-          exit 1
-        fi
-
-        echo "value=$value" >> "$GITHUB_OUTPUT"
-        echo "AWS_REGION=${AWS_REGION}" >> "$GITHUB_ENV"
-        echo "AWS_DEFAULT_REGION=${AWS_DEFAULT_REGION}" >> "$GITHUB_ENV"
+      run: ./resolve-aws-region.sh
diff --git a/actions/aws-auth/resolve-aws-region.sh b/actions/aws-auth/resolve-aws-region.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+# Pulled from catnekaise/cognito-idpool-auth/action.yml
+# https://github.com/catnekaise/cognito-idpool-auth/blob/83ae9e159de469b3acd87ecb361d6b5957ee35ae/action.yml#L192-L227
+value=""
+
+if [ -n "${AWS_REGION}" ] && [ -n "${AWS_DEFAULT_REGION}" ]; then
+  value="$AWS_REGION"
+fi
+
+readonly value
+
+if [ -z "${value}" ]; then
+  echo 'Unable to resolve what AWS region to use'
+  exit 1
+fi
+
+# Some-effort validation of aws region
+if echo "${value}" | grep -Eqv '^[a-z]{2}-[a-z]{4,9}-[0-9]$'; then
+  echo 'Resolved value for AWS region is invalid'
+  exit 1
+fi
+
+echo "value=${value}" >> "${GITHUB_OUTPUT}"
+echo "AWS_REGION=${AWS_REGION}" >> "${GITHUB_ENV}"
+echo "AWS_DEFAULT_REGION=${AWS_DEFAULT_REGION}" >> "${GITHUB_ENV}"
diff --git a/...ns/get-vault-secrets/translate-secrets.sh → .../get-vault-secrets/translate-secrets.bash b/...ns/get-vault-secrets/translate-secrets.sh → .../get-vault-secrets/translate-secrets.bash
@@ -1,7 +1,7 @@
 #!/usr/bin/env bash
 
 # Input env:
-# - REPO => Repository name 
+# - REPO => Repository name
 # - COMMON_SECRETS => Common secrets (in the ci/data/common/<path> vault path): {{ Env Variable Name }}={{ Secret Path }}:{{ Secret Key }}
 # - REPO_SECRETS => Repo secrets (in the ci/data/repo/${REPO}/<path> vault path): {{ Env Variable Name }}={{ Secret Path }}:{{ Secret Key }}
 # Output format: "{{ Secret Path }} {{ Secret Key }} | {{ Env Variable Name }}" in the $GITHUB_OUTPUT file
@@ -19,6 +19,8 @@ if [ -z "$GITHUB_OUTPUT" ]; then
   exit 1
 fi
 
+readonly COMMON_SECRETS GITHUB_OUTPUT REPO REPO_SECRETS
+
 RESULT=""
 
 # Function to split a string into parts
@@ -43,18 +45,20 @@ split_string() {
 if [ -n "$COMMON_SECRETS" ]; then
     for common_secret in $COMMON_SECRETS; do
         split_string "$common_secret"
-        RESULT="${RESULT}$(echo "ci/data/common/$secret_path $secret_key | $env_variable_name");\n"
+        RESULT="${RESULT}ci/data/common/$secret_path $secret_key | $env_variable_name;\n"
     done
 fi
 
 # Translate the repo secrets
 if [ -n "$REPO_SECRETS" ]; then
     for repo_secret in $REPO_SECRETS; do
         split_string "$repo_secret"
-        RESULT="${RESULT}$(echo "ci/data/repo/$REPO/$secret_path $secret_key | $env_variable_name");\n"
+        RESULT="${RESULT}ci/data/repo/$REPO/$secret_path $secret_key | $env_variable_name;\n"
     done
 fi
 
+readonly RESULT
+
 # Print the contents of the output file
 echo -e "Secrets that will be queried from Vault:\n$RESULT"
 echo -e "secrets<<EOF\n${RESULT}EOF" > "$GITHUB_OUTPUT"
diff --git a/actions/get-vault-secrets/translate-secrets.bats b/actions/get-vault-secrets/translate-secrets.bats
@@ -18,20 +18,20 @@ teardown() {
 
 @test "Check if REPO environment variable is set" {
   export REPO=
-  run ./translate-secrets.sh
+  run ./translate-secrets.bash
   [ "$status" -ne 0 ]
   [ "${lines[0]}" = "Error: REPO environment variable is not set." ]
 }
 
 @test "Check if GITHUB_OUTPUT environment variable is set" {
   export GITHUB_OUTPUT=
-  run ./translate-secrets.sh
+  run ./translate-secrets.bash
   [ "$status" -ne 0 ]
   [ "${lines[0]}" = "Error: GITHUB_OUTPUT environment variable is not set." ]
 }
 
 @test "Translate secrets" {
-  run ./translate-secrets.sh
+  run ./translate-secrets.bash
   echo "$output" >&3
   [ "$status" -eq 0 ]
   [ "$output" = "Secrets that will be queried from Vault: