Adopt Tool Dependencies

[hanzei]

Summary

Update the go version to go1.24. That allows us to to use Tool Dependencies, reducing the amount of custom Makefile code.

Ticket Link

None

[fmartingr]

This looks good to me, but two questions tho:

Should we keep the plugins go version one major version behind the same way we do in the Mattermost server or are we deferring that decision to the plugin maintainers?

Should we involve prodsec in the approval (as per the above)? (Not sure since this is not an actual plugin, but will became several plugins over time).

[hanzei]

Should we keep the plugins go version one major version behind the same way we do in the Mattermost server or are we deferring that decision to the plugin maintainers?

Due to the long development cycles, new plugins will (naturally) fall behind in Go versions. By the time a plugin is released, the Go release cycle has moved forward significantly. I'm more concerned about outdated Go versions then (too) new ones. Hence, the push to update early.

@enzowritescode Thoughts?

[enzowritescode]

2/5 I prefer if the versions are the same everywhere so we aren't having to keep track of multiple versions of everything. It makes managing potential vulnerabilities easier. It may also prevent confusion for everyone working on multiple projects not having to worry about the Go version and if the thing they want to use is available or not in that version.

I think there is some historical knowledge that I am missing on the versioning selection though so maybe @esarafianou or @jupenur can chime in.

[hanzei]

cc @streamer45 as worked on go version updates recently

[streamer45]

I agree on being consistent. We could probably add a step to take care of this to our existing Playbook. /cc @agarciamontoro

The existing strategy is to always be one version behind the tip. So when 1.24 went out, we updated our pipelines to 1.23. The rationale is to limit breaking changes and potential security vulnerabilities that often come in new versions.

[wiggin77]

+1 on alignment with MM server. I think breaking changes in latest Go is more likely to affect us than being on slightly older versions.

[agarciamontoro]

I agree that consistency is important, and having to change Go versions between projects we control would be a bit painful.

We could probably add a step to take care of this to our existing Playbook. /cc @agarciamontoro

@streamer45, do you mean updating plugins in general? This starter template in particular?

[esarafianou]

+1 on alignment with MM server. Claudio described the existing strategy and the rationale perfectly.

[lieut-data]

No objection to matching the MM server, but one note:

I agree that consistency is important, and having to change Go versions between projects we control would be a bit painful.

Is this an issue in practice, @agarciamontoro, after https://go.dev/doc/toolchain?

[hanzei]

Thanks, everyone, for chiming in! I love to see so many people jumping into the discussion.

I see the point of being consistent with the MM server, though we did have issues in the past where we adopted too slowly (e.g., the plugin RPC issue).

However, this PR, at its core, aims to make the Go versions of plugins more consistent with the MM server. By putting the Starter Template one version ahead of the MM server, we give Plugins a head start - which they need. To name some examples: The AI plugin uses go1.21 (an unsupported version). JIRA, GitLab, GitHub, and Zoom are all on go1.22. I don't expect plugins to be too far ahead of the server; I expect them to be behind. And this PR tries to help with that.

