fix: Don't replace namespace annotation when kubevirt namespace is set

in ssp CR When ssp CR contains kubevirt namespace as a target namespace for pipelines, ssp operator recognized this namespace as user defined and deployed configmaps and roleBindings to a single namespace, instead of two namespaces. That broke our default pipeline flow. This change adds a check if the pipeline namespace in the CR is kubevirt, then it is not used as a user defined namespace and kubevirt.io/tekton-piplines-deploy-namespace annotation is used instead. rename kubevirt.io/deploy-namespace annotation to kubevirt.io/tekton-piplines-deploy-namespace Signed-off-by: Karel Simon <[email protected]>
kubevirt · Sep 11, 2023 · 25dd686 · 25dd686
1 parent 72c7fba
commit 25dd686
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 23 deletions.
diff --git a/data/tekton-pipelines/pipelines-rbac.yaml b/data/tekton-pipelines/pipelines-rbac.yaml
@@ -4,7 +4,7 @@ kind: RoleBinding
 metadata:
   name: windows-pipelines-modify-data-object
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 roleRef:
   kind: ClusterRole
   name: modify-data-object-task
@@ -18,7 +18,7 @@ kind: RoleBinding
 metadata:
   name: windows-pipelines-create-vm
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 roleRef:
   kind: ClusterRole
   name: create-vm-from-manifest-task
@@ -32,7 +32,7 @@ kind: RoleBinding
 metadata:
   name: windows-pipelines-wait-for-vmi
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 roleRef:
   kind: ClusterRole
   name: modify-data-object-task
@@ -46,7 +46,7 @@ kind: RoleBinding
 metadata:
   name: windows-pipelines-cleanup-vm
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 roleRef:
   kind: ClusterRole
   name: cleanup-vm-task

diff --git a/data/tekton-pipelines/windows-bios-installer-configmaps.yaml b/data/tekton-pipelines/windows-bios-installer-configmaps.yaml
@@ -4,7 +4,7 @@ kind: ConfigMap
 metadata:
   name: windows10-bios-autounattend
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 data:
   autounattend.xml: |
     <?xml version="1.0" encoding="utf-8"?>

diff --git a/data/tekton-pipelines/windows-customize-configmaps.yaml b/data/tekton-pipelines/windows-customize-configmaps.yaml
@@ -4,7 +4,7 @@ kind: ConfigMap
 metadata:
   name: windows-sqlserver
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 data:
   unattend.xml: |
     <?xml version="1.0" encoding="utf-8"?>
@@ -63,7 +63,7 @@ kind: ConfigMap
 metadata:
   name: windows-vs-code
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 data:
   unattend.xml: |
     <?xml version="1.0" encoding="utf-8"?>
@@ -122,7 +122,7 @@ kind: ConfigMap
 metadata:
   name: windows10-unattend
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 data:
   unattend.xml: |
     <?xml version="1.0" encoding="utf-8"?>
@@ -195,7 +195,7 @@ kind: ConfigMap
 metadata:
   name: windows11-unattend
   annotations:
-    "kubevirt.io/deploy-namespace": "kubevirt-os-images"
+    "kubevirt.io/tekton-piplines-deploy-namespace": "kubevirt-os-images"
 data:
   unattend.xml: |
     <?xml version="1.0" encoding="utf-8"?>

diff --git a/internal/operands/tekton-pipelines/reconcile.go b/internal/operands/tekton-pipelines/reconcile.go
@@ -25,8 +25,9 @@ const (
 	operandName                = "tekton-pipelines"
 	operandComponent           = common.AppComponentTektonPipelines
 	tektonCrd                  = "tasks.tekton.dev"
-	deployNamespaceAnnotation  = "kubevirt.io/deploy-namespace"
+	deployNamespaceAnnotation  = "kubevirt.io/tekton-piplines-deploy-namespace"
 	pipelineServiceAccountName = "pipeline"
+	kubevirtNamespace          = "kubevirt"
 )
 
 var namespaceRegex = regexp.MustCompile(namespacePattern)
