From 9a83d9fd53ad4b70b331423a2876d2ae07fc78c6 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 28 Sep 2017 17:10:06 -0400 Subject: [PATCH 1/4] Use gotestyourself env patching Signed-off-by: Daniel Nephin Upstream-commit: 2b445a53c17ae6526c11a729cb6d1d1dc57490ff Component: engine --- components/engine/client/client_test.go | 68 +++++-------------------- 1 file changed, 13 insertions(+), 55 deletions(-) diff --git a/components/engine/client/client_test.go b/components/engine/client/client_test.go index fe452c7aa2..5ae3a51239 100644 --- a/components/engine/client/client_test.go +++ b/components/engine/client/client_test.go @@ -6,18 +6,19 @@ import ( "net/url" "os" "runtime" - "strings" "testing" "github.com/docker/docker/api" "github.com/docker/docker/api/types" "github.com/docker/docker/internal/testutil" + "github.com/gotestyourself/gotestyourself/env" "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewEnvClient(t *testing.T) { - skip.IfCondition(t, runtime.GOOS == "windows") + skip.If(t, runtime.GOOS == "windows") testcases := []struct { doc string @@ -83,10 +84,9 @@ func TestNewEnvClient(t *testing.T) { }, } - env := envToMap() - defer mapToEnv(env) + defer env.PatchAll(t, nil)() for _, c := range testcases { - mapToEnv(c.envs) + env.PatchAll(t, c.envs) apiclient, err := NewEnvClient() if c.expectedError != "" { assert.Error(t, err, c.doc) @@ -196,16 +196,12 @@ func TestParseHostURL(t *testing.T) { } func TestNewEnvClientSetsDefaultVersion(t *testing.T) { - env := envToMap() - defer mapToEnv(env) - - envMap := map[string]string{ + defer env.PatchAll(t, map[string]string{ "DOCKER_HOST": "", "DOCKER_API_VERSION": "", "DOCKER_TLS_VERIFY": "", "DOCKER_CERT_PATH": "", - } - mapToEnv(envMap) + })() client, err := NewEnvClient() if err != nil { @@ -225,18 +221,10 @@ func TestNewEnvClientSetsDefaultVersion(t *testing.T) { // TestNegotiateAPIVersionEmpty asserts that client.Client can // negotiate a compatible APIVersion when omitted func TestNegotiateAPIVersionEmpty(t *testing.T) { - env := envToMap() - defer mapToEnv(env) - - envMap := map[string]string{ - "DOCKER_API_VERSION": "", - } - mapToEnv(envMap) + defer env.PatchAll(t, map[string]string{"DOCKER_API_VERSION": ""}) client, err := NewEnvClient() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) ping := types.Ping{ APIVersion: "", @@ -260,12 +248,9 @@ func TestNegotiateAPIVersionEmpty(t *testing.T) { // negotiate a compatible APIVersion with the server func TestNegotiateAPIVersion(t *testing.T) { client, err := NewEnvClient() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) expected := "1.21" - ping := types.Ping{ APIVersion: expected, OSType: "linux", @@ -291,18 +276,11 @@ func TestNegotiateAPIVersion(t *testing.T) { // TestNegotiateAPIVersionOverride asserts that we honor // the environment variable DOCKER_API_VERSION when negotianing versions func TestNegotiateAPVersionOverride(t *testing.T) { - env := envToMap() - defer mapToEnv(env) - - envMap := map[string]string{ - "DOCKER_API_VERSION": "9.99", - } - mapToEnv(envMap) + expected := "9.99" + defer env.PatchAll(t, map[string]string{"DOCKER_API_VERSION": expected})() client, err := NewEnvClient() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) ping := types.Ping{ APIVersion: "1.24", @@ -310,31 +288,11 @@ func TestNegotiateAPVersionOverride(t *testing.T) { Experimental: false, } - expected := envMap["DOCKER_API_VERSION"] - // test that we honored the env var client.NegotiateAPIVersionPing(ping) assert.Equal(t, expected, client.version) } -// mapToEnv takes a map of environment variables and sets them -func mapToEnv(env map[string]string) { - for k, v := range env { - os.Setenv(k, v) - } -} - -// envToMap returns a map of environment variables -func envToMap() map[string]string { - env := make(map[string]string) - for _, e := range os.Environ() { - kv := strings.SplitAfterN(e, "=", 2) - env[kv[0]] = kv[1] - } - - return env -} - type roundTripFunc func(*http.Request) (*http.Response, error) func (rtf roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { From 180ce350666476a5e285ff145ed5e8c5fb42c4bf Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 16 Feb 2018 13:24:57 -0500 Subject: [PATCH 2/4] Improve docstrings and small cleanup in client Use client instead of helpers for TLS in integration test Signed-off-by: Daniel Nephin Upstream-commit: a68ae4a2d95b1ff143025a435195af0f1ab30ace Component: engine --- components/engine/client/client.go | 83 ++++++++++++------- .../integration/internal/request/client.go | 34 -------- .../plugin/authz/authz_plugin_test.go | 14 +++- 3 files changed, 66 insertions(+), 65 deletions(-) diff --git a/components/engine/client/client.go b/components/engine/client/client.go index 6ce0cdba1f..f838501760 100644 --- a/components/engine/client/client.go +++ b/components/engine/client/client.go @@ -42,8 +42,8 @@ For example, to list running containers (the equivalent of "docker ps"): package client // import "github.com/docker/docker/client" import ( - "errors" "fmt" + "net" "net/http" "net/url" "os" @@ -56,6 +56,7 @@ import ( "github.com/docker/docker/api/types/versions" "github.com/docker/go-connections/sockets" "github.com/docker/go-connections/tlsconfig" + "github.com/pkg/errors" "golang.org/x/net/context" ) @@ -103,18 +104,21 @@ func CheckRedirect(req *http.Request, via []*http.Request) error { } // NewEnvClient initializes a new API client based on environment variables. -// Use DOCKER_HOST to set the url to the docker server. -// Use DOCKER_API_VERSION to set the version of the API to reach, leave empty for latest. -// Use DOCKER_CERT_PATH to load the TLS certificates from. -// Use DOCKER_TLS_VERIFY to enable or disable TLS verification, off by default. -// deprecated: use NewClientWithOpts(FromEnv) +// See FromEnv for a list of support environment variables. +// +// Deprecated: use NewClientWithOpts(FromEnv) func NewEnvClient() (*Client, error) { return NewClientWithOpts(FromEnv) } -// FromEnv enhance the default client with values from environment variables +// 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. func FromEnv(c *Client) error { - var httpClient *http.Client if dockerCertPath := os.Getenv("DOCKER_CERT_PATH"); dockerCertPath != "" { options := tlsconfig.Options{ CAFile: filepath.Join(dockerCertPath, "ca.pem"), @@ -127,30 +131,58 @@ func FromEnv(c *Client) error { return err } - httpClient = &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: tlsc, - }, + c.client = &http.Client{ + Transport: &http.Transport{TLSClientConfig: tlsc}, CheckRedirect: CheckRedirect, } - WithHTTPClient(httpClient)(c) } - host := os.Getenv("DOCKER_HOST") - if host != "" { - // WithHost will create an API client if it doesn't exist + if host := os.Getenv("DOCKER_HOST"); host != "" { if err := WithHost(host)(c); err != nil { return err } } - version := os.Getenv("DOCKER_API_VERSION") - if version != "" { + + if version := os.Getenv("DOCKER_API_VERSION"); version != "" { c.version = version c.manualOverride = true } return nil } +// WithTLSClientConfig applies a tls config to the client transport. +func WithTLSClientConfig(cacertPath, certPath, keyPath string) func(*Client) error { + return func(c *Client) error { + opts := tlsconfig.Options{ + CAFile: cacertPath, + CertFile: certPath, + KeyFile: keyPath, + ExclusiveRootPools: true, + } + config, err := tlsconfig.Client(opts) + if err != nil { + return errors.Wrap(err, "failed to create tls config") + } + if transport, ok := c.client.Transport.(*http.Transport); ok { + transport.TLSClientConfig = config + return nil + } + return errors.Errorf("cannot apply tls config to transport: %T", c.client.Transport) + } +} + +// WithDialer applies the dialer.DialContext to the client transport. This can be +// used to set the Timeout and KeepAlive settings of the client. +func WithDialer(dialer *net.Dialer) func(*Client) error { + return func(c *Client) error { + if transport, ok := c.client.Transport.(*http.Transport); ok { + transport.DialContext = dialer.DialContext + return nil + } + return errors.Errorf("cannot apply dialer to transport: %T", c.client.Transport) + } +} + // WithVersion overrides the client version with the specified one func WithVersion(version string) func(*Client) error { return func(c *Client) error { @@ -159,8 +191,7 @@ func WithVersion(version string) func(*Client) error { } } -// WithHost overrides the client host with the specified one, creating a new -// http client if one doesn't exist +// WithHost overrides the client host with the specified one. func WithHost(host string) func(*Client) error { return func(c *Client) error { hostURL, err := ParseHostURL(host) @@ -171,17 +202,10 @@ func WithHost(host string) func(*Client) error { c.proto = hostURL.Scheme c.addr = hostURL.Host c.basePath = hostURL.Path - if c.client == nil { - client, err := defaultHTTPClient(host) - if err != nil { - return err - } - return WithHTTPClient(client)(c) - } if transport, ok := c.client.Transport.(*http.Transport); ok { return sockets.ConfigureTransport(transport, c.proto, c.addr) } - return fmt.Errorf("cannot apply host to http transport") + return errors.Errorf("cannot apply host to transport: %T", c.client.Transport) } } @@ -266,7 +290,7 @@ func defaultHTTPClient(host string) (*http.Client, error) { // 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. -// deprecated: use NewClientWithOpts +// Deprecated: use NewClientWithOpts func NewClient(host string, version string, client *http.Client, httpHeaders map[string]string) (*Client, error) { return NewClientWithOpts(WithHost(host), WithVersion(version), WithHTTPClient(client), WithHTTPHeaders(httpHeaders)) } @@ -378,6 +402,7 @@ func (cli *Client) CustomHTTPHeaders() map[string]string { } // SetCustomHTTPHeaders that will be set on every HTTP request made by the client. +// Deprecated: use WithHTTPHeaders when creating the client. func (cli *Client) SetCustomHTTPHeaders(headers map[string]string) { cli.customHTTPHeaders = headers } diff --git a/components/engine/integration/internal/request/client.go b/components/engine/integration/internal/request/client.go index 4bfb5c7673..367db14c59 100644 --- a/components/engine/integration/internal/request/client.go +++ b/components/engine/integration/internal/request/client.go @@ -2,8 +2,6 @@ package request // import "github.com/docker/docker/integration/internal/request import ( "fmt" - "net" - "net/http" "testing" "time" @@ -11,8 +9,6 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/internal/test/environment" - "github.com/docker/go-connections/sockets" - "github.com/docker/go-connections/tlsconfig" "github.com/stretchr/testify/require" ) @@ -24,36 +20,6 @@ func NewAPIClient(t *testing.T, ops ...func(*client.Client) error) client.APICli return clt } -// NewTLSAPIClient returns a docker API client configured with the -// provided TLS settings -func NewTLSAPIClient(t *testing.T, host, cacertPath, certPath, keyPath string) (client.APIClient, error) { - opts := tlsconfig.Options{ - CAFile: cacertPath, - CertFile: certPath, - KeyFile: keyPath, - ExclusiveRootPools: true, - } - config, err := tlsconfig.Client(opts) - require.Nil(t, err) - tr := &http.Transport{ - TLSClientConfig: config, - DialContext: (&net.Dialer{ - KeepAlive: 30 * time.Second, - Timeout: 30 * time.Second, - }).DialContext, - } - proto, addr, _, err := client.ParseHost(host) - require.Nil(t, err) - - sockets.ConfigureTransport(tr, proto, addr) - - httpClient := &http.Client{ - Transport: tr, - CheckRedirect: client.CheckRedirect, - } - return client.NewClientWithOpts(client.WithHost(host), client.WithHTTPClient(httpClient)) -} - // daemonTime provides the current time on the daemon host func daemonTime(ctx context.Context, t *testing.T, client client.APIClient, testEnv *environment.Execution) time.Time { if testEnv.IsLocalDaemon() { diff --git a/components/engine/integration/plugin/authz/authz_plugin_test.go b/components/engine/integration/plugin/authz/authz_plugin_test.go index 94f7b896a7..667fc3d3cc 100644 --- a/components/engine/integration/plugin/authz/authz_plugin_test.go +++ b/components/engine/integration/plugin/authz/authz_plugin_test.go @@ -22,7 +22,6 @@ import ( eventtypes "github.com/docker/docker/api/types/events" "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" - "github.com/docker/docker/integration/internal/request" "github.com/docker/docker/internal/test/environment" "github.com/docker/docker/pkg/authorization" "github.com/gotestyourself/gotestyourself/skip" @@ -126,7 +125,7 @@ func TestAuthZPluginTLS(t *testing.T) { ctrl.reqRes.Allow = true ctrl.resRes.Allow = true - client, err := request.NewTLSAPIClient(t, testDaemonHTTPSAddr, cacertPath, clientCertPath, clientKeyPath) + client, err := newTLSAPIClient(testDaemonHTTPSAddr, cacertPath, clientCertPath, clientKeyPath) require.Nil(t, err) _, err = client.ServerVersion(context.Background()) @@ -136,6 +135,17 @@ func TestAuthZPluginTLS(t *testing.T) { require.Equal(t, "client", ctrl.resUser) } +func newTLSAPIClient(host, cacertPath, certPath, keyPath string) (client.APIClient, error) { + dialer := &net.Dialer{ + KeepAlive: 30 * time.Second, + Timeout: 30 * time.Second, + } + return client.NewClientWithOpts( + client.WithTLSClientConfig(cacertPath, certPath, keyPath), + client.WithDialer(dialer), + client.WithHost(host)) +} + func TestAuthZPluginDenyRequest(t *testing.T) { defer setupTestV1(t)() d.Start(t, "--authorization-plugin="+testAuthZPlugin) From 49adb54d7197eb5a5bd814f68080079e1c59cc04 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 19 Sep 2017 16:12:29 -0400 Subject: [PATCH 3/4] Remove duplicate calls for getting an APIClient Remove request.SockRequest Remove request.SockRequestHijack Remove request.SockRequestRaw() Remove deprecated ParseHost Deprecate and unexport more helpers Signed-off-by: Daniel Nephin Upstream-commit: 0a91ba2d8cfe16df0ba37c1e283c8e3dbbb086d4 Component: engine --- components/engine/client/client.go | 11 -- components/engine/client/client_test.go | 26 ---- components/engine/client/interface.go | 1 + components/engine/client/request.go | 5 +- components/engine/client/request_test.go | 17 ++- .../engine/integration-cli/daemon/daemon.go | 33 +---- .../integration-cli/daemon/daemon_swarm.go | 35 +++--- .../integration-cli/docker_api_attach_test.go | 58 +++++++-- .../integration-cli/docker_api_build_test.go | 16 +-- .../docker_api_containers_windows_test.go | 49 ++++---- .../docker_api_exec_resize_test.go | 2 +- .../integration-cli/docker_api_images_test.go | 24 +--- .../docker_api_ipcmode_test.go | 11 +- .../integration-cli/docker_api_swarm_test.go | 42 +++---- .../docker_cli_plugins_test.go | 16 +-- .../engine/integration-cli/request/request.go | 117 +++--------------- .../integration-cli/trust_server_test.go | 6 +- 17 files changed, 158 insertions(+), 311 deletions(-) diff --git a/components/engine/client/client.go b/components/engine/client/client.go index f838501760..e129bb20f3 100644 --- a/components/engine/client/client.go +++ b/components/engine/client/client.go @@ -356,17 +356,6 @@ func (cli *Client) DaemonHost() string { return cli.host } -// ParseHost parses a url string, validates the strings is a host url, and returns -// the parsed host as: protocol, address, and base path -// Deprecated: use ParseHostURL -func ParseHost(host string) (string, string, string, error) { - hostURL, err := ParseHostURL(host) - if err != nil { - return "", "", "", err - } - return hostURL.Scheme, hostURL.Host, hostURL.Path, nil -} - // ParseHostURL parses a url string, validates the string is a host url, and // returns the parsed URL func ParseHostURL(host string) (*url.URL, error) { diff --git a/components/engine/client/client_test.go b/components/engine/client/client_test.go index 5ae3a51239..db394e24b5 100644 --- a/components/engine/client/client_test.go +++ b/components/engine/client/client_test.go @@ -132,32 +132,6 @@ func TestGetAPIPath(t *testing.T) { } } -func TestParseHost(t *testing.T) { - cases := []struct { - host string - proto string - addr string - base string - err bool - }{ - {"", "", "", "", true}, - {"foobar", "", "", "", true}, - {"foo://bar", "foo", "bar", "", false}, - {"tcp://localhost:2476", "tcp", "localhost:2476", "", false}, - {"tcp://localhost:2476/path", "tcp", "localhost:2476", "/path", false}, - } - - for _, cs := range cases { - p, a, b, e := ParseHost(cs.host) - if cs.err { - assert.Error(t, e) - } - assert.Equal(t, cs.proto, p) - assert.Equal(t, cs.addr, a) - assert.Equal(t, cs.base, b) - } -} - func TestParseHostURL(t *testing.T) { testcases := []struct { host string diff --git a/components/engine/client/interface.go b/components/engine/client/interface.go index e928e647a7..8517546abd 100644 --- a/components/engine/client/interface.go +++ b/components/engine/client/interface.go @@ -37,6 +37,7 @@ type CommonAPIClient interface { NegotiateAPIVersion(ctx context.Context) NegotiateAPIVersionPing(types.Ping) DialSession(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) + Close() error } // ContainerAPIClient defines API client methods for the containers diff --git a/components/engine/client/request.go b/components/engine/client/request.go index 986b512dda..302f599f5c 100644 --- a/components/engine/client/request.go +++ b/components/engine/client/request.go @@ -123,10 +123,7 @@ func (cli *Client) sendRequest(ctx context.Context, method, path string, query u if err != nil { return resp, err } - if err := cli.checkResponseErr(resp); err != nil { - return resp, err - } - return resp, nil + return resp, cli.checkResponseErr(resp) } func (cli *Client) doRequest(ctx context.Context, req *http.Request) (serverResponse, error) { diff --git a/components/engine/client/request_test.go b/components/engine/client/request_test.go index f3c136a04f..1dbfed62c7 100644 --- a/components/engine/client/request_test.go +++ b/components/engine/client/request_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/docker/docker/api/types" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -44,10 +45,8 @@ func TestSetHostHeader(t *testing.T) { } for c, test := range testCases { - proto, addr, basePath, err := ParseHost(test.host) - if err != nil { - t.Fatal(err) - } + hostURL, err := ParseHostURL(test.host) + require.NoError(t, err) client := &Client{ client: newMockClient(func(req *http.Request) (*http.Response, error) { @@ -66,15 +65,13 @@ func TestSetHostHeader(t *testing.T) { }, nil }), - proto: proto, - addr: addr, - basePath: basePath, + proto: hostURL.Scheme, + addr: hostURL.Host, + basePath: hostURL.Path, } _, err = client.sendRequest(context.Background(), "GET", testURL, nil, nil, nil) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } } diff --git a/components/engine/integration-cli/daemon/daemon.go b/components/engine/integration-cli/daemon/daemon.go index 69a87d9d9c..9672d160f8 100644 --- a/components/engine/integration-cli/daemon/daemon.go +++ b/components/engine/integration-cli/daemon/daemon.go @@ -1,7 +1,6 @@ package daemon // import "github.com/docker/docker/integration-cli/daemon" import ( - "bytes" "encoding/json" "fmt" "io" @@ -14,7 +13,6 @@ import ( "strings" "time" - "github.com/docker/docker/api" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/events" "github.com/docker/docker/client" @@ -596,28 +594,6 @@ func (d *Daemon) PrependHostArg(args []string) []string { return append([]string{"--host", d.Sock()}, args...) } -// SockRequest executes a socket request on a daemon and returns statuscode and output. -func (d *Daemon) SockRequest(method, endpoint string, data interface{}) (int, []byte, error) { - jsonData := bytes.NewBuffer(nil) - if err := json.NewEncoder(jsonData).Encode(data); err != nil { - return -1, nil, err - } - - res, body, err := d.SockRequestRaw(method, endpoint, jsonData, "application/json") - if err != nil { - return -1, nil, err - } - b, err := request.ReadBody(body) - return res.StatusCode, b, err -} - -// SockRequestRaw executes a socket request on a daemon and returns an http -// response and a reader for the output data. -// Deprecated: use request package instead -func (d *Daemon) SockRequestRaw(method, endpoint string, data io.Reader, ct string) (*http.Response, io.ReadCloser, error) { - return request.SockRequestRaw(method, endpoint, data, ct, d.Sock()) -} - // LogFileName returns the path the daemon's log file func (d *Daemon) LogFileName() string { return d.logFile.Name() @@ -746,12 +722,9 @@ func (d *Daemon) ReloadConfig() error { // NewClient creates new client based on daemon's socket path func (d *Daemon) NewClient() (*client.Client, error) { - httpClient, err := request.NewHTTPClient(d.Sock()) - if err != nil { - return nil, err - } - - return client.NewClient(d.Sock(), api.DefaultVersion, httpClient, nil) + return client.NewClientWithOpts( + client.FromEnv, + client.WithHost(d.Sock())) } // WaitInspectWithArgs waits for the specified expression to be equals to the specified expected string in the given time. diff --git a/components/engine/integration-cli/daemon/daemon_swarm.go b/components/engine/integration-cli/daemon/daemon_swarm.go index 4be0acc9e9..cb44f63f23 100644 --- a/components/engine/integration-cli/daemon/daemon_swarm.go +++ b/components/engine/integration-cli/daemon/daemon_swarm.go @@ -1,18 +1,18 @@ package daemon // import "github.com/docker/docker/integration-cli/daemon" import ( - "encoding/json" "fmt" - "net/http" "strings" "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/client" "github.com/docker/docker/integration-cli/checker" "github.com/go-check/check" "github.com/pkg/errors" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -234,31 +234,28 @@ func (d *Swarm) CheckServiceUpdateState(service string) func(*check.C) (interfac // CheckPluginRunning returns the runtime state of the plugin func (d *Swarm) CheckPluginRunning(plugin string) func(c *check.C) (interface{}, check.CommentInterface) { return func(c *check.C) (interface{}, check.CommentInterface) { - status, out, err := d.SockRequest("GET", "/plugins/"+plugin+"/json", nil) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - if status != http.StatusOK { - return false, nil + apiclient, err := d.NewClient() + require.NoError(c, err) + resp, _, err := apiclient.PluginInspectWithRaw(context.Background(), plugin) + if client.IsErrNotFound(err) { + return false, check.Commentf("%v", err) } - - var p types.Plugin - c.Assert(json.Unmarshal(out, &p), checker.IsNil, check.Commentf(string(out))) - - return p.Enabled, check.Commentf("%+v", p) + require.NoError(c, err) + return resp.Enabled, check.Commentf("%+v", resp) } } // CheckPluginImage returns the runtime state of the plugin func (d *Swarm) CheckPluginImage(plugin string) func(c *check.C) (interface{}, check.CommentInterface) { return func(c *check.C) (interface{}, check.CommentInterface) { - status, out, err := d.SockRequest("GET", "/plugins/"+plugin+"/json", nil) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - if status != http.StatusOK { - return false, nil + apiclient, err := d.NewClient() + require.NoError(c, err) + resp, _, err := apiclient.PluginInspectWithRaw(context.Background(), plugin) + if client.IsErrNotFound(err) { + return false, check.Commentf("%v", err) } - - var p types.Plugin - c.Assert(json.Unmarshal(out, &p), checker.IsNil, check.Commentf(string(out))) - return p.PluginReference, check.Commentf("%+v", p) + require.NoError(c, err) + return resp.PluginReference, check.Commentf("%+v", resp) } } diff --git a/components/engine/integration-cli/docker_api_attach_test.go b/components/engine/integration-cli/docker_api_attach_test.go index a3aa40f6a4..e191f278f7 100644 --- a/components/engine/integration-cli/docker_api_attach_test.go +++ b/components/engine/integration-cli/docker_api_attach_test.go @@ -4,9 +4,11 @@ import ( "bufio" "bytes" "context" + "fmt" "io" "net" "net/http" + "net/http/httputil" "strings" "time" @@ -73,10 +75,8 @@ func (s *DockerSuite) TestGetContainersAttachWebsocket(c *check.C) { // regression gh14320 func (s *DockerSuite) TestPostContainersAttachContainerNotFound(c *check.C) { - client, err := request.NewHTTPClient(daemonHost()) + resp, _, err := request.Post("/containers/doesnotexist/attach") c.Assert(err, checker.IsNil) - req, err := request.New(daemonHost(), "/containers/doesnotexist/attach", request.Method(http.MethodPost)) - resp, err := client.Do(req) // connection will shutdown, err should be "persistent connection closed" c.Assert(resp.StatusCode, checker.Equals, http.StatusNotFound) content, err := request.ReadBody(resp.Body) @@ -140,12 +140,12 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { cid, _ := dockerCmd(c, "run", "-di", "busybox", "cat") cid = strings.TrimSpace(cid) // Attach to the container's stdout stream. - conn, br, err := request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) + conn, br, err := sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) // Check if the data from stdout can be received. expectSuccess(conn, br, "stdout", false) // Attach to the container's stderr stream. - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) // Since the container only emits stdout, attaching to stderr should return nothing. expectTimeout(conn, br, "stdout") @@ -153,10 +153,10 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { // Test the similar functions of the stderr stream. cid, _ = dockerCmd(c, "run", "-di", "busybox", "/bin/sh", "-c", "cat >&2") cid = strings.TrimSpace(cid) - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) expectSuccess(conn, br, "stderr", false) - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) expectTimeout(conn, br, "stderr") @@ -164,12 +164,12 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { cid, _ = dockerCmd(c, "run", "-dit", "busybox", "/bin/sh", "-c", "cat >&2") cid = strings.TrimSpace(cid) // Attach to stdout only. - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) expectSuccess(conn, br, "stdout", true) // Attach without stdout stream. - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) // Nothing should be received because both the stdout and stderr of the container will be // sent to the client as stdout when tty is enabled. @@ -210,3 +210,43 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { stdcopy.StdCopy(actualStdout, actualStderr, resp.Reader) c.Assert(actualStdout.Bytes(), checker.DeepEquals, []byte("hello\nsuccess"), check.Commentf("Attach didn't return the expected data from stdout")) } + +// SockRequestHijack creates a connection to specified host (with method, contenttype, …) and returns a hijacked connection +// and the output as a `bufio.Reader` +func sockRequestHijack(method, endpoint string, data io.Reader, ct string, daemon string, modifiers ...func(*http.Request)) (net.Conn, *bufio.Reader, error) { + req, client, err := newRequestClient(method, endpoint, data, ct, daemon, modifiers...) + if err != nil { + return nil, nil, err + } + + client.Do(req) + conn, br := client.Hijack() + return conn, br, nil +} + +// FIXME(vdemeester) httputil.ClientConn is deprecated, use http.Client instead (closer to actual client) +// Deprecated: Use New instead of NewRequestClient +// Deprecated: use request.Do (or Get, Delete, Post) instead +func newRequestClient(method, endpoint string, data io.Reader, ct, daemon string, modifiers ...func(*http.Request)) (*http.Request, *httputil.ClientConn, error) { + c, err := request.SockConn(time.Duration(10*time.Second), daemon) + if err != nil { + return nil, nil, fmt.Errorf("could not dial docker daemon: %v", err) + } + + client := httputil.NewClientConn(c, nil) + + req, err := http.NewRequest(method, endpoint, data) + if err != nil { + client.Close() + return nil, nil, fmt.Errorf("could not create new request: %v", err) + } + + for _, opt := range modifiers { + opt(req) + } + + if ct != "" { + req.Header.Set("Content-Type", ct) + } + return req, client, nil +} diff --git a/components/engine/integration-cli/docker_api_build_test.go b/components/engine/integration-cli/docker_api_build_test.go index 6da8e5bb8f..e5423a4741 100644 --- a/components/engine/integration-cli/docker_api_build_test.go +++ b/components/engine/integration-cli/docker_api_build_test.go @@ -319,8 +319,7 @@ func (s *DockerSuite) TestBuildOnBuildCache(c *check.C) { assert.Len(c, imageIDs, 2) parentID, childID := imageIDs[0], imageIDs[1] - client, err := request.NewClient() - require.NoError(c, err) + client := testEnv.APIClient() // check parentID is correct image, _, err := client.ImageInspectWithRaw(context.Background(), childID) @@ -329,12 +328,11 @@ func (s *DockerSuite) TestBuildOnBuildCache(c *check.C) { } func (s *DockerRegistrySuite) TestBuildCopyFromForcePull(c *check.C) { - client, err := request.NewClient() - require.NoError(c, err) + client := testEnv.APIClient() repoName := fmt.Sprintf("%v/dockercli/busybox", privateRegistryURL) // tag the image to upload it to the private registry - err = client.ImageTag(context.TODO(), "busybox", repoName) + err := client.ImageTag(context.TODO(), "busybox", repoName) assert.Nil(c, err) // push the image to the registry rc, err := client.ImagePush(context.TODO(), repoName, types.ImagePushOptions{RegistryAuth: "{}"}) @@ -545,9 +543,7 @@ func (s *DockerSuite) TestBuildWithSession(c *check.C) { assert.Equal(c, strings.Count(out, "Using cache"), 2) assert.Contains(c, out, "contentcontent") - client, err := request.NewClient() - require.NoError(c, err) - + client := testEnv.APIClient() du, err := client.DiskUsage(context.TODO()) assert.Nil(c, err) assert.True(c, du.BuilderSize > 10) @@ -582,9 +578,7 @@ func (s *DockerSuite) TestBuildWithSession(c *check.C) { } func testBuildWithSession(c *check.C, dir, dockerfile string) (outStr string) { - client, err := request.NewClient() - require.NoError(c, err) - + client := testEnv.APIClient() sess, err := session.NewSession("foo1", "foo") assert.Nil(c, err) diff --git a/components/engine/integration-cli/docker_api_containers_windows_test.go b/components/engine/integration-cli/docker_api_containers_windows_test.go index 25bc767e55..eb2892575c 100644 --- a/components/engine/integration-cli/docker_api_containers_windows_test.go +++ b/components/engine/integration-cli/docker_api_containers_windows_test.go @@ -6,13 +6,16 @@ import ( "fmt" "io/ioutil" "math/rand" - "net/http" "strings" winio "github.com/Microsoft/go-winio" - "github.com/docker/docker/integration-cli/checker" - "github.com/docker/docker/integration-cli/request" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/mount" "github.com/go-check/check" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/net/context" ) func (s *DockerSuite) TestContainersAPICreateMountsBindNamedPipe(c *check.C) { @@ -44,28 +47,30 @@ func (s *DockerSuite) TestContainersAPICreateMountsBindNamedPipe(c *check.C) { containerPipeName := `\\.\pipe\docker-cli-test-pipe` text := "hello from a pipe" cmd := fmt.Sprintf("echo %s > %s", text, containerPipeName) - name := "test-bind-npipe" - data := map[string]interface{}{ - "Image": testEnv.PlatformDefaults.BaseImage, - "Cmd": []string{"cmd", "/c", cmd}, - "HostConfig": map[string]interface{}{"Mounts": []map[string]interface{}{{"Type": "npipe", "Source": hostPipeName, "Target": containerPipeName}}}, - } - status, resp, err := request.SockRequest("POST", "/containers/create?name="+name, data, daemonHost()) - c.Assert(err, checker.IsNil, check.Commentf(string(resp))) - c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(resp))) + ctx := context.Background() + client := testEnv.APIClient() + _, err = client.ContainerCreate(ctx, + &container.Config{ + Image: testEnv.PlatformDefaults.BaseImage, + Cmd: []string{"cmd", "/c", cmd}, + }, &container.HostConfig{ + Mounts: []mount.Mount{ + { + Type: "npipe", + Source: hostPipeName, + Target: containerPipeName, + }, + }, + }, + nil, name) + require.NoError(c, err) - status, _, err = request.SockRequest("POST", "/containers/"+name+"/start", nil, daemonHost()) - c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusNoContent) + err = client.ContainerStart(ctx, name, types.ContainerStartOptions{}) + require.NoError(c, err) err = <-ch - if err != nil { - c.Fatal(err) - } - result := strings.TrimSpace(string(b)) - if result != text { - c.Errorf("expected pipe to contain %s, got %s", text, result) - } + require.NoError(c, err) + assert.Equal(c, text, strings.TrimSpace(string(b))) } diff --git a/components/engine/integration-cli/docker_api_exec_resize_test.go b/components/engine/integration-cli/docker_api_exec_resize_test.go index 961d911cbd..7289b2689c 100644 --- a/components/engine/integration-cli/docker_api_exec_resize_test.go +++ b/components/engine/integration-cli/docker_api_exec_resize_test.go @@ -59,7 +59,7 @@ func (s *DockerSuite) TestExecResizeImmediatelyAfterExecStart(c *check.C) { } payload := bytes.NewBufferString(`{"Tty":true}`) - conn, _, err := request.SockRequestHijack("POST", fmt.Sprintf("/exec/%s/start", execID), payload, "application/json", daemonHost()) + conn, _, err := sockRequestHijack("POST", fmt.Sprintf("/exec/%s/start", execID), payload, "application/json", daemonHost()) if err != nil { return fmt.Errorf("Failed to start the exec: %q", err.Error()) } diff --git a/components/engine/integration-cli/docker_api_images_test.go b/components/engine/integration-cli/docker_api_images_test.go index a5628e41b9..7c6c235354 100644 --- a/components/engine/integration-cli/docker_api_images_test.go +++ b/components/engine/integration-cli/docker_api_images_test.go @@ -157,33 +157,21 @@ func (s *DockerSuite) TestAPIImagesSearchJSONContentType(c *check.C) { // Test case for 30027: image size reported as -1 in v1.12 client against v1.13 daemon. // This test checks to make sure both v1.12 and v1.13 client against v1.13 daemon get correct `Size` after the fix. func (s *DockerSuite) TestAPIImagesSizeCompatibility(c *check.C) { - cli, err := client.NewEnvClient() - c.Assert(err, checker.IsNil) - defer cli.Close() + apiclient := testEnv.APIClient() + defer apiclient.Close() - images, err := cli.ImageList(context.Background(), types.ImageListOptions{}) + images, err := apiclient.ImageList(context.Background(), types.ImageListOptions{}) c.Assert(err, checker.IsNil) c.Assert(len(images), checker.Not(checker.Equals), 0) for _, image := range images { c.Assert(image.Size, checker.Not(checker.Equals), int64(-1)) } - type v124Image struct { - ID string `json:"Id"` - ParentID string `json:"ParentId"` - RepoTags []string - RepoDigests []string - Created int64 - Size int64 - VirtualSize int64 - Labels map[string]string - } - - cli, err = client.NewClientWithOpts(client.FromEnv, client.WithVersion("v1.24")) + apiclient, err = client.NewClientWithOpts(client.FromEnv, client.WithVersion("v1.24")) c.Assert(err, checker.IsNil) - defer cli.Close() + defer apiclient.Close() - v124Images, err := cli.ImageList(context.Background(), types.ImageListOptions{}) + v124Images, err := apiclient.ImageList(context.Background(), types.ImageListOptions{}) c.Assert(err, checker.IsNil) c.Assert(len(v124Images), checker.Not(checker.Equals), 0) for _, image := range v124Images { diff --git a/components/engine/integration-cli/docker_api_ipcmode_test.go b/components/engine/integration-cli/docker_api_ipcmode_test.go index 02548875a3..5bca6d0d78 100644 --- a/components/engine/integration-cli/docker_api_ipcmode_test.go +++ b/components/engine/integration-cli/docker_api_ipcmode_test.go @@ -12,7 +12,6 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/cli" - "github.com/docker/docker/integration-cli/request" "github.com/go-check/check" "golang.org/x/net/context" ) @@ -59,8 +58,7 @@ func testIpcNonePrivateShareable(c *check.C, mode string, mustBeMounted bool, mu } ctx := context.Background() - client, err := request.NewClient() - c.Assert(err, checker.IsNil) + client := testEnv.APIClient() resp, err := client.ContainerCreate(ctx, &cfg, &hostCfg, nil, "") c.Assert(err, checker.IsNil) @@ -125,8 +123,7 @@ func testIpcContainer(s *DockerSuite, c *check.C, donorMode string, mustWork boo } ctx := context.Background() - client, err := request.NewClient() - c.Assert(err, checker.IsNil) + client := testEnv.APIClient() // create and start the "donor" container resp, err := client.ContainerCreate(ctx, &cfg, &hostCfg, nil, "") @@ -195,9 +192,7 @@ func (s *DockerSuite) TestAPIIpcModeHost(c *check.C) { } ctx := context.Background() - client, err := request.NewClient() - c.Assert(err, checker.IsNil) - + client := testEnv.APIClient() resp, err := client.ContainerCreate(ctx, &cfg, &hostCfg, nil, "") c.Assert(err, checker.IsNil) c.Assert(len(resp.Warnings), checker.Equals, 0) diff --git a/components/engine/integration-cli/docker_api_swarm_test.go b/components/engine/integration-cli/docker_api_swarm_test.go index a9b0458038..fd0b4e6278 100644 --- a/components/engine/integration-cli/docker_api_swarm_test.go +++ b/components/engine/integration-cli/docker_api_swarm_test.go @@ -3,12 +3,10 @@ package main import ( - "encoding/json" "fmt" "io/ioutil" "net" "net/http" - "net/url" "os" "path/filepath" "strings" @@ -21,11 +19,14 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/client" "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/daemon" "github.com/docker/docker/integration-cli/request" "github.com/docker/swarmkit/ca" "github.com/go-check/check" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -1006,32 +1007,19 @@ func (s *DockerSwarmSuite) TestSwarmRepeatedRootRotation(c *check.C) { func (s *DockerSwarmSuite) TestAPINetworkInspectWithScope(c *check.C) { d := s.AddDaemon(c, true, true) - name := "foo" - networkCreateRequest := types.NetworkCreateRequest{ - Name: name, - } + name := "test-scoped-network" + ctx := context.Background() + apiclient, err := d.NewClient() + require.NoError(c, err) - var n types.NetworkCreateResponse - networkCreateRequest.NetworkCreate.Driver = "overlay" + resp, err := apiclient.NetworkCreate(ctx, name, types.NetworkCreate{Driver: "overlay"}) + require.NoError(c, err) - status, out, err := d.SockRequest("POST", "/networks/create", networkCreateRequest) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(out))) - c.Assert(json.Unmarshal(out, &n), checker.IsNil) + network, err := apiclient.NetworkInspect(ctx, name, types.NetworkInspectOptions{}) + require.NoError(c, err) + assert.Equal(c, "swarm", network.Scope) + assert.Equal(c, resp.ID, network.ID) - var r types.NetworkResource - - status, body, err := d.SockRequest("GET", "/networks/"+name, nil) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - c.Assert(status, checker.Equals, http.StatusOK, check.Commentf(string(out))) - c.Assert(json.Unmarshal(body, &r), checker.IsNil) - c.Assert(r.Scope, checker.Equals, "swarm") - c.Assert(r.ID, checker.Equals, n.ID) - - v := url.Values{} - v.Set("scope", "local") - - status, _, err = d.SockRequest("GET", "/networks/"+name+"?"+v.Encode(), nil) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - c.Assert(status, checker.Equals, http.StatusNotFound, check.Commentf(string(out))) + _, err = apiclient.NetworkInspect(ctx, name, types.NetworkInspectOptions{Scope: "local"}) + assert.True(c, client.IsErrNotFound(err)) } diff --git a/components/engine/integration-cli/docker_cli_plugins_test.go b/components/engine/integration-cli/docker_cli_plugins_test.go index e49c428490..8ca7254440 100644 --- a/components/engine/integration-cli/docker_cli_plugins_test.go +++ b/components/engine/integration-cli/docker_cli_plugins_test.go @@ -15,7 +15,6 @@ import ( "github.com/docker/docker/integration-cli/cli" "github.com/docker/docker/integration-cli/daemon" "github.com/docker/docker/integration-cli/fixtures/plugin" - "github.com/docker/docker/integration-cli/request" "github.com/go-check/check" "github.com/gotestyourself/gotestyourself/icmd" "golang.org/x/net/context" @@ -159,9 +158,7 @@ func (s *DockerSuite) TestPluginInstallDisableVolumeLs(c *check.C) { } func (ps *DockerPluginSuite) TestPluginSet(c *check.C) { - // Create a new plugin with extra settings - client, err := request.NewClient() - c.Assert(err, checker.IsNil, check.Commentf("failed to create test client")) + client := testEnv.APIClient() name := "test" ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) @@ -171,7 +168,8 @@ func (ps *DockerPluginSuite) TestPluginSet(c *check.C) { mntSrc := "foo" devPath := "/dev/bar" - err = plugin.Create(ctx, client, name, func(cfg *plugin.Config) { + // Create a new plugin with extra settings + err := plugin.Create(ctx, client, name, func(cfg *plugin.Config) { cfg.Env = []types.PluginEnv{{Name: "DEBUG", Value: &initialValue, Settable: []string{"value"}}} cfg.Mounts = []types.PluginMount{ {Name: "pmount1", Settable: []string{"source"}, Type: "none", Source: &mntSrc}, @@ -401,12 +399,11 @@ func (s *DockerTrustSuite) TestPluginUntrustedInstall(c *check.C) { func (ps *DockerPluginSuite) TestPluginIDPrefix(c *check.C) { name := "test" - client, err := request.NewClient() - c.Assert(err, checker.IsNil, check.Commentf("error creating test client")) + client := testEnv.APIClient() ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) initialValue := "0" - err = plugin.Create(ctx, client, name, func(cfg *plugin.Config) { + err := plugin.Create(ctx, client, name, func(cfg *plugin.Config) { cfg.Env = []types.PluginEnv{{Name: "DEBUG", Value: &initialValue, Settable: []string{"value"}}} }) cancel() @@ -466,8 +463,7 @@ func (ps *DockerPluginSuite) TestPluginListDefaultFormat(c *check.C) { c.Assert(err, check.IsNil) name := "test:latest" - client, err := request.NewClient() - c.Assert(err, checker.IsNil, check.Commentf("error creating test client")) + client := testEnv.APIClient() ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() diff --git a/components/engine/integration-cli/request/request.go b/components/engine/integration-cli/request/request.go index e4a1e3e892..4e61e09359 100644 --- a/components/engine/integration-cli/request/request.go +++ b/components/engine/integration-cli/request/request.go @@ -1,16 +1,13 @@ package request // import "github.com/docker/docker/integration-cli/request" import ( - "bufio" "bytes" "crypto/tls" "encoding/json" - "fmt" "io" "io/ioutil" "net" "net/http" - "net/http/httputil" "net/url" "os" "path/filepath" @@ -95,11 +92,11 @@ func Do(endpoint string, modifiers ...func(*http.Request) error) (*http.Response // DoOnHost creates and execute a request on the specified host and endpoint, with the specified request modifiers func DoOnHost(host, endpoint string, modifiers ...func(*http.Request) error) (*http.Response, io.ReadCloser, error) { - req, err := New(host, endpoint, modifiers...) + req, err := newRequest(host, endpoint, modifiers...) if err != nil { return nil, nil, err } - client, err := NewHTTPClient(host) + client, err := newHTTPClient(host) if err != nil { return nil, nil, err } @@ -114,18 +111,15 @@ func DoOnHost(host, endpoint string, modifiers ...func(*http.Request) error) (*h return resp, body, err } -// New creates a new http Request to the specified host and endpoint, with the specified request modifiers -func New(host, endpoint string, modifiers ...func(*http.Request) error) (*http.Request, error) { - _, addr, _, err := dclient.ParseHost(host) +// newRequest creates a new http Request to the specified host and endpoint, with the specified request modifiers +func newRequest(host, endpoint string, modifiers ...func(*http.Request) error) (*http.Request, error) { + hostUrl, err := dclient.ParseHostURL(host) if err != nil { - return nil, err - } - if err != nil { - return nil, errors.Wrapf(err, "could not parse url %q", host) + return nil, errors.Wrapf(err, "failed parsing url %q", host) } req, err := http.NewRequest("GET", endpoint, nil) if err != nil { - return nil, fmt.Errorf("could not create new request: %v", err) + return nil, errors.Wrap(err, "failed to create request") } if os.Getenv("DOCKER_TLS_VERIFY") != "" { @@ -133,7 +127,7 @@ func New(host, endpoint string, modifiers ...func(*http.Request) error) (*http.R } else { req.URL.Scheme = "http" } - req.URL.Host = addr + req.URL.Host = hostUrl.Host for _, config := range modifiers { if err := config(req); err != nil { @@ -143,15 +137,16 @@ func New(host, endpoint string, modifiers ...func(*http.Request) error) (*http.R return req, nil } -// NewHTTPClient creates an http client for the specific host -func NewHTTPClient(host string) (*http.Client, error) { +// newHTTPClient creates an http client for the specific host +// TODO: Share more code with client.defaultHTTPClient +func newHTTPClient(host string) (*http.Client, error) { // FIXME(vdemeester) 10*time.Second timeout of SockRequest… ? - proto, addr, _, err := dclient.ParseHost(host) + hostUrl, err := dclient.ParseHostURL(host) if err != nil { return nil, err } transport := new(http.Transport) - if proto == "tcp" && os.Getenv("DOCKER_TLS_VERIFY") != "" { + if hostUrl.Scheme == "tcp" && os.Getenv("DOCKER_TLS_VERIFY") != "" { // Setup the socket TLS configuration. tlsConfig, err := getTLSConfig() if err != nil { @@ -160,102 +155,22 @@ func NewHTTPClient(host string) (*http.Client, error) { transport = &http.Transport{TLSClientConfig: tlsConfig} } transport.DisableKeepAlives = true - err = sockets.ConfigureTransport(transport, proto, addr) - return &http.Client{ - Transport: transport, - }, err + err = sockets.ConfigureTransport(transport, hostUrl.Scheme, hostUrl.Host) + return &http.Client{Transport: transport}, err } // NewClient returns a new Docker API client +// Deprecated: Use Execution.APIClient() func NewClient() (dclient.APIClient, error) { return dclient.NewClientWithOpts(dclient.WithHost(DaemonHost())) } -// FIXME(vdemeester) httputil.ClientConn is deprecated, use http.Client instead (closer to actual client) -// Deprecated: Use New instead of NewRequestClient -// Deprecated: use request.Do (or Get, Delete, Post) instead -func newRequestClient(method, endpoint string, data io.Reader, ct, daemon string, modifiers ...func(*http.Request)) (*http.Request, *httputil.ClientConn, error) { - c, err := SockConn(time.Duration(10*time.Second), daemon) - if err != nil { - return nil, nil, fmt.Errorf("could not dial docker daemon: %v", err) - } - - client := httputil.NewClientConn(c, nil) - - req, err := http.NewRequest(method, endpoint, data) - if err != nil { - client.Close() - return nil, nil, fmt.Errorf("could not create new request: %v", err) - } - - for _, opt := range modifiers { - opt(req) - } - - if ct != "" { - req.Header.Set("Content-Type", ct) - } - return req, client, nil -} - -// SockRequest create a request against the specified host (with method, endpoint and other request modifier) and -// returns the status code, and the content as an byte slice -// Deprecated: use request.Do instead -func SockRequest(method, endpoint string, data interface{}, daemon string, modifiers ...func(*http.Request)) (int, []byte, error) { - jsonData := bytes.NewBuffer(nil) - if err := json.NewEncoder(jsonData).Encode(data); err != nil { - return -1, nil, err - } - - res, body, err := SockRequestRaw(method, endpoint, jsonData, "application/json", daemon, modifiers...) - if err != nil { - return -1, nil, err - } - b, err := ReadBody(body) - return res.StatusCode, b, err -} - // ReadBody read the specified ReadCloser content and returns it func ReadBody(b io.ReadCloser) ([]byte, error) { defer b.Close() return ioutil.ReadAll(b) } -// SockRequestRaw create a request against the specified host (with method, endpoint and other request modifier) and -// returns the http response, the output as a io.ReadCloser -// Deprecated: use request.Do (or Get, Delete, Post) instead -func SockRequestRaw(method, endpoint string, data io.Reader, ct, daemon string, modifiers ...func(*http.Request)) (*http.Response, io.ReadCloser, error) { - req, client, err := newRequestClient(method, endpoint, data, ct, daemon, modifiers...) - if err != nil { - return nil, nil, err - } - - resp, err := client.Do(req) - if err != nil { - client.Close() - return resp, nil, err - } - body := ioutils.NewReadCloserWrapper(resp.Body, func() error { - defer resp.Body.Close() - return client.Close() - }) - - return resp, body, err -} - -// SockRequestHijack creates a connection to specified host (with method, contenttype, …) and returns a hijacked connection -// and the output as a `bufio.Reader` -func SockRequestHijack(method, endpoint string, data io.Reader, ct string, daemon string, modifiers ...func(*http.Request)) (net.Conn, *bufio.Reader, error) { - req, client, err := newRequestClient(method, endpoint, data, ct, daemon, modifiers...) - if err != nil { - return nil, nil, err - } - - client.Do(req) - conn, br := client.Hijack() - return conn, br, nil -} - // SockConn opens a connection on the specified socket func SockConn(timeout time.Duration, daemon string) (net.Conn, error) { daemonURL, err := url.Parse(daemon) diff --git a/components/engine/integration-cli/trust_server_test.go b/components/engine/integration-cli/trust_server_test.go index 19abe87196..f312083ee3 100644 --- a/components/engine/integration-cli/trust_server_test.go +++ b/components/engine/integration-cli/trust_server_test.go @@ -17,7 +17,6 @@ import ( "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/cli" "github.com/docker/docker/integration-cli/fixtures/plugin" - "github.com/docker/docker/integration-cli/request" "github.com/docker/go-connections/tlsconfig" "github.com/go-check/check" "github.com/gotestyourself/gotestyourself/icmd" @@ -230,11 +229,10 @@ func (s *DockerTrustSuite) setupTrustedImage(c *check.C, name string) string { func (s *DockerTrustSuite) setupTrustedplugin(c *check.C, source, name string) string { repoName := fmt.Sprintf("%v/dockercli/%s:latest", privateRegistryURL, name) - client, err := request.NewClient() - c.Assert(err, checker.IsNil, check.Commentf("could not create test client")) + client := testEnv.APIClient() ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) - err = plugin.Create(ctx, client, repoName) + err := plugin.Create(ctx, client, repoName) cancel() c.Assert(err, checker.IsNil, check.Commentf("could not create test plugin")) From 10bf273bff640955a23c9e8f33ddcb94cc3805ff Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 16 Feb 2018 17:29:19 -0500 Subject: [PATCH 4/4] Remove explicit DOCKER_API_VERSION from integration env setup Use the default version because it is used by the client package Signed-off-by: Daniel Nephin Upstream-commit: e73d742cd7deee396eac3c97664b40264ee358cb Component: engine --- components/engine/hack/make/.integration-daemon-start | 4 ---- 1 file changed, 4 deletions(-) diff --git a/components/engine/hack/make/.integration-daemon-start b/components/engine/hack/make/.integration-daemon-start index 7314f71c5c..20801fccee 100644 --- a/components/engine/hack/make/.integration-daemon-start +++ b/components/engine/hack/make/.integration-daemon-start @@ -7,10 +7,6 @@ export PATH="$base/binary-daemon:$base/dynbinary-daemon:$PATH" export TEST_CLIENT_BINARY=docker -# Do not bump this version! Integration tests should no longer rely on the docker cli, they should be -# API tests instead. For the existing tests the scripts will use a frozen version of the docker cli -# with a DOCKER_API_VERSION frozen to 1.30, which should ensure that the CI remains green at all times. -export DOCKER_API_VERSION=1.30 if [ -n "$DOCKER_CLI_PATH" ]; then export TEST_CLIENT_BINARY=/usr/local/cli/$(basename "$DOCKER_CLI_PATH") fi