Fixes the bundle push and list commands to conform to the bundle spec.

The original implementation did not comply with the Tekton bundle spec which explcitely says the `kind` must be lowercased. We were forcing it upper-cased which is not legal. This PR fixes that bug and: - allows the user to `list` with any of the uppercase/lowercase and/or singular/plural forms of a `kind` to make using this easier - the push will set the correct annotation on the bundle object To make this problem easier to diagnose, this change will be paired with a change to the controller to make the errors more obvious.
khulnasoft-lab · Apr 6, 2021 · 2bcf91f · 2bcf91f
1 parent c996b30
commit 2bcf91f
Show file tree

Hide file tree

Showing 13 changed files with 75 additions and 48 deletions.
diff --git a/docs/cmd/tkn_bundle.md b/docs/cmd/tkn_bundle.md
@@ -24,6 +24,6 @@ Manage Tekton Bundles
 ### SEE ALSO
 
 * [tkn](tkn.md)	 - CLI for tekton pipelines
-* [tkn bundle list](tkn_bundle_list.md)	 - List a Tekton bundle's contents
-* [tkn bundle push](tkn_bundle_push.md)	 - Push a new Tekton bundle
+* [tkn bundle list](tkn_bundle_list.md)	 - List and print a Tekton bundle's contents
+* [tkn bundle push](tkn_bundle_push.md)	 - Create or replace a Tekton bundle
 
diff --git a/docs/cmd/tkn_bundle_list.md b/docs/cmd/tkn_bundle_list.md
@@ -1,6 +1,6 @@
 ## tkn bundle list
 
-List a Tekton bundle's contents
+List and print a Tekton bundle's contents
 
 ### Usage
 
@@ -14,10 +14,12 @@ List the contents of a Tekton Bundle from a registry. You can further narrow dow
 optionally specifying the kind, and then the name:
 
 	tkn bundle list docker.io/myorg/mybundle:latest // fetches all objects
-	tkn bundle list docker.io/myorg/mybundle:1.0 Task // fetches all Tekton tasks
-	tkn bundle list docker.io/myorg/mybundle:1.0 Task foo // fetches the Tekton task "foo"
+	tkn bundle list docker.io/myorg/mybundle:1.0 task // fetches all Tekton tasks
+	tkn bundle list docker.io/myorg/mybundle:1.0 task foo // fetches the Tekton task "foo"
 
-As with other "list" commands, you can specify the desired output format using the "-o" flag.
+As with other "list" commands, you can specify the desired output format using the "-o" flag. You may specify the kind
+in its "Kind" form (eg Task), its "Resource" form (eg tasks), or in the form specified by the Tekton Bundle contract (
+eg task).
 
 Authentication:
 	There are three ways to authenticate against your registry.

diff --git a/docs/cmd/tkn_bundle_push.md b/docs/cmd/tkn_bundle_push.md
@@ -1,6 +1,6 @@
 ## tkn bundle push
 
-Push a new Tekton bundle
+Create or replace a Tekton bundle
 
 ### Usage
 

diff --git a/docs/man/man1/tkn-bundle-list.1 b/docs/man/man1/tkn-bundle-list.1
@@ -5,7 +5,7 @@
 
 .SH NAME
 .PP
-tkn\-bundle\-list \- List a Tekton bundle's contents
+tkn\-bundle\-list \- List and print a Tekton bundle's contents
 
 
 .SH SYNOPSIS
@@ -23,14 +23,16 @@ optionally specifying the kind, and then the name:
 
 .nf
 tkn bundle list docker.io/myorg/mybundle:latest // fetches all objects
-tkn bundle list docker.io/myorg/mybundle:1.0 Task // fetches all Tekton tasks
-tkn bundle list docker.io/myorg/mybundle:1.0 Task foo // fetches the Tekton task "foo"
+tkn bundle list docker.io/myorg/mybundle:1.0 task // fetches all Tekton tasks
+tkn bundle list docker.io/myorg/mybundle:1.0 task foo // fetches the Tekton task "foo"
 
 .fi
 .RE
 
 .PP
-As with other "list" commands, you can specify the desired output format using the "\-o" flag.
+As with other "list" commands, you can specify the desired output format using the "\-o" flag. You may specify the kind
+in its "Kind" form (eg Task), its "Resource" form (eg tasks), or in the form specified by the Tekton Bundle contract (
+eg task).
 
 .PP
 Authentication:

diff --git a/docs/man/man1/tkn-bundle-push.1 b/docs/man/man1/tkn-bundle-push.1
@@ -5,7 +5,7 @@
 
 .SH NAME
 .PP
