Merge pull request #16451 from ncdc/exec-interop-testing

pkg/client/unversioned/remotecommand/remotecommand.go

@@ -26,6 +26,7 @@ import (
 
 	"k8s.io/kubernetes/pkg/client/restclient"
 	"k8s.io/kubernetes/pkg/client/transport"
+	"k8s.io/kubernetes/pkg/kubelet/server/remotecommand"
 	"k8s.io/kubernetes/pkg/util/httpstream"
 	"k8s.io/kubernetes/pkg/util/httpstream/spdy"
 )
@@ -36,7 +37,7 @@ type Executor interface {
 	// non-nil stream to a remote system, and return an error if a problem occurs. If tty
 	// is set, the stderr stream is not used (raw TTY manages stdout and stderr over the
 	// stdout stream).
-	Stream(stdin io.Reader, stdout, stderr io.Writer, tty bool) error
+	Stream(supportedProtocols []string, stdin io.Reader, stdout, stderr io.Writer, tty bool) error
 }
 
 // StreamExecutor supports the ability to dial an httpstream connection and the ability to
@@ -128,26 +129,13 @@ func (e *streamExecutor) Dial(protocols ...string) (httpstream.Connection, strin
 	return conn, resp.Header.Get(httpstream.HeaderProtocolVersion), nil
 }
 
-const (
-	// The SPDY subprotocol "channel.k8s.io" is used for remote command
-	// attachment/execution. This represents the initial unversioned subprotocol,
-	// which has the known bugs http://issues.k8s.io/13394 and
-	// http://issues.k8s.io/13395.
-	StreamProtocolV1Name = "channel.k8s.io"
-	// The SPDY subprotocol "v2.channel.k8s.io" is used for remote command
-	// attachment/execution. It is the second version of the subprotocol and
-	// resolves the issues present in the first version.
-	StreamProtocolV2Name = "v2.channel.k8s.io"
-)
-
 type streamProtocolHandler interface {
 	stream(httpstream.Connection) error
 }
 
 // Stream opens a protocol streamer to the server and streams until a client closes
 // the connection or the server disconnects.
