From 36fd223dd878e805643446a63e071fa0136d100b Mon Sep 17 00:00:00 2001 From: Michael Zalimeni Date: Wed, 25 Sep 2024 10:22:29 -0400 Subject: [PATCH] ci: fix conditional skip and add safeguard Adopt a third-party action to avoid script bugs, and to fix a current issue where the script fails to detect all changes when processing push events on PR branches. Adapted from hashicorp/consul-dataplane#637. See that PR for testing details and background context. --- .github/scripts/check_skip_ci.sh | 50 -------------- .github/workflows/go-tests.yml | 13 +--- .../workflows/reusable-conditional-skip.yml | 69 +++++++++++++++++++ .github/workflows/test-integrations.yml | 13 +--- 4 files changed, 71 insertions(+), 74 deletions(-) delete mode 100755 .github/scripts/check_skip_ci.sh create mode 100644 .github/workflows/reusable-conditional-skip.yml diff --git a/.github/scripts/check_skip_ci.sh b/.github/scripts/check_skip_ci.sh deleted file mode 100755 index a22b990f2692e..0000000000000 --- a/.github/scripts/check_skip_ci.sh +++ /dev/null @@ -1,50 +0,0 @@ -#!/bin/bash -# Copyright (c) HashiCorp, Inc. -# SPDX-License-Identifier: BUSL-1.1 - -set -euo pipefail - -# Get the list of changed files -# Using `git merge-base` ensures that we're always comparing against the correct branch point. -#For example, given the commits: -# -# A---B---C---D---W---X---Y---Z # origin/main -# \---E---F # feature/branch -# -# ... `git merge-base origin/$SKIP_CHECK_BRANCH HEAD` would return commit `D` -# `...HEAD` specifies from the common ancestor to the latest commit on the current branch (HEAD).. -skip_check_branch=${SKIP_CHECK_BRANCH:?SKIP_CHECK_BRANCH is required} -files_to_check=$(git diff --name-only "$(git merge-base origin/$skip_check_branch HEAD~)"...HEAD) - -# Define the directories to check -skipped_directories=("docs/" "ui/" "website/" "grafana/" ".changelog/") - -# Loop through the changed files and find directories/files outside the skipped ones -files_to_check_array=($files_to_check) -for file_to_check in "${files_to_check_array[@]}"; do - file_is_skipped=false - echo "checking file: $file_to_check" - - # Allow changes to: - # - This script - # - Files in the skipped directories - # - Markdown files - for dir in "${skipped_directories[@]}"; do - if [[ "$file_to_check" == */check_skip_ci.sh ]] || - [[ "$file_to_check" == "$dir"* ]] || - [[ "$file_to_check" == *.md ]]; then - file_is_skipped=true - break - fi - done - - if [ "$file_is_skipped" != "true" ]; then - echo -e "non-skippable file changed: $file_to_check" - echo "Changes detected in non-documentation files - will not skip tests and build" - echo "skip-ci=false" >> "$GITHUB_OUTPUT" - exit 0 ## if file is outside of the skipped_directory exit script - fi -done - -echo "Changes detected in only documentation files - skipping tests and build" -echo "skip-ci=true" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 6ea14b4fab031..c30e6e81946ab 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -22,7 +22,6 @@ permissions: env: TEST_RESULTS: /tmp/test-results GOPRIVATE: github.com/hashicorp # Required for enterprise deps - SKIP_CHECK_BRANCH: ${{ github.head_ref || github.ref_name }} # concurrency concurrency: @@ -31,17 +30,7 @@ concurrency: jobs: conditional-skip: - runs-on: ubuntu-latest - name: Get files changed and conditionally skip CI - outputs: - skip-ci: ${{ steps.read-files.outputs.skip-ci }} - steps: - - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - with: - fetch-depth: 0 - - name: Get changed files - id: read-files - run: ./.github/scripts/check_skip_ci.sh + uses: ./.github/workflows/reusable-conditional-skip.yml setup: needs: [conditional-skip] diff --git a/.github/workflows/reusable-conditional-skip.yml b/.github/workflows/reusable-conditional-skip.yml new file mode 100644 index 0000000000000..3fb1dc448b900 --- /dev/null +++ b/.github/workflows/reusable-conditional-skip.yml @@ -0,0 +1,69 @@ +name: conditional-skip + +on: + workflow_call: + outputs: + skip-ci: + description: "Whether we should skip build and test jobs" + value: ${{ jobs.check-skip.outputs.skip-ci }} + +jobs: + check-skip: + runs-on: ubuntu-latest + name: Check whether to skip build and tests + outputs: + skip-ci: ${{ steps.maybe-skip-ci.outputs.skip-ci }} + steps: + # We only allow use of conditional skip in two scenarios: + # 1. PRs + # 2. Pushes (merges) to protected branches (`main`, `release/**`) + # + # The second scenario is the only place we can be sure that checking just the + # latest change on the branch is sufficient. In PRs, we need to check _all_ commits. + # The ability to do this is ultimately determined by the triggers of the calling + # workflow, since `base_ref` (the target branch of a PR) is only available in + # `pull_request` events, not `push`. + - name: Error if conditional check is not allowed + if: ${{ !github.base_ref && !github.ref_protected }} + run: | + echo "Conditional skip requires a PR event with 'base_ref' or 'push' to a protected branch." + echo "github.base_ref: ${{ github.base_ref }}" + echo "github.ref_protected: ${{ github.ref_protected }}" + echo "github.ref_name: ${{ github.ref_name }}" + echo "Check the triggers of the calling workflow to ensure that these requirements are met." + exit 1 + - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + with: + fetch-depth: 0 + - name: Check for skippable file changes + id: changed-files + uses: tj-actions/changed-files@v45 + with: + # This is a multi-line YAML string with one match pattern per line. + # Do not use quotes around values, as it's not supported. + # See https://github.com/tj-actions/changed-files/blob/main/README.md#inputs-%EF%B8%8F + # for usage, options, and more details on match syntax. + files: | + .github/workflows/reusable-conditional-skip.yml + **.md + docs/** + ui/** + website/** + grafana/** + .changelog/** + - name: Print changed files + env: + SKIPPABLE_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }} + NON_SKIPPABLE_FILES: ${{ steps.changed-files.outputs.other_changed_files }} + run: | + echo "Skippable changed files:" + for file in ${SKIPPABLE_CHANGED_FILES}; do echo " $file"; done + echo + echo "Non-skippable files:" + for file in ${NON_SKIPPABLE_FILES}; do echo " $file"; done + - name: Skip tests and build if only skippable files changed + id: maybe-skip-ci + if: ${{ steps.changed-files.outputs.only_changed == 'true' }} + run: | + echo "Skipping tests and build because only skippable files changed" + echo "skip-ci=true" >> $GITHUB_OUTPUT \ No newline at end of file diff --git a/.github/workflows/test-integrations.yml b/.github/workflows/test-integrations.yml index c83338ee0c9d2..f5d8e30eca2e0 100644 --- a/.github/workflows/test-integrations.yml +++ b/.github/workflows/test-integrations.yml @@ -29,7 +29,6 @@ env: # strip the hashicorp/ off the front of github.repository for consul CONSUL_LATEST_IMAGE_NAME: ${{ endsWith(github.repository, '-enterprise') && github.repository || 'hashicorp/consul' }} GOPRIVATE: github.com/hashicorp # Required for enterprise deps - SKIP_CHECK_BRANCH: ${{ github.head_ref || github.ref_name }} concurrency: group: "${{ github.workflow }}-${{ github.head_ref || github.ref }}" @@ -37,17 +36,7 @@ concurrency: jobs: conditional-skip: - runs-on: ubuntu-latest - name: Get files changed and conditionally skip CI - outputs: - skip-ci: ${{ steps.read-files.outputs.skip-ci }} - steps: - - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - with: - fetch-depth: 0 - - name: Get changed files - id: read-files - run: ./.github/scripts/check_skip_ci.sh + uses: ./.github/workflows/reusable-conditional-skip.yml setup: needs: [conditional-skip]