Merge pull request #55 from genexpr/cherry-pick-filter

Add filter helper method

cert/secret.go

@@ -1,16 +0,0 @@
-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/gen.go

@@ -179,7 +179,6 @@ 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))
@@ -209,10 +208,7 @@ 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 {
-	if secret != nil && secret.Annotations != nil {
-		return secret.Annotations[Static] == "true"
-	}
-	return false
+	return secret.Annotations[Static] == "true"
 }
 
 // NeedsUpdate returns true if any of the CNs are not currently present on the

go.mod

@@ -1,6 +1,6 @@
 module github.com/rancher/dynamiclistener
 
-go 1.12
+go 1.16
 
 require (
 	github.com/rancher/wrangler v0.8.9
@@ -8,5 +8,4 @@ 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

@@ -408,9 +408,6 @@ 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
 	}

server/server.go

@@ -64,10 +64,7 @@ func ListenAndServe(ctx context.Context, httpsPort, httpPort int, handler http.H
 		}
 
 		tlsServer := http.Server{
-			Handler: handler,
-			BaseContext: func(listener net.Listener) context.Context {
-				return ctx
-			},
+			Handler:  handler,
 			ErrorLog: errorLog,
 		}
 
@@ -89,9 +86,6 @@ 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/controller.go

@@ -6,7 +6,6 @@ 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"
@@ -14,7 +13,6 @@ 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
@@ -41,9 +39,10 @@ func New(ctx context.Context, core CoreGetter, namespace, name string, backing d
 	// lazy init
 	go func() {
 		for {
-			if coreFactory := core(); coreFactory != nil {
-				storage.init(coreFactory.Core().V1().Secret())
-				_ = start.All(ctx, 5, coreFactory)
+			core := core()
+			if core != nil {
+				storage.init(core.Core().V1().Secret())
+				_ = start.All(ctx, 5, core)
 				return
 			}
 
@@ -59,11 +58,11 @@ func New(ctx context.Context, core CoreGetter, namespace, name string, backing d
 }
 
 type storage struct {
-	sync.RWMutex
+	sync.Mutex
 
 	namespace, name string
 	storage         dynamiclistener.TLSStorage
-	secrets         v1controller.SecretController
+	secrets         v1controller.SecretClient
 	ctx             context.Context
 	tls             dynamiclistener.TLSFactory
 }
@@ -91,7 +90,7 @@ func (s *storage) init(secrets v1controller.SecretController) {
 	s.secrets = secrets
 
 	secret, err := s.storage.Get()
-	if err == nil && cert.IsValidTLSSecret(secret) {
+	if err == nil && secret != nil && len(secret.Data) > 0 {
 		// local storage had a cached secret, ensure that it exists in Kubernetes
 		_, err := s.secrets.Create(&v1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
@@ -120,16 +119,13 @@ func (s *storage) init(secrets v1controller.SecretController) {
 }
 
 func (s *storage) Get() (*v1.Secret, error) {
-	s.RLock()
-	defer s.RUnlock()
+	s.Lock()
+	defer s.Unlock()
 
 	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{
@@ -137,14 +133,13 @@ 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.controllerHasSynced() {
+	if s.secrets == nil {
 		return secret, nil
 	}
 
@@ -157,14 +152,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.Get(); err == nil && cert.IsValidTLSSecret(existing) {
+		if existing, err := s.storage.Get(); err == nil && existing != nil && len(existing.Data) > 0 {
 			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 cert.IsValidTLSSecret(targetSecret) {
+		if len(targetSecret.Data) > 0 {
 			if newSecret, updated, err := s.tls.Merge(targetSecret, secret); err != nil {
 				return nil, err
 			} else if !updated {
@@ -175,58 +170,36 @@ 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 %s/%s (count: %d): %v", targetSecret.Namespace, targetSecret.Name, len(targetSecret.Annotations)-1, targetSecret.Annotations)
+		logrus.Infof("Creating new TLS secret for %v (count: %d): %v", targetSecret.Name, len(targetSecret.Annotations)-1, targetSecret.Annotations)
 		return s.secrets.Create(targetSecret)
 	}
-	logrus.Infof("Updating TLS secret for %s/%s (count: %d): %v", targetSecret.Namespace, targetSecret.Name, len(targetSecret.Annotations)-1, targetSecret.Annotations)
+	logrus.Infof("Updating TLS secret for %v (count: %d): %v", targetSecret.Name, len(targetSecret.Annotations)-1, targetSecret.Annotations)
 	return s.secrets.Update(targetSecret)
 }
 
-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)
-		}
-	}()
-	return nil
-}
+func (s *storage) Update(secret *v1.Secret) (err error) {
+	s.Lock()
+	defer s.Unlock()
 
-func (s *storage) update(secret *v1.Secret) (err error) {
-	err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
+	for i := 0; i < 3; i++ {
 		secret, err = s.saveInK8s(secret)
-		return err
-	})
+		if errors.IsConflict(err) {
+			continue
+		} else if err != nil {
+			return err
+		}
+		break
+	}
 
 	if err != nil {
 		return err
 	}
 
-	// Only hold the lock while updating underlying storage
-	s.Lock()
-	defer s.Unlock()
+	// update underlying storage
 	return s.storage.Update(secret)
 }
-
-func (s *storage) controllerHasSynced() bool {
-	s.RLock()
-	defer s.RUnlock()
-	if s.secrets == nil {
-		return false
-	}
-	return s.secrets.Informer().HasSynced()
-}

storage/memory/memory.go

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