add validate to not allow mix --config with other arguments

Author: xilabao

Diff for: cmd/kubeadm/app/apis/kubeadm/validation/BUILD

@@ -15,6 +15,7 @@ go_test(
     tags = ["automanaged"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
+        "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
     ],
 )
@@ -30,6 +31,7 @@ go_library(
         "//pkg/api/validation:go_default_library",
         "//pkg/kubeapiserver/authorizer/modes:go_default_library",
         "//pkg/registry/core/service/ipallocator:go_default_library",
+        "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
     ],

Diff for: cmd/kubeadm/app/apis/kubeadm/validation/validation.go

@@ -24,6 +24,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/spf13/pflag"
+
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -239,3 +241,10 @@ func ValidateCloudProvider(provider string, fldPath *field.Path) field.ErrorList
 	allErrs = append(allErrs, field.Invalid(fldPath, provider, "cloudprovider not supported"))
 	return allErrs
 }
+
+func ValidateMixedArguments(flag *pflag.FlagSet) error {
+	if flag.Changed("config") && flag.NFlag() != 1 {
+		return fmt.Errorf("can not mix '--config' with other arguments")
+	}
+	return nil
+}

Diff for: cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go

@@ -19,6 +19,8 @@ package validation
 import (
 	"testing"
 
+	"github.com/spf13/pflag"
+
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 )
@@ -227,3 +229,42 @@ func TestValidateNodeConfiguration(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateMixedArguments(t *testing.T) {
+	var tests = []struct {
+		args     []string
+		expected bool
+	}{
+		{[]string{"--foo=bar"}, true},
+		{[]string{"--config=hello"}, true},
+		{[]string{"--foo=bar", "--config=hello"}, false},
+	}
+
+	var cfgPath string
+	var skipPreFlight bool
+
+	for _, rt := range tests {
+		f := pflag.NewFlagSet("test", pflag.ContinueOnError)
+		if f.Parsed() {
+			t.Error("f.Parse() = true before Parse")
+		}
+		f.String("foo", "", "string value")
+		f.StringVar(&cfgPath, "config", cfgPath, "Path to kubeadm config file")
+		f.BoolVar(
+			&skipPreFlight, "skip-preflight-checks", skipPreFlight,
+			"Skip preflight checks normally run before modifying the system",
+		)
+		if err := f.Parse(rt.args); err != nil {
+			t.Fatal(err)
+		}
+
+		actual := ValidateMixedArguments(f)
+		if (actual == nil) != rt.expected {
+			t.Errorf(
+				"failed ValidateMixedArguments:\n\texpected: %t\n\t  actual: %t",
+				rt.expected,
+				(actual == nil),
+			)
+		}
+	}
+}

Diff for: cmd/kubeadm/app/cmd/init.go

@@ -83,7 +83,7 @@ func NewCmdInit(out io.Writer) *cobra.Command {
 
 			i, err := NewInit(cfgPath, internalcfg, skipPreFlight, skipTokenPrint)
 			kubeadmutil.CheckErr(err)
-			kubeadmutil.CheckErr(i.Validate())
+			kubeadmutil.CheckErr(i.Validate(cmd))
 			kubeadmutil.CheckErr(i.Run(out))
 		},
 	}
@@ -191,7 +191,10 @@ type Init struct {
 }
 
 // Validate validates configuration passed to "kubeadm init"
-func (i *Init) Validate() error {
+func (i *Init) Validate(cmd *cobra.Command) error {
+	if err := validation.ValidateMixedArguments(cmd.PersistentFlags()); err != nil {
+		return err
+	}
 	return validation.ValidateMasterConfiguration(i.cfg).ToAggregate()
 }
 

Diff for: cmd/kubeadm/app/cmd/join.go

@@ -63,26 +63,26 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
 		Use:   "join <flags> [DiscoveryTokenAPIServers]",
 		Short: "Run this on any machine you wish to join an existing cluster",
 		Long: dedent.Dedent(`
-		When joining a kubeadm initialized cluster, we need to establish 
-		bidirectional trust. This is split into discovery (having the Node 
-		trust the Kubernetes Master) and TLS bootstrap (having the Kubernetes 
+		When joining a kubeadm initialized cluster, we need to establish
+		bidirectional trust. This is split into discovery (having the Node
+		trust the Kubernetes Master) and TLS bootstrap (having the Kubernetes
 		Master trust the Node).
 
-		There are 2 main schemes for discovery. The first is to use a shared 
-		token along with the IP address of the API server. The second is to 
-		provide a file (a subset of the standard kubeconfig file). This file 
-		can be a local file or downloaded via an HTTPS URL. The forms are 
-		kubeadm join --discovery-token abcdef.1234567890abcdef 1.2.3.4:6443, 
+		There are 2 main schemes for discovery. The first is to use a shared
+		token along with the IP address of the API server. The second is to
+		provide a file (a subset of the standard kubeconfig file). This file
+		can be a local file or downloaded via an HTTPS URL. The forms are
+		kubeadm join --discovery-token abcdef.1234567890abcdef 1.2.3.4:6443,
 		kubeadm join --discovery-file path/to/file.conf, or kubeadm join
-		--discovery-file https://url/file.conf. Only one form can be used. If 
-		the discovery information is loaded from a URL, HTTPS must be used and 
+		--discovery-file https://url/file.conf. Only one form can be used. If
+		the discovery information is loaded from a URL, HTTPS must be used and
 		the host installed CA bundle is used to verify the connection.
 
-		The TLS bootstrap mechanism is also driven via a shared token. This is 
+		The TLS bootstrap mechanism is also driven via a shared token. This is
 		used to temporarily authenticate with the Kubernetes Master to submit a
-		certificate signing request (CSR) for a locally created key pair. By 
-		default kubeadm will set up the Kubernetes Master to automatically 
-		approve these signing requests. This token is passed in with the 
+		certificate signing request (CSR) for a locally created key pair. By
+		default kubeadm will set up the Kubernetes Master to automatically
+		approve these signing requests. This token is passed in with the
 		--tls-bootstrap-token abcdef.1234567890abcdef flag.
 
 		Often times the same token is used for both parts. In this case, the
@@ -97,7 +97,7 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
 
 			j, err := NewJoin(cfgPath, args, internalcfg, skipPreFlight)
 			kubeadmutil.CheckErr(err)
-			kubeadmutil.CheckErr(j.Validate())
+			kubeadmutil.CheckErr(j.Validate(cmd))
 			kubeadmutil.CheckErr(j.Run(out))
 		},
 	}
@@ -166,7 +166,10 @@ func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, s
 	return &Join{cfg: cfg}, nil
 }
 
-func (j *Join) Validate() error {
+func (j *Join) Validate(cmd *cobra.Command) error {
+	if err := validation.ValidateMixedArguments(cmd.PersistentFlags()); err != nil {
+		return err
+	}
 	return validation.ValidateNodeConfiguration(j.cfg).ToAggregate()
 }
 

Diff for: cmd/kubeadm/test/cmd/init_test.go

@@ -137,3 +137,31 @@ func TestCmdInitAPIPort(t *testing.T) {
 		kubeadmReset()
 	}
 }
+
+func TestCmdInitArgsMixed(t *testing.T) {
+	if *kubeadmCmdSkip {
+		t.Log("kubeadm cmd tests being skipped")
+		t.Skip()
+	}
+
+	var initTest = []struct {
+		args     string
+		expected bool
+	}{
+		{"--api-port=1000 --config=/etc/kubernets/kubeadm.config", false},
+	}
+
+	for _, rt := range initTest {
+		_, _, actual := RunCmd(*kubeadmPath, "init", rt.args, "--skip-preflight-checks")
+		if (actual == nil) != rt.expected {
+			t.Errorf(
+				"failed CmdInitArgsMixed running 'kubeadm init %s' with an error: %v\n\texpected: %t\n\t  actual: %t",
+				rt.args,
+				actual,
+				rt.expected,
+				(actual == nil),
+			)
+		}
+		kubeadmReset()
+	}
+}

Diff for: cmd/kubeadm/test/cmd/join_test.go

@@ -191,3 +191,31 @@ func TestCmdJoinBadArgs(t *testing.T) {
 		kubeadmReset()
 	}
 }
+
+func TestCmdJoinArgsMixed(t *testing.T) {
+	if *kubeadmCmdSkip {
+		t.Log("kubeadm cmd tests being skipped")
+		t.Skip()
+	}
+
+	var initTest = []struct {
+		args     string
+		expected bool
+	}{
+		{"--discovery-token=abcdef.1234567890abcdef --config=/etc/kubernets/kubeadm.config", false},
+	}
+
+	for _, rt := range initTest {
+		_, _, actual := RunCmd(*kubeadmPath, "join", rt.args, "--skip-preflight-checks")
+		if (actual == nil) != rt.expected {
+			t.Errorf(
+				"failed CmdJoinArgsMixed running 'kubeadm join %s' with an error: %v\n\texpected: %t\n\t  actual: %t",
+				rt.args,
+				actual,
+				rt.expected,
+				(actual == nil),
+			)
+		}
+		kubeadmReset()
+	}
+}