Add unit tests for bundle resolver

pkg/remote/oci/resolver_test.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"net/http/httptest"
 	"net/url"
-	"reflect"
 	"strings"
 	"testing"
 
@@ -39,7 +38,7 @@ import (
 
 func asIsMapper(obj runtime.Object) map[string]string {
 	annotations := map[string]string{
-		oci.TitleAnnotation: getObjectName(obj),
+		oci.TitleAnnotation: test.GetObjectName(obj),
 	}
 	if obj.GetObjectKind().GroupVersionKind().Kind != "" {
 		annotations[oci.KindAnnotation] = obj.GetObjectKind().GroupVersionKind().Kind
@@ -204,7 +203,7 @@ func TestOCIResolver(t *testing.T) {
 			}
 
 			for _, obj := range tc.objs {
-				actual, err := resolver.Get(context.Background(), strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind), getObjectName(obj))
+				actual, err := resolver.Get(context.Background(), strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind), test.GetObjectName(obj))
 				if err != nil {
 					t.Fatalf("could not retrieve object from image: %#v", err)
 				}
@@ -216,7 +215,3 @@ func TestOCIResolver(t *testing.T) {
 		})
 	}
 }
-
-func getObjectName(obj runtime.Object) string {
-	return reflect.Indirect(reflect.ValueOf(obj)).FieldByName("ObjectMeta").FieldByName("Name").String()
-}

pkg/resolution/resolver/bundle/bundle.go

@@ -82,8 +82,8 @@ func GetEntry(ctx context.Context, keychain authn.Keychain, opts RequestOptions)
 		return nil, fmt.Errorf("could not parse image manifest: %w", err)
 	}
 
-	if err := checkImageCompliance(opts.Bundle, manifest); err != nil {
-		return nil, err
+	if err := checkImageCompliance(manifest); err != nil {
+		return nil, fmt.Errorf("invalid tekton bundle %s, error: %w", opts.Bundle, err)
 	}
 
 	layers, err := img.Layers()
@@ -133,29 +133,28 @@ func retrieveImage(ctx context.Context, keychain authn.Keychain, ref string) (v1
 }
 
 // checkImageCompliance will perform common checks to ensure the Tekton Bundle is compliant to our spec.
-func checkImageCompliance(ref string, manifest *v1.Manifest) error {
+func checkImageCompliance(manifest *v1.Manifest) error {
 	// Check the manifest's layers to ensure there are a maximum of 10.
 	if len(manifest.Layers) > MaximumBundleObjects {
-		return fmt.Errorf("bundle %s contained more than the maximum %d allow objects", ref, MaximumBundleObjects)
+		return fmt.Errorf("contained more than the maximum %d allow objects", MaximumBundleObjects)
 	}
 
 	// Ensure each layer complies to the spec.
-	for _, l := range manifest.Layers {
-		refDigest := fmt.Sprintf("%s:%s", ref, l.Digest.String())
+	for i, l := range manifest.Layers {
 		if _, ok := l.Annotations[BundleAnnotationAPIVersion]; !ok {
-			return fmt.Errorf("invalid tekton bundle: %s does not contain a %s annotation", refDigest, BundleAnnotationKind)
+			return fmt.Errorf("the layer %v does not contain a %s annotation", i, BundleAnnotationAPIVersion)
 		}
 
 		if _, ok := l.Annotations[BundleAnnotationName]; !ok {
-			return fmt.Errorf("invalid tekton bundle: %s does not contain a %s annotation", refDigest, BundleAnnotationName)
+			return fmt.Errorf("the layer %v does not contain a %s annotation", i, BundleAnnotationName)
 		}
 
 		kind, ok := l.Annotations[BundleAnnotationKind]
 		if !ok {
-			return fmt.Errorf("invalid tekton bundle: %s does not contain a %s annotation", refDigest, BundleAnnotationKind)
+			return fmt.Errorf("the layer %v does not contain a %s annotation", i, BundleAnnotationKind)
 		}
 		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", BundleAnnotationKind, refDigest, kind)
+			return fmt.Errorf("the layer %v the annotation %s must be lowercased and singular, found %s", i, BundleAnnotationKind, kind)
 		}
 	}
 

pkg/resolution/resolver/bundle/config.go

@@ -13,10 +13,13 @@ limitations under the License.
 
 package bundle
 
-// ConfigServiceAccount is the configuration field name for controlling
-// the Service Account name to use for bundle requests.
-const ConfigServiceAccount = "default-service-account"
-
-// ConfigKind is the configuration field name for controlling
-// what the layer name in the bundle image is.
-const ConfigKind = "default-kind"
+const (
+	// ConfigMapName is the bundle resolver's config map
+	ConfigMapName = "bundleresolver-config"
+	// ConfigServiceAccount is the configuration field name for controlling
+	// the Service Account name to use for bundle requests.
+	ConfigServiceAccount = "default-service-account"
+	// ConfigKind is the configuration field name for controlling
+	// what the layer name in the bundle image is.
+	ConfigKind = "default-kind"
+)

pkg/resolution/resolver/bundle/params.go

@@ -54,7 +54,7 @@ func OptionsFromParams(ctx context.Context, params []pipelinev1beta1.Param) (Req
 		if saString, ok := conf[ConfigServiceAccount]; ok {
 			sa = saString
 		} else {
-			return opts, fmt.Errorf("default Service Account  was not set during installation of the bundle resolver")
+			return opts, fmt.Errorf("default Service Account was not set during installation of the bundle resolver")
 		}
 	} else {
 		sa = saVal.StringVal

pkg/resolution/resolver/bundle/resolver.go

@@ -30,15 +30,20 @@ import (
 	"knative.dev/pkg/client/injection/kube/client"
 )
 
-const disabledError = "cannot handle resolution request, enable-bundles-resolver feature flag not true"
+const (
+	disabledError = "cannot handle resolution request, enable-bundles-resolver feature flag not true"
 
-// LabelValueBundleResolverType is the value to use for the
-// resolution.tekton.dev/type label on resource requests
-const LabelValueBundleResolverType string = "bundles"
+	// LabelValueBundleResolverType is the value to use for the
+	// resolution.tekton.dev/type label on resource requests
+	LabelValueBundleResolverType string = "bundles"
 
-// TODO(sbwsg): This should be exposed as a configurable option for
-// admins (e.g. via ConfigMap)
-const timeoutDuration = time.Minute
+	// TODO(sbwsg): This should be exposed as a configurable option for
+	// admins (e.g. via ConfigMap)
+	timeoutDuration = time.Minute
+
+	// BundleResolverName is the name that the bundle resolver should be associated with.
+	BundleResolverName = "bundleresolver"
+)
 
 // Resolver implements a framework.Resolver that can fetch files from OCI bundles.
 type Resolver struct {
@@ -53,12 +58,12 @@ func (r *Resolver) Initialize(ctx context.Context) error {
 
 // GetName returns a string name to refer to this Resolver by.
 func (r *Resolver) GetName(context.Context) string {
-	return "bundleresolver"
+	return BundleResolverName
 }
 
 // GetConfigName returns the name of the bundle resolver's configmap.
 func (r *Resolver) GetConfigName(context.Context) string {
-	return "bundleresolver-config"
+	return ConfigMapName
 }
 
 // GetSelector returns a map of labels to match requests to this Resolver.