From dd00deb5e248042908b14d3484c404bee29bdfcd Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Fri, 7 Aug 2026 11:48:56 +0300 Subject: [PATCH] ci(labeler): add area/crds for the CRD-bundle packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit packages/system/ holds eight charts that exist only to ship a CRD bundle. Seven of them ship nothing else; gateway-api-crds also carries upstream's safe-upgrades ValidatingAdmissionPolicy, which is why the label and the map say CRD-bundle rather than CRD-only. None of the eight is a key in .github/workflows/pr-labeler.yaml, so a PR scoped to one lands on area/uncategorized — or pushes its author to a broader scope that does map, which is the shoehorning AGENTS.md tells contributors not to do. Seven get a key here; the eighth is left out for a reason of its own, below. No existing area fits. Routing each package to its component's area — cert-manager-crds to platform, prometheus-operator-crds to monitoring — misdescribes the work these directories receive: clean the chart dir before helm pull, ship the full upstream CRD bundle, make CRD vendoring fail loudly, extract the CRDs into their own package. That is packaging, not the component, and it would send a chart-vendoring change to whoever filters monitoring. It also adds nothing, since fix(cert-manager) already resolves to area/platform. area/crds names the concern that has none: vendoring, install ordering and retention across all of them. area/* already mixes subsystems with cross-cutting concerns — build, ci, testing, release, dependencies — so this joins an existing group rather than opening a third axis. The map asked for a scope to recur 3+ times before it earned an entry. That rule is for discovering scopes from traffic; here the set is closed and enumerable, seven existing directories with no area at all. Bare `crds` covers a change spanning several of them. One key is observed and the rest are seeded, and the comment says which is which. The observed one is prometheus-operator-crds: a PR titled `fix(prometheus-operator-crds): ship full upstream CRD bundle` landed on area/uncategorized, which is exactly the failure this block ends. It is also the only one -- across every commit touching those seven directories no other directory name has been used as a scope, and the title is the only place the labeler takes a scope from. Authors reach for the component scope -- cert-manager, etcd-operator, linstor, vpa -- which has to keep meaning the component, since a change to cert-manager itself is not CRD packaging. The directory name is the one scope that names the packaging without taking the component's. `labeler` and `pr-labeler` map to area/ci. Both are observed: three PRs changing this file carried one of them in the title, and every one 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. This commit is scoped `labeler` for the same reason. Bare `crds` is the one key that is a guess rather than a directory name, and the comment says so: it has never been used as a scope either, and it is here for a change spanning several of the packages. The exposure is an author reaching for it on a CRD change that is not packaging; a differently scoped one cannot reach it, since the lookup is an exact key match. application-definition-crd is deliberately left out: its CRD is cozystack's own API surface rather than a vendored upstream bundle, its changes ship under the `api` scope, and `api` already resolves to area/api. Add hack/pr-labeler-areas.bats. What issues.addLabels does with a label the repository has not declared is not documented for that endpoint, and the test does not need it to be: either it creates one undeclared, which nothing reports, or it fails the job on every PR whose title hits that scope. The first test requires every label the workflow can apply -- area/* and kind/* alike, since both travel through that same call -- to be declared in .github/labels.yml. The second requires every CRD-bundle package to carry a scope key that maps to area/crds, so the ninth one does not land back on area/uncategorized. The labeler does warn on an unmapped scope, but only once someone opens such a PR; the test fires when the package is added, which is when it is still cheap to fix. .github/workflows/labels.yaml validates labels.yml alone, and its path filter names labels.yml and itself, so neither check runs today. Both tests evaluate the labeler rather than pattern-match it, because the pattern-matched version was defeated five ways: a double-quoted key, a space before the colon, a value built by concatenation, a key repeated lower down that wins at runtime, and an object parked where nothing consults it. Fixing those spellings one at a time had already produced the next three, so hack/pr-labeler-probe.js runs the step in node and reports the labels it reaches and the map it resolves. node and python3 become requirements of the two tests, failing loudly when absent on the precedent hack/md-no-hardwrap.bats sets for its own interpreter. All five defeats were reproduced as green before the change and red after. The two rules this departs from are amended rather than left standing, though not identically. The map now says the 3+ threshold is how a scope is discovered out of traffic rather than a bar every key must clear. The label file keeps its 3+ open issues and gains a second way in: a set of packages that share a concern and have no area at all. docs/agents/contributing.md said the labeler also runs on `edited`. That trigger was removed deliberately, so retitling an open PR does not relabel it, and the sentence promised the opposite. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- .github/labels.yml | 9 +- .github/workflows/pr-labeler.yaml | 38 +++++++- docs/agents/contributing.md | 5 +- hack/pr-labeler-areas.bats | 138 ++++++++++++++++++++++++++++++ hack/pr-labeler-probe.js | 92 ++++++++++++++++++++ 5 files changed, 276 insertions(+), 6 deletions(-) create mode 100644 hack/pr-labeler-areas.bats create mode 100644 hack/pr-labeler-probe.js 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); +});