Merge pull request #35732 from fabianofranz/run_cant_dryrun_with_attach

Automatic merge from submit-queue

Better kubectl run validations

Adds more validations to flags that must be mutually exclusive in `kubectl run`. For example, `--dry-run` must not be used with `--attach`, `--stdin` or `--tty`. Adds unit tests for these new validations and some previously existing ones.

**Release note**:
<!--  Steps to write your release note:
1. Use the release-note-* labels to set the release note state (if you have access) 
2. Enter your extended release note in the below block; leaving it blank means using the PR title as the release note. If no release note is required, just write `NONE`. 
-->
```release-note
NONE
```

pkg/kubectl/cmd/run.go

@@ -174,6 +174,22 @@ func Run(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *cobr
 		return cmdutil.UsageError(cmd, fmt.Sprintf("--restart=%s requires that --replicas=1, found %d", restartPolicy, replicas))
 	}
 
+	attachFlag := cmd.Flags().Lookup("attach")
+	attach := cmdutil.GetFlagBool(cmd, "attach")
+
+	if !attachFlag.Changed && interactive {
+		attach = true
+	}
+
+	remove := cmdutil.GetFlagBool(cmd, "rm")
+	if !attach && remove {
+		return cmdutil.UsageError(cmd, "--rm should only be used for attached containers")
+	}
+
+	if attach && cmdutil.GetDryRunFlag(cmd) {
+		return cmdutil.UsageError(cmd, "--dry-run can't be used with attached containers options (--attach, --stdin, or --tty)")
+	}
+
 	if err := verifyImagePullPolicy(cmd); err != nil {
 		return err
 	}
@@ -241,18 +257,6 @@ func Run(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *cobr
 		}
 	}
 
-	attachFlag := cmd.Flags().Lookup("attach")
-	attach := cmdutil.GetFlagBool(cmd, "attach")
-
-	if !attachFlag.Changed && interactive {
-		attach = true
-	}
-
-	remove := cmdutil.GetFlagBool(cmd, "rm")
-	if !attach && remove {
-		return cmdutil.UsageError(cmd, "--rm should only be used for attached containers")
-	}
-
 	if attach {
 		quiet := cmdutil.GetFlagBool(cmd, "quiet")
 		opts := &AttachOptions{

pkg/kubectl/cmd/run_test.go

@@ -23,10 +23,12 @@ import (
 	"net/http"
 	"os"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/apimachinery/registered"
 	"k8s.io/kubernetes/pkg/client/restclient"
 	"k8s.io/kubernetes/pkg/client/restclient/fake"
@@ -332,3 +334,97 @@ func TestGenerateService(t *testing.T) {
 		}
 	}
 }
+
+func TestRunValidations(t *testing.T) {
+	tests := []struct {
+		args        []string
+		flags       map[string]string
+		expectedErr string
+	}{
+		{
+			expectedErr: "NAME is required",
+		},
+		{
+			args:        []string{"test"},
+			expectedErr: "Invalid image name",
+		},
+		{
+			args: []string{"test"},
+			flags: map[string]string{
+				"image":    "busybox",
+				"stdin":    "true",
+				"replicas": "2",
+			},
+			expectedErr: "stdin requires that replicas is 1",
+		},
+		{
+			args: []string{"test"},
+			flags: map[string]string{
+				"image": "busybox",
+				"rm":    "true",
+			},
+			expectedErr: "rm should only be used for attached containers",
+		},
+		{
+			args: []string{"test"},
+			flags: map[string]string{
+				"image":   "busybox",
+				"attach":  "true",
+				"dry-run": "true",
+			},
+			expectedErr: "can't be used with attached containers options",
+		},
+		{
+			args: []string{"test"},
+			flags: map[string]string{
+				"image":   "busybox",
+				"stdin":   "true",
+				"dry-run": "true",
+			},
+			expectedErr: "can't be used with attached containers options",
+		},
+		{
+			args: []string{"test"},
+			flags: map[string]string{
+				"image":   "busybox",
+				"tty":     "true",
+				"stdin":   "true",
+				"dry-run": "true",
+			},
+			expectedErr: "can't be used with attached containers options",
+		},
+		{
+			args: []string{"test"},
+			flags: map[string]string{
+				"image": "busybox",
+				"tty":   "true",
+			},
+			expectedErr: "stdin is required for containers with -t/--tty",
+		},
+	}
+	for _, test := range tests {
+		f, tf, codec, ns := cmdtesting.NewTestFactory()
+		tf.Printer = &testPrinter{}
+		tf.Client = &fake.RESTClient{
+			NegotiatedSerializer: ns,
+			Resp:                 &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, cmdtesting.NewInternalType("", "", ""))},
+		}
+		tf.Namespace = "test"
+		tf.ClientConfig = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &unversioned.GroupVersion{Version: "v1"}}}
+		inBuf := bytes.NewReader([]byte{})
+		outBuf := bytes.NewBuffer([]byte{})
+		errBuf := bytes.NewBuffer([]byte{})
+
+		cmd := NewCmdRun(f, inBuf, outBuf, errBuf)
+		for flagName, flagValue := range test.flags {
+			cmd.Flags().Set(flagName, flagValue)
+		}
+		err := Run(f, inBuf, outBuf, errBuf, cmd, test.args, cmd.ArgsLenAtDash())
+		if err != nil && len(test.expectedErr) > 0 {
+			if !strings.Contains(err.Error(), test.expectedErr) {
+				t.Errorf("unexpected error: %v", err)
+			}
+		}
+	}
+
+}

pkg/kubectl/cmd/testing/fake.go

@@ -281,8 +281,15 @@ func (f *FakeFactory) DefaultNamespace() (string, bool, error) {
 	return f.tf.Namespace, false, f.tf.Err
 }
 
-func (f *FakeFactory) Generators(string) map[string]kubectl.Generator {
+func (f *FakeFactory) Generators(cmdName string) map[string]kubectl.Generator {
-	return nil
+	var generator map[string]kubectl.Generator
+	switch cmdName {
+	case "run":
+		generator = map[string]kubectl.Generator{
+			cmdutil.DeploymentV1Beta1GeneratorName: kubectl.DeploymentV1Beta1{},
+		}
+	}
+	return generator
 }
 
 func (f *FakeFactory) CanBeExposed(unversioned.GroupKind) error {