-func (e *streamExecutor) Stream(stdin io.Reader, stdout, stderr io.Writer, tty bool) error {
-	supportedProtocols := []string{StreamProtocolV2Name, StreamProtocolV1Name}
+func (e *streamExecutor) Stream(supportedProtocols []string, stdin io.Reader, stdout, stderr io.Writer, tty bool) error {
 	conn, protocol, err := e.Dial(supportedProtocols...)
 	if err != nil {
 		return err
@@ -157,7 +145,7 @@ func (e *streamExecutor) Stream(stdin io.Reader, stdout, stderr io.Writer, tty b
 	var streamer streamProtocolHandler
 
 	switch protocol {
-	case StreamProtocolV2Name:
+	case remotecommand.StreamProtocolV2Name:
 		streamer = &streamProtocolV2{
 			stdin:  stdin,
 			stdout: stdout,
@@ -165,9 +153,9 @@ func (e *streamExecutor) Stream(stdin io.Reader, stdout, stderr io.Writer, tty b
 			tty:    tty,
 		}
 	case "":
-		glog.V(4).Infof("The server did not negotiate a streaming protocol version. Falling back to %s", StreamProtocolV1Name)
+		glog.V(4).Infof("The server did not negotiate a streaming protocol version. Falling back to %s", remotecommand.StreamProtocolV1Name)
 		fallthrough
-	case StreamProtocolV1Name:
+	case remotecommand.StreamProtocolV1Name:
 		streamer = &streamProtocolV1{
 			stdin:  stdin,
 			stdout: stdout,

pkg/kubectl/cmd/attach.go

@@ -29,6 +29,7 @@ import (
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/client/unversioned/remotecommand"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
+	remotecommandserver "k8s.io/kubernetes/pkg/kubelet/server/remotecommand"
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 	"k8s.io/kubernetes/pkg/util/interrupt"
 	"k8s.io/kubernetes/pkg/util/term"
@@ -87,7 +88,7 @@ func (*DefaultRemoteAttach) Attach(method string, url *url.URL, config *restclie
 	if err != nil {
 		return err
 	}
-	return exec.Stream(stdin, stdout, stderr, tty)
+	return exec.Stream(remotecommandserver.SupportedStreamingProtocols, stdin, stdout, stderr, tty)
 }
 
 // AttachOptions declare the arguments accepted by the Exec command

pkg/kubectl/cmd/exec.go

@@ -32,6 +32,7 @@ import (
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/client/unversioned/remotecommand"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
+	remotecommandserver "k8s.io/kubernetes/pkg/kubelet/server/remotecommand"
 )
 
 const (
@@ -87,7 +88,7 @@ func (*DefaultRemoteExecutor) Execute(method string, url *url.URL, config *restc
 	if err != nil {
 		return err
 	}
-	return exec.Stream(stdin, stdout, stderr, tty)
+	return exec.Stream(remotecommandserver.SupportedStreamingProtocols, stdin, stdout, stderr, tty)
 }
 
 // ExecOptions declare the arguments accepted by the Exec command

pkg/kubelet/server/remotecommand/attach.go

+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package remotecommand
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+	"time"
+
+	"k8s.io/kubernetes/pkg/types"
+	"k8s.io/kubernetes/pkg/util/runtime"
+)
+
+// Attacher knows how to attach to a running container in a pod.
+type Attacher interface {
+	// AttachContainer attaches to the running container in the pod, copying data between in/out/err
+	// and the container's stdin/stdout/stderr.
+	AttachContainer(name string, uid types.UID, container string, in io.Reader, out, err io.WriteCloser, tty bool) error
+}
+
+// ServeAttach handles requests to attach to a container. After creating/receiving the required
+// streams, it delegates the actual attaching to attacher.
+func ServeAttach(w http.ResponseWriter, req *http.Request, attacher Attacher, podName string, uid types.UID, container string, idleTimeout, streamCreationTimeout time.Duration, supportedProtocols []string) {
+	ctx, ok := createStreams(req, w, supportedProtocols, idleTimeout, streamCreationTimeout)
+	if !ok {
+		// error is handled by createStreams
+		return
+	}
+	defer ctx.conn.Close()
+
+	err := attacher.AttachContainer(podName, uid, container, ctx.stdinStream, ctx.stdoutStream, ctx.stderrStream, ctx.tty)
+	if err != nil {
+		msg := fmt.Sprintf("error attaching to container: %v", err)
+		runtime.HandleError(errors.New(msg))
+		fmt.Fprint(ctx.errorStream, msg)
+	}
+}

pkg/kubelet/server/remotecommand/contants.go

+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package remotecommand
+
+import "time"
+
+const (
+	DefaultStreamCreationTimeout = 30 * time.Second
+
+	// The SPDY subprotocol "channel.k8s.io" is used for remote command
+	// attachment/execution. This represents the initial unversioned subprotocol,
+	// which has the known bugs http://issues.k8s.io/13394 and
+	// http://issues.k8s.io/13395.
+	StreamProtocolV1Name = "channel.k8s.io"
+
+	// The SPDY subprotocol "v2.channel.k8s.io" is used for remote command
+	// attachment/execution. It is the second version of the subprotocol and
+	// resolves the issues present in the first version.
+	StreamProtocolV2Name = "v2.channel.k8s.io"
+)
+
+var SupportedStreamingProtocols = []string{StreamProtocolV2Name, StreamProtocolV1Name}

pkg/kubelet/server/remotecommand/doc.go

+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// package remotecommand contains functions related to executing commands in and attaching to pods.
+package remotecommand

pkg/kubelet/server/remotecommand/exec.go

+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package remotecommand
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+	"time"
+
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/types"
+	"k8s.io/kubernetes/pkg/util/runtime"
+)
+
+// Executor knows how to execute a command in a container in a pod.
+type Executor interface {
+	// ExecInContainer executes a command in a container in the pod, copying data
+	// between in/out/err and the container's stdin/stdout/stderr.
+	ExecInContainer(name string, uid types.UID, container string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool) error
+}
+
+// ServeExec handles requests to execute a command in a container. After
+// creating/receiving the required streams, it delegates the actual execution
+// to the executor.
+func ServeExec(w http.ResponseWriter, req *http.Request, executor Executor, podName string, uid types.UID, container string, idleTimeout, streamCreationTimeout time.Duration, supportedProtocols []string) {
+	ctx, ok := createStreams(req, w, supportedProtocols, idleTimeout, streamCreationTimeout)
+	if !ok {
+		// error is handled by createStreams
+		return
+	}
+	defer ctx.conn.Close()
+
+	cmd := req.URL.Query()[api.ExecCommandParamm]
+
+	err := executor.ExecInContainer(podName, uid, container, cmd, ctx.stdinStream, ctx.stdoutStream, ctx.stderrStream, ctx.tty)
+	if err != nil {
+		msg := fmt.Sprintf("error executing command in container: %v", err)
+		runtime.HandleError(errors.New(msg))
+		fmt.Fprint(ctx.errorStream, msg)
+	}
+}

pkg/kubelet/server/remotecommand/httpstream.go

+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package remotecommand
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+	"time"
+
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/util/httpstream"
+	"k8s.io/kubernetes/pkg/util/httpstream/spdy"
+	"k8s.io/kubernetes/pkg/util/runtime"
+	"k8s.io/kubernetes/pkg/util/wsstream"
+
+	"github.com/golang/glog"
+)
+
+// options contains details about which streams are required for
+// remote command execution.
+type options struct {
+	stdin           bool
+	stdout          bool
+	stderr          bool
+	tty             bool
+	expectedStreams int
+}
+
+// newOptions creates a new options from the Request.
+func newOptions(req *http.Request) (*options, error) {
+	tty := req.FormValue(api.ExecTTYParam) == "1"
+	stdin := req.FormValue(api.ExecStdinParam) == "1"
+	stdout := req.FormValue(api.ExecStdoutParam) == "1"
+	stderr := req.FormValue(api.ExecStderrParam) == "1"
+	if tty && stderr {
+		// TODO: make this an error before we reach this method
+		glog.V(4).Infof("Access to exec with tty and stderr is not supported, bypassing stderr")
+		stderr = false
+	}
+
+	// count the streams client asked for, starting with 1
+	expectedStreams := 1
+	if stdin {
+		expectedStreams++
+	}
+	if stdout {
+		expectedStreams++
+	}
+	if stderr {
+		expectedStreams++
+	}
+
+	if expectedStreams == 1 {
+		return nil, fmt.Errorf("you must specify at least 1 of stdin, stdout, stderr")
+	}
+
+	return &options{
+		stdin:           stdin,
+		stdout:          stdout,
+		stderr:          stderr,
+		tty:             tty,
+		expectedStreams: expectedStreams,
+	}, nil
+}
+
+// context contains the connection and streams used when
+// forwarding an attach or execute session into a container.
+type context struct {
+	conn         io.Closer
+	stdinStream  io.ReadCloser
+	stdoutStream io.WriteCloser
+	stderrStream io.WriteCloser
+	errorStream  io.WriteCloser
+	tty          bool
+}
+
+// streamAndReply holds both a Stream and a channel that is closed when the stream's reply frame is
+// enqueued. Consumers can wait for replySent to be closed prior to proceeding, to ensure that the
+// replyFrame is enqueued before the connection's goaway frame is sent (e.g. if a stream was
+// received and right after, the connection gets closed).
+type streamAndReply struct {
+	httpstream.Stream
+	replySent <-chan struct{}
+}
+
+// waitStreamReply waits until either replySent or stop is closed. If replySent is closed, it sends
+// an empty struct to the notify channel.
+func waitStreamReply(replySent <-chan struct{}, notify chan<- struct{}, stop <-chan struct{}) {
+	select {
+	case <-replySent:
+		notify <- struct{}{}
+	case <-stop:
+	}
+}
+
+func createStreams(req *http.Request, w http.ResponseWriter, supportedStreamProtocols []string, idleTimeout, streamCreationTimeout time.Duration) (*context, bool) {
+	opts, err := newOptions(req)
+	if err != nil {
+		runtime.HandleError(err)
+		w.WriteHeader(http.StatusBadRequest)
+		fmt.Fprint(w, err.Error())
+		return nil, false
+	}
+
+	if wsstream.IsWebSocketRequest(req) {
+		return createWebSocketStreams(req, w, opts, idleTimeout)
+	}
+
+	protocol, err := httpstream.Handshake(req, w, supportedStreamProtocols)
+	if err != nil {
+		w.WriteHeader(http.StatusBadRequest)
+		fmt.Fprint(w, err.Error())
+		return nil, false
+	}
+
+	streamCh := make(chan streamAndReply)
+
+	upgrader := spdy.NewResponseUpgrader()
+	conn := upgrader.UpgradeResponse(w, req, func(stream httpstream.Stream, replySent <-chan struct{}) error {
+		streamCh <- streamAndReply{Stream: stream, replySent: replySent}
+		return nil
+	})
+	// from this point on, we can no longer call methods on response
+	if conn == nil {
+		// The upgrader is responsible for notifying the client of any errors that
+		// occurred during upgrading. All we can do is return here at this point
+		// if we weren't successful in upgrading.
+		return nil, false
+	}
+
+	conn.SetIdleTimeout(idleTimeout)
+
+	var handler protocolHandler
+	switch protocol {
+	case StreamProtocolV2Name:
+		handler = &v2ProtocolHandler{}
+	case "":
+		glog.V(4).Infof("Client did not request protocol negotiaion. Falling back to %q", StreamProtocolV1Name)
+		fallthrough
+	case StreamProtocolV1Name:
+		handler = &v1ProtocolHandler{}
+	}
+
+	expired := time.NewTimer(streamCreationTimeout)
+
+	ctx, err := handler.waitForStreams(streamCh, opts.expectedStreams, expired.C)
+	if err != nil {
+		runtime.HandleError(err)
+		return nil, false
+	}
+
+	ctx.conn = conn
+	ctx.tty = opts.tty
+	return ctx, true
+}
+
+type protocolHandler interface {
+	// waitForStreams waits for the expected streams or a timeout, returning a
+	// remoteCommandContext if all the streams were received, or an error if not.
+	waitForStreams(streams <-chan streamAndReply, expectedStreams int, expired <-chan time.Time) (*context, error)
+}
+
+// v2ProtocolHandler implements the V2 protocol version for streaming command execution.
+type v2ProtocolHandler struct{}
+
+func (*v2ProtocolHandler) waitForStreams(streams <-chan streamAndReply, expectedStreams int, expired <-chan time.Time) (*context, error) {
+	ctx := &context{}
+	receivedStreams := 0
+	replyChan := make(chan struct{})
+	stop := make(chan struct{})
+	defer close(stop)
+WaitForStreams:
+	for {
+		select {
+		case stream := <-streams:
+			streamType := stream.Headers().Get(api.StreamType)
+			switch streamType {
+			case api.StreamTypeError:
+				ctx.errorStream = stream
+				go waitStreamReply(stream.replySent, replyChan, stop)
+			case api.StreamTypeStdin:
+				ctx.stdinStream = stream
+				go waitStreamReply(stream.replySent, replyChan, stop)
+			case api.StreamTypeStdout:
+				ctx.stdoutStream = stream
+				go waitStreamReply(stream.replySent, replyChan, stop)
+			case api.StreamTypeStderr:
+				ctx.stderrStream = stream
+				go waitStreamReply(stream.replySent, replyChan, stop)
+			default:
+				runtime.HandleError(fmt.Errorf("Unexpected stream type: %q", streamType))
+			}
+		case <-replyChan:
+			receivedStreams++
+			if receivedStreams == expectedStreams {
+				break WaitForStreams
+			}
+		case <-expired:
+			// TODO find a way to return the error to the user. Maybe use a separate
+			// stream to report errors?
+			return nil, errors.New("timed out waiting for client to create streams")
+		}
+	}
+
+	return ctx, nil
+}
+
+// v1ProtocolHandler implements the V1 protocol version for streaming command execution.
+type v1ProtocolHandler struct{}
+
+func (*v1ProtocolHandler) waitForStreams(streams <-chan streamAndReply, expectedStreams int, expired <-chan time.Time) (*context, error) {
+	ctx := &context{}
+	receivedStreams := 0
+	replyChan := make(chan struct{})
+	stop := make(chan struct{})
+	defer close(stop)
+WaitForStreams:
+	for {
+		select {
+		case stream := <-streams:
+			streamType := stream.Headers().Get(api.StreamType)
+			switch streamType {
+			case api.StreamTypeError:
+				ctx.errorStream = stream
+
+				// This defer statement shouldn't be here, but due to previous refactoring, it ended up in
+				// here. This is what 1.0.x kubelets do, so we're retaining that behavior. This is fixed in
+				// the v2ProtocolHandler.
+				defer stream.Reset()
+
+				go waitStreamReply(stream.replySent, replyChan, stop)
+			case api.StreamTypeStdin:
+				ctx.stdinStream = stream
+				go waitStreamReply(stream.replySent, replyChan, stop)
+			case api.StreamTypeStdout:
+				ctx.stdoutStream = stream
+				go waitStreamReply(stream.replySent, replyChan, stop)
+			case api.StreamTypeStderr:
+				ctx.stderrStream = stream
+				go waitStreamReply(stream.replySent, replyChan, stop)
+			default:
+				runtime.HandleError(fmt.Errorf("Unexpected stream type: %q", streamType))
+			}
+		case <-replyChan:
+			receivedStreams++
+			if receivedStreams == expectedStreams {
+				break WaitForStreams
+			}
+		case <-expired:
+			// TODO find a way to return the error to the user. Maybe use a separate
+			// stream to report errors?
+			return nil, errors.New("timed out waiting for client to create streams")
+		}
+	}
+
+	if ctx.stdinStream != nil {
+		ctx.stdinStream.Close()
+	}
+
+	return ctx, nil
+}

pkg/kubelet/server/remotecommand/websocket.go

+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package remotecommand
+
+import (
+	"net/http"
+	"time"
+
+	"k8s.io/kubernetes/pkg/httplog"
+	"k8s.io/kubernetes/pkg/util/wsstream"
+
+	"github.com/golang/glog"
+)
+
+// standardShellChannels returns the standard channel types for a shell connection (STDIN 0, STDOUT 1, STDERR 2)
+// along with the approximate duplex value. Supported subprotocols are "channel.k8s.io" and
+// "base64.channel.k8s.io".
+func standardShellChannels(stdin, stdout, stderr bool) []wsstream.ChannelType {
+	// open three half-duplex channels
+	channels := []wsstream.ChannelType{wsstream.ReadChannel, wsstream.WriteChannel, wsstream.WriteChannel}
+	if !stdin {
+		channels[0] = wsstream.IgnoreChannel
+	}
+	if !stdout {
+		channels[1] = wsstream.IgnoreChannel
+	}
+	if !stderr {
+		channels[2] = wsstream.IgnoreChannel
+	}
+	return channels
+}
+
+// createWebSocketStreams returns a remoteCommandContext containing the websocket connection and
+// streams needed to perform an exec or an attach.
+func createWebSocketStreams(req *http.Request, w http.ResponseWriter, opts *options, idleTimeout time.Duration) (*context, bool) {
+	// open the requested channels, and always open the error channel
+	channels := append(standardShellChannels(opts.stdin, opts.stdout, opts.stderr), wsstream.WriteChannel)
+	conn := wsstream.NewConn(channels...)
+	conn.SetIdleTimeout(idleTimeout)
+	streams, err := conn.Open(httplog.Unlogged(w), req)
+	if err != nil {
+		glog.Errorf("Unable to upgrade websocket connection: %v", err)
+		return nil, false
+	}
+	// Send an empty message to the lowest writable channel to notify the client the connection is established
+	// TODO: make generic to SPDY and WebSockets and do it outside of this method?
+	switch {
+	case opts.stdout:
+		streams[1].Write([]byte{})
+	case opts.stderr:
+		streams[2].Write([]byte{})
+	default:
+		streams[3].Write([]byte{})
+	}
+	return &context{
+		conn:         conn,
+		stdinStream:  streams[0],
+		stdoutStream: streams[1],
+		stderrStream: streams[2],
+		errorStream:  streams[3],
+		tty:          opts.tty,
+	}, true
+}

pkg/util/httpstream/httpstream.go

@@ -114,20 +114,24 @@ func negotiateProtocol(clientProtocols, serverProtocols []string) string {
 	return ""
 }
 
-// Handshake performs a subprotocol negotiation. If the client did not request
-// a specific subprotocol, defaultProtocol is used. If the client did request a
+// Handshake performs a subprotocol negotiation. If the client did request a
 // subprotocol, Handshake will select the first common value found in
 // serverProtocols. If a match is found, Handshake adds a response header
 // indicating the chosen subprotocol. If no match is found, HTTP forbidden is
 // returned, along with a response header containing the list of protocols the
 // server can accept.
-func Handshake(req *http.Request, w http.ResponseWriter, serverProtocols []string, defaultProtocol string) (string, error) {
+func Handshake(req *http.Request, w http.ResponseWriter, serverProtocols []string) (string, error) {
 	clientProtocols := req.Header[http.CanonicalHeaderKey(HeaderProtocolVersion)]
 	if len(clientProtocols) == 0 {
-		// Kube 1.0 client that didn't support subprotocol negotiation
-		// TODO remove this defaulting logic once Kube 1.0 is no longer supported
-		w.Header().Add(HeaderProtocolVersion, defaultProtocol)
-		return defaultProtocol, nil
+		// Kube 1.0 clients didn't support subprotocol negotiation.
+		// TODO require clientProtocols once Kube 1.0 is no longer supported
+		return "", nil
+	}
+
+	if len(serverProtocols) == 0 {
+		// Kube 1.0 servers didn't support subprotocol negotiation. This is mainly for testing.
+		// TODO require serverProtocols once Kube 1.0 is no longer supported
+		return "", nil
 	}
 
 	negotiatedProtocol := negotiateProtocol(clientProtocols, serverProtocols)

pkg/util/httpstream/httpstream_test.go

@@ -20,6 +20,8 @@ import (
 	"net/http"
 	"reflect"
 	"testing"
+
+	"k8s.io/kubernetes/pkg/api"
 )
 
 type responseWriter struct {
@@ -46,8 +48,6 @@ func (r *responseWriter) Write([]byte) (int, error) {
 }
 
 func TestHandshake(t *testing.T) {
-	defaultProtocol := "default"
-
 	tests := map[string]struct {
 		clientProtocols  []string
 		serverProtocols  []string
@@ -57,7 +57,7 @@ func TestHandshake(t *testing.T) {
 		"no client protocols": {
 			clientProtocols:  []string{},
 			serverProtocols:  []string{"a", "b"},
-			expectedProtocol: defaultProtocol,
+			expectedProtocol: "",
 		},
 		"no common protocol": {
 			clientProtocols:  []string{"c"},
@@ -83,7 +83,7 @@ func TestHandshake(t *testing.T) {
 		}
 
 		w := newResponseWriter()
-		negotiated, err := Handshake(req, w, test.serverProtocols, defaultProtocol)
+		negotiated, err := Handshake(req, w, test.serverProtocols)
 
 		// verify negotiated protocol
 		if e, a := test.expectedProtocol, negotiated; e != a {
@@ -112,8 +112,15 @@ func TestHandshake(t *testing.T) {
 			t.Errorf("%s: unexpected non-nil w.statusCode: %d", name, w.statusCode)
 		}
 
+		if len(test.expectedProtocol) == 0 {
+			if len(w.Header()[HeaderProtocolVersion]) > 0 {
+				t.Errorf("%s: unexpected protocol version response header: %s", w.Header()[HeaderProtocolVersion])
+			}
+			continue
+		}
+
 		// verify response headers
-		if e, a := []string{test.expectedProtocol}, w.Header()[HeaderProtocolVersion]; !reflect.DeepEqual(e, a) {
+		if e, a := []string{test.expectedProtocol}, w.Header()[HeaderProtocolVersion]; !api.Semantic.DeepEqual(e, a) {
 			t.Errorf("%s: protocol response header: expected %v, got %v", name, e, a)
 		}
 	}