From 6b43b23bdf03f8e2f8e60f95f7a327c275e04c80 Mon Sep 17 00:00:00 2001 From: everettraven Date: Mon, 8 Jan 2024 15:16:31 -0500 Subject: [PATCH] wip: (feat): add graph deprecation logic to signal when an installed bundle has deprecations associated with it and prefer bundles with deprecations less than non-deprecated bundles Signed-off-by: everettraven --- api/v1alpha1/clusterextension_types.go | 12 ++++ cmd/resolutioncli/client.go | 13 +++- internal/catalogmetadata/client/client.go | 37 ++++++++++- .../catalogmetadata/client/client_test.go | 39 +++++++++++ .../filter/bundle_predicates.go | 6 ++ .../filter/bundle_predicates_test.go | 16 +++++ internal/catalogmetadata/sort/sort.go | 19 ++++++ internal/catalogmetadata/sort/sort_test.go | 31 +++++++++ internal/catalogmetadata/types.go | 15 ++++- internal/catalogmetadata/types_test.go | 28 ++++++++ .../clusterextension_controller.go | 65 +++++++++++++++++++ .../variablesources/required_package.go | 3 + .../variablesources/required_package_test.go | 20 ++++++ 13 files changed, 296 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index 783e0c182..c617324dd 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/api/v1alpha1/clusterextension_types.go @@ -71,6 +71,12 @@ const ( // TODO(user): add more Types, here and into init() TypeInstalled = "Installed" TypeResolved = "Resolved" + // TypeDeprecated is a rollup condition that is present when + // any of the deprecated conditions are present. + TypeDeprecated = "Deprecated" + TypePackageDeprecated = "PackageDeprecated" + TypeChannelDeprecated = "ChannelDeprecated" + TypeBundleDeprecated = "BundleDeprecated" ReasonBundleLookupFailed = "BundleLookupFailed" ReasonInstallationFailed = "InstallationFailed" @@ -80,6 +86,7 @@ const ( ReasonResolutionFailed = "ResolutionFailed" ReasonResolutionUnknown = "ResolutionUnknown" ReasonSuccess = "Success" + ReasonDeprecated = "Deprecated" ) func init() { @@ -87,6 +94,10 @@ func init() { conditionsets.ConditionTypes = append(conditionsets.ConditionTypes, TypeInstalled, TypeResolved, + TypeDeprecated, + TypePackageDeprecated, + TypeChannelDeprecated, + TypeBundleDeprecated, ) // TODO(user): add Reasons from above conditionsets.ConditionReasons = append(conditionsets.ConditionReasons, @@ -98,6 +109,7 @@ func init() { ReasonInstallationStatusUnknown, ReasonInvalidSpec, ReasonSuccess, + ReasonDeprecated, ) } diff --git a/cmd/resolutioncli/client.go b/cmd/resolutioncli/client.go index b9b931cf7..0d36a8909 100644 --- a/cmd/resolutioncli/client.go +++ b/cmd/resolutioncli/client.go @@ -47,8 +47,9 @@ func (c *indexRefClient) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle } var ( - channels []*catalogmetadata.Channel - bundles []*catalogmetadata.Bundle + channels []*catalogmetadata.Channel + bundles []*catalogmetadata.Bundle + deprecations []*catalogmetadata.Deprecation ) for i := range cfg.Channels { @@ -63,10 +64,16 @@ func (c *indexRefClient) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle }) } + for i := range cfg.Deprecations { + deprecations = append(deprecations, &catalogmetadata.Deprecation{ + Deprecation: cfg.Deprecations[i], + }) + } + // TODO: update fake catalog name string to be catalog name once we support multiple catalogs in CLI catalogName := "offline-catalog" - bundles, err = client.PopulateExtraFields(catalogName, channels, bundles) + bundles, err = client.PopulateExtraFields(catalogName, channels, bundles, deprecations) if err != nil { return nil, err } diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go index 80d879824..af6506990 100644 --- a/internal/catalogmetadata/client/client.go +++ b/internal/catalogmetadata/client/client.go @@ -57,6 +57,7 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) } channels := []*catalogmetadata.Channel{} bundles := []*catalogmetadata.Bundle{} + deprecations := []*catalogmetadata.Deprecation{} rc, err := c.fetcher.FetchCatalogContents(ctx, catalog.DeepCopy()) if err != nil { @@ -81,6 +82,12 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) return fmt.Errorf("error unmarshalling bundle from catalog metadata: %s", err) } bundles = append(bundles, &content) + case declcfg.SchemaDeprecation: + var content catalogmetadata.Deprecation + if err := json.Unmarshal(meta.Blob, &content); err != nil { + return fmt.Errorf("error unmarshalling deprecation from catalog metadata: %s", err) + } + deprecations = append(deprecations, &content) } return nil @@ -89,7 +96,7 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) return nil, fmt.Errorf("error processing response: %s", err) } - bundles, err = PopulateExtraFields(catalog.Name, channels, bundles) + bundles, err = PopulateExtraFields(catalog.Name, channels, bundles, deprecations) if err != nil { return nil, err } @@ -100,7 +107,7 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) return allBundles, nil } -func PopulateExtraFields(catalogName string, channels []*catalogmetadata.Channel, bundles []*catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { +func PopulateExtraFields(catalogName string, channels []*catalogmetadata.Channel, bundles []*catalogmetadata.Bundle, deprecations []*catalogmetadata.Deprecation) ([]*catalogmetadata.Bundle, error) { bundlesMap := map[string]*catalogmetadata.Bundle{} for i := range bundles { bundleKey := fmt.Sprintf("%s-%s", bundles[i].Package, bundles[i].Name) @@ -121,5 +128,31 @@ func PopulateExtraFields(catalogName string, channels []*catalogmetadata.Channel } } + // if package is deprecated, add deprecation to bundle + // if channel is deprecated, add deprecation to bundle + // if bundle is deprecated, add deprecation to bundle + for i := range bundles { + for _, dep := range deprecations { + if bundles[i].Package == dep.Package { + for _, entry := range dep.Entries { + switch entry.Reference.Schema { + case declcfg.SchemaPackage: + bundles[i].Deprecations = append(bundles[i].Deprecations, entry) + case declcfg.SchemaChannel: + for _, ch := range bundles[i].InChannels { + if ch.Name == entry.Reference.Name { + bundles[i].Deprecations = append(bundles[i].Deprecations, entry) + } + } + case declcfg.SchemaBundle: + if bundles[i].Name == entry.Reference.Name { + bundles[i].Deprecations = append(bundles[i].Deprecations, entry) + } + } + } + } + } + } + return bundles, nil } diff --git a/internal/catalogmetadata/client/client_test.go b/internal/catalogmetadata/client/client_test.go index 89ddcfac9..9bcc54850 100644 --- a/internal/catalogmetadata/client/client_test.go +++ b/internal/catalogmetadata/client/client_test.go @@ -126,6 +126,45 @@ func TestClient(t *testing.T) { }, fetcher: &MockFetcher{}, }, + { + name: "deprecated at the package, channel, and bundle level", + fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte) { + objs, bundles, catalogContentMap := defaultFakeCatalog() + + catalogContentMap["catalog-1"] = append(catalogContentMap["catalog-1"], + []byte(`{"schema": "olm.deprecations", "package":"fake1", "entries":[{"message": "fake1 is deprecated", "reference": {"schema": "olm.package"}}, {"message":"channel stable is deprecated", "reference": {"schema": "olm.channel", "name": "stable"}}, {"message": "bundle fake1.v1.0.0 is deprecated", "reference":{"schema":"olm.bundle", "name":"fake1.v1.0.0"}}]}`)...) + + for i := range bundles { + if bundles[i].Package == "fake1" && bundles[i].CatalogName == "catalog-1" && bundles[i].Name == "fake1.v1.0.0" { + bundles[i].Deprecations = append(bundles[i].Deprecations, declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: "olm.package", + }, + Message: "fake1 is deprecated", + }) + + bundles[i].Deprecations = append(bundles[i].Deprecations, declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: "olm.channel", + Name: "stable", + }, + Message: "channel stable is deprecated", + }) + + bundles[i].Deprecations = append(bundles[i].Deprecations, declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: "olm.bundle", + Name: "fake1.v1.0.0", + }, + Message: "bundle fake1.v1.0.0 is deprecated", + }) + } + } + + return objs, bundles, catalogContentMap + }, + fetcher: &MockFetcher{}, + }, } { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index e62df9187..1d5761a95 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -70,3 +70,9 @@ func Replaces(bundleName string) Predicate[catalogmetadata.Bundle] { return false } } + +func WithDeprecation(deprecated bool) Predicate[catalogmetadata.Bundle] { + return func(bundle *catalogmetadata.Bundle) bool { + return bundle.HasDeprecation() == deprecated + } +} diff --git a/internal/catalogmetadata/filter/bundle_predicates_test.go b/internal/catalogmetadata/filter/bundle_predicates_test.go index 95e9482f0..3617f47ce 100644 --- a/internal/catalogmetadata/filter/bundle_predicates_test.go +++ b/internal/catalogmetadata/filter/bundle_predicates_test.go @@ -158,3 +158,19 @@ func TestReplaces(t *testing.T) { assert.False(t, f(b2)) assert.False(t, f(b3)) } + +func TestWithDeprecation(t *testing.T) { + b1 := &catalogmetadata.Bundle{ + Deprecations: []declcfg.DeprecationEntry{ + { + Reference: declcfg.PackageScopedReference{}, + }, + }, + } + + b2 := &catalogmetadata.Bundle{} + + f := filter.WithDeprecation(true) + assert.True(t, f(b1)) + assert.False(t, f(b2)) +} diff --git a/internal/catalogmetadata/sort/sort.go b/internal/catalogmetadata/sort/sort.go index 7b220b7db..fd94887b5 100644 --- a/internal/catalogmetadata/sort/sort.go +++ b/internal/catalogmetadata/sort/sort.go @@ -18,6 +18,25 @@ func ByVersion(b1, b2 *catalogmetadata.Bundle) bool { return ver1.GT(*ver2) } +// ByDeprecation is a sort "less" function that orders bundles +// with deprecations lower than ones without deprecations +func ByDeprecation(b1, b2 *catalogmetadata.Bundle) bool { + b1Val := 1 + b2Val := 1 + + if b1.HasDeprecation() { + b1Val = b1Val - 1 + } + + if b2.HasDeprecation() { + b2Val = b2Val - 1 + } + + // Check for "greater than" because we + // non deprecated on top + return b1Val > b2Val +} + // compareErrors returns 0 if both errors are either nil or not nil // -1 if err1 is nil and err2 is not nil // +1 if err1 is not nil and err2 is nil diff --git a/internal/catalogmetadata/sort/sort_test.go b/internal/catalogmetadata/sort/sort_test.go index b18ce616f..ec9f88026 100644 --- a/internal/catalogmetadata/sort/sort_test.go +++ b/internal/catalogmetadata/sort/sort_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" @@ -81,3 +82,33 @@ func TestByVersion(t *testing.T) { assert.Equal(t, b5empty, toSort[4]) }) } + +func TestByDeprecation(t *testing.T) { + b1 := &catalogmetadata.Bundle{ + CatalogName: "foo", + Bundle: declcfg.Bundle{ + Name: "bar", + }, + } + + b2 := &catalogmetadata.Bundle{ + CatalogName: "foo", + Bundle: declcfg.Bundle{ + Name: "baz", + }, + Deprecations: []declcfg.DeprecationEntry{ + { + Reference: declcfg.PackageScopedReference{}, + }, + }, + } + + toSort := []*catalogmetadata.Bundle{b2, b1} + sort.SliceStable(toSort, func(i, j int) bool { + return catalogsort.ByDeprecation(toSort[i], toSort[j]) + }) + + 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 4eb72bcf4..8f376737a 100644 --- a/internal/catalogmetadata/types.go +++ b/internal/catalogmetadata/types.go @@ -18,7 +18,7 @@ const ( ) type Schemas interface { - Package | Bundle | Channel + Package | Bundle | Channel | Deprecation } type Package struct { @@ -29,6 +29,10 @@ type Channel struct { declcfg.Channel } +type Deprecation struct { + declcfg.Deprecation +} + type PackageRequired struct { property.PackageRequired SemverRange bsemver.Range `json:"-"` @@ -36,8 +40,9 @@ type PackageRequired struct { type Bundle struct { declcfg.Bundle - CatalogName string - InChannels []*Channel + CatalogName string + InChannels []*Channel + Deprecations []declcfg.DeprecationEntry mu sync.RWMutex // these properties are lazy loaded as they are requested @@ -140,6 +145,10 @@ func (b *Bundle) propertiesByType(propType string) []*property.Property { return b.propertiesMap[propType] } +func (b *Bundle) HasDeprecation() bool { + return len(b.Deprecations) > 0 +} + func loadOneFromProps[T any](bundle *Bundle, propType string, required bool) (T, error) { r, err := loadFromProps[T](bundle, propType, required) if err != nil { diff --git a/internal/catalogmetadata/types_test.go b/internal/catalogmetadata/types_test.go index 8ecb92508..a148ed738 100644 --- a/internal/catalogmetadata/types_test.go +++ b/internal/catalogmetadata/types_test.go @@ -201,3 +201,31 @@ func TestBundleMediaType(t *testing.T) { }) } } + +func TestBundleHasDeprecation(t *testing.T) { + for _, tt := range []struct { + name string + bundle *catalogmetadata.Bundle + deprecated bool + }{ + { + name: "has deprecation entries", + bundle: &catalogmetadata.Bundle{ + Deprecations: []declcfg.DeprecationEntry{ + { + Reference: declcfg.PackageScopedReference{}, + }, + }, + }, + deprecated: true, + }, + { + name: "has no deprecation entries", + bundle: &catalogmetadata.Bundle{}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.deprecated, tt.bundle.HasDeprecation()) + }) + } +} diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 56c256e3d..2eed22e8c 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -19,13 +19,16 @@ package controllers import ( "context" "fmt" + "strings" "github.com/go-logr/logr" catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/solver" + "github.com/operator-framework/operator-registry/alpha/declcfg" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/meta" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -39,6 +42,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/operator-framework/operator-controller/api/v1alpha1" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/controllers/validators" @@ -201,6 +205,8 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp // existing BundleDeployment object status. mapBDStatusToInstalledCondition(existingTypedBundleDeployment, ext) + setDeprecationStatus(ext, bundle) + // set the status of the cluster extension based on the respective bundle deployment status conditions. return ctrl.Result{}, nil } @@ -267,6 +273,65 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph } } +// setDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension +// based on the provided bundle +func setDeprecationStatus(ext *v1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) { + // unset conditions + meta.RemoveStatusCondition(&ext.Status.Conditions, v1alpha1.TypePackageDeprecated) + meta.RemoveStatusCondition(&ext.Status.Conditions, v1alpha1.TypeChannelDeprecated) + meta.RemoveStatusCondition(&ext.Status.Conditions, v1alpha1.TypeBundleDeprecated) + meta.RemoveStatusCondition(&ext.Status.Conditions, v1alpha1.TypeDeprecated) + + if !bundle.HasDeprecation() { + return + } + + deprecationMessages := []string{} + + for _, deprecation := range bundle.Deprecations { + switch deprecation.Reference.Schema { + case declcfg.SchemaPackage: + meta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: v1alpha1.TypePackageDeprecated, + Reason: v1alpha1.ReasonDeprecated, + Status: metav1.ConditionTrue, + Message: deprecation.Message, + ObservedGeneration: ext.Generation, + }) + case declcfg.SchemaChannel: + if ext.Spec.Channel != deprecation.Reference.Name { + continue + } + + meta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: v1alpha1.TypeChannelDeprecated, + Reason: v1alpha1.ReasonDeprecated, + Status: metav1.ConditionTrue, + Message: deprecation.Message, + ObservedGeneration: ext.Generation, + }) + case declcfg.SchemaBundle: + meta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: v1alpha1.TypeBundleDeprecated, + Reason: v1alpha1.ReasonDeprecated, + Status: metav1.ConditionTrue, + Message: deprecation.Message, + ObservedGeneration: ext.Generation, + }) + } + + deprecationMessages = append(deprecationMessages, deprecation.Message) + } + + meta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: v1alpha1.TypeDeprecated, + Reason: v1alpha1.ReasonDeprecated, + Status: metav1.ConditionTrue, + Message: strings.Join(deprecationMessages, ";"), + ObservedGeneration: ext.Generation, + }) +} + func (r *ClusterExtensionReconciler) bundleFromSolution(selection []deppy.Variable, packageName string) (*catalogmetadata.Bundle, error) { for _, variable := range selection { switch v := variable.(type) { diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index a06f50062..2a611f771 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -56,6 +56,9 @@ func MakeRequiredPackageVariables(allBundles []*catalogmetadata.Bundle, clusterE sort.SliceStable(resultSet, func(i, j int) bool { return catalogsort.ByVersion(resultSet[i], resultSet[j]) }) + sort.SliceStable(resultSet, func(i, j int) bool { + return catalogsort.ByDeprecation(resultSet[i], resultSet[j]) + }) result = append(result, olmvariables.NewRequiredPackageVariable(packageName, resultSet)) } diff --git a/internal/resolution/variablesources/required_package_test.go b/internal/resolution/variablesources/required_package_test.go index b510b5be3..bb9be0d89 100644 --- a/internal/resolution/variablesources/required_package_test.go +++ b/internal/resolution/variablesources/required_package_test.go @@ -63,6 +63,25 @@ func TestMakeRequiredPackageVariables(t *testing.T) { }, InChannels: []*catalogmetadata.Channel{&stableChannel}, }, + "test-package.v4.0.0": { + Bundle: declcfg.Bundle{ + Name: "test-package.v4.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "4.0.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&stableChannel}, + Deprecations: []declcfg.DeprecationEntry{ + { + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaBundle, + Name: "test-package.v4.0.0", + }, + Message: "test-package.v4.0.0 has been deprecated", + }, + }, + }, // We need at least one bundle from different package // to make sure that we are filtering it out. @@ -111,6 +130,7 @@ func TestMakeRequiredPackageVariables(t *testing.T) { bundleSet["test-package.v3.0.0"], bundleSet["test-package.v2.0.0"], bundleSet["test-package.v1.0.0"], + bundleSet["test-package.v4.0.0"], }), }, },