diff --git a/.github/workflows/compat.yml b/.github/workflows/compat.yml index fd79d1c..2a93ba4 100644 --- a/.github/workflows/compat.yml +++ b/.github/workflows/compat.yml @@ -130,17 +130,19 @@ jobs: - name: matrix run: cat /tmp/matrix.md - # Keep the README's table true. A support matrix that quietly goes stale is not - # a stale doc -- it is a false promise to somebody deciding whether to trust - # this with their backups. + # Keep the README's table true — but as a PULL REQUEST, not a push to main. # - # Only ever touches the block between the COMPAT MATRIX markers, and only on - # the canonical host (Gitea) so the two forges cannot race each other. The - # `paths:` trigger above does not include README.md, so this cannot re-trigger - # itself; and a README change is documentation-only, which by design raises no - # update alert on anyone's box. - - name: refresh the README matrix - if: ${{ github.event_name == 'schedule' && !contains(github.server_url, 'github.com') }} + # This used to `git push origin HEAD:main` from CI. Unattended writes to main are + # exactly what the release barrier exists to prevent; a bot that can move main can + # move it somewhere nobody looked. A PR is reviewable, and nothing lands by itself. + # + # A stale support matrix is not a stale doc — it is a false promise to somebody + # deciding whether to trust this with their backups. So it still gets refreshed + # daily; it just asks first. + - name: refresh the README matrix (as a PR) + if: ${{ github.event_name == 'schedule' && contains(github.server_url, 'github.com') }} + env: + GH_TOKEN: ${{ github.token }} run: | python3 - <<'PY' import json, sys @@ -151,87 +153,46 @@ jobs: print("changed" if compat.update_readme(rows) else "unchanged") PY - if ! git diff --quiet -- README.md; then - git config user.name "truecloud-patch bot" - git config user.email "bot@onetick.ninja" - git add README.md - git commit -m "docs: refresh the TrueNAS compatibility matrix" - git push origin HEAD:main + git diff --quiet -- README.md && { echo "matrix unchanged"; exit 0; } + + git config user.name "truecloud-patch bot" + git config user.email "bot@onetick.ninja" + + branch="bot/compat-matrix" + git checkout -B "$branch" + git add README.md + git commit -m "docs: refresh the TrueNAS compatibility matrix" + git push -f origin "$branch" + + # One long-lived PR, updated in place — not a new one every morning. + if ! gh pr view "$branch" --json number >/dev/null 2>&1; then + gh pr create --head "$branch" --base main \ + --title "docs: refresh the TrueNAS compatibility matrix" \ + --body "The daily compatibility check found that the support matrix in the README no longer matches what iXsystems' middleware actually looks like. + + This PR only touches the block between the \`COMPAT MATRIX\` markers. It is regenerated by \`tools/compat.py --matrix --update-readme\` and force-pushed, so it always reflects the latest run." fi - # A broken SHIPPED release is an outage: users are on it right now. + # ONE bug report, kept in sync. It is edited in place when the findings change and + # says NOTHING when they do not. + # + # The first version commented on every run and left 11 identical 3,000-character + # comments on one issue in a single day. A bot that repeats itself daily gets + # muted, and then the next real finding is scrolled past — which defeats the whole + # reason for building it. + # + # Runs on whichever forge it lands on; compat_publish.py handles both, so the two + # cannot drift. + - name: file / update / close the bug report + env: + TOKEN: ${{ secrets.GITEA_TOKEN || github.token }} + API: ${{ contains(github.server_url, 'github.com') && 'https://api.github.com' || format('{0}/api/v1', github.server_url) }}/repos/${{ github.repository }} + run: python3 tools/compat_publish.py --api "$API" --token "$TOKEN" --matrix /tmp/matrix.json + + # A broken SHIPPED release is an outage: users are on it right now. Fails LAST, so + # the report is filed before the job goes red. - name: fail if a shipped release is broken if: ${{ steps.check.outputs.shipped_broken != '0' }} run: | echo "::error::The patch is broken on a SHIPPED TrueNAS release." exit 1 - - - name: file a bug report (GitHub) - if: ${{ steps.report.outputs.broken == '1' && contains(github.server_url, 'github.com') }} - env: - GH_TOKEN: ${{ github.token }} - TITLE: "TrueNAS compatibility: the patch's assumptions no longer hold" - run: | - # One issue per set of broken refs, reopened/updated rather than duplicated - # daily -- a bot that files the same issue every morning gets muted, and - # then it is not a warning system any more. - # Lowest-numbered match, for the same reason as the Gitea step below. - existing="$(gh issue list --state all --search "$TITLE" \ - --json number,title \ - --jq '[.[] | select(.title == env.TITLE) | .number] | min // empty')" - - if [ -n "$existing" ]; then - gh issue comment "$existing" --body-file /tmp/issue.md - gh issue reopen "$existing" 2>/dev/null || true - else - gh issue create --title "$TITLE" --body-file /tmp/issue.md - fi - - - name: file a bug report (Gitea) - if: ${{ steps.report.outputs.broken == '1' && !contains(github.server_url, 'github.com') }} - env: - TOKEN: ${{ secrets.GITEA_TOKEN || github.token }} - API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }} - TITLE: "TrueNAS compatibility: the patch's assumptions no longer hold" - run: | - # python3, not jq: jq is not guaranteed on a self-hosted runner, and a bug - # report that dies on a missing tool is a warning system that does not warn. - python3 - <<'PY' - import json, os, urllib.error, urllib.request - - api, token, title = os.environ["API"], os.environ["TOKEN"], os.environ["TITLE"] - with open("/tmp/issue.md", encoding="utf-8") as fh: - body = fh.read() - headers = {"Authorization": f"token {token}", - "Content-Type": "application/json"} - - def call(url, method, data=None): - req = urllib.request.Request( - url, method=method, headers=headers, - data=json.dumps(data).encode() if data else None) - with urllib.request.urlopen(req) as r: # noqa: S310 - return json.load(r) if r.length != 0 else {} - - # Same title => same issue. Comment on it rather than filing a new one every - # morning: a bot that duplicates itself daily gets muted, and then it is not - # a warning system any more. - # LOWEST-numbered match, not "whichever the API returns first". Two issues - # with the same title already existed once (the old title embedded the ref - # list, so the identity changed when that set changed), and an - # order-dependent pick would have alternated between them, reopening one and - # commenting on the other. Lowest number is stable no matter what the API - # sorts by. - issues = call(f"{api}/issues?state=all&type=issues", "GET") - matches = sorted((i for i in issues if i["title"] == title), - key=lambda i: i["number"]) - match = matches[0] if matches else None - - if match: - n = match["number"] - call(f"{api}/issues/{n}/comments", "POST", {"body": body}) - call(f"{api}/issues/{n}", "PATCH", {"state": "open"}) - print(f"commented on and reopened issue #{n}") - else: - made = call(f"{api}/issues", "POST", {"title": title, "body": body}) - print(f"filed issue #{made['number']}") - PY diff --git a/tests/test_workflows.py b/tests/test_workflows.py index e015359..e760798 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -10,7 +10,8 @@ import re import pytest -WORKFLOWS = os.path.join(os.path.dirname(__file__), "..", ".github", "workflows") +ROOT = os.path.join(os.path.dirname(__file__), "..") +WORKFLOWS = os.path.join(ROOT, ".github", "workflows") def workflow_files(): @@ -84,11 +85,53 @@ class TestBothForges: assert "if: ${{ contains(github.server_url, 'github.com') }}" in src assert "if: ${{ !contains(github.server_url, 'github.com') }}" in src - def test_compat_files_an_issue_on_each_forge(self): + def test_compat_files_its_report_through_ONE_implementation(self): + # It used to be two near-identical shell steps, one per forge. Two copies of + # "find the issue, decide whether to comment, post it" is two chances to drift, + # and the Gitea one duplicated an issue for real. with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh: src = fh.read() - assert "file a bug report (GitHub)" in src - assert "file a bug report (Gitea)" in src + assert "tools/compat_publish.py" in src + assert "file a bug report (GitHub)" not in src + assert "file a bug report (Gitea)" not in src + + +class TestTheBotDoesNotSpam: + """It left 11 identical 3,000-character comments on one issue in a single day. + + A bot that repeats itself daily gets muted — and then the next REAL finding is + scrolled past, which defeats the entire reason for building it. + """ + + def publisher(self): + with open(os.path.join(ROOT, "tools", "compat_publish.py"), encoding="utf-8") as fh: + return fh.read() + + def test_it_compares_a_fingerprint_before_saying_anything(self): + src = self.publisher() + assert "extract_fingerprint" in src + assert "staying quiet" in src + + def test_the_body_is_edited_in_place_not_appended_to(self): + src = self.publisher() + assert '"PATCH"' in src, "the issue body must be updated, not commented onto" + + def test_it_closes_the_issue_when_everything_is_fixed(self): + src = self.publisher() + assert '"state": "closed"' in src + + def test_the_matrix_refresh_opens_a_PR_rather_than_pushing_to_main(self): + # An unattended push to main from CI is exactly what the release barrier exists + # to prevent: a bot that can move main can move it somewhere nobody looked. + # + # Checked against CODE, not comments — the step's own commentary explains what + # it replaced, and that mention must not read as the thing itself. + with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh: + code = "\n".join( + ln for ln in fh.read().splitlines() if not ln.lstrip().startswith("#") + ) + assert "gh pr create" in code + assert "HEAD:main" not in code, "CI still pushes straight to main" class TestCompatCannotSilentlyPass: diff --git a/tools/compat.py b/tools/compat.py index ccf616c..f190c0b 100644 --- a/tools/compat.py +++ b/tools/compat.py @@ -43,6 +43,7 @@ from __future__ import annotations import argparse import ast +import hashlib import json import os import sys @@ -892,6 +893,90 @@ def render_markdown(rows: list[dict]) -> str: return "\n".join(out) + "\n" + _LEGEND +FINGERPRINT = "", 1)[0].strip() or None + + +def render_issue(rows: list[dict]) -> str: + """The bug report body: what is broken, why it matters, and nothing else up front. + + Short by design. The full matrix and the healthy versions go in a fold -- somebody + opening this wants to know what broke and whether it can hurt them, not to re-read + a table they can see in the README. + """ + broken = [r for r in rows if any(is_broken(m) for m in r["modules"].values())] + + out = [ + "`tools/compat.py` checks what this patch assumes about middlewared against " + "iXsystems' actual source, every day. Those assumptions no longer hold on the " + "versions below.", + "", + "**This does not break anyone today.** `apply.sh` re-checks on every boot and " + "**declines to apply** a module whose assumptions fail, so TrueNAS is left " + "stock rather than half-patched. The cost is the module's feature, not a " + "broken backup.", + "", + ] + + for r in broken: + out.append(f"### `{r['ref']}`") + out.append("") + for mod, m in sorted(r["modules"].items()): + if not is_broken(m): + continue + out.append(f"**{mod}**") + out.append("") + for p in m["problems"]: + out.append(f"- {p['detail']}") + out.append(f"
{p['why']}") + out.append("") + + out += [ + "
Full support matrix", + "", + render_markdown(rows), + "
", + "", + "_Filed and kept up to date by " + "[`compat.yml`](.github/workflows/compat.yml). It edits this body when the " + "findings change, and stays quiet when they do not._", + "", + f"{FINGERPRINT} {fingerprint(rows)} -->", + ] + return "\n".join(out) + + def render_matrix(rows: list[dict]) -> str: """A support table. diff --git a/tools/compat_publish.py b/tools/compat_publish.py new file mode 100644 index 0000000..441f2f7 --- /dev/null +++ b/tools/compat_publish.py @@ -0,0 +1,148 @@ +#!/usr/bin/env python3 +"""Keep ONE bug report in sync with what compat.py currently finds. + +WHY THIS IS NOT JUST "POST A COMMENT" +------------------------------------- +The first version commented on every run that found a break. In one day it left +**11 identical 3,000-character comments** on the same issue. That is not a warning +system; it is a mute button with extra steps. The next real finding would have been +scrolled past, which defeats the entire point of building it. + +So: + + * **The issue body is the current truth.** It is edited in place, never appended to. + * **Comments are a changelog of CHANGES.** A run whose findings are identical to the + last one says nothing at all -- no comment, no edit, no notification. + * A fingerprint of the findings (broken ref/module/problem triples only) is embedded + in the body. It deliberately ignores things that move on their own -- healthy rows, + the hardware-verified column, TrueNAS point releases -- so `TS-25.10.4` becoming + `TS-25.10.5` is not news, and does not wake anybody up. + + * When everything is fixed, the issue is **closed** with a comment saying so. + +Works against GitHub and Gitea, which differ only in the auth header and the issue +list URL. One implementation, so the two cannot drift. + + python3 tools/compat_publish.py --api --token --matrix /tmp/matrix.json +""" + +from __future__ import annotations + +import argparse +import json +import sys +import urllib.error +import urllib.request + +sys.path.insert(0, __file__.rsplit("/", 1)[0]) + +from compat import ( # noqa: E402 + extract_fingerprint, + fingerprint, + is_broken, + render_issue, +) + +TITLE = "TrueNAS compatibility: the patch's assumptions no longer hold" + + +def _call(url, token, method="GET", data=None): + req = urllib.request.Request( + url, method=method, + headers={ + # Gitea wants `token `; GitHub accepts `Bearer `. GitHub also + # accepts `token `, so one header serves both. + "Authorization": f"token {token}", + "Content-Type": "application/json", + "Accept": "application/vnd.github+json", + }, + data=json.dumps(data).encode() if data else None, + ) + with urllib.request.urlopen(req) as r: # noqa: S310 + return json.load(r) if r.length != 0 else {} + + +def find_issue(api, token, title): + """The LOWEST-numbered issue with this title, open or closed. + + Lowest, not "whichever the API returns first": two issues with the same title + existed once (an earlier version put the ref list in the title, so the identity + changed whenever that set changed), and an order-dependent pick would alternate + between them -- reopening one while commenting on the other. + """ + issues = _call(f"{api}/issues?state=all&per_page=100", token) + mine = [ + i for i in issues + if i.get("title") == title and "pull_request" not in i # GitHub lists PRs here + ] + return min(mine, key=lambda i: i["number"]) if mine else None + + +def main(argv): + ap = argparse.ArgumentParser(description=__doc__.split("\n")[0]) + ap.add_argument("--api", required=True, help="…/repos//") + ap.add_argument("--token", required=True) + ap.add_argument("--matrix", required=True, help="compat.py --matrix --json output") + args = ap.parse_args(argv[1:]) + + with open(args.matrix, encoding="utf-8") as fh: + rows = json.load(fh) + + broken = [r for r in rows if any(is_broken(m) for m in r["modules"].values())] + issue = find_issue(args.api, args.token, TITLE) + + # ── everything is healthy ──────────────────────────────────────────────── + if not broken: + if issue and issue["state"] == "open": + _call(f"{args.api}/issues/{issue['number']}/comments", args.token, "POST", + {"body": "All of the patch's assumptions hold again on every " + "checked TrueNAS version. Closing."}) + _call(f"{args.api}/issues/{issue['number']}", args.token, "PATCH", + {"state": "closed"}) + print(f"closed #{issue['number']} — nothing is broken any more") + else: + print("nothing broken; no open report to close") + return 0 + + body = render_issue(rows) + want = fingerprint(rows) + + # ── nothing to file yet ────────────────────────────────────────────────── + if issue is None: + made = _call(f"{args.api}/issues", args.token, "POST", + {"title": TITLE, "body": body}) + print(f"filed #{made['number']}") + return 0 + + have = extract_fingerprint(issue.get("body") or "") + n = issue["number"] + + # ── the findings are UNCHANGED: say nothing ────────────────────────────── + # + # This is the whole point. A daily "still broken, same as yesterday" comment is + # what taught everyone to ignore the last one. + if have == want and issue["state"] == "open": + print(f"#{n} is already current ({want}) — staying quiet") + return 0 + + _call(f"{args.api}/issues/{n}", args.token, "PATCH", {"body": body, "state": "open"}) + + if have != want: + refs = ", ".join(f"`{r['ref']}`" for r in broken) + note = ( + "The findings changed — the report above has been updated.\n\n" + f"Currently broken on: {refs}." + if have else + "This report is now kept up to date automatically: the body above always " + "reflects the current findings, and a comment is only added when they " + "change." + ) + _call(f"{args.api}/issues/{n}/comments", args.token, "POST", {"body": note}) + print(f"updated #{n}: {have} -> {want}") + else: + print(f"reopened #{n}") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv))