Updates the Tekton Bundle spec and resolver to be more explicit.

The current spec is ambiguous with regards to the form of the `kind` annotation. In the code, we implicitely enforce that the kind must be singular, and lowercased. The spec half-way asks for this and the resolver doesn't enforce this. Therefore, this change:

- adds validation to the oci resolver to enforce the annotation rules
- adds corresponding tests to ensure the enforcement is invoked
- modifies the bundle contract to explicitely state the pluralization requirement
- modifies a test utility for invoking this case

The tkn cli has been updated as well to conform to this explicit spec.

docs/tekton-bundle-contracts.md

@@ -22,7 +22,7 @@ Each layer of the image must map 1:1 with a single Tekton resource (eg Task).
 Each layer must contain all of the following annotations:
 
 - `dev.tekton.image.name` => `ObjectMeta.Name` of the resource
-- `dev.tekton.image.kind` => `TypeMeta.Kind` of the resource, all lowercased (eg, `task`)
+- `dev.tekton.image.kind` => `TypeMeta.Kind` of the resource, all lower-cased and singular (eg, `task`)
 - `dev.tekton.image.apiVersion` => `TypeMeta.APIVersion` of the resource (eg 
 "tekton.dev/v1alpha1")  
 
@@ -38,7 +38,7 @@ Furthermore, each layer must contain a YAML or JSON representation of the underl
 missing any identifying fields (missing an `apiVersion` for instance) then it will be considered invalid.
 
 Any tool creating a Tekton bundle must enforce this format and ensure that the annotations and contents all match and
-confirm to this spec. Additionally, the Tekton controller will reject non-conforming Tekton Bundles.
+conform to this spec. Additionally, the Tekton controller will reject non-conforming Tekton Bundles.
 
 ## Examples
 

pkg/reconciler/taskrun/resources/taskref.go

@@ -59,7 +59,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
 
 			// Because the resolver will only return references with the same kind (eg ClusterTask), this will ensure we
 			// don't accidentally return a Task with the same name but different kind.
-			obj, err := resolver.Get(strings.ToLower(string(kind)), name)
+			obj, err := resolver.Get(strings.TrimSuffix(strings.ToLower(string(kind)), "s"), name)
 			if err != nil {
 				return nil, err
 			}

pkg/remote/oci/resolver.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"strings"
 	"time"
 
 	"github.com/google/go-containerregistry/pkg/authn"
@@ -104,12 +105,21 @@ func (o *Resolver) Get(kind, name string) (runtime.Object, error) {
 		return nil, fmt.Errorf("could not read image layers: %w", err)
 	}
 
