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

chore: Update Go to supported version #100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hairyhenderson
Copy link
Contributor

👋 Go 1.20 has been out of support since February, and the image no longer scans clean (especially due to CVE-2024-24790). This PR updates to Go 1.22.

@hairyhenderson
Copy link
Contributor Author

CI failure seems unrelated (Codecov rate-limiting 🤦‍♂️):

[2024-06-27T12:33:37.883Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 429 - {'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 1202s.', code='throttled')}

@tianon
Copy link
Member

tianon commented Jun 27, 2024

This is actually intentionally at 1.20 to match

go 1.20
(which is also intentional, since it's the minimum version supported) 🙈

@hairyhenderson
Copy link
Contributor Author

1.20 is unsupported though, since 1.22 was released in February...

@hairyhenderson
Copy link
Contributor Author

I suppose updating to Go 1.21 would maybe be the more conservative approach since it's the minimum supported version at the moment, however it will cease to be in August when Go 1.23 is out 😉

@tianon
Copy link
Member

tianon commented Jun 28, 2024

Maybe instead we can meet in the middle somewhere by updating all the images to latest Go and then add something using actions/setup-go to validate our minimum supported version in a minimal way 🤔

(I guess we could also do something hanky like --build-context or sed on our test Dockerfile to force the older version for testing)

@hairyhenderson
Copy link
Contributor Author

Sure, testing against a matrix of versions sounds reasonable.

Ultimately I guess what I'm not quite grasping is what you mean by "minimum supported version"...

@tianon
Copy link
Member

tianon commented Jul 1, 2024

Oh, I think https://research.swtch.com/vgo-mvs is perhaps the best reference about that -- the design of Go modules is such that any dependency which requires a newer version of something effectively forces anything using that dependency to use the newer thing too, so we try to ensure our dependencies are sufficiently minimal (in other words, our builds of this code are not the only builds of this code that happen).

When updating something like the version of Go we build against without also making sure we're still at least smoke testing that older version means that eventually we might accidentally use a newer feature than we're "allowed" to given that other constraint, so I want to make sure that we still have something verifying that older version (such that if we think some new Go feature is worth bumping that minimum version, it becomes an explicit choice/action, not something that sneaks in accidentally). Up until now, the easiest way to do that was to simply have the two versions match without drift (our version and our minimal version), but it is reasonable for them to disconnect.

@hairyhenderson
Copy link
Contributor Author

Ok, I think we're experiencing an impedance mismatch here 😉

What I mean by "supported" in this context has nothing to do with MVS or any sort of compatibility, but rather Go's support policy as documented here: https://go.dev/doc/devel/release#policy

The scan failure that led me to issue this PR is an example of somewhere where the Go 1.20 release didn't get patched for CVE-2024-24790 because it was out of support.

The other PR bumps the go entry in go.mod, which is why I didn't do it here. But if you'd like me to make the change here too, just say the word.

without also making sure we're still at least smoke testing that older version means that eventually we might accidentally use a newer feature than we're "allowed" to given that other constraint

Go will already prevent you from using 1.22-specific features if the go.mod does not contain an appropriate go line (this is relatively new behaviour, to be sure - the line used not to mean much of anything, but now it does).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants