Merge pull request #46133 from thaJeztah/client_cleanup

client/buildkit/buildkit.go

@@ -10,19 +10,18 @@ import (
 
 // ClientOpts returns a list of buildkit client options which allows the
 // caller to create a buildkit client which will connect to the buildkit
-// API provided by the daemon.
+// API provided by the daemon. These options can be passed to [bkclient.New].
 //
-// Example: bkclient.New(ctx, "", ClientOpts(c)...)
+// Example:
+//
+//	bkclient.New(ctx, "", ClientOpts(c)...)
 func ClientOpts(c client.CommonAPIClient) []bkclient.ClientOpt {
-	session := func(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) {
-		return c.DialHijack(ctx, "/session", proto, meta)
-	}
-	grpc := func(ctx context.Context, _ string) (net.Conn, error) {
-		return c.DialHijack(ctx, "/grpc", "h2c", nil)
-	}
-
 	return []bkclient.ClientOpt{
-		bkclient.WithSessionDialer(session),
-		bkclient.WithContextDialer(grpc),
+		bkclient.WithSessionDialer(func(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) {
+			return c.DialHijack(ctx, "/session", proto, meta)
+		}),
+		bkclient.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) {
+			return c.DialHijack(ctx, "/grpc", "h2c", nil)
+		}),
 	}
 }

client/client.go

@@ -43,11 +43,13 @@ package client // import "github.com/docker/docker/client"
 
 import (
 	"context"
+	"crypto/tls"
 	"net"
 	"net/http"
 	"net/url"
 	"path"
 	"strings"
+	"time"
 
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/types"
@@ -187,16 +189,15 @@ func NewClientWithOpts(ops ...Opt) (*Client, error) {
 	}
 
 	if c.scheme == "" {
-		c.scheme = "http"
-
-		tlsConfig := resolveTLSConfig(c.client.Transport)
-		if tlsConfig != nil {
-			// TODO(stevvooe): This isn't really the right way to write clients in Go.
-			// `NewClient` should probably only take an `*http.Client` and work from there.
-			// Unfortunately, the model of having a host-ish/url-thingy as the connection
-			// string has us confusing protocol and transport layers. We continue doing
-			// this to avoid breaking existing clients but this should be addressed.
+		// TODO(stevvooe): This isn't really the right way to write clients in Go.
+		// `NewClient` should probably only take an `*http.Client` and work from there.
+		// Unfortunately, the model of having a host-ish/url-thingy as the connection
+		// string has us confusing protocol and transport layers. We continue doing
+		// this to avoid breaking existing clients but this should be addressed.
+		if c.tlsConfig() != nil {
 			c.scheme = "https"
+		} else {
+			c.scheme = "http"
 		}
 	}
 
@@ -215,6 +216,16 @@ func defaultHTTPClient(hostURL *url.URL) (*http.Client, error) {
 	}, nil
 }
 
+// tlsConfig returns the TLS configuration from the client's transport.
+// It returns nil if the transport is not a [http.Transport], or if no
+// TLSClientConfig is set.
+func (cli *Client) tlsConfig() *tls.Config {
+	if tr, ok := cli.client.Transport.(*http.Transport); ok {
+		return tr.TLSClientConfig
+	}
+	return nil
+}
+
 // Close the transport used by the client
 func (cli *Client) Close() error {
 	if t, ok := cli.client.Transport.(*http.Transport); ok {
@@ -357,6 +368,16 @@ func (cli *Client) Dialer() func(context.Context) (net.Conn, error) {
 				return transport.DialContext(ctx, cli.proto, cli.addr)
 			}
 		}
-		return fallbackDial(cli.proto, cli.addr, resolveTLSConfig(cli.client.Transport))
+		switch cli.proto {
+		case "unix":
+			return net.Dial(cli.proto, cli.addr)
+		case "npipe":
+			return sockets.DialPipe(cli.addr, 32*time.Second)
+		default:
+			if tlsConfig := cli.tlsConfig(); tlsConfig != nil {
+				return tls.Dial(cli.proto, cli.addr, tlsConfig)
+			}
+			return net.Dial(cli.proto, cli.addr)
+		}
 	}
 }

client/hijack.go

@@ -3,7 +3,6 @@ package client // import "github.com/docker/docker/client"
 import (
 	"bufio"
 	"context"
-	"crypto/tls"
 	"fmt"
 	"net"
 	"net/http"
@@ -13,7 +12,6 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/versions"
-	"github.com/docker/go-connections/sockets"
 	"github.com/pkg/errors"
 )
 
@@ -47,18 +45,6 @@ func (cli *Client) DialHijack(ctx context.Context, url, proto string, meta map[s
 	return conn, err
 }
 
-// fallbackDial is used when WithDialer() was not called.
-// See cli.Dialer().
-func fallbackDial(proto, addr string, tlsConfig *tls.Config) (net.Conn, error) {
-	if tlsConfig != nil && proto != "unix" && proto != "npipe" {
-		return tls.Dial(proto, addr, tlsConfig)
-	}
-	if proto == "npipe" {
-		return sockets.DialPipe(addr, 32*time.Second)
-	}
-	return net.Dial(proto, addr)
-}
-
 func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto string) (net.Conn, string, error) {
 	req.Header.Set("Connection", "Upgrade")
 	req.Header.Set("Upgrade", proto)

client/transport.go

@@ -1,17 +0,0 @@
-package client // import "github.com/docker/docker/client"
-
-import (
-	"crypto/tls"
-	"net/http"
-)
-
-// resolveTLSConfig attempts to resolve the TLS configuration from the
-// RoundTripper.
-func resolveTLSConfig(transport http.RoundTripper) *tls.Config {
-	switch tr := transport.(type) {
-	case *http.Transport:
-		return tr.TLSClientConfig
-	default:
-		return nil
-	}
-}