Move rbacs for config reader to a separate handler

cmd/hyperconverged-cluster-operator/main.go

@@ -165,6 +165,14 @@ func getNewManagerCache(operatorNamespace string) cache.NewCacheFunc {
 					Label: labelSelector,
 					Field: namespaceSelector,
 				},
+				&rbacv1.Role{}: {
+					Label: labelSelector,
+					Field: namespaceSelector,
+				},
+				&rbacv1.RoleBinding{}: {
+					Label: labelSelector,
+					Field: namespaceSelector,
+				},
 			},
 		},
 	)

pkg/controller/hyperconverged/hyperconverged_controller.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/metrics"
+	rbacv1 "k8s.io/api/rbac/v1"
 	"os"
 	"reflect"
 	"strings"
@@ -136,6 +137,8 @@ func add(mgr manager.Manager, r reconcile.Reconciler, ci hcoutil.ClusterInfo) er
 		&schedulingv1.PriorityClass{},
 		&vmimportv1beta1.VMImportConfig{},
 		&corev1.ConfigMap{},
+		&rbacv1.Role{},
+		&rbacv1.RoleBinding{},
 	}
 	if ci.IsOpenshift() {
 		secondaryResources = append(secondaryResources, []client.Object{

pkg/controller/hyperconverged/testUtils_test.go

@@ -144,11 +144,11 @@ func getBasicDeployment() *BasicExpected {
 	expectedKVStorageConfig := operands.NewKubeVirtStorageConfigForCR(hco, namespace)
 	expectedKVStorageConfig.ObjectMeta.SelfLink = fmt.Sprintf("/apis/v1/namespaces/%s/configmaps/%s", expectedKVStorageConfig.Namespace, expectedKVStorageConfig.Name)
 	res.kvStorageConfig = expectedKVStorageConfig
-	expectedKVStorageRole := operands.NewKubeVirtStorageRoleForCR(hco, namespace, commonTestUtils.GetScheme())
+	expectedKVStorageRole := operands.NewConfigReaderRoleForCR(hco, namespace)
 	expectedKVStorageRole.ObjectMeta.SelfLink = fmt.Sprintf("/apis/v1/namespaces/%s/roles/%s", expectedKVStorageConfig.Namespace, expectedKVStorageConfig.Name)
 	res.kvStorageRole = expectedKVStorageRole
 
-	expectedKVStorageRoleBinding := operands.NewKubeVirtStorageRoleBindingForCR(hco, namespace, commonTestUtils.GetScheme())
+	expectedKVStorageRoleBinding := operands.NewConfigReaderRoleBindingForCR(hco, namespace)
 	expectedKVStorageRoleBinding.ObjectMeta.SelfLink = fmt.Sprintf("/apis/v1/namespaces/%s/rolebindings/%s", expectedKVStorageConfig.Namespace, expectedKVStorageConfig.Name)
 	res.kvStorageRoleBinding = expectedKVStorageRoleBinding
 

pkg/controller/operands/cdi.go

@@ -4,21 +4,16 @@ import (
 	"errors"
 	"reflect"
 
-	objectreferencesv1 "github.com/openshift/custom-resource-status/objectreferences/v1"
+	hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis/hco/v1beta1"
+	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/controller/common"
+	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
+	hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/tools/reference"
 	cdiv1beta1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-
-	hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis/hco/v1beta1"
-	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/controller/common"
-	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
-	hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
 )
 
 const (
@@ -104,20 +99,6 @@ func getDefaultFeatureGates() []string {
 	return []string{HonorWaitForFirstConsumerGate}
 }
 
-func (h *cdiHooks) postFound(req *common.HcoRequest, exists runtime.Object) error {
-	err := h.ensureKubeVirtStorageRole(req)
-	if err != nil {
-		return err
-	}
-
-	err = h.ensureKubeVirtStorageRoleBinding(req)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func NewCDI(hc *hcov1beta1.HyperConverged, opts ...string) (*cdiv1beta1.CDI, error) {
 	uninstallStrategy := cdiv1beta1.CDIUninstallStrategyBlockUninstallIfWorkloadsExist
 
@@ -186,120 +167,6 @@ func NewCDIWithNameOnly(hc *hcov1beta1.HyperConverged, opts ...string) *cdiv1bet
 	}
 }
 
-func (h *cdiHooks) ensureKubeVirtStorageRole(req *common.HcoRequest) error {
-	kubevirtStorageRole := NewKubeVirtStorageRoleForCR(req.Instance, req.Namespace, h.Scheme)
-
-	found := &rbacv1.Role{}
-	err := h.Client.Get(req.Ctx, client.ObjectKeyFromObject(kubevirtStorageRole), found)
-	if apierrors.IsNotFound(err) {
-		req.Logger.Info("Creating kubevirt storage role")
-		return h.Client.Create(req.Ctx, kubevirtStorageRole)
-	}
-
-	if err != nil {
-		return err
-	}
-
-	if !reflect.DeepEqual(found.Labels, kubevirtStorageRole.Labels) {
-		req.Logger.Info("Updating KubeVirt storage role for labels")
-		util.DeepCopyLabels(&kubevirtStorageRole.ObjectMeta, &found.ObjectMeta)
-		return h.Client.Update(req.Ctx, found)
-	}
-
-	req.Logger.Info("KubeVirt storage role already exists", "KubeVirtConfig.Namespace", found.Namespace, "KubeVirtConfig.Name", found.Name)
-	// Add it to the list of RelatedObjects if found
-	objectRef, err := reference.GetReference(h.Scheme, found)
-	if err != nil {
-		return err
-	}
-	if err = objectreferencesv1.SetObjectReference(&req.Instance.Status.RelatedObjects, *objectRef); err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func (h *cdiHooks) ensureKubeVirtStorageRoleBinding(req *common.HcoRequest) error {
-	kubevirtStorageRoleBinding := NewKubeVirtStorageRoleBindingForCR(req.Instance, req.Namespace, h.Scheme)
-
-	found := &rbacv1.RoleBinding{}
-	err := h.Client.Get(req.Ctx, client.ObjectKeyFromObject(kubevirtStorageRoleBinding), found)
-	if err != nil && apierrors.IsNotFound(err) {
-		req.Logger.Info("Creating kubevirt storage rolebinding")
-		return h.Client.Create(req.Ctx, kubevirtStorageRoleBinding)
-	}
-
-	if err != nil {
-		return err
-	}
-
-	if !reflect.DeepEqual(found.Labels, kubevirtStorageRoleBinding.Labels) {
-		req.Logger.Info("Updating KubeVirt storage rolebinding for labels")
-		util.DeepCopyLabels(&kubevirtStorageRoleBinding.ObjectMeta, &found.ObjectMeta)
-		return h.Client.Update(req.Ctx, found)
-	}
-
-	req.Logger.Info("KubeVirt storage rolebinding already exists", "KubeVirtConfig.Namespace", found.Namespace, "KubeVirtConfig.Name", found.Name)
-	// Add it to the list of RelatedObjects if found
-	objectRef, err := reference.GetReference(h.Scheme, found)
-	if err != nil {
-		return err
-	}
-	err = objectreferencesv1.SetObjectReference(&req.Instance.Status.RelatedObjects, *objectRef)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func NewKubeVirtStorageRoleForCR(cr *hcov1beta1.HyperConverged, namespace string, scheme *runtime.Scheme) *rbacv1.Role {
-	role := &rbacv1.Role{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      cdiRoleName,
-			Labels:    getLabels(cr, hcoutil.AppComponentStorage),
-			Namespace: namespace,
-		},
-		Rules: []rbacv1.PolicyRule{
-			{
-				APIGroups:     []string{""},
-				Resources:     []string{"configmaps"},
-				ResourceNames: []string{"kubevirt-storage-class-defaults"},
-				Verbs:         []string{"get", "watch", "list"},
-			},
-		},
-	}
-
-	_ = controllerutil.SetControllerReference(cr, role, scheme)
-	return role
-}
-
-func NewKubeVirtStorageRoleBindingForCR(cr *hcov1beta1.HyperConverged, namespace string, scheme *runtime.Scheme) *rbacv1.RoleBinding {
-	roleBinding := &rbacv1.RoleBinding{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      cdiRoleName,
-			Labels:    getLabels(cr, hcoutil.AppComponentStorage),
-			Namespace: namespace,
-		},
-		RoleRef: rbacv1.RoleRef{
-			APIGroup: "rbac.authorization.k8s.io",
-			Kind:     "Role",
-			Name:     cdiRoleName,
-		},
-		Subjects: []rbacv1.Subject{
-			{
-				APIGroup: "rbac.authorization.k8s.io",
-				Kind:     "Group",
-				Name:     "system:authenticated",
-			},
-		},
-	}
-
-	_ = controllerutil.SetControllerReference(cr, roleBinding, scheme)
-
-	return roleBinding
-}
-
 // ************** CDI Storage Config Handler **************
 type storageConfigHandler genericOperand
 
@@ -319,8 +186,7 @@ type storageConfigHooks struct{}
 func (h storageConfigHooks) getFullCr(hc *hcov1beta1.HyperConverged) (client.Object, error) {
 	return NewKubeVirtStorageConfigForCR(hc, hc.Namespace), nil
 }
-func (h storageConfigHooks) getEmptyCr() client.Object                              { return &corev1.ConfigMap{} }
-func (h storageConfigHooks) postFound(_ *common.HcoRequest, _ runtime.Object) error { return nil }
+func (h storageConfigHooks) getEmptyCr() client.Object { return &corev1.ConfigMap{} }
 func (h storageConfigHooks) getObjectMeta(cr runtime.Object) *metav1.ObjectMeta {
 	return &cr.(*corev1.ConfigMap).ObjectMeta
 }
@@ -382,3 +248,144 @@ func NewKubeVirtStorageConfigForCR(cr *hcov1beta1.HyperConverged, namespace stri
 		},
 	}
 }
+
+// ************** Config Reader Role Handler **************
+type configReaderRoleHandler genericOperand
+
+func newConfigReaderRoleHandler(Client client.Client, Scheme *runtime.Scheme) *configReaderRoleHandler {
+	return &configReaderRoleHandler{
+		Client:                 Client,
+		Scheme:                 Scheme,
+		crType:                 "Role",
+		removeExistingOwner:    false,
+		setControllerReference: true,
+		hooks:                  &configReaderRoleHooks{},
+	}
+}
+
+type configReaderRoleHooks struct{}
+
+func (h configReaderRoleHooks) getFullCr(hc *hcov1beta1.HyperConverged) (client.Object, error) {
+	return NewConfigReaderRoleForCR(hc, hc.Namespace), nil
+}
+func (h configReaderRoleHooks) getEmptyCr() client.Object { return &rbacv1.Role{} }
+func (h configReaderRoleHooks) getObjectMeta(cr runtime.Object) *metav1.ObjectMeta {
+	return &cr.(*rbacv1.Role).ObjectMeta
+}
+func (h *configReaderRoleHooks) updateCr(req *common.HcoRequest, Client client.Client, exists runtime.Object, required runtime.Object) (bool, bool, error) {
+	configReaderRole, ok1 := required.(*rbacv1.Role)
+	found, ok2 := exists.(*rbacv1.Role)
+	if !ok1 || !ok2 {
+		return false, false, errors.New("can't convert to a Role")
+	}
+
+	if !reflect.DeepEqual(found.Labels, configReaderRole.Labels) ||
+		!reflect.DeepEqual(found.Rules, configReaderRole.Rules) {
+
+		req.Logger.Info("Updating existing Config Reader Role to its default values")
+
+		found.Rules = make([]rbacv1.PolicyRule, len(configReaderRole.Rules))
+		for i := range configReaderRole.Rules {
+			configReaderRole.Rules[i].DeepCopyInto(&found.Rules[i])
+		}
+		util.DeepCopyLabels(&configReaderRole.ObjectMeta, &found.ObjectMeta)
+
+		err := Client.Update(req.Ctx, found)
+		if err != nil {
+			return false, false, err
+		}
+		return true, !req.HCOTriggered, nil
+	}
+
+	return false, false, nil
+}
+
+func NewConfigReaderRoleForCR(cr *hcov1beta1.HyperConverged, namespace string) *rbacv1.Role {
+	return &rbacv1.Role{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      cdiRoleName,
+			Labels:    getLabels(cr, hcoutil.AppComponentStorage),
+			Namespace: namespace,
+		},
+		Rules: []rbacv1.PolicyRule{
+			{
+				APIGroups:     []string{""},
+				Resources:     []string{"configmaps"},
+				ResourceNames: []string{"kubevirt-storage-class-defaults"},
+				Verbs:         []string{"get", "watch", "list"},
+			},
+		},
+	}
+}
+
+// ************** Config Reader Role Binding Handler **************
+type configReaderRoleBindingHandler genericOperand
+
+func newConfigReaderRoleBindingHandler(Client client.Client, Scheme *runtime.Scheme) *configReaderRoleBindingHandler {
+	return &configReaderRoleBindingHandler{
+		Client:                 Client,
+		Scheme:                 Scheme,
+		crType:                 "RoleBinding",
+		removeExistingOwner:    false,
+		setControllerReference: true,
+		hooks:                  &configReaderRoleBindingHooks{},
+	}
+}
+
+type configReaderRoleBindingHooks struct{}
+
+func (h configReaderRoleBindingHooks) getFullCr(hc *hcov1beta1.HyperConverged) (client.Object, error) {
+	return NewConfigReaderRoleBindingForCR(hc, hc.Namespace), nil
+}
+func (h configReaderRoleBindingHooks) getEmptyCr() client.Object { return &rbacv1.RoleBinding{} }
+func (h configReaderRoleBindingHooks) getObjectMeta(cr runtime.Object) *metav1.ObjectMeta {
+	return &cr.(*rbacv1.RoleBinding).ObjectMeta
+}
+func (h *configReaderRoleBindingHooks) updateCr(req *common.HcoRequest, Client client.Client, exists runtime.Object, required runtime.Object) (bool, bool, error) {
+	configReaderRoleBinding, ok1 := required.(*rbacv1.RoleBinding)
+	found, ok2 := exists.(*rbacv1.RoleBinding)
+	if !ok1 || !ok2 {
+		return false, false, errors.New("can't convert to a RoleBinding")
+	}
+
+	if !reflect.DeepEqual(found.Labels, configReaderRoleBinding.Labels) ||
+		!reflect.DeepEqual(found.Subjects, configReaderRoleBinding.Subjects) ||
+		!reflect.DeepEqual(found.RoleRef, configReaderRoleBinding.RoleRef) {
+		req.Logger.Info("Updating existing Config Reader RoleBinding to its default values")
+
+		found.Subjects = make([]rbacv1.Subject, len(configReaderRoleBinding.Subjects))
+		copy(found.Subjects, configReaderRoleBinding.Subjects)
+		found.RoleRef = configReaderRoleBinding.RoleRef
+		util.DeepCopyLabels(&configReaderRoleBinding.ObjectMeta, &found.ObjectMeta)
+
+		err := Client.Update(req.Ctx, found)
+		if err != nil {
+			return false, false, err
+		}
+		return true, !req.HCOTriggered, nil
+	}
+
+	return false, false, nil
+}
+
+func NewConfigReaderRoleBindingForCR(cr *hcov1beta1.HyperConverged, namespace string) *rbacv1.RoleBinding {
+	return &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      cdiRoleName,
+			Labels:    getLabels(cr, hcoutil.AppComponentStorage),
+			Namespace: namespace,
+		},
+		RoleRef: rbacv1.RoleRef{
+			APIGroup: "rbac.authorization.k8s.io",
+			Kind:     "Role",
+			Name:     cdiRoleName,
+		},
+		Subjects: []rbacv1.Subject{
+			{
+				APIGroup: "rbac.authorization.k8s.io",
+				Kind:     "Group",
+				Name:     "system:authenticated",
+			},
+		},
+	}
+}