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

Update go version to 1.22.4 #4919

Merged
merged 9 commits into from
Jun 18, 2024
Merged

Update go version to 1.22.4 #4919

merged 9 commits into from
Jun 18, 2024

Conversation

ffjlabo
Copy link
Member

@ffjlabo ffjlabo commented May 20, 2024

What this PR does / why we need it:

Update go version to 1.22.4

TODO:

  • Update the base image of Dockerfiles and go.mod in tools/
    • actions-gh-release
    • actions-plan-preview
    • codegen
  • Update the base image of Dockerfiles in docs/
  • Update go version in docs document.
  • Update env.GO_VERSION in github actions workflow definitions.
    • test.yaml
    • publish_image_chart.yaml
    • publish_binary.yaml
    • lint.yaml
    • test_tool.yaml
    • build.yaml
  • Update go.mod in the root of the repo.

Which issue(s) this PR fixes:

Fixes #4874

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

Copy link

codecov bot commented May 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.01%. Comparing base (9b82e65) to head (f2322e7).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4919      +/-   ##
==========================================
- Coverage   29.32%   22.01%   -7.32%     
==========================================
  Files         323      518     +195     
  Lines       40984    57164   +16180     
==========================================
+ Hits        12020    12585     +565     
- Misses      28002    43561   +15559     
- Partials      962     1018      +56     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@t-kikuc
Copy link
Member

t-kikuc commented May 21, 2024

/review

Copy link
Contributor

PR Analysis

Main theme

"Update Go versions"

PR summary

"This PR updates the Go version to 1.22.3 across various Dockerfiles, GitHub actions configurations, and go.mod files."

Type of PR

"Enhancement"

PR Feedback:

General suggestions

The PR increases the Go version used for building and testing to 1.22.3 across different workflows and Docker images. Updating to this version is beneficial for leveraging improvements and security fixes introduced in the latest Go release. It is crucial to ensure all dependencies are compatible with the new Go version and the upgrade does not introduce any building or testing issues. Verify through testing that the update does not break the existing functionality.

