From e38fae1eae39a685dc7000b89c4c85f5491f46b8 Mon Sep 17 00:00:00 2001
From: Varsha Prasad Narsing <varshaprasad96@gmail.com>
Date: Tue, 8 Aug 2023 16:08:10 -0400
Subject: [PATCH] fix: Adding namespace when converting bundle from registryv1
 to plain

Currently, if the objects being passed through the bundle do not have their
namespaces set, they were being created on the "rukpak-system" namespace.
This PR fixes this issue for only "registryV1" bundles. It sets the
namespaces to installNs if not provided by default.

Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
---
 internal/convert/registryv1.go      |   3 +
 internal/convert/registryv1_test.go | 122 ++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+)
 create mode 100644 internal/convert/registryv1_test.go

diff --git a/internal/convert/registryv1.go b/internal/convert/registryv1.go
index 1bdb00fb..13fc1518 100644
--- a/internal/convert/registryv1.go
+++ b/internal/convert/registryv1.go
@@ -321,6 +321,9 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 	}
 	for _, obj := range in.Others {
 		obj := obj
+		if obj.GetNamespace() == "" {
+			obj.SetNamespace(installNamespace)
+		}
 		objs = append(objs, &obj)
 	}
 	for _, obj := range deployments {
diff --git a/internal/convert/registryv1_test.go b/internal/convert/registryv1_test.go
new file mode 100644
index 00000000..daa8c994
--- /dev/null
+++ b/internal/convert/registryv1_test.go
@@ -0,0 +1,122 @@
+package convert
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func TestRegistryV1Converter(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "RegstryV1 suite")
+}
+
+var _ = Describe("RegistryV1 Suite", func() {
+	var _ = Describe("Convert", func() {
+		var (
+			registryv1Bundle RegistryV1
+			installNamespace string
+			targetNamespaces []string
+		)
+		Context("Should set the namespaces of object correctly", func() {
+			var (
+				svc corev1.Service
+				csv v1alpha1.ClusterServiceVersion
+			)
+			BeforeEach(func() {
+				csv = v1alpha1.ClusterServiceVersion{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "testCSV",
+					},
+					Spec: v1alpha1.ClusterServiceVersionSpec{
+						InstallModes: []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}},
+					},
+				}
+				svc = corev1.Service{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "testService",
+					},
+				}
+				svc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"})
+				installNamespace = "testInstallNamespace"
+			})
+
+			It("should set the namespace to installnamespace if not available", func() {
+				By("creating a registry v1 bundle")
+				unstructuredSvc := convertToUnstructured(svc)
+				registryv1Bundle = RegistryV1{
+					PackageName: "testPkg",
+					CSV:         csv,
+					Others:      []unstructured.Unstructured{unstructuredSvc},
+				}
+
+				By("converting to plain")
+				plainBundle, err := Convert(registryv1Bundle, installNamespace, targetNamespaces)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("verifying if plain bundle has required objects")
+				Expect(plainBundle).NotTo(BeNil())
+				Expect(len(plainBundle.Objects)).To(BeEquivalentTo(2))
+
+				By("verifying if ns has been set correctly")
+				found, resObj := containsObject(unstructuredSvc, plainBundle.Objects)
+				Expect(found).To(BeTrue())
+				Expect(resObj).NotTo(BeNil())
+				Expect(resObj.GetNamespace()).To(BeEquivalentTo(installNamespace))
+			})
+
+			It("should not override namespace if already available", func() {
+				By("creating a registry v1 bundle")
+				svc.SetNamespace("otherNs")
+				unstructuredSvc := convertToUnstructured(svc)
+				unstructuredSvc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"})
+
+				registryv1Bundle = RegistryV1{
+					PackageName: "testPkg",
+					CSV:         csv,
+					Others:      []unstructured.Unstructured{unstructuredSvc},
+				}
+
+				By("converting to plain")
+				plainBundle, err := Convert(registryv1Bundle, installNamespace, targetNamespaces)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("verifying if plain bundle has required objects")
+				Expect(plainBundle).NotTo(BeNil())
+				Expect(len(plainBundle.Objects)).To(BeEquivalentTo(2))
+
+				By("verifying if ns has been set correctly")
+				found, resObj := containsObject(unstructuredSvc, plainBundle.Objects)
+				Expect(found).To(BeTrue())
+				Expect(resObj).NotTo(BeNil())
+				Expect(resObj.GetNamespace()).To(BeEquivalentTo("otherNs"))
+			})
+		})
+	})
+})
+
+func convertToUnstructured(obj interface{}) unstructured.Unstructured {
+	unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&obj)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(unstructuredObj).NotTo(BeNil())
+	return unstructured.Unstructured{Object: unstructuredObj}
+}
+
+func containsObject(obj unstructured.Unstructured, result []client.Object) (bool, 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.
+		if o.GetName() == obj.GetName() {
+			return true, o
+		}
+	}
+	return false, nil
+}