summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHuanHuan Ye <logindaveye@gmail.com>2019-10-11 17:54:35 +0800
committerHuanHuan Ye <logindaveye@gmail.com>2019-10-22 10:55:29 +0800
commit203ba72fc549b5ce306e78030579300f6e5ffe10 (patch)
treeb3f2722e0707f704e7e3cfc1400213bc088dabd2
parent248c136f986db89f7f283d9da4a5b08e0b13b905 (diff)
downloaddocker-203ba72fc549b5ce306e78030579300f6e5ffe10.tar.gz
integration-cli: Fix `SA1019: httputil.ClientConn is deprecated`
Rewrite sockRequestHijack to requestHijack which use writable Transport's Response.Body to replace deprecated hijacked httputil.ClientConn. ``` // As of Go 1.12, the Body will also implement io.Writer // on a successful "101 Switching Protocols" response, // as used by WebSockets and HTTP/2's "h2c" mode. Body io.ReadCloser ```. TestPostContainersAttach and TestExecResizeImmediatelyAfterExecStart replace all sockRequestHijack to requestHijack. Signed-off-by: HuanHuan Ye <logindaveye@gmail.com>
-rw-r--r--hack/validate/golangci-lint.yml4
-rw-r--r--integration-cli/docker_api_attach_test.go131
-rw-r--r--integration-cli/docker_api_exec_resize_test.go4
3 files changed, 83 insertions, 56 deletions
diff --git a/hack/validate/golangci-lint.yml b/hack/validate/golangci-lint.yml
index e547c09917..296839c670 100644
--- a/hack/validate/golangci-lint.yml
+++ b/hack/validate/golangci-lint.yml
@@ -86,10 +86,6 @@ issues:
- text: "SA1019: httputil.NewClientConn is deprecated"
linters:
- staticcheck
- # FIXME temporarily suppress these. See #39927
- - text: "SA1019: httputil.ClientConn is deprecated"
- linters:
- - staticcheck
# FIXME temporarily suppress these (related to the ones above)
- text: "SA1019: httputil.ErrPersistEOF is deprecated"
linters:
diff --git a/integration-cli/docker_api_attach_test.go b/integration-cli/docker_api_attach_test.go
index 5f0acfd6a9..634a06722b 100644
--- a/integration-cli/docker_api_attach_test.go
+++ b/integration-cli/docker_api_attach_test.go
@@ -4,11 +4,9 @@ import (
"bufio"
"bytes"
"context"
- "fmt"
"io"
"net"
"net/http"
- "net/http/httputil"
"strings"
"testing"
"time"
@@ -17,6 +15,7 @@ import (
"github.com/docker/docker/client"
"github.com/docker/docker/pkg/stdcopy"
"github.com/docker/docker/testutil/request"
+ "github.com/docker/go-connections/sockets"
"github.com/pkg/errors"
"golang.org/x/net/websocket"
"gotest.tools/assert"
@@ -100,19 +99,18 @@ func (s *DockerSuite) TestGetContainersWsAttachContainerNotFound(c *testing.T) {
func (s *DockerSuite) TestPostContainersAttach(c *testing.T) {
testRequires(c, DaemonIsLinux)
- expectSuccess := func(conn net.Conn, br *bufio.Reader, stream string, tty bool) {
- defer conn.Close()
+ expectSuccess := func(wc io.WriteCloser, br *bufio.Reader, stream string, tty bool) {
+ defer wc.Close()
expected := []byte("success")
- _, err := conn.Write(expected)
+ _, err := wc.Write(expected)
assert.NilError(c, err)
- conn.SetReadDeadline(time.Now().Add(time.Second))
lenHeader := 0
if !tty {
lenHeader = 8
}
actual := make([]byte, len(expected)+lenHeader)
- _, err = io.ReadFull(br, actual)
+ _, err = readTimeout(br, actual, time.Second)
assert.NilError(c, err)
if !tty {
fdMap := map[string]byte{
@@ -125,57 +123,56 @@ func (s *DockerSuite) TestPostContainersAttach(c *testing.T) {
assert.Assert(c, is.DeepEqual(actual[lenHeader:], expected), "Attach didn't return the expected data from %s", stream)
}
- expectTimeout := func(conn net.Conn, br *bufio.Reader, stream string) {
- defer conn.Close()
- _, err := conn.Write([]byte{'t'})
+ expectTimeout := func(wc io.WriteCloser, br *bufio.Reader, stream string) {
+ defer wc.Close()
+ _, err := wc.Write([]byte{'t'})
assert.NilError(c, err)
- conn.SetReadDeadline(time.Now().Add(time.Second))
actual := make([]byte, 1)
- _, err = io.ReadFull(br, actual)
- opErr, ok := err.(*net.OpError)
- assert.Assert(c, ok, "Error is expected to be *net.OpError, got %v", err)
- assert.Assert(c, opErr.Timeout(), "Read from %s is expected to timeout", stream)
+ _, err = readTimeout(br, actual, time.Second)
+ assert.Assert(c, err.Error() == "Timeout", "Read from %s is expected to timeout", stream)
}
// Create a container that only emits stdout.
cid, _ := dockerCmd(c, "run", "-di", "busybox", "cat")
cid = strings.TrimSpace(cid)
+
// Attach to the container's stdout stream.
- conn, br, err := sockRequestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", request.DaemonHost())
+ wc, br, err := requestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", request.DaemonHost())
assert.NilError(c, err)
// Check if the data from stdout can be received.
- expectSuccess(conn, br, "stdout", false)
+ expectSuccess(wc, br, "stdout", false)
+
// Attach to the container's stderr stream.
- conn, br, err = sockRequestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", request.DaemonHost())
+ wc, br, err = requestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", request.DaemonHost())
assert.NilError(c, err)
// Since the container only emits stdout, attaching to stderr should return nothing.
- expectTimeout(conn, br, "stdout")
+ expectTimeout(wc, br, "stdout")
// 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 = sockRequestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", request.DaemonHost())
+ wc, br, err = requestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", request.DaemonHost())
assert.NilError(c, err)
- expectSuccess(conn, br, "stderr", false)
- conn, br, err = sockRequestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", request.DaemonHost())
+ expectSuccess(wc, br, "stderr", false)
+ wc, br, err = requestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", request.DaemonHost())
assert.NilError(c, err)
- expectTimeout(conn, br, "stderr")
+ expectTimeout(wc, br, "stderr")
// Test with tty.
cid, _ = dockerCmd(c, "run", "-dit", "busybox", "/bin/sh", "-c", "cat >&2")
cid = strings.TrimSpace(cid)
// Attach to stdout only.
- conn, br, err = sockRequestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", request.DaemonHost())
+ wc, br, err = requestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", request.DaemonHost())
assert.NilError(c, err)
- expectSuccess(conn, br, "stdout", true)
+ expectSuccess(wc, br, "stdout", true)
// Attach without stdout stream.
- conn, br, err = sockRequestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", request.DaemonHost())
+ wc, br, err = requestHijack(http.MethodPost, "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", request.DaemonHost())
assert.NilError(c, err)
// 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.
- expectTimeout(conn, br, "stdout")
+ expectTimeout(wc, br, "stdout")
// Test the client API
client, err := client.NewClientWithOpts(client.FromEnv)
@@ -220,35 +217,22 @@ func (s *DockerSuite) TestPostContainersAttach(c *testing.T) {
assert.Equal(c, outBuf.String(), "hello\nsuccess")
}
-// 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
- }
+// requestHijack create a http requst to specified host with `Upgrade` header (with method
+// , contenttype, …), if receive a successful "101 Switching Protocols" response return
+// a `io.WriteCloser` and `bufio.Reader`
+func requestHijack(method, endpoint string, data io.Reader, ct, daemon string, modifiers ...func(*http.Request)) (io.WriteCloser, *bufio.Reader, error) {
- 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(10*time.Second, daemon)
+ hostURL, err := client.ParseHostURL(daemon)
if err != nil {
- return nil, nil, fmt.Errorf("could not dial docker daemon: %v", err)
+ return nil, nil, errors.Wrap(err, "parse daemon host error")
}
- 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)
+ return nil, nil, errors.Wrap(err, "could not create new request")
}
+ req.URL.Scheme = "http"
+ req.URL.Host = hostURL.Host
for _, opt := range modifiers {
opt(req)
@@ -257,5 +241,52 @@ func newRequestClient(method, endpoint string, data io.Reader, ct, daemon string
if ct != "" {
req.Header.Set("Content-Type", ct)
}
- return req, client, nil
+
+ // must have Upgrade header
+ // server api return 101 Switching Protocols
+ req.Header.Set("Upgrade", "tcp")
+
+ // new client
+ // FIXME use testutil/request newHTTPClient
+ transport := &http.Transport{}
+ err = sockets.ConfigureTransport(transport, hostURL.Scheme, hostURL.Host)
+ if err != nil {
+ return nil, nil, errors.Wrap(err, "configure Transport error")
+ }
+
+ client := http.Client{
+ Transport: transport,
+ }
+
+ resp, err := client.Do(req)
+ if err != nil {
+ return nil, nil, errors.Wrap(err, "client.Do")
+ }
+
+ if !bodyIsWritable(resp) {
+ return nil, nil, errors.New("response.Body not writable")
+ }
+
+ return resp.Body.(io.WriteCloser), bufio.NewReader(resp.Body), nil
+}
+
+// bodyIsWritable check Response.Body is writable
+func bodyIsWritable(r *http.Response) bool {
+ _, ok := r.Body.(io.Writer)
+ return ok
+}
+
+// readTimeout read from io.Reader with timeout
+func readTimeout(r io.Reader, buf []byte, timeout time.Duration) (n int, err error) {
+ ch := make(chan bool)
+ go func() {
+ n, err = io.ReadFull(r, buf)
+ ch <- true
+ }()
+ select {
+ case <-ch:
+ return
+ case <-time.After(timeout):
+ return 0, errors.New("Timeout")
+ }
}
diff --git a/integration-cli/docker_api_exec_resize_test.go b/integration-cli/docker_api_exec_resize_test.go
index 89de25a261..467dff9e23 100644
--- a/integration-cli/docker_api_exec_resize_test.go
+++ b/integration-cli/docker_api_exec_resize_test.go
@@ -65,11 +65,11 @@ func (s *DockerSuite) TestExecResizeImmediatelyAfterExecStart(c *testing.T) {
}
payload := bytes.NewBufferString(`{"Tty":true}`)
- conn, _, err := sockRequestHijack(http.MethodPost, fmt.Sprintf("/exec/%s/start", execID), payload, "application/json", request.DaemonHost())
+ wc, _, err := requestHijack(http.MethodPost, fmt.Sprintf("/exec/%s/start", execID), payload, "application/json", request.DaemonHost())
if err != nil {
return errors.Wrap(err, "failed to start the exec")
}
- defer conn.Close()
+ defer wc.Close()
_, rc, err := request.Post(fmt.Sprintf("/exec/%s/resize?h=24&w=80", execID), request.ContentType("text/plain"))
if err != nil {