-tkn\-bundle\-push \- Push a new Tekton bundle
+tkn\-bundle\-push \- Create or replace a Tekton bundle
 
 
 .SH SYNOPSIS

diff --git a/pkg/bundle/builder.go b/pkg/bundle/builder.go
@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/empty"
@@ -61,7 +62,7 @@ func BuildTektonBundle(contents []string, log io.Writer) (v1.Image, error) {
 				Layer: l,
 				Annotations: map[string]string{
 					tkremote.APIVersionAnnotation: gvr.Version,
-					tkremote.KindAnnotation:       gvr.Kind,
+					tkremote.KindAnnotation:       strings.ToLower(gvr.Kind),
 					tkremote.TitleAnnotation:      name,
 				},
 			})

diff --git a/pkg/bundle/builder_test.go b/pkg/bundle/builder_test.go
@@ -51,7 +51,7 @@ func TestBuildTektonBundle(t *testing.T) {
 	if apiVersion, ok := l.Annotations[tkremote.APIVersionAnnotation]; !ok || apiVersion != "v1beta1" {
 		t.Errorf("Did not receive expected APIVersion v1beta1. Found %s", apiVersion)
 	}
-	if kind, ok := l.Annotations[tkremote.KindAnnotation]; !ok || kind != "Task" {
+	if kind, ok := l.Annotations[tkremote.KindAnnotation]; !ok || kind != "task" {
 		t.Errorf("Did not receive expected Kind Task. Found %s", kind)
 	}
 	if name, ok := l.Annotations[tkremote.TitleAnnotation]; !ok || name != "foo" {

diff --git a/pkg/bundle/reader.go b/pkg/bundle/reader.go
@@ -9,12 +9,11 @@ import (
 	"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/scheme"
 	tkremote "github.com/tektoncd/pipeline/pkg/remote/oci"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 // ObjectVisitor is an input function that callers of this file's methods can implement to act on the read contents of a
-// Tekton bundle.
-type ObjectVisitor func(gvk schema.GroupVersionKind, name string, element runtime.Object, raw []byte)
+// Tekton bundle. The `version`, `kind`, and `name` fields map 1:1 with the same named fields in the Tekton Bundle spec.
+type ObjectVisitor func(version, kind, name string, element runtime.Object, raw []byte)
 
 // List will call visitor for every single layer in the img.
 func List(img v1.Image, visitor ObjectVisitor) error {
@@ -48,48 +47,55 @@ func List(img v1.Image, visitor ObjectVisitor) error {
 			return fmt.Errorf("failed to read layer %s: %w", l.Digest, err)
 		}
 
-		obj, gvk, err := scheme.Codecs.UniversalDeserializer().Decode(contents, nil, nil)
+		obj, _, err := scheme.Codecs.UniversalDeserializer().Decode(contents, nil, nil)
 		if err != nil {
 			return fmt.Errorf("failed to decode layer %s to a Tekton object: %w", l.Digest, err)
 		}
-		visitor(*gvk, l.Annotations[tkremote.TitleAnnotation], obj, contents)
+		visitor(
+			l.Annotations[tkremote.APIVersionAnnotation],
+			l.Annotations[tkremote.KindAnnotation],
+			l.Annotations[tkremote.TitleAnnotation],
+			obj,
+			contents,
+		)
 	}
 
 	return nil
 }
 
-// ListKind is like #List but only returns elements of a single kind.
-func ListKind(img v1.Image, kind string, visitor ObjectVisitor) error {
+// ListKind is like #List but only returns elements of a single kind. Kind will be lowercased to conform to the Tekton
+// Bundle spec.
+func ListKind(img v1.Image, pkind string, visitor ObjectVisitor) error {
 	listedItems := 0
-	if err := List(img, func(gvk schema.GroupVersionKind, name string, element runtime.Object, raw []byte) {
-		if gvk.Kind == kind {
+	if err := List(img, func(version, kind string, name string, element runtime.Object, raw []byte) {
+		if kind == pkind {
 			listedItems++
-			visitor(gvk, name, element, raw)
+			visitor(version, kind, name, element, raw)
 		}
 	}); err != nil {
 		return err
 	}
 
 	if listedItems == 0 {
-		return fmt.Errorf("no objects of kind %s found in img", kind)
+		return fmt.Errorf("no objects of kind %q found in img", pkind)
 	}
 	return nil
 }
 
 // Get returns a single named element of a specific kind from the Tekton Bundle.
-func Get(img v1.Image, kind, name string, visitor ObjectVisitor) error {
+func Get(img v1.Image, pkind, name string, visitor ObjectVisitor) error {
 	objectFound := false
-	if err := ListKind(img, kind, func(gvk schema.GroupVersionKind, foundName string, element runtime.Object, raw []byte) {
+	if err := ListKind(img, pkind, func(version, kind, foundName string, element runtime.Object, raw []byte) {
 		if foundName == name {
 			objectFound = true
-			visitor(gvk, foundName, element, raw)
+			visitor(version, kind, foundName, element, raw)
 		}
 	}); err != nil {
 		return err
 	}
 
 	if !objectFound {
-		return fmt.Errorf("no objects of kind %s named %s found in img", kind, name)
+		return fmt.Errorf("no objects of kind %q named %q found in img", pkind, name)
 	}
 	return nil
 }

diff --git a/pkg/bundle/reader_test.go b/pkg/bundle/reader_test.go
@@ -17,7 +17,6 @@ import (
 	tkremote "github.com/tektoncd/pipeline/pkg/remote/oci"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 func writeTarLayer(t *testing.T, img v1.Image, obj runtime.Object) (v1.Image, error) {
@@ -61,7 +60,7 @@ func writeTarLayer(t *testing.T, img v1.Image, obj runtime.Object) (v1.Image, er
 		Annotations: map[string]string{
 			tkremote.TitleAnnotation:      name,
 			tkremote.KindAnnotation:       strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind),
-			tkremote.APIVersionAnnotation: strings.ToLower(obj.GetObjectKind().GroupVersionKind().Version),
+			tkremote.APIVersionAnnotation: obj.GetObjectKind().GroupVersionKind().Version,
 		},
 	})
 }
@@ -105,21 +104,21 @@ func TestReader(t *testing.T) {
 	}
 
 	// Expect list to return all the elements.
-	if err := List(img, func(_ schema.GroupVersionKind, _ string, element runtime.Object, _ []byte) {
+	if err := List(img, func(_, _, _ string, element runtime.Object, _ []byte) {
 		test.Contains(t, []runtime.Object{&task1, &task2, &pipeline}, element)
 	}); err != nil {
 		t.Error(err)
 	}
 
 	// Expect ListKind to return the two tasks.
-	if err := ListKind(img, task1.GroupVersionKind().Kind, func(_ schema.GroupVersionKind, _ string, element runtime.Object, _ []byte) {
+	if err := ListKind(img, strings.ToLower(task1.GroupVersionKind().Kind), func(_, _, _ string, element runtime.Object, _ []byte) {
 		test.Contains(t, []runtime.Object{&task1, &task2}, element)
 	}); err != nil {
 		t.Error(err)
 	}
 
 	// Expect Get to return a single object.
-	if err := Get(img, task1.GroupVersionKind().Kind, task1.GetName(), func(_ schema.GroupVersionKind, _ string, element runtime.Object, _ []byte) {
+	if err := Get(img, strings.ToLower(task1.GroupVersionKind().Kind), task1.GetName(), func(_, _, _ string, element runtime.Object, _ []byte) {
 		test.Contains(t, []runtime.Object{&task1}, element)
 	}); err != nil {
 		t.Error(err)

diff --git a/pkg/cmd/bundle/list.go b/pkg/cmd/bundle/list.go
@@ -15,22 +15,26 @@ package bundle
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/spf13/cobra"
 	"github.com/tektoncd/cli/pkg/bundle"
 	"github.com/tektoncd/cli/pkg/cli"
 	"github.com/tektoncd/cli/pkg/printer"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	cliopts "k8s.io/cli-runtime/pkg/genericclioptions"
 )
 
 // TODO: Find a more generic way to do this
 var (
-	allowedKinds = []string{"Task", "Pipeline", "ClusterTask"}
+	allowedKinds = []string{"task", "pipeline", "clustertask"}
 )
 
+func normalizeKind(kind string) string {
+	return strings.TrimSuffix(strings.ToLower(kind), "s")
+}
+
 type listOptions struct {
 	cliparams     cli.Params
 	stream        *cli.Stream
@@ -49,10 +53,12 @@ func listCommand(p cli.Params) *cobra.Command {
 optionally specifying the kind, and then the name:
 
 	tkn bundle list docker.io/myorg/mybundle:latest // fetches all objects
-	tkn bundle list docker.io/myorg/mybundle:1.0 Task // fetches all Tekton tasks
-	tkn bundle list docker.io/myorg/mybundle:1.0 Task foo // fetches the Tekton task "foo"
+	tkn bundle list docker.io/myorg/mybundle:1.0 task // fetches all Tekton tasks
+	tkn bundle list docker.io/myorg/mybundle:1.0 task foo // fetches the Tekton task "foo"
 
-As with other "list" commands, you can specify the desired output format using the "-o" flag.
+As with other "list" commands, you can specify the desired output format using the "-o" flag. You may specify the kind
+in its "Kind" form (eg Task), its "Resource" form (eg tasks), or in the form specified by the Tekton Bundle contract (
+eg task).
 
 Authentication:
 	There are three ways to authenticate against your registry.
@@ -67,7 +73,7 @@ Caching:
 
 	c := &cobra.Command{
 		Use:   "list",
-		Short: "List a Tekton bundle's contents",
+		Short: "List and print a Tekton bundle's contents",
 		Long:  longHelp,
 		Annotations: map[string]string{
 			"commandType": "main",
@@ -87,7 +93,7 @@ Caching:
 			if len(args) > 1 {
 				allowedKind := false
 				for _, op := range allowedKinds {
-					if args[1] == op {
+					if normalizeKind(args[1]) == op {
 						allowedKind = true
 					}
 				}
@@ -106,7 +112,7 @@ Caching:
 				Err: cmd.OutOrStderr(),
 			}
 
-			return opts.Run(args, func(_ schema.GroupVersionKind, _ string, element runtime.Object, _ []byte) {
+			return opts.Run(args, func(_, _, _ string, element runtime.Object, _ []byte) {
 				_ = printer.PrintObject(opts.stream.Out, element, f)
 			})
 		},
@@ -132,11 +138,11 @@ func (l *listOptions) Run(args []string, formatter bundle.ObjectVisitor) error {
 			return err
 		}
 	case 2:
-		if err := bundle.ListKind(img, args[1], formatter); err != nil {
+		if err := bundle.ListKind(img, normalizeKind(args[1]), formatter); err != nil {
 			return err
 		}
 	case 3:
-		if err := bundle.Get(img, args[1], args[2], formatter); err != nil {
+		if err := bundle.Get(img, normalizeKind(args[1]), args[2], formatter); err != nil {
 			return err
 		}
 	}

diff --git a/pkg/cmd/bundle/list_test.go b/pkg/cmd/bundle/list_test.go
@@ -62,6 +62,16 @@ func TestListCommand(t *testing.T) {
 			format:         "name",
 			expectedStdout: "task.tekton.dev/foobar\n",
 			additionalArgs: []string{"Task"},
+		}, {
+			name:           "specify-kind-task-lowercase-plural",
+			format:         "name",
+			expectedStdout: "task.tekton.dev/foobar\n",
+			additionalArgs: []string{"tasks"},
+		}, {
+			name:           "specify-kind-task-lowercase-singular",
+			format:         "name",
+			expectedStdout: "task.tekton.dev/foobar\n",
+			additionalArgs: []string{"task"},
 		}, {
 			name:           "specify-kind-pipeline",
 			format:         "name",
@@ -71,7 +81,7 @@ func TestListCommand(t *testing.T) {
 			name:           "specify-kind-name-dne",
 			format:         "name",
 			additionalArgs: []string{"Pipeline", "does-not-exist"},
-			expectedErr:    "no objects of kind Pipeline named does-not-exist found in img",
+			expectedErr:    `no objects of kind "pipeline" named "does-not-exist" found in img`,
 		}, {
 			name:           "specify-kind-name",
 			format:         "name",

diff --git a/pkg/cmd/bundle/push.go b/pkg/cmd/bundle/push.go
@@ -53,7 +53,7 @@ Input:
 
 	c := &cobra.Command{
 		Use:   "push",
-		Short: "Push a new Tekton bundle",
+		Short: "Create or replace a Tekton bundle",
 		Long:  longHelp,
 		Annotations: map[string]string{
 			"commandType": "main",

diff --git a/pkg/cmd/bundle/push_test.go b/pkg/cmd/bundle/push_test.go
@@ -150,9 +150,10 @@ func TestPushCommand(t *testing.T) {
 					t.Errorf("layer %d with title %s does not match an expected layer", i, title)
 				}
 
-				if l.Annotations[tkremote.KindAnnotation] != layer.kind ||
+				kind := normalizeKind(layer.kind)
+				if l.Annotations[tkremote.KindAnnotation] != kind ||
 					l.Annotations[tkremote.APIVersionAnnotation] != layer.apiVersion {
-					t.Errorf("layer annotations (%s, %s) do not match expected (%s, %s)", l.Annotations[tkremote.KindAnnotation], l.Annotations[tkremote.APIVersionAnnotation], layer.kind, layer.apiVersion)
+					t.Errorf("layer annotations (%s, %s) do not match expected (%s, %s)", l.Annotations[tkremote.KindAnnotation], l.Annotations[tkremote.APIVersionAnnotation], kind, layer.apiVersion)
 				}
 
 				actual := readTarLayer(t, layers[i])