+	layerMap := map[string]v1.Layer{}
+	for _, l := range layers {
+		digest, err := l.Digest()
+		if err != nil {
+			return nil, fmt.Errorf("failed to find digest for layer: %w", err)
+		}
+		layerMap[digest.String()] = l
+	}
+
 	for idx, l := range manifest.Layers {
 		lKind := l.Annotations[KindAnnotation]
 		lName := l.Annotations[TitleAnnotation]
 
 		if kind == lKind && name == lName {
-			obj, err := readTarLayer(layers[idx])
+			obj, err := readTarLayer(layerMap[l.Digest.String()])
 			if err != nil {
 				// This could still be a raw layer so try to read it as that instead.
 				return readRawLayer(layers[idx])
@@ -135,6 +145,27 @@ func (o *Resolver) checkImageCompliance(manifest *v1.Manifest) error {
 	if len(manifest.Layers) > MaximumBundleObjects {
 		return fmt.Errorf("bundle %s contained more than the maximum %d allow objects", o.imageReference, MaximumBundleObjects)
 	}
+
+	// Ensure each layer complies to the spec.
+	for _, l := range manifest.Layers {
+		refDigest := fmt.Sprintf("%s:%s", o.imageReference, l.Digest.String())
+		if _, ok := l.Annotations[APIVersionAnnotation]; !ok {
+			return fmt.Errorf("invalid tekton bundle: %s does not contain a %s annotation", refDigest, APIVersionAnnotation)
+		}
+
+		if _, ok := l.Annotations[TitleAnnotation]; !ok {
+			return fmt.Errorf("invalid tekton bundle: %s does not contain a %s annotation", refDigest, TitleAnnotation)
+		}
+
+		kind, ok := l.Annotations[KindAnnotation]
+		if !ok {
+			return fmt.Errorf("invalid tekton bundle: %s does not contain a %s annotation", refDigest, KindAnnotation)
+		}
+		if strings.TrimSuffix(strings.ToLower(kind), "s") != kind {
+			return fmt.Errorf("invalid tekton bundle: %s annotation for %s must be lowercased and singular, found %s", KindAnnotation, refDigest, kind)
+		}
+	}
+
 	return nil
 }
 

pkg/remote/oci/resolver_test.go

@@ -28,13 +28,30 @@ import (
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/registry"
 	tb "github.com/tektoncd/pipeline/internal/builder/v1beta1"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/remote"
 	"github.com/tektoncd/pipeline/pkg/remote/oci"
 	"github.com/tektoncd/pipeline/test"
 	"github.com/tektoncd/pipeline/test/diff"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+func asIsMapper(obj runtime.Object) map[string]string {
+	annotations := map[string]string{
+		oci.TitleAnnotation: getObjectName(obj),
+	}
+	if obj.GetObjectKind().GroupVersionKind().Kind != "" {
+		annotations[oci.KindAnnotation] = obj.GetObjectKind().GroupVersionKind().Kind
+	}
+	if obj.GetObjectKind().GroupVersionKind().Version != "" {
+		annotations[oci.APIVersionAnnotation] = obj.GetObjectKind().GroupVersionKind().Version
+	}
+	return annotations
+}
+
+var _ test.ObjectAnnotationMapper = asIsMapper
+
 func TestOCIResolver(t *testing.T) {
 	// Set up a fake registry to push an image to.
 	s := httptest.NewServer(registry.New())
@@ -48,20 +65,23 @@ func TestOCIResolver(t *testing.T) {
 		name         string
 		objs         []runtime.Object
 		listExpected []remote.ResolvedObject
+		mapper       test.ObjectAnnotationMapper
 		wantErr      string
 	}{
 		{
 			name: "single-task",
 			objs: []runtime.Object{
 				tb.Task("simple-task", tb.TaskType),
 			},
+			mapper:       test.DefaultObjectAnnotationMapper,
 			listExpected: []remote.ResolvedObject{{Kind: "task", APIVersion: "v1beta1", Name: "simple-task"}},
 		},
 		{
 			name: "cluster-task",
 			objs: []runtime.Object{
 				tb.ClusterTask("simple-task", tb.ClusterTaskType),
 			},
+			mapper:       test.DefaultObjectAnnotationMapper,
 			listExpected: []remote.ResolvedObject{{Kind: "clustertask", APIVersion: "v1beta1", Name: "simple-task"}},
 		},
 		{
@@ -70,6 +90,7 @@ func TestOCIResolver(t *testing.T) {
 				tb.Task("first-task", tb.TaskType),
 				tb.Task("second-task", tb.TaskType),
 			},
+			mapper: test.DefaultObjectAnnotationMapper,
 			listExpected: []remote.ResolvedObject{
 				{Kind: "task", APIVersion: "v1beta1", Name: "first-task"},
 				{Kind: "task", APIVersion: "v1beta1", Name: "second-task"},
@@ -90,15 +111,37 @@ func TestOCIResolver(t *testing.T) {
 				tb.Task("tenth-task", tb.TaskType),
 				tb.Task("eleventh-task", tb.TaskType),
 			},
+			mapper:       test.DefaultObjectAnnotationMapper,
 			listExpected: []remote.ResolvedObject{},
 			wantErr:      "contained more than the maximum 10 allow objects",
 		},
+		{
+			name:         "single-task-no-version",
+			objs:         []runtime.Object{&v1beta1.Task{TypeMeta: metav1.TypeMeta{Kind: "task"}, ObjectMeta: metav1.ObjectMeta{Name: "foo"}}},
+			listExpected: []remote.ResolvedObject{},
+			mapper:       asIsMapper,
+			wantErr:      "does not contain a dev.tekton.image.apiVersion annotation",
+		},
+		{
+			name:         "single-task-no-kind",
+			objs:         []runtime.Object{&v1beta1.Task{TypeMeta: metav1.TypeMeta{APIVersion: "tekton.dev/v1beta1"}, ObjectMeta: metav1.ObjectMeta{Name: "foo"}}},
+			listExpected: []remote.ResolvedObject{},
+			mapper:       asIsMapper,
+			wantErr:      "does not contain a dev.tekton.image.kind annotation",
+		},
+		{
+			name:         "single-task-kind-incorrect-form",
+			objs:         []runtime.Object{&v1beta1.Task{TypeMeta: metav1.TypeMeta{APIVersion: "tekton.dev/v1beta1", Kind: "Task"}, ObjectMeta: metav1.ObjectMeta{Name: "foo"}}},
+			listExpected: []remote.ResolvedObject{},
+			mapper:       asIsMapper,
+			wantErr:      "must be lowercased and singular, found Task",
+		},
 	}
 
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
 			// Create a new image with the objects.
-			ref, err := test.CreateImage(fmt.Sprintf("%s/testociresolve/%s", u.Host, tc.name), tc.objs...)
+			ref, err := test.CreateImageWithAnnotations(fmt.Sprintf("%s/testociresolve/%s", u.Host, tc.name), tc.mapper, tc.objs...)
 			if err != nil {
 				t.Fatalf("could not push image: %#v", err)
 			}

test/remote.go

@@ -33,10 +33,30 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+// ObjectAnnotationMapper is a func alias that maps a runtime Object to the Tekton Bundle annotations map.
+type ObjectAnnotationMapper func(object runtime.Object) map[string]string
+
+var (
+	// DefaultObjectAnnotationMapper does the "right" thing by conforming to the Tekton Bundle spec.
+	DefaultObjectAnnotationMapper = func(obj runtime.Object) map[string]string {
+		return map[string]string{
+			tkremote.TitleAnnotation:      getObjectName(obj),
+			tkremote.KindAnnotation:       strings.TrimSuffix(strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind), "s"),
+			tkremote.APIVersionAnnotation: obj.GetObjectKind().GroupVersionKind().Version,
+		}
+	}
+)
+
 // CreateImage will push a new OCI image artifact with the provided raw data object as a layer and return the full image
 // reference with a digest to fetch the image. Key must be specified as [lowercase kind]/[object name]. The image ref
 // with a digest is returned.
 func CreateImage(ref string, objs ...runtime.Object) (string, error) {
+	return CreateImageWithAnnotations(ref, DefaultObjectAnnotationMapper, objs...)
+}
+
+// CreateImageWithAnnotations is the base form of #CreateImage which accepts an ObjectAnnotationMapper to map an object
+// to the annotations for it.
+func CreateImageWithAnnotations(ref string, mapper ObjectAnnotationMapper, objs ...runtime.Object) (string, error) {
 	imgRef, err := name.ParseReference(ref)
 	if err != nil {
 		return "", fmt.Errorf("undexpected error producing image reference %w", err)
@@ -73,13 +93,10 @@ func CreateImage(ref string, objs ...runtime.Object) (string, error) {
 			return "", fmt.Errorf("unexpected error adding layer to image %w", err)
 		}
 
+		annotations := mapper(obj)
 		img, err = mutate.Append(img, mutate.Addendum{
-			Layer: layer,
-			Annotations: map[string]string{
-				tkremote.TitleAnnotation:      getObjectName(obj),
-				tkremote.KindAnnotation:       strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind),
-				tkremote.APIVersionAnnotation: strings.ToLower(obj.GetObjectKind().GroupVersionKind().Version),
-			},
+			Layer:       layer,
+			Annotations: annotations,
 		})
 		if err != nil {
 			return "", fmt.Errorf("could not add layer to image %w", err)