Merge pull request #69536 from awly/robust-cert-loading

staging/src/k8s.io/client-go/util/certificate/BUILD

@@ -24,7 +24,6 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/typed/certificates/v1beta1:go_default_library",
-        "//staging/src/k8s.io/client-go/util/cert:go_default_library",
     ],
 )
 

staging/src/k8s.io/client-go/util/certificate/certificate_store.go

@@ -21,7 +21,6 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -171,11 +170,9 @@ func (s *fileStore) Current() (*tls.Certificate, error) {
 }
 
 func loadFile(pairFile string) (*tls.Certificate, error) {
-	certBlock, keyBlock, err := loadCertKeyBlocks(pairFile)
-	if err != nil {
-		return nil, err
-	}
-	cert, err := tls.X509KeyPair(pem.EncodeToMemory(certBlock), pem.EncodeToMemory(keyBlock))
+	// LoadX509KeyPair knows how to parse combined cert and private key from
+	// the same file.
+	cert, err := tls.LoadX509KeyPair(pairFile, pairFile)
 	if err != nil {
 		return nil, fmt.Errorf("could not convert data from %q into cert/key pair: %v", pairFile, err)
 	}
@@ -187,22 +184,6 @@ func loadFile(pairFile string) (*tls.Certificate, error) {
 	return &cert, nil
 }
 
-func loadCertKeyBlocks(pairFile string) (cert *pem.Block, key *pem.Block, err error) {
-	data, err := ioutil.ReadFile(pairFile)
-	if err != nil {
-		return nil, nil, fmt.Errorf("could not load cert/key pair from %q: %v", pairFile, err)
-	}
-	certBlock, rest := pem.Decode(data)
-	if certBlock == nil {
-		return nil, nil, fmt.Errorf("could not decode the first block from %q from expected PEM format", pairFile)
-	}
-	keyBlock, _ := pem.Decode(rest)
-	if keyBlock == nil {
-		return nil, nil, fmt.Errorf("could not decode the second block from %q from expected PEM format", pairFile)
-	}
-	return certBlock, keyBlock, nil
-}
-
 func (s *fileStore) Update(certData, keyData []byte) (*tls.Certificate, error) {
 	ts := time.Now().Format("2006-01-02-15-04-05")
 	pemFilename := s.filename(ts)

staging/src/k8s.io/client-go/util/certificate/certificate_store_test.go

@@ -17,12 +17,11 @@ limitations under the License.
 package certificate
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
-
-	"k8s.io/client-go/util/cert"
 )
 
 func TestUpdateSymlinkExistingFileError(t *testing.T) {
@@ -178,96 +177,6 @@ func TestUpdateSymlinkReplaceExistingSymlink(t *testing.T) {
 	}
 }
 
-func TestLoadCertKeyBlocksNoFile(t *testing.T) {
-	dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks")
-	if err != nil {
-		t.Fatalf("Unable to create the test directory %q: %v", dir, err)
-	}
-	defer func() {
-		if err := os.RemoveAll(dir); err != nil {
-			t.Errorf("Unable to clean up test directory %q: %v", dir, err)
-		}
-	}()
-
-	pairFile := filepath.Join(dir, "kubelet-pair.pem")
-
-	if _, _, err := loadCertKeyBlocks(pairFile); err == nil {
-		t.Errorf("Got no error, but expected %q not found.", pairFile)
-	}
-}
-
-func TestLoadCertKeyBlocksEmptyFile(t *testing.T) {
-	dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks")
-	if err != nil {
-		t.Fatalf("Unable to create the test directory %q: %v", dir, err)
-	}
-	defer func() {
-		if err := os.RemoveAll(dir); err != nil {
-			t.Errorf("Unable to clean up test directory %q: %v", dir, err)
-		}
-	}()
-
-	pairFile := filepath.Join(dir, "kubelet-pair.pem")
-	if err := ioutil.WriteFile(pairFile, nil, 0600); err != nil {
-		t.Fatalf("Unable to create the file %q: %v", pairFile, err)
-	}
-
-	if _, _, err := loadCertKeyBlocks(pairFile); err == nil {
-		t.Errorf("Got no error, but expected %q not found.", pairFile)
-	}
-}
-
-func TestLoadCertKeyBlocksPartialFile(t *testing.T) {
-	dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks")
-	if err != nil {
-		t.Fatalf("Unable to create the test directory %q: %v", dir, err)
-	}
-	defer func() {
-		if err := os.RemoveAll(dir); err != nil {
-			t.Errorf("Unable to clean up test directory %q: %v", dir, err)
-		}
-	}()
-
-	pairFile := filepath.Join(dir, "kubelet-pair.pem")
-	if err := ioutil.WriteFile(pairFile, storeCertData.certificatePEM, 0600); err != nil {
-		t.Fatalf("Unable to create the file %q: %v", pairFile, err)
-	}
-
-	if _, _, err := loadCertKeyBlocks(pairFile); err == nil {
-		t.Errorf("Got no error, but expected %q invalid.", pairFile)
-	}
-}
-
-func TestLoadCertKeyBlocks(t *testing.T) {
-	dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks")
-	if err != nil {
-		t.Fatalf("Unable to create the test directory %q: %v", dir, err)
-	}
-	defer func() {
-		if err := os.RemoveAll(dir); err != nil {
-			t.Errorf("Unable to clean up test directory %q: %v", dir, err)
-		}
-	}()
-
-	pairFile := filepath.Join(dir, "kubelet-pair.pem")
-	data := append(storeCertData.certificatePEM, []byte("\n")...)
-	data = append(data, storeCertData.keyPEM...)
-	if err := ioutil.WriteFile(pairFile, data, 0600); err != nil {
-		t.Fatalf("Unable to create the file %q: %v", pairFile, err)
-	}
-
-	certBlock, keyBlock, err := loadCertKeyBlocks(pairFile)
-	if err != nil {
-		t.Errorf("Got %v, but expected no error.", pairFile)
-	}
-	if certBlock.Type != cert.CertificateBlockType {
-		t.Errorf("Got %q loaded from the pair file, expected a %q.", certBlock.Type, cert.CertificateBlockType)
-	}
-	if keyBlock.Type != cert.RSAPrivateKeyBlockType {
-		t.Errorf("Got %q loaded from the pair file, expected a %q.", keyBlock.Type, cert.RSAPrivateKeyBlockType)
-	}
-}
-
 func TestLoadFile(t *testing.T) {
 	dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks")
 	if err != nil {
@@ -280,21 +189,30 @@ func TestLoadFile(t *testing.T) {
 	}()
 
 	pairFile := filepath.Join(dir, "kubelet-pair.pem")
-	data := append(storeCertData.certificatePEM, []byte("\n")...)
-	data = append(data, storeCertData.keyPEM...)
-	if err := ioutil.WriteFile(pairFile, data, 0600); err != nil {
-		t.Fatalf("Unable to create the file %q: %v", pairFile, err)
-	}
 
-	cert, err := loadFile(pairFile)
-	if err != nil {
-		t.Fatalf("Could not load certificate from disk: %v", err)
-	}
-	if cert == nil {
-		t.Fatalf("There was no error, but no certificate data was returned.")
-	}
-	if cert.Leaf == nil {
-		t.Fatalf("Got an empty leaf, expected private data.")
+	tests := []struct {
+		desc string
+		data []byte
+	}{
+		{desc: "cert and key", data: bytes.Join([][]byte{storeCertData.certificatePEM, storeCertData.keyPEM}, []byte("\n"))},
+		{desc: "key and cert", data: bytes.Join([][]byte{storeCertData.keyPEM, storeCertData.certificatePEM}, []byte("\n"))},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			if err := ioutil.WriteFile(pairFile, tt.data, 0600); err != nil {
+				t.Fatalf("Unable to create the file %q: %v", pairFile, err)
+			}
+			cert, err := loadFile(pairFile)
+			if err != nil {
+				t.Fatalf("Could not load certificate from disk: %v", err)
+			}
+			if cert == nil {
+				t.Fatalf("There was no error, but no certificate data was returned.")
+			}
+			if cert.Leaf == nil {
+				t.Fatalf("Got an empty leaf, expected private data.")
+			}
+		})
 	}
 }