From 184953ea52ff28bb5c6c8efd610f4a6f75a639df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 12 Apr 2023 11:17:01 +0300 Subject: [PATCH 1/3] Add warning log callback in client-go loading rules This provides a way to consumers use their own custom warning mechanisms instead default klog warning. --- .../client-go/tools/clientcmd/loader.go | 16 ++++- .../client-go/tools/clientcmd/loader_test.go | 67 ++++++++++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/loader.go b/staging/src/k8s.io/client-go/tools/clientcmd/loader.go index 44de1d41d836..f3c249b7c671 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/loader.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/loader.go @@ -128,6 +128,20 @@ type ClientConfigLoadingRules struct { // WarnIfAllMissing indicates whether the configuration files pointed by KUBECONFIG environment variable are present or not. // In case of missing files, it warns the user about the missing files. WarnIfAllMissing bool + + // Warner is the warning log callback to use in case of missing files. + Warner WarningHandler +} + +// WarningHandler allows to set the logging function to use +type WarningHandler func(...interface{}) + +func (handler WarningHandler) Warn(message string) { + if handler == nil { + klog.V(1).Info(message) + } else { + handler(message) + } } // ClientConfigLoadingRules implements the ClientConfigLoader interface. @@ -219,7 +233,7 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { } if rules.WarnIfAllMissing && len(missingList) > 0 && len(kubeconfigs) == 0 { - klog.Warningf("Config not found: %s", strings.Join(missingList, ", ")) + rules.Warner.Warn(fmt.Sprintf("Config not found: %s", strings.Join(missingList, ", "))) } // first merge all of our maps diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go index b641d1a2b790..39992130138f 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go @@ -18,6 +18,7 @@ package clientcmd import ( "bytes" + "flag" "fmt" "os" "path" @@ -32,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/util/diff" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" clientcmdlatest "k8s.io/client-go/tools/clientcmd/api/latest" + "k8s.io/klog/v2" ) var ( @@ -120,14 +122,77 @@ func TestNonExistentCommandLineFile(t *testing.T) { } func TestToleratingMissingFiles(t *testing.T) { + envVarValue := "bogus" loadingRules := ClientConfigLoadingRules{ - Precedence: []string{"bogus1", "bogus2", "bogus3"}, + Precedence: []string{"bogus1", "bogus2", "bogus3"}, + WarnIfAllMissing: true, + Warner: klog.Warning, } + buffer := &bytes.Buffer{} + + klog.LogToStderr(false) + klog.SetOutput(buffer) + _, err := loadingRules.Load() if err != nil { t.Fatalf("Unexpected error: %v", err) } + klog.Flush() + expectedLog := fmt.Sprintf("Config not found: %s", envVarValue) + if !strings.Contains(buffer.String(), expectedLog) { + t.Fatalf("expected log: \"%s\"", expectedLog) + } +} + +func TestWarningMissingFiles(t *testing.T) { + envVarValue := "bogus" + os.Setenv(RecommendedConfigPathEnvVar, envVarValue) + loadingRules := NewDefaultClientConfigLoadingRules() + + buffer := &bytes.Buffer{} + + flags := &flag.FlagSet{} + klog.InitFlags(flags) + flags.Set("v", "1") + klog.LogToStderr(false) + klog.SetOutput(buffer) + + _, err := loadingRules.Load() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + klog.Flush() + + expectedLog := fmt.Sprintf("Config not found: %s", envVarValue) + if !strings.Contains(buffer.String(), expectedLog) { + t.Fatalf("expected log: \"%s\"", expectedLog) + } +} + +func TestNoWarningMissingFiles(t *testing.T) { + envVarValue := "bogus" + os.Setenv(RecommendedConfigPathEnvVar, envVarValue) + loadingRules := NewDefaultClientConfigLoadingRules() + + buffer := &bytes.Buffer{} + + flags := &flag.FlagSet{} + klog.InitFlags(flags) + flags.Set("v", "0") + klog.LogToStderr(false) + klog.SetOutput(buffer) + + _, err := loadingRules.Load() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + klog.Flush() + + logNotExpected := fmt.Sprintf("Config not found: %s", envVarValue) + if strings.Contains(buffer.String(), logNotExpected) { + t.Fatalf("log not expected: \"%s\"", logNotExpected) + } } func TestErrorReadingFile(t *testing.T) { From 823316ba837f690b9fd167e66033e9fbc077b814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 24 May 2023 09:12:39 +0300 Subject: [PATCH 2/3] Use typed error instead plain string --- .../k8s.io/client-go/tools/clientcmd/loader.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/loader.go b/staging/src/k8s.io/client-go/tools/clientcmd/loader.go index f3c249b7c671..b75737f1c904 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/loader.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/loader.go @@ -134,16 +134,24 @@ type ClientConfigLoadingRules struct { } // WarningHandler allows to set the logging function to use -type WarningHandler func(...interface{}) +type WarningHandler func(error) -func (handler WarningHandler) Warn(message string) { +func (handler WarningHandler) Warn(err error) { if handler == nil { - klog.V(1).Info(message) + klog.V(1).Info(err) } else { - handler(message) + handler(err) } } +type MissingConfigError struct { + Missing []string +} + +func (c MissingConfigError) Error() string { + return fmt.Sprintf("Config not found: %s", strings.Join(c.Missing, ", ")) +} + // ClientConfigLoadingRules implements the ClientConfigLoader interface. var _ ClientConfigLoader = &ClientConfigLoadingRules{} @@ -233,7 +241,7 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { } if rules.WarnIfAllMissing && len(missingList) > 0 && len(kubeconfigs) == 0 { - rules.Warner.Warn(fmt.Sprintf("Config not found: %s", strings.Join(missingList, ", "))) + rules.Warner.Warn(MissingConfigError{Missing: missingList}) } // first merge all of our maps From 2a7acd2c08f842edf8dd0eee8070ea1800aa26be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 24 May 2023 09:22:54 +0300 Subject: [PATCH 3/3] Fix interface change in unit test --- staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go index 39992130138f..8ceb1022cd2e 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go @@ -126,7 +126,7 @@ func TestToleratingMissingFiles(t *testing.T) { loadingRules := ClientConfigLoadingRules{ Precedence: []string{"bogus1", "bogus2", "bogus3"}, WarnIfAllMissing: true, - Warner: klog.Warning, + Warner: func(err error) { klog.Warning(err) }, } buffer := &bytes.Buffer{}