Merge pull request #634 from codingben/CNV-31248

refactor(CNV-31248): remove namespace annotation

automation/e2e-upgrade-functests/run.sh

@@ -70,6 +70,5 @@ export SKIP_CLEANUP_AFTER_TESTS="true"
 export TEST_EXISTING_CR_NAME="${SSP_NAME}"
 export TEST_EXISTING_CR_NAMESPACE="${SSP_NAMESPACE}"
 export IS_UPGRADE_LANE="true"
-export VM_CONSOLE_PROXY_NAMESPACE="kubevirt"
 
 make deploy functest

config/samples/ssp_v1beta2_ssp.yaml

@@ -1,8 +1,6 @@
 apiVersion: ssp.kubevirt.io/v1beta2
 kind: SSP
 metadata:
-  annotations:
-    ssp.kubevirt.io/vm-console-proxy-namespace: "kubevirt"
   name: ssp-sample
   namespace: kubevirt
 spec:

data/olm-catalog/ssp-operator.clusterserviceversion.yaml

@@ -9,9 +9,6 @@ metadata:
           "apiVersion": "ssp.kubevirt.io/v1beta2",
           "kind": "SSP",
           "metadata": {
-            "annotations": {
-              "ssp.kubevirt.io/vm-console-proxy-namespace": "kubevirt"
-            },
             "name": "ssp-sample",
             "namespace": "kubevirt"
           },
@@ -259,19 +256,19 @@ spec:
         - apiGroups:
           - ""
           resources:
-          - pods
+          - persistentvolumes
           verbs:
           - get
           - list
           - watch
         - apiGroups:
-            - ""
+          - ""
           resources:
-            - persistentvolumes
+          - pods
           verbs:
-            - get
-            - list
-            - watch
+          - get
+          - list
+          - watch
         - apiGroups:
           - ""
           resources:

docs/configuration.md

@@ -21,8 +21,6 @@ To activate the operator, create the SSP Custom Resource (CR):
 apiVersion: ssp.kubevirt.io/v1beta2
 kind: SSP
 metadata:
-  annotations:
-    ssp.kubevirt.io/vm-console-proxy-namespace: "kubevirt"
   name: ssp-sample
   namespace: kubevirt
 spec:
@@ -49,8 +47,6 @@ This annotation is used by VM console proxy operand.
 apiVersion: ssp.kubevirt.io/v1beta1
 kind: SSP
 metadata:
-  annotations:
-    ssp.kubevirt.io/vm-console-proxy-namespace: "kubevirt" # If not set, then default namespace is "kubevirt"
   name: ssp-sample
   namespace: kubevirt
 spec: {}

internal/operands/vm-console-proxy/reconcile.go

