Fix Panic for kn source apiserver and kn source binding describe with Sink URI

CHANGELOG.adoc

@@ -18,6 +18,10 @@
 |===
 | | Description | PR
 
+| 🐛
+| Fix panic for `kn source apiserver` and `kn source binding` describe with Sink URI
+| https://github.com/knative/client/pull/901[#901]
+
 | 🐛
 | Fix Panic for `kn trigger describe` with Sink URI
 | https://github.com/knative/client/pull/900[#900]

pkg/kn/commands/source/apiserver/apiserver_test.go

@@ -29,7 +29,6 @@ import (
 
 const testNamespace = "default"
 
-// Helper methods
 var blankConfig clientcmd.ClientConfig
 
 // TODO: Remove that blankConfig hack for tests in favor of overwriting GetConfig()
@@ -83,20 +82,12 @@ func cleanupAPIServerMockClient() {
 	apiServerSourceClientFactory = nil
 }
 
-func createAPIServerSource(name, resourceKind, resourceVersion, serviceAccount, mode, service string, ceOverrides map[string]string) *v1alpha2.ApiServerSource {
+func createAPIServerSource(name, resourceKind, resourceVersion, serviceAccount, mode string, ceOverrides map[string]string, sink duckv1.Destination) *v1alpha2.ApiServerSource {
 	resources := []v1alpha2.APIVersionKindSelector{{
 		APIVersion: resourceVersion,
 		Kind:       resourceKind,
 	}}
 
-	sink := duckv1.Destination{
-		Ref: &duckv1.KReference{
-			Kind:       "Service",
-			Name:       service,
-			APIVersion: "serving.knative.dev/v1",
-			Namespace:  "default",
-		}}
-
 	return clientv1alpha2.NewAPIServerSourceBuilder(name).
 		Resources(resources).
 		ServiceAccount(serviceAccount).
@@ -105,3 +96,14 @@ func createAPIServerSource(name, resourceKind, resourceVersion, serviceAccount,
 		CloudEventOverrides(ceOverrides, []string{}).
 		Build()
 }
+
+func createSinkv1(serviceName, namespace string) duckv1.Destination {
+	return duckv1.Destination{
+		Ref: &duckv1.KReference{
+			Kind:       "Service",
+			Name:       serviceName,
+			APIVersion: "serving.knative.dev/v1",
+			Namespace:  namespace,
+		},
+	}
+}

pkg/kn/commands/source/apiserver/create_test.go

@@ -35,11 +35,11 @@ func TestCreateApiServerSource(t *testing.T) {
 	apiServerClient := v1alpha2.NewMockKnAPIServerSourceClient(t)
 
 	apiServerRecorder := apiServerClient.Recorder()
-	apiServerRecorder.CreateAPIServerSource(createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", "testsvc", map[string]string{"bla": "blub", "foo": "bar"}), nil)
+	apiServerRecorder.CreateAPIServerSource(createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", map[string]string{"bla": "blub", "foo": "bar"}, createSinkv1("testsvc", "default")), nil)
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, dynamicClient, "create", "testsource", "--resource", "Event:v1", "--service-account", "testsa", "--sink", "svc:testsvc", "--mode", "Reference", "--ce-override", "bla=blub", "--ce-override", "foo=bar")
 	assert.NilError(t, err, "ApiServer source should be created")
-	util.ContainsAll(out, "created", "default", "testsource")
+	assert.Assert(t, util.ContainsAll(out, "created", "default", "testsource"))
 
 	apiServerRecorder.Validate()
 }

pkg/kn/commands/source/apiserver/delete_test.go

@@ -33,7 +33,7 @@ func TestApiServerSourceDelete(t *testing.T) {
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "delete", "testsource")
 	assert.NilError(t, err)
-	util.ContainsAll(out, "deleted", "testns", "testsource")
+	assert.Assert(t, util.ContainsAll(out, "deleted", "default", "testsource"))
 
 	apiServerRecorder.Validate()
 }
@@ -47,7 +47,7 @@ func TestDeleteWithError(t *testing.T) {
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "delete", "testsource")
 	assert.ErrorContains(t, err, "testsource")
-	util.ContainsAll(out, "apiserver", "source", "testsource", "not found")
+	assert.Assert(t, util.ContainsAll(out, "apiserver", "source", "testsource", "not found"))
 
 	apiServerRecorder.Validate()
 }

pkg/kn/commands/source/apiserver/describe.go

@@ -29,7 +29,6 @@ import (
 
 // NewAPIServerDescribeCommand to describe an ApiServer source object
 func NewAPIServerDescribeCommand(p *commands.KnParams) *cobra.Command {
-
 	apiServerDescribe := &cobra.Command{
 		Use:   "describe NAME",
 		Short: "Show details of an api-server source",
@@ -110,10 +109,10 @@ func writeResources(dw printers.PrefixWriter, apiVersionKindSelectors []v1alpha2
 
 func writeSink(dw printers.PrefixWriter, sink duckv1.Destination) {
 	subWriter := dw.WriteAttribute("Sink", "")
-	subWriter.WriteAttribute("Name", sink.Ref.Name)
-	subWriter.WriteAttribute("Namespace", sink.Ref.Namespace)
 	ref := sink.Ref
 	if ref != nil {
+		subWriter.WriteAttribute("Name", sink.Ref.Name)
+		subWriter.WriteAttribute("Namespace", sink.Ref.Namespace)
 		subWriter.WriteAttribute("Kind", fmt.Sprintf("%s (%s)", sink.Ref.Kind, sink.Ref.APIVersion))
 	}
 	uri := sink.URI

pkg/kn/commands/source/apiserver/describe_test.go

@@ -22,18 +22,30 @@ import (
 
 	"knative.dev/client/pkg/sources/v1alpha2"
 	"knative.dev/client/pkg/util"
+	"knative.dev/pkg/apis"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+)
+
+var (
+	sinkURI = duckv1.Destination{
+		URI: &apis.URL{
+			Scheme: "https",
+			Host:   "foo",
+		}}
 )
 
 func TestSimpleDescribe(t *testing.T) {
 	apiServerClient := v1alpha2.NewMockKnAPIServerSourceClient(t, "mynamespace")
 
 	apiServerRecorder := apiServerClient.Recorder()
-	sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", "testsvc", map[string]string{"foo": "bar"})
+	sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", map[string]string{"foo": "bar"}, createSinkv1("testsvc", "default"))
+	sampleSource.Namespace = "mynamespace"
 	apiServerRecorder.GetAPIServerSource("testsource", sampleSource, nil)
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "describe", "testsource")
 	assert.NilError(t, err)
-	util.ContainsAll(out, "testsource", "testsa", "Reference", "testsvc", "Service", "Resources", "Event", "v1", "false", "Conditions", "foo", "bar")
+	assert.Assert(t, util.ContainsAll(out, "testsource", "testsa", "Reference", "testsvc", "Service (serving.knative.dev/v1)", "Resources", "Event", "v1", "Conditions", "foo", "bar", "mynamespace", "default"))
+	assert.Assert(t, util.ContainsNone(out, "URI"))
 
 	apiServerRecorder.Validate()
 }
@@ -46,7 +58,22 @@ func TestDescribeError(t *testing.T) {
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "describe", "testsource")
 	assert.ErrorContains(t, err, "testsource")
-	util.ContainsAll(out, "Usage", "testsource")
+	assert.Assert(t, util.ContainsAll(out, "Usage", "testsource"))
+
+	apiServerRecorder.Validate()
+}
+
+func TestDescribeWithSinkURI(t *testing.T) {
+	apiServerClient := v1alpha2.NewMockKnAPIServerSourceClient(t, "mynamespace")
+
+	apiServerRecorder := apiServerClient.Recorder()
+	sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", map[string]string{"foo": "bar"}, sinkURI)
+	sampleSource.Namespace = "mynamespace"
+	apiServerRecorder.GetAPIServerSource("testsource", sampleSource, nil)
+
+	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "describe", "testsource")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(out, "testsource", "testsa", "Reference", "Resources", "Event", "v1", "Conditions", "foo", "bar", "URI", "https", "foo", "mynamespace"))
 
 	apiServerRecorder.Validate()
 }

pkg/kn/commands/source/apiserver/list_test.go

@@ -29,16 +29,16 @@ func TestListAPIServerSource(t *testing.T) {
 	apiServerClient := v1alpha22.NewMockKnAPIServerSourceClient(t)
 
 	apiServerRecorder := apiServerClient.Recorder()
-	sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", "testsvc", nil)
+	sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", nil, createSinkv1("testsvc", "default"))
 	sampleSourceList := v1alpha2.ApiServerSourceList{}
 	sampleSourceList.Items = []v1alpha2.ApiServerSource{*sampleSource}
 
 	apiServerRecorder.ListAPIServerSource(&sampleSourceList, nil)
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "list")
 	assert.NilError(t, err, "sources should be listed")
-	util.ContainsAll(out, "NAME", "RESOURCES", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
-	util.ContainsAll(out, "testsource", "Eventing:v1:false", "mysvc")
+	assert.Assert(t, util.ContainsAll(out, "NAME", "RESOURCES", "SINK", "AGE", "CONDITIONS", "READY", "REASON"))
+	assert.Assert(t, util.ContainsAll(out, "testsource", "Event:v1", "svc:testsvc"))
 
 	apiServerRecorder.Validate()
 }
@@ -53,8 +53,8 @@ func TestListAPIServerSourceEmpty(t *testing.T) {
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsNone(out, "NAME", "RESOURCES", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
-	util.ContainsAll(out, "No", "ApiServer", "source", "found")
+	assert.Assert(t, util.ContainsNone(out, "NAME", "RESOURCES", "SINK", "AGE", "CONDITIONS", "READY", "REASON"))
+	assert.Assert(t, util.ContainsAll(out, "No", "ApiServer", "source", "found"))
 
 	apiServerRecorder.Validate()
 }

pkg/kn/commands/source/apiserver/update_test.go

@@ -37,10 +37,10 @@ func TestApiServerSourceUpdate(t *testing.T) {
 
 	apiServerRecorder := apiServerClient.Recorder()
 
-	present := createAPIServerSource("testsource", "Event", "v1", "testsa1", "Reference", "svc1", map[string]string{"bla": "blub", "foo": "bar"})
+	present := createAPIServerSource("testsource", "Event", "v1", "testsa1", "Reference", map[string]string{"bla": "blub", "foo": "bar"}, createSinkv1("svc1", "default"))
 	apiServerRecorder.GetAPIServerSource("testsource", present, nil)
 
-	updated := createAPIServerSource("testsource", "Event", "v1", "testsa2", "Reference", "svc2", map[string]string{"foo": "baz"})
+	updated := createAPIServerSource("testsource", "Event", "v1", "testsa2", "Reference", map[string]string{"foo": "baz"}, createSinkv1("svc2", "default"))
 	apiServerRecorder.UpdateAPIServerSource(updated, nil)
 
 	output, err := executeAPIServerSourceCommand(apiServerClient, dynamicClient, "update", "testsource", "--service-account", "testsa2", "--sink", "svc:svc2", "--ce-override", "bla-", "--ce-override", "foo=baz")
@@ -54,7 +54,7 @@ func TestApiServerSourceUpdateDeletionTimestampNotNil(t *testing.T) {
 	apiServerClient := v1alpha2.NewMockKnAPIServerSourceClient(t)
 	apiServerRecorder := apiServerClient.Recorder()
 
-	present := createAPIServerSource("testsource", "Event", "v1", "testsa1", "Ref", "svc1", nil)
+	present := createAPIServerSource("testsource", "Event", "v1", "testsa1", "Ref", nil, createSinkv1("svc1", "default"))
 	present.DeletionTimestamp = &metav1.Time{Time: time.Now()}
 	apiServerRecorder.GetAPIServerSource("testsource", present, nil)
 

pkg/kn/commands/source/binding/binding_test.go

@@ -85,8 +85,8 @@ func cleanupSinkBindingClient() {
 	sinkBindingClientFactory = nil
 }
 
-func createSinkBinding(name, service string, subjectGvk schema.GroupVersionKind, subjectName string, ceOverrides map[string]string) *v1alpha2.SinkBinding {
-	sink := createServiceSink(service)
+func createSinkBinding(name, service string, subjectGvk schema.GroupVersionKind, subjectName, namespace string, ceOverrides map[string]string) *v1alpha2.SinkBinding {
+	sink := createServiceSink(service, namespace)
 	builder := clientv1alpha2.NewSinkBindingBuilder(name).
 		Namespace("default").
 		Sink(&sink).
@@ -99,8 +99,12 @@ func createSinkBinding(name, service string, subjectGvk schema.GroupVersionKind,
 	return binding
 }
 
-func createServiceSink(service string) v1.Destination {
+func createServiceSink(service, namespace string) v1.Destination {
 	return v1.Destination{
-		Ref: &v1.KReference{Name: service, Kind: "Service", APIVersion: "serving.knative.dev/v1", Namespace: "default"},
+		Ref: &v1.KReference{Name: service,
+			Kind:       "Service",
+			APIVersion: "serving.knative.dev/v1",
+			Namespace:  namespace,
+		},
 	}
 }

pkg/kn/commands/source/binding/create_test.go

@@ -31,11 +31,11 @@ func TestSimpleCreateBinding(t *testing.T) {
 
 	bindingClient := v1alpha2.NewMockKnSinkBindingClient(t)
 	bindingRecorder := bindingClient.Recorder()
-	bindingRecorder.CreateSinkBinding(createSinkBinding("testbinding", "mysvc", deploymentGvk, "mydeploy", map[string]string{"bla": "blub", "foo": "bar"}), nil)
+	bindingRecorder.CreateSinkBinding(createSinkBinding("testbinding", "mysvc", deploymentGvk, "mydeploy", "default", map[string]string{"bla": "blub", "foo": "bar"}), nil)
 
 	out, err := executeSinkBindingCommand(bindingClient, dynamicClient, "create", "testbinding", "--sink", "svc:mysvc", "--subject", "deployment:apps/v1:mydeploy", "--ce-override", "bla=blub", "--ce-override", "foo=bar")
 	assert.NilError(t, err, "Source should have been created")
-	util.ContainsAll(out, "created", "default", "testbinding")
+	assert.Assert(t, util.ContainsAll(out, "created", "default", "testbinding"))
 
 	bindingRecorder.Validate()
 }

pkg/kn/commands/source/binding/delete_test.go

@@ -33,7 +33,7 @@ func TestSimpleDelete(t *testing.T) {
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "delete", "mybinding")
 	assert.NilError(t, err)
-	util.ContainsAll(out, "deleted", "mynamespace", "mybinding", "sink binding")
+	assert.Assert(t, util.ContainsAll(out, "deleted", "mynamespace", "mybinding", "Sink binding"))
 
 	bindingRecorder.Validate()
 }
@@ -47,7 +47,7 @@ func TestDeleteWithError(t *testing.T) {
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "delete", "mybinding")
 	assert.ErrorContains(t, err, "mybinding")
-	util.ContainsAll(out, "no such", "mybinding")
+	assert.Assert(t, util.ContainsAll(out, "no such", "mybinding"))
 
 	bindingRecorder.Validate()
 }

pkg/kn/commands/source/binding/describe.go

@@ -93,12 +93,12 @@ func writeSinkBinding(dw printers.PrefixWriter, binding *v1alpha2.SinkBinding, p
 
 func writeSink(dw printers.PrefixWriter, namespace string, sink *duckv1.Destination) {
 	subWriter := dw.WriteAttribute("Sink", "")
-	if sink.Ref.Namespace != "" && sink.Ref.Namespace != namespace {
-		subWriter.WriteAttribute("Namespace", sink.Ref.Namespace)
-	}
-	subWriter.WriteAttribute("Name", sink.Ref.Name)
 	ref := sink.Ref
 	if ref != nil {
+		subWriter.WriteAttribute("Name", sink.Ref.Name)
+		if sink.Ref.Namespace != "" && sink.Ref.Namespace != namespace {
+			subWriter.WriteAttribute("Namespace", sink.Ref.Namespace)
+		}
 		subWriter.WriteAttribute("Resource", fmt.Sprintf("%s (%s)", sink.Ref.Kind, sink.Ref.APIVersion))
 	}
 	uri := sink.URI

pkg/kn/commands/source/binding/describe_test.go

@@ -22,6 +22,7 @@ import (
 	"gotest.tools/assert"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1alpha2 "knative.dev/eventing/pkg/apis/sources/v1alpha2"
+	"knative.dev/pkg/apis"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
 	"knative.dev/pkg/apis/duck/v1alpha1"
 	"knative.dev/pkg/tracker"
@@ -30,15 +31,24 @@ import (
 	"knative.dev/client/pkg/util"
 )
 
+var (
+	sinkURI = duckv1.Destination{
+		URI: &apis.URL{
+			Scheme: "https",
+			Host:   "foo",
+		}}
+)
+
 func TestSimpleDescribeWitName(t *testing.T) {
 	bindingClient := clientv1alpha2.NewMockKnSinkBindingClient(t, "mynamespace")
 
 	bindingRecorder := bindingClient.Recorder()
-	bindingRecorder.GetSinkBinding("mybinding", getSinkBindingSource("myapp", map[string]string{"foo": "bar"}), nil)
+	bindingRecorder.GetSinkBinding("mybinding", getSinkBindingSource("myapp", map[string]string{"foo": "bar"}, createServiceSink("mysvc", "myservicenamespace")), nil)
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "describe", "mybinding")
 	assert.NilError(t, err)
-	util.ContainsAll(out, "mybinding", "myapp", "Deployment", "app/v1", "mynamespace", "mysvc", "foo", "bar")
+	assert.Assert(t, util.ContainsAll(out, "mysinkbinding", "myapp", "Deployment", "apps/v1", "mynamespace", "mysvc", "foo", "bar", "myservicenamespace", "Service (serving.knative.dev/v1)"))
+	assert.Assert(t, util.ContainsNone(out, "URI"))
 
 	bindingRecorder.Validate()
 }
@@ -47,11 +57,12 @@ func TestSimpleDescribeWithSelector(t *testing.T) {
 	bindingClient := clientv1alpha2.NewMockKnSinkBindingClient(t, "mynamespace")
 
 	bindingRecorder := bindingClient.Recorder()
-	bindingRecorder.GetSinkBinding("mybinding", getSinkBindingSource("app=myapp,type=test", nil), nil)
+	bindingRecorder.GetSinkBinding("mybinding", getSinkBindingSource("app=myapp,type=test", nil, createServiceSink("mysvc", "myservicenamespace")), nil)
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "describe", "mybinding")
 	assert.NilError(t, err)
-	util.ContainsAll(out, "mybinding", "app:", "myapp", "type:", "test", "Deployment", "app/v1", "mynamespace", "mysvc")
+	assert.Assert(t, util.ContainsAll(out, "mysinkbinding", "app:", "myapp", "type:", "test", "Deployment", "apps/v1", "mynamespace", "mysvc", "myservicenamespace", "Service (serving.knative.dev/v1)"))
+	assert.Assert(t, util.ContainsNone(out, "URI"))
 
 	bindingRecorder.Validate()
 }
@@ -64,26 +75,33 @@ func TestDescribeError(t *testing.T) {
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "describe", "mybinding")
 	assert.ErrorContains(t, err, "mybinding")
-	util.ContainsAll(out, "mybinding")
+	assert.Assert(t, util.ContainsAll(out, "mybinding"))
 
 	bindingRecorder.Validate()
 }
 
-func getSinkBindingSource(nameOrSelector string, ceOverrides map[string]string) *v1alpha2.SinkBinding {
+func TestDescribeWithSinkURI(t *testing.T) {
+	bindingClient := clientv1alpha2.NewMockKnSinkBindingClient(t, "mynamespace")
+
+	bindingRecorder := bindingClient.Recorder()
+	bindingRecorder.GetSinkBinding("mybinding", getSinkBindingSource("myapp", map[string]string{"foo": "bar"}, sinkURI), nil)
+
+	out, err := executeSinkBindingCommand(bindingClient, nil, "describe", "mybinding")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(out, "mysinkbinding", "myapp", "Deployment", "apps/v1", "mynamespace", "foo", "bar", "URI", "https", "foo"))
+
+	bindingRecorder.Validate()
+}
+
+func getSinkBindingSource(nameOrSelector string, ceOverrides map[string]string, sink duckv1.Destination) *v1alpha2.SinkBinding {
 	binding := &v1alpha2.SinkBinding{
 		TypeMeta: v1.TypeMeta{},
 		ObjectMeta: v1.ObjectMeta{
 			Name: "mysinkbinding",
 		},
 		Spec: v1alpha2.SinkBindingSpec{
 			SourceSpec: duckv1.SourceSpec{
-				Sink: duckv1.Destination{
-					Ref: &duckv1.KReference{
-						Kind:      "Service",
-						Namespace: "myservicenamespace",
-						Name:      "mysvc",
-					},
-				},
+				Sink: sink,
 			},
 			BindingSpec: v1alpha1.BindingSpec{
 				Subject: tracker.Reference{