certificate/vault: Change cache from a map to sync.Map (openserviceme…

…sh#2074)
draychev · Nov 19, 2020 · 03c8f52 · 03c8f52
1 parent f9aa3dc
commit 03c8f52
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 37 deletions.
diff --git a/pkg/certificate/providers/vault/certificate_manager.go b/pkg/certificate/providers/vault/certificate_manager.go
@@ -33,10 +33,8 @@ const (
 
 // NewCertManager implements certificate.Manager and wraps a Hashi Vault with methods to allow easy certificate issuance.
 func NewCertManager(vaultAddr, token string, role string, cfg configurator.Configurator) (*CertManager, error) {
-	cache := make(map[certificate.CommonName]certificate.Certificater)
 	c := &CertManager{
 		announcements: make(chan announcements.Announcement),
-		cache:         &cache,
 		role:          vaultRole(role),
 		cfg:           cfg,
 	}
@@ -82,15 +80,12 @@ func (cm *CertManager) issue(cn certificate.CommonName, validityPeriod time.Dura
 }
 
 func (cm *CertManager) deleteFromCache(cn certificate.CommonName) {
-	cm.cacheLock.Lock()
-	delete(*cm.cache, cn)
-	cm.cacheLock.Unlock()
+	cm.cache.Delete(cn)
 }
 
 func (cm *CertManager) getFromCache(cn certificate.CommonName) certificate.Certificater {
-	cm.cacheLock.Lock()
-	defer cm.cacheLock.Unlock()
-	if cert, exists := (*cm.cache)[cn]; exists {
+	if certificateInterface, exists := cm.cache.Load(cn); exists {
+		cert := certificateInterface.(certificate.Certificater)
 		log.Trace().Msgf("Certificate found in cache CN=%s", cn)
 		if rotor.ShouldRotate(cert) {
 			log.Trace().Msgf("Certificate found in cache but has expired CN=%s", cn)
@@ -116,9 +111,7 @@ func (cm *CertManager) IssueCertificate(cn certificate.CommonName, validityPerio
 		return cert, err
 	}
 
-	cm.cacheLock.Lock()
-	(*cm.cache)[cn] = cert
-	cm.cacheLock.Unlock()
+	cm.cache.Store(cn, cert)
 
 	log.Info().Msgf("Issuing new certificate for CN=%s took %+v", cn, time.Since(start))
 
@@ -133,11 +126,10 @@ func (cm *CertManager) ReleaseCertificate(cn certificate.CommonName) {
 // ListCertificates lists all certificates issued
 func (cm *CertManager) ListCertificates() ([]certificate.Certificater, error) {
 	var certs []certificate.Certificater
-	cm.cacheLock.Lock()
-	for _, cert := range *cm.cache {
-		certs = append(certs, cert)
-	}
-	cm.cacheLock.Unlock()
+	cm.cache.Range(func(cnInterface interface{}, certInterface interface{}) bool {
+		certs = append(certs, certInterface.(certificate.Certificater))
+		return true
+	})
 	return certs, nil
 }
 
@@ -170,9 +162,7 @@ func (cm *CertManager) RotateCertificate(cn certificate.CommonName) (certificate
 		return cert, err
 	}
 
-	cm.cacheLock.Lock()
-	(*cm.cache)[cn] = cert
-	cm.cacheLock.Unlock()
+	cm.cache.Store(cn, cert)
 	cm.announcements <- announcements.Announcement{}
 
 	log.Info().Msgf("Rotating certificate CN=%s took %+v", cn, time.Since(start))

diff --git a/pkg/certificate/providers/vault/certificate_manager_test.go b/pkg/certificate/providers/vault/certificate_manager_test.go
@@ -101,12 +101,10 @@ var _ = Describe("Test client helpers", func() {
 
 	Context("Test Hashi Vault functions", func() {
 		cm := CertManager{
-			cache: &map[certificate.CommonName]certificate.Certificater{
-				expiredCertCN: expiredCert,
-				validCertCN:   validCert,
-			},
 			ca: rootCert,
 		}
+		cm.cache.Store(expiredCertCN, expiredCert)
+		cm.cache.Store(validCertCN, validCert)
 
 		It("gets certs from cache", func() {
 			// This cert does not exist - returns nil

diff --git a/pkg/certificate/providers/vault/debugger.go b/pkg/certificate/providers/vault/debugger.go
@@ -7,10 +7,9 @@ import (
 // ListIssuedCertificates implements CertificateDebugger interface and returns the list of issued certificates.
 func (cm *CertManager) ListIssuedCertificates() []certificate.Certificater {
 	var certs []certificate.Certificater
-	cm.cacheLock.Lock()
-	defer cm.cacheLock.Unlock()
-	for _, cert := range *cm.cache {
-		certs = append(certs, cert)
-	}
+	cm.cache.Range(func(cnInterface interface{}, certInterface interface{}) bool {
+		certs = append(certs, certInterface.(certificate.Certificater))
+		return true
+	})
 	return certs
 }
diff --git a/pkg/certificate/providers/vault/debugger_test.go b/pkg/certificate/providers/vault/debugger_test.go
@@ -19,13 +19,9 @@ var _ = Describe("Test Vault Debugger", func() {
 			expiration: time.Now(),
 			commonName: "foo.bar.co.uk",
 		}
-		cache := map[certificate.CommonName]certificate.Certificater{
-			"foo": cert,
-		}
-		cm := CertManager{
-			cache: &cache,
-		}
-		It("lists all issued certificets", func() {
+		cm := CertManager{}
+		cm.cache.Store("foo", cert)
+		It("lists all issued certificates", func() {
 			actual := cm.ListIssuedCertificates()
 			expected := []certificate.Certificater{cert}
 			Expect(actual).To(Equal(expected))

diff --git a/pkg/certificate/providers/vault/types.go b/pkg/certificate/providers/vault/types.go
@@ -19,8 +19,8 @@ type CertManager struct {
 	announcements chan announcements.Announcement
 
 	// Cache for all the certificates issued
-	cache     *map[certificate.CommonName]certificate.Certificater
-	cacheLock sync.Mutex
+	// Types: map[certificate.CommonName]certificate.Certificater
+	cache sync.Map
 
 	// Hashicorp Vault client
 	client *api.Client