From be581fa7fff0d5409dfb5e976c6569d0567dda3b Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Fri, 22 Feb 2019 13:19:23 +0100 Subject: [PATCH 1/5] Add logger interface and stop relying on Logrus directly --- cmd/dex/config.go | 4 +- cmd/dex/serve.go | 7 ++-- connector/authproxy/authproxy.go | 7 ++-- connector/bitbucketcloud/bitbucketcloud.go | 7 ++-- connector/github/github.go | 9 ++--- connector/gitlab/gitlab.go | 6 +-- connector/keystone/keystone.go | 7 ++-- connector/ldap/ldap.go | 11 +++--- connector/ldap/ldap_test.go | 3 +- connector/linkedin/linkedin.go | 7 ++-- connector/microsoft/microsoft.go | 7 ++-- connector/mock/connectortest.go | 13 +++---- connector/oidc/oidc.go | 6 +-- connector/saml/saml.go | 11 +++--- connector/saml/saml_test.go | 7 ++-- pkg/log/logger.go | 17 ++++++++ pkg/log/logrus.go | 45 ++++++++++++++++++++++ pkg/log/logrus_test.go | 10 +++++ server/api.go | 7 ++-- server/api_test.go | 19 ++++----- server/rotation.go | 5 +-- server/rotation_test.go | 5 ++- server/server.go | 19 +++++---- server/server_test.go | 5 ++- storage/etcd/config.go | 6 +-- storage/etcd/etcd.go | 4 +- storage/etcd/etcd_test.go | 5 ++- storage/kubernetes/client.go | 6 +-- storage/kubernetes/storage.go | 7 ++-- storage/kubernetes/storage_test.go | 5 ++- storage/memory/memory.go | 9 ++--- storage/memory/memory_test.go | 5 ++- storage/memory/static_test.go | 13 ++++--- storage/sql/config.go | 10 ++--- storage/sql/config_test.go | 5 ++- storage/sql/migrate_test.go | 6 ++- storage/sql/sql.go | 5 +-- storage/static.go | 6 +-- 38 files changed, 203 insertions(+), 133 deletions(-) create mode 100644 pkg/log/logger.go create mode 100644 pkg/log/logrus.go create mode 100644 pkg/log/logrus_test.go diff --git a/cmd/dex/config.go b/cmd/dex/config.go index 0071b4fd..ed1cc9f2 100644 --- a/cmd/dex/config.go +++ b/cmd/dex/config.go @@ -6,9 +6,9 @@ import ( "fmt" "os" - "github.com/sirupsen/logrus" "golang.org/x/crypto/bcrypt" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/server" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/etcd" @@ -127,7 +127,7 @@ type Storage struct { // StorageConfig is a configuration that can create a storage. type StorageConfig interface { - Open(logrus.FieldLogger) (storage.Storage, error) + Open(logger log.Logger) (storage.Storage, error) } var storages = map[string]func() StorageConfig{ diff --git a/cmd/dex/serve.go b/cmd/dex/serve.go index b9f940cc..0933eb4f 100644 --- a/cmd/dex/serve.go +++ b/cmd/dex/serve.go @@ -23,6 +23,7 @@ import ( "google.golang.org/grpc/credentials" "github.com/dexidp/dex/api" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/server" "github.com/dexidp/dex/storage" ) @@ -324,7 +325,7 @@ func (f *utcFormatter) Format(e *logrus.Entry) ([]byte, error) { return f.f.Format(e) } -func newLogger(level string, format string) (logrus.FieldLogger, error) { +func newLogger(level string, format string) (log.Logger, error) { var logLevel logrus.Level switch strings.ToLower(level) { case "debug": @@ -347,9 +348,9 @@ func newLogger(level string, format string) (logrus.FieldLogger, error) { return nil, fmt.Errorf("log format is not one of the supported values (%s): %s", strings.Join(logFormats, ", "), format) } - return &logrus.Logger{ + return log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &formatter, Level: logLevel, - }, nil + }), nil } diff --git a/connector/authproxy/authproxy.go b/connector/authproxy/authproxy.go index b4f3958d..f58c4b68 100644 --- a/connector/authproxy/authproxy.go +++ b/connector/authproxy/authproxy.go @@ -8,9 +8,8 @@ import ( "net/http" "net/url" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) // Config holds the configuration parameters for a connector which returns an @@ -18,14 +17,14 @@ import ( type Config struct{} // Open returns an authentication strategy which requires no user interaction. -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { return &callback{logger: logger, pathSuffix: "/" + id}, nil } // Callback is a connector which returns an identity with the HTTP header // X-Remote-User as verified email. type callback struct { - logger logrus.FieldLogger + logger log.Logger pathSuffix string } diff --git a/connector/bitbucketcloud/bitbucketcloud.go b/connector/bitbucketcloud/bitbucketcloud.go index 27a63c46..6c391be7 100644 --- a/connector/bitbucketcloud/bitbucketcloud.go +++ b/connector/bitbucketcloud/bitbucketcloud.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/dexidp/dex/pkg/log" "io/ioutil" "net/http" "sync" @@ -14,8 +15,6 @@ import ( "golang.org/x/oauth2" "golang.org/x/oauth2/bitbucket" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/connector" ) @@ -40,7 +39,7 @@ type Config struct { } // Open returns a strategy for logging in through Bitbucket. -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { b := bitbucketConnector{ redirectURI: c.RedirectURI, @@ -70,7 +69,7 @@ type bitbucketConnector struct { teams []string clientID string clientSecret string - logger logrus.FieldLogger + logger log.Logger apiURL string // the following are used only for tests diff --git a/connector/github/github.go b/connector/github/github.go index 9f7c2782..8658f8cd 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -16,11 +16,11 @@ import ( "strings" "time" - "github.com/sirupsen/logrus" "golang.org/x/oauth2" "golang.org/x/oauth2/github" "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) const ( @@ -53,7 +53,6 @@ type Config struct { // Org holds org-team filters, in which teams are optional. type Org struct { - // Organization name in github (not slug, full name). Only users in this github // organization can authenticate. Name string `json:"name"` @@ -66,14 +65,14 @@ type Org struct { } // Open returns a strategy for logging in through GitHub. -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { if c.Org != "" { // Return error if both 'org' and 'orgs' fields are used. if len(c.Orgs) > 0 { return nil, errors.New("github: cannot use both 'org' and 'orgs' fields simultaneously") } - logger.Warnln("github: legacy field 'org' being used. Switch to the newer 'orgs' field structure") + logger.Warn("github: legacy field 'org' being used. Switch to the newer 'orgs' field structure") } g := githubConnector{ @@ -137,7 +136,7 @@ type githubConnector struct { orgs []Org clientID string clientSecret string - logger logrus.FieldLogger + logger log.Logger // apiURL defaults to "https://api.github.com" apiURL string // hostName of the GitHub enterprise account. diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go index 41b0beb2..58ecba17 100644 --- a/connector/gitlab/gitlab.go +++ b/connector/gitlab/gitlab.go @@ -10,10 +10,10 @@ import ( "net/http" "strconv" - "github.com/sirupsen/logrus" "golang.org/x/oauth2" "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) const ( @@ -42,7 +42,7 @@ type gitlabUser struct { } // Open returns a strategy for logging in through GitLab. -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { if c.BaseURL == "" { c.BaseURL = "https://gitlab.com" } @@ -71,7 +71,7 @@ type gitlabConnector struct { org string clientID string clientSecret string - logger logrus.FieldLogger + logger log.Logger } func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config { diff --git a/connector/keystone/keystone.go b/connector/keystone/keystone.go index a86e957d..0a2440db 100644 --- a/connector/keystone/keystone.go +++ b/connector/keystone/keystone.go @@ -9,9 +9,8 @@ import ( "io/ioutil" "net/http" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) type conn struct { @@ -19,7 +18,7 @@ type conn struct { Host string AdminUsername string AdminPassword string - Logger logrus.FieldLogger + Logger log.Logger } type userKeystone struct { @@ -102,7 +101,7 @@ var ( ) // Open returns an authentication strategy using Keystone. -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { return &conn{ c.Domain, c.Host, diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index a7aa6c4f..cf8e5006 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -12,9 +12,8 @@ import ( "gopkg.in/ldap.v2" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) // Config holds the configuration parameters for the LDAP connector. The LDAP @@ -165,7 +164,7 @@ func parseScope(s string) (int, bool) { } // Open returns an authentication strategy using LDAP. -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { conn, err := c.OpenConnector(logger) if err != nil { return nil, err @@ -179,7 +178,7 @@ type refreshData struct { } // OpenConnector is the same as Open but returns a type with all implemented connector interfaces. -func (c *Config) OpenConnector(logger logrus.FieldLogger) (interface { +func (c *Config) OpenConnector(logger log.Logger) (interface { connector.Connector connector.PasswordConnector connector.RefreshConnector @@ -187,7 +186,7 @@ func (c *Config) OpenConnector(logger logrus.FieldLogger) (interface { return c.openConnector(logger) } -func (c *Config) openConnector(logger logrus.FieldLogger) (*ldapConnector, error) { +func (c *Config) openConnector(logger log.Logger) (*ldapConnector, error) { requiredFields := []struct { name string @@ -259,7 +258,7 @@ type ldapConnector struct { tlsConfig *tls.Config - logger logrus.FieldLogger + logger log.Logger } var ( diff --git a/connector/ldap/ldap_test.go b/connector/ldap/ldap_test.go index fa6b7062..c6f30807 100644 --- a/connector/ldap/ldap_test.go +++ b/connector/ldap/ldap_test.go @@ -17,6 +17,7 @@ import ( "github.com/sirupsen/logrus" "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) const envVar = "DEX_LDAP_TESTS" @@ -875,7 +876,7 @@ func runTests(t *testing.T, schema string, connMethod connectionMethod, config * c.BindDN = "cn=admin,dc=example,dc=org" c.BindPW = "admin" - l := &logrus.Logger{Out: ioutil.Discard, Formatter: &logrus.TextFormatter{}} + l := log.NewLogrusLogger(&logrus.Logger{Out: ioutil.Discard, Formatter: &logrus.TextFormatter{}}) conn, err := c.openConnector(l) if err != nil { diff --git a/connector/linkedin/linkedin.go b/connector/linkedin/linkedin.go index 9ab67e57..df5725b3 100644 --- a/connector/linkedin/linkedin.go +++ b/connector/linkedin/linkedin.go @@ -11,9 +11,8 @@ import ( "golang.org/x/oauth2" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) const ( @@ -30,7 +29,7 @@ type Config struct { } // Open returns a strategy for logging in through LinkedIn -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { return &linkedInConnector{ oauth2Config: &oauth2.Config{ ClientID: c.ClientID, @@ -52,7 +51,7 @@ type connectorData struct { type linkedInConnector struct { oauth2Config *oauth2.Config - logger logrus.FieldLogger + logger log.Logger } // LinkedIn doesn't provide refresh tokens, so refresh tokens issued by Dex diff --git a/connector/microsoft/microsoft.go b/connector/microsoft/microsoft.go index ad6b3e73..730c77e3 100644 --- a/connector/microsoft/microsoft.go +++ b/connector/microsoft/microsoft.go @@ -14,9 +14,8 @@ import ( "golang.org/x/oauth2" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) const ( @@ -39,7 +38,7 @@ type Config struct { } // Open returns a strategy for logging in through Microsoft. -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { m := microsoftConnector{ redirectURI: c.RedirectURI, clientID: c.ClientID, @@ -76,7 +75,7 @@ type microsoftConnector struct { tenant string onlySecurityGroups bool groups []string - logger logrus.FieldLogger + logger log.Logger } func (c *microsoftConnector) isOrgTenant() bool { diff --git a/connector/mock/connectortest.go b/connector/mock/connectortest.go index ef8afd46..5089914c 100644 --- a/connector/mock/connectortest.go +++ b/connector/mock/connectortest.go @@ -8,14 +8,13 @@ import ( "net/http" "net/url" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) // NewCallbackConnector returns a mock connector which requires no user interaction. It always returns // the same (fake) identity. -func NewCallbackConnector(logger logrus.FieldLogger) connector.Connector { +func NewCallbackConnector(logger log.Logger) connector.Connector { return &Callback{ Identity: connector.Identity{ UserID: "0-385-28089-0", @@ -40,7 +39,7 @@ var ( type Callback struct { // The returned identity. Identity connector.Identity - Logger logrus.FieldLogger + Logger log.Logger } // LoginURL returns the URL to redirect the user to login with. @@ -71,7 +70,7 @@ func (m *Callback) Refresh(ctx context.Context, s connector.Scopes, identity con type CallbackConfig struct{} // Open returns an authentication strategy which requires no user interaction. -func (c *CallbackConfig) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *CallbackConfig) Open(id string, logger log.Logger) (connector.Connector, error) { return NewCallbackConnector(logger), nil } @@ -83,7 +82,7 @@ type PasswordConfig struct { } // Open returns an authentication strategy which prompts for a predefined username and password. -func (c *PasswordConfig) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *PasswordConfig) Open(id string, logger log.Logger) (connector.Connector, error) { if c.Username == "" { return nil, errors.New("no username supplied") } @@ -96,7 +95,7 @@ func (c *PasswordConfig) Open(id string, logger logrus.FieldLogger) (connector.C type passwordConnector struct { username string password string - logger logrus.FieldLogger + logger log.Logger } func (p passwordConnector) Close() error { return nil } diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 4468edc4..ac24acc9 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -11,10 +11,10 @@ import ( "sync" "github.com/coreos/go-oidc" - "github.com/sirupsen/logrus" "golang.org/x/oauth2" "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" ) // Config holds configuration options for OpenID Connect logins. @@ -75,7 +75,7 @@ func registerBrokenAuthHeaderProvider(url string) { // Open returns a connector which can be used to login users through an upstream // OpenID Connect provider. -func (c *Config) Open(id string, logger logrus.FieldLogger) (conn connector.Connector, err error) { +func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, err error) { ctx, cancel := context.WithCancel(context.Background()) provider, err := oidc.NewProvider(ctx, c.Issuer) @@ -130,7 +130,7 @@ type oidcConnector struct { verifier *oidc.IDTokenVerifier ctx context.Context cancel context.CancelFunc - logger logrus.FieldLogger + logger log.Logger hostedDomains []string } diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 2b092b3f..3358583d 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -14,11 +14,10 @@ import ( "time" "github.com/beevik/etree" + "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/log" dsig "github.com/russellhaering/goxmldsig" "github.com/russellhaering/goxmldsig/etreeutils" - "github.com/sirupsen/logrus" - - "github.com/dexidp/dex/connector" ) const ( @@ -126,11 +125,11 @@ func (c certStore) Certificates() (roots []*x509.Certificate, err error) { // Open validates the config and returns a connector. It does not actually // validate connectivity with the provider. -func (c *Config) Open(id string, logger logrus.FieldLogger) (connector.Connector, error) { +func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { return c.openConnector(logger) } -func (c *Config) openConnector(logger logrus.FieldLogger) (*provider, error) { +func (c *Config) openConnector(logger log.Logger) (*provider, error) { requiredFields := []struct { name, val string }{ @@ -241,7 +240,7 @@ type provider struct { nameIDPolicyFormat string - logger logrus.FieldLogger + logger log.Logger } func (p *provider) POSTData(s connector.Scopes, id string) (action, value string, err error) { diff --git a/connector/saml/saml_test.go b/connector/saml/saml_test.go index 4497d059..e7ccbbb8 100644 --- a/connector/saml/saml_test.go +++ b/connector/saml/saml_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/pem" "errors" + "github.com/dexidp/dex/pkg/log" "io/ioutil" "sort" "testing" @@ -304,7 +305,7 @@ func (r responseTest) run(t *testing.T) { t.Fatalf("parse test time: %v", err) } - conn, err := c.openConnector(logrus.New()) + conn, err := c.openConnector(log.NewLogrusLogger(logrus.New())) if err != nil { t.Fatal(err) } @@ -338,7 +339,7 @@ func (r responseTest) run(t *testing.T) { } func TestConfigCAData(t *testing.T) { - logger := logrus.New() + logger := log.NewLogrusLogger(logrus.New()) validPEM, err := ioutil.ReadFile("testdata/ca.crt") if err != nil { t.Fatal(err) @@ -475,7 +476,7 @@ func newProvider(ssoIssuer string, redirectURI string) *provider { usernameAttr: "user", emailAttr: "email", redirectURI: redirectURI, - logger: logrus.New(), + logger: log.NewLogrusLogger(logrus.New()), } } diff --git a/pkg/log/logger.go b/pkg/log/logger.go new file mode 100644 index 00000000..60a651b4 --- /dev/null +++ b/pkg/log/logger.go @@ -0,0 +1,17 @@ +// Package log provides a logger interface for logger libraries +// so that dex does not depend on any of them directly. +// It also includes a default implementation using Logrus (used by dex previously). +package log + +// Logger serves as an adapter interface for logger libraries +// so that dex does not depend on any of them directly. +type Logger interface { + WithField(key string, value interface{}) Logger + + Info(msg string) + Warn(msg string) + + Debugf(format string, args ...interface{}) + Infof(format string, args ...interface{}) + Errorf(format string, args ...interface{}) +} diff --git a/pkg/log/logrus.go b/pkg/log/logrus.go new file mode 100644 index 00000000..4efd6482 --- /dev/null +++ b/pkg/log/logrus.go @@ -0,0 +1,45 @@ +package log + +import "github.com/sirupsen/logrus" + +// LogrusLogger is an adapter for Logrus implementing the Logger interface. +type LogrusLogger struct { + logger logrus.FieldLogger +} + +// NewLogrusLogger returns a new Logger wrapping Logrus. +func NewLogrusLogger(logger logrus.FieldLogger) *LogrusLogger { + return &LogrusLogger{ + logger: logger, + } +} + +// WithField adds a field to the log entry. +func (l *LogrusLogger) WithField(key string, value interface{}) Logger { + return NewLogrusLogger(l.logger.WithField(key, value)) +} + +// Info logs an Info level event. +func (l *LogrusLogger) Info(msg string) { + l.logger.Info(msg) +} + +// Warn logs a Warn level event. +func (l *LogrusLogger) Warn(msg string) { + l.logger.Warn(msg) +} + +// Debugf formats and logs a Debug level event. +func (l *LogrusLogger) Debugf(format string, args ...interface{}) { + l.logger.Debugf(format, args...) +} + +// Infof formats and logs an Info level event. +func (l *LogrusLogger) Infof(format string, args ...interface{}) { + l.logger.Infof(format, args...) +} + +// Errorf formats and logs n Error level event. +func (l *LogrusLogger) Errorf(format string, args ...interface{}) { + l.logger.Errorf(format, args...) +} diff --git a/pkg/log/logrus_test.go b/pkg/log/logrus_test.go new file mode 100644 index 00000000..cb60c6b9 --- /dev/null +++ b/pkg/log/logrus_test.go @@ -0,0 +1,10 @@ +package log + +import "testing" + +func TestLogrusLoggerImplementsLoggerInterface(t *testing.T) { + var i interface{} = new(LogrusLogger) + if _, ok := i.(Logger); !ok { + t.Errorf("expected %T to implement Logger interface", i) + } +} diff --git a/server/api.go b/server/api.go index c1107e4c..f900cd1c 100644 --- a/server/api.go +++ b/server/api.go @@ -10,9 +10,8 @@ import ( // https://github.com/grpc/grpc-go/issues/711 "golang.org/x/net/context" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/api" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/server/internal" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/version" @@ -34,7 +33,7 @@ const ( ) // NewAPI returns a server which implements the gRPC API interface. -func NewAPI(s storage.Storage, logger logrus.FieldLogger) api.DexServer { +func NewAPI(s storage.Storage, logger log.Logger) api.DexServer { return dexAPI{ s: s, logger: logger, @@ -43,7 +42,7 @@ func NewAPI(s storage.Storage, logger logrus.FieldLogger) api.DexServer { type dexAPI struct { s storage.Storage - logger logrus.FieldLogger + logger log.Logger } func (d dexAPI) CreateClient(ctx context.Context, req *api.CreateClientReq) (*api.CreateClientResp, error) { diff --git a/server/api_test.go b/server/api_test.go index 14759e63..dbd67b0a 100644 --- a/server/api_test.go +++ b/server/api_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/grpc" "github.com/dexidp/dex/api" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/server/internal" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/memory" @@ -28,7 +29,7 @@ type apiClient struct { } // newAPI constructs a gRCP client connected to a backing server. -func newAPI(s storage.Storage, logger logrus.FieldLogger, t *testing.T) *apiClient { +func newAPI(s storage.Storage, logger log.Logger, t *testing.T) *apiClient { l, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatal(err) @@ -57,11 +58,11 @@ func newAPI(s storage.Storage, logger logrus.FieldLogger, t *testing.T) *apiClie // Attempts to create, update and delete a test Password func TestPassword(t *testing.T) { - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) s := memory.New(logger) client := newAPI(s, logger, t) @@ -122,11 +123,11 @@ func TestPassword(t *testing.T) { // Ensures checkCost returns expected values func TestCheckCost(t *testing.T) { - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) s := memory.New(logger) client := newAPI(s, logger, t) @@ -179,11 +180,11 @@ func TestCheckCost(t *testing.T) { // Attempts to list and revoke an exisiting refresh token. func TestRefreshToken(t *testing.T) { - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) s := memory.New(logger) client := newAPI(s, logger, t) @@ -292,11 +293,11 @@ func TestRefreshToken(t *testing.T) { } func TestUpdateClient(t *testing.T) { - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) s := memory.New(logger) client := newAPI(s, logger, t) diff --git a/server/rotation.go b/server/rotation.go index bb857450..579fe3d1 100644 --- a/server/rotation.go +++ b/server/rotation.go @@ -12,8 +12,7 @@ import ( "gopkg.in/square/go-jose.v2" - "github.com/sirupsen/logrus" - + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" ) @@ -62,7 +61,7 @@ type keyRotater struct { strategy rotationStrategy now func() time.Time - logger logrus.FieldLogger + logger log.Logger } // startKeyRotation begins key rotation in a new goroutine, closing once the context is canceled. diff --git a/server/rotation_test.go b/server/rotation_test.go index 66c269ce..f77ec783 100644 --- a/server/rotation_test.go +++ b/server/rotation_test.go @@ -8,6 +8,7 @@ import ( "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/memory" ) @@ -67,11 +68,11 @@ func TestKeyRotater(t *testing.T) { // Only the last 5 verification keys are expected to be kept around. maxVerificationKeys := 5 - l := &logrus.Logger{ + l := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) r := &keyRotater{ Storage: memory.New(l), diff --git a/server/server.go b/server/server.go index 58968a1b..49d6fc2e 100644 --- a/server/server.go +++ b/server/server.go @@ -16,12 +16,6 @@ import ( "golang.org/x/crypto/bcrypt" - "github.com/felixge/httpsnoop" - "github.com/gorilla/handlers" - "github.com/gorilla/mux" - "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" - "github.com/dexidp/dex/connector" "github.com/dexidp/dex/connector/authproxy" "github.com/dexidp/dex/connector/bitbucketcloud" @@ -34,7 +28,12 @@ import ( "github.com/dexidp/dex/connector/mock" "github.com/dexidp/dex/connector/oidc" "github.com/dexidp/dex/connector/saml" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" + "github.com/felixge/httpsnoop" + "github.com/gorilla/handlers" + "github.com/gorilla/mux" + "github.com/prometheus/client_golang/prometheus" ) // LocalConnector is the local passwordDB connector which is an internal @@ -80,7 +79,7 @@ type Config struct { Web WebConfig - Logger logrus.FieldLogger + Logger log.Logger PrometheusRegistry *prometheus.Registry } @@ -142,7 +141,7 @@ type Server struct { idTokensValidFor time.Duration authRequestsValidFor time.Duration - logger logrus.FieldLogger + logger log.Logger } // NewServer constructs a server from the provided config. @@ -431,7 +430,7 @@ func (s *Server) startGarbageCollection(ctx context.Context, frequency time.Dura // ConnectorConfig is a configuration that can open a connector. type ConnectorConfig interface { - Open(id string, logger logrus.FieldLogger) (connector.Connector, error) + Open(id string, logger log.Logger) (connector.Connector, error) } // ConnectorsConfig variable provides an easy way to return a config struct @@ -454,7 +453,7 @@ var ConnectorsConfig = map[string]func() ConnectorConfig{ } // openConnector will parse the connector config and open the connector. -func openConnector(logger logrus.FieldLogger, conn storage.Connector) (connector.Connector, error) { +func openConnector(logger log.Logger, conn storage.Connector) (connector.Connector, error) { var c connector.Connector f, ok := ConnectorsConfig[conn.Type] diff --git a/server/server_test.go b/server/server_test.go index 536387c4..1433162b 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -30,6 +30,7 @@ import ( "github.com/dexidp/dex/connector" "github.com/dexidp/dex/connector/mock" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/memory" ) @@ -74,11 +75,11 @@ FDWV28nTP9sqbtsmU8Tem2jzMvZ7C/Q0AuDoKELFUpux8shm8wfIhyaPnXUGZoAZ Np4vUwMSYV5mopESLWOg3loBxKyLGFtgGKVCjGiQvy6zISQ4fQo= -----END RSA PRIVATE KEY-----`) -var logger = &logrus.Logger{ +var logger = log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, -} +}) func newTestServer(ctx context.Context, t *testing.T, updateConfig func(c *Config)) (*httptest.Server, *Server) { var server *Server diff --git a/storage/etcd/config.go b/storage/etcd/config.go index 260ec257..f36a11bb 100644 --- a/storage/etcd/config.go +++ b/storage/etcd/config.go @@ -6,8 +6,8 @@ import ( "github.com/coreos/etcd/clientv3" "github.com/coreos/etcd/clientv3/namespace" "github.com/coreos/etcd/pkg/transport" - "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" ) @@ -36,11 +36,11 @@ type Etcd struct { } // Open creates a new storage implementation backed by Etcd -func (p *Etcd) Open(logger logrus.FieldLogger) (storage.Storage, error) { +func (p *Etcd) Open(logger log.Logger) (storage.Storage, error) { return p.open(logger) } -func (p *Etcd) open(logger logrus.FieldLogger) (*conn, error) { +func (p *Etcd) open(logger log.Logger) (*conn, error) { cfg := clientv3.Config{ Endpoints: p.Endpoints, DialTimeout: defaultDialTimeout, diff --git a/storage/etcd/etcd.go b/storage/etcd/etcd.go index e323f5ab..d106f443 100644 --- a/storage/etcd/etcd.go +++ b/storage/etcd/etcd.go @@ -8,8 +8,8 @@ import ( "time" "github.com/coreos/etcd/clientv3" - "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" ) @@ -29,7 +29,7 @@ const ( type conn struct { db *clientv3.Client - logger logrus.FieldLogger + logger log.Logger } func (c *conn) Close() error { diff --git a/storage/etcd/etcd_test.go b/storage/etcd/etcd_test.go index bd412536..ac5ab175 100644 --- a/storage/etcd/etcd_test.go +++ b/storage/etcd/etcd_test.go @@ -3,6 +3,7 @@ package etcd import ( "context" "fmt" + "github.com/dexidp/dex/pkg/log" "os" "runtime" "strings" @@ -53,11 +54,11 @@ func cleanDB(c *conn) error { return nil } -var logger = &logrus.Logger{ +var logger = log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, -} +}) func TestEtcd(t *testing.T) { testEtcdEnv := "DEX_ETCD_ENDPOINTS" diff --git a/storage/kubernetes/client.go b/storage/kubernetes/client.go index cb7fd280..d6d81f98 100644 --- a/storage/kubernetes/client.go +++ b/storage/kubernetes/client.go @@ -24,9 +24,9 @@ import ( "github.com/ghodss/yaml" "github.com/gtank/cryptopasta" - "github.com/sirupsen/logrus" "golang.org/x/net/http2" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/kubernetes/k8sapi" ) @@ -35,7 +35,7 @@ type client struct { client *http.Client baseURL string namespace string - logger logrus.FieldLogger + logger log.Logger // Hash function to map IDs (which could span a large range) to Kubernetes names. // While this is not currently upgradable, it could be in the future. @@ -253,7 +253,7 @@ func (c *client) put(resource, name string, v interface{}) error { return checkHTTPErr(resp, http.StatusOK) } -func newClient(cluster k8sapi.Cluster, user k8sapi.AuthInfo, namespace string, logger logrus.FieldLogger, useTPR bool) (*client, error) { +func newClient(cluster k8sapi.Cluster, user k8sapi.AuthInfo, namespace string, logger log.Logger, useTPR bool) (*client, error) { tlsConfig := cryptopasta.DefaultTLSConfig() data := func(b string, file string) ([]byte, error) { if b != "" { diff --git a/storage/kubernetes/storage.go b/storage/kubernetes/storage.go index 86f3f209..193c0503 100644 --- a/storage/kubernetes/storage.go +++ b/storage/kubernetes/storage.go @@ -7,8 +7,7 @@ import ( "strings" "time" - "github.com/sirupsen/logrus" - + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/kubernetes/k8sapi" ) @@ -43,7 +42,7 @@ type Config struct { } // Open returns a storage using Kubernetes third party resource. -func (c *Config) Open(logger logrus.FieldLogger) (storage.Storage, error) { +func (c *Config) Open(logger log.Logger) (storage.Storage, error) { cli, err := c.open(logger, false) if err != nil { return nil, err @@ -56,7 +55,7 @@ func (c *Config) Open(logger logrus.FieldLogger) (storage.Storage, error) { // // waitForResources controls if errors creating the resources cause this method to return // immediately (used during testing), or if the client will asynchronously retry. -func (c *Config) open(logger logrus.FieldLogger, waitForResources bool) (*client, error) { +func (c *Config) open(logger log.Logger, waitForResources bool) (*client, error) { if c.InCluster && (c.KubeConfigFile != "") { return nil, errors.New("cannot specify both 'inCluster' and 'kubeConfigFile'") } diff --git a/storage/kubernetes/storage_test.go b/storage/kubernetes/storage_test.go index 27d65416..d6e4a1af 100644 --- a/storage/kubernetes/storage_test.go +++ b/storage/kubernetes/storage_test.go @@ -7,6 +7,7 @@ import ( "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/conformance" ) @@ -24,11 +25,11 @@ func loadClient(t *testing.T) *client { if config.KubeConfigFile == "" { t.Skipf("test environment variable %q not set, skipping", testKubeConfigEnv) } - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) s, err := config.open(logger, true) if err != nil { t.Fatal(err) diff --git a/storage/memory/memory.go b/storage/memory/memory.go index d8817fbd..681d204e 100644 --- a/storage/memory/memory.go +++ b/storage/memory/memory.go @@ -6,13 +6,12 @@ import ( "sync" "time" - "github.com/sirupsen/logrus" - + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" ) // New returns an in memory storage. -func New(logger logrus.FieldLogger) storage.Storage { +func New(logger log.Logger) storage.Storage { return &memStorage{ clients: make(map[string]storage.Client), authCodes: make(map[string]storage.AuthCode), @@ -33,7 +32,7 @@ type Config struct { } // Open always returns a new in memory storage. -func (c *Config) Open(logger logrus.FieldLogger) (storage.Storage, error) { +func (c *Config) Open(logger log.Logger) (storage.Storage, error) { return New(logger), nil } @@ -50,7 +49,7 @@ type memStorage struct { keys storage.Keys - logger logrus.FieldLogger + logger log.Logger } type offlineSessionID struct { diff --git a/storage/memory/memory_test.go b/storage/memory/memory_test.go index 84a8826e..804bd6fe 100644 --- a/storage/memory/memory_test.go +++ b/storage/memory/memory_test.go @@ -6,16 +6,17 @@ import ( "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/conformance" ) func TestStorage(t *testing.T) { - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) newStorage := func() storage.Storage { return New(logger) diff --git a/storage/memory/static_test.go b/storage/memory/static_test.go index 8513e0ee..8ef8b270 100644 --- a/storage/memory/static_test.go +++ b/storage/memory/static_test.go @@ -8,15 +8,16 @@ import ( "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" ) func TestStaticClients(t *testing.T) { - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) backing := New(logger) c1 := storage.Client{ID: "foo", Secret: "foo_secret"} @@ -99,11 +100,11 @@ func TestStaticClients(t *testing.T) { } func TestStaticPasswords(t *testing.T) { - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) backing := New(logger) p1 := storage.Password{Email: "foo@example.com", Username: "foo_secret"} @@ -211,11 +212,11 @@ func TestStaticPasswords(t *testing.T) { } func TestStaticConnectors(t *testing.T) { - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) backing := New(logger) config1 := []byte(`{"issuer": "https://accounts.google.com"}`) diff --git a/storage/sql/config.go b/storage/sql/config.go index 07fe6cf9..8fc715b8 100644 --- a/storage/sql/config.go +++ b/storage/sql/config.go @@ -11,8 +11,8 @@ import ( "github.com/lib/pq" sqlite3 "github.com/mattn/go-sqlite3" - "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" ) @@ -28,7 +28,7 @@ type SQLite3 struct { } // Open creates a new storage implementation backed by SQLite3 -func (s *SQLite3) Open(logger logrus.FieldLogger) (storage.Storage, error) { +func (s *SQLite3) Open(logger log.Logger) (storage.Storage, error) { conn, err := s.open(logger) if err != nil { return nil, err @@ -36,7 +36,7 @@ func (s *SQLite3) Open(logger logrus.FieldLogger) (storage.Storage, error) { return conn, nil } -func (s *SQLite3) open(logger logrus.FieldLogger) (*conn, error) { +func (s *SQLite3) open(logger log.Logger) (*conn, error) { db, err := sql.Open("sqlite3", s.File) if err != nil { return nil, err @@ -99,7 +99,7 @@ type Postgres struct { } // Open creates a new storage implementation backed by Postgres. -func (p *Postgres) Open(logger logrus.FieldLogger) (storage.Storage, error) { +func (p *Postgres) Open(logger log.Logger) (storage.Storage, error) { conn, err := p.open(logger, p.createDataSourceName()) if err != nil { return nil, err @@ -179,7 +179,7 @@ func (p *Postgres) createDataSourceName() string { return strings.Join(parameters, " ") } -func (p *Postgres) open(logger logrus.FieldLogger, dataSourceName string) (*conn, error) { +func (p *Postgres) open(logger log.Logger, dataSourceName string) (*conn, error) { db, err := sql.Open("postgres", dataSourceName) if err != nil { return nil, err diff --git a/storage/sql/config_test.go b/storage/sql/config_test.go index d823e099..6cb52d36 100644 --- a/storage/sql/config_test.go +++ b/storage/sql/config_test.go @@ -9,6 +9,7 @@ import ( "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/conformance" ) @@ -43,11 +44,11 @@ func cleanDB(c *conn) error { return err } -var logger = &logrus.Logger{ +var logger = log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, -} +}) func TestSQLite3(t *testing.T) { newStorage := func() storage.Storage { diff --git a/storage/sql/migrate_test.go b/storage/sql/migrate_test.go index e94e819f..d9bfa98a 100644 --- a/storage/sql/migrate_test.go +++ b/storage/sql/migrate_test.go @@ -7,6 +7,8 @@ import ( sqlite3 "github.com/mattn/go-sqlite3" "github.com/sirupsen/logrus" + + "github.com/dexidp/dex/pkg/log" ) func TestMigrate(t *testing.T) { @@ -16,11 +18,11 @@ func TestMigrate(t *testing.T) { } defer db.Close() - logger := &logrus.Logger{ + logger := log.NewLogrusLogger(&logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - } + }) errCheck := func(err error) bool { sqlErr, ok := err.(sqlite3.Error) diff --git a/storage/sql/sql.go b/storage/sql/sql.go index dc6be4a1..d27b67ce 100644 --- a/storage/sql/sql.go +++ b/storage/sql/sql.go @@ -3,11 +3,10 @@ package sql import ( "database/sql" + "github.com/dexidp/dex/pkg/log" "regexp" "time" - "github.com/sirupsen/logrus" - // import third party drivers _ "github.com/lib/pq" _ "github.com/mattn/go-sqlite3" @@ -113,7 +112,7 @@ func (c *conn) translateArgs(args []interface{}) []interface{} { type conn struct { db *sql.DB flavor flavor - logger logrus.FieldLogger + logger log.Logger alreadyExistsCheck func(err error) bool } diff --git a/storage/static.go b/storage/static.go index 5ae4f783..8a383853 100644 --- a/storage/static.go +++ b/storage/static.go @@ -4,7 +4,7 @@ import ( "errors" "strings" - "github.com/sirupsen/logrus" + "github.com/dexidp/dex/pkg/log" ) // Tests for this code are in the "memory" package, since this package doesn't @@ -89,11 +89,11 @@ type staticPasswordsStorage struct { // A map of passwords that is indexed by lower-case email ids passwordsByEmail map[string]Password - logger logrus.FieldLogger + logger log.Logger } // WithStaticPasswords returns a storage with a read-only set of passwords. -func WithStaticPasswords(s Storage, staticPasswords []Password, logger logrus.FieldLogger) Storage { +func WithStaticPasswords(s Storage, staticPasswords []Password, logger log.Logger) Storage { passwordsByEmail := make(map[string]Password, len(staticPasswords)) for _, p := range staticPasswords { //Enable case insensitive email comparison. From d1c8f8d095d2478fbb0ac55c5ad91500469c9833 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Fri, 22 Feb 2019 21:26:30 +0100 Subject: [PATCH 2/5] Remove structured logging from the logger interface --- pkg/log/logger.go | 2 -- pkg/log/logrus.go | 5 ----- server/server.go | 2 +- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/pkg/log/logger.go b/pkg/log/logger.go index 60a651b4..bf276f48 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -6,8 +6,6 @@ package log // Logger serves as an adapter interface for logger libraries // so that dex does not depend on any of them directly. type Logger interface { - WithField(key string, value interface{}) Logger - Info(msg string) Warn(msg string) diff --git a/pkg/log/logrus.go b/pkg/log/logrus.go index 4efd6482..c7acd6b2 100644 --- a/pkg/log/logrus.go +++ b/pkg/log/logrus.go @@ -14,11 +14,6 @@ func NewLogrusLogger(logger logrus.FieldLogger) *LogrusLogger { } } -// WithField adds a field to the log entry. -func (l *LogrusLogger) WithField(key string, value interface{}) Logger { - return NewLogrusLogger(l.logger.WithField(key, value)) -} - // Info logs an Info level event. func (l *LogrusLogger) Info(msg string) { l.logger.Info(msg) diff --git a/server/server.go b/server/server.go index 49d6fc2e..c338584e 100644 --- a/server/server.go +++ b/server/server.go @@ -485,7 +485,7 @@ func (s *Server) OpenConnector(conn storage.Connector) (Connector, error) { c = newPasswordDB(s.storage) } else { var err error - c, err = openConnector(s.logger.WithField("connector", conn.Name), conn) + c, err = openConnector(s.logger, conn) if err != nil { return Connector{}, fmt.Errorf("failed to open connector: %v", err) } From aec2edb4413a7259aee8b80c09822036adcb0957 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Fri, 22 Feb 2019 21:27:54 +0100 Subject: [PATCH 3/5] Match the interface to logrus implementation --- pkg/log/logger.go | 4 ++-- pkg/log/logrus.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/log/logger.go b/pkg/log/logger.go index bf276f48..18de32aa 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -6,8 +6,8 @@ package log // Logger serves as an adapter interface for logger libraries // so that dex does not depend on any of them directly. type Logger interface { - Info(msg string) - Warn(msg string) + Info(args ...interface{}) + Warn(args ...interface{}) Debugf(format string, args ...interface{}) Infof(format string, args ...interface{}) diff --git a/pkg/log/logrus.go b/pkg/log/logrus.go index c7acd6b2..839f79e6 100644 --- a/pkg/log/logrus.go +++ b/pkg/log/logrus.go @@ -15,13 +15,13 @@ func NewLogrusLogger(logger logrus.FieldLogger) *LogrusLogger { } // Info logs an Info level event. -func (l *LogrusLogger) Info(msg string) { - l.logger.Info(msg) +func (l *LogrusLogger) Info(args ...interface{}) { + l.logger.Info(args...) } // Warn logs a Warn level event. -func (l *LogrusLogger) Warn(msg string) { - l.logger.Warn(msg) +func (l *LogrusLogger) Warn(args ...interface{}) { + l.logger.Warn(args...) } // Debugf formats and logs a Debug level event. From 06521ffa49b6d7ea81a756f1e297abd4ec39a071 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Fri, 22 Feb 2019 21:31:46 +0100 Subject: [PATCH 4/5] Remove the logrus logger wrapper --- cmd/dex/serve.go | 4 +-- connector/ldap/ldap_test.go | 3 +-- connector/saml/saml_test.go | 7 +++--- pkg/log/logrus.go | 40 ------------------------------ pkg/log/logrus_test.go | 10 -------- server/api_test.go | 16 ++++++------ server/rotation_test.go | 5 ++-- server/server_test.go | 5 ++-- storage/etcd/etcd_test.go | 5 ++-- storage/kubernetes/storage_test.go | 5 ++-- storage/memory/memory_test.go | 5 ++-- storage/memory/static_test.go | 13 +++++----- storage/sql/config_test.go | 5 ++-- storage/sql/migrate_test.go | 6 ++--- 14 files changed, 34 insertions(+), 95 deletions(-) delete mode 100644 pkg/log/logrus.go delete mode 100644 pkg/log/logrus_test.go diff --git a/cmd/dex/serve.go b/cmd/dex/serve.go index 0933eb4f..8ebaaea0 100644 --- a/cmd/dex/serve.go +++ b/cmd/dex/serve.go @@ -348,9 +348,9 @@ func newLogger(level string, format string) (log.Logger, error) { return nil, fmt.Errorf("log format is not one of the supported values (%s): %s", strings.Join(logFormats, ", "), format) } - return log.NewLogrusLogger(&logrus.Logger{ + return &logrus.Logger{ Out: os.Stderr, Formatter: &formatter, Level: logLevel, - }), nil + }, nil } diff --git a/connector/ldap/ldap_test.go b/connector/ldap/ldap_test.go index c6f30807..fa6b7062 100644 --- a/connector/ldap/ldap_test.go +++ b/connector/ldap/ldap_test.go @@ -17,7 +17,6 @@ import ( "github.com/sirupsen/logrus" "github.com/dexidp/dex/connector" - "github.com/dexidp/dex/pkg/log" ) const envVar = "DEX_LDAP_TESTS" @@ -876,7 +875,7 @@ func runTests(t *testing.T, schema string, connMethod connectionMethod, config * c.BindDN = "cn=admin,dc=example,dc=org" c.BindPW = "admin" - l := log.NewLogrusLogger(&logrus.Logger{Out: ioutil.Discard, Formatter: &logrus.TextFormatter{}}) + l := &logrus.Logger{Out: ioutil.Discard, Formatter: &logrus.TextFormatter{}} conn, err := c.openConnector(l) if err != nil { diff --git a/connector/saml/saml_test.go b/connector/saml/saml_test.go index e7ccbbb8..4497d059 100644 --- a/connector/saml/saml_test.go +++ b/connector/saml/saml_test.go @@ -5,7 +5,6 @@ import ( "encoding/base64" "encoding/pem" "errors" - "github.com/dexidp/dex/pkg/log" "io/ioutil" "sort" "testing" @@ -305,7 +304,7 @@ func (r responseTest) run(t *testing.T) { t.Fatalf("parse test time: %v", err) } - conn, err := c.openConnector(log.NewLogrusLogger(logrus.New())) + conn, err := c.openConnector(logrus.New()) if err != nil { t.Fatal(err) } @@ -339,7 +338,7 @@ func (r responseTest) run(t *testing.T) { } func TestConfigCAData(t *testing.T) { - logger := log.NewLogrusLogger(logrus.New()) + logger := logrus.New() validPEM, err := ioutil.ReadFile("testdata/ca.crt") if err != nil { t.Fatal(err) @@ -476,7 +475,7 @@ func newProvider(ssoIssuer string, redirectURI string) *provider { usernameAttr: "user", emailAttr: "email", redirectURI: redirectURI, - logger: log.NewLogrusLogger(logrus.New()), + logger: logrus.New(), } } diff --git a/pkg/log/logrus.go b/pkg/log/logrus.go deleted file mode 100644 index 839f79e6..00000000 --- a/pkg/log/logrus.go +++ /dev/null @@ -1,40 +0,0 @@ -package log - -import "github.com/sirupsen/logrus" - -// LogrusLogger is an adapter for Logrus implementing the Logger interface. -type LogrusLogger struct { - logger logrus.FieldLogger -} - -// NewLogrusLogger returns a new Logger wrapping Logrus. -func NewLogrusLogger(logger logrus.FieldLogger) *LogrusLogger { - return &LogrusLogger{ - logger: logger, - } -} - -// Info logs an Info level event. -func (l *LogrusLogger) Info(args ...interface{}) { - l.logger.Info(args...) -} - -// Warn logs a Warn level event. -func (l *LogrusLogger) Warn(args ...interface{}) { - l.logger.Warn(args...) -} - -// Debugf formats and logs a Debug level event. -func (l *LogrusLogger) Debugf(format string, args ...interface{}) { - l.logger.Debugf(format, args...) -} - -// Infof formats and logs an Info level event. -func (l *LogrusLogger) Infof(format string, args ...interface{}) { - l.logger.Infof(format, args...) -} - -// Errorf formats and logs n Error level event. -func (l *LogrusLogger) Errorf(format string, args ...interface{}) { - l.logger.Errorf(format, args...) -} diff --git a/pkg/log/logrus_test.go b/pkg/log/logrus_test.go deleted file mode 100644 index cb60c6b9..00000000 --- a/pkg/log/logrus_test.go +++ /dev/null @@ -1,10 +0,0 @@ -package log - -import "testing" - -func TestLogrusLoggerImplementsLoggerInterface(t *testing.T) { - var i interface{} = new(LogrusLogger) - if _, ok := i.(Logger); !ok { - t.Errorf("expected %T to implement Logger interface", i) - } -} diff --git a/server/api_test.go b/server/api_test.go index dbd67b0a..568748bf 100644 --- a/server/api_test.go +++ b/server/api_test.go @@ -58,11 +58,11 @@ func newAPI(s storage.Storage, logger log.Logger, t *testing.T) *apiClient { // Attempts to create, update and delete a test Password func TestPassword(t *testing.T) { - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } s := memory.New(logger) client := newAPI(s, logger, t) @@ -123,11 +123,11 @@ func TestPassword(t *testing.T) { // Ensures checkCost returns expected values func TestCheckCost(t *testing.T) { - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } s := memory.New(logger) client := newAPI(s, logger, t) @@ -180,11 +180,11 @@ func TestCheckCost(t *testing.T) { // Attempts to list and revoke an exisiting refresh token. func TestRefreshToken(t *testing.T) { - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } s := memory.New(logger) client := newAPI(s, logger, t) @@ -293,11 +293,11 @@ func TestRefreshToken(t *testing.T) { } func TestUpdateClient(t *testing.T) { - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } s := memory.New(logger) client := newAPI(s, logger, t) diff --git a/server/rotation_test.go b/server/rotation_test.go index f77ec783..66c269ce 100644 --- a/server/rotation_test.go +++ b/server/rotation_test.go @@ -8,7 +8,6 @@ import ( "github.com/sirupsen/logrus" - "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/memory" ) @@ -68,11 +67,11 @@ func TestKeyRotater(t *testing.T) { // Only the last 5 verification keys are expected to be kept around. maxVerificationKeys := 5 - l := log.NewLogrusLogger(&logrus.Logger{ + l := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } r := &keyRotater{ Storage: memory.New(l), diff --git a/server/server_test.go b/server/server_test.go index 1433162b..536387c4 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -30,7 +30,6 @@ import ( "github.com/dexidp/dex/connector" "github.com/dexidp/dex/connector/mock" - "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/memory" ) @@ -75,11 +74,11 @@ FDWV28nTP9sqbtsmU8Tem2jzMvZ7C/Q0AuDoKELFUpux8shm8wfIhyaPnXUGZoAZ Np4vUwMSYV5mopESLWOg3loBxKyLGFtgGKVCjGiQvy6zISQ4fQo= -----END RSA PRIVATE KEY-----`) -var logger = log.NewLogrusLogger(&logrus.Logger{ +var logger = &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, -}) +} func newTestServer(ctx context.Context, t *testing.T, updateConfig func(c *Config)) (*httptest.Server, *Server) { var server *Server diff --git a/storage/etcd/etcd_test.go b/storage/etcd/etcd_test.go index ac5ab175..bd412536 100644 --- a/storage/etcd/etcd_test.go +++ b/storage/etcd/etcd_test.go @@ -3,7 +3,6 @@ package etcd import ( "context" "fmt" - "github.com/dexidp/dex/pkg/log" "os" "runtime" "strings" @@ -54,11 +53,11 @@ func cleanDB(c *conn) error { return nil } -var logger = log.NewLogrusLogger(&logrus.Logger{ +var logger = &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, -}) +} func TestEtcd(t *testing.T) { testEtcdEnv := "DEX_ETCD_ENDPOINTS" diff --git a/storage/kubernetes/storage_test.go b/storage/kubernetes/storage_test.go index d6e4a1af..27d65416 100644 --- a/storage/kubernetes/storage_test.go +++ b/storage/kubernetes/storage_test.go @@ -7,7 +7,6 @@ import ( "github.com/sirupsen/logrus" - "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/conformance" ) @@ -25,11 +24,11 @@ func loadClient(t *testing.T) *client { if config.KubeConfigFile == "" { t.Skipf("test environment variable %q not set, skipping", testKubeConfigEnv) } - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } s, err := config.open(logger, true) if err != nil { t.Fatal(err) diff --git a/storage/memory/memory_test.go b/storage/memory/memory_test.go index 804bd6fe..84a8826e 100644 --- a/storage/memory/memory_test.go +++ b/storage/memory/memory_test.go @@ -6,17 +6,16 @@ import ( "github.com/sirupsen/logrus" - "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/conformance" ) func TestStorage(t *testing.T) { - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } newStorage := func() storage.Storage { return New(logger) diff --git a/storage/memory/static_test.go b/storage/memory/static_test.go index 8ef8b270..8513e0ee 100644 --- a/storage/memory/static_test.go +++ b/storage/memory/static_test.go @@ -8,16 +8,15 @@ import ( "github.com/sirupsen/logrus" - "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" ) func TestStaticClients(t *testing.T) { - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } backing := New(logger) c1 := storage.Client{ID: "foo", Secret: "foo_secret"} @@ -100,11 +99,11 @@ func TestStaticClients(t *testing.T) { } func TestStaticPasswords(t *testing.T) { - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } backing := New(logger) p1 := storage.Password{Email: "foo@example.com", Username: "foo_secret"} @@ -212,11 +211,11 @@ func TestStaticPasswords(t *testing.T) { } func TestStaticConnectors(t *testing.T) { - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } backing := New(logger) config1 := []byte(`{"issuer": "https://accounts.google.com"}`) diff --git a/storage/sql/config_test.go b/storage/sql/config_test.go index 6cb52d36..d823e099 100644 --- a/storage/sql/config_test.go +++ b/storage/sql/config_test.go @@ -9,7 +9,6 @@ import ( "github.com/sirupsen/logrus" - "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/conformance" ) @@ -44,11 +43,11 @@ func cleanDB(c *conn) error { return err } -var logger = log.NewLogrusLogger(&logrus.Logger{ +var logger = &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, -}) +} func TestSQLite3(t *testing.T) { newStorage := func() storage.Storage { diff --git a/storage/sql/migrate_test.go b/storage/sql/migrate_test.go index d9bfa98a..e94e819f 100644 --- a/storage/sql/migrate_test.go +++ b/storage/sql/migrate_test.go @@ -7,8 +7,6 @@ import ( sqlite3 "github.com/mattn/go-sqlite3" "github.com/sirupsen/logrus" - - "github.com/dexidp/dex/pkg/log" ) func TestMigrate(t *testing.T) { @@ -18,11 +16,11 @@ func TestMigrate(t *testing.T) { } defer db.Close() - logger := log.NewLogrusLogger(&logrus.Logger{ + logger := &logrus.Logger{ Out: os.Stderr, Formatter: &logrus.TextFormatter{DisableColors: true}, Level: logrus.DebugLevel, - }) + } errCheck := func(err error) bool { sqlErr, ok := err.(sqlite3.Error) From d877fca09260e55e919690dc72e6be448f375fd2 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Fri, 22 Feb 2019 21:43:55 +0100 Subject: [PATCH 5/5] Fix coding style --- storage/sql/sql.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storage/sql/sql.go b/storage/sql/sql.go index d27b67ce..26e65f7d 100644 --- a/storage/sql/sql.go +++ b/storage/sql/sql.go @@ -3,13 +3,14 @@ package sql import ( "database/sql" - "github.com/dexidp/dex/pkg/log" "regexp" "time" // import third party drivers _ "github.com/lib/pq" _ "github.com/mattn/go-sqlite3" + + "github.com/dexidp/dex/pkg/log" ) // flavor represents a specific SQL implementation, and is used to translate query strings