Fix Geo SSH proxy hang on error responses over HTTP/2

Chloe Fonsrequested to merge
604776-geo-ssh-direct-http2-error-hang into main

What does this MR do and why?

This MR fixes a hang in gitlab-shell when Geo direct-to-primary SSH proxying forwards a push or fetch to a primary whose internal URL uses HTTPS with HTTP/2. When the primary rejects the request, for example with a 404 for a user without access, gitlab-shell's HTTP/2 client blocks waiting for the request body upload goroutine to finish, so the error is never surfaced until the user presses Ctrl-C. The fix cancels the request context before closing the response body on the error path in do(), so the HTTP/2 client's response Close call returns immediately instead of waiting on a stdin read that will never complete. On success, a small cancelOnCloseBody wrapper cancels the context when the caller closes the response body, so the context is not leaked. Because the change is in do(), it covers every caller of the client, not only the two SSH routes.

Why gitlab-shell hung

With geo_proxy_push_ssh_to_primary and geo_proxy_fetch_ssh_to_primary both on by default, gitlab-shell on a Geo secondary sends one streaming POST /<repo>.git/ssh-receive-pack (or ssh-upload-pack) request to the primary. The request body is the SSH client's stdin, passed as io.NopCloser(rw.In) in executeSSHRequest (internal/command/githttp/util.go). Git sends nothing until it receives the server's reply, so the body stays open and silent.

If the primary's Geo internal URL is HTTPS, Go's HTTP client negotiates HTTP/2. In git.(*Client).do (internal/gitlabnet/git/client.go), the error path for a status >= 400 reads the response body and then the deferred response.Body.Close() runs. In Go's HTTP/2 client, closing a response body waits for the request-body upload goroutine to finish. That goroutine is blocked reading from stdin, and the transport's own attempt to abort the upload closes the request body — but because the body is wrapped in io.NopCloser, that close is a no-op and never unblocks the read. So Execute never returns, and gitlab-shell shows no error until the user cancels the SSH session manually. Over plain HTTP/1.1, Go's client does not wait for the upload on Close, so the hang does not happen there.

References

  • Related to gitlab#604776 (closed). The original report used an HTTPS primary, which is exactly the case this MR fixes.
  • A companion fix in Workhorse, gitlab!257308 (merged), addresses a separate hang on Cloud Native Hybrid primaries behind an Envoy Gateway, where Workhorse itself held back the 404 response; together the two MRs cover both hang scenarios. This MR uses "Related to" rather than "Closes" because the gitlab-shell change still needs a version bump in the GitLab repository before it ships.

How was this validated?

New test TestSSHErrorResponseWithOpenRequestBodyOverHTTP2 in internal/gitlabnet/git/client_test.go starts an HTTP/2 TLS httptest server that returns 404, points the package's HTTP client at it, and sends a request whose body is an io.Pipe that nothing ever writes to, wrapped in io.NopCloser the same way executeSSHRequest wraps stdin. The test runs for both SSHUploadPack and SSHReceivePack and asserts the error returns within 5 seconds. Without the fix, both subtests fail after 5 seconds with "error not returned while the request body was open"; with the fix, both pass. go test ./... and make lint pass with the repo's pinned Go version.

A live reproduction on a Geo pair running the Linux package with gitlab-shell 14.56.1, with the primary's Geo internal URL switched to https:// and NGINX's http2 on, showed the hang directly. A push by a user with no access to the project hung. The secondary's gitlab-shell log showed the 404 from POST https://.../ssh-receive-pack with duration_s: 1.03, and no "command execution failed" line appeared until Ctrl-C. A goroutine dump (SIGQUIT) of the stuck process confirmed the blocked upload:

goroutine 1 [select]:
net/http.http2transportResponseBody.Close(...)
    net/http/h2_bundle.go:9924
gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/git.(*Client).do.func1()
    internal/gitlabnet/git/client.go:96
gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/git.(*Client).do(...)
    internal/gitlabnet/git/client.go:107
...

goroutine 86 [syscall]:
syscall.read(...)                 <- fd 0 (stdin)
os.(*File).Read(...)
io.(*nopCloserWriterTo).Read(...)
net/http.(*http2clientStream).writeRequestBody(...)

Replacing the secondary's gitlab-shell binary with 14.56.1 plus this change produced these results:

Scenario Before After
Push by a user with no access Hangs until Ctrl-C; 404 logged at duration_s: 1.03, error never shown Fails immediately; error shown 0.2 ms after the 404 log line
Push by a user with access — Succeeds; POST .../ssh-receive-pack returns 200

The no-access case now fails right away with "The project you were looking for could not be found or you don't have permission to view it." A push by a user with access through the same secondary still succeeds, confirming the fix does not affect the normal path.

How to validate locally

  1. From the repository root, run go test ./internal/gitlabnet/git/ -run TestSSHErrorResponseWithOpenRequestBodyOverHTTP2 -count=1.
  2. To see the test catch the regression, remove the cancel() call from the error path in do() and rerun the same command; both subtests fail after 5 seconds.
  3. On a Geo pair whose primary internal URL uses HTTPS with HTTP/2, an optional end-to-end check is to push over SSH to the secondary as a user who cannot see the project and confirm the error appears immediately, then push again as a user with access.

🤖 Generated with Claude Code

Merge request reports