@@ -18,8 +18,6 @@ import (
 )
 
 const (
-	VmConsoleProxyNamespaceAnnotation = "ssp.kubevirt.io/vm-console-proxy-namespace"
-
 	operandName      = "vm-console-proxy"
 	operandComponent = "vm-console-proxy"
 
@@ -55,11 +53,16 @@ func WatchClusterTypes() []operands.WatchType {
 	return []operands.WatchType{
 		{Object: &rbac.ClusterRole{}},
 		{Object: &rbac.ClusterRoleBinding{}},
+		{Object: &apiregv1.APIService{}},
+	}
+}
+
+func WatchTypes() []operands.WatchType {
+	return []operands.WatchType{
 		{Object: &core.ServiceAccount{}},
 		{Object: &core.Service{}},
 		{Object: &apps.Deployment{}, WatchFullObject: true},
 		{Object: &core.ConfigMap{}},
-		{Object: &apiregv1.APIService{}},
 		{Object: &routev1.Route{}},
 	}
 }
@@ -95,7 +98,7 @@ func (v *vmConsoleProxy) Name() string {
 }
 
 func (v *vmConsoleProxy) WatchTypes() []operands.WatchType {
-	return nil
+	return WatchTypes()
 }
 
 func (v *vmConsoleProxy) WatchClusterTypes() []operands.WatchType {
@@ -229,9 +232,9 @@ func (v *vmConsoleProxy) deleteRoute(request *common.Request) ([]common.CleanupR
 
 func reconcileServiceAccount(serviceAccount core.ServiceAccount) common.ReconcileFunc {
 	return func(request *common.Request) (common.ReconcileResult, error) {
-		serviceAccount.Namespace = getVmConsoleProxyNamespace(request)
+		serviceAccount.Namespace = request.Instance.Namespace
 		return common.CreateOrUpdate(request).
-			ClusterResource(&serviceAccount).
+			NamespacedResource(&serviceAccount).
 			WithAppLabels(operandName, operandComponent).
 			Reconcile()
 	}
@@ -248,6 +251,7 @@ func reconcileClusterRole(clusterRole rbac.ClusterRole) common.ReconcileFunc {
 
 func reconcileClusterRoleBinding(clusterRoleBinding rbac.ClusterRoleBinding) common.ReconcileFunc {
 	return func(request *common.Request) (common.ReconcileResult, error) {
+		clusterRoleBinding.Subjects[0].Namespace = request.Instance.Namespace
 		return common.CreateOrUpdate(request).
 			ClusterResource(&clusterRoleBinding).
 			WithAppLabels(operandName, operandComponent).
@@ -257,6 +261,7 @@ func reconcileClusterRoleBinding(clusterRoleBinding rbac.ClusterRoleBinding) com
 
 func reconcileRoleBinding(roleBinding *rbac.RoleBinding) common.ReconcileFunc {
 	return func(request *common.Request) (common.ReconcileResult, error) {
+		roleBinding.Subjects[0].Namespace = request.Instance.Namespace
 		return common.CreateOrUpdate(request).
 			ClusterResource(roleBinding).
 			WithAppLabels(operandName, operandComponent).
@@ -266,38 +271,38 @@ func reconcileRoleBinding(roleBinding *rbac.RoleBinding) common.ReconcileFunc {
 
 func reconcileConfigMap(configMap core.ConfigMap) common.ReconcileFunc {
 	return func(request *common.Request) (common.ReconcileResult, error) {
-		configMap.Namespace = getVmConsoleProxyNamespace(request)
+		configMap.Namespace = request.Instance.Namespace
 		return common.CreateOrUpdate(request).
-			ClusterResource(&configMap).
+			NamespacedResource(&configMap).
 			WithAppLabels(operandName, operandComponent).
 			Reconcile()
 	}
 }
 
 func reconcileService(service core.Service) common.ReconcileFunc {
 	return func(request *common.Request) (common.ReconcileResult, error) {
-		service.Namespace = getVmConsoleProxyNamespace(request)
+		service.Namespace = request.Instance.Namespace
 		return common.CreateOrUpdate(request).
-			ClusterResource(&service).
+			NamespacedResource(&service).
 			WithAppLabels(operandName, operandComponent).
 			Reconcile()
 	}
 }
 
 func reconcileDeployment(deployment apps.Deployment) common.ReconcileFunc {
 	return func(request *common.Request) (common.ReconcileResult, error) {
-		deployment.Namespace = getVmConsoleProxyNamespace(request)
+		deployment.Namespace = request.Instance.Namespace
 		deployment.Spec.Template.Spec.Containers[0].Image = getVmConsoleProxyImage()
 		return common.CreateOrUpdate(request).
-			ClusterResource(&deployment).
+			NamespacedResource(&deployment).
 			WithAppLabels(operandName, operandComponent).
 			Reconcile()
 	}
 }
 
 func reconcileApiService(apiService *apiregv1.APIService) common.ReconcileFunc {
 	return func(request *common.Request) (common.ReconcileResult, error) {
-		apiService.Spec.Service.Namespace = getVmConsoleProxyNamespace(request)
+		apiService.Spec.Service.Namespace = request.Instance.Namespace
 		return common.CreateOrUpdate(request).
 			ClusterResource(apiService).
 			WithAppLabels(operandName, operandComponent).
@@ -314,17 +319,6 @@ func reconcileApiService(apiService *apiregv1.APIService) common.ReconcileFunc {
 	}
 }
 
-func getVmConsoleProxyNamespace(request *common.Request) string {
-	const defaultNamespace = "kubevirt"
-	if request.Instance.GetAnnotations() == nil {
-		return defaultNamespace
-	}
-	if namespace, isFound := request.Instance.GetAnnotations()[VmConsoleProxyNamespaceAnnotation]; isFound {
-		return namespace
-	}
-	return defaultNamespace
-}
-
 func getVmConsoleProxyImage() string {
 	return common.EnvOrDefault(common.VmConsoleProxyImageKey, defaultVmConsoleProxyImage)
 }

internal/operands/vm-console-proxy/reconcile_test.go

@@ -241,7 +241,7 @@ var _ = Describe("VM Console Proxy Operand", func() {
 		}
 	})
 
-	It("should delete resources when enabled annotation is removed", func() {
+	It("should delete resources when feature gate was disabled", func() {
 		_, err := operand.Reconcile(&request)
 		Expect(err).ToNot(HaveOccurred())
 
@@ -268,146 +268,6 @@ var _ = Describe("VM Console Proxy Operand", func() {
 		ExpectResourceNotExists(bundle.Deployment, request)
 		ExpectResourceNotExists(bundle.ApiService, request)
 	})
-
-	Context("with namespace annotation", func() {
-		const otherNamespace = "some-namespace"
-
-		var (
-			serviceAccount *core.ServiceAccount
-			configMap      *core.ConfigMap
-			service        *core.Service
-			deployment     *apps.Deployment
-		)
-
-		BeforeEach(func() {
-			serviceAccount = bundle.ServiceAccount.DeepCopy()
-			serviceAccount.Namespace = otherNamespace
-
-			configMap = bundle.ConfigMap.DeepCopy()
-			configMap.Namespace = otherNamespace
-
-			service = bundle.Service.DeepCopy()
-			service.Namespace = otherNamespace
-
-			deployment = bundle.Deployment.DeepCopy()
-			deployment.Namespace = otherNamespace
-
-			request.Instance.GetAnnotations()[VmConsoleProxyNamespaceAnnotation] = otherNamespace
-		})
-
-		It("should deploy resources in namespace provided by annotation", func() {
-			_, err := operand.Reconcile(&request)
-			Expect(err).ToNot(HaveOccurred())
-
-			ExpectResourceExists(serviceAccount, request)
-			ExpectResourceExists(bundle.ClusterRole, request)
-			ExpectResourceExists(bundle.ClusterRoleBinding, request)
-			ExpectResourceExists(bundle.RoleBinding, request)
-			ExpectResourceExists(configMap, request)
-			ExpectResourceExists(service, request)
-			ExpectResourceExists(deployment, request)
-			ExpectResourceExists(bundle.ApiService, request)
-		})
-
-		It("should cleanup resources from namespace provided by annotation", func() {
-			_, err := operand.Reconcile(&request)
-			Expect(err).ToNot(HaveOccurred())
-
-			ExpectResourceExists(serviceAccount, request)
-			ExpectResourceExists(bundle.ClusterRole, request)
-			ExpectResourceExists(bundle.ClusterRoleBinding, request)
-			ExpectResourceExists(bundle.RoleBinding, request)
-			ExpectResourceExists(configMap, request)
-			ExpectResourceExists(service, request)
-			ExpectResourceExists(deployment, request)
-			ExpectResourceExists(bundle.ApiService, request)
-
-			_, err = operand.Cleanup(&request)
-			Expect(err).ToNot(HaveOccurred())
-
-			ExpectResourceNotExists(serviceAccount, request)
-			ExpectResourceNotExists(bundle.ClusterRole, request)
-			ExpectResourceNotExists(bundle.ClusterRoleBinding, request)
-			ExpectResourceNotExists(bundle.RoleBinding, request)
-			ExpectResourceNotExists(configMap, request)
-			ExpectResourceNotExists(service, request)
-			ExpectResourceNotExists(deployment, request)
-			ExpectResourceNotExists(bundle.ApiService, request)
-		})
-
-		It("should deploy APIService with ServiceReference pointing to the right namespace", func() {
-			_, err := operand.Reconcile(&request)
-			Expect(err).ToNot(HaveOccurred())
-
-			apiService := &apiregv1.APIService{}
-			key := client.ObjectKeyFromObject(bundle.ApiService)
-			Expect(request.Client.Get(request.Context, key, apiService)).To(Succeed())
-
-			Expect(apiService.Spec.Service.Namespace).To(Equal(otherNamespace))
-		})
-
-		It("should remove resources from the namespace", func() {
-			_, err := operand.Reconcile(&request)
-			Expect(err).ToNot(HaveOccurred())
-
-			ExpectResourceExists(serviceAccount, request)
-			ExpectResourceExists(bundle.ClusterRole, request)
-			ExpectResourceExists(bundle.ClusterRoleBinding, request)
-			ExpectResourceExists(bundle.RoleBinding, request)
-			ExpectResourceExists(configMap, request)
-			ExpectResourceExists(service, request)
-			ExpectResourceExists(deployment, request)
-			ExpectResourceExists(bundle.ApiService, request)
-
-			request.Instance.Spec.FeatureGates.DeployVmConsoleProxy = false
-
-			delete(request.Instance.Annotations, VmConsoleProxyNamespaceAnnotation)
-
-			_, err = operand.Reconcile(&request)
-			Expect(err).ToNot(HaveOccurred())
-
-			ExpectResourceNotExists(serviceAccount, request)
-			ExpectResourceNotExists(bundle.ClusterRole, request)
-			ExpectResourceNotExists(bundle.ClusterRoleBinding, request)
-			ExpectResourceNotExists(bundle.RoleBinding, request)
-			ExpectResourceNotExists(configMap, request)
-			ExpectResourceNotExists(service, request)
-			ExpectResourceNotExists(deployment, request)
-			ExpectResourceNotExists(bundle.ApiService, request)
-		})
-
-		DescribeTable("should delete Route leftover from previous version", func(op func() error) {
-			route := &routev1.Route{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      routeName,
-					Namespace: otherNamespace,
-					Annotations: map[string]string{
-						libhandler.TypeAnnotation:           "SSP.ssp.kubevirt.io",
-						libhandler.NamespacedNameAnnotation: namespace + "/" + name,
-					},
-					Labels: map[string]string{
-						common.AppKubernetesNameLabel:      operandName,
-						common.AppKubernetesComponentLabel: operandComponent,
-						common.AppKubernetesManagedByLabel: common.AppKubernetesManagedByValue,
-					},
-				},
-				Spec: routev1.RouteSpec{},
-			}
-
-			Expect(request.Client.Create(request.Context, route)).To(Succeed())
-			Expect(op()).To(Succeed())
-			ExpectResourceNotExists(route, request)
-		},
-			Entry("on reconcile", func() error {
-				_, err := operand.Reconcile(&request)
-				return err
-			}),
-			Entry("on cleanup", func() error {
-				_, err := operand.Cleanup(&request)
-				return err
-			}),
-		)
-	})
 })
 
 func TestVmConsoleProxyBundle(t *testing.T) {
@@ -436,9 +296,6 @@ func getMockedRequest() common.Request {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      name,
 				Namespace: namespace,
-				Annotations: map[string]string{
-					VmConsoleProxyNamespaceAnnotation: namespace,
-				},
 			},
 			Spec: ssp.SSPSpec{
 				FeatureGates: &ssp.FeatureGates{