predicate error tracking sketch

Signed-off-by: Jordan Keister <jordan@nimblewidget.com>

Author: grokspawn

internal/catalogmetadata/filter/bundle_predicates.go

@@ -1,6 +1,8 @@
 package filter
 
 import (
+	"fmt"
+
 	mmsemver "github.com/Masterminds/semver/v3"
 	bsemver "github.com/blang/semver/v4"
 
@@ -11,59 +13,80 @@ import (
 )
 
 func WithPackageName(packageName string) Predicate[catalogmetadata.Bundle] {
-	return func(bundle *catalogmetadata.Bundle) bool {
-		return bundle.Package == packageName
+	return func(bundle *catalogmetadata.Bundle) (bool, []string) {
+		value := bundle.Package == packageName
+		if !value {
+			return false, []string{packageName}
+		}
+		return value, nil
 	}
 }
 
 func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[catalogmetadata.Bundle] {
-	return func(bundle *catalogmetadata.Bundle) bool {
+	return func(bundle *catalogmetadata.Bundle) (bool, []string) {
 		bVersion, err := bundle.Version()
 		if err != nil {
-			return false
+			return false, []string{err.Error()}
 		}
 		// No error should occur here because the simple version was successfully parsed by blang
 		// We are unaware of any tests cases that would cause one to fail but not the other
 		// This will cause code coverage to drop for this line. We don't ignore the error because
 		// there might be that one extreme edge case that might cause one to fail but not the other
 		mVersion, err := mmsemver.NewVersion(bVersion.String())
 		if err != nil {
-			return false
+			return false, []string{err.Error()}
+		}
+		res := semverRange.Check(mVersion)
+		if !res {
+			return false, []string{fmt.Sprintf("no package %q matching version %q found", bundle.Package, semverRange.String())}
 		}
-		return semverRange.Check(mVersion)
+		return true, nil
 	}
 }
 
-func InBlangSemverRange(semverRange bsemver.Range) Predicate[catalogmetadata.Bundle] {
-	return func(bundle *catalogmetadata.Bundle) bool {
+func InBlangSemverRange(semverRange bsemver.Range, semverRangeString string) Predicate[catalogmetadata.Bundle] {
+	return func(bundle *catalogmetadata.Bundle) (bool, []string) {
 		bundleVersion, err := bundle.Version()
 		if err != nil {
-			return false
+			return false, []string{err.Error()}
 		}
-		return semverRange(*bundleVersion)
+		if !semverRange(*bundleVersion) {
+			return false, []string{fmt.Sprintf("no package %q matching version %q found", bundle.Package, semverRangeString)}
+		}
+		return true, nil
 	}
 }
 
 func InChannel(channelName string) Predicate[catalogmetadata.Bundle] {
-	return func(bundle *catalogmetadata.Bundle) bool {
+	return func(bundle *catalogmetadata.Bundle) (bool, []string) {
 		for _, ch := range bundle.InChannels {
 			if ch.Name == channelName {
-				return true
+				return true, nil
 			}
 		}
-		return false
+		return false, []string{fmt.Sprintf("no package %q found in channel %q", bundle.Package, channelName)}
 	}
 }
 
 func WithBundleImage(bundleImage string) Predicate[catalogmetadata.Bundle] {
-	return func(bundle *catalogmetadata.Bundle) bool {
-		return bundle.Image == bundleImage
+	return func(bundle *catalogmetadata.Bundle) (bool, []string) {
+		res := bundle.Image == bundleImage
+		if !res {
+			return false, []string{fmt.Sprintf("no matching bundle image %q found for package %s", bundleImage, bundle.Package)}
+		} else {
+			return true, nil
+		}
 	}
 }
 
 func WithBundleName(bundleName string) Predicate[catalogmetadata.Bundle] {
-	return func(bundle *catalogmetadata.Bundle) bool {
-		return bundle.Name == bundleName
+	return func(bundle *catalogmetadata.Bundle) (bool, []string) {
+		res := bundle.Name == bundleName
+		if !res {
+			return false, []string{fmt.Sprintf("no matching bundle name %q found for package %s", bundleName, bundle.Package)}
+		} else {
+			return true, nil
+		}
 	}
 }
 
@@ -87,20 +110,25 @@ func LegacySuccessor(installedBundle *ocv1alpha1.BundleMetadata) Predicate[catal
 		return false
 	}
 
-	return func(candidateBundle *catalogmetadata.Bundle) bool {
+	return func(candidateBundle *catalogmetadata.Bundle) (bool, []string) {
 		for _, ch := range candidateBundle.InChannels {
 			for _, chEntry := range ch.Entries {
 				if candidateBundle.Name == chEntry.Name && isSuccessor(chEntry) {
-					return true
+					return true, nil
 				}
 			}
 		}
-		return false
+		return false, []string{fmt.Sprintf("no legacy successor found for bundle name %q", candidateBundle.Name)}
 	}
 }
 
 func WithDeprecation(deprecated bool) Predicate[catalogmetadata.Bundle] {
-	return func(bundle *catalogmetadata.Bundle) bool {
-		return bundle.HasDeprecation() == deprecated
+	return func(bundle *catalogmetadata.Bundle) (bool, []string) {
+		res := bundle.HasDeprecation() == deprecated
+		if !res {
+			return false, []string{fmt.Sprintf("no bundle %q found with desired deprecation status %t", bundle.Name, deprecated)}
+		} else {
+			return true, nil
+		}
 	}
 }

internal/catalogmetadata/filter/bundle_predicates_test.go

@@ -92,7 +92,7 @@ func TestInBlangSemverRange(t *testing.T) {
 
 	vRange := bsemver.MustParseRange(">=1.0.0")
 
-	f := filter.InBlangSemverRange(vRange)
+	f := filter.InBlangSemverRange(vRange, ">=1.0.0")
 
 	assert.True(t, f(b1))
 	assert.False(t, f(b2))

internal/catalogmetadata/filter/filter.go

@@ -5,43 +5,54 @@ import (
 )
 
 // Predicate returns true if the object should be kept when filtering
-type Predicate[T catalogmetadata.Schemas] func(entity *T) bool
+type Predicate[T catalogmetadata.Schemas] func(entity *T) (bool, []string)
 
 // Filter filters a slice accordingly to
-func Filter[T catalogmetadata.Schemas](in []*T, test Predicate[T]) []*T {
+func Filter[T catalogmetadata.Schemas](in []*T, test Predicate[T]) ([]*T, []string) {
 	out := []*T{}
+	var errs []string
 	for i := range in {
-		if test(in[i]) {
+		res, e := test(in[i])
+		if res {
 			out = append(out, in[i])
+		} else {
+			errs = append(errs, e...)
 		}
 	}
-	return out
+	return out, errs
 }
 
 func And[T catalogmetadata.Schemas](predicates ...Predicate[T]) Predicate[T] {
-	return func(obj *T) bool {
+	return func(obj *T) (bool, []string) {
 		for _, predicate := range predicates {
-			if !predicate(obj) {
-				return false
+			if res, errs := predicate(obj); !res {
+				return false, errs
 			}
 		}
-		return true
+		return true, nil
 	}
 }
 
 func Or[T catalogmetadata.Schemas](predicates ...Predicate[T]) Predicate[T] {
-	return func(obj *T) bool {
+	return func(obj *T) (bool, []string) {
+		var errs []string
 		for _, predicate := range predicates {
-			if predicate(obj) {
-				return true
+			if res, e := predicate(obj); !res {
+				errs = append(errs, e...)
+			} else {
+				return true, nil
 			}
 		}
-		return false
+		return false, errs
 	}
 }
 
 func Not[T catalogmetadata.Schemas](predicate Predicate[T]) Predicate[T] {
-	return func(obj *T) bool {
-		return !predicate(obj)
+	return func(obj *T) (bool, []string) {
+		predicateTrue, errs := predicate(obj)
+		if predicateTrue {
+			return false, errs
+		}
+		return true, nil
 	}
 }

internal/controllers/clusterextension_controller.go

@@ -402,7 +402,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1
 		predicates = append(predicates, upgradePredicate)
 	}
 
-	resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...))
+	resultSet, errs := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...))
 
 	var upgradeErrorPrefix string
 	if installedBundle != nil {
@@ -413,16 +413,17 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1
 		upgradeErrorPrefix = fmt.Sprintf("error upgrading from currently installed version %q: ", installedBundleVersion.String())
 	}
 	if len(resultSet) == 0 {
-		switch {
-		case versionRange != "" && channelName != "":
-			return nil, fmt.Errorf("%sno package %q matching version %q in channel %q found", upgradeErrorPrefix, packageName, versionRange, channelName)
-		case versionRange != "":
-			return nil, fmt.Errorf("%sno package %q matching version %q found", upgradeErrorPrefix, packageName, versionRange)
-		case channelName != "":
-			return nil, fmt.Errorf("%sno package %q in channel %q found", upgradeErrorPrefix, packageName, channelName)
-		default:
-			return nil, fmt.Errorf("%sno package %q found", upgradeErrorPrefix, packageName)
-		}
+		return nil, fmt.Errorf("%s %s", upgradeErrorPrefix, errs)
+		// switch {
+		// case versionRange != "" && channelName != "":
+		// 	return nil, fmt.Errorf("%sno package %q matching version %q in channel %q found", upgradeErrorPrefix, packageName, versionRange, channelName)
+		// case versionRange != "":
+		// 	return nil, fmt.Errorf("%sno package %q matching version %q found", upgradeErrorPrefix, packageName, versionRange)
+		// case channelName != "":
+		// 	return nil, fmt.Errorf("%sno package %q in channel %q found", upgradeErrorPrefix, packageName, channelName)
+		// default:
+		// 	return nil, fmt.Errorf("%sno package %q found", upgradeErrorPrefix, packageName)
+		// }
 	}
 
 	sort.SliceStable(resultSet, func(i, j int) bool {