add support for handling plain+v0 bundle types (#242)

Author: everettraven

Diff for: Makefile

@@ -105,8 +105,10 @@ kind-cluster-cleanup: $(KIND) ## Delete the kind cluster
 
 kind-load-test-artifacts: $(KIND) ## Load the e2e testdata container images into a kind cluster
 	$(CONTAINER_RUNTIME) build $(TESTDATA_DIR)/bundles/registry-v1/prometheus-operator.v0.47.0 -t localhost/testdata/bundles/registry-v1/prometheus-operator:v0.47.0
+	$(CONTAINER_RUNTIME) build $(TESTDATA_DIR)/bundles/plain-v0/plain.v0.1.0 -t localhost/testdata/bundles/plain-v0/plain:v0.1.0
 	$(CONTAINER_RUNTIME) build $(TESTDATA_DIR)/catalogs -f $(TESTDATA_DIR)/catalogs/test-catalog.Dockerfile -t localhost/testdata/catalogs/test-catalog:e2e
 	$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v0.47.0 --name $(KIND_CLUSTER_NAME)
+	$(KIND) load docker-image localhost/testdata/bundles/plain-v0/plain:v0.1.0 --name $(KIND_CLUSTER_NAME)
 	$(KIND) load docker-image localhost/testdata/catalogs/test-catalog:e2e --name $(KIND_CLUSTER_NAME)
 
 ##@ Build

Diff for: internal/controllers/operator_controller.go

@@ -156,9 +156,19 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
 	op.Status.ResolvedBundleResource = bundleImage
 	setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundleImage), op.GetGeneration())
 
+	mediaType, err := bundleEntity.MediaType()
+	if err != nil {
+		setInstalledStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration())
+		return ctrl.Result{}, err
+	}
+	bundleProvisioner, err := mapBundleMediaTypeToBundleProvisioner(mediaType)
+	if err != nil {
+		setInstalledStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration())
+		return ctrl.Result{}, err
+	}
 	// Ensure a BundleDeployment exists with its bundle source from the bundle
 	// image we just looked up in the solution.
-	dep := r.generateExpectedBundleDeployment(*op, bundleImage)
+	dep := r.generateExpectedBundleDeployment(*op, bundleImage, bundleProvisioner)
 	if err := r.ensureBundleDeployment(ctx, dep); err != nil {
 		// originally Reason: operatorsv1alpha1.ReasonInstallationFailed
 		op.Status.InstalledBundleResource = ""
@@ -244,12 +254,13 @@ func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Soluti
 	return nil, fmt.Errorf("entity for package %q not found in solution", packageName)
 }
 
-func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string) *unstructured.Unstructured {
+func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string, bundleProvisioner string) *unstructured.Unstructured {
 	// We use unstructured here to avoid problems of serializing default values when sending patches to the apiserver.
 	// If you use a typed object, any default values from that struct get serialized into the JSON patch, which could
 	// cause unrelated fields to be patched back to the default value even though that isn't the intention. Using an
 	// unstructured ensures that the patch contains only what is specified. Using unstructured like this is basically
 	// identical to "kubectl apply -f"
+
 	bd := &unstructured.Unstructured{Object: map[string]interface{}{
 		"apiVersion": rukpakv1alpha1.GroupVersion.String(),
 		"kind":       rukpakv1alpha1.BundleDeploymentKind,
@@ -261,8 +272,7 @@ func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha
 			"provisionerClassName": "core-rukpak-io-plain",
 			"template": map[string]interface{}{
 				"spec": map[string]interface{}{
-					// TODO: Don't assume registry provisioner
-					"provisionerClassName": "core-rukpak-io-registry",
+					"provisionerClassName": bundleProvisioner,
 					"source": map[string]interface{}{
 						// TODO: Don't assume image type
 						"type": string(rukpakv1alpha1.SourceTypeImage),
@@ -363,6 +373,23 @@ func isBundleDepStale(bd *rukpakv1alpha1.BundleDeployment) bool {
 	return bd != nil && bd.Status.ObservedGeneration != bd.GetGeneration()
 }
 
+// mapBundleMediaTypeToBundleProvisioner maps an olm.bundle.mediatype property to a
+// rukpak bundle provisioner class name that is capable of unpacking the bundle type
+func mapBundleMediaTypeToBundleProvisioner(mediaType string) (string, error) {
+	switch mediaType {
+	case entity.MediaTypePlain:
+		return "core-rukpak-io-plain", nil
+	// To ensure compatibility with bundles created with OLMv0 where the
+	// olm.bundle.mediatype property doesn't exist, we assume that if the
+	// property is empty (i.e doesn't exist) that the bundle is one created
+	// with OLMv0 and therefore should use the registry provisioner
+	case entity.MediaTypeRegistry, "":
+		return "core-rukpak-io-registry", nil
+	default:
+		return "", fmt.Errorf("unknown bundle mediatype: %s", mediaType)
+	}
+}
+
 // setResolvedStatusConditionSuccess sets the resolved status condition to success.
 func setResolvedStatusConditionSuccess(conditions *[]metav1.Condition, message string, generation int64) {
 	apimeta.SetStatusCondition(conditions, metav1.Condition{

Diff for: internal/controllers/operator_controller_test.go

@@ -904,6 +904,107 @@ var _ = Describe("Operator Controller Test", func() {
 				Expect(cond.Message).To(Equal("installation has not been attempted as resolution failed"))
 			})
 		})
+		When("the operator specifies a package with a plain+v0 bundle", func() {
+			var pkgName string
+			var pkgVer string
+			var pkgChan string
+			BeforeEach(func() {
+				By("initializing cluster state")
+				pkgName = "plain"
+				pkgVer = "0.1.0"
+				pkgChan = "beta"
+				operator = &operatorsv1alpha1.Operator{
+					ObjectMeta: metav1.ObjectMeta{Name: opKey.Name},
+					Spec: operatorsv1alpha1.OperatorSpec{
+						PackageName: pkgName,
+						Version:     pkgVer,
+						Channel:     pkgChan,
+					},
+				}
+				err := cl.Create(ctx, operator)
+				Expect(err).NotTo(HaveOccurred())
+			})
+			It("sets resolution success status", func() {
+				By("running reconcile")
+				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
+				Expect(res).To(Equal(ctrl.Result{}))
+				Expect(err).NotTo(HaveOccurred())
+				By("fetching updated operator after reconcile")
+				Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
+
+				By("Checking the status fields")
+				Expect(operator.Status.ResolvedBundleResource).To(Equal("quay.io/operatorhub/plain@sha256:plain"))
+				Expect(operator.Status.InstalledBundleResource).To(Equal(""))
+
+				By("checking the expected conditions")
+				cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeResolved)
+				Expect(cond).NotTo(BeNil())
+				Expect(cond.Status).To(Equal(metav1.ConditionTrue))
+				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonSuccess))
+				Expect(cond.Message).To(Equal("resolved to \"quay.io/operatorhub/plain@sha256:plain\""))
+				cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled)
+				Expect(cond).NotTo(BeNil())
+				Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
+				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
+				Expect(cond.Message).To(Equal("bundledeployment status is unknown"))
+
+				By("fetching the bundled deployment")
+				bd := &rukpakv1alpha1.BundleDeployment{}
+				Expect(cl.Get(ctx, types.NamespacedName{Name: opKey.Name}, bd)).NotTo(HaveOccurred())
+				Expect(bd.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-plain"))
+				Expect(bd.Spec.Template.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-plain"))
+				Expect(bd.Spec.Template.Spec.Source.Type).To(Equal(rukpakv1alpha1.SourceTypeImage))
+				Expect(bd.Spec.Template.Spec.Source.Image).NotTo(BeNil())
+				Expect(bd.Spec.Template.Spec.Source.Image.Ref).To(Equal("quay.io/operatorhub/plain@sha256:plain"))
+			})
+		})
+		When("the operator specifies a package with a bad bundle mediatype", func() {
+			var pkgName string
+			var pkgVer string
+			var pkgChan string
+			BeforeEach(func() {
+				By("initializing cluster state")
+				pkgName = "badmedia"
+				pkgVer = "0.1.0"
+				pkgChan = "beta"
+				operator = &operatorsv1alpha1.Operator{
+					ObjectMeta: metav1.ObjectMeta{Name: opKey.Name},
+					Spec: operatorsv1alpha1.OperatorSpec{
+						PackageName: pkgName,
+						Version:     pkgVer,
+						Channel:     pkgChan,
+					},
+				}
+				err := cl.Create(ctx, operator)
+				Expect(err).NotTo(HaveOccurred())
+			})
+			It("sets resolution success status", func() {
+				By("running reconcile")
+				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
+				Expect(res).To(Equal(ctrl.Result{}))
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).To(Equal("unknown bundle mediatype: badmedia+v1"))
+
+				By("fetching updated operator after reconcile")
+				Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
+
+				By("Checking the status fields")
+				Expect(operator.Status.ResolvedBundleResource).To(Equal("quay.io/operatorhub/badmedia@sha256:badmedia"))
+				Expect(operator.Status.InstalledBundleResource).To(Equal(""))
+
+				By("checking the expected conditions")
+				cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeResolved)
+				Expect(cond).NotTo(BeNil())
+				Expect(cond.Status).To(Equal(metav1.ConditionTrue))
+				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonSuccess))
+				Expect(cond.Message).To(Equal("resolved to \"quay.io/operatorhub/badmedia@sha256:badmedia\""))
+				cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled)
+				Expect(cond).NotTo(BeNil())
+				Expect(cond.Status).To(Equal(metav1.ConditionFalse))
+				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationFailed))
+				Expect(cond.Message).To(Equal("unknown bundle mediatype: badmedia+v1"))
+			})
+		})
 		When("an invalid semver is provided that bypasses the regex validation", func() {
 			var (
 				pkgName    string
@@ -955,7 +1056,6 @@ var _ = Describe("Operator Controller Test", func() {
 				Expect(cond.Message).To(Equal("installation has not been attempted as spec is invalid"))
 			})
 		})
-
 	})
 })
 
@@ -1000,4 +1100,18 @@ var testEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{
 		"olm.package":     `{"packageName":"badimage","version":"0.1.0"}`,
 		"olm.gvk":         `[]`,
 	}),
+	"operatorhub/plain/0.1.0": *input.NewEntity("operatorhub/plain/0.1.0", map[string]string{
+		"olm.bundle.path":      `"quay.io/operatorhub/plain@sha256:plain"`,
+		"olm.channel":          `{"channelName":"beta","priority":0}`,
+		"olm.package":          `{"packageName":"plain","version":"0.1.0"}`,
+		"olm.gvk":              `[]`,
+		"olm.bundle.mediatype": `"plain+v0"`,
+	}),
+	"operatorhub/badmedia/0.1.0": *input.NewEntity("operatorhub/badmedia/0.1.0", map[string]string{
+		"olm.bundle.path":      `"quay.io/operatorhub/badmedia@sha256:badmedia"`,
+		"olm.channel":          `{"channelName":"beta","priority":0}`,
+		"olm.package":          `{"packageName":"badmedia","version":"0.1.0"}`,
+		"olm.gvk":              `[]`,
+		"olm.bundle.mediatype": `"badmedia+v1"`,
+	}),
 })

Diff for: internal/resolution/entitysources/catalogdsource.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/operator-framework/deppy/pkg/deppy"
 	"github.com/operator-framework/deppy/pkg/deppy/input"
+	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"
 	"github.com/operator-framework/operator-registry/alpha/property"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -80,12 +81,16 @@ func getEntities(ctx context.Context, client client.Client) (input.EntityList, e
 	for _, bundle := range bundleMetadatas.Items {
 		props := map[string]string{}
 
+		// TODO: We should make sure all properties are forwarded
+		// through and avoid a lossy translation from FBC --> entity
 		for _, prop := range bundle.Spec.Properties {
 			switch prop.Type {
 			case property.TypePackage:
 				// this is already a json marshalled object, so it doesn't need to be marshalled
 				// like the other ones
 				props[property.TypePackage] = string(prop.Value)
+			case entity.PropertyBundleMediaType:
+				props[entity.PropertyBundleMediaType] = string(prop.Value)
 			}
 		}
 

Diff for: internal/resolution/variable_sources/entity/bundle_entity.go

@@ -12,6 +12,19 @@ import (
 
 const PropertyBundlePath = "olm.bundle.path"
 
+// TODO: Is this the right place for these?
+// ----
+const PropertyBundleMediaType = "olm.bundle.mediatype"
+
+type MediaType string
+
+const (
+	MediaTypePlain    = "plain+v0"
+	MediaTypeRegistry = "registry+v1"
+)
+
+// ----
+
 type ChannelProperties struct {
 	property.Channel
 	Replaces  string   `json:"replaces,omitempty"`
@@ -58,6 +71,7 @@ type BundleEntity struct {
 	channelProperties *ChannelProperties
 	semVersion        *semver.Version
 	bundlePath        string
+	mediaType         string
 	mu                sync.RWMutex
 }
 
@@ -124,6 +138,27 @@ func (b *BundleEntity) BundlePath() (string, error) {
 	return b.bundlePath, nil
 }
 
+func (b *BundleEntity) MediaType() (string, error) {
+	if err := b.loadMediaType(); err != nil {
+		return "", err
+	}
+
+	return b.mediaType, nil
+}
+
+func (b *BundleEntity) loadMediaType() error {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	if b.mediaType == "" {
+		mediaType, err := loadFromEntity[string](b.Entity, PropertyBundleMediaType, optional)
+		if err != nil {
+			return fmt.Errorf("error determining bundle mediatype for entity '%s': %w", b.ID, err)
+		}
+		b.mediaType = mediaType
+	}
+	return nil
+}
+
 func (b *BundleEntity) loadPackage() error {
 	b.mu.Lock()
 	defer b.mu.Unlock()

Diff for: internal/resolution/variable_sources/entity/bundle_entity_test.go

@@ -1,6 +1,7 @@
 package entity_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/blang/semver/v4"
@@ -267,4 +268,32 @@ var _ = Describe("BundleEntity", func() {
 			Expect(err.Error()).To(Equal("error determining bundle path for entity 'operatorhub/prometheus/0.14.0': property 'olm.bundle.path' ('badBundlePath') could not be parsed: invalid character 'b' looking for beginning of value"))
 		})
 	})
+
+	Describe("MediaType", func() {
+		It("should return the bundle mediatype property if present", func() {
+			entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{
+				olmentity.PropertyBundleMediaType: fmt.Sprintf(`"%s"`, olmentity.MediaTypePlain),
+			})
+			bundleEntity := olmentity.NewBundleEntity(entity)
+			mediaType, err := bundleEntity.MediaType()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(mediaType).To(Equal(olmentity.MediaTypePlain))
+		})
+		It("should not return an error if the property is not found", func() {
+			entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{})
+			bundleEntity := olmentity.NewBundleEntity(entity)
+			mediaType, err := bundleEntity.MediaType()
+			Expect(mediaType).To(BeEmpty())
+			Expect(err).To(BeNil())
+		})
+		It("should return error if the property is malformed", func() {
+			entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{
+				olmentity.PropertyBundleMediaType: "badtype",
+			})
+			bundleEntity := olmentity.NewBundleEntity(entity)
+			mediaType, err := bundleEntity.MediaType()
+			Expect(mediaType).To(BeEmpty())
+			Expect(err.Error()).To(Equal("error determining bundle mediatype for entity 'operatorhub/prometheus/0.14.0': property 'olm.bundle.mediatype' ('badtype') could not be parsed: invalid character 'b' looking for beginning of value"))
+		})
+	})
 })