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

Golang update to 1.18 #2153

Closed
taoyuanyuan opened this issue Oct 27, 2022 · 15 comments · Fixed by #2160
Closed

Golang update to 1.18 #2153

taoyuanyuan opened this issue Oct 27, 2022 · 15 comments · Fixed by #2160
Milestone

Comments

@taoyuanyuan
Copy link
Contributor

The New Feature

Describe the new feature you want to support clearly.

Your scenes

Describe your use scenes (why need this feature).

Your advice

Describe the advice or solution of this new feature.

Environment

  • MOSN Version
@taoyuanyuan taoyuanyuan added this to the 1.3.0 milestone Oct 27, 2022
@taoyuanyuan taoyuanyuan assigned 3062 and unassigned 3062 Oct 27, 2022
codefromthecrypt added a commit to codefromthecrypt/mosn that referenced this issue Oct 31, 2022
This updates to go 1.18 and converts build tags appropriately. This took
out special casing for go <1.11.

Fixes mosn#2153

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor

#2160

@codefromthecrypt
Copy link
Contributor

As a result of this I think we should document that 1.18 is the floor version (due to it being the floor version Go supports). Even if we are slow to move the floor up again it will help. Imagine libraries will start using any and any of those dependencies mosn uses will break if we decide that it is not really 1.18 but 1.18 and also 1.16. Whatever the decision is it should be clear. It would be even better to test on a matrix if deciding to formalize support for very old versions of go.

@codefromthecrypt
Copy link
Contributor

@taoyuanyuan clearly @antJack does not want go 1.18 to be the minimum version. What's not clear yet, is what is the minimum version. For example, grpc-go which is one of your deps is 1.16-1.19 at the moment https://github.com/grpc/grpc-go/blob/master/.github/workflows/testing.yml#L36-L71 On the other hand, grpc is more a library than a process, and often times processes can decide more narrow versions as it is more convenient and also helps deal with the many different version ranges of their dependencies. For example, you could inherit go's range, which I thought we were doing, or go only 1 version below it instead of 2 or more. The problem of not setting a version policy is people don't know when it is ok to update anything. You'll notice the library dependencies are severely out of date, not just fasthttp. Without knowing the go version, it is hard to decide which deps can be updated vs left stale in fear of violating some go version. So instead of keeping up with CVEs, we just don't update? I think it is better to make a policy and be progressive and forward. People can upgrade go on their local laptops and should.

my 2p

@codefromthecrypt
Copy link
Contributor

fasthttp also dropped 1.15 valyala/fasthttp#1379

I'd recommend auditing the core dependencies to make sure whatever mosn policy isn't wider than possible to achieve.

@codefromthecrypt
Copy link
Contributor

added mosn/api#51 to try to reach a point of transparency as without it, it is very hard to decide what effort should be made on this project. For example, the current wasm integration is extremely inefficient. The wasmer option not only uses CGO, but it also uses a lot of reflection. Also that project hasn't done a release since last august. If folks want to modernize, we need a transparent policy on Go.

@codefromthecrypt
Copy link
Contributor

I will start documenting the revlocks on this issue.

First is github.com/uber/jaeger-client-go which is revlocked because current version doesn't compile with go 1.16 anymore. It needs at least 1.17

@codefromthecrypt
Copy link
Contributor

as mentioned on another thread, grpc-go and fasthttp have floor version of 1.16, so are revlocked.. except we already use latest fasthttp so implicitly can't set the floor below 1.16.

@codefromthecrypt
Copy link
Contributor

go.uber.org/zap also does not build below 1.17 anymore, so this version is revlocked unless minimum is of mosn 1.17

@codefromthecrypt
Copy link
Contributor

github.com/miekg/dns is also revlocked unless floor version is at least 1.17

@codefromthecrypt
Copy link
Contributor

I'll stop because I think it is clear the minimum can't be below 1.17 without forgoing security and bugfixes in core dependencies. Because this is already far beyond the 1.14, certainly 1.11, I think it is better to move to 1.18 vs tell people to move from 1.14 then to 1.17 and again to 1.18 soon. Please think about it and let me know.

@taoyuanyuan
Copy link
Contributor Author

Thank you very much for your suggestion that indeed we need a mosn policy, we have also discussed this issue and we can refer to Go policy, but to ensure better stability, we will delay a version, for example, Go policy now is 1.17-1.19, our policy will be 1.16-1.18, 1.16 is the minimum version, 1.18 is the recommended version.

@codefromthecrypt
Copy link
Contributor

@taoyuanyuan actually to be clear go policy isn't 1.17-1.19, it is 1.18-1.19. There is impact including revlock I mentioned about using a version before 1.17 which is already below go's policy. For example, I need to close the wazero changes and wait for you to at least get to 1.17

Knowing the policy you are suggesting is 2 versions under Go's and also keeps people from upgrading things like Zap, Are you sure you want to do that?

https://go.dev/doc/devel/release#policy

@taoyuanyuan
Copy link
Contributor Author

@codefromthecrypt my mistake that I thought Go was 1.17-1.19, if Go is 1.18-1.19, then mosn's policy is 1.17-1.18.
However, we do not recommend upgrading all vendors to the latest version directly, and upgrading the versions individually when someone has a need for new features or critical CVEs.

@codefromthecrypt
Copy link
Contributor

@taoyuanyuan ok I don't plan to update the other libraries. Just note some are very out of date. ex grpc is 12 versions behind latest. You may want to consider occasionally upgrading to keep up to date, but that's up to you.

For me, I'll go back and add the test matrix.

Should we say that mosn binaries are built with latest Go? (currently 1.19) or Go's floor (currently 1.18). Basically even if we test on 3 versions we can only build once.

@taoyuanyuan
Copy link
Contributor Author

@codefromthecrypt built with Go's floor, thx~

codefromthecrypt added a commit to codefromthecrypt/pkg that referenced this issue Nov 1, 2022
cc @taoyuanyuan @antJack

See mosn/mosn#2153

Signed-off-by: Adrian Cole <adrian@tetrate.io>
codefromthecrypt added a commit to codefromthecrypt/holmes that referenced this issue Nov 2, 2022
This uses a test matrix to ensure the floor version of Mosn is two
behind latest Go, even if one behind latest (currently 1.18) is used to
build binaries.

See mosn/mosn#2153

Signed-off-by: Adrian Cole <adrian@tetrate.io>
codefromthecrypt added a commit to codefromthecrypt/proxy-wasm-go-host that referenced this issue Nov 2, 2022
This also lowers go.mod to 1.17 accordingly

cc @taoyuanyuan @antJack

See mosn/mosn#2153

Signed-off-by: Adrian Cole <adrian@tetrate.io>
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 a pull request may close this issue.

3 participants