From c2c7e9d4492a52c926a860cfabf7f1289f168985 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 7 Mar 2022 11:29:38 +0100 Subject: [PATCH] client: improve GoDoc, and minor touch-ups - Improve documentation of various functions to better describe their behavior. - Rename some variables to be more descriptive (as this is client code, used by external consumers, it's nice to be a bit more explicit). - Remove a redundant check in `WithVersionFromEnv()`, as `WithVersion()` already checks for empty values. Signed-off-by: Sebastiaan van Stijn --- client/client.go | 109 +++++++++++++++++++++++++-------------- client/client_unix.go | 3 +- client/client_windows.go | 3 +- client/options.go | 36 ++++++++----- 4 files changed, 96 insertions(+), 55 deletions(-) diff --git a/client/client.go b/client/client.go index 6a8b4d4fea..dc2834f1a5 100644 --- a/client/client.go +++ b/client/client.go @@ -43,7 +43,6 @@ package client // import "github.com/docker/docker/client" import ( "context" - "fmt" "net" "net/http" "net/url" @@ -93,15 +92,18 @@ type Client struct { } // CheckRedirect specifies the policy for dealing with redirect responses: -// If the request is non-GET return `ErrRedirect`. Otherwise use the last response. +// If the request is non-GET return ErrRedirect, otherwise use the last response. // -// Go 1.8 changes behavior for HTTP redirects (specifically 301, 307, and 308) in the client . -// The Docker client (and by extension docker API client) can be made to send a request -// like POST /containers//start where what would normally be in the name section of the URL is empty. -// This triggers an HTTP 301 from the daemon. -// In go 1.8 this 301 will be converted to a GET request, and ends up getting a 404 from the daemon. -// This behavior change manifests in the client in that before the 301 was not followed and -// the client did not generate an error, but now results in a message like Error response from daemon: page not found. +// Go 1.8 changes behavior for HTTP redirects (specifically 301, 307, and 308) +// in the client. The Docker client (and by extension docker API client) can be +// made to send a request like POST /containers//start where what would normally +// be in the name section of the URL is empty. This triggers an HTTP 301 from +// the daemon. +// +// In go 1.8 this 301 will be converted to a GET request, and ends up getting +// a 404 from the daemon. This behavior change manifests in the client in that +// before, the 301 was not followed and the client did not generate an error, +// but now results in a message like Error response from daemon: page not found. func CheckRedirect(req *http.Request, via []*http.Request) error { if via[0].Method == http.MethodGet { return http.ErrUseLastResponse @@ -109,13 +111,22 @@ func CheckRedirect(req *http.Request, via []*http.Request) error { return ErrRedirect } -// NewClientWithOpts initializes a new API client with default values. It takes functors -// to modify values when creating it, like `NewClientWithOpts(WithVersion(…))` -// It also initializes the custom http headers to add to each request. +// NewClientWithOpts initializes a new API client with a default HTTPClient, and +// default API host and version. It also initializes the custom HTTP headers to +// add to each request. +// +// It takes an optional list of Opt functional arguments, which are applied in +// the order they're provided, which allows modifying the defaults when creating +// the client. For example, the following initializes a client that configures +// itself with values from environment variables (client.FromEnv), and has +// automatic API version negotiation enabled (client.WithAPIVersionNegotiation()). +// +// +// cli, err := client.NewClientWithOpts( +// client.FromEnv, +// client.WithAPIVersionNegotiation(), +// ) // -// It won't send any version information if the version number is empty. It is -// highly recommended that you set a version or your client may break if the -// server is upgraded. func NewClientWithOpts(ops ...Opt) (*Client, error) { client, err := defaultHTTPClient(DefaultDockerHost) if err != nil { @@ -153,12 +164,12 @@ func NewClientWithOpts(ops ...Opt) (*Client, error) { } func defaultHTTPClient(host string) (*http.Client, error) { - url, err := ParseHostURL(host) + hostURL, err := ParseHostURL(host) if err != nil { return nil, err } - transport := new(http.Transport) - sockets.ConfigureTransport(transport, url.Scheme, url.Host) + transport := &http.Transport{} + _ = sockets.ConfigureTransport(transport, hostURL.Scheme, hostURL.Host) return &http.Client{ Transport: transport, CheckRedirect: CheckRedirect, @@ -194,11 +205,21 @@ func (cli *Client) ClientVersion() string { return cli.version } -// NegotiateAPIVersion queries the API and updates the version to match the -// API version. Any errors are silently ignored. If a manual override is in place, -// either through the `DOCKER_API_VERSION` environment variable, or if the client -// was initialized with a fixed version (`opts.WithVersion(xx)`), no negotiation -// will be performed. +// NegotiateAPIVersion queries the API and updates the version to match the API +// version. NegotiateAPIVersion downgrades the client's API version to match the +// APIVersion if the ping version is lower than the default version. If the API +// version reported by the server is higher than the maximum version supported +// by the client, it uses the client's maximum version. +// +// If a manual override is in place, either through the "DOCKER_API_VERSION" +// environment variable, or if the client is initialized +// with a fixed version (WithVersion(xx)), no negotiation is performed. +// +// If the API server's ping response does not contain an API version, or if the +// client did not get a successful ping response, it assumes it is connected with +// an old daemon that does not support API version negotiation, in which case it +// downgrades to the latest version of the API before version negotiation was +// added (1.24). func (cli *Client) NegotiateAPIVersion(ctx context.Context) { if !cli.manualOverride { ping, _ := cli.Ping(ctx) @@ -206,23 +227,31 @@ func (cli *Client) NegotiateAPIVersion(ctx context.Context) { } } -// NegotiateAPIVersionPing updates the client version to match the Ping.APIVersion -// if the ping version is less than the default version. If a manual override is -// in place, either through the `DOCKER_API_VERSION` environment variable, or if -// the client was initialized with a fixed version (`opts.WithVersion(xx)`), no -// negotiation is performed. -func (cli *Client) NegotiateAPIVersionPing(p types.Ping) { +// NegotiateAPIVersionPing downgrades the client's API version to match the +// APIVersion in the ping response. If the API version in pingResponse is higher +// than the maximum version supported by the client, it uses the client's maximum +// version. +// +// If a manual override is in place, either through the "DOCKER_API_VERSION" +// environment variable, or if the client is initialized +// with a fixed version (WithVersion(xx)), no negotiation is performed. +// +// If the API server's ping response does not contain an API version, we assume +// we are connected with an old daemon without API version negotiation support, +// and downgrade to the latest version of the API before version negotiation was +// added (1.24). +func (cli *Client) NegotiateAPIVersionPing(pingResponse types.Ping) { if !cli.manualOverride { - cli.negotiateAPIVersionPing(p) + cli.negotiateAPIVersionPing(pingResponse) } } // negotiateAPIVersionPing queries the API and updates the version to match the -// API version. Any errors are silently ignored. -func (cli *Client) negotiateAPIVersionPing(p types.Ping) { - // try the latest version before versioning headers existed - if p.APIVersion == "" { - p.APIVersion = "1.24" +// API version from the ping response. +func (cli *Client) negotiateAPIVersionPing(pingResponse types.Ping) { + // default to the latest version before versioning headers existed + if pingResponse.APIVersion == "" { + pingResponse.APIVersion = "1.24" } // if the client is not initialized with a version, start with the latest supported version @@ -231,8 +260,8 @@ func (cli *Client) negotiateAPIVersionPing(p types.Ping) { } // if server version is lower than the client version, downgrade - if versions.LessThan(p.APIVersion, cli.version) { - cli.version = p.APIVersion + if versions.LessThan(pingResponse.APIVersion, cli.version) { + cli.version = pingResponse.APIVersion } // Store the results, so that automatic API version negotiation (if enabled) @@ -258,7 +287,7 @@ func (cli *Client) HTTPClient() *http.Client { func ParseHostURL(host string) (*url.URL, error) { protoAddrParts := strings.SplitN(host, "://", 2) if len(protoAddrParts) == 1 { - return nil, fmt.Errorf("unable to parse docker host `%s`", host) + return nil, errors.Errorf("unable to parse docker host `%s`", host) } var basePath string @@ -278,7 +307,9 @@ func ParseHostURL(host string) (*url.URL, error) { }, nil } -// Dialer returns a dialer for a raw stream connection, with HTTP/1.1 header, that can be used for proxying the daemon connection. +// Dialer returns a dialer for a raw stream connection, with an HTTP/1.1 header, +// that can be used for proxying the daemon connection. +// // Used by `docker dial-stdio` (docker/cli#889). func (cli *Client) Dialer() func(context.Context) (net.Conn, error) { return func(ctx context.Context) (net.Conn, error) { diff --git a/client/client_unix.go b/client/client_unix.go index 5846f888fe..44b575606c 100644 --- a/client/client_unix.go +++ b/client/client_unix.go @@ -3,7 +3,8 @@ package client // import "github.com/docker/docker/client" -// DefaultDockerHost defines os specific default if DOCKER_HOST is unset +// DefaultDockerHost defines OS-specific default host if the DOCKER_HOST +// environment variable is unset or empty. const DefaultDockerHost = "unix:///var/run/docker.sock" const defaultProto = "unix" diff --git a/client/client_windows.go b/client/client_windows.go index c649e54412..601918a936 100644 --- a/client/client_windows.go +++ b/client/client_windows.go @@ -1,6 +1,7 @@ package client // import "github.com/docker/docker/client" -// DefaultDockerHost defines os specific default if DOCKER_HOST is unset +// DefaultDockerHost defines OS-specific default host if the DOCKER_HOST +// environment variable is unset or empty. const DefaultDockerHost = "npipe:////./pipe/docker_engine" const defaultProto = "npipe" diff --git a/client/options.go b/client/options.go index 77a9abc141..028f61c00a 100644 --- a/client/options.go +++ b/client/options.go @@ -18,11 +18,18 @@ type Opt func(*Client) error // FromEnv configures the client with values from environment variables. // -// Supported environment variables: -// DOCKER_HOST to set the url to the docker server. -// DOCKER_API_VERSION to set the version of the API to reach, leave empty for latest. -// DOCKER_CERT_PATH to load the TLS certificates from. -// DOCKER_TLS_VERIFY to enable or disable TLS verification, off by default. +// FromEnv uses the following environment variables: +// +// DOCKER_HOST to set the URL to the docker server. +// +// DOCKER_API_VERSION to set the version of the API to +// use, leave empty for latest. +// +// DOCKER_CERT_PATH to specify the directory from which to +// load the TLS certificates (ca.pem, cert.pem, key.pem). +// +// DOCKER_TLS_VERIFY to enable or disable TLS verification (off by +// default). func FromEnv(c *Client) error { ops := []Opt{ WithTLSClientConfigFromEnv(), @@ -75,8 +82,8 @@ func WithHost(host string) Opt { } // WithHostFromEnv overrides the client host with the host specified in the -// DOCKER_HOST environment variable. If DOCKER_HOST is not set, the host is -// not modified. +// DOCKER_HOST environment variable. If DOCKER_HOST is not set, +// or set to an empty value, the host is not modified. func WithHostFromEnv() Opt { return func(c *Client) error { if host := os.Getenv("DOCKER_HOST"); host != "" { @@ -145,9 +152,13 @@ func WithTLSClientConfig(cacertPath, certPath, keyPath string) Opt { // settings in the DOCKER_CERT_PATH and DOCKER_TLS_VERIFY environment variables. // If DOCKER_CERT_PATH is not set or empty, TLS configuration is not modified. // -// Supported environment variables: -// DOCKER_CERT_PATH directory to load the TLS certificates (ca.pem, cert.pem, key.pem) from. -// DOCKER_TLS_VERIFY to enable or disable TLS verification, off by default. +// WithTLSClientConfigFromEnv uses the following environment variables: +// +// DOCKER_CERT_PATH to specify the directory from which to +// load the TLS certificates (ca.pem, cert.pem, key.pem). +// +// DOCKER_TLS_VERIFY to enable or disable TLS verification (off by +// default). func WithTLSClientConfigFromEnv() Opt { return func(c *Client) error { dockerCertPath := os.Getenv("DOCKER_CERT_PATH") @@ -190,10 +201,7 @@ func WithVersion(version string) Opt { // the version is not modified. func WithVersionFromEnv() Opt { return func(c *Client) error { - if version := os.Getenv("DOCKER_API_VERSION"); version != "" { - return WithVersion(version)(c) - } - return nil + return WithVersion(os.Getenv("DOCKER_API_VERSION"))(c) } }