ci: add on-demand + auto-on-approval PR CI, piloted on two workflows#32012
ci: add on-demand + auto-on-approval PR CI, piloted on two workflows#32012gkarat wants to merge 5 commits into
Conversation
openssl/project#2027. Only ct-validation and aarch64 are wired for now; the rest stay schedule-only until they graduate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| pull_request_review: | ||
| types: [submitted] |
There was a problem hiding this comment.
IMO this will run it too early. The original idea was to run it when the label approval: done is assigned - i.e. when there are two approvals, not just a single one. Also this will run the job multiple times if multiple approvals are submitted, AFAIK. That's definitely not wanted.
There was a problem hiding this comment.
Good point, I have switched to using approval: done as you suggested. Seems to be the right source of truth (and it also cuts some good bunch of code for ci-authorize)
| `@${commenter}: \`/ci all\` dispatches **${Object.keys(WORKFLOWS).length} ` | ||
| + `workflows** (~**${total} compute-min** per PR). To confirm, comment:\n\n` | ||
| + `\`/ci all confirm ${nonce}\`\n\n_(valid for 60 minutes)_\n` | ||
| + `<!-- ${marker(nonce)} pr=${prNum} sha=${head_sha} by=${commenter} exp=${exp} -->`); |
There was a problem hiding this comment.
is it really necessary to confirm /ci all?
There was a problem hiding this comment.
When we later enable all workflows to be run on PRs, the computing cost of running ci all is quite expensive. I thought of a little confirmation guard to be useful here. But assuming committers member knows what they are doing, we can drop this check
There was a problem hiding this comment.
If it can be run only by commiters, I think confirmation is not needed.
Switch the auto trigger from pull_request_review to pull_request: labeled gated on 'approval: done', so CI runs at the 2-approval merge threshold and not again on later approvals. The gate trusts the label, scopes to stale-dismissing base branches, and pins the label-payload head SHA. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I'm a bit concerned that we keep all this logic in this repository. It's my bad that I didn't read the proposed design in time. But I think implementing this via webhooks and deploying it as a separate service will remove burden from committers to review all the changes which will need to be landed. |
|
Putting this to draft, let's explore the webhooks path, and I will either close or update this PR. |
FIne, but please take this as priority, we really need to run nightly tests on-demand now. |
|
Continued at #32053. |
Relates to openssl/project#2027.
What
Adds a mechanism for committers to run selected non-PR workflows against a pull request's code, two ways:
/ci <jobs>on a PR (e.g./ci ct-validation aarch64, or/ci all).How it works
Two controllers plus a shared gate:
/ci, gates on repository write/admin permission, pins the PR head SHA at command time, opens a pending check-run per requested job, and dispatches each workflow with{pr, head_sha, check_run_id}. It never checks out PR code, and the comment body is only ever passed through the API (never into a shell)./ci allrequires a nonce confirmation because of its cost.workflow_run.completed, recovering the routing from a trusted artifact written by the authorize gate, or from the trusted run-name if a run was cancelled while still queued.workflow_call) holding the single copy of the authorize gate (contents: read, no PR checkout). Each wired workflow calls it viauses:; it decides whether the build runs and which commit it builds:review.commit_id, required to equal the PR head);/cipath → the pinned head SHA;Security model
PR code is treated as untrusted regardless of head repository. It runs only on GitHub-hosted, secret-less runners (contents: read, persist-credentials: false). The privileged controllers and the authorize gate never check out PR code.
Gradual rollout
This PR wires only ct-validation and aarch64 — both cheap and GitHub-hosted, and together they exercise the full plumbing plus the trickier per-job guards. The remaining eligible workflows stay schedule-only until they graduate (add them to the controller lists and point their authorize job at the shared reusable workflow).
Checklist