Skip to content

Commit

Permalink
Merge pull request #226 from serathius/fix-nil
Browse files Browse the repository at this point in the history
Fix cases where KMetadata interface has nil object
  • Loading branch information
k8s-ci-robot committed Mar 12, 2021
2 parents 0f69eb5 + a9e27c3 commit 407242c
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 7 deletions.
4 changes: 4 additions & 0 deletions klog.go
Expand Up @@ -81,6 +81,7 @@ import (
"math"
"os"
"path/filepath"
"reflect"
"runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -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(),
Expand Down
77 changes: 70 additions & 7 deletions klog_test.go
Expand Up @@ -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())
Expand Down Expand Up @@ -734,6 +754,11 @@ func TestInfoObjectRef(t *testing.T) {
},
want: "test-name",
},
{
name: "empty",
ref: ObjectRef{},
want: "",
},
}

for _, tt := range tests {
Expand All @@ -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
}

Expand All @@ -763,22 +799,37 @@ 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,
want: ObjectRef{},
},
{
name: "with ns",
obj: mockKmeta{"test-name", "test-ns"},
obj: &kMetadataMock{"test-name", "test-ns"},
want: ObjectRef{
Name: "test-name",
Namespace: "test-ns",
},
},
{
name: "without ns",
obj: mockKmeta{"test-name", ""},
obj: &kMetadataMock{"test-name", ""},
want: ObjectRef{
Name: "test-name",
},
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 407242c

Please sign in to comment.