Extend CodeQL language gating to push events (main + release branches)#68085
Extend CodeQL language gating to push events (main + release branches)#68085shahar1 wants to merge 2 commits into
Conversation
running all five languages unconditionally. Extend the detect-languages job to use the GitHub compare API (before…after) for push events, so a docs-only or single-language merge to main no longer fans out all five CodeQL jobs. schedule runs are unchanged — they still scan every language to maintain periodic full-branch coverage. Falls back to all languages when the compare API is unavailable or the before SHA is all zeros (branch creation). related: apache#67972
bbb4699 to
ec7a1ea
Compare
|
Need your opinion - more than often we merge one PR after another, so the latest commit cancels the previous CodeQL runs. Considering that anyway we have the scheduled runs: |
jscheffl
left a comment
There was a problem hiding this comment.
in my opinion if we scan (fltered/selectively) on PR prior merge then it is okay to only do a full scan on schedule and have a small risk if multiple PRs are merged and not each merge is a complete check.
potiuk
left a comment
There was a problem hiding this comment.
Nice — gating push and bringing release branches to parity both make sense, and failing open to a full scan on compare failure is the right call.
One non-blocking robustness nit: the push compare call isn't paginated while the PR path is (--paginate). GitHub's compare API returns at most 300 files per page, so a large merge (>300 files) could under-detect a changed language and skip scanning it — most relevant on release branches, which have no daily schedule full-scan to back them up. Adding --paginate to the compare call would close that and match the PR path. Up to you whether to fold it in here or as a follow-up.
Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting
The GitHub compare API returns at most 300 changed files and does not paginate the file list (only commits paginate), so a merge touching more than 300 files truncates the list and could under-detect a changed language. Detect that cap and fall back to scanning every language — release branches have no daily schedule full-scan to back them up.
|
Failure in "Latest Boto test" is surfaced in this PR but unrelated, see: |
What
Extends the language-gating logic from #67972 — which only applied to
pull_request— topushevents as well, and brings thepushtrigger to parity withpull_requestso it also fires on release branches.Two changes:
detect-languagesjob now uses the GitHub compare API (repos/{repo}/compare/{before}...{after}) to find which languages actually changed in a push, and builds the analysis matrix from that — the same way it already does for PRs. A docs-only or single-language merge no longer fans out all five CodeQL jobs.pushtrigger now matches thepull_requesttrigger (main,v[0-9]+-[0-9]+-test,v[0-9]+-[0-9]+-stable) instead ofmainonly.scheduleruns are unchanged: they still scan all five languages.Why
#67972 intentionally left
pushscanning all languages, reasoning that full coverage onmainwas the goal. But the dailyschedulerun already provides that full-coverage guarantee — and it only runs on the default branch (main). That left two gaps:main: every merge commit fanned out five CodeQL jobs even when nothing relevant changed (the daily schedule already covers full-branch scanning).v*-test/v*-stablegot no push-time CodeQL at all (only PRs targeting them were scanned, andscheduledoesn't run there). Adding gated push runs gives maintained release branches the same security coverage cheaply.The detect logic is branch-agnostic, so the same compare-based gating works for every push branch.
Edge cases handled
The compare call is followed by a fallback to a full scan when it fails or returns nothing — covering a force-push, or a newly created release branch whose
beforeSHA is all zeros (no base commit to diff against).Behaviour summary
pull_request(main / release)pushtomainpushtov*-test/v*-stableschedule(daily, main)related: #67972
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (claude-opus-4-8) following the guidelines