Skip to content

Commit a68ae4a

Browse files
committed
Improve docstrings and small cleanup in client
Use client instead of helpers for TLS in integration test Signed-off-by: Daniel Nephin <[email protected]>
1 parent 2b445a5 commit a68ae4a

3 files changed

Lines changed: 66 additions & 65 deletions

File tree

client/client.go

Lines changed: 54 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ For example, to list running containers (the equivalent of "docker ps"):
4242
package client // import "github.com/docker/docker/client"
4343

4444
import (
45-
"errors"
4645
"fmt"
46+
"net"
4747
"net/http"
4848
"net/url"
4949
"os"
@@ -56,6 +56,7 @@ import (
5656
"github.com/docker/docker/api/types/versions"
5757
"github.com/docker/go-connections/sockets"
5858
"github.com/docker/go-connections/tlsconfig"
59+
"github.com/pkg/errors"
5960
"golang.org/x/net/context"
6061
)
6162

@@ -103,18 +104,21 @@ func CheckRedirect(req *http.Request, via []*http.Request) error {
103104
}
104105

105106
// NewEnvClient initializes a new API client based on environment variables.
106-
// Use DOCKER_HOST to set the url to the docker server.
107-
// Use DOCKER_API_VERSION to set the version of the API to reach, leave empty for latest.
108-
// Use DOCKER_CERT_PATH to load the TLS certificates from.
109-
// Use DOCKER_TLS_VERIFY to enable or disable TLS verification, off by default.
110-
// deprecated: use NewClientWithOpts(FromEnv)
107+
// See FromEnv for a list of support environment variables.
108+
//
109+
// Deprecated: use NewClientWithOpts(FromEnv)
111110
func NewEnvClient() (*Client, error) {
112111
return NewClientWithOpts(FromEnv)
113112
}
114113

115-
// FromEnv enhance the default client with values from environment variables
114+
// FromEnv configures the client with values from environment variables.
115+
//
116+
// Supported environment variables:
117+
// DOCKER_HOST to set the url to the docker server.
118+
// DOCKER_API_VERSION to set the version of the API to reach, leave empty for latest.
119+
// DOCKER_CERT_PATH to load the TLS certificates from.
120+
// DOCKER_TLS_VERIFY to enable or disable TLS verification, off by default.
116121
func FromEnv(c *Client) error {
117-
var httpClient *http.Client
118122
if dockerCertPath := os.Getenv("DOCKER_CERT_PATH"); dockerCertPath != "" {
119123
options := tlsconfig.Options{
120124
CAFile: filepath.Join(dockerCertPath, "ca.pem"),
@@ -127,30 +131,58 @@ func FromEnv(c *Client) error {
127131
return err
128132
}
129133

130-
httpClient = &http.Client{
131-
Transport: &http.Transport{
132-
TLSClientConfig: tlsc,
133-
},
134+
c.client = &http.Client{
135+
Transport: &http.Transport{TLSClientConfig: tlsc},
134136
CheckRedirect: CheckRedirect,
135137
}
136-
WithHTTPClient(httpClient)(c)
137138
}
138139

139-
host := os.Getenv("DOCKER_HOST")
140-
if host != "" {
141-
// WithHost will create an API client if it doesn't exist
140+
if host := os.Getenv("DOCKER_HOST"); host != "" {
142141
if err := WithHost(host)(c); err != nil {
143142
return err
144143
}
145144
}
146-
version := os.Getenv("DOCKER_API_VERSION")
147-
if version != "" {
145+
146+
if version := os.Getenv("DOCKER_API_VERSION"); version != "" {
148147
c.version = version
149148
c.manualOverride = true
150149
}
151150
return nil
152151
}
153152

153+
// WithTLSClientConfig applies a tls config to the client transport.
154+
func WithTLSClientConfig(cacertPath, certPath, keyPath string) func(*Client) error {
155+
return func(c *Client) error {
156+
opts := tlsconfig.Options{
157+
CAFile: cacertPath,
158+
CertFile: certPath,
159+
KeyFile: keyPath,
160+
ExclusiveRootPools: true,
161+
}
162+
config, err := tlsconfig.Client(opts)
163+
if err != nil {
164+
return errors.Wrap(err, "failed to create tls config")
165+
}
166+
if transport, ok := c.client.Transport.(*http.Transport); ok {
167+
transport.TLSClientConfig = config
168+
return nil
169+
}
170+
return errors.Errorf("cannot apply tls config to transport: %T", c.client.Transport)
171+
}
172+
}
173+
174+
// WithDialer applies the dialer.DialContext to the client transport. This can be
175+
// used to set the Timeout and KeepAlive settings of the client.
176+
func WithDialer(dialer *net.Dialer) func(*Client) error {
177+
return func(c *Client) error {
178+
if transport, ok := c.client.Transport.(*http.Transport); ok {
179+
transport.DialContext = dialer.DialContext
180+
return nil
181+
}
182+
return errors.Errorf("cannot apply dialer to transport: %T", c.client.Transport)
183+
}
184+
}
185+
154186
// WithVersion overrides the client version with the specified one
155187
func WithVersion(version string) func(*Client) error {
156188
return func(c *Client) error {
@@ -159,8 +191,7 @@ func WithVersion(version string) func(*Client) error {
159191
}
160192
}
161193

162-
// WithHost overrides the client host with the specified one, creating a new
163-
// http client if one doesn't exist
194+
// WithHost overrides the client host with the specified one.
164195
func WithHost(host string) func(*Client) error {
165196
return func(c *Client) error {
166197
hostURL, err := ParseHostURL(host)
@@ -171,17 +202,10 @@ func WithHost(host string) func(*Client) error {
171202
c.proto = hostURL.Scheme
172203
c.addr = hostURL.Host
173204
c.basePath = hostURL.Path
174-
if c.client == nil {
175-
client, err := defaultHTTPClient(host)
176-
if err != nil {
177-
return err
178-
}
179-
return WithHTTPClient(client)(c)
180-
}
181205
if transport, ok := c.client.Transport.(*http.Transport); ok {
182206
return sockets.ConfigureTransport(transport, c.proto, c.addr)
183207
}
184-
return fmt.Errorf("cannot apply host to http transport")
208+
return errors.Errorf("cannot apply host to transport: %T", c.client.Transport)
185209
}
186210
}
187211

@@ -266,7 +290,7 @@ func defaultHTTPClient(host string) (*http.Client, error) {
266290
// It won't send any version information if the version number is empty. It is
267291
// highly recommended that you set a version or your client may break if the
268292
// server is upgraded.
269-
// deprecated: use NewClientWithOpts
293+
// Deprecated: use NewClientWithOpts
270294
func NewClient(host string, version string, client *http.Client, httpHeaders map[string]string) (*Client, error) {
271295
return NewClientWithOpts(WithHost(host), WithVersion(version), WithHTTPClient(client), WithHTTPHeaders(httpHeaders))
272296
}
@@ -378,6 +402,7 @@ func (cli *Client) CustomHTTPHeaders() map[string]string {
378402
}
379403

380404
// SetCustomHTTPHeaders that will be set on every HTTP request made by the client.
405+
// Deprecated: use WithHTTPHeaders when creating the client.
381406
func (cli *Client) SetCustomHTTPHeaders(headers map[string]string) {
382407
cli.customHTTPHeaders = headers
383408
}

integration/internal/request/client.go

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,13 @@ package request // import "github.com/docker/docker/integration/internal/request
22

33
import (
44
"fmt"
5-
"net"
6-
"net/http"
75
"testing"
86
"time"
97

108
"golang.org/x/net/context"
119

1210
"github.com/docker/docker/client"
1311
"github.com/docker/docker/internal/test/environment"
14-
"github.com/docker/go-connections/sockets"
15-
"github.com/docker/go-connections/tlsconfig"
1612
"github.com/stretchr/testify/require"
1713
)
1814

@@ -24,36 +20,6 @@ func NewAPIClient(t *testing.T, ops ...func(*client.Client) error) client.APICli
2420
return clt
2521
}
2622

27-
// NewTLSAPIClient returns a docker API client configured with the
28-
// provided TLS settings
29-
func NewTLSAPIClient(t *testing.T, host, cacertPath, certPath, keyPath string) (client.APIClient, error) {
30-
opts := tlsconfig.Options{
31-
CAFile: cacertPath,
32-
CertFile: certPath,
33-
KeyFile: keyPath,
34-
ExclusiveRootPools: true,
35-
}
36-
config, err := tlsconfig.Client(opts)
37-
require.Nil(t, err)
38-
tr := &http.Transport{
39-
TLSClientConfig: config,
40-
DialContext: (&net.Dialer{
41-
KeepAlive: 30 * time.Second,
42-
Timeout: 30 * time.Second,
43-
}).DialContext,
44-
}
45-
proto, addr, _, err := client.ParseHost(host)
46-
require.Nil(t, err)
47-
48-
sockets.ConfigureTransport(tr, proto, addr)
49-
50-
httpClient := &http.Client{
51-
Transport: tr,
52-
CheckRedirect: client.CheckRedirect,
53-
}
54-
return client.NewClientWithOpts(client.WithHost(host), client.WithHTTPClient(httpClient))
55-
}
56-
5723
// daemonTime provides the current time on the daemon host
5824
func daemonTime(ctx context.Context, t *testing.T, client client.APIClient, testEnv *environment.Execution) time.Time {
5925
if testEnv.IsLocalDaemon() {

integration/plugin/authz/authz_plugin_test.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
eventtypes "github.com/docker/docker/api/types/events"
2323
"github.com/docker/docker/client"
2424
"github.com/docker/docker/integration/internal/container"
25-
"github.com/docker/docker/integration/internal/request"
2625
"github.com/docker/docker/internal/test/environment"
2726
"github.com/docker/docker/pkg/authorization"
2827
"github.com/gotestyourself/gotestyourself/skip"
@@ -126,7 +125,7 @@ func TestAuthZPluginTLS(t *testing.T) {
126125
ctrl.reqRes.Allow = true
127126
ctrl.resRes.Allow = true
128127

129-
client, err := request.NewTLSAPIClient(t, testDaemonHTTPSAddr, cacertPath, clientCertPath, clientKeyPath)
128+
client, err := newTLSAPIClient(testDaemonHTTPSAddr, cacertPath, clientCertPath, clientKeyPath)
130129
require.Nil(t, err)
131130

132131
_, err = client.ServerVersion(context.Background())
@@ -136,6 +135,17 @@ func TestAuthZPluginTLS(t *testing.T) {
136135
require.Equal(t, "client", ctrl.resUser)
137136
}
138137

138+
func newTLSAPIClient(host, cacertPath, certPath, keyPath string) (client.APIClient, error) {
139+
dialer := &net.Dialer{
140+
KeepAlive: 30 * time.Second,
141+
Timeout: 30 * time.Second,
142+
}
143+
return client.NewClientWithOpts(
144+
client.WithTLSClientConfig(cacertPath, certPath, keyPath),
145+
client.WithDialer(dialer),
146+
client.WithHost(host))
147+
}
148+
139149
func TestAuthZPluginDenyRequest(t *testing.T) {
140150
defer setupTestV1(t)()
141151
d.Start(t, "--authorization-plugin="+testAuthZPlugin)

0 commit comments

Comments
 (0)