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

chore: Update quic-go to v0.41.0, bump Go minimum to 1.21 #6043

Merged
merged 4 commits into from
Jan 25, 2024

Conversation

marten-seemann
Copy link
Contributor

Tons of changes in the v0.41.0 release. Most relevant for caddy is probably quic-go/quic-go#4181, which should fix #5951 on HTTP/3.

quic-go/quic-go#4230 might also be useful. I see that you use ConnContext on HTTP/1.1 and HTTP/2 (haven't looked at it in detail though), you can now do that on HTTP/3 as well.

@@ -1,6 +1,8 @@
module github.com/caddyserver/caddy/v2

go 1.20
go 1.21
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we're ready to bump our minimum version yet. Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

quic-go dropped support Go 1.20 a bit earlier than usual this team. See release notes for justification.

So yes, updating to v0.41.0 will unfortunately force you move away from Go 1.20.

Copy link
Member

Choose a reason for hiding this comment

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

Okay. It might take us a little while to merge this. I think our next release v2.8.0 is still a little ways away (partly cause we have a lot of pending PRs to review and merge, partly because Matt is busy being a parent). So I think by the time we'll be ready, 1.22 will be out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think our next release v2.8.0 is still a little ways away

A smaller patch release with #5982 would still be good though.

Copy link
Member

Choose a reason for hiding this comment

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

Understood @bt90 but that would take time to make a branch & cherrypick etc. I don't think @mholt has the time to do that without it being sponsored/paid for.

Copy link
Member

Choose a reason for hiding this comment

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

Go 1.22 is slated for just a couple/few weeks. I'm willing to bet v2.8 won't be ready before then. Should we just merge this?

Copy link
Member

Choose a reason for hiding this comment

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

@mholt sure, we'll need to also update CI config (and repo config) to test 1.21 + 1.22

Copy link
Member

Choose a reason for hiding this comment

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

The justification for dropping Go 1.20 support is really good: no more crypto/tls fork.

One other thing to consider is that v0.42 will have an improved API for a security feature. quic-go/quic-go#3549 (comment) -- we could also just wait until that gets released and kill 2 birds with 1 stone. Either way is fine with me.

@francislavoie francislavoie added the dependencies ⛓️ Pull requests that update a dependency file label Jan 17, 2024
@francislavoie francislavoie added this to the v2.8.0 milestone Jan 17, 2024
@francislavoie francislavoie force-pushed the update-quic-go-v0410 branch 2 times, most recently from 10d9cfa to f7c2973 Compare January 25, 2024 04:21
Comment on lines +472 to +476
e, err := key.ECDH()
if err != nil {
return nil, err
}
return e.Bytes(), nil
Copy link
Member

Choose a reason for hiding this comment

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

elliptic.Marshal is deprecated. I'm not sure if this change is correct, wouldn't mind a second pair of eyes. But I think this is the intended use of the new API?

@francislavoie francislavoie changed the title update quic-go to v0.41.0 chore: Update quic-go to v0.41.0, bump Go minimum to 1.21 Jan 25, 2024
@francislavoie
Copy link
Member

Alright, minimum Go bumped, & CI updated to 1.21 + 1.22-rc.2 for now. Some code changes to match.

@mholt just need to remove the Go 1.20 CI requirement then I think we can merge.

@mholt
Copy link
Member

mholt commented Jan 25, 2024

@francislavoie Updated repo requirements. Go 1.21 and 1.22 now required. Thanks!

@francislavoie francislavoie merged commit 697cc59 into caddyserver:master Jan 25, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies ⛓️ Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http/2 connection is not aborted properly if underlying connection is aborted
4 participants