(wip): reduce complexity of bundle type used to build resolution vari…

…ables

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

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)

cmd/resolutioncli/client.go

@@ -22,7 +22,6 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/action"
 
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
-	"github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
 )
 
 type indexRefClient struct {
@@ -71,12 +70,12 @@ func (c *indexRefClient) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle
 		}
 
 		// TODO: update fake catalog name string to be catalog name once we support multiple catalogs in CLI
-		catalogName := "offline-catalog"
+		// catalogName := "offline-catalog"
 
-		bundles, err = client.PopulateExtraFields(catalogName, channels, bundles, deprecations)
-		if err != nil {
-			return nil, err
-		}
+		// bundles, err = client.PopulateExtraFields(catalogName, channels, bundles, deprecations)
+		// if err != nil {
+		// 	return nil, err
+		// }
 
 		c.bundlesCache = bundles
 	}

cmd/resolutioncli/main.go

@@ -36,6 +36,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"
 )
@@ -162,7 +163,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(&client.Contents{Bundles: allBundles}, clusterExtensionList.Items, bundleDeploymentList.Items)
 	if err != nil {
 		return err
 	}

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
 }

internal/catalogmetadata/client/client_test.go

@@ -127,37 +127,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"}}]}`)...)
 
 					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].Deprecation = &declcfg.DeprecationEntry{
 								Reference: declcfg.PackageScopedReference{
 									Schema: "olm.bundle",
 									Name:   "fake1.v1.0.0",
 								},
 								Message: "bundle fake1.v1.0.0 is deprecated",
-							})
+							}
 						}
 					}
 
@@ -176,7 +161,7 @@ func TestClient(t *testing.T) {
 					tt.fetcher,
 				)
 
-				bundles, err := fakeCatalogClient.Bundles(ctx)
+				bundles, err := fakeCatalogClient.CatalogContents(ctx)
 				if tt.wantErr == "" {
 					assert.NoError(t, err)
 					assert.Equal(t, expectedBundles, bundles)
@@ -262,7 +247,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 +260,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 +275,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",
-							},
-						},
-					},
-				},
-			},
 		},
 	}
 

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
 	}
 }