I also wonder how other Go projects handle updates. For example, Tailscale updated to go1.24 for their production release a month ago (tailscale/tailscale#15016).

[wiggin77]

By putting the Starter Template one version ahead of the MM server, we give Plugins a head start - which they need.

I don't think this is really an issue. It is not hard to upgrade the Go version in any plugin. I think adopting the latest Go version too early will bite us far more often than upgrading slowly, despite one potential example outlier. I like the one version behind policy we have now, and I think the starter-template should sync with server.

[hanzei]

Ci seems to be broken by mattermost/actions-workflows#71

[hanzei]

The server has been updated to go1.24. I think we are good to proceed with this PR.

[hanzei]

/update-branch

[lieut-data]

Actually, I just tried doing this on the monorepo and realized it merges the dependencies of the tools into the core go.mod. I'm worried this increases the surface area we have to track for security vulnerabilities, plus we're doing version management on a larger set of binaries, some of which are outside of our control.

It seems like the Go team identified this concern, but didn't address it with v1.24. @agnivade, thoughts on this? I think we should be consistent one way or another: if it's ok, let's do it everywhere.

[agnivade]

Actually, I just tried doing this on the monorepo and realized it merges the dependencies of the tools into the core go.mod.

Could you clarify this a bit more? Which tools are we talking about? Tools like mockery, gotestsum, golangci-lint?

[lieut-data]

Could you clarify this a bit more? Which tools are we talking about? Tools like mockery, gotestsum, golangci-lint?

I've pushed the branch to illustrate, but yes all of those tools and more: mattermost/mattermost@master...go-tool

[agnivade]

Ahh sorry, I didn't even read the PR message properly. I missed that this PR was about the go tool command and upgrading Go toolchain.

I don't follow Go design discussions that closely now. But this seems like a more formalized implementation of the go.tools.mod file we had in the past. We moved from that to simple go install binary@version commands because they were simpler to maintain.

There might be cases where a company wants to track dependencies of tools more closely, but I think the cost we are paying by having to track all tool dependencies in our go.mod is higher than the benefit of reducing a few lines in Makefile.

I'd be of the opinion to keep using go install to install our tools, and keep our go.mod clean.

[lieut-data]

I'd be of the opinion to keep using go install to install our tools, and keep our go.mod clean.

I'm 👍 on that, though I admit that go tool was /much/ faster, because there's no network lookup for the go install step at all. It's unfortunate the Go team mixed these two notions together. I imagine we'll end up with a devDependencies block eventually :p

[hanzei]

Thanks for sharing your thoughts, @agnivade and @lieut-data. I agree that the default way the goo tool works has its downsides. (The community seems to agree on that, see here and here).

I did play around with using a separate tools.mod file. That eliminates the concern of bloating our go.mod file and hopefully keeps the security scanner in check (Thought, I think Security appreciates the option of tracking the supply chain of our tools). The cost is a bit of extra complexity, which is partly handled by the makefile.

Overall, I'm leaning towards using the separate go.mod file alongside go tool for two main reasons:

1. Speed: As @lieut-data pointed out, go tools X is much faster then go install X && ~/go/bin/X. Developers (and AI!) run these tools every day, a dozen times. Every second matters for the development experience here
2. Security: The version of each tool is pinned via tools.sum, making supply chain attacks much harder. That is not much of a big deal for the plugins repo, but it is a big one for the Mono Repo, as that contains closed-source code that we don't want to leak.

What do you think about this solution? Please let me know your thoughts.

[lieut-data]

I'm 👍🏼 for the tools.mod file, seems elegant.

One slightly weird thing about this setup is that I don't understand why the tool block doesn't specify versions like the require block does:

tool (
	github.com/golang/mock/mockgen
	github.com/golangci/golangci-lint/cmd/golangci-lint
	gotest.tools/gotestsum
)

require (
	github.com/golang/mock v1.6.0
	github.com/gorilla/mux v1.8.1
	github.com/mattermost/mattermost/server/public v0.1.15
	github.com/pkg/errors v0.9.1
	github.com/stretchr/testify v1.10.0
)

Instead, I have to dig into the indirect dependencies to find a version:

github.com/golangci/golangci-lint v1.64.8 // indirect

I don't think this is a blocker - just weird :/

[agnivade]

The tools.mod approach looks good to me. Thanks Ben 👍

[lieut-data]

Regrettably, it looks like this approach won't work on the monorepo for now due to our use of workspaces: golang/go#73968

[agnivade]

Ahhh 😭

[hanzei]

I don't think this is a blocker - just weird :/

Agreed

Regrettably, it looks like this approach won't work on the monorepo

Ahhh indeed.

Do we still want to continue with this approach? While I like the separate tools.mod file for the reasons outlined above, I also think there is a value in using similar tooling in our whole ecosystem.

[lieut-data]

@hanzei, I tend to agree that it would probably be better if we kept things uniform, as much as I'd love to adopt go tool. I've pinged the maintainers in golang/go#73968 (comment) with our story, but for now would probably support just upgrading to Go and sticking with the existing go run pattern.

[hanzei]

I'm going to close this PR. It doesn't seem like Tool Dependencies are ready for wide use within Mattermost.

[hanzei]

Here is a PR to update to go1.24 without the tooling changes: #225