diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 0ec5a95d3b..d2886ec4fc 100644 --- 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 index fb1c558a7b..2873c5e559 100644 --- 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 index dce199fd45..24ed9c5d6f 100644 --- 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 index f5ca6f4126..78bea88b8d 100644 --- 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 index 7343fc86bf..69043d9976 100644 --- 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 index 06b677567d..7ad14956c3 100644 --- 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 index 7ed81c1d37..57d5c31935 100644 --- 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 index c9a0cd5454..e58f3b4cc7 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -26,7 +26,7 @@ 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], " "))) @@ -34,6 +34,6 @@ func newNoRouteToHost(errString string) error { 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 index d0b473856f..48b076ee55 100644 --- 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 index 9c94b94952..991f4c3b31 100644 --- 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) + }) + } +} diff --git a/pkg/kn/commands/revision/delete.go b/pkg/kn/commands/revision/delete.go index 78d101cabf..9e6875f709 100644 --- a/pkg/kn/commands/revision/delete.go +++ b/pkg/kn/commands/revision/delete.go @@ -17,6 +17,7 @@ package revision import ( "errors" "fmt" + "strings" "time" "github.com/spf13/cobra" @@ -47,6 +48,7 @@ func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command { return err } + errs := []string{} for _, name := range args { timeout := time.Duration(0) if waitFlags.Wait { @@ -54,11 +56,14 @@ func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command { } err = client.DeleteRevision(name, timeout) if err != nil { - fmt.Fprintf(cmd.OutOrStdout(), "%s.\n", err) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cmd.OutOrStdout(), "Revision '%s' deleted in namespace '%s'.\n", name, namespace) } } + if len(errs) > 0 { + return errors.New("Error: " + strings.Join(errs, "\nError: ")) + } return nil }, } diff --git a/pkg/kn/commands/revision/delete_test.go b/pkg/kn/commands/revision/delete_test.go index 66d5d9fb23..6eb5363bcf 100644 --- a/pkg/kn/commands/revision/delete_test.go +++ b/pkg/kn/commands/revision/delete_test.go @@ -25,7 +25,9 @@ import ( clienttesting "k8s.io/client-go/testing" "knative.dev/client/pkg/kn/commands" + clientservingv1 "knative.dev/client/pkg/serving/v1" "knative.dev/client/pkg/util" + "knative.dev/client/pkg/util/mock" "knative.dev/client/pkg/wait" servingv1 "knative.dev/serving/pkg/apis/serving/v1" ) @@ -102,3 +104,23 @@ func getRevisionDeleteEvents(name string) []watch.Event { {watch.Deleted, &servingv1.Revision{ObjectMeta: metav1.ObjectMeta{Name: name}}}, } } + +func TestRevisionDeleteCheckErrorForNotFoundRevisionsMock(t *testing.T) { + // New mock client + client := clientservingv1.NewMockKnServiceClient(t) + + // Recording: + r := client.Recorder() + + r.DeleteRevision("foo", mock.Any(), nil) + r.DeleteRevision("bar", mock.Any(), errors.New("revisions.serving.knative.dev \"bar\" not found.")) + r.DeleteRevision("baz", mock.Any(), errors.New("revisions.serving.knative.dev \"baz\" not found.")) + + output, err := executeRevisionCommand(client, "delete", "foo", "bar", "baz") + if err == nil { + t.Fatal("Expected revision not found error, returned nil") + } + assert.Assert(t, util.ContainsAll(output, "'foo' deleted", "\"bar\" not found", "\"baz\" not found")) + + r.Validate() +} diff --git a/pkg/kn/commands/revision/revision_test.go b/pkg/kn/commands/revision/revision_test.go index 63e49e70a3..27cbb9942d 100644 --- a/pkg/kn/commands/revision/revision_test.go +++ b/pkg/kn/commands/revision/revision_test.go @@ -15,15 +15,24 @@ package revision import ( + "bytes" "strings" "testing" + "github.com/spf13/cobra" "gotest.tools/assert" + "k8s.io/client-go/tools/clientcmd" + knflags "knative.dev/client/pkg/kn/flags" servingv1 "knative.dev/serving/pkg/apis/serving/v1" + "knative.dev/client/pkg/kn/commands" + clientservingv1 "knative.dev/client/pkg/serving/v1" "knative.dev/client/pkg/util" ) +// Helper methods +var blankConfig clientcmd.ClientConfig + func TestExtractTrafficAndTag(t *testing.T) { service := &servingv1.Service{ @@ -52,3 +61,23 @@ func createTarget(rev string, percent int64, tag string) servingv1.TrafficTarget Percent: &percent, } } + +func executeRevisionCommand(client clientservingv1.KnServingClient, args ...string) (string, error) { + knParams := &commands.KnParams{} + knParams.ClientConfig = blankConfig + + output := new(bytes.Buffer) + knParams.Output = output + knParams.NewServingClient = func(namespace string) (clientservingv1.KnServingClient, error) { + return client, nil + } + cmd := NewRevisionCommand(knParams) + cmd.SetArgs(args) + cmd.SetOutput(output) + + cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + return knflags.ReconcileBoolFlags(cmd.Flags()) + } + err := cmd.Execute() + return output.String(), err +} diff --git a/pkg/kn/commands/service/delete.go b/pkg/kn/commands/service/delete.go index 54953cf4e6..698cb52da8 100644 --- a/pkg/kn/commands/service/delete.go +++ b/pkg/kn/commands/service/delete.go @@ -17,6 +17,7 @@ package service import ( "errors" "fmt" + "strings" "time" "github.com/spf13/cobra" @@ -77,6 +78,7 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command { } } + errs := []string{} for _, name := range args { timeout := time.Duration(0) if waitFlags.Wait { @@ -84,11 +86,14 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command { } err = client.DeleteService(name, timeout) if err != nil { - fmt.Fprintf(cmd.OutOrStdout(), "%s.\n", err) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' successfully deleted in namespace '%s'.\n", name, namespace) } } + if len(errs) > 0 { + return errors.New("Error: " + strings.Join(errs, "\nError: ")) + } return nil }, } diff --git a/pkg/kn/commands/service/delete_mock_test.go b/pkg/kn/commands/service/delete_mock_test.go index 8b5d918fae..40eb44f20b 100644 --- a/pkg/kn/commands/service/delete_mock_test.go +++ b/pkg/kn/commands/service/delete_mock_test.go @@ -15,6 +15,7 @@ package service import ( + "errors" "testing" "gotest.tools/assert" @@ -141,3 +142,23 @@ func TestServiceDeleteNoSvcNameMock(t *testing.T) { r.Validate() } + +func TestServiceDeleteCheckErrorForNotFoundServicesMock(t *testing.T) { + // New mock client + client := clientservingv1.NewMockKnServiceClient(t) + + // Recording: + r := client.Recorder() + + r.DeleteService("foo", mock.Any(), nil) + r.DeleteService("bar", mock.Any(), errors.New("services.serving.knative.dev \"bar\" not found.")) + r.DeleteService("baz", mock.Any(), errors.New("services.serving.knative.dev \"baz\" not found.")) + + output, err := executeServiceCommand(client, "delete", "foo", "bar", "baz") + if err == nil { + t.Fatal("Expected service not found error, returned nil") + } + assert.Assert(t, util.ContainsAll(output, "'foo' successfully deleted", "\"bar\" not found", "\"baz\" not found")) + + r.Validate() +} diff --git a/pkg/kn/commands/source/human_readable_flags.go b/pkg/kn/commands/source/human_readable_flags.go index 2357c88d17..1e74af0ad3 100644 --- a/pkg/kn/commands/source/human_readable_flags.go +++ b/pkg/kn/commands/source/human_readable_flags.go @@ -47,6 +47,7 @@ func ListTypesHandlers(h printers.PrintHandler) { // ListHandlers handles printing human readable table for `kn source list` func ListHandlers(h printers.PrintHandler) { sourceListColumnDefinitions := []metav1beta1.TableColumnDefinition{ + {Name: "Namespace", Type: "string", Description: "Namespace of the source", Priority: 0}, {Name: "Name", Type: "string", Description: "Name of the created source", Priority: 1}, {Name: "Type", Type: "string", Description: "Type of the source", Priority: 1}, {Name: "Resource", Type: "string", Description: "Source type name", Priority: 1}, diff --git a/pkg/kn/commands/source/list.go b/pkg/kn/commands/source/list.go index a45c0afd9f..ce29e9dc14 100644 --- a/pkg/kn/commands/source/list.go +++ b/pkg/kn/commands/source/list.go @@ -20,9 +20,11 @@ import ( "github.com/spf13/cobra" "knative.dev/client/pkg/dynamic" + knerrors "knative.dev/client/pkg/errors" "knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/kn/commands/flags" "knative.dev/client/pkg/kn/commands/source/duck" + sourcesv1alpha2 "knative.dev/client/pkg/sources/v1alpha2" ) var listExample = ` @@ -52,16 +54,26 @@ func NewListCommand(p *commands.KnParams) *cobra.Command { if err != nil { return err } + var filters dynamic.WithTypes for _, filter := range filterFlags.Filters { filters = append(filters, dynamic.WithTypeFilter(filter)) } + sourceList, err := dynamicClient.ListSources(filters...) - if err != nil { - return err + + switch { + case knerrors.IsForbiddenError(err): + gvks := sourcesv1alpha2.BuiltInSourcesGVKs() + if sourceList, err = dynamicClient.ListSourcesUsingGVKs(&gvks, filters...); err != nil { + return knerrors.GetError(err) + } + case err != nil: + return knerrors.GetError(err) } - if len(sourceList.Items) == 0 { - fmt.Fprintf(cmd.OutOrStdout(), "No sources found in %s namespace.\n", namespace) + + if sourceList == nil || len(sourceList.Items) == 0 { + fmt.Fprintf(cmd.OutOrStdout(), "No sources found.\n") return nil } // empty namespace indicates all namespaces flag is specified diff --git a/pkg/kn/commands/source/list_test.go b/pkg/kn/commands/source/list_test.go index 170be23a36..4550bfe592 100644 --- a/pkg/kn/commands/source/list_test.go +++ b/pkg/kn/commands/source/list_test.go @@ -23,6 +23,8 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + dynamicfake "k8s.io/client-go/dynamic/fake" + clientdynamic "knative.dev/client/pkg/dynamic" "knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/util" ) @@ -51,6 +53,12 @@ func sourceFakeCmd(args []string, objects ...runtime.Object) (output []string, e return } +func TestSourceListTypesNoSourcesInstalled(t *testing.T) { + _, err := sourceFakeCmd([]string{"source", "list-types"}) + assert.Check(t, err != nil) + assert.Check(t, util.ContainsAll(err.Error(), "no sources", "found", "backend", "verify", "installation")) +} + func TestSourceListTypes(t *testing.T) { output, err := sourceFakeCmd([]string{"source", "list-types"}, newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), @@ -71,6 +79,20 @@ func TestSourceListTypesNoHeaders(t *testing.T) { assert.Check(t, util.ContainsAll(output[0], "PingSource")) } +func TestListBuiltInSourceTypes(t *testing.T) { + fakeDynamic := dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) + sources, err := listBuiltInSourceTypes(clientdynamic.NewKnDynamicClient(fakeDynamic, "current")) + assert.NilError(t, err) + assert.Check(t, sources != nil) + assert.Equal(t, len(sources.Items), 4) +} + +func TestSourceListNoSourcesInstalled(t *testing.T) { + _, err := sourceFakeCmd([]string{"source", "list"}) + assert.Check(t, err != nil) + assert.Check(t, util.ContainsAll(err.Error(), "no sources", "found", "backend", "verify", "installation")) +} + func TestSourceList(t *testing.T) { output, err := sourceFakeCmd([]string{"source", "list"}, newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), @@ -160,3 +182,19 @@ func newSourceUnstructuredObj(name, apiVersion, kind string) *unstructured.Unstr }, } } + +func TestSourceListAllNamespace(t *testing.T) { + output, err := sourceFakeCmd([]string{"source", "list", "--all-namespaces"}, + newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), + newSourceCRDObjWithSpec("sinkbindings", "sources.knative.dev", "v1alpha1", "SinkBinding"), + newSourceCRDObjWithSpec("apiserversources", "sources.knative.dev", "v1alpha1", "ApiServerSource"), + newSourceUnstructuredObj("p1", "sources.knative.dev/v1alpha1", "PingSource"), + newSourceUnstructuredObj("s1", "sources.knative.dev/v1alpha1", "SinkBinding"), + newSourceUnstructuredObj("a1", "sources.knative.dev/v1alpha1", "ApiServerSource"), + ) + assert.NilError(t, err) + assert.Check(t, util.ContainsAll(output[0], "NAMESPACE", "NAME", "TYPE", "RESOURCE", "SINK", "READY")) + assert.Check(t, util.ContainsAll(output[1], "current", "a1", "ApiServerSource", "apiserversources.sources.knative.dev", "ksvc:foo", "True")) + assert.Check(t, util.ContainsAll(output[2], "current", "p1", "PingSource", "pingsources.sources.knative.dev", "ksvc:foo", "True")) + assert.Check(t, util.ContainsAll(output[3], "current", "s1", "SinkBinding", "sinkbindings.sources.knative.dev", "ksvc:foo", "True")) +} diff --git a/pkg/kn/commands/source/list_types.go b/pkg/kn/commands/source/list_types.go index 67f44131be..3454a7a93c 100644 --- a/pkg/kn/commands/source/list_types.go +++ b/pkg/kn/commands/source/list_types.go @@ -18,9 +18,14 @@ import ( "fmt" "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "knative.dev/client/pkg/dynamic" + knerrors "knative.dev/client/pkg/errors" "knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/kn/commands/flags" + sourcesv1alpha2 "knative.dev/client/pkg/sources/v1alpha2" ) // NewListTypesCommand defines and processes `kn source list-types` @@ -47,13 +52,17 @@ func NewListTypesCommand(p *commands.KnParams) *cobra.Command { } sourceListTypes, err := dynamicClient.ListSourcesTypes() - if err != nil { - return err + switch { + case knerrors.IsForbiddenError(err): + if sourceListTypes, err = listBuiltInSourceTypes(dynamicClient); err != nil { + return knerrors.GetError(err) + } + case err != nil: + return knerrors.GetError(err) } - if len(sourceListTypes.Items) == 0 { - fmt.Fprintf(cmd.OutOrStdout(), "No sources found.\n") - return nil + if sourceListTypes == nil || len(sourceListTypes.Items) == 0 { + return fmt.Errorf("no sources found on the backend, please verify the installation") } printer, err := listTypesFlags.ToPrinter() @@ -73,3 +82,22 @@ func NewListTypesCommand(p *commands.KnParams) *cobra.Command { listTypesFlags.AddFlags(listTypesCommand) return listTypesCommand } + +func listBuiltInSourceTypes(d dynamic.KnDynamicClient) (*unstructured.UnstructuredList, error) { + var err error + uList := unstructured.UnstructuredList{} + gvks := sourcesv1alpha2.BuiltInSourcesGVKs() + for _, gvk := range gvks { + _, err = d.ListSourcesUsingGVKs(&[]schema.GroupVersionKind{gvk}) + if err != nil { + continue + } + u := dynamic.UnstructuredCRDFromGVK(gvk) + uList.Items = append(uList.Items, *u) + } + // if not even one source is found + if len(uList.Items) == 0 && err != nil { + return nil, knerrors.GetError(err) + } + return &uList, nil +} diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index bbabb41e54..057fc32aca 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -501,26 +501,44 @@ func UpdateImagePullSecrets(template *servingv1.RevisionTemplateSpec, pullsecret } // GenerateVolumeName generates a volume name with respect to a given path string. -// Current implementation basically sanitizes the path string by changing "/" into "." +// Current implementation basically sanitizes the path string by replacing "/" with "-" // To reduce any chance of duplication, a checksum part generated from the path string is appended to the sanitized string. +// The volume name must 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 func GenerateVolumeName(path string) string { builder := &strings.Builder{} for idx, r := range path { switch { - case unicode.IsLower(r) || unicode.IsDigit(r) || r == '-' || r == '.': + case unicode.IsLower(r) || unicode.IsDigit(r) || r == '-': builder.WriteRune(r) case unicode.IsUpper(r): builder.WriteRune(unicode.ToLower(r)) case r == '/': if idx != 0 { - builder.WriteRune('.') + builder.WriteRune('-') } default: builder.WriteRune('-') } } - return appendCheckSum(builder.String(), path) + vname := appendCheckSum(builder.String(), path) + + // the name must start with an alphanumeric character + if !unicode.IsLetter(rune(vname[0])) && !unicode.IsNumber(rune(vname[0])) { + vname = fmt.Sprintf("k-%s", vname) + } + + // contain at most 63 characters + if len(vname) > 63 { + // must end with an alphanumeric character + vname = fmt.Sprintf("%s-n", vname[0:61]) + } + + return vname } // ======================================================================================= @@ -802,8 +820,8 @@ func existsVolumeNameInVolumeMounts(volumeName string, volumeMounts []corev1.Vol return false } -func appendCheckSum(sanitiedString string, path string) string { +func appendCheckSum(sanitizedString, path string) string { checkSum := sha1.Sum([]byte(path)) shortCheckSum := checkSum[0:4] - return fmt.Sprintf("%s-%x", sanitiedString, shortCheckSum) + return fmt.Sprintf("%s-%x", sanitizedString, shortCheckSum) } diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index e341c5b0fc..6a43d270fc 100644 --- a/pkg/serving/config_changes_test.go +++ b/pkg/serving/config_changes_test.go @@ -670,13 +670,17 @@ func TestGenerateVolumeName(t *testing.T) { "/Ab12~`!@#$%^&*()-=_+[]{}|/\\<>,./?:;\"'xZ/", "", "/", + "/path.mypath/", + "/.path.mypath", } expected := []string{ - "ab12---------------------.----..-----xz", - "ab12---------------------.----..-----xz.", - "", - "", + "ab12---------------------------------xz", + "ab12---------------------------------xz-", + "k-", + "k-", + "path-mypath-", + "k--path-mypath", } for i := range actual { @@ -684,6 +688,11 @@ func TestGenerateVolumeName(t *testing.T) { expectedName := appendCheckSum(expected[i], actual[i]) assert.Equal(t, actualName, expectedName) } + + // 63 char limit case, no need to append the checksum in expected string + expName_63 := "k---ab12---------------------------------xz-ab12--------------n" + assert.Equal(t, len(expName_63), 63) + assert.Equal(t, GenerateVolumeName("/./Ab12~`!@#$%^&*()-=_+[]{}|/\\<>,./?:;\"'xZ/Ab12~`!@#$%^&*()-=_+[]{}|/\\<>,./?:;\"'xZ/"), expName_63) } func TestUpdateUser(t *testing.T) { diff --git a/pkg/sources/v1alpha2/apiserver_client.go b/pkg/sources/v1alpha2/apiserver_client.go index 3f66511c6d..cb972e3545 100644 --- a/pkg/sources/v1alpha2/apiserver_client.go +++ b/pkg/sources/v1alpha2/apiserver_client.go @@ -111,7 +111,7 @@ func (c *apiServerSourcesClient) Namespace() string { func (c *apiServerSourcesClient) ListAPIServerSource() (*v1alpha2.ApiServerSourceList, error) { sourceList, err := c.client.List(metav1.ListOptions{}) if err != nil { - return nil, err + return nil, knerrors.GetError(err) } return updateAPIServerSourceListGVK(sourceList) diff --git a/pkg/sources/v1alpha2/client.go b/pkg/sources/v1alpha2/client.go index c97129a394..0ea81c68f4 100644 --- a/pkg/sources/v1alpha2/client.go +++ b/pkg/sources/v1alpha2/client.go @@ -15,6 +15,8 @@ package v1alpha2 import ( + "k8s.io/apimachinery/pkg/runtime/schema" + sourcesv1alpha2 "knative.dev/eventing/pkg/apis/sources/v1alpha2" clientv1alpha2 "knative.dev/eventing/pkg/client/clientset/versioned/typed/sources/v1alpha2" ) @@ -61,3 +63,13 @@ func (c *sourcesClient) SinkBindingClient() KnSinkBindingClient { func (c *sourcesClient) APIServerSourcesClient() KnAPIServerSourcesClient { return newKnAPIServerSourcesClient(c.client.ApiServerSources(c.namespace), c.namespace) } + +// BuiltInSourcesGVKs returns the GVKs for built in sources +func BuiltInSourcesGVKs() []schema.GroupVersionKind { + return []schema.GroupVersionKind{ + sourcesv1alpha2.SchemeGroupVersion.WithKind("ApiServerSource"), + sourcesv1alpha2.SchemeGroupVersion.WithKind("ContainerSource"), + sourcesv1alpha2.SchemeGroupVersion.WithKind("PingSource"), + sourcesv1alpha2.SchemeGroupVersion.WithKind("SinkBinding"), + } +} diff --git a/pkg/sources/v1alpha2/client_test.go b/pkg/sources/v1alpha2/client_test.go new file mode 100644 index 0000000000..6f48399b24 --- /dev/null +++ b/pkg/sources/v1alpha2/client_test.go @@ -0,0 +1,31 @@ +// Copyright © 2020 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. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1alpha2 + +import ( + "testing" + + "gotest.tools/assert" + + sourcesv1alpha2 "knative.dev/eventing/pkg/apis/sources/v1alpha2" +) + +func TestBuiltInSourcesGVks(t *testing.T) { + gvks := BuiltInSourcesGVKs() + for _, each := range gvks { + assert.DeepEqual(t, each.GroupVersion(), sourcesv1alpha2.SchemeGroupVersion) + } + assert.Equal(t, len(gvks), 4) +} diff --git a/test/e2e/service_test.go b/test/e2e/service_test.go index 900c5d3aeb..cdbd755c35 100644 --- a/test/e2e/service_test.go +++ b/test/e2e/service_test.go @@ -125,8 +125,8 @@ func serviceDeleteNonexistent(r *test.KnRunResultCollector, serviceName string) assert.Check(r.T(), !strings.Contains(out.Stdout, serviceName), "The service exists") out = r.KnTest().Kn().Run("service", "delete", serviceName) - r.AssertNoError(out) - assert.Check(r.T(), util.ContainsAll(out.Stdout, "hello", "not found"), "Failed to get 'not found' error") + r.AssertError(out) + assert.Check(r.T(), util.ContainsAll(out.Stderr, "hello", "not found"), "Failed to get 'not found' error") } func serviceMultipleDelete(r *test.KnRunResultCollector, existService, nonexistService string) { @@ -136,12 +136,12 @@ func serviceMultipleDelete(r *test.KnRunResultCollector, existService, nonexistS assert.Check(r.T(), !strings.Contains(out.Stdout, nonexistService), "The service", nonexistService, " exists (but is supposed to be not)") out = r.KnTest().Kn().Run("service", "delete", existService, nonexistService) - r.AssertNoError(out) + r.AssertError(out) expectedSuccess := fmt.Sprintf(`Service '%s' successfully deleted in namespace '%s'.`, existService, r.KnTest().Kn().Namespace()) expectedErr := fmt.Sprintf(`services.serving.knative.dev "%s" not found`, nonexistService) assert.Check(r.T(), strings.Contains(out.Stdout, expectedSuccess), "Failed to get 'successfully deleted' message") - assert.Check(r.T(), strings.Contains(out.Stdout, expectedErr), "Failed to get 'not found' error") + assert.Check(r.T(), strings.Contains(out.Stderr, expectedErr), "Failed to get 'not found' error") } func serviceUntagTagThatDoesNotExist(r *test.KnRunResultCollector, serviceName string) { diff --git a/test/e2e/source_list_test.go b/test/e2e/source_list_test.go index 0d38b6baa5..74b3336ecd 100644 --- a/test/e2e/source_list_test.go +++ b/test/e2e/source_list_test.go @@ -64,7 +64,7 @@ func TestSourceList(t *testing.T) { t.Log("List sources empty case") output := sourceList(r) - assert.Check(t, util.ContainsAll(output, "No", "sources", "found", "namespace")) + assert.Check(t, util.ContainsAll(output, "No", "sources", "found.")) assert.Check(t, util.ContainsNone(output, "NAME", "TYPE", "RESOURCE", "SINK", "READY")) t.Log("Create API Server") @@ -86,9 +86,9 @@ func TestSourceList(t *testing.T) { t.Log("List sources filter invalid case") output = sourceList(r, "--type", "testapisource0") - assert.Check(t, util.ContainsAll(output, "No", "sources", "found", "namespace")) + assert.Check(t, util.ContainsAll(output, "No", "sources", "found.")) output = sourceList(r, "--type", "TestSource", "-oyaml") - assert.Check(t, util.ContainsAll(output, "No", "sources", "found", "namespace")) + assert.Check(t, util.ContainsAll(output, "No", "sources", "found.")) t.Log("List available source in YAML format") output = sourceList(r, "--type", "PingSource,ApiServerSource", "-oyaml") diff --git a/test/e2e/tekton_test.go b/test/e2e/tekton_test.go index 99dec1da23..a4951ec2f7 100644 --- a/test/e2e/tekton_test.go +++ b/test/e2e/tekton_test.go @@ -17,6 +17,7 @@ package e2e import ( + "fmt" "strings" "testing" "time" @@ -55,16 +56,16 @@ func TestTektonPipeline(t *testing.T) { _, err = kubectl.Run("apply", "-f", basedir+"/kn-deployer-rbac.yaml") assert.NilError(t, err) - _, err = kubectl.Run("apply", "-f", "https://raw.githubusercontent.com/tektoncd/catalog/v1beta1/git/git-clone.yaml") + _, err = kubectl.Run("apply", "-f", tektonCatalogTask("git-clone", "0.1")) assert.NilError(t, err) _, err = kubectl.Run("apply", "-f", basedir+"/resources.yaml") assert.NilError(t, err) - _, err = kubectl.Run("apply", "-f", "https://raw.githubusercontent.com/tektoncd/catalog/v1beta1/buildah/buildah.yaml") + _, err = kubectl.Run("apply", "-f", tektonCatalogTask("buildah", "0.1")) assert.NilError(t, err) - _, err = kubectl.Run("apply", "-f", "https://raw.githubusercontent.com/tektoncd/catalog/v1beta1/kn/kn.yaml") + _, err = kubectl.Run("apply", "-f", tektonCatalogTask("kn", "0.1")) assert.NilError(t, err) _, err = kubectl.Run("apply", "-f", basedir+"/kn-pipeline.yaml") @@ -91,3 +92,11 @@ func waitForPipelineSuccess(k test.Kubectl) error { return strings.Contains(out, "True"), err }) } + +func tektonCatalogTask(taskName, version string) string { + return fmt.Sprintf( + "https://raw.githubusercontent.com/tektoncd/catalog/"+ + "master/task/%s/%s/%s.yaml", + taskName, version, taskName, + ) +}