Repository: knative/client (go)

Issue:
Allow to use toReference method from outside, in ex. kn plugins
https://github.com/knative/client/blob/c503b88be21d859227d4e35471130e03c62def47/pkg/kn/commands/source/binding/binding.go#L77

Writing a kn-event plugin I wanted to parse the addressable in the same way as it is parsed in `kn source binding --subject` option. To be able to do it, this function needs to be moved to a public place. Maybe `knative.dev/client/pkg/util` is a good place?

Pull request description:
Making a toReference public to enable reuse from kn-plugins
## Description

Moving a toReference to a public package, so that is it possible to invoke it, from kn plugins.

<!-- Please add a short summary about what the pull request is going to bring on the table -->

## Changes

<!-- Please add list of more detailed changes. These changes should be reflected also in the commit messages -->

* Move `toReference` function to `knative.dev/client/pkg/util.ToTrackerReference`
* Move `parseSelector` function to `knative.dev/client/pkg/util.ParseSelector`

## Reference

<!-- Please add the corresponding issue number which this pull request is about to fix -->
Fixes #1202

<!--
Please add an entry to CHANGELOG.adoc file, too, as part of your Pull Request.

In the following cases, add a short description of PR to the unreleased section in CHANGELOG.adoc:

- 🎁 New feature
- 🐛 Bug fix
- ✨ Feature Update
- 🐣 Refactoring
- 🗑️ Remove feature or internal logic

See other entries in CHANGELOG.adoc as an example for how to add the entry, including a reference to the corresponding issue/PR

PLEASE DON'T ADD THAT LINE HERE IN THE PULL-REQUEST DESCRIPTION BUT DIRECTLY IN CHANGELOG.ADOC AND ADD CHANGELOG.ADOC AS PART OF YOUR PULL-REQUEST.
-->

<!--
To automatically lint go code in this pull request uncomment the line below. You get feedback as comments on your pull request then -->

<!--
/lint
-->


Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 2ef92f84..bd85bf19 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -17,6 +17,9 @@
 |===
 | | Description | PR
 
+| 🐣
+| Making a toReference public to enable reuse from kn-plugins
+| https://github.com/knative/client/pull/1203[#1203]
 | 🐣
 | Do not print serviceUID and configUID labels in service export result
 | https://github.com/knative/client/pull/1194[#1194]
diff --git a/pkg/kn/commands/source/binding/binding.go b/pkg/kn/commands/source/binding/binding.go
index bab932fa..7dd0fec7 100644
--- a/pkg/kn/commands/source/binding/binding.go
+++ b/pkg/kn/commands/source/binding/binding.go
@@ -15,12 +15,9 @@
 package binding
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/spf13/cobra"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/tools/clientcmd"
 	v1alpha2 "knative.dev/eventing/pkg/client/clientset/versioned/typed/sources/v1alpha2"
 	"knative.dev/pkg/tracker"
@@ -74,45 +71,6 @@ func newSinkBindingClient(p *commands.KnParams, cmd *cobra.Command) (clientsourc
 	return clientsourcesv1alpha1.NewKnSourcesClient(client, namespace).SinkBindingClient(), nil
 }
 
-func toReference(subjectArg string, namespace string) (*tracker.Reference, error) {
-	parts := strings.SplitN(subjectArg, ":", 3)
-	if len(parts) < 3 {
-		return nil, fmt.Errorf("invalid subject argument '%s': not in format kind:api/version:nameOrSelector", subjectArg)
-	}
-	kind := parts[0]
-	gv, err := schema.ParseGroupVersion(parts[1])
-	if err != nil {
-		return nil, err
-	}
-	reference := &tracker.Reference{
-		APIVersion: gv.String(),
-		Kind:       kind,
-		Namespace:  namespace,
-	}
-	if !strings.Contains(parts[2], "=") {
-		reference.Name = parts[2]
-	} else {
-		selector, err := parseSelector(parts[2])
-		if err != nil {
-			return nil, err
-		}
-		reference.Selector = &metav1.LabelSelector{MatchLabels: selector}
-	}
-	return reference, nil
-}
-
-func parseSelector(labelSelector string) (map[string]string, error) {
-	selector := map[string]string{}
-	for _, p := range strings.Split(labelSelector, ",") {
-		keyValue := strings.SplitN(p, "=", 2)
-		if len(keyValue) != 2 {
-			return nil, fmt.Errorf("invalid subject label selector '%s', expected format: key1=value,key2=value", labelSelector)
-		}
-		selector[keyValue[0]] = keyValue[1]
-	}
-	return selector, nil
-}
-
 // subjectToString converts a reference to a string representation
 func subjectToString(ref tracker.Reference) string {
 
diff --git a/pkg/kn/commands/source/binding/create.go b/pkg/kn/commands/source/binding/create.go
index 69da93c8..b1edcafe 100644
--- a/pkg/kn/commands/source/binding/create.go
+++ b/pkg/kn/commands/source/binding/create.go
@@ -64,7 +64,7 @@ func NewBindingCreateCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
-			reference, err := toReference(bindingFlags.subject, namespace)
+			reference, err := util.ToTrackerReference(bindingFlags.subject, namespace)
 			if err != nil {
 				return err
 			}
diff --git a/pkg/kn/commands/source/binding/update.go b/pkg/kn/commands/source/binding/update.go
index c438b970..9b499cce 100644
--- a/pkg/kn/commands/source/binding/update.go
+++ b/pkg/kn/commands/source/binding/update.go
@@ -75,7 +75,7 @@ func NewBindingUpdateCommand(p *commands.KnParams) *cobra.Command {
 				b.Sink(destination)
 			}
 			if cmd.Flags().Changed("subject") {
-				reference, err := toReference(bindingFlags.subject, namespace)
+				reference, err := util.ToTrackerReference(bindingFlags.subject, namespace)
 				if err != nil {
 					return err
 				}
diff --git a/pkg/util/parsing_helper.go b/pkg/util/parsing_helper.go
index c800be7c..e35a7f36 100644
--- a/pkg/util/parsing_helper.go
+++ b/pkg/util/parsing_helper.go
@@ -1,4 +1,4 @@
-// Copyright © 2019 The Knative Authors
+// Copyright © 2019-2021 The Knative Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,6 +17,10 @@ package util
 import (
 	"fmt"
 	"strings"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"knative.dev/pkg/tracker"
 )
 
 // OrderedMapAndRemovalListFromArray creates a list of key-value pair using MapFromArrayAllowingSingles, and a list of removal entries
@@ -104,6 +108,49 @@ func AddedAndRemovalListsFromArray(m []string) ([]string, []string) {
 	return stringToAdd, stringToRemove
 }
 
+// ToTrackerReference will parse a subject in form of kind:apiVersion:name for
+// named resources or kind:apiVersion:labelKey1=value1,labelKey2=value2 for
+// matching via a label selector.
+func ToTrackerReference(subject, namespace string) (*tracker.Reference, error) {
+	parts := strings.SplitN(subject, ":", 3)
+	if len(parts) < 3 {
+		return nil, fmt.Errorf("invalid subject argument '%s': not in format kind:api/version:nameOrSelector", subject)
+	}
+	kind := parts[0]
+	gv, err := schema.ParseGroupVersion(parts[1])
+	if err != nil {
+		return nil, err
+	}
+	reference := &tracker.Reference{
+		APIVersion: gv.String(),
+		Kind:       kind,
+		Namespace:  namespace,
+	}
+	if !strings.Contains(parts[2], "=") {
+		reference.Name = parts[2]
+	} else {
+		selector, err := ParseSelector(parts[2])
+		if err != nil {
+			return nil, err
+		}
+		reference.Selector = &metav1.LabelSelector{MatchLabels: selector}
+	}
+	return reference, nil
+}
+
+// ParseSelector will parse a label selector in form of labelKey1=value1,labelKey2=value2.
+func ParseSelector(labelSelector string) (map[string]string, error) {
+	selector := map[string]string{}
+	for _, p := range strings.Split(labelSelector, ",") {
+		keyValue := strings.SplitN(p, "=", 2)
+		if len(keyValue) != 2 {
+			return nil, fmt.Errorf("invalid subject label selector '%s', expected format: key1=value,key2=value", labelSelector)
+		}
+		selector[keyValue[0]] = keyValue[1]
+	}
+	return selector, nil
+}
+
 // mapFromArray takes an array of strings where each item is a (key, value) pair
 // separated by a delimiter and returns a map where keys are mapped to their respective values.
 // If allowSingles is true, values without a delimiter will be added as keys pointing to empty strings


Test patch:
diff --git a/pkg/kn/commands/source/binding/describe_test.go b/pkg/kn/commands/source/binding/describe_test.go
index 1552aab5..e97dee41 100644
--- a/pkg/kn/commands/source/binding/describe_test.go
+++ b/pkg/kn/commands/source/binding/describe_test.go
@@ -130,7 +130,7 @@ func getSinkBindingSource(nameOrSelector string, ceOverrides map[string]string,
 	}
 
 	if strings.Contains(nameOrSelector, "=") {
-		selector, _ := parseSelector(nameOrSelector)
+		selector, _ := util.ParseSelector(nameOrSelector)
 		binding.Spec.Subject.Selector = &v1.LabelSelector{
 			MatchLabels: selector,
 		}
diff --git a/pkg/util/parsing_helper_test.go b/pkg/util/parsing_helper_test.go
index e5914b50..c97d5ff2 100644
--- a/pkg/util/parsing_helper_test.go
+++ b/pkg/util/parsing_helper_test.go
@@ -15,9 +15,12 @@
 package util
 
 import (
+	"fmt"
 	"testing"
 
 	"gotest.tools/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/pkg/tracker"
 )
 
 func TestMapFromArray(t *testing.T) {
@@ -116,3 +119,70 @@ func TestAddedAndRemovalListFromArray(t *testing.T) {
 	assert.DeepEqual(t, []string{"addvalue1"}, addList)
 	assert.DeepEqual(t, []string{}, removeList)
 }
+
+func TestToTrackerReference(t *testing.T) {
+	testToTrackerReference(t,
+		"Broker:eventing.knative.dev/v1beta1:default", "demo",
+		&tracker.Reference{
+			APIVersion: "eventing.knative.dev/v1beta1",
+			Kind:       "Broker",
+			Namespace:  "demo",
+			Name:       "default",
+			Selector:   nil,
+		}, nil)
+	testToTrackerReference(t,
+		"Broker:eventing.knative.dev/v1beta1:default", "",
+		&tracker.Reference{
+			APIVersion: "eventing.knative.dev/v1beta1",
+			Kind:       "Broker",
+			Namespace:  "",
+			Name:       "default",
+			Selector:   nil,
+		}, nil)
+	testToTrackerReference(t,
+		"Job:batch/v1:app=heartbeat-cron,priority=high", "demo",
+		&tracker.Reference{
+			APIVersion: "batch/v1",
+			Kind:       "Job",
+			Namespace:  "demo",
+			Name:       "",
+			Selector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"app":      "heartbeat-cron",
+					"priority": "high",
+				},
+			},
+		}, nil)
+	testToTrackerReference(t, "", "demo", nil,
+		funcRef(func(t *testing.T, err error) {
+			assert.ErrorContains(t, err, "not in format kind:api/version:nameOrSelector")
+		}))
+	testToTrackerReference(t, "Job:batch/v1:app=acme,cmea", "demo", nil,
+		funcRef(func(t *testing.T, err error) {
+			assert.ErrorContains(t, err, "expected format: key1=value,key2=value")
+		}))
+	testToTrackerReference(t, "Job:batch/v1/next:acme", "demo", nil,
+		funcRef(func(t *testing.T, err error) {
+			assert.ErrorContains(t, err, "unexpected GroupVersion string")
+		}))
+}
+
+func testToTrackerReference(t *testing.T, input, namespace string, expected *tracker.Reference, errMatch *func(*testing.T, error)) {
+	t.Helper()
+	t.Run(fmt.Sprintf("%s:%s", input, namespace), func(t *testing.T) {
+		ref, err := ToTrackerReference(input, namespace)
+		if err != nil {
+			if errMatch != nil {
+				m := *errMatch
+				m(t, err)
+			} else {
+				assert.NilError(t, err, "unexpected error")
+			}
+		}
+		assert.DeepEqual(t, expected, ref)
+	})
+}
+
+func funcRef(ref func(t *testing.T, err error)) *func(*testing.T, error) {
+	return &ref
+}