Refactor QUIC connection handling and update dependencies - #1
Merged
Conversation
The bump of the QUIC library introduces a cyclic dependency between the connection and quic modules hence it is necessary to break this coupling. Right now, the connection module depends on the quic module for the datagram v2/v3 and to which a QUIC connection (currently an interface) is passed. As it is there is no issue however, under the hood, interface is a wrapper around an UDP connection and a QUIC connection meaning this type must be exposed to the quic module since the QUIC Connection will no longer be a interface but a struct. Given the above, these changes introduce an interface, QUICConnection, with the surface used today in cloudflared and a struct, ConnWithCloser, that implements said interface within the quic module. Closes TUN-10563
…org/grpc to v1.81.1 Closes TUN-10558
Publishing internal image in cloudflared. This allows us to remove the dependency from cloudflare/plat/dockerfiles. In addition, our acceptance tests should now be able to use the latest image instead of relying on a fixed version for testing, which will allow us to detect potential failures earlier.
Bumps quic-go to v0.59.1 (chungthuang fork rebased from upstream v0.45 onto v0.59.1). Upstream removed the `logging` package and replaced its callback-based ConnectionTracer with the structured `qlog`/`qlogwriter` event API, which required migrating cloudflared's QUIC metrics collection. Migrations: - quic/tracing.go: connTracer no longer fills a logging.ConnectionTracer callback struct. It implements qlogwriter.Trace + qlogwriter.Recorder and dispatches qlog events (PacketSent, PacketReceived, MetricsUpdated, ...) to the collector through RecordEvent. NewClientTracer now returns a function compatible with quic.Config.Tracer. - quic/metrics.go: collector methods take qlog types (qlog.Frame, qlog.PacketType, qlog.MetricsUpdated, ...) and plain int64 in place of the removed logging.ByteCount/Frame/RTTStats/TransportParameters. - quic/conversion.go: PacketType, PacketDropReason and PacketLossReason are strings upstream rather than numeric iotas, so the converters become pass-through allowlists. CongestionState is also a string; congestionStateToFloat maps it back to the numeric gauge values cloudflared exports. - quic.Connection/quic.Stream became *quic.Conn/*quic.Stream; updated ConnWithCloser, SafeStreamCloser and the connection package accordingly. Tests and generated mocks (mocks/mock_quic_connection.go) were adapted to the new pointer-based API. Closes TUN-10557
Fixing warnings in cloudflared before making any further changes.
As it stands, cloudflared prechecks are not taking the `protocol` flag into consideration and is instead falling back to the default protocol, which is QUIC. Prechecks should report the protocol cloudflared will use, not the default protocol.
This reverts merge request !1850
This adds back the quic-go bump.
Fixing renovate in cloudflared.
This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | gcr.io/distroless/base-debian13 | final | pinDigest | → `ab7554b` | --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At 12:00 AM through 04:59 AM and 10:00 PM through 11:59 PM, Monday through Friday (`* 0-4,22-23 * * 1-5`) - Only on Sunday and Saturday (`* * * * 0,6`) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yMjQuMCIsInVwZGF0ZWRJblZlciI6IjQzLjIyNC4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->
Generate a clean init script at /etc/init.d/cloudflared backed by OpenRC's supervise-daemon supervisor, plus a companion /etc/conf.d/cloudflared for operator overrides.
systemctl list-units exits 0 whether or not any pattern matches, so the filter was a no-op and every unit was always treated as installed. Detect installed units by checking for the unit file on disk instead.
This PR addresses an issue where cloudflared prematurely closes the origin connection before the upstream-to-downstream goroutine finishes reading, causing intermittent connection drops when a client immediately closes the write-side of a connection. When a client finishes writing data, it immediately closes its side of the connection. Under the current implementation in cloudflared's downstream-to-upstream goroutine does not wait for the second stream to complete. It unblocks the moment the first stream writes to the channel. Once this happens, the pipe returns control to `proxyTCPStream`, which prematurely closes the origin connection. Consequently, when the upstream-to-downstream goroutine attempts to read the remaining data from the origin connection, the connection is already gone, leading to unexpected failures. We started propagating the `TimeoutAfterFirstClose` configuration/parameter. This allows the proxy to wait for a designated period, giving the second stream sufficient time to finish processing and read all remaining data before `proxyTCPStream` tears down the origin connection.
## Summary Port from cloudflare/cloudflared#1639 to GitLab
Adding a test that tries to exercise the race condition for the websocket path. The sequence is as follows: 1. The test starts by writing the payload to the mock server 2. Then, it closes the write side of the connection. 3. It waits for 3 seconds to make the race window explicit: the bug in cloudflared would have closed the connection and we would not be able to read. 4. Read the message. With the fix applied, we should be able to read the full message.
This reverts commit 02eb75b.
This reverts merge request !1859
There was a problem hiding this comment.
Pull request overview
This PR refactors cloudflared’s QUIC usage behind a QUICConnection abstraction and bumps several vendored/core dependencies (notably gRPC, x/crypto, and chi), alongside CI/container automation updates.
Changes:
- Introduce
quic.QUICConnection+ wrapper (ConnWithCloser) and migrate QUIC call sites to depend on the interface. - Vendor and module updates: gRPC → 1.81.1, chi → 5.3.1, x/crypto → 0.52.0 (plus related vendored internal changes).
- Infra/ops updates: renovate config, internal image build pipeline, Docker base image digest pinning, Semgrep workflow adjustments, Go toolchain bump in Docker/CI.
Reviewed changes
Copilot reviewed 41 out of 92 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/modules.txt | Update vendored module list for bumped dependencies (chi, x/crypto, oauth2, genproto, grpc) |
| vendor/google.golang.org/grpc/version.go | gRPC version bump to 1.81.1 |
| vendor/google.golang.org/grpc/stream.go | gRPC retry error wrapping improvement |
| vendor/google.golang.org/grpc/server.go | gRPC server strict path checking + buffer config changes |
| vendor/google.golang.org/grpc/rpc_util.go | gRPC decompression reader lifecycle/buffer reuse fixes |
| vendor/google.golang.org/grpc/resolver/map.go | gRPC resolver maps: iterator-based APIs (iter.Seq2) |
| vendor/google.golang.org/grpc/picker_wrapper.go | Reduce noisy picker logging behind verbosity guard |
| vendor/google.golang.org/grpc/mem/buffers.go | Add Buffer slicing support + refcount semantics |
| vendor/google.golang.org/grpc/mem/buffer_slice.go | Reader buffer-empty logic updated to use Buffer.Len() |
| vendor/google.golang.org/grpc/mem/buffer_pool.go | Buffer pool refactor to internal mem implementations |
| vendor/google.golang.org/grpc/internal/transport/transport.go | Add GoAwayInfo/OnCloseFunc transport metadata |
| vendor/google.golang.org/grpc/internal/transport/readyreader/ready_reader.go | New readyreader package for non-pinning reads |
| vendor/google.golang.org/grpc/internal/transport/readyreader/raw_conn_nonlinux.go | readyreader RawConn stubs for non-Linux |
| vendor/google.golang.org/grpc/internal/transport/readyreader/raw_conn_linux.go | readyreader RawConn support for Linux |
| vendor/google.golang.org/grpc/internal/transport/http2_server.go | Header list size accounting + upcoming limit warning |
| vendor/google.golang.org/grpc/internal/transport/http2_client.go | OnClose callback signature + GOAWAY metadata + header warnings |
| vendor/google.golang.org/grpc/internal/transport/http_util.go | Read buffer pooling + IO buffer pooling refactor |
| vendor/google.golang.org/grpc/internal/transport/defaults.go | Add upcoming default header list size constant |
| vendor/google.golang.org/grpc/internal/resolver/config_selector.go | Interceptor Close() lifecycle support |
| vendor/google.golang.org/grpc/internal/mem/buffer_pool.go | New internal mem pool implementations |
| vendor/google.golang.org/grpc/internal/envconfig/xds.go | New xDS env flags |
| vendor/google.golang.org/grpc/internal/envconfig/envconfig.go | New/changed env defaults (path checking, framer pooling, etc.) |
| vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.go | Regen metadata (protoc-gen-go bump) |
| vendor/google.golang.org/grpc/health/grpc_health_v1/health_grpc.pb.go | Regen metadata (protoc-gen-go-grpc bump) |
| vendor/google.golang.org/grpc/experimental/stats/metrics.go | Add custom label propagation helpers |
| vendor/google.golang.org/grpc/dialoptions.go | Set SharedWriteBuffer default for transport opts |
| vendor/google.golang.org/grpc/credentials/tls.go | Authority validation behavior tightened (leaf cert) |
| vendor/google.golang.org/grpc/clientconn.go | Disconnection metrics labeling and GOAWAY info plumbing |
| vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.go | Regen metadata (protoc-gen-go bump) |
| vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.go | Migrate to iterator APIs (All()) |
| vendor/google.golang.org/grpc/balancer/endpointsharding/endpointsharding.go | Migrate to iterator APIs (All()) |
| vendor/google.golang.org/grpc/balancer/base/balancer.go | Migrate to iterator APIs (All()) |
| vendor/google.golang.org/grpc/balancer/balancer.go | Case-sensitive balancer registry env gating |
| vendor/google.golang.org/grpc/attributes/attributes.go | Attributes refactor to persistent chain + iterators |
| vendor/golang.org/x/crypto/ssh/server.go | SSH auth/sk key behavior changes + permission semantics |
| vendor/golang.org/x/crypto/ssh/mux.go | SSH mux response gating to prevent malicious staging |
| vendor/golang.org/x/crypto/ssh/keys.go | RSA/DSA validation hardening + SK UP flag support |
| vendor/golang.org/x/crypto/ssh/cipher.go | Padding bounds check fix |
| vendor/golang.org/x/crypto/ssh/channel.go | Channel request response gating + payload sizing safety |
| vendor/golang.org/x/crypto/ssh/certs.go | Cert verification guardrails + SK CA signature behavior |
| vendor/golang.org/x/crypto/chacha20poly1305/chacha20poly1305_amd64.s | Vendored assembly update |
| vendor/golang.org/x/crypto/chacha20poly1305/chacha20poly1305_amd64.go | AVX2 gating logic fix (SSSE3 requirement) |
| vendor/golang.org/x/crypto/blake2b/go125.go | Remove go1.25-specific file |
| vendor/github.com/go-chi/chi/v5/tree.go | chi router update incl QUERY method and traversal changes |
| vendor/github.com/go-chi/chi/v5/README.md | chi docs updated (ClientIPFrom* guidance, QUERY, etc.) |
| vendor/github.com/go-chi/chi/v5/path_value.go | Removed in newer chi version |
| vendor/github.com/go-chi/chi/v5/path_value_fallback.go | Removed in newer chi version |
| vendor/github.com/go-chi/chi/v5/mux.go | Handle parsing tweaks + QUERY method + path values behavior |
| vendor/github.com/go-chi/chi/v5/context.go | Wildcard replacement behavior tweak |
| vendor/github.com/go-chi/chi/v5/chi.go | chi package docs + Router interface adds Query() |
| stream/stream.go | Error-checking/errcheck-friendly CloseWrite handling; comment tweaks |
| renovate.json | Add Renovate config focused on distroless digest pinning |
| RELEASE_NOTES | Add 2026.7.x release notes entries |
| quic/quic_connection.go | Add QUICConnection abstraction + ConnWithCloser wrapper |
| quic/quic_connection_test.go | Unit tests for NewQUICConnection/CloseWithError behavior |
| quic/datagramv2.go | Switch DatagramMuxerV2 to QUICConnection + demux strictness |
| proxy/proxy.go | Minor request/close handling fixes + error message casing |
| prechecks/types.go | Add ProtocolOverride to precheck config |
| prechecks/resolvers.go | QUIC dialer interface now returns QUICConnection |
| prechecks/probes.go | QUIC probe dialer now returns QUICConnection |
| prechecks/checker.go | SuggestedProtocol logic updated to honor protocol override |
| prechecks/checker_test.go | Add protocol override behavior tests |
| mocks/mock_resolvers.go | Update mocks for QUICConnection return types |
| Makefile | Add generate-internal-image-version target |
| ingress/origin_service.go | Add unix TCP-over-WS service helper |
| ingress/origin_proxy.go | Dial unix sockets for absolute-path destinations |
| ingress/origin_connection.go | Use embedded SetWriteDeadline method directly |
| ingress/origin_connection_test.go | Improve close/error handling in tests |
| ingress/ingress.go | Add unix+ssh: ingress scheme support |
| go.sum | Dependency checksum updates (grpc, chi, x/crypto, oauth2, etc.) |
| go.mod | Dependency bumps (chi, x/crypto, oauth2, grpc/genproto) |
| Dockerfile.arm64 | Go builder bump + distroless digest pin |
| Dockerfile.amd64 | Go builder bump + distroless digest pin |
| Dockerfile | Go builder bump + distroless digest pin |
| connection/quic.go | DialQuic returns QUICConnection and uses ConnWithCloser wrapper |
| connection/quic_datagram_v3.go | Swap direct quic-go usage to QUICConnection + v3 package split |
| connection/quic_datagram_v2.go | Swap direct quic-go usage to QUICConnection + cleanup |
| connection/quic_connection.go | Tunnel QUIC connection now depends on QUICConnection |
| connection/connection_test.go | Close error handling cleanup in tests |
| cmd/cloudflared/updater/update.go | Consolidate init detection via new inits package |
| cmd/cloudflared/tunnel/cmd.go | Pass protocol override into prechecks |
| cmd/cloudflared/linux_service.go | OpenRC support + init detection refactor + error handling tweaks |
| cmd/cloudflared/inits/inits.go | New shared init-system detection helpers |
| carrier/websocket.go | Close error handling cleanup |
| carrier/websocket_test.go | Switch websocket test randomness to crypto/rand + cleanup |
| .gitlab-ci.yml | Go version bump + include internal-image pipeline |
| .github/workflows/semgrep.yml | Switch to OSS Semgrep scan workflow + caching/concurrency |
| .docker-images-internal | Define internal docker image build manifest |
| .ci/windows.gitlab-ci.yml | Use canary Windows runner tag |
| .ci/linux.gitlab-ci.yml | Update internal CI image version |
| .ci/internal-image.gitlab-ci.yml | New pipeline for building/pushing internal image |
| .ci/image/Dockerfile | Bump go-boring version in CI image |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The ingress ICMP tests (TestReuseFunnel, TestTraceICMPRouterEcho, and the
other ICMP router tests) create unprivileged ICMP (ping) sockets. On the
GitHub Linux runners the invoking user's GID is outside the default
net.ipv4.ping_group_range ("1 0"), so socket creation fails with
"Group ID N is not between ping group ..." and the tests error out.
Widen ping_group_range on Linux runners before running the suite so these
tests can create ping sockets. Guarded with 'if: runner.os == Linux' so the
step is skipped on the macOS and Windows matrix legs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLDdJQ6FSNfm2MGXWZ6L2N
Author
CI status noteThe remaining red on
What this PR's CI commit ( Net: CI on this branch is strictly better than on Also noting for reviewers: Copilot's two comments claiming Generated by Claude Code |
Author
|
Correction to the last paragraph above — the precise reason Copilot's two "will not compile" comments are false positives: Go 1.26 supports Generated by Claude Code |
- quic: fix doc references to the renamed constructor ([NewConnWithCloser] -> [NewQUICConnection]) and correct the stale claim that a nil conn is safe for CloseWithError — the constructor rejects nil conns and CloseWithError dereferences the connection unconditionally. - inits: add GOOS guards to IsSystemd and IsOpenRC so they return false on non-Linux platforms as the package documentation promises (IsSysV already had the guard). - linux_service: grammar fix in the generated OpenRC conf comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLDdJQ6FSNfm2MGXWZ6L2N
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the QUIC connection handling in cloudflared by introducing a new
QUICConnectioninterface abstraction and updates several key dependencies including gRPC, Go crypto libraries, and the chi router.Key Changes
QUIC Connection Abstraction
quic/quic_connection.gowithQUICConnectioninterface defining the subset ofquic.Connectionmethods used by cloudflaredquic/quic_connection_test.gowith tests for the new interfaceconnection/quic_connection.goto use the new abstraction instead of directquic.Connreferencesconnection/quic_datagram_v2.goandconnection/quic_datagram_v3.goto remove direct quic-go imports and use the abstractionprechecks/resolvers.goandmocks/mock_resolvers.goto work with the new interfacegRPC Updates
vendor/google.golang.org/grpc/internal/mem/buffer_pool.govendor/google.golang.org/grpc/internal/transport/readyreader/Go Crypto and SSH Updates
Chi Router Updates
Infrastructure and Configuration
cmd/cloudflared/inits/inits.gofor init system detection (systemd, OpenRC, SysV)renovate.jsonfor automated dependency management.ci/internal-image.gitlab-ci.ymlfor internal image builds.docker-images-internalconfiguration fileBug Fixes and Improvements
Notable Implementation Details
QUICConnectioninterface provides a clean abstraction boundary for QUIC operationshttps://claude.ai/code/session_01HLDdJQ6FSNfm2MGXWZ6L2N