From e37bb1c84ec0a593d245949311e5ea2078df1cd7 Mon Sep 17 00:00:00 2001 From: everettraven Date: Wed, 31 Jan 2024 13:30:32 -0500 Subject: [PATCH 1/3] refactor catalogmetadata types used for resolution Signed-off-by: everettraven --- cmd/manager/main.go | 8 +- cmd/resolutioncli/client.go | 82 +++- cmd/resolutioncli/main.go | 4 +- internal/catalogmetadata/client/client.go | 107 +++--- .../catalogmetadata/client/client_test.go | 94 +---- .../filter/bundle_predicates.go | 22 +- .../filter/bundle_predicates_test.go | 64 +-- internal/catalogmetadata/sort/sort_test.go | 50 +-- internal/catalogmetadata/types.go | 146 +++---- internal/catalogmetadata/types_test.go | 63 +-- .../clusterextension_controller.go | 110 +++--- .../clusterextension_controller_test.go | 363 ++++++++++++++---- internal/controllers/suite_test.go | 10 +- internal/controllers/variables.go | 10 +- internal/controllers/variables_test.go | 9 +- internal/resolution/variables/bundle.go | 2 +- internal/resolution/variables/bundle_test.go | 9 - internal/resolution/variablesources/bundle.go | 2 +- .../resolution/variablesources/bundle_test.go | 23 +- .../variablesources/bundle_uniqueness_test.go | 10 +- .../variablesources/installed_package.go | 41 +- .../variablesources/installed_package_test.go | 56 ++- .../variablesources/required_package.go | 25 +- .../variablesources/required_package_test.go | 77 ++-- test/util/fake_catalog_client.go | 17 +- 25 files changed, 727 insertions(+), 677 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 3e872f5f6..fec3183ad 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -115,10 +115,10 @@ func main() { } if err = (&controllers.ClusterExtensionReconciler{ - Client: cl, - BundleProvider: catalogClient, - Scheme: mgr.GetScheme(), - Resolver: resolver, + Client: cl, + CatalogProvider: catalogClient, + Scheme: mgr.GetScheme(), + Resolver: resolver, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") os.Exit(1) diff --git a/cmd/resolutioncli/client.go b/cmd/resolutioncli/client.go index 0d36a8909..93d0697c9 100644 --- a/cmd/resolutioncli/client.go +++ b/cmd/resolutioncli/client.go @@ -20,16 +20,22 @@ import ( "context" "github.com/operator-framework/operator-registry/alpha/action" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" + "github.com/operator-framework/operator-controller/internal/controllers" ) type indexRefClient struct { - renderer action.Render - bundlesCache []*catalogmetadata.Bundle + renderer action.Render + bundlesCache []*catalogmetadata.Bundle + channelsCache []*catalogmetadata.Channel + packagesCache []*catalogmetadata.Package } +var _ controllers.CatalogProvider = &indexRefClient{} + func newIndexRefClient(indexRef string) *indexRefClient { return &indexRefClient{ renderer: action.Render{ @@ -39,47 +45,81 @@ func newIndexRefClient(indexRef string) *indexRefClient { } } -func (c *indexRefClient) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) { - if c.bundlesCache == nil { +func (c *indexRefClient) CatalogContents(ctx context.Context) (*client.Contents, error) { + if c.bundlesCache == nil || c.channelsCache == nil || c.packagesCache == nil { cfg, err := c.renderer.Run(ctx) if err != nil { return nil, err } var ( - channels []*catalogmetadata.Channel - bundles []*catalogmetadata.Bundle - deprecations []*catalogmetadata.Deprecation + channels []*catalogmetadata.Channel + bundles []*catalogmetadata.Bundle + packages []*catalogmetadata.Package ) + // TODO: update fake catalog name string to be catalog name once we support multiple catalogs in CLI + catalogName := "offline-catalog" + + for i := range cfg.Packages { + packages = append(packages, &catalogmetadata.Package{ + Package: cfg.Packages[i], + Catalog: catalogName, + }) + } + for i := range cfg.Channels { channels = append(channels, &catalogmetadata.Channel{ Channel: cfg.Channels[i], + Catalog: catalogName, }) } for i := range cfg.Bundles { bundles = append(bundles, &catalogmetadata.Bundle{ - Bundle: cfg.Bundles[i], + Bundle: cfg.Bundles[i], + Catalog: catalogName, }) } - for i := range cfg.Deprecations { - deprecations = append(deprecations, &catalogmetadata.Deprecation{ - Deprecation: cfg.Deprecations[i], - }) - } - - // TODO: update fake catalog name string to be catalog name once we support multiple catalogs in CLI - catalogName := "offline-catalog" - - bundles, err = client.PopulateExtraFields(catalogName, channels, bundles, deprecations) - if err != nil { - return nil, err + for _, deprecation := range cfg.Deprecations { + for _, entry := range deprecation.Entries { + switch entry.Reference.Schema { + case declcfg.SchemaPackage: + for _, pkg := range packages { + if pkg.Name == deprecation.Package { + pkg.Deprecation = &declcfg.DeprecationEntry{ + Reference: entry.Reference, + Message: entry.Message, + } + } + } + case declcfg.SchemaChannel: + for _, channel := range channels { + if channel.Package == deprecation.Package && channel.Name == entry.Reference.Name { + channel.Deprecation = &declcfg.DeprecationEntry{ + Reference: entry.Reference, + Message: entry.Message, + } + } + } + case declcfg.SchemaBundle: + for _, bundle := range bundles { + if bundle.Package == deprecation.Package && bundle.Name == entry.Reference.Name { + bundle.Deprecation = &declcfg.DeprecationEntry{ + Reference: entry.Reference, + Message: entry.Message, + } + } + } + } + } } c.bundlesCache = bundles + c.channelsCache = channels + c.packagesCache = packages } - return c.bundlesCache, nil + return &client.Contents{}, nil } diff --git a/cmd/resolutioncli/main.go b/cmd/resolutioncli/main.go index 34815e9a6..c46872acd 100644 --- a/cmd/resolutioncli/main.go +++ b/cmd/resolutioncli/main.go @@ -150,7 +150,7 @@ func run(ctx context.Context, packageName, packageChannel, packageVersionRange, cl := clientBuilder.Build() catalogClient := newIndexRefClient(indexRef) - allBundles, err := catalogClient.Bundles(ctx) + contents, err := catalogClient.CatalogContents(ctx) if err != nil { return err } @@ -162,7 +162,7 @@ func run(ctx context.Context, packageName, packageChannel, packageVersionRange, if err := cl.List(ctx, &bundleDeploymentList); err != nil { return err } - variables, err := controllers.GenerateVariables(allBundles, clusterExtensionList.Items, bundleDeploymentList.Items) + variables, err := controllers.GenerateVariables(contents, clusterExtensionList.Items, bundleDeploymentList.Items) if err != nil { return err } diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go index 172f686c6..c96699f92 100644 --- a/internal/catalogmetadata/client/client.go +++ b/internal/catalogmetadata/client/client.go @@ -43,18 +43,31 @@ type Client struct { fetcher Fetcher } -func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) { - var allBundles []*catalogmetadata.Bundle +type Contents struct { + Packages []*catalogmetadata.Package + Channels []*catalogmetadata.Channel + Bundles []*catalogmetadata.Bundle +} +func (c *Client) CatalogContents(ctx context.Context) (*Contents, error) { var catalogList catalogd.CatalogList if err := c.cl.List(ctx, &catalogList); err != nil { return nil, err } + + contents := &Contents{ + Packages: []*catalogmetadata.Package{}, + Channels: []*catalogmetadata.Channel{}, + Bundles: []*catalogmetadata.Bundle{}, + } + for _, catalog := range catalogList.Items { // if the catalog has not been successfully unpacked, skip it if !meta.IsStatusConditionPresentAndEqual(catalog.Status.Conditions, catalogd.TypeUnpacked, metav1.ConditionTrue) { continue } + + packages := []*catalogmetadata.Package{} channels := []*catalogmetadata.Channel{} bundles := []*catalogmetadata.Bundle{} deprecations := []*catalogmetadata.Deprecation{} @@ -70,93 +83,81 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) return fmt.Errorf("error was provided to the WalkMetasReaderFunc: %s", err) } switch meta.Schema { + case declcfg.SchemaPackage: + var content catalogmetadata.Package + if err := json.Unmarshal(meta.Blob, &content); err != nil { + return fmt.Errorf("error unmarshalling channel from catalog metadata: %s", err) + } + content.Catalog = catalog.Name + packages = append(packages, &content) 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: %s", err) } + content.Catalog = catalog.Name 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: %s", err) } + content.Catalog = catalog.Name bundles = append(bundles, &content) case declcfg.SchemaDeprecation: var content catalogmetadata.Deprecation if err := json.Unmarshal(meta.Blob, &content); err != nil { return fmt.Errorf("error unmarshalling deprecation from catalog metadata: %s", err) } + content.Catalog = catalog.Name deprecations = append(deprecations, &content) } return nil }) - if err != nil { - return nil, fmt.Errorf("error processing response: %s", err) - } - bundles, err = PopulateExtraFields(catalog.Name, channels, bundles, deprecations) if err != nil { - return nil, err - } - - allBundles = append(allBundles, bundles...) - } - - 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 - } - - 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) + return nil, fmt.Errorf("error processing response: %s", err) } - } - - // 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 { + for _, deprecation := range deprecations { + for _, entry := range deprecation.Entries { switch entry.Reference.Schema { case declcfg.SchemaPackage: - bundles[i].Deprecations = append(bundles[i].Deprecations, entry) + for _, pkg := range packages { + if pkg.Name == deprecation.Package { + pkg.Deprecation = &declcfg.DeprecationEntry{ + Reference: entry.Reference, + Message: entry.Message, + } + } + } case declcfg.SchemaChannel: - for _, ch := range bundles[i].InChannels { - if ch.Name == entry.Reference.Name { - bundles[i].Deprecations = append(bundles[i].Deprecations, entry) - break + for _, channel := range channels { + if channel.Package == deprecation.Package && channel.Name == entry.Reference.Name { + channel.Deprecation = &declcfg.DeprecationEntry{ + Reference: entry.Reference, + Message: entry.Message, + } } } case declcfg.SchemaBundle: - if bundles[i].Name == entry.Reference.Name { - bundles[i].Deprecations = append(bundles[i].Deprecations, entry) + for _, bundle := range bundles { + if bundle.Package == deprecation.Package && bundle.Name == entry.Reference.Name { + bundle.Deprecation = &declcfg.DeprecationEntry{ + Reference: entry.Reference, + Message: entry.Message, + } + } } } } } + + contents.Packages = append(contents.Packages, packages...) + contents.Channels = append(contents.Channels, channels...) + contents.Bundles = append(contents.Bundles, bundles...) } - return bundles, nil + return contents, nil } diff --git a/internal/catalogmetadata/client/client_test.go b/internal/catalogmetadata/client/client_test.go index 9bcc54850..0c4dc3a43 100644 --- a/internal/catalogmetadata/client/client_test.go +++ b/internal/catalogmetadata/client/client_test.go @@ -52,27 +52,6 @@ func TestClient(t *testing.T) { fetcher: &MockFetcher{shouldError: true}, wantErr: "error fetching catalog contents: mock cache error", }, - { - name: "channel has a ref to a missing bundle", - fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte) { - objs, _, catalogContentMap := defaultFakeCatalog() - - catalogContentMap["catalog-1"] = append(catalogContentMap["catalog-1"], []byte(`{ - "schema": "olm.channel", - "name": "channel-with-missing-bundle", - "package": "fake1", - "entries": [ - { - "name": "fake1.v9.9.9" - } - ] - }`)...) - - 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{}, - }, { name: "invalid meta", fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte) { @@ -127,37 +106,22 @@ func TestClient(t *testing.T) { fetcher: &MockFetcher{}, }, { - name: "deprecated at the package, channel, and bundle level", + name: "bundle deprecated", fakeCatalog: func() ([]client.Object, []*catalogmetadata.Bundle, map[string][]byte) { objs, bundles, catalogContentMap := defaultFakeCatalog() catalogContentMap["catalog-1"] = append(catalogContentMap["catalog-1"], - []byte(`{"schema": "olm.deprecations", "package":"fake1", "entries":[{"message": "fake1 is deprecated", "reference": {"schema": "olm.package"}}, {"message":"channel stable is deprecated", "reference": {"schema": "olm.channel", "name": "stable"}}, {"message": "bundle fake1.v1.0.0 is deprecated", "reference":{"schema":"olm.bundle", "name":"fake1.v1.0.0"}}]}`)...) + []byte(`{"schema": "olm.deprecations", "package":"fake1", "entries":[{"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{ + if bundles[i].Name == "fake1.v1.0.0" && bundles[i].Catalog == "catalog-1" { + bundles[i].Deprecation = &declcfg.DeprecationEntry{ Reference: declcfg.PackageScopedReference{ Schema: "olm.bundle", Name: "fake1.v1.0.0", }, Message: "bundle fake1.v1.0.0 is deprecated", - }) + } } } @@ -176,10 +140,10 @@ func TestClient(t *testing.T) { tt.fetcher, ) - bundles, err := fakeCatalogClient.Bundles(ctx) + contents, err := fakeCatalogClient.CatalogContents(ctx) if tt.wantErr == "" { assert.NoError(t, err) - assert.Equal(t, expectedBundles, bundles) + assert.Equal(t, expectedBundles, contents.Bundles) } else { assert.EqualError(t, err, tt.wantErr) } @@ -262,7 +226,7 @@ func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle, map[strin expectedBundles := []*catalogmetadata.Bundle{ { - CatalogName: "catalog-1", + Catalog: "catalog-1", Bundle: declcfg.Bundle{ Schema: declcfg.SchemaBundle, Name: "fake1.v1.0.0", @@ -275,35 +239,9 @@ func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle, map[strin }, }, }, - InChannels: []*catalogmetadata.Channel{ - { - Channel: declcfg.Channel{ - Schema: declcfg.SchemaChannel, - Name: "stable", - Package: "fake1", - Entries: []declcfg.ChannelEntry{ - { - Name: "fake1.v1.0.0", - }, - }, - }, - }, - { - Channel: declcfg.Channel{ - Schema: declcfg.SchemaChannel, - Name: "beta", - Package: "fake1", - Entries: []declcfg.ChannelEntry{ - { - Name: "fake1.v1.0.0", - }, - }, - }, - }, - }, }, { - CatalogName: "catalog-2", + Catalog: "catalog-2", Bundle: declcfg.Bundle{ Schema: declcfg.SchemaBundle, Name: "fake1.v1.0.0", @@ -316,20 +254,6 @@ func defaultFakeCatalog() ([]client.Object, []*catalogmetadata.Bundle, map[strin }, }, }, - InChannels: []*catalogmetadata.Channel{ - { - Channel: declcfg.Channel{ - Schema: declcfg.SchemaChannel, - Name: "stable", - Package: "fake1", - Entries: []declcfg.ChannelEntry{ - { - Name: "fake1.v1.0.0", - }, - }, - }, - }, - }, }, } diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index 1d5761a95..3cb00fd31 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -41,26 +41,30 @@ func InBlangSemverRange(semverRange bsemver.Range) Predicate[catalogmetadata.Bun } } -func InChannel(channelName string) Predicate[catalogmetadata.Bundle] { +func InChannel(channelName string, channels []*catalogmetadata.Channel) Predicate[catalogmetadata.Bundle] { return func(bundle *catalogmetadata.Bundle) bool { - for _, ch := range bundle.InChannels { + for _, ch := range channels { if ch.Name == channelName { - return true + for _, entry := range ch.Entries { + if entry.Name == bundle.Name { + return true + } + } } } return false } } -func WithBundleImage(bundleImage string) Predicate[catalogmetadata.Bundle] { +func WithImage(image string) Predicate[catalogmetadata.Bundle] { return func(bundle *catalogmetadata.Bundle) bool { - return bundle.Image == bundleImage + return bundle.Image == image } } -func Replaces(bundleName string) Predicate[catalogmetadata.Bundle] { +func Replaces(bundleName string, channels []*catalogmetadata.Channel) Predicate[catalogmetadata.Bundle] { return func(bundle *catalogmetadata.Bundle) bool { - for _, ch := range bundle.InChannels { + for _, ch := range channels { for _, chEntry := range ch.Entries { if bundle.Name == chEntry.Name && chEntry.Replaces == bundleName { return true @@ -71,8 +75,8 @@ func Replaces(bundleName string) Predicate[catalogmetadata.Bundle] { } } -func WithDeprecation(deprecated bool) Predicate[catalogmetadata.Bundle] { +func WithDeprecated(deprecated bool) Predicate[catalogmetadata.Bundle] { return func(bundle *catalogmetadata.Bundle) bool { - return bundle.HasDeprecation() == deprecated + return bundle.IsDeprecated() == deprecated } } diff --git a/internal/catalogmetadata/filter/bundle_predicates_test.go b/internal/catalogmetadata/filter/bundle_predicates_test.go index 3617f47ce..0ca60e785 100644 --- a/internal/catalogmetadata/filter/bundle_predicates_test.go +++ b/internal/catalogmetadata/filter/bundle_predicates_test.go @@ -97,80 +97,28 @@ func TestInBlangSemverRange(t *testing.T) { 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) { +func TestWithImage(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") + f := filter.WithImage("fake-image-uri-1") assert.True(t, f(b1)) assert.False(t, f(b2)) assert.False(t, f(b3)) } -func TestReplaces(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", - }, - }, - }, - } - +func TestWithDeprecated(t *testing.T) { b1 := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{Name: "package1.v0.0.2"}, - InChannels: []*catalogmetadata.Channel{fakeChannel}, - } - b2 := &catalogmetadata.Bundle{ - Bundle: declcfg.Bundle{Name: "package1.v0.0.3"}, - InChannels: []*catalogmetadata.Channel{fakeChannel}, - } - b3 := &catalogmetadata.Bundle{} - - f := filter.Replaces("package1.v0.0.1") - - assert.True(t, f(b1)) - assert.False(t, f(b2)) - assert.False(t, f(b3)) -} - -func TestWithDeprecation(t *testing.T) { - b1 := &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{}, - }, + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{}, }, } b2 := &catalogmetadata.Bundle{} - f := filter.WithDeprecation(true) + f := filter.WithDeprecated(true) assert.True(t, f(b1)) assert.False(t, f(b2)) } diff --git a/internal/catalogmetadata/sort/sort_test.go b/internal/catalogmetadata/sort/sort_test.go index e0c73c9ca..baf24b20c 100644 --- a/internal/catalogmetadata/sort/sort_test.go +++ b/internal/catalogmetadata/sort/sort_test.go @@ -85,23 +85,21 @@ func TestByVersion(t *testing.T) { func TestByDeprecated(t *testing.T) { b1 := &catalogmetadata.Bundle{ - CatalogName: "foo", + Catalog: "foo", Bundle: declcfg.Bundle{ Name: "bar", }, } b2 := &catalogmetadata.Bundle{ - CatalogName: "foo", + Catalog: "foo", Bundle: declcfg.Bundle{ Name: "baz", }, - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{ - Schema: "olm.bundle", - Name: "baz", - }, + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: "olm.bundle", + Name: "baz", }, }, } @@ -115,13 +113,7 @@ func TestByDeprecated(t *testing.T) { 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", - }, - } + b2.Deprecation = nil toSort = []*catalogmetadata.Bundle{b2, b1} sort.SliceStable(toSort, func(i, j int) bool { @@ -131,32 +123,4 @@ func TestByDeprecated(t *testing.T) { require.Len(t, toSort, 2) assert.Equal(t, b2, toSort[0]) assert.Equal(t, b1, toSort[1]) - - b1.Deprecations = []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{ - Schema: "olm.package", - }, - }, - } - b2.Deprecations = append(b2.Deprecations, declcfg.DeprecationEntry{ - Reference: declcfg.PackageScopedReference{ - Schema: "olm.package", - }, - }, declcfg.DeprecationEntry{ - Reference: declcfg.PackageScopedReference{ - Schema: "olm.bundle", - Name: "baz", - }, - }) - - toSort = []*catalogmetadata.Bundle{b2, b1} - sort.SliceStable(toSort, func(i, j int) bool { - return catalogsort.ByDeprecated(toSort[i], toSort[j]) - }) - // Both are deprecated at package level, b2 is deprecated - // explicitly, b2 should be preferred less - require.Len(t, toSort, 2) - assert.Equal(t, b1, toSort[0]) - assert.Equal(t, b2, toSort[1]) } diff --git a/internal/catalogmetadata/types.go b/internal/catalogmetadata/types.go index e1cb8e2e8..9b8ba1548 100644 --- a/internal/catalogmetadata/types.go +++ b/internal/catalogmetadata/types.go @@ -3,7 +3,6 @@ package catalogmetadata import ( "encoding/json" "fmt" - "sync" bsemver "github.com/blang/semver/v4" @@ -23,14 +22,27 @@ type Schemas interface { type Package struct { declcfg.Package + Deprecation *declcfg.DeprecationEntry + Catalog string +} + +func (p *Package) IsDeprecated() bool { + return p.Deprecation != nil } type Channel struct { declcfg.Channel + Deprecation *declcfg.DeprecationEntry + Catalog string +} + +func (c *Channel) IsDeprecated() bool { + return c.Deprecation != nil } type Deprecation struct { declcfg.Deprecation + Catalog string } type PackageRequired struct { @@ -40,119 +52,59 @@ type PackageRequired struct { 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 - mediaType *string + Deprecation *declcfg.DeprecationEntry + Catalog string } 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 { + pkg, err := loadOneFromProps[property.Package](b, property.TypePackage, true) + if err != nil { return nil, err } - return b.requiredPackages, nil -} - -func (b *Bundle) MediaType() (string, error) { - if err := b.loadMediaType(); err != nil { - return "", err + semVer, err := bsemver.Parse(pkg.Version) + if err != nil { + return nil, fmt.Errorf("could not parse semver %q for bundle '%s': %s", pkg.Version, b.Name, err) } - - return *b.mediaType, nil + return &semVer, 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 +func (b *Bundle) RequiredPackages() ([]PackageRequired, error) { + requiredPackages, err := loadFromProps[PackageRequired](b, property.TypePackageRequired, false) + if err != nil { + return nil, fmt.Errorf("error determining bundle required packages for bundle %q: %s", b.Name, err) } - if b.semVersion == nil { - semVer, err := bsemver.Parse(b.bundlePackage.Version) + for i := range requiredPackages { + semverRange, err := bsemver.ParseRange(requiredPackages[i].VersionRange) if err != nil { - return fmt.Errorf("could not parse semver %q for bundle '%s': %s", b.bundlePackage.Version, b.Name, err) + return nil, fmt.Errorf( + "error parsing bundle required package semver range for bundle %q (required package %q): %s", + b.Name, + requiredPackages[i].PackageName, + err, + ) } - b.semVersion = &semVer + requiredPackages[i].SemverRange = semverRange } - return nil + return requiredPackages, 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].VersionRange) - if err != nil { - return fmt.Errorf( - "error parsing bundle required package semver range for bundle %q (required package %q): %s", - b.Name, - requiredPackages[i].PackageName, - err, - ) - } - requiredPackages[i].SemverRange = semverRange - } - b.requiredPackages = requiredPackages +func (b *Bundle) MediaType() (string, error) { + mediaType, err := loadOneFromProps[string](b, PropertyBundleMediaType, false) + if err != nil { + return "", fmt.Errorf("error determining bundle mediatype for bundle %q: %s", b.Name, err) } - return nil -} -func (b *Bundle) loadMediaType() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.mediaType == nil { - mediaType, err := loadOneFromProps[string](b, PropertyBundleMediaType, false) - if err != nil { - return fmt.Errorf("error determining bundle mediatype for bundle %q: %s", b.Name, err) - } - b.mediaType = &mediaType - } - return nil + return mediaType, nil } func (b *Bundle) propertiesByType(propType string) []*property.Property { - if b.propertiesMap == nil { - b.propertiesMap = make(map[string][]*property.Property) - for i := range b.Properties { - prop := b.Properties[i] - b.propertiesMap[prop.Type] = append(b.propertiesMap[prop.Type], &prop) - } + propertiesMap := make(map[string][]*property.Property) + for i := range b.Properties { + prop := b.Properties[i] + propertiesMap[prop.Type] = append(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 + return propertiesMap[propType] } // IsDeprecated returns true if the bundle @@ -165,13 +117,7 @@ func (b *Bundle) HasDeprecation() bool { // 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.Name { - return true - } - } - - return false + return b.Deprecation != nil } func loadOneFromProps[T any](bundle *Bundle, propType string, required bool) (T, error) { diff --git a/internal/catalogmetadata/types_test.go b/internal/catalogmetadata/types_test.go index 3129ecde3..a94d35eab 100644 --- a/internal/catalogmetadata/types_test.go +++ b/internal/catalogmetadata/types_test.go @@ -202,34 +202,6 @@ func TestBundleMediaType(t *testing.T) { } } -func TestBundleHasDeprecation(t *testing.T) { - for _, tt := range []struct { - name string - bundle *catalogmetadata.Bundle - deprecated bool - }{ - { - name: "has deprecation entries", - bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{}, - }, - }, - }, - deprecated: true, - }, - { - name: "has no deprecation entries", - bundle: &catalogmetadata.Bundle{}, - }, - } { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.deprecated, tt.bundle.HasDeprecation()) - }) - } -} - func TestBundleIsDeprecated(t *testing.T) { for _, tt := range []struct { name string @@ -237,19 +209,13 @@ func TestBundleIsDeprecated(t *testing.T) { deprecated bool }{ { - name: "has package and channel deprecations, not deprecated", + name: "has bundle deprecation entry, deprecated", + deprecated: true, bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{ - Schema: "olm.package", - }, - }, - { - Reference: declcfg.PackageScopedReference{ - Schema: "olm.channel", - Name: "foo", - }, + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: "olm.bundle", + Name: "foo", }, }, }, @@ -258,23 +224,6 @@ func TestBundleIsDeprecated(t *testing.T) { 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 126b673f4..dafcc2289 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -25,7 +25,6 @@ import ( catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/solver" - "github.com/operator-framework/operator-registry/alpha/declcfg" rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" "k8s.io/apimachinery/pkg/api/equality" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -43,22 +42,24 @@ import ( ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" + "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" "github.com/operator-framework/operator-controller/internal/controllers/validators" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) // 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) ([]*catalogmetadata.Bundle, error) +type CatalogProvider interface { + CatalogContents(ctx context.Context) (*catalogclient.Contents, error) } // ClusterExtensionReconciler reconciles a ClusterExtension object type ClusterExtensionReconciler struct { client.Client - BundleProvider BundleProvider - Scheme *runtime.Scheme - Resolver *solver.Solver + CatalogProvider CatalogProvider + Scheme *runtime.Scheme + Resolver *solver.Solver } //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch @@ -138,7 +139,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp } // gather vars for resolution - vars, err := r.variables(ctx) + vars, contents, err := r.variables(ctx) if err != nil { ext.Status.InstalledBundleResource = "" setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted due to failure to gather data for resolution", ext.GetGeneration()) @@ -217,27 +218,31 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp // existing BundleDeployment object status. mapBDStatusToInstalledCondition(existingTypedBundleDeployment, ext) - SetDeprecationStatus(ext, bundle) + SetDeprecationStatus(ext, bundle, contents) // set the status of the cluster extension based on the respective bundle deployment status conditions. return ctrl.Result{}, nil } -func (r *ClusterExtensionReconciler) variables(ctx context.Context) ([]deppy.Variable, error) { - allBundles, err := r.BundleProvider.Bundles(ctx) +func (r *ClusterExtensionReconciler) variables(ctx context.Context) ([]deppy.Variable, *catalogclient.Contents, error) { + contents, err := r.CatalogProvider.CatalogContents(ctx) if err != nil { - return nil, err + return nil, nil, err } clusterExtensionList := ocv1alpha1.ClusterExtensionList{} if err := r.Client.List(ctx, &clusterExtensionList); err != nil { - return nil, err + return nil, nil, err } bundleDeploymentList := rukpakv1alpha2.BundleDeploymentList{} if err := r.Client.List(ctx, &bundleDeploymentList); err != nil { - return nil, err + return nil, nil, err } - return GenerateVariables(allBundles, clusterExtensionList.Items, bundleDeploymentList.Items) + variables, err := GenerateVariables(contents, clusterExtensionList.Items, bundleDeploymentList.Items) + if err != nil { + return nil, nil, err + } + return variables, contents, nil } func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alpha2.BundleDeployment, ext *ocv1alpha1.ClusterExtension) { @@ -287,7 +292,7 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph // setDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension // based on the provided bundle -func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) { +func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle, contents *catalogclient.Contents) { // reset conditions to false conditionTypes := []string{ ocv1alpha1.TypeDeprecated, @@ -306,54 +311,55 @@ func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetad }) } - // 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: + packages := filter.Filter[catalogmetadata.Package](contents.Packages, + func(entity *catalogmetadata.Package) bool { + return entity.Name == ext.Spec.PackageName && entity.Catalog == bundle.Catalog + }, + ) + if len(packages) > 0 { + if packages[0].IsDeprecated() { apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1alpha1.TypePackageDeprecated, Reason: ocv1alpha1.ReasonDeprecated, Status: metav1.ConditionTrue, - Message: deprecation.Message, + Message: packages[0].Deprecation.Message, ObservedGeneration: ext.Generation, }) - case declcfg.SchemaChannel: - if ext.Spec.Channel != deprecation.Reference.Name { - continue - } + deprecationMessages = append(deprecationMessages, packages[0].Deprecation.Message) + } + } - 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, - }) + if ext.Spec.Channel != "" { + channels := filter.Filter[catalogmetadata.Channel](contents.Channels, + func(entity *catalogmetadata.Channel) bool { + return entity.Package == ext.Spec.PackageName && entity.Name == ext.Spec.Channel && entity.Catalog == bundle.Catalog + }, + ) + if len(channels) > 0 { + if channels[0].IsDeprecated() { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1alpha1.TypeChannelDeprecated, + Reason: ocv1alpha1.ReasonDeprecated, + Status: metav1.ConditionTrue, + Message: channels[0].Deprecation.Message, + ObservedGeneration: ext.Generation, + }) + deprecationMessages = append(deprecationMessages, channels[0].Deprecation.Message) + } } + } - deprecationMessages = append(deprecationMessages, deprecation.Message) + if bundle.IsDeprecated() { + apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1alpha1.TypeBundleDeprecated, + Reason: ocv1alpha1.ReasonDeprecated, + Status: metav1.ConditionTrue, + Message: bundle.Deprecation.Message, + ObservedGeneration: ext.Generation, + }) + deprecationMessages = append(deprecationMessages, bundle.Deprecation.Message) } if len(deprecationMessages) > 0 { diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index bfe2d7cd2..095ff05eb 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -27,6 +27,7 @@ import ( ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/conditionsets" "github.com/operator-framework/operator-controller/internal/controllers" "github.com/operator-framework/operator-controller/pkg/features" @@ -1604,6 +1605,7 @@ func TestSetDeprecationStatus(t *testing.T) { clusterExtension *ocv1alpha1.ClusterExtension expectedClusterExtension *ocv1alpha1.ClusterExtension bundle *catalogmetadata.Bundle + catalogContents *catalogclient.Contents }{ { name: "non-deprecated bundle, no deprecations associated with bundle, all deprecation statuses set to False", @@ -1611,6 +1613,10 @@ func TestSetDeprecationStatus(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Generation: 1, }, + Spec: ocv1alpha1.ClusterExtensionSpec{ + PackageName: "foo", + Channel: "stable", + }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{}, }, @@ -1619,6 +1625,10 @@ func TestSetDeprecationStatus(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Generation: 1, }, + Spec: ocv1alpha1.ClusterExtensionSpec{ + PackageName: "foo", + Channel: "stable", + }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{ { @@ -1648,7 +1658,32 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, }, - bundle: &catalogmetadata.Bundle{}, + bundle: &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "foo.v1.0.0", + Package: "foo", + }, + Catalog: "bar", + }, + catalogContents: &catalogclient.Contents{ + Packages: []*catalogmetadata.Package{ + { + Package: declcfg.Package{ + Name: "foo", + }, + Catalog: "bar", + }, + }, + Channels: []*catalogmetadata.Channel{ + { + Channel: declcfg.Channel{ + Name: "stable", + Package: "foo", + }, + Catalog: "bar", + }, + }, + }, }, { name: "non-deprecated bundle, olm.channel deprecations associated with bundle, no channel specified, all deprecation statuses set to False", @@ -1656,6 +1691,9 @@ func TestSetDeprecationStatus(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Generation: 1, }, + Spec: ocv1alpha1.ClusterExtensionSpec{ + PackageName: "foo", + }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{}, }, @@ -1664,6 +1702,9 @@ func TestSetDeprecationStatus(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Generation: 1, }, + Spec: ocv1alpha1.ClusterExtensionSpec{ + PackageName: "foo", + }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{ { @@ -1694,11 +1735,34 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + Bundle: declcfg.Bundle{ + Name: "foo.v1.0.0", + Package: "foo", + }, + Catalog: "bar", + }, + catalogContents: &catalogclient.Contents{ + Packages: []*catalogmetadata.Package{ + { + Package: declcfg.Package{ + Name: "foo", + }, + Catalog: "bar", + }, + }, + Channels: []*catalogmetadata.Channel{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaChannel, - Name: "badchannel", + Channel: declcfg.Channel{ + Name: "stable", + Package: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaChannel, + Name: "stable", + }, + Message: "stable channel deprecated", }, }, }, @@ -1711,7 +1775,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "nondeprecated", + PackageName: "foo", + Channel: "nondeprecated", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{}, @@ -1722,7 +1787,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "nondeprecated", + PackageName: "foo", + Channel: "nondeprecated", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{ @@ -1754,12 +1820,42 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + Bundle: declcfg.Bundle{ + Name: "foo.v1.0.0", + Package: "foo", + }, + Catalog: "bar", + }, + catalogContents: &catalogclient.Contents{ + Packages: []*catalogmetadata.Package{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaChannel, - Name: "badchannel", + Package: declcfg.Package{ + Name: "foo", }, + Catalog: "bar", + }, + }, + Channels: []*catalogmetadata.Channel{ + { + Channel: declcfg.Channel{ + Name: "stable", + Package: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaChannel, + Name: "stable", + }, + Message: "stable channel deprecated", + }, + }, + { + Channel: declcfg.Channel{ + Name: "nondeprecated", + Package: "foo", + }, + Catalog: "bar", }, }, }, @@ -1771,7 +1867,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "badchannel", + PackageName: "foo", + Channel: "stable", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{}, @@ -1782,7 +1879,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "badchannel", + PackageName: "foo", + Channel: "stable", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{ @@ -1814,13 +1912,35 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + Bundle: declcfg.Bundle{ + Name: "foo.v1.0.0", + Package: "foo", + }, + Catalog: "bar", + }, + catalogContents: &catalogclient.Contents{ + Packages: []*catalogmetadata.Package{ + { + Package: declcfg.Package{ + Name: "foo", + }, + Catalog: "bar", + }, + }, + Channels: []*catalogmetadata.Channel{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaChannel, - Name: "badchannel", + Channel: declcfg.Channel{ + Name: "stable", + Package: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaChannel, + Name: "stable", + }, + Message: "stable channel deprecated", }, - Message: "bad channel!", }, }, }, @@ -1832,7 +1952,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "badchannel", + PackageName: "foo", + Channel: "stable", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{}, @@ -1843,7 +1964,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "badchannel", + PackageName: "foo", + Channel: "stable", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{ @@ -1875,26 +1997,48 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaChannel, - Name: "badchannel", - }, - Message: "bad channel!", + Bundle: declcfg.Bundle{ + Name: "foo.v1.0.0", + Package: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaBundle, + Name: "foo.v1.0.0", }, + Message: "foo.v1.0.0 deprecated", + }, + }, + catalogContents: &catalogclient.Contents{ + Packages: []*catalogmetadata.Package{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaPackage, + Package: declcfg.Package{ + Name: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaPackage, + }, + Message: "foo package deprecated", }, - Message: "bad package!", }, + }, + Channels: []*catalogmetadata.Channel{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaBundle, - Name: "badbundle", + Channel: declcfg.Channel{ + Name: "stable", + Package: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaChannel, + Name: "stable", + }, + Message: "stable channel deprecated", }, - Message: "bad bundle!", }, }, }, @@ -1906,7 +2050,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "badchannel", + PackageName: "foo", + Channel: "stable", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{}, @@ -1917,7 +2062,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "badchannel", + PackageName: "foo", + Channel: "stable", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{ @@ -1949,20 +2095,42 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + Bundle: declcfg.Bundle{ + Name: "foo.v1.0.0", + Package: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaBundle, + Name: "foo.v1.0.0", + }, + Message: "foo.v1.0.0 deprecated", + }, + }, + catalogContents: &catalogclient.Contents{ + Packages: []*catalogmetadata.Package{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaChannel, - Name: "badchannel", + Package: declcfg.Package{ + Name: "foo", }, - Message: "bad channel!", + Catalog: "bar", }, + }, + Channels: []*catalogmetadata.Channel{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaBundle, - Name: "badbundle", + Channel: declcfg.Channel{ + Name: "stable", + Package: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaChannel, + Name: "stable", + }, + Message: "stable channel deprecated", }, - Message: "bad bundle!", }, }, }, @@ -1974,7 +2142,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "badchannel", + PackageName: "foo", + Channel: "stable", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{}, @@ -1985,7 +2154,8 @@ func TestSetDeprecationStatus(t *testing.T) { Generation: 1, }, Spec: ocv1alpha1.ClusterExtensionSpec{ - Channel: "badchannel", + PackageName: "foo", + Channel: "stable", }, Status: ocv1alpha1.ClusterExtensionStatus{ Conditions: []metav1.Condition{ @@ -2017,26 +2187,48 @@ func TestSetDeprecationStatus(t *testing.T) { }, }, bundle: &catalogmetadata.Bundle{ - Deprecations: []declcfg.DeprecationEntry{ + Bundle: declcfg.Bundle{ + Name: "foo.v1.0.0", + Package: "foo", + }, + Catalog: "bar", + }, + catalogContents: &catalogclient.Contents{ + Packages: []*catalogmetadata.Package{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaChannel, - Name: "badchannel", + Package: declcfg.Package{ + Name: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaPackage, + }, + Message: "foo package deprecated", }, - Message: "bad channel!", }, + }, + Channels: []*catalogmetadata.Channel{ { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaPackage, + Channel: declcfg.Channel{ + Name: "stable", + Package: "foo", + }, + Catalog: "bar", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaChannel, + Name: "stable", + }, + Message: "stable channel deprecated", }, - Message: "bad package!", }, }, }, }, } { t.Run(tc.name, func(t *testing.T) { - controllers.SetDeprecationStatus(tc.clusterExtension, tc.bundle) + controllers.SetDeprecationStatus(tc.clusterExtension, tc.bundle, tc.catalogContents) assert.Equal(t, "", cmp.Diff(tc.expectedClusterExtension, tc.clusterExtension, cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime"))) }) } @@ -2076,16 +2268,54 @@ var ( Channel: declcfg.Channel{ Name: "beta", Package: "plain", + Entries: []declcfg.ChannelEntry{ + { + Name: "operatorhub/plain/0.1.0", + }, + }, }, } badmediaBetaChannel = catalogmetadata.Channel{ Channel: declcfg.Channel{ Name: "beta", Package: "badmedia", + Entries: []declcfg.ChannelEntry{ + { + Name: "operatorhub/badmedia/0.1.0", + }, + }, }, } ) +var testPackageList = []*catalogmetadata.Package{ + { + Package: declcfg.Package{ + Name: "prometheus", + }, + Catalog: "fake-catalog", + }, + { + Package: declcfg.Package{ + Name: "plain", + }, + Catalog: "fake-catalog", + }, + { + Package: declcfg.Package{ + Name: "badmedia", + }, + Catalog: "fake-catalog", + }, +} + +var testChannelList = []*catalogmetadata.Channel{ + &prometheusAlphaChannel, + &prometheusBetaChannel, + &plainBetaChannel, + &badmediaBetaChannel, +} + var testBundleList = []*catalogmetadata.Bundle{ { Bundle: declcfg.Bundle{ @@ -2097,8 +2327,7 @@ var testBundleList = []*catalogmetadata.Bundle{ {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusAlphaChannel}, + Catalog: "fake-catalog", }, { Bundle: declcfg.Bundle{ @@ -2110,8 +2339,7 @@ var testBundleList = []*catalogmetadata.Bundle{ {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + Catalog: "fake-catalog", }, { Bundle: declcfg.Bundle{ @@ -2123,8 +2351,7 @@ var testBundleList = []*catalogmetadata.Bundle{ {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + Catalog: "fake-catalog", }, { Bundle: declcfg.Bundle{ @@ -2136,8 +2363,7 @@ var testBundleList = []*catalogmetadata.Bundle{ {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + Catalog: "fake-catalog", }, { Bundle: declcfg.Bundle{ @@ -2149,8 +2375,7 @@ var testBundleList = []*catalogmetadata.Bundle{ {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, }, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + Catalog: "fake-catalog", }, { Bundle: declcfg.Bundle{ @@ -2163,8 +2388,7 @@ var testBundleList = []*catalogmetadata.Bundle{ {Type: "olm.bundle.mediatype", Value: json.RawMessage(`"plain+v0"`)}, }, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&plainBetaChannel}, + Catalog: "fake-catalog", }, { Bundle: declcfg.Bundle{ @@ -2177,7 +2401,6 @@ var testBundleList = []*catalogmetadata.Bundle{ {Type: "olm.bundle.mediatype", Value: json.RawMessage(`"badmedia+v1"`)}, }, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&badmediaBetaChannel}, + Catalog: "fake-catalog", }, } diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 8ee4abcd3..99e8ca366 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -49,12 +49,12 @@ func newClientAndReconciler(t *testing.T) (client.Client, *controllers.ClusterEx require.NoError(t, err) cl := newClient(t) - fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) + fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList, testChannelList, testPackageList) reconciler := &controllers.ClusterExtensionReconciler{ - Client: cl, - BundleProvider: &fakeCatalogClient, - Scheme: sch, - Resolver: resolver, + Client: cl, + CatalogProvider: &fakeCatalogClient, + Scheme: sch, + Resolver: resolver, } return cl, reconciler } diff --git a/internal/controllers/variables.go b/internal/controllers/variables.go index d03f8bcae..1c5174bc5 100644 --- a/internal/controllers/variables.go +++ b/internal/controllers/variables.go @@ -21,22 +21,22 @@ import ( rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" 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/client" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func GenerateVariables(allBundles []*catalogmetadata.Bundle, clusterExtensions []ocv1alpha1.ClusterExtension, bundleDeployments []rukpakv1alpha2.BundleDeployment) ([]deppy.Variable, error) { - requiredPackages, err := variablesources.MakeRequiredPackageVariables(allBundles, clusterExtensions) +func GenerateVariables(catalogContent *client.Contents, clusterExtensions []ocv1alpha1.ClusterExtension, bundleDeployments []rukpakv1alpha2.BundleDeployment) ([]deppy.Variable, error) { + requiredPackages, err := variablesources.MakeRequiredPackageVariables(catalogContent.Bundles, catalogContent.Channels, clusterExtensions) if err != nil { return nil, err } - installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, clusterExtensions, bundleDeployments) + installedPackages, err := variablesources.MakeInstalledPackageVariables(catalogContent.Bundles, catalogContent.Channels, clusterExtensions, bundleDeployments) if err != nil { return nil, err } - bundles, err := variablesources.MakeBundleVariables(allBundles, requiredPackages, installedPackages) + bundles, err := variablesources.MakeBundleVariables(catalogContent.Bundles, requiredPackages, installedPackages) if err != nil { return nil, err } diff --git a/internal/controllers/variables_test.go b/internal/controllers/variables_test.go index fa3cc1315..29526aed3 100644 --- a/internal/controllers/variables_test.go +++ b/internal/controllers/variables_test.go @@ -22,6 +22,7 @@ import ( 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/client" "github.com/operator-framework/operator-controller/internal/controllers" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) @@ -32,7 +33,8 @@ func TestVariableSource(t *testing.T) { utilruntime.Must(rukpakv1alpha2.AddToScheme(sch)) stableChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "stable", + Name: "stable", + Package: "packageA", Entries: []declcfg.ChannelEntry{ { Name: "packageA.v2.0.0", @@ -49,8 +51,7 @@ func TestVariableSource(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"packageA","version":"2.0.0"}`)}, }, }, - CatalogName: "fake-catalog", - InChannels: []*catalogmetadata.Channel{&stableChannel}, + Catalog: "fake-catalog", }, } allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet)) @@ -94,7 +95,7 @@ func TestVariableSource(t *testing.T) { }, } - vars, err := controllers.GenerateVariables(allBundles, []ocv1alpha1.ClusterExtension{clusterExtension}, []rukpakv1alpha2.BundleDeployment{bd}) + vars, err := controllers.GenerateVariables(&client.Contents{Bundles: allBundles, Channels: []*catalogmetadata.Channel{&stableChannel}}, []ocv1alpha1.ClusterExtension{clusterExtension}, []rukpakv1alpha2.BundleDeployment{bd}) require.NoError(t, err) expectedVars := []deppy.Variable{ diff --git a/internal/resolution/variables/bundle.go b/internal/resolution/variables/bundle.go index 51723fb12..42db1fefa 100644 --- a/internal/resolution/variables/bundle.go +++ b/internal/resolution/variables/bundle.go @@ -62,6 +62,6 @@ func NewBundleUniquenessVariable(id deppy.Identifier, atMostIDs ...deppy.Identif // BundleVariableID returns an ID for a given bundle. func BundleVariableID(bundle *catalogmetadata.Bundle) deppy.Identifier { return deppy.Identifier( - fmt.Sprintf("%s-%s-%s", bundle.CatalogName, bundle.Package, bundle.Name), + fmt.Sprintf("%s-%s-%s", bundle.Catalog, bundle.Package, bundle.Name), ) } diff --git a/internal/resolution/variables/bundle_test.go b/internal/resolution/variables/bundle_test.go index 26d831f54..81231d078 100644 --- a/internal/resolution/variables/bundle_test.go +++ b/internal/resolution/variables/bundle_test.go @@ -15,26 +15,17 @@ import ( func TestBundleVariable(t *testing.T) { bundle := &catalogmetadata.Bundle{ - InChannels: []*catalogmetadata.Channel{ - {Channel: declcfg.Channel{Name: "stable"}}, - }, Bundle: declcfg.Bundle{Name: "bundle-1", Properties: []property.Property{ {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, }}, } dependencies := []*catalogmetadata.Bundle{ { - InChannels: []*catalogmetadata.Channel{ - {Channel: declcfg.Channel{Name: "stable"}}, - }, Bundle: declcfg.Bundle{Name: "bundle-2", Properties: []property.Property{ {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, }}, }, { - InChannels: []*catalogmetadata.Channel{ - {Channel: declcfg.Channel{Name: "stable"}}, - }, Bundle: declcfg.Bundle{Name: "bundle-3", Properties: []property.Property{ {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, }}, diff --git a/internal/resolution/variablesources/bundle.go b/internal/resolution/variablesources/bundle.go index 4a4851a21..e18be2dcd 100644 --- a/internal/resolution/variablesources/bundle.go +++ b/internal/resolution/variablesources/bundle.go @@ -45,7 +45,7 @@ func MakeBundleVariables( // get bundle dependencies dependencies, err := filterBundleDependencies(allBundles, head) if err != nil { - return nil, fmt.Errorf("could not determine dependencies for bundle %q from package %q in catalog %q: %s", head.Name, head.Package, head.CatalogName, err) + return nil, fmt.Errorf("could not determine dependencies for bundle %q from package %q in catalog %q: %s", head.Name, head.Package, head.Catalog, err) } // add bundle dependencies to queue for processing diff --git a/internal/resolution/variablesources/bundle_test.go b/internal/resolution/variablesources/bundle_test.go index b4c55bdbd..3b48ad582 100644 --- a/internal/resolution/variablesources/bundle_test.go +++ b/internal/resolution/variablesources/bundle_test.go @@ -21,7 +21,6 @@ import ( func TestMakeBundleVariables_ValidDepedencies(t *testing.T) { const fakeCatalogName = "fake-catalog" - fakeChannel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} bundleSet := map[string]*catalogmetadata.Bundle{ // Test package which we will be using as input into // the testable function @@ -34,8 +33,7 @@ func TestMakeBundleVariables_ValidDepedencies(t *testing.T) { {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "first-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&fakeChannel}, + Catalog: fakeCatalogName, }, // First level dependency of test-package. Will be explicitly @@ -52,8 +50,7 @@ func TestMakeBundleVariables_ValidDepedencies(t *testing.T) { {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "second-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&fakeChannel}, + Catalog: fakeCatalogName, }, // Second level dependency that matches requirements of the first level dependency. @@ -65,8 +62,7 @@ func TestMakeBundleVariables_ValidDepedencies(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "1.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&fakeChannel}, + Catalog: fakeCatalogName, }, // Second level dependency that matches requirements of the first level dependency. @@ -78,8 +74,7 @@ func TestMakeBundleVariables_ValidDepedencies(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "1.0.1"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&fakeChannel}, + Catalog: fakeCatalogName, }, // Second level dependency that does not match requirements of the first level dependency. @@ -91,8 +86,7 @@ func TestMakeBundleVariables_ValidDepedencies(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "2.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&fakeChannel}, + Catalog: fakeCatalogName, }, // Package that is in a our fake catalog, but is not involved @@ -106,8 +100,7 @@ func TestMakeBundleVariables_ValidDepedencies(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "uninvolved-package", "version": "1.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&fakeChannel}, + Catalog: fakeCatalogName, }, } @@ -162,7 +155,6 @@ func TestMakeBundleVariables_ValidDepedencies(t *testing.T) { func TestMakeBundleVariables_NonExistentDepedencies(t *testing.T) { const fakeCatalogName = "fake-catalog" - fakeChannel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} bundleSet := map[string]*catalogmetadata.Bundle{ "test-package.v1.0.0": { Bundle: declcfg.Bundle{ @@ -173,8 +165,7 @@ func TestMakeBundleVariables_NonExistentDepedencies(t *testing.T) { {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "first-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&fakeChannel}, + Catalog: fakeCatalogName, }, } diff --git a/internal/resolution/variablesources/bundle_uniqueness_test.go b/internal/resolution/variablesources/bundle_uniqueness_test.go index 6404bbcd3..2f79a4708 100644 --- a/internal/resolution/variablesources/bundle_uniqueness_test.go +++ b/internal/resolution/variablesources/bundle_uniqueness_test.go @@ -20,7 +20,6 @@ import ( func TestMakeBundleUniquenessVariables(t *testing.T) { const fakeCatalogName = "fake-catalog" - channel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} bundleSet := map[string]*catalogmetadata.Bundle{ "test-package.v1.0.0": { Bundle: declcfg.Bundle{ @@ -31,8 +30,7 @@ func TestMakeBundleUniquenessVariables(t *testing.T) { {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&channel}, + Catalog: fakeCatalogName, }, "test-package.v1.0.1": { Bundle: declcfg.Bundle{ @@ -43,8 +41,7 @@ func TestMakeBundleUniquenessVariables(t *testing.T) { {Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&channel}, + Catalog: fakeCatalogName, }, "some-package.v1.0.0": { @@ -55,8 +52,7 @@ func TestMakeBundleUniquenessVariables(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "1.0.0"}`)}, }, }, - CatalogName: fakeCatalogName, - InChannels: []*catalogmetadata.Channel{&channel}, + Catalog: fakeCatalogName, }, } diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index 339bded3b..6b3bbca07 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -11,7 +11,7 @@ import ( 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/internal/catalogmetadata/filter" catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/pkg/features" @@ -23,6 +23,7 @@ import ( // has own variable. func MakeInstalledPackageVariables( allBundles []*catalogmetadata.Bundle, + allChannels []*catalogmetadata.Channel, clusterExtensions []ocv1alpha1.ClusterExtension, bundleDeployments []rukpakv1alpha2.BundleDeployment, ) ([]*olmvariables.InstalledPackageVariable, error) { @@ -60,9 +61,9 @@ func MakeInstalledPackageVariables( bundleImage := sourceImage.Ref // find corresponding bundle for the installed content - resultSet := catalogfilter.Filter(allBundles, catalogfilter.And( - catalogfilter.WithPackageName(clusterExtension.Spec.PackageName), - catalogfilter.WithBundleImage(bundleImage), + resultSet := filter.Filter(allBundles, filter.And( + filter.WithPackageName(clusterExtension.Spec.PackageName), + filter.WithImage(bundleImage), )) if len(resultSet) == 0 { return nil, fmt.Errorf("bundle with image %q for package %q not found in available catalogs but is currently installed via BundleDeployment %q", bundleImage, clusterExtension.Spec.PackageName, bundleDeployment.Name) @@ -73,7 +74,19 @@ func MakeInstalledPackageVariables( }) installedBundle := resultSet[0] - upgradeEdges, err := successors(allBundles, installedBundle) + channels := filter.Filter[catalogmetadata.Channel](allChannels, filter.And[catalogmetadata.Channel]( + func(entity *catalogmetadata.Channel) bool { + return entity.Package == clusterExtension.Spec.PackageName + }, + func(entity *catalogmetadata.Channel) bool { + if clusterExtension.Spec.Channel != "" { + return entity.Name == clusterExtension.Spec.Channel + } + return true + }, + )) + + upgradeEdges, err := successors(allBundles, installedBundle, channels) if err != nil { return nil, err } @@ -89,16 +102,16 @@ func MakeInstalledPackageVariables( // successorsFunc must return successors of a currently installed bundle // from a list of all bundles provided to the function. // Must not return installed bundle as a successor -type successorsFunc func(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) +type successorsFunc func(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle, channels []*catalogmetadata.Channel) ([]*catalogmetadata.Bundle, error) // legacySemanticsSuccessors returns successors based on legacy OLMv0 semantics // which rely on Replaces, Skips and skipRange. -func legacySemanticsSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { +func legacySemanticsSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle, channels []*catalogmetadata.Channel) ([]*catalogmetadata.Bundle, error) { // find the bundles that replace the bundle provided // TODO: this algorithm does not yet consider skips and skipRange - upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.And( - catalogfilter.WithPackageName(installedBundle.Package), - catalogfilter.Replaces(installedBundle.Name), + upgradeEdges := filter.Filter(allBundles, filter.And( + filter.WithPackageName(installedBundle.Package), + filter.Replaces(installedBundle.Name, channels), )) sort.SliceStable(upgradeEdges, func(i, j int) bool { return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j]) @@ -110,7 +123,7 @@ func legacySemanticsSuccessors(allBundles []*catalogmetadata.Bundle, installedBu // semverSuccessors returns 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 semverSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { +func semverSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *catalogmetadata.Bundle, _ []*catalogmetadata.Channel) ([]*catalogmetadata.Bundle, error) { currentVersion, err := installedBundle.Version() if err != nil { return nil, err @@ -124,9 +137,9 @@ func semverSuccessors(allBundles []*catalogmetadata.Bundle, installedBundle *cat return nil, err } - upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.And( - catalogfilter.WithPackageName(installedBundle.Package), - catalogfilter.InMastermindsSemverRange(wantedVersionRangeConstraint), + upgradeEdges := filter.Filter(allBundles, filter.And( + filter.WithPackageName(installedBundle.Package), + filter.InMastermindsSemverRange(wantedVersionRangeConstraint), )) sort.SliceStable(upgradeEdges, func(i, j int) bool { return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j]) diff --git a/internal/resolution/variablesources/installed_package_test.go b/internal/resolution/variablesources/installed_package_test.go index 8dec706ee..b2513c9c5 100644 --- a/internal/resolution/variablesources/installed_package_test.go +++ b/internal/resolution/variablesources/installed_package_test.go @@ -29,10 +29,47 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t 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.v0.0.1", + }, + { + Name: "test-package.v0.0.2", + }, + { + Name: "test-package.v0.1.0", + }, + { + Name: "test-package.v0.1.1", + }, + { + Name: "test-package.v0.1.2", + }, + { + Name: "test-package.v0.2.0", + }, + { + Name: "test-package.v2.0.0", + }, + { + Name: "test-package.v2.1.0", + }, + { + Name: "test-package.v2.2.0", + }, + { + Name: "test-package.v3.0.0", + }, + }, }} bundleSet := map[string]*catalogmetadata.Bundle{ // Major version zero is for initial development and @@ -49,7 +86,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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{ @@ -60,7 +96,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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{ @@ -71,7 +106,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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{ @@ -82,7 +116,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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{ @@ -93,7 +126,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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{ @@ -104,7 +136,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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{ @@ -115,7 +146,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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{ @@ -126,7 +156,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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{ @@ -137,7 +166,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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 @@ -150,7 +178,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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 @@ -163,7 +190,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {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)) @@ -242,7 +268,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "9.0.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, }, expectedError: `bundle with image "registry.io/repo/test-package@v9.0.0" for package "test-package" not found in available catalogs but is currently installed via BundleDeployment "test-package-bd"`, }, @@ -256,6 +281,7 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsEnabled(t installedPackages, err := variablesources.MakeInstalledPackageVariables( allBundles, + []*catalogmetadata.Channel{&testPackageChannel, &someOtherPackageChannel}, []ocv1alpha1.ClusterExtension{fakeOwnerClusterExtension}, bundleDeployments, ) @@ -318,7 +344,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( {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{ @@ -329,7 +354,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( {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{ @@ -340,7 +364,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.2.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, }, // We need a bundle from different package to ensure that // we filter out bundles certain bundle image @@ -353,7 +376,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( {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)) @@ -405,7 +427,6 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "9.0.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&testPackageChannel}, }, expectedError: `bundle with image "registry.io/repo/test-package@v9.0.0" for package "test-package" not found in available catalogs but is currently installed via BundleDeployment "test-package-bd"`, }, @@ -419,6 +440,7 @@ func TestMakeInstalledPackageVariablesWithForceSemverUpgradeConstraintsDisabled( installedPackages, err := variablesources.MakeInstalledPackageVariables( allBundles, + []*catalogmetadata.Channel{&testPackageChannel, &someOtherPackageChannel}, []ocv1alpha1.ClusterExtension{fakeOwnerClusterExtension}, bundleDeployments, ) diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index 2f23a6062..2d7041411 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -8,7 +8,7 @@ import ( 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/internal/catalogmetadata/filter" catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) @@ -16,7 +16,7 @@ import ( // MakeRequiredPackageVariables returns a variable which represent // explicit requirement for a package from an user. // This is when a user explicitly asks "install this" via ClusterExtension API. -func MakeRequiredPackageVariables(allBundles []*catalogmetadata.Bundle, clusterExtensions []ocv1alpha1.ClusterExtension) ([]*olmvariables.RequiredPackageVariable, error) { +func MakeRequiredPackageVariables(allBundles []*catalogmetadata.Bundle, allChannels []*catalogmetadata.Channel, clusterExtensions []ocv1alpha1.ClusterExtension) ([]*olmvariables.RequiredPackageVariable, error) { result := make([]*olmvariables.RequiredPackageVariable, 0, len(clusterExtensions)) for _, clusterExtension := range clusterExtensions { @@ -24,12 +24,23 @@ func MakeRequiredPackageVariables(allBundles []*catalogmetadata.Bundle, clusterE channelName := clusterExtension.Spec.Channel versionRange := clusterExtension.Spec.Version - predicates := []catalogfilter.Predicate[catalogmetadata.Bundle]{ - catalogfilter.WithPackageName(packageName), + // get all channels that belong to the specified + // package. If a channel is specified that does not exist, no + // bundles should be returned from the filtering + // TODO: could probably optimize the InChannel filter to only accept + // a single channel. + channels := filter.Filter[catalogmetadata.Channel](allChannels, + func(entity *catalogmetadata.Channel) bool { + return entity.Package == packageName + }, + ) + + predicates := []filter.Predicate[catalogmetadata.Bundle]{ + filter.WithPackageName(packageName), } if channelName != "" { - predicates = append(predicates, catalogfilter.InChannel(channelName)) + predicates = append(predicates, filter.InChannel(channelName, channels)) } if versionRange != "" { @@ -37,10 +48,10 @@ func MakeRequiredPackageVariables(allBundles []*catalogmetadata.Bundle, clusterE if err != nil { return nil, fmt.Errorf("invalid version range %q: %w", versionRange, err) } - predicates = append(predicates, catalogfilter.InMastermindsSemverRange(vr)) + predicates = append(predicates, filter.InMastermindsSemverRange(vr)) } - resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...)) + resultSet := filter.Filter(allBundles, filter.And(predicates...)) if len(resultSet) == 0 { if versionRange != "" && channelName != "" { return nil, fmt.Errorf("no package %q matching version %q found in channel %q", packageName, versionRange, channelName) diff --git a/internal/resolution/variablesources/required_package_test.go b/internal/resolution/variablesources/required_package_test.go index 164f9a411..92cbc86d7 100644 --- a/internal/resolution/variablesources/required_package_test.go +++ b/internal/resolution/variablesources/required_package_test.go @@ -24,10 +24,37 @@ import ( func TestMakeRequiredPackageVariables(t *testing.T) { stableChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "stable", + Name: "stable", + Package: "test-package", + Entries: []declcfg.ChannelEntry{ + { + Name: "test-package.v1.0.0", + }, + { + Name: "test-package.v2.0.0", + }, + { + Name: "test-package.v3.0.0", + }, + { + Name: "test-package.v4.0.0", + }, + { + Name: "test-package.v4.1.0", + }, + { + Name: "test-package.v5.0.0", + }, + }, }} betaChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ - Name: "beta", + Name: "beta", + Package: "test-package", + Entries: []declcfg.ChannelEntry{ + { + Name: "test-package.v3.0.0", + }, + }, }} bundleSet := map[string]*catalogmetadata.Bundle{ // Bundles which belong to test-package we will be using @@ -41,7 +68,6 @@ func TestMakeRequiredPackageVariables(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&stableChannel}, }, "test-package.v3.0.0": { Bundle: declcfg.Bundle{ @@ -51,7 +77,6 @@ func TestMakeRequiredPackageVariables(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&stableChannel, &betaChannel}, }, "test-package.v2.0.0": { Bundle: declcfg.Bundle{ @@ -61,7 +86,6 @@ func TestMakeRequiredPackageVariables(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&stableChannel}, }, "test-package.v4.0.0": { Bundle: declcfg.Bundle{ @@ -71,15 +95,12 @@ func TestMakeRequiredPackageVariables(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "4.0.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&stableChannel}, - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaBundle, - Name: "test-package.v4.0.0", - }, - Message: "test-package.v4.0.0 has been deprecated", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaBundle, + Name: "test-package.v4.0.0", }, + Message: "test-package.v4.0.0 has been deprecated", }, }, "test-package.v4.1.0": { @@ -90,15 +111,12 @@ func TestMakeRequiredPackageVariables(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "4.1.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&stableChannel}, - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaBundle, - Name: "test-package.v4.1.0", - }, - Message: "test-package.v4.1.0 has been deprecated", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaBundle, + Name: "test-package.v4.1.0", }, + Message: "test-package.v4.1.0 has been deprecated", }, }, "test-package.v5.0.0": { @@ -109,15 +127,12 @@ func TestMakeRequiredPackageVariables(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "5.0.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&stableChannel}, - Deprecations: []declcfg.DeprecationEntry{ - { - Reference: declcfg.PackageScopedReference{ - Schema: declcfg.SchemaBundle, - Name: "test-package.v5.0.0", - }, - Message: "test-package.v5.0.0 has been deprecated", + Deprecation: &declcfg.DeprecationEntry{ + Reference: declcfg.PackageScopedReference{ + Schema: declcfg.SchemaBundle, + Name: "test-package.v5.0.0", }, + Message: "test-package.v5.0.0 has been deprecated", }, }, @@ -131,7 +146,6 @@ func TestMakeRequiredPackageVariables(t *testing.T) { {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "1.0.0"}`)}, }, }, - InChannels: []*catalogmetadata.Channel{&stableChannel}, }, } allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet)) @@ -233,7 +247,8 @@ func TestMakeRequiredPackageVariables(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - vars, err := variablesources.MakeRequiredPackageVariables(allBundles, tt.clusterExtensions) + channels := []*catalogmetadata.Channel{&stableChannel, &betaChannel} + vars, err := variablesources.MakeRequiredPackageVariables(allBundles, channels, tt.clusterExtensions) if tt.expectedError == "" { assert.NoError(t, err) } else { diff --git a/test/util/fake_catalog_client.go b/test/util/fake_catalog_client.go index 20c3c8b29..3478bf9a7 100644 --- a/test/util/fake_catalog_client.go +++ b/test/util/fake_catalog_client.go @@ -4,16 +4,21 @@ import ( "context" "github.com/operator-framework/operator-controller/internal/catalogmetadata" + "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" ) type FakeCatalogClient struct { - bundles []*catalogmetadata.Bundle - err error + bundles []*catalogmetadata.Bundle + channels []*catalogmetadata.Channel + packages []*catalogmetadata.Package + err error } -func NewFakeCatalogClient(b []*catalogmetadata.Bundle) FakeCatalogClient { +func NewFakeCatalogClient(b []*catalogmetadata.Bundle, c []*catalogmetadata.Channel, p []*catalogmetadata.Package) FakeCatalogClient { return FakeCatalogClient{ - bundles: b, + bundles: b, + channels: c, + packages: p, } } @@ -23,9 +28,9 @@ func NewFakeCatalogClientWithError(e error) FakeCatalogClient { } } -func (c *FakeCatalogClient) Bundles(_ context.Context) ([]*catalogmetadata.Bundle, error) { +func (c *FakeCatalogClient) CatalogContents(_ context.Context) (*client.Contents, error) { if c.err != nil { return nil, c.err } - return c.bundles, nil + return &client.Contents{Bundles: c.bundles, Channels: c.channels, Packages: c.packages}, nil } From 94fe3dfe15ff13aaf556bfa109c2e4f500042f64 Mon Sep 17 00:00:00 2001 From: everettraven Date: Fri, 2 Feb 2024 09:22:26 -0500 Subject: [PATCH 2/3] add lazy loading/caching back Signed-off-by: everettraven --- internal/catalogmetadata/types.go | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/internal/catalogmetadata/types.go b/internal/catalogmetadata/types.go index 9b8ba1548..b9436d55b 100644 --- a/internal/catalogmetadata/types.go +++ b/internal/catalogmetadata/types.go @@ -52,11 +52,18 @@ type PackageRequired struct { type Bundle struct { declcfg.Bundle - Deprecation *declcfg.DeprecationEntry - Catalog string + Deprecation *declcfg.DeprecationEntry + Catalog string + version *bsemver.Version + requiredPackages []PackageRequired + mediaType string } func (b *Bundle) Version() (*bsemver.Version, error) { + if b.version != nil { + return b.version, nil + } + pkg, err := loadOneFromProps[property.Package](b, property.TypePackage, true) if err != nil { return nil, err @@ -65,10 +72,15 @@ func (b *Bundle) Version() (*bsemver.Version, error) { if err != nil { return nil, fmt.Errorf("could not parse semver %q for bundle '%s': %s", pkg.Version, b.Name, err) } - return &semVer, nil + b.version = &semVer + return b.version, nil } func (b *Bundle) RequiredPackages() ([]PackageRequired, error) { + if len(b.requiredPackages) > 0 { + return b.requiredPackages, nil + } + requiredPackages, err := loadFromProps[PackageRequired](b, property.TypePackageRequired, false) if err != nil { return nil, fmt.Errorf("error determining bundle required packages for bundle %q: %s", b.Name, err) @@ -85,16 +97,21 @@ func (b *Bundle) RequiredPackages() ([]PackageRequired, error) { } requiredPackages[i].SemverRange = semverRange } - return requiredPackages, nil + b.requiredPackages = requiredPackages + return b.requiredPackages, nil } func (b *Bundle) MediaType() (string, error) { + if b.mediaType != "" { + return b.mediaType, nil + } + mediaType, err := loadOneFromProps[string](b, PropertyBundleMediaType, false) if err != nil { return "", fmt.Errorf("error determining bundle mediatype for bundle %q: %s", b.Name, err) } - - return mediaType, nil + b.mediaType = mediaType + return b.mediaType, nil } func (b *Bundle) propertiesByType(propType string) []*property.Property { From 32466fa1a827c8d58e8c34dbcf3b5981c481bd79 Mon Sep 17 00:00:00 2001 From: everettraven Date: Fri, 2 Feb 2024 09:27:47 -0500 Subject: [PATCH 3/3] better type checking Signed-off-by: everettraven --- cmd/resolutioncli/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/resolutioncli/client.go b/cmd/resolutioncli/client.go index 93d0697c9..88a64f434 100644 --- a/cmd/resolutioncli/client.go +++ b/cmd/resolutioncli/client.go @@ -34,7 +34,7 @@ type indexRefClient struct { packagesCache []*catalogmetadata.Package } -var _ controllers.CatalogProvider = &indexRefClient{} +var _ controllers.CatalogProvider = (*indexRefClient)(nil) func newIndexRefClient(indexRef string) *indexRefClient { return &indexRefClient{