Merge pull request #65063 from awly/fix-csr-private-key-reuse

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Re-use private key after failed CSR

**What this PR does / why we need it**:
This fixes a regression introduced in 1.11.

If we create a new key on each CSR, if CSR fails the next attempt will
create a new one instead of reusing previous CSR.

If approver/signer don't handle CSRs as quickly as new nodes come up,
they can pile up and approver would keep handling old abandoned CSRs and
Nodes would keep timing out on startup.

**Release note**:
```release-note
NONE
```

pkg/kubelet/certificate/bootstrap/bootstrap.go

@@ -19,6 +19,7 @@ package bootstrap
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"time"
 
 	"github.com/golang/glog"
@@ -35,6 +36,8 @@ import (
 	"k8s.io/client-go/util/certificate/csr"
 )
 
+const tmpPrivateKeyFile = "kubelet-client.key.tmp"
+
 // LoadClientCert requests a client cert for kubelet if the kubeconfigPath file does not exist.
 // The kubeconfig at bootstrapPath is used to request a client certificate from the API server.
 // On success, a kubeconfig file referencing the generated key and obtained certificate is written to kubeconfigPath.
@@ -75,9 +78,15 @@ func LoadClientCert(kubeconfigPath string, bootstrapPath string, certDir string,
 			}
 		}
 	}
+	// Cache the private key in a separate file until CSR succeeds. This has to
+	// be a separate file because store.CurrentPath() points to a symlink
+	// managed by the store.
+	privKeyPath := filepath.Join(certDir, tmpPrivateKeyFile)
 	if !verifyKeyData(keyData) {
-		glog.V(2).Infof("No valid private key found for bootstrapping, creating a new one")
-		keyData, err = certutil.MakeEllipticPrivateKeyPEM()
+		glog.V(2).Infof("No valid private key and/or certificate found, reusing existing private key or creating a new one")
+		// Note: always call LoadOrGenerateKeyFile so that private key is
+		// reused on next startup if CSR request fails.
+		keyData, _, err = certutil.LoadOrGenerateKeyFile(privKeyPath)
 		if err != nil {
 			return err
 		}
@@ -90,6 +99,9 @@ func LoadClientCert(kubeconfigPath string, bootstrapPath string, certDir string,
 	if _, err := store.Update(certData, keyData); err != nil {
 		return err
 	}
+	if err := os.Remove(privKeyPath); err != nil && !os.IsNotExist(err) {
+		glog.V(2).Infof("failed cleaning up private key file %q: %v", privKeyPath, err)
+	}
 
 	pemPath := store.CurrentPath()
 
@@ -192,8 +204,6 @@ func verifyKeyData(data []byte) bool {
 	if len(data) == 0 {
 		return false
 	}
-	if _, err := certutil.ParsePrivateKeyPEM(data); err != nil {
-		return false
-	}
-	return true
+	_, err := certutil.ParsePrivateKeyPEM(data)
+	return err == nil
 }

staging/src/k8s.io/client-go/util/cert/io.go

@@ -88,7 +88,8 @@ func WriteKey(keyPath string, data []byte) error {
 // can't find one, it will generate a new key and store it there.
 func LoadOrGenerateKeyFile(keyPath string) (data []byte, wasGenerated bool, err error) {
 	loadedData, err := ioutil.ReadFile(keyPath)
-	if err == nil {
+	// Call verifyKeyData to ensure the file wasn't empty/corrupt.
+	if err == nil && verifyKeyData(loadedData) {
 		return loadedData, false, err
 	}
 	if !os.IsNotExist(err) {
@@ -181,3 +182,12 @@ func PublicKeysFromFile(file string) ([]interface{}, error) {
 	}
 	return keys, nil
 }
+
+// verifyKeyData returns true if the provided data appears to be a valid private key.
+func verifyKeyData(data []byte) bool {
+	if len(data) == 0 {
+		return false
+	}
+	_, err := ParsePrivateKeyPEM(data)
+	return err == nil
+}