diff --git a/.github/labels.yml b/.github/labels.yml index b74b2a7c5..6863bd73a 100644 --- a/.github/labels.yml +++ b/.github/labels.yml @@ -18,7 +18,7 @@ # priority/ urgency # triage/ review state # lifecycle/ issue or PR lifecycle -# area/ subsystem; extensible — add when 3+ open issues exist +# area/ subsystem; extensible — see the area/ block below for when to add # do-not-merge/ PR merge blockers # security/ security-finding severity and status (Cozystack-specific) # size/ PR size (auto-applied) @@ -151,7 +151,8 @@ # ────────────────────────────────────────────── # area/ — subsystem (extensible) -# Add a new area/* when there are 3+ open issues on the topic. +# Add a new area/* when there are 3+ open issues on the topic, or when a set of +# packages that share a concern has no area at all and nothing else fits. # ────────────────────────────────────────────── - name: area/api @@ -230,6 +231,10 @@ color: 'bfd4f2' description: Issues or PRs related to the tenant chart and multi-tenancy +- name: area/crds + color: 'bfd4f2' + description: Issues or PRs related to the vendored CRD-bundle packages under packages/system/*-crd(s) + - name: area/uncategorized color: 'fbca04' description: PR auto-labeler could not map title scope to a known area/*; please review diff --git a/.github/workflows/pr-labeler.yaml b/.github/workflows/pr-labeler.yaml index ff3ee7178..bc12989c9 100644 --- a/.github/workflows/pr-labeler.yaml +++ b/.github/workflows/pr-labeler.yaml @@ -34,8 +34,11 @@ jobs: // style, perf, test, build, ci, revert — no kind mapping }; - // scope -> area/* mapping. Keys are the scopes observed in cozystack issues - // and PRs. Add new entries when a scope recurs (3+ times). + // scope -> area/* mapping. Keys are mostly the scopes observed in cozystack + // issues and PRs; add one when a scope recurs (3+ times). The threshold is + // how a scope is discovered out of traffic, not a bar every key has to + // clear: a closed, enumerable set of directories may be seeded ahead of use + // instead, and a block that does that says so and says why. const scopeToArea = { // area/api 'api': 'area/api', @@ -50,6 +53,37 @@ jobs: // area/ci 'ci': 'area/ci', + // Both observed in the unit that matters here, the PR title: three PRs + // changing this very file carried one, and each landed on + // area/uncategorized. + // Not `labels`, and on evidence rather than on a rule: the sole commit + // carrying it as a scope was about Kubernetes object labels on chart + // templates, so the one observed use points somewhere else entirely. + // Mapping it would answer that use confidently wrong, which is worse + // than area/uncategorized saying out loud that it could not tell. + 'labeler': 'area/ci', + 'pr-labeler': 'area/ci', + + // area/crds — the CRD-bundle packages under packages/system/, keyed by + // directory name. One key is observed rather than seeded: a PR titled + // `fix(prometheus-operator-crds): ship full upstream CRD bundle` landed + // on area/uncategorized, which is the failure this block exists to end. + // The rest are seeded, because authors otherwise reach for the component + // scope, and that has to keep meaning the component so a change to + // cert-manager itself does not become CRD packaging. Bare `crds` has + // never been used at all and is here for a change spanning several + // packages. application-definition-crd is left out on purpose: its CRD + // is cozystack's own API surface and its changes ship under `api`. + // "CRD-bundle" rather than "CRD-only": gateway-api-crds also ships + // upstream's safe-upgrades ValidatingAdmissionPolicy and its binding. + 'crds': 'area/crds', + 'cert-manager-crds': 'area/crds', + 'etcd-operator-crds': 'area/crds', + 'gateway-api-crds': 'area/crds', + 'piraeus-operator-crds': 'area/crds', + 'prometheus-operator-crds': 'area/crds', + 'vertical-pod-autoscaler-crds': 'area/crds', + 'vsnap-crd': 'area/crds', // area/dependencies — Renovate uses scope `deps` (see .github/renovate.json) 'deps': 'area/dependencies', diff --git a/docs/agents/contributing.md b/docs/agents/contributing.md index 9f34d4495..bec351578 100644 --- a/docs/agents/contributing.md +++ b/docs/agents/contributing.md @@ -70,7 +70,7 @@ git commit --signoff -m "docs(contributing): add installation guide" ## PR Title Auto-Labeling -`.github/workflows/pr-labeler.yaml` parses the PR title on `opened`, `edited`, `reopened`, and `synchronize` events and applies labels additively (never removes). The title is expected to follow Conventional Commits — same format as commit messages above. +`.github/workflows/pr-labeler.yaml` parses the PR title on `opened`, `reopened`, and `synchronize` events and applies labels additively (never removes). `edited` is deliberately not among them, so retitling an open PR does not relabel it — push or reopen instead. The title is expected to follow Conventional Commits — same format as commit messages above. **Type → `kind/*`:** @@ -90,7 +90,8 @@ git commit --signoff -m "docs(contributing): add installation guide" | agents, ai | area/ai | | api, cozystack-api | area/api | | build | area/build | -| ci | area/ci | +| ci, labeler, pr-labeler | area/ci | +| crds, cert-manager-crds, etcd-operator-crds, gateway-api-crds, ... | area/crds | | dashboard | area/dashboard | | postgres, mariadb, redis, etcd, kafka, clickhouse, postgres-operator, mariadb-operator | area/database | | extra | area/extra | diff --git a/hack/pr-labeler-areas.bats b/hack/pr-labeler-areas.bats new file mode 100644 index 000000000..f92e18feb --- /dev/null +++ b/hack/pr-labeler-areas.bats @@ -0,0 +1,138 @@ +#!/usr/bin/env bats +# ----------------------------------------------------------------------------- +# Consistency between .github/workflows/pr-labeler.yaml and .github/labels.yml. +# +# The labeler applies its result with issues.addLabels. What that call does with +# a label the repository has not declared is not documented for this endpoint, +# and the test does not need it to be: either it creates one with a default +# colour and no description, and the failure is a stray label nobody declared +# that nothing reports, or it errors and the job goes red on every PR whose +# title happens to hit that scope. The first is silent and the second is loud, +# both are bad, and both are what this test prevents. +# .github/workflows/labels.yaml validates labels.yml on its own but never reads +# the workflow, and its path filter names labels.yml and itself, so an edit to +# the map alone does not even run it. +# +# The second test guards the other direction for CRD-bundle packages: a new one +# added without a scope key falls back to area/uncategorized. The labeler does +# emit a core.warning naming the unmapped scope, so the fallback is not silent +# -- but it only fires once someone opens such a PR, whereas this test fires +# when the package is added, which is when it is still cheap to fix. +# +# The labeler is evaluated rather than pattern-matched. A pattern over the +# workflow source answers "is this spelling present", and the same break spelled +# differently walks past it — a double-quoted key, a space before the colon, a +# value built by concatenation, a key repeated lower down that wins at runtime, +# an object parked where nothing consults it. All five were demonstrated against +# a pattern-based version of these tests. hack/pr-labeler-probe.js runs the step +# in node and reports what it would apply, which removes the class rather than +# the instances. +# +# node and python3 are therefore required, and a missing one fails loudly rather +# than skipping, on the precedent hack/md-no-hardwrap.bats sets for its own +# interpreter: a green suite that ran nothing is worse than a red one. +# +# cozytest.sh's awk parser recognizes @test blocks and rewrites a bare `}` on its +# own line; every other line it emits verbatim, so file-scope helpers work while +# bats `setup`/`teardown` are never called. There is no `run` or `$status`. +# Assertions are direct shell tests, and one that must hold when a command +# SUCCEEDS is written as `if ... exit 1` rather than `! cmd`: errexit is +# suppressed for a `!`-inverted command, for an `if`/`while` condition, and for a +# command that is not the LAST in an `&&`/`||` list — the last one fires +# normally, so `true && false` does abort. +# +# Run with: hack/cozytest.sh hack/pr-labeler-areas.bats +# ----------------------------------------------------------------------------- + +have_node() { + command -v node >/dev/null 2>&1 && return 0 + echo "node is required to evaluate .github/workflows/pr-labeler.yaml" >&2 + return 1 +} + +have_python3() { + command -v python3 >/dev/null 2>&1 && return 0 + echo "python3 is required to read .github/workflows/pr-labeler.yaml" >&2 + return 1 +} + +# Extract the github-script body, evaluate it, leave the JSON report in $1. +labeler_report() { + python3 -c 'import yaml;w=yaml.safe_load(open(".github/workflows/pr-labeler.yaml"));print([st["with"]["script"] for j in w["jobs"].values() for st in j["steps"] if "script" in st.get("with",{})][0])' >"$1/labeler.js" + node hack/pr-labeler-probe.js "$1/labeler.js" >"$1/report.json" +} + +@test "every label the PR labeler can apply is declared in .github/labels.yml" { + have_node + have_python3 + WORK=$(mktemp -d) + labeler_report "$WORK" + + # Every declared label, and every label the step actually reaches. No + # namespace filter on either side: kind/* travels through the same addLabels + # call as area/*, and so would anything else somebody adds. + declared=$(grep -E '^- name: ' .github/labels.yml | sed 's/^- name: //' | tr -d "'\"" | sort -u) + used=$(sed -n '/"labels":/,/\]/p' "$WORK/report.json" | sed -nE 's/^ *"([^"]+)",?$/\1/p' | sort -u) + + # Neither side may be empty: an extraction that silently stops matching + # would turn this test into a vacuous pass. + if [ -z "$declared" ] || [ -z "$used" ]; then + echo "extraction produced nothing — declared=[$declared] used=[$used]" >&2 + exit 1 + fi + + missing="" + for a in $used; do + printf '%s\n' "$declared" | grep -qx "$a" || missing="$missing $a" + done + + if [ -n "$missing" ]; then + echo "pr-labeler.yaml can apply labels that labels.yml does not declare:$missing" >&2 + echo "addLabels either creates each one undeclared or fails the job" >&2 + exit 1 + fi + + rm -rf "$WORK" +} + +@test "every CRD-bundle package under packages/system has a scope in the labeler map" { + # application-definition-crd is deliberately absent from the map. Its CRD is + # cozystack's own API surface rather than a vendored upstream bundle, its + # changes ship under the `api` scope, and `api` already maps to area/api. + # Grouping it by packaging shape would cost accuracy for uniformity. + have_node + have_python3 + WORK=$(mktemp -d) + labeler_report "$WORK" + + found=0 + missing="" + + for d in packages/system/*-crd packages/system/*-crds; do + [ -d "$d" ] || continue + n=$(basename "$d") + case "$n" in + application-definition-crd) continue ;; + esac + found=$((found + 1)) + # The value the scope RESOLVES to, so a key that is present but + # unreachable does not count as mapped: parked in an object nothing + # consults, commented out, or overridden by a duplicate lower down. + grep -qE "^ *\"$n\": \"area/crds\",?\$" "$WORK/report.json" || missing="$missing $n" + done + + if [ "$found" -eq 0 ]; then + echo "no *-crd/*-crds packages matched under packages/system — glob broke" >&2 + exit 1 + fi + + if [ -n "$missing" ]; then + echo "CRD-bundle packages whose scope does not resolve to area/crds:$missing" >&2 + echo "a PR scoped to one of them falls back to area/uncategorized." >&2 + echo "Add the key, or -- if the package ships cozystack's own API rather" >&2 + echo "than a vendored bundle -- add it to the skip list above instead." >&2 + exit 1 + fi + + rm -rf "$WORK" +} diff --git a/hack/pr-labeler-probe.js b/hack/pr-labeler-probe.js new file mode 100644 index 000000000..1c715fd2e --- /dev/null +++ b/hack/pr-labeler-probe.js @@ -0,0 +1,92 @@ +// Evaluates the github-script body of .github/workflows/pr-labeler.yaml and +// reports what it would actually do, as JSON on stdout: +// +// {"labels": [...], "scopeToArea": {...}, "typeToKind": {...}} +// +// `labels` is the union of everything handed to issues.addLabels across a title +// per (type, scope) pair plus the breaking-change and backport paths, so it is +// the set the labeler can apply rather than the set spelled out as literals. +// +// This exists because a pattern over the workflow source answers "is this +// spelling present", and the same break spelled differently walks past it: a +// double-quoted key, a space before the colon, a value built by concatenation, +// a key repeated lower down that wins at runtime, an object parked where +// nothing consults it. Each of those was demonstrated against a regex-based +// version of the tests. Asking the engine that runs in CI removes the class +// rather than the instances. +// +// Usage: node hack/pr-labeler-probe.js + +'use strict'; + +const fs = require('fs'); +const vm = require('vm'); + +const src = fs.readFileSync(process.argv[2], 'utf8'); +const applied = new Set(); + +// The step is a bare script that github-script wraps in an async function, so +// it is run the same way here. The epilogue publishes the lookup tables; it is +// appended rather than parsed out, so no assumption about their spelling or +// position survives into this file. +const wrapped = '(async () => {' + src + '\n;globalThis.__tables = {scopeToArea, typeToKind};})()'; + +async function run(title, body, existing) { + const ctx = { + context: { + payload: { + pull_request: { + title: title, + body: body || '', + labels: (existing || []).map(function (n) { return {name: n}; }), + number: 1, + }, + }, + repo: {owner: 'owner', repo: 'repo'}, + }, + core: {warning: function () {}, info: function () {}, notice: function () {}, setFailed: function () {}}, + github: { + rest: { + issues: { + addLabels: async function (args) { + (args.labels || []).forEach(function (l) { applied.add(l); }); + }, + }, + }, + }, + console: {log: function () {}, error: function () {}}, + }; + ctx.globalThis = ctx; + vm.createContext(ctx); + await vm.runInContext(wrapped, ctx); + return ctx.__tables; +} + +async function main() { + const tables = await run('fix(api): probe'); + const scopes = Object.keys(tables.scopeToArea); + const types = Object.keys(tables.typeToKind); + + for (const t of types) { + await run(t + ': probe'); + for (const s of scopes) { + await run(t + '(' + s + '): probe'); + } + } + await run('fix(api)!: probe'); + await run('fix(api): probe', 'BREAKING CHANGE: probe'); + await run('[Backport release-1.0] fix(api): probe'); + // A scope that is deliberately absent, so the fallback label is reachable too. + await run('fix(no-such-scope-probe): probe'); + + process.stdout.write(JSON.stringify({ + labels: Array.from(applied).sort(), + scopeToArea: tables.scopeToArea, + typeToKind: tables.typeToKind, + }, null, 1) + '\n'); +} + +main().catch(function (e) { + process.stderr.write('probe failed: ' + (e && e.stack ? e.stack : e) + '\n'); + process.exit(1); +});