Add automated Go version update workflow and version schemas - #21
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthroughAdds reusable schema-driven automation for updating Go versions and CI operator images in operator repositories, with build verification, dry-run reporting, and optional pull request creation. Documents schemas and adds mappings for OCP 4.19 through 4.22. ChangesGo version management
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Caller
participant UpdateWorkflow
participant VersionSchema
participant OperatorRepository
participant PullRequestAction
Caller->>UpdateWorkflow: Provide version-schema and dry-run
UpdateWorkflow->>VersionSchema: Fetch Go and CI image values
VersionSchema-->>UpdateWorkflow: Return schema configuration
UpdateWorkflow->>OperatorRepository: Update repository configuration and dependencies
OperatorRepository-->>UpdateWorkflow: Return build result and change status
UpdateWorkflow->>PullRequestAction: Create pull request when changes exist
PullRequestAction-->>OperatorRepository: Open update pull request
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
387a621 to
80840fd
Compare
Add a reusable GitHub Actions workflow that operator repos can call to automate Go version updates. Version schemas are defined as flat YAML files in go-versions/, keyed by OCP release. Design principles: - go.mod tracks Go minor version only (e.g. "1.25", not "1.25.0") - No toolchain directive — patch updates handled by CI builder image - Each operator repo adds a thin caller workflow referencing a schema - Workflow updates go.mod, .ci-operator.yaml, runs tidy/vendor, and creates a PR via peter-evans/create-pull-request - Uses only pre-installed runner tools (curl, grep, sed) — no yq needed Signed-off-by: Michal Pryc <mpryc@redhat.com>
80840fd to
75f05da
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
.github/workflows/update-go-version.yaml (5)
28-31: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable credential persistence to prevent token leakage.
By default,
actions/checkoutpersists the GitHub token in the local Git configuration. Because this workflow subsequently executesmake buildorgo build(which executes code from the repository), it is a security best practice to disable credential persistence to prevent unintended access to the token during the build step.🛡️ Proposed refactor
- name: Checkout operator repo uses: actions/checkout@v6 with: fetch-depth: 0 + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/update-go-version.yaml around lines 28 - 31, Update the “Checkout operator repo” actions/checkout step to disable credential persistence by setting persist-credentials to false, while preserving the existing full-fetch configuration.Source: Linters/SAST tools
124-131: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
git statusto reliably detect all changes, including untracked files.
git diff --quietonly detects changes to already-tracked files. Ifgo mod vendorintroduces new dependencies and generates new files in thevendor/directory, they remain untracked until added. While changes to the trackedgo.sumfile usually accompany new dependencies and trigger the condition anyway, usinggit status --porcelainis a more robust way to verify if the working directory is completely clean.💡 Proposed refactor
- name: Check for changes id: changes run: | - if git diff --quiet; then + if [ -z "$(git status --porcelain)" ]; then echo "No file changes — everything is up to date" echo "has-changes=false" >> "${GITHUB_OUTPUT}" else echo "has-changes=true" >> "${GITHUB_OUTPUT}" - git diff --stat + git status --short fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/update-go-version.yaml around lines 124 - 131, Replace the git diff --quiet check in the workflow’s change-detection step with git status --porcelain so both tracked modifications and untracked files are detected. Preserve the existing has-changes outputs and status summary behavior for clean and changed working trees.
102-113: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMove
toolchainremoval after dependency updates.Starting with Go 1.21,
go mod tidycan automatically append a newtoolchaindirective togo.modif any downloaded dependency requires a newer patch version. To guarantee the directive remains entirely removed as intended, move thesedcommand to execute after thego mod tidystep.♻️ Proposed refactor
- # Remove toolchain directive — patch versions are handled by the builder - sed -i '/^toolchain /d' go.mod - name: Update dependencies run: | # Always run tidy + vendor to pick up dependency security patches, # even when the Go version itself hasn't changed go mod tidy if [ -d vendor ]; then go mod vendor fi + + # Remove toolchain directive — patch versions are handled by the builder + sed -i '/^toolchain /d' go.mod🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/update-go-version.yaml around lines 102 - 113, Move the `sed -i '/^toolchain /d' go.mod` command from before the “Update dependencies” step to after `go mod tidy` completes, so any automatically added toolchain directive is removed. Keep the existing conditional `go mod vendor` flow unchanged.
153-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNote: PRs created with
GITHUB_TOKENwill not trigger downstream GitHub Actions workflows.By default, GitHub prevents workflows triggered by the
GITHUB_TOKENfrom initiating subsequent workflow runs (such as CI checks running on the newly created PR). If the operator repositories rely on GitHub Actions workflows for PR validation, those checks will not run automatically. You can bypass this limitation by using a Personal Access Token (PAT) or a GitHub App token instead.If the repositories strictly use an external CI system like OpenShift Prow (which listens to webhook events independently), this setup is perfectly fine as-is.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/update-go-version.yaml around lines 153 - 156, Update the create-pull-request step using the github.token input so it authenticates with the repository’s configured PAT or GitHub App token instead, while preserving the existing dry-run and has-changes condition. Use the appropriate existing secret or token configuration rather than introducing an unrelated credential.
45-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
yqis pre-installed on GitHub-hosted runners.The comment indicates that
yqis not pre-installed, but it is officially included onubuntu-24.04and other standard GitHub-hosted runners.While the
grepapproach works fine for this flat configuration file, avoidingyqintroduces critical parsing bugs for the nested.ci-operator.yamlfile later in the workflow. Sinceyqis already pre-installed, using it fully satisfies your goal of leveraging only pre-installed tools while ensuring robust YAML manipulation.♻️ Proposed refactor to use yq
- # Parse simple key: "value" YAML without yq (not pre-installed on runners) - GO_VERSION=$(grep '^go:' /tmp/go-version.yaml | sed 's/^go: *"\?\([^"]*\)"\?/\1/') - CI_IMAGE=$(grep '^ci-operator-image:' /tmp/go-version.yaml | sed 's/^ci-operator-image: *"\?\([^"]*\)"\?/\1/') + # Extract configuration values using the pre-installed yq + GO_VERSION=$(yq '.go' /tmp/go-version.yaml) + CI_IMAGE=$(yq '.ci-operator-image' /tmp/go-version.yaml)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/update-go-version.yaml around lines 45 - 48, Update the YAML parsing in the workflow to use the pre-installed yq tool instead of the grep/sed commands for GO_VERSION and CI_IMAGE, and remove the inaccurate comment. Reuse yq consistently for the later nested .ci-operator.yaml parsing and preserve the existing extracted values and workflow behavior.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/update-go-version.yaml:
- Around line 75-83: Update the CURRENT_IMAGE assignment in the
.ci-operator.yaml comparison block to use the pre-installed yq tool and extract
only the builder image tag, rather than grepping every tag entry. Preserve the
existing equality check and ci-update outputs once CURRENT_IMAGE contains the
single intended value.
- Around line 119-121: Replace the sed-based update in the workflow step with a
yq expression that targets the exact build_root.tag field in .ci-operator.yaml,
assigning TARGET_CI_IMAGE while leaving unrelated tag fields such as those under
base_images unchanged.
---
Nitpick comments:
In @.github/workflows/update-go-version.yaml:
- Around line 28-31: Update the “Checkout operator repo” actions/checkout step
to disable credential persistence by setting persist-credentials to false, while
preserving the existing full-fetch configuration.
- Around line 124-131: Replace the git diff --quiet check in the workflow’s
change-detection step with git status --porcelain so both tracked modifications
and untracked files are detected. Preserve the existing has-changes outputs and
status summary behavior for clean and changed working trees.
- Around line 102-113: Move the `sed -i '/^toolchain /d' go.mod` command from
before the “Update dependencies” step to after `go mod tidy` completes, so any
automatically added toolchain directive is removed. Keep the existing
conditional `go mod vendor` flow unchanged.
- Around line 153-156: Update the create-pull-request step using the
github.token input so it authenticates with the repository’s configured PAT or
GitHub App token instead, while preserving the existing dry-run and has-changes
condition. Use the appropriate existing secret or token configuration rather
than introducing an unrelated credential.
- Around line 45-48: Update the YAML parsing in the workflow to use the
pre-installed yq tool instead of the grep/sed commands for GO_VERSION and
CI_IMAGE, and remove the inaccurate comment. Reuse yq consistently for the later
nested .ci-operator.yaml parsing and preserve the existing extracted values and
workflow behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42c5025f-4718-4524-934b-9ac38f2f70c6
📒 Files selected for processing (6)
.github/workflows/update-go-version.yamlgo-versions/README.mdgo-versions/ocp-4.19.yamlgo-versions/ocp-4.20.yamlgo-versions/ocp-4.21.yamlgo-versions/ocp-4.22.yaml
| if [ -f .ci-operator.yaml ]; then | ||
| CURRENT_IMAGE=$(grep 'tag:' .ci-operator.yaml | sed 's/.*tag: *"\?\([^"]*\)"\?/\1/') | ||
| if [ "${CURRENT_IMAGE}" = "${TARGET_CI_IMAGE}" ]; then | ||
| echo "CI image already at ${TARGET_CI_IMAGE}" | ||
| echo "ci-update=false" >> "${GITHUB_OUTPUT}" | ||
| else | ||
| echo "CI image update needed: ${CURRENT_IMAGE} -> ${TARGET_CI_IMAGE}" | ||
| echo "ci-update=true" >> "${GITHUB_OUTPUT}" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
grep will capture multiple tags, breaking the evaluation.
Using grep 'tag:' captures all occurrences of tag: in .ci-operator.yaml (e.g., tags often found under base_images configurations). This produces a multiline string in CURRENT_IMAGE, which causes the equality check [ "${CURRENT_IMAGE}" = "${TARGET_CI_IMAGE}" ] to silently fail, forcing an unnecessary update attempt.
Use the pre-installed yq tool to precisely extract the builder image tag.
🐛 Proposed fix
if [ -f .ci-operator.yaml ]; then
- CURRENT_IMAGE=$(grep 'tag:' .ci-operator.yaml | sed 's/.*tag: *"\?\([^"]*\)"\?/\1/')
+ CURRENT_IMAGE=$(yq '.build_root.image_stream_tag.tag' .ci-operator.yaml)
if [ "${CURRENT_IMAGE}" = "${TARGET_CI_IMAGE}" ]; then📝 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.
| if [ -f .ci-operator.yaml ]; then | |
| CURRENT_IMAGE=$(grep 'tag:' .ci-operator.yaml | sed 's/.*tag: *"\?\([^"]*\)"\?/\1/') | |
| if [ "${CURRENT_IMAGE}" = "${TARGET_CI_IMAGE}" ]; then | |
| echo "CI image already at ${TARGET_CI_IMAGE}" | |
| echo "ci-update=false" >> "${GITHUB_OUTPUT}" | |
| else | |
| echo "CI image update needed: ${CURRENT_IMAGE} -> ${TARGET_CI_IMAGE}" | |
| echo "ci-update=true" >> "${GITHUB_OUTPUT}" | |
| fi | |
| if [ -f .ci-operator.yaml ]; then | |
| CURRENT_IMAGE=$(yq '.build_root.image_stream_tag.tag' .ci-operator.yaml) | |
| if [ "${CURRENT_IMAGE}" = "${TARGET_CI_IMAGE}" ]; then | |
| echo "CI image already at ${TARGET_CI_IMAGE}" | |
| echo "ci-update=false" >> "${GITHUB_OUTPUT}" | |
| else | |
| echo "CI image update needed: ${CURRENT_IMAGE} -> ${TARGET_CI_IMAGE}" | |
| echo "ci-update=true" >> "${GITHUB_OUTPUT}" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/update-go-version.yaml around lines 75 - 83, Update the
CURRENT_IMAGE assignment in the .ci-operator.yaml comparison block to use the
pre-installed yq tool and extract only the builder image tag, rather than
grepping every tag entry. Preserve the existing equality check and ci-update
outputs once CURRENT_IMAGE contains the single intended value.
| run: | | ||
| sed -i "s|^\( tag: \).*|\1${TARGET_CI_IMAGE}|" .ci-operator.yaml | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Incorrect indentation assumption in sed replacement.
The regex ^\( tag: \) strictly expects two spaces of indentation. In OpenShift .ci-operator.yaml files, the tag field under build_root is typically indented with four spaces ( tag:). Consequently, this sed command will silently fail to update the file, submitting a PR without the expected CI image change. Additionally, if other configuration blocks (like base_images) happen to use two-space indentation for their tags, they will be incorrectly corrupted.
Use yq to safely update the exact field.
🐛 Proposed fix
env:
TARGET_CI_IMAGE: ${{ steps.config.outputs.ci-image }}
run: |
- sed -i "s|^\( tag: \).*|\1${TARGET_CI_IMAGE}|" .ci-operator.yaml
+ yq -i '.build_root.image_stream_tag.tag = env(TARGET_CI_IMAGE)' .ci-operator.yaml📝 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: | | |
| sed -i "s|^\( tag: \).*|\1${TARGET_CI_IMAGE}|" .ci-operator.yaml | |
| run: | | |
| yq -i '.build_root.image_stream_tag.tag = env(TARGET_CI_IMAGE)' .ci-operator.yaml |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/update-go-version.yaml around lines 119 - 121, Replace the
sed-based update in the workflow step with a yq expression that targets the
exact build_root.tag field in .ci-operator.yaml, assigning TARGET_CI_IMAGE while
leaving unrelated tag fields such as those under base_images unchanged.
There was a problem hiding this comment.
Very good intention but I am not sure I like the per repo changes and using cron, see my suggestion at the end.
Reviewed against recent merged Go bump PRs across all 6 operators: NHC #421, SNR #324, FAR #201, MDR #186, NMO #155, SBR #77. See inline threads for details.
All 6 repos keep the toolchain directive (e.g., go 1.26.0 / toolchain go1.26.5) — removing it lets go mod tidy and go build silently download a different Go patch version, breaking build reproducibility across contributors and CI.
Consider a centralized approach: a single workflow_dispatch workflow that reads a config file (target repos, Go version, API packages to bump in go.mod), runs the update across all repos, and creates PRs — with a dry-run option to preview changes before PR creation. One trigger replaces per-repo cron jobs and eliminates the need for any changes in operator repos.
This workflow may fit better in medik8s/tools rather than .github — tools already hosts shared dev tooling (dev.mk, setup.sh), while .github is for org-wide defaults (profile, shared workflow templates). A config-driven multi-repo dispatch workflow aligns with the tools mandate. The version schema YAML files could live alongside it.
| go mod edit -go="${TARGET_GO}" | ||
|
|
||
| # Remove toolchain directive — patch versions are handled by the builder | ||
| sed -i '/^toolchain /d' go.mod |
There was a problem hiding this comment.
This contradicts how every operator actually manages Go versions. All 6 repos use patch versions with toolchain:
| Repo | go.mod |
|---|---|
| SNR, NHC, MDR, NMO, SBR | go 1.26.0 / toolchain go1.26.5 |
| FAR | go 1.25.0 / toolchain go1.25.9 |
The sed removal deletes what every repo intentionally keeps, and go mod tidy (next step) will likely re-add it anyway since the installed Go patch version (e.g., 1.25.3 via setup-go) is newer than go 1.25 (equivalent to 1.25.0).
Recommendation: keep toolchain and update the schema to include the patch version:
# go-versions/latest.yaml
go: "1.26.5" # full patch version
ci-operator-image: "rhel-9-release-golang-1.26-openshift-5.0"Then replace these lines with:
go mod edit -go="${TARGET_GO}" -toolchain="go${TARGET_GO}"There was a problem hiding this comment.
Acknowledged, will address in a follow-up — need to update the schema to include patch versions first.
| # Always run tidy + vendor to pick up dependency security patches, | ||
| # even when the Go version itself hasn't changed | ||
| go mod tidy | ||
|
|
||
| if [ -d vendor ]; then | ||
| go mod vendor | ||
| fi |
There was a problem hiding this comment.
Two issues here:
1. go mod tidy doesn't update dependency versions — it adds missing and removes unused modules, but won't pull in security patches. Every actual Go bump PR (NHC #421, SNR #324, etc.) includes explicit dependency bumps like golang.org/x/net v0.53.0 → v0.57.0.
It's fine to keep this PR focused on Go + go.sum only, but as a follow-up consider extending the version schema to cover packages, tools, and test framework alignment:
# go-versions/latest.yaml
go: "1.26.5"
ci-operator-image: "rhel-9-release-golang-1.26-openshift-5.0"
packages:
- golang.org/x/net@latest
- golang.org/x/sys@latest
# K8s packages follow release branches matching OCP version
- k8s.io/api@release-0.32
- k8s.io/client-go@release-0.32
tools:
kustomize: latest # auto-resolve from GitHub releases
controller-gen: latest
ginkgo: "2.23.4" # must match go.mod
gomega: "1.37.1" # must match go.modKey design points:
- Auto-discovery: a helper workflow that queries GitHub releases API and Go module proxy to generate/update this config — so you don't have to look up versions manually
- Release branch targeting: K8s packages (
k8s.io/api,client-go) need@release-0.XXmatching the OCP version, not@latest - Ginkgo/gomega alignment: Makefile
GINKGO_VERSIONandgo.modgithub.com/onsi/ginkgo/v2must stay in sync — a centralized config that bumps both in one pass prevents drift
2. All 6 operators (excluding CUR) have go-verify or equivalent Makefile targets that run go mod tidy + go mod vendor + go mod verify. Consider calling make go-verify instead of raw tidy/vendor — 5/6 repos use go-verify; NHC uses vendor (can be aligned separately).
There was a problem hiding this comment.
Good suggestions for follow-up — keeping this PR focused on Go version + go.sum for now.
- Validate GO_VERSION and CI_IMAGE after parsing, fail early if empty - Anchor grep for tag: in .ci-operator.yaml to avoid matching multiple tag fields, use head -1 as safety net - Use git status --porcelain instead of git diff --quiet to detect untracked files (e.g. new vendor/ dependencies) - SHA-pin peter-evans/create-pull-request to v7.0.11 for supply chain security (action receives contents:write + pull-requests:write) - Fix ocp-4.21.yaml comment: used by release branches, not main Signed-off-by: Michal Pryc <mpryc@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
razo7
left a comment
razo7
left a comment
There was a problem hiding this comment.
Ready for merge. My two suggestions will be done in a follow up.
Holding for now in case someone else has more suggestions (feel free to unhold)
/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mpryc, razo7 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 |
2 participants
Summary
Adds a reusable GitHub Actions workflow and centralized Go version schemas
to automate Go version updates across all medik8s operator repos.
What's included
go-versions/— flat YAML config files keyed by OCP version (ocp-4.19.yaml,ocp-4.20.yaml,ocp-4.21.yaml), each defining the Go minor version and CI operator image tag.github/workflows/update-go-version.yaml— reusableworkflow_callworkflow that:go.modto the target Go minor version (e.g.go 1.25, not1.25.0)toolchaindirective (patch versions handled by CI builder image)go mod tidy+go mod vendor(when vendor dir exists).ci-operator.yamlbuilder image tagmake buildpassespeter-evans/create-pull-requestDesign principles
go 1.25, notgo 1.25.0curl,grep,sed(pre-installed on GitHub runners)Usage in operator repos
Bumping Go for a new OCP release
go-versions/ocp-X.Y.yamlto this repoTest plan
workflow_dispatchSummary by CodeRabbit
New Features
Documentation