ci go improvements#1631
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates GitHub Actions workflows for Go to derive the Go toolchain version from go.mod and ensures the govulncheck workflow also runs when go.mod changes.
Changes:
- Update
govulncheckworkflow triggers to includego.modchanges. - Switch Go installation in
govulncheckand CI jobs from a hard-coded version togo-version-file: go.mod. - Remove
check-latest: truefrom Go setup steps in the main CI workflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| .github/workflows/govulncheck.yml | Trigger govulncheck on go.mod changes and install Go based on go.mod. |
| .github/workflows/ci.yml | Install Go based on go.mod across Go-related CI jobs (tests/lint/manifests). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 | ||
| with: | ||
| go-version: "1.26" | ||
| check-latest: true # Always check for the latest patch release | ||
| go-version-file: "go.mod" | ||
| - run: make test-go |
There was a problem hiding this comment.
actions/setup-go is now driven by go.mod, but this step no longer sets check-latest: true. That can lead to CI running a different (and potentially older) Go patch level than other workflows (e.g. govulncheck still forces the latest patch). Consider re-adding check-latest: true here (or alternatively pin via a toolchain go1.x.y directive in go.mod if you want deterministic patch selection).
| with: | ||
| go-version: "1.26" | ||
| check-latest: true # Always check for the latest patch release | ||
| go-version-file: "go.mod" |
There was a problem hiding this comment.
This setup-go invocation switched to go-version-file, but dropped check-latest: true. If the intent is still “latest patch for the declared Go version”, re-add check-latest: true so e2e tests don’t run against a different patch than other Go jobs.
| go-version-file: "go.mod" | |
| go-version-file: "go.mod" | |
| check-latest: true |
| - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 | ||
| with: | ||
| go-version: "1.26" | ||
| check-latest: true # Always check for the latest patch release | ||
| go-version-file: "go.mod" | ||
| - name: golangci-lint |
There was a problem hiding this comment.
Same as other Go jobs: after switching to go-version-file, check-latest: true was removed. Unless you intentionally want to test an older cached patch, re-add check-latest: true to keep linting aligned with the patch level used elsewhere.
| with: | ||
| go-version: "1.26" | ||
| check-latest: true | ||
| go-version-file: "go.mod" |
There was a problem hiding this comment.
This job also switched to go-version-file without check-latest: true. If the goal is “use Go version from go.mod but always get the newest patch”, add check-latest: true here to avoid running controller-gen/manifests generation on a different patch level than other Go checks.
| go-version-file: "go.mod" | |
| go-version-file: "go.mod" | |
| check-latest: true |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1631 +/- ##
==========================================
- Coverage 80.48% 80.46% -0.03%
==========================================
Files 127 127
Lines 16411 16411
==========================================
- Hits 13209 13205 -4
- Misses 3202 3206 +4
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:
|
| go-version: "1.26" | ||
| go-version-file: "go.mod" |
There was a problem hiding this comment.
We do not need this step. govulncheck-action already install go. You can define the go.mod file in its inputs as well
Considering the action has the check-latest as true by default. I suspect that means the go lang version will be update under the hood. That's what copilot is warning us. Therefore, I would say to remove the setup-go step here and move this configuration to the govulncheck-action instead.
There was a problem hiding this comment.
I think I need to remove the chack-latest, then the govulncheck is going to use the version cached by this step (hence the one we have inside of go.mod:
The precedence for inputs go-version-input, go-version-file, check-latest, cache, and cache-dependency-path specifying Go version and caches is inherited from actions/setup-go
From the README of the project.
What do you think?
There was a problem hiding this comment.
Ah, I just saw the check-latest in this setup-go action. But I think you cannot remove the input. Actually, we need to keep the go version configuration in sync with the govulncheck input values. Otherwise, you will install a go version here and govulncheck may update the version before run.
jvanz
left a comment
There was a problem hiding this comment.
I think we need to do a little change. See my comment in the govulncheck.yml file
Check also `go.mod`, plus use the same version of go mentioned inside of `go.mod`. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Checkout the source code to ensure `go.mod` is available. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
511e050 to
b0d018d
Compare
Apply lessons learnt from sbomscanner Signed-off-by: Flavio Castelli <fcastelli@suse.com>
|
@jvanz ready for review again |
jvanz
left a comment
There was a problem hiding this comment.
Thanks for the changes! But I think we need to consider the copilot comments. Just to ensure that we are run the same go version in all CI jobs. Maybe we can follow the suggestion from its first comment and define the version including the patch version.
| go-version-input: "" | ||
| go-version-file: "go.mod" |
There was a problem hiding this comment.
I would include the input to disable patch version updates
This PR does some improvements to some GHA that deal with Go.
We now install the same version of go that is mentioned inside of the
go.modfile. Moreover, we intruct govulncheck to look also at thego.mod.