Repository: kubernetes/klog
Pull request #179 (2020-09-19 18:03:40)

Prevents calling `logr.(Info|Error)` with a wrapped slice in slice.
**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes *none*

**Release note**:
<!--  Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
Fixes: Prevents calling `logr.(Info|Error)` with a wrapped slice in slice.
```


Patch:
diff --git a/klog.go b/klog.go
index f48bbba..23cced6 100644
--- a/klog.go
+++ b/klog.go
@@ -777,7 +777,7 @@ func (l *loggingT) errorS(err error, loggr logr.Logger, filter LogFilter, msg st
 		msg, keysAndValues = filter.FilterS(msg, keysAndValues)
 	}
 	if loggr != nil {
-		loggr.Error(err, msg, keysAndValues)
+		loggr.Error(err, msg, keysAndValues...)
 		return
 	}
 	l.printS(err, msg, keysAndValues...)
@@ -789,7 +789,7 @@ func (l *loggingT) infoS(loggr logr.Logger, filter LogFilter, msg string, keysAn
 		msg, keysAndValues = filter.FilterS(msg, keysAndValues)
 	}
 	if loggr != nil {
-		loggr.Info(msg, keysAndValues)
+		loggr.Info(msg, keysAndValues...)
 		return
 	}
 	l.printS(nil, msg, keysAndValues...)


Test patch:
diff --git a/klog_test.go b/klog_test.go
index dcbff7f..d71cc96 100644
--- a/klog_test.go
+++ b/klog_test.go
@@ -21,10 +21,12 @@ import (
 	"errors"
 	"flag"
 	"fmt"
+	"github.com/go-logr/logr"
 	"io/ioutil"
 	stdLog "log"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"runtime"
 	"strconv"
@@ -1241,3 +1243,156 @@ func TestLogFilter(t *testing.T) {
 		}
 	}
 }
+
+func TestInfoSWithLogr(t *testing.T) {
+	logger := new(testLogr)
+
+	testDataInfo := []struct {
+		msg        string
+		keysValues []interface{}
+		expected   testLogrEntry
+	}{{
+		msg:        "foo",
+		keysValues: []interface{}{},
+		expected: testLogrEntry{
+			severity:      infoLog,
+			msg:           "foo",
+			keysAndValues: []interface{}{},
+		},
+	}, {
+		msg:        "bar",
+		keysValues: []interface{}{"a", 1},
+		expected: testLogrEntry{
+			severity:      infoLog,
+			msg:           "bar",
+			keysAndValues: []interface{}{"a", 1},
+		},
+	}}
+
+	for _, data := range testDataInfo {
+		t.Run(data.msg, func(t *testing.T) {
+			SetLogger(logger)
+			defer SetLogger(nil)
+			defer logger.reset()
+
+			InfoS(data.msg, data.keysValues...)
+
+			if !reflect.DeepEqual(logger.entries, []testLogrEntry{data.expected}) {
+				t.Errorf("expected: %+v; but got: %+v", []testLogrEntry{data.expected}, logger.entries)
+			}
+		})
+	}
+}
+
+func TestErrorSWithLogr(t *testing.T) {
+	logger := new(testLogr)
+
+	testError := errors.New("testError")
+
+	testDataInfo := []struct {
+		err        error
+		msg        string
+		keysValues []interface{}
+		expected   testLogrEntry
+	}{{
+		err:        testError,
+		msg:        "foo1",
+		keysValues: []interface{}{},
+		expected: testLogrEntry{
+			severity:      errorLog,
+			msg:           "foo1",
+			keysAndValues: []interface{}{},
+			err:           testError,
+		},
+	}, {
+		err:        testError,
+		msg:        "bar1",
+		keysValues: []interface{}{"a", 1},
+		expected: testLogrEntry{
+			severity:      errorLog,
+			msg:           "bar1",
+			keysAndValues: []interface{}{"a", 1},
+			err:           testError,
+		},
+	}, {
+		err:        nil,
+		msg:        "foo2",
+		keysValues: []interface{}{},
+		expected: testLogrEntry{
+			severity:      errorLog,
+			msg:           "foo2",
+			keysAndValues: []interface{}{},
+			err:           nil,
+		},
+	}, {
+		err:        nil,
+		msg:        "bar2",
+		keysValues: []interface{}{"a", 1},
+		expected: testLogrEntry{
+			severity:      errorLog,
+			msg:           "bar2",
+			keysAndValues: []interface{}{"a", 1},
+			err:           nil,
+		},
+	}}
+
+	for _, data := range testDataInfo {
+		t.Run(data.msg, func(t *testing.T) {
+			SetLogger(logger)
+			defer SetLogger(nil)
+			defer logger.reset()
+
+			ErrorS(data.err, data.msg, data.keysValues...)
+
+			if !reflect.DeepEqual(logger.entries, []testLogrEntry{data.expected}) {
+				t.Errorf("expected: %+v; but got: %+v", []testLogrEntry{data.expected}, logger.entries)
+			}
+		})
+	}
+}
+
+type testLogr struct {
+	entries []testLogrEntry
+	mutex   sync.Mutex
+}
+
+type testLogrEntry struct {
+	severity      severity
+	msg           string
+	keysAndValues []interface{}
+	err           error
+}
+
+func (l *testLogr) reset() {
+	l.mutex.Lock()
+	defer l.mutex.Unlock()
+	l.entries = []testLogrEntry{}
+}
+
+func (l *testLogr) Info(msg string, keysAndValues ...interface{}) {
+	l.mutex.Lock()
+	defer l.mutex.Unlock()
+	l.entries = append(l.entries, testLogrEntry{
+		severity:      infoLog,
+		msg:           msg,
+		keysAndValues: keysAndValues,
+	})
+}
+
+func (l *testLogr) Error(err error, msg string, keysAndValues ...interface{}) {
+	l.mutex.Lock()
+	defer l.mutex.Unlock()
+	l.entries = append(l.entries, testLogrEntry{
+		severity:      errorLog,
+		msg:           msg,
+		keysAndValues: keysAndValues,
+		err:           err,
+	})
+}
+
+func (l *testLogr) Enabled() bool               { panic("not implemented") }
+func (l *testLogr) V(int) logr.Logger           { panic("not implemented") }
+func (l *testLogr) WithName(string) logr.Logger { panic("not implemented") }
+func (l *testLogr) WithValues(...interface{}) logr.Logger {
+	panic("not implemented")
+}