avoid incorrectly preferring a bundle less based on channel deprecati…

…on association

by adding a new IsDeprecated() method that only returns true
if a bundle itself or the package it belongs to has been
deprecated. Update the implementation where necessary to
use this new method

Signed-off-by: everettraven <everettraven@gmail.com>

internal/catalogmetadata/sort/sort.go

@@ -19,16 +19,16 @@ func ByVersion(b1, b2 *catalogmetadata.Bundle) bool {
 }
 
 // ByDeprecation is a sort "less" function that orders bundles
-// with deprecations lower than ones without deprecations
-func ByDeprecation(b1, b2 *catalogmetadata.Bundle) bool {
+// that are deprecated lower than ones without deprecations
+func ByDeprecated(b1, b2 *catalogmetadata.Bundle) bool {
 	b1Val := 1
 	b2Val := 1
 
-	if b1.HasDeprecation() {
+	if b1.IsDeprecated() {
 		b1Val = b1Val - 1
 	}
 
-	if b2.HasDeprecation() {
+	if b2.IsDeprecated() {
 		b2Val = b2Val - 1
 	}
 

internal/catalogmetadata/sort/sort_test.go

@@ -83,7 +83,7 @@ func TestByVersion(t *testing.T) {
 	})
 }
 
-func TestByDeprecation(t *testing.T) {
+func TestByDeprecated(t *testing.T) {
 	b1 := &catalogmetadata.Bundle{
 		CatalogName: "foo",
 		Bundle: declcfg.Bundle{
@@ -98,17 +98,37 @@ func TestByDeprecation(t *testing.T) {
 		},
 		Deprecations: []declcfg.DeprecationEntry{
 			{
-				Reference: declcfg.PackageScopedReference{},
+				Reference: declcfg.PackageScopedReference{
+					Schema: "olm.bundle",
+					Name:   "baz",
+				},
 			},
 		},
 	}
 
 	toSort := []*catalogmetadata.Bundle{b2, b1}
 	sort.SliceStable(toSort, func(i, j int) bool {
-		return catalogsort.ByDeprecation(toSort[i], toSort[j])
+		return catalogsort.ByDeprecated(toSort[i], toSort[j])
 	})
 
 	require.Len(t, toSort, 2)
 	assert.Equal(t, b1, toSort[0])
 	assert.Equal(t, b2, toSort[1])
+
+	// Channel deprecation association != bundle deprecated
+	b2.Deprecations[0] = declcfg.DeprecationEntry{
+		Reference: declcfg.PackageScopedReference{
+			Schema: "olm.channel",
+			Name:   "badchannel",
+		},
+	}
+
+	toSort = []*catalogmetadata.Bundle{b2, b1}
+	sort.SliceStable(toSort, func(i, j int) bool {
+		return catalogsort.ByDeprecated(toSort[i], toSort[j])
+	})
+	// No bundles are deprecated so ordering should remain the same
+	require.Len(t, toSort, 2)
+	assert.Equal(t, b2, toSort[0])
+	assert.Equal(t, b1, toSort[1])
 }

internal/catalogmetadata/types.go

@@ -155,6 +155,29 @@ func (b *Bundle) HasDeprecation() bool {
 	return len(b.Deprecations) > 0
 }
 
+// IsDeprecated returns true if the bundle
+// has been explicitly deprecated. This can occur
+// in one of two ways:
+// - the olm.package the bundle belongs to has been deprecated
+// - the bundle itself has been deprecated
+// this function does not take into consideration
+// olm.channel deprecations associated with the bundle
+// as a bundle can be present in multiple channels with
+// some channels being deprecated and some not.
+func (b *Bundle) IsDeprecated() bool {
+	for _, dep := range b.Deprecations {
+		if dep.Reference.Schema == declcfg.SchemaPackage && dep.Reference.Name == b.Package {
+			return true
+		}
+
+		if dep.Reference.Schema == declcfg.SchemaBundle && dep.Reference.Name == b.Name {
+			return true
+		}
+	}
+
+	return false
+}
+
 func loadOneFromProps[T any](bundle *Bundle, propType string, required bool) (T, error) {
 	r, err := loadFromProps[T](bundle, propType, required)
 	if err != nil {

internal/controllers/clusterextension_controller.go

@@ -217,7 +217,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 	// existing BundleDeployment object status.
 	mapBDStatusToInstalledCondition(existingTypedBundleDeployment, ext)
 
-	setDeprecationStatus(ext, bundle)
+	SetDeprecationStatus(ext, bundle)
 
 	// set the status of the cluster extension based on the respective bundle deployment status conditions.
 	return ctrl.Result{}, nil
@@ -287,7 +287,7 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph
 
 // setDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension
 // based on the provided bundle
-func setDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) {
+func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) {
 	// reset conditions to false
 	conditionTypes := []string{
 		ocv1alpha1.TypeDeprecated,
@@ -306,7 +306,12 @@ func setDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetad
 		})
 	}
 
-	if !bundle.HasDeprecation() {
+	// Even if the bundle is not deprecated, there may be channel
+	// deprecations associated with this bundle. If there are we need
+	// verify if it matches the channel specified in the ClusterExtension.
+	// if the bundle is not deprecated and there is no channel specified
+	// in the ClusterExtension there is no deprecations to be applied.
+	if (!bundle.IsDeprecated() && !bundle.HasDeprecation()) || (!bundle.IsDeprecated() && ext.Spec.Channel == "") {
 		return
 	}
 
@@ -347,13 +352,15 @@ func setDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetad
 		deprecationMessages = append(deprecationMessages, deprecation.Message)
 	}
 
-	apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
-		Type:               ocv1alpha1.TypeDeprecated,
-		Reason:             ocv1alpha1.ReasonDeprecated,
-		Status:             metav1.ConditionTrue,
-		Message:            strings.Join(deprecationMessages, ";"),
-		ObservedGeneration: ext.Generation,
-	})
+	if len(deprecationMessages) > 0 {
+		apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+			Type:               ocv1alpha1.TypeDeprecated,
+			Reason:             ocv1alpha1.ReasonDeprecated,
+			Status:             metav1.ConditionTrue,
+			Message:            strings.Join(deprecationMessages, ";"),
+			ObservedGeneration: ext.Generation,
+		})
+	}
 }
 
 func (r *ClusterExtensionReconciler) bundleFromSolution(selection []deppy.Variable, packageName string) (*catalogmetadata.Bundle, error) {