BREAKING: *: replace ErrorLogger interface with logr.Logger

dynamictls.go

@@ -21,20 +21,12 @@ import (
 	"sync/atomic"
 
 	"bursavich.dev/dynamictls/internal/forked/go/http2"
+	"github.com/go-logr/logr"
 	fsnotify "gopkg.in/fsnotify.v1"
 )
 
 const hashSize = 16 // 128-bit
 
-// An ErrorLogger logs errors.
-type ErrorLogger interface {
-	Errorf(format string, args ...interface{})
-}
-
-type noopLogger struct{}
-
-func (noopLogger) Errorf(format string, args ...interface{}) {}
-
 // NotifyFunc is a function that is called when new config data
 // is loaded or an error occurs loading new config data.
 type NotifyFunc func(cfg *tls.Config, err error)
@@ -126,10 +118,10 @@ func WithNotifyFunc(notify NotifyFunc) Option {
 	})
 }
 
-// WithErrorLogger returns an Option that sets the logger for errors.
-func WithErrorLogger(logger ErrorLogger) Option {
+// WithLogger returns an Option that sets the logger for errors.
+func WithLogger(log logr.Logger) Option {
 	return optionFunc(func(c *Config) error {
-		c.logger = logger
+		c.log = log
 		return nil
 	})
 }
@@ -192,7 +184,7 @@ type Config struct {
 	clientCAs []string
 	certs     []keyPair
 	notifyFns []NotifyFunc
-	logger    ErrorLogger
+	log       logr.Logger
 
 	watcher *fsnotify.Watcher
 	close   chan struct{} // signals watch goroutine to end
@@ -215,7 +207,7 @@ func NewConfig(options ...Option) (cfg *Config, err error) {
 	}()
 	cfg = &Config{
 		base:     &tls.Config{},
-		logger:   noopLogger{},
+		log:      logr.Discard(),
 		watcher:  w,
 		close:    make(chan struct{}),
 		done:     make(chan struct{}),
@@ -344,13 +336,13 @@ func (cfg *Config) watch() {
 		case <-cfg.watcher.Events:
 			// TODO: ignore unrelated events
 			if err := cfg.read(); err != nil {
-				cfg.logger.Errorf("%v", err) // errors already decorated
+				cfg.log.Error(err, "Read failure") // errors already decorated
 				for _, fn := range cfg.notifyFns {
 					fn(nil, err)
 				}
 			}
 		case err := <-cfg.watcher.Errors:
-			cfg.logger.Errorf("dynamictls: watch error: %v", err)
+			cfg.log.Error(err, "Watch failure")
 		case <-cfg.close:
 			return
 		}

dynamictls_test.go

@@ -182,7 +182,7 @@ func TestOptions(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			c, err := NewConfig(append(tt.options, WithErrorLogger(t))...)
+			c, err := NewConfig(append(tt.options, WithLogger(tlstest.Logr(t)))...)
 			if err != nil {
 				if tt.err {
 					return // error is expected
@@ -411,7 +411,7 @@ func TestMTLS(t *testing.T) {
 		),
 		WithRootCAs(caFile),
 		WithClientCAs(caFile),
-		WithErrorLogger(t),
+		WithLogger(tlstest.Logr(t)),
 		WithHTTP(),
 	)
 	check(t, "Failed to create server TLS config", err)
@@ -435,7 +435,7 @@ func TestMTLS(t *testing.T) {
 			createFile(t, dir, "client.key", clientKeyPEM),
 		),
 		WithRootCAs(caFile),
-		WithErrorLogger(t),
+		WithLogger(tlstest.Logr(t)),
 		WithHTTP(),
 	)
 	check(t, "Failed to create client TLS config", err)
@@ -491,7 +491,7 @@ func TestListenError(t *testing.T) {
 		WithHTTP2(),
 		WithCertificate(certFile, keyFile),
 		WithRootCAs(certFile),
-		WithErrorLogger(t),
+		WithLogger(tlstest.Logr(t)),
 	)
 	check(t, "Failed to create dynamic TLS config", err)
 	defer cfg.Close()
@@ -520,7 +520,7 @@ func TestDialErrors(t *testing.T) {
 		}),
 		WithHTTP2(),
 		WithRootCAs(caFile),
-		WithErrorLogger(t),
+		WithLogger(tlstest.Logr(t)),
 	)
 	check(t, "Failed to create dynamic TLS config", err)
 	defer cfg.Close()

go.mod

@@ -4,6 +4,7 @@ go 1.17
 
 require (
 	github.com/fsnotify/fsnotify v1.4.9 // indirect
+	github.com/go-logr/logr v1.2.2
 	github.com/google/go-cmp v0.5.5
 	github.com/prometheus/client_golang v1.11.0
 	github.com/prometheus/common v0.32.1 // indirect

go.sum

@@ -74,6 +74,8 @@ github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vb
 github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE=
 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
 github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
+github.com/go-logr/logr v1.2.2 h1:ahHml/yUpnlb96Rp8HCvtYVPY8ZYpxq3g7UYchIYwbs=
+github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
 github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=

grpctls/grpctls_test.go

@@ -42,7 +42,7 @@ func TestInvalidConfig(t *testing.T) {
 			CipherSuites: []uint16{tls.TLS_RSA_WITH_RC4_128_SHA},
 		}),
 		dynamictls.WithClientCAs(caFile),
-		dynamictls.WithErrorLogger(t),
+		dynamictls.WithLogger(tlstest.Logr(t)),
 	)
 	check(t, "Failed to create dynamic TLS config", err)
 	defer cfg.Close()
@@ -84,7 +84,7 @@ func TestHandshakeErrors(t *testing.T) {
 		dynamictls.WithHTTP2(),
 		dynamictls.WithCertificate(certFile, keyFile),
 		dynamictls.WithRootCAs(caFile),
-		dynamictls.WithErrorLogger(t),
+		dynamictls.WithLogger(tlstest.Logr(t)),
 	)
 	check(t, "Failed to create dynamic TLS config", err)
 	defer cfg.Close()
@@ -170,7 +170,7 @@ func TestGRPC(t *testing.T) {
 		),
 		dynamictls.WithRootCAs(caFile),
 		dynamictls.WithClientCAs(caFile),
-		dynamictls.WithErrorLogger(t),
+		dynamictls.WithLogger(tlstest.Logr(t)),
 	)
 	check(t, "Failed to create server TLS config", err)
 	defer serverCfg.Close()
@@ -207,7 +207,7 @@ func TestGRPC(t *testing.T) {
 			createFile(t, dir, "client.key", clientKeyPEM),
 		),
 		dynamictls.WithRootCAs(caFile),
-		dynamictls.WithErrorLogger(t),
+		dynamictls.WithLogger(tlstest.Logr(t)),
 	)
 	check(t, "Failed to create client TLS config", err)
 	defer clientCfg.Close()

internal/tlstest/gen.go

@@ -18,9 +18,19 @@ import (
 	"encoding/pem"
 	"fmt"
 	"math/big"
+	"testing"
 	"time"
+
+	"github.com/go-logr/logr"
+	"github.com/go-logr/logr/funcr"
 )
 
+func Logr(t *testing.T) logr.Logger {
+	// TODO: Differentiate between info and error logs.
+	// This is sufficient for now, because we only use error.
+	return funcr.New(func(prefix, args string) { t.Error(prefix, args) }, funcr.Options{})
+}
+
 // CertOptions are certificate options.
 type CertOptions struct {
 	Parent     *tls.Certificate

tlsprom/tlsprom.go

@@ -13,6 +13,7 @@ import (
 	"sort"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/prometheus/client_golang/prometheus"
 )
 
@@ -22,20 +23,11 @@ const (
 	expirationName  = "tls_config_earliest_certificate_expiration_time_seconds"
 )
 
-// An ErrorLogger logs errors.
-type ErrorLogger interface {
-	Errorf(format string, args ...interface{})
-}
-
-type noopLogger struct{}
-
-func (noopLogger) Errorf(format string, args ...interface{}) {}
-
 type config struct {
 	namespace string
 	subsystem string
 	usages    []x509.ExtKeyUsage
-	logger    ErrorLogger
+	log       logr.Logger
 }
 
 // An Option applies optional configuration.
@@ -76,10 +68,10 @@ func sortedOptions(options []Option) []Option {
 	return options
 }
 
-// WithErrorLogger returns an Option that sets the logger for errors.
-func WithErrorLogger(logger ErrorLogger) Option {
+// WithLogger returns an Option that sets the logger for errors.
+func WithLogger(log logr.Logger) Option {
 	return optionFunc(func(c *config) error {
-		c.logger = logger
+		c.log = log
 		return nil
 	})
 }
@@ -152,14 +144,14 @@ type Metrics struct {
 	expiration  prometheus.Gauge
 
 	usages []x509.ExtKeyUsage
-	logger ErrorLogger
+	log    logr.Logger
 }
 
 // NewMetrics returns new Metrics with the given options.
 func NewMetrics(options ...Option) (*Metrics, error) {
 	cfg := &config{
 		usages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny},
-		logger: &noopLogger{},
+		log:    logr.Discard(),
 	}
 	for _, o := range sortedOptions(options) {
 		if err := o.apply(cfg); err != nil {
@@ -186,7 +178,7 @@ func NewMetrics(options ...Option) (*Metrics, error) {
 			Help:      "Earliest expiration time of the TLS configuration's certificates in seconds since the Unix epoch.",
 		}),
 		usages: cfg.usages,
-		logger: cfg.logger,
+		log:    cfg.log,
 	}
 	return m, nil
 }
@@ -231,7 +223,7 @@ func (m *Metrics) earliestExpiration(cfg *tls.Config) (time.Time, error) {
 		if x509Cert == nil {
 			var err error
 			if x509Cert, err = x509.ParseCertificate(cert.Certificate[0]); err != nil {
-				m.logger.Errorf("tlsprom: cert parsing error: %v", err)
+				m.log.Error(err, "Failed to parse TLS certificate")
 				return time.Time{}, err
 			}
 		}
@@ -240,7 +232,7 @@ func (m *Metrics) earliestExpiration(cfg *tls.Config) (time.Time, error) {
 			KeyUsages: m.usages,
 		})
 		if err != nil {
-			m.logger.Errorf("tlsprom: cert verification error: %v", err)
+			m.log.Error(err, "Failed to validate TLS certificate")
 			return time.Time{}, err
 		}
 		for _, chain := range chains {
@@ -252,7 +244,7 @@ func (m *Metrics) earliestExpiration(cfg *tls.Config) (time.Time, error) {
 		}
 	}
 	if t.IsZero() {
-		m.logger.Errorf("tlsprom: no certificates in config")
+		m.log.Error(nil, "Failed to find a certificate in the TLS config")
 	}
 	return t, nil
 }

tlsprom/tlsprom_test.go

@@ -286,7 +286,7 @@ func TestExpiration(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			m, err := NewMetrics(WithErrorLogger(t))
+			m, err := NewMetrics(WithLogger(tlstest.Logr(t)))
 			check(t, "Failed to create metrics", err)
 			m.Update(tt.config, nil)
 			got := readGauge(t, m.expiration)