From c60f19a1f816ac0cde9b574855b05c7f84d91ebf Mon Sep 17 00:00:00 2001
From: Fabiano Franz <ffranz@redhat.com>
Date: Thu, 29 Oct 2015 18:32:58 -0200
Subject: [PATCH] Attach must only allow a tty when container supports it

---
 pkg/kubectl/cmd/attach.go      | 28 +++++++++++---
 pkg/kubectl/cmd/attach_test.go | 67 ++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 6 deletions(-)

diff --git a/pkg/kubectl/cmd/attach.go b/pkg/kubectl/cmd/attach.go
index 611cc0b6565..4de88e80947 100644
--- a/pkg/kubectl/cmd/attach.go
+++ b/pkg/kubectl/cmd/attach.go
@@ -160,9 +160,16 @@ func (p *AttachOptions) Run() error {
 		return fmt.Errorf("pod %s is not running and cannot be attached to; current phase is %s", p.PodName, pod.Status.Phase)
 	}
 
-	// TODO: refactor with terminal helpers from the edit utility once that is merged
 	var stdin io.Reader
 	tty := p.TTY
+
+	containerToAttach := p.GetContainer(pod)
+	if tty && !containerToAttach.TTY {
+		tty = false
+		fmt.Fprintf(p.Err, "Unable to use a TTY - container %s doesn't allocate one\n", containerToAttach.Name)
+	}
+
+	// TODO: refactor with terminal helpers from the edit utility once that is merged
 	if p.Stdin {
 		stdin = p.In
 		if tty {
@@ -204,7 +211,7 @@ func (p *AttachOptions) Run() error {
 		Namespace(pod.Namespace).
 		SubResource("attach")
 	req.VersionedParams(&api.PodAttachOptions{
-		Container: p.GetContainerName(pod),
+		Container: containerToAttach.Name,
 		Stdin:     stdin != nil,
 		Stdout:    p.Out != nil,
 		Stderr:    p.Err != nil,
@@ -214,12 +221,21 @@ func (p *AttachOptions) Run() error {
 	return p.Attach.Attach("POST", req.URL(), p.Config, stdin, p.Out, p.Err, tty)
 }
 
-// GetContainerName returns the name of the container to attach to, with a fallback.
-func (p *AttachOptions) GetContainerName(pod *api.Pod) string {
+// GetContainer returns the container to attach to, with a fallback.
+func (p *AttachOptions) GetContainer(pod *api.Pod) api.Container {
 	if len(p.ContainerName) > 0 {
-		return p.ContainerName
+		for _, container := range pod.Spec.Containers {
+			if container.Name == p.ContainerName {
+				return container
+			}
+		}
 	}
 
 	glog.V(4).Infof("defaulting container name to %s", pod.Spec.Containers[0].Name)
-	return pod.Spec.Containers[0].Name
+	return pod.Spec.Containers[0]
+}
+
+// GetContainerName returns the name of the container to attach to, with a fallback.
+func (p *AttachOptions) GetContainerName(pod *api.Pod) string {
+	return p.GetContainer(pod).Name
 }
diff --git a/pkg/kubectl/cmd/attach_test.go b/pkg/kubectl/cmd/attach_test.go
index c508fcf27c6..9d8e46283cc 100644
--- a/pkg/kubectl/cmd/attach_test.go
+++ b/pkg/kubectl/cmd/attach_test.go
@@ -22,6 +22,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -192,6 +193,72 @@ func TestAttach(t *testing.T) {
 	}
 }
 
+func TestAttachWarnings(t *testing.T) {
+	version := testapi.Default.Version()
+	tests := []struct {
+		name, container, version, podPath, expectedErr, expectedOut string
+		pod                                                         *api.Pod
+		stdin, tty                                                  bool
+	}{
+		{
+			name:        "fallback tty if not supported",
+			version:     version,
+			podPath:     "/api/" + version + "/namespaces/test/pods/foo",
+			pod:         attachPod(),
+			stdin:       true,
+			tty:         true,
+			expectedErr: "Unable to use a TTY - container bar doesn't allocate one",
+		},
+	}
+	for _, test := range tests {
+		f, tf, codec := NewAPIFactory()
+		tf.Client = &fake.RESTClient{
+			Codec: codec,
+			Client: fake.HTTPClientFunc(func(req *http.Request) (*http.Response, error) {
+				switch p, m := req.URL.Path, req.Method; {
+				case p == test.podPath && m == "GET":
+					body := objBody(codec, test.pod)
+					return &http.Response{StatusCode: 200, Body: body}, nil
+				default:
+					t.Errorf("%s: unexpected request: %s %#v\n%#v", test.name, req.Method, req.URL, req)
+					return nil, fmt.Errorf("unexpected request")
+				}
+			}),
+		}
+		tf.Namespace = "test"
+		tf.ClientConfig = &client.Config{Version: test.version}
+		bufOut := bytes.NewBuffer([]byte{})
+		bufErr := bytes.NewBuffer([]byte{})
+		bufIn := bytes.NewBuffer([]byte{})
+		ex := &fakeRemoteAttach{}
+		params := &AttachOptions{
+			ContainerName: test.container,
+			In:            bufIn,
+			Out:           bufOut,
+			Err:           bufErr,
+			Stdin:         test.stdin,
+			TTY:           test.tty,
+			Attach:        ex,
+		}
+		cmd := &cobra.Command{}
+		if err := params.Complete(f, cmd, []string{"foo"}); err != nil {
+			t.Fatal(err)
+		}
+		if err := params.Run(); err != nil {
+			t.Fatal(err)
+		}
+
+		if test.stdin && test.tty {
+			if !test.pod.Spec.Containers[0].TTY {
+				if !strings.Contains(bufErr.String(), test.expectedErr) {
+					t.Errorf("%s: Expected TTY fallback warning for attach request: %s", test.name, bufErr.String())
+					continue
+				}
+			}
+		}
+	}
+}
+
 func attachPod() *api.Pod {
 	return &api.Pod{
 		ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "test", ResourceVersion: "10"},