CNTRLPLANE-3278: fix verify-workflows to check PR head, not merge commit#78025
Conversation
ci-operator runs tests on a merge commit (PR merged into base branch). The verify-workflows script was comparing workflow files on HEAD, which is the merge result and always contains main's files. This caused the check to always pass, even for PRs with outdated workflows. Fix by extracting the actual PR head commit (HEAD^2, the second parent of the merge) and comparing that against main instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@bryan-cox: This pull request references CNTRLPLANE-3278 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughUpdated the git comparison logic in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 inconclusive)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse skip We can't test this on hypershift here unfortunately |
|
@bryan-cox: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, clebs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@bryan-cox: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
verify-workflowspresubmit foropenshift/hypershiftthat was introduced in CNTRLPLANE-3278: Add Prow presubmit to enforce GitHub Actions workflow consistency for HyperShift #77164HEADin that context is the merge result — which always has main's workflow filesProblem
ci-operator creates a merge commit when building the
srcimage:HEAD= merge of PR into mainHEAD^1= main (base branch)HEAD^2= actual PR head commitThe original script compared
HEAD:.github/workflows/*against main, but sinceHEADis the merge result, workflow files from main are always present — the check always passed regardless of the PR branch state.Verified locally:
Fix
Extract the actual PR head commit via
HEAD^2(second parent of the merge commit) and compare that against main instead:Falls back to
HEADif there's no second parent (e.g., non-merge context).Test plan
HEAD^2🤖 Generated with Claude Code
Summary by CodeRabbit