From de23d3fd004c6af7a91bfa6c3d90e2af5955de6d Mon Sep 17 00:00:00 2001 From: Jacob Simpson Date: Mon, 1 May 2017 17:56:44 -0700 Subject: [PATCH] Allow certificate manager to be initialized with client. Add test coverage to the certificate manager covering the initialization scenario where it is initialized with no Certificate Request Signing client, then the client is added later. This matches how it will be used when the Certificate Request Signing client is also the consumer of the certificate manager. --- .../certificate/certificate_manager.go | 17 ++++ .../certificate/certificate_manager_test.go | 98 ++++++++++++++++++- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/certificate/certificate_manager.go b/pkg/kubelet/certificate/certificate_manager.go index b547cf591d3..ada732e9be2 100644 --- a/pkg/kubelet/certificate/certificate_manager.go +++ b/pkg/kubelet/certificate/certificate_manager.go @@ -46,6 +46,9 @@ const ( // manager. In the background it communicates with the API server to get new // certificates for certificates about to expire. type Manager interface { + // CertificateSigningRequestClient sets the client interface that is used for + // signing new certificates generated as part of rotation. + SetCertificateSigningRequestClient(certificatesclient.CertificateSigningRequestInterface) error // Start the API server status sync loop. Start() // Current returns the currently selected certificate from the @@ -156,6 +159,20 @@ func (m *manager) Current() *tls.Certificate { return m.cert } +// SetCertificateSigningRequestClient sets the client interface that is used +// for signing new certificates generated as part of rotation. It must be +// called before Start() and can not be used to change the +// CertificateSigningRequestClient that has already been set. This method is to +// support the one specific scenario where the CertificateSigningRequestClient +// uses the CertificateManager. +func (m *manager) SetCertificateSigningRequestClient(certSigningRequestClient certificatesclient.CertificateSigningRequestInterface) error { + if m.certSigningRequestClient == nil { + m.certSigningRequestClient = certSigningRequestClient + return nil + } + return fmt.Errorf("CertificateSigningRequestClient is already set.") +} + // Start will start the background work of rotating the certificates. func (m *manager) Start() { // Certificate rotation depends on access to the API server certificate diff --git a/pkg/kubelet/certificate/certificate_manager_test.go b/pkg/kubelet/certificate/certificate_manager_test.go index 748ea07f5c1..f3980ccab29 100644 --- a/pkg/kubelet/certificate/certificate_manager_test.go +++ b/pkg/kubelet/certificate/certificate_manager_test.go @@ -227,6 +227,7 @@ func TestRotateCertWaitingForResultError(t *testing.T) { func TestNewManagerBootstrap(t *testing.T) { store := &fakeStore{} + var cm Manager cm, err := NewManager(&Config{ Template: &x509.CertificateRequest{}, Usages: []certificates.KeyUsage{}, @@ -234,7 +235,6 @@ func TestNewManagerBootstrap(t *testing.T) { BootstrapCertificatePEM: bootstrapCertData.certificatePEM, BootstrapKeyPEM: bootstrapCertData.keyPEM, }) - if err != nil { t.Fatalf("Failed to initialize the certificate manager: %v", err) } @@ -356,6 +356,98 @@ func TestGetCurrentCertificateOrBootstrap(t *testing.T) { } } +func TestInitializeCertificateSigningRequestClient(t *testing.T) { + var nilCertificate = &certificateData{} + testCases := []struct { + description string + storeCert *certificateData + bootstrapCert *certificateData + apiCert *certificateData + expectedCertBeforeStart *certificateData + expectedCertAfterStart *certificateData + }{ + { + description: "No current certificate, no bootstrap certificate", + storeCert: nilCertificate, + bootstrapCert: nilCertificate, + apiCert: apiServerCertData, + expectedCertBeforeStart: nilCertificate, + expectedCertAfterStart: apiServerCertData, + }, + { + description: "No current certificate, bootstrap certificate", + storeCert: nilCertificate, + bootstrapCert: bootstrapCertData, + apiCert: apiServerCertData, + expectedCertBeforeStart: bootstrapCertData, + expectedCertAfterStart: apiServerCertData, + }, + { + description: "Current certificate, no bootstrap certificate", + storeCert: storeCertData, + bootstrapCert: nilCertificate, + apiCert: apiServerCertData, + expectedCertBeforeStart: storeCertData, + expectedCertAfterStart: storeCertData, + }, + { + description: "Current certificate, bootstrap certificate", + storeCert: storeCertData, + bootstrapCert: bootstrapCertData, + apiCert: apiServerCertData, + expectedCertBeforeStart: storeCertData, + expectedCertAfterStart: storeCertData, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + certificateStore := &fakeStore{ + cert: tc.storeCert.certificate, + } + + certificateManager, err := NewManager(&Config{ + Template: &x509.CertificateRequest{ + Subject: pkix.Name{ + Organization: []string{"system:nodes"}, + CommonName: "system:node:fake-node-name", + }, + }, + Usages: []certificates.KeyUsage{ + certificates.UsageDigitalSignature, + certificates.UsageKeyEncipherment, + certificates.UsageClientAuth, + }, + CertificateStore: certificateStore, + BootstrapCertificatePEM: tc.bootstrapCert.certificatePEM, + BootstrapKeyPEM: tc.bootstrapCert.keyPEM, + }) + if err != nil { + t.Errorf("Got %v, wanted no error.", err) + } + + certificate := certificateManager.Current() + if !certificatesEqual(certificate, tc.expectedCertBeforeStart.certificate) { + t.Errorf("Got %v, wanted %v", certificateString(certificate), certificateString(tc.expectedCertBeforeStart.certificate)) + } + if err := certificateManager.SetCertificateSigningRequestClient(&fakeClient{ + certificatePEM: tc.apiCert.certificatePEM, + }); err != nil { + t.Errorf("Got error %v, expected none.", err) + } + + if err := certificateManager.(*manager).rotateCerts(); err != nil { + t.Errorf("Got error %v, expected none.", err) + } + + certificate = certificateManager.Current() + if !certificatesEqual(certificate, tc.expectedCertAfterStart.certificate) { + t.Errorf("Got %v, wanted %v", certificateString(certificate), certificateString(tc.expectedCertAfterStart.certificate)) + } + }) + } +} + func TestInitializeOtherRESTClients(t *testing.T) { var nilCertificate = &certificateData{} testCases := []struct { @@ -434,7 +526,9 @@ func TestInitializeOtherRESTClients(t *testing.T) { t.Errorf("Got %v, wanted %v", certificateString(certificate), certificateString(tc.expectedCertBeforeStart.certificate)) } - certificateManager.Start() + if err := certificateManager.(*manager).rotateCerts(); err != nil { + t.Errorf("Got error %v, expected none.", err) + } certificate = certificateManager.Current() if !certificatesEqual(certificate, tc.expectedCertAfterStart.certificate) {