Merge pull request kubernetes#30798 from smarterclayton/fix_kubeconfig

Automatic merge from submit-queue Allow a flag that forces kubelet to have a valid kubeconfig `--require-kubeconfig` forces the kubelet to use the kubeconfig for all APIserver communication, and exit cleanly. Allows cluster lifecycle to loop waiting for config to be available. Fixes kubernetes#30515 A follow up PR will handle the issue discovered where the DefaultCluster rules applied to kubeconfig allow a malicious party who can bind to localhost:8080 to take advantage of an admin misconfiguration. @lukemarsden @mikedanese ```release-note The Kubelet now supports the `--force-kubeconfig` option which reads all client config from the provided `--kubeconfig` file and will cause the Kubelet to exit with error code 1 on error. It also forces the Kubelet to use the server URL from the kubeconfig file rather than the `--api-servers` flag. Without this flag set, a failure to read the kubeconfig file would only result in a warning message. In a future release, the value of this flag will be defaulted to `true`. ```
lavalamp · Aug 18, 2016 · ec4d645 · ec4d645
2 parents 6fb10dd + a66828d
commit ec4d645
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 34 deletions.
diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go
@@ -41,9 +41,11 @@ const (
 type KubeletServer struct {
 	componentconfig.KubeletConfiguration
 
-	AuthPath      util.StringFlag // Deprecated -- use KubeConfig instead
-	KubeConfig    util.StringFlag
-	APIServerList []string
+	KubeConfig util.StringFlag
+	// If true, an invalid KubeConfig will result in the Kubelet exiting with an error.
+	RequireKubeConfig bool
+	AuthPath          util.StringFlag // Deprecated -- use KubeConfig instead
+	APIServerList     []string        // Deprecated -- use KubeConfig instead
 
 	RunOnce bool
 
@@ -60,12 +62,22 @@ func NewKubeletServer() *KubeletServer {
 	return &KubeletServer{
 		AuthPath:             util.NewStringFlag("/var/lib/kubelet/kubernetes_auth"), // deprecated
 		KubeConfig:           util.NewStringFlag("/var/lib/kubelet/kubeconfig"),
+		RequireKubeConfig:    false, // in 1.5, default to true
 		KubeletConfiguration: config,
 	}
 }
 
 // AddFlags adds flags for a specific KubeletServer to the specified FlagSet
 func (s *KubeletServer) AddFlags(fs *pflag.FlagSet) {
+	fs.Var(&s.KubeConfig, "kubeconfig", "Path to a kubeconfig file, specifying how to connect to the API server. --api-servers will be used for the location unless --require-kubeconfig is set.")
+	fs.BoolVar(&s.RequireKubeConfig, "require-kubeconfig", s.RequireKubeConfig, "If true the Kubelet will exit if there are configuration errors, and will ignore the value of --api-servers in favor of the server defined in the kubeconfig file.")
+
+	// DEPRECATED: Remove these flags at the beginning of 1.5.
+	fs.Var(&s.AuthPath, "auth-path", "Path to .kubernetes_auth file, specifying how to authenticate to API server.")
+	fs.MarkDeprecated("auth-path", "will be removed in a future version")
+	fs.StringSliceVar(&s.APIServerList, "api-servers", []string{}, "List of Kubernetes API servers for publishing events, and reading pods and services. (ip:port), comma separated.")
+	fs.MarkDeprecated("api-servers", "Use --kubeconfig instead. Will be removed in a future version.")
+
 	fs.StringVar(&s.PodManifestPath, "config", s.PodManifestPath, "Path to to the directory containing pod manifest files to run, or the path to a single pod manifest file.")
 	fs.MarkDeprecated("config", "Use --pod-manifest-path instead. Will be removed in a future version.")
 	fs.StringVar(&s.PodManifestPath, "pod-manifest-path", s.PodManifestPath, "Path to to the directory containing pod manifest files to run, or the path to a single pod manifest file.")
@@ -106,14 +118,10 @@ func (s *KubeletServer) AddFlags(fs *pflag.FlagSet) {
 	fs.MarkDeprecated("maximum-dead-containers-per-container", "Use --eviction-hard or --eviction-soft instead. Will be removed in a future version.")
 	fs.Int32Var(&s.MaxContainerCount, "maximum-dead-containers", s.MaxContainerCount, "Maximum number of old instances of containers to retain globally.  Each container takes up some disk space.  Default: 100.")
 	fs.MarkDeprecated("maximum-dead-containers", "Use --eviction-hard or --eviction-soft instead. Will be removed in a future version.")
-	fs.Var(&s.AuthPath, "auth-path", "Path to .kubernetes_auth file, specifying how to authenticate to API server.")
-	fs.MarkDeprecated("auth-path", "will be removed in a future version")
-	fs.Var(&s.KubeConfig, "kubeconfig", "Path to a kubeconfig file, specifying how to authenticate to API server (the master location is set by the api-servers flag).")
 	fs.Int32Var(&s.CAdvisorPort, "cadvisor-port", s.CAdvisorPort, "The port of the localhost cAdvisor endpoint")
 	fs.Int32Var(&s.HealthzPort, "healthz-port", s.HealthzPort, "The port of the localhost healthz endpoint")
 	fs.Var(componentconfig.IPVar{Val: &s.HealthzBindAddress}, "healthz-bind-address", "The IP address for the healthz server to serve on, defaulting to 127.0.0.1 (set to 0.0.0.0 for all interfaces)")
 	fs.Int32Var(&s.OOMScoreAdj, "oom-score-adj", s.OOMScoreAdj, "The oom-score-adj value for kubelet process. Values must be within the range [-1000, 1000]")
-	fs.StringSliceVar(&s.APIServerList, "api-servers", []string{}, "List of Kubernetes API servers for publishing events, and reading pods and services. (ip:port), comma separated.")
 	fs.BoolVar(&s.RegisterNode, "register-node", s.RegisterNode, "Register the node with the apiserver (defaults to true if --api-servers is set)")
 	fs.StringVar(&s.ClusterDomain, "cluster-domain", s.ClusterDomain, "Domain for this cluster.  If set, kubelet will configure all containers to search this domain in addition to the host's search domains")
 	fs.StringVar(&s.MasterServiceNamespace, "master-service-namespace", s.MasterServiceNamespace, "The namespace from which the kubernetes master services should be injected into pods")

diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go
@@ -294,11 +294,10 @@ func UnsecuredKubeletConfig(s *options.KubeletServer) (*KubeletConfig, error) {
 // Otherwise, the caller is assumed to have set up the KubeletConfig object and all defaults
 // will be ignored.
 func Run(s *options.KubeletServer, kcfg *KubeletConfig) error {
-	err := run(s, kcfg)
-	if err != nil {
-		glog.Errorf("Failed running kubelet: %v", err)
+	if err := run(s, kcfg); err != nil {
+		return fmt.Errorf("failed to run Kubelet: %v", err)
 	}
-	return err
+	return nil
 }
 
 func checkPermissions() error {
@@ -314,9 +313,6 @@ func run(s *options.KubeletServer, kcfg *KubeletConfig) (err error) {
 	if s.ExitOnLockContention && s.LockFilePath == "" {
 		return errors.New("cannot exit on lock file contention: no lock file specified")
 	}
-	if err := checkPermissions(); err != nil {
-		glog.Error(err)
-	}
 
 	done := make(chan struct{})
 	if s.LockFilePath != "" {
@@ -338,25 +334,34 @@ func run(s *options.KubeletServer, kcfg *KubeletConfig) (err error) {
 	}
 
 	if kcfg == nil {
-		cfg, err := UnsecuredKubeletConfig(s)
-		if err != nil {
-			return err
-		}
-		kcfg = cfg
-
+		var kubeClient, eventClient *clientset.Clientset
 		clientConfig, err := CreateAPIServerClientConfig(s)
 		if err == nil {
-			kcfg.KubeClient, err = clientset.NewForConfig(clientConfig)
+			kubeClient, err = clientset.NewForConfig(clientConfig)
 
 			// make a separate client for events
 			eventClientConfig := *clientConfig
 			eventClientConfig.QPS = float32(s.EventRecordQPS)
 			eventClientConfig.Burst = int(s.EventBurst)
-			kcfg.EventClient, err = clientset.NewForConfig(&eventClientConfig)
+			eventClient, err = clientset.NewForConfig(&eventClientConfig)
 		}
-		if err != nil && len(s.APIServerList) > 0 {
-			glog.Warningf("No API client: %v", err)
+		if err != nil {
+			if s.RequireKubeConfig {
+				return fmt.Errorf("invalid kubeconfig: %v", err)
+			}
+			// TODO: this should be replaced by a --standalone flag
+			if len(s.APIServerList) > 0 {
+				glog.Warningf("No API client: %v", err)
+			}
+		}
+
+		cfg, err := UnsecuredKubeletConfig(s)
+		if err != nil {
+			return err
 		}
+		kcfg = cfg
+		kcfg.KubeClient = kubeClient
+		kcfg.EventClient = eventClient
 
 		if s.CloudProvider == kubeExternal.AutoDetectCloudProvider {
 			kcfg.AutoDetectCloudProvider = true
@@ -399,6 +404,10 @@ func run(s *options.KubeletServer, kcfg *KubeletConfig) (err error) {
 		}
 	}
 
+	if err := checkPermissions(); err != nil {
+		glog.Error(err)
+	}
+
 	runtime.ReallyCrash = s.ReallyCrashForTesting
 	rand.Seed(time.Now().UTC().UnixNano())
 
@@ -481,9 +490,17 @@ func authPathClientConfig(s *options.KubeletServer, useDefaults bool) (*restclie
 }
 
 func kubeconfigClientConfig(s *options.KubeletServer) (*restclient.Config, error) {
+	if s.RequireKubeConfig {
+		// Ignores the values of s.APIServerList
+		return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
+			&clientcmd.ClientConfigLoadingRules{ExplicitPath: s.KubeConfig.Value()},
+			&clientcmd.ConfigOverrides{},
+		).ClientConfig()
+	}
 	return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
 		&clientcmd.ClientConfigLoadingRules{ExplicitPath: s.KubeConfig.Value()},
-		&clientcmd.ConfigOverrides{ClusterInfo: clientcmdapi.Cluster{Server: s.APIServerList[0]}}).ClientConfig()
+		&clientcmd.ConfigOverrides{ClusterInfo: clientcmdapi.Cluster{Server: s.APIServerList[0]}},
+	).ClientConfig()
 }
 
 // createClientConfig creates a client configuration from the command line
@@ -493,6 +510,20 @@ func kubeconfigClientConfig(s *options.KubeletServer) (*restclient.Config, error
 // fall back to the default auth (none) without an error.
 // TODO(roberthbailey): Remove support for --auth-path
 func createClientConfig(s *options.KubeletServer) (*restclient.Config, error) {
+	if s.RequireKubeConfig {
+		return kubeconfigClientConfig(s)
+	}
+
+	// TODO: handle a new --standalone flag that bypasses kubeconfig loading and returns no error.
+	// DEPRECATED: all subsequent code is deprecated
+	if len(s.APIServerList) == 0 {
+		return nil, fmt.Errorf("no api servers specified")
+	}
+	// TODO: adapt Kube client to support LB over several servers
+	if len(s.APIServerList) > 1 {
+		glog.Infof("Multiple api servers specified.  Picking first one")
+	}
+
 	if s.KubeConfig.Provided() && s.AuthPath.Provided() {
 		return nil, fmt.Errorf("cannot specify both --kubeconfig and --auth-path")
 	}
@@ -516,14 +547,6 @@ func createClientConfig(s *options.KubeletServer) (*restclient.Config, error) {
 // the configuration via addChaosToClientConfig. This func is exported to support
 // integration with third party kubelet extensions (e.g. kubernetes-mesos).
 func CreateAPIServerClientConfig(s *options.KubeletServer) (*restclient.Config, error) {
-	if len(s.APIServerList) < 1 {
-		return nil, fmt.Errorf("no api servers specified")
-	}
-	// TODO: adapt Kube client to support LB over several servers
-	if len(s.APIServerList) > 1 {
-		glog.Infof("Multiple api servers specified.  Picking first one")
-	}
-
 	clientConfig, err := createClientConfig(s)
 	if err != nil {
 		return nil, err

diff --git a/cmd/kubelet/kubelet.go b/cmd/kubelet/kubelet.go
@@ -44,7 +44,7 @@ func main() {
 	verflag.PrintAndExitIfRequested()
 
 	if err := app.Run(s, nil); err != nil {
-		fmt.Fprintf(os.Stderr, "%v\n", err)
+		fmt.Fprintf(os.Stderr, "error: %v\n", err)
 		os.Exit(1)
 	}
 }
diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt
@@ -410,6 +410,7 @@ repo-root
 report-dir
 report-prefix
 required-contexts
+require-kubeconfig
 resolv-conf
 resource-container
 resource-quota-sync-period