Whitelist for kubeadm extra args

cmd/minikube/cmd/start.go

@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	. "k8s.io/minikube/pkg/minikube/bootstrapper/kubeadm"
 	"net"
 	"os"
 	"os/exec"
@@ -79,7 +80,6 @@ const (
 	apiServerPort         = "apiserver-port"
 	dnsDomain             = "dns-domain"
 	serviceCIDR           = "service-cluster-ip-range"
-	podSubnet             = "pod-network-cidr"
 	imageRepository       = "image-repository"
 	imageMirrorCountry    = "image-mirror-country"
 	mountString           = "mount-string"
@@ -132,7 +132,6 @@ func init() {
 	startCmd.Flags().IPSliceVar(&apiServerIPs, "apiserver-ips", nil, "A set of apiserver IP Addresses which are used in the generated certificate for kubernetes.  This can be used if you want to make the apiserver available from outside the machine")
 	startCmd.Flags().String(dnsDomain, constants.ClusterDNSDomain, "The cluster dns domain name used in the kubernetes cluster")
 	startCmd.Flags().String(serviceCIDR, pkgutil.DefaultServiceCIDR, "The CIDR to be used for service cluster IPs.")
-	startCmd.Flags().String(podSubnet, "", "Specify range of IP addresses for the pod network. If set, the control plane will automatically allocate CIDRs for every node.")
 	startCmd.Flags().StringSliceVar(&insecureRegistry, "insecure-registry", nil, "Insecure Docker registries to pass to the Docker daemon.  The default service CIDR range will automatically be added.")
 	startCmd.Flags().StringSliceVar(&registryMirror, "registry-mirror", nil, "Registry mirrors to pass to the Docker daemon")
 	startCmd.Flags().String(imageRepository, "", "Alternative image repository to pull docker images from. This can be used when you have limited access to gcr.io. Set it to \"auto\" to let minikube decide one for you. For Chinese mainland users, you may use local gcr.io mirrors such as registry.cn-hangzhou.aliyuncs.com/google_containers")
@@ -148,7 +147,8 @@ func init() {
 	startCmd.Flags().Var(&extraOptions, "extra-config",
 		`A set of key=value pairs that describe configuration that may be passed to different components.
 		The key should be '.' separated, and the first part before the dot is the component to apply the configuration to.
-		Valid components are: kubelet, kubeadm, apiserver, controller-manager, etcd, proxy, scheduler.`)
+		Valid components are: kubelet, kubeadm, apiserver, controller-manager, etcd, proxy, scheduler
+		Valid kubeadm parameters: `+fmt.Sprintf("%s, %s", strings.Join(KubeadmExtraArgsWhitelist[KubeadmCmdParam], ", "), strings.Join(KubeadmExtraArgsWhitelist[KubeadmConfigParam], ",")))
 	startCmd.Flags().String(uuid, "", "Provide VM UUID to restore MAC address (only supported with Hyperkit driver).")
 	startCmd.Flags().String(vpnkitSock, "", "Location of the VPNKit socket used for networking. If empty, disables Hyperkit VPNKitSock, if 'auto' uses Docker for Mac VPNKit connection, otherwise uses the specified VSock.")
 	startCmd.Flags().StringSlice(vsockPorts, []string{}, "List of guest VSock ports that should be exposed as sockets on the host (Only supported on with hyperkit now).")
@@ -339,6 +339,14 @@ func validateConfig() {
 	if viper.GetBool(hidden) && viper.GetString(vmDriver) != "kvm2" {
 		exit.Usage("Sorry, the --hidden feature is currently only supported with --vm-driver=kvm2")
 	}
+
+	// check that kubeadm extra args contain only whitelisted parameters
+	for param := range extraOptions.AsMap().Get(Kubeadm) {
+		if !pkgutil.ContainsString(KubeadmExtraArgsWhitelist[KubeadmCmdParam], param) &&
+			!pkgutil.ContainsString(KubeadmExtraArgsWhitelist[KubeadmConfigParam], param) {
+			exit.Usage("Sorry, the kubeadm.%s parameter is currently not supported by --extra-config", param)
+		}
+	}
 }
 
 // doCacheBinaries caches Kubernetes binaries in the foreground
@@ -459,7 +467,6 @@ func generateConfig(cmd *cobra.Command, k8sVersion string) (cfg.Config, error) {
 			CRISocket:              viper.GetString(criSocket),
 			NetworkPlugin:          selectedNetworkPlugin,
 			ServiceCIDR:            viper.GetString(serviceCIDR),
-			PodSubnet:              viper.GetString(podSubnet),
 			ImageRepository:        repository,
 			ExtraOptions:           extraOptions,
 			ShouldLoadCachedImages: viper.GetBool(cacheImages),

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -43,6 +43,35 @@ import (
 	"k8s.io/minikube/pkg/util"
 )
 
+// enum to differentiate kubeadm command line parameters from kubeadm config file parameters (see the
+// KubeadmExtraArgsWhitelist variable below for more info)
+const (
+	KubeadmCmdParam    = iota
+	KubeadmConfigParam = iota
+)
+
+// KubeadmExtraArgsWhitelist is a whitelist of supported kubeadm params that can be supplied to kubeadm through
+// minikube's ExtraArgs parameter. The list is split into two parts - params that can be supplied as flags on the
+// command line and params that have to be inserted into the kubeadm config file. This is because of a kubeadm
+// constraint which allows only certain params to be provided from the command line when the --config parameter
+// is specified
+var KubeadmExtraArgsWhitelist = map[int][]string{
+	KubeadmCmdParam: {
+		"ignore-preflight-errors",
+		"dry-run",
+		"kubeconfig",
+		"kubeconfig-dir",
+		"node-name",
+		"cri-socket",
+		"experimental-upload-certs",
+		"certificate-key",
+		"rootfs",
+	},
+	KubeadmConfigParam: {
+		"pod-network-cidr",
+	},
+}
+
 // SkipPreflights are preflight checks we always skip.
 var SkipPreflights = []string{
 	// We use --ignore-preflight-errors=DirAvailable since we have our own custom addons
@@ -163,18 +192,29 @@ func (k *Bootstrapper) LogCommands(o bootstrapper.LogOptions) map[string]string
 	}
 }
 
+// createFlagsFromExtraArgs converts kubeadm extra args into flags to be supplied from the commad linne
+func createFlagsFromExtraArgs(extraOptions util.ExtraOptionSlice) string {
+	kubeadmExtraOpts := extraOptions.AsMap().Get(Kubeadm)
+
+	// kubeadm allows only a small set of parameters to be supplied from the command line when the --config param
+	// is specified, here we remove those that are not allowed
+	for opt := range kubeadmExtraOpts {
+		if !util.ContainsString(KubeadmExtraArgsWhitelist[KubeadmCmdParam], opt) == true {
+			// kubeadmExtraOpts is a copy so safe to delete
+			delete(kubeadmExtraOpts, opt)
+		}
+	}
+	return convertToFlags(kubeadmExtraOpts)
+}
+
 // StartCluster starts the cluster
 func (k *Bootstrapper) StartCluster(k8s config.KubernetesConfig) error {
 	version, err := ParseKubernetesVersion(k8s.KubernetesVersion)
 	if err != nil {
 		return errors.Wrap(err, "parsing kubernetes version")
 	}
 
-	extraOpts, err := ExtraConfigForComponent(Kubeadm, k8s.ExtraOptions, version)
-	if err != nil {
-		return errors.Wrap(err, "generating extra configuration for kubelet")
-	}
-	extraFlags := convertToFlags(extraOpts)
+	extraFlags := createFlagsFromExtraArgs(k8s.ExtraOptions)
 
 	r, err := cruntime.New(cruntime.Config{Type: k8s.ContainerRuntime})
 	if err != nil {
@@ -474,6 +514,25 @@ sudo systemctl start kubelet
 	return nil
 }
 
+// createExtraComponentConfig generates a map of component to extra args for all of the components except kubeadm
+func createExtraComponentConfig(extraOptions util.ExtraOptionSlice, version semver.Version, componentFeatureArgs string) ([]ComponentExtraArgs, error) {
+	extraArgsSlice, err := NewComponentExtraArgs(extraOptions, version, componentFeatureArgs)
+	if err != nil {
+		return nil, err
+	}
+
+	// kubeadm extra args should not be included in the kubeadm config in the extra args section (instead, they must
+	// be inserted explicitly in the appropriate places or supplied from the command line); here we remove all of the
+	// kubeadm extra args from the slice
+	for i, extraArgs := range extraArgsSlice {
+		if extraArgs.Component == Kubeadm {
+			extraArgsSlice = append(extraArgsSlice[:i], extraArgsSlice[i+1:]...)
+			break
+		}
+	}
+	return extraArgsSlice, nil
+}
+
 func generateConfig(k8s config.KubernetesConfig, r cruntime.Manager) (string, error) {
 	version, err := ParseKubernetesVersion(k8s.KubernetesVersion)
 	if err != nil {
@@ -486,8 +545,7 @@ func generateConfig(k8s config.KubernetesConfig, r cruntime.Manager) (string, er
 		return "", errors.Wrap(err, "parses feature gate config for kubeadm and component")
 	}
 
-	// generates a map of component to extra args for apiserver, controller-manager, and scheduler
-	extraComponentConfig, err := NewComponentExtraArgs(k8s.ExtraOptions, version, componentFeatureArgs)
+	extraComponentConfig, err := createExtraComponentConfig(k8s.ExtraOptions, version, componentFeatureArgs)
 	if err != nil {
 		return "", errors.Wrap(err, "generating extra component config for kubeadm")
 	}
@@ -515,7 +573,7 @@ func generateConfig(k8s config.KubernetesConfig, r cruntime.Manager) (string, er
 	}{
 		CertDir:           util.DefaultCertPath,
 		ServiceCIDR:       util.DefaultServiceCIDR,
-		PodSubnet:         k8s.PodSubnet,
+		PodSubnet:         k8s.ExtraOptions.Get("pod-network-cidr", Kubeadm),
 		AdvertiseAddress:  k8s.NodeIP,
 		APIServerPort:     nodePort,
 		KubernetesVersion: k8s.KubernetesVersion,

pkg/minikube/bootstrapper/kubeadm/kubeadm_test.go

@@ -153,6 +153,24 @@ func TestGenerateConfig(t *testing.T) {
 			Key:       "scheduler-name",
 			Value:     "mini-scheduler",
 		},
+		util.ExtraOption{
+			Component: Kubeadm,
+			Key:       "ignore-preflight-errors",
+			Value:     "true",
+		},
+		util.ExtraOption{
+			Component: Kubeadm,
+			Key:       "dry-run",
+			Value:     "true",
+		},
+	}
+
+	extraOptsPodCidr := util.ExtraOptionSlice{
+		util.ExtraOption{
+			Component: Kubeadm,
+			Key:       "pod-network-cidr",
+			Value:     "192.168.32.0/20",
+		},
 	}
 
 	// Test version policy: Last 4 major releases (slightly looser than our general policy)
@@ -177,7 +195,7 @@ func TestGenerateConfig(t *testing.T) {
 		{"crio-options-gates", "crio", false, config.KubernetesConfig{ExtraOptions: extraOpts, FeatureGates: "a=b"}},
 		{"unknown-component", "docker", true, config.KubernetesConfig{ExtraOptions: util.ExtraOptionSlice{util.ExtraOption{Component: "not-a-real-component", Key: "killswitch", Value: "true"}}}},
 		{"containerd-api-port", "containerd", false, config.KubernetesConfig{NodePort: 12345}},
-		{"containerd-pod-network-cidr", "containerd", false, config.KubernetesConfig{PodSubnet: "192.168.32.0/20"}},
+		{"containerd-pod-network-cidr", "containerd", false, config.KubernetesConfig{ExtraOptions: extraOptsPodCidr}},
 		{"image-repository", "docker", false, config.KubernetesConfig{ImageRepository: "test/repo"}},
 	}
 	for vname, version := range versions {

pkg/minikube/bootstrapper/kubeadm/testdata/containerd-pod-network-cidr__default.yaml

@@ -28,7 +28,7 @@ dns:
 etcd:
   local:
     dataDir: /data/minikube
-kubernetesVersion: v1.14.0
+kubernetesVersion: v1.14.1
 networking:
   dnsDomain: cluster.local
   podSubnet: ""

pkg/minikube/bootstrapper/kubeadm/testdata/crio-options-gates__default.yaml

@@ -26,9 +26,6 @@ controllerManager:
   extraArgs:
     feature-gates: "a=b"
     kube-api-burst: "32"
-kubeadm:
-  extraArgs:
-    feature-gates: "a=b"
 scheduler:
   extraArgs:
     feature-gates: "a=b"

pkg/minikube/bootstrapper/kubeadm/testdata/crio-options-gates__new.yaml

@@ -26,9 +26,6 @@ controllerManager:
   extraArgs:
     feature-gates: "a=b"
     kube-api-burst: "32"
-kubeadm:
-  extraArgs:
-    feature-gates: "a=b"
 scheduler:
   extraArgs:
     feature-gates: "a=b"

pkg/minikube/bootstrapper/kubeadm/testdata/crio-options-gates__obsolete.yaml

@@ -20,8 +20,6 @@ apiServerExtraArgs:
 controllerManagerExtraArgs:
   feature-gates: "a=b"
   kube-api-burst: "32"
-kubeadmExtraArgs:
-  feature-gates: "a=b"
 schedulerExtraArgs:
   feature-gates: "a=b"
   scheduler-name: "mini-scheduler"

pkg/minikube/bootstrapper/kubeadm/testdata/crio-options-gates__old.yaml

@@ -24,8 +24,6 @@ apiServerExtraArgs:
 controllerManagerExtraArgs:
   feature-gates: "a=b"
   kube-api-burst: "32"
-kubeadmExtraArgs:
-  feature-gates: "a=b"
 schedulerExtraArgs:
   feature-gates: "a=b"
   scheduler-name: "mini-scheduler"

pkg/minikube/bootstrapper/kubeadm/testdata/crio-options-gates__recent.yaml

@@ -24,8 +24,6 @@ apiServerExtraArgs:
 controllerManagerExtraArgs:
   feature-gates: "a=b"
   kube-api-burst: "32"
-kubeadmExtraArgs:
-  feature-gates: "a=b"
 schedulerExtraArgs:
   feature-gates: "a=b"
   scheduler-name: "mini-scheduler"

pkg/minikube/config/types.go

@@ -71,7 +71,6 @@ type KubernetesConfig struct {
 	NetworkPlugin     string
 	FeatureGates      string
 	ServiceCIDR       string
-	PodSubnet         string
 	ImageRepository   string
 	ExtraOptions      util.ExtraOptionSlice
 

pkg/util/extra_options.go

@@ -35,6 +35,9 @@ func (e *ExtraOption) String() string {
 // ExtraOptionSlice is a slice of ExtraOption
 type ExtraOptionSlice []ExtraOption
 
+// ComponentExtraOptionMap maps components to their extra opts, which is a map of keys to values
+type ComponentExtraOptionMap map[string]map[string]string
+
 // Set parses the string value into a slice
 func (es *ExtraOptionSlice) Set(value string) error {
 	// The component is the value before the first dot.
@@ -68,7 +71,39 @@ func (es *ExtraOptionSlice) String() string {
 	return strings.Join(s, " ")
 }
 
+// Get finds and returns the value of an argument with the specified key and component (optional) or an empty string
+// if not found. If component contains more than one value, the value for the first component found is returned. If
+// component is not specified, all of the components are used.
+func (es *ExtraOptionSlice) Get(key string, component ...string) string {
+	for _, opt := range *es {
+		if component == nil || ContainsString(component, opt.Component) {
+			if opt.Key == key {
+				return opt.Value
+			}
+		}
+	}
+	return ""
+}
+
+// AsMap converts the slice to a map of components to a map of keys and values.
+func (es *ExtraOptionSlice) AsMap() ComponentExtraOptionMap {
+	ret := ComponentExtraOptionMap{}
+	for _, opt := range *es {
+		if _, ok := ret[opt.Component]; !ok {
+			ret[opt.Component] = map[string]string{opt.Key: opt.Value}
+		} else {
+			ret[opt.Component][opt.Key] = opt.Value
+		}
+	}
+	return ret
+}
+
 // Type returns the type
 func (es *ExtraOptionSlice) Type() string {
 	return "ExtraOption"
 }
+
+// Get returns the extra option map of keys to values for the specified component
+func (cm ComponentExtraOptionMap) Get(component string) map[string]string {
+	return cm[component]
+}

pkg/util/extra_options_test.go

@@ -78,3 +78,57 @@ func TestValidFlags(t *testing.T) {
 		}
 	}
 }
+
+func TestGet(t *testing.T) {
+	extraOptions := ExtraOptionSlice{
+		ExtraOption{Component: "c1", Key: "bar", Value: "c1-bar"},
+		ExtraOption{Component: "c1", Key: "bar-baz", Value: "c1-bar-baz"},
+		ExtraOption{Component: "c2", Key: "bar", Value: "c2-bar"},
+		ExtraOption{Component: "c3", Key: "bar", Value: "c3-bar"},
+	}
+
+	for _, tc := range []struct {
+		searchKey       string
+		searchComponent []string
+		expRes          string
+		values          ExtraOptionSlice
+	}{
+		{"nonexistent", nil, "", extraOptions},
+		{"nonexistent", []string{"c1"}, "", extraOptions},
+		{"bar", []string{"c2"}, "c2-bar", extraOptions},
+		{"bar", []string{"c2", "c3"}, "c2-bar", extraOptions},
+		{"bar", nil, "c1-bar", extraOptions},
+	} {
+		if res := tc.values.Get(tc.searchKey, tc.searchComponent...); res != tc.expRes {
+			t.Errorf("Unexpected value. Expected %s, got %s", tc.expRes, res)
+		}
+	}
+}
+
+func TestAsMap(t *testing.T) {
+	extraOptions := ExtraOptionSlice{
+		ExtraOption{Component: "c1", Key: "bar", Value: "c1-bar"},
+		ExtraOption{Component: "c1", Key: "bar-baz", Value: "c1-bar-baz"},
+		ExtraOption{Component: "c2", Key: "bar", Value: "c2-bar"},
+		ExtraOption{Component: "c3", Key: "bar", Value: "c3-bar"},
+	}
+
+	expectedRes := ComponentExtraOptionMap{
+		"c1": {
+			"bar":     "c1-bar",
+			"bar-baz": "c1-bar-baz",
+		},
+		"c2": {
+			"bar": "c2-bar",
+		},
+		"c3": {
+			"bar": "c3-bar",
+		},
+	}
+
+	res := extraOptions.AsMap()
+
+	if !reflect.DeepEqual(expectedRes, res) {
+		t.Errorf("Unexpected value. Expected %s, got %s", expectedRes, res)
+	}
+}

pkg/util/utils.go

@@ -273,3 +273,14 @@ func ConcatStrings(src []string, prefix string, postfix string) []string {
 	}
 	return ret
 }
+
+// ContainsString checks if a given slice of strings contains the provided string.
+// If a modifier func is provided, it is called with the slice item before the comparation.
+func ContainsString(slice []string, s string) bool {
+	for _, item := range slice {
+		if item == s {
+			return true
+		}
+	}
+	return false
+}