Skip to content

Commit 8a8a913

Browse files
committed
fix(desktop): preserve transport defaults; nil-fall-back for resolver
Address Copilot review on #13770: - ProxyTransport now clones http.DefaultTransport and overrides only Proxy and DialContext, keeping stdlib timeouts, idle pool, and HTTP/2 defaults (was a bare *http.Transport that dropped them). - When DD is unavailable or detection fails, return nil instead of http.DefaultTransport so oci.NewResolver lets containerd use its own built-in default transport — preserving prior behavior for non-DD users. Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
1 parent a365264 commit 8a8a913

2 files changed

Lines changed: 38 additions & 22 deletions

File tree

internal/desktop/proxy.go

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,32 +68,38 @@ func httpProxySocketEndpoint(endpoint string) string {
6868
}
6969

7070
// ProxyTransport returns an http.RoundTripper that routes traffic through
71-
// Docker Desktop's PAC-aware HTTP proxy when DD exposes the proxy socket.
72-
// Otherwise http.DefaultTransport is returned. Pass "" for endpoint when DD
73-
// is not the active engine.
71+
// Docker Desktop's PAC-aware HTTP proxy when DD exposes the proxy socket,
72+
// or nil when no override is needed (callers should use their own default
73+
// transport in that case — for the OCI resolver this means containerd's
74+
// built-in transport). Pass "" for endpoint when DD is not the active
75+
// engine.
76+
//
77+
// When DD is available, the returned transport is a clone of
78+
// http.DefaultTransport with only Proxy and DialContext overridden, so it
79+
// preserves stdlib timeout, pooling, and HTTP/2 defaults.
7480
func ProxyTransport(endpoint string) http.RoundTripper {
7581
proxyEndpoint := httpProxySocketEndpoint(endpoint)
7682
if proxyEndpoint == "" {
77-
logrus.Debug("Docker Desktop HTTP proxy not available; using default HTTP transport")
78-
return http.DefaultTransport
83+
logrus.Debug("Docker Desktop HTTP proxy not available; deferring to caller's default transport")
84+
return nil
7985
}
8086
logrus.Debugf("routing OCI traffic through Docker Desktop HTTP proxy at %s", proxyEndpoint)
81-
return &http.Transport{
82-
Proxy: http.ProxyURL(&url.URL{Scheme: "http"}),
83-
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
84-
return memnet.DialEndpoint(ctx, proxyEndpoint)
85-
},
87+
tr := http.DefaultTransport.(*http.Transport).Clone()
88+
tr.Proxy = http.ProxyURL(&url.URL{Scheme: "http"})
89+
tr.DialContext = func(ctx context.Context, _, _ string) (net.Conn, error) {
90+
return memnet.DialEndpoint(ctx, proxyEndpoint)
8691
}
92+
return tr
8793
}
8894

8995
// ProxyTransportFor discovers the Docker Desktop endpoint via apiClient and
90-
// returns the matching transport, falling back to http.DefaultTransport on
91-
// discovery failure.
96+
// returns the matching transport, or nil when DD is not active or discovery
97+
// fails (so callers fall back to their own default transport).
9298
func ProxyTransportFor(ctx context.Context, apiClient client.APIClient) http.RoundTripper {
9399
endpoint, err := Endpoint(ctx, apiClient)
94100
if err != nil {
95-
logrus.Debugf("could not detect Docker Desktop endpoint, using default HTTP transport: %v", err)
96-
return http.DefaultTransport
101+
logrus.Debugf("could not detect Docker Desktop endpoint, deferring to caller's default transport: %v", err)
102+
return nil
97103
}
98104
return ProxyTransport(endpoint)
99105
}

internal/desktop/proxy_test.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,19 @@ func TestHTTPProxySocketEndpoint_EmptyOrUnknown(t *testing.T) {
5757
assert.Equal(t, httpProxySocketEndpoint("tcp://localhost:1234"), "")
5858
}
5959

60-
func TestProxyTransport_FallsBackWhenNoDockerDesktop(t *testing.T) {
61-
got := ProxyTransport("")
62-
assert.Equal(t, got, http.DefaultTransport)
60+
func TestProxyTransport_NilWhenNoDockerDesktop(t *testing.T) {
61+
assert.Assert(t, ProxyTransport("") == nil,
62+
"must return nil so callers fall back to their own (e.g. containerd's) default transport")
6363
}
6464

65-
func TestProxyTransport_FallsBackWhenSocketMissing(t *testing.T) {
65+
func TestProxyTransport_NilWhenSocketMissing(t *testing.T) {
6666
// no httpproxy.sock created
6767
dir := t.TempDir()
6868
cliSock := filepath.Join(dir, "docker-cli.sock")
6969
mustTouch(t, cliSock)
7070

71-
got := ProxyTransport("unix://" + cliSock)
72-
assert.Equal(t, got, http.DefaultTransport,
73-
"when DD endpoint is set but proxy socket is missing, must not return a transport that would dial a dead socket")
71+
assert.Assert(t, ProxyTransport("unix://"+cliSock) == nil,
72+
"must return nil when DD endpoint is set but proxy socket is missing, not a transport that would dial a dead socket")
7473
}
7574

7675
func TestProxyTransport_RoutesThroughDockerDesktop(t *testing.T) {
@@ -86,7 +85,18 @@ func TestProxyTransport_RoutesThroughDockerDesktop(t *testing.T) {
8685
got := ProxyTransport("unix://" + cliSock)
8786
tr, ok := got.(*http.Transport)
8887
assert.Assert(t, ok, "expected *http.Transport when DD endpoint is set and socket exists")
89-
assert.Assert(t, tr != http.DefaultTransport)
88+
assert.Assert(t, tr != http.DefaultTransport, "must be a clone, not DefaultTransport itself")
89+
90+
// Verify the clone preserved http.DefaultTransport's production
91+
// settings (timeouts, idle pool, HTTP/2). Compare to the source
92+
// fields rather than asserting fixed values so this test follows
93+
// stdlib changes.
94+
src := http.DefaultTransport.(*http.Transport)
95+
assert.Equal(t, tr.MaxIdleConns, src.MaxIdleConns)
96+
assert.Equal(t, tr.IdleConnTimeout, src.IdleConnTimeout)
97+
assert.Equal(t, tr.TLSHandshakeTimeout, src.TLSHandshakeTimeout)
98+
assert.Equal(t, tr.ExpectContinueTimeout, src.ExpectContinueTimeout)
99+
assert.Equal(t, tr.ForceAttemptHTTP2, src.ForceAttemptHTTP2)
90100
}
91101

92102
func mustTouch(t *testing.T, path string) {

0 commit comments

Comments
 (0)