Merge pull request kubernetes#68296 from liztio/fix-kubeadm-external-…

…certs

Automatic merge from submit-queue (batch tested with PRs 68087, 68256, 64621, 68299, 68296). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

Fixes using externally managed certs for kubeadm

**What this PR does / why we need it**:
The certificates overhaul caused a regression when using external certificates. This fixes that issue so external CAs no longer require a key if all certificates exist.

Walk the certificate tree, at each step checking for a CACert.
If the CACert is found, try to use it to generate certificates.
Otherwise, generate a new CA cert.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes kubernetes/kubeadm#918

**Special notes for your reviewer**:

**Release note**:

```release-note
External CAs can now be used for kubeadm with only a certificate, as long as all required certificates already exist.
```

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

@@ -62,14 +62,18 @@ func (k *KubeadmCert) CreateFromCA(ic *kubeadmapi.InitConfiguration, caCert *x50
 	if err != nil {
 		return err
 	}
-	writeCertificateFilesIfNotExist(
+	err = writeCertificateFilesIfNotExist(
 		ic.CertificatesDir,
 		k.BaseName,
 		caCert,
 		cert,
 		key,
 	)
 
+	if err != nil {
+		return fmt.Errorf("failed to write certificate %q: %v", k.Name, err)
+	}
+
 	return nil
 }
 
@@ -108,26 +112,55 @@ func (t CertificateTree) CreateTree(ic *kubeadmapi.InitConfiguration) error {
 			return err
 		}
 
-		caCert, caKey, err := NewCACertAndKey(cfg)
-		if err != nil {
-			return err
+		var caKey *rsa.PrivateKey
+
+		caCert, err := pkiutil.TryLoadCertFromDisk(ic.CertificatesDir, ca.BaseName)
+		if err == nil {
+			// Cert exists already, make sure it's valid
+			if !caCert.IsCA {
+				return fmt.Errorf("certificate %q is not a CA", ca.Name)
+			}
+			// Try and load a CA Key
+			caKey, err = pkiutil.TryLoadKeyFromDisk(ic.CertificatesDir, ca.BaseName)
+			if err != nil {
+				// If there's no CA key, make sure every certificate exists.
+				for _, leaf := range leaves {
+					cl := certKeyLocation{
+						pkiDir:   ic.CertificatesDir,
+						baseName: leaf.BaseName,
+						uxName:   leaf.Name,
+					}
+					if err := validateSignedCertWithCA(cl, caCert); err != nil {
+						return fmt.Errorf("could not load expected certificate %q or validate the existence of key %q for it: %v", leaf.Name, ca.Name, err)
+					}
+				}
+				// CACert exists and all clients exist, continue to next CA.
+				continue
+			}
+			// CA key exists; just use that to create new certificates.
+		} else {
+			// CACert doesn't already exist, create a new cert and key.
+			caCert, caKey, err = NewCACertAndKey(cfg)
+			if err != nil {
+				return err
+			}
+
+			err = writeCertificateAuthorithyFilesIfNotExist(
+				ic.CertificatesDir,
+				ca.BaseName,
+				caCert,
+				caKey,
+			)
+			if err != nil {
+				return err
+			}
 		}
 
 		for _, leaf := range leaves {
 			if err := leaf.CreateFromCA(ic, caCert, caKey); err != nil {
 				return err
 			}
 		}
-
-		err = writeCertificateAuthorithyFilesIfNotExist(
-			ic.CertificatesDir,
-			ca.BaseName,
-			caCert,
-			caKey,
-		)
-		if err != nil {
-			return err
-		}
 	}
 	return nil
 }

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

@@ -396,6 +396,11 @@ func validateSignedCert(l certKeyLocation) error {
 		return fmt.Errorf("failure loading certificate authority for %s: %v", l.uxName, err)
 	}
 
+	return validateSignedCertWithCA(l, caCert)
+}
+
+// validateSignedCertWithCA tries to load a certificate and validate it with the given caCert
+func validateSignedCertWithCA(l certKeyLocation, caCert *x509.Certificate) error {
 	// Try to load key and signed certificate
 	signedCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(l.pkiDir, l.baseName)
 	if err != nil {

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

@@ -374,6 +374,89 @@ func TestSharedCertificateExists(t *testing.T) {
 	}
 }
 
+func TestCreatePKIAssetsWithSparseCerts(t *testing.T) {
+	caCert, caKey := createCACert(t)
+	fpCACert, fpCAKey := createCACert(t)
+	etcdCACert, etcdCAKey := createCACert(t)
+
+	fpCert, fpKey := createTestCert(t, fpCACert, fpCAKey)
+
+	tests := []struct {
+		name        string
+		files       pkiFiles
+		expectError bool
+	}{
+		{
+			name: "nothing present",
+		},
+		{
+			name: "CAs already exist",
+			files: pkiFiles{
+				"ca.crt":             caCert,
+				"ca.key":             caKey,
+				"front-proxy-ca.crt": fpCACert,
+				"front-proxy-ca.key": fpCAKey,
+				"etcd/ca.crt":        etcdCACert,
+				"etcd/ca.key":        etcdCAKey,
+			},
+		},
+		{
+			name: "CA certs only",
+			files: pkiFiles{
+				"ca.crt":             caCert,
+				"front-proxy-ca.crt": fpCACert,
+				"etcd/ca.crt":        etcdCACert,
+			},
+			expectError: true,
+		},
+		{
+			name: "FrontProxyCA with certs",
+			files: pkiFiles{
+				"ca.crt":                 caCert,
+				"ca.key":                 caKey,
+				"front-proxy-ca.crt":     fpCACert,
+				"front-proxy-client.crt": fpCert,
+				"front-proxy-client.key": fpKey,
+				"etcd/ca.crt":            etcdCACert,
+				"etcd/ca.key":            etcdCAKey,
+			},
+		},
+		{
+			name: "FrontProxy certs missing CA",
+			files: pkiFiles{
+				"front-proxy-client.crt": fpCert,
+				"front-proxy-client.key": fpKey,
+			},
+			expectError: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			tmpdir := testutil.SetupTempDir(t)
+			defer os.RemoveAll(tmpdir)
+
+			cfg := testutil.GetDefaultInternalConfig(t)
+			cfg.ClusterConfiguration.CertificatesDir = tmpdir
+
+			writePKIFiles(t, tmpdir, test.files)
+
+			err := CreatePKIAssets(cfg)
+			if err != nil {
+				if test.expectError {
+					return
+				}
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			if test.expectError {
+				t.Fatal("Expected error from CreatePKIAssets, got none")
+			}
+			assertCertsExist(t, tmpdir)
+		})
+	}
+
+}
+
 func TestUsingExternalCA(t *testing.T) {
 	tests := []struct {
 		setupFuncs []func(cfg *kubeadmapi.InitConfiguration) error
@@ -610,3 +693,24 @@ func deleteFrontProxyCAKey(cfg *kubeadmapi.InitConfiguration) error {
 	}
 	return nil
 }
+
+func assertCertsExist(t *testing.T, dir string) {
+	tree, err := GetDefaultCertList().AsMap().CertTree()
+	if err != nil {
+		t.Fatalf("unexpected error getting certificates: %v", err)
+	}
+
+	for caCert, certs := range tree {
+		if err := validateCACert(certKeyLocation{dir, caCert.BaseName, "", caCert.Name}); err != nil {
+			t.Errorf("couldn't validate CA certificate %v: %v", caCert.Name, err)
+			// Don't bother validating child certs, but do try the other CAs
+			continue
+		}
+
+		for _, cert := range certs {
+			if err := validateSignedCert(certKeyLocation{dir, caCert.BaseName, cert.BaseName, cert.Name}); err != nil {
+				t.Errorf("couldn't validate certificate %v: %v", cert.Name, err)
+			}
+		}
+	}
+}

cmd/kubeadm/test/BUILD

@@ -11,8 +11,10 @@ go_library(
     importpath = "k8s.io/kubernetes/cmd/kubeadm/test",
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
+        "//cmd/kubeadm/app/apis/kubeadm/v1alpha3:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/app/phases/certs/pkiutil:go_default_library",
+        "//cmd/kubeadm/app/util/config:go_default_library",
         "//cmd/kubeadm/test/certs:go_default_library",
         "//vendor/github.com/renstrom/dedent:go_default_library",
     ],

cmd/kubeadm/test/util.go

@@ -26,8 +26,10 @@ import (
 	"github.com/renstrom/dedent"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmapiv1alpha3 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha3"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil"
+	configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
 	certtestutil "k8s.io/kubernetes/cmd/kubeadm/test/certs"
 )
 
@@ -140,3 +142,13 @@ func AssertFileExists(t *testing.T, dirName string, fileNames ...string) {
 		}
 	}
 }
+
+// GetDefaultInternalConfig returns a defaulted kubeadmapi.InitConfiguration
+func GetDefaultInternalConfig(t *testing.T) *kubeadmapi.InitConfiguration {
+	internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig("", &kubeadmapiv1alpha3.InitConfiguration{})
+	if err != nil {
+		t.Fatalf("unexpected error getting default config: %v", err)
+	}
+
+	return internalcfg
+}