Switch to catalogd's CatalogMetadata APIs

Signed-off-by: Mikalai Radchuk <[email protected]>

Author: Mikalai Radchuk

cmd/resolutioncli/entity_source.go

@@ -18,17 +18,12 @@ package main
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
 
 	"github.com/operator-framework/deppy/pkg/deppy"
 	"github.com/operator-framework/deppy/pkg/deppy/input"
 	"github.com/operator-framework/operator-registry/alpha/action"
-	"github.com/operator-framework/operator-registry/alpha/declcfg"
-	"github.com/operator-framework/operator-registry/alpha/model"
-	"github.com/operator-framework/operator-registry/alpha/property"
 
-	olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities"
+	"github.com/operator-framework/operator-controller/internal/resolution/entitysources"
 )
 
 type indexRefEntitySource struct {
@@ -101,12 +96,8 @@ func (es *indexRefEntitySource) entities(ctx context.Context) (input.EntityList,
 			return nil, err
 		}
 
-		model, err := declcfg.ConvertToModel(*cfg)
-		if err != nil {
-			return nil, err
-		}
-
-		entities, err := modelToEntities(model)
+		// TODO: update empty catalog name string to be catalog name once we support multiple catalogs in CLI
+		entities, err := entitysources.MetadataToEntities("", cfg.Channels, cfg.Bundles)
 		if err != nil {
 			return nil, err
 		}
@@ -116,46 +107,3 @@ func (es *indexRefEntitySource) entities(ctx context.Context) (input.EntityList,
 
 	return es.entitiesCache, nil
 }
-
-// TODO: Reduce code duplication: share a function with catalogdEntitySource (see getEntities)
-// We don't want to maintain two functions performing conversion into input.EntityList.
-// For this we need some common format. So we need a package which will be able
-// to convert from declfcg structs into CRD structs directly or via model.Model.
-// One option would be to make this piece of code from catalogd reusable and exportable:
-// https://github.com/operator-framework/catalogd/blob/9fe45a628de2e74d9cd73c3650fa2582aaac5213/pkg/controllers/core/catalog_controller.go#L200-L360
-func modelToEntities(model model.Model) (input.EntityList, error) {
-	entities := input.EntityList{}
-
-	for _, pkg := range model {
-		for _, ch := range pkg.Channels {
-			for _, bundle := range ch.Bundles {
-				props := map[string]string{}
-
-				for _, prop := range bundle.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)
-					}
-				}
-
-				imgValue, err := json.Marshal(bundle.Image)
-				if err != nil {
-					return nil, err
-				}
-				props[olmentity.PropertyBundlePath] = string(imgValue)
-
-				channelValue, _ := json.Marshal(property.Channel{ChannelName: ch.Name, Priority: 0})
-				props[property.TypeChannel] = string(channelValue)
-				entity := input.Entity{
-					ID:         deppy.IdentifierFromString(fmt.Sprintf("%s%s%s", bundle.Name, bundle.Package.Name, ch.Name)),
-					Properties: props,
-				}
-				entities = append(entities, entity)
-			}
-		}
-	}
-
-	return entities, nil
-}

config/rbac/role.yaml

@@ -7,7 +7,7 @@ rules:
 - apiGroups:
   - catalogd.operatorframework.io
   resources:
-  - bundlemetadata
+  - catalogmetadata
   verbs:
   - list
   - watch

internal/controllers/operator_controller.go

@@ -62,9 +62,9 @@ type OperatorReconciler struct {
 
 //+kubebuilder:rbac:groups=core.rukpak.io,resources=bundledeployments,verbs=get;list;watch;create;update;patch
 
-//+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=bundlemetadata,verbs=list;watch
 //+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=packages,verbs=list;watch
 //+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=catalogs,verbs=list;watch
+//+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=catalogmetadata,verbs=list;watch
 
 func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	l := log.FromContext(ctx).WithName("operator-controller")

internal/resolution/entitysources/catalogdsource.go

@@ -8,6 +8,7 @@ import (
 	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
 	"github.com/operator-framework/deppy/pkg/deppy"
 	"github.com/operator-framework/deppy/pkg/deppy/input"
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"github.com/operator-framework/operator-registry/alpha/property"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -71,70 +72,119 @@ func (es *CatalogdEntitySource) Iterate(ctx context.Context, fn input.IteratorFu
 	return nil
 }
 
-func getEntities(ctx context.Context, client client.Client) (input.EntityList, error) {
-	entityList := input.EntityList{}
-	bundleMetadatas, packageMetdatas, err := fetchMetadata(ctx, client)
-	if err != nil {
+func getEntities(ctx context.Context, cl client.Client) (input.EntityList, error) {
+	allEntitiesList := input.EntityList{}
+
+	var catalogList catalogd.CatalogList
+	if err := cl.List(ctx, &catalogList); err != nil {
 		return nil, err
 	}
-	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 entities.PropertyBundleMediaType:
-				props[entities.PropertyBundleMediaType] = string(prop.Value)
-			}
+	for _, catalog := range catalogList.Items {
+		channels, bundles, err := fetchCatalogMetadata(ctx, cl, catalog.Name)
+		if err != nil {
+			return nil, err
 		}
 
-		imgValue, err := json.Marshal(bundle.Spec.Image)
+		catalogEntitiesList, err := MetadataToEntities(catalog.Name, channels, bundles)
 		if err != nil {
 			return nil, err
 		}
-		props[entities.PropertyBundlePath] = string(imgValue)
-		catalogScopedPkgName := fmt.Sprintf("%s-%s", bundle.Spec.Catalog.Name, bundle.Spec.Package)
-		bundlePkg := packageMetdatas[catalogScopedPkgName]
-		for _, ch := range bundlePkg.Spec.Channels {
-			for _, b := range ch.Entries {
-				catalogScopedEntryName := fmt.Sprintf("%s-%s", bundle.Spec.Catalog.Name, b.Name)
-				if catalogScopedEntryName == bundle.Name {
-					channelValue, _ := json.Marshal(property.Channel{ChannelName: ch.Name, Priority: 0})
-					props[property.TypeChannel] = string(channelValue)
-					replacesValue, _ := json.Marshal(entities.ChannelEntry{
-						Name:     b.Name,
-						Replaces: b.Replaces,
-					})
-					props[entities.PropertyBundleChannelEntry] = string(replacesValue)
-					entity := input.Entity{
-						ID:         deppy.IdentifierFromString(fmt.Sprintf("%s%s%s", bundle.Name, bundle.Spec.Package, ch.Name)),
-						Properties: props,
-					}
-					entityList = append(entityList, entity)
+
+		allEntitiesList = append(allEntitiesList, catalogEntitiesList...)
+	}
+
+	return allEntitiesList, nil
+}
+
+func MetadataToEntities(catalogName string, channels []declcfg.Channel, bundles []declcfg.Bundle) (input.EntityList, error) {
+	entityList := input.EntityList{}
+
+	bundlesMap := map[string]*declcfg.Bundle{}
+	for i := range bundles {
+		bundleKey := fmt.Sprintf("%s-%s", bundles[i].Package, bundles[i].Name)
+		bundlesMap[bundleKey] = &bundles[i]
+	}
+
+	for _, ch := range channels {
+		for _, chEntry := range ch.Entries {
+			bundleKey := fmt.Sprintf("%s-%s", ch.Package, chEntry.Name)
+			bundle, ok := bundlesMap[bundleKey]
+			if !ok {
+				return nil, fmt.Errorf("bundle %q not found in catalog %q (package %q, channel %q)", chEntry.Name, catalogName, ch.Package, ch.Name)
+			}
+
+			props := map[string]string{}
+
+			for _, prop := range bundle.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 entities.PropertyBundleMediaType:
+					props[entities.PropertyBundleMediaType] = string(prop.Value)
 				}
 			}
+
+			imgValue, err := json.Marshal(bundle.Image)
+			if err != nil {
+				return nil, err
+			}
+			props[entities.PropertyBundlePath] = string(imgValue)
+
+			channelValue, _ := json.Marshal(property.Channel{ChannelName: ch.Name, Priority: 0})
+			props[property.TypeChannel] = string(channelValue)
+			replacesValue, _ := json.Marshal(entities.ChannelEntry{
+				Name:     bundle.Name,
+				Replaces: chEntry.Replaces,
+			})
+			props[entities.PropertyBundleChannelEntry] = string(replacesValue)
+
+			catalogScopedEntryName := fmt.Sprintf("%s-%s", catalogName, bundle.Name)
+			entity := input.Entity{
+				ID:         deppy.IdentifierFromString(fmt.Sprintf("%s%s%s", catalogScopedEntryName, bundle.Package, ch.Name)),
+				Properties: props,
+			}
+			entityList = append(entityList, entity)
 		}
 	}
+
 	return entityList, nil
 }
 
-func fetchMetadata(ctx context.Context, client client.Client) (catalogd.BundleMetadataList, map[string]catalogd.Package, error) {
-	packageMetdatas := catalogd.PackageList{}
-	if err := client.List(ctx, &packageMetdatas); err != nil {
-		return catalogd.BundleMetadataList{}, nil, err
+func fetchCatalogMetadata(ctx context.Context, cl client.Client, catalogName string) ([]declcfg.Channel, []declcfg.Bundle, error) {
+	channels, err := fetchCatalogMetadataByScheme[declcfg.Channel](ctx, cl, declcfg.SchemaChannel, catalogName)
+	if err != nil {
+		return nil, nil, err
+	}
+	bundles, err := fetchCatalogMetadataByScheme[declcfg.Bundle](ctx, cl, declcfg.SchemaBundle, catalogName)
+	if err != nil {
+		return nil, nil, err
 	}
-	bundleMetadatas := catalogd.BundleMetadataList{}
-	if err := client.List(ctx, &bundleMetadatas); err != nil {
-		return catalogd.BundleMetadataList{}, nil, err
+
+	return channels, bundles, nil
+}
+
+type declcfgSchema interface {
+	declcfg.Package | declcfg.Bundle | declcfg.Channel
+}
+
+// TODO: Cleanup once https://github.com/golang/go/issues/45380 implemented
+// We should be able to get rid of the schema arg and switch based on the type passed to this generic
+func fetchCatalogMetadataByScheme[T declcfgSchema](ctx context.Context, cl client.Client, schema, catalogName string) ([]T, error) {
+	cmList := catalogd.CatalogMetadataList{}
+	if err := cl.List(ctx, &cmList, client.MatchingLabels{"schema": schema, "catalog": catalogName}); err != nil {
+		return nil, err
 	}
-	packages := map[string]catalogd.Package{}
-	for _, pkg := range packageMetdatas.Items {
-		packages[pkg.Name] = pkg
+
+	contents := []T{}
+	for _, cm := range cmList.Items {
+		var content T
+		if err := json.Unmarshal(cm.Spec.Content, &content); err != nil {
+			return nil, err
+		}
+		contents = append(contents, content)
 	}
-	return bundleMetadatas, packages, nil
+
+	return contents, nil
 }

test/e2e/e2e_suite_test.go

@@ -95,8 +95,9 @@ var _ = AfterSuite(func() {
 	}).Should(Succeed())
 
 	// speed up delete without waiting for gc
-	Expect(c.DeleteAllOf(ctx, &catalogd.BundleMetadata{})).To(Succeed())
 	Expect(c.DeleteAllOf(ctx, &catalogd.Package{})).To(Succeed())
+	Expect(c.DeleteAllOf(ctx, &catalogd.BundleMetadata{})).To(Succeed())
+	Expect(c.DeleteAllOf(ctx, &catalogd.CatalogMetadata{})).To(Succeed())
 
 	Eventually(func(g Gomega) {
 		// ensure resource cleanup
@@ -107,6 +108,10 @@ var _ = AfterSuite(func() {
 		bmd := &catalogd.BundleMetadataList{}
 		g.Expect(c.List(ctx, bmd)).To(Succeed())
 		g.Expect(bmd.Items).To(BeEmpty())
+
+		cmd := &catalogd.CatalogMetadataList{}
+		g.Expect(c.List(ctx, cmd)).To(Succeed())
+		g.Expect(bmd.Items).To(BeEmpty())
 	}).Should(Succeed())
 })
 

test/operator-framework-e2e/operator_framework_test.go

@@ -8,7 +8,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -27,6 +26,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
+	"github.com/operator-framework/operator-registry/alpha/property"
 	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
 
 	operatorv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
@@ -833,8 +834,7 @@ func createCatalogCheckResources(operatorCatalog *catalogd.Catalog, catalogDInfo
 	// checking if the bundle metadatas are created
 	By("Eventually checking if bundle metadata is created")
 	Eventually(func(g Gomega) {
-		err = validateBundleMetadataCreation(operatorCatalog, catalogDInfo.operatorName, bundleVersions)
-		g.Expect(err).ToNot(HaveOccurred())
+		validateCatalogMetadataCreation(g, operatorCatalog, catalogDInfo.operatorName, bundleVersions)
 	}).Should(Succeed())
 	return operatorCatalog, nil
 }
@@ -886,39 +886,38 @@ func validatePackageCreation(operatorCatalog *catalogd.Catalog, pkgName string)
 	return nil
 }
 
-// Checks if the bundle metadatas are created from the catalog and returns error if not.
+// Checks if the CatalogMetadata was created from the catalog and returns error if not.
 // The expected pkgNames and their versions are taken as input. This is then compared against the collected bundle versions.
 // The collected bundle versions are formed by reading the version from "olm.package" property type whose catalog name
 // matches the catalog name and pkgName matches the pkgName under consideration.
-func validateBundleMetadataCreation(operatorCatalog *catalogd.Catalog, pkgName string, versions []string) error {
-	type Package struct {
-		PackageName string `json:"packageName"`
-		Version     string `json:"version"`
-	}
-	var pkgValue Package
-	collectedBundleVersions := make([]string, 0)
-	bmList := &catalogd.BundleMetadataList{}
-	if err := c.List(ctx, bmList); err != nil {
-		return fmt.Errorf("Error retrieving the bundle metadata after %v catalog instance creation: %v", operatorCatalog.Name, err)
-	}
-
-	for _, bm := range bmList.Items {
-		if bm.Spec.Catalog.Name == operatorCatalog.Name {
-			for _, prop := range bm.Spec.Properties {
-				if prop.Type == "olm.package" {
-					err := json.Unmarshal(prop.Value, &pkgValue)
-					if err == nil && pkgValue.PackageName == pkgName {
-						collectedBundleVersions = append(collectedBundleVersions, pkgValue.Version)
-					}
-				}
+func validateCatalogMetadataCreation(g Gomega, operatorCatalog *catalogd.Catalog, pkgName string, versions []string) {
+	cmList := catalogd.CatalogMetadataList{}
+	err := c.List(ctx, &cmList, client.MatchingLabels{
+		"schema":  declcfg.SchemaBundle,
+		"catalog": operatorCatalog.Name,
+		"package": pkgName,
+	})
+	g.Expect(err).ToNot(HaveOccurred())
+
+	collectedBundleVersions := []string{}
+	for _, cm := range cmList.Items {
+		bundle := declcfg.Bundle{}
+
+		err := json.Unmarshal(cm.Spec.Content, &bundle)
+		g.Expect(err).ToNot(HaveOccurred())
+
+		for _, prop := range bundle.Properties {
+			if prop.Type == "olm.package" {
+				var pkgValue property.Package
+				err := json.Unmarshal(prop.Value, &pkgValue)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				collectedBundleVersions = append(collectedBundleVersions, pkgValue.Version)
 			}
 		}
 	}
-	if !reflect.DeepEqual(collectedBundleVersions, versions) {
-		return fmt.Errorf("Package %v for the catalog %v is not created", pkgName, operatorCatalog.Name)
-	}
 
-	return nil
+	g.Expect(collectedBundleVersions).To(ConsistOf(versions))
 }
 
 // Checks for a successful resolution and bundle path for the operator and returns error if not.

test/operator-framework-e2e/plain-example/.dockerignore

@@ -0,0 +1,4 @@
+# More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file
+# Ignore build and test binaries.
+bin/
+testbin/