From 53e8284b637a3100d11bf9fa456c164433da96af Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Tue, 27 Aug 2019 14:48:44 +0900 Subject: [PATCH] Move to k8s.io/klog (#139) --- docs/dex.md | 6 +- docs/google.md | 6 +- docs/keycloak.md | 8 +-- e2e_test/logger/logger.go | 39 +++++++++--- go.mod | 1 + pkg/adaptors/cmd/cmd.go | 2 +- pkg/adaptors/cmd/cmd_test.go | 53 ++++++++-------- pkg/adaptors/cmd/get_token.go | 2 - pkg/adaptors/cmd/root.go | 4 +- pkg/adaptors/interfaces.go | 13 ++-- pkg/adaptors/logger/logger.go | 44 ++++++------- pkg/adaptors/logger/logger_test.go | 62 ------------------- pkg/adaptors/mock_adaptors/logger.go | 41 ++++++++---- pkg/adaptors/mock_adaptors/mock_adaptors.go | 34 +++++----- pkg/adaptors/oidc/logging/transport.go | 26 +++----- pkg/adaptors/oidc/logging/transport_test.go | 43 +------------ pkg/adaptors/oidc/oidc.go | 6 +- pkg/adaptors/oidc/tls/tls.go | 6 +- pkg/adaptors/oidc/tls/tls_test.go | 1 - pkg/usecases/auth/auth.go | 16 ++--- pkg/usecases/auth/auth_test.go | 14 ++--- pkg/usecases/credentialplugin/get_token.go | 10 +-- .../credentialplugin/get_token_test.go | 6 +- pkg/usecases/login/login.go | 10 +-- pkg/usecases/login/login_test.go | 10 +-- 25 files changed, 189 insertions(+), 274 deletions(-) delete mode 100644 pkg/adaptors/logger/logger_test.go diff --git a/docs/dex.md b/docs/dex.md index e20cc5e..3fa4276 100644 --- a/docs/dex.md +++ b/docs/dex.md @@ -49,9 +49,9 @@ kubectl oidc-login get-token -v1 \ You should get claims like: ``` -17:21:32.052655 get_token.go:57: ID token has the claim: iss=https://dex.example.com -17:21:32.052672 get_token.go:57: ID token has the claim: sub=YOUR_SUBJECT -17:21:32.052683 get_token.go:57: ID token has the claim: aud=kubernetes +I0827 12:29:03.086531 23722 get_token.go:59] the ID token has the claim: aud=kubernetes +I0827 12:29:03.086553 23722 get_token.go:59] the ID token has the claim: iss=https://dex.example.com +I0827 12:29:03.086561 23722 get_token.go:59] the ID token has the claim: sub=YOUR_SUBJECT ``` ## 3. Setup Kubernetes API server diff --git a/docs/google.md b/docs/google.md index 1c4769c..6ed9078 100644 --- a/docs/google.md +++ b/docs/google.md @@ -25,9 +25,9 @@ kubectl oidc-login get-token -v1 \ You should get claims like: ``` -17:21:32.052655 get_token.go:57: ID token has the claim: iss=https://accounts.google.com -17:21:32.052672 get_token.go:57: ID token has the claim: sub=YOUR_SUBJECT -17:21:32.052683 get_token.go:57: ID token has the claim: aud=YOUR_CLIENT_ID.apps.googleusercontent.com +I0827 12:29:03.086531 23722 get_token.go:59] the ID token has the claim: aud=YOUR_CLIENT_ID.apps.googleusercontent.com +I0827 12:29:03.086553 23722 get_token.go:59] the ID token has the claim: iss=https://accounts.google.com +I0827 12:29:03.086561 23722 get_token.go:59] the ID token has the claim: sub=YOUR_SUBJECT ``` ## 2. Setup Kubernetes API server diff --git a/docs/keycloak.md b/docs/keycloak.md index d94bc3f..9fb242c 100644 --- a/docs/keycloak.md +++ b/docs/keycloak.md @@ -40,10 +40,10 @@ kubectl oidc-login get-token -v1 \ You should get claims like: ``` -17:21:32.052655 get_token.go:57: ID token has the claim: iss=https://keycloak.example.com/auth/realms/YOUR_REALM -17:21:32.052672 get_token.go:57: ID token has the claim: sub=YOUR_SUBJECT -17:21:32.052683 get_token.go:57: ID token has the claim: aud=kubernetes -17:21:32.052694 get_token.go:57: ID token has the claim: groups=[kubernetes:admin] +I0827 12:29:03.086476 23722 get_token.go:59] the ID token has the claim: groups=[kubernetes:admin] +I0827 12:29:03.086531 23722 get_token.go:59] the ID token has the claim: aud=kubernetes +I0827 12:29:03.086553 23722 get_token.go:59] the ID token has the claim: iss=https://keycloak.example.com/auth/realms/YOUR_REALM +I0827 12:29:03.086561 23722 get_token.go:59] the ID token has the claim: sub=f08655e2-901f-48e5-8c64-bb9f7784d5df ``` ## 2. Setup Kubernetes API server diff --git a/e2e_test/logger/logger.go b/e2e_test/logger/logger.go index c9dee56..d9733f2 100644 --- a/e2e_test/logger/logger.go +++ b/e2e_test/logger/logger.go @@ -1,27 +1,46 @@ package logger import ( - "github.com/int128/kubelogin/pkg/adaptors/logger" + "fmt" + + "github.com/int128/kubelogin/pkg/adaptors" + "github.com/spf13/pflag" ) -func New(t testingLogger) *logger.Logger { - b := &bridge{t} - return logger.NewWith(b, b) +func New(t testingLogger) *Logger { + return &Logger{t} } type testingLogger interface { Logf(format string, v ...interface{}) } -type bridge struct { +// Logger provides logging facility using testing.T. +type Logger struct { t testingLogger } -func (b *bridge) Printf(format string, v ...interface{}) { - b.t.Logf(format, v...) +func (*Logger) AddFlags(f *pflag.FlagSet) { + f.IntP("v", "v", 0, "dummy flag used in the tests") } -func (b *bridge) Output(calldepth int, s string) error { - b.t.Logf("%s", s) - return nil +func (l *Logger) Printf(format string, args ...interface{}) { + l.t.Logf(format, args...) +} + +type Verbose struct { + t testingLogger + level int +} + +func (v *Verbose) Infof(format string, args ...interface{}) { + v.t.Logf(fmt.Sprintf("I%d] ", v.level)+format, args...) +} + +func (l *Logger) V(level int) adaptors.Verbose { + return &Verbose{l.t, level} +} + +func (*Logger) IsEnabled(level int) bool { + return true } diff --git a/go.mod b/go.mod index 3fdce37..2bb29de 100644 --- a/go.mod +++ b/go.mod @@ -20,4 +20,5 @@ require ( gopkg.in/yaml.v2 v2.2.2 k8s.io/apimachinery v0.0.0-20190612205821-1799e75a0719 k8s.io/client-go v0.0.0-20190620085101-78d2af792bab + k8s.io/klog v0.3.1 ) diff --git a/pkg/adaptors/cmd/cmd.go b/pkg/adaptors/cmd/cmd.go index 4554cc2..1b53b57 100644 --- a/pkg/adaptors/cmd/cmd.go +++ b/pkg/adaptors/cmd/cmd.go @@ -63,7 +63,7 @@ func (cmd *Cmd) Run(ctx context.Context, args []string, version string) int { rootCmd.SetArgs(args[1:]) if err := rootCmd.Execute(); err != nil { cmd.Logger.Printf("error: %s", err) - cmd.Logger.Debugf(1, "stacktrace: %+v", err) + cmd.Logger.V(1).Infof("stacktrace: %+v", err) return 1 } return 0 diff --git a/pkg/adaptors/cmd/cmd_test.go b/pkg/adaptors/cmd/cmd_test.go index af9ea0a..ca870b7 100644 --- a/pkg/adaptors/cmd/cmd_test.go +++ b/pkg/adaptors/cmd/cmd_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/golang/mock/gomock" - "github.com/int128/kubelogin/pkg/adaptors" "github.com/int128/kubelogin/pkg/adaptors/mock_adaptors" "github.com/int128/kubelogin/pkg/usecases" "github.com/int128/kubelogin/pkg/usecases/mock_usecases" @@ -26,15 +25,12 @@ func TestCmd_Run(t *testing.T) { ListenPort: defaultListenPort, }) - logger := mock_adaptors.NewLogger(t, ctrl) - logger.EXPECT().SetLevel(adaptors.LogLevel(0)) - cmd := Cmd{ Root: &Root{ Login: login, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), }, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), } exitCode := cmd.Run(ctx, []string{executable}, version) if exitCode != 0 { @@ -61,15 +57,12 @@ func TestCmd_Run(t *testing.T) { Password: "PASS", }) - logger := mock_adaptors.NewLogger(t, ctrl) - logger.EXPECT().SetLevel(adaptors.LogLevel(1)) - cmd := Cmd{ Root: &Root{ Login: login, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), }, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), } exitCode := cmd.Run(ctx, []string{executable, "--kubeconfig", "/path/to/kubeconfig", @@ -95,9 +88,9 @@ func TestCmd_Run(t *testing.T) { cmd := Cmd{ Root: &Root{ Login: mock_usecases.NewMockLogin(ctrl), - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), }, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } exitCode := cmd.Run(context.TODO(), []string{executable, "some"}, version) if exitCode != 1 { @@ -119,15 +112,15 @@ func TestCmd_Run(t *testing.T) { ClientID: "YOUR_CLIENT_ID", }) - logger := mock_adaptors.NewLogger(t, ctrl) - logger.EXPECT().SetLevel(adaptors.LogLevel(0)) - cmd := Cmd{ + Root: &Root{ + Logger: mock_adaptors.NewLogger(t), + }, GetToken: &GetToken{ GetToken: getToken, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), }, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), } exitCode := cmd.Run(ctx, []string{executable, "get-token", @@ -160,15 +153,15 @@ func TestCmd_Run(t *testing.T) { Password: "PASS", }) - logger := mock_adaptors.NewLogger(t, ctrl) - logger.EXPECT().SetLevel(adaptors.LogLevel(1)) - cmd := Cmd{ + Root: &Root{ + Logger: mock_adaptors.NewLogger(t), + }, GetToken: &GetToken{ GetToken: getToken, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), }, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), } exitCode := cmd.Run(ctx, []string{executable, "get-token", @@ -196,11 +189,14 @@ func TestCmd_Run(t *testing.T) { defer ctrl.Finish() ctx := context.TODO() cmd := Cmd{ + Root: &Root{ + Logger: mock_adaptors.NewLogger(t), + }, GetToken: &GetToken{ GetToken: mock_usecases.NewMockGetToken(ctrl), - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), }, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } exitCode := cmd.Run(ctx, []string{executable, "get-token"}, version) if exitCode != 1 { @@ -213,11 +209,14 @@ func TestCmd_Run(t *testing.T) { defer ctrl.Finish() ctx := context.TODO() cmd := Cmd{ + Root: &Root{ + Logger: mock_adaptors.NewLogger(t), + }, GetToken: &GetToken{ GetToken: mock_usecases.NewMockGetToken(ctrl), - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), }, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } exitCode := cmd.Run(ctx, []string{executable, "get-token", "foo"}, version) if exitCode != 1 { diff --git a/pkg/adaptors/cmd/get_token.go b/pkg/adaptors/cmd/get_token.go index f94d1b1..faf5658 100644 --- a/pkg/adaptors/cmd/get_token.go +++ b/pkg/adaptors/cmd/get_token.go @@ -32,7 +32,6 @@ func (o *getTokenOptions) register(f *pflag.FlagSet) { f.StringSliceVar(&o.ExtraScopes, "oidc-extra-scope", nil, "Scopes to request to the provider") f.StringVar(&o.CertificateAuthority, "certificate-authority", "", "Path to a cert file for the certificate authority") f.BoolVar(&o.SkipTLSVerify, "insecure-skip-tls-verify", false, "If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure") - f.IntVarP(&o.Verbose, "v", "v", 0, "If set to 1 or greater, it shows debug log") f.StringVar(&o.TokenCacheDir, "token-cache-dir", defaultTokenCacheDir, "Path to a directory for caching tokens") } @@ -59,7 +58,6 @@ func (cmd *GetToken) New(ctx context.Context) *cobra.Command { return nil }, RunE: func(*cobra.Command, []string) error { - cmd.Logger.SetLevel(adaptors.LogLevel(o.Verbose)) in := usecases.GetTokenIn{ IssuerURL: o.IssuerURL, ClientID: o.ClientID, diff --git a/pkg/adaptors/cmd/root.go b/pkg/adaptors/cmd/root.go index 0c5e69d..4aa02dd 100644 --- a/pkg/adaptors/cmd/root.go +++ b/pkg/adaptors/cmd/root.go @@ -19,7 +19,6 @@ type kubectlOptions struct { User string CertificateAuthority string SkipTLSVerify bool - Verbose int } func (o *kubectlOptions) register(f *pflag.FlagSet) { @@ -29,7 +28,6 @@ func (o *kubectlOptions) register(f *pflag.FlagSet) { f.StringVar(&o.User, "user", "", "The name of the kubeconfig user to use. Prior to --context") f.StringVar(&o.CertificateAuthority, "certificate-authority", "", "Path to a cert file for the certificate authority") f.BoolVar(&o.SkipTLSVerify, "insecure-skip-tls-verify", false, "If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure") - f.IntVarP(&o.Verbose, "v", "v", 0, "If set to 1 or greater, it shows debug log") } // loginOptions represents the options for Login use-case. @@ -64,7 +62,6 @@ func (cmd *Root) New(ctx context.Context, executable string) *cobra.Command { Example: fmt.Sprintf(examples, executable), Args: cobra.NoArgs, RunE: func(*cobra.Command, []string) error { - cmd.Logger.SetLevel(adaptors.LogLevel(o.Verbose)) in := usecases.LoginIn{ KubeconfigFilename: o.Kubeconfig, KubeconfigContext: kubeconfig.ContextName(o.Context), @@ -84,5 +81,6 @@ func (cmd *Root) New(ctx context.Context, executable string) *cobra.Command { } o.kubectlOptions.register(rootCmd.Flags()) o.loginOptions.register(rootCmd.Flags()) + cmd.Logger.AddFlags(rootCmd.PersistentFlags()) return rootCmd } diff --git a/pkg/adaptors/interfaces.go b/pkg/adaptors/interfaces.go index 8495cab..612980f 100644 --- a/pkg/adaptors/interfaces.go +++ b/pkg/adaptors/interfaces.go @@ -6,6 +6,7 @@ import ( "github.com/int128/kubelogin/pkg/models/credentialplugin" "github.com/int128/kubelogin/pkg/models/kubeconfig" + "github.com/spf13/pflag" ) //go:generate mockgen -destination mock_adaptors/mock_adaptors.go github.com/int128/kubelogin/pkg/adaptors Kubeconfig,TokenCacheRepository,CredentialPluginInteraction,OIDC,OIDCClient,OIDCDecoder,Env,Logger @@ -86,10 +87,14 @@ type Env interface { } type Logger interface { - Printf(format string, v ...interface{}) - Debugf(level LogLevel, format string, v ...interface{}) - SetLevel(level LogLevel) - IsEnabled(level LogLevel) bool + AddFlags(f *pflag.FlagSet) + Printf(format string, args ...interface{}) + V(level int) Verbose + IsEnabled(level int) bool +} + +type Verbose interface { + Infof(format string, args ...interface{}) } // LogLevel represents a log level for debug. diff --git a/pkg/adaptors/logger/logger.go b/pkg/adaptors/logger/logger.go index 42154da..fed77ab 100644 --- a/pkg/adaptors/logger/logger.go +++ b/pkg/adaptors/logger/logger.go @@ -1,12 +1,14 @@ package logger import ( - "fmt" + "flag" "log" "os" "github.com/google/wire" "github.com/int128/kubelogin/pkg/adaptors" + "github.com/spf13/pflag" + "k8s.io/klog" ) // Set provides an implementation and interface for Logger. @@ -14,43 +16,35 @@ var Set = wire.NewSet( New, ) -// New returns a Logger with the standard log.Logger for messages and debug. +// New returns a Logger with the standard log.Logger and klog. func New() adaptors.Logger { return &Logger{ - stdLogger: log.New(os.Stderr, "", 0), - debugLogger: log.New(os.Stderr, "", log.Ltime|log.Lmicroseconds|log.Lshortfile), + goLogger: log.New(os.Stderr, "", 0), } } -func NewWith(s stdLogger, d debugLogger) *Logger { - return &Logger{s, d, 0} -} - -type stdLogger interface { +type goLogger interface { Printf(format string, v ...interface{}) } -type debugLogger interface { - Output(calldepth int, s string) error -} - -// Logger wraps the standard log.Logger and just provides debug level. +// Logger provides logging facility using log.Logger and klog. type Logger struct { - stdLogger - debugLogger - level adaptors.LogLevel + goLogger } -func (l *Logger) Debugf(level adaptors.LogLevel, format string, v ...interface{}) { - if l.IsEnabled(level) { - _ = l.debugLogger.Output(2, fmt.Sprintf(format, v...)) - } +// AddFlags adds the flags such as -v. +func (*Logger) AddFlags(f *pflag.FlagSet) { + gf := flag.NewFlagSet("", flag.ContinueOnError) + klog.InitFlags(gf) + f.AddGoFlagSet(gf) } -func (l *Logger) SetLevel(level adaptors.LogLevel) { - l.level = level +// V returns a logger enabled only if the level is enabled. +func (*Logger) V(level int) adaptors.Verbose { + return klog.V(klog.Level(level)) } -func (l *Logger) IsEnabled(level adaptors.LogLevel) bool { - return level <= l.level +// IsEnabled returns true if the level is enabled. +func (*Logger) IsEnabled(level int) bool { + return bool(klog.V(klog.Level(level))) } diff --git a/pkg/adaptors/logger/logger_test.go b/pkg/adaptors/logger/logger_test.go deleted file mode 100644 index 47e1a5e..0000000 --- a/pkg/adaptors/logger/logger_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package logger - -import ( - "fmt" - "testing" - - "github.com/int128/kubelogin/pkg/adaptors" -) - -type mockDebugLogger struct { - count int -} - -func (l *mockDebugLogger) Output(int, string) error { - l.count++ - return nil -} - -func TestLogger_Debugf(t *testing.T) { - for _, c := range []struct { - loggerLevel adaptors.LogLevel - debugfLevel adaptors.LogLevel - count int - }{ - {0, 0, 1}, - {0, 1, 0}, - - {1, 0, 1}, - {1, 1, 1}, - {1, 2, 0}, - - {2, 1, 1}, - {2, 2, 1}, - {2, 3, 0}, - } { - t.Run(fmt.Sprintf("%+v", c), func(t *testing.T) { - m := &mockDebugLogger{} - l := &Logger{debugLogger: m, level: c.loggerLevel} - l.Debugf(c.debugfLevel, "hello") - if m.count != c.count { - t.Errorf("count wants %d but %d", c.count, m.count) - } - }) - } -} - -type mockStdLogger struct { - count int -} - -func (l *mockStdLogger) Printf(format string, v ...interface{}) { - l.count++ -} - -func TestLogger_Printf(t *testing.T) { - m := &mockStdLogger{} - l := &Logger{stdLogger: m} - l.Printf("hello") - if m.count != 1 { - t.Errorf("count wants %d but %d", 1, m.count) - } -} diff --git a/pkg/adaptors/mock_adaptors/logger.go b/pkg/adaptors/mock_adaptors/logger.go index ad7ca8a..e028a0f 100644 --- a/pkg/adaptors/mock_adaptors/logger.go +++ b/pkg/adaptors/mock_adaptors/logger.go @@ -1,31 +1,46 @@ package mock_adaptors import ( - "github.com/golang/mock/gomock" + "fmt" + "github.com/int128/kubelogin/pkg/adaptors" + "github.com/spf13/pflag" ) -func NewLogger(t testingLogger, ctrl *gomock.Controller) *Logger { - return &Logger{ - MockLogger: NewMockLogger(ctrl), - testingLogger: t, - } +func NewLogger(t testingLogger) *Logger { + return &Logger{t} } type testingLogger interface { Logf(format string, v ...interface{}) } -// Logger provides mock feature but overrides output methods with actual logging. +// Logger provides logging facility using testing.T. type Logger struct { - *MockLogger - testingLogger testingLogger + t testingLogger } -func (l *Logger) Printf(format string, v ...interface{}) { - l.testingLogger.Logf(format, v...) +func (*Logger) AddFlags(f *pflag.FlagSet) { + f.IntP("v", "v", 0, "dummy flag used in the tests") } -func (l *Logger) Debugf(level adaptors.LogLevel, format string, v ...interface{}) { - l.testingLogger.Logf(format, v...) +func (l *Logger) Printf(format string, args ...interface{}) { + l.t.Logf(format, args...) +} + +type Verbose struct { + t testingLogger + level int +} + +func (v *Verbose) Infof(format string, args ...interface{}) { + v.t.Logf(fmt.Sprintf("I%d] ", v.level)+format, args...) +} + +func (l *Logger) V(level int) adaptors.Verbose { + return &Verbose{l.t, level} +} + +func (*Logger) IsEnabled(level int) bool { + return true } diff --git a/pkg/adaptors/mock_adaptors/mock_adaptors.go b/pkg/adaptors/mock_adaptors/mock_adaptors.go index fd2c093..3fc24c5 100644 --- a/pkg/adaptors/mock_adaptors/mock_adaptors.go +++ b/pkg/adaptors/mock_adaptors/mock_adaptors.go @@ -10,6 +10,7 @@ import ( adaptors "github.com/int128/kubelogin/pkg/adaptors" credentialplugin "github.com/int128/kubelogin/pkg/models/credentialplugin" kubeconfig "github.com/int128/kubelogin/pkg/models/kubeconfig" + pflag "github.com/spf13/pflag" reflect "reflect" ) @@ -337,23 +338,18 @@ func (m *MockLogger) EXPECT() *MockLoggerMockRecorder { return m.recorder } -// Debugf mocks base method -func (m *MockLogger) Debugf(arg0 adaptors.LogLevel, arg1 string, arg2 ...interface{}) { - varargs := []interface{}{arg0, arg1} - for _, a := range arg2 { - varargs = append(varargs, a) - } - m.ctrl.Call(m, "Debugf", varargs...) +// AddFlags mocks base method +func (m *MockLogger) AddFlags(arg0 *pflag.FlagSet) { + m.ctrl.Call(m, "AddFlags", arg0) } -// Debugf indicates an expected call of Debugf -func (mr *MockLoggerMockRecorder) Debugf(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { - varargs := append([]interface{}{arg0, arg1}, arg2...) - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Debugf", reflect.TypeOf((*MockLogger)(nil).Debugf), varargs...) +// AddFlags indicates an expected call of AddFlags +func (mr *MockLoggerMockRecorder) AddFlags(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddFlags", reflect.TypeOf((*MockLogger)(nil).AddFlags), arg0) } // IsEnabled mocks base method -func (m *MockLogger) IsEnabled(arg0 adaptors.LogLevel) bool { +func (m *MockLogger) IsEnabled(arg0 int) bool { ret := m.ctrl.Call(m, "IsEnabled", arg0) ret0, _ := ret[0].(bool) return ret0 @@ -379,12 +375,14 @@ func (mr *MockLoggerMockRecorder) Printf(arg0 interface{}, arg1 ...interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Printf", reflect.TypeOf((*MockLogger)(nil).Printf), varargs...) } -// SetLevel mocks base method -func (m *MockLogger) SetLevel(arg0 adaptors.LogLevel) { - m.ctrl.Call(m, "SetLevel", arg0) +// V mocks base method +func (m *MockLogger) V(arg0 int) adaptors.Verbose { + ret := m.ctrl.Call(m, "V", arg0) + ret0, _ := ret[0].(adaptors.Verbose) + return ret0 } -// SetLevel indicates an expected call of SetLevel -func (mr *MockLoggerMockRecorder) SetLevel(arg0 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLevel", reflect.TypeOf((*MockLogger)(nil).SetLevel), arg0) +// V indicates an expected call of V +func (mr *MockLoggerMockRecorder) V(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "V", reflect.TypeOf((*MockLogger)(nil).V), arg0) } diff --git a/pkg/adaptors/oidc/logging/transport.go b/pkg/adaptors/oidc/logging/transport.go index 57745c1..5a77d75 100644 --- a/pkg/adaptors/oidc/logging/transport.go +++ b/pkg/adaptors/oidc/logging/transport.go @@ -8,8 +8,8 @@ import ( ) const ( - logLevelDumpHeaders = 2 - logLevelDumpBody = 3 + levelDumpHeaders = 2 + levelDumpBody = 3 ) type Transport struct { @@ -18,33 +18,25 @@ type Transport struct { } func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { - if !t.IsDumpEnabled() { + if !t.Logger.IsEnabled(levelDumpHeaders) { return t.Base.RoundTrip(req) } - reqDump, err := httputil.DumpRequestOut(req, t.IsDumpBodyEnabled()) + reqDump, err := httputil.DumpRequestOut(req, t.Logger.IsEnabled(levelDumpBody)) if err != nil { - t.Logger.Debugf(logLevelDumpHeaders, "could not dump the request: %s", err) + t.Logger.V(levelDumpHeaders).Infof("could not dump the request: %s", err) return t.Base.RoundTrip(req) } - t.Logger.Debugf(logLevelDumpHeaders, "%s", string(reqDump)) + t.Logger.V(levelDumpHeaders).Infof("%s", string(reqDump)) resp, err := t.Base.RoundTrip(req) if err != nil { return resp, err } - respDump, err := httputil.DumpResponse(resp, t.IsDumpBodyEnabled()) + respDump, err := httputil.DumpResponse(resp, t.Logger.IsEnabled(levelDumpBody)) if err != nil { - t.Logger.Debugf(logLevelDumpHeaders, "could not dump the response: %s", err) + t.Logger.V(levelDumpHeaders).Infof("could not dump the response: %s", err) return resp, err } - t.Logger.Debugf(logLevelDumpHeaders, "%s", string(respDump)) + t.Logger.V(levelDumpHeaders).Infof("%s", string(respDump)) return resp, err } - -func (t *Transport) IsDumpEnabled() bool { - return t.Logger.IsEnabled(logLevelDumpHeaders) -} - -func (t *Transport) IsDumpBodyEnabled() bool { - return t.Logger.IsEnabled(logLevelDumpBody) -} diff --git a/pkg/adaptors/oidc/logging/transport_test.go b/pkg/adaptors/oidc/logging/transport_test.go index f479ea3..3f9fdb0 100644 --- a/pkg/adaptors/oidc/logging/transport_test.go +++ b/pkg/adaptors/oidc/logging/transport_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/golang/mock/gomock" - "github.com/int128/kubelogin/pkg/adaptors" "github.com/int128/kubelogin/pkg/adaptors/mock_adaptors" ) @@ -26,12 +25,6 @@ func TestLoggingTransport_RoundTrip(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - logger := mock_adaptors.NewLogger(t, ctrl) - logger.EXPECT(). - IsEnabled(gomock.Any()). - Return(true). - AnyTimes() - req := httptest.NewRequest("GET", "http://example.com/hello", nil) resp, err := http.ReadResponse(bufio.NewReader(strings.NewReader(`HTTP/1.1 200 OK Host: example.com @@ -44,7 +37,7 @@ dummy`)), req) transport := &Transport{ Base: &mockTransport{resp: resp}, - Logger: logger, + Logger: mock_adaptors.NewLogger(t), } gotResp, err := transport.RoundTrip(req) if err != nil { @@ -54,37 +47,3 @@ dummy`)), req) t.Errorf("resp wants %v but %v", resp, gotResp) } } - -func TestLoggingTransport_IsDumpEnabled(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - logger := mock_adaptors.NewLogger(t, ctrl) - logger.EXPECT(). - IsEnabled(adaptors.LogLevel(logLevelDumpHeaders)). - Return(true) - - transport := &Transport{ - Logger: logger, - } - if transport.IsDumpEnabled() != true { - t.Errorf("IsDumpEnabled wants true") - } -} - -func TestLoggingTransport_IsDumpBodyEnabled(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - logger := mock_adaptors.NewLogger(t, ctrl) - logger.EXPECT(). - IsEnabled(adaptors.LogLevel(logLevelDumpBody)). - Return(true) - - transport := &Transport{ - Logger: logger, - } - if transport.IsDumpBodyEnabled() != true { - t.Errorf("IsDumpBodyEnabled wants true") - } -} diff --git a/pkg/adaptors/oidc/oidc.go b/pkg/adaptors/oidc/oidc.go index 4aa9ed1..f6fdde5 100644 --- a/pkg/adaptors/oidc/oidc.go +++ b/pkg/adaptors/oidc/oidc.go @@ -121,7 +121,7 @@ func (c *client) AuthenticateByCode(ctx context.Context, in adaptors.OIDCAuthent } claims, err := dumpClaims(verifiedIDToken) if err != nil { - c.logger.Debugf(1, "incomplete claims of the ID token: %w", err) + c.logger.V(1).Infof("incomplete claims of the ID token: %w", err) } return &adaptors.OIDCAuthenticateOut{ IDToken: idToken, @@ -157,7 +157,7 @@ func (c *client) AuthenticateByPassword(ctx context.Context, in adaptors.OIDCAut } claims, err := dumpClaims(verifiedIDToken) if err != nil { - c.logger.Debugf(1, "incomplete claims of the ID token: %w", err) + c.logger.V(1).Infof("incomplete claims of the ID token: %w", err) } return &adaptors.OIDCAuthenticateOut{ IDToken: idToken, @@ -190,7 +190,7 @@ func (c *client) Refresh(ctx context.Context, in adaptors.OIDCRefreshIn) (*adapt } claims, err := dumpClaims(verifiedIDToken) if err != nil { - c.logger.Debugf(1, "incomplete claims of the ID token: %w", err) + c.logger.V(1).Infof("incomplete claims of the ID token: %w", err) } return &adaptors.OIDCAuthenticateOut{ IDToken: idToken, diff --git a/pkg/adaptors/oidc/tls/tls.go b/pkg/adaptors/oidc/tls/tls.go index ebe0ece..290cad5 100644 --- a/pkg/adaptors/oidc/tls/tls.go +++ b/pkg/adaptors/oidc/tls/tls.go @@ -14,21 +14,21 @@ import ( func NewConfig(config adaptors.OIDCClientConfig, logger adaptors.Logger) (*tls.Config, error) { pool := x509.NewCertPool() if config.Config.IDPCertificateAuthority != "" { - logger.Debugf(1, "Loading the certificate %s", config.Config.IDPCertificateAuthority) + logger.V(1).Infof("Loading the certificate %s", config.Config.IDPCertificateAuthority) err := appendCertificateFromFile(pool, config.Config.IDPCertificateAuthority) if err != nil { return nil, xerrors.Errorf("could not load the certificate of idp-certificate-authority: %w", err) } } if config.Config.IDPCertificateAuthorityData != "" { - logger.Debugf(1, "Loading the certificate of idp-certificate-authority-data") + logger.V(1).Infof("Loading the certificate of idp-certificate-authority-data") err := appendEncodedCertificate(pool, config.Config.IDPCertificateAuthorityData) if err != nil { return nil, xerrors.Errorf("could not load the certificate of idp-certificate-authority-data: %w", err) } } if config.CACertFilename != "" { - logger.Debugf(1, "Loading the certificate %s", config.CACertFilename) + logger.V(1).Infof("Loading the certificate %s", config.CACertFilename) err := appendCertificateFromFile(pool, config.CACertFilename) if err != nil { return nil, xerrors.Errorf("could not load the certificate: %w", err) diff --git a/pkg/adaptors/oidc/tls/tls_test.go b/pkg/adaptors/oidc/tls/tls_test.go index 7ca4039..14f707d 100644 --- a/pkg/adaptors/oidc/tls/tls_test.go +++ b/pkg/adaptors/oidc/tls/tls_test.go @@ -11,7 +11,6 @@ import ( func TestNewConfig(t *testing.T) { testingLogger := logger.New(t) - testingLogger.SetLevel(1) t.Run("Defaults", func(t *testing.T) { c, err := NewConfig(adaptors.OIDCClientConfig{}, testingLogger) diff --git a/pkg/usecases/auth/auth.go b/pkg/usecases/auth/auth.go index 1cb0908..6be4a93 100644 --- a/pkg/usecases/auth/auth.go +++ b/pkg/usecases/auth/auth.go @@ -47,7 +47,7 @@ type Authentication struct { func (u *Authentication) Do(ctx context.Context, in usecases.AuthenticationIn) (*usecases.AuthenticationOut, error) { if in.OIDCConfig.IDToken != "" { - u.Logger.Debugf(1, "checking expiration of the existing token") + u.Logger.V(1).Infof("checking expiration of the existing token") // Skip verification of the token to reduce time of a discovery request. // Here it trusts the signature and claims and checks only expiration, // because the token has been verified before caching. @@ -56,7 +56,7 @@ func (u *Authentication) Do(ctx context.Context, in usecases.AuthenticationIn) ( return nil, xerrors.Errorf("invalid token and you need to remove the cache: %w", err) } if token.IDTokenExpiry.After(time.Now()) { //TODO: inject time service - u.Logger.Debugf(1, "you already have a valid token until %s", token.IDTokenExpiry) + u.Logger.V(1).Infof("you already have a valid token until %s", token.IDTokenExpiry) return &usecases.AuthenticationOut{ AlreadyHasValidIDToken: true, IDToken: in.OIDCConfig.IDToken, @@ -65,10 +65,10 @@ func (u *Authentication) Do(ctx context.Context, in usecases.AuthenticationIn) ( IDTokenClaims: token.IDTokenClaims, }, nil } - u.Logger.Debugf(1, "you have an expired token at %s", token.IDTokenExpiry) + u.Logger.V(1).Infof("you have an expired token at %s", token.IDTokenExpiry) } - u.Logger.Debugf(1, "initializing an OIDC client") + u.Logger.V(1).Infof("initializing an OIDC client") client, err := u.OIDC.New(ctx, adaptors.OIDCClientConfig{ Config: in.OIDCConfig, CACertFilename: in.CACertFilename, @@ -79,7 +79,7 @@ func (u *Authentication) Do(ctx context.Context, in usecases.AuthenticationIn) ( } if in.OIDCConfig.RefreshToken != "" { - u.Logger.Debugf(1, "refreshing the token") + u.Logger.V(1).Infof("refreshing the token") out, err := client.Refresh(ctx, adaptors.OIDCRefreshIn{ RefreshToken: in.OIDCConfig.RefreshToken, }) @@ -91,11 +91,11 @@ func (u *Authentication) Do(ctx context.Context, in usecases.AuthenticationIn) ( IDTokenClaims: out.IDTokenClaims, }, nil } - u.Logger.Debugf(1, "could not refresh the token: %s", err) + u.Logger.V(1).Infof("could not refresh the token: %s", err) } if in.Username == "" { - u.Logger.Debugf(1, "performing the authentication code flow") + u.Logger.V(1).Infof("performing the authentication code flow") out, err := client.AuthenticateByCode(ctx, adaptors.OIDCAuthenticateByCodeIn{ LocalServerPort: in.ListenPort, SkipOpenBrowser: in.SkipOpenBrowser, @@ -112,7 +112,7 @@ func (u *Authentication) Do(ctx context.Context, in usecases.AuthenticationIn) ( }, nil } - u.Logger.Debugf(1, "performing the resource owner password credentials flow") + u.Logger.V(1).Infof("performing the resource owner password credentials flow") if in.Password == "" { in.Password, err = u.Env.ReadPassword(passwordPrompt) if err != nil { diff --git a/pkg/usecases/auth/auth_test.go b/pkg/usecases/auth/auth_test.go index 570cca2..889c19e 100644 --- a/pkg/usecases/auth/auth_test.go +++ b/pkg/usecases/auth/auth_test.go @@ -55,7 +55,7 @@ func TestAuthentication_Do(t *testing.T) { Return(mockOIDCClient, nil) u := Authentication{ OIDC: mockOIDC, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } out, err := u.Do(ctx, in) if err != nil { @@ -108,7 +108,7 @@ func TestAuthentication_Do(t *testing.T) { Return(mockOIDCClient, nil) u := Authentication{ OIDC: mockOIDC, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } out, err := u.Do(ctx, in) if err != nil { @@ -159,7 +159,7 @@ func TestAuthentication_Do(t *testing.T) { u := Authentication{ OIDC: mockOIDC, Env: mockEnv, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } out, err := u.Do(ctx, in) if err != nil { @@ -198,7 +198,7 @@ func TestAuthentication_Do(t *testing.T) { u := Authentication{ OIDC: mockOIDC, Env: mockEnv, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } out, err := u.Do(ctx, in) if err == nil { @@ -230,7 +230,7 @@ func TestAuthentication_Do(t *testing.T) { u := Authentication{ OIDC: mock_adaptors.NewMockOIDC(ctrl), OIDCDecoder: mockOIDCDecoder, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } out, err := u.Do(ctx, in) if err != nil { @@ -286,7 +286,7 @@ func TestAuthentication_Do(t *testing.T) { u := Authentication{ OIDC: mockOIDC, OIDCDecoder: mockOIDCDecoder, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } out, err := u.Do(ctx, in) if err != nil { @@ -348,7 +348,7 @@ func TestAuthentication_Do(t *testing.T) { u := Authentication{ OIDC: mockOIDC, OIDCDecoder: mockOIDCDecoder, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } out, err := u.Do(ctx, in) if err != nil { diff --git a/pkg/usecases/credentialplugin/get_token.go b/pkg/usecases/credentialplugin/get_token.go index db74661..0c4d789 100644 --- a/pkg/usecases/credentialplugin/get_token.go +++ b/pkg/usecases/credentialplugin/get_token.go @@ -27,13 +27,13 @@ type GetToken struct { } func (u *GetToken) Do(ctx context.Context, in usecases.GetTokenIn) error { - u.Logger.Debugf(1, "WARNING: log may contain your secrets such as token or password") + u.Logger.V(1).Infof("WARNING: log may contain your secrets such as token or password") - u.Logger.Debugf(1, "finding a token from cache directory %s", in.TokenCacheDir) + u.Logger.V(1).Infof("finding a token from cache directory %s", in.TokenCacheDir) cacheKey := credentialplugin.TokenCacheKey{IssuerURL: in.IssuerURL, ClientID: in.ClientID} cache, err := u.TokenCacheRepository.FindByKey(in.TokenCacheDir, cacheKey) if err != nil { - u.Logger.Debugf(1, "could not find a token cache: %s", err) + u.Logger.V(1).Infof("could not find a token cache: %s", err) cache = &credentialplugin.TokenCache{} } out, err := u.Authentication.Do(ctx, usecases.AuthenticationIn{ @@ -56,7 +56,7 @@ func (u *GetToken) Do(ctx context.Context, in usecases.GetTokenIn) error { return xerrors.Errorf("error while authentication: %w", err) } for k, v := range out.IDTokenClaims { - u.Logger.Debugf(1, "the ID token has the claim: %s=%v", k, v) + u.Logger.V(1).Infof("the ID token has the claim: %s=%v", k, v) } if !out.AlreadyHasValidIDToken { u.Logger.Printf("You got a valid token until %s", out.IDTokenExpiry) @@ -69,7 +69,7 @@ func (u *GetToken) Do(ctx context.Context, in usecases.GetTokenIn) error { } } - u.Logger.Debugf(1, "writing the token to client-go") + u.Logger.V(1).Infof("writing the token to client-go") if err := u.Interaction.Write(credentialplugin.Output{Token: out.IDToken, Expiry: out.IDTokenExpiry}); err != nil { return xerrors.Errorf("could not write the token to client-go: %w", err) } diff --git a/pkg/usecases/credentialplugin/get_token_test.go b/pkg/usecases/credentialplugin/get_token_test.go index da184c5..ef7c3d9 100644 --- a/pkg/usecases/credentialplugin/get_token_test.go +++ b/pkg/usecases/credentialplugin/get_token_test.go @@ -82,7 +82,7 @@ func TestGetToken_Do(t *testing.T) { Authentication: mockAuthentication, TokenCacheRepository: tokenCacheRepository, Interaction: credentialPluginInteraction, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } if err := u.Do(ctx, in); err != nil { t.Errorf("Do returned error: %+v", err) @@ -134,7 +134,7 @@ func TestGetToken_Do(t *testing.T) { Authentication: mockAuthentication, TokenCacheRepository: tokenCacheRepository, Interaction: credentialPluginInteraction, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } if err := u.Do(ctx, in); err != nil { t.Errorf("Do returned error: %+v", err) @@ -172,7 +172,7 @@ func TestGetToken_Do(t *testing.T) { Authentication: mockAuthentication, TokenCacheRepository: tokenCacheRepository, Interaction: mock_adaptors.NewMockCredentialPluginInteraction(ctrl), - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } if err := u.Do(ctx, in); err == nil { t.Errorf("err wants non-nil but nil") diff --git a/pkg/usecases/login/login.go b/pkg/usecases/login/login.go index d408a92..2a2f73a 100644 --- a/pkg/usecases/login/login.go +++ b/pkg/usecases/login/login.go @@ -35,15 +35,15 @@ type Login struct { } func (u *Login) Do(ctx context.Context, in usecases.LoginIn) error { - u.Logger.Debugf(1, "WARNING: log may contain your secrets such as token or password") + u.Logger.V(1).Infof("WARNING: log may contain your secrets such as token or password") authProvider, err := u.Kubeconfig.GetCurrentAuthProvider(in.KubeconfigFilename, in.KubeconfigContext, in.KubeconfigUser) if err != nil { u.Logger.Printf(oidcConfigErrorMessage) return xerrors.Errorf("could not find the current authentication provider: %w", err) } - u.Logger.Debugf(1, "using the authentication provider of the user %s", authProvider.UserName) - u.Logger.Debugf(1, "a token will be written to %s", authProvider.LocationOfOrigin) + u.Logger.V(1).Infof("using the authentication provider of the user %s", authProvider.UserName) + u.Logger.V(1).Infof("a token will be written to %s", authProvider.LocationOfOrigin) out, err := u.Authentication.Do(ctx, usecases.AuthenticationIn{ OIDCConfig: authProvider.OIDCConfig, @@ -58,7 +58,7 @@ func (u *Login) Do(ctx context.Context, in usecases.LoginIn) error { return xerrors.Errorf("error while authentication: %w", err) } for k, v := range out.IDTokenClaims { - u.Logger.Debugf(1, "the ID token has the claim: %s=%v", k, v) + u.Logger.V(1).Infof("the ID token has the claim: %s=%v", k, v) } if out.AlreadyHasValidIDToken { u.Logger.Printf("You already have a valid token until %s", out.IDTokenExpiry) @@ -68,7 +68,7 @@ func (u *Login) Do(ctx context.Context, in usecases.LoginIn) error { u.Logger.Printf("You got a valid token until %s", out.IDTokenExpiry) authProvider.OIDCConfig.IDToken = out.IDToken authProvider.OIDCConfig.RefreshToken = out.RefreshToken - u.Logger.Debugf(1, "writing the ID token and refresh token to %s", authProvider.LocationOfOrigin) + u.Logger.V(1).Infof("writing the ID token and refresh token to %s", authProvider.LocationOfOrigin) if err := u.Kubeconfig.UpdateAuthProvider(authProvider); err != nil { return xerrors.Errorf("could not write the token to the kubeconfig: %w", err) } diff --git a/pkg/usecases/login/login_test.go b/pkg/usecases/login/login_test.go index 2f930ed..c386f27 100644 --- a/pkg/usecases/login/login_test.go +++ b/pkg/usecases/login/login_test.go @@ -77,7 +77,7 @@ func TestLogin_Do(t *testing.T) { u := Login{ Authentication: mockAuthentication, Kubeconfig: mockKubeconfig, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } if err := u.Do(ctx, in); err != nil { t.Errorf("Do returned error: %+v", err) @@ -114,7 +114,7 @@ func TestLogin_Do(t *testing.T) { u := Login{ Authentication: mockAuthentication, Kubeconfig: mockKubeconfig, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } if err := u.Do(ctx, in); err != nil { t.Errorf("Do returned error: %+v", err) @@ -134,7 +134,7 @@ func TestLogin_Do(t *testing.T) { u := Login{ Authentication: mockAuthentication, Kubeconfig: mockKubeconfig, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } if err := u.Do(ctx, in); err == nil { t.Errorf("err wants non-nil but nil") @@ -166,7 +166,7 @@ func TestLogin_Do(t *testing.T) { u := Login{ Authentication: mockAuthentication, Kubeconfig: mockKubeconfig, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } if err := u.Do(ctx, in); err == nil { t.Errorf("err wants non-nil but nil") @@ -216,7 +216,7 @@ func TestLogin_Do(t *testing.T) { u := Login{ Authentication: mockAuthentication, Kubeconfig: mockKubeconfig, - Logger: mock_adaptors.NewLogger(t, ctrl), + Logger: mock_adaptors.NewLogger(t), } if err := u.Do(ctx, in); err == nil { t.Errorf("err wants non-nil but nil")