Skip to content

Commit 9aa045f

Browse files
lexfreilexfrei
authored
feat(ci): gate sizeable API changes behind API-owner review (#3167)
## What this PR does Adds a CI gate that requires an approving review from a designated API owner (`lllamnyp` or `kvaps`) when a pull request makes a **sizeable** change to the Cozystack API surface. Sizeable means one of: 1. A new API group. 2. A new resource. 3. A breaking change to an existing resource's schema. 4. Removal of a resource or an entire API group. Detection is fully deterministic — no LLM or network calls, so it runs on every PR. ### How it works - **`cmd/api-gate` + `internal/apigate`** — a small Go tool that loads the API surface from two checkouts (merge base and PR head) and classifies the delta. Every resource's schema is normalized to a parsed OpenAPIv3Schema; the tool only unwraps it from its two container formats and reads identity (group/kind/plural) from the wrapper. | Surface | Files scanned | Schema source | | --- | --- | --- | | Apps (`apps.cozystack.io`) | `packages/system/*-rd/cozyrds/*.yaml` | `spec.application.openAPISchema` | | Typed groups (`cozystack.io`, `backups.*`, `network.*`, `gateway.*`) | every `*.cozystack.io` CRD discovered under a `crds/` or `definitions/` directory in `packages/` and `internal/` | served `spec.versions[].schema.openAPIV3Schema` | | Static Go-backed (`core.*`, `sdn.*`) | `pkg/apiserver/apiserver.go` storage registrations | none — new/removed group / resource only | CRDs are found by location + content rather than a fixed file list, so a first-party CRD that moves (e.g. into a chart's `crds/` dir) stays covered. Only `served` CRD versions count as live API surface, so the standard deprecation step (`served: true → false`) registers as a removed version. An empty snapshot is treated as an error, never a silent pass. - **Breaking classification is semantic, not textual.** Additive changes (new optional field, new enum value, widened type, relaxed or removed constraint, description/default edits) pass. Flagged as breaking: removed fields, type narrowing (including adding a type to a previously-unconstrained field), enum-value removal, newly-required fields, added/tightened `pattern`/min/max/length constraints, `additionalProperties` restricted to `false`, added `x-kubernetes-validations` (CEL) rules, and removed served versions. - **`.github/workflows/api-review-gate.yaml`** builds the tool, diffs the PR head against its true **merge base**, and — when the change is sizeable — requires an `APPROVED` review from an API owner. Only each reviewer's *latest* review *on the current head commit* counts, so a stale or superseded approval no longer satisfies the gate. It re-runs on `pull_request_review` so an approval flips the check without a new push. The token is scoped to the approval step alone (`persist-credentials: false` on checkout) so PR-controlled build steps can't read it. The allowlist lives in one place (`API_OWNERS`). The job runs on every PR (no path filter) so it can safely be marked a required check. ### Follow-up needed after merge Mark **"Require API owner review for sizeable API changes"** as a required status check in `main` branch protection for the gate to block merges. ### Tests Table-driven unit tests cover the schema-diff rules, the classifier (new group / new resource / breaking, including the double-count and pure-removal edge cases), all three parsers, and an on-disk end-to-end snapshot test. Verified against the real repo tree: identical trees pass; injected new-group/new-resource/breaking changes are all caught; an additive-only edit passes. ### Release note ```release-note feat(ci): require a review from an API owner (lllamnyp or kvaps) when a PR introduces a new API group, a new resource, or a breaking change to an existing API schema ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an API review gate workflow that flags “sizeable” API changes and requires at least one approving API-owner review. * Introduced an API-gate CLI to compare API snapshots, classify changes (new/removed resources and groups, plus breaking schema updates), and generate readable reports. * Enhanced snapshot parsing and deterministic reporting across multiple input sources. * **Tests** * Added comprehensive unit tests covering parsing, snapshot loading, schema diff detection, and classification/report output behavior. * **Chores** * Updated API gate ownership rules for the new workflow and related tooling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 9e89ae8 + dc61566 commit 9aa045f

12 files changed

Lines changed: 2069 additions & 0 deletions

File tree

‎.github/CODEOWNERS‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,11 @@
11
* @kvaps @lllamnyp @lexfrei @androndo @IvanHunters @sircthulhu @myasnikovdaniil
2+
3+
# The API review gate must not be editable without an API-owner review, or a
4+
# PR could neuter the detector (it is compiled from the head checkout during
5+
# bootstrap) and slip a sizeable API change through unreviewed. Scoping these
6+
# paths to the API owners overrides the catch-all above (last match wins), so
7+
# changes here require @kvaps or @lllamnyp specifically. Enable "Require review
8+
# from Code Owners" in branch protection for this to bind.
9+
/cmd/api-gate/ @kvaps @lllamnyp
10+
/internal/apigate/ @kvaps @lllamnyp
11+
/.github/workflows/api-review-gate.yaml @kvaps @lllamnyp

‎.github/workflows/api-review-gate.yaml‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
name: API Review Gate
2+
3+
# Requires a review from a designated API owner (see API_OWNERS below) when a
4+
# pull request makes a "sizeable" change to the Cozystack API surface: a new
5+
# API group, a new resource, or a breaking change to an existing resource's
6+
# schema. Detection is deterministic (cmd/api-gate); no LLM or network calls.
7+
#
8+
# The job runs on every PR (no path filter) so it can always report a status
9+
# and therefore be marked as a required check in branch protection without
10+
# leaving PRs that touch no API stuck in "pending". When nothing sizeable
11+
# changed it passes immediately.
12+
13+
on:
14+
pull_request:
15+
types: [opened, synchronize, reopened]
16+
# Re-evaluate when a review is submitted or dismissed so an approval by an
17+
# API owner flips the check from failing to passing without a new push.
18+
pull_request_review:
19+
types: [submitted, dismissed]
20+
21+
concurrency:
22+
group: api-review-gate-${{ github.event.pull_request.number }}
23+
cancel-in-progress: true
24+
25+
permissions:
26+
contents: read
27+
pull-requests: read
28+
29+
jobs:
30+
api-review-gate:
31+
name: Require API owner review for sizeable API changes
32+
runs-on: ubuntu-22.04
33+
env:
34+
# Space-separated GitHub logins allowed to satisfy the gate. Edit here to
35+
# change who can approve sizeable API changes.
36+
API_OWNERS: "lllamnyp kvaps"
37+
BASE_REF: ${{ github.event.pull_request.base.ref }}
38+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
39+
PR_NUMBER: ${{ github.event.pull_request.number }}
40+
steps:
41+
- name: Checkout head
42+
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
43+
with:
44+
ref: ${{ github.event.pull_request.head.sha }}
45+
fetch-depth: 0
46+
# Do not leave a usable token in the repo's git config: the PR-authored
47+
# build and api-gate steps below must not have access to it. The token
48+
# is scoped to the approval step alone, which is the only step that
49+
# calls the GitHub API.
50+
persist-credentials: false
51+
52+
- name: Set up Go
53+
uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5
54+
with:
55+
go-version-file: go.mod
56+
cache: true
57+
58+
- name: Materialize base checkout at the merge base
59+
run: |
60+
set -euo pipefail
61+
# Diff against the PR's actual merge base, not the base branch tip:
62+
# otherwise unrelated commits landing on the base branch while the PR
63+
# is open would invent or hide API findings.
64+
git fetch --no-tags origin "${BASE_REF}"
65+
merge_base="$(git merge-base "${HEAD_SHA}" "origin/${BASE_REF}")"
66+
echo "Merge base: ${merge_base}"
67+
git worktree add --detach "${RUNNER_TEMP}/base" "${merge_base}"
68+
69+
- name: Build api-gate
70+
run: |
71+
set -euo pipefail
72+
# Prefer building the detector from the trusted base checkout so a PR
73+
# cannot neuter the gate by editing its own detector code — the tool
74+
# analyzes the head as *data*, not as code. Fall back to the head
75+
# checkout only while the tool has not yet landed on the base branch
76+
# (bootstrap); in that window CODEOWNERS on cmd/api-gate,
77+
# internal/apigate, and this workflow keeps tampering behind an
78+
# API-owner review.
79+
if [ -d "${RUNNER_TEMP}/base/cmd/api-gate" ]; then
80+
build_dir="${RUNNER_TEMP}/base"
81+
echo "Building api-gate from the trusted base checkout."
82+
else
83+
build_dir="${GITHUB_WORKSPACE}"
84+
echo "::warning::api-gate not present on the base branch yet; building from head (bootstrap). CODEOWNERS enforces API-owner review on the gate's own files."
85+
fi
86+
( cd "${build_dir}" && go build -o "${RUNNER_TEMP}/api-gate" ./cmd/api-gate )
87+
88+
- name: Detect sizeable API changes
89+
id: detect
90+
run: |
91+
set -uo pipefail
92+
set +e
93+
"${RUNNER_TEMP}/api-gate" \
94+
--base "${RUNNER_TEMP}/base" \
95+
--head "${GITHUB_WORKSPACE}" \
96+
--format markdown > "${RUNNER_TEMP}/report.md"
97+
code=$?
98+
set -e
99+
echo "code=${code}" >> "$GITHUB_OUTPUT"
100+
cat "${RUNNER_TEMP}/report.md"
101+
case "${code}" in
102+
0) echo "sizeable=false" >> "$GITHUB_OUTPUT" ;;
103+
2) echo "sizeable=true" >> "$GITHUB_OUTPUT" ;;
104+
*) echo "::error::api-gate failed with exit code ${code}"; exit "${code}" ;;
105+
esac
106+
107+
- name: Check for an API owner approval
108+
if: steps.detect.outputs.sizeable == 'true'
109+
env:
110+
GH_TOKEN: ${{ github.token }}
111+
run: |
112+
set -euo pipefail
113+
{
114+
echo "## API Review Gate"
115+
echo
116+
cat "${RUNNER_TEMP}/report.md"
117+
} >> "$GITHUB_STEP_SUMMARY"
118+
119+
# An APPROVED review by any API owner satisfies the gate, but only
120+
# each reviewer's *latest* review *on the current head commit* counts:
121+
# this drops approvals that a later commit invalidated, and approvals
122+
# an owner subsequently superseded with CHANGES_REQUESTED. GitHub
123+
# forbids self-approval, so a sizeable PR authored by an owner still
124+
# needs the other owner.
125+
approvals="$(
126+
gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate |
127+
jq -rs --arg head "${HEAD_SHA}" '
128+
add
129+
| map(select(.commit_id == $head))
130+
| sort_by(.submitted_at // "")
131+
| group_by(.user.login)
132+
| map(last)
133+
| .[]
134+
| select(.state == "APPROVED")
135+
| .user.login
136+
'
137+
)"
138+
139+
for owner in ${API_OWNERS}; do
140+
if grep -qxi "${owner}" <<<"${approvals}"; then
141+
echo "Sizeable API change approved by @${owner}."
142+
exit 0
143+
fi
144+
done
145+
146+
echo "::error::This PR makes a sizeable API change (new group, new resource, or breaking change) and needs an approving review from one of: ${API_OWNERS}. See the job summary for details."
147+
exit 1

