🌱 Move applier to use Bundle interface

internal/operator-controller/applier/helm.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/fs"
 	"slices"
 	"strings"
 
@@ -26,6 +25,7 @@ import (
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
@@ -121,8 +121,8 @@ func (h *Helm) runPreAuthorizationChecks(ctx context.Context, ext *ocv1.ClusterE
 	return nil
 }
 
-func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
-	chrt, err := h.buildHelmChart(contentFS, ext)
+func (h *Helm) Apply(ctx context.Context, bundle bundle.Bundle, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
+	chrt, err := h.buildHelmChart(bundle, ext)
 	if err != nil {
 		return nil, "", err
 	}
@@ -203,10 +203,11 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
 	return relObjects, state, nil
 }
 
-func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) {
+func (h *Helm) buildHelmChart(bundle bundle.Bundle, ext *ocv1.ClusterExtension) (*chart.Chart, error) {
 	if h.BundleToHelmChartConverter == nil {
 		return nil, errors.New("BundleToHelmChartConverter is nil")
 	}
+	bundleFS := bundle.FS()
 	watchNamespace, err := GetWatchNamespace(ext)
 	if err != nil {
 		return nil, err

internal/operator-controller/applier/helm_test.go

@@ -25,6 +25,7 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
@@ -115,8 +116,8 @@ func (mag *mockActionGetter) Reconcile(rel *release.Release) error {
 }
 
 var (
-	// required for unmockable call to convert.RegistryV1ToHelmChart
-	validFS = fstest.MapFS{
+	// required for unmockable call to bundle
+	validBundle = bundle.FromFS(fstest.MapFS{
 		"metadata/annotations.yaml": &fstest.MapFile{Data: []byte(`annotations:
   operators.operatorframework.io.bundle.package.v1: my-package`)},
 		"manifests": &fstest.MapFile{Data: []byte(`apiVersion: operators.operatorframework.io/v1alpha1
@@ -133,7 +134,7 @@ spec:
       supported: true
     - type: AllNamespaces
       supported: true`)},
-	}
+	})
 
 	// required for unmockable call to util.ManifestObjects
 	validManifest = `apiVersion: v1
@@ -188,7 +189,7 @@ func TestApply_Base(t *testing.T) {
 	t.Run("fails converting content FS to helm chart", func(t *testing.T) {
 		helmApplier := applier.Helm{}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), os.DirFS("/"), testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), bundle.FromFS(os.DirFS("/")), testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.Nil(t, objs)
 		require.Empty(t, state)
@@ -201,7 +202,7 @@ func TestApply_Base(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "getting action client")
 		require.Nil(t, objs)
@@ -215,7 +216,7 @@ func TestApply_Base(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "getting current release")
 		require.Nil(t, objs)
@@ -234,7 +235,7 @@ func TestApply_Installation(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "attempting to dry-run install chart")
 		require.Nil(t, objs)
@@ -253,7 +254,7 @@ func TestApply_Installation(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "install pre-flight check")
 		require.Equal(t, applier.StateNeedsInstall, state)
@@ -270,7 +271,7 @@ func TestApply_Installation(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "installing chart")
 		require.Equal(t, applier.StateNeedsInstall, state)
@@ -290,7 +291,7 @@ func TestApply_Installation(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.NoError(t, err)
 		require.Equal(t, applier.StateNeedsInstall, state)
 		require.NotNil(t, objs)
@@ -310,7 +311,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "attempting to dry-run install chart")
 		require.Nil(t, objs)
@@ -334,7 +335,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "install pre-flight check")
 		require.Equal(t, applier.StateNeedsInstall, state)
@@ -363,7 +364,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 				},
 			},
 		}
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, validCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, validCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "problem running preauthorization")
 		require.Empty(t, state)
@@ -392,7 +393,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 				},
 			},
 		}
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, validCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, validCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, errMissingRBAC)
 		require.Empty(t, state)
@@ -423,7 +424,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 			},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, validCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, validCE, testObjectLabels, testStorageLabels)
 		require.NoError(t, err)
 		require.Equal(t, applier.StateNeedsInstall, state)
 		require.NotNil(t, objs)
@@ -446,7 +447,7 @@ func TestApply_Upgrade(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "attempting to dry-run upgrade chart")
 		require.Nil(t, objs)
@@ -469,7 +470,7 @@ func TestApply_Upgrade(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "upgrade pre-flight check")
 		require.Equal(t, applier.StateNeedsUpgrade, state)
@@ -491,7 +492,7 @@ func TestApply_Upgrade(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "upgrading chart")
 		require.Equal(t, applier.StateNeedsUpgrade, state)
@@ -514,7 +515,7 @@ func TestApply_Upgrade(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "reconciling charts")
 		require.Equal(t, applier.StateUnchanged, state)
@@ -534,7 +535,7 @@ func TestApply_Upgrade(t *testing.T) {
 			BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
 		}
 
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		objs, state, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.NoError(t, err)
 		require.Equal(t, applier.StateNeedsUpgrade, state)
 		require.NotNil(t, objs)
@@ -573,7 +574,7 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin
 			},
 		}
 
-		_, _, _ = helmApplier.Apply(context.TODO(), validFS, testExt, testObjectLabels, testStorageLabels)
+		_, _, _ = helmApplier.Apply(context.TODO(), validBundle, testExt, testObjectLabels, testStorageLabels)
 	})
 }
 
@@ -597,7 +598,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
 			},
 		}
 
-		_, _, _ = helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		_, _, _ = helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 	})
 
 	t.Run("surfaces chart generation errors", func(t *testing.T) {
@@ -616,7 +617,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
 			},
 		}
 
-		_, _, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		_, _, err := helmApplier.Apply(context.TODO(), validBundle, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
 	})
 }

internal/operator-controller/bundle/bundle.go

@@ -0,0 +1,19 @@
+package bundle
+
+import "io/fs"
+
+type Bundle interface {
+	FS() fs.FS
+}
+
+type fsBundle struct {
+	bundleFS fs.FS
+}
+
+func (f fsBundle) FS() fs.FS {
+	return f.bundleFS
+}
+
+func FromFS(bundleFS fs.FS) Bundle {
+	return fsBundle{bundleFS}
+}

internal/operator-controller/controllers/clusterextension_controller.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io/fs"
 	"strings"
 	"time"
 
@@ -50,6 +49,7 @@ import (
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager"
@@ -83,7 +83,7 @@ type Applier interface {
 	// Apply applies the content in the provided fs.FS using the configuration of the provided ClusterExtension.
 	// It also takes in a map[string]string to be applied to all applied resources as labels and another
 	// map[string]string used to create a unique identifier for a stored reference to the resources created.
-	Apply(context.Context, fs.FS, *ocv1.ClusterExtension, map[string]string, map[string]string) ([]client.Object, string, error)
+	Apply(context.Context, bundle.Bundle, *ocv1.ClusterExtension, map[string]string, map[string]string) ([]client.Object, string, error)
 }
 
 type InstalledBundleGetter interface {
@@ -296,7 +296,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl
 	// to ensure exponential backoff can occur:
 	//   - Permission errors (it is not possible to watch changes to permissions.
 	//     The only way to eventually recover from permission errors is to keep retrying).
-	managedObjs, _, err := r.Applier.Apply(ctx, imageFS, ext, objLbls, storeLbls)
+	managedObjs, _, err := r.Applier.Apply(ctx, bundle.FromFS(imageFS), ext, objLbls, storeLbls)
 	if err != nil {
 		setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
 		// Now that we're actually trying to install, use the error
@@ -338,13 +338,14 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl
 // based on the provided bundle
 func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, deprecation *declcfg.Deprecation) {
 	deprecations := map[string][]declcfg.DeprecationEntry{}
-	channelSet := sets.New[string]()
-	if ext.Spec.Source.Catalog != nil {
-		for _, channel := range ext.Spec.Source.Catalog.Channels {
-			channelSet.Insert(channel)
-		}
-	}
 	if deprecation != nil {
+		channelSet := sets.New[string]()
+		if ext.Spec.Source.Catalog != nil {
+			for _, channel := range ext.Spec.Source.Catalog.Channels {
+				channelSet.Insert(channel)
+			}
+		}
+
 		for _, entry := range deprecation.Entries {
 			switch entry.Reference.Schema {
 			case declcfg.SchemaPackage:

internal/operator-controller/controllers/suite_test.go

@@ -18,7 +18,6 @@ package controllers_test
 
 import (
 	"context"
-	"io/fs"
 	"log"
 	"os"
 	"path/filepath"
@@ -36,6 +35,7 @@ import (
 	helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager"
 	cmcache "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager/cache"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
@@ -73,7 +73,7 @@ type MockApplier struct {
 	state string
 }
 
-func (m *MockApplier) Apply(_ context.Context, _ fs.FS, _ *ocv1.ClusterExtension, _ map[string]string, _ map[string]string) ([]client.Object, string, error) {
+func (m *MockApplier) Apply(_ context.Context, _ bundle.Bundle, _ *ocv1.ClusterExtension, _ map[string]string, _ map[string]string) ([]client.Object, string, error) {
 	if m.err != nil {
 		return nil, m.state, m.err
 	}