From 3c2c46a04e21e77665bfd23aa825faf199119da8 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Wed, 17 Jan 2024 15:19:11 -0500 Subject: [PATCH] :seedling: don't consider bundle deprecated if the package is deprecated (#577) * (fix): don't consider bundle deprecated if the package is deprecated Signed-off-by: everettraven * update IsDeprecated() comment Signed-off-by: everettraven --------- Signed-off-by: everettraven --- internal/catalogmetadata/sort/sort_test.go | 28 ++++++++++ internal/catalogmetadata/types.go | 16 +++--- internal/catalogmetadata/types_test.go | 52 +++++++++++++++++++ .../clusterextension_controller.go | 4 +- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/internal/catalogmetadata/sort/sort_test.go b/internal/catalogmetadata/sort/sort_test.go index 9d1e2e275..e0c73c9ca 100644 --- a/internal/catalogmetadata/sort/sort_test.go +++ b/internal/catalogmetadata/sort/sort_test.go @@ -131,4 +131,32 @@ func TestByDeprecated(t *testing.T) { require.Len(t, toSort, 2) assert.Equal(t, b2, toSort[0]) assert.Equal(t, b1, toSort[1]) + + b1.Deprecations = []declcfg.DeprecationEntry{ + { + Reference: declcfg.PackageScopedReference{ + Schema: "olm.package", + }, + }, + } + b2.Deprecations = append(b2.Deprecations, declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: "olm.package", + }, + }, declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: "olm.bundle", + Name: "baz", + }, + }) + + toSort = []*catalogmetadata.Bundle{b2, b1} + sort.SliceStable(toSort, func(i, j int) bool { + return catalogsort.ByDeprecated(toSort[i], toSort[j]) + }) + // Both are deprecated at package level, b2 is deprecated + // explicitly, b2 should be preferred less + require.Len(t, toSort, 2) + assert.Equal(t, b1, toSort[0]) + assert.Equal(t, b2, toSort[1]) } diff --git a/internal/catalogmetadata/types.go b/internal/catalogmetadata/types.go index bcbdd3390..e1cb8e2e8 100644 --- a/internal/catalogmetadata/types.go +++ b/internal/catalogmetadata/types.go @@ -157,19 +157,15 @@ func (b *Bundle) HasDeprecation() bool { // 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 +// if 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. +// olm.channel or olm.package deprecations associated +// with the bundle as a bundle can be present in multiple +// channels with some channels being deprecated and some not +// Package deprecation does not carry the same meaning as an individual +// bundle deprecation, so package deprecation is not considered. 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 } diff --git a/internal/catalogmetadata/types_test.go b/internal/catalogmetadata/types_test.go index a148ed738..3129ecde3 100644 --- a/internal/catalogmetadata/types_test.go +++ b/internal/catalogmetadata/types_test.go @@ -229,3 +229,55 @@ func TestBundleHasDeprecation(t *testing.T) { }) } } + +func TestBundleIsDeprecated(t *testing.T) { + for _, tt := range []struct { + name string + bundle *catalogmetadata.Bundle + deprecated bool + }{ + { + name: "has package and channel deprecations, not deprecated", + bundle: &catalogmetadata.Bundle{ + Deprecations: []declcfg.DeprecationEntry{ + { + Reference: declcfg.PackageScopedReference{ + Schema: "olm.package", + }, + }, + { + Reference: declcfg.PackageScopedReference{ + Schema: "olm.channel", + Name: "foo", + }, + }, + }, + }, + }, + { + name: "has no deprecation entries, not deprecated", + bundle: &catalogmetadata.Bundle{}, + }, + { + name: "has bundle deprecation entry, deprecated", + bundle: &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "foo", + }, + Deprecations: []declcfg.DeprecationEntry{ + { + Reference: declcfg.PackageScopedReference{ + Schema: "olm.bundle", + Name: "foo", + }, + }, + }, + }, + deprecated: true, + }, + } { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.deprecated, tt.bundle.IsDeprecated()) + }) + } +} diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 0a0ac5233..70f25fa42 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -307,10 +307,10 @@ func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetad } // There are two early return scenarios here: - // 1) The bundle is not deprecated (i.e no package or bundle deprecations) + // 1) The bundle is not deprecated (i.e bundle deprecations) // AND there are no other deprecations associated with the bundle // 2) The bundle is not deprecated, there are deprecations associated - // with the bundle (i.e at least one channel the bundle is present in is deprecated), + // with the bundle (i.e at least one channel the bundle is present in is deprecated OR whole package is deprecated), // and the ClusterExtension does not specify a channel. This is because the channel deprecations // are a loose deprecation coupling on the bundle. A ClusterExtension installation is only // considered deprecated by a channel deprecation when a deprecated channel is specified via