-
Notifications
You must be signed in to change notification settings - Fork 232
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
Dependency fixes and updates to development process #137
base: master
Are you sure you want to change the base?
Conversation
I'm not a fan of Makefiles in Go projects 😅 but as long as it's optional and just your preference I guess that's OK 🤷♂️ 👍 Any reason in particular to bump min version to Go 1.22 and to bump the Caddy version? |
@mholt Removed the Makefile. Given this is a secury related project it made sense to the latest release. I can change it to 1.21 if needed. Several of the dependencies needed to be updated because |
Well, the versions in go.mod of libraries don't really matter (AFAIK), since, in the case of Caddy plugins, the versions used are determined by Caddy's go.mod. @francislavoie and @mohammed90 understand better than I do why needlessly upgrading versions in plugins' go.mod can be problematic, but I know we ran into some troubles in the recent past. As far as govulncheck, we also have problems with lots of false positives in vulnerability scanners. Again, maybe Francis and Mohammed can elaborate as to what is happening but I think our new advice to plugin authors will soon be to only upgrade go.mod versions if really needed to compile. |
Yeah all that will do is make it impossible to build the latest version of this plugin with anything lower than Caddy 2.8.4. If you're trying to build the latest version of Caddy with this plugin, then it will already use the correct versions. You can confirm by looking at There's no need to touch go.mod unless you actually need it due to an API change in some dependency. One reason to do it would be to use a new |
The rule of thumb is
In this case, you have caddy and x/net (zap is brought in by caddy). As Francis said, you don't need to upgrade Caddy unless you need new API. The go.mod file specifies the minimum version needed for your module to work. Bumping it up for no reason only brings pain.
The |
I believe the one that was breaking the CI was |
@mholt Caddy v2.7.x uses a vulnerable version of Vulnerability #1: GO-2024-2682
Denial of service via connection starvation in github.com/quic-go/quic-go
More info: https://pkg.go.dev/vuln/GO-2024-2682
Module: github.com/quic-go/quic-go
Found in: github.com/quic-go/quic-go@v0.40.0
Fixed in: github.com/quic-go/quic-go@v0.42.0 |
2.7 is an old version though. The go.mod in this repo doesn't dictate what version of quic-go gets used (unless it needs an old version, AFAIK). It's the go.mod in Caddy that dictates which version of quic-go is used, since Caddy is the main. |
Like I said, do If we change the go.mod here to Caddy latest, then all it will do is prevent I still suggest adding |
1. What does this change do, exactly?
gotestsum
for running tests.shuffle
enabledMakefile
to ease development1.22