‎cmd/api-gate/main.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
Copyright 2026 The Cozystack Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Command api-gate compares the Cozystack API surface between two checkouts of
18+
// the repository and reports whether the change is "sizeable" — a new API
19+
// group, a new resource, or a breaking change to an existing resource. CI uses
20+
// a sizeable verdict to require a review from a designated API owner.
21+
//
22+
// Usage:
23+
//
24+
// api-gate --base <dir> --head <dir>
25+
//
26+
// Both flags point at a full repository checkout (the merge base and the PR
27+
// head). Exit code 0 means "not sizeable"; exit code 2 means "sizeable"
28+
// (findings are printed to stdout as a Markdown report); any other non-zero
29+
// exit is an operational error.
30+
package main
31+
32+
import (
33+
"flag"
34+
"fmt"
35+
"os"
36+
37+
"github.com/cozystack/cozystack/internal/apigate"
38+
)
39+
40+
// exitSizeable is a distinct, non-error exit code so the calling workflow can
41+
// tell "sizeable change, needs owner review" apart from a tool malfunction.
42+
const exitSizeable = 2
43+
44+
func main() {
45+
baseDir := flag.String("base", "", "path to the base (merge-base) repository checkout")
46+
headDir := flag.String("head", "", "path to the head (PR) repository checkout")
47+
format := flag.String("format", "markdown", "report format: markdown or text")
48+
flag.Parse()
49+
50+
if *baseDir == "" || *headDir == "" {
51+
fmt.Fprintln(os.Stderr, "api-gate: both --base and --head are required")
52+
flag.Usage()
53+
os.Exit(1)
54+
}
55+
56+
base, err := apigate.LoadSnapshot(*baseDir)
57+
if err != nil {
58+
fmt.Fprintf(os.Stderr, "api-gate: loading base snapshot: %v\n", err)
59+
os.Exit(1)
60+
}
61+
head, err := apigate.LoadSnapshot(*headDir)
62+
if err != nil {
63+
fmt.Fprintf(os.Stderr, "api-gate: loading head snapshot: %v\n", err)
64+
os.Exit(1)
65+
}
66+
67+
findings := apigate.Classify(base, head)
68+
if len(findings) == 0 {
69+
fmt.Println("No sizeable API changes detected.")
70+
return
71+
}
72+
73+
fmt.Print(apigate.Report(findings, *format))
74+
os.Exit(exitSizeable)
75+
}

0 commit comments

Comments
 (0)