ci(terraform): fail mixed app and Terraform changes (#157)

* ci(terraform): fail mixed app and Terraform changes

* fix(ci): count deletions and honor the Terraform working directory

Deleted paths were excluded from the isolation diff, so a mixed change could pass. The checker now treats working-directory as the Terraform prefix.

* 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

@@ -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'

.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

.github/actions/app-terraform-isolation/check_app_terraform_isolation.py

@@ -0,0 +1,117 @@
+#!/usr/bin/env python3
+"""Fail when a change set mixes Terraform with deployable application files.
+
+APP_PATHS is newline-separated. A trailing slash is a directory prefix.
+Any other entry is an exact file. Paths that are not listed are neutral, so
+workflows, docs, and tests may travel with either side. An empty APP_PATHS
+skips the check.
+
+TERRAFORM_DIR is the Terraform working directory (default terraform). A path
+is Terraform when it equals that directory or sits under it.
+"""
+
+from __future__ import annotations
+
+import argparse
+import os
+import sys
+
+
+def parse_app_rules(raw: str) -> tuple[frozenset[str], frozenset[str]]:
+    prefixes: set[str] = set()
+    exact: set[str] = set()
+    for line in raw.splitlines():
+        item = line.strip().replace("\\", "/")
+        if not item or item.startswith("#"):
+            continue
+        if item.endswith("/"):
+            prefixes.add(item)
+        else:
+            exact.add(item)
+    return frozenset(prefixes), frozenset(exact)
+
+
+def terraform_prefix(terraform_dir: str) -> str:
+    prefix = terraform_dir.replace("\\", "/").strip().strip("/")
+    return prefix or "terraform"
+
+
+def is_terraform_path(path: str, terraform_dir: str = "terraform") -> bool:
+    normalized = path.replace("\\", "/")
+    prefix = terraform_prefix(terraform_dir)
+    return normalized == prefix or normalized.startswith(f"{prefix}/")
+
+
+def is_app_path(path: str, prefixes: frozenset[str], exact: frozenset[str]) -> bool:
+    normalized = path.replace("\\", "/")
+    if normalized in exact:
+        return True
+    for prefix in prefixes:
+        if normalized.startswith(prefix) or f"{normalized}/" == prefix:
+            return True
+    return False
+
+
+def isolation_violation(
+    paths: list[str],
+    app_paths: str,
+    terraform_dir: str = "terraform",
+) -> tuple[list[str], list[str]] | None:
+    prefixes, exact = parse_app_rules(app_paths)
+    if not prefixes and not exact:
+        return None
+    terraform_files = sorted(
+        {path for path in paths if is_terraform_path(path, terraform_dir)}
+    )
+    app_files = sorted({path for path in paths if is_app_path(path, prefixes, exact)})
+    if terraform_files and app_files:
+        return terraform_files, app_files
+    return None
+
+
+def first_isolation_violation(
+    file_sets: list[list[str]],
+    app_paths: str,
+    terraform_dir: str = "terraform",
+) -> tuple[list[str], list[str]] | None:
+    for paths in file_sets:
+        violation = isolation_violation(paths, app_paths, terraform_dir)
+        if violation is not None:
+            return violation
+    return None
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
+        "paths",
+        nargs="*",
+        help="Changed paths. Omit and pass newline-separated paths on stdin.",
+    )
+    args = parser.parse_args()
+    paths = list(args.paths)
+    if not paths and not sys.stdin.isatty():
+        paths = [line.strip() for line in sys.stdin if line.strip()]
+    terraform_dir = terraform_prefix(os.environ.get("TERRAFORM_DIR", "terraform"))
+    violation = isolation_violation(
+        paths, os.environ.get("APP_PATHS", ""), terraform_dir
+    )
+    if violation is None:
+        print("PASS: application and Terraform changes are isolated")
+        return 0
+    terraform_files, app_files = violation
+    print(
+        f"FAIL: do not mix deployable application files with {terraform_dir}/",
+        file=sys.stderr,
+    )
+    print("terraform:", file=sys.stderr)
+    for path in terraform_files:
+        print(f"  {path}", file=sys.stderr)
+    print("application:", file=sys.stderr)
+    for path in app_files:
+        print(f"  {path}", file=sys.stderr)
+    return 1
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

.github/workflows/ci-terraform.yaml

@@ -10,6 +10,20 @@ name: CI — Terraform
 #       uses: Sea-Haven-Industries/.github/.github/workflows/ci-terraform.yaml@<sha> # vX.Y.Z
 #       with:
 #         terraform-version: "1.16.0"
+#         app-paths: |
+#           src/
+#           package.json
+#           package-lock.json
+#
+# app-paths is optional. Empty skips isolation and leaves fmt/init/validate
+# unchanged. A trailing slash is a directory prefix. Any other line is an
+# exact file. pull_request classifies the merge-base of the base SHA to HEAD.
+# 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:
@@ -22,6 +36,10 @@ on:
         description: "Directory containing Terraform sources"
         type: string
         default: "terraform"
+      app-paths:
+        description: "Newline-separated deployable paths. A trailing slash is a prefix. Any other entry is an exact file. Empty skips isolation."
+        type: string
+        default: ""
 
 permissions:
   contents: read
@@ -41,6 +59,7 @@ jobs:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
           persist-credentials: false
+          fetch-depth: 0
 
       - uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4.0.1
         with:
@@ -55,3 +74,13 @@ jobs:
 
       - name: Terraform validate
         run: terraform validate
+
+      - name: App and Terraform isolation
+        if: inputs.app-paths != ''
+        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 }}

.github/workflows/ci.yaml

@@ -53,6 +53,10 @@ jobs:
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
 
+      - name: Isolation checker tests
+        run: python3 scripts/test_check_app_terraform_isolation.py
+        shell: bash
+
       - name: Install actionlint
         env:
           ACTIONLINT_VERSION: 1.7.12

scripts/test_check_app_terraform_isolation.py

@@ -0,0 +1,239 @@
+#!/usr/bin/env python3
+"""Tests for check_app_terraform_isolation."""
+
+from __future__ import annotations
+
+import os
+import re
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+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))
+
+from check_app_terraform_isolation import (  # noqa: E402
+    first_isolation_violation,
+    isolation_violation,
+)
+
+APP_PATHS = "src/\npackage.json\npackage-lock.json\n"
+
+
+class IsolationTests(unittest.TestCase):
+    def test_terraform_only(self) -> None:
+        self.assertIsNone(
+            isolation_violation(
+                ["terraform/lambda.tf", "terraform/README.md"],
+                APP_PATHS,
+            )
+        )
+
+    def test_app_only(self) -> None:
+        self.assertIsNone(
+            isolation_violation(
+                ["src/processPaymentCsv.js", "package.json", "package-lock.json"],
+                APP_PATHS,
+            )
+        )
+
+    def test_docs_and_workflows_with_terraform(self) -> None:
+        self.assertIsNone(
+            isolation_violation(
+                [
+                    "terraform/lambda.tf",
+                    ".github/workflows/deploy.yaml",
+                    "SETUP.md",
+                    "scripts/check_app_terraform_isolation.py",
+                ],
+                APP_PATHS,
+            )
+        )
+
+    def test_mixed_app_and_terraform_fails(self) -> None:
+        violation = isolation_violation(
+            ["terraform/lambda.tf", "src/processPaymentCsv.js", "package.json"],
+            APP_PATHS,
+        )
+        self.assertIsNotNone(violation)
+        terraform_files, app_files = violation or ([], [])
+        self.assertEqual(terraform_files, ["terraform/lambda.tf"])
+        self.assertEqual(app_files, ["package.json", "src/processPaymentCsv.js"])
+
+    def test_empty_app_paths_skips(self) -> None:
+        self.assertIsNone(
+            isolation_violation(
+                ["terraform/lambda.tf", "src/processPaymentCsv.js"],
+                "",
+            )
+        )
+
+    def test_separate_commits_pass_when_classified_alone(self) -> None:
+        self.assertIsNone(
+            first_isolation_violation(
+                [
+                    ["terraform/lambda.tf"],
+                    ["src/processPaymentCsv.js"],
+                ],
+                APP_PATHS,
+            )
+        )
+
+    def test_union_of_separate_commits_fails(self) -> None:
+        violation = isolation_violation(
+            ["terraform/lambda.tf", "src/processPaymentCsv.js"],
+            APP_PATHS,
+        )
+        self.assertIsNotNone(violation)
+
+    def test_non_default_terraform_dir_mixed_fails(self) -> None:
+        violation = isolation_violation(
+            ["infra/main.tf", "src/app.js"],
+            "src/\n",
+            terraform_dir="infra",
+        )
+        self.assertIsNotNone(violation)
+        terraform_files, app_files = violation or ([], [])
+        self.assertEqual(terraform_files, ["infra/main.tf"])
+        self.assertEqual(app_files, ["src/app.js"])
+
+    def test_default_dir_leaves_other_prefixes_neutral(self) -> None:
+        self.assertIsNone(
+            isolation_violation(["infra/main.tf", "src/app.js"], "src/\n")
+        )
+
+    def test_terraform_prefix_does_not_match_a_longer_directory(self) -> None:
+        self.assertIsNone(
+            isolation_violation(
+                ["infrastructure/main.tf", "src/app.js"],
+                "src/\n",
+                terraform_dir="infra",
+            )
+        )
+
+    def test_terraform_dir_trailing_slash(self) -> None:
+        violation = isolation_violation(
+            ["infra/main.tf", "src/app.js"],
+            "src/\n",
+            terraform_dir="infra/",
+        )
+        self.assertIsNotNone(violation)
+
+    def test_blank_terraform_dir_defaults_to_terraform(self) -> None:
+        violation = isolation_violation(
+            ["terraform/lambda.tf", "src/app.js"],
+            "src/\n",
+            terraform_dir="  ",
+        )
+        self.assertIsNotNone(violation)
+
+
+class WorkflowDiffTests(unittest.TestCase):
+    def test_classified_diffs_include_deletions(self) -> None:
+        self.assertEqual(_workflow_diff_filters(), ["ACMRD", "ACMRD"])
+
+    def test_workflow_passes_working_directory(self) -> None:
+        self.assertIn(
+            "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:
+            repo = Path(tmp)
+            base = _commit_base(repo)
+            (repo / "terraform" / "lambda.tf").write_text("changed\n")
+            (repo / "src" / "processPaymentCsv.js").unlink()
+            _git(repo, "add", "-A")
+            _git(repo, "commit", "-m", "decommission handler")
+            omitted = _changed_paths(repo, base, "HEAD", "ACMR")
+            included = _changed_paths(repo, base, "HEAD", diff_filter)
+        self.assertNotIn("src/processPaymentCsv.js", omitted)
+        self.assertIn("src/processPaymentCsv.js", included)
+        self.assertIn("terraform/lambda.tf", included)
+        self.assertIsNotNone(isolation_violation(included, APP_PATHS))
+
+    def test_deleted_terraform_file_is_classified(self) -> None:
+        diff_filter = _workflow_diff_filters()[0]
+        with tempfile.TemporaryDirectory() as tmp:
+            repo = Path(tmp)
+            base = _commit_base(repo)
+            (repo / "terraform" / "lambda.tf").unlink()
+            (repo / "src" / "processPaymentCsv.js").write_text("changed\n")
+            _git(repo, "add", "-A")
+            _git(repo, "commit", "-m", "remove lambda")
+            omitted = _changed_paths(repo, base, "HEAD", "ACMR")
+            included = _changed_paths(repo, base, "HEAD", diff_filter)
+        self.assertNotIn("terraform/lambda.tf", omitted)
+        violation = isolation_violation(included, APP_PATHS)
+        self.assertIsNotNone(violation)
+        terraform_files, app_files = violation or ([], [])
+        self.assertEqual(terraform_files, ["terraform/lambda.tf"])
+        self.assertEqual(app_files, ["src/processPaymentCsv.js"])
+
+
+def _workflow_diff_filters() -> list[str]:
+    action = (ACTION / "action.yml").read_text()
+    return re.findall(r"--diff-filter=([A-Z]+)", action)
+
+
+def _git(repo: Path, *args: str) -> str:
+    env = os.environ.copy()
+    env["GIT_CONFIG_GLOBAL"] = os.devnull
+    env["GIT_CONFIG_NOSYSTEM"] = "1"
+    completed = subprocess.run(
+        [
+            "git",
+            "-c",
+            "commit.gpgsign=false",
+            "-c",
+            "user.name=test",
+            "-c",
+            "user.email=test@example.com",
+            *args,
+        ],
+        cwd=repo,
+        check=True,
+        capture_output=True,
+        text=True,
+        env=env,
+    )
+    return completed.stdout
+
+
+def _commit_base(repo: Path) -> str:
+    _git(repo, "init", "-b", "main")
+    (repo / "terraform").mkdir()
+    (repo / "src").mkdir()
+    (repo / "terraform" / "lambda.tf").write_text("resource\n")
+    (repo / "src" / "processPaymentCsv.js").write_text("export {}\n")
+    _git(repo, "add", ".")
+    _git(repo, "commit", "-m", "base")
+    return _git(repo, "rev-parse", "HEAD").strip()
+
+
+def _changed_paths(repo: Path, base: str, head: str, diff_filter: str) -> list[str]:
+    output = _git(
+        repo,
+        "diff",
+        "--name-only",
+        f"--diff-filter={diff_filter}",
+        base,
+        head,
+    )
+    return [line for line in output.splitlines() if line]
+
+
+if __name__ == "__main__":
+    unittest.main()