@@ -133,9 +134,14 @@ func (t *tektonPipelines) Cleanup(request *common.Request) ([]common.CleanupResu
 		o := sa.DeepCopy()
 		objects = append(objects, o)
 	}
-	namespace, _ := getTektonPipelinesNamespace(request)
-	for i := range objects {
-		objects[i].SetNamespace(namespace)
+
+	namespace, isUserDefinedNamespace := getTektonPipelinesNamespace(request)
+	for i, o := range objects {
+		objectNamespace := namespace
+		if value, ok := o.GetAnnotations()[deployNamespaceAnnotation]; ok && !isUserDefinedNamespace {
+			objectNamespace = value
+		}
+		objects[i].SetNamespace(objectNamespace)
 	}
 
 	for _, cr := range t.clusterRoles {
@@ -180,12 +186,12 @@ func reconcileTektonPipelinesFuncs(pipelines []pipeline.Pipeline) []common.Recon
 
 func reconcileConfigMapsFuncs(configMaps []v1.ConfigMap) []common.ReconcileFunc {
 	funcs := make([]common.ReconcileFunc, 0, len(configMaps))
-	var userDefinedNamespace bool
+	var isUserDefinedNamespace bool
 	for i := range configMaps {
 		configMap := &configMaps[i]
 		funcs = append(funcs, func(request *common.Request) (common.ReconcileResult, error) {
-			configMap.Namespace, userDefinedNamespace = getTektonPipelinesNamespace(request)
-			if value, ok := configMap.Annotations[deployNamespaceAnnotation]; ok && !userDefinedNamespace {
+			configMap.Namespace, isUserDefinedNamespace = getTektonPipelinesNamespace(request)
+			if value, ok := configMap.Annotations[deployNamespaceAnnotation]; ok && !isUserDefinedNamespace {
 				configMap.Namespace = value
 			}
 			return common.CreateOrUpdate(request).
@@ -261,8 +267,8 @@ func reconcileRoleBindingsFuncs(rolebindings []rbac.RoleBinding) []common.Reconc
 	for i := range rolebindings {
 		roleBinding := &rolebindings[i]
 		funcs = append(funcs, func(request *common.Request) (common.ReconcileResult, error) {
-			namespace, userDefinedNamespace := getTektonPipelinesNamespace(request)
-			if value, ok := roleBinding.Annotations[deployNamespaceAnnotation]; ok && !userDefinedNamespace {
+			namespace, isUserDefinedNamespace := getTektonPipelinesNamespace(request)
+			if value, ok := roleBinding.Annotations[deployNamespaceAnnotation]; ok && !isUserDefinedNamespace {
 				roleBinding.Namespace = value
 			} else {
 				roleBinding.Namespace = namespace
@@ -281,7 +287,7 @@ func reconcileRoleBindingsFuncs(rolebindings []rbac.RoleBinding) []common.Reconc
 }
 
 func getTektonPipelinesNamespace(request *common.Request) (string, bool) {
-	if request.Instance.Spec.TektonPipelines != nil && request.Instance.Spec.TektonPipelines.Namespace != "" {
+	if request.Instance.Spec.TektonPipelines != nil && request.Instance.Spec.TektonPipelines.Namespace != "" && request.Instance.Spec.TektonPipelines.Namespace != kubevirtNamespace {
 		return request.Instance.Spec.TektonPipelines.Namespace, true
 	}
 	return request.Instance.Namespace, false

diff --git a/internal/operands/tekton-pipelines/reconcile_test.go b/internal/operands/tekton-pipelines/reconcile_test.go
@@ -175,7 +175,7 @@ var _ = Describe("environments", func() {
 			}
 		})
 
-		It("kubevirt.io/deploy-namespace annotation in configMaps should be replaced by user defined namespace", func() {
+		It("kubevirt.io/tekton-piplines-deploy-namespace annotation in configMaps should be replaced by user defined namespace", func() {
 			_, err := operand.Reconcile(request)
 			Expect(err).ToNot(HaveOccurred())
 
@@ -188,7 +188,7 @@ var _ = Describe("environments", func() {
 			}
 		})
 
-		It("kubevirt.io/deploy-namespace annotation in roleBindings should be replaced by user defined namespace", func() {
+		It("kubevirt.io/tekton-piplines-deploy-namespace annotation in roleBindings should be replaced by user defined namespace", func() {
 			_, err := operand.Reconcile(request)
 			Expect(err).ToNot(HaveOccurred())
 
@@ -202,13 +202,56 @@ var _ = Describe("environments", func() {
 		})
 	})
 
+	Context("With kubevirt namespace in ssp CR for pipelines", func() {
+		BeforeEach(func() {
+			request.Instance.Spec.FeatureGates.DeployTektonTaskResources = true
+			request.Instance.Spec.TektonPipelines = &ssp.TektonPipelines{
+				Namespace: kubevirtNamespace,
+			}
+		})
+
+		It("kubevirt.io/tekton-piplines-deploy-namespace annotation in configMaps should not be replaced", func() {
+			_, err := operand.Reconcile(request)
+			Expect(err).ToNot(HaveOccurred())
+
+			for _, configMap := range bundle.ConfigMaps {
+				expectedNamespace := namespace
+				if annotationNamespace, ok := configMap.Annotations[deployNamespaceAnnotation]; ok {
+					expectedNamespace = annotationNamespace
+				}
+
+				key := client.ObjectKeyFromObject(&configMap)
+				cm := &v1.ConfigMap{}
+				Expect(request.Client.Get(request.Context, key, cm)).ToNot(HaveOccurred())
+				Expect(cm.Namespace).To(Equal(expectedNamespace), cm.Name+" configMap namespace should equal")
+			}
+		})
+
+		It("kubevirt.io/tekton-piplines-deploy-namespace annotation in roleBindings should not be replaced", func() {
+			_, err := operand.Reconcile(request)
+			Expect(err).ToNot(HaveOccurred())
+
+			for _, roleBinding := range bundle.RoleBindings {
+				expectedNamespace := namespace
+				if annotationNamespace, ok := roleBinding.Annotations[deployNamespaceAnnotation]; ok {
+					expectedNamespace = annotationNamespace
+				}
+
+				key := client.ObjectKeyFromObject(&roleBinding)
+				rb := &rbac.RoleBinding{}
+				Expect(request.Client.Get(request.Context, key, rb)).ToNot(HaveOccurred())
+				Expect(rb.Namespace).To(Equal(expectedNamespace), rb.Name+" roleBinding namespace should equal")
+			}
+		})
+	})
+
 	Context("Without user defined namespace in ssp CR for pipelines", func() {
 		BeforeEach(func() {
 			request.Instance.Spec.FeatureGates.DeployTektonTaskResources = true
 			request.Instance.Spec.TektonPipelines = nil
 		})
 
-		It("kubevirt.io/deploy-namespace annotation in configMaps should replace default namespace", func() {
+		It("kubevirt.io/tekton-piplines-deploy-namespace annotation in configMaps should replace default namespace", func() {
 			_, err := operand.Reconcile(request)
 			Expect(err).ToNot(HaveOccurred())
 
@@ -227,7 +270,7 @@ var _ = Describe("environments", func() {
 			}
 		})
 
-		It("kubevirt.io/deploy-namespace annotation in roleBindings should replace default namespace", func() {
+		It("kubevirt.io/tekton-piplines-deploy-namespace annotation in roleBindings should replace default namespace", func() {
 			_, err := operand.Reconcile(request)
 			Expect(err).ToNot(HaveOccurred())