Code feedback

  • relevant file: .github/workflows/build.yaml
    suggestion: "Ensure that all dependencies and code are compatible with Go 1.22.3 as this will be the new version used in your build process." (important)
    relevant line: + GO_VERSION: 1.22.3

  • relevant file: docs/Dockerfile
    suggestion: "The use of Go 1.22.3 for building the documentation server should be validated to ensure that the building process is not affected by the version change." (important)
    relevant line: +FROM golang:1.22.3-alpine3.19 AS builder

  • relevant file: go.mod
    suggestion: "By updating the Go directive to 1.22, you are declaring that your code uses features or behaviors introduced in that Go version. This implies compatibility with Go 1.22's modules behavior. Make sure all module dependencies are tested and compatible with Go 1.22." (important)
    relevant line: +go 1.22

  • relevant file: go.sum
    suggestion: "It's good practice to verify that the checksums stored in go.sum after the Go version upgrade align with the expected hashes to ensure module authenticity." (medium)
    relevant line: +github.com/google/martian/v3 v3.3.2/go.mod h1:oBOf6HBosgwRXnUGWUB05QECsc6uvmMiJ3+6W4l/CUk=

  • relevant file: cmd/pipecd/README.md
    suggestion: "The update to Go 1.22 in the README should also link to Go's release notes or documentation providing users insights into what changes come with the Go 1.22 release." (medium)
    relevant line: +- [Go 1.22 or later](https://go.dev/)

Security concerns:

"no"

The changes are focused on the Go version update, which should not lead to any security concerns as long as the used dependencies are secure and up-to-date with the newer Go version. The maintainers should verify that dependencies are indeed compatible and free from vulnerabilities that might affect the newer environment.

khanhtc1202
khanhtc1202 previously approved these changes May 21, 2024
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement, thanks 🐇
I left a nits comment, please check it before merging 👍

t-kikuc
t-kikuc previously approved these changes May 21, 2024
Copy link
Member

@t-kikuc t-kikuc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work, thank you so much! 🚀


COPY protoc-gen-auth /protoc-gen-auth
RUN cd /protoc-gen-auth \
&& go build -o /usr/local/bin/protoc-gen-auth . \
&& chmod +x /usr/local/bin/protoc-gen-auth

# Codegen image which is actually being used.
FROM golang:1.20.4-alpine3.16
FROM golang:1.22.3-alpine3.19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please be sure that this image works with the specified protoc version. I have a hard memory with this update 😢

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can try this locally using this command with image version point to your built image in the local
https://github.com/pipe-cd/pipecd/blob/master/Makefile#L202

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it and encountered the error as you said 😇

% make gen/code                                                                                 
# NOTE: Specify a specific version temporally until the next release.
docker run --rm -v /Users/s14218/oss/pipe-cd/pipecd:/repo -it --entrypoint ./tool/codegen/codegen.sh codegen-test /repo #v0.44.0-38-g7229285

- pkg/model
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/apiservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/pipedservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/webservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/helloworld/service
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/plugin/api/v1alpha1/platform
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/pipedv1/cmd/piped/service
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/model
deleting previously generated JS files...
successfully deleted
generating new JS files...
protoc-gen-js: program not found or is not executable
Please specify a program using absolute path or make sure the program is available in your PATH system variable
--js_out: protoc-gen-js: Plugin failed with status code 1.
make: *** [gen/code] Error 1

Copy link
Member Author

@ffjlabo ffjlabo May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be the problem, as the protobuf doesn't include the protocol-gen-js around v3.21.
This base image uses alpine 3.19, which can install protobuf but only v24.4-r0.
https://pkgs.alpinelinux.org/packages?name=protobuf&branch=v3.19&repo=&arch=&maintainer=

Also the alpine linux v3.16 is the final version which can install the proper one.

There is a way to build protoc-gen-js from the source someone said.
protocolbuffers/protobuf-javascript#127 (comment)

So, I tried to build protoc-gen-js from the source with the multi-stage build and copy it to the image like this.
Finally, this is wrong with the libc problem qemu-x86_64: Could not open '/lib64/ld-linux-x86-64.so.2': No such file or directory.

...
# build `protoc-gen-js`
FROM gcr.io/bazel-public/bazel as PROTOC_GEN_JS_BUILDER
WORKDIR /protoc-gen-js

RUN git clone https://github.com/protocolbuffers/protobuf-javascript \
  && cd protobuf-javascript/ \
  && bazel build //generator:protoc-gen-js \
  && cp bazel-bin/generator/protoc-gen-js /protoc-gen-js

# Codegen image which is actually being used.
FROM golang:1.22.3-alpine3.19
...
# protoc-gen-js
COPY --from=PROTOC_GEN_JS_BUILDER /protoc-gen-js/protoc-gen-js /usr/local/bin/
# Note: codegen-final is the image bult using above
% make gen/code                                                                                              (git)-[update-go-version]
# NOTE: Specify a specific version temporally until the next release.
docker run --rm -v /Users/s14218/oss/pipe-cd/pipecd:/repo -it --entrypoint ./tool/codegen/codegen.sh codegen-final /repo #v0.44.0-38-g7229285

- pkg/model
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/apiservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/pipedservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/webservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/helloworld/service
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/plugin/api/v1alpha1/platform
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/pipedv1/cmd/piped/service
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/model
deleting previously generated JS files...
successfully deleted
generating new JS files...
qemu-x86_64: Could not open '/lib64/ld-linux-x86-64.so.2': No such file or directory
--js_out: protoc-gen-js: Plugin failed with status code 255.
make: *** [gen/code] Error 1

Copy link
Member Author

@ffjlabo ffjlabo May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I will use the old go version for codegen image for now.
BTW, I think we should not use protoc-gen-js and find other protoc plugin for js.

Copy link
Member Author

@ffjlabo ffjlabo May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for many times. I forgot to check it using Makefile 🙏
I tried and there are some warnings. So I'll check it.

generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/golang/mock/mockgen/model
go: finding module for package github.com/pipe-cd/pipecd/pkg/insight
successfully generated

Updated: Also, it is strange behavior. service.pb.auth.go has the diff after doing so.

Entire logs

% make gen/code                                                                                
# NOTE: Specify a specific version temporally until the next release.
docker run --rm -v /Users/s14218/oss/pipe-cd/pipecd:/repo -it --entrypoint ./tool/codegen/codegen.sh codegen-with-go1.22.3-builder /repo #v0.44.0-38-g7229285

- pkg/model
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/apiservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/pipedservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/server/service/webservice
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/helloworld/service
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/plugin/api/v1alpha1/platform
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/app/pipedv1/cmd/piped/service
deleting previously generated Go files...
successfully deleted
generating new Go files...
successfully generated

- pkg/model
deleting previously generated JS files...
successfully deleted
generating new JS files...
successfully generated

- pkg/app/server/service/webservice
deleting previously generated JS files...
successfully deleted
generating new JS files...
successfully generated

- pkg/redis/redistest/redis.mock.go
generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/pipe-cd/pipecd/pkg/redis
go: finding module for package github.com/golang/mock/mockgen/model
go: downloading github.com/pipe-cd/pipecd v0.47.2
go: downloading github.com/gomodule/redigo v2.0.0+incompatible
successfully generated

- pkg/datastore/datastoretest/datastore.mock.go
generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/pipe-cd/pipecd/pkg/datastore
go: finding module for package github.com/golang/mock/mockgen/model
go: downloading github.com/envoyproxy/protoc-gen-validate v0.10.1
go: downloading golang.org/x/crypto v0.21.0
go: downloading golang.org/x/oauth2 v0.7.0
go: downloading google.golang.org/protobuf v1.33.0
successfully generated

- pkg/filestore/filestoretest/filestore.mock.go
generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/pipe-cd/pipecd/pkg/filestore
go: finding module for package github.com/golang/mock/mockgen/model
successfully generated

- pkg/app/piped/platformprovider/kubernetes/kubernetestest/kubernetes.mock.go
generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/pipe-cd/pipecd/pkg/app/piped/platformprovider/kubernetes
go: finding module for package github.com/golang/mock/mockgen/model
go: downloading k8s.io/api v0.24.3
go: downloading go.uber.org/zap v1.10.1-0.20190709142728-9a9fa7d4b5f0
go: downloading k8s.io/apimachinery v0.24.3
go: downloading k8s.io/client-go v0.24.3
go: downloading sigs.k8s.io/yaml v1.2.0
go: downloading golang.org/x/sync v0.1.0
go: downloading github.com/prometheus/client_golang v1.11.1
go: downloading github.com/creasty/defaults v1.6.0
go: downloading github.com/golang/protobuf v1.5.3
go: downloading go.uber.org/atomic v1.7.0
go: downloading go.uber.org/multierr v1.2.0
go: downloading k8s.io/klog/v2 v2.60.1
go: downloading github.com/gogo/protobuf v1.3.2
go: downloading sigs.k8s.io/structured-merge-diff/v4 v4.2.1
go: downloading gopkg.in/yaml.v2 v2.4.0
go: downloading github.com/beorn7/perks v1.0.1
go: downloading github.com/cespare/xxhash/v2 v2.2.0
go: downloading github.com/prometheus/client_model v0.2.0
go: downloading github.com/prometheus/common v0.26.0
go: downloading github.com/prometheus/procfs v0.6.0
go: downloading github.com/google/gofuzz v1.1.0
go: downloading sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2
go: downloading github.com/go-logr/logr v1.2.0
go: downloading github.com/json-iterator/go v1.1.12
go: downloading github.com/matttproud/golang_protobuf_extensions v1.0.1
go: downloading gopkg.in/inf.v0 v0.9.1
go: downloading k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9
go: downloading golang.org/x/sys v0.18.0
go: downloading golang.org/x/net v0.23.0
go: downloading github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd
go: downloading github.com/modern-go/reflect2 v1.0.2
go: downloading golang.org/x/term v0.18.0
go: downloading github.com/davecgh/go-spew v1.1.1
go: downloading golang.org/x/time v0.1.0
go: downloading golang.org/x/text v0.14.0
successfully generated

- pkg/cache/cachetest/cache.mock.go
generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/pipe-cd/pipecd/pkg/cache
go: finding module for package github.com/golang/mock/mockgen/model
successfully generated

- pkg/git/gittest/git.mock.go
generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/pipe-cd/pipecd/pkg/git
go: finding module for package github.com/golang/mock/mockgen/model
successfully generated

- pkg/jwt/jwttest/jwt.mock.go
generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/pipe-cd/pipecd/pkg/jwt
go: finding module for package github.com/golang/mock/mockgen/model
go: downloading github.com/golang-jwt/jwt v3.2.1+incompatible
successfully generated

- pkg/insight/insighttest/insight.mock.go
generating mock...
go: errors parsing go.mod:
/repo/go.mod:5: unknown directive: toolchain
go: finding module for package github.com/golang/mock/mockgen/model
go: finding module for package github.com/pipe-cd/pipecd/pkg/insight
successfully generated

Successfully generated all code
less

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to update codegen image in order to ensure this passes test codegen CI step 👀 Also why does this golang change PR effect this? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ffjlabo I will fix this code generate failed behavior 🙏

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I investigated, the codegen script works until codegen image v0.46.0. I can narrow down the reason for this strange behavior is caused by this PR 🔍
#4832

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This problem was fixed by changing the base image and nit code fix 🙏
#4968

@@ -1,5 +1,7 @@
module github.com/pipe-cd/pipecd/tool/codegen/protoc-gen-auth

go 1.20
go 1.22
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 update with go get -u go

@@ -1,5 +1,5 @@
# Builder image to build go program.
FROM golang:1.20.4-alpine3.16 as BUILDER
FROM golang:1.22.3-alpine3.19 as BUILDER
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use older version because we don't need to use the latest for codegen.

@khanhtc1202
Copy link
Member

@ffjlabo I created this issue for codegen image update. Let's address the codegen problem by that issue and clean up this PR 👍
#4963

…s/ to 1.22

- actions-gh-release
- actions-plan-preview
- codegen

Signed-off-by: Yoshiki Fujikane <[email protected]>
Signed-off-by: Yoshiki Fujikane <[email protected]>
@ffjlabo ffjlabo requested a review from Warashi as a code owner June 14, 2024 03:39
Copy link
Contributor

@Warashi Warashi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A toolchain go1.22.4 has been released. So let's use this.

@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 14, 2024

Thank you for the comment! I'm updating to go 1.22.4 now. 👍

@ffjlabo ffjlabo changed the title Update go version to 1.22 Update go version to 1.22.4 Jun 14, 2024
@ffjlabo ffjlabo requested a review from Warashi June 14, 2024 05:31
Copy link
Contributor

@Warashi Warashi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you go 🚀

@khanhtc1202 khanhtc1202 merged commit d03c7b1 into master Jun 18, 2024
22 of 25 checks passed
@khanhtc1202 khanhtc1202 deleted the update-go-version branch June 18, 2024 09:03
@github-actions github-actions bot mentioned this pull request Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to Go 1.22
4 participants