diff --git a/cli/command/cli.go b/cli/command/cli.go index 85fac56502..b657ff313d 100644 --- a/cli/command/cli.go +++ b/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...) } diff --git a/cli/command/cli_options.go b/cli/command/cli_options.go index dd3c947336..a8bdd88eb4 100644 --- a/cli/command/cli_options.go +++ b/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, +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 + } + + 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) + + 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(fields) == 0 { - 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, + )) } - 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) - - 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, - )) - } - - // 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, - )) - } - - 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 - } - - // 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 }