Merge pull request #75431 from fabriziopandini/fix-external-ca

fix kubeadm external CA Mode

cmd/kubeadm/app/cmd/init.go

@@ -332,9 +332,17 @@ func newInitData(cmd *cobra.Command, args []string, options *initOptions, out io
 		}
 	}
 
-	// Checks if an external CA is provided by the user.
+	// Checks if an external CA is provided by the user (when the CA Cert is present but the CA Key is not)
-	externalCA, _ := certsphase.UsingExternalCA(&cfg.ClusterConfiguration)
+	externalCA, err := certsphase.UsingExternalCA(&cfg.ClusterConfiguration)
 	if externalCA {
+		// In case the certificates signed by CA (that should be provided by the user) are missing or invalid,
+		// returns, because kubeadm can't regenerate them without the CA Key
+		if err != nil {
+			return nil, errors.Wrapf(err, "invalid or incomplete external CA")
+		}
+
+		// Validate that also the required kubeconfig files exists and are invalid, because
+		// kubeadm can't regenerate them without the CA Key
 		kubeconfigDir := options.kubeconfigDir
 		if options.dryRun {
 			kubeconfigDir = dryRunDir
@@ -342,11 +350,22 @@ func newInitData(cmd *cobra.Command, args []string, options *initOptions, out io
 		if err := kubeconfigphase.ValidateKubeconfigsForExternalCA(kubeconfigDir, cfg); err != nil {
 			return nil, err
 		}
-		if options.uploadCerts {
+	}
-			return nil, errors.New("can't use externalCA mode and upload-certs")
+
+	// Checks if an external Front-Proxy CA is provided by the user (when the Front-Proxy CA Cert is present but the Front-Proxy CA Key is not)
+	externalFrontProxyCA, err := certsphase.UsingExternalFrontProxyCA(&cfg.ClusterConfiguration)
+	if externalFrontProxyCA {
+		// In case the certificates signed by Front-Proxy CA (that should be provided by the user) are missing or invalid,
+		// returns, because kubeadm can't regenerate them without the Front-Proxy CA Key
+		if err != nil {
+			return nil, errors.Wrapf(err, "invalid or incomplete external front-proxy CA")
 		}
 	}
 
+	if options.uploadCerts && (externalCA || externalFrontProxyCA) {
+		return nil, errors.New("can't use upload-certs with an external CA or an external front-proxy CA")
+	}
+
 	return &initData{
 		cfg:                     cfg,
 		certificatesDir:         cfg.CertificatesDir,

cmd/kubeadm/app/cmd/phases/init/certs.go

@@ -190,7 +190,7 @@ func runCertsSa(c workflow.RunData) error {
 
 	// if external CA mode, skip service account key generation
 	if data.ExternalCA() {
-		fmt.Printf("[certs] External CA mode: Using existing sa keys\n")
+		fmt.Printf("[certs] Using existing sa keys\n")
 		return nil
 	}
 
@@ -220,7 +220,7 @@ func runCAPhase(ca *certsphase.KubeadmCert) func(c workflow.RunData) error {
 				fmt.Printf("[certs] Using existing %s certificate authority\n", ca.BaseName)
 				return nil
 			}
-			fmt.Printf("[certs] Using existing %s keyless certificate authority", ca.BaseName)
+			fmt.Printf("[certs] Using existing %s keyless certificate authority\n", ca.BaseName)
 			return nil
 		}
 

cmd/kubeadm/app/phases/certs/certs.go

@@ -354,8 +354,9 @@ func SharedCertificateExists(cfg *kubeadmapi.ClusterConfiguration) (bool, error)
 }
 
 // UsingExternalCA determines whether the user is relying on an external CA.  We currently implicitly determine this is the case
-// when both the CA Cert and the front proxy CA Cert are present but the CA Key and front proxy CA Key are not.
+// when the CA Cert is present but the CA Key is not.
 // This allows us to, e.g., skip generating certs or not start the csr signing controller.
+// In case we are using an external front-proxy CA, the function validates the certificates signed by front-proxy CA that should be provided by the user.
 func UsingExternalCA(cfg *kubeadmapi.ClusterConfiguration) (bool, error) {
 
 	if err := validateCACert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName, "", "CA"}); err != nil {
@@ -364,20 +365,24 @@ func UsingExternalCA(cfg *kubeadmapi.ClusterConfiguration) (bool, error) {
 
 	caKeyPath := filepath.Join(cfg.CertificatesDir, kubeadmconstants.CAKeyName)
 	if _, err := os.Stat(caKeyPath); !os.IsNotExist(err) {
-		return false, errors.Errorf("%s exists", kubeadmconstants.CAKeyName)
+		return false, nil
 	}
 
 	if err := validateSignedCert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName, kubeadmconstants.APIServerCertAndKeyBaseName, "API server"}); err != nil {
-		return false, err
+		return true, err
 	}
 
 	if err := validateSignedCert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName, kubeadmconstants.APIServerKubeletClientCertAndKeyBaseName, "API server kubelet client"}); err != nil {
-		return false, err
+		return true, err
 	}
 
