From 739a75fc32c5337ddbd13691e9bf6648fb13ff0d Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 15 May 2019 11:47:23 -0400 Subject: [PATCH] Interrupt WaitForCertificate if desired kubelet serving cert changes --- .../app/phases/certs/renewal/apirenewer.go | 6 ++- .../certificate/bootstrap/bootstrap.go | 6 ++- .../util/certificate/certificate_manager.go | 41 ++++++++++++++----- .../client-go/util/certificate/csr/csr.go | 4 +- 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/cmd/kubeadm/app/phases/certs/renewal/apirenewer.go b/cmd/kubeadm/app/phases/certs/renewal/apirenewer.go index d118420ed15..d76b3c957a0 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/apirenewer.go +++ b/cmd/kubeadm/app/phases/certs/renewal/apirenewer.go @@ -17,6 +17,7 @@ limitations under the License. package renewal import ( + "context" "crypto" "crypto/x509" "crypto/x509/pkix" @@ -97,7 +98,10 @@ func (r *APIRenewer) Renew(cfg *certutil.Config) (*x509.Certificate, crypto.Sign fmt.Printf("[certs] Certificate request %q created\n", req.Name) - certData, err := csrutil.WaitForCertificate(r.client.CertificateSigningRequests(), req, watchTimeout) + ctx, cancel := context.WithTimeout(context.Background(), watchTimeout) + defer cancel() + + certData, err := csrutil.WaitForCertificate(ctx, r.client.CertificateSigningRequests(), req) if err != nil { return nil, nil, errors.Wrap(err, "certificate failed to appear") } diff --git a/pkg/kubelet/certificate/bootstrap/bootstrap.go b/pkg/kubelet/certificate/bootstrap/bootstrap.go index 75e13961c67..8aef15c5212 100644 --- a/pkg/kubelet/certificate/bootstrap/bootstrap.go +++ b/pkg/kubelet/certificate/bootstrap/bootstrap.go @@ -335,7 +335,11 @@ func requestNodeCertificate(client certificatesv1beta1.CertificateSigningRequest if err != nil { return nil, err } - return csr.WaitForCertificate(client, req, 3600*time.Second) + + ctx, cancel := context.WithTimeout(context.Background(), 3600*time.Second) + defer cancel() + + return csr.WaitForCertificate(ctx, client, req) } // This digest should include all the relevant pieces of the CSR we care about. diff --git a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go index 5aa232a42af..e8170044244 100644 --- a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go +++ b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go @@ -17,6 +17,7 @@ limitations under the License. package certificate import ( + "context" "crypto/ecdsa" "crypto/elliptic" cryptorand "crypto/rand" @@ -148,9 +149,13 @@ type CSRClientFunc func(current *tls.Certificate) (certificatesclient.Certificat func (e *NoCertKeyError) Error() string { return string(*e) } type manager struct { - getTemplate func() *x509.CertificateRequest - lastRequestLock sync.Mutex - lastRequest *x509.CertificateRequest + getTemplate func() *x509.CertificateRequest + + // lastRequestLock guards lastRequestCancel and lastRequest + lastRequestLock sync.Mutex + lastRequestCancel context.CancelFunc + lastRequest *x509.CertificateRequest + dynamicTemplate bool usages []certificates.KeyUsage forceRotation bool @@ -261,7 +266,8 @@ func (m *manager) Start() { case <-timer.C: // unblock when deadline expires case <-templateChanged: - if reflect.DeepEqual(m.getLastRequest(), m.getTemplate()) { + _, lastRequestTemplate := m.getLastRequest() + if reflect.DeepEqual(lastRequestTemplate, m.getTemplate()) { // if the template now matches what we last requested, restart the rotation deadline loop return } @@ -289,10 +295,19 @@ func (m *manager) Start() { if m.dynamicTemplate { go wait.Until(func() { // check if the current template matches what we last requested - if !m.certSatisfiesTemplate() && !reflect.DeepEqual(m.getLastRequest(), m.getTemplate()) { + lastRequestCancel, lastRequestTemplate := m.getLastRequest() + + if !m.certSatisfiesTemplate() && !reflect.DeepEqual(lastRequestTemplate, m.getTemplate()) { // if the template is different, queue up an interrupt of the rotation deadline loop. // if we've requested a CSR that matches the new template by the time the interrupt is handled, the interrupt is disregarded. - templateChanged <- struct{}{} + if lastRequestCancel != nil { + // if we're currently waiting on a submitted request that no longer matches what we want, stop waiting + lastRequestCancel() + } + select { + case templateChanged <- struct{}{}: + case <-m.stopCh: + } } }, time.Second, m.stopCh) } @@ -386,12 +401,15 @@ func (m *manager) rotateCerts() (bool, error) { return false, m.updateServerError(err) } + ctx, cancel := context.WithTimeout(context.Background(), certificateWaitTimeout) + defer cancel() + // Once we've successfully submitted a CSR for this template, record that we did so - m.setLastRequest(template) + m.setLastRequest(cancel, template) // Wait for the certificate to be signed. This interface and internal timout // is a remainder after the old design using raw watch wrapped with backoff. - crtPEM, err := csr.WaitForCertificate(client, req, certificateWaitTimeout) + crtPEM, err := csr.WaitForCertificate(ctx, client, req) if err != nil { utilruntime.HandleError(fmt.Errorf("Certificate request was not signed: %v", err)) return false, nil @@ -561,14 +579,15 @@ func (m *manager) generateCSR() (template *x509.CertificateRequest, csrPEM []byt return template, csrPEM, keyPEM, privateKey, nil } -func (m *manager) getLastRequest() *x509.CertificateRequest { +func (m *manager) getLastRequest() (context.CancelFunc, *x509.CertificateRequest) { m.lastRequestLock.Lock() defer m.lastRequestLock.Unlock() - return m.lastRequest + return m.lastRequestCancel, m.lastRequest } -func (m *manager) setLastRequest(r *x509.CertificateRequest) { +func (m *manager) setLastRequest(cancel context.CancelFunc, r *x509.CertificateRequest) { m.lastRequestLock.Lock() defer m.lastRequestLock.Unlock() + m.lastRequestCancel = cancel m.lastRequest = r } diff --git a/staging/src/k8s.io/client-go/util/certificate/csr/csr.go b/staging/src/k8s.io/client-go/util/certificate/csr/csr.go index 1c8d0eb89c7..e36f2d3e133 100644 --- a/staging/src/k8s.io/client-go/util/certificate/csr/csr.go +++ b/staging/src/k8s.io/client-go/util/certificate/csr/csr.go @@ -82,7 +82,7 @@ func RequestCertificate(client certificatesclient.CertificateSigningRequestInter } // WaitForCertificate waits for a certificate to be issued until timeout, or returns an error. -func WaitForCertificate(client certificatesclient.CertificateSigningRequestInterface, req *certificates.CertificateSigningRequest, timeout time.Duration) (certData []byte, err error) { +func WaitForCertificate(ctx context.Context, client certificatesclient.CertificateSigningRequestInterface, req *certificates.CertificateSigningRequest) (certData []byte, err error) { fieldSelector := fields.OneTermEqualSelector("metadata.name", req.Name).String() lw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { @@ -94,8 +94,6 @@ func WaitForCertificate(client certificatesclient.CertificateSigningRequestInter return client.Watch(options) }, } - ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), timeout) - defer cancel() event, err := watchtools.UntilWithSync( ctx, lw,