Browse code

Make it explicit raw|multiplexed stream implementation being used

fix #35761

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

Nicolas De Loof authored on 2019/08/28 20:46:32
Showing 14 changed files
... ...
@@ -153,6 +153,12 @@ func (s *containerRouter) getContainersLogs(ctx context.Context, w http.Response
153 153
 		return err
154 154
 	}
155 155
 
156
+	contentType := types.MediaTypeRawStream
157
+	if !tty && versions.GreaterThanOrEqualTo(httputils.VersionFromContext(ctx), "1.42") {
158
+		contentType = types.MediaTypeMultiplexedStream
159
+	}
160
+	w.Header().Set("Content-Type", contentType)
161
+
156 162
 	// if has a tty, we're not muxing streams. if it doesn't, we are. simple.
157 163
 	// this is the point of no return for writing a response. once we call
158 164
 	// WriteLogStream, the response has been started and errors will be
... ...
@@ -598,7 +604,8 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo
598 598
 		return errdefs.InvalidParameter(errors.Errorf("error attaching to container %s, hijack connection missing", containerName))
599 599
 	}
600 600
 
601
-	setupStreams := func() (io.ReadCloser, io.Writer, io.Writer, error) {
601
+	contentType := types.MediaTypeRawStream
602
+	setupStreams := func(multiplexed bool) (io.ReadCloser, io.Writer, io.Writer, error) {
602 603
 		conn, _, err := hijacker.Hijack()
603 604
 		if err != nil {
604 605
 			return nil, nil, nil, err
... ...
@@ -608,7 +615,10 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo
608 608
 		conn.Write([]byte{})
609 609
 
610 610
 		if upgrade {
611
-			fmt.Fprintf(conn, "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n")
611
+			if multiplexed && versions.GreaterThanOrEqualTo(httputils.VersionFromContext(ctx), "1.42") {
612
+				contentType = types.MediaTypeMultiplexedStream
613
+			}
614
+			fmt.Fprintf(conn, "HTTP/1.1 101 UPGRADED\r\nContent-Type: "+contentType+"\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n")
612 615
 		} else {
613 616
 			fmt.Fprintf(conn, "HTTP/1.1 200 OK\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n")
614 617
 		}
... ...
@@ -632,16 +642,16 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo
632 632
 	}
633 633
 
634 634
 	if err = s.backend.ContainerAttach(containerName, attachConfig); err != nil {
635
-		logrus.Errorf("Handler for %s %s returned error: %v", r.Method, r.URL.Path, err)
635
+		logrus.WithError(err).Errorf("Handler for %s %s returned error", r.Method, r.URL.Path)
636 636
 		// Remember to close stream if error happens
637 637
 		conn, _, errHijack := hijacker.Hijack()
638
-		if errHijack == nil {
638
+		if errHijack != nil {
639
+			logrus.WithError(err).Errorf("Handler for %s %s: unable to close stream; error when hijacking connection", r.Method, r.URL.Path)
640
+		} else {
639 641
 			statusCode := httpstatus.FromError(err)
640 642
 			statusText := http.StatusText(statusCode)
641
-			fmt.Fprintf(conn, "HTTP/1.1 %d %s\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n%s\r\n", statusCode, statusText, err.Error())
643
+			fmt.Fprintf(conn, "HTTP/1.1 %d %s\r\nContent-Type: %s\r\n\r\n%s\r\n", statusCode, statusText, contentType, err.Error())
642 644
 			httputils.CloseStreams(conn)
643
-		} else {
644
-			logrus.Errorf("Error Hijacking: %v", err)
645 645
 		}
646 646
 	}
647 647
 	return nil
... ...
@@ -661,7 +671,7 @@ func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.Respons
661 661
 
662 662
 	version := httputils.VersionFromContext(ctx)
663 663
 
664
-	setupStreams := func() (io.ReadCloser, io.Writer, io.Writer, error) {
664
+	setupStreams := func(multiplexed bool) (io.ReadCloser, io.Writer, io.Writer, error) {
665 665
 		wsChan := make(chan *websocket.Conn)
666 666
 		h := func(conn *websocket.Conn) {
667 667
 			wsChan <- conn
... ...
@@ -98,7 +98,11 @@ func (s *containerRouter) postContainerExecStart(ctx context.Context, w http.Res
98 98
 		defer httputils.CloseStreams(inStream, outStream)
99 99
 
100 100
 		if _, ok := r.Header["Upgrade"]; ok {
101
-			fmt.Fprint(outStream, "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n")
101
+			contentType := types.MediaTypeRawStream
102
+			if !execStartCheck.Tty && versions.GreaterThanOrEqualTo(httputils.VersionFromContext(ctx), "1.42") {
103
+				contentType = types.MediaTypeMultiplexedStream
104
+			}
105
+			fmt.Fprint(outStream, "HTTP/1.1 101 UPGRADED\r\nContent-Type: "+contentType+"\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n")
102 106
 		} else {
103 107
 			fmt.Fprint(outStream, "HTTP/1.1 200 OK\r\nContent-Type: application/vnd.docker.raw-stream\r\n")
104 108
 		}
... ...
@@ -3,7 +3,6 @@ package swarm // import "github.com/docker/docker/api/server/router/swarm"
3 3
 import (
4 4
 	"context"
5 5
 	"fmt"
6
-	"io"
7 6
 	"net/http"
8 7
 
9 8
 	"github.com/docker/docker/api/server/httputils"
... ...
@@ -15,7 +14,7 @@ import (
15 15
 
16 16
 // swarmLogs takes an http response, request, and selector, and writes the logs
17 17
 // specified by the selector to the response
18
-func (sr *swarmRouter) swarmLogs(ctx context.Context, w io.Writer, r *http.Request, selector *backend.LogSelector) error {
18
+func (sr *swarmRouter) swarmLogs(ctx context.Context, w http.ResponseWriter, r *http.Request, selector *backend.LogSelector) error {
19 19
 	// Args are validated before the stream starts because when it starts we're
20 20
 	// sending HTTP 200 by writing an empty chunk of data to tell the client that
21 21
 	// daemon is going to stream. By sending this initial HTTP 200 we can't report
... ...
@@ -63,6 +62,11 @@ func (sr *swarmRouter) swarmLogs(ctx context.Context, w io.Writer, r *http.Reque
63 63
 		return err
64 64
 	}
65 65
 
66
+	contentType := basictypes.MediaTypeRawStream
67
+	if !tty && versions.GreaterThanOrEqualTo(httputils.VersionFromContext(ctx), "1.42") {
68
+		contentType = basictypes.MediaTypeMultiplexedStream
69
+	}
70
+	w.Header().Set("Content-Type", contentType)
66 71
 	httputils.WriteLogStream(ctx, w, msgs, logsConfig, !tty)
67 72
 	return nil
68 73
 }
... ...
@@ -6478,6 +6478,9 @@ paths:
6478 6478
 
6479 6479
         Note: This endpoint works only for containers with the `json-file` or
6480 6480
         `journald` logging driver.
6481
+      produces:
6482
+        - "application/vnd.docker.raw-stream"
6483
+        - "application/vnd.docker.multiplexed-stream"
6481 6484
       operationId: "ContainerLogs"
6482 6485
       responses:
6483 6486
         200:
... ...
@@ -7189,7 +7192,8 @@ paths:
7189 7189
         ### Stream format
7190 7190
 
7191 7191
         When the TTY setting is disabled in [`POST /containers/create`](#operation/ContainerCreate),
7192
-        the stream over the hijacked connected is multiplexed to separate out
7192
+        the HTTP Content-Type header is set to application/vnd.docker.multiplexed-stream
7193
+        and the stream over the hijacked connected is multiplexed to separate out
7193 7194
         `stdout` and `stderr`. The stream consists of a series of frames, each
7194 7195
         containing a header and a payload.
7195 7196
 
... ...
@@ -7233,6 +7237,7 @@ paths:
7233 7233
       operationId: "ContainerAttach"
7234 7234
       produces:
7235 7235
         - "application/vnd.docker.raw-stream"
7236
+        - "application/vnd.docker.multiplexed-stream"
7236 7237
       responses:
7237 7238
         101:
7238 7239
           description: "no error, hints proxy about hijacking"
... ...
@@ -9015,6 +9020,7 @@ paths:
9015 9015
         - "application/json"
9016 9016
       produces:
9017 9017
         - "application/vnd.docker.raw-stream"
9018
+        - "application/vnd.docker.multiplexed-stream"
9018 9019
       responses:
9019 9020
         200:
9020 9021
           description: "No error"
... ...
@@ -10913,6 +10919,9 @@ paths:
10913 10913
 
10914 10914
         **Note**: This endpoint works only for services with the `local`,
10915 10915
         `json-file` or `journald` logging drivers.
10916
+      produces:
10917
+        - "application/vnd.docker.raw-stream"
10918
+        - "application/vnd.docker.multiplexed-stream"
10916 10919
       operationId: "ServiceLogs"
10917 10920
       responses:
10918 10921
         200:
... ...
@@ -11168,6 +11177,9 @@ paths:
11168 11168
         **Note**: This endpoint works only for services with the `local`,
11169 11169
         `json-file` or `journald` logging drivers.
11170 11170
       operationId: "TaskLogs"
11171
+      produces:
11172
+        - "application/vnd.docker.raw-stream"
11173
+        - "application/vnd.docker.multiplexed-stream"
11171 11174
       responses:
11172 11175
         200:
11173 11176
           description: "logs returned as a stream in response body"
... ...
@@ -10,7 +10,7 @@ import (
10 10
 
11 11
 // ContainerAttachConfig holds the streams to use when connecting to a container to view logs.
12 12
 type ContainerAttachConfig struct {
13
-	GetStreams func() (io.ReadCloser, io.Writer, io.Writer, error)
13
+	GetStreams func(multiplexed bool) (io.ReadCloser, io.Writer, io.Writer, error)
14 14
 	UseStdin   bool
15 15
 	UseStdout  bool
16 16
 	UseStderr  bool
... ...
@@ -112,10 +112,16 @@ type NetworkListOptions struct {
112 112
 	Filters filters.Args
113 113
 }
114 114
 
115
+// NewHijackedResponse intializes a HijackedResponse type
116
+func NewHijackedResponse(conn net.Conn, mediaType string) HijackedResponse {
117
+	return HijackedResponse{Conn: conn, Reader: bufio.NewReader(conn), mediaType: mediaType}
118
+}
119
+
115 120
 // HijackedResponse holds connection information for a hijacked request.
116 121
 type HijackedResponse struct {
117
-	Conn   net.Conn
118
-	Reader *bufio.Reader
122
+	mediaType string
123
+	Conn      net.Conn
124
+	Reader    *bufio.Reader
119 125
 }
120 126
 
121 127
 // Close closes the hijacked connection and reader.
... ...
@@ -123,6 +129,15 @@ func (h *HijackedResponse) Close() {
123 123
 	h.Conn.Close()
124 124
 }
125 125
 
126
+// MediaType let client know if HijackedResponse hold a raw or multiplexed stream.
127
+// returns false if HTTP Content-Type is not relevant, and container must be inspected
128
+func (h *HijackedResponse) MediaType() (string, bool) {
129
+	if h.mediaType == "" {
130
+		return "", false
131
+	}
132
+	return h.mediaType, true
133
+}
134
+
126 135
 // CloseWriter is an interface that implements structs
127 136
 // that close input streams to prevent from writing.
128 137
 type CloseWriter interface {
... ...
@@ -18,6 +18,14 @@ import (
18 18
 	"github.com/docker/go-connections/nat"
19 19
 )
20 20
 
21
+const (
22
+	// MediaTypeRawStream is vendor specific MIME-Type set for raw TTY streams
23
+	MediaTypeRawStream = "application/vnd.docker.raw-stream"
24
+
25
+	// MediaTypeMultiplexedStream is vendor specific MIME-Type set for stdin/stdout/stderr multiplexed streams
26
+	MediaTypeMultiplexedStream = "application/vnd.docker.multiplexed-stream"
27
+)
28
+
21 29
 // RootFS returns Image's RootFS description including the layer IDs.
22 30
 type RootFS struct {
23 31
 	Type   string   `json:",omitempty"`
... ...
@@ -52,6 +52,8 @@ func (cli *Client) ContainerAttach(ctx context.Context, container string, option
52 52
 		query.Set("logs", "1")
53 53
 	}
54 54
 
55
-	headers := map[string][]string{"Content-Type": {"text/plain"}}
55
+	headers := map[string][]string{
56
+		"Content-Type": {"text/plain"},
57
+	}
56 58
 	return cli.postHijacked(ctx, "/containers/"+container+"/attach", query, nil, headers)
57 59
 }
... ...
@@ -36,7 +36,9 @@ func (cli *Client) ContainerExecStart(ctx context.Context, execID string, config
36 36
 // and the a reader to get output. It's up to the called to close
37 37
 // the hijacked connection by calling types.HijackedResponse.Close.
38 38
 func (cli *Client) ContainerExecAttach(ctx context.Context, execID string, config types.ExecStartCheck) (types.HijackedResponse, error) {
39
-	headers := map[string][]string{"Content-Type": {"application/json"}}
39
+	headers := map[string][]string{
40
+		"Content-Type": {"application/json"},
41
+	}
40 42
 	return cli.postHijacked(ctx, "/exec/"+execID+"/start", nil, config, headers)
41 43
 }
42 44
 
... ...
@@ -12,6 +12,7 @@ import (
12 12
 	"time"
13 13
 
14 14
 	"github.com/docker/docker/api/types"
15
+	"github.com/docker/docker/api/types/versions"
15 16
 	"github.com/docker/go-connections/sockets"
16 17
 	"github.com/pkg/errors"
17 18
 )
... ...
@@ -30,12 +31,12 @@ func (cli *Client) postHijacked(ctx context.Context, path string, query url.Valu
30 30
 	}
31 31
 	req = cli.addHeaders(req, headers)
32 32
 
33
-	conn, err := cli.setupHijackConn(ctx, req, "tcp")
33
+	conn, mediaType, err := cli.setupHijackConn(ctx, req, "tcp")
34 34
 	if err != nil {
35 35
 		return types.HijackedResponse{}, err
36 36
 	}
37 37
 
38
-	return types.HijackedResponse{Conn: conn, Reader: bufio.NewReader(conn)}, err
38
+	return types.NewHijackedResponse(conn, mediaType), err
39 39
 }
40 40
 
41 41
 // DialHijack returns a hijacked connection with negotiated protocol proto.
... ...
@@ -46,7 +47,8 @@ func (cli *Client) DialHijack(ctx context.Context, url, proto string, meta map[s
46 46
 	}
47 47
 	req = cli.addHeaders(req, meta)
48 48
 
49
-	return cli.setupHijackConn(ctx, req, proto)
49
+	conn, _, err := cli.setupHijackConn(ctx, req, proto)
50
+	return conn, err
50 51
 }
51 52
 
52 53
 // fallbackDial is used when WithDialer() was not called.
... ...
@@ -61,7 +63,7 @@ func fallbackDial(proto, addr string, tlsConfig *tls.Config) (net.Conn, error) {
61 61
 	return net.Dial(proto, addr)
62 62
 }
63 63
 
64
-func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto string) (net.Conn, error) {
64
+func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto string) (net.Conn, string, error) {
65 65
 	req.Host = cli.addr
66 66
 	req.Header.Set("Connection", "Upgrade")
67 67
 	req.Header.Set("Upgrade", proto)
... ...
@@ -69,7 +71,7 @@ func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto
69 69
 	dialer := cli.Dialer()
70 70
 	conn, err := dialer(ctx)
71 71
 	if err != nil {
72
-		return nil, errors.Wrap(err, "cannot connect to the Docker daemon. Is 'docker daemon' running on this host?")
72
+		return nil, "", errors.Wrap(err, "cannot connect to the Docker daemon. Is 'docker daemon' running on this host?")
73 73
 	}
74 74
 
75 75
 	// When we set up a TCP connection for hijack, there could be long periods
... ...
@@ -91,18 +93,18 @@ func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto
91 91
 	//nolint:staticcheck // ignore SA1019 for connecting to old (pre go1.8) daemons
92 92
 	if err != httputil.ErrPersistEOF {
93 93
 		if err != nil {
94
-			return nil, err
94
+			return nil, "", err
95 95
 		}
96 96
 		if resp.StatusCode != http.StatusSwitchingProtocols {
97 97
 			resp.Body.Close()
98
-			return nil, fmt.Errorf("unable to upgrade to %s, received %d", proto, resp.StatusCode)
98
+			return nil, "", fmt.Errorf("unable to upgrade to %s, received %d", proto, resp.StatusCode)
99 99
 		}
100 100
 	}
101 101
 
102 102
 	c, br := clientconn.Hijack()
103 103
 	if br.Buffered() > 0 {
104 104
 		// If there is buffered content, wrap the connection.  We return an
105
-		// object that implements CloseWrite iff the underlying connection
105
+		// object that implements CloseWrite if the underlying connection
106 106
 		// implements it.
107 107
 		if _, ok := c.(types.CloseWriter); ok {
108 108
 			c = &hijackedConnCloseWriter{&hijackedConn{c, br}}
... ...
@@ -113,7 +115,13 @@ func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto
113 113
 		br.Reset(nil)
114 114
 	}
115 115
 
116
-	return c, nil
116
+	var mediaType string
117
+	if versions.GreaterThanOrEqualTo(cli.ClientVersion(), "1.42") {
118
+		// Prior to 1.42, Content-Type is always set to raw-stream and not relevant
119
+		mediaType = resp.Header.Get("Content-Type")
120
+	}
121
+
122
+	return c, mediaType, nil
117 123
 }
118 124
 
119 125
 // hijackedConn wraps a net.Conn and is returned by setupHijackConn in the case
... ...
@@ -50,13 +50,14 @@ func (daemon *Daemon) ContainerAttach(prefixOrName string, c *backend.ContainerA
50 50
 	}
51 51
 	ctr.StreamConfig.AttachStreams(&cfg)
52 52
 
53
-	inStream, outStream, errStream, err := c.GetStreams()
53
+	multiplexed := !ctr.Config.Tty && c.MuxStreams
54
+	inStream, outStream, errStream, err := c.GetStreams(multiplexed)
54 55
 	if err != nil {
55 56
 		return err
56 57
 	}
57 58
 	defer inStream.Close()
58 59
 
59
-	if !ctr.Config.Tty && c.MuxStreams {
60
+	if multiplexed {
60 61
 		errStream = stdcopy.NewStdWriter(errStream, stdcopy.Stderr)
61 62
 		outStream = stdcopy.NewStdWriter(outStream, stdcopy.Stdout)
62 63
 	}
... ...
@@ -73,6 +73,10 @@ keywords: "API, Docker, rcli, REST, documentation"
73 73
   syntax, `<IDType>://<ID>` is now recognised. Support for specific `<IDType>` values
74 74
   depends on the underlying implementation and Windows version. This change is not
75 75
   versioned, and affects all API versions if the daemon has this patch.
76
+* `GET /containers/{id}/attach`, `GET /exec/{id}/start`, `GET /containers/{id}/logs`
77
+  `GET /services/{id}/logs` and `GET /tasks/{id}/logs` now set Content-Type header
78
+  to `application/vnd.docker.multiplexed-stream` when a multiplexed stdout/stderr 
79
+  stream is sent to client, `application/vnd.docker.raw-stream` otherwise.
76 80
 
77 81
 ## v1.41 API changes
78 82
 
... ...
@@ -193,6 +193,9 @@ func (s *DockerSuite) TestPostContainersAttach(c *testing.T) {
193 193
 
194 194
 	resp, err := client.ContainerAttach(context.Background(), cid, attachOpts)
195 195
 	assert.NilError(c, err)
196
+	mediaType, b := resp.MediaType()
197
+	assert.Check(c, b)
198
+	assert.Equal(c, mediaType, types.MediaTypeMultiplexedStream)
196 199
 	expectSuccess(resp.Conn, resp.Reader, "stdout", false)
197 200
 
198 201
 	// Make sure we do see "hello" if Logs is true
199 202
new file mode 100644
... ...
@@ -0,0 +1,50 @@
0
+package container // import "github.com/docker/docker/integration/container"
1
+
2
+import (
3
+	"context"
4
+	"testing"
5
+
6
+	"github.com/docker/docker/api/types"
7
+	"github.com/docker/docker/api/types/container"
8
+	"github.com/docker/docker/api/types/network"
9
+	"gotest.tools/v3/assert"
10
+)
11
+
12
+func TestAttachWithTTY(t *testing.T) {
13
+	testAttach(t, true, types.MediaTypeRawStream)
14
+}
15
+
16
+func TestAttachWithoutTTy(t *testing.T) {
17
+	testAttach(t, false, types.MediaTypeMultiplexedStream)
18
+}
19
+
20
+func testAttach(t *testing.T, tty bool, expected string) {
21
+	defer setupTest(t)()
22
+	client := testEnv.APIClient()
23
+
24
+	resp, err := client.ContainerCreate(context.Background(),
25
+		&container.Config{
26
+			Image: "busybox",
27
+			Cmd:   []string{"echo", "hello"},
28
+			Tty:   tty,
29
+		},
30
+		&container.HostConfig{},
31
+		&network.NetworkingConfig{},
32
+		nil,
33
+		"",
34
+	)
35
+	assert.NilError(t, err)
36
+	container := resp.ID
37
+	defer client.ContainerRemove(context.Background(), container, types.ContainerRemoveOptions{
38
+		Force: true,
39
+	})
40
+
41
+	attach, err := client.ContainerAttach(context.Background(), container, types.ContainerAttachOptions{
42
+		Stdout: true,
43
+		Stderr: true,
44
+	})
45
+	assert.NilError(t, err)
46
+	mediaType, ok := attach.MediaType()
47
+	assert.Check(t, ok)
48
+	assert.Check(t, mediaType == expected)
49
+}