-	if err := validatePrivatePublicKey(certKeyLocation{cfg.CertificatesDir, "", kubeadmconstants.ServiceAccountKeyBaseName, "service account"}); err != nil {
+	return true, nil
-		return false, err
+}
-	}
+
+// UsingExternalFrontProxyCA determines whether the user is relying on an external front-proxy CA.  We currently implicitly determine this is the case
+// when the front proxy CA Cert is present but the front proxy CA Key is not.
+// In case we are using an external front-proxy CA, the function validates the certificates signed by front-proxy CA that should be provided by the user.
+func UsingExternalFrontProxyCA(cfg *kubeadmapi.ClusterConfiguration) (bool, error) {
 
 	if err := validateCACert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.FrontProxyCACertAndKeyBaseName, "", "front-proxy CA"}); err != nil {
 		return false, err
@@ -385,11 +390,11 @@ func UsingExternalCA(cfg *kubeadmapi.ClusterConfiguration) (bool, error) {
 
 	frontProxyCAKeyPath := filepath.Join(cfg.CertificatesDir, kubeadmconstants.FrontProxyCAKeyName)
 	if _, err := os.Stat(frontProxyCAKeyPath); !os.IsNotExist(err) {
-		return false, errors.Errorf("%s exists", kubeadmconstants.FrontProxyCAKeyName)
+		return false, nil
 	}
 
 	if err := validateSignedCert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.FrontProxyCACertAndKeyBaseName, kubeadmconstants.FrontProxyClientCertAndKeyBaseName, "front-proxy client"}); err != nil {
-		return false, err
+		return true, err
 	}
 
 	return true, nil

cmd/kubeadm/app/phases/certs/certs_test.go

@@ -514,47 +514,108 @@ func TestCreatePKIAssetsWithSparseCerts(t *testing.T) {
 
 func TestUsingExternalCA(t *testing.T) {
 	tests := []struct {
-		setupFuncs []func(cfg *kubeadmapi.InitConfiguration) error
+		name           string
-		expected   bool
+		setupFuncs     []func(cfg *kubeadmapi.InitConfiguration) error
+		externalCAFunc func(*kubeadmapi.ClusterConfiguration) (bool, error)
+		expected       bool
+		expectedErr    bool
 	}{
 		{
+			name: "Test External CA, when complete PKI exists",
 			setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{
 				CreatePKIAssets,
 			},
-			expected: false,
+			externalCAFunc: UsingExternalCA,
+			expected:       false,
 		},
 		{
+			name: "Test External CA, when ca.key missing",
 			setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{
 				CreatePKIAssets,
-				deleteCAKey,
+				deleteCertOrKey(kubeadmconstants.CAKeyName),
-				deleteFrontProxyCAKey,
 			},
-			expected: true,
+			externalCAFunc: UsingExternalCA,
+			expected:       true,
+		},
+		{
+			name: "Test External CA, when ca.key missing and signed certs are missing",
+			setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{
+				CreatePKIAssets,
+				deleteCertOrKey(kubeadmconstants.CAKeyName),
+				deleteCertOrKey(kubeadmconstants.APIServerCertName),
+			},
+			externalCAFunc: UsingExternalCA,
+			expected:       true,
+			expectedErr:    true,
+		},
+		{
+			name: "Test External CA, when ca.key missing",
+			setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{
+				CreatePKIAssets,
+				deleteCertOrKey(kubeadmconstants.CAKeyName),
+			},
+			externalCAFunc: UsingExternalCA,
+			expected:       true,
+		},
+		{
+			name: "Test External Front Proxy CA, when complete PKI exists",
+			setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{
+				CreatePKIAssets,
+			},
+			externalCAFunc: UsingExternalFrontProxyCA,
+			expected:       false,
+		},
+		{
+			name: "Test External Front Proxy CA, when front-proxy-ca.key missing",
+			setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{
+				CreatePKIAssets,
+				deleteCertOrKey(kubeadmconstants.FrontProxyCAKeyName),
+			},
+			externalCAFunc: UsingExternalFrontProxyCA,
+			expected:       true,
+		},
+		{
+			name: "Test External Front Proxy CA, when front-proxy-.key missing and signed certs are missing",
+			setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{
+				CreatePKIAssets,
+				deleteCertOrKey(kubeadmconstants.FrontProxyCAKeyName),
+				deleteCertOrKey(kubeadmconstants.FrontProxyClientCertName),
+			},
+			externalCAFunc: UsingExternalFrontProxyCA,
+			expected:       true,
+			expectedErr:    true,
 		},
 	}
 
 	for _, test := range tests {
-		dir := testutil.SetupTempDir(t)
+		t.Run(test.name, func(t *testing.T) {
-		defer os.RemoveAll(dir)
+			dir := testutil.SetupTempDir(t)
+			defer os.RemoveAll(dir)
 
-		cfg := &kubeadmapi.InitConfiguration{
+			cfg := &kubeadmapi.InitConfiguration{
-			LocalAPIEndpoint: kubeadmapi.APIEndpoint{AdvertiseAddress: "1.2.3.4"},
+				LocalAPIEndpoint: kubeadmapi.APIEndpoint{AdvertiseAddress: "1.2.3.4"},
-			ClusterConfiguration: kubeadmapi.ClusterConfiguration{
+				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
-				Networking:      kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"},
+					Networking:      kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"},
-				CertificatesDir: dir,
+					CertificatesDir: dir,
-			},
+				},
-			NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: "valid-hostname"},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: "valid-hostname"},
-		}
-
-		for _, f := range test.setupFuncs {
-			if err := f(cfg); err != nil {
-				t.Errorf("error executing setup function: %v", err)
 			}
-		}
 
-		if val, _ := UsingExternalCA(&cfg.ClusterConfiguration); val != test.expected {
+			for _, f := range test.setupFuncs {
-			t.Errorf("UsingExternalCA did not match expected: %v", test.expected)
+				if err := f(cfg); err != nil {
-		}
+					t.Errorf("error executing setup function: %v", err)
+				}
+			}
+
+			val, err := test.externalCAFunc(&cfg.ClusterConfiguration)
+			if val != test.expected {
+				t.Errorf("UsingExternalCA did not match expected: %v", test.expected)
+			}
+
+			if (err != nil) != test.expectedErr {
+				t.Errorf("UsingExternalCA returned un expected err: %v", err)
+			}
+		})
 	}
 }
 
@@ -742,18 +803,13 @@ func TestCreateCertificateFilesMethods(t *testing.T) {
 	}
 }
 
-func deleteCAKey(cfg *kubeadmapi.InitConfiguration) error {
+func deleteCertOrKey(name string) func(*kubeadmapi.InitConfiguration) error {
-	if err := os.Remove(filepath.Join(cfg.CertificatesDir, kubeadmconstants.CAKeyName)); err != nil {
+	return func(cfg *kubeadmapi.InitConfiguration) error {
-		return errors.Wrapf(err, "failed removing %s", kubeadmconstants.CAKeyName)
+		if err := os.Remove(filepath.Join(cfg.CertificatesDir, name)); err != nil {
+			return errors.Wrapf(err, "failed removing %s", name)
+		}
+		return nil
 	}
-	return nil
-}
-
-func deleteFrontProxyCAKey(cfg *kubeadmapi.InitConfiguration) error {
-	if err := os.Remove(filepath.Join(cfg.CertificatesDir, kubeadmconstants.FrontProxyCAKeyName)); err != nil {
-		return errors.Wrapf(err, "failed removing %s", kubeadmconstants.FrontProxyCAKeyName)
-	}
-	return nil
 }
 
 func assertCertsExist(t *testing.T, dir string) {

cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go

@@ -362,24 +362,24 @@ func ValidateKubeconfigsForExternalCA(outDir string, cfg *kubeadmapi.InitConfigu
 		kubeadmconstants.SchedulerKubeConfigFileName,
 	}
 
-	specs, err := getKubeConfigSpecs(cfg)
+	// Creates a kubeconfig file with the target CA and server URL
+	// to be used as a input for validating user provided kubeconfig files
+	caCert, err := pkiutil.TryLoadCertFromDisk(cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName)
+	if err != nil {
+		return errors.Wrapf(err, "the CA file couldn't be loaded")
+	}
+
+	controlPlaneEndpoint, err := kubeadmutil.GetControlPlaneEndpoint(cfg.ControlPlaneEndpoint, &cfg.LocalAPIEndpoint)
 	if err != nil {
 		return err
 	}
 
+	validationConfig := kubeconfigutil.CreateBasic(controlPlaneEndpoint, "dummy", "dummy", pkiutil.EncodeCertPEM(caCert))
+
+	// validate user provided kubeconfig files
 	for _, kubeConfigFileName := range kubeConfigFileNames {
-		spec, exists := specs[kubeConfigFileName]
+		if err = validateKubeConfig(outDir, kubeConfigFileName, validationConfig); err != nil {
-		if !exists {
+			return errors.Wrapf(err, "the %s file does not exists or it is not valid", kubeConfigFileName)
-			return errors.Errorf("couldn't retrive KubeConfigSpec for %s", kubeConfigFileName)
-		}
-
-		kubeconfig, err := buildKubeConfigFromSpec(spec, cfg.ClusterName)
-		if err != nil {
-			return err
-		}
-
-		if err = validateKubeConfig(outDir, kubeConfigFileName, kubeconfig); err != nil {
-			return err
 		}
 	}
 	return nil

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

@@ -512,14 +512,23 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) {
 		},
 	}
 
+	// creates CA, write to pkiDir and remove ca.key to get into external CA condition
 	caCert, caKey := certstestutil.SetupCertificateAuthorithy(t)
-	anotherCaCert, anotherCaKey := certstestutil.SetupCertificateAuthorithy(t)
 	if err := pkiutil.WriteCertAndKey(pkiDir, kubeadmconstants.CACertAndKeyBaseName, caCert, caKey); err != nil {
 		t.Fatalf("failure while saving CA certificate and key: %v", err)
 	}
+	if err := os.Remove(filepath.Join(pkiDir, kubeadmconstants.CAKeyName)); err != nil {
+		t.Fatalf("failure while deleting ca.key: %v", err)
+	}
 
+	// create a valid config
 	config := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://1.2.3.4:1234", "test-cluster", "myOrg1")
+
+	// create a config with another CA
+	anotherCaCert, anotherCaKey := certstestutil.SetupCertificateAuthorithy(t)
 	configWithAnotherClusterCa := setupdKubeConfigWithClientAuth(t, anotherCaCert, anotherCaKey, "https://1.2.3.4:1234", "test-cluster", "myOrg1")
+
+	// create a config with another server URL
 	configWithAnotherServerURL := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://4.3.2.1:4321", "test-cluster", "myOrg1")
 
 	tests := map[string]struct {
@@ -539,11 +548,21 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) {
 			initConfig:    initConfig,
 			expectedError: true,
 		},
-		"some files are invalid": {
+		"some files have invalid CA": {
 			filesToWrite: map[string]*clientcmdapi.Config{
 				kubeadmconstants.AdminKubeConfigFileName:             config,
 				kubeadmconstants.KubeletKubeConfigFileName:           config,
 				kubeadmconstants.ControllerManagerKubeConfigFileName: configWithAnotherClusterCa,
+				kubeadmconstants.SchedulerKubeConfigFileName:         config,
+			},
+			initConfig:    initConfig,
+			expectedError: true,
+		},
+		"some files have invalid Server Url": {
+			filesToWrite: map[string]*clientcmdapi.Config{
+				kubeadmconstants.AdminKubeConfigFileName:             config,
+				kubeadmconstants.KubeletKubeConfigFileName:           config,
+				kubeadmconstants.ControllerManagerKubeConfigFileName: config,
 				kubeadmconstants.SchedulerKubeConfigFileName:         configWithAnotherServerURL,
 			},
 			initConfig:    initConfig,
@@ -567,7 +586,7 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) {
 
 		for name, config := range test.filesToWrite {
 			if err := createKubeConfigFileIfNotExists(tmpdir, name, config); err != nil {
-				t.Errorf("createKubeConfigFileIfNotExists failed")
+				t.Errorf("createKubeConfigFileIfNotExists failed: %v", err)
 			}
 		}