diff --git a/pkg/kubelet/certificate/kubelet.go b/pkg/kubelet/certificate/kubelet.go index c7c4a92d952bb..212e35bc25833 100644 --- a/pkg/kubelet/certificate/kubelet.go +++ b/pkg/kubelet/certificate/kubelet.go @@ -297,9 +297,9 @@ func (m *kubeletServerCertificateDynamicFileManager) Start() { } // Stop stops watching the certificate and key files -func (fm *kubeletServerCertificateDynamicFileManager) Stop() { - if fm.cancelFn != nil { - fm.cancelFn() +func (m *kubeletServerCertificateDynamicFileManager) Stop() { + if m.cancelFn != nil { + m.cancelFn() } } diff --git a/pkg/kubelet/certificate/kubelet_test.go b/pkg/kubelet/certificate/kubelet_test.go index c3007fab56370..fad1ec621897a 100644 --- a/pkg/kubelet/certificate/kubelet_test.go +++ b/pkg/kubelet/certificate/kubelet_test.go @@ -18,8 +18,6 @@ package certificate import ( "bytes" - "crypto/tls" - "fmt" "net" "os" "path/filepath" @@ -220,31 +218,49 @@ func TestKubeletServerCertificateFromFiles(t *testing.T) { if err != nil { t.Fatalf("Unable to setup cert dir: %v", err) } - defer os.RemoveAll(certDir) + + rotateCertErrs := make(chan error, 10) + + defer func() { + if err := os.RemoveAll(certDir); err != nil { + t.Errorf("Unable to clean up test directory %q: %v", certDir, err) + } + if len(rotateCertErrs) != 0 { + t.Errorf("got errors when rotating certificate files in the test") + } + close(rotateCertErrs) + }() certPath := filepath.Join(certDir, "kubelet.cert") keyPath := filepath.Join(certDir, "kubelet.key") createCertFn := func(cert, key []byte) error { - os.Remove(certPath) - os.Remove(keyPath) if err := os.WriteFile(certPath, cert, os.FileMode(0644)); err != nil { return err } - if err := os.WriteFile(keyPath, key, os.FileMode(0600)); err != nil { - return err } - fmt.Println("created files") return nil } err = createCertFn([]byte(cert1), []byte(key1)) if err != nil { t.Fatalf("Unable to setup cert: %v", err) } + + // simulate certificate files update in the background go func() { time.Sleep(500 * time.Millisecond) - createCertFn([]byte(cert2), []byte(key2)) + if err := os.Remove(certPath); err != nil { + rotateCertErrs <- err + return + } + if err := os.Remove(keyPath); err != nil { + rotateCertErrs <- err + return + } + if err := createCertFn([]byte(cert2), []byte(key2)); err != nil { + rotateCertErrs <- err + } }() m, err := NewKubeletServerCertificateDynamicFileManager(certPath, keyPath) @@ -255,15 +271,14 @@ func TestKubeletServerCertificateFromFiles(t *testing.T) { m.Start() defer m.Stop() - var c *tls.Certificate - c = m.Current() + c := m.Current() if c == nil { - t.Errorf("failed to provide valid certificate") + t.Fatal("failed to provide valid certificate") } time.Sleep(100 * time.Millisecond) c2 := m.Current() if c2 == nil { - t.Errorf("failed to provide valid certificate") + t.Fatal("failed to provide valid certificate") } if c2 != c { t.Errorf("expected the same loaded certificate object when there is no cert file change, got different") @@ -273,12 +288,14 @@ func TestKubeletServerCertificateFromFiles(t *testing.T) { c3 := m.Current() if c3 == nil { t.Errorf("failed to provide valid certificate after file update") - } - if bytes.Equal(c.Certificate[0], c3.Certificate[0]) { + } else if bytes.Equal(c.Certificate[0], c3.Certificate[0]) { t.Errorf("failed to provide the updated certificate") } - os.Remove(certPath) + if err = os.Remove(certPath); err != nil { + t.Errorf("could not delete file in order to perform test") + } + if m.Current() == nil { t.Errorf("expected the manager still provide cached content when certificate file was not available") } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index e0b9b31b02e3b..4b8572bb982fc 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -774,26 +774,28 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, } klet.imageManager = imageManager - if kubeCfg.ServerTLSBootstrap && kubeDeps.TLSOptions != nil && utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletServerCertificate) { - klet.serverCertificateManager, err = kubeletcertificate.NewKubeletServerCertificateManager(klet.kubeClient, kubeCfg, klet.nodeName, klet.getLastObservedNodeAddresses, certDirectory) - if err != nil { - return nil, fmt.Errorf("failed to initialize certificate manager: %v", err) - } + if kubeDeps.TLSOptions != nil { + if kubeCfg.ServerTLSBootstrap && utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletServerCertificate) { + klet.serverCertificateManager, err = kubeletcertificate.NewKubeletServerCertificateManager(klet.kubeClient, kubeCfg, klet.nodeName, klet.getLastObservedNodeAddresses, certDirectory) + if err != nil { + return nil, fmt.Errorf("failed to initialize certificate manager: %w", err) + } - } else if kubeDeps.TLSOptions != nil && kubeDeps.TLSOptions.CertFile != "" && kubeDeps.TLSOptions.KeyFile != "" { - klet.serverCertificateManager, err = kubeletcertificate.NewKubeletServerCertificateDynamicFileManager(kubeDeps.TLSOptions.CertFile, kubeDeps.TLSOptions.KeyFile) - if err != nil { - return nil, fmt.Errorf("failed to initialize file based certificate manager: %v", err) + } else if kubeDeps.TLSOptions.CertFile != "" && kubeDeps.TLSOptions.KeyFile != "" { + klet.serverCertificateManager, err = kubeletcertificate.NewKubeletServerCertificateDynamicFileManager(kubeDeps.TLSOptions.CertFile, kubeDeps.TLSOptions.KeyFile) + if err != nil { + return nil, fmt.Errorf("failed to initialize file based certificate manager: %w", err) + } } - } - if klet.serverCertificateManager != nil { - kubeDeps.TLSOptions.Config.GetCertificate = func(*tls.ClientHelloInfo) (*tls.Certificate, error) { - cert := klet.serverCertificateManager.Current() - if cert == nil { - return nil, fmt.Errorf("no serving certificate available for the kubelet") + if klet.serverCertificateManager != nil { + kubeDeps.TLSOptions.Config.GetCertificate = func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + cert := klet.serverCertificateManager.Current() + if cert == nil { + return nil, fmt.Errorf("no serving certificate available for the kubelet") + } + return cert, nil } - return cert, nil } }