From f764902323db5ea872fdeb25847fa139d5c67edc Mon Sep 17 00:00:00 2001 From: Michael Zalimeni Date: Tue, 24 Sep 2024 10:49:51 -0400 Subject: [PATCH] ci: fix conditional skip and add safeguard --- .github/scripts/check_skip_ci.sh | 49 ------------------ .github/workflows/build.yml | 14 ++++-- .../workflows/consul-dataplane-checks.yaml | 4 +- .../workflows/reusable-conditional-skip.yml | 50 ++++++++++++++++--- .github/workflows/security-scan.yml | 4 ++ 5 files changed, 61 insertions(+), 60 deletions(-) delete mode 100755 .github/scripts/check_skip_ci.sh diff --git a/.github/scripts/check_skip_ci.sh b/.github/scripts/check_skip_ci.sh deleted file mode 100755 index 0f65e46e..00000000 --- a/.github/scripts/check_skip_ci.sh +++ /dev/null @@ -1,49 +0,0 @@ -#!/bin/bash -# Copyright (c) HashiCorp, Inc. -# SPDX-License-Identifier: MPL-2.0 - -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).. -files_to_check=$(git diff --name-only "$(git merge-base origin/$SKIP_CHECK_BRANCH HEAD~)"...HEAD) - -# Define the directories to check -skipped_directories=("_doc/" ".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" \ No newline at end of file diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 42b38b49..e323c9c3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,15 +1,23 @@ name: build -# We now default to running this workflow on every push to every branch. +# We now default to running this workflow on every pull_request push +# in addition to protected branch push. +# # This provides fast feedback when build issues occur, so they can be -# fixed prior to being merged to the main branch. +# fixed prior to being merged. # # If you want to opt out of this, and only run the build on certain branches # please refer to the documentation on branch filtering here: # # https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onpushbranchestagsbranches-ignoretags-ignore # -on: [workflow_dispatch, push] +on: + push: + branches: + - main + - release/** + pull_request: + workflow_dispatch: env: PKG_NAME: "consul-dataplane" diff --git a/.github/workflows/consul-dataplane-checks.yaml b/.github/workflows/consul-dataplane-checks.yaml index d0b59434..79d821ca 100644 --- a/.github/workflows/consul-dataplane-checks.yaml +++ b/.github/workflows/consul-dataplane-checks.yaml @@ -3,8 +3,8 @@ name: consul-dataplane-checks on: push: branches: - - main - - 'release/*.*.x' + - main + - release/** pull_request: jobs: diff --git a/.github/workflows/reusable-conditional-skip.yml b/.github/workflows/reusable-conditional-skip.yml index ef469ee9..3671ebe5 100644 --- a/.github/workflows/reusable-conditional-skip.yml +++ b/.github/workflows/reusable-conditional-skip.yml @@ -12,13 +12,51 @@ jobs: runs-on: ubuntu-latest name: Check whether to skip build and tests outputs: - skip-ci: ${{ steps.check-changed-files.outputs.skip-ci }} - env: - SKIP_CHECK_BRANCH: ${{ github.head_ref || github.ref_name }} + 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: Ensure conditional check is 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 changed files - id: check-changed-files - run: ./.github/scripts/check_skip_ci.sh \ No newline at end of file + - name: Check for skippable file changes + id: changed-files + uses: tj-actions/changed-files@v45 + with: + files: | + .github/workflows/reusable-conditional-skip.yml + **.md + _doc/** + .changelog/** + - name: Print changed files (debug) + 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/security-scan.yml b/.github/workflows/security-scan.yml index 9bd0c638..ddcbb547 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -1,3 +1,5 @@ +# This job runs a non-blocking informational security scan on the repository. +# For release-blocking security scans, see .release/security-scan.hcl. name: Security Scan on: @@ -9,6 +11,8 @@ on: branches: - main - release/** + # paths-ignore only works for non-required checks. + # Jobs that are required for merge must use reusable-conditional-skip.yml. paths-ignore: - '_doc/**' - '.changelog/**'