Merge pull request #64 from w13915984028/fix63

fix63 use sleep instead of force scheduling

.drone.yml

@@ -0,0 +1,15 @@
+---
+
+kind: pipeline
+name: fossa
+
+steps:
+- name: fossa
+  image: rancher/drone-fossa:latest
+  settings:
+    api_key:
+      from_secret: FOSSA_API_KEY
+  when:
+    instance:
+      - drone-publish.rancher.io
+

cert/cert.go

@@ -33,7 +33,9 @@ import (
 	"math"
 	"math/big"
 	"net"
+	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -109,11 +111,18 @@ func NewSignedCert(cfg Config, key crypto.Signer, caCert *x509.Certificate, caKe
 	if len(cfg.Usages) == 0 {
 		return nil, errors.New("must specify at least one ExtKeyUsage")
 	}
-	var expiresAt time.Duration
+	expiresAt := duration365d
 	if cfg.ExpiresAt > 0 {
 		expiresAt = time.Duration(cfg.ExpiresAt)
 	} else {
-		expiresAt = duration365d
+		envExpirationDays := os.Getenv("CATTLE_NEW_SIGNED_CERT_EXPIRATION_DAYS")
+		if envExpirationDays != "" {
+			if envExpirationDaysInt, err := strconv.Atoi(envExpirationDays); err != nil {
+				logrus.Infof("[NewSignedCert] expiration days from ENV (%s) could not be converted to int (falling back to default value: %d)", envExpirationDays, expiresAt)
+			} else {
+				expiresAt = time.Hour * 24 * time.Duration(envExpirationDaysInt)
+			}
+		}
 	}
 
 	certTmpl := x509.Certificate{

cert/secret.go

@@ -0,0 +1,16 @@
+package cert
+
+import v1 "k8s.io/api/core/v1"
+
+func IsValidTLSSecret(secret *v1.Secret) bool {
+	if secret == nil {
+		return false
+	}
+	if _, ok := secret.Data[v1.TLSCertKey]; !ok {
+		return false
+	}
+	if _, ok := secret.Data[v1.TLSPrivateKeyKey]; !ok {
+		return false
+	}
+	return true
+}

factory/ca.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"time"
 
 	"github.com/rancher/dynamiclistener/cert"
 )
@@ -16,7 +17,7 @@ func GenCA() (*x509.Certificate, crypto.Signer, error) {
 		return nil, nil, err
 	}
 
-	caCert, err := NewSelfSignedCACert(caKey, "dynamiclistener-ca", "dynamiclistener-org")
+	caCert, err := NewSelfSignedCACert(caKey, fmt.Sprintf("dynamiclistener-ca@%d", time.Now().Unix()), "dynamiclistener-org")
 	if err != nil {
 		return nil, nil, err
 	}

factory/gen.go

@@ -15,6 +15,7 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+	"time"
 
 	"github.com/rancher/dynamiclistener/cert"
 	"github.com/sirupsen/logrus"
@@ -32,11 +33,12 @@ var (
 )
 
 type TLS struct {
-	CACert       *x509.Certificate
-	CAKey        crypto.Signer
-	CN           string
-	Organization []string
-	FilterCN     func(...string) []string
+	CACert              *x509.Certificate
+	CAKey               crypto.Signer
+	CN                  string
+	Organization        []string
+	FilterCN            func(...string) []string
+	ExpirationDaysCheck int
 }
 
 func cns(secret *v1.Secret) (cns []string) {
@@ -72,7 +74,8 @@ func collectCNs(secret *v1.Secret) (domains []string, ips []net.IP, err error) {
 
 // Merge combines the SAN lists from the target and additional Secrets, and
 // returns a potentially modified Secret, along with a bool indicating if the
-// returned Secret is not the same as the target Secret.
+// returned Secret is not the same as the target Secret. Secrets with expired
+// certificates will never be returned.
 //
 // If the merge would not add any CNs to the additional Secret, the additional
 // Secret is returned, to allow for certificate rotation/regeneration.
@@ -93,17 +96,17 @@ func (t *TLS) Merge(target, additional *v1.Secret) (*v1.Secret, bool, error) {
 
 	// if the additional secret already has all the CNs, use it in preference to the
 	// current one. This behavior is required to allow for renewal or regeneration.
-	if !NeedsUpdate(0, additional, mergedCNs...) {
+	if !NeedsUpdate(0, additional, mergedCNs...) && !t.IsExpired(additional) {
 		return additional, true, nil
 	}
 
 	// if the target secret already has all the CNs, continue using it. The additional
 	// cert had only a subset of the current CNs, so nothing needs to be added.
-	if !NeedsUpdate(0, target, mergedCNs...) {
+	if !NeedsUpdate(0, target, mergedCNs...) && !t.IsExpired(target) {
 		return target, false, nil
 	}
 
-	// neither cert currently has all the necessary CNs; generate a new one.
+	// neither cert currently has all the necessary CNs or is unexpired; generate a new one.
 	return t.generateCert(target, mergedCNs...)
 }
 
@@ -154,6 +157,10 @@ func (t *TLS) generateCert(secret *v1.Secret, cn ...string) (*v1.Secret, bool, e
 		secret = &v1.Secret{}
 	}
 
+	if err := t.Verify(secret); err != nil {
+		logrus.Warnf("unable to verify existing certificate: %v - signing operation may change certificate issuer", err)
+	}
+
 	secret = populateCN(secret, cn...)
 
 	privateKey, err := getPrivateKey(secret)
@@ -171,7 +178,7 @@ func (t *TLS) generateCert(secret *v1.Secret, cn ...string) (*v1.Secret, bool, e
 		return nil, false, err
 	}
 
-	certBytes, keyBytes, err := Marshal(newCert, privateKey)
+	keyBytes, certBytes, err := MarshalChain(privateKey, newCert, t.CACert)
 	if err != nil {
 		return nil, false, err
 	}
@@ -179,6 +186,7 @@ func (t *TLS) generateCert(secret *v1.Secret, cn ...string) (*v1.Secret, bool, e
 	if secret.Data == nil {
 		secret.Data = map[string][]byte{}
 	}
+	secret.Type = v1.SecretTypeTLS
 	secret.Data[v1.TLSCertKey] = certBytes
 	secret.Data[v1.TLSPrivateKeyKey] = keyBytes
 	secret.Annotations[fingerprint] = fmt.Sprintf("SHA1=%X", sha1.Sum(newCert.Raw))
@@ -186,6 +194,44 @@ func (t *TLS) generateCert(secret *v1.Secret, cn ...string) (*v1.Secret, bool, e
 	return secret, true, nil
 }
 
+func (t *TLS) IsExpired(secret *v1.Secret) bool {
+	certsPem := secret.Data[v1.TLSCertKey]
+	if len(certsPem) == 0 {
+		return false
+	}
+
+	certificates, err := cert.ParseCertsPEM(certsPem)
+	if err != nil || len(certificates) == 0 {
+		return false
+	}
+
+	expirationDays := time.Duration(t.ExpirationDaysCheck) * time.Hour * 24
+	return time.Now().Add(expirationDays).After(certificates[0].NotAfter)
+}
+
+func (t *TLS) Verify(secret *v1.Secret) error {
+	certsPem := secret.Data[v1.TLSCertKey]
+	if len(certsPem) == 0 {
+		return nil
+	}
+
+	certificates, err := cert.ParseCertsPEM(certsPem)
+	if err != nil || len(certificates) == 0 {
+		return err
+	}
+
+	verifyOpts := x509.VerifyOptions{
+		Roots: x509.NewCertPool(),
+		KeyUsages: []x509.ExtKeyUsage{
+			x509.ExtKeyUsageAny,
+		},
+	}
+	verifyOpts.Roots.AddCert(t.CACert)
+
+	_, err = certificates[0].Verify(verifyOpts)
+	return err
+}
+
 func (t *TLS) newCert(domains []string, ips []net.IP, privateKey crypto.Signer) (*x509.Certificate, error) {
 	return NewSignedCert(privateKey, t.CACert, t.CAKey, t.CN, t.Organization, domains, ips)
 }
@@ -208,7 +254,10 @@ func populateCN(secret *v1.Secret, cn ...string) *v1.Secret {
 // IsStatic returns true if the Secret has an attribute indicating that it contains
 // a static (aka user-provided) certificate, which should not be modified.
 func IsStatic(secret *v1.Secret) bool {
-	return secret.Annotations[Static] == "true"
+	if secret != nil && secret.Annotations != nil {
+		return secret.Annotations[Static] == "true"
+	}
+	return false
 }
 
 // NeedsUpdate returns true if any of the CNs are not currently present on the
@@ -246,14 +295,33 @@ func getPrivateKey(secret *v1.Secret) (crypto.Signer, error) {
 	return NewPrivateKey()
 }
 
+// MarshalChain returns given key and certificates as byte slices.
+func MarshalChain(privateKey crypto.Signer, certs ...*x509.Certificate) (keyBytes, certChainBytes []byte, err error) {
+	keyBytes, err = cert.MarshalPrivateKeyToPEM(privateKey)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	for _, cert := range certs {
+		if cert != nil {
+			certBlock := pem.Block{
+				Type:  CertificateBlockType,
+				Bytes: cert.Raw,
+			}
+			certChainBytes = append(certChainBytes, pem.EncodeToMemory(&certBlock)...)
+		}
+	}
+	return keyBytes, certChainBytes, nil
+}
+
 // Marshal returns the given cert and key as byte slices.
-func Marshal(x509Cert *x509.Certificate, privateKey crypto.Signer) ([]byte, []byte, error) {
+func Marshal(x509Cert *x509.Certificate, privateKey crypto.Signer) (certBytes, keyBytes []byte, err error) {
 	certBlock := pem.Block{
 		Type:  CertificateBlockType,
 		Bytes: x509Cert.Raw,
 	}
 
-	keyBytes, err := cert.MarshalPrivateKeyToPEM(privateKey)
+	keyBytes, err = cert.MarshalPrivateKeyToPEM(privateKey)
 	if err != nil {
 		return nil, nil, err
 	}

go.mod

@@ -1,6 +1,6 @@
 module github.com/rancher/dynamiclistener
 
-go 1.16
+go 1.12
 
 require (
 	github.com/rancher/wrangler v0.8.9
@@ -8,4 +8,5 @@ require (
 	golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2
 	k8s.io/api v0.18.8
 	k8s.io/apimachinery v0.18.8
+	k8s.io/client-go v0.18.8
 )

listener.go

@@ -7,7 +7,6 @@ import (
 	"crypto/x509"
 	"net"
 	"net/http"
-	"runtime"
 	"strings"
 	"sync"
 	"time"
@@ -45,14 +44,18 @@ func NewListener(l net.Listener, storage TLSStorage, caCert *x509.Certificate, c
 	if config.TLSConfig == nil {
 		config.TLSConfig = &tls.Config{}
 	}
+	if config.ExpirationDaysCheck == 0 {
+		config.ExpirationDaysCheck = 90
+	}
 
 	dynamicListener := &listener{
 		factory: &factory.TLS{
-			CACert:       caCert,
-			CAKey:        caKey,
-			CN:           config.CN,
-			Organization: config.Organization,
-			FilterCN:     allowDefaultSANs(config.SANs, config.FilterCN),
+			CACert:              caCert,
+			CAKey:               caKey,
+			CN:                  config.CN,
+			Organization:        config.Organization,
+			FilterCN:            allowDefaultSANs(config.SANs, config.FilterCN),
+			ExpirationDaysCheck: config.ExpirationDaysCheck,
 		},
 		Listener:  l,
 		storage:   &nonNil{storage: storage},
@@ -82,10 +85,6 @@ func NewListener(l net.Listener, storage TLSStorage, caCert *x509.Certificate, c
 		}
 	}
 
-	if config.ExpirationDaysCheck == 0 {
-		config.ExpirationDaysCheck = 30
-	}
-
 	tlsListener := tls.NewListener(dynamicListener.WrapExpiration(config.ExpirationDaysCheck), dynamicListener.tlsConfig)
 
 	return tlsListener, dynamicListener.cacheHandler(), nil
@@ -163,15 +162,15 @@ type listener struct {
 func (l *listener) WrapExpiration(days int) net.Listener {
 	ctx, cancel := context.WithCancel(context.Background())
 	go func() {
-		// busy-wait for certificate preload to complete
+		// loop on short sleeps until certificate preload completes
 		for l.cert == nil {
-			runtime.Gosched()
+			time.Sleep(time.Millisecond)
 		}
 
 		for {
 			wait := 6 * time.Hour
 			if err := l.checkExpiration(days); err != nil {
-				logrus.Errorf("failed to check and renew dynamic cert: %v", err)
+				logrus.Errorf("dynamiclistener %s: failed to check and renew dynamic cert: %v", l.Addr(), err)
 				// Don't go into short retry loop if we're using a static (user-provided) cert.
 				// We will still check and print an error every six hours until the user updates the secret with
 				// a cert that is not about to expire. Hopefully this will prompt them to take action.
@@ -263,12 +262,18 @@ func (l *listener) Accept() (net.Conn, error) {
 	l.init.Do(func() {
 		if len(l.sans) > 0 {
 			if err := l.updateCert(l.sans...); err != nil {
-				logrus.Errorf("failed to update cert with configured SANs: %v", err)
+				logrus.Errorf("dynamiclistener %s: failed to update cert with configured SANs: %v", l.Addr(), err)
 				return
 			}
-			if _, err := l.loadCert(nil); err != nil {
-				logrus.Errorf("failed to preload certificate: %v", err)
-			}
+		}
+		if cert, err := l.loadCert(nil); err != nil {
+			logrus.Errorf("dynamiclistener %s: failed to preload certificate: %v", l.Addr(), err)
+		} else if cert == nil {
+			// This should only occur on the first startup when no SANs are configured in the listener config, in which
+			// case no certificate can be created, as dynamiclistener will not create certificates until at least one IP
+			// or DNS SAN is set. It will also occur when using the Kubernetes storage without a local File cache.
+			// For reliable serving of requests, callers should configure a local cache and/or a default set of SANs.
+			logrus.Warnf("dynamiclistener %s: no cached certificate available for preload - deferring certificate load until storage initialization or first client request", l.Addr())
 		}
 	})
 
@@ -284,14 +289,12 @@ func (l *listener) Accept() (net.Conn, error) {
 
 	host, _, err := net.SplitHostPort(addr.String())
 	if err != nil {
-		logrus.Errorf("failed to parse network %s: %v", addr.Network(), err)
+		logrus.Errorf("dynamiclistener %s: failed to parse connection local address %s: %v", l.Addr(), addr, err)
 		return conn, nil
 	}
 
-	if !strings.Contains(host, ":") {
-		if err := l.updateCert(host); err != nil {
-			logrus.Errorf("failed to update cert with listener address: %v", err)
-		}
+	if err := l.updateCert(host); err != nil {
+		logrus.Errorf("dynamiclistener %s: failed to update cert with connection local address: %v", l.Addr(), err)
 	}
 
 	if l.conns != nil {
@@ -338,7 +341,7 @@ func (l *listener) getCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate,
 	newConn := hello.Conn
 	if hello.ServerName != "" {
 		if err := l.updateCert(hello.ServerName); err != nil {
-			logrus.Errorf("failed to update cert with TLS ServerName: %v", err)
+			logrus.Errorf("dynamiclistener %s: failed to update cert with TLS ServerName: %v", l.Addr(), err)
 			return nil, err
 		}
 	}
@@ -408,6 +411,9 @@ func (l *listener) loadCert(currentConn net.Conn) (*tls.Certificate, error) {
 	if err != nil {
 		return nil, err
 	}
+	if !cert.IsValidTLSSecret(secret) {
+		return l.cert, nil
+	}
 	if l.cert != nil && l.version == secret.ResourceVersion && secret.ResourceVersion != "" {
 		return l.cert, nil
 	}
@@ -452,7 +458,7 @@ func (l *listener) cacheHandler() http.Handler {
 			}
 
 			if err := l.updateCert(h); err != nil {
-				logrus.Errorf("failed to update cert with HTTP request Host header: %v", err)
+				logrus.Errorf("dynamiclistener %s: failed to update cert with HTTP request Host header: %v", l.Addr(), err)
 			}
 		}
 	})

server/server.go

@@ -64,7 +64,10 @@ func ListenAndServe(ctx context.Context, httpsPort, httpPort int, handler http.H
 		}
 
 		tlsServer := http.Server{
-			Handler:  handler,
+			Handler: handler,
+			BaseContext: func(listener net.Listener) context.Context {
+				return ctx
+			},
 			ErrorLog: errorLog,
 		}
 
@@ -86,6 +89,9 @@ func ListenAndServe(ctx context.Context, httpsPort, httpPort int, handler http.H
 			Addr:     fmt.Sprintf("%s:%d", opts.BindHost, httpPort),
 			Handler:  handler,
 			ErrorLog: errorLog,
+			BaseContext: func(listener net.Listener) context.Context {
+				return ctx
+			},
 		}
 		go func() {
 			logrus.Infof("Listening on %s:%d", opts.BindHost, httpPort)

storage/kubernetes/ca.go

@@ -56,7 +56,7 @@ func createAndStoreClientCert(secrets v1controller.SecretClient, namespace strin
 		return nil, err
 	}
 
-	certPem, keyPem, err := factory.Marshal(cert, key)
+	keyPem, certPem, err := factory.MarshalChain(key, cert, caCert)
 	if err != nil {
 		return nil, err
 	}

storage/kubernetes/controller.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/rancher/dynamiclistener"
+	"github.com/rancher/dynamiclistener/cert"
 	"github.com/rancher/wrangler/pkg/generated/controllers/core"
 	v1controller "github.com/rancher/wrangler/pkg/generated/controllers/core/v1"
 	"github.com/rancher/wrangler/pkg/start"
@@ -13,6 +14,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/util/retry"
 )
 
 type CoreGetter func() *core.Factory
@@ -39,10 +41,9 @@ func New(ctx context.Context, core CoreGetter, namespace, name string, backing d
 	// lazy init
 	go func() {
 		for {
-			core := core()
-			if core != nil {
-				storage.init(core.Core().V1().Secret())
-				_ = start.All(ctx, 5, core)
+			if coreFactory := core(); coreFactory != nil {
+				storage.init(coreFactory.Core().V1().Secret())
+				_ = start.All(ctx, 5, coreFactory)
 				return
 			}
 
@@ -58,16 +59,18 @@ func New(ctx context.Context, core CoreGetter, namespace, name string, backing d
 }
 
 type storage struct {
-	sync.Mutex
+	sync.RWMutex
 
 	namespace, name string
 	storage         dynamiclistener.TLSStorage
-	secrets         v1controller.SecretClient
+	secrets         v1controller.SecretController
 	ctx             context.Context
 	tls             dynamiclistener.TLSFactory
 }
 
 func (s *storage) SetFactory(tls dynamiclistener.TLSFactory) {
+	s.Lock()
+	defer s.Unlock()
 	s.tls = tls
 }
 
@@ -90,7 +93,7 @@ func (s *storage) init(secrets v1controller.SecretController) {
 	s.secrets = secrets
 
 	secret, err := s.storage.Get()
-	if err == nil && secret != nil && len(secret.Data) > 0 {
+	if err == nil && cert.IsValidTLSSecret(secret) {
 		// local storage had a cached secret, ensure that it exists in Kubernetes
 		_, err := s.secrets.Create(&v1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
@@ -108,7 +111,9 @@ func (s *storage) init(secrets v1controller.SecretController) {
 		// local storage was empty, try to populate it
 		secret, err := s.secrets.Get(s.namespace, s.name, metav1.GetOptions{})
 		if err != nil {
-			logrus.Warnf("Failed to init Kubernetes secret: %v", err)
+			if !errors.IsNotFound(err) {
+				logrus.Warnf("Failed to init Kubernetes secret: %v", err)
+			}
 			return
 		}
 
@@ -119,13 +124,16 @@ func (s *storage) init(secrets v1controller.SecretController) {
 }
 
 func (s *storage) Get() (*v1.Secret, error) {
-	s.Lock()
-	defer s.Unlock()
+	s.RLock()
+	defer s.RUnlock()
 
 	return s.storage.Get()
 }
 
 func (s *storage) targetSecret() (*v1.Secret, error) {
+	s.RLock()
+	defer s.RUnlock()
+
 	existingSecret, err := s.secrets.Get(s.namespace, s.name, metav1.GetOptions{})
 	if errors.IsNotFound(err) {
 		return &v1.Secret{
@@ -133,13 +141,14 @@ func (s *storage) targetSecret() (*v1.Secret, error) {
 				Name:      s.name,
 				Namespace: s.namespace,
 			},
+			Type: v1.SecretTypeTLS,
 		}, nil
 	}
 	return existingSecret, err
 }
 
 func (s *storage) saveInK8s(secret *v1.Secret) (*v1.Secret, error) {
-	if s.secrets == nil {
+	if !s.initComplete() {
 		return secret, nil
 	}
 
@@ -152,14 +161,14 @@ func (s *storage) saveInK8s(secret *v1.Secret) (*v1.Secret, error) {
 	// in favor of just blindly replacing the fields on the Kubernetes secret.
 	if s.tls != nil {
 		// merge new secret with secret from backing storage, if one exists
-		if existing, err := s.storage.Get(); err == nil && existing != nil && len(existing.Data) > 0 {
+		if existing, err := s.Get(); err == nil && cert.IsValidTLSSecret(existing) {
 			if newSecret, updated, err := s.tls.Merge(existing, secret); err == nil && updated {
 				secret = newSecret
 			}
 		}
 
 		// merge new secret with existing secret from Kubernetes, if one exists
-		if len(targetSecret.Data) > 0 {
+		if cert.IsValidTLSSecret(targetSecret) {
 			if newSecret, updated, err := s.tls.Merge(targetSecret, secret); err != nil {
 				return nil, err
 			} else if !updated {
@@ -170,36 +179,60 @@ func (s *storage) saveInK8s(secret *v1.Secret) (*v1.Secret, error) {
 		}
 	}
 
+	// ensure that the merged secret actually contains data before overwriting the existing fields
+	if !cert.IsValidTLSSecret(secret) {
+		logrus.Warnf("Skipping save of TLS secret for %s/%s due to missing certificate data", secret.Namespace, secret.Name)
+		return targetSecret, nil
+	}
+
 	targetSecret.Annotations = secret.Annotations
 	targetSecret.Type = v1.SecretTypeTLS
 	targetSecret.Data = secret.Data
 
 	if targetSecret.UID == "" {
-		logrus.Infof("Creating new TLS secret for %v (count: %d): %v", targetSecret.Name, len(targetSecret.Annotations)-1, targetSecret.Annotations)
+		logrus.Infof("Creating new TLS secret for %s/%s (count: %d): %v", targetSecret.Namespace, targetSecret.Name, len(targetSecret.Annotations)-1, targetSecret.Annotations)
 		return s.secrets.Create(targetSecret)
 	}
-	logrus.Infof("Updating TLS secret for %v (count: %d): %v", targetSecret.Name, len(targetSecret.Annotations)-1, targetSecret.Annotations)
+	logrus.Infof("Updating TLS secret for %s/%s (count: %d): %v", targetSecret.Namespace, targetSecret.Name, len(targetSecret.Annotations)-1, targetSecret.Annotations)
 	return s.secrets.Update(targetSecret)
 }
 
-func (s *storage) Update(secret *v1.Secret) (err error) {
-	s.Lock()
-	defer s.Unlock()
-
-	for i := 0; i < 3; i++ {
-		secret, err = s.saveInK8s(secret)
-		if errors.IsConflict(err) {
-			continue
-		} else if err != nil {
-			return err
+func (s *storage) Update(secret *v1.Secret) error {
+	// Asynchronously update the Kubernetes secret, as doing so inline may block the listener from
+	// accepting new connections if the apiserver becomes unavailable after the Secrets controller
+	// has been initialized. We're not passing around any contexts here, nor does the controller
+	// accept any, so there's no good way to soft-fail with a reasonable timeout.
+	go func() {
+		if err := s.update(secret); err != nil {
+			logrus.Errorf("Failed to save TLS secret for %s/%s: %v", secret.Namespace, secret.Name, err)
 		}
-		break
-	}
+	}()
+	return nil
+}
+
+func isConflictOrAlreadyExists(err error) bool {
+	return errors.IsConflict(err) || errors.IsAlreadyExists(err)
+}
+
+func (s *storage) update(secret *v1.Secret) (err error) {
+	var newSecret *v1.Secret
+	err = retry.OnError(retry.DefaultRetry, isConflictOrAlreadyExists, func() error {
+		newSecret, err = s.saveInK8s(secret)
+		return err
+	})
 
 	if err != nil {
 		return err
 	}
 
-	// update underlying storage
-	return s.storage.Update(secret)
+	// Only hold the lock while updating underlying storage
+	s.Lock()
+	defer s.Unlock()
+	return s.storage.Update(newSecret)
+}
+
+func (s *storage) initComplete() bool {
+	s.RLock()
+	defer s.RUnlock()
+	return s.secrets != nil
 }

storage/memory/memory.go

@@ -39,7 +39,7 @@ func (m *memory) Update(secret *v1.Secret) error {
 			}
 		}
 
-		logrus.Infof("Active TLS secret %s (ver=%s) (count %d): %v", secret.Name, secret.ResourceVersion, len(secret.Annotations)-1, secret.Annotations)
+		logrus.Infof("Active TLS secret %s/%s (ver=%s) (count %d): %v", secret.Namespace, secret.Name, secret.ResourceVersion, len(secret.Annotations)-1, secret.Annotations)
 		m.secret = secret
 	}
 	return nil