mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-04 02:57:27 +09:00
Timofei Larkin
e31f518df7
## What this PR does Publishes the `API Review Gate` merge context as a commit status, so a skipped or cancelled run cannot pass it or poison it, and gives fork PRs the same gate through a privileged `workflow_run` workflow. ### Why The merge context was a job name. A required job that skips counts as passed, and check runs are not deduplicated per name, so one cancelled run on a head keeps the context failed until the next push. Measured on this repo: `41d1153c` has four check runs with the gate's job name (one `cancelled`, three `success`). Commit statuses are deduplicated per context and the newest wins, so a run with no verdict posts nothing and the previous status stays. ### What changes Same-repo PRs get status from `api-gate-report`. A `pending` opener marks the head before the evaluation on approve, dismiss and request-changes reviews, so an old `success` is not mergeable for the minute the evaluation takes. `COMMENTED` reviews no longer count as a reviewer's latest review, so an owner who approved and then commented keeps the approval. Concurrency is split, so bot comment run cannot cancel live evaluation. Fork PRs: `api-review-gate-fork.yaml` runs from the default branch and reaches the verdict itself, the fork's run is only a trigger. `resolve` finds PR by head SHA in open PR list (review runs on a fork report base repo as head repository and have no pull requests). `detect` fetches the head as data and runs the detector in a job with `contents: read` only. `conclude` applies the approval rule and posts. A sweep every 15 minutes moves the status of fork PRs whose runs wait for a maintainer in `action_required`, those never fire `workflow_run`. This is lllamnyp's patch, applied whole, with these changes on top: - two open fork PRs on one head fail closed in `resolve` and in the sweep, a head carried by a same-repo PR stays with `api-gate-report` - detector is built from default branch only, merge base is picked by PR author and an old one would mean an old detector - the fork `detect` step runs under `bash -e` and detector exit 2 ended it before the code was read, so it has `set +e` like the same-repo job - head is checked out with links as links, like the same-repo gate does, because flattening them hid a CRD behind a file link from the detector while Helm still ships it ### Limits - A fork PR whose push run waits for a maintainer's approval has no status until that run executes, same as `E2E Tests`. - A fork PR on a head that a same-repo PR also carries shows the verdict standing there, one commit status cannot hold a verdict per base. - The sweep can publish a stale `success` if it reads an approval, stalls, and the owner withdraws it meanwhile. The next sweep flips it back, so the window is up to 15 minutes. - Two copies of one first-party CRD and CRDs behind directory links are gaps of the detector on main, same for same-repo PRs, they are in https://github.com/cozystack/cozystack/issues/5170. ### Testing `actionlint` is clean on both workflows. `zizmor` is clean too, but `dangerous-triggers` and `template-injection` are disabled repo-wide in `.github/zizmor.yml`, so that says little about a privileged workflow. I ran the `github-script` bodies under node with a mocked API (every conclusion, approval and shared head case) and the run steps under `bash -e` with a stub detector exiting 0, 2 and 1. `make unit-tests` passes with `LC_ALL=C` (a kafka bats test depends on the locale, nothing to do with this PR). Not exercised live: the fork path and the sweep run only from the default branch, so their first run is after merge. ### Repo admin Branch protection requires the old job name `Require API owner review for sizeable API changes`. This PR renames that job, so after merge the required context has to be changed to the `API Review Gate` status. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ```