Merge pull request #71046 from liztio/certs-renew-use-api-fix

Certs renew use api fix

cmd/kubeadm/app/phases/certs/renewal/BUILD

@@ -14,11 +14,10 @@ go_library(
         "//cmd/kubeadm/app/util/pkiutil:go_default_library",
         "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes: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/csr:go_default_library",
         "//vendor/github.com/pkg/errors:go_default_library",
     ],
 )

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

@@ -17,30 +17,25 @@ limitations under the License.
 package renewal
 
 import (
-	"crypto/rand"
 	"crypto/rsa"
 	"crypto/x509"
 	"crypto/x509/pkix"
+	"fmt"
 	"time"
 
 	"github.com/pkg/errors"
 
 	certsapi "k8s.io/api/certificates/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/fields"
-	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/kubernetes"
 	certstype "k8s.io/client-go/kubernetes/typed/certificates/v1beta1"
 	certutil "k8s.io/client-go/util/cert"
+	csrutil "k8s.io/client-go/util/certificate/csr"
 )
 
-const (
-	certAPIPrefixName = "kubeadm-cert"
-)
+const certAPIPrefixName = "kubeadm-cert"
 
-var (
-	watchTimeout = 5 * time.Minute
-)
+var watchTimeout = 5 * time.Minute
 
 // CertsAPIRenewal creates new certificates using the certs API
 type CertsAPIRenewal struct {
@@ -70,7 +65,7 @@ func (r *CertsAPIRenewal) Renew(cfg *certutil.Config) (*x509.Certificate, *rsa.P
 		return nil, nil, errors.Wrap(err, "couldn't create new private key")
 	}
 
-	csr, err := x509.CreateCertificateRequest(rand.Reader, reqTmp, key)
+	csr, err := certutil.MakeCSRFromTemplate(key, reqTmp)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "couldn't create certificate signing request")
 	}
@@ -86,7 +81,7 @@ func (r *CertsAPIRenewal) Renew(cfg *certutil.Config) (*x509.Certificate, *rsa.P
 
 	k8sCSR := &certsapi.CertificateSigningRequest{
 		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: certAPIPrefixName,
+			GenerateName: fmt.Sprintf("%s-%s-", certAPIPrefixName, cfg.CommonName),
 		},
 		Spec: certsapi.CertificateSigningRequestSpec{
 			Request: csr,
@@ -99,43 +94,23 @@ func (r *CertsAPIRenewal) Renew(cfg *certutil.Config) (*x509.Certificate, *rsa.P
 		return nil, nil, errors.Wrap(err, "couldn't create certificate signing request")
 	}
 
-	watcher, err := r.client.CertificateSigningRequests().Watch(metav1.ListOptions{
-		Watch:         true,
-		FieldSelector: fields.Set{"metadata.name": req.Name}.String(),
-	})
+	fmt.Printf("[certs] certificate request %q created\n", req.Name)
+
+	certData, err := csrutil.WaitForCertificate(r.client.CertificateSigningRequests(), req, watchTimeout)
 	if err != nil {
-		return nil, nil, errors.Wrap(err, "couldn't watch for certificate creation")
-	}
-	defer watcher.Stop()
-
-	select {
-	case ev := <-watcher.ResultChan():
-		if ev.Type != watch.Modified {
-			return nil, nil, errors.Errorf("unexpected event received: %q", ev.Type)
-		}
-	case <-time.After(watchTimeout):
-		return nil, nil, errors.New("timeout trying to sign certificate")
+		return nil, nil, errors.Wrap(err, "certificate failed to appear")
 	}
 
-	req, err = r.client.CertificateSigningRequests().Get(req.Name, metav1.GetOptions{})
-	if err != nil {
-		return nil, nil, errors.Wrap(err, "couldn't get certificate signing request")
-	}
-	if len(req.Status.Conditions) < 1 {
-		return nil, nil, errors.New("certificate signing request has no statuses")
-	}
-
-	// TODO: under what circumstances are there more than one?
-	if status := req.Status.Conditions[0].Type; status != certsapi.CertificateApproved {
-		return nil, nil, errors.Errorf("unexpected certificate status: %v", status)
-	}
-
-	cert, err := x509.ParseCertificate(req.Status.Certificate)
+	cert, err := certutil.ParseCertsPEM(certData)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "couldn't parse issued certificate")
 	}
 
-	return cert, key, nil
+	if len(cert) != 1 {
+		return nil, nil, errors.Errorf("certificate request %q has %d certificates, wanted exactly 1", req.Name, len(cert))
+	}
+
+	return cert[0], key, nil
 }
 
 var usageMap = map[x509.ExtKeyUsage]certsapi.KeyUsage{

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

@@ -49,8 +49,12 @@ func TestRenewImplementations(t *testing.T) {
 		Fake: &k8stesting.Fake{},
 	}
 	certReq := getCertReq(t, caCert, caKey)
+	certReqNoCert := certReq.DeepCopy()
+	certReqNoCert.Status.Certificate = nil
 	client.AddReactor("get", "certificatesigningrequests", defaultReactionFunc(certReq))
-	watcher := watch.NewFakeWithChanSize(1, false)
+	watcher := watch.NewFakeWithChanSize(3, false)
+	watcher.Add(certReqNoCert)
+	watcher.Modify(certReqNoCert)
 	watcher.Modify(certReq)
 	client.AddWatchReactor("certificatesigningrequests", k8stesting.DefaultWatchReactor(watcher, nil))
 
@@ -132,7 +136,7 @@ func getCertReq(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey) *
 					Type: certsapi.CertificateApproved,
 				},
 			},
-			Certificate: cert.Raw,
+			Certificate: certutil.EncodeCertPEM(cert),
 		},
 	}
 }