From 6dbe622bff7817af584ea424df0a66b1b1bee3fa Mon Sep 17 00:00:00 2001
From: wojtekt <wojtekt@google.com>
Date: Mon, 5 Aug 2019 08:37:55 +0200
Subject: [PATCH] Fix GetReference function

Kubernetes-commit: 399d09ce4ad1728fcecdce09503a07cf1bfecef6
---
 tools/reference/ref.go      | 53 +++++++++++++------------------------
 tools/reference/ref_test.go | 16 ++++++-----
 2 files changed, 27 insertions(+), 42 deletions(-)

diff --git a/tools/reference/ref.go b/tools/reference/ref.go
index 573d948a..442a991c 100644
--- a/tools/reference/ref.go
+++ b/tools/reference/ref.go
@@ -19,8 +19,6 @@ package reference
 import (
 	"errors"
 	"fmt"
-	"net/url"
-	"strings"
 
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -30,8 +28,7 @@ import (
 
 var (
 	// Errors that could be returned by GetReference.
-	ErrNilObject  = errors.New("can't reference a nil object")
-	ErrNoSelfLink = errors.New("selfLink was empty, can't make reference")
+	ErrNilObject = errors.New("can't reference a nil object")
 )
 
 // GetReference returns an ObjectReference which refers to the given
@@ -47,20 +44,6 @@ func GetReference(scheme *runtime.Scheme, obj runtime.Object) (*v1.ObjectReferen
 		return ref, nil
 	}
 
-	gvk := obj.GetObjectKind().GroupVersionKind()
-
-	// if the object referenced is actually persisted, we can just get kind from meta
-	// if we are building an object reference to something not yet persisted, we should fallback to scheme
-	kind := gvk.Kind
-	if len(kind) == 0 {
-		// TODO: this is wrong
-		gvks, _, err := scheme.ObjectKinds(obj)
-		if err != nil {
-			return nil, err
-		}
-		kind = gvks[0].Kind
-	}
-
 	// An object that implements only List has enough metadata to build a reference
 	var listMeta metav1.Common
 	objectMeta, err := meta.Accessor(obj)
@@ -73,29 +56,29 @@ func GetReference(scheme *runtime.Scheme, obj runtime.Object) (*v1.ObjectReferen
 		listMeta = objectMeta
 	}
 
-	// if the object referenced is actually persisted, we can also get version from meta
-	version := gvk.GroupVersion().String()
-	if len(version) == 0 {
-		selfLink := listMeta.GetSelfLink()
-		if len(selfLink) == 0 {
-			return nil, ErrNoSelfLink
-		}
-		selfLinkUrl, err := url.Parse(selfLink)
+	gvk := obj.GetObjectKind().GroupVersionKind()
+
+	// If object meta doesn't contain data about kind and/or version,
+	// we are falling back to scheme.
+	//
+	// TODO: This doesn't work for CRDs, which are not registered in scheme.
+	if gvk.Empty() {
+		gvks, _, err := scheme.ObjectKinds(obj)
 		if err != nil {
 			return nil, err
 		}
-		// example paths: /<prefix>/<version>/*
-		parts := strings.Split(selfLinkUrl.Path, "/")
-		if len(parts) < 4 {
-			return nil, fmt.Errorf("unexpected self link format: '%v'; got version '%v'", selfLink, version)
-		}
-		if parts[1] == "api" {
-			version = parts[2]
-		} else {
-			version = parts[2] + "/" + parts[3]
+		if len(gvks) == 0 || gvks[0].Empty() {
+			return nil, fmt.Errorf("unexpected gvks registered for object %T: %v", obj, gvks)
 		}
+		// TODO: The same object can be registered for multiple group versions
+		// (although in practise this doesn't seem to be used).
+		// In such case, the version set may not be correct.
+		gvk = gvks[0]
 	}
 
+	kind := gvk.Kind
+	version := gvk.GroupVersion().String()
+
 	// only has list metadata
 	if objectMeta == nil {
 		return &v1.ObjectReference{
diff --git a/tools/reference/ref_test.go b/tools/reference/ref_test.go
index b0cf06a9..7a478374 100644
--- a/tools/reference/ref_test.go
+++ b/tools/reference/ref_test.go
@@ -37,29 +37,31 @@ func TestGetReferenceRefVersion(t *testing.T) {
 	tests := []struct {
 		name               string
 		input              *TestRuntimeObj
+		groupVersion       schema.GroupVersion
 		expectedRefVersion string
 	}{
 		{
-			name: "api from selflink",
+			name: "v1 GV from scheme",
 			input: &TestRuntimeObj{
-				ObjectMeta: metav1.ObjectMeta{SelfLink: "/api/v1/namespaces"},
+				ObjectMeta: metav1.ObjectMeta{SelfLink: "/bad-selflink/unused"},
 			},
+			groupVersion:       schema.GroupVersion{Group: "", Version: "v1"},
 			expectedRefVersion: "v1",
 		},
 		{
-			name: "foo.group/v3 from selflink",
+			name: "foo.group/v3 GV from scheme",
 			input: &TestRuntimeObj{
-				ObjectMeta: metav1.ObjectMeta{SelfLink: "/apis/foo.group/v3/namespaces"},
+				ObjectMeta: metav1.ObjectMeta{SelfLink: "/bad-selflink/unused"},
 			},
+			groupVersion:       schema.GroupVersion{Group: "foo.group", Version: "v3"},
 			expectedRefVersion: "foo.group/v3",
 		},
 	}
 
-	scheme := runtime.NewScheme()
-	scheme.AddKnownTypes(schema.GroupVersion{Group: "this", Version: "is ignored"}, &TestRuntimeObj{})
-
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			scheme := runtime.NewScheme()
+			scheme.AddKnownTypes(test.groupVersion, &TestRuntimeObj{})
 			ref, err := GetReference(scheme, test.input)
 			if err != nil {
 				t.Fatal(err)