v0.16.1 release backports (#982)

* fix(docs): Fix tekton task link in README (#934) * Fix missing NAMESPACE column header (#951) * Fix missing NAMESPACE column header * Fix missing namespace column header for 'kn source list -A' * List inbuilt sources if CRD access is restricted (#948) * List inbuilt sources if CRD access is restricted Fixes #947 - Identify restricted access error - If server returns restricted access error, fallback to listing only eventing inbuilt sources using their GVKs. - List any inbuilt source (ApiServerSource) object and read the error to know if eventing is installed for `kn source list-types`. * Fix golint warnings * Remove unused imports * Verify each built in source before listing source types * Improve the check if sources are not installed in the cluster * Update finding forbidden error * Update finding errors * Add unit tests for IsForbiddenError util * Add unit tests * Add tests for dynamic pkg library * Add unit tests for case when no sources are installed * Update test name * Use Tekton Catalog GA structure for tasks (#966) * fix: `kn source list` command print spelling problems (#963) * Fix exit code on service delete and revision delete (#971) * Fix exit code 0 upon service delete * Mentioned bug fix in CHANGELOG.adoc * Add error check test for service not found * Fix for kn revision delete failure exit code and add test cases * Testing changes in test cases for failure in intergration tests * Fix test case error causing integration test failure * fix(volume): Generate volume name compliant with DNS Label names (#975) * fix(volume): Volume names to not contain dots Replace non alphanumberic characters with hyphen as the volumen name must be a DNS_LABEL (https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names) ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#volume-v1-core * Generate volume name compliant with DNS Label names Volume names to follow the DNS label standard as defined in RFC 1123. This means the name must: - contain at most 63 characters - contain only lowercase alphanumeric characters or '-' - start with an alphanumeric character - end with an alphanumeric character * Set client custom GVK for source list for machine readable output (#980) - Use custom GVK {Group: client.knative.dev, Version: v1alpha1, Kind: SourceList} - Source list may contain different source types CO and machine readable output (using -o) requires List object to have GVK set, since the list contains different types of source COs, we set a custom client GVK on it. Fixes #953 * Update CHANGELOG for v0.16.1 Co-authored-by: kaustubh <[email protected]> Co-authored-by: Chris Suszynski <[email protected]> Co-authored-by: tianfeiyu <[email protected]> Co-authored-by: Himanshu Ranjan <[email protected]>
knative · Aug 19, 2020 · 4828914 · 4828914
1 parent 5cb7830
commit 4828914
Show file tree

Hide file tree

Showing 27 changed files with 491 additions and 48 deletions.
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -12,6 +12,37 @@
 | https://github.com/knative/client/pull/[#]
 ////
 
+## v0.16.1 (2020-08-18)
+
+[cols="1,10,3", options="header", width="100%"]
+|===
+| | Description | PR
+
+| 🐛
+| Fix missing NAMESPACE column header for 'kn source list -A'
+| https://github.com/knative/client/pull/951[#951]
+
+| 🐛
+| Fix exit code for `kn service delete` and `kn revision delete` failures
+| https://github.com/knative/client/pull/971[#971]
+
+| 🐛
+| Fix client side volume name generation
+| https://github.com/knative/client/pull/975[#975]
+
+| ✨
+| `kn source list` output now has client custom GVK set as `{Group: client.knative.dev, Version: v1alpha1, Kind: SourceList}`
+| https://github.com/knative/client/pull/980[#980]
+
+| 🐛
+| fix(kn source list): list inbuilt sources if crd access is restricted
+| https://github.com/knative/client/pull/948[#948]
+
+| 🐛
+| fix(tekton e2e): Refer tasks from new tekton catalog task structure
+| https://github.com/knative/client/pull/966[#966]
+|===
+
 ## v0.16.0 (2020-07-14)
 
 [cols="1,10,3", options="header", width="100%"]

diff --git a/README.md b/README.md
@@ -21,7 +21,8 @@ line or from within Shell scripts.
 - A thin client-specific API in golang which helps in tasks like synchronously
   waiting on Knative service write operations.
 - An easy integration of Knative into Tekton Pipelines by using
-  [`kn` in a Tekton `Task`](https://github.com/tektoncd/catalog/tree/master/kn).
+  [`kn` in a Tekton `Task`](https://github.com/tektoncd/catalog/tree/master/task/kn).
+
 
 This client uses the
 [Knative Serving](https://github.com/knative/docs/blob/master/docs/serving/spec/knative-api-specification-1.0.md)

diff --git a/lib/test/revision.go b/lib/test/revision.go
@@ -60,11 +60,11 @@ func RevisionMultipleDelete(r *KnRunResultCollector, existRevision1, existRevisi
 	assert.Check(r.T(), strings.Contains(out.Stdout, existRevision2), "Required revision2 does not exist")
 
 	out = r.KnTest().Kn().Run("revision", "delete", existRevision1, existRevision2, nonexistRevision)
-	r.AssertNoError(out)
+	r.AssertError(out)
 
 	assert.Check(r.T(), util.ContainsAll(out.Stdout, "Revision", existRevision1, "deleted", "namespace", r.KnTest().Kn().Namespace()), "Failed to get 'deleted' first revision message")
 	assert.Check(r.T(), util.ContainsAll(out.Stdout, "Revision", existRevision2, "deleted", "namespace", r.KnTest().Kn().Namespace()), "Failed to get 'deleted' second revision message")
-	assert.Check(r.T(), util.ContainsAll(out.Stdout, "revisions.serving.knative.dev", nonexistRevision, "not found"), "Failed to get 'not found' error")
+	assert.Check(r.T(), util.ContainsAll(out.Stderr, "revisions.serving.knative.dev", nonexistRevision, "not found"), "Failed to get 'not found' error")
 }
 
 // RevisionDescribeWithPrintFlags verifies describing given revision using print flag '--output=name'

diff --git a/pkg/dynamic/client.go b/pkg/dynamic/client.go
@@ -15,6 +15,9 @@
 package dynamic
 
 import (
+	"errors"
+	"strings"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
@@ -31,6 +34,9 @@ const (
 	crdKinds          = "customresourcedefinitions"
 	sourcesLabelKey   = "duck.knative.dev/source"
 	sourcesLabelValue = "true"
+	sourceListGroup   = "client.knative.dev"
+	sourceListVersion = "v1alpha1"
+	sourceListKind    = "SourceList"
 )
 
 // KnDynamicClient to client-go Dynamic client. All methods are relative to the
@@ -42,12 +48,15 @@ type KnDynamicClient interface {
 	// ListCRDs returns list of CRDs with their type and name
 	ListCRDs(options metav1.ListOptions) (*unstructured.UnstructuredList, error)
 
-	// ListSourceCRDs returns list of eventing sources CRDs
+	// ListSourcesTypes returns list of eventing sources CRDs
 	ListSourcesTypes() (*unstructured.UnstructuredList, error)
 
 	// ListSources returns list of available source objects
 	ListSources(types ...WithType) (*unstructured.UnstructuredList, error)
 
+	// ListSourcesUsingGVKs returns list of available source objects using given list of GVKs
+	ListSourcesUsingGVKs(*[]schema.GroupVersionKind, ...WithType) (*unstructured.UnstructuredList, error)
+
 	// RawClient returns the raw dynamic client interface
 	RawClient() dynamic.Interface
 }
@@ -105,14 +114,18 @@ func (c knDynamicClient) RawClient() dynamic.Interface {
 // only given types of source objects
 func (c *knDynamicClient) ListSources(types ...WithType) (*unstructured.UnstructuredList, error) {
 	var (
-		sourceList               unstructured.UnstructuredList
-		options                  metav1.ListOptions
-		numberOfsourceTypesFound int
+		sourceList unstructured.UnstructuredList
+		options    metav1.ListOptions
 	)
 	sourceTypes, err := c.ListSourcesTypes()
 	if err != nil {
 		return nil, err
 	}
+
+	if sourceTypes == nil || len(sourceTypes.Items) == 0 {
+		return nil, errors.New("no sources found on the backend, please verify the installation")
+	}
+
 	namespace := c.Namespace()
 	filters := WithTypes(types).List()
 	// For each source type available, find out each source types objects
@@ -140,16 +153,47 @@ func (c *knDynamicClient) ListSources(types ...WithType) (*unstructured.Unstruct
 		}
 
 		if len(sList.Items) > 0 {
-			// keep a track if we found source objects of different types
-			numberOfsourceTypesFound++
 			sourceList.Items = append(sourceList.Items, sList.Items...)
-			sourceList.SetGroupVersionKind(sList.GetObjectKind().GroupVersionKind())
 		}
 	}
-	// Clear the Group and Version for list if there are multiple types of source objects found
-	// Keep the source's GVK if there is only one type of source objects found or requested via --type filter
-	if numberOfsourceTypesFound > 1 {
-		sourceList.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "", Kind: "List"})
+	if len(sourceList.Items) > 0 {
+		sourceList.SetGroupVersionKind(schema.GroupVersionKind{Group: sourceListGroup, Version: sourceListVersion, Kind: sourceListKind})
+	}
+	return &sourceList, nil
+}
+
+// ListSourcesUsingGVKs returns list of available source objects using given list of GVKs
+func (c *knDynamicClient) ListSourcesUsingGVKs(gvks *[]schema.GroupVersionKind, types ...WithType) (*unstructured.UnstructuredList, error) {
+	if gvks == nil {
+		return nil, nil
+	}
+
+	var (
+		sourceList unstructured.UnstructuredList
+		options    metav1.ListOptions
+	)
+	namespace := c.Namespace()
+	filters := WithTypes(types).List()
+
+	for _, gvk := range *gvks {
+		if len(filters) > 0 && !util.SliceContainsIgnoreCase(filters, gvk.Kind) {
+			continue
+		}
+
+		gvr := gvk.GroupVersion().WithResource(strings.ToLower(gvk.Kind) + "s")
+
+		// list objects of source type with this GVR
+		sList, err := c.client.Resource(gvr).Namespace(namespace).List(options)
+		if err != nil {
+			return nil, err
+		}
+
+		if len(sList.Items) > 0 {
+			sourceList.Items = append(sourceList.Items, sList.Items...)
+		}
+	}
+	if len(sourceList.Items) > 0 {
+		sourceList.SetGroupVersionKind(schema.GroupVersionKind{Group: sourceListGroup, Version: sourceListVersion, Kind: sourceListKind})
 	}
 	return &sourceList, nil
 }
diff --git a/pkg/dynamic/client_test.go b/pkg/dynamic/client_test.go
@@ -95,6 +95,13 @@ func TestListSources(t *testing.T) {
 		assert.Check(t, util.ContainsAll(err.Error(), "can't", "find", "source", "kind", "CRD"))
 	})
 
+	t.Run("sources not installed", func(t *testing.T) {
+		client := createFakeKnDynamicClient(testNamespace)
+		_, err := client.ListSources()
+		assert.Check(t, err != nil)
+		assert.Check(t, util.ContainsAll(err.Error(), "no sources", "found", "backend", "verify", "installation"))
+	})
+
 	t.Run("source list empty", func(t *testing.T) {
 		client := createFakeKnDynamicClient(testNamespace,
 			newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"),
@@ -115,7 +122,44 @@ func TestListSources(t *testing.T) {
 		sources, err := client.ListSources(WithTypeFilter("pingsource"), WithTypeFilter("ApiServerSource"))
 		assert.NilError(t, err)
 		assert.Equal(t, len(sources.Items), 2)
+		assert.DeepEqual(t, sources.GroupVersionKind(), schema.GroupVersionKind{sourceListGroup, sourceListVersion, sourceListKind})
+	})
+}
+
+func TestListSourcesUsingGVKs(t *testing.T) {
+	t.Run("No GVKs given", func(t *testing.T) {
+		client := createFakeKnDynamicClient(testNamespace)
+		assert.Check(t, client.RawClient() != nil)
+		s, err := client.ListSourcesUsingGVKs(nil)
+		assert.NilError(t, err)
+		assert.Check(t, s == nil)
 	})
+
+	t.Run("source list with given GVKs", func(t *testing.T) {
+		client := createFakeKnDynamicClient(testNamespace,
+			newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"),
+			newSourceCRDObjWithSpec("apiserversources", "sources.knative.dev", "v1alpha1", "ApiServerSource"),
+			newSourceUnstructuredObj("p1", "sources.knative.dev/v1alpha1", "PingSource"),
+			newSourceUnstructuredObj("a1", "sources.knative.dev/v1alpha1", "ApiServerSource"),
+		)
+		assert.Check(t, client.RawClient() != nil)
+		gv := schema.GroupVersion{"sources.knative.dev", "v1alpha1"}
+		gvks := []schema.GroupVersionKind{gv.WithKind("ApiServerSource"), gv.WithKind("PingSource")}
+
+		s, err := client.ListSourcesUsingGVKs(&gvks)
+		assert.NilError(t, err)
+		assert.Check(t, s != nil)
+		assert.Equal(t, len(s.Items), 2)
+		assert.DeepEqual(t, s.GroupVersionKind(), schema.GroupVersionKind{sourceListGroup, sourceListVersion, sourceListKind})
+
+		// withType
+		s, err = client.ListSourcesUsingGVKs(&gvks, WithTypeFilter("PingSource"))
+		assert.NilError(t, err)
+		assert.Check(t, s != nil)
+		assert.Equal(t, len(s.Items), 1)
+		assert.DeepEqual(t, s.GroupVersionKind(), schema.GroupVersionKind{sourceListGroup, sourceListVersion, sourceListKind})
+	})
+
 }
 
 // createFakeKnDynamicClient gives you a dynamic client for testing containing the given objects.

diff --git a/pkg/dynamic/lib.go b/pkg/dynamic/lib.go
@@ -16,6 +16,7 @@ package dynamic
 
 import (
 	"fmt"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -123,3 +124,25 @@ func (types WithTypes) List() []string {
 	}
 	return stypes
 }
+
+// UnstructuredCRDFromGVK constructs an unstructured object using the given GVK
+func UnstructuredCRDFromGVK(gvk schema.GroupVersionKind) *unstructured.Unstructured {
+	name := fmt.Sprintf("%ss.%s", strings.ToLower(gvk.Kind), gvk.Group)
+	plural := fmt.Sprintf("%ss", strings.ToLower(gvk.Kind))
+	u := &unstructured.Unstructured{}
+	u.SetUnstructuredContent(map[string]interface{}{
+		"metadata": map[string]interface{}{
+			"name": name,
+		},
+		"spec": map[string]interface{}{
+			"group":   gvk.Group,
+			"version": gvk.Version,
+			"names": map[string]interface{}{
+				"kind":   gvk.Kind,
+				"plural": plural,
+			},
+		},
+	})
+
+	return u
+}
diff --git a/pkg/dynamic/lib_test.go b/pkg/dynamic/lib_test.go
@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"gotest.tools/assert"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"knative.dev/client/pkg/util"
 )
@@ -77,3 +78,22 @@ func TestGVRFromUnstructured(t *testing.T) {
 	assert.Check(t, err != nil)
 	assert.Check(t, util.ContainsAll(err.Error(), "can't", "find", "version"))
 }
+
+func TestUnstructuredCRDFromGVK(t *testing.T) {
+	u := UnstructuredCRDFromGVK(schema.GroupVersionKind{"sources.knative.dev", "v1alpha2", "ApiServerSource"})
+	g, err := groupFromUnstructured(u)
+	assert.NilError(t, err)
+	assert.Equal(t, g, "sources.knative.dev")
+
+	v, err := versionFromUnstructured(u)
+	assert.NilError(t, err)
+	assert.Equal(t, v, "v1alpha2")
+
+	k, err := kindFromUnstructured(u)
+	assert.NilError(t, err)
+	assert.Equal(t, k, "ApiServerSource")
+
+	r, err := resourceFromUnstructured(u)
+	assert.NilError(t, err)
+	assert.Equal(t, r, "apiserversources")
+}
diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go
@@ -26,14 +26,14 @@ func newInvalidCRD(apiGroup string) *KNError {
 	return NewKNError(msg)
 }
 
-func newNoRouteToHost(errString string) error {
+func newNoRouteToHost(errString string) *KNError {
 	parts := strings.SplitAfter(errString, "dial tcp")
 	if len(parts) == 2 {
 		return NewKNError(fmt.Sprintf("error connecting to the cluster, please verify connection at: %s", strings.Trim(parts[1], " ")))
 	}
 	return NewKNError(fmt.Sprintf("error connecting to the cluster: %s", errString))
 }
 
-func newNoKubeConfig(errString string) error {
+func newNoKubeConfig(errString string) *KNError {
 	return NewKNError("no kubeconfig has been provided, please use a valid configuration to connect to the cluster")
 }
diff --git a/pkg/errors/factory.go b/pkg/errors/factory.go
@@ -15,6 +15,7 @@
 package errors
 
 import (
+	"net/http"
 	"strings"
 
 	api_errors "k8s.io/apimachinery/pkg/api/errors"
@@ -63,3 +64,11 @@ func GetError(err error) error {
 		return err
 	}
 }
+
+// IsForbiddenError returns true if given error can be converted to API status and of type forbidden access else false
+func IsForbiddenError(err error) bool {
+	if status, ok := err.(api_errors.APIStatus); ok {
+		return status.Status().Code == int32(http.StatusForbidden)
+	}
+	return false
+}
diff --git a/pkg/errors/factory_test.go b/pkg/errors/factory_test.go
@@ -120,6 +120,11 @@ func TestKnErrors(t *testing.T) {
 			Error:       errors.New("no route to host 192.168.1.1"),
 			ExpectedMsg: "error connecting to the cluster: no route to host 192.168.1.1",
 		},
+		{
+			Name:        "foo error which cant be converted to APIStatus",
+			Error:       errors.New("foo error"),
+			ExpectedMsg: "foo error",
+		},
 	}
 	for _, tc := range cases {
 		tc := tc
@@ -130,3 +135,29 @@ func TestKnErrors(t *testing.T) {
 		})
 	}
 }
+
+func TestIsForbiddenError(t *testing.T) {
+	cases := []struct {
+		Name      string
+		Error     error
+		Forbidden bool
+	}{
+		{
+			Name:      "forbidden error",
+			Error:     api_errors.NewForbidden(schema.GroupResource{Group: "apiextensions.k8s.io", Resource: "CustomResourceDefinition"}, "", nil),
+			Forbidden: true,
+		},
+		{
+			Name:      "non forbidden error",
+			Error:     errors.New("panic"),
+			Forbidden: false,
+		},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, IsForbiddenError(tc.Error), tc.Forbidden)
+		})
+	}
+}