mirror of
https://github.com/Sea-Haven-Industries/shoc-frontend-new.git
synced 2026-09-30 08:03:13 +00:00
ci(terraform-isolation): re-evaluate the gate on label changes (#177)
* ci(terraform-isolation): re-evaluate the gate on label changes * test(terraform-isolation): lock the ci.yaml label-event contract * fix(terraform-isolation): do not treat terraform markdown as a mixed change * fix(ci): do not skip Frontend checks on isolation label events * ci(terraform-isolation): run label retriggers in a dedicated workflow * fix: apply eslint formatting * fix: apply additional missed eslint formatting
This commit is contained in:
parent
763b82a77d
commit
8b5281d357
7 changed files with 135 additions and 43 deletions
24
.github/workflows/ci.yaml
vendored
24
.github/workflows/ci.yaml
vendored
|
|
@ -71,30 +71,6 @@ jobs:
|
|||
env:
|
||||
GOVERNANCE_BASE: ${{ steps.governance-ref.outputs.base }}
|
||||
|
||||
terraform-isolation:
|
||||
# Fails a pull request that changes `terraform/**` together with deployable
|
||||
# application code (scripts/check-terraform-isolation.mjs). A merge that
|
||||
# does both queues an HCP VCS run and a content release at the same time,
|
||||
# and the two race for the workspace lock. The
|
||||
# `terraform-isolation-override` label is the reviewed exception; it is
|
||||
# read when the job runs, so re-run this workflow after labeling.
|
||||
name: Terraform and application changes are isolated
|
||||
if: github.event_name == 'pull_request'
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
with:
|
||||
fetch-depth: 0
|
||||
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
|
||||
with:
|
||||
node-version: "24"
|
||||
- name: Check changed files
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
TERRAFORM_ISOLATION_OVERRIDE: ${{ contains(github.event.pull_request.labels.*.name, 'terraform-isolation-override') }}
|
||||
run: node scripts/check-terraform-isolation.mjs --base "${BASE_SHA}" --head "${HEAD_SHA}"
|
||||
|
||||
visual-regression:
|
||||
name: Visual regression
|
||||
runs-on: ubuntu-latest
|
||||
|
|
|
|||
43
.github/workflows/terraform-isolation.yaml
vendored
Normal file
43
.github/workflows/terraform-isolation.yaml
vendored
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
name: Terraform isolation
|
||||
|
||||
# Own workflow so labeled/unlabeled re-evaluate this gate without starting a
|
||||
# new Frontend checks run. Skipping jobs inside `ci.yaml` on those events
|
||||
# would report required checks as success and could merge a failing SHA.
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
branches: [main, dev, staging]
|
||||
types:
|
||||
- opened
|
||||
- synchronize
|
||||
- reopened
|
||||
- labeled
|
||||
- unlabeled
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
terraform-isolation:
|
||||
# Fails a pull request that changes Terraform infrastructure together with
|
||||
# deployable application code (scripts/check-terraform-isolation.mjs). A
|
||||
# merge that does both queues an HCP VCS run and a content release at the
|
||||
# same time, and the two race for the workspace lock. The
|
||||
# `terraform-isolation-override` label is the reviewed exception. This
|
||||
# job is unconditional so adding or removing that label always reads the
|
||||
# current label set; a previous green check does not survive removal.
|
||||
name: Terraform and application changes are isolated
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
with:
|
||||
fetch-depth: 0
|
||||
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
|
||||
with:
|
||||
node-version: "24"
|
||||
- name: Check changed files
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
TERRAFORM_ISOLATION_OVERRIDE: ${{ contains(github.event.pull_request.labels.*.name, 'terraform-isolation-override') }}
|
||||
run: node scripts/check-terraform-isolation.mjs --base "${BASE_SHA}" --head "${HEAD_SHA}"
|
||||
|
|
@ -30,7 +30,7 @@ and synthesis. A task is not done until this is green.
|
|||
| Terraform isolation gate contract | `npm run test:terraform-isolation` → `scripts/check-terraform-isolation.test.mjs` | `governance` + CI | Changed-file classifier |
|
||||
| Terraform formatting/validation | `npm run test:terraform` → `scripts/terraform-validate.mjs` | `governance` + CI | `terraform/live/dev` |
|
||||
| CDK build, tests, synthesis | `npm run test:infra` | `governance` + CI | `infra/cdk/**`, both synth modes |
|
||||
| Terraform/app change isolation | `ci.yaml` job `terraform-isolation` → `scripts/check-terraform-isolation.mjs` | CI (PR) | Changed files of the PR |
|
||||
| Terraform/app change isolation | `terraform-isolation.yaml` job `terraform-isolation` → `scripts/check-terraform-isolation.mjs` | CI (PR) | Changed files of the PR |
|
||||
|
||||
## No-false-pass guarantees
|
||||
|
||||
|
|
@ -49,10 +49,13 @@ and synthesis. A task is not done until this is green.
|
|||
against synthetic plan JSON. Real import and controlled-update plans from HCP
|
||||
are migration evidence reviewed by a human before an approved apply
|
||||
(`terraform/README.md`).
|
||||
- **The isolation gate reads the override when it runs** — the
|
||||
- **The isolation gate re-evaluates on label changes** — the
|
||||
`terraform-isolation-override` label is the only way to merge a mixed
|
||||
Terraform/application PR, the gate logs the override on the run, and the
|
||||
workflow must be re-run after the label is added or removed.
|
||||
Terraform/application PR. `.github/workflows/terraform-isolation.yaml`
|
||||
runs `terraform-isolation` on `labeled` and `unlabeled` as well as the
|
||||
default pull-request types, so adding or removing the label re-checks
|
||||
the current labels without starting a new Frontend checks run. Removing
|
||||
the label fails a mixed PR that had previously passed with the override.
|
||||
|
||||
## Where the gates run
|
||||
|
||||
|
|
@ -63,9 +66,10 @@ and synthesis. A task is not done until this is green.
|
|||
format/lint/build/tests, **and** a repo-owned `governance` job runs
|
||||
`npm run verify` (with Terraform 1.16.0 installed) so the maintainability
|
||||
ratchets and repository gates are guaranteed from this repository regardless
|
||||
of the reusable workflow. On pull requests the same workflow's
|
||||
`terraform-isolation` job fails when `terraform/**` and application code
|
||||
change together.
|
||||
of the reusable workflow.
|
||||
- **Terraform isolation ([`.github/workflows/terraform-isolation.yaml`](.github/workflows/terraform-isolation.yaml)):**
|
||||
on pull requests, fails when Terraform infrastructure and application code
|
||||
change together. Label add/remove re-runs only this workflow.
|
||||
|
||||
## Toolchain pin
|
||||
|
||||
|
|
|
|||
|
|
@ -156,10 +156,10 @@ No stored AWS keys — OIDC only. Infrastructure and content deploy separately:
|
|||
[`AGENTS.md`](AGENTS.md), [`QUALITY_GATES.md`](QUALITY_GATES.md),
|
||||
[`ARCHITECTURE_AND_CODE_QUALITY.md`](ARCHITECTURE_AND_CODE_QUALITY.md), and
|
||||
[`REVIEW_AND_PR_FRAMEWORK.md`](REVIEW_AND_PR_FRAMEWORK.md).
|
||||
- **Terraform isolation** (the `terraform-isolation` job in
|
||||
[`.github/workflows/ci.yaml`](.github/workflows/ci.yaml)) — fails a PR that
|
||||
mixes `terraform/**` with application code, so a Terraform merge never races
|
||||
a content release for the HCP workspace.
|
||||
- **Terraform isolation**
|
||||
([`.github/workflows/terraform-isolation.yaml`](.github/workflows/terraform-isolation.yaml))
|
||||
— fails a PR that mixes `terraform/**` with application code, so a Terraform
|
||||
merge never races a content release for the HCP workspace.
|
||||
- **Dev content** ([`.github/workflows/deploy.yml`](.github/workflows/deploy.yml))
|
||||
— `workflow_dispatch` on `dev` only while the Terraform adoption is in
|
||||
progress. Runs `npm run verify`, assumes `githubdeploy-shoc-frontend-new-dev`,
|
||||
|
|
|
|||
|
|
@ -17,7 +17,9 @@
|
|||
// TERRAFORM_ISOLATION_OVERRIDE=true downgrades a failure to a warning. CI sets
|
||||
// it only when the PR carries the `terraform-isolation-override` label, which
|
||||
// reviewers grant to the rare change that must introduce Terraform variables
|
||||
// together with the workflow that consumes them.
|
||||
// together with the workflow that consumes them. The checker has no memory of
|
||||
// a previous pass: the same mixed diff fails again as soon as the override
|
||||
// env is unset (label removal).
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { readFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
|
|
@ -31,6 +33,13 @@ export function isTerraformPath(file) {
|
|||
return file.startsWith("terraform/");
|
||||
}
|
||||
|
||||
// Markdown under terraform/ does not queue an HCP VCS run (workspace triggers
|
||||
// are terraform/live/dev/** and terraform/live/modules/**), so it is not a
|
||||
// Terraform change for the mixed-PR check.
|
||||
export function isTerraformInfrastructurePath(file) {
|
||||
return isTerraformPath(file) && !file.endsWith(".md");
|
||||
}
|
||||
|
||||
export function mayAccompanyTerraform(file) {
|
||||
if (isTerraformPath(file)) return true;
|
||||
if (file.endsWith(".md")) return true;
|
||||
|
|
@ -45,7 +54,7 @@ export function mayAccompanyTerraform(file) {
|
|||
*/
|
||||
export function classifyChangedFiles(files) {
|
||||
const unique = [...new Set(files.map((file) => file.trim()).filter(Boolean))].sort();
|
||||
const terraform = unique.filter(isTerraformPath);
|
||||
const terraform = unique.filter(isTerraformInfrastructurePath);
|
||||
const application = unique.filter((file) => !mayAccompanyTerraform(file));
|
||||
return {
|
||||
terraform,
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import assert from "node:assert/strict";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { readFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { test } from "node:test";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
|
@ -7,6 +8,7 @@ import { fileURLToPath } from "node:url";
|
|||
import {
|
||||
OVERRIDE_LABEL,
|
||||
classifyChangedFiles,
|
||||
isTerraformInfrastructurePath,
|
||||
mayAccompanyTerraform,
|
||||
} from "./check-terraform-isolation.mjs";
|
||||
|
||||
|
|
@ -71,6 +73,18 @@ test("terraform-only and application-only changes are not mixed", () => {
|
|||
assert.equal(classifyChangedFiles([]).mixed, false);
|
||||
});
|
||||
|
||||
test("terraform documentation does not mix with application or workflow changes", () => {
|
||||
assert.equal(isTerraformInfrastructurePath("terraform/README.md"), false);
|
||||
assert.equal(isTerraformInfrastructurePath("terraform/live/dev/main.tf"), true);
|
||||
assert.equal(
|
||||
classifyChangedFiles(["terraform/README.md", ".github/workflows/ci.yaml"]).mixed,
|
||||
false,
|
||||
);
|
||||
const docsOnly = runGate(["terraform/README.md", ".github/workflows/ci.yaml"]);
|
||||
assert.equal(docsOnly.status, 0, docsOnly.stdout + docsOnly.stderr);
|
||||
assert.match(docsOnly.stdout, /PASS/);
|
||||
});
|
||||
|
||||
test("terraform plus application is mixed and lists the offending files", () => {
|
||||
const result = classifyChangedFiles([
|
||||
"terraform/live/dev/main.tf",
|
||||
|
|
@ -109,7 +123,49 @@ test("CLI override downgrades a mixed change to a warning that names the label",
|
|||
assert.equal(notTrue.status, 1);
|
||||
});
|
||||
|
||||
test("removing the override fails a mixed change that was previously green", () => {
|
||||
const files = ["terraform/live/dev/main.tf", ".github/workflows/deploy.yml"];
|
||||
const previouslyGreen = runGate(files, {
|
||||
TERRAFORM_ISOLATION_OVERRIDE: "true",
|
||||
});
|
||||
assert.equal(previouslyGreen.status, 0, previouslyGreen.stdout + previouslyGreen.stderr);
|
||||
assert.match(previouslyGreen.stdout, /WARNING/);
|
||||
|
||||
// CI sets TERRAFORM_ISOLATION_OVERRIDE from contains(...labels), which is
|
||||
// the string "false" after the label is removed. A stale green check must
|
||||
// not survive that.
|
||||
const afterLabelRemoved = runGate(files, {
|
||||
TERRAFORM_ISOLATION_OVERRIDE: "false",
|
||||
});
|
||||
assert.equal(afterLabelRemoved.status, 1, afterLabelRemoved.stdout + afterLabelRemoved.stderr);
|
||||
assert.match(afterLabelRemoved.stdout, /FAIL/);
|
||||
assert.match(afterLabelRemoved.stdout, /deploy\.yml/);
|
||||
});
|
||||
|
||||
test("CLI refuses to run without a base ref or --stdin", () => {
|
||||
const result = spawnSync(process.execPath, [SCRIPT], { encoding: "utf8" });
|
||||
assert.notEqual(result.status, 0);
|
||||
});
|
||||
|
||||
test("isolation workflow re-evaluates on labeled and unlabeled without rerunning Frontend checks", () => {
|
||||
const workflows = path.join(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
"..",
|
||||
".github/workflows",
|
||||
);
|
||||
const ciYaml = readFileSync(path.join(workflows, "ci.yaml"), "utf8");
|
||||
const isolationYaml = readFileSync(path.join(workflows, "terraform-isolation.yaml"), "utf8");
|
||||
|
||||
for (const eventType of ["opened", "synchronize", "reopened", "labeled", "unlabeled"]) {
|
||||
assert.match(isolationYaml, new RegExp(`^ {6}- ${eventType}$`, "m"), eventType);
|
||||
}
|
||||
|
||||
assert.doesNotMatch(ciYaml, /^ {6}- labeled$/m);
|
||||
assert.doesNotMatch(ciYaml, /^ {6}- unlabeled$/m);
|
||||
assert.doesNotMatch(ciYaml, /^ {2}terraform-isolation:\n/m);
|
||||
assert.doesNotMatch(ciYaml, /github\.event\.action != 'labeled'/);
|
||||
|
||||
assert.match(isolationYaml, /^ {2}terraform-isolation:\n/m);
|
||||
assert.match(isolationYaml, /name: Terraform and application changes are isolated/);
|
||||
assert.doesNotMatch(isolationYaml, /github\.event\.action != 'labeled'/);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -218,14 +218,18 @@ the labels. Push-to-`dev` releases return behind the repository variable
|
|||
|
||||
- **Terraform-only PRs.** A PR that changes `terraform/**` may not change
|
||||
deployable application code. The `terraform-isolation` job in
|
||||
`.github/workflows/ci.yaml` enforces this; documentation and the
|
||||
`.github/workflows/terraform-isolation.yaml` enforces this; documentation and the
|
||||
`scripts/*terraform*` tooling are allowed alongside. A reviewer may add the
|
||||
`terraform-isolation-override` label for the rare change that must introduce
|
||||
Terraform variables together with the workflow that consumes them (PR A and
|
||||
PR C), then re-run the workflow so the job reads the label. The label is the
|
||||
approval record. The override is temporary: a follow-up PR after PR C
|
||||
removes the label path from the checker and workflow so the gate has no
|
||||
exception.
|
||||
Terraform variables together with the workflow that consumes them (PR C).
|
||||
Adding or removing that label re-runs only that workflow against the labels
|
||||
currently on the PR; Frontend checks does not start a new run. Removing the
|
||||
label fails a mixed PR that had previously passed with the override, so a
|
||||
stale green check cannot merge. Markdown under `terraform/` does not count as a Terraform
|
||||
change for this gate; it does not match the workspace trigger patterns.
|
||||
The label is the approval record. The override is temporary:
|
||||
a follow-up PR after PR C removes the label path from the checker and
|
||||
workflow so the gate has no exception.
|
||||
- **Every Terraform merge produces a VCS run.** A human confirms or discards
|
||||
it before the next content release. Do not leave a pending run on the
|
||||
workspace.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue