diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index ecfb3fdc2..a12c8a4a0 100644 --- a/internal/operator-controller/applier/helm.go +++ b/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 diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 89c94df88..f4b490cc7 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/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) }) } diff --git a/internal/operator-controller/bundle/bundle.go b/internal/operator-controller/bundle/bundle.go new file mode 100644 index 000000000..f576345b1 --- /dev/null +++ b/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} +} diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 24824bfd1..ef761ede2 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/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: diff --git a/internal/operator-controller/controllers/suite_test.go b/internal/operator-controller/controllers/suite_test.go index f287982ec..3ac137bcb 100644 --- a/internal/operator-controller/controllers/suite_test.go +++ b/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 }