From a9e27c32cca91e98fac876813f8ba7e7cbab3235 Mon Sep 17 00:00:00 2001
From: Marek Siarkowicz <siarkowicz@google.com>
Date: Fri, 12 Mar 2021 14:48:10 +0100
Subject: [PATCH] Fix cases where KMetadata interface has nil pointer

---
 klog.go      |  4 +++
 klog_test.go | 77 +++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/klog.go b/klog.go
index 4892da6a..25483fad 100644
--- a/klog.go
+++ b/klog.go
@@ -81,6 +81,7 @@ import (
 	"math"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strconv"
 	"strings"
@@ -1580,6 +1581,9 @@ func KObj(obj KMetadata) ObjectRef {
 	if obj == nil {
 		return ObjectRef{}
 	}
+	if val := reflect.ValueOf(obj); val.Kind() == reflect.Ptr && val.IsNil() {
+		return ObjectRef{}
+	}
 
 	return ObjectRef{
 		Name:      obj.GetName(),
diff --git a/klog_test.go b/klog_test.go
index 671e4c1f..1e368cba 100644
--- a/klog_test.go
+++ b/klog_test.go
@@ -621,6 +621,26 @@ func BenchmarkHeaderWithDir(b *testing.B) {
 	}
 }
 
+// Ensure that benchmarks have side effects to avoid compiler optimization
+var result ObjectRef
+
+func BenchmarkKRef(b *testing.B) {
+	var r ObjectRef
+	for i := 0; i < b.N; i++ {
+		r = KRef("namespace", "name")
+	}
+	result = r
+}
+
+func BenchmarkKObj(b *testing.B) {
+	a := kMetadataMock{name: "a", ns: "a"}
+	var r ObjectRef
+	for i := 0; i < b.N; i++ {
+		r = KObj(&a)
+	}
+	result = r
+}
+
 func BenchmarkLogs(b *testing.B) {
 	setFlags()
 	defer logging.swap(logging.newBuffers())
@@ -734,6 +754,11 @@ func TestInfoObjectRef(t *testing.T) {
 			},
 			want: "test-name",
 		},
+		{
+			name: "empty",
+			ref:  ObjectRef{},
+			want: "",
+		},
 	}
 
 	for _, tt := range tests {
@@ -746,14 +771,25 @@ func TestInfoObjectRef(t *testing.T) {
 	}
 }
 
-type mockKmeta struct {
+type kMetadataMock struct {
+	name, ns string
+}
+
+func (m kMetadataMock) GetName() string {
+	return m.name
+}
+func (m kMetadataMock) GetNamespace() string {
+	return m.ns
+}
+
+type ptrKMetadataMock struct {
 	name, ns string
 }
 
-func (m mockKmeta) GetName() string {
+func (m *ptrKMetadataMock) GetName() string {
 	return m.name
 }
-func (m mockKmeta) GetNamespace() string {
+func (m *ptrKMetadataMock) GetNamespace() string {
 	return m.ns
 }
 
@@ -763,6 +799,21 @@ func TestKObj(t *testing.T) {
 		obj  KMetadata
 		want ObjectRef
 	}{
+		{
+			name: "nil passed as pointer KMetadata implementation",
+			obj:  (*ptrKMetadataMock)(nil),
+			want: ObjectRef{},
+		},
+		{
+			name: "empty struct passed as non-pointer KMetadata implementation",
+			obj:  kMetadataMock{},
+			want: ObjectRef{},
+		},
+		{
+			name: "nil pointer passed to non-pointer KMetadata implementation",
+			obj:  (*kMetadataMock)(nil),
+			want: ObjectRef{},
+		},
 		{
 			name: "nil",
 			obj:  nil,
@@ -770,7 +821,7 @@ func TestKObj(t *testing.T) {
 		},
 		{
 			name: "with ns",
-			obj:  mockKmeta{"test-name", "test-ns"},
+			obj:  &kMetadataMock{"test-name", "test-ns"},
 			want: ObjectRef{
 				Name:      "test-name",
 				Namespace: "test-ns",
@@ -778,7 +829,7 @@ func TestKObj(t *testing.T) {
 		},
 		{
 			name: "without ns",
-			obj:  mockKmeta{"test-name", ""},
+			obj:  &kMetadataMock{"test-name", ""},
 			want: ObjectRef{
 				Name: "test-name",
 			},
@@ -1029,13 +1080,25 @@ func TestKvListFormat(t *testing.T) {
 			want:       " pod=\"kubedns\" status=\"ready\"",
 		},
 		{
-			keysValues: []interface{}{"pod", KObj(mockKmeta{"test-name", "test-ns"}), "status", "ready"},
+			keysValues: []interface{}{"pod", KObj(kMetadataMock{"test-name", "test-ns"}), "status", "ready"},
 			want:       " pod=\"test-ns/test-name\" status=\"ready\"",
 		},
 		{
-			keysValues: []interface{}{"pod", KObj(mockKmeta{"test-name", ""}), "status", "ready"},
+			keysValues: []interface{}{"pod", KObj(kMetadataMock{"test-name", ""}), "status", "ready"},
 			want:       " pod=\"test-name\" status=\"ready\"",
 		},
+		{
+			keysValues: []interface{}{"pod", KObj(nil), "status", "ready"},
+			want:       " pod=\"\" status=\"ready\"",
+		},
+		{
+			keysValues: []interface{}{"pod", KObj((*ptrKMetadataMock)(nil)), "status", "ready"},
+			want:       " pod=\"\" status=\"ready\"",
+		},
+		{
+			keysValues: []interface{}{"pod", KObj((*kMetadataMock)(nil)), "status", "ready"},
+			want:       " pod=\"\" status=\"ready\"",
+		},
 	}
 
 	for _, d := range testKVList {