From b59ac9e63dc97ce173f873bae14fd69573b54c08 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 27 Jun 2024 16:13:21 -0400 Subject: [PATCH] remove catalogmetadata types, introduce to resolver interface Signed-off-by: Joe Lanford --- cmd/manager/main.go | 58 +- internal/bundleutil/bundle.go | 38 + internal/catalogmetadata/cache/cache_test.go | 24 +- internal/catalogmetadata/client/client.go | 177 +-- .../catalogmetadata/client/client_test.go | 380 ++----- internal/catalogmetadata/compare/compare.go | 57 + .../catalogmetadata/compare/compare_test.go | 89 ++ .../filter/bundle_predicates.go | 84 +- .../filter/bundle_predicates_test.go | 184 +-- internal/catalogmetadata/filter/filter.go | 26 +- .../catalogmetadata/filter/filter_test.go | 49 +- internal/catalogmetadata/filter/successors.go | 106 ++ .../catalogmetadata/filter/successors_test.go | 390 +++++++ internal/catalogmetadata/sort/sort.go | 52 - internal/catalogmetadata/sort/sort_test.go | 162 --- internal/catalogmetadata/types.go | 181 --- internal/catalogmetadata/types_test.go | 227 ---- .../clusterextension_controller.go | 298 ++--- .../clusterextension_controller_test.go | 1013 +---------------- ...terextension_registryv1_validation_test.go | 87 +- internal/controllers/common_controller.go | 9 - internal/controllers/successors.go | 80 -- internal/controllers/successors_test.go | 438 ------- internal/controllers/suite_test.go | 16 +- internal/resolve/catalog.go | 168 +++ internal/resolve/catalog_test.go | 569 +++++++++ internal/resolve/resolver.go | 21 + internal/testutil/fake_catalog_client.go | 38 - test/e2e/cluster_extension_install_test.go | 2 +- test/upgrade-e2e/post_upgrade_test.go | 4 +- 30 files changed, 1843 insertions(+), 3184 deletions(-) create mode 100644 internal/bundleutil/bundle.go create mode 100644 internal/catalogmetadata/compare/compare.go create mode 100644 internal/catalogmetadata/compare/compare_test.go create mode 100644 internal/catalogmetadata/filter/successors.go create mode 100644 internal/catalogmetadata/filter/successors_test.go delete mode 100644 internal/catalogmetadata/sort/sort.go delete mode 100644 internal/catalogmetadata/sort/sort_test.go delete mode 100644 internal/catalogmetadata/types.go delete mode 100644 internal/catalogmetadata/types_test.go delete mode 100644 internal/controllers/successors.go delete mode 100644 internal/controllers/successors_test.go create mode 100644 internal/resolve/catalog.go create mode 100644 internal/resolve/catalog_test.go create mode 100644 internal/resolve/resolver.go delete mode 100644 internal/testutil/fake_catalog_client.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index e09d65406..a7d4bcda7 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -47,7 +47,8 @@ import ( "github.com/operator-framework/operator-controller/internal/controllers" "github.com/operator-framework/operator-controller/internal/httputil" "github.com/operator-framework/operator-controller/internal/labels" - crdupgradesafety "github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety" + "github.com/operator-framework/operator-controller/internal/resolve" + "github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety" "github.com/operator-framework/operator-controller/internal/rukpak/source" "github.com/operator-framework/operator-controller/internal/version" "github.com/operator-framework/operator-controller/pkg/features" @@ -153,25 +154,6 @@ func main() { os.Exit(1) } - certPool, err := httputil.NewCertPool(caCertDir) - if err != nil { - setupLog.Error(err, "unable to create CA certificate pool") - os.Exit(1) - } - - httpClient, err := httputil.BuildHTTPClient(certPool) - if err != nil { - setupLog.Error(err, "unable to create catalogd http client") - } - - cl := mgr.GetClient() - catalogsCachePath := filepath.Join(cachePath, "catalogs") - if err := os.MkdirAll(catalogsCachePath, 0700); err != nil { - setupLog.Error(err, "unable to create catalogs cache directory") - os.Exit(1) - } - catalogClient := catalogclient.New(cl, cache.NewFilesystemCache(catalogsCachePath, httpClient)) - installNamespaceMapper := helmclient.ObjectToStringMapper(func(obj client.Object) (string, error) { ext := obj.(*ocv1alpha1.ClusterExtension) return ext.Spec.InstallNamespace, nil @@ -194,7 +176,11 @@ func main() { os.Exit(1) } - clusterExtensionFinalizers := crfinalizer.NewFinalizers() + certPool, err := httputil.NewCertPool(caCertDir) + if err != nil { + setupLog.Error(err, "unable to create CA certificate pool") + os.Exit(1) + } unpacker := &source.ImageRegistry{ BaseCachePath: filepath.Join(cachePath, "unpack"), // TODO: This needs to be derived per extension via ext.Spec.InstallNamespace @@ -202,6 +188,7 @@ func main() { CaCertPool: certPool, } + clusterExtensionFinalizers := crfinalizer.NewFinalizers() domain := ocv1alpha1.GroupVersion.Group cleanupUnpackCacheKey := fmt.Sprintf("%s/cleanup-unpack-cache", domain) if err := clusterExtensionFinalizers.Register(cleanupUnpackCacheKey, finalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { @@ -222,9 +209,36 @@ func main() { crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()), } + cl := mgr.GetClient() + httpClient, err := httputil.BuildHTTPClient(certPool) + if err != nil { + setupLog.Error(err, "unable to create catalogd http client") + os.Exit(1) + } + + catalogsCachePath := filepath.Join(cachePath, "catalogs") + if err := os.MkdirAll(catalogsCachePath, 0700); err != nil { + setupLog.Error(err, "unable to create catalogs cache directory") + os.Exit(1) + } + catalogClient := catalogclient.New(cache.NewFilesystemCache(catalogsCachePath, httpClient)) + + resolver := &resolve.CatalogResolver{ + WalkCatalogsFunc: resolve.CatalogWalker( + func(ctx context.Context, option ...client.ListOption) ([]catalogd.ClusterCatalog, error) { + var catalogs catalogd.ClusterCatalogList + if err := cl.List(ctx, &catalogs, option...); err != nil { + return nil, err + } + return catalogs.Items, nil + }, + catalogClient.GetPackage, + ), + } + if err = (&controllers.ClusterExtensionReconciler{ Client: cl, - BundleProvider: catalogClient, + Resolver: resolver, ActionClientGetter: acg, Unpacker: unpacker, InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg}, diff --git a/internal/bundleutil/bundle.go b/internal/bundleutil/bundle.go new file mode 100644 index 000000000..1bfd0c063 --- /dev/null +++ b/internal/bundleutil/bundle.go @@ -0,0 +1,38 @@ +package bundleutil + +import ( + "encoding/json" + "fmt" + + bsemver "github.com/blang/semver/v4" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" +) + +func GetVersion(b declcfg.Bundle) (*bsemver.Version, error) { + for _, p := range b.Properties { + if p.Type == property.TypePackage { + var pkg property.Package + if err := json.Unmarshal(p.Value, &pkg); err != nil { + return nil, fmt.Errorf("error unmarshalling package property: %w", err) + } + vers, err := bsemver.Parse(pkg.Version) + if err != nil { + return nil, err + } + return &vers, nil + } + } + return nil, fmt.Errorf("no package property found in bundle %q", b.Name) +} + +// MetadataFor returns a BundleMetadata for the given bundle name and version. +func MetadataFor(bundleName string, bundleVersion bsemver.Version) *ocv1alpha1.BundleMetadata { + return &ocv1alpha1.BundleMetadata{ + Name: bundleName, + Version: bundleVersion.String(), + } +} diff --git a/internal/catalogmetadata/cache/cache_test.go b/internal/catalogmetadata/cache/cache_test.go index 4b24d90cb..168b32581 100644 --- a/internal/catalogmetadata/cache/cache_test.go +++ b/internal/catalogmetadata/cache/cache_test.go @@ -68,7 +68,7 @@ func TestFilesystemCache(t *testing.T) { catalog *catalogd.ClusterCatalog contents fstest.MapFS wantErr bool - tripper *MockTripper + tripper *mockTripper testCaching bool shouldHitCache bool } @@ -89,7 +89,7 @@ func TestFilesystemCache(t *testing.T) { }, }, contents: defaultFS, - tripper: &MockTripper{}, + tripper: &mockTripper{}, }, { name: "valid cached fetch", @@ -107,7 +107,7 @@ func TestFilesystemCache(t *testing.T) { }, }, contents: defaultFS, - tripper: &MockTripper{}, + tripper: &mockTripper{}, testCaching: true, shouldHitCache: true, }, @@ -127,7 +127,7 @@ func TestFilesystemCache(t *testing.T) { }, }, contents: defaultFS, - tripper: &MockTripper{}, + tripper: &mockTripper{}, testCaching: true, shouldHitCache: false, }, @@ -147,7 +147,7 @@ func TestFilesystemCache(t *testing.T) { }, }, contents: defaultFS, - tripper: &MockTripper{shouldError: true}, + tripper: &mockTripper{shouldError: true}, wantErr: true, }, { @@ -166,14 +166,14 @@ func TestFilesystemCache(t *testing.T) { }, }, contents: defaultFS, - tripper: &MockTripper{serverError: true}, + tripper: &mockTripper{serverError: true}, wantErr: true, }, { name: "nil catalog", catalog: nil, contents: defaultFS, - tripper: &MockTripper{serverError: true}, + tripper: &mockTripper{serverError: true}, wantErr: true, }, { @@ -187,7 +187,7 @@ func TestFilesystemCache(t *testing.T) { }, }, contents: defaultFS, - tripper: &MockTripper{serverError: true}, + tripper: &mockTripper{serverError: true}, wantErr: true, }, { @@ -203,7 +203,7 @@ func TestFilesystemCache(t *testing.T) { }, }, contents: defaultFS, - tripper: &MockTripper{serverError: true}, + tripper: &mockTripper{serverError: true}, wantErr: true, }, } { @@ -242,15 +242,15 @@ func TestFilesystemCache(t *testing.T) { } } -var _ http.RoundTripper = &MockTripper{} +var _ http.RoundTripper = &mockTripper{} -type MockTripper struct { +type mockTripper struct { content fstest.MapFS shouldError bool serverError bool } -func (mt *MockTripper) RoundTrip(_ *http.Request) (*http.Response, error) { +func (mt *mockTripper) RoundTrip(_ *http.Request) (*http.Response, error) { if mt.shouldError { return nil, errors.New("mock tripper error") } diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go index a3e7339ca..7f352ea88 100644 --- a/internal/catalogmetadata/client/client.go +++ b/internal/catalogmetadata/client/client.go @@ -1,23 +1,16 @@ package client import ( - "cmp" "context" - "encoding/json" "errors" "fmt" "io/fs" - "slices" - "sync" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" "github.com/operator-framework/operator-registry/alpha/declcfg" - - "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) // Fetcher is an interface to facilitate fetching @@ -30,174 +23,44 @@ type Fetcher interface { FetchCatalogContents(ctx context.Context, catalog *catalogd.ClusterCatalog) (fs.FS, error) } -func New(cl client.Client, fetcher Fetcher) *Client { +func New(fetcher Fetcher) *Client { return &Client{ - cl: cl, fetcher: fetcher, } } // Client is reading catalog metadata type Client struct { - // Note that eventually we will be reading from catalogd http API - // instead of kube API server. We will need to swap this implementation. - cl client.Client - // fetcher is the Fetcher to use for fetching catalog contents fetcher Fetcher } -func (c *Client) Bundles(ctx context.Context, packageName string) ([]*catalogmetadata.Bundle, error) { - var allBundles []*catalogmetadata.Bundle - - var catalogList catalogd.ClusterCatalogList - if err := c.cl.List(ctx, &catalogList); err != nil { - return nil, err - } - - var errs []error - for _, catalog := range catalogList.Items { - // if the catalog has not been successfully unpacked, report an error. This ensures that our - // reconciles are deterministic and wait for all desired catalogs to be ready. - if !meta.IsStatusConditionPresentAndEqual(catalog.Status.Conditions, catalogd.TypeUnpacked, metav1.ConditionTrue) { - errs = append(errs, fmt.Errorf("catalog %q is not unpacked", catalog.Name)) - continue - } - channels := []*catalogmetadata.Channel{} - bundles := []*catalogmetadata.Bundle{} - deprecations := []*catalogmetadata.Deprecation{} - - catalogFS, err := c.fetcher.FetchCatalogContents(ctx, catalog.DeepCopy()) - if err != nil { - errs = append(errs, fmt.Errorf("error fetching catalog %q contents: %v", catalog.Name, err)) - continue - } - - packageFS, err := fs.Sub(catalogFS, packageName) - if err != nil { - errs = append(errs, fmt.Errorf("error reading package subdirectory %q from catalog %q filesystem: %v", packageName, catalog.Name, err)) - continue - } - - var ( - channelsMu sync.Mutex - bundlesMu sync.Mutex - deprecationsMu sync.Mutex - ) - - if err := declcfg.WalkMetasFS(ctx, packageFS, func(_ string, meta *declcfg.Meta, err error) error { - if err != nil { - return fmt.Errorf("error parsing package metadata: %v", err) - } - switch meta.Schema { - case declcfg.SchemaChannel: - var content catalogmetadata.Channel - if err := json.Unmarshal(meta.Blob, &content); err != nil { - return fmt.Errorf("error unmarshalling channel from catalog metadata: %v", err) - } - channelsMu.Lock() - defer channelsMu.Unlock() - channels = append(channels, &content) - case declcfg.SchemaBundle: - var content catalogmetadata.Bundle - if err := json.Unmarshal(meta.Blob, &content); err != nil { - return fmt.Errorf("error unmarshalling bundle from catalog metadata: %v", err) - } - bundlesMu.Lock() - defer bundlesMu.Unlock() - 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: %v", err) - } - deprecationsMu.Lock() - defer deprecationsMu.Unlock() - deprecations = append(deprecations, &content) - } - return nil - }); err != nil { - if !errors.Is(err, fs.ErrNotExist) { - errs = append(errs, fmt.Errorf("error reading package %q from catalog %q: %v", packageName, catalog.Name, err)) - } - continue - } - - bundles, err = PopulateExtraFields(catalog.Name, channels, bundles, deprecations) - if err != nil { - errs = append(errs, err) - continue - } - - allBundles = append(allBundles, bundles...) - } - if len(errs) > 0 { - return nil, errors.Join(errs...) +func (c *Client) GetPackage(ctx context.Context, catalog *catalogd.ClusterCatalog, pkgName string) (*declcfg.DeclarativeConfig, error) { + // if the catalog has not been successfully unpacked, report an error. This ensures that our + // reconciles are deterministic and wait for all desired catalogs to be ready. + if !meta.IsStatusConditionPresentAndEqual(catalog.Status.Conditions, catalogd.TypeUnpacked, metav1.ConditionTrue) { + return nil, fmt.Errorf("catalog %q is not unpacked", catalog.Name) } - return allBundles, nil -} - -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) - bundlesMap[bundleKey] = bundles[i] - - bundles[i].CatalogName = catalogName + catalogFsys, err := c.fetcher.FetchCatalogContents(ctx, catalog) + if err != nil { + return nil, fmt.Errorf("error fetching catalog contents: %v", err) } - for _, ch := range channels { - for _, chEntry := range ch.Entries { - bundleKey := fmt.Sprintf("%s-%s", ch.Package, chEntry.Name) - bundle, ok := bundlesMap[bundleKey] - if !ok { - return nil, fmt.Errorf("bundle %q not found in catalog %q (package %q, channel %q)", chEntry.Name, catalogName, ch.Package, ch.Name) - } - - bundle.InChannels = append(bundle.InChannels, ch) + pkgFsys, err := fs.Sub(catalogFsys, pkgName) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, fmt.Errorf("error getting package %q: %v", pkgName, err) } + return &declcfg.DeclarativeConfig{}, nil } - // We sort the channels here because the order that channels appear in this list is non-deterministic. - // They are non-deterministic because they are originally read from the cache in a concurrent manner that - // provides no ordering guarantees. - // - // This sort isn't strictly necessary for correctness, but it makes the output consistent and easier to - // reason about. - for _, bundle := range bundles { - slices.SortFunc(bundle.InChannels, func(a, b *catalogmetadata.Channel) int { return cmp.Compare(a.Name, b.Name) }) - } - - // According to https://docs.google.com/document/d/1EzefSzoGZL2ipBt-eCQwqqNwlpOIt7wuwjG6_8ZCi5s/edit?usp=sharing - // the olm.deprecations FBC object is only valid when either 0 or 1 instances exist - // for any given package - deprecationMap := make(map[string]*catalogmetadata.Deprecation, len(deprecations)) - for _, deprecation := range deprecations { - deprecationMap[deprecation.Package] = deprecation - } - - for i := range bundles { - if dep, ok := deprecationMap[bundles[i].Package]; ok { - 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) - break - } - } - case declcfg.SchemaBundle: - if bundles[i].Name == entry.Reference.Name { - bundles[i].Deprecations = append(bundles[i].Deprecations, entry) - } - } - } + pkgFBC, err := declcfg.LoadFS(ctx, pkgFsys) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, fmt.Errorf("error loading package %q: %v", pkgName, err) } + return &declcfg.DeclarativeConfig{}, nil } - - return bundles, nil + return pkgFBC, nil } diff --git a/internal/catalogmetadata/client/client_test.go b/internal/catalogmetadata/client/client_test.go index d2e6dfabb..beacc4512 100644 --- a/internal/catalogmetadata/client/client_test.go +++ b/internal/catalogmetadata/client/client_test.go @@ -2,7 +2,6 @@ package client_test import ( "context" - "encoding/json" "errors" "io/fs" "testing" @@ -10,351 +9,106 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" catalogClient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" - "github.com/operator-framework/operator-controller/pkg/scheme" ) -func TestClient(t *testing.T) { - for _, tt := range []struct { - name string - fakeCatalog func() ([]client.Object, []*catalogmetadata.Bundle, map[string]fstest.MapFS) - wantErr string - fetcher *MockFetcher - }{ - { - name: "valid catalog", - fakeCatalog: defaultFakeCatalog, - fetcher: &MockFetcher{}, - }, - { - name: "cache error", - fakeCatalog: defaultFakeCatalog, - fetcher: &MockFetcher{shouldError: true}, - wantErr: `error fetching catalog "catalog-1" contents: mock cache error -error fetching catalog "catalog-2" contents: mock cache error`, - }, - { - name: "channel has a ref to a missing bundle", - fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string]fstest.MapFS) { - objs, _, catalogContentMap := defaultFakeCatalog() - - catalogContentMap["catalog-1"]["fake1/olm.channel/bad-channel-entry.json"] = &fstest.MapFile{Data: []byte(`{ - "schema": "olm.channel", - "name": "channel-with-missing-bundle", - "package": "fake1", - "entries": [ - { - "name": "fake1.v9.9.9" - } - ] - }`)} +func TestClientNew(t *testing.T) { + testFS := fstest.MapFS{ + "pkg-present/olm.package/pkg-present.json": &fstest.MapFile{Data: []byte(`{"schema": "olm.package","name": "pkg-present"}`)}, + } - return objs, nil, catalogContentMap - }, - wantErr: `bundle "fake1.v9.9.9" not found in catalog "catalog-1" (package "fake1", channel "channel-with-missing-bundle")`, - fetcher: &MockFetcher{}, - }, + type testCase struct { + name string + catalog *catalogd.ClusterCatalog + pkgName string + fetcher catalogClient.Fetcher + assert func(*testing.T, *declcfg.DeclarativeConfig, error) + } + for _, tc := range []testCase{ { - name: "invalid meta", - fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string]fstest.MapFS) { - objs, _, catalogContentMap := defaultFakeCatalog() - - catalogContentMap["catalog-1"]["fake1/olm.bundle/invalid-meta.json"] = &fstest.MapFile{Data: []byte(`{"schema": "olm.bundle", "package":"fake1", "name":123123123}`)} - - return objs, nil, catalogContentMap + name: "not unpacked", + catalog: &catalogd.ClusterCatalog{ObjectMeta: metav1.ObjectMeta{Name: "catalog-1"}}, + fetcher: fetcherFunc(func(context.Context, *catalogd.ClusterCatalog) (fs.FS, error) { return testFS, nil }), + assert: func(t *testing.T, dc *declcfg.DeclarativeConfig, err error) { + assert.ErrorContains(t, err, `catalog "catalog-1" is not unpacked`) }, - wantErr: `expected value for key "name" to be a string, got %!t(float64=1.23123123e+08): 1.23123123e+08`, - fetcher: &MockFetcher{}, }, { - name: "invalid bundle", - fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string]fstest.MapFS) { - objs, _, catalogContentMap := defaultFakeCatalog() - - catalogContentMap["catalog-1"]["fake1/olm.bundle/invalid-bundle.json"] = &fstest.MapFile{Data: []byte(`{"schema": "olm.bundle", "name":"foo", "package":"fake1", "image":123123123}`)} - - return objs, nil, catalogContentMap + name: "unpacked, fetcher returns error", + catalog: &catalogd.ClusterCatalog{ + Status: catalogd.ClusterCatalogStatus{Conditions: []metav1.Condition{{Type: catalogd.TypeUnpacked, Status: metav1.ConditionTrue}}}, }, - wantErr: "error unmarshalling bundle from catalog metadata: json: cannot unmarshal number into Go struct field Bundle.image of type string", - fetcher: &MockFetcher{}, - }, - { - name: "invalid channel", - fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string]fstest.MapFS) { - objs, _, catalogContentMap := defaultFakeCatalog() - - catalogContentMap["catalog-1"]["fake1/olm.channel/invalid-channel.json"] = &fstest.MapFile{ - Data: []byte(`{"schema": "olm.channel", "name":"foo", "package":"fake1", "entries":[{"name":123123123}]}`), - } - - return objs, nil, catalogContentMap + fetcher: fetcherFunc(func(context.Context, *catalogd.ClusterCatalog) (fs.FS, error) { return nil, errors.New("fetch error") }), + assert: func(t *testing.T, dc *declcfg.DeclarativeConfig, err error) { + assert.ErrorContains(t, err, `error fetching catalog contents: fetch error`) }, - wantErr: "error unmarshalling channel from catalog metadata: json: cannot unmarshal number into Go struct field ChannelEntry.entries.name of type string", - fetcher: &MockFetcher{}, }, { - name: "error on catalog missing Unpacked status condition", - fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string]fstest.MapFS) { - objs, bundles, catalogContentMap := defaultFakeCatalog() - objs = append(objs, &catalogd.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foobar", - }, - }) - catalogContentMap["foobar"] = catalogContentMap["catalog-1"] - - return objs, bundles, catalogContentMap + name: "unpacked, invalid package path", + catalog: &catalogd.ClusterCatalog{ + Status: catalogd.ClusterCatalogStatus{Conditions: []metav1.Condition{{Type: catalogd.TypeUnpacked, Status: metav1.ConditionTrue}}}, + }, + fetcher: fetcherFunc(func(context.Context, *catalogd.ClusterCatalog) (fs.FS, error) { return testFS, nil }), + pkgName: "/", + assert: func(t *testing.T, dc *declcfg.DeclarativeConfig, err error) { + assert.ErrorContains(t, err, `error getting package "/"`) }, - wantErr: `catalog "foobar" is not unpacked`, - fetcher: &MockFetcher{}, }, { - name: "deprecated at the package, channel, and bundle level", - fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string]fstest.MapFS) { - objs, bundles, catalogContentMap := defaultFakeCatalog() - - catalogContentMap["catalog-1"]["fake1/olm.deprecations/olm.deprecations.json"] = &fstest.MapFile{ - Data: []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 + name: "unpacked, package missing", + catalog: &catalogd.ClusterCatalog{ + Status: catalogd.ClusterCatalogStatus{Conditions: []metav1.Condition{{Type: catalogd.TypeUnpacked, Status: metav1.ConditionTrue}}}, }, - fetcher: &MockFetcher{}, - }, - } { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - objs, expectedBundles, catalogContentMap := tt.fakeCatalog() - tt.fetcher.contentMap = catalogContentMap - - fakeCatalogClient := catalogClient.New( - fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(objs...).Build(), - tt.fetcher, - ) - - bundles, err := fakeCatalogClient.Bundles(ctx, "fake1") - if tt.wantErr == "" { + pkgName: "pkg-missing", + fetcher: fetcherFunc(func(context.Context, *catalogd.ClusterCatalog) (fs.FS, error) { return testFS, nil }), + assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) { assert.NoError(t, err) - assert.Equal(t, expectedBundles, bundles) - } else { - assert.ErrorContains(t, err, tt.wantErr) - } - }) - } -} - -func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle, map[string]fstest.MapFS) { - package1 := `{ - "schema": "olm.package", - "name": "fake1" - }` - - bundle1 := `{ - "schema": "olm.bundle", - "name": "fake1.v1.0.0", - "package": "fake1", - "image": "fake-image", - "properties": [ - { - "type": "olm.package", - "value": {"packageName":"fake1","version":"1.0.0"} - } - ] - }` - - stableChannel := `{ - "schema": "olm.channel", - "name": "stable", - "package": "fake1", - "entries": [ - { - "name": "fake1.v1.0.0" - } - ] - }` - - betaChannel := `{ - "schema": "olm.channel", - "name": "beta", - "package": "fake1", - "entries": [ - { - "name": "fake1.v1.0.0" - } - ] - }` - - objs := []client.Object{ - &catalogd.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "catalog-1", - }, - Status: catalogd.ClusterCatalogStatus{ - Conditions: []metav1.Condition{ - { - Type: catalogd.TypeUnpacked, - Status: metav1.ConditionTrue, - Reason: catalogd.ReasonUnpackSuccessful, - }, - }, + assert.Equal(t, &declcfg.DeclarativeConfig{}, fbc) }, }, - &catalogd.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "catalog-2", - }, - Status: catalogd.ClusterCatalogStatus{ - Conditions: []metav1.Condition{ - { - Type: catalogd.TypeUnpacked, - Status: metav1.ConditionTrue, - Reason: catalogd.ReasonUnpackSuccessful, - }, - }, - }, - }, - } - - expectedBundles := []*catalogmetadata.Bundle{ { - CatalogName: "catalog-1", - Bundle: declcfg.Bundle{ - Schema: declcfg.SchemaBundle, - Name: "fake1.v1.0.0", - Package: "fake1", - Image: "fake-image", - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName":"fake1","version":"1.0.0"}`), - }, - }, + name: "unpacked, invalid package present", + catalog: &catalogd.ClusterCatalog{ + Status: catalogd.ClusterCatalogStatus{Conditions: []metav1.Condition{{Type: catalogd.TypeUnpacked, Status: metav1.ConditionTrue}}}, }, - InChannels: []*catalogmetadata.Channel{ - { - Channel: declcfg.Channel{ - Schema: declcfg.SchemaChannel, - Name: "beta", - Package: "fake1", - Entries: []declcfg.ChannelEntry{ - { - Name: "fake1.v1.0.0", - }, - }, - }, - }, - { - Channel: declcfg.Channel{ - Schema: declcfg.SchemaChannel, - Name: "stable", - Package: "fake1", - Entries: []declcfg.ChannelEntry{ - { - Name: "fake1.v1.0.0", - }, - }, - }, - }, + pkgName: "invalid-pkg-present", + fetcher: fetcherFunc(func(context.Context, *catalogd.ClusterCatalog) (fs.FS, error) { + return fstest.MapFS{ + "invalid-pkg-present/olm.package/invalid-pkg-present.json": &fstest.MapFile{Data: []byte(`{"schema": "olm.package","name": 12345}`)}, + }, nil + }), + assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) { + assert.ErrorContains(t, err, `error loading package "invalid-pkg-present"`) + assert.Nil(t, fbc) }, }, { - CatalogName: "catalog-2", - Bundle: declcfg.Bundle{ - Schema: declcfg.SchemaBundle, - Name: "fake1.v1.0.0", - Package: "fake1", - Image: "fake-image", - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName":"fake1","version":"1.0.0"}`), - }, - }, + name: "unpacked, package present", + catalog: &catalogd.ClusterCatalog{ + Status: catalogd.ClusterCatalogStatus{Conditions: []metav1.Condition{{Type: catalogd.TypeUnpacked, Status: metav1.ConditionTrue}}}, }, - InChannels: []*catalogmetadata.Channel{ - { - Channel: declcfg.Channel{ - Schema: declcfg.SchemaChannel, - Name: "stable", - Package: "fake1", - Entries: []declcfg.ChannelEntry{ - { - Name: "fake1.v1.0.0", - }, - }, - }, - }, + pkgName: "pkg-present", + fetcher: fetcherFunc(func(context.Context, *catalogd.ClusterCatalog) (fs.FS, error) { return testFS, nil }), + assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) { + assert.NoError(t, err) + assert.Equal(t, &declcfg.DeclarativeConfig{Packages: []declcfg.Package{{Schema: declcfg.SchemaPackage, Name: "pkg-present"}}}, fbc) }, }, + } { + t.Run(tc.name, func(t *testing.T) { + c := catalogClient.New(tc.fetcher) + fbc, err := c.GetPackage(context.Background(), tc.catalog, tc.pkgName) + tc.assert(t, fbc, err) + }) } - - catalog1FS := fstest.MapFS{ - "fake1/olm.package/fake1.json": &fstest.MapFile{Data: []byte(package1)}, - "fake1/olm.bundle/fake1.v1.0.0.json": &fstest.MapFile{Data: []byte(bundle1)}, - "fake1/olm.channel/stable.json": &fstest.MapFile{Data: []byte(stableChannel)}, - "fake1/olm.channel/beta.json": &fstest.MapFile{Data: []byte(betaChannel)}, - } - catalog2FS := fstest.MapFS{ - "fake1/olm.package/fake1.json": &fstest.MapFile{Data: []byte(package1)}, - "fake1/olm.bundle/fake1.v1.0.0.json": &fstest.MapFile{Data: []byte(bundle1)}, - "fake1/olm.channel/stable.json": &fstest.MapFile{Data: []byte(stableChannel)}, - } - - catalogContents := map[string]fstest.MapFS{ - "catalog-1": catalog1FS, - "catalog-2": catalog2FS, - } - - return objs, expectedBundles, catalogContents } -var _ catalogClient.Fetcher = &MockFetcher{} - -type MockFetcher struct { - contentMap map[string]fstest.MapFS - shouldError bool -} - -func (mc *MockFetcher) FetchCatalogContents(_ context.Context, catalog *catalogd.ClusterCatalog) (fs.FS, error) { - if mc.shouldError { - return nil, errors.New("mock cache error") - } +type fetcherFunc func(context.Context, *catalogd.ClusterCatalog) (fs.FS, error) - data := mc.contentMap[catalog.Name] - return data, nil +func (f fetcherFunc) FetchCatalogContents(ctx context.Context, catalog *catalogd.ClusterCatalog) (fs.FS, error) { + return f(ctx, catalog) } diff --git a/internal/catalogmetadata/compare/compare.go b/internal/catalogmetadata/compare/compare.go new file mode 100644 index 000000000..1b76d0cde --- /dev/null +++ b/internal/catalogmetadata/compare/compare.go @@ -0,0 +1,57 @@ +package compare + +import ( + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + + "github.com/operator-framework/operator-controller/internal/bundleutil" +) + +// ByVersion is a sort "less" function that orders bundles +// in inverse version order (higher versions on top). +func ByVersion(b1, b2 declcfg.Bundle) int { + v1, err1 := bundleutil.GetVersion(b1) + v2, err2 := bundleutil.GetVersion(b2) + if err1 != nil || err2 != nil { + return compareErrors(err1, err2) + } + + // Check for "greater than" because + // we want higher versions on top + return v2.Compare(*v1) +} + +func ByDeprecationFunc(deprecation declcfg.Deprecation) func(a, b declcfg.Bundle) int { + deprecatedBundles := sets.New[string]() + for _, entry := range deprecation.Entries { + if entry.Reference.Schema == declcfg.SchemaBundle { + deprecatedBundles.Insert(entry.Reference.Name) + } + } + return func(a, b declcfg.Bundle) int { + aDeprecated := deprecatedBundles.Has(a.Name) + bDeprecated := deprecatedBundles.Has(b.Name) + if aDeprecated && !bDeprecated { + return 1 + } + if !aDeprecated && bDeprecated { + return -1 + } + return 0 + } +} + +// 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 +func compareErrors(err1 error, err2 error) int { + if err1 != nil && err2 == nil { + return 1 + } + + if err1 == nil && err2 != nil { + return -1 + } + return 0 +} diff --git a/internal/catalogmetadata/compare/compare_test.go b/internal/catalogmetadata/compare/compare_test.go new file mode 100644 index 000000000..095c879c1 --- /dev/null +++ b/internal/catalogmetadata/compare/compare_test.go @@ -0,0 +1,89 @@ +package compare_test + +import ( + "encoding/json" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + + "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" +) + +func TestByVersion(t *testing.T) { + b1 := declcfg.Bundle{ + Name: "package1.v1.0.0", + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0"}`), + }, + }, + } + b2 := declcfg.Bundle{ + Name: "package1.v0.0.1", + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"packageName": "package1", "version": "0.0.1"}`), + }, + }, + } + b3 := declcfg.Bundle{ + Name: "package1.v1.0.0-alpha+001", + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0-alpha+001"}`), + }, + }, + } + b4noVersion := declcfg.Bundle{ + Name: "package1.no-version", + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"packageName": "package1"}`), + }, + }, + } + b5empty := declcfg.Bundle{ + Name: "package1.empty", + } + + t.Run("all bundles valid", func(t *testing.T) { + toSort := []declcfg.Bundle{b3, b2, b1} + slices.SortStableFunc(toSort, compare.ByVersion) + assert.Equal(t, []declcfg.Bundle{b1, b3, b2}, toSort) + }) + + t.Run("some bundles are missing version", func(t *testing.T) { + toSort := []declcfg.Bundle{b3, b4noVersion, b2, b5empty, b1} + slices.SortStableFunc(toSort, compare.ByVersion) + assert.Equal(t, []declcfg.Bundle{b1, b3, b2, b4noVersion, b5empty}, toSort) + }) +} + +func TestByDeprecationFunc(t *testing.T) { + deprecation := declcfg.Deprecation{ + Entries: []declcfg.DeprecationEntry{ + {Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaBundle, Name: "a"}}, + {Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaBundle, Name: "b"}}, + }, + } + byDeprecation := compare.ByDeprecationFunc(deprecation) + a := declcfg.Bundle{Name: "a"} + b := declcfg.Bundle{Name: "b"} + c := declcfg.Bundle{Name: "c"} + d := declcfg.Bundle{Name: "d"} + + assert.Equal(t, 0, byDeprecation(a, b)) + assert.Equal(t, 0, byDeprecation(b, a)) + assert.Equal(t, 1, byDeprecation(a, c)) + assert.Equal(t, -1, byDeprecation(c, a)) + assert.Equal(t, 0, byDeprecation(c, d)) + assert.Equal(t, 0, byDeprecation(d, c)) +} diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index 8f16c6c5e..98e3ab4cd 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -2,23 +2,15 @@ package filter import ( mmsemver "github.com/Masterminds/semver/v3" - bsemver "github.com/blang/semver/v4" "github.com/operator-framework/operator-registry/alpha/declcfg" - ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" + "github.com/operator-framework/operator-controller/internal/bundleutil" ) -func WithPackageName(packageName string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - return bundle.Package == packageName - } -} - -func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - bVersion, err := bundle.Version() +func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[declcfg.Bundle] { + return func(b declcfg.Bundle) bool { + bVersion, err := bundleutil.GetVersion(b) if err != nil { return false } @@ -34,63 +26,11 @@ func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[catal } } -func InBlangSemverRange(semverRange bsemver.Range) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - bundleVersion, err := bundle.Version() - if err != nil { - return false - } - return semverRange(*bundleVersion) - } -} - -func InChannel(channelName string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - for _, ch := range bundle.InChannels { - if ch.Name == channelName { - return true - } - } - return false - } -} - -func WithBundleImage(bundleImage string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - return bundle.Image == bundleImage - } -} - -func WithBundleName(bundleName string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - return bundle.Name == bundleName - } -} - -func LegacySuccessor(installedBundle *ocv1alpha1.BundleMetadata) Predicate[catalogmetadata.Bundle] { - isSuccessor := func(candidateBundleEntry declcfg.ChannelEntry) bool { - if candidateBundleEntry.Replaces == installedBundle.Name { - return true - } - for _, skip := range candidateBundleEntry.Skips { - if skip == installedBundle.Name { - return true - } - } - if candidateBundleEntry.SkipRange != "" { - installedBundleVersion, vErr := bsemver.Parse(installedBundle.Version) - skipRange, srErr := bsemver.ParseRange(candidateBundleEntry.SkipRange) - if vErr == nil && srErr == nil && skipRange(installedBundleVersion) { - return true - } - } - return false - } - - return func(candidateBundle *catalogmetadata.Bundle) bool { - for _, ch := range candidateBundle.InChannels { - for _, chEntry := range ch.Entries { - if candidateBundle.Name == chEntry.Name && isSuccessor(chEntry) { +func InAnyChannel(channels ...declcfg.Channel) Predicate[declcfg.Bundle] { + return func(bundle declcfg.Bundle) bool { + for _, ch := range channels { + for _, entry := range ch.Entries { + if entry.Name == bundle.Name { return true } } @@ -98,9 +38,3 @@ func LegacySuccessor(installedBundle *ocv1alpha1.BundleMetadata) Predicate[catal 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 51bb5746a..dd6abe6d7 100644 --- a/internal/catalogmetadata/filter/bundle_predicates_test.go +++ b/internal/catalogmetadata/filter/bundle_predicates_test.go @@ -5,54 +5,39 @@ import ( "testing" mmsemver "github.com/Masterminds/semver/v3" - bsemver "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" - ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" ) -func TestWithPackageName(t *testing.T) { - b1 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{Package: "package1"}} - b2 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{Package: "package2"}} - b3 := &catalogmetadata.Bundle{} - - f := filter.WithPackageName("package1") - - assert.True(t, f(b1)) - assert.False(t, f(b2)) - assert.False(t, f(b3)) -} - func TestInMastermindsSemverRange(t *testing.T) { - b1 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ + b1 := declcfg.Bundle{ Properties: []property.Property{ { Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0"}`), }, }, - }} - b2 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ + } + b2 := declcfg.Bundle{ Properties: []property.Property{ { Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package1", "version": "0.0.1"}`), }, }, - }} - b3 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ + } + b3 := declcfg.Bundle{ Properties: []property.Property{ { Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package1", "version": "broken"}`), }, }, - }} + } vRange, err := mmsemver.NewConstraint(">=1.0.0") assert.NoError(t, err) @@ -64,150 +49,21 @@ func TestInMastermindsSemverRange(t *testing.T) { assert.False(t, f(b3)) } -func TestInBlangSemverRange(t *testing.T) { - b1 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0"}`), - }, - }, - }} - b2 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1", "version": "0.0.1"}`), - }, - }, - }} - b3 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1", "version": "broken"}`), - }, - }, - }} - - vRange := bsemver.MustParseRange(">=1.0.0") - - f := filter.InBlangSemverRange(vRange) - - assert.True(t, f(b1)) - assert.False(t, f(b2)) - assert.False(t, f(b3)) -} - -func TestInChannel(t *testing.T) { - b1 := &catalogmetadata.Bundle{InChannels: []*catalogmetadata.Channel{ - {Channel: declcfg.Channel{Name: "alpha"}}, - {Channel: declcfg.Channel{Name: "stable"}}, - }} - b2 := &catalogmetadata.Bundle{InChannels: []*catalogmetadata.Channel{ - {Channel: declcfg.Channel{Name: "alpha"}}, - }} - b3 := &catalogmetadata.Bundle{} - - f := filter.InChannel("stable") - - assert.True(t, f(b1)) - assert.False(t, f(b2)) - assert.False(t, f(b3)) -} - -func TestWithBundleImage(t *testing.T) { - b1 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{Image: "fake-image-uri-1"}} - b2 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{Image: "fake-image-uri-2"}} - b3 := &catalogmetadata.Bundle{} - - f := filter.WithBundleImage("fake-image-uri-1") - - assert.True(t, f(b1)) - assert.False(t, f(b2)) - assert.False(t, f(b3)) -} - -func TestLegacySuccessor(t *testing.T) { - fakeChannel := &catalogmetadata.Channel{ - Channel: declcfg.Channel{ - Entries: []declcfg.ChannelEntry{ - { - Name: "package1.v0.0.2", - Replaces: "package1.v0.0.1", - }, - { - Name: "package1.v0.0.3", - Replaces: "package1.v0.0.2", - }, - { - Name: "package1.v0.0.4", - Skips: []string{"package1.v0.0.1"}, - }, - { - Name: "package1.v0.0.5", - SkipRange: "<=0.0.1", - }, - }, - }, - } - installedBundle := &ocv1alpha1.BundleMetadata{ - Name: "package1.v0.0.1", - Version: "0.0.1", - } - - b2 := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{Name: "package1.v0.0.2"}, - InChannels: []*catalogmetadata.Channel{fakeChannel}, - } - b3 := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{Name: "package1.v0.0.3"}, - InChannels: []*catalogmetadata.Channel{fakeChannel}, - } - b4 := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{Name: "package1.v0.0.4"}, - InChannels: []*catalogmetadata.Channel{fakeChannel}, - } - b5 := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{Name: "package1.v0.0.5"}, - InChannels: []*catalogmetadata.Channel{fakeChannel}, - } - emptyBundle := &catalogmetadata.Bundle{} - - f := filter.LegacySuccessor(installedBundle) - - assert.True(t, f(b2)) - assert.False(t, f(b3)) - assert.True(t, f(b4)) - assert.True(t, f(b5)) - assert.False(t, f(emptyBundle)) -} - -func TestWithDeprecation(t *testing.T) { - b1 := &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{}, - }, - }, - } +func TestInAnyChannel(t *testing.T) { + alpha := declcfg.Channel{Name: "alpha", Entries: []declcfg.ChannelEntry{{Name: "b1"}, {Name: "b2"}}} + stable := declcfg.Channel{Name: "stable", Entries: []declcfg.ChannelEntry{{Name: "b1"}}} - b2 := &catalogmetadata.Bundle{} + b1 := declcfg.Bundle{Name: "b1"} + b2 := declcfg.Bundle{Name: "b2"} + b3 := declcfg.Bundle{Name: "b3"} - f := filter.WithDeprecation(true) - assert.True(t, f(b1)) - assert.False(t, f(b2)) -} + fAlpha := filter.InAnyChannel(alpha) + assert.True(t, fAlpha(b1)) + assert.True(t, fAlpha(b2)) + assert.False(t, fAlpha(b3)) -func TestWithBundleName(t *testing.T) { - b1 := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{Name: "package1.v0.0.1"}, - } - b2 := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{Name: "package1.v0.0.2"}, - } - - f := filter.WithBundleName("package1.v0.0.1") - assert.True(t, f(b1)) - assert.False(t, f(b2)) + fStable := filter.InAnyChannel(stable) + assert.True(t, fStable(b1)) + assert.False(t, fStable(b2)) + assert.False(t, fStable(b3)) } diff --git a/internal/catalogmetadata/filter/filter.go b/internal/catalogmetadata/filter/filter.go index 36af20661..9074c926d 100644 --- a/internal/catalogmetadata/filter/filter.go +++ b/internal/catalogmetadata/filter/filter.go @@ -1,25 +1,19 @@ package filter import ( - "github.com/operator-framework/operator-controller/internal/catalogmetadata" + "slices" ) // Predicate returns true if the object should be kept when filtering -type Predicate[T catalogmetadata.Schemas] func(entity *T) bool +type Predicate[T any] func(entity T) bool // Filter filters a slice accordingly to -func Filter[T catalogmetadata.Schemas](in []*T, test Predicate[T]) []*T { - out := []*T{} - for i := range in { - if test(in[i]) { - out = append(out, in[i]) - } - } - return out +func Filter[T any](in []T, test Predicate[T]) []T { + return slices.DeleteFunc(in, Not(test)) } -func And[T catalogmetadata.Schemas](predicates ...Predicate[T]) Predicate[T] { - return func(obj *T) bool { +func And[T any](predicates ...Predicate[T]) Predicate[T] { + return func(obj T) bool { for _, predicate := range predicates { if !predicate(obj) { return false @@ -29,8 +23,8 @@ func And[T catalogmetadata.Schemas](predicates ...Predicate[T]) Predicate[T] { } } -func Or[T catalogmetadata.Schemas](predicates ...Predicate[T]) Predicate[T] { - return func(obj *T) bool { +func Or[T any](predicates ...Predicate[T]) Predicate[T] { + return func(obj T) bool { for _, predicate := range predicates { if predicate(obj) { return true @@ -40,8 +34,8 @@ func Or[T catalogmetadata.Schemas](predicates ...Predicate[T]) Predicate[T] { } } -func Not[T catalogmetadata.Schemas](predicate Predicate[T]) Predicate[T] { - return func(obj *T) bool { +func Not[T any](predicate Predicate[T]) Predicate[T] { + return func(obj T) bool { return !predicate(obj) } } diff --git a/internal/catalogmetadata/filter/filter_test.go b/internal/catalogmetadata/filter/filter_test.go index 7aeb4fd54..77d12c99f 100644 --- a/internal/catalogmetadata/filter/filter_test.go +++ b/internal/catalogmetadata/filter/filter_test.go @@ -7,72 +7,71 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" ) func TestFilter(t *testing.T) { - in := []*catalogmetadata.Bundle{ - {Bundle: declcfg.Bundle{Name: "extension1.v1", Package: "extension1", Image: "fake1"}}, - {Bundle: declcfg.Bundle{Name: "extension1.v2", Package: "extension1", Image: "fake2"}}, - {Bundle: declcfg.Bundle{Name: "extension2.v1", Package: "extension2", Image: "fake1"}}, - } - for _, tt := range []struct { name string - predicate filter.Predicate[catalogmetadata.Bundle] - want []*catalogmetadata.Bundle + predicate filter.Predicate[declcfg.Bundle] + want []declcfg.Bundle }{ { name: "simple filter with one predicate", - predicate: func(bundle *catalogmetadata.Bundle) bool { + predicate: func(bundle declcfg.Bundle) bool { return bundle.Name == "extension1.v1" }, - want: []*catalogmetadata.Bundle{ - {Bundle: declcfg.Bundle{Name: "extension1.v1", Package: "extension1", Image: "fake1"}}, + want: []declcfg.Bundle{ + {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, }, }, { name: "filter with Not predicate", - predicate: filter.Not(func(bundle *catalogmetadata.Bundle) bool { + predicate: filter.Not(func(bundle declcfg.Bundle) bool { return bundle.Name == "extension1.v1" }), - want: []*catalogmetadata.Bundle{ - {Bundle: declcfg.Bundle{Name: "extension1.v2", Package: "extension1", Image: "fake2"}}, - {Bundle: declcfg.Bundle{Name: "extension2.v1", Package: "extension2", Image: "fake1"}}, + want: []declcfg.Bundle{ + {Name: "extension1.v2", Package: "extension1", Image: "fake2"}, + {Name: "extension2.v1", Package: "extension2", Image: "fake1"}, }, }, { name: "filter with And predicate", predicate: filter.And( - func(bundle *catalogmetadata.Bundle) bool { + func(bundle declcfg.Bundle) bool { return bundle.Name == "extension1.v1" }, - func(bundle *catalogmetadata.Bundle) bool { + func(bundle declcfg.Bundle) bool { return bundle.Image == "fake1" }, ), - want: []*catalogmetadata.Bundle{ - {Bundle: declcfg.Bundle{Name: "extension1.v1", Package: "extension1", Image: "fake1"}}, + want: []declcfg.Bundle{ + {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, }, }, { name: "filter with Or predicate", predicate: filter.Or( - func(bundle *catalogmetadata.Bundle) bool { + func(bundle declcfg.Bundle) bool { return bundle.Name == "extension1.v1" }, - func(bundle *catalogmetadata.Bundle) bool { + func(bundle declcfg.Bundle) bool { return bundle.Image == "fake1" }, ), - want: []*catalogmetadata.Bundle{ - {Bundle: declcfg.Bundle{Name: "extension1.v1", Package: "extension1", Image: "fake1"}}, - {Bundle: declcfg.Bundle{Name: "extension2.v1", Package: "extension2", Image: "fake1"}}, + want: []declcfg.Bundle{ + {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, + {Name: "extension2.v1", Package: "extension2", Image: "fake1"}, }, }, } { t.Run(tt.name, func(t *testing.T) { + in := []declcfg.Bundle{ + {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, + {Name: "extension1.v2", Package: "extension1", Image: "fake2"}, + {Name: "extension2.v1", Package: "extension2", Image: "fake1"}, + } + actual := filter.Filter(in, tt.predicate) assert.Equal(t, tt.want, actual) }) diff --git a/internal/catalogmetadata/filter/successors.go b/internal/catalogmetadata/filter/successors.go new file mode 100644 index 000000000..8d02bb65d --- /dev/null +++ b/internal/catalogmetadata/filter/successors.go @@ -0,0 +1,106 @@ +package filter + +import ( + "fmt" + + mmsemver "github.com/Masterminds/semver/v3" + bsemver "github.com/blang/semver/v4" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/pkg/features" +) + +func SuccessorsOf(installedBundle *ocv1alpha1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) { + var successors successorsPredicateFunc = legacySuccessor + if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) { + successors = semverSuccessor + } + + installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) + if err != nil { + return nil, err + } + + installedVersionConstraint, err := mmsemver.NewConstraint(installedBundleVersion.String()) + if err != nil { + return nil, err + } + + successorsPredicate, err := successors(installedBundle, channels...) + if err != nil { + return nil, err + } + + // We need either successors or current version (no upgrade) + return Or( + successorsPredicate, + InMastermindsSemverRange(installedVersionConstraint), + ), nil +} + +// successorsPredicateFunc returns a predicate to find successors +// for a bundle. Predicate must not include the current version. +type successorsPredicateFunc func(installedBundle *ocv1alpha1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) + +func legacySuccessor(installedBundle *ocv1alpha1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) { + installedBundleVersion, err := bsemver.Parse(installedBundle.Version) + if err != nil { + return nil, fmt.Errorf("error parsing installed bundle version: %w", err) + } + + isSuccessor := func(candidateBundleEntry declcfg.ChannelEntry) bool { + if candidateBundleEntry.Replaces == installedBundle.Name { + return true + } + for _, skip := range candidateBundleEntry.Skips { + if skip == installedBundle.Name { + return true + } + } + if candidateBundleEntry.SkipRange != "" { + skipRange, err := bsemver.ParseRange(candidateBundleEntry.SkipRange) + if err == nil && skipRange(installedBundleVersion) { + return true + } + } + return false + } + + return func(candidateBundle declcfg.Bundle) bool { + for _, ch := range channels { + for _, chEntry := range ch.Entries { + if candidateBundle.Name == chEntry.Name && isSuccessor(chEntry) { + return true + } + } + } + return false + }, nil +} + +// semverSuccessor returns a predicate to find successors based on Semver. +// Successors will not include versions outside the major version of the +// installed bundle as major version is intended to indicate breaking changes. +// +// NOTE: semverSuccessor does not consider channels since there is no information +// in a channel entry that is necessary to determine if a bundle is a successor. +// A semver range check is the only necessary element. If filtering by channel +// membership is necessary, an additional filter for that purpose should be applied. +func semverSuccessor(installedBundle *ocv1alpha1.BundleMetadata, _ ...declcfg.Channel) (Predicate[declcfg.Bundle], error) { + currentVersion, err := mmsemver.NewVersion(installedBundle.Version) + if err != nil { + return nil, err + } + + // Based on current version create a caret range comparison constraint + // to allow only minor and patch version as successors and exclude current version. + constraintStr := fmt.Sprintf("^%[1]s, != %[1]s", currentVersion.String()) + wantedVersionRangeConstraint, err := mmsemver.NewConstraint(constraintStr) + if err != nil { + return nil, err + } + + return InMastermindsSemverRange(wantedVersionRangeConstraint), nil +} diff --git a/internal/catalogmetadata/filter/successors_test.go b/internal/catalogmetadata/filter/successors_test.go new file mode 100644 index 000000000..e487b14f7 --- /dev/null +++ b/internal/catalogmetadata/filter/successors_test.go @@ -0,0 +1,390 @@ +package filter + +import ( + "slices" + "testing" + + bsemver "github.com/blang/semver/v4" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + featuregatetesting "k8s.io/component-base/featuregate/testing" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/bundleutil" + "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" + "github.com/operator-framework/operator-controller/pkg/features" +) + +func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() + + const testPackageName = "test-package" + channelSet := map[string]declcfg.Channel{ + testPackageName: { + Package: testPackageName, + Name: "stable", + }, + } + + bundleSet := map[string]declcfg.Bundle{ + // Major version zero is for initial development and + // has different update behaviour than versions >= 1.0.0: + // - In versions 0.0.y updates are not allowed when using semver constraints + // - In versions 0.x.y only patch updates are allowed (>= 0.x.y and < 0.x+1.0) + // This means that we need in test data bundles that cover these three version ranges. + "test-package.v0.0.1": { + Name: "test-package.v0.0.1", + Package: testPackageName, + Image: "registry.io/repo/test-package@v0.0.1", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "0.0.1"), + }, + }, + "test-package.v0.0.2": { + Name: "test-package.v0.0.2", + Package: testPackageName, + Image: "registry.io/repo/test-package@v0.0.2", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "0.0.2"), + }, + }, + "test-package.v0.1.0": { + Name: "test-package.v0.1.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v0.1.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "0.1.0"), + }, + }, + "test-package.v0.1.1": { + Name: "test-package.v0.1.1", + Package: testPackageName, + Image: "registry.io/repo/test-package@v0.1.1", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "0.1.1"), + }, + }, + "test-package.v0.1.2": { + Name: "test-package.v0.1.2", + Package: testPackageName, + Image: "registry.io/repo/test-package@v0.1.2", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "0.1.2"), + }, + }, + "test-package.v0.2.0": { + Name: "test-package.v0.2.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v0.2.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "0.2.0"), + }, + }, + "test-package.v2.0.0": { + Name: "test-package.v2.0.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.0.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.0.0"), + }, + }, + "test-package.v2.1.0": { + Name: "test-package.v2.1.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.1.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.1.0"), + }, + }, + "test-package.v2.2.0": { + Name: "test-package.v2.2.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.2.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.2.0"), + }, + }, + // We need a bundle with a different major version to ensure + // that we do not allow upgrades from one major version to another + "test-package.v3.0.0": { + Name: "test-package.v3.0.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v3.0.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "3.0.0"), + }, + }, + } + + for _, b := range bundleSet { + ch := channelSet[b.Package] + ch.Entries = append(ch.Entries, declcfg.ChannelEntry{Name: b.Name}) + channelSet[b.Package] = ch + } + + for _, tt := range []struct { + name string + installedBundle *ocv1alpha1.BundleMetadata + expectedResult []declcfg.Bundle + }{ + { + name: "with non-zero major version", + installedBundle: bundleutil.MetadataFor("test-package.v2.0.0", bsemver.MustParse("2.0.0")), + expectedResult: []declcfg.Bundle{ + // Updates are allowed within the major version + bundleSet["test-package.v2.2.0"], + bundleSet["test-package.v2.1.0"], + bundleSet["test-package.v2.0.0"], + }, + }, + { + name: "with zero major and zero minor version", + installedBundle: bundleutil.MetadataFor("test-package.v0.0.1", bsemver.MustParse("0.0.1")), + expectedResult: []declcfg.Bundle{ + // No updates are allowed in major version zero when minor version is also zero + bundleSet["test-package.v0.0.1"], + }, + }, + { + name: "with zero major and non-zero minor version", + installedBundle: bundleutil.MetadataFor("test-package.v0.1.0", bsemver.MustParse("0.1.0")), + expectedResult: []declcfg.Bundle{ + // Patch version updates are allowed within the minor version + bundleSet["test-package.v0.1.2"], + bundleSet["test-package.v0.1.1"], + bundleSet["test-package.v0.1.0"], + }, + }, + { + name: "installed bundle not found", + installedBundle: &ocv1alpha1.BundleMetadata{ + Name: "test-package.v9.0.0", + Version: "9.0.0", + }, + expectedResult: []declcfg.Bundle{}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName]) + assert.NoError(t, err) + + allBundles := make([]declcfg.Bundle, 0, len(bundleSet)) + for _, bundle := range bundleSet { + allBundles = append(allBundles, bundle) + } + result := Filter(allBundles, successors) + + // sort before comparison for stable order + slices.SortFunc(result, compare.ByVersion) + + gocmpopts := []cmp.Option{ + cmpopts.IgnoreUnexported(declcfg.Bundle{}), + } + require.Empty(t, cmp.Diff(result, tt.expectedResult, gocmpopts...)) + }) + } +} + +func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() + + const testPackageName = "test-package" + channelSet := map[string]declcfg.Channel{ + testPackageName: { + Name: "stable", + Package: testPackageName, + Entries: []declcfg.ChannelEntry{ + { + Name: "test-package.v2.0.0", + }, + { + Name: "test-package.v2.1.0", + Replaces: "test-package.v2.0.0", + }, + { + Name: "test-package.v2.2.0", + Replaces: "test-package.v2.1.0", + }, + { + Name: "test-package.v2.2.1", + }, + { + Name: "test-package.v2.3.0", + Replaces: "test-package.v2.2.0", + Skips: []string{ + "test-package.v2.2.1", + }, + }, + { + Name: "test-package.v2.4.0", + SkipRange: ">=2.3.0 <2.4.0", + }, + }, + }, + } + + bundleSet := map[string]declcfg.Bundle{ + "test-package.v2.0.0": { + Name: "test-package.v2.0.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.0.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.0.0"), + }, + }, + "test-package.v2.1.0": { + Name: "test-package.v2.1.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.1.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.1.0"), + }, + }, + "test-package.v2.2.0": { + Name: "test-package.v2.2.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.2.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.2.0"), + }, + }, + "test-package.v2.2.1": { + Name: "test-package.v2.2.1", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.2.1", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.2.1"), + }, + }, + "test-package.v2.3.0": { + Name: "test-package.v2.3.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.3.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.3.0"), + }, + }, + "test-package.v2.4.0": { + Name: "test-package.v2.4.0", + Package: testPackageName, + Image: "registry.io/repo/test-package@v2.4.0", + Properties: []property.Property{ + property.MustBuildPackage(testPackageName, "2.4.0"), + }, + }, + } + + for _, tt := range []struct { + name string + installedBundle *ocv1alpha1.BundleMetadata + expectedResult []declcfg.Bundle + }{ + { + name: "respect replaces directive from catalog", + installedBundle: bundleutil.MetadataFor("test-package.v2.0.0", bsemver.MustParse("2.0.0")), + expectedResult: []declcfg.Bundle{ + // Must only have two bundle: + // - the one which replaces the current version + // - the current version (to allow to stay on the current version) + bundleSet["test-package.v2.1.0"], + bundleSet["test-package.v2.0.0"], + }, + }, + { + name: "respect skips directive from catalog", + installedBundle: bundleutil.MetadataFor("test-package.v2.2.1", bsemver.MustParse("2.2.1")), + expectedResult: []declcfg.Bundle{ + // Must only have two bundle: + // - the one which skips the current version + // - the current version (to allow to stay on the current version) + bundleSet["test-package.v2.3.0"], + bundleSet["test-package.v2.2.1"], + }, + }, + { + name: "respect skipRange directive from catalog", + installedBundle: bundleutil.MetadataFor("test-package.v2.3.0", bsemver.MustParse("2.3.0")), + expectedResult: []declcfg.Bundle{ + // Must only have two bundle: + // - the one which is skipRanges the current version + // - the current version (to allow to stay on the current version) + bundleSet["test-package.v2.4.0"], + bundleSet["test-package.v2.3.0"], + }, + }, + { + name: "installed bundle not found", + installedBundle: &ocv1alpha1.BundleMetadata{ + Name: "test-package.v9.0.0", + Version: "9.0.0", + }, + expectedResult: []declcfg.Bundle{}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName]) + assert.NoError(t, err) + + allBundles := make([]declcfg.Bundle, 0, len(bundleSet)) + for _, bundle := range bundleSet { + allBundles = append(allBundles, bundle) + } + result := Filter(allBundles, successors) + + // sort before comparison for stable order + slices.SortFunc(result, compare.ByVersion) + + gocmpopts := []cmp.Option{ + cmpopts.IgnoreUnexported(declcfg.Bundle{}), + } + require.Empty(t, cmp.Diff(result, tt.expectedResult, gocmpopts...)) + }) + } +} + +func TestLegacySuccessor(t *testing.T) { + fakeChannel := declcfg.Channel{ + Entries: []declcfg.ChannelEntry{ + { + Name: "package1.v0.0.2", + Replaces: "package1.v0.0.1", + }, + { + Name: "package1.v0.0.3", + Replaces: "package1.v0.0.2", + }, + { + Name: "package1.v0.0.4", + Skips: []string{"package1.v0.0.1"}, + }, + { + Name: "package1.v0.0.5", + SkipRange: "<=0.0.1", + }, + }, + } + installedBundle := &ocv1alpha1.BundleMetadata{ + Name: "package1.v0.0.1", + Version: "0.0.1", + } + + b2 := declcfg.Bundle{Name: "package1.v0.0.2"} + b3 := declcfg.Bundle{Name: "package1.v0.0.3"} + b4 := declcfg.Bundle{Name: "package1.v0.0.4"} + b5 := declcfg.Bundle{Name: "package1.v0.0.5"} + emptyBundle := declcfg.Bundle{} + + f, err := legacySuccessor(installedBundle, fakeChannel) + assert.NoError(t, err) + + assert.True(t, f(b2)) + assert.False(t, f(b3)) + assert.True(t, f(b4)) + assert.True(t, f(b5)) + assert.False(t, f(emptyBundle)) +} diff --git a/internal/catalogmetadata/sort/sort.go b/internal/catalogmetadata/sort/sort.go deleted file mode 100644 index adfd0dd5c..000000000 --- a/internal/catalogmetadata/sort/sort.go +++ /dev/null @@ -1,52 +0,0 @@ -package sort - -import ( - "github.com/operator-framework/operator-controller/internal/catalogmetadata" -) - -// ByVersion is a sort "less" function that orders bundles -// in inverse version order (higher versions on top). -func ByVersion(b1, b2 *catalogmetadata.Bundle) bool { - ver1, err1 := b1.Version() - ver2, err2 := b2.Version() - if err1 != nil || err2 != nil { - return compareErrors(err1, err2) < 0 - } - - // Check for "greater than" because - // we want higher versions on top - return ver1.GT(*ver2) -} - -// ByDeprecation is a sort "less" function that orders bundles -// that are deprecated lower than ones without deprecations -func ByDeprecated(b1, b2 *catalogmetadata.Bundle) bool { - b1Val := 1 - b2Val := 1 - - if b1.IsDeprecated() { - b1Val = b1Val - 1 - } - - if b2.IsDeprecated() { - 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 -func compareErrors(err1 error, err2 error) int { - if err1 != nil && err2 == nil { - return 1 - } - - if err1 == nil && err2 != nil { - return -1 - } - return 0 -} diff --git a/internal/catalogmetadata/sort/sort_test.go b/internal/catalogmetadata/sort/sort_test.go deleted file mode 100644 index e0c73c9ca..000000000 --- a/internal/catalogmetadata/sort/sort_test.go +++ /dev/null @@ -1,162 +0,0 @@ -package sort_test - -import ( - "encoding/json" - "sort" - "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" - - "github.com/operator-framework/operator-controller/internal/catalogmetadata" - catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" -) - -func TestByVersion(t *testing.T) { - b1 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "package1.v1.0.0", - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0"}`), - }, - }, - }} - b2 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "package1.v0.0.1", - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1", "version": "0.0.1"}`), - }, - }, - }} - b3 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "package1.v1.0.0-alpha+001", - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0-alpha+001"}`), - }, - }, - }} - b4noVersion := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "package1.no-version", - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1"}`), - }, - }, - }} - b5empty := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "package1.empty", - }} - - t.Run("all bundles valid", func(t *testing.T) { - toSort := []*catalogmetadata.Bundle{b3, b2, b1} - sort.SliceStable(toSort, func(i, j int) bool { - return catalogsort.ByVersion(toSort[i], toSort[j]) - }) - - assert.Len(t, toSort, 3) - assert.Equal(t, b1, toSort[0]) - assert.Equal(t, b3, toSort[1]) - assert.Equal(t, b2, toSort[2]) - }) - - t.Run("some bundles are missing version", func(t *testing.T) { - toSort := []*catalogmetadata.Bundle{b3, b4noVersion, b2, b5empty, b1} - sort.SliceStable(toSort, func(i, j int) bool { - return catalogsort.ByVersion(toSort[i], toSort[j]) - }) - - assert.Len(t, toSort, 5) - assert.Equal(t, b1, toSort[0]) - assert.Equal(t, b3, toSort[1]) - assert.Equal(t, b2, toSort[2]) - assert.Equal(t, b4noVersion, toSort[3]) - assert.Equal(t, b5empty, toSort[4]) - }) -} - -func TestByDeprecated(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{ - 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]) - }) - - 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]) - - 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 deleted file mode 100644 index 1cf662ef9..000000000 --- a/internal/catalogmetadata/types.go +++ /dev/null @@ -1,181 +0,0 @@ -package catalogmetadata - -import ( - "encoding/json" - "fmt" - "sync" - - bsemver "github.com/blang/semver/v4" - - "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" -) - -type Schemas interface { - Package | Bundle | Channel | Deprecation -} - -type Package struct { - declcfg.Package -} - -type Channel struct { - declcfg.Channel -} - -type Deprecation struct { - declcfg.Deprecation -} - -type PackageRequired struct { - property.PackageRequired - SemverRange bsemver.Range `json:"-"` -} - -type Bundle struct { - declcfg.Bundle - CatalogName string - InChannels []*Channel - Deprecations []declcfg.DeprecationEntry - - mu sync.RWMutex - // these properties are lazy loaded as they are requested - propertiesMap map[string][]*property.Property - bundlePackage *property.Package - semVersion *bsemver.Version - requiredPackages []PackageRequired -} - -func (b *Bundle) Version() (*bsemver.Version, error) { - if err := b.loadPackage(); err != nil { - return nil, err - } - return b.semVersion, nil -} - -func (b *Bundle) RequiredPackages() ([]PackageRequired, error) { - if err := b.loadRequiredPackages(); err != nil { - return nil, err - } - return b.requiredPackages, nil -} - -func (b *Bundle) loadPackage() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.bundlePackage == nil { - bundlePackage, err := loadOneFromProps[property.Package](b, property.TypePackage, true) - if err != nil { - return err - } - b.bundlePackage = &bundlePackage - } - if b.semVersion == nil { - semVer, err := bsemver.Parse(b.bundlePackage.Version) - if err != nil { - return fmt.Errorf("could not parse semver %q for bundle '%s': %s", b.bundlePackage.Version, b.Name, err) - } - b.semVersion = &semVer - } - return nil -} - -func (b *Bundle) loadRequiredPackages() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.requiredPackages == nil { - requiredPackages, err := loadFromProps[PackageRequired](b, property.TypePackageRequired, false) - if err != nil { - return fmt.Errorf("error determining bundle required packages for bundle %q: %s", b.Name, err) - } - for i := range requiredPackages { - semverRange, err := bsemver.ParseRange(requiredPackages[i].PackageRequired.VersionRange) - if err != nil { - return fmt.Errorf( - "error parsing bundle required package semver range for bundle %q (required package %q): %s", - b.Bundle.Name, - requiredPackages[i].PackageRequired.PackageName, - err, - ) - } - requiredPackages[i].SemverRange = semverRange - } - b.requiredPackages = requiredPackages - } - return nil -} - -func (b *Bundle) propertiesByType(propType string) []*property.Property { - if b.propertiesMap == nil { - b.propertiesMap = make(map[string][]*property.Property) - for i := range b.Bundle.Properties { - prop := b.Bundle.Properties[i] - b.propertiesMap[prop.Type] = append(b.propertiesMap[prop.Type], &prop) - } - } - - return b.propertiesMap[propType] -} - -// HasDeprecation returns true if the bundle -// has any deprecations associated with it. -// This may return true even in cases where the bundle -// may be associated with an olm.channel deprecation -// but the bundle is not considered "deprecated" because -// the bundle is selected via a non-deprecated channel. -func (b *Bundle) HasDeprecation() bool { - return len(b.Deprecations) > 0 -} - -// IsDeprecated returns true if the bundle -// has been explicitly deprecated. This can occur -// if the bundle itself has been deprecated. -// this function does not take into consideration -// 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.SchemaBundle && dep.Reference.Name == b.Bundle.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 { - return *new(T), err - } - if len(r) > 1 { - return *new(T), fmt.Errorf("expected 1 instance of property with type %q, got %d", propType, len(r)) - } - if !required && len(r) == 0 { - return *new(T), nil - } - - return r[0], nil -} - -func loadFromProps[T any](bundle *Bundle, propType string, required bool) ([]T, error) { - props := bundle.propertiesByType(propType) - if len(props) != 0 { - result := []T{} - for i := range props { - parsedProp := *new(T) - if err := json.Unmarshal(props[i].Value, &parsedProp); err != nil { - return nil, fmt.Errorf("property %q with value %q could not be parsed: %s", propType, props[i].Value, err) - } - result = append(result, parsedProp) - } - return result, nil - } else if required { - return nil, fmt.Errorf("bundle property with type %q not found", propType) - } - - return nil, nil -} diff --git a/internal/catalogmetadata/types_test.go b/internal/catalogmetadata/types_test.go deleted file mode 100644 index 743555de2..000000000 --- a/internal/catalogmetadata/types_test.go +++ /dev/null @@ -1,227 +0,0 @@ -package catalogmetadata_test - -import ( - "encoding/json" - "testing" - - bsemver "github.com/blang/semver/v4" - "github.com/stretchr/testify/assert" - - "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" - - "github.com/operator-framework/operator-controller/internal/catalogmetadata" -) - -func TestBundleVersion(t *testing.T) { - for _, tt := range []struct { - name string - bundle *catalogmetadata.Bundle - wantVersion *bsemver.Version - wantErr string - }{ - { - name: "valid version", - bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "fake-bundle.v1", - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0"}`), - }, - }, - }}, - wantVersion: &bsemver.Version{Major: 1}, - }, - { - name: "invalid version", - bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "fake-bundle.invalidVersion", - Properties: []property.Property{ - { - Type: property.TypePackage, - Value: json.RawMessage(`{"packageName": "package1", "version": "broken"}`), - }, - }, - }}, - wantVersion: nil, - wantErr: `could not parse semver "broken" for bundle 'fake-bundle.invalidVersion': No Major.Minor.Patch elements found`, - }, - { - name: "not found", - bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "fake-bundle.noVersion", - }}, - wantVersion: nil, - wantErr: `bundle property with type "olm.package" not found`, - }, - } { - t.Run(tt.name, func(t *testing.T) { - version, err := tt.bundle.Version() - assert.Equal(t, tt.wantVersion, version) - if tt.wantErr != "" { - assert.EqualError(t, err, tt.wantErr) - } else { - assert.NoError(t, err) - } - }) - } -} - -func TestBundleRequiredPackages(t *testing.T) { - for _, tt := range []struct { - name string - bundle *catalogmetadata.Bundle - wantRequiredPackages []catalogmetadata.PackageRequired - wantErr string - }{ - { - name: "valid package requirements", - bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "fake-bundle.v1", - Properties: []property.Property{ - { - Type: property.TypePackageRequired, - Value: json.RawMessage(`{"packageName": "packageA", "versionRange": ">1.0.0"}`), - }, - { - Type: property.TypePackageRequired, - Value: json.RawMessage(`{"packageName": "packageB", "versionRange": ">0.5.0 <0.8.6"}`), - }, - }, - }}, - wantRequiredPackages: []catalogmetadata.PackageRequired{ - {PackageRequired: property.PackageRequired{PackageName: "packageA", VersionRange: ">1.0.0"}}, - {PackageRequired: property.PackageRequired{PackageName: "packageB", VersionRange: ">0.5.0 <0.8.6"}}, - }, - }, - { - name: "bad package requirement", - bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "fake-bundle.badPackageRequirement", - Properties: []property.Property{ - { - Type: property.TypePackageRequired, - Value: json.RawMessage(`badRequiredPackageStructure`), - }, - }, - }}, - wantRequiredPackages: nil, - wantErr: `error determining bundle required packages for bundle "fake-bundle.badPackageRequirement": property "olm.package.required" with value "badRequiredPackageStructure" could not be parsed: invalid character 'b' looking for beginning of value`, - }, - { - name: "invalid version range", - bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{ - Name: "fake-bundle.badVersionRange", - Properties: []property.Property{ - { - Type: property.TypePackageRequired, - Value: json.RawMessage(`{"packageName": "packageA", "versionRange": "invalid"}`), - }, - }, - }}, - wantRequiredPackages: nil, - wantErr: `error parsing bundle required package semver range for bundle "fake-bundle.badVersionRange" (required package "packageA"): Could not get version from string: "invalid"`, - }, - } { - t.Run(tt.name, func(t *testing.T) { - packages, err := tt.bundle.RequiredPackages() - assert.Len(t, packages, len(tt.wantRequiredPackages)) - for i := range packages { - // SemverRange is a function which is not comparable - // so we just make sure that it is set. - assert.NotNil(t, packages[i].SemverRange) - - // And then we set it to nil for ease of further comparisons - packages[i].SemverRange = nil - } - - assert.Equal(t, tt.wantRequiredPackages, packages) - if tt.wantErr != "" { - assert.EqualError(t, err, tt.wantErr) - } else { - assert.NoError(t, err) - } - }) - } -} - -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()) - }) - } -} - -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 b34c4a0f0..9b91e3750 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -23,13 +23,10 @@ import ( "errors" "fmt" "io" - "sort" "strings" "sync" "time" - mmsemver "github.com/Masterminds/semver/v3" - bsemver "github.com/blang/semver/v4" "github.com/go-logr/logr" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart" @@ -66,14 +63,13 @@ import ( "github.com/operator-framework/operator-registry/alpha/property" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" - catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" - catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" + "github.com/operator-framework/operator-controller/internal/bundleutil" "github.com/operator-framework/operator-controller/internal/conditionsets" "github.com/operator-framework/operator-controller/internal/labels" + "github.com/operator-framework/operator-controller/internal/resolve" "github.com/operator-framework/operator-controller/internal/rukpak/bundledeployment" "github.com/operator-framework/operator-controller/internal/rukpak/convert" - crdupgradesafety "github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety" + "github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety" rukpaksource "github.com/operator-framework/operator-controller/internal/rukpak/source" "github.com/operator-framework/operator-controller/internal/rukpak/util" ) @@ -85,7 +81,7 @@ const ( // ClusterExtensionReconciler reconciles a ClusterExtension object type ClusterExtensionReconciler struct { client.Client - BundleProvider BundleProvider + Resolver resolve.Resolver Unpacker rukpaksource.Unpacker ActionClientGetter helmclient.ActionClientGetter dynamicWatchMutex sync.RWMutex @@ -140,11 +136,9 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, client.IgnoreNotFound(err) } - var updateError error - reconciledExt := existingExt.DeepCopy() res, err := r.reconcile(ctx, reconciledExt) - updateError = errors.Join(updateError, err) + updateError := err // Do checks before any Update()s, as Update() may modify the resource structure! updateStatus := !equality.Semantic.DeepEqual(existingExt.Status, reconciledExt.Status) @@ -152,8 +146,9 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req unexpectedFieldsChanged := checkForUnexpectedFieldChange(*existingExt, *reconciledExt) if updateStatus { - err = r.Client.Status().Update(ctx, reconciledExt) - updateError = errors.Join(updateError, err) + if err := r.Client.Status().Update(ctx, reconciledExt); err != nil { + updateError = errors.Join(updateError, fmt.Errorf("error updating status: %v", err)) + } } if unexpectedFieldsChanged { @@ -161,8 +156,9 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req } if updateFinalizers { - err = r.Client.Update(ctx, reconciledExt) - updateError = errors.Join(updateError, err) + if err := r.Client.Update(ctx, reconciledExt); err != nil { + updateError = errors.Join(updateError, fmt.Errorf("error updating finalizers: %v", err)) + } } return res, updateError @@ -234,9 +230,17 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp return ctrl.Result{}, nil } + installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, ext) + if err != nil { + ext.Status.InstalledBundle = nil + // TODO: use Installed=Unknown + setInstalledStatusConditionFailed(ext, err.Error()) + return ctrl.Result{}, err + } + // run resolution l.V(1).Info("resolving bundle") - bundle, err := r.resolve(ctx, *ext) + resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, installedBundle) if err != nil { // Note: We don't distinguish between resolution-specific errors and generic errors ext.Status.ResolvedBundle = nil @@ -247,7 +251,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp } l.V(1).Info("validating bundle") - if err := r.validateBundle(bundle); err != nil { + if err := r.validateBundle(resolvedBundle); err != nil { ext.Status.ResolvedBundle = nil ext.Status.InstalledBundle = nil setResolvedStatusConditionFailed(ext, err.Error()) @@ -256,24 +260,28 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp return ctrl.Result{}, err } // set deprecation status after _successful_ resolution - SetDeprecationStatus(ext, bundle) - - bundleVersion, err := bundle.Version() - if err != nil { - ext.Status.ResolvedBundle = nil - ext.Status.InstalledBundle = nil - setResolvedStatusConditionFailed(ext, err.Error()) - setInstalledStatusConditionFailed(ext, err.Error()) - return ctrl.Result{}, err - } - - ext.Status.ResolvedBundle = bundleMetadataFor(bundle) - setResolvedStatusConditionSuccess(ext, fmt.Sprintf("resolved to %q", bundle.Image)) + // TODO: + // 1. It seems like deprecation status should reflect the currently installed bundle, not the resolved + // bundle. So perhaps we should set package and channel deprecations directly after resolution, but + // defer setting the bundle deprecation until we successfully install the bundle. + // 2. If resolution fails because it can't find a bundle, that doesn't mean we wouldn't be able to find + // a deprecation for the ClusterExtension's spec.packageName. Perhaps we should check for a non-nil + // resolvedDeprecation even if resolution returns an error. If present, we can still update some of + // our deprecation status. + // - Open question though: what if different catalogs have different opinions of what's deprecated. + // If we can't resolve a bundle, how do we know which catalog to trust for deprecation information? + // Perhaps if the package shows up in multiple catalogs and deprecations don't match, we can set + // the deprecation status to unknown? Or perhaps we somehow combine the deprecation information from + // all catalogs? + SetDeprecationStatus(ext, resolvedBundle.Name, resolvedDeprecation) + + ext.Status.ResolvedBundle = bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion) + setResolvedStatusConditionSuccess(ext, fmt.Sprintf("resolved to %q", resolvedBundle.Image)) // Generate a BundleDeployment from the ClusterExtension to Unpack. // Note: The BundleDeployment here is not a k8s API, its a simple Go struct which // necessary embedded values. - bd := r.generateBundleDeploymentForUnpack(bundle.Image, ext) + bd := r.generateBundleDeploymentForUnpack(resolvedBundle.Image, ext) l.V(1).Info("unpacking resolved bundle") unpackResult, err := r.Unpacker.Unpack(ctx, bd) if err != nil { @@ -315,9 +323,9 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp labels: map[string]string{ labels.OwnerKindKey: ocv1alpha1.ClusterExtensionKind, labels.OwnerNameKey: ext.GetName(), - labels.BundleNameKey: bundle.Name, - labels.PackageNameKey: bundle.Package, - labels.BundleVersionKey: bundleVersion.String(), + labels.BundleNameKey: resolvedBundle.Name, + labels.PackageNameKey: resolvedBundle.Package, + labels.BundleVersionKey: resolvedBundleVersion.String(), }, } @@ -358,7 +366,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp case stateNeedsInstall: rel, err = ac.Install(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(install *action.Install) error { install.CreateNamespace = false - install.Labels = map[string]string{labels.BundleNameKey: bundle.Name, labels.PackageNameKey: bundle.Package, labels.BundleVersionKey: bundleVersion.String()} + install.Labels = map[string]string{labels.BundleNameKey: resolvedBundle.Name, labels.PackageNameKey: resolvedBundle.Package, labels.BundleVersionKey: resolvedBundleVersion.String()} return nil }, helmclient.AppendInstallPostRenderer(post)) if err != nil { @@ -368,7 +376,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp case stateNeedsUpgrade: rel, err = ac.Upgrade(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(upgrade *action.Upgrade) error { upgrade.MaxHistory = maxHelmReleaseHistory - upgrade.Labels = map[string]string{labels.BundleNameKey: bundle.Name, labels.PackageNameKey: bundle.Package, labels.BundleVersionKey: bundleVersion.String()} + upgrade.Labels = map[string]string{labels.BundleNameKey: resolvedBundle.Name, labels.PackageNameKey: resolvedBundle.Package, labels.BundleVersionKey: resolvedBundleVersion.String()} return nil }, helmclient.AppendUpgradePostRenderer(post)) if err != nil { @@ -402,6 +410,12 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp source.Kind(r.cache, obj, crhandler.EnqueueRequestForOwner(r.Scheme(), r.RESTMapper(), ext, crhandler.OnlyControllerOwner()), + predicate.Funcs{ + CreateFunc: func(ce event.CreateEvent) bool { return false }, + UpdateFunc: func(ue event.UpdateEvent) bool { return true }, + DeleteFunc: func(de event.DeleteEvent) bool { return true }, + GenericFunc: func(ge event.GenericEvent) bool { return true }, + }, ), ); err != nil { return err @@ -415,163 +429,77 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp return ctrl.Result{}, err } } - ext.Status.InstalledBundle = bundleMetadataFor(bundle) - setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Instantiated bundle %s successfully", ext.GetName())) + ext.Status.InstalledBundle = bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion) + setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", resolvedBundle.Image)) return ctrl.Result{}, nil } -// resolve returns a Bundle from the catalog that needs to get installed on the cluster. -func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - packageName := ext.Spec.PackageName - channelName := ext.Spec.Channel - versionRange := ext.Spec.Version - - allBundles, err := r.BundleProvider.Bundles(ctx, packageName) - if err != nil { - return nil, fmt.Errorf("error fetching bundles: %w", err) - } - - installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, &ext) - if err != nil { - return nil, err - } - - predicates := []catalogfilter.Predicate[catalogmetadata.Bundle]{ - catalogfilter.WithPackageName(packageName), - } - - if channelName != "" { - predicates = append(predicates, catalogfilter.InChannel(channelName)) - } - - if versionRange != "" { - vr, err := mmsemver.NewConstraint(versionRange) - if err != nil { - return nil, fmt.Errorf("invalid version range %q: %w", versionRange, err) +// SetDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension +// based on the provided bundle +func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundleName string, deprecation *declcfg.Deprecation) { + deprecations := map[string]declcfg.DeprecationEntry{} + if deprecation != nil { + for _, entry := range deprecation.Entries { + switch entry.Reference.Schema { + case declcfg.SchemaPackage: + deprecations[ocv1alpha1.TypePackageDeprecated] = entry + case declcfg.SchemaChannel: + if ext.Spec.Channel != entry.Reference.Name { + continue + } + deprecations[ocv1alpha1.TypeChannelDeprecated] = entry + case declcfg.SchemaBundle: + if bundleName != entry.Reference.Name { + continue + } + deprecations[ocv1alpha1.TypeBundleDeprecated] = entry + } } - predicates = append(predicates, catalogfilter.InMastermindsSemverRange(vr)) } - if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore && installedBundle != nil { - upgradePredicate, err := SuccessorsPredicate(ext.Spec.PackageName, installedBundle) - if err != nil { - return nil, err - } - - predicates = append(predicates, upgradePredicate) - } - - resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...)) - - var upgradeErrorPrefix string - if installedBundle != nil { - installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) - if err != nil { - return nil, err - } - 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) + // first get ordered deprecation messages that we'll join in the Deprecated condition message + var deprecationMessages []string + for _, conditionType := range []string{ + ocv1alpha1.TypePackageDeprecated, + ocv1alpha1.TypeChannelDeprecated, + ocv1alpha1.TypeBundleDeprecated, + } { + if entry, ok := deprecations[conditionType]; ok { + deprecationMessages = append(deprecationMessages, entry.Message) } } - 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.ByDeprecated(resultSet[i], resultSet[j]) + // next, set the Deprecated condition + status, reason, message := metav1.ConditionFalse, ocv1alpha1.ReasonDeprecated, "" + if len(deprecationMessages) > 0 { + status, reason, message = metav1.ConditionTrue, ocv1alpha1.ReasonDeprecated, strings.Join(deprecationMessages, ";") + } + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1alpha1.TypeDeprecated, + Reason: reason, + Status: status, + Message: message, + ObservedGeneration: ext.Generation, }) - return resultSet[0], nil -} - -// SetDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension -// based on the provided bundle -func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) { - // reset conditions to false - conditionTypes := []string{ - ocv1alpha1.TypeDeprecated, + // finally, set the individual deprecation conditions for package, channel, and bundle + for _, conditionType := range []string{ ocv1alpha1.TypePackageDeprecated, ocv1alpha1.TypeChannelDeprecated, ocv1alpha1.TypeBundleDeprecated, - } - - for _, conditionType := range conditionTypes { - apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ - Type: conditionType, - Reason: ocv1alpha1.ReasonDeprecated, - Status: metav1.ConditionFalse, - Message: "", - ObservedGeneration: ext.Generation, - }) - } - - // There are two early return scenarios here: - // 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 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 - // the spec.channel field. - if (!bundle.IsDeprecated() && !bundle.HasDeprecation()) || (!bundle.IsDeprecated() && ext.Spec.Channel == "") { - return - } - - deprecationMessages := []string{} - - for _, deprecation := range bundle.Deprecations { - switch deprecation.Reference.Schema { - case declcfg.SchemaPackage: - apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ - Type: ocv1alpha1.TypePackageDeprecated, - Reason: ocv1alpha1.ReasonDeprecated, - Status: metav1.ConditionTrue, - Message: deprecation.Message, - ObservedGeneration: ext.Generation, - }) - case declcfg.SchemaChannel: - if ext.Spec.Channel != deprecation.Reference.Name { - continue - } - - apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ - Type: ocv1alpha1.TypeChannelDeprecated, - Reason: ocv1alpha1.ReasonDeprecated, - Status: metav1.ConditionTrue, - Message: deprecation.Message, - ObservedGeneration: ext.Generation, - }) - case declcfg.SchemaBundle: - apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ - Type: ocv1alpha1.TypeBundleDeprecated, - Reason: ocv1alpha1.ReasonDeprecated, - Status: metav1.ConditionTrue, - Message: deprecation.Message, - ObservedGeneration: ext.Generation, - }) + } { + entry, ok := deprecations[conditionType] + status, reason, message := metav1.ConditionFalse, ocv1alpha1.ReasonDeprecated, "" + if ok { + status, reason, message = metav1.ConditionTrue, ocv1alpha1.ReasonDeprecated, entry.Message + deprecationMessages = append(deprecationMessages, message) } - - deprecationMessages = append(deprecationMessages, deprecation.Message) - } - - if len(deprecationMessages) > 0 { apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ - Type: ocv1alpha1.TypeDeprecated, - Reason: ocv1alpha1.ReasonDeprecated, - Status: metav1.ConditionTrue, - Message: strings.Join(deprecationMessages, ";"), + Type: conditionType, + Reason: reason, + Status: status, + Message: message, ObservedGeneration: ext.Generation, }) } @@ -752,23 +680,7 @@ func (p *postrenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, erro return &buf, nil } -// bundleMetadataFor returns a BundleMetadata for the given bundle. If the provided bundle is nil, -// this function panics. It is up to the caller to ensure that the bundle is non-nil. -func bundleMetadataFor(bundle *catalogmetadata.Bundle) *ocv1alpha1.BundleMetadata { - if bundle == nil { - panic("programmer error: provided bundle must be non-nil to create BundleMetadata") - } - ver, err := bundle.Version() - if err != nil { - ver = &bsemver.Version{} - } - return &ocv1alpha1.BundleMetadata{ - Name: bundle.Name, - Version: ver.String(), - } -} - -func (r *ClusterExtensionReconciler) validateBundle(bundle *catalogmetadata.Bundle) error { +func (r *ClusterExtensionReconciler) validateBundle(bundle *declcfg.Bundle) error { unsupportedProps := sets.New[string]( property.TypePackageRequired, property.TypeGVKRequired, diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index 87b7f0170..d85fe2166 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -2,11 +2,10 @@ package controllers_test import ( "context" - "encoding/json" - "errors" "fmt" "testing" + bsemver "github.com/blang/semver/v4" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" @@ -16,20 +15,16 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" - featuregatetesting "k8s.io/component-base/featuregate/testing" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/conditionsets" "github.com/operator-framework/operator-controller/internal/controllers" + "github.com/operator-framework/operator-controller/internal/resolve" "github.com/operator-framework/operator-controller/internal/rukpak/source" - "github.com/operator-framework/operator-controller/internal/testutil" - "github.com/operator-framework/operator-controller/pkg/features" ) // Describe: ClusterExtension Controller Test @@ -43,14 +38,17 @@ func TestClusterExtensionDoesNotExist(t *testing.T) { require.NoError(t, err) } -func TestClusterExtensionNonExistentPackage(t *testing.T) { +func TestClusterExtensionResolutionFails(t *testing.T) { + pkgName := fmt.Sprintf("non-existent-%s", rand.String(6)) cl, reconciler := newClientAndReconciler(t, nil) + reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + return nil, nil, nil, fmt.Errorf("no package %q found", pkgName) + }) ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} t.Log("When the cluster extension specifies a non-existent package") t.Log("By initializing cluster state") - pkgName := fmt.Sprintf("non-existent-%s", rand.String(6)) clusterExtension := &ocv1alpha1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, Spec: ocv1alpha1.ClusterExtensionSpec{ @@ -87,56 +85,7 @@ func TestClusterExtensionNonExistentPackage(t *testing.T) { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) } -func TestClusterExtensionNonExistentVersion(t *testing.T) { - cl, reconciler := newClientAndReconciler(t, nil) - ctx := context.Background() - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - - t.Log("When the cluster extension specifies a version that does not exist") - t.Log("By initializing cluster state") - pkgName := "prometheus" - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: pkgName, - Version: "0.50.0", // this version of the package does not exist - InstallNamespace: "default", - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: "default", - }, - }, - } - require.NoError(t, cl.Create(ctx, clusterExtension)) - - t.Log("It sets resolution failure status") - t.Log("By running reconcile") - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Equal(t, ctrl.Result{}, res) - require.EqualError(t, err, fmt.Sprintf(`no package %q matching version "0.50.0" found`, pkgName)) - - t.Log("By fetching updated cluster extension after reconcile") - require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) - - t.Log("By checking the status fields") - require.Empty(t, clusterExtension.Status.ResolvedBundle) - require.Empty(t, clusterExtension.Status.InstalledBundle) - - t.Log("By checking the expected conditions") - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - require.Equal(t, fmt.Sprintf(`no package %q matching version "0.50.0" found`, pkgName), cond.Message) - - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - - verifyInvariants(ctx, t, reconciler.Client, clusterExtension) - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) -} - -func TestClusterExtensionChannelVersionExists(t *testing.T) { +func TestClusterExtensionResolutionSucceeds(t *testing.T) { cl, reconciler := newClientAndReconciler(t, nil) mockUnpacker := unpacker.(*MockUnpacker) // Set up the Unpack method to return a result with StateUnpacked @@ -172,6 +121,14 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) { t.Log("It sets resolution success status") t.Log("By running reconcile") + reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v := bsemver.MustParse("1.0.0") + return &declcfg.Bundle{ + Name: "prometheus.v1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + }, &v, nil, nil + }) res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) require.NoError(t, err) @@ -199,227 +156,6 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) } -func TestClusterExtensionChannelExistsNoVersion(t *testing.T) { - cl, reconciler := newClientAndReconciler(t, nil) - mockUnpacker := unpacker.(*MockUnpacker) - // Set up the Unpack method to return a result with StateUnpacked - mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*bundledeployment.BundleDeployment")).Return(&source.Result{ - State: source.StatePending, - }, nil) - - ctx := context.Background() - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - - t.Log("When the cluster extension specifies a package that exists within a channel but no version specified") - t.Log("By initializing cluster state") - pkgName := "prometheus" - pkgVer := "" - pkgChan := "beta" - installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) - - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: pkgName, - Version: pkgVer, - Channel: pkgChan, - InstallNamespace: installNamespace, - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: serviceAccount, - }, - }, - } - err := cl.Create(ctx, clusterExtension) - require.NoError(t, err) - - t.Log("It sets resolution success status") - t.Log("By running reconcile") - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Equal(t, ctrl.Result{}, res) - require.NoError(t, err) - - t.Log("By fetching updated cluster extension after reconcile") - require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) - - t.Log("By checking the status fields") - require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) - require.Empty(t, clusterExtension.Status.InstalledBundle) - - t.Log("By checking the expected conditions") - resolvedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, resolvedCond) - require.Equal(t, metav1.ConditionTrue, resolvedCond.Status) - require.Equal(t, ocv1alpha1.ReasonSuccess, resolvedCond.Reason) - require.Equal(t, "resolved to \"quay.io/operatorhubio/prometheus@fake2.0.0\"", resolvedCond.Message) - - t.Log("By checking the expected unpacked conditions") - unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeUnpacked) - require.NotNil(t, unpackedCond) - require.Equal(t, metav1.ConditionFalse, unpackedCond.Status) - require.Equal(t, ocv1alpha1.ReasonUnpackPending, unpackedCond.Reason) - - verifyInvariants(ctx, t, reconciler.Client, clusterExtension) - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) -} - -func TestClusterExtensionVersionNoChannel(t *testing.T) { - cl, reconciler := newClientAndReconciler(t, nil) - ctx := context.Background() - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - - t.Log("When the cluster extension specifies a package version in a channel that does not exist") - t.Log("By initializing cluster state") - pkgName := "prometheus" - pkgVer := "0.47.0" - pkgChan := "alpha" - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: pkgName, - Version: pkgVer, - Channel: pkgChan, - InstallNamespace: "default", - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: "default", - }, - }, - } - require.NoError(t, cl.Create(ctx, clusterExtension)) - - t.Log("It sets resolution failure status") - t.Log("By running reconcile") - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Equal(t, ctrl.Result{}, res) - require.EqualError(t, err, fmt.Sprintf("no package %q matching version %q in channel %q found", pkgName, pkgVer, pkgChan)) - - t.Log("By fetching updated cluster extension after reconcile") - require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) - - t.Log("By checking the status fields") - require.Empty(t, clusterExtension.Status.ResolvedBundle) - require.Empty(t, clusterExtension.Status.InstalledBundle) - - t.Log("By checking the expected conditions") - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - require.Equal(t, fmt.Sprintf("no package %q matching version %q in channel %q found", pkgName, pkgVer, pkgChan), cond.Message) - - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - - verifyInvariants(ctx, t, reconciler.Client, clusterExtension) - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) -} - -func TestClusterExtensionNoChannel(t *testing.T) { - cl, reconciler := newClientAndReconciler(t, nil) - ctx := context.Background() - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - - t.Log("When the cluster extension specifies a package in a channel that does not exist") - t.Log("By initializing cluster state") - pkgName := "prometheus" - pkgChan := "non-existent" - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: pkgName, - Channel: pkgChan, - InstallNamespace: "default", - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: "default", - }, - }, - } - require.NoError(t, cl.Create(ctx, clusterExtension)) - - t.Log("It sets resolution failure status") - t.Log("By running reconcile") - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Equal(t, ctrl.Result{}, res) - require.EqualError(t, err, fmt.Sprintf("no package %q in channel %q found", pkgName, pkgChan)) - - t.Log("By fetching updated cluster extension after reconcile") - require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) - - t.Log("By checking the status fields") - require.Empty(t, clusterExtension.Status.ResolvedBundle) - require.Empty(t, clusterExtension.Status.InstalledBundle) - - t.Log("By checking the expected conditions") - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - require.Equal(t, fmt.Sprintf("no package %q in channel %q found", pkgName, pkgChan), cond.Message) - - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - - verifyInvariants(ctx, t, reconciler.Client, clusterExtension) - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) -} - -func TestClusterExtensionNoVersion(t *testing.T) { - cl, reconciler := newClientAndReconciler(t, nil) - ctx := context.Background() - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - - t.Log("When the cluster extension specifies a package version that does not exist in the channel") - t.Log("By initializing cluster state") - pkgName := "prometheus" - pkgVer := "0.57.0" - pkgChan := "non-existent" - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: pkgName, - Version: pkgVer, - Channel: pkgChan, - InstallNamespace: "default", - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: "default", - }, - }, - } - require.NoError(t, cl.Create(ctx, clusterExtension)) - - t.Log("It sets resolution failure status") - t.Log("By running reconcile") - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Equal(t, ctrl.Result{}, res) - require.EqualError(t, err, fmt.Sprintf("no package %q matching version %q in channel %q found", pkgName, pkgVer, pkgChan)) - - t.Log("By fetching updated cluster extension after reconcile") - require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) - - t.Log("By checking the status fields") - require.Empty(t, clusterExtension.Status.ResolvedBundle) - require.Empty(t, clusterExtension.Status.InstalledBundle) - - t.Log("By checking the expected conditions") - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - require.Equal(t, fmt.Sprintf("no package %q matching version %q in channel %q found", pkgName, pkgVer, pkgChan), cond.Message) - - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - - verifyInvariants(ctx, t, reconciler.Client, clusterExtension) - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) -} - func verifyInvariants(ctx context.Context, t *testing.T, c client.Client, ext *ocv1alpha1.ClusterExtension) { key := client.ObjectKeyFromObject(ext) require.NoError(t, c.Get(ctx, key, ext)) @@ -442,521 +178,16 @@ func verifyConditionsInvariants(t *testing.T, ext *ocv1alpha1.ClusterExtension) } } -func TestClusterExtensionUpgrade(t *testing.T) { - mockUnpacker := unpacker.(*MockUnpacker) - // Set up the Unpack method to return a result with StateUnpackPending - mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*bundledeployment.BundleDeployment")).Return(&source.Result{ - State: source.StatePending, - }, nil) - ctx := context.Background() - - t.Run("semver upgrade constraints enforcement of upgrades within major version", func(t *testing.T) { - bundle := &ocv1alpha1.BundleMetadata{ - Name: "prometheus.v1.0.0", - Version: "1.0.0", - } - - cl, reconciler := newClientAndReconciler(t, bundle) - - defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() - defer func() { - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) - }() - - pkgName := "prometheus" - pkgVer := "1.0.0" - pkgChan := "beta" - installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: pkgName, - Version: pkgVer, - Channel: pkgChan, - InstallNamespace: installNamespace, - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: serviceAccount, - }, - }, - } - // Create a cluster extension - err := cl.Create(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake1.0.0"`, cond.Message) - - // Invalid update: can not go to the next major version - clusterExtension.Spec.Version = "2.0.0" - err = cl.Update(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile again - res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Error(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - // TODO: https://github.com/operator-framework/operator-controller/issues/320 - assert.Nil(t, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionFalse, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - assert.Equal(t, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"2.0.0\" in channel \"beta\" found", cond.Message) - - // Valid update skipping one version - clusterExtension.Spec.Version = "1.2.0" - err = cl.Update(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile again - res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.2.0", Version: "1.2.0"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake1.2.0"`, cond.Message) - }) - - t.Run("legacy semantics upgrade constraints enforcement", func(t *testing.T) { - bundle := &ocv1alpha1.BundleMetadata{ - Name: "prometheus.v1.0.0", - Version: "1.0.0", - } - - cl, reconciler := newClientAndReconciler(t, bundle) - - defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() - defer func() { - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) - }() - - pkgName := "prometheus" - pkgVer := "1.0.0" - pkgChan := "beta" - installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: pkgName, - Version: pkgVer, - Channel: pkgChan, - InstallNamespace: installNamespace, - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: serviceAccount, - }, - }, - } - // Create a cluster extension - err := cl.Create(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake1.0.0"`, cond.Message) - - // Invalid update: can not upgrade by skipping a version in the replaces chain - clusterExtension.Spec.Version = "1.2.0" - err = cl.Update(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile again - res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Error(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - // TODO: https://github.com/operator-framework/operator-controller/issues/320 - assert.Nil(t, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionFalse, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - assert.Equal(t, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"1.2.0\" in channel \"beta\" found", cond.Message) - - // Valid update skipping one version - clusterExtension.Spec.Version = "1.0.1" - err = cl.Update(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile again - res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.1", Version: "1.0.1"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake1.0.1"`, cond.Message) - }) - - t.Run("ignore upgrade constraints", func(t *testing.T) { - for _, tt := range []struct { - name string - flagState bool - }{ - { - name: "ForceSemverUpgradeConstraints feature gate enabled", - flagState: true, - }, - { - name: "ForceSemverUpgradeConstraints feature gate disabled", - flagState: false, - }, - } { - t.Run(tt.name, func(t *testing.T) { - bundle := &ocv1alpha1.BundleMetadata{ - Name: "prometheus.v1.0.0", - Version: "1.0.0", - } - - cl, reconciler := newClientAndReconciler(t, bundle) - - defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, tt.flagState)() - defer func() { - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) - }() - - installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: "prometheus", - Version: "1.0.0", - Channel: "beta", - UpgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyIgnore, - InstallNamespace: installNamespace, - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: serviceAccount, - }, - }, - } - // Create a cluster extension - err := cl.Create(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake1.0.0"`, cond.Message) - - // We can go to the next major version when using semver - // as well as to the version which is not next in the channel - // when using legacy constraints - clusterExtension.Spec.Version = "2.0.0" - err = cl.Update(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile again - res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake2.0.0"`, cond.Message) - }) - } - }) -} - -func TestClusterExtensionDowngrade(t *testing.T) { - mockUnpacker := unpacker.(*MockUnpacker) - // Set up the Unpack method to return a result with StateUnpacked - mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*bundledeployment.BundleDeployment")).Return(&source.Result{ - State: source.StatePending, - }, nil) - ctx := context.Background() - - t.Run("enforce upgrade constraints", func(t *testing.T) { - for _, tt := range []struct { - name string - flagState bool - }{ - { - name: "ForceSemverUpgradeConstraints feature gate enabled", - flagState: true, - }, - { - name: "ForceSemverUpgradeConstraints feature gate disabled", - flagState: false, - }, - } { - t.Run(tt.name, func(t *testing.T) { - bundle := &ocv1alpha1.BundleMetadata{ - Name: "prometheus.v1.0.1", - Version: "1.0.1", - } - - cl, reconciler := newClientAndReconciler(t, bundle) - - defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, tt.flagState)() - defer func() { - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) - }() - - installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: "prometheus", - Version: "1.0.1", - Channel: "beta", - InstallNamespace: installNamespace, - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: serviceAccount, - }, - }, - } - // Create a cluster extension - err := cl.Create(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.1", Version: "1.0.1"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake1.0.1"`, cond.Message) - - // Invalid operation: can not downgrade - clusterExtension.Spec.Version = "1.0.0" - err = cl.Update(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile again - res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Error(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - // TODO: https://github.com/operator-framework/operator-controller/issues/320 - assert.Nil(t, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionFalse, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - assert.Equal(t, "error upgrading from currently installed version \"1.0.1\": no package \"prometheus\" matching version \"1.0.0\" in channel \"beta\" found", cond.Message) - }) - } - }) - - t.Run("ignore upgrade constraints", func(t *testing.T) { - for _, tt := range []struct { - name string - flagState bool - }{ - { - name: "ForceSemverUpgradeConstraints feature gate enabled", - flagState: true, - }, - { - name: "ForceSemverUpgradeConstraints feature gate disabled", - flagState: false, - }, - } { - t.Run(tt.name, func(t *testing.T) { - bundle := &ocv1alpha1.BundleMetadata{ - Name: "prometheus.v2.0.0", - Version: "2.0.0", - } - - cl, reconciler := newClientAndReconciler(t, bundle) - defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, tt.flagState)() - defer func() { - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) - }() - - installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: "prometheus", - Version: "2.0.0", - Channel: "beta", - UpgradeConstraintPolicy: ocv1alpha1.UpgradeConstraintPolicyIgnore, - InstallNamespace: installNamespace, - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: serviceAccount, - }, - }, - } - // Create a cluster extension - err := cl.Create(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake2.0.0"`, cond.Message) - - // We downgrade - clusterExtension.Spec.Version = "1.0.0" - err = cl.Update(ctx, clusterExtension) - require.NoError(t, err) - - // Run reconcile again - res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.NoError(t, err) - assert.Equal(t, ctrl.Result{}, res) - - // Refresh the cluster extension after reconcile - err = cl.Get(ctx, extKey, clusterExtension) - require.NoError(t, err) - - // Checking the status fields - assert.Equal(t, &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) - - // checking the expected conditions - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Equal(t, `resolved to "quay.io/operatorhubio/prometheus@fake1.0.0"`, cond.Message) - }) - } - }) -} - func TestSetDeprecationStatus(t *testing.T) { for _, tc := range []struct { name string clusterExtension *ocv1alpha1.ClusterExtension expectedClusterExtension *ocv1alpha1.ClusterExtension - bundle *catalogmetadata.Bundle + bundle *declcfg.Bundle + deprecation *declcfg.Deprecation }{ { - name: "non-deprecated bundle, no deprecations associated with bundle, all deprecation statuses set to False", + name: "no deprecations, all deprecation statuses set to False", clusterExtension: &ocv1alpha1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, @@ -998,10 +229,11 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, }, - bundle: &catalogmetadata.Bundle{}, + bundle: &declcfg.Bundle{}, + deprecation: nil, }, { - name: "non-deprecated bundle, olm.channel deprecations associated with bundle, no channel specified, all deprecation statuses set to False", + name: "deprecated channel, but no channel specified, all deprecation statuses set to False", clusterExtension: &ocv1alpha1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, @@ -1043,19 +275,18 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, }, - bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaChannel, - Name: "badchannel", - }, + bundle: &declcfg.Bundle{}, + deprecation: &declcfg.Deprecation{ + Entries: []declcfg.DeprecationEntry{{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaChannel, + Name: "badchannel", }, - }, + }}, }, }, { - name: "non-deprecated bundle, olm.channel deprecations associated with bundle, non-deprecated channel specified, all deprecation statuses set to False", + name: "deprecated channel, but a non-deprecated channel specified, all deprecation statuses set to False", clusterExtension: &ocv1alpha1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, @@ -1103,8 +334,9 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, }, - bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + bundle: &declcfg.Bundle{}, + deprecation: &declcfg.Deprecation{ + Entries: []declcfg.DeprecationEntry{ { Reference: declcfg.PackageScopedReference{ Schema: declcfg.SchemaChannel, @@ -1115,7 +347,7 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, { - name: "non-deprecated bundle, olm.channel deprecations associated with bundle, deprecated channel specified, ChannelDeprecated and Deprecated status set to true", + name: "deprecated channel specified, ChannelDeprecated and Deprecated status set to true, others set to false", clusterExtension: &ocv1alpha1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, @@ -1163,8 +395,9 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, }, - bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + bundle: &declcfg.Bundle{}, + deprecation: &declcfg.Deprecation{ + Entries: []declcfg.DeprecationEntry{ { Reference: declcfg.PackageScopedReference{ Schema: declcfg.SchemaChannel, @@ -1176,7 +409,7 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, { - name: "deprecated package + bundle, olm.channel deprecations associated with bundle, deprecated channel specified, all deprecation statuses set to true", + name: "deprecated package and channel specified, deprecated bundle, all deprecation statuses set to true", clusterExtension: &ocv1alpha1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, @@ -1224,8 +457,9 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, }, - bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + bundle: &declcfg.Bundle{Name: "badbundle"}, + deprecation: &declcfg.Deprecation{ + Entries: []declcfg.DeprecationEntry{ { Reference: declcfg.PackageScopedReference{ Schema: declcfg.SchemaChannel, @@ -1250,7 +484,7 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, { - name: "deprecated bundle, olm.channel deprecations associated with bundle, deprecated channel specified, all deprecation statuses set to true except PackageDeprecated", + name: "deprecated channel specified, deprecated bundle, all deprecation statuses set to true, all deprecation statuses set to true except PackageDeprecated", clusterExtension: &ocv1alpha1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, @@ -1298,8 +532,9 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, }, - bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + bundle: &declcfg.Bundle{Name: "badbundle"}, + deprecation: &declcfg.Deprecation{ + Entries: []declcfg.DeprecationEntry{ { Reference: declcfg.PackageScopedReference{ Schema: declcfg.SchemaChannel, @@ -1318,7 +553,7 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, { - name: "deprecated package, olm.channel deprecations associated with bundle, deprecated channel specified, all deprecation statuses set to true except BundleDeprecated", + name: "deprecated package and channel specified, all deprecation statuses set to true except BundleDeprecated", clusterExtension: &ocv1alpha1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, @@ -1366,8 +601,9 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, }, - bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + bundle: &declcfg.Bundle{}, + deprecation: &declcfg.Deprecation{ + Entries: []declcfg.DeprecationEntry{ { Reference: declcfg.PackageScopedReference{ Schema: declcfg.SchemaChannel, @@ -1386,155 +622,10 @@ func TestSetDeprecationStatus(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - controllers.SetDeprecationStatus(tc.clusterExtension, tc.bundle) + controllers.SetDeprecationStatus(tc.clusterExtension, tc.bundle.Name, tc.deprecation) + // TODO: we should test for unexpected changes to lastTransitionTime. We only expect + // lastTransitionTime to change when the status of the condition changes. assert.Equal(t, "", cmp.Diff(tc.expectedClusterExtension, tc.clusterExtension, cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime"))) }) } } - -var ( - prometheusAlphaChannel = catalogmetadata.Channel{ - Channel: declcfg.Channel{ - Name: "alpha", - Package: "prometheus", - }, - } - prometheusBetaChannel = catalogmetadata.Channel{ - Channel: declcfg.Channel{ - Name: "beta", - Package: "prometheus", - Entries: []declcfg.ChannelEntry{ - { - Name: "prometheus.v1.0.0", - }, - { - Name: "prometheus.v1.0.1", - Replaces: "prometheus.v1.0.0", - }, - { - Name: "prometheus.v1.2.0", - Replaces: "prometheus.v1.0.1", - }, - { - Name: "prometheus.v2.0.0", - Replaces: "prometheus.v1.2.0", - }, - }, - }, - } -) - -var testBundleList = []*catalogmetadata.Bundle{ - { - Bundle: declcfg.Bundle{ - Name: "prometheus.v0.37.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.37.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusAlphaChannel}, - }, - { - Bundle: declcfg.Bundle{ - Name: "prometheus.v1.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, - { - Bundle: declcfg.Bundle{ - Name: "prometheus.v1.0.1", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.0.1", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.1"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, - { - Bundle: declcfg.Bundle{ - Name: "prometheus.v1.2.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake1.2.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.2.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, - { - Bundle: declcfg.Bundle{ - Name: "prometheus.v2.0.0", - Package: "prometheus", - Image: "quay.io/operatorhubio/prometheus@fake2.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"2.0.0"}`)}, - {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, - }, - }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, - }, -} - -func TestClusterExtensionErrorGettingBundles(t *testing.T) { - ctx := context.Background() - fakeBundleProvider := testutil.NewFakeCatalogClientWithError(errors.New("fake-test-error")) - cl, reconciler := newClientAndReconciler(t, nil) - reconciler.BundleProvider = &fakeBundleProvider - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - - t.Log("Creating a test cluster extension object") - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: "prometheus", - InstallNamespace: "default", - ServiceAccount: ocv1alpha1.ServiceAccountReference{ - Name: "default", - }, - }, - } - require.NoError(t, cl.Create(ctx, clusterExtension)) - defer func() { - require.NoError(t, cl.Delete(ctx, &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - })) - }() - - t.Log("Running reconcile") - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Equal(t, ctrl.Result{}, res) - require.ErrorContains(t, err, "error fetching bundles: fake-test-error") - - t.Log("Fetching updated cluster extension after reconcile") - require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) - - t.Log("Checking the status fields") - require.Empty(t, clusterExtension.Status.ResolvedBundle) - require.Empty(t, clusterExtension.Status.InstalledBundle) - - t.Log("Checking the expected conditions") - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - require.Contains(t, cond.Message, "error fetching bundles: fake-test-error") - - verifyInvariants(ctx, t, reconciler.Client, clusterExtension) -} diff --git a/internal/controllers/clusterextension_registryv1_validation_test.go b/internal/controllers/clusterextension_registryv1_validation_test.go index a04804587..dcfceedff 100644 --- a/internal/controllers/clusterextension_registryv1_validation_test.go +++ b/internal/controllers/clusterextension_registryv1_validation_test.go @@ -6,6 +6,7 @@ import ( "fmt" "testing" + bsemver "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -20,10 +21,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/property" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" + "github.com/operator-framework/operator-controller/internal/bundleutil" "github.com/operator-framework/operator-controller/internal/controllers" + "github.com/operator-framework/operator-controller/internal/resolve" "github.com/operator-framework/operator-controller/internal/rukpak/source" - "github.com/operator-framework/operator-controller/internal/testutil" ) func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { @@ -32,68 +33,56 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { for _, tt := range []struct { name string - bundle *catalogmetadata.Bundle + bundle declcfg.Bundle wantErr string }{ { name: "package with no dependencies", - bundle: &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "fake-catalog/no-dependencies-package/alpha/1.0.0", - Package: "no-dependencies-package", - Image: "quay.io/fake-catalog/no-dependencies-package@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"no-dependencies-package","version":"1.0.0"}`)}, - }, + bundle: declcfg.Bundle{ + Name: "fake-catalog/no-dependencies-package/alpha/1.0.0", + Package: "no-dependencies-package", + Image: "quay.io/fake-catalog/no-dependencies-package@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"no-dependencies-package","version":"1.0.0"}`)}, }, - CatalogName: "fake-catalog", }, }, { name: "package with olm.package.required property", - bundle: &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "fake-catalog/package-required-test/alpha/1.0.0", - Package: "package-required-test", - Image: "quay.io/fake-catalog/package-required-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"package-required-test","version":"1.0.0"}`)}, - {Type: property.TypePackageRequired, Value: json.RawMessage("content-is-not-relevant")}, - }, + bundle: declcfg.Bundle{ + Name: "fake-catalog/package-required-test/alpha/1.0.0", + Package: "package-required-test", + Image: "quay.io/fake-catalog/package-required-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"package-required-test","version":"1.0.0"}`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage("content-is-not-relevant")}, }, - CatalogName: "fake-catalog", }, wantErr: `bundle "fake-catalog/package-required-test/alpha/1.0.0" has a dependency declared via property "olm.package.required" which is currently not supported`, }, { name: "package with olm.gvk.required property", - bundle: &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "fake-catalog/gvk-required-test/alpha/1.0.0", - Package: "gvk-required-test", - Image: "quay.io/fake-catalog/gvk-required-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"gvk-required-test","version":"1.0.0"}`)}, - {Type: property.TypeGVKRequired, Value: json.RawMessage(`content-is-not-relevant`)}, - }, + bundle: declcfg.Bundle{ + Name: "fake-catalog/gvk-required-test/alpha/1.0.0", + Package: "gvk-required-test", + Image: "quay.io/fake-catalog/gvk-required-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"gvk-required-test","version":"1.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`content-is-not-relevant`)}, }, - CatalogName: "fake-catalog", }, wantErr: `bundle "fake-catalog/gvk-required-test/alpha/1.0.0" has a dependency declared via property "olm.gvk.required" which is currently not supported`, }, { name: "package with olm.constraint property", - bundle: &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{ - Name: "fake-catalog/constraint-test/alpha/1.0.0", - Package: "constraint-test", - Image: "quay.io/fake-catalog/constraint-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"constraint-test","version":"1.0.0"}`)}, - {Type: property.TypeConstraint, Value: json.RawMessage(`content-is-not-relevant`)}, - }, + bundle: declcfg.Bundle{ + Name: "fake-catalog/constraint-test/alpha/1.0.0", + Package: "constraint-test", + Image: "quay.io/fake-catalog/constraint-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"constraint-test","version":"1.0.0"}`)}, + {Type: property.TypeConstraint, Value: json.RawMessage(`content-is-not-relevant`)}, }, - CatalogName: "fake-catalog", }, wantErr: `bundle "fake-catalog/constraint-test/alpha/1.0.0" has a dependency declared via property "olm.constraint" which is currently not supported`, }, @@ -102,21 +91,25 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) }() - fakeCatalogClient := testutil.NewFakeCatalogClient([]*catalogmetadata.Bundle{tt.bundle}) + resolver := resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + v, err := bundleutil.GetVersion(tt.bundle) + if err != nil { + return nil, nil, nil, err + } + return &tt.bundle, v, nil, nil + }) mockUnpacker := unpacker.(*MockUnpacker) // Set up the Unpack method to return a result with StatePending mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ State: source.StatePending, }, nil) - // Create and configure the mock InstalledBundleGetter - mockInstalledBundleGetter := &MockInstalledBundleGetter{} reconciler := &controllers.ClusterExtensionReconciler{ Client: cl, - BundleProvider: &fakeCatalogClient, + Resolver: resolver, ActionClientGetter: helmClientGetter, Unpacker: unpacker, - InstalledBundleGetter: mockInstalledBundleGetter, + InstalledBundleGetter: &MockInstalledBundleGetter{}, Finalizers: crfinalizer.NewFinalizers(), } diff --git a/internal/controllers/common_controller.go b/internal/controllers/common_controller.go index 8b07f4049..5900ced30 100644 --- a/internal/controllers/common_controller.go +++ b/internal/controllers/common_controller.go @@ -17,21 +17,12 @@ limitations under the License. package controllers import ( - "context" - apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) -// BundleProvider provides the way to retrieve a list of Bundles from a source, -// generally from a catalog client of some kind. -type BundleProvider interface { - Bundles(ctx context.Context, packageName string) ([]*catalogmetadata.Bundle, error) -} - // setResolvedStatusConditionSuccess sets the resolved status condition to success. func setResolvedStatusConditionSuccess(ext *ocv1alpha1.ClusterExtension, message string) { apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ diff --git a/internal/controllers/successors.go b/internal/controllers/successors.go deleted file mode 100644 index 0076b7c20..000000000 --- a/internal/controllers/successors.go +++ /dev/null @@ -1,80 +0,0 @@ -package controllers - -import ( - "fmt" - - mmsemver "github.com/Masterminds/semver/v3" - - ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" - catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" - "github.com/operator-framework/operator-controller/pkg/features" -) - -func SuccessorsPredicate(packageName string, installedBundle *ocv1alpha1.BundleMetadata) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { - var successors successorsPredicateFunc = legacySemanticsSuccessorsPredicate - if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) { - successors = semverSuccessorsPredicate - } - - installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) - if err != nil { - return nil, err - } - - installedVersionConstraint, err := mmsemver.NewConstraint(installedBundleVersion.String()) - if err != nil { - return nil, err - } - - successorsPredicate, err := successors(packageName, installedBundle) - if err != nil { - return nil, err - } - - // We need either successors or current version (no upgrade) - return catalogfilter.Or( - successorsPredicate, - catalogfilter.And( - catalogfilter.WithPackageName(packageName), - catalogfilter.InMastermindsSemverRange(installedVersionConstraint), - ), - ), nil -} - -// successorsPredicateFunc returns a predicate to find successors -// for a bundle. Predicate must not include the current version. -type successorsPredicateFunc func(packageName string, bundle *ocv1alpha1.BundleMetadata) (catalogfilter.Predicate[catalogmetadata.Bundle], error) - -// legacySemanticsSuccessorsPredicate returns a predicate to find successors -// based on legacy OLMv0 semantics which rely on Replaces, Skips and skipRange. -func legacySemanticsSuccessorsPredicate(packageName string, bundle *ocv1alpha1.BundleMetadata) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { - // find the bundles that replace, skip, or skipRange the bundle provided - return catalogfilter.And( - catalogfilter.WithPackageName(packageName), - catalogfilter.LegacySuccessor(bundle), - ), nil -} - -// semverSuccessorsPredicate returns a predicate to find successors based on Semver. -// Successors will not include versions outside the major version of the -// installed bundle as major version is intended to indicate breaking changes. -func semverSuccessorsPredicate(packageName string, bundle *ocv1alpha1.BundleMetadata) (catalogfilter.Predicate[catalogmetadata.Bundle], error) { - currentVersion, err := mmsemver.NewVersion(bundle.Version) - if err != nil { - return nil, err - } - - // Based on current version create a caret range comparison constraint - // to allow only minor and patch version as successors and exclude current version. - constraintStr := fmt.Sprintf("^%s, != %s", currentVersion.String(), currentVersion.String()) - wantedVersionRangeConstraint, err := mmsemver.NewConstraint(constraintStr) - if err != nil { - return nil, err - } - - return catalogfilter.And( - catalogfilter.WithPackageName(packageName), - catalogfilter.InMastermindsSemverRange(wantedVersionRangeConstraint), - ), nil -} diff --git a/internal/controllers/successors_test.go b/internal/controllers/successors_test.go deleted file mode 100644 index 946dc2a19..000000000 --- a/internal/controllers/successors_test.go +++ /dev/null @@ -1,438 +0,0 @@ -package controllers_test - -import ( - "encoding/json" - "sort" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - featuregatetesting "k8s.io/component-base/featuregate/testing" - - "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" - - ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/catalogmetadata" - catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" - catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" - "github.com/operator-framework/operator-controller/internal/controllers" - "github.com/operator-framework/operator-controller/pkg/features" -) - -func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() - - const testPackageName = "test-package" - someOtherPackageChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "stable", - Package: "some-other-package", - }} - testPackageChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "stable", - Package: testPackageName, - }} - bundleSet := map[string]*catalogmetadata.Bundle{ - // Major version zero is for initial development and - // has different update behaviour than versions >= 1.0.0: - // - In versions 0.0.y updates are not allowed when using semver constraints - // - In versions 0.x.y only patch updates are allowed (>= 0.x.y and < 0.x+1.0) - // This means that we need in test data bundles that cover these three version ranges. - "test-package.v0.0.1": { - Bundle: declcfg.Bundle{ - Name: "test-package.v0.0.1", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.0.1", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.0.1"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v0.0.2": { - Bundle: declcfg.Bundle{ - Name: "test-package.v0.0.2", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.0.2", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.0.2"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v0.1.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v0.1.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.1.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.1.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v0.1.1": { - Bundle: declcfg.Bundle{ - Name: "test-package.v0.1.1", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.1.1", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.1.1"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v0.1.2": { - Bundle: declcfg.Bundle{ - Name: "test-package.v0.1.2", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.1.2", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.1.2"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v0.2.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v0.2.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v0.2.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "0.2.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v2.0.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.0.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v2.1.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.1.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.1.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.1.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v2.2.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.2.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.2.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.2.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - // We need a bundle with a different major version to ensure - // that we do not allow upgrades from one major version to another - "test-package.v3.0.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v3.0.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v3.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - // We need a bundle from different package to ensure that - // we filter out bundles certain bundle image - "some-other-package.v2.3.0": { - Bundle: declcfg.Bundle{ - Name: "some-other-package.v2.3.0", - Package: "some-other-package", - Image: "registry.io/repo/some-other-package@v2.3.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-other-package", "version": "2.3.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&someOtherPackageChannel}, - }, - } - allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet)) - for _, bundle := range bundleSet { - allBundles = append(allBundles, bundle) - } - - for _, tt := range []struct { - name string - installedBundle *ocv1alpha1.BundleMetadata - expectedResult []*catalogmetadata.Bundle - }{ - { - name: "with non-zero major version", - installedBundle: bundleMetadataFor(bundleSet["test-package.v2.0.0"]), - expectedResult: []*catalogmetadata.Bundle{ - // Updates are allowed within the major version - bundleSet["test-package.v2.2.0"], - bundleSet["test-package.v2.1.0"], - bundleSet["test-package.v2.0.0"], - }, - }, - { - name: "with zero major and zero minor version", - installedBundle: bundleMetadataFor(bundleSet["test-package.v0.0.1"]), - expectedResult: []*catalogmetadata.Bundle{ - // No updates are allowed in major version zero when minor version is also zero - bundleSet["test-package.v0.0.1"], - }, - }, - { - name: "with zero major and non-zero minor version", - installedBundle: bundleMetadataFor(bundleSet["test-package.v0.1.0"]), - expectedResult: []*catalogmetadata.Bundle{ - // Patch version updates are allowed within the minor version - bundleSet["test-package.v0.1.2"], - bundleSet["test-package.v0.1.1"], - bundleSet["test-package.v0.1.0"], - }, - }, - { - name: "installed bundle not found", - installedBundle: &ocv1alpha1.BundleMetadata{ - Name: "test-package.v9.0.0", - Version: "9.0.0", - }, - expectedResult: []*catalogmetadata.Bundle{}, - }, - } { - t.Run(tt.name, func(t *testing.T) { - successors, err := controllers.SuccessorsPredicate("test-package", tt.installedBundle) - assert.NoError(t, err) - result := catalogfilter.Filter(allBundles, successors) - - // sort before comparison for stable order - sort.SliceStable(result, func(i, j int) bool { - return catalogsort.ByVersion(result[i], result[j]) - }) - - gocmpopts := []cmp.Option{ - cmpopts.IgnoreUnexported(catalogmetadata.Bundle{}), - } - require.Empty(t, cmp.Diff(result, tt.expectedResult, gocmpopts...)) - }) - } -} - -func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() - - const testPackageName = "test-package" - someOtherPackageChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "stable", - Package: "some-other-package", - Entries: []declcfg.ChannelEntry{ - { - Name: "some-other-package.v2.3.0", - }, - }, - }} - testPackageChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "stable", - Package: testPackageName, - Entries: []declcfg.ChannelEntry{ - { - Name: "test-package.v2.0.0", - }, - { - Name: "test-package.v2.1.0", - Replaces: "test-package.v2.0.0", - }, - { - Name: "test-package.v2.2.0", - Replaces: "test-package.v2.1.0", - }, - { - Name: "test-package.v2.2.1", - }, - { - Name: "test-package.v2.3.0", - Replaces: "test-package.v2.2.0", - Skips: []string{ - "test-package.v2.2.1", - }, - }, - { - Name: "test-package.v2.4.0", - SkipRange: ">=2.3.0 <2.4.0", - }, - }, - }} - bundleSet := map[string]*catalogmetadata.Bundle{ - "test-package.v2.0.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.0.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.0.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v2.1.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.1.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.1.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.1.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v2.2.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.2.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.2.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.2.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v2.2.1": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.2.1", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.2.1", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.2.1"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v2.3.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.3.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.3.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.3.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - "test-package.v2.4.0": { - Bundle: declcfg.Bundle{ - Name: "test-package.v2.4.0", - Package: testPackageName, - Image: "registry.io/repo/test-package@v2.4.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.4.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, - }, - // We need a bundle from different package to ensure that - // we filter out certain bundle image - "some-other-package.v2.3.0": { - Bundle: declcfg.Bundle{ - Name: "some-other-package.v2.3.0", - Package: "some-other-package", - Image: "registry.io/repo/some-other-package@v2.3.0", - Properties: []property.Property{ - {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-other-package", "version": "2.3.0"}`)}, - }, - }, - InChannels: []*catalogmetadata.Channel{&someOtherPackageChannel}, - }, - } - allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet)) - for _, bundle := range bundleSet { - allBundles = append(allBundles, bundle) - } - - for _, tt := range []struct { - name string - installedBundle *ocv1alpha1.BundleMetadata - expectedResult []*catalogmetadata.Bundle - }{ - { - name: "respect replaces directive from catalog", - installedBundle: bundleMetadataFor(bundleSet["test-package.v2.0.0"]), - expectedResult: []*catalogmetadata.Bundle{ - // Must only have two bundle: - // - the one which replaces the current version - // - the current version (to allow to stay on the current version) - bundleSet["test-package.v2.1.0"], - bundleSet["test-package.v2.0.0"], - }, - }, - { - name: "respect skips directive from catalog", - installedBundle: bundleMetadataFor(bundleSet["test-package.v2.2.1"]), - expectedResult: []*catalogmetadata.Bundle{ - // Must only have two bundle: - // - the one which skips the current version - // - the current version (to allow to stay on the current version) - bundleSet["test-package.v2.3.0"], - bundleSet["test-package.v2.2.1"], - }, - }, - { - name: "respect skipRange directive from catalog", - installedBundle: bundleMetadataFor(bundleSet["test-package.v2.3.0"]), - expectedResult: []*catalogmetadata.Bundle{ - // Must only have two bundle: - // - the one which is skipRanges the current version - // - the current version (to allow to stay on the current version) - bundleSet["test-package.v2.4.0"], - bundleSet["test-package.v2.3.0"], - }, - }, - { - name: "installed bundle not found", - installedBundle: &ocv1alpha1.BundleMetadata{ - Name: "test-package.v9.0.0", - Version: "9.0.0", - }, - expectedResult: []*catalogmetadata.Bundle{}, - }, - } { - t.Run(tt.name, func(t *testing.T) { - successors, err := controllers.SuccessorsPredicate("test-package", tt.installedBundle) - assert.NoError(t, err) - result := catalogfilter.Filter(allBundles, successors) - - // sort before comparison for stable order - sort.SliceStable(result, func(i, j int) bool { - return catalogsort.ByVersion(result[i], result[j]) - }) - - gocmpopts := []cmp.Option{ - cmpopts.IgnoreUnexported(catalogmetadata.Bundle{}), - } - require.Empty(t, cmp.Diff(result, tt.expectedResult, gocmpopts...)) - }) - } -} - -func bundleMetadataFor(bundle *catalogmetadata.Bundle) *ocv1alpha1.BundleMetadata { - if bundle == nil { - return nil - } - v, _ := bundle.Version() - return &ocv1alpha1.BundleMetadata{ - Name: bundle.Name, - Version: v.String(), - } -} diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index c3dd97d5f..75d157650 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -38,8 +39,6 @@ import ( "github.com/operator-framework/operator-controller/internal/controllers" bd "github.com/operator-framework/operator-controller/internal/rukpak/bundledeployment" "github.com/operator-framework/operator-controller/internal/rukpak/source" - "github.com/operator-framework/operator-controller/internal/testutil" - "github.com/operator-framework/operator-controller/pkg/scheme" ) // MockUnpacker is a mock of Unpacker interface @@ -59,7 +58,11 @@ func (m *MockUnpacker) Cleanup(ctx context.Context, bundle *bd.BundleDeployment) } func newClient(t *testing.T) client.Client { - cl, err := client.New(config, client.Options{Scheme: scheme.Scheme}) + // TODO: this is a live client, which behaves differently than a cache client. + // We may want to use a caching client instead to get closer to real behavior. + sch := runtime.NewScheme() + require.NoError(t, ocv1alpha1.AddToScheme(sch)) + cl, err := client.New(config, client.Options{Scheme: sch}) require.NoError(t, err) require.NotNil(t, cl) return cl @@ -79,17 +82,12 @@ func (m *MockInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext func newClientAndReconciler(t *testing.T, bundle *ocv1alpha1.BundleMetadata) (client.Client, *controllers.ClusterExtensionReconciler) { cl := newClient(t) - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) - - mockInstalledBundleGetter := &MockInstalledBundleGetter{} - mockInstalledBundleGetter.SetBundle(bundle) reconciler := &controllers.ClusterExtensionReconciler{ Client: cl, - BundleProvider: &fakeCatalogClient, ActionClientGetter: helmClientGetter, Unpacker: unpacker, - InstalledBundleGetter: mockInstalledBundleGetter, + InstalledBundleGetter: &MockInstalledBundleGetter{bundle}, Finalizers: crfinalizer.NewFinalizers(), } return cl, reconciler diff --git a/internal/resolve/catalog.go b/internal/resolve/catalog.go new file mode 100644 index 000000000..d4f920425 --- /dev/null +++ b/internal/resolve/catalog.go @@ -0,0 +1,168 @@ +package resolve + +import ( + "context" + "fmt" + "slices" + + mmsemver "github.com/Masterminds/semver/v3" + bsemver "github.com/blang/semver/v4" + "sigs.k8s.io/controller-runtime/pkg/client" + + catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" + "github.com/operator-framework/operator-registry/alpha/declcfg" + + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/bundleutil" + "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" + "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" +) + +type CatalogResolver struct { + WalkCatalogsFunc func(context.Context, string, CatalogWalkFunc, ...client.ListOption) error +} + +// Resolve returns a Bundle from a catalog that needs to get installed on the cluster. +func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1alpha1.ClusterExtension, installedBundle *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + packageName := ext.Spec.PackageName + versionRange := ext.Spec.Version + channelName := ext.Spec.Channel + + var versionRangeConstraints *mmsemver.Constraints + if versionRange != "" { + var err error + versionRangeConstraints, err = mmsemver.NewConstraint(versionRange) + if err != nil { + return nil, nil, nil, fmt.Errorf("desired version range %q is invalid: %w", versionRange, err) + } + } + + var ( + resolvedBundle *declcfg.Bundle + resolvedDeprecation *declcfg.Deprecation + ) + + if err := r.WalkCatalogsFunc(ctx, packageName, func(ctx context.Context, cat *catalogd.ClusterCatalog, packageFBC *declcfg.DeclarativeConfig, err error) error { + if err != nil { + return fmt.Errorf("error getting package %q from catalog %q: %w", packageName, cat.Name, err) + } + + var predicates []filter.Predicate[declcfg.Bundle] + if channelName != "" { + channels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool { + return channelName != "" && c.Name != channelName + }) + predicates = append(predicates, filter.InAnyChannel(channels...)) + } + + if versionRangeConstraints != nil { + predicates = append(predicates, filter.InMastermindsSemverRange(versionRangeConstraints)) + } + + if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore && installedBundle != nil { + successorPredicate, err := filter.SuccessorsOf(installedBundle, packageFBC.Channels...) + if err != nil { + return fmt.Errorf("error finding upgrade edges: %w", err) + } + predicates = append(predicates, successorPredicate) + } + + // Apply the predicates to get the candidate bundles + packageFBC.Bundles = filter.Filter(packageFBC.Bundles, filter.And(predicates...)) + if len(packageFBC.Bundles) == 0 { + return nil + } + + // If this package has a deprecation, we: + // 1. Want to sort deprecated bundles to the end of the list + // 2. Want to keep track of it so that we can return it if we end + // up resolving a bundle from this package. + byDeprecation := func(a, b declcfg.Bundle) int { return 0 } + var thisDeprecation *declcfg.Deprecation + if len(packageFBC.Deprecations) > 0 { + thisDeprecation = &packageFBC.Deprecations[0] + byDeprecation = compare.ByDeprecationFunc(*thisDeprecation) + } + + // Sort the bundles by deprecation and then by version + slices.SortStableFunc(packageFBC.Bundles, func(a, b declcfg.Bundle) int { + if lessDep := byDeprecation(a, b); lessDep != 0 { + return lessDep + } + return compare.ByVersion(a, b) + }) + + thisBundle := packageFBC.Bundles[0] + if resolvedBundle != nil { + // Cases where we stick with `resolvedBundle`: + // 1. If `thisBundle` is deprecated and `resolvedBundle` is not + // 2. If `thisBundle` and `resolvedBundle` have the same deprecation status AND `resolvedBundle` is a higher version + if isDeprecated(thisBundle, thisDeprecation) && !isDeprecated(*resolvedBundle, resolvedDeprecation) { + return nil + } + if compare.ByVersion(*resolvedBundle, thisBundle) < 0 { + return nil + } + } + resolvedBundle = &thisBundle + resolvedDeprecation = thisDeprecation + return nil + }); err != nil { + return nil, nil, nil, fmt.Errorf("error walking catalogs: %w", err) + } + + if resolvedBundle == nil { + errPrefix := "" + if installedBundle != nil { + errPrefix = fmt.Sprintf("error upgrading from currently installed version %q: ", installedBundle.Version) + } + switch { + case versionRange != "" && channelName != "": + return nil, nil, nil, fmt.Errorf("%sno package %q matching version %q in channel %q found", errPrefix, packageName, versionRange, channelName) + case versionRange != "": + return nil, nil, nil, fmt.Errorf("%sno package %q matching version %q found", errPrefix, packageName, versionRange) + case channelName != "": + return nil, nil, nil, fmt.Errorf("%sno package %q in channel %q found", errPrefix, packageName, channelName) + default: + return nil, nil, nil, fmt.Errorf("%sno package %q found", errPrefix, packageName) + } + } + + resolvedBundleVersion, err := bundleutil.GetVersion(*resolvedBundle) + if err != nil { + return nil, nil, nil, fmt.Errorf("error getting resolved bundle version for bundle %q: %w", resolvedBundle.Name, err) + } + return resolvedBundle, resolvedBundleVersion, resolvedDeprecation, nil +} + +func isDeprecated(bundle declcfg.Bundle, deprecation *declcfg.Deprecation) bool { + if deprecation == nil { + return false + } + for _, entry := range deprecation.Entries { + if entry.Reference.Schema == declcfg.SchemaBundle && entry.Reference.Name == bundle.Name { + return true + } + } + return false +} + +type CatalogWalkFunc func(context.Context, *catalogd.ClusterCatalog, *declcfg.DeclarativeConfig, error) error + +func CatalogWalker(listCatalogs func(context.Context, ...client.ListOption) ([]catalogd.ClusterCatalog, error), getPackage func(context.Context, *catalogd.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error)) func(ctx context.Context, packageName string, f CatalogWalkFunc, catalogListOpts ...client.ListOption) error { + return func(ctx context.Context, packageName string, f CatalogWalkFunc, catalogListOpts ...client.ListOption) error { + catalogs, err := listCatalogs(ctx, catalogListOpts...) + if err != nil { + return fmt.Errorf("error listing catalogs: %w", err) + } + + for i := range catalogs { + cat := &catalogs[i] + fbc, fbcErr := getPackage(ctx, cat, packageName) + if walkErr := f(ctx, cat, fbc, fbcErr); walkErr != nil { + return walkErr + } + } + return nil + } +} diff --git a/internal/resolve/catalog_test.go b/internal/resolve/catalog_test.go new file mode 100644 index 000000000..749710ffa --- /dev/null +++ b/internal/resolve/catalog_test.go @@ -0,0 +1,569 @@ +package resolve + +import ( + "context" + "fmt" + "testing" + + bsemver "github.com/blang/semver/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/sets" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/pkg/features" +) + +func TestInvalidClusterExtensionVersionRange(t *testing.T) { + r := CatalogResolver{} + pkgName := randPkg() + ce := buildFooClusterExtension(pkgName, "", "foobar", ocv1alpha1.UpgradeConstraintPolicyEnforce) + _, _, _, err := r.Resolve(context.Background(), ce, nil) + assert.EqualError(t, err, `desired version range "foobar" is invalid: improper constraint: foobar`) +} + +func TestErrorWalkingCatalogs(t *testing.T) { + r := CatalogResolver{WalkCatalogsFunc: func(context.Context, string, CatalogWalkFunc, ...client.ListOption) error { + return fmt.Errorf("fake error") + }} + pkgName := randPkg() + ce := buildFooClusterExtension(pkgName, "", "", ocv1alpha1.UpgradeConstraintPolicyEnforce) + _, _, _, err := r.Resolve(context.Background(), ce, nil) + assert.EqualError(t, err, "error walking catalogs: fake error") +} + +func TestErrorGettingPackage(t *testing.T) { + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return nil, fmt.Errorf("fake error") }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + pkgName := randPkg() + ce := buildFooClusterExtension(pkgName, "", "", ocv1alpha1.UpgradeConstraintPolicyEnforce) + _, _, _, err := r.Resolve(context.Background(), ce, nil) + assert.EqualError(t, err, fmt.Sprintf(`error walking catalogs: error getting package %q from catalog "a": fake error`, pkgName)) +} + +func TestPackageDoesNotExist(t *testing.T) { + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + pkgName := randPkg() + ce := buildFooClusterExtension(pkgName, "", "", ocv1alpha1.UpgradeConstraintPolicyEnforce) + _, _, _, err := r.Resolve(context.Background(), ce, nil) + assert.EqualError(t, err, fmt.Sprintf(`no package %q found`, pkgName)) +} + +func TestPackageExists(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", "", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "2.0.0"), *gotBundle) + assert.Equal(t, bsemver.MustParse("2.0.0"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} + +func TestVersionDoesNotExist(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", "3.0.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + _, _, _, err := r.Resolve(context.Background(), ce, nil) + assert.EqualError(t, err, fmt.Sprintf(`no package %q matching version "3.0.0" found`, pkgName)) +} + +func TestVersionExists(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", ">=1.0.0 <2.0.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle) + assert.Equal(t, bsemver.MustParse("1.0.2"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} + +func TestChannelDoesNotExist(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "stable", "", ocv1alpha1.UpgradeConstraintPolicyEnforce) + _, _, _, err := r.Resolve(context.Background(), ce, nil) + assert.EqualError(t, err, fmt.Sprintf(`no package %q in channel "stable" found`, pkgName)) +} + +func TestChannelExists(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "beta", "", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle) + assert.Equal(t, bsemver.MustParse("1.0.2"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} + +func TestChannelExistsButNotVersion(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "beta", "3.0.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + _, _, _, err := r.Resolve(context.Background(), ce, nil) + assert.EqualError(t, err, fmt.Sprintf(`no package %q matching version "3.0.0" in channel "beta" found`, pkgName)) +} + +func TestVersionExistsButNotChannel(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "stable", "1.0.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + _, _, _, err := r.Resolve(context.Background(), ce, nil) + assert.EqualError(t, err, fmt.Sprintf(`no package %q matching version "1.0.0" in channel "stable" found`, pkgName)) +} + +func TestChannelAndVersionExist(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "alpha", "0.1.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "0.1.0"), *gotBundle) + assert.Equal(t, bsemver.MustParse("0.1.0"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} + +func TestPreferNonDeprecated(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", ">=0.1.0 <=1.0.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "0.1.0"), *gotBundle) + assert.Equal(t, bsemver.MustParse("0.1.0"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} + +func TestAcceptDeprecated(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", ">=1.0.0 <=1.0.1", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "1.0.1"), *gotBundle) + assert.Equal(t, bsemver.MustParse("1.0.1"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} + +func TestPackageVariationsBetweenCatalogs(t *testing.T) { + pkgName := randPkg() + genImgRef := func(catalog, name string) string { + return fmt.Sprintf("%s/%s", catalog, name) + } + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { + fbc := genPackage(pkgName) + fbc.Bundles = append(fbc.Bundles, genBundle(pkgName, "1.0.3")) + for i := range fbc.Bundles { + fbc.Bundles[i].Image = genImgRef("catalog-b", fbc.Bundles[i].Name) + } + return fbc, nil + }, + "c": func() (*declcfg.DeclarativeConfig, error) { + fbc := genPackage(pkgName) + fbc.Bundles = append(fbc.Bundles, genBundle(pkgName, "0.1.1")) + fbc.Deprecations = nil + for i := range fbc.Bundles { + fbc.Bundles[i].Image = genImgRef("catalog-c", fbc.Bundles[i].Name) + } + return fbc, nil + }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + + t.Run("always prefer non-deprecated when versions match", func(t *testing.T) { + for i := 0; i < 100; i++ { + // When the same version exists in both catalogs, we prefer the non-deprecated one. + ce := buildFooClusterExtension(pkgName, "", ">=1.0.0 <=1.0.1", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "1.0.1").Name, gotBundle.Name) + assert.Equal(t, bsemver.MustParse("1.0.1"), *gotVersion) + assert.Nil(t, gotDeprecation) + } + }) + + t.Run("when catalog b has a newer version that matches the range", func(t *testing.T) { + // When one version exists in one catalog but not the other, we prefer the one that exists. + ce := buildFooClusterExtension(pkgName, "", ">=1.0.0 <=1.0.3", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "1.0.3").Name, gotBundle.Name) + assert.Equal(t, genImgRef("catalog-b", gotBundle.Name), gotBundle.Image) + assert.Equal(t, bsemver.MustParse("1.0.3"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) + }) + + t.Run("when catalog c has a newer version that matches the range", func(t *testing.T) { + ce := buildFooClusterExtension(pkgName, "", ">=0.1.0 <1.0.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "0.1.1").Name, gotBundle.Name) + assert.Equal(t, genImgRef("catalog-c", gotBundle.Name), gotBundle.Image) + assert.Equal(t, bsemver.MustParse("0.1.1"), *gotVersion) + assert.Nil(t, gotDeprecation) + }) + + t.Run("when there is ambiguity between catalogs", func(t *testing.T) { + // When there is no way to disambiguate between two versions, the choice is undefined. + foundImages := sets.New[string]() + foundDeprecations := sets.New[*declcfg.Deprecation]() + for i := 0; i < 100; i++ { + ce := buildFooClusterExtension(pkgName, "", "0.1.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "0.1.0").Name, gotBundle.Name) + assert.Equal(t, bsemver.MustParse("0.1.0"), *gotVersion) + foundImages.Insert(gotBundle.Image) + foundDeprecations.Insert(gotDeprecation) + } + assert.ElementsMatch(t, []string{ + genImgRef("catalog-b", bundleName(pkgName, "0.1.0")), + genImgRef("catalog-c", bundleName(pkgName, "0.1.0")), + }, foundImages.UnsortedList()) + + assert.Contains(t, foundDeprecations, (*declcfg.Deprecation)(nil)) + assert.Contains(t, foundDeprecations, ptr.To(packageDeprecation(pkgName))) + }) +} + +func TestUpgradeFoundLegacy(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", "", ocv1alpha1.UpgradeConstraintPolicyEnforce) + installedBundle := &ocv1alpha1.BundleMetadata{ + Name: bundleName(pkgName, "0.1.0"), + Version: "0.1.0", + } + // 0.1.0 => 1.0.2 would not be allowed using semver semantics + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, installedBundle) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle) + assert.Equal(t, bsemver.MustParse("1.0.2"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} + +func TestUpgradeNotFoundLegacy(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", "<1.0.0 >=2.0.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + installedBundle := &ocv1alpha1.BundleMetadata{ + Name: bundleName(pkgName, "0.1.0"), + Version: "0.1.0", + } + // 0.1.0 only upgrades to 1.0.x with its legacy upgrade edges, so this fails. + _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) + assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "0.1.0": no package %q matching version "<1.0.0 >=2.0.0" found`, pkgName)) +} + +func TestUpgradeFoundSemver(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", "", ocv1alpha1.UpgradeConstraintPolicyEnforce) + installedBundle := &ocv1alpha1.BundleMetadata{ + Name: bundleName(pkgName, "1.0.0"), + Version: "1.0.0", + } + // there is a legacy upgrade edge from 1.0.0 to 2.0.0, but we are using semver semantics here. + // therefore: + // 1.0.0 => 1.0.2 is what we expect + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, installedBundle) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle) + assert.Equal(t, bsemver.MustParse("1.0.2"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} +func TestUpgradeNotFoundSemver(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", "!=0.1.0", ocv1alpha1.UpgradeConstraintPolicyEnforce) + installedBundle := &ocv1alpha1.BundleMetadata{ + Name: bundleName(pkgName, "0.1.0"), + Version: "0.1.0", + } + // there are legacy upgrade edges from 0.1.0 to 1.0.x, but we are using semver semantics here. + // therefore, we expect to fail because there are no semver-compatible upgrade edges from 0.1.0. + _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) + assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "0.1.0": no package %q matching version "!=0.1.0" found`, pkgName)) +} + +func TestDowngradeFound(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", "<1.0.2", ocv1alpha1.UpgradeConstraintPolicyIgnore) + installedBundle := &ocv1alpha1.BundleMetadata{ + Name: bundleName(pkgName, "1.0.2"), + Version: "1.0.2", + } + // 1.0.2 => 0.1.0 is a downgrade, but it is allowed because of the upgrade constraint policy. + // note: we chose 0.1.0 because 1.0.0 and 1.0.1 are deprecated. + gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, installedBundle) + require.NoError(t, err) + assert.Equal(t, genBundle(pkgName, "0.1.0"), *gotBundle) + assert.Equal(t, bsemver.MustParse("0.1.0"), *gotVersion) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) +} + +func TestDowngradeNotFound(t *testing.T) { + pkgName := randPkg() + w := staticCatalogWalker{ + "a": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "b": func() (*declcfg.DeclarativeConfig, error) { return &declcfg.DeclarativeConfig{}, nil }, + "c": func() (*declcfg.DeclarativeConfig, error) { return genPackage(pkgName), nil }, + } + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, "", ">0.1.0 <1.0.0", ocv1alpha1.UpgradeConstraintPolicyIgnore) + installedBundle := &ocv1alpha1.BundleMetadata{ + Name: bundleName(pkgName, "1.0.2"), + Version: "1.0.2", + } + // Downgrades are allowed via the upgrade constraint policy, but there is no bundle in the specified range. + _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) + assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "1.0.2": no package %q matching version ">0.1.0 <1.0.0" found`, pkgName)) +} + +func TestCatalogWalker(t *testing.T) { + t.Run("error listing catalogs", func(t *testing.T) { + w := CatalogWalker( + func(ctx context.Context, option ...client.ListOption) ([]catalogd.ClusterCatalog, error) { + return nil, fmt.Errorf("fake error") + }, + func(context.Context, *catalogd.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error) { + return nil, nil + }, + ) + walkFunc := func(ctx context.Context, cat *catalogd.ClusterCatalog, fbc *declcfg.DeclarativeConfig, err error) error { + return nil + } + assert.EqualError(t, w(context.Background(), "", walkFunc), "error listing catalogs: fake error") + }) + + t.Run("error getting package", func(t *testing.T) { + w := CatalogWalker( + func(ctx context.Context, option ...client.ListOption) ([]catalogd.ClusterCatalog, error) { + return []catalogd.ClusterCatalog{{ObjectMeta: metav1.ObjectMeta{Name: "a"}}}, nil + }, + func(context.Context, *catalogd.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error) { + return nil, fmt.Errorf("fake error getting package FBC") + }, + ) + walkFunc := func(ctx context.Context, cat *catalogd.ClusterCatalog, fbc *declcfg.DeclarativeConfig, err error) error { + return err + } + assert.EqualError(t, w(context.Background(), "", walkFunc), "fake error getting package FBC") + }) + + t.Run("success", func(t *testing.T) { + w := CatalogWalker( + func(ctx context.Context, option ...client.ListOption) ([]catalogd.ClusterCatalog, error) { + return []catalogd.ClusterCatalog{ + {ObjectMeta: metav1.ObjectMeta{Name: "a"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "b"}}, + }, nil + }, + func(context.Context, *catalogd.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error) { + return &declcfg.DeclarativeConfig{}, nil + }, + ) + + seenCatalogs := []string{} + walkFunc := func(ctx context.Context, cat *catalogd.ClusterCatalog, fbc *declcfg.DeclarativeConfig, err error) error { + seenCatalogs = append(seenCatalogs, cat.Name) + return nil + } + assert.NoError(t, w(context.Background(), "", walkFunc)) + assert.Equal(t, []string{"a", "b"}, seenCatalogs) + }) +} + +func buildFooClusterExtension(pkg, channel, version string, upgradeConstraintPolicy ocv1alpha1.UpgradeConstraintPolicy) *ocv1alpha1.ClusterExtension { + return &ocv1alpha1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: pkg, + }, + Spec: ocv1alpha1.ClusterExtensionSpec{ + InstallNamespace: "default", + ServiceAccount: ocv1alpha1.ServiceAccountReference{Name: "default"}, + PackageName: pkg, + Channel: channel, + Version: version, + UpgradeConstraintPolicy: upgradeConstraintPolicy, + }, + } +} + +type getPackageFunc func() (*declcfg.DeclarativeConfig, error) + +type staticCatalogWalker map[string]getPackageFunc + +func (w staticCatalogWalker) WalkCatalogs(ctx context.Context, _ string, f CatalogWalkFunc, _ ...client.ListOption) error { + for k, v := range w { + cat := &catalogd.ClusterCatalog{ + ObjectMeta: metav1.ObjectMeta{Name: k}, + } + fbc, fbcErr := v() + if err := f(ctx, cat, fbc, fbcErr); err != nil { + return err + } + } + return nil +} + +func randPkg() string { + return fmt.Sprintf("pkg-%s", rand.String(5)) +} + +func bundleName(pkg, version string) string { + return fmt.Sprintf("%s.v%s", pkg, version) +} + +func genBundle(pkg, version string) declcfg.Bundle { + return declcfg.Bundle{ + Package: pkg, + Name: bundleName(pkg, version), + Properties: []property.Property{ + property.MustBuildPackage(pkg, version), + }, + } +} + +func packageDeprecation(pkg string) declcfg.Deprecation { + return declcfg.Deprecation{ + Package: pkg, + Entries: []declcfg.DeprecationEntry{ + { + Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaBundle, Name: bundleName(pkg, "1.0.0")}, + Message: fmt.Sprintf("bundle %s is deprecated", bundleName(pkg, "1.0.0")), + }, + { + Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaBundle, Name: bundleName(pkg, "1.0.1")}, + Message: fmt.Sprintf("bundle %s is deprecated", bundleName(pkg, "1.0.1")), + }, + }, + } +} + +func genPackage(pkg string) *declcfg.DeclarativeConfig { + return &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{{Name: pkg}}, + Channels: []declcfg.Channel{ + {Package: pkg, Name: "alpha", Entries: []declcfg.ChannelEntry{ + {Name: bundleName(pkg, "0.1.0")}, + {Name: bundleName(pkg, "1.0.0"), SkipRange: "<1.0.0"}, + {Name: bundleName(pkg, "1.0.1"), SkipRange: "<1.0.1"}, + {Name: bundleName(pkg, "1.0.2"), SkipRange: "<1.0.2"}, + {Name: bundleName(pkg, "2.0.0"), SkipRange: ">=1.0.0 <2.0.0"}, + }}, + {Package: pkg, Name: "beta", Entries: []declcfg.ChannelEntry{ + {Name: bundleName(pkg, "0.1.0")}, + {Name: bundleName(pkg, "1.0.2"), SkipRange: "<1.0.2"}, + }}, + }, + Bundles: []declcfg.Bundle{ + genBundle(pkg, "0.1.0"), + genBundle(pkg, "1.0.0"), + genBundle(pkg, "1.0.1"), + genBundle(pkg, "1.0.2"), + genBundle(pkg, "2.0.0"), + }, + Deprecations: []declcfg.Deprecation{packageDeprecation(pkg)}, + } +} diff --git a/internal/resolve/resolver.go b/internal/resolve/resolver.go new file mode 100644 index 000000000..de9b952b0 --- /dev/null +++ b/internal/resolve/resolver.go @@ -0,0 +1,21 @@ +package resolve + +import ( + "context" + + bsemver "github.com/blang/semver/v4" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" +) + +type Resolver interface { + Resolve(ctx context.Context, ext *ocv1alpha1.ClusterExtension, installedBundle *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) +} + +type Func func(ctx context.Context, ext *ocv1alpha1.ClusterExtension, installedBundle *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) + +func (f Func) Resolve(ctx context.Context, ext *ocv1alpha1.ClusterExtension, installedBundle *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) { + return f(ctx, ext, installedBundle) +} diff --git a/internal/testutil/fake_catalog_client.go b/internal/testutil/fake_catalog_client.go deleted file mode 100644 index 7e855cb34..000000000 --- a/internal/testutil/fake_catalog_client.go +++ /dev/null @@ -1,38 +0,0 @@ -package testutil - -import ( - "context" - - "github.com/operator-framework/operator-controller/internal/catalogmetadata" -) - -type FakeCatalogClient struct { - bundles []*catalogmetadata.Bundle - err error -} - -func NewFakeCatalogClient(b []*catalogmetadata.Bundle) FakeCatalogClient { - return FakeCatalogClient{ - bundles: b, - } -} - -func NewFakeCatalogClientWithError(e error) FakeCatalogClient { - return FakeCatalogClient{ - err: e, - } -} - -func (c *FakeCatalogClient) Bundles(_ context.Context, packageName string) ([]*catalogmetadata.Bundle, error) { - if c.err != nil { - return nil, c.err - } - - var out []*catalogmetadata.Bundle - for _, b := range c.bundles { - if b.Package == packageName { - out = append(out, b) - } - } - return out, nil -} diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index e08d603e9..cac2edde1 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -119,7 +119,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) { } assert.Equal(ct, metav1.ConditionTrue, cond.Status) assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Contains(ct, cond.Message, "Instantiated bundle") + assert.Contains(ct, cond.Message, "Installed bundle") assert.NotEmpty(ct, clusterExtension.Status.InstalledBundle) }, pollDuration, pollInterval) } diff --git a/test/upgrade-e2e/post_upgrade_test.go b/test/upgrade-e2e/post_upgrade_test.go index 7900bf23e..4abf1f180 100644 --- a/test/upgrade-e2e/post_upgrade_test.go +++ b/test/upgrade-e2e/post_upgrade_test.go @@ -85,7 +85,7 @@ func TestClusterExtensionAfterOLMUpgrade(t *testing.T) { } assert.Equal(ct, metav1.ConditionTrue, cond.Status) assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Contains(ct, cond.Message, "Instantiated bundle") + assert.Contains(ct, cond.Message, "Installed bundle") if assert.NotEmpty(ct, clusterExtension.Status.InstalledBundle) { assert.NotEmpty(ct, clusterExtension.Status.InstalledBundle.Version) } @@ -106,7 +106,7 @@ func TestClusterExtensionAfterOLMUpgrade(t *testing.T) { return } assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Contains(ct, cond.Message, "Instantiated bundle") + assert.Contains(ct, cond.Message, "Installed bundle") assert.Equal(ct, &ocv1alpha1.BundleMetadata{Name: "prometheus-operator.1.0.1", Version: "1.0.1"}, clusterExtension.Status.ResolvedBundle) assert.Equal(ct, &ocv1alpha1.BundleMetadata{Name: "prometheus-operator.1.0.1", Version: "1.0.1"}, clusterExtension.Status.InstalledBundle) assert.NotEqual(ct, previousVersion, clusterExtension.Status.InstalledBundle.Version)