-
Notifications
You must be signed in to change notification settings - Fork 660
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
Remove explict go toolchain versions #5723
Remove explict go toolchain versions #5723
Conversation
- flyteorg#5032 updated builds to use Go 1.22 -- but in practice it looks like flytectl is being built with an outdated toolchain based on Go 1.22.0 instead of Go 1.22.5 or 1.22.6 (i.e. latest) See https://go.dev/doc/toolchain for further details For instance, flytectl v0.9.0 reports being built with Go 1.22.0 ❯ go version -m -v flytectl flytectl: go1.22.0 Ensure the proper Go environment for security reasons Signed-off-by: ddl-ebrown <[email protected]>
b519361
to
c94d2f4
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5723 +/- ##
==========================================
- Coverage 36.16% 36.16% -0.01%
==========================================
Files 1303 1303
Lines 109672 109672
==========================================
- Hits 39668 39665 -3
- Misses 65858 65861 +3
Partials 4146 4146
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
If I build source locally, I get my local 1.23.0 toolchain already
So I'm not sure this PR is the actual solution here b/c I didn't see how flytectl is being built / shipped. |
https://go.dev/doc/toolchain#select has the answer for this. It's kind of convoluted, but essentially, if I verified this by running:
That env var is set in the case of that gh action: https://github.com/flyteorg/flyte/actions/runs/10269861937/job/28416285221#step:3:42. If you run the same code but using the
we use this gh action to release flytectl: https://github.com/flyteorg/flyte/blob/master/.github/workflows/flytectl-release.yml. The last time we released flytectl: https://github.com/flyteorg/flyte/actions/runs/10269861937. Notice how we specify go 1.21 but somehow we end up with go 1.22.0 installed. Reading the code for that specific version of That said, I feel like removing toolchain is the right call. Also, we should probably switch to setting up the go version from the go.mod file (so that it's easier to maintain the right version across go.mod and gh workflows). wdyt, @ddl-ebrown ? |
- flyteorg#5032 updated builds to use Go 1.22 -- but in practice it looks like flytectl is being built with an outdated toolchain based on Go 1.22.0 instead of Go 1.22.5 or 1.22.6 (i.e. latest) See https://go.dev/doc/toolchain for further details For instance, flytectl v0.9.0 reports being built with Go 1.22.0 ❯ go version -m -v flytectl flytectl: go1.22.0 Ensure the proper Go environment for security reasons Signed-off-by: ddl-ebrown <[email protected]> Signed-off-by: Bugra Gedik <[email protected]>
Trying out this idea in #5736. |
Tracking issue
Why are the changes needed?
[Housekeeping] Bump Go version to 1.22 #5032 updated builds to use Go 1.22 -- but in practice it looks like flytectl is being built with an outdated toolchain based on Go 1.22.0 instead of Go 1.22.5 or 1.22.6 (i.e. latest)
For instance, flytectl v0.9.0 reports being built with Go 1.22.0
Ensure the proper Go environment for security reasons
What changes were proposed in this pull request?
How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link