From d6bcfb7a8736ca4a1c09535b01559ea36cd8aad5 Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 12 Jun 2024 13:53:28 +0100 Subject: [PATCH 01/10] Add workflow that lints shell scripts with ShellCheck Signed-off-by: Jack Baldry --- .github/workflows/build-lint-pr.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/build-lint-pr.yml b/.github/workflows/build-lint-pr.yml index 2f2f84dd4..935b5ef6a 100644 --- a/.github/workflows/build-lint-pr.yml +++ b/.github/workflows/build-lint-pr.yml @@ -36,3 +36,11 @@ jobs: body: "This PR contains the code built after dependabot updated dependencies on lint-pr-title action" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + shellcheck-shell-scripts: + name: Shellcheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Run ShellCheck + uses: ludeeus/action-shellcheck@cd81f4475ab741e097ec0fe73b692f3e49d66b8c From a9752a31695e731e07b7fbe7289327e13235216d Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 12 Jun 2024 13:57:35 +0100 Subject: [PATCH 02/10] Replace inline script with script file Signed-off-by: Jack Baldry --- actions/aws-auth/action.yaml | 24 ++---------------------- actions/aws-auth/resolve-aws-region.sh | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 22 deletions(-) create mode 100644 actions/aws-auth/resolve-aws-region.sh diff --git a/actions/aws-auth/action.yaml b/actions/aws-auth/action.yaml index 64c874c26..0519af1c5 100644 --- 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 new file mode 100644 index 000000000..85e80119f --- /dev/null +++ b/actions/aws-auth/resolve-aws-region.sh @@ -0,0 +1,23 @@ +#!/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 [ ! -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" From 3659bf09bc3d2d6732bac6ed9db0409fd8d440d2 Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 12 Jun 2024 13:59:24 +0100 Subject: [PATCH 03/10] 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 --- actions/aws-auth/resolve-aws-region.sh | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/actions/aws-auth/resolve-aws-region.sh b/actions/aws-auth/resolve-aws-region.sh index 85e80119f..9ff78f403 100644 --- a/actions/aws-auth/resolve-aws-region.sh +++ b/actions/aws-auth/resolve-aws-region.sh @@ -3,21 +3,23 @@ # https://github.com/catnekaise/cognito-idpool-auth/blob/83ae9e159de469b3acd87ecb361d6b5957ee35ae/action.yml#L192-L227 value="" -if [ ! -z "${AWS_REGION}" ] && [ ! -z "${AWS_DEFAULT_REGION}" ]; then +if [ -n "${AWS_REGION}" ] && [ -n "${AWS_DEFAULT_REGION}" ]; then value="$AWS_REGION" fi -if [ -z "$value" ]; then - echo "Unable to resolve what AWS Region to use" +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" +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" +echo "value=${value}" >> "${GITHUB_OUTPUT}" +echo "AWS_REGION=${AWS_REGION}" >> "${GITHUB_ENV}" +echo "AWS_DEFAULT_REGION=${AWS_DEFAULT_REGION}" >> "${GITHUB_ENV}" From f0a2f161df98b68f357adcf742346db58f473582 Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 12 Jun 2024 14:03:43 +0100 Subject: [PATCH 04/10] Move to lint-shared-workflows action Signed-off-by: Jack Baldry --- .github/workflows/build-lint-pr.yml | 8 -------- .github/workflows/lint-shared-workflows.yaml | 3 +++ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-lint-pr.yml b/.github/workflows/build-lint-pr.yml index 935b5ef6a..2f2f84dd4 100644 --- a/.github/workflows/build-lint-pr.yml +++ b/.github/workflows/build-lint-pr.yml @@ -36,11 +36,3 @@ jobs: body: "This PR contains the code built after dependabot updated dependencies on lint-pr-title action" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - shellcheck-shell-scripts: - name: Shellcheck - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: Run ShellCheck - uses: ludeeus/action-shellcheck@cd81f4475ab741e097ec0fe73b692f3e49d66b8c diff --git a/.github/workflows/lint-shared-workflows.yaml b/.github/workflows/lint-shared-workflows.yaml index 5e535fe08..0c843055c 100644 --- a/.github/workflows/lint-shared-workflows.yaml +++ b/.github/workflows/lint-shared-workflows.yaml @@ -24,6 +24,9 @@ jobs: - name: Lint workflow files uses: raven-actions/actionlint@789059c543ab20522fb3e7240794e13b0f69ad67 # v1.0.3 + - 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: runs-on: ubuntu-latest From c82ab4be77ab54c1ae00e44048438f1c13bbf87f Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 12 Jun 2024 14:10:01 +0100 Subject: [PATCH 05/10] 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 --- .../{translate-secrets.sh => translate-secrets.bash} | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) rename actions/get-vault-secrets/{translate-secrets.sh => translate-secrets.bash} (85%) diff --git a/actions/get-vault-secrets/translate-secrets.sh b/actions/get-vault-secrets/translate-secrets.bash similarity index 85% rename from actions/get-vault-secrets/translate-secrets.sh rename to actions/get-vault-secrets/translate-secrets.bash index fa2749e16..29e254b92 100755 --- a/actions/get-vault-secrets/translate-secrets.sh +++ b/actions/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/ vault path): {{ Env Variable Name }}={{ Secret Path }}:{{ Secret Key }} # - REPO_SECRETS => Repo secrets (in the ci/data/repo/${REPO}/ 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,7 +45,7 @@ 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 @@ -51,10 +53,12 @@ fi 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< "$GITHUB_OUTPUT" From 4278510cd7f2665a363390e0309e811a75084931 Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 12 Jun 2024 14:24:16 +0100 Subject: [PATCH 06/10] 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 --- README.md | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 47f133691..bee20dc47 100644 --- a/README.md +++ b/README.md @@ -1,18 +1,17 @@ # shared-workflows -A public-facing, centralized place to store reusable GitHub workflows and action -used by Grafana Labs. See the `actions/` directory for the individual actions -themselves. +A public-facing, centralized place to store reusable workflows and GitHub Actions used by Grafana Labs. +Refer to the [`actions/`](./actions) directory for the individual actions themselves. ## Notes ### Configure your IDE to run Prettier -[Prettier] will run in CI to ensure that files are formatted correctly. To ensure -that your code is formatted correctly before you commit, set up your IDE to run +[Prettier][] run in CI to ensure that files are formatted correctly. +To ensure that your code is formatted correctly before you commit, set up your IDE to run Prettier on save. -Or from the commandline, you can run Prettier using [`npx`][npx]: +Or from the command line, you can run Prettier using [`npx`][npx]: ```sh npx prettier --check . @@ -25,14 +24,12 @@ Or, of course, install it in any other way you want. ### Pin versions -When referencing third-party actions, [always pin the version to a specific -commit hash][hardening]. This ensures that the workflow will always use the same -version of the action, even if the action's maintainers release a new version or -if the tag itself is updated. +When using third-party actions, [always pin the version to a specific commit hash][hardening]. +This ensures that the workflow always uses the same version of the action, even if the action's maintainers release a new version or update the Git tag. -Dependabot can update these SHA references when there are new versions. If you -include a tag in a commend after the SHA, it can update the comment too. For -example: +Dependabot updates these SHA references when there are new versions. +If you include a tag in a commend after the SHA, it updates the comment too. +For example: ```yaml - uses: action/foo@abcdef0123456789abcdef0123456789 # v1.2.3 @@ -40,10 +37,11 @@ example: [hardening]: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions -### Refer to other `shared-workflows` actions using relative paths +### Use other `shared-workflows` actions with relative paths -When referencing other actions in this repository, use a relative path. This -will ensure actions in this repo are always used at the same commit. To do this: +When using other actions in this repository, use a relative path. +This means that workflows always use actions at the same commit. +To do this: ```yaml - name: Checkout @@ -57,9 +55,9 @@ will ensure actions in this repo are always used at the same commit. To do this: with: repository: ${{ env.action_repo }} ref: ${{ env.action_ref }} - # substitute your-action with a unique name (within `shared-repos` for your + # Substitute your-action with a unique name (within `shared-repos` for your # action), so if multiple actions check `shared-workflows` out, they don't - # overwrite each other + # overwrite each other. path: _shared-workflows-your-action - name: Use another action From fd58bbd7e78fa10fa3e4128a393b7c6c2fd11757 Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 12 Jun 2024 14:25:37 +0100 Subject: [PATCH 07/10] Document preference for separate shell scripts Signed-off-by: Jack Baldry --- README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/README.md b/README.md index bee20dc47..42d3de18e 100644 --- a/README.md +++ b/README.md @@ -65,3 +65,24 @@ To do this: with: 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 +``` From 8782b40e07dfd938e1db4663cb8a0ecfe3264da0 Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 16 Oct 2024 14:04:16 +0100 Subject: [PATCH 08/10] Revert "Lint README for Grafana Labs style" This reverts commit 4278510cd7f2665a363390e0309e811a75084931. --- README.md | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 0283dc47a..f819d3631 100644 --- a/README.md +++ b/README.md @@ -1,17 +1,18 @@ # shared-workflows -A public-facing, centralized place to store reusable workflows and GitHub Actions used by Grafana Labs. -Refer to the [`actions/`](./actions) directory for the individual actions themselves. +A public-facing, centralized place to store reusable GitHub workflows and action +used by Grafana Labs. See the `actions/` directory for the individual actions +themselves. ## Notes ### Configure your IDE to run Prettier -[Prettier][] run in CI to ensure that files are formatted correctly. -To ensure that your code is formatted correctly before you commit, set up your IDE to run +[Prettier] will run in CI to ensure that files are formatted correctly. To ensure +that your code is formatted correctly before you commit, set up your IDE to run Prettier on save. -Or from the command line, you can run Prettier using [`npx`][npx]: +Or from the commandline, you can run Prettier using [`npx`][npx]: ```sh npx prettier --check . @@ -24,12 +25,14 @@ Or, of course, install it in any other way you want. ### Pin versions -When using third-party actions, [always pin the version to a specific commit hash][hardening]. -This ensures that the workflow always uses the same version of the action, even if the action's maintainers release a new version or update the Git tag. +When referencing third-party actions, [always pin the version to a specific +commit hash][hardening]. This ensures that the workflow will always use the same +version of the action, even if the action's maintainers release a new version or +if the tag itself is updated. -Dependabot updates these SHA references when there are new versions. -If you include a tag in a commend after the SHA, it updates the comment too. -For example: +Dependabot can update these SHA references when there are new versions. If you +include a tag in a commend after the SHA, it can update the comment too. For +example: ```yaml - uses: action/foo@abcdef0123456789abcdef0123456789 # v1.2.3 @@ -37,11 +40,10 @@ For example: [hardening]: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions -### Use other `shared-workflows` actions with relative paths +### Refer to other `shared-workflows` actions using relative paths -When using other actions in this repository, use a relative path. -This means that workflows always use actions at the same commit. -To do this: +When referencing other actions in this repository, use a relative path. This +will ensure actions in this repo are always used at the same commit. To do this: ```yaml - name: Checkout @@ -55,9 +57,9 @@ To do this: with: repository: ${{ env.action_repo }} ref: ${{ env.action_ref }} - # Substitute your-action with a unique name (within `shared-repos` for your + # substitute your-action with a unique name (within `shared-repos` for your # action), so if multiple actions check `shared-workflows` out, they don't - # overwrite each other. + # overwrite each other path: _shared-workflows-your-action - name: Use another action From 7957be8d7baf066c42687eb54e5caaca1091e15b Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 16 Oct 2024 14:05:13 +0100 Subject: [PATCH 09/10] Restore documentation dropped in conflict resolution Signed-off-by: Jack Baldry --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index f819d3631..b6a0a88e3 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,10 @@ In order for the release action to work properly, which means to generate a CHAN Minor version bumps are indicated by new features: `feat: add support for eating lollipops`. Major version bumps are indicated by an `!` after the type in the commit message/description, for example: `feat!: rename foo input to bar`. +Also, the PR description needs to be filled and should never be empty. + +Failing to follow any of the aforementioned necessary steps, will lead to CI failing on your pull request. + More about how the upstream action works can be found [here](https://github.com/googleapis/release-please-action). ### Add new components to Release Please config file From d1c44671b286ec04d48bc03e9c75a11b0b1f954f Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Wed, 16 Oct 2024 14:06:09 +0100 Subject: [PATCH 10/10] Clean up whitespace Signed-off-by: Jack Baldry --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b6a0a88e3..9185edc1a 100644 --- a/README.md +++ b/README.md @@ -99,9 +99,9 @@ In order for the release action to work properly, which means to generate a CHAN Minor version bumps are indicated by new features: `feat: add support for eating lollipops`. Major version bumps are indicated by an `!` after the type in the commit message/description, for example: `feat!: rename foo input to bar`. -Also, the PR description needs to be filled and should never be empty. +Also, the PR description needs to be filled and should never be empty. -Failing to follow any of the aforementioned necessary steps, will lead to CI failing on your pull request. +Failing to follow any of the aforementioned necessary steps, will lead to CI failing on your pull request. More about how the upstream action works can be found [here](https://github.com/googleapis/release-please-action).