chore: bump Go to 1.25.7, add version check job#19803
chore: bump Go to 1.25.7, add version check job#19803
Conversation
Update go directive to 1.25.7 in root go.mod and tools/linters/go.mod to match the CI container image (apollo-ci:stackrox-test-0.5.3). Add check-go-version job to style workflow that warns when a newer Go patch version is available. Runs on master pushes only. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
check-go-versionjob is gated only ongithub.event_name == 'push', so it will run on all branch pushes rather than justmasteras described; consider narrowing the condition (e.g., torefs/heads/master) if that’s the intent. - In the
Check for newer Go patch versionstep, you rely oncurl/jqwithout error handling, which could yield an empty or invalidlatestvalue; consider addingset -euo pipefailand explicit checks thatlatestis non-empty and well-formed before comparing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `check-go-version` job is gated only on `github.event_name == 'push'`, so it will run on all branch pushes rather than just `master` as described; consider narrowing the condition (e.g., to `refs/heads/master`) if that’s the intent.
- In the `Check for newer Go patch version` step, you rely on `curl`/`jq` without error handling, which could yield an empty or invalid `latest` value; consider adding `set -euo pipefail` and explicit checks that `latest` is non-empty and well-formed before comparing.
## Individual Comments
### Comment 1
<location path=".github/workflows/style.yaml" line_range="373-374" />
<code_context>
+ - name: Check for newer Go patch version
+ run: |
+ # Extract major.minor from go.mod
+ current=$(grep '^go ' go.mod | awk '{print $2}')
+ major_minor="${current%.*}"
+ echo "Current: $current (series: $major_minor)"
+
</code_context>
<issue_to_address>
**suggestion:** Consider handling the case where no `go` directive is found in go.mod
If `grep '^go ' go.mod` finds nothing, `current`/`major_minor` will be empty and the step will print an empty version and compare it to `latest`, leading to a misleading warning instead of failing. Consider checking for an empty `current` and exiting with an error, e.g.:
```sh
[ -z "$current" ] && {
echo '::error::No go directive found in go.mod';
exit 1
}
```
Suggested implementation:
```
- name: Check for newer Go patch version
run: |
# Extract major.minor from go.mod
current=$(grep '^go ' go.mod | awk '{print $2}')
if [ -z "$current" ]; then
echo '::error::No go directive found in go.mod'
exit 1
fi
major_minor="${current%.*}"
echo "Current: $current (series: $major_minor)"
```
There is a second `- name: Check for newer Go patch version` step further down in the `check-go-version` job whose `run:` body is not visible in the snippet. If that step also extracts `current` from `go.mod`, you should apply the same empty-check and error handling pattern there as well to keep behavior consistent across both jobs.
</issue_to_address>
### Comment 2
<location path=".github/workflows/style.yaml" line_range="378-380" />
<code_context>
+ echo "Current: $current (series: $major_minor)"
+
+ # Query the latest patch version from go.dev
+ latest=$(curl -sS "https://go.dev/dl/?mode=json" | \
+ jq -r "[.[].version | select(startswith(\"go${major_minor}\"))] | first" | \
+ sed 's/^go//')
+
+ echo "Latest: $latest"
</code_context>
<issue_to_address>
**suggestion:** Handle cases where the Go downloads API doesn’t return a matching patch version
If the JSON structure changes or there’s no version starting with `go${major_minor}`, the `jq` expression returns `null`, which becomes an empty string after `sed`. That leaves `latest` empty, so the comparison will always emit the warning even though we don’t have a valid value. Consider explicitly handling an empty/`null` `latest` (e.g., error out or skip the comparison) and using `curl -f` so HTTP errors fail the step instead of feeding error pages into `jq`.
</issue_to_address>
### Comment 3
<location path=".github/workflows/style.yaml" line_range="361-364" />
<code_context>
source scripts/ci/lib.sh
slack_workflow_failure
+
+ check-go-version:
+ # Warn if a newer Go patch version is available. The go directive in
+ # go.mod should track the latest patch to match the CI container image.
+ if: github.event_name == 'push'
+ runs-on: ubuntu-latest
+ steps:
</code_context>
<issue_to_address>
**question:** Clarify whether this check should run on all pushes or only on the default branch
Because this job runs on every `push`, any feature branch that bumps `go.mod` will see warnings until `main` is updated and the CI image is rolled. If this check is meant to enforce the canonical CI Go version, consider limiting it to the default branch with a condition like `if: github.event_name == 'push' && github.ref == 'refs/heads/main'` to avoid noisy warnings on short-lived branches.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
.github/workflows/style.yaml
Outdated
| current=$(grep '^go ' go.mod | awk '{print $2}') | ||
| major_minor="${current%.*}" |
There was a problem hiding this comment.
suggestion: Consider handling the case where no go directive is found in go.mod
If grep '^go ' go.mod finds nothing, current/major_minor will be empty and the step will print an empty version and compare it to latest, leading to a misleading warning instead of failing. Consider checking for an empty current and exiting with an error, e.g.:
[ -z "$current" ] && {
echo '::error::No go directive found in go.mod';
exit 1
}Suggested implementation:
- name: Check for newer Go patch version
run: |
# Extract major.minor from go.mod
current=$(grep '^go ' go.mod | awk '{print $2}')
if [ -z "$current" ]; then
echo '::error::No go directive found in go.mod'
exit 1
fi
major_minor="${current%.*}"
echo "Current: $current (series: $major_minor)"
There is a second - name: Check for newer Go patch version step further down in the check-go-version job whose run: body is not visible in the snippet. If that step also extracts current from go.mod, you should apply the same empty-check and error handling pattern there as well to keep behavior consistent across both jobs.
.github/workflows/style.yaml
Outdated
| latest=$(curl -sS "https://go.dev/dl/?mode=json" | \ | ||
| jq -r "[.[].version | select(startswith(\"go${major_minor}\"))] | first" | \ | ||
| sed 's/^go//') |
There was a problem hiding this comment.
suggestion: Handle cases where the Go downloads API doesn’t return a matching patch version
If the JSON structure changes or there’s no version starting with go${major_minor}, the jq expression returns null, which becomes an empty string after sed. That leaves latest empty, so the comparison will always emit the warning even though we don’t have a valid value. Consider explicitly handling an empty/null latest (e.g., error out or skip the comparison) and using curl -f so HTTP errors fail the step instead of feeding error pages into jq.
| check-go-version: | ||
| # Warn if a newer Go patch version is available. The go directive in | ||
| # go.mod should track the latest patch to match the CI container image. | ||
| if: github.event_name == 'push' |
There was a problem hiding this comment.
question: Clarify whether this check should run on all pushes or only on the default branch
Because this job runs on every push, any feature branch that bumps go.mod will see warnings until main is updated and the CI image is rolled. If this check is meant to enforce the canonical CI Go version, consider limiting it to the default branch with a condition like if: github.event_name == 'push' && github.ref == 'refs/heads/main' to avoid noisy warnings on short-lived branches.
Use actions/setup-go with '1.25' (latest patch) and compare against go.mod, instead of querying go.dev API directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated Go toolchain version from 1.25.0 to 1.25.7 in root and tools/linters go.mod files. Added new GitHub Actions workflow job to validate Go version alignment between go.mod and installed Go version in CI. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Use go-version: stable instead of hardcoded '1.25' so the check also catches new minor versions (e.g. 1.26.0). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/style.yaml (1)
376-384: Consider also checkingtools/linters/go.modfor consistency.This PR updates both
go.modandtools/linters/go.modto stay in sync, but the new job only checks the rootgo.mod. If someone updates only the root based on this warning, the linters module could drift.Also, the comment on line 362 says "patch version" but using
go-version: stablewill also flag minor version changes (e.g., 1.26.0). The behavior is fine per PR objectives, but consider updating the comment to match.🔧 Suggested change to check both modules
- name: Check go.mod matches latest Go run: | current=$(grep '^go ' go.mod | awk '{print $2}') + current_linters=$(grep '^go ' tools/linters/go.mod | awk '{print $2}') latest=$(go version | awk '{print $3}' | sed 's/^go//') echo "go.mod: $current" + echo "tools/linters/go.mod: $current_linters" echo "latest: $latest" if [[ "$current" != "$latest" ]]; then echo "::warning::Go $latest is available but go.mod has $current. Update go.mod and the CI container image." fi + if [[ "$current" != "$current_linters" ]]; then + echo "::warning::go.mod ($current) and tools/linters/go.mod ($current_linters) are out of sync." + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/style.yaml around lines 376 - 384, Update the workflow step that checks go.mod to also read and compare the Go version from tools/linters/go.mod (in addition to the root go.mod) and emit a warning if either file's "go " version differs from the current go version; modify the script referenced in the job (the block starting with the run: | that extracts current and latest via grep and go version) to inspect both files and print which file is out-of-sync, and update the comment above the job (the line referencing "patch version") to accurately describe that using go-version: stable can flag minor (and patch) version changes rather than only patch bumps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/style.yaml:
- Around line 376-384: Update the workflow step that checks go.mod to also read
and compare the Go version from tools/linters/go.mod (in addition to the root
go.mod) and emit a warning if either file's "go " version differs from the
current go version; modify the script referenced in the job (the block starting
with the run: | that extracts current and latest via grep and go version) to
inspect both files and print which file is out-of-sync, and update the comment
above the job (the line referencing "patch version") to accurately describe that
using go-version: stable can flag minor (and patch) version changes rather than
only patch bumps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9e532792-df0e-4f51-81ac-96e6898a951c
📒 Files selected for processing (3)
.github/workflows/style.yamlgo.modtools/linters/go.mod
🚀 Build Images ReadyImages are ready for commit 7210abc. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-557-g7210abcac1 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19803 +/- ##
=======================================
Coverage 49.60% 49.60%
=======================================
Files 2763 2763
Lines 208271 208271
=======================================
+ Hits 103317 103323 +6
+ Misses 97288 97284 -4
+ Partials 7666 7664 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Two changes:
1. Bump Go to 1.25.7 — Update
godirective in rootgo.modandtools/linters/go.modfrom 1.25.0 to 1.25.7, matching the CI container image (apollo-ci:stackrox-test-0.5.3).This ensures that
setup-gowithgo-version-file: go.modinstalls the same Go version as the CI container, which matters as we remove containers from CI jobs.go mod tidyproduces identical go.sum — no dependency changes.2. Add
check-go-versionjob — Runs on master pushes in the style workflow. Queries go.dev for the latest patch version and emits a warning if go.mod is behind. Helps catch when the CI container updates Go ahead of go.mod.User-facing documentation
Testing and quality
How I validated my change
go mod tidywith 1.25.7 produces no go.sum changes. CI validation.🤖 Generated with Claude Code