cli/command: don't wrap client options

We may still change this, but in the client module, the signature
of the client.Opt changed to now include a non-exported type, which
means that we can't construct a custom option that is implemented
using client options:

    #18 16.94 # github.com/docker/cli/cli/context/docker
    #18 16.94 cli/context/docker/load.go:105:29: cannot use withHTTPClient(tlsConfig) (value of type func(*client.Client) error) as client.Opt value in argument to append
    #18 16.94 cli/context/docker/load.go:152:6: cannot use c (variable of type *client.Client) as *client.clientConfig value in argument to client.WithHTTPClient(&http.Client{…})

We can consider exporting the `client.clientConfig` type (but keep its
fields non-exported), but for this use, we don't strictly need it, so
let's change the implementation to not having to depend on that.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit e7d14d9)
Signed-off-by: Sebastiaan van Stijn <[email protected]>

Author: thaJeztah

cli/command/cli.go

@@ -324,7 +324,14 @@ func newAPIClientFromEndpoint(ep docker.Endpoint, configFile *configfile.ConfigF
 	if len(configFile.HTTPHeaders) > 0 {
 		opts = append(opts, client.WithHTTPHeaders(configFile.HTTPHeaders))
 	}
-	opts = append(opts, withCustomHeadersFromEnv(), client.WithUserAgent(UserAgent()))
+	withCustomHeaders, err := withCustomHeadersFromEnv()
+	if err != nil {
+		return nil, err
+	}
+	if withCustomHeaders != nil {
+		opts = append(opts, withCustomHeaders)
+	}
+	opts = append(opts, client.WithUserAgent(UserAgent()))
 	return client.NewClientWithOpts(opts...)
 }
 

cli/command/cli_options.go

@@ -180,61 +180,59 @@ const envOverrideHTTPHeaders = "DOCKER_CUSTOM_HEADERS"
 // override headers with the same name).
 //
 // TODO(thaJeztah): this is a client Option, and should be moved to the client. It is non-exported for that reason.
-func withCustomHeadersFromEnv() client.Opt {
-	return func(apiClient *client.Client) error {
-		value := os.Getenv(envOverrideHTTPHeaders)
-		if value == "" {
-			return nil
-		}
-		csvReader := csv.NewReader(strings.NewReader(value))
-		fields, err := csvReader.Read()
-		if err != nil {
-			return invalidParameter(errors.Errorf(
-				"failed to parse custom headers from %s environment variable: value must be formatted as comma-separated key=value pairs",
-				envOverrideHTTPHeaders,
-			))
-		}
-		if len(fields) == 0 {
-			return nil
-		}
-
-		env := map[string]string{}
-		for _, kv := range fields {
-			k, v, hasValue := strings.Cut(kv, "=")
-
-			// Only strip whitespace in keys; preserve whitespace in values.
-			k = strings.TrimSpace(k)
+func withCustomHeadersFromEnv() (client.Opt, error) {
+	value := os.Getenv(envOverrideHTTPHeaders)
+	if value == "" {
+		return nil, nil
+	}
+	csvReader := csv.NewReader(strings.NewReader(value))
+	fields, err := csvReader.Read()
+	if err != nil {
+		return nil, invalidParameter(errors.Errorf(
+			"failed to parse custom headers from %s environment variable: value must be formatted as comma-separated key=value pairs",
+			envOverrideHTTPHeaders,
+		))
+	}
+	if len(fields) == 0 {
+		return nil, nil
+	}
 
-			if k == "" {
-				return invalidParameter(errors.Errorf(
-					`failed to set custom headers from %s environment variable: value contains a key=value pair with an empty key: '%s'`,
-					envOverrideHTTPHeaders, kv,
-				))
-			}
+	env := map[string]string{}
+	for _, kv := range fields {
+		k, v, hasValue := strings.Cut(kv, "=")
 
-			// We don't currently allow empty key=value pairs, and produce an error.
-			// This is something we could allow in future (e.g. to read value
-			// from an environment variable with the same name). In the meantime,
-			// produce an error to prevent users from depending on this.
-			if !hasValue {
-				return invalidParameter(errors.Errorf(
-					`failed to set custom headers from %s environment variable: missing "=" in key=value pair: '%s'`,
-					envOverrideHTTPHeaders, kv,
-				))
-			}
+		// Only strip whitespace in keys; preserve whitespace in values.
+		k = strings.TrimSpace(k)
 
-			env[http.CanonicalHeaderKey(k)] = v
+		if k == "" {
+			return nil, invalidParameter(errors.Errorf(
+				`failed to set custom headers from %s environment variable: value contains a key=value pair with an empty key: '%s'`,
+				envOverrideHTTPHeaders, kv,
+			))
 		}
 
-		if len(env) == 0 {
-			// We should probably not hit this case, as we don't skip values
-			// (only return errors), but we don't want to discard existing
-			// headers with an empty set.
-			return nil
+		// We don't currently allow empty key=value pairs, and produce an error.
+		// This is something we could allow in future (e.g. to read value
+		// from an environment variable with the same name). In the meantime,
+		// produce an error to prevent users from depending on this.
+		if !hasValue {
+			return nil, invalidParameter(errors.Errorf(
+				`failed to set custom headers from %s environment variable: missing "=" in key=value pair: '%s'`,
+				envOverrideHTTPHeaders, kv,
+			))
 		}
 
-		// TODO(thaJeztah): add a client.WithExtraHTTPHeaders() function to allow these headers to be _added_ to existing ones, instead of _replacing_
-		//  see https://github.com/docker/cli/pull/5098#issuecomment-2147403871  (when updating, also update the WARNING in the function and env-var GoDoc)
-		return client.WithHTTPHeaders(env)(apiClient)
+		env[http.CanonicalHeaderKey(k)] = v
+	}
+
+	if len(env) == 0 {
+		// We should probably not hit this case, as we don't skip values
+		// (only return errors), but we don't want to discard existing
+		// headers with an empty set.
+		return nil, nil
 	}
+
+	// TODO(thaJeztah): add a client.WithExtraHTTPHeaders() function to allow these headers to be _added_ to existing ones, instead of _replacing_
+	//  see https://github.com/docker/cli/pull/5098#issuecomment-2147403871  (when updating, also update the WARNING in the function and env-var GoDoc)
+	return client.WithHTTPHeaders(env), nil
 }