Fix tests, bump catalogd, address comments.

Signed-off-by: dtfranz <[email protected]>

Author: dtfranz

cmd/resolutioncli/main.go

@@ -151,24 +151,24 @@ func resolve(ctx context.Context, resolver *solver.DeppySolver, packageName stri
 		return "", err
 	}
 
-	bundleEntity, err := getBundleEntityFromSolution(solution, packageName)
+	bundle, err := bundleFromSolution(solution, packageName)
 	if err != nil {
 		return "", err
 	}
 
 	// Get the bundle image reference for the bundle
-	return bundleEntity.Image, nil
+	return bundle.Image, nil
 }
 
-func getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
+func bundleFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
 	for _, variable := range solution.SelectedVariables() {
 		switch v := variable.(type) {
 		case *olmvariables.BundleVariable:
-			entityPkgName := v.BundleEntity().Package
-			if packageName == entityPkgName {
-				return v.BundleEntity(), nil
+			bundlePkgName := v.Bundle().Package
+			if packageName == bundlePkgName {
+				return v.Bundle(), nil
 			}
 		}
 	}
-	return nil, fmt.Errorf("entity for package %q not found in solution", packageName)
+	return nil, fmt.Errorf("bundle for package %q not found in solution", packageName)
 }

internal/catalogmetadata/types_test.go

@@ -123,7 +123,13 @@ func TestBundleRequiredPackages(t *testing.T) {
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			packages, err := tt.bundle.RequiredPackages()
-			assert.Equal(t, tt.wantRequiredPackages, packages)
+			assert.Equal(t, len(tt.wantRequiredPackages), len(packages))
+			for i, pkg := range packages {
+				// Must custom compare due to Semver function type
+				assert.Equal(t, tt.wantRequiredPackages[i].PackageRequired, pkg.PackageRequired)
+				assert.Equal(t, tt.wantRequiredPackages[i].PackageName, pkg.PackageName)
+				assert.Equal(t, tt.wantRequiredPackages[i].VersionRange, pkg.VersionRange)
+			}
 			if tt.wantErr != "" {
 				assert.EqualError(t, err, tt.wantErr)
 			} else {

internal/controllers/operator_controller.go

@@ -154,9 +154,9 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
 		return ctrl.Result{}, unsat
 	}
 
-	// lookup the bundle entity in the solution that corresponds to the
+	// lookup the bundle in the solution that corresponds to the
 	// Operator's desired package name.
-	bundleEntity, err := r.getBundleEntityFromSolution(solution, op.Spec.PackageName)
+	bundle, err := r.bundleFromSolution(solution, op.Spec.PackageName)
 	if err != nil {
 		op.Status.InstalledBundleResource = ""
 		setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution failed", op.GetGeneration())
@@ -165,11 +165,11 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
 		return ctrl.Result{}, err
 	}
 
-	// Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundleEntity.Image value.
-	op.Status.ResolvedBundleResource = bundleEntity.Image
-	setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundleEntity.Image), op.GetGeneration())
+	// Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundle.Image value.
+	op.Status.ResolvedBundleResource = bundle.Image
+	setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundle.Image), op.GetGeneration())
 
-	mediaType, err := bundleEntity.MediaType()
+	mediaType, err := bundle.MediaType()
 	if err != nil {
 		setInstalledStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration())
 		return ctrl.Result{}, err
@@ -181,7 +181,7 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
 	}
 	// Ensure a BundleDeployment exists with its bundle source from the bundle
 	// image we just looked up in the solution.
-	dep := r.generateExpectedBundleDeployment(*op, bundleEntity.Image, bundleProvisioner)
+	dep := r.generateExpectedBundleDeployment(*op, bundle.Image, bundleProvisioner)
 	if err := r.ensureBundleDeployment(ctx, dep); err != nil {
 		// originally Reason: operatorsv1alpha1.ReasonInstallationFailed
 		op.Status.InstalledBundleResource = ""
@@ -251,17 +251,17 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph
 	}
 }
 
-func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
+func (r *OperatorReconciler) bundleFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) {
 	for _, variable := range solution.SelectedVariables() {
 		switch v := variable.(type) {
 		case *olmvariables.BundleVariable:
-			entityPkgName := v.BundleEntity().Package
-			if packageName == entityPkgName {
-				return v.BundleEntity(), nil
+			bundlePkgName := v.Bundle().Package
+			if packageName == bundlePkgName {
+				return v.Bundle(), nil
 			}
 		}
 	}
-	return nil, fmt.Errorf("entity for package %q not found in solution", packageName)
+	return nil, fmt.Errorf("bundle for package %q not found in solution", packageName)
 }
 
 func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string, bundleProvisioner string) *unstructured.Unstructured {

internal/controllers/operator_controller_test.go

@@ -2,13 +2,14 @@ package controllers_test
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/operator-framework/deppy/pkg/deppy"
-	"github.com/operator-framework/deppy/pkg/deppy/input"
 	"github.com/operator-framework/deppy/pkg/deppy/solver"
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
+	"github.com/operator-framework/operator-registry/alpha/property"
 	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -20,21 +21,25 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
+	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
 	"github.com/operator-framework/operator-controller/internal/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/controllers"
+	testutil "github.com/operator-framework/operator-controller/test/util"
 )
 
 var _ = Describe("Operator Controller Test", func() {
 	var (
-		ctx        context.Context
-		reconciler *controllers.OperatorReconciler
+		ctx               context.Context
+		fakeCatalogClient testutil.FakeCatalogClient
+		reconciler        *controllers.OperatorReconciler
 	)
 	BeforeEach(func() {
 		ctx = context.Background()
+		fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList)
 		reconciler = &controllers.OperatorReconciler{
 			Client:   cl,
 			Scheme:   sch,
-			Resolver: solver.NewDeppySolver(testEntitySource, controllers.NewVariableSource(cl)),
+			Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)),
 		}
 	})
 	When("the operator does not exist", func() {
@@ -575,43 +580,6 @@ var _ = Describe("Operator Controller Test", func() {
 				})
 			})
 		})
-		When("the selected bundle's image ref cannot be parsed", func() {
-			const pkgName = "badimage"
-			BeforeEach(func() {
-				By("initializing cluster state")
-				operator = &operatorsv1alpha1.Operator{
-					ObjectMeta: metav1.ObjectMeta{Name: opKey.Name},
-					Spec:       operatorsv1alpha1.OperatorSpec{PackageName: pkgName},
-				}
-				err := cl.Create(ctx, operator)
-				Expect(err).NotTo(HaveOccurred())
-			})
-			It("sets resolution failure status and returns an error", func() {
-				By("running reconcile")
-				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
-				Expect(res).To(Equal(ctrl.Result{}))
-				Expect(err).To(MatchError(ContainSubstring(`error determining bundle path for entity`)))
-
-				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(""))
-				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.ConditionFalse))
-				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed))
-				Expect(cond.Message).To(ContainSubstring(`error determining bundle path for entity`))
-				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("installation has not been attempted as resolution failed"))
-			})
-		})
 		When("the operator specifies a duplicate package", func() {
 			const pkgName = "prometheus"
 			var dupOperator *operatorsv1alpha1.Operator
@@ -1080,41 +1048,62 @@ func verifyConditionsInvariants(op *operatorsv1alpha1.Operator) {
 	}
 }
 
-var testEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{
-	"operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{
-		"olm.bundle.path":         `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`,
-		"olm.bundle.channelEntry": `{"name":"prometheus.0.37.0"}`,
-		"olm.channel":             `{"channelName":"beta","priority":0}`,
-		"olm.package":             `{"packageName":"prometheus","version":"0.37.0"}`,
-		"olm.gvk":                 `[]`,
-	}),
-	"operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{
-		"olm.bundle.path":         `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`,
-		"olm.bundle.channelEntry": `{"name":"prometheus.0.47.0"}`,
-		"olm.channel":             `{"channelName":"beta","priority":0,"replaces":"prometheusoperator.0.37.0"}`,
-		"olm.package":             `{"packageName":"prometheus","version":"0.47.0"}`,
-		"olm.gvk":                 `[]`,
-	}),
-	"operatorhub/badimage/0.1.0": *input.NewEntity("operatorhub/badimage/0.1.0", map[string]string{
-		"olm.bundle.path":         `{"name": "quay.io/operatorhubio/badimage:v0.1.0"}`,
-		"olm.bundle.channelEntry": `{"name":"badimage.0.1.0"}`,
-		"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.bundle.channelEntry": `{"name":"plain.0.1.0"}`,
-		"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.bundle.channelEntry": `{"name":"badmedia.0.1.0"}`,
-		"olm.channel":             `{"channelName":"beta","priority":0}`,
-		"olm.package":             `{"packageName":"badmedia","version":"0.1.0"}`,
-		"olm.gvk":                 `[]`,
-		"olm.bundle.mediatype":    `"badmedia+v1"`,
-	}),
-})
+var betaChannel = catalogmetadata.Channel{Channel: declcfg.Channel{
+	Name: "beta",
+	Entries: []declcfg.ChannelEntry{
+		{
+			Name: "operatorhub/prometheus/0.37.0",
+		},
+		{
+			Name:     "operatorhub/prometheus/0.47.0",
+			Replaces: "operatorhub/prometheus/0.37.0",
+		},
+		{
+			Name: "operatorhub/plain/0.1.0",
+		},
+		{
+			Name: "operatorhub/badmedia/0.1.0",
+		},
+	},
+}}
+
+var testBundleList = []*catalogmetadata.Bundle{
+	{Bundle: declcfg.Bundle{
+		Name:    "operatorhub/prometheus/0.37.0",
+		Package: "prometheus",
+		Image:   "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35",
+		Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.37.0"}`)},
+			{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
+		},
+	}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
+	{Bundle: declcfg.Bundle{
+		Name:    "operatorhub/prometheus/0.47.0",
+		Package: "prometheus",
+		Image:   "quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed",
+		Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.47.0"}`)},
+			{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
+		},
+	}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
+	{Bundle: declcfg.Bundle{
+		Name:    "operatorhub/plain/0.1.0",
+		Package: "plain",
+		Image:   "quay.io/operatorhub/plain@sha256:plain",
+		Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"plain","version":"0.1.0"}`)},
+			{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
+			{Type: "olm.bundle.mediatype", Value: json.RawMessage(`"plain+v0"`)},
+		},
+	}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
+	{Bundle: declcfg.Bundle{
+		Name:    "operatorhub/badmedia/0.1.0",
+		Package: "badmedia",
+		Image:   "quay.io/operatorhub/badmedia@sha256:badmedia",
+		Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"badmedia","version":"0.1.0"}`)},
+			{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
+			{Type: "olm.bundle.mediatype", Value: json.RawMessage(`"badmedia+v1"`)},
+		},
+	}, InChannels: []*catalogmetadata.Channel{&betaChannel}},
+}

internal/resolution/variables/bundle.go

@@ -18,7 +18,7 @@ type BundleVariable struct {
 	dependencies []*catalogmetadata.Bundle
 }
 
-func (b *BundleVariable) BundleEntity() *catalogmetadata.Bundle {
+func (b *BundleVariable) Bundle() *catalogmetadata.Bundle {
 	return b.bundle
 }
 

internal/resolution/variables/bundle_test.go

@@ -1,37 +1,55 @@
 package variables_test
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/operator-framework/deppy/pkg/deppy"
 	"github.com/operator-framework/deppy/pkg/deppy/constraint"
-	"github.com/operator-framework/deppy/pkg/deppy/input"
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"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/catalogmetadata"
 	olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
 )
 
 func TestBundleVariable(t *testing.T) {
-	bundleEntity := olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{
-		property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`,
-		property.TypeChannel: `{"channelName":"stable","priority":0}`,
-	}))
-	dependencies := []*olmentity.BundleEntity{
-		olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{
-			property.TypePackage: `{"packageName": "test-package-2", "version": "2.0.0"}`,
-			property.TypeChannel: `{"channelName":"stable","priority":0}`,
-		})),
-		olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{
-			property.TypePackage: `{"packageName": "test-package-3", "version": "2.0.0"}`,
-			property.TypeChannel: `{"channelName":"stable","priority":0}`,
-		})),
-	}
-	bv := olmvariables.NewBundleVariable(bundleEntity, dependencies)
-
-	if bv.BundleEntity() != bundleEntity {
-		t.Errorf("bundle entity '%v' does not match expected '%v'", bv.BundleEntity(), bundleEntity)
+	bundle := &catalogmetadata.Bundle{
+		InChannels: []*catalogmetadata.Channel{
+			{Channel: declcfg.Channel{Name: "stable"}},
+		},
+		Bundle: declcfg.Bundle{Name: "bundle-1", Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)},
+		}},
 	}
+	dependencies := []*catalogmetadata.Bundle{
+		{
+			InChannels: []*catalogmetadata.Channel{
+				{Channel: declcfg.Channel{Name: "stable"}},
+			},
+			Bundle: declcfg.Bundle{Name: "bundle-2", Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)},
+			}},
+		},
+		{
+			InChannels: []*catalogmetadata.Channel{
+				{Channel: declcfg.Channel{Name: "stable"}},
+			},
+			Bundle: declcfg.Bundle{Name: "bundle-3", Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)},
+			}},
+		},
+	}
+	ids := olmvariables.BundleToBundleVariableIDs(bundle)
+	if len(ids) != len(bundle.InChannels) {
+		t.Fatalf("bundle should produce one variable ID per channel; received: %d", len(bundle.InChannels))
+	}
+	bv := olmvariables.NewBundleVariable(ids[0], bundle, dependencies)
+
+	if bv.Bundle() != bundle {
+		t.Errorf("bundle '%v' does not match expected '%v'", bv.Bundle(), bundle)
+	}
+
 	for i, d := range bv.Dependencies() {
 		if d != dependencies[i] {
 			t.Errorf("dependency[%v] '%v' does not match expected '%v'", i, d, dependencies[i])

internal/resolution/variables/installed_package.go

@@ -17,7 +17,7 @@ type InstalledPackageVariable struct {
 	bundles []*catalogmetadata.Bundle
 }
 
-func (r *InstalledPackageVariable) BundleEntities() []*catalogmetadata.Bundle {
+func (r *InstalledPackageVariable) Bundles() []*catalogmetadata.Bundle {
 	return r.bundles
 }