Add interface for client mocking, fix bug with semver

Signed-off-by: dtfranz <dfranz@redhat.com>
operator-framework · Sep 19, 2023 · 19bd853 · 19bd853
1 parent 0be1726
commit 19bd853
Show file tree

Hide file tree

Showing 9 changed files with 19 additions and 15 deletions.
diff --git a/cmd/resolutioncli/variable_source.go b/cmd/resolutioncli/variable_source.go
@@ -23,7 +23,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
 )
 
-func newPackageVariableSource(catalog *catalogclient.Client, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
+func newPackageVariableSource(catalog catalogclient.CatalogClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
 	return func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
 		pkgSource, err := variablesources.NewRequiredPackageVariableSource(
 			catalog,

diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go
@@ -12,6 +12,10 @@ import (
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
 )
 
+type CatalogClient interface {
+	Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error)
+}
+
 func NewClient(cl client.Client) *Client {
 	return &Client{cl: cl}
 }

diff --git a/internal/catalogmetadata/types.go b/internal/catalogmetadata/types.go
@@ -98,7 +98,7 @@ func (b *Bundle) loadRequiredPackages() error {
 		if err != nil {
 			return fmt.Errorf("error determining bundle required packages for bundle %q: %s", b.Name, err)
 		}
-		for _, requiredPackage := range requiredPackages {
+		for index, requiredPackage := range requiredPackages {
 			semverRange, err := bsemver.ParseRange(requiredPackage.VersionRange)
 			if err != nil {
 				return fmt.Errorf(
@@ -108,7 +108,7 @@ func (b *Bundle) loadRequiredPackages() error {
 					err,
 				)
 			}
-			requiredPackage.SemverRange = &semverRange
+			requiredPackages[index].SemverRange = &semverRange
 		}
 		b.requiredPackages = requiredPackages
 	}

diff --git a/internal/controllers/variable_source.go b/internal/controllers/variable_source.go
@@ -25,7 +25,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
 )
 
-func NewVariableSource(cl client.Client, catalog *catalogclient.Client) variablesources.NestedVariableSource {
+func NewVariableSource(cl client.Client, catalog catalogclient.CatalogClient) variablesources.NestedVariableSource {
 	return variablesources.NestedVariableSource{
 		func(inputVariableSource input.VariableSource) (input.VariableSource, error) {
 			return variablesources.NewOperatorVariableSource(cl, catalog, inputVariableSource), nil

diff --git a/internal/resolution/variablesources/bundle_deployment.go b/internal/resolution/variablesources/bundle_deployment.go
@@ -15,11 +15,11 @@ var _ input.VariableSource = &BundleDeploymentVariableSource{}
 
 type BundleDeploymentVariableSource struct {
 	client              client.Client
-	catalog             *catalogclient.Client
+	catalog             catalogclient.CatalogClient
 	inputVariableSource input.VariableSource
 }
 
-func NewBundleDeploymentVariableSource(cl client.Client, catalog *catalogclient.Client, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource {
+func NewBundleDeploymentVariableSource(cl client.Client, catalog catalogclient.CatalogClient, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource {
 	return &BundleDeploymentVariableSource{
 		client:              cl,
 		catalog:             catalog,

diff --git a/internal/resolution/variablesources/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go
@@ -18,11 +18,11 @@ import (
 var _ input.VariableSource = &BundlesAndDepsVariableSource{}
 
 type BundlesAndDepsVariableSource struct {
-	catalog         *catalogclient.Client
+	catalog         catalogclient.CatalogClient
 	variableSources []input.VariableSource
 }
 
-func NewBundlesAndDepsVariableSource(catalog *catalogclient.Client, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource {
+func NewBundlesAndDepsVariableSource(catalog catalogclient.CatalogClient, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource {
 	return &BundlesAndDepsVariableSource{
 		catalog:         catalog,
 		variableSources: inputVariableSources,
@@ -98,7 +98,7 @@ func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*ca
 	for _, requiredPackage := range requiredPackages {
 		packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(*requiredPackage.SemverRange)))
 		if len(packageDependencyBundles) == 0 {
-			return nil, fmt.Errorf("could not find package dependencies for bundle %q", bundle.Name)
+			return nil, fmt.Errorf("could not find package dependencies for bundle '%s'", bundle.Name)
 		}
 		for i := 0; i < len(packageDependencyBundles); i++ {
 			bundle := packageDependencyBundles[i]

diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go
@@ -17,7 +17,7 @@ import (
 var _ input.VariableSource = &InstalledPackageVariableSource{}
 
 type InstalledPackageVariableSource struct {
-	catalog     *catalogclient.Client
+	catalog     catalogclient.CatalogClient
 	bundleImage string
 }
 
@@ -60,7 +60,7 @@ func (r *InstalledPackageVariableSource) notFoundError() error {
 	return fmt.Errorf("bundleImage %q not found", r.bundleImage)
 }
 
-func NewInstalledPackageVariableSource(catalog *catalogclient.Client, bundleImage string) (*InstalledPackageVariableSource, error) {
+func NewInstalledPackageVariableSource(catalog catalogclient.CatalogClient, bundleImage string) (*InstalledPackageVariableSource, error) {
 	return &InstalledPackageVariableSource{
 		catalog:     catalog,
 		bundleImage: bundleImage,

diff --git a/internal/resolution/variablesources/operator.go b/internal/resolution/variablesources/operator.go
@@ -15,11 +15,11 @@ var _ input.VariableSource = &OperatorVariableSource{}
 
 type OperatorVariableSource struct {
 	client              client.Client
-	catalog             *catalogclient.Client
+	catalog             catalogclient.CatalogClient
 	inputVariableSource input.VariableSource
 }
 
-func NewOperatorVariableSource(cl client.Client, catalog *catalogclient.Client, inputVariableSource input.VariableSource) *OperatorVariableSource {
+func NewOperatorVariableSource(cl client.Client, catalog catalogclient.CatalogClient, inputVariableSource input.VariableSource) *OperatorVariableSource {
 	return &OperatorVariableSource{
 		client:              cl,
 		catalog:             catalog,

diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go
@@ -47,15 +47,15 @@ func InChannel(channelName string) RequiredPackageVariableSourceOption {
 }
 
 type RequiredPackageVariableSource struct {
-	catalog *catalogclient.Client
+	catalog catalogclient.CatalogClient
 
 	packageName  string
 	versionRange string
 	channelName  string
 	predicates   []catalogfilter.Predicate[catalogmetadata.Bundle]
 }
 
-func NewRequiredPackageVariableSource(catalog *catalogclient.Client, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) {
+func NewRequiredPackageVariableSource(catalog catalogclient.CatalogClient, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) {
 	if packageName == "" {
 		return nil, fmt.Errorf("package name must not be empty")
 	}