Simplify, and better document, newDockerClient

Use a switch instead of nested ifs.  Update the documentation,
both to be a bit more specific about the mechanism, and also
to say that we keep the HTTP special-case now that it exists,
and document an alternative.

Signed-off-by: Miloslav Trmač <[email protected]>

Author: mtrmac

docker/daemon/client.go

@@ -26,30 +26,41 @@ func newDockerClient(sys *types.SystemContext) (*dockerclient.Client, error) {
 		dockerclient.WithVersion(defaultAPIVersion),
 	}
 
-	// Sadly, unix:// sockets don't work transparently with dockerclient.NewClient.
-	// They work fine with a nil httpClient; with a non-nil httpClient, the transport’s
-	// TLSClientConfig must be nil (or the client will try using HTTPS over the PF_UNIX socket
-	// regardless of the values in the *tls.Config), and we would have to call sockets.ConfigureTransport.
+	// We conditionalize building the TLS configuration only to TLS sockets:
 	//
-	// We don't really want to configure anything for unix:// sockets, so just pass a nil *http.Client.
+	// The dockerclient.Client implementation differentiates between
+	// - Client.proto, which is ~how the connection is establishe (IP / AF_UNIX/Windows)
+	// - Client.scheme, which is what is sent over the connection (HTTP with/without TLS).
 	//
-	// Similarly, if we want to communicate over plain HTTP on a TCP socket, we also need to set
-	// TLSClientConfig to nil. This can be achieved by using the form `http://`
+	// Only Client.proto is set from the URL in dockerclient.WithHost(),
+	// Client.scheme is detected based on a http.Client.TLSClientConfig presence;
+	// dockerclient.WithHTTPClient with a client that has TLSClientConfig set
+	// will, by default, trigger an attempt to use TLS.
+	//
+	// So, don’t use WithHTTPClient for unix:// sockets at all.
+	//
+	// Similarly, if we want to communicate over plain HTTP on a TCP socket (http://),
+	// we also should not set TLSClientConfig.  We continue to use WithHTTPClient
+	// with our slightly non-default settings to avoid a behavior change on updates of c/image.
+	//
+	// Alternatively we could use dockerclient.WithScheme to drive the TLS/non-TLS logic
+	// explicitly, but we would still want to set WithHTTPClient (differently) for https:// and http:// ;
+	// so that would not be any simpler.
 	serverURL, err := dockerclient.ParseHostURL(host)
 	if err != nil {
 		return nil, err
 	}
-	if serverURL.Scheme != "unix" {
-		if serverURL.Scheme == "http" {
-			hc := httpConfig()
-			opts = append(opts, dockerclient.WithHTTPClient(hc))
-		} else {
-			hc, err := tlsConfig(sys)
-			if err != nil {
-				return nil, err
-			}
-			opts = append(opts, dockerclient.WithHTTPClient(hc))
+	switch serverURL.Scheme {
+	case "unix": // Nothing
+	case "http":
+		hc := httpConfig()
+		opts = append(opts, dockerclient.WithHTTPClient(hc))
+	default:
+		hc, err := tlsConfig(sys)
+		if err != nil {
+			return nil, err
 		}
+		opts = append(opts, dockerclient.WithHTTPClient(hc))
 	}
 
 	return dockerclient.NewClientWithOpts(opts...)