Merge pull request #80675 from fabriziopandini/fix-file-discovery

Fix kubeadm file discovery

cmd/kubeadm/app/discovery/BUILD

@@ -19,6 +19,7 @@ go_library(
         "//cmd/kubeadm/app/util/kubeconfig:go_default_library",
         "//staging/src/k8s.io/client-go/tools/clientcmd/api:go_default_library",
         "//vendor/github.com/pkg/errors:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
     ],
 )
 

cmd/kubeadm/app/discovery/discovery.go

@@ -22,6 +22,7 @@ import (
 	"github.com/pkg/errors"
 
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
+	"k8s.io/klog"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiv1beta2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2"
 	"k8s.io/kubernetes/cmd/kubeadm/app/discovery/file"
@@ -43,17 +44,29 @@ func For(cfg *kubeadmapi.JoinConfiguration) (*clientcmdapi.Config, error) {
 		return nil, errors.Wrap(err, "couldn't validate the identity of the API Server")
 	}
 
-	if len(cfg.Discovery.TLSBootstrapToken) == 0 {
+	// If the users has provided a TLSBootstrapToken use it for the join process.
+	// This is usually the case of Token discovery, but it can also be used with a discovery file
+	// without embedded authentication credentials.
+	if len(cfg.Discovery.TLSBootstrapToken) != 0 {
+		klog.V(1).Info("[discovery] Using provided TLSBootstrapToken as authentication credentials for the join process")
+
+		clusterinfo := kubeconfigutil.GetClusterFromKubeConfig(config)
+		return kubeconfigutil.CreateWithToken(
+			clusterinfo.Server,
+			kubeadmapiv1beta2.DefaultClusterName,
+			TokenUser,
+			clusterinfo.CertificateAuthorityData,
+			cfg.Discovery.TLSBootstrapToken,
+		), nil
+	}
+
+	// if the config returned from discovery has authentication credentials, proceed with the TLS boostrap process
+	if kubeconfigutil.HasAuthenticationCredentials(config) {
 		return config, nil
 	}
-	clusterinfo := kubeconfigutil.GetClusterFromKubeConfig(config)
+
-	return kubeconfigutil.CreateWithToken(
+	// if there are no authentication credentials (nor in the config returned from discovery, nor in the TLSBootstrapToken), fail
-		clusterinfo.Server,
+	return nil, errors.New("couldn't find authentication credentials for the TLS boostrap process. Please use Token discovery, a discovery file with embedded authentication credentials or a discovery file without authentication credentials but with the TLSBootstrapToken flag")
-		kubeadmapiv1beta2.DefaultClusterName,
-		TokenUser,
-		clusterinfo.CertificateAuthorityData,
-		cfg.Discovery.TLSBootstrapToken,
-	), nil
 }
 
 // DiscoverValidatedKubeConfig returns a validated Config object that specifies where the cluster is and the CA cert to trust

cmd/kubeadm/app/discovery/file/BUILD

@@ -1,9 +1,6 @@
 package(default_visibility = ["//visibility:public"])
 
-load(
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
-    "@io_bazel_rules_go//go:def.bzl",
-    "go_library",
-)
 
 go_library(
     name = "go_default_library",

cmd/kubeadm/app/discovery/file/file.go

@@ -17,8 +17,6 @@ limitations under the License.
 package file
 
 import (
-	"io/ioutil"
-
 	"github.com/pkg/errors"
 
 	"k8s.io/api/core/v1"
@@ -53,58 +51,44 @@ func ValidateConfigInfo(config *clientcmdapi.Config, clustername string) (*clien
 		return nil, err
 	}
 
-	// This is the cluster object we've got from the cluster-info kubeconfig file
+	var kubeconfig *clientcmdapi.Config
-	defaultCluster := kubeconfigutil.GetClusterFromKubeConfig(config)
 
-	// Create a new kubeconfig object from the given, just copy over the server and the CA cert
+	// If the discovery file config contains authentication credentials
-	// We do this in order to not pick up other possible misconfigurations in the clusterinfo file
+	if kubeconfigutil.HasAuthenticationCredentials(config) {
-	kubeconfig := kubeconfigutil.CreateBasic(
+		klog.V(1).Info("[discovery] Using authentication credentials from the discovery file for validating TLS connection")
-		defaultCluster.Server,
-		clustername,
-		"", // no user provided
-		defaultCluster.CertificateAuthorityData,
-	)
-	// load pre-existing client certificates
-	if config.Contexts[config.CurrentContext] != nil && len(config.AuthInfos) > 0 {
-		user := config.Contexts[config.CurrentContext].AuthInfo
-		authInfo, ok := config.AuthInfos[user]
-		if !ok || authInfo == nil {
-			return nil, errors.Errorf("empty settings for user %q", user)
-		}
-		if len(authInfo.ClientCertificateData) == 0 && len(authInfo.ClientCertificate) != 0 {
-			clientCert, err := ioutil.ReadFile(authInfo.ClientCertificate)
-			if err != nil {
-				return nil, err
-			}
-			authInfo.ClientCertificateData = clientCert
-		}
-		if len(authInfo.ClientKeyData) == 0 && len(authInfo.ClientKey) != 0 {
-			clientKey, err := ioutil.ReadFile(authInfo.ClientKey)
-			if err != nil {
-				return nil, err
-			}
-			authInfo.ClientKeyData = clientKey
-		}
 
-		if len(authInfo.ClientCertificateData) == 0 || len(authInfo.ClientKeyData) == 0 {
+		// Use the discovery file config for starting the join process
-			return nil, errors.New("couldn't read authentication info from the given kubeconfig file")
+		kubeconfig = config
+
+		// We should ensure that all the authentication info is embedded in config file, so everything will work also when
+		// the kubeconfig file will be stored in /etc/kubernetes/boostrap-kubelet.conf
+		if err := kubeconfigutil.EnsureAuthenticationInfoAreEmbedded(kubeconfig); err != nil {
+			return nil, errors.Wrap(err, "error while reading client cert file or client key file")
 		}
-		kubeconfig = kubeconfigutil.CreateWithCerts(
+	} else {
-			defaultCluster.Server,
+		// If the discovery file config does not contains authentication credentials
+		klog.V(1).Info("[discovery] Discovery file does not contains authentication credentials, using unauthenticated request for validating TLS connection")
+
+		// Create a new kubeconfig object from the discovery file config, with only the server and the CA cert.
+		// NB. We do this in order to not pick up other possible misconfigurations in the clusterinfo file
+		var fileCluster = kubeconfigutil.GetClusterFromKubeConfig(config)
+		kubeconfig = kubeconfigutil.CreateBasic(
+			fileCluster.Server,
 			clustername,
 			"", // no user provided
-			defaultCluster.CertificateAuthorityData,
+			fileCluster.CertificateAuthorityData,
-			authInfo.ClientKeyData,
-			authInfo.ClientCertificateData,
 		)
 	}
 
+	// Try to read the cluster-info config map; this step was required by the original design in order
+	// to validate the TLS connection to the server early in the process
 	client, err := kubeconfigutil.ToClientSet(kubeconfig)
 	if err != nil {
 		return nil, err
 	}
 
-	klog.V(1).Infof("[discovery] Created cluster-info discovery client, requesting info from %q\n", defaultCluster.Server)
+	currentCluster := kubeconfigutil.GetClusterFromKubeConfig(kubeconfig)
+	klog.V(1).Infof("[discovery] Created cluster-info discovery client, requesting info from %q\n", currentCluster.Server)
 
 	var clusterinfoCM *v1.ConfigMap
 	wait.PollInfinite(constants.DiscoveryRetryInterval, func() (bool, error) {
@@ -113,11 +97,11 @@ func ValidateConfigInfo(config *clientcmdapi.Config, clustername string) (*clien
 		if err != nil {
 			if apierrors.IsForbidden(err) {
 				// If the request is unauthorized, the cluster admin has not granted access to the cluster info configmap for unauthenticated users
-				// In that case, trust the cluster admin and do not refresh the cluster-info credentials
+				// In that case, trust the cluster admin and do not refresh the cluster-info data
 				klog.Warningf("[discovery] Could not access the %s ConfigMap for refreshing the cluster-info information, but the TLS cert is valid so proceeding...\n", bootstrapapi.ConfigMapClusterInfo)
 				return true, nil
 			}
-			klog.V(1).Infof("[discovery] Failed to validate the API Server's identity, will try again: [%v]\n", err)
+			klog.V(1).Infof("[discovery] Error reading the %s ConfigMap, will try again: %v\n", bootstrapapi.ConfigMapClusterInfo, err)
 			return false, nil
 		}
 		return true, nil
@@ -135,9 +119,12 @@ func ValidateConfigInfo(config *clientcmdapi.Config, clustername string) (*clien
 		return kubeconfig, nil
 	}
 
-	klog.V(1).Infoln("[discovery] Synced cluster-info information from the API Server so we have got the latest information")
+	refreshedCluster := kubeconfigutil.GetClusterFromKubeConfig(refreshedBaseKubeConfig)
-	// In an HA world in the future, this will make more sense, because now we've got new information, possibly about new API Servers to talk to
+	currentCluster.Server = refreshedCluster.Server
-	return refreshedBaseKubeConfig, nil
+	currentCluster.CertificateAuthorityData = refreshedCluster.CertificateAuthorityData
+
+	klog.V(1).Infof("[discovery] Synced Server and CertificateAuthorityData from the %s ConfigMap", bootstrapapi.ConfigMapClusterInfo)
+	return kubeconfig, nil
 }
 
 // tryParseClusterInfoFromConfigMap tries to parse a kubeconfig file from a ConfigMap key

cmd/kubeadm/app/util/kubeconfig/BUILD

@@ -10,6 +10,7 @@ go_test(
     name = "go_default_test",
     srcs = ["kubeconfig_test.go"],
     embed = [":go_default_library"],
+    deps = ["//staging/src/k8s.io/client-go/tools/clientcmd/api:go_default_library"],
 )
 
 go_library(

cmd/kubeadm/app/util/kubeconfig/kubeconfig.go

@@ -18,6 +18,7 @@ package kubeconfig
 
 import (
 	"fmt"
+	"io/ioutil"
 
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
@@ -112,3 +113,73 @@ func GetClusterFromKubeConfig(config *clientcmdapi.Config) *clientcmdapi.Cluster
 	}
 	return nil
 }
+
+// HasAuthenticationCredentials returns true if the current user has valid authentication credentials for
+// token authentication, basic authentication or X509 authentication
+func HasAuthenticationCredentials(config *clientcmdapi.Config) bool {
+	authInfo := getCurrentAuthInfo(config)
+	if authInfo == nil {
+		return false
+	}
+
+	// token authentication
+	if len(authInfo.Token) != 0 {
+		return true
+	}
+
+	// basic authentication
+	if len(authInfo.Username) != 0 && len(authInfo.Password) != 0 {
+		return true
+	}
+
+	// X509 authentication
+	if (len(authInfo.ClientCertificate) != 0 || len(authInfo.ClientCertificateData) != 0) &&
+		(len(authInfo.ClientKey) != 0 || len(authInfo.ClientKeyData) != 0) {
+		return true
+	}
+
+	return false
+}
+
+// EnsureAuthenticationInfoAreEmbedded check if some authentication info are provided as external key/certificate
+// files, and eventually embeds such files into the kubeconfig file
+func EnsureAuthenticationInfoAreEmbedded(config *clientcmdapi.Config) error {
+	authInfo := getCurrentAuthInfo(config)
+	if authInfo == nil {
+		return errors.New("invalid kubeconfig file. AuthInfo is not defined for the current user")
+	}
+
+	if len(authInfo.ClientCertificateData) == 0 && len(authInfo.ClientCertificate) != 0 {
+		clientCert, err := ioutil.ReadFile(authInfo.ClientCertificate)
+		if err != nil {
+			return err
+		}
+		authInfo.ClientCertificateData = clientCert
+		authInfo.ClientCertificate = ""
+	}
+	if len(authInfo.ClientKeyData) == 0 && len(authInfo.ClientKey) != 0 {
+		clientKey, err := ioutil.ReadFile(authInfo.ClientKey)
+		if err != nil {
+			return err
+		}
+		authInfo.ClientKeyData = clientKey
+		authInfo.ClientKey = ""
+	}
+
+	return nil
+}
+
+// getCurrentAuthInfo returns current authInfo, if defined
+func getCurrentAuthInfo(config *clientcmdapi.Config) *clientcmdapi.AuthInfo {
+	if config == nil || config.CurrentContext == "" ||
+		len(config.Contexts) == 0 || config.Contexts[config.CurrentContext] == nil {
+		return nil
+	}
+	user := config.Contexts[config.CurrentContext].AuthInfo
+
+	if user == "" || len(config.AuthInfos) == 0 || config.AuthInfos[user] == nil {
+		return nil
+	}
+
+	return config.AuthInfos[user]
+}

cmd/kubeadm/app/util/kubeconfig/kubeconfig_test.go

@@ -22,6 +22,8 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
+
+	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 )
 
 const (
@@ -186,3 +188,143 @@ func TestWriteKubeconfigToDisk(t *testing.T) {
 		})
 	}
 }
+
+func TestGetCurrentAuthInfo(t *testing.T) {
+	var testCases = []struct {
+		name     string
+		config   *clientcmdapi.Config
+		expected bool
+	}{
+		{
+			name:     "nil context",
+			config:   nil,
+			expected: false,
+		},
+		{
+			name:     "no CurrentContext value",
+			config:   &clientcmdapi.Config{},
+			expected: false,
+		},
+		{
+			name:     "no CurrentContext object",
+			config:   &clientcmdapi.Config{CurrentContext: "kubernetes"},
+			expected: false,
+		},
+		{
+			name: "CurrentContext object with bad contents",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"NOTkubernetes": {}},
+			},
+			expected: false,
+		},
+		{
+			name: "no AuthInfo value",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"kubernetes": {}},
+			},
+			expected: false,
+		},
+		{
+			name: "no AuthInfo object",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"kubernetes": {AuthInfo: "kubernetes"}},
+			},
+			expected: false,
+		},
+		{
+			name: "AuthInfo object with bad contents",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"kubernetes": {AuthInfo: "kubernetes"}},
+				AuthInfos:      map[string]*clientcmdapi.AuthInfo{"NOTkubernetes": {}},
+			},
+			expected: false,
+		},
+		{
+			name: "valid AuthInfo",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"kubernetes": {AuthInfo: "kubernetes"}},
+				AuthInfos:      map[string]*clientcmdapi.AuthInfo{"kubernetes": {}},
+			},
+			expected: true,
+		},
+	}
+	for _, rt := range testCases {
+		t.Run(rt.name, func(t *testing.T) {
+			r := getCurrentAuthInfo(rt.config)
+			if rt.expected != (r != nil) {
+				t.Errorf(
+					"failed TestHasCredentials:\n\texpected: %v\n\t  actual: %v",
+					rt.expected,
+					r,
+				)
+			}
+		})
+	}
+}
+
+func TestHasCredentials(t *testing.T) {
+	var testCases = []struct {
+		name     string
+		config   *clientcmdapi.Config
+		expected bool
+	}{
+		{
+			name:     "no authInfo",
+			config:   nil,
+			expected: false,
+		},
+		{
+			name: "no credentials",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"kubernetes": {AuthInfo: "kubernetes"}},
+				AuthInfos:      map[string]*clientcmdapi.AuthInfo{"kubernetes": {}},
+			},
+			expected: false,
+		},
+		{
+			name: "token authentication credentials",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"kubernetes": {AuthInfo: "kubernetes"}},
+				AuthInfos:      map[string]*clientcmdapi.AuthInfo{"kubernetes": {Token: "123"}},
+			},
+			expected: true,
+		},
+		{
+			name: "basic authentication credentials",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"kubernetes": {AuthInfo: "kubernetes"}},
+				AuthInfos:      map[string]*clientcmdapi.AuthInfo{"kubernetes": {Username: "A", Password: "B"}},
+			},
+			expected: true,
+		},
+		{
+			name: "X509 authentication credentials",
+			config: &clientcmdapi.Config{
+				CurrentContext: "kubernetes",
+				Contexts:       map[string]*clientcmdapi.Context{"kubernetes": {AuthInfo: "kubernetes"}},
+				AuthInfos:      map[string]*clientcmdapi.AuthInfo{"kubernetes": {ClientKey: "A", ClientCertificate: "B"}},
+			},
+			expected: true,
+		},
+	}
+	for _, rt := range testCases {
+		t.Run(rt.name, func(t *testing.T) {
+			r := HasAuthenticationCredentials(rt.config)
+			if rt.expected != r {
+				t.Errorf(
+					"failed TestHasCredentials:\n\texpected: %v\n\t  actual: %v",
+					rt.expected,
+					r,
+				)
+			}
+		})
+	}
+}