Repository: operator-framework/operator-controller
Pull request #1735 (2025-02-10 12:04:01)

:seedling: Decompose RegistryV1ToHelmChart function
# Description

Refactor `RegistryV1ToHelmChart` function in the `rukpak/convert` package by taking out the code that parses a bundle filesystem into a `RegistryV1` object and putting it into its own function `ParseFS`. This facilitates manipulation of bundles in other activities, e.g. tooling/CLI

## Reviewer Checklist

- [ ] API Go Documentation
- [ ] Tests: Unit Tests (and E2E Tests, if appropriate)
- [ ] Comprehensive Commit Messages
- [ ] Links to related GitHub Issue(s)


netlify[bot]: ### <span aria-hidden="true">👷</span> Deploy Preview for *olmv1* processing.


|  Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | e4070ecdf6b373959ba127f0d0fc56f1341a28f3 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/olmv1/deploys/67a9eb33cb29660008aa0b68 |
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1735?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) Report
Attention: Patch coverage is `36.36364%` with `7 lines` in your changes missing coverage. Please review.
> Project coverage is 68.22%. Comparing base [(`d72e551`)](https://app.codecov.io/gh/operator-framework/operator-controller/commit/d72e5518c1605bde9359f643c44934c9885fdadf?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) to head [(`e4070ec`)](https://app.codecov.io/gh/operator-framework/operator-controller/commit/e4070ecdf6b373959ba127f0d0fc56f1341a28f3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).

| [Files with missing lines](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1735?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | Patch % | Lines |
|---|---|---|
| [internal/rukpak/convert/registryv1.go](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1735?src=pr&el=tree&filepath=internal%2Frukpak%2Fconvert%2Fregistryv1.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework#diff-aW50ZXJuYWwvcnVrcGFrL2NvbnZlcnQvcmVnaXN0cnl2MS5nbw==) | 36.36% | [6 Missing and 1 partial :warning: ](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1735?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1735      +/-   ##
==========================================
- Coverage   68.24%   68.22%   -0.03%     
==========================================
  Files          58       58              
  Lines        4988     4994       +6     
==========================================
+ Hits         3404     3407       +3     
- Misses       1342     1344       +2     
- Partials      242      243       +1     
```

| [Flag](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1735/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | Coverage Δ | |
|---|---|---|
| [e2e](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1735/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `52.92% <36.36%> (-0.01%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1735/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `55.44% <36.36%> (-0.01%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1735?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).

tmshort: broken unit test due to removal of function.
perdasilva: > broken unit test due to removal of function.

Sorry about that. Shot from the hip as I was getting rushed off to dinner. I've fixed it up by removing the unit test. The test below it performs the same test. I also moved it to the test package to help us avoid testing private functions in the future.
openshift-ci[bot]: New changes are detected. LGTM label has been removed.
LalatenduMohanty: @perdasilva #1737 got merged, so this PR need a rebase. 
openshift-merge-robot: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
openshift-ci[bot]: New changes are detected. LGTM label has been removed.

Patch:
diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/convert/registryv1.go
index 0bbc7df..b08c4a9 100644
--- a/internal/operator-controller/rukpak/convert/registryv1.go
+++ b/internal/operator-controller/rukpak/convert/registryv1.go
@@ -42,20 +42,57 @@ type Plain struct {
 }
 
 func RegistryV1ToHelmChart(ctx context.Context, rv1 fs.FS, installNamespace string, watchNamespaces []string) (*chart.Chart, error) {
+	reg, err := ParseFS(ctx, rv1)
+	if err != nil {
+		return nil, err
+	}
+
+	plain, err := Convert(reg, installNamespace, watchNamespaces)
+	if err != nil {
+		return nil, err
+	}
+
+	chrt := &chart.Chart{Metadata: &chart.Metadata{}}
+	chrt.Metadata.Annotations = reg.CSV.GetAnnotations()
+	for _, obj := range plain.Objects {
+		jsonData, err := json.Marshal(obj)
+		if err != nil {
+			return nil, err
+		}
+		hash := sha256.Sum256(jsonData)
+		chrt.Templates = append(chrt.Templates, &chart.File{
+			Name: fmt.Sprintf("object-%x.json", hash[0:8]),
+			Data: jsonData,
+		})
+	}
+
+	return chrt, nil
+}
+
+// ParseFS converts the rv1 filesystem into a RegistryV1.
+// ParseFS expects the filesystem to conform to the registry+v1 format:
+// metadata/annotations.yaml
+// manifests/
+//   - csv.yaml
+//   - ...
+//
+// manifests directory does not contain subdirectories
+func ParseFS(ctx context.Context, rv1 fs.FS) (RegistryV1, error) {
 	l := log.FromContext(ctx)
 
 	reg := RegistryV1{}
 	annotationsFileData, err := fs.ReadFile(rv1, filepath.Join("metadata", "annotations.yaml"))
 	if err != nil {
-		return nil, err
+		return reg, err
 	}
 	annotationsFile := registry.AnnotationsFile{}
 	if err := yaml.Unmarshal(annotationsFileData, &annotationsFile); err != nil {
-		return nil, err
+		return reg, err
 	}
 	reg.PackageName = annotationsFile.Annotations.PackageName
 
 	const manifestsDir = "manifests"
+	foundCSV := false
 	if err := fs.WalkDir(rv1, manifestsDir, func(path string, e fs.DirEntry, err error) error {
 		if err != nil {
 			return err
@@ -91,6 +128,7 @@ func RegistryV1ToHelmChart(ctx context.Context, rv1 fs.FS, installNamespace stri
 					return err
 				}
 				reg.CSV = csv
+				foundCSV = true
 			default:
 				reg.Others = append(reg.Others, *info.Object.(*unstructured.Unstructured))
 			}
@@ -100,14 +138,18 @@ func RegistryV1ToHelmChart(ctx context.Context, rv1 fs.FS, installNamespace stri
 		}
 		return nil
 	}); err != nil {
-		return nil, err
+		return reg, err
+	}
+
+	if !foundCSV {
+		return reg, fmt.Errorf("no ClusterServiceVersion found in %q", manifestsDir)
 	}
 
 	if err := copyMetadataPropertiesToCSV(&reg.CSV, rv1); err != nil {
-		return nil, err
+		return reg, err
 	}
 
-	return toChart(reg, installNamespace, watchNamespaces)
+	return reg, nil
 }
 
 // copyMetadataPropertiesToCSV copies properties from `metadata/propeties.yaml` (in the filesystem fsys) into
@@ -158,29 +200,6 @@ func copyMetadataPropertiesToCSV(csv *v1alpha1.ClusterServiceVersion, fsys fs.FS
 	return nil
 }
 
-func toChart(in RegistryV1, installNamespace string, watchNamespaces []string) (*chart.Chart, error) {
-	plain, err := Convert(in, installNamespace, watchNamespaces)
-	if err != nil {
-		return nil, err
-	}
-
-	chrt := &chart.Chart{Metadata: &chart.Metadata{}}
-	chrt.Metadata.Annotations = in.CSV.GetAnnotations()
-	for _, obj := range plain.Objects {
-		jsonData, err := json.Marshal(obj)
-		if err != nil {
-			return nil, err
-		}
-		hash := sha256.Sum256(jsonData)
-		chrt.Templates = append(chrt.Templates, &chart.File{
-			Name: fmt.Sprintf("object-%x.json", hash[0:8]),
-			Data: jsonData,
-		})
-	}
-
-	return chrt, nil
-}
-
 func validateTargetNamespaces(supportedInstallModes sets.Set[string], installNamespace string, targetNamespaces []string) error {
 	set := sets.New[string](targetNamespaces...)
 	switch {


Test patch:
diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go
index 4e36059..42786fa 100644
--- a/internal/operator-controller/rukpak/convert/registryv1_test.go
+++ b/internal/operator-controller/rukpak/convert/registryv1_test.go
@@ -1,11 +1,13 @@
-package convert
+package convert_test
 
 import (
 	"context"
 	"fmt"
+	"io/fs"
 	"os"
 	"strings"
 	"testing"
+	"testing/fstest"
 
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
@@ -20,12 +22,17 @@ import (
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/operator-framework/operator-registry/alpha/property"
+
+	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
 )
 
 const (
 	olmNamespaces    = "olm.targetNamespaces"
 	olmProperties    = "olm.properties"
 	installNamespace = "testInstallNamespace"
+
+	bundlePathAnnotations = "metadata/annotations.yaml"
+	bundlePathCSV         = "manifests/csv.yaml"
 )
 
 func getCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) {
@@ -57,14 +64,14 @@ func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) {
 	csv, svc := getCsvAndService()
 
 	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, targetNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -91,14 +98,14 @@ func TestRegistryV1SuiteNamespaceAvailable(t *testing.T) {
 	unstructuredSvc := convertToUnstructured(t, svc)
 	unstructuredSvc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"})
 
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, targetNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -132,14 +139,14 @@ func TestRegistryV1SuiteNamespaceUnsupportedKind(t *testing.T) {
 	unstructuredEvt := convertToUnstructured(t, event)
 	unstructuredEvt.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Event"})
 
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         csv,
 		Others:      []unstructured.Unstructured{unstructuredEvt},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, targetNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "bundle contains unsupported resource")
 	require.Nil(t, plainBundle)
@@ -166,14 +173,14 @@ func TestRegistryV1SuiteNamespaceClusterScoped(t *testing.T) {
 	unstructuredpriorityclass := convertToUnstructured(t, pc)
 	unstructuredpriorityclass.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "PriorityClass"})
 
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         csv,
 		Others:      []unstructured.Unstructured{unstructuredpriorityclass},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, targetNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -253,14 +260,14 @@ func TestRegistryV1SuiteGenerateAllNamespace(t *testing.T) {
 	t.Log("By creating a registry v1 bundle")
 	watchNamespaces := []string{""}
 	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         *csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -286,14 +293,14 @@ func TestRegistryV1SuiteGenerateMultiNamespace(t *testing.T) {
 	t.Log("By creating a registry v1 bundle")
 	watchNamespaces := []string{"testWatchNs1", "testWatchNs2"}
 	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         *csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -319,14 +326,14 @@ func TestRegistryV1SuiteGenerateSingleNamespace(t *testing.T) {
 	t.Log("By creating a registry v1 bundle")
 	watchNamespaces := []string{"testWatchNs1"}
 	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         *csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -352,14 +359,14 @@ func TestRegistryV1SuiteGenerateOwnNamespace(t *testing.T) {
 	t.Log("By creating a registry v1 bundle")
 	watchNamespaces := []string{installNamespace}
 	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         *csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -385,14 +392,14 @@ func TestRegistryV1SuiteGenerateErrorMultiNamespaceEmpty(t *testing.T) {
 	t.Log("By creating a registry v1 bundle")
 	watchNamespaces := []string{"testWatchNs1", ""}
 	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         *csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.Error(t, err)
 	require.Nil(t, plainBundle)
 }
@@ -409,14 +416,14 @@ func TestRegistryV1SuiteGenerateErrorSingleNamespaceDisabled(t *testing.T) {
 	t.Log("By creating a registry v1 bundle")
 	watchNamespaces := []string{"testWatchNs1", "testWatchNs2"}
 	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         *csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.Error(t, err)
 	require.Nil(t, plainBundle)
 }
@@ -438,50 +445,68 @@ func TestRegistryV1SuiteGenerateErrorAllNamespaceDisabled(t *testing.T) {
 	t.Log("By creating a registry v1 bundle")
 	watchNamespaces := []string{""}
 	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         *csv,
 		Others:      []unstructured.Unstructured{unstructuredSvc},
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.Error(t, err)
 	require.Nil(t, plainBundle)
 }
 
-func TestRegistryV1SuiteGeneratePropagateCsvAnnotations(t *testing.T) {
+func TestRegistryV1SuiteReadBundleFileSystem(t *testing.T) {
 	t.Log("RegistryV1 Suite Convert")
 	t.Log("It should generate objects successfully based on target namespaces")
 
-	t.Log("It should propagate csv annotations to chart metadata annotation")
-	baseCSV, svc := getBaseCsvAndService()
-	csv := baseCSV.DeepCopy()
-	csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}}
-
-	t.Log("By creating a registry v1 bundle")
-	watchNamespaces := []string{"testWatchNs1", "testWatchNs2"}
-	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := RegistryV1{
-		PackageName: "testPkg",
-		CSV:         *csv,
-		Others:      []unstructured.Unstructured{unstructuredSvc},
-	}
-
-	t.Log("By converting to helm")
-	chrt, err := toChart(registryv1Bundle, installNamespace, watchNamespaces)
+	t.Log("It should read the registry+v1 bundle filesystem correctly")
+	t.Log("It should include metadata/properties.yaml and csv.metadata.annotations['olm.properties'] in chart metadata")
+	fsys := os.DirFS("testdata/combine-properties-bundle")
+	chrt, err := convert.RegistryV1ToHelmChart(context.Background(), fsys, "", nil)
 	require.NoError(t, err)
+	require.NotNil(t, chrt)
+	require.NotNil(t, chrt.Metadata)
 	require.Contains(t, chrt.Metadata.Annotations, olmProperties)
+	require.JSONEq(t, `[{"type":"from-csv-annotations-key","value":"from-csv-annotations-value"},{"type":"from-file-key","value":"from-file-value"}]`, chrt.Metadata.Annotations[olmProperties])
 }
 
-func TestRegistryV1SuiteReadBundleFileSystem(t *testing.T) {
+func TestParseFSFails(t *testing.T) {
+	for _, tt := range []struct {
+		name string
+		FS   fs.FS
+	}{
+		{
+			name: "bundle missing ClusterServiceVersion manifest",
+			FS:   removePaths(newBundleFS(), bundlePathCSV),
+		}, {
+			name: "bundle missing metadata/annotations.yaml",
+			FS:   removePaths(newBundleFS(), bundlePathAnnotations),
+		}, {
+			name: "bundle missing metadata/ directory",
+			FS:   removePaths(newBundleFS(), "metadata/"),
+		}, {
+			name: "bundle missing manifests/ directory",
+			FS:   removePaths(newBundleFS(), "manifests/"),
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := convert.ParseFS(context.Background(), tt.FS)
+			require.Error(t, err)
+		})
+	}
+}
+
+func TestRegistryV1SuiteReadBundleFileSystemFailsOnNoCSV(t *testing.T) {
 	t.Log("RegistryV1 Suite Convert")
 	t.Log("It should generate objects successfully based on target namespaces")
 
 	t.Log("It should read the registry+v1 bundle filesystem correctly")
 	t.Log("It should include metadata/properties.yaml and csv.metadata.annotations['olm.properties'] in chart metadata")
 	fsys := os.DirFS("testdata/combine-properties-bundle")
-	chrt, err := RegistryV1ToHelmChart(context.Background(), fsys, "", nil)
+
+	chrt, err := convert.RegistryV1ToHelmChart(context.Background(), fsys, "", nil)
 	require.NoError(t, err)
 	require.NotNil(t, chrt)
 	require.NotNil(t, chrt.Metadata)
@@ -506,13 +531,13 @@ func TestRegistryV1SuiteGenerateNoWebhooks(t *testing.T) {
 		},
 	}
 	watchNamespaces := []string{metav1.NamespaceAll}
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         csv,
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "webhookDefinitions are not supported")
 	require.Nil(t, plainBundle)
@@ -537,13 +562,13 @@ func TestRegistryV1SuiteGenerateNoAPISerciceDefinitions(t *testing.T) {
 		},
 	}
 	watchNamespaces := []string{metav1.NamespaceAll}
-	registryv1Bundle := RegistryV1{
+	registryv1Bundle := convert.RegistryV1{
 		PackageName: "testPkg",
 		CSV:         csv,
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "apiServiceDefintions are not supported")
 	require.Nil(t, plainBundle)
@@ -559,10 +584,47 @@ func convertToUnstructured(t *testing.T, obj interface{}) unstructured.Unstructu
 func findObjectByName(name string, result []client.Object) client.Object {
 	for _, o := range result {
 		// Since this is a controlled env, comparing only the names is sufficient for now.
-		// In future, compare GVKs too by ensuring its set on the unstructuredObj.
+		// In the future, compare GVKs too by ensuring its set on the unstructuredObj.
 		if o.GetName() == name {
 			return o
 		}
 	}
 	return nil
 }
+
+func newBundleFS() fstest.MapFS {
+	annotationsYml := `
+annotations:
+  operators.operatorframework.io.bundle.mediatype.v1: registry+v1
+  operators.operatorframework.io.bundle.package.v1: test
+`
+
+	csvYml := `
+apiVersion: operators.operatorframework.io/v1alpha1
+kind: ClusterServiceVersion
+metadata:
+  name: test.v1.0.0
+  annotations:
+    olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]'
+spec:
+  installModes:
+    - type: AllNamespaces
+      supported: true
+`
+
+	return fstest.MapFS{
+		bundlePathAnnotations: &fstest.MapFile{Data: []byte(strings.Trim(annotationsYml, "\n"))},
+		bundlePathCSV:         &fstest.MapFile{Data: []byte(strings.Trim(csvYml, "\n"))},
+	}
+}
+
+func removePaths(mapFs fstest.MapFS, paths ...string) fstest.MapFS {
+	for k := range mapFs {
+		for _, path := range paths {
+			if strings.HasPrefix(k, path) {
+				delete(mapFs, k)
+			}
+		}
+	}
+	return mapFs
+}