Extract reuseable part of sink flag

go.mod

@@ -42,6 +42,7 @@ require (
 	github.com/go-openapi/jsonpointer v0.21.0 // indirect
 	github.com/go-openapi/jsonreference v0.21.0 // indirect
 	github.com/go-openapi/swag v0.23.0 // indirect
+	github.com/gobuffalo/flect v1.0.2 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.4 // indirect

go.sum

@@ -136,6 +136,8 @@ github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/me
 github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI=
 github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI=
 github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8=
+github.com/gobuffalo/flect v1.0.2 h1:eqjPGSo2WmjgY2XlpGwo2NXgL3RucAKo4k4qQMNA5sA=
+github.com/gobuffalo/flect v1.0.2/go.mod h1:A5msMlrHtLqh9umBSnvabjsMrCcCpAyzglnDvkbYKHs=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
 github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=

pkg/commands/flags/sink.go

@@ -16,19 +16,18 @@ package flags
 
 import (
 	"context"
-	"fmt"
-	"strings"
 
 	"github.com/spf13/cobra"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"github.com/spf13/pflag"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"knative.dev/client/pkg/config"
-	"knative.dev/pkg/apis"
-	duckv1 "knative.dev/pkg/apis/duck/v1"
-
 	clientdynamic "knative.dev/client/pkg/dynamic"
+	"knative.dev/client/pkg/flags/sink"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
 )
 
+// SinkFlags holds information about given sink together with optional mappings
+// to allow ease of referencing the common types.
 type SinkFlags struct {
 	Sink         string
 	SinkMappings map[string]schema.GroupVersionResource
@@ -42,26 +41,27 @@ func NewSinkFlag(mapping map[string]schema.GroupVersionResource) *SinkFlags {
 }
 
 // AddWithFlagName configures Sink flag with given flag name and a short flag name
-// pass empty short flag name if you don't want to set one
+// pass empty short flag name if you don't want to set one.
 func (i *SinkFlags) AddWithFlagName(cmd *cobra.Command, fname, short string) {
-	flag := "--" + fname
+	i.AddToFlagSet(cmd.Flags(), fname, short)
+}
+
+// AddToFlagSet configures Sink flag with given flag name and a short flag name
+// pass empty short flag name if you don't want to set one
+func (i *SinkFlags) AddToFlagSet(fs *pflag.FlagSet, fname, short string) {
 	if short == "" {
-		cmd.Flags().StringVar(&i.Sink, fname, "", "")
+		fs.StringVar(&i.Sink, fname, "", "")
 	} else {
-		cmd.Flags().StringVarP(&i.Sink, fname, short, "", "")
+		fs.StringVarP(&i.Sink, fname, short, "", "")
 	}
-	cmd.Flag(fname).Usage = "Addressable sink for events. " +
-		"You can specify a broker, channel, Knative service or URI. " +
-		"Examples: '" + flag + " broker:nest' for a broker 'nest', " +
-		"'" + flag + " channel:pipe' for a channel 'pipe', " +
-		"'" + flag + " ksvc:mysvc:mynamespace' for a Knative service 'mysvc' in another namespace 'mynamespace', " +
-		"'" + flag + " https://event.receiver.uri' for an HTTP URI, " +
-		"'" + flag + " ksvc:receiver' or simply '" + flag + " receiver' for a Knative service 'receiver' in the current namespace. " +
-		"'" + flag + " special.eventing.dev/v1alpha1/channels:pipe' for GroupVersionResource of v1alpha1 'pipe'. " +
-		"If a prefix is not provided, it is considered as a Knative service in the current namespace."
+	fs.Lookup(fname).Usage = sink.Usage(fname)
 	// Use default mapping if empty
 	if i.SinkMappings == nil {
-		i.SinkMappings = defaultSinkMappings
+		i.SinkMappings = make(map[string]schema.GroupVersionResource,
+			len(sink.DefaultMappings))
+		for k, v := range sink.DefaultMappings {
+			i.SinkMappings[k] = v
+		}
 	}
 	for _, p := range config.GlobalConfig.SinkMappings() {
 		//user configuration might override the default configuration
@@ -75,123 +75,48 @@ func (i *SinkFlags) AddWithFlagName(cmd *cobra.Command, fname, short string) {
 
 // Add configures Sink flag with name 'Sink' amd short name 's'
 func (i *SinkFlags) Add(cmd *cobra.Command) {
-	i.AddWithFlagName(cmd, "sink", "s")
+	i.AddWithFlagName(cmd, sink.DefaultFlagName, sink.DefaultFlagShorthand)
+}
+
+// WithDefaultMappings will return a copy of SinkFlags with provided mappings
+// and the default ones.
+func (i *SinkFlags) WithDefaultMappings() *SinkFlags {
+	sf := &SinkFlags{
+		Sink: i.Sink,
+		SinkMappings: make(map[string]schema.GroupVersionResource,
+			len(i.SinkMappings)+len(sink.DefaultMappings)),
+	}
+	for k, v := range sink.DefaultMappings {
+		sf.SinkMappings[k] = v
+	}
+	for k, v := range i.SinkMappings {
+		sf.SinkMappings[k] = v
+	}
+	return sf
 }
 
-// SinkPrefixes maps prefixes used for sinks to their GroupVersionResources.
-var defaultSinkMappings = map[string]schema.GroupVersionResource{
-	"broker": {
-		Resource: "brokers",
-		Group:    "eventing.knative.dev",
-		Version:  "v1",
-	},
-	// Shorthand alias for service
-	"ksvc": {
-		Resource: "services",
-		Group:    "serving.knative.dev",
-		Version:  "v1",
-	},
-	"channel": {
-		Resource: "channels",
-		Group:    "messaging.knative.dev",
-		Version:  "v1",
-	},
+// Parse returns the sink reference, which may refer to URL or to Kubernetes
+// resource. The namespace given should be the current namespace withing the
+// context.
+func (i *SinkFlags) Parse(namespace string) (*sink.Reference, error) {
+	// Use default mapping if empty
+	sf := i.WithDefaultMappings()
+	return sink.Parse(sf.Sink, namespace, sf.SinkMappings)
 }
 
 // ResolveSink returns the Destination referred to by the flags in the acceptor.
 // It validates that any object the user is referring to exists.
 func (i *SinkFlags) ResolveSink(ctx context.Context, knclient clientdynamic.KnDynamicClient, namespace string) (*duckv1.Destination, error) {
-	client := knclient.RawClient()
-	if i.Sink == "" {
-		return nil, nil
-	}
-	// Use default mapping if empty
-	if i.SinkMappings == nil {
-		i.SinkMappings = defaultSinkMappings
-	}
-	prefix, name, ns := parseSink(i.Sink)
-	if prefix == "" {
-		// URI target
-		uri, err := apis.ParseURL(name)
-		if err != nil {
-			return nil, err
-		}
-		return &duckv1.Destination{URI: uri}, nil
-	}
-	gvr, ok := i.SinkMappings[prefix]
-	if !ok {
-		if prefix == "svc" || prefix == "service" {
-			return nil, fmt.Errorf("unsupported Sink prefix: '%s', please use prefix 'ksvc' for knative service", prefix)
-		}
-		idx := strings.LastIndex(prefix, "/")
-		var groupVersion string
-		var kind string
-		if idx != -1 && idx < len(prefix)-1 {
-			groupVersion, kind = prefix[:idx], prefix[idx+1:]
-		} else {
-			kind = prefix
-		}
-		parsedVersion, err := schema.ParseGroupVersion(groupVersion)
-		if err != nil {
-			return nil, err
-		}
-
-		// For the RAWclient the resource name must be in lower case plural form.
-		// This is the best effort to sanitize the inputs, but the safest way is to provide
-		// the appropriate form in user's input.
-		if !strings.HasSuffix(kind, "s") {
-			kind = kind + "s"
-		}
-		kind = strings.ToLower(kind)
-		gvr = parsedVersion.WithResource(kind)
-	}
-	if ns != "" {
-		namespace = ns
-	}
-	obj, err := client.Resource(gvr).Namespace(namespace).Get(ctx, name, metav1.GetOptions{})
+	s, err := i.Parse(namespace)
 	if err != nil {
 		return nil, err
 	}
-
-	destination := &duckv1.Destination{
-		Ref: &duckv1.KReference{
-			Kind:       obj.GetKind(),
-			APIVersion: obj.GetAPIVersion(),
-			Name:       obj.GetName(),
-			Namespace:  namespace,
-		},
-	}
-	return destination, nil
-}
-
-// parseSink takes the string given by the user into the prefix, name and namespace of
-// the object. If the user put a URI instead, the prefix is empty and the name
-// is the whole URI.
-func parseSink(sink string) (string, string, string) {
-	parts := strings.SplitN(sink, ":", 3)
-	switch {
-	case len(parts) == 1:
-		return "ksvc", parts[0], ""
-	case parts[0] == "http" || parts[0] == "https":
-		return "", sink, ""
-	case len(parts) == 3:
-		return parts[0], parts[1], parts[2]
-	default:
-		return parts[0], parts[1], ""
-	}
+	return s.Resolve(ctx, knclient)
 }
 
 // SinkToString prepares a Sink for list output
-func SinkToString(sink duckv1.Destination) string {
-	if sink.Ref != nil {
-		if sink.Ref.Kind == "Service" && strings.HasPrefix(sink.Ref.APIVersion, defaultSinkMappings["ksvc"].Group) {
-			return fmt.Sprintf("ksvc:%s", sink.Ref.Name)
-		} else {
-			return fmt.Sprintf("%s:%s", strings.ToLower(sink.Ref.Kind), sink.Ref.Name)
-		}
-	}
-	if sink.URI != nil {
-		return sink.URI.String()
-	}
-	return ""
+// Deprecated: use (*sink.Reference).AsText instead.
+func SinkToString(dest duckv1.Destination) string {
+	ref := sink.GuessFromDestination(dest)
+	return ref.String()
 }

pkg/commands/flags/sink_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package flags
+package flags_test
 
 import (
 	"context"
@@ -21,6 +21,7 @@ import (
 	"github.com/spf13/cobra"
 	"gotest.tools/v3/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/client/pkg/commands/flags"
 	eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1"
 	messagingv1 "knative.dev/eventing/pkg/apis/messaging/v1"
 	"knative.dev/eventing/pkg/apis/sources/v1beta2"
@@ -58,7 +59,7 @@ func TestSinkFlagAdd(t *testing.T) {
 	}
 	for _, tc := range cases {
 		c := &cobra.Command{Use: "sinktest"}
-		sinkFlags := SinkFlags{}
+		sinkFlags := flags.SinkFlags{}
 		if tc.flagName == "" {
 			sinkFlags.Add(c)
 			assert.Equal(t, tc.expectedFlagName, c.Flag("sink").Name)
@@ -153,7 +154,7 @@ func TestResolve(t *testing.T) {
 	dynamicClient := dynamicfake.CreateFakeKnDynamicClient("default", mysvc, defaultBroker, pipeChannel, pingSource)
 
 	for _, c := range cases {
-		i := &SinkFlags{Sink: c.sink}
+		i := &flags.SinkFlags{Sink: c.sink}
 		result, err := i.ResolveSink(context.Background(), dynamicClient, "default")
 		if c.destination != nil {
 			assert.DeepEqual(t, result, c.destination)
@@ -197,7 +198,7 @@ func TestResolveWithNamespace(t *testing.T) {
 	}
 	dynamicClient := dynamicfake.CreateFakeKnDynamicClient("my-namespace", mysvc, defaultBroker, pipeChannel)
 	for _, c := range cases {
-		i := &SinkFlags{Sink: c.sink}
+		i := &flags.SinkFlags{Sink: c.sink}
 		result, err := i.ResolveSink(context.Background(), dynamicClient, "default")
 		if c.destination != nil {
 			assert.DeepEqual(t, result, c.destination)
@@ -210,34 +211,59 @@ func TestResolveWithNamespace(t *testing.T) {
 }
 
 func TestSinkToString(t *testing.T) {
-	sink := duckv1.Destination{
-		Ref: &duckv1.KReference{Kind: "Service",
+	tcs := []resolveCase{{
+		sink: "mysvc",
+		destination: &duckv1.Destination{Ref: &duckv1.KReference{
+			Kind:       "Service",
 			APIVersion: "serving.knative.dev/v1",
 			Namespace:  "my-namespace",
-			Name:       "mysvc"}}
-	expected := "ksvc:mysvc"
-	assert.Equal(t, expected, SinkToString(sink))
-	sink = duckv1.Destination{
-		Ref: &duckv1.KReference{Kind: "Broker",
+			Name:       "mysvc",
+		}},
+	}, {
+		sink: "broker:default",
+		destination: &duckv1.Destination{Ref: &duckv1.KReference{
+			Kind:       "Broker",
 			APIVersion: "eventing.knative.dev/v1",
 			Namespace:  "my-namespace",
-			Name:       "default"}}
-	expected = "broker:default"
-	assert.Equal(t, expected, SinkToString(sink))
-	sink = duckv1.Destination{
-		Ref: &duckv1.KReference{Kind: "Service",
+			Name:       "default",
+		}},
+	}, {
+		sink: "svc:mysvc",
+		destination: &duckv1.Destination{Ref: &duckv1.KReference{
+			Kind:       "Service",
 			APIVersion: "v1",
 			Namespace:  "my-namespace",
-			Name:       "mysvc"}}
-	expected = "service:mysvc"
-	assert.Equal(t, expected, SinkToString(sink))
+			Name:       "mysvc",
+		}},
+	}, {
+		sink: "things.acme.dev/v1alpha1:abc",
+		destination: &duckv1.Destination{Ref: &duckv1.KReference{
+			Kind:       "Thing",
+			APIVersion: "acme.dev/v1alpha1",
+			Namespace:  "my-namespace",
+			Name:       "abc",
+		}},
+	}, {
+		sink: "http://target.example.com",
+		destination: &duckv1.Destination{
+			URI: url(t, "http://target.example.com"),
+		},
+	}, {
+		sink:        "",
+		destination: &duckv1.Destination{},
+	}}
+	for i := range tcs {
+		tc := tcs[i]
+		t.Run(tc.sink, func(t *testing.T) {
+			got := flags.SinkToString(*tc.destination)
+			assert.Equal(t, got, tc.sink)
+		})
+	}
+}
 
-	uri := "http://target.example.com"
-	targetExampleCom, err := apis.ParseURL(uri)
+func url(t testing.TB, uri string) *apis.URL {
+	t.Helper()
+	u, err := apis.ParseURL(uri)
 	assert.NilError(t, err)
-	sink = duckv1.Destination{
-		URI: targetExampleCom,
-	}
-	assert.Equal(t, uri, SinkToString(sink))
-	assert.Equal(t, "", SinkToString(duckv1.Destination{}))
+	return u
 }