From 39a6a91c9cb82600441dcafc6480fc580a8f7416 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 1 Oct 2026 20:52:08 -0400 Subject: [PATCH] fix(ci): load the isolation checker from this workflow's commit The second checkout used the caller's SHA and the caller's token, so a private clone of this repo could not resolve the script. The checker is now a composite action referenced with $/. --- .github/actionlint.yaml | 6 ++ .../app-terraform-isolation/action.yml | 64 +++++++++++++++++++ .../check_app_terraform_isolation.py | 0 .github/workflows/ci-terraform.yaml | 55 +++------------- scripts/test_check_app_terraform_isolation.py | 22 +++++-- 5 files changed, 97 insertions(+), 50 deletions(-) create mode 100644 .github/actionlint.yaml create mode 100644 .github/actions/app-terraform-isolation/action.yml rename {scripts => .github/actions/app-terraform-isolation}/check_app_terraform_isolation.py (100%) diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml new file mode 100644 index 0000000..ce3828a --- /dev/null +++ b/.github/actionlint.yaml @@ -0,0 +1,6 @@ +# actionlint 1.7.12 rejects `$/`, which GitHub accepts as a self-repository +# action reference (runner 2.336.0+). Drop this ignore when a release parses it. +paths: + .github/workflows/ci-terraform.yaml: + ignore: + - 'specifying action "\$/.github/actions/app-terraform-isolation" in invalid format because ref is missing' diff --git a/.github/actions/app-terraform-isolation/action.yml b/.github/actions/app-terraform-isolation/action.yml new file mode 100644 index 0000000..cad89a5 --- /dev/null +++ b/.github/actions/app-terraform-isolation/action.yml @@ -0,0 +1,64 @@ +name: App and Terraform isolation +description: Fail when a change set mixes Terraform with deployable application files. + +inputs: + app-paths: + description: Newline-separated deployable paths. A trailing slash is a prefix. Any other entry is an exact file. + required: true + terraform-dir: + description: Directory containing Terraform sources. + required: true + default: terraform + event-name: + description: github.event_name from the calling workflow. + required: true + pr-base-sha: + description: pull_request base SHA. Empty outside pull_request. + required: false + default: "" + merge-group-base-sha: + description: merge_group base SHA. Empty outside merge_group. + required: false + default: "" + +runs: + using: composite + steps: + - name: Classify changed paths + shell: bash + working-directory: ${{ github.workspace }} + env: + APP_PATHS: ${{ inputs.app-paths }} + TERRAFORM_DIR: ${{ inputs.terraform-dir }} + EVENT_NAME: ${{ inputs.event-name }} + PR_BASE_SHA: ${{ inputs.pr-base-sha }} + MERGE_GROUP_BASE_SHA: ${{ inputs.merge-group-base-sha }} + CHECKER: ${{ github.action_path }}/check_app_terraform_isolation.py + run: | + set -euo pipefail + classify() { + python3 "${CHECKER}" + } + case "${EVENT_NAME}" in + pull_request) + if [[ -z "${PR_BASE_SHA}" ]]; then + echo "FAIL: pull_request base SHA is empty" >&2 + exit 1 + fi + merge_base="$(git merge-base "${PR_BASE_SHA}" HEAD)" + git diff --name-only --diff-filter=ACMRD "${merge_base}" HEAD | classify + ;; + merge_group) + if [[ -z "${MERGE_GROUP_BASE_SHA}" ]]; then + echo "FAIL: merge_group base SHA is empty" >&2 + exit 1 + fi + while IFS= read -r sha; do + [[ -z "${sha}" ]] && continue + git diff --name-only --diff-filter=ACMRD "${sha}^" "${sha}" | classify + done < <(git rev-list --reverse --first-parent "${MERGE_GROUP_BASE_SHA}..HEAD") + ;; + *) + echo "SKIP: live isolation runs on pull_request and merge_group (event: ${EVENT_NAME})" + ;; + esac diff --git a/scripts/check_app_terraform_isolation.py b/.github/actions/app-terraform-isolation/check_app_terraform_isolation.py similarity index 100% rename from scripts/check_app_terraform_isolation.py rename to .github/actions/app-terraform-isolation/check_app_terraform_isolation.py diff --git a/.github/workflows/ci-terraform.yaml b/.github/workflows/ci-terraform.yaml index 04cb14c..29dc9f9 100644 --- a/.github/workflows/ci-terraform.yaml +++ b/.github/workflows/ci-terraform.yaml @@ -21,6 +21,9 @@ name: CI — Terraform # merge_group classifies each first-parent commit against its parent, so a # Terraform-only PR stacked with an app-only PR still passes. The classified # diff includes deletions. Terraform paths are those under working-directory. +# The checker is a composite action in this repository. `$/` resolves that +# action at this workflow's commit, so callers do not clone this private +# repository with their GITHUB_TOKEN. on: workflow_call: @@ -72,50 +75,12 @@ jobs: - name: Terraform validate run: terraform validate - - name: Checkout isolation checker - if: inputs.app-paths != '' - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - repository: Sea-Haven-Industries/.github - ref: ${{ github.workflow_sha }} - path: .ci-org-github - persist-credentials: false - - name: App and Terraform isolation if: inputs.app-paths != '' - working-directory: ${{ github.workspace }} - env: - APP_PATHS: ${{ inputs.app-paths }} - TERRAFORM_DIR: ${{ inputs.working-directory }} - EVENT_NAME: ${{ github.event_name }} - PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} - MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} - CHECKER: ${{ github.workspace }}/.ci-org-github/scripts/check_app_terraform_isolation.py - run: | - set -euo pipefail - classify() { - python3 "${CHECKER}" - } - case "${EVENT_NAME}" in - pull_request) - if [[ -z "${PR_BASE_SHA}" ]]; then - echo "FAIL: pull_request base SHA is empty" >&2 - exit 1 - fi - merge_base="$(git merge-base "${PR_BASE_SHA}" HEAD)" - git diff --name-only --diff-filter=ACMRD "${merge_base}" HEAD | classify - ;; - merge_group) - if [[ -z "${MERGE_GROUP_BASE_SHA}" ]]; then - echo "FAIL: merge_group base SHA is empty" >&2 - exit 1 - fi - while IFS= read -r sha; do - [[ -z "${sha}" ]] && continue - git diff --name-only --diff-filter=ACMRD "${sha}^" "${sha}" | classify - done < <(git rev-list --reverse --first-parent "${MERGE_GROUP_BASE_SHA}..HEAD") - ;; - *) - echo "SKIP: live isolation runs on pull_request and merge_group (event: ${EVENT_NAME})" - ;; - esac + uses: $/.github/actions/app-terraform-isolation + with: + app-paths: ${{ inputs.app-paths }} + terraform-dir: ${{ inputs.working-directory }} + event-name: ${{ github.event_name }} + pr-base-sha: ${{ github.event.pull_request.base.sha }} + merge-group-base-sha: ${{ github.event.merge_group.base_sha }} diff --git a/scripts/test_check_app_terraform_isolation.py b/scripts/test_check_app_terraform_isolation.py index a5d1745..e367cec 100644 --- a/scripts/test_check_app_terraform_isolation.py +++ b/scripts/test_check_app_terraform_isolation.py @@ -6,14 +6,19 @@ from __future__ import annotations import os import re import subprocess +import sys import tempfile import unittest from pathlib import Path -from check_app_terraform_isolation import first_isolation_violation, isolation_violation +ROOT = Path(__file__).resolve().parents[1] +ACTION = ROOT / ".github" / "actions" / "app-terraform-isolation" +WORKFLOW = ROOT / ".github" / "workflows" / "ci-terraform.yaml" +sys.path.insert(0, str(ACTION)) -WORKFLOW = ( - Path(__file__).resolve().parents[1] / ".github" / "workflows" / "ci-terraform.yaml" +from check_app_terraform_isolation import ( # noqa: E402 + first_isolation_violation, + isolation_violation, ) APP_PATHS = "src/\npackage.json\npackage-lock.json\n" @@ -133,10 +138,16 @@ class WorkflowDiffTests(unittest.TestCase): def test_workflow_passes_working_directory(self) -> None: self.assertIn( - "TERRAFORM_DIR: ${{ inputs.working-directory }}", + "terraform-dir: ${{ inputs.working-directory }}", WORKFLOW.read_text(), ) + def test_checker_is_this_repos_action_at_the_workflow_commit(self) -> None: + text = WORKFLOW.read_text() + self.assertIn("uses: $/.github/actions/app-terraform-isolation", text) + self.assertNotIn("github.workflow_sha", text) + self.assertNotIn("repository: Sea-Haven-Industries/.github", text) + def test_deleted_app_file_is_classified(self) -> None: diff_filter = _workflow_diff_filters()[0] with tempfile.TemporaryDirectory() as tmp: @@ -173,7 +184,8 @@ class WorkflowDiffTests(unittest.TestCase): def _workflow_diff_filters() -> list[str]: - return re.findall(r"--diff-filter=([A-Z]+)", WORKFLOW.read_text()) + action = (ACTION / "action.yml").read_text() + return re.findall(r"--diff-filter=([A-Z]+)", action) def _git(repo: Path, *args: str) -> str: