Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fully qualified resource reference type #351

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 38 additions & 0 deletions k8s_references.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package klog
import (
"fmt"
"reflect"
"strings"

"github.com/go-logr/logr"
)
Expand Down Expand Up @@ -156,3 +157,40 @@ func (ks kobjSlice) process() ([]interface{}, error) {
}
return objectRefs, nil
}

// ResourceRef is a full Kubernetes resource reference composed of Group, Version, Kind, Namespace and Name.
killianmuldoon marked this conversation as resolved.
Show resolved Hide resolved
type ResourceRef struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please check if this can be named as KResourceRef now that there is no func KResourceRef(....) ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels slightly strange that KObj and KRef are functions while this would be a struct, but I'm happy to go either way on this if there's consensus.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. @pohly which one do you think would be more natural to the framework ?

Copy link

@pohly pohly Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sake of clarity and usability, I prefer just the struct (see comment above) But for the sake of consistency we would have to use a function.

@serathius: what do you think?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @pohly that struct should be preferred.

Group string `json:"group,omitempty"`
Version string `json:"version,omitempty"`
Kind string `json:"kind,omitempty"`
Namespace string `json:"namespace,omitempty"`
Name string `json:"name,omitempty"`
}

// KResourceRef returns ResourceRef from group, version, kind, namespace and name.
func KResourceRef(group, version, kind, namespace, name string) ResourceRef {
killianmuldoon marked this conversation as resolved.
Show resolved Hide resolved
return ResourceRef{
Group: group,
Version: version,
Kind: kind,
Namespace: namespace,
Name: name,
}
}

func (l ResourceRef) String() string {
var parts []string
for _, s := range []string{l.Group, l.Version, l.Kind, l.Namespace, l.Name} {
if strings.TrimSpace(s) != "" {
parts = append(parts, s)
}
}
return strings.Join(parts, "/")
}

// MarshalLog ensures that loggers with support for structured output will log
// as a struct by removing the String method via a custom type.
func (l ResourceRef) MarshalLog() interface{} {
type kResourceRefWithoutStringFunc ResourceRef
killianmuldoon marked this conversation as resolved.
Show resolved Hide resolved
return kResourceRefWithoutStringFunc(l)
}
132 changes: 132 additions & 0 deletions klog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,138 @@ func TestKRef(t *testing.T) {
}
}

func TestKResourceRef(t *testing.T) {
tests := []struct {
testname string
group string
version string
kind string
name string
namespace string
want ResourceRef
}{
{
testname: "with group, version, kind, namespace and name",
name: "test-name",
namespace: "test-ns",
group: "test-group",
version: "test-version",
kind: "test-kind",
want: ResourceRef{
Group: "test-group",
Version: "test-version",
Kind: "test-kind",
Name: "test-name",
Namespace: "test-ns",
},
},
{
testname: "with group, version, namespace and name",
name: "test-name",
namespace: "test-ns",
group: "test-group",
version: "test-version",
want: ResourceRef{
Group: "test-group",
Version: "test-version",
Name: "test-name",
Namespace: "test-ns",
},
},
{
testname: "with group, namespace and name",
name: "test-name",
namespace: "test-ns",
group: "test-group",
want: ResourceRef{
Group: "test-group",
Name: "test-name",
Namespace: "test-ns",
},
},
{
testname: "with namespace and name",
name: "test-name",
namespace: "test-ns",
want: ResourceRef{
Name: "test-name",
Namespace: "test-ns",
},
},
{
testname: "with only name",
name: "test-name",
want: ResourceRef{
Name: "test-name",
},
},
}

for _, tt := range tests {
t.Run(tt.testname, func(t *testing.T) {
if KResourceRef(tt.group, tt.version, tt.kind, tt.namespace, tt.name) != tt.want {
t.Errorf("expected %v, got %v", tt.want, KResourceRef(tt.group, tt.version, tt.kind, tt.namespace, tt.name))
}
})
}
}

func TestKResourceString(t *testing.T) {
tests := []struct {
testname string
group string
version string
kind string
name string
namespace string
want string
}{
{
testname: "with group, version, kind, namespace and name",
group: "test-group",
version: "test-version",
killianmuldoon marked this conversation as resolved.
Show resolved Hide resolved
kind: "test-kind",
namespace: "test-ns",
name: "test-name",
want: "test-group/test-version/test-kind/test-ns/test-name",
},
{
testname: "with group, version, namespace and name",
group: "test-group",
version: "test-version",
namespace: "test-ns",
name: "test-name",
want: "test-group/test-version/test-ns/test-name",
},
{
testname: "with group, namespace and name",
group: "test-group",
namespace: "test-ns",
name: "test-name",
want: "test-group/test-ns/test-name",
},
{
testname: "with namespace and name",
namespace: "test-ns",
name: "test-name",
want: "test-ns/test-name",
},
{
testname: "with only name",
name: "test-name",
want: "test-name",
},
}

for _, tt := range tests {
t.Run(tt.testname, func(t *testing.T) {
if KResourceRef(tt.group, tt.version, tt.kind, tt.namespace, tt.name).String() != tt.want {
t.Errorf("expected %v, got %v", tt.want, KResourceRef(tt.group, tt.version, tt.kind, tt.namespace, tt.name))
}
})
}
}

// Test that InfoS and InfoSDepth work as advertised.
func TestInfoS(t *testing.T) {
defer CaptureState().Restore()
Expand Down