Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/vuln/cmd/govulncheck: check against toolchain/go directive version #62050

Open
zpavlinovic opened this issue Aug 15, 2023 · 7 comments
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo

Comments

@zpavlinovic
Copy link
Contributor

Should govulncheck use the Go version specified by toolchain or go directive in go.mod files? Currently, we use the Go version at PATH.

This issue was created based on discussions here and here.

@zpavlinovic zpavlinovic added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo labels Aug 15, 2023
@seankhliao seankhliao changed the title golang.org/x/vuln/cmd/govulncheck: check against toolchain/go directive version x/vuln/cmd/govulncheck: check against toolchain/go directive version Aug 15, 2023
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Aug 15, 2023
@gaby
Copy link

gaby commented Aug 31, 2023

This broke all our govulncheck CI jobs when running with setup-go and 1.21.x. Our go.mod specify go1.18 and they won't start unless we run go mod tidy before hand.

@zpavlinovic
Copy link
Contributor Author

zpavlinovic commented Aug 31, 2023

If this is happening in Github Actions, then I suggest for the time being looking into govulncheck-action. It recently added support for specifying whether the version in go.mod should be used. (it can be fetched at master currently, but we'll release a new version soon)

@gaby
Copy link

gaby commented Aug 31, 2023

@zpavlinovic That's basically forcing people to change a their CI files, we have +30 files. This shouldnt be a breaking change.

I don't understand why the Go team is forcing a toolchain. The default value for GOTOOLCHAIN should be "local" or "path", with "auto" go forces you to run go mod tidy if your go.mod files specify a version other than 1.21.

@ianlancetaylor
Copy link
Member

We are trying to find the approach that is most useful for most people. It seems to us that most people are not running Go in a CI, they are running Go from the command line. We've tried to document the new behavior as many places as we could. Sorry for the trouble.

@gaby
Copy link

gaby commented Aug 31, 2023

@ianlancetaylor What are all the teams developing applications in Go using to run tests/etc on their CI if not for Go itself?

We also noticed that all the dependabot jobs stopped and are failing because when go mod tidy runs it cant parse go 1.21 as seen here #62278

Also related dependabot/dependabot-core#7895

@ianlancetaylor
Copy link
Member

Personally I expect them to use Go, and I expect them to set GOTOOLCHAIN=local in the environment or in a go.env file, as discussed at https://go.dev/doc/toolchain. I agree that there is a transition cost, but I think we can get through it.

@gaby
Copy link

gaby commented Sep 1, 2023

Personally I expect them to use Go, and I expect them to set GOTOOLCHAIN=local in the environment or in a go.env file, as discussed at https://go.dev/doc/toolchain. I agree that there is a transition cost, but I think we can get through it.

Not sure what to say, when even Dependabot doesnt work anymore. I will remove 1.21 from the CI, until a solution is available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

4 participants