CNTRLPLANE-3352: Add reusable GHA workflow definitions - #8386
Conversation
Add dormant reusable workflow files that will be referenced by caller stubs in a follow-up commit. This two-step approach avoids the bootstrap problem where @main refs would fail if reusable files don't exist on main yet. Reusable workflows include three improvements over the originals: - lint: add persist-credentials: false (was missing) - test: pin actions/setup-go to SHA (v5.6.0) - envtest-kube/ocp: update change detection grep to match both original and reusable filenames Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@bryan-cox: This pull request references CNTRLPLANE-3352 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. |
|
Skipping CI for Draft Pull Request. |
📝 WalkthroughWalkthroughThis pull request introduces nine new reusable GitHub Actions workflows that standardize CI/CD operations across the repository. The workflows include checks for code spelling, CPO container synchronization, documentation building, Kubernetes and OpenShift Container Platform API validation via envtest, git commit linting, general linting, unit testing with coverage reporting, and code verification. Each workflow is triggered via 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox 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 |
|
/lgtm |
|
/area ci-tooling |
|
/verified bypass |
|
@bryan-cox: The 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. |
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/envtest-ocp-reusable.yaml:
- Around line 27-44: The workflow's ref and event handling uses context fields
that can be empty in reusable/workflow_call contexts (EVENT_NAME, CREATED,
PR_DIFF_REF, PUSH_DIFF_REF) which breaks the git diff; fix by computing refs
from explicit SHAs: set CREATED based on github.event_name ==
'workflow_dispatch' (don’t rely on github.event.created), build PR_DIFF_REF
using github.event.pull_request.base.sha...github.sha (use
github.event.pull_request.base.sha only when pull_request exists), build
PUSH_DIFF_REF using github.event.before..github.sha (and if github.event.before
is empty fall back to github.sha..github.sha), then use that computed ref for
git diff --name-only; update the ref selection logic (the ref variable and the
git diff call) to use these explicit SHA-based values so git diff never receives
an empty ref.
In @.github/workflows/lint-reusable.yaml:
- Line 19: Guard the git fetch step that uses the refspec "git fetch origin ${{
github.base_ref }}:${{ github.base_ref }}" so it only runs when github.base_ref
is non-empty (e.g., PR runs); update the workflow step that contains that run
command to check the variable first (either with an if: conditional on
github.base_ref or by wrapping the fetch in a shell check like "if [ -n '${{
github.base_ref }}' ]; then ...; fi") to avoid executing "git fetch origin :"
when base_ref is empty.
In @.github/workflows/verify-reusable.yaml:
- Around line 19-32: The git-clean validation block (the sequence starting with
the git update-index --refresh / git diff-* / STATUS checks) is placed before
running make staticcheck, make fmt, and make vet so any changes those targets
introduce won't be detected; move that entire git-state check block to after the
make staticcheck, make fmt, and make vet runs so the workflow re-checks the
working tree after formatting/linting, ensuring no changes remain uncommitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9398d6af-a9cb-4815-8b6c-03a033bc465a
📒 Files selected for processing (9)
.github/workflows/codespell-reusable.yaml.github/workflows/cpo-container-sync-reusable.yaml.github/workflows/docs-build-reusable.yaml.github/workflows/envtest-kube-reusable.yaml.github/workflows/envtest-ocp-reusable.yaml.github/workflows/gitlint-reusable.yaml.github/workflows/lint-reusable.yaml.github/workflows/test-reusable.yaml.github/workflows/verify-reusable.yaml
| EVENT_NAME: ${{ github.event_name }} | ||
| CREATED: ${{ github.event.created }} | ||
| PR_DIFF_REF: origin/${{ github.base_ref }}...HEAD | ||
| PUSH_DIFF_REF: ${{ github.event.before }}..HEAD | ||
| run: | | ||
| if [ "$EVENT_NAME" = "workflow_dispatch" ] || [ "$CREATED" = "true" ]; then | ||
| echo "should_run=true" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
| ref="$PUSH_DIFF_REF" | ||
| if [ "$EVENT_NAME" = "pull_request" ]; then | ||
| ref="$PR_DIFF_REF" | ||
| fi | ||
| if git diff --name-only "$ref" | grep -qE '^(api/|test/envtest/|cmd/install/assets/crds/hypershift-operator/tests/|karpenter-operator/controllers/karpenter/assets/tests/|\.github/workflows/envtest-ocp(-reusable)?\.yaml$)'; then | ||
| echo "should_run=true" | ||
| else | ||
| echo "should_run=false" | ||
| fi >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For a called reusable workflow, what are the documented behaviors of github.event_name, github.eventpayload inheritance, andgithub.base_ref availability?
💡 Result:
In a called reusable workflow (triggered by workflow_call): - github.event_name: Inherits the event name from the calling (top-level) workflow. It is NOT workflow_call; instead, it reflects the original trigger event of the caller (e.g., push, pull_request, workflow_dispatch). This is confirmed by GitHub Docs stating the github context is associated with the caller workflow, and practical evidence from GitHub issues where echo ${{ github.event_name }} in called workflows shows the caller's event. - github.event payload: Fully inherits the event payload from the calling workflow. Official GitHub Docs explicitly state: "When a workflow is triggered with the workflow_call event, the event payload in the called workflow is the same event payload from the calling workflow." Also, GITHUB_SHA and GITHUB_REF are the same as the caller. - github.base_ref availability: Available if the inherited github.event_name is pull_request or pull_request_target (from the caller). Per contexts docs, github.base_ref is set for those event types, containing the target branch of the PR. For other inherited events (e.g., push), it is empty/undefined.
Citations:
- 1: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows
- 2: https://docs.github.com/en/actions/using-workflows/reusing-workflows
- 3: if: ${{ github.event_name != 'workflow_call' }} not working actions/runner#3146
- 4: https://docs.github.com/en/actions/reference/events-that-trigger-workflows
- 5: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 6: https://docs.github.com/actions/using-workflows/events-that-trigger-workflows
- 7: https://docs.github.com/en/actions/reference/contexts-reference
- 8: https://help.github.com/en/actions/reference/reusable-workflows-reference
🏁 Script executed:
find .github/workflows -type f -name "*.yaml" -o -name "*.yml" | head -20Repository: openshift/hypershift
Length of output: 817
🏁 Script executed:
cat -n .github/workflows/envtest-kube-reusable.yaml | head -80Repository: openshift/hypershift
Length of output: 3210
🏁 Script executed:
cat -n .github/workflows/envtest-ocp-reusable.yaml | head -80Repository: openshift/hypershift
Length of output: 3266
🏁 Script executed:
cat -n .github/workflows/envtest-ocp.yaml | head -40Repository: openshift/hypershift
Length of output: 1373
🏁 Script executed:
git log --oneline -20 -- .github/workflows/envtest-kube-reusable.yaml .github/workflows/envtest-ocp-reusable.yaml 2>/dev/null || echo "Git history not available"Repository: openshift/hypershift
Length of output: 136
🏁 Script executed:
cd .github/workflows && git log --all --source --grep="workflow_call\|ref-resolution" -i --oneline 2>/dev/null | head -20 || echo "No relevant commits found"Repository: openshift/hypershift
Length of output: 46
Apply ref-resolution fix for workflow_call context in this reusable workflow.
This workflow has the same event/ref handling issues that affect workflow_call contexts. When called as a reusable workflow with push, pull_request, or workflow_dispatch triggers, the current code relies on context variables that may be undefined:
- Line 29:
github.base_refis only populated for pull_request events; it will be empty in other contexts - Line 28:
github.event.createdis only set for workflow_dispatch events - The ref resolution logic doesn't account for all event types properly
This causes git diff to fail or produce incorrect results, potentially skipping OCP envtest runs unintentionally.
Use direct SHA references instead of event name/payload assumptions:
Suggested fix
- name: Check for relevant file changes
id: check
env:
- EVENT_NAME: ${{ github.event_name }}
- CREATED: ${{ github.event.created }}
- PR_DIFF_REF: origin/${{ github.base_ref }}...HEAD
- PUSH_DIFF_REF: ${{ github.event.before }}..HEAD
+ PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+ PUSH_BEFORE_SHA: ${{ github.event.before }}
run: |
- if [ "$EVENT_NAME" = "workflow_dispatch" ] || [ "$CREATED" = "true" ]; then
- echo "should_run=true" >> "$GITHUB_OUTPUT"
- exit 0
- fi
- ref="$PUSH_DIFF_REF"
- if [ "$EVENT_NAME" = "pull_request" ]; then
- ref="$PR_DIFF_REF"
- fi
+ set -euo pipefail
+ if [ -n "${PR_BASE_SHA:-}" ] && [ -n "${PR_HEAD_SHA:-}" ]; then
+ ref="${PR_BASE_SHA}...${PR_HEAD_SHA}"
+ elif [ -n "${PUSH_BEFORE_SHA:-}" ] && [ "$PUSH_BEFORE_SHA" != "0000000000000000000000000000000000000000" ]; then
+ ref="${PUSH_BEFORE_SHA}..HEAD"
+ else
+ echo "should_run=true" >> "$GITHUB_OUTPUT"
+ exit 0
+ fi
if git diff --name-only "$ref" | grep -qE '^(api/|test/envtest/|cmd/install/assets/crds/hypershift-operator/tests/|karpenter-operator/controllers/karpenter/assets/tests/|\.github/workflows/envtest-ocp(-reusable)?\.yaml$)'; then
echo "should_run=true"
else
echo "should_run=false"
fi >> "$GITHUB_OUTPUT"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| EVENT_NAME: ${{ github.event_name }} | |
| CREATED: ${{ github.event.created }} | |
| PR_DIFF_REF: origin/${{ github.base_ref }}...HEAD | |
| PUSH_DIFF_REF: ${{ github.event.before }}..HEAD | |
| run: | | |
| if [ "$EVENT_NAME" = "workflow_dispatch" ] || [ "$CREATED" = "true" ]; then | |
| echo "should_run=true" >> "$GITHUB_OUTPUT" | |
| exit 0 | |
| fi | |
| ref="$PUSH_DIFF_REF" | |
| if [ "$EVENT_NAME" = "pull_request" ]; then | |
| ref="$PR_DIFF_REF" | |
| fi | |
| if git diff --name-only "$ref" | grep -qE '^(api/|test/envtest/|cmd/install/assets/crds/hypershift-operator/tests/|karpenter-operator/controllers/karpenter/assets/tests/|\.github/workflows/envtest-ocp(-reusable)?\.yaml$)'; then | |
| echo "should_run=true" | |
| else | |
| echo "should_run=false" | |
| fi >> "$GITHUB_OUTPUT" | |
| PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} | |
| PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} | |
| PUSH_BEFORE_SHA: ${{ github.event.before }} | |
| run: | | |
| set -euo pipefail | |
| if [ -n "${PR_BASE_SHA:-}" ] && [ -n "${PR_HEAD_SHA:-}" ]; then | |
| ref="${PR_BASE_SHA}...${PR_HEAD_SHA}" | |
| elif [ -n "${PUSH_BEFORE_SHA:-}" ] && [ "$PUSH_BEFORE_SHA" != "0000000000000000000000000000000000000000" ]; then | |
| ref="${PUSH_BEFORE_SHA}..HEAD" | |
| else | |
| echo "should_run=true" >> "$GITHUB_OUTPUT" | |
| exit 0 | |
| fi | |
| if git diff --name-only "$ref" | grep -qE '^(api/|test/envtest/|cmd/install/assets/crds/hypershift-operator/tests/|karpenter-operator/controllers/karpenter/assets/tests/|\.github/workflows/envtest-ocp(-reusable)?\.yaml$)'; then | |
| echo "should_run=true" | |
| else | |
| echo "should_run=false" | |
| fi >> "$GITHUB_OUTPUT" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/envtest-ocp-reusable.yaml around lines 27 - 44, The
workflow's ref and event handling uses context fields that can be empty in
reusable/workflow_call contexts (EVENT_NAME, CREATED, PR_DIFF_REF,
PUSH_DIFF_REF) which breaks the git diff; fix by computing refs from explicit
SHAs: set CREATED based on github.event_name == 'workflow_dispatch' (don’t rely
on github.event.created), build PR_DIFF_REF using
github.event.pull_request.base.sha...github.sha (use
github.event.pull_request.base.sha only when pull_request exists), build
PUSH_DIFF_REF using github.event.before..github.sha (and if github.event.before
is empty fall back to github.sha..github.sha), then use that computed ref for
git diff --name-only; update the ref selection logic (the ref variable and the
git diff call) to use these explicit SHA-based values so git diff never receives
an empty ref.
| with: | ||
| fetch-depth: 0 | ||
| persist-credentials: false | ||
| - run: git fetch origin ${{ github.base_ref }}:${{ github.base_ref }} |
There was a problem hiding this comment.
Guard base-ref fetch to avoid invalid refspec failures.
Line 19 can fail when github.base_ref is empty (non-PR caller), causing git fetch origin :. Add a guard before fetching.
Proposed fix
- - run: git fetch origin ${{ github.base_ref }}:${{ github.base_ref }}
+ - run: |
+ if [ -n "${{ github.base_ref }}" ]; then
+ git fetch origin "${{ github.base_ref }}:${{ github.base_ref }}"
+ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/lint-reusable.yaml at line 19, Guard the git fetch step
that uses the refspec "git fetch origin ${{ github.base_ref }}:${{
github.base_ref }}" so it only runs when github.base_ref is non-empty (e.g., PR
runs); update the workflow step that contains that run command to check the
variable first (either with an if: conditional on github.base_ref or by wrapping
the fetch in a shell check like "if [ -n '${{ github.base_ref }}' ]; then ...;
fi") to avoid executing "git fetch origin :" when base_ref is empty.
| - run: | | ||
| git update-index --refresh | ||
| git diff-index --cached --quiet --ignore-submodules HEAD -- | ||
| git diff-files --quiet --ignore-submodules | ||
| git diff --exit-code HEAD -- | ||
| STATUS=$(git status -s) | ||
| if [ -n "$STATUS" ]; then | ||
| echo "untracked files detected:" | ||
| echo "$STATUS" | ||
| exit 1 | ||
| fi | ||
| - run: make staticcheck | ||
| - run: make fmt | ||
| - run: make vet |
There was a problem hiding this comment.
Git-clean validation is in the wrong position.
The git-state check runs before make fmt/make vet. If Line 31 (make fmt) changes files, this workflow won’t catch it. Move the git-clean block to the end.
Proposed fix
- run: make generate update
- - run: |
- git update-index --refresh
- git diff-index --cached --quiet --ignore-submodules HEAD --
- git diff-files --quiet --ignore-submodules
- git diff --exit-code HEAD --
- STATUS=$(git status -s)
- if [ -n "$STATUS" ]; then
- echo "untracked files detected:"
- echo "$STATUS"
- exit 1
- fi
- run: make staticcheck
- run: make fmt
- run: make vet
+ - run: |
+ git update-index --refresh
+ git diff-index --cached --quiet --ignore-submodules HEAD --
+ git diff-files --quiet --ignore-submodules
+ git diff --exit-code HEAD --
+ STATUS=$(git status -s)
+ if [ -n "$STATUS" ]; then
+ echo "untracked files detected:"
+ echo "$STATUS"
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - run: | | |
| git update-index --refresh | |
| git diff-index --cached --quiet --ignore-submodules HEAD -- | |
| git diff-files --quiet --ignore-submodules | |
| git diff --exit-code HEAD -- | |
| STATUS=$(git status -s) | |
| if [ -n "$STATUS" ]; then | |
| echo "untracked files detected:" | |
| echo "$STATUS" | |
| exit 1 | |
| fi | |
| - run: make staticcheck | |
| - run: make fmt | |
| - run: make vet | |
| - run: make staticcheck | |
| - run: make fmt | |
| - run: make vet | |
| - run: | | |
| git update-index --refresh | |
| git diff-index --cached --quiet --ignore-submodules HEAD -- | |
| git diff-files --quiet --ignore-submodules | |
| git diff --exit-code HEAD -- | |
| STATUS=$(git status -s) | |
| if [ -n "$STATUS" ]; then | |
| echo "untracked files detected:" | |
| echo "$STATUS" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/verify-reusable.yaml around lines 19 - 32, The git-clean
validation block (the sequence starting with the git update-index --refresh /
git diff-* / STATUS checks) is placed before running make staticcheck, make fmt,
and make vet so any changes those targets introduce won't be detected; move that
entire git-state check block to after the make staticcheck, make fmt, and make
vet runs so the workflow re-checks the working tree after formatting/linting,
ensuring no changes remain uncommitted.
|
/override "ci/prow/verify-workflows" |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/verify-workflows 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 kubernetes-sigs/prow repository. |
|
@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. |
…#78674) GHA workflows in openshift/hypershift now use reusable workflow callers that reference definitions via @main, so PRs always run the latest workflow version without needing to rebase. The verify-workflows job that enforced rebasing is no longer needed. See openshift/hypershift#8386 and openshift/hypershift#8387. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…openshift#78674) GHA workflows in openshift/hypershift now use reusable workflow callers that reference definitions via @main, so PRs always run the latest workflow version without needing to rebase. The verify-workflows job that enforced rebasing is no longer needed. See openshift/hypershift#8386 and openshift/hypershift#8387. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…openshift#78674) GHA workflows in openshift/hypershift now use reusable workflow callers that reference definitions via @main, so PRs always run the latest workflow version without needing to rebase. The verify-workflows job that enforced rebasing is no longer needed. See openshift/hypershift#8386 and openshift/hypershift#8387. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…openshift#78674) GHA workflows in openshift/hypershift now use reusable workflow callers that reference definitions via @main, so PRs always run the latest workflow version without needing to rebase. The verify-workflows job that enforced rebasing is no longer needed. See openshift/hypershift#8386 and openshift/hypershift#8387. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…openshift#78674) GHA workflows in openshift/hypershift now use reusable workflow callers that reference definitions via @main, so PRs always run the latest workflow version without needing to rebase. The verify-workflows job that enforced rebasing is no longer needed. See openshift/hypershift#8386 and openshift/hypershift#8387. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…openshift#78674) GHA workflows in openshift/hypershift now use reusable workflow callers that reference definitions via @main, so PRs always run the latest workflow version without needing to rebase. The verify-workflows job that enforced rebasing is no longer needed. See openshift/hypershift#8386 and openshift/hypershift#8387. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…openshift#78674) GHA workflows in openshift/hypershift now use reusable workflow callers that reference definitions via @main, so PRs always run the latest workflow version without needing to rebase. The verify-workflows job that enforced rebasing is no longer needed. See openshift/hypershift#8386 and openshift/hypershift#8387. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
What this PR does / why we need it:
Adds 9 dormant reusable workflow files (
*-reusable.yaml) that will be referenced by caller stubs in a follow-up PR. This is the first of two PRs implementing reusable workflows to eliminate theverify-workflowsProw job and forced rebases.Workflows added:
Improvements over originals:
persist-credentials: false(was missing in original)actions/setup-goto SHA40f1582b2485089dde7abd97c1529aa768e1baff(v5.6.0)These files are inert until callers reference them. The follow-up PR (#8387) converts all 9 callers to thin stubs that
uses: ...@main.Why two PRs: The
@mainref in caller stubs requires the reusable files to exist on main first. Merging the reusable files first avoids this bootstrap problem.Future release branches (4.23+): Will need push/PR trigger branch lists updated in both caller and reusable files.
Which issue(s) this PR fixes:
Fixes CNTRLPLANE-3352
Special notes for your reviewer:
@mainrefs.workflow_callandpushtriggers so direct pushes to main/release branches still work after the caller conversion.Checklist:
🤖 Generated with Claude Code