Turn required package variable source into a func (#500)

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

Author: m1kola

internal/controllers/operator_controller_test.go

@@ -83,7 +83,7 @@ var _ = Describe("Operator Controller Test", func() {
 				By("running reconcile")
 				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
 				Expect(res).To(Equal(ctrl.Result{}))
-				Expect(err).To(MatchError(fmt.Sprintf("no package '%s' found", pkgName)))
+				Expect(err).To(MatchError(fmt.Sprintf("no package %q found", pkgName)))
 
 				By("fetching updated operator after reconcile")
 				Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
@@ -97,7 +97,7 @@ var _ = Describe("Operator Controller Test", func() {
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionFalse))
 				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed))
-				Expect(cond.Message).To(Equal(fmt.Sprintf("no package '%s' found", pkgName)))
+				Expect(cond.Message).To(Equal(fmt.Sprintf("no package %q found", pkgName)))
 			})
 		})
 		When("the operator specifies a version that does not exist", func() {
@@ -119,7 +119,7 @@ var _ = Describe("Operator Controller Test", func() {
 				By("running reconcile")
 				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
 				Expect(res).To(Equal(ctrl.Result{}))
-				Expect(err).To(MatchError(fmt.Sprintf("no package '%s' matching version '0.50.0' found", pkgName)))
+				Expect(err).To(MatchError(fmt.Sprintf(`no package %q matching version "0.50.0" found`, pkgName)))
 
 				By("fetching updated operator after reconcile")
 				Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
@@ -133,7 +133,7 @@ var _ = Describe("Operator Controller Test", func() {
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionFalse))
 				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed))
-				Expect(cond.Message).To(Equal(fmt.Sprintf("no package '%s' matching version '0.50.0' found", pkgName)))
+				Expect(cond.Message).To(Equal(fmt.Sprintf(`no package %q matching version "0.50.0" found`, pkgName)))
 				cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled)
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
@@ -765,7 +765,7 @@ var _ = Describe("Operator Controller Test", func() {
 				By("running reconcile")
 				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
 				Expect(res).To(Equal(ctrl.Result{}))
-				Expect(err).To(MatchError(fmt.Sprintf("no package '%s' matching version '%s' found in channel '%s'", pkgName, pkgVer, pkgChan)))
+				Expect(err).To(MatchError(fmt.Sprintf("no package %q matching version %q found in channel %q", pkgName, pkgVer, pkgChan)))
 
 				By("fetching updated operator after reconcile")
 				Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
@@ -779,7 +779,7 @@ var _ = Describe("Operator Controller Test", func() {
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionFalse))
 				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed))
-				Expect(cond.Message).To(Equal(fmt.Sprintf("no package '%s' matching version '%s' found in channel '%s'", pkgName, pkgVer, pkgChan)))
+				Expect(cond.Message).To(Equal(fmt.Sprintf("no package %q matching version %q found in channel %q", pkgName, pkgVer, pkgChan)))
 				cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled)
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
@@ -808,7 +808,7 @@ var _ = Describe("Operator Controller Test", func() {
 				By("running reconcile")
 				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
 				Expect(res).To(Equal(ctrl.Result{}))
-				Expect(err).To(MatchError(fmt.Sprintf("no package '%s' found in channel '%s'", pkgName, pkgChan)))
+				Expect(err).To(MatchError(fmt.Sprintf("no package %q found in channel %q", pkgName, pkgChan)))
 
 				By("fetching updated operator after reconcile")
 				Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
@@ -822,7 +822,7 @@ var _ = Describe("Operator Controller Test", func() {
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionFalse))
 				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed))
-				Expect(cond.Message).To(Equal(fmt.Sprintf("no package '%s' found in channel '%s'", pkgName, pkgChan)))
+				Expect(cond.Message).To(Equal(fmt.Sprintf("no package %q found in channel %q", pkgName, pkgChan)))
 				cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled)
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
@@ -854,7 +854,7 @@ var _ = Describe("Operator Controller Test", func() {
 				By("running reconcile")
 				res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
 				Expect(res).To(Equal(ctrl.Result{}))
-				Expect(err).To(MatchError(fmt.Sprintf("no package '%s' matching version '%s' found in channel '%s'", pkgName, pkgVer, pkgChan)))
+				Expect(err).To(MatchError(fmt.Sprintf("no package %q matching version %q found in channel %q", pkgName, pkgVer, pkgChan)))
 
 				By("fetching updated operator after reconcile")
 				Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred())
@@ -868,7 +868,7 @@ var _ = Describe("Operator Controller Test", func() {
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionFalse))
 				Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed))
-				Expect(cond.Message).To(Equal(fmt.Sprintf("no package '%s' matching version '%s' found in channel '%s'", pkgName, pkgVer, pkgChan)))
+				Expect(cond.Message).To(Equal(fmt.Sprintf("no package %q matching version %q found in channel %q", pkgName, pkgVer, pkgChan)))
 				cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled)
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.Status).To(Equal(metav1.ConditionUnknown))

internal/resolution/variablesources/operator.go

@@ -32,19 +32,18 @@ func (o *OperatorVariableSource) GetVariables(ctx context.Context) ([]deppy.Vari
 		variableSources = append(variableSources, o.inputVariableSource)
 	}
 
-	// build required package variable sources
-	for _, operator := range o.operators {
-		rps, err := NewRequiredPackageVariableSource(
-			o.allBundles,
-			operator.Spec.PackageName,
-			InVersionRange(operator.Spec.Version),
-			InChannel(operator.Spec.Channel),
-		)
-		if err != nil {
-			return nil, err
-		}
-		variableSources = append(variableSources, rps)
+	requiredPackages, err := MakeRequiredPackageVariables(o.allBundles, o.operators)
+	if err != nil {
+		return nil, err
 	}
 
-	return variableSources.GetVariables(ctx)
+	variables, err := variableSources.GetVariables(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, v := range requiredPackages {
+		variables = append(variables, v)
+	}
+	return variables, nil
 }

internal/resolution/variablesources/required_package.go

@@ -1,99 +1,64 @@
 package variablesources
 
 import (
-	"context"
 	"fmt"
 	"sort"
 
 	mmsemver "github.com/Masterminds/semver/v3"
-	"github.com/operator-framework/deppy/pkg/deppy"
-	"github.com/operator-framework/deppy/pkg/deppy/input"
 
+	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
 	catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
 	catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
 	olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
 )
 
-var _ input.VariableSource = &RequiredPackageVariableSource{}
+// MakeRequiredPackageVariables returns a variable which represent
+// explicit requirement for a package from an user.
+// This is when an user explicitly asks "install this" via Operator API.
+func MakeRequiredPackageVariables(allBundles []*catalogmetadata.Bundle, operators []operatorsv1alpha1.Operator) ([]*olmvariables.RequiredPackageVariable, error) {
+	result := make([]*olmvariables.RequiredPackageVariable, 0, len(operators))
 
-type RequiredPackageVariableSourceOption func(*RequiredPackageVariableSource) error
+	for _, operator := range operators {
+		packageName := operator.Spec.PackageName
+		channelName := operator.Spec.Channel
+		versionRange := operator.Spec.Version
 
-func InVersionRange(versionRange string) RequiredPackageVariableSourceOption {
-	return func(r *RequiredPackageVariableSource) error {
-		if versionRange != "" {
-			vr, err := mmsemver.NewConstraint(versionRange)
-			if err == nil {
-				r.versionRange = versionRange
-				r.predicates = append(r.predicates, catalogfilter.InMastermindsSemverRange(vr))
-				return nil
-			}
-
-			return fmt.Errorf("invalid version range '%s': %w", versionRange, err)
+		predicates := []catalogfilter.Predicate[catalogmetadata.Bundle]{
+			catalogfilter.WithPackageName(packageName),
 		}
-		return nil
-	}
-}
 
-func InChannel(channelName string) RequiredPackageVariableSourceOption {
-	return func(r *RequiredPackageVariableSource) error {
 		if channelName != "" {
-			r.channelName = channelName
-			r.predicates = append(r.predicates, catalogfilter.InChannel(channelName))
+			predicates = append(predicates, catalogfilter.InChannel(channelName))
 		}
-		return nil
-	}
-}
-
-type RequiredPackageVariableSource struct {
-	allBundles []*catalogmetadata.Bundle
-
-	packageName  string
-	versionRange string
-	channelName  string
-	predicates   []catalogfilter.Predicate[catalogmetadata.Bundle]
-}
 
-func NewRequiredPackageVariableSource(allBundles []*catalogmetadata.Bundle, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) {
-	if packageName == "" {
-		return nil, fmt.Errorf("package name must not be empty")
-	}
-	r := &RequiredPackageVariableSource{
-		allBundles: allBundles,
+		if versionRange != "" {
+			vr, err := mmsemver.NewConstraint(versionRange)
+			if err != nil {
+				return nil, fmt.Errorf("invalid version range %q: %w", versionRange, err)
+			}
+			predicates = append(predicates, catalogfilter.InMastermindsSemverRange(vr))
+		}
 
-		packageName: packageName,
-		predicates:  []catalogfilter.Predicate[catalogmetadata.Bundle]{catalogfilter.WithPackageName(packageName)},
-	}
-	for _, option := range options {
-		if err := option(r); err != nil {
-			return nil, err
+		resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...))
+		if len(resultSet) == 0 {
+			if versionRange != "" && channelName != "" {
+				return nil, fmt.Errorf("no package %q matching version %q found in channel %q", packageName, versionRange, channelName)
+			}
+			if versionRange != "" {
+				return nil, fmt.Errorf("no package %q matching version %q found", packageName, versionRange)
+			}
+			if channelName != "" {
+				return nil, fmt.Errorf("no package %q found in channel %q", packageName, channelName)
+			}
+			return nil, fmt.Errorf("no package %q found", packageName)
 		}
-	}
-	return r, nil
-}
+		sort.SliceStable(resultSet, func(i, j int) bool {
+			return catalogsort.ByVersion(resultSet[i], resultSet[j])
+		})
 
-func (r *RequiredPackageVariableSource) GetVariables(_ context.Context) ([]deppy.Variable, error) {
-	resultSet := catalogfilter.Filter(r.allBundles, catalogfilter.And(r.predicates...))
-	if len(resultSet) == 0 {
-		return nil, r.notFoundError()
+		result = append(result, olmvariables.NewRequiredPackageVariable(packageName, resultSet))
 	}
-	sort.SliceStable(resultSet, func(i, j int) bool {
-		return catalogsort.ByVersion(resultSet[i], resultSet[j])
-	})
-	return []deppy.Variable{
-		olmvariables.NewRequiredPackageVariable(r.packageName, resultSet),
-	}, nil
-}
 
-func (r *RequiredPackageVariableSource) notFoundError() error {
-	if r.versionRange != "" && r.channelName != "" {
-		return fmt.Errorf("no package '%s' matching version '%s' found in channel '%s'", r.packageName, r.versionRange, r.channelName)
-	}
-	if r.versionRange != "" {
-		return fmt.Errorf("no package '%s' matching version '%s' found", r.packageName, r.versionRange)
-	}
-	if r.channelName != "" {
-		return fmt.Errorf("no package '%s' found in channel '%s'", r.packageName, r.channelName)
-	}
-	return fmt.Errorf("no package '%s' found", r.packageName)
+	return result, nil
 }