ci: replace some third-party actions - #5384
Conversation
fde2b12 to
3611c5c
Compare
kolyshkin
added
the
backport/1.5-todo
kolyshkin
changed the title
|
This is part of workflow hardening; PTAL @opencontainers/runc-maintainers |
dceeb86 to
4ee26a5
Compare
|
rebased; PTAL @opencontainers/runc-maintainers 🙏🏻 wanted to get this out of my plate |
|
PTAL @opencontainers/runc-maintainers this is easy to review and it blocks my other work (adding zizmor, which we'd better have in before the rc1 release) |
4ee26a5 to
85074e8
Compare
|
@opencontainers/runc-maintainers can we please merge this? |
| - name: succeed (not a PR) # Allow all-done to succeed for non-PRs. | ||
| if: github.event_name != 'pull_request' | ||
| run: echo "Nothing to check here." | ||
| # Only need the PR commits. |
There was a problem hiding this comment.
I recently made changes to NOT do a checkout for these checks, and use the GitHub API, which is MUCH faster (1 - 2 seconds), and means we can check before checking out the source (nice!)
But there's also (haven't really tested it yet) a feature now to check commits through branch rules; https://docs.github.com/en/enterprise-cloud@latest/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets#metadata-restrictions
There was a problem hiding this comment.
Thanks, the API approach is much nicer — I've adopted it here! The savings are negligible though (maybe your moby checkout was too greedy?), but I like that we don't use any files from the PR itself.
As for rulesets, they are applied just before the merge, not that PR is opened/updated, so a contributor can't see red CI, making things more complicated than they should.
85074e8 to
47006aa
Compare
Replace two third-party actions (tim-actions/get-pr-commits and tim-actions/commit-message-checker-with-regex) with a few lines of shell using the GitHub API. Listing the PR commits via the API means no checkout is needed at all, so the job takes a couple of seconds. Merge commits are skipped, as their subjects are generated by git and GitHub, not by the contributor. The API output is saved to a file before being fed to jq, rather than piped into it. In a pipeline, a gh failure would be masked, as jq happily succeeds on empty input. With the file, a gh failure fails the step via the default bash -e. The if is kept at the step level rather than the job level, so that the job still succeeds (with its step skipped) for non-PR events. A skipped job would cascade and skip all-done. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The lumaxis/shellcheck-problem-matchers action merely ships a JSON file and echoes an ::add-matcher:: command, so do that ourselves. Its matcher only annotated warning and error findings. Two reasons: the severity capture group listed (note|warning|error), while shellcheck severities are error, warning, info and style; and the runner honors only error, warning and notice, skipping any other match (see Runner.Worker/Handlers/OutputManager.cs). As info and style are the bulk of what shellcheck reports, most findings were never annotated. Use a fixed severity instead (any finding fails the job anyway), and keep the severity word as part of the message. Note the same applies to the usual gcc-format matchers, as shellcheck -f gcc reports both info and style as "note", which is not "notice". Verified on a test PR: with no matcher, a file with four shellcheck problems produces no annotations at all; with this one, all four are annotated at the right lines. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
47006aa to
deaf96c
Compare
2 participants
Two independent cleanups in the
validateworkflow, each replacing athird-party action with a few lines in-tree.
1.
commitjob: simplify the subject length checkReplace two third-party actions (
tim-actions/get-pr-commitsandtim-actions/commit-message-checker-with-regex) with a few lines of shellusing the GitHub API (based off of moby/moby#53327, thanks @thaJeztah!).
No checkout is needed at all, so no source reaches the runner.
Verified on this PR: the job concludes
successin 3 steps and ~3s (downfrom 5 steps and 4s with a shallow checkout), and a temporary commit with a
100-character subject was flagged (and only that one), exiting 1.
2.
shellcheckjob: droplumaxis/shellcheck-problem-matchersThat action does nothing but ship a JSON file and echo an
::add-matcher::command, so we now do that ourselves, with the matcher in
.github/shellcheck-tty.json.Its matcher was also only annotating
warninganderrorfindings, for tworeasons: its severity capture group lists
(note|warning|error), whileshellcheck severities are
error,warning,infoandstyle; and therunner honors only
error,warningandnotice, silently skipping anyother match (
Runner.Worker/Handlers/OutputManager.cs). Asinfoandstyleare the bulk of what shellcheck reports, most findings were neverannotated. Ours uses a fixed severity instead (any finding fails the job
anyway) and keeps the severity word in the message.
Verified on a fork PR, on a file with four shellcheck problems (one
info,one
style, twowarning):exit code 2";
Note that the usual gcc-format matchers have the same problem, as
shellcheck -f gccreports bothinfoandstyleasnote, which is notnotice.