code-refactoring: add/modify helper functions

controllers/argocd/applicationset/deployment.go

@@ -65,31 +65,27 @@ func (asr *ApplicationSetReconciler) reconcileDeployment() error {
 	}
 	deploymentChanged := false
 
-	fieldsToCompare := []struct {
-		existing, desired interface{}
-		extraAction       func()
-	}{
-		{&existingDeployment.Spec.Template.Spec.Containers[0].Image, &desiredDeployment.Spec.Template.Spec.Containers[0].Image,
-			func() {
+	fieldsToCompare := []argocdcommon.FieldToCompare{
+		{Existing: &existingDeployment.Spec.Template.Spec.Containers[0].Image, Desired: &desiredDeployment.Spec.Template.Spec.Containers[0].Image,
+			ExtraAction: func() {
 				existingDeployment.Spec.Template.ObjectMeta.Labels[common.ImageUpgradedKey] = time.Now().UTC().Format(common.TimeFormatMST)
 			},
 		},
-		{&existingDeployment.Spec.Template.Spec.Containers[0].Command, &desiredDeployment.Spec.Template.Spec.Containers[0].Command, nil},
-		{&existingDeployment.Spec.Template.Spec.Containers[0].Env, &desiredDeployment.Spec.Template.Spec.Containers[0].Env, nil},
-		{&existingDeployment.Spec.Template.Spec.Containers[0].Resources, &desiredDeployment.Spec.Template.Spec.Containers[0].Resources, nil},
-		{&existingDeployment.Spec.Template.Spec.Volumes, &desiredDeployment.Spec.Template.Spec.Volumes, nil},
-		{&existingDeployment.Spec.Template.Spec.NodeSelector, &desiredDeployment.Spec.Template.Spec.NodeSelector, nil},
-		{&existingDeployment.Spec.Template.Spec.Tolerations, &desiredDeployment.Spec.Template.Spec.Tolerations, nil},
-		{&existingDeployment.Spec.Template.Spec.ServiceAccountName, &desiredDeployment.Spec.Template.Spec.ServiceAccountName, nil},
-		{&existingDeployment.Spec.Template.Labels, &desiredDeployment.Spec.Template.Labels, nil},
-		{&existingDeployment.Spec.Replicas, &desiredDeployment.Spec.Replicas, nil},
-		{&existingDeployment.Spec.Selector, &desiredDeployment.Spec.Selector, nil},
-		{&existingDeployment.Labels, &desiredDeployment.Labels, nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Containers[0].Command, Desired: &desiredDeployment.Spec.Template.Spec.Containers[0].Command, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Containers[0].Env, Desired: &desiredDeployment.Spec.Template.Spec.Containers[0].Env, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Containers[0].Resources, Desired: &desiredDeployment.Spec.Template.Spec.Containers[0].Resources, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Volumes, Desired: &desiredDeployment.Spec.Template.Spec.Volumes, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.NodeSelector, Desired: &desiredDeployment.Spec.Template.Spec.NodeSelector, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Tolerations, Desired: &desiredDeployment.Spec.Template.Spec.Tolerations, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.ServiceAccountName, Desired: &desiredDeployment.Spec.Template.Spec.ServiceAccountName, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Labels, Desired: &desiredDeployment.Spec.Template.Labels, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Replicas, Desired: &desiredDeployment.Spec.Replicas, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Selector, Desired: &desiredDeployment.Spec.Selector, ExtraAction: nil},
+		{Existing: &existingDeployment.Labels, Desired: &desiredDeployment.Labels, ExtraAction: nil},
+		{Existing: &existingDeployment.Annotations, Desired: &desiredDeployment.Annotations, ExtraAction: nil},
 	}
 
-	for _, field := range fieldsToCompare {
-		argocdcommon.UpdateIfChanged(field.existing, field.desired, field.extraAction, &deploymentChanged)
-	}
+	argocdcommon.UpdateIfChanged(fieldsToCompare, &deploymentChanged)
 
 	if deploymentChanged {
 

controllers/argocd/applicationset/rolebinding.go

@@ -1,13 +1,15 @@
 package applicationset
 
 import (
+	"reflect"
+
 	"github.com/argoproj-labs/argocd-operator/common"
 	"github.com/argoproj-labs/argocd-operator/controllers/argocd/argocdcommon"
 	"github.com/argoproj-labs/argocd-operator/pkg/cluster"
 	"github.com/argoproj-labs/argocd-operator/pkg/permissions"
+	"github.com/pkg/errors"
 
 	rbacv1 "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -61,7 +63,7 @@ func (asr *ApplicationSetReconciler) reconcileRoleBinding() error {
 
 	existingRoleBinding, err := permissions.GetRoleBinding(desiredRoleBinding.Name, desiredRoleBinding.Namespace, asr.Client)
 	if err != nil {
-		if !errors.IsNotFound(err) {
+		if !apierrors.IsNotFound(err) {
 			asr.Logger.Error(err, "reconcileRoleBinding: failed to retrieve roleBinding", "name", desiredRoleBinding.Name, "namespace", desiredRoleBinding.Namespace)
 			return err
 		}
@@ -78,32 +80,32 @@ func (asr *ApplicationSetReconciler) reconcileRoleBinding() error {
 		return nil
 	}
 
-	roleBindingChanged := false
-	fieldsToCompare := []struct {
-		existing, desired interface{}
-	}{
-		{
-			&existingRoleBinding.RoleRef,
-			&desiredRoleBinding.RoleRef,
-		},
-		{
-			&existingRoleBinding.Subjects,
-			&desiredRoleBinding.Subjects,
-		},
+	// if roleRef differs, we must delete the rolebinding as kubernetes does not allow updation of roleRef
+	if !reflect.DeepEqual(existingRoleBinding.RoleRef, desiredRoleBinding.RoleRef) {
+		asr.Logger.Info("detected drift in roleRef for rolebinding", "name", existingRoleBinding.Name, "namespace", existingRoleBinding.Namespace)
+		if err := asr.deleteRoleBinding(resourceName, asr.Instance.Namespace); err != nil {
+			return errors.Wrapf(err, "reconcileRoleBinding: unable to delete obsolete rolebinding %s", existingRoleBinding.Name)
+		}
+		return nil
+	}
+
+	rbChanged := false
+
+	fieldsToCompare := []argocdcommon.FieldToCompare{
+		{Existing: &existingRoleBinding.Subjects, Desired: &desiredRoleBinding.Subjects, ExtraAction: nil},
 	}
 
-	for _, field := range fieldsToCompare {
-		argocdcommon.UpdateIfChanged(field.existing, field.desired, nil, &roleBindingChanged)
+	argocdcommon.UpdateIfChanged(fieldsToCompare, &rbChanged)
+
+	if !rbChanged {
+		return nil
 	}
 
-	if roleBindingChanged {
-		if err = permissions.UpdateRoleBinding(existingRoleBinding, asr.Client); err != nil {
-			asr.Logger.Error(err, "reconcileRoleBinding: failed to update roleBinding", "name", existingRoleBinding.Name, "namespace", existingRoleBinding.Namespace)
-			return err
-		}
+	if err = permissions.UpdateRoleBinding(existingRoleBinding, asr.Client); err != nil {
+		return errors.Wrapf(err, "reconcileRoleBinding: failed to update role %s", existingRoleBinding.Name)
 	}
 
-	asr.Logger.V(0).Info("reconcileRoleBinding: roleBinding updated", "name", existingRoleBinding.Name, "namespace", existingRoleBinding.Namespace)
+	asr.Logger.Info("rolebinding updated", "name", existingRoleBinding.Name, "namespace", existingRoleBinding.Namespace)
 
 	return nil
 }

controllers/argocd/applicationset/webhookroute.go

@@ -62,22 +62,17 @@ func (asr *ApplicationSetReconciler) reconcileWebhookRoute() error {
 
 	webhookRouteChanged := false
 
-	fieldsToCompare := []struct {
-		existing, desired interface{}
-		extraAction       func()
-	}{
-		{&existingRoute.Annotations, &desiredWebhookRoute.Annotations, nil},
-		{&existingRoute.Labels, &desiredWebhookRoute.Labels, nil},
-		{&existingRoute.Spec.WildcardPolicy, &desiredWebhookRoute.Spec.WildcardPolicy, nil},
-		{&existingRoute.Spec.Host, &desiredWebhookRoute.Spec.Host, nil},
-		{&existingRoute.Spec.Port, &desiredWebhookRoute.Spec.Port, nil},
-		{&existingRoute.Spec.TLS, &desiredWebhookRoute.Spec.TLS, nil},
-		{&existingRoute.Spec.To, &desiredWebhookRoute.Spec.To, nil},
+	fieldsToCompare := []argocdcommon.FieldToCompare{
+		{Existing: &existingRoute.Annotations, Desired: &desiredWebhookRoute.Annotations, ExtraAction: nil},
+		{Existing: &existingRoute.Labels, Desired: &desiredWebhookRoute.Labels, ExtraAction: nil},
+		{Existing: &existingRoute.Spec.WildcardPolicy, Desired: &desiredWebhookRoute.Spec.WildcardPolicy, ExtraAction: nil},
+		{Existing: &existingRoute.Spec.Host, Desired: &desiredWebhookRoute.Spec.Host, ExtraAction: nil},
+		{Existing: &existingRoute.Spec.Port, Desired: &desiredWebhookRoute.Spec.Port, ExtraAction: nil},
+		{Existing: &existingRoute.Spec.TLS, Desired: &desiredWebhookRoute.Spec.TLS, ExtraAction: nil},
+		{Existing: &existingRoute.Spec.To, Desired: &desiredWebhookRoute.Spec.To, ExtraAction: nil},
 	}
 
-	for _, field := range fieldsToCompare {
-		argocdcommon.UpdateIfChanged(field.existing, field.desired, field.extraAction, &webhookRouteChanged)
-	}
+	argocdcommon.UpdateIfChanged(fieldsToCompare, &webhookRouteChanged)
 
 	if webhookRouteChanged {
 		if err = openshift.UpdateRoute(existingRoute, asr.Client); err != nil {

controllers/argocd/argocdcommon/helper.go

@@ -7,14 +7,32 @@ import (
 	"github.com/argoproj-labs/argocd-operator/pkg/util"
 )
 
-func UpdateIfChanged(existingVal, desiredVal interface{}, extraAction func(), changed *bool) {
-	if util.IsPtr(existingVal) && util.IsPtr(desiredVal) {
-		if !reflect.DeepEqual(existingVal, desiredVal) {
-			reflect.ValueOf(existingVal).Elem().Set(reflect.ValueOf(desiredVal).Elem())
-			if extraAction != nil {
-				extraAction()
+type FieldToCompare struct {
+	Existing    interface{}
+	Desired     interface{}
+	ExtraAction func()
+}
+
+// UpdateIfChanged accepts a slice of fields to be compared, along with a bool ptr. It compares all the provided fields, updating any fields and setting the bool ptr to true if a drift is detected
+func UpdateIfChanged(ftc []FieldToCompare, changed *bool) {
+	for _, field := range ftc {
+		if util.IsPtr(field.Existing) && util.IsPtr(field.Desired) {
+			if !reflect.DeepEqual(field.Existing, field.Desired) {
+				reflect.ValueOf(field.Existing).Elem().Set(reflect.ValueOf(field.Desired).Elem())
+				if field.ExtraAction != nil {
+					field.ExtraAction()
+				}
+				*changed = true
 			}
-			*changed = true
+		}
+	}
+}
+
+// PartialMatch accepts a slice of fields to be compared, along with a bool ptr. It compares all the provided fields and sets the bool to false if a drift is detected
+func PartialMatch(ftc []FieldToCompare, match *bool) {
+	for _, field := range ftc {
+		if !reflect.DeepEqual(field.Existing, field.Desired) {
+			*match = false
 		}
 	}
 }

controllers/argocd/argocdcommon/tls.go

@@ -0,0 +1,44 @@
+package argocdcommon
+
+import (
+	"reflect"
+
+	"github.com/argoproj-labs/argocd-operator/pkg/util"
+	"github.com/argoproj-labs/argocd-operator/pkg/workloads"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// UseTLS, on being invoked by a component, looks for a specified TLS secret on the cluster. If this secret is found, and is owned (either directly or indirectly) by an Argo CD instance, UseTLS returns true. In all other cases it returns false
+func UseTLS(secretName, secretNs string, client client.Client, logger *util.Logger) bool {
+	tlsSecret, err := workloads.GetSecret(secretName, secretNs, client)
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			logger.Debug("TLS secret not found; skipping TLS enforcement")
+			return false
+		}
+		logger.Error(err, "UseTLS: failed to retrieve tls secret", "name", secretName, "namespace", secretNs)
+		return false
+	}
+
+	if tlsSecret.Type != corev1.SecretTypeTLS {
+		// We only process secrets of type kubernetes.io/tls
+		logger.Debug("secret is not of type kubernetes.io/tls ; skipping TLS enforcement", "name", tlsSecret.Name, "namespace", tlsSecret.Namespace)
+		return false
+	}
+
+	secretOwner, err := FindSecretOwnerInstance(types.NamespacedName{Name: tlsSecret.Name, Namespace: tlsSecret.Namespace}, client)
+	if err != nil {
+		logger.Error(err, "UseTLS: failed to find tls secret owner", "name", tlsSecret.Name, "namespace", tlsSecret.Namespace)
+		return false
+	}
+
+	if !reflect.DeepEqual(secretOwner, types.NamespacedName{}) {
+		return true
+	}
+
+	logger.Debug("no owner instance found for secret ; skipping TLS enforcement", "name", tlsSecret.Name, "namespace", tlsSecret.Namespace)
+	return false
+}

controllers/argocd/argocdcommon/workloads.go

@@ -13,6 +13,10 @@ func TriggerDeploymentRollout(name, namespace, key string, client cntrlClient.Cl
 		return err
 	}
 
+	if deployment.Spec.Template.ObjectMeta.Labels == nil {
+		deployment.Spec.Template.ObjectMeta.Labels = make(map[string]string)
+	}
+
 	deployment.Spec.Template.ObjectMeta.Labels[key] = util.NowNano()
 	return workloads.UpdateDeployment(deployment, client)
 }
@@ -24,6 +28,10 @@ func TriggerStatefulSetRollout(name, namespace, key string, client cntrlClient.C
 		return err
 	}
 
+	if statefulset.Spec.Template.ObjectMeta.Labels == nil {
+		statefulset.Spec.Template.ObjectMeta.Labels = make(map[string]string)
+	}
+
 	statefulset.Spec.Template.ObjectMeta.Labels[key] = util.NowNano()
 	return workloads.UpdateStatefulSet(statefulset, client)
 }

controllers/argocd/notifications/deployment.go

@@ -65,31 +65,26 @@ func (nr *NotificationsReconciler) reconcileDeployment() error {
 	}
 	deploymentChanged := false
 
-	fieldsToCompare := []struct {
-		existing, desired interface{}
-		extraAction       func()
-	}{
-		{&existingDeployment.Spec.Template.Spec.Containers[0].Image, &desiredDeployment.Spec.Template.Spec.Containers[0].Image,
-			func() {
+	fieldsToCompare := []argocdcommon.FieldToCompare{
+		{Existing: &existingDeployment.Spec.Template.Spec.Containers[0].Image, Desired: &desiredDeployment.Spec.Template.Spec.Containers[0].Image,
+			ExtraAction: func() {
 				existingDeployment.Spec.Template.ObjectMeta.Labels[common.ImageUpgradedKey] = time.Now().UTC().Format(common.TimeFormatMST)
 			},
 		},
-		{&existingDeployment.Spec.Template.Spec.Containers[0].Command, &desiredDeployment.Spec.Template.Spec.Containers[0].Command, nil},
-		{&existingDeployment.Spec.Template.Spec.Containers[0].Env, &desiredDeployment.Spec.Template.Spec.Containers[0].Env, nil},
-		{&existingDeployment.Spec.Template.Spec.Containers[0].Resources, &desiredDeployment.Spec.Template.Spec.Containers[0].Resources, nil},
-		{&existingDeployment.Spec.Template.Spec.Volumes, &desiredDeployment.Spec.Template.Spec.Volumes, nil},
-		{&existingDeployment.Spec.Template.Spec.NodeSelector, &desiredDeployment.Spec.Template.Spec.NodeSelector, nil},
-		{&existingDeployment.Spec.Template.Spec.Tolerations, &desiredDeployment.Spec.Template.Spec.Tolerations, nil},
-		{&existingDeployment.Spec.Template.Spec.ServiceAccountName, &desiredDeployment.Spec.Template.Spec.ServiceAccountName, nil},
-		{&existingDeployment.Spec.Template.Labels, &desiredDeployment.Spec.Template.Labels, nil},
-		{&existingDeployment.Spec.Replicas, &desiredDeployment.Spec.Replicas, nil},
-		{&existingDeployment.Spec.Selector, &desiredDeployment.Spec.Selector, nil},
-		{&existingDeployment.Labels, &desiredDeployment.Labels, nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Containers[0].Command, Desired: &desiredDeployment.Spec.Template.Spec.Containers[0].Command, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Containers[0].Env, Desired: &desiredDeployment.Spec.Template.Spec.Containers[0].Env, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Containers[0].Resources, Desired: &desiredDeployment.Spec.Template.Spec.Containers[0].Resources, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Volumes, Desired: &desiredDeployment.Spec.Template.Spec.Volumes, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.NodeSelector, Desired: &desiredDeployment.Spec.Template.Spec.NodeSelector, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.Tolerations, Desired: &desiredDeployment.Spec.Template.Spec.Tolerations, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Spec.ServiceAccountName, Desired: &desiredDeployment.Spec.Template.Spec.ServiceAccountName, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Template.Labels, Desired: &desiredDeployment.Spec.Template.Labels, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Replicas, Desired: &desiredDeployment.Spec.Replicas, ExtraAction: nil},
+		{Existing: &existingDeployment.Spec.Selector, Desired: &desiredDeployment.Spec.Selector, ExtraAction: nil},
+		{Existing: &existingDeployment.Labels, Desired: &desiredDeployment.Labels, ExtraAction: nil},
 	}
 
-	for _, field := range fieldsToCompare {
-		argocdcommon.UpdateIfChanged(field.existing, field.desired, field.extraAction, &deploymentChanged)
-	}
+	argocdcommon.UpdateIfChanged(fieldsToCompare, &deploymentChanged)
 
 	if deploymentChanged {
 

controllers/argocd/notifications/rolebinding.go

@@ -1,13 +1,15 @@
 package notifications
 
 import (
+	"reflect"
+
 	"github.com/argoproj-labs/argocd-operator/common"
 	"github.com/argoproj-labs/argocd-operator/controllers/argocd/argocdcommon"
 	"github.com/argoproj-labs/argocd-operator/pkg/cluster"
 	"github.com/argoproj-labs/argocd-operator/pkg/permissions"
+	"github.com/pkg/errors"
 
 	rbacv1 "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -61,7 +63,7 @@ func (nr *NotificationsReconciler) reconcileRoleBinding() error {
 
 	existingRoleBinding, err := permissions.GetRoleBinding(desiredRoleBinding.Name, desiredRoleBinding.Namespace, nr.Client)
 	if err != nil {
-		if !errors.IsNotFound(err) {
+		if !apierrors.IsNotFound(err) {
 			nr.Logger.Error(err, "reconcileRoleBinding: failed to retrieve roleBinding", "name", desiredRoleBinding.Name, "namespace", desiredRoleBinding.Namespace)
 			return err
 		}
@@ -78,32 +80,32 @@ func (nr *NotificationsReconciler) reconcileRoleBinding() error {
 		return nil
 	}
 
-	roleBindingChanged := false
-	fieldsToCompare := []struct {
-		existing, desired interface{}
-	}{
-		{
-			&existingRoleBinding.RoleRef,
-			&desiredRoleBinding.RoleRef,
-		},
-		{
-			&existingRoleBinding.Subjects,
-			&desiredRoleBinding.Subjects,
-		},
+	// if roleRef differs, we must delete the rolebinding as kubernetes does not allow updation of roleRef
+	if !reflect.DeepEqual(existingRoleBinding.RoleRef, desiredRoleBinding.RoleRef) {
+		nr.Logger.Info("detected drift in roleRef for rolebinding", "name", existingRoleBinding.Name, "namespace", existingRoleBinding.Namespace)
+		if err := nr.deleteRoleBinding(resourceName, nr.Instance.Namespace); err != nil {
+			return errors.Wrapf(err, "reconcileRoleBinding: unable to delete obsolete rolebinding %s", existingRoleBinding.Name)
+		}
+		return nil
+	}
+
+	rbChanged := false
+
+	fieldsToCompare := []argocdcommon.FieldToCompare{
+		{Existing: &existingRoleBinding.Subjects, Desired: &desiredRoleBinding.Subjects, ExtraAction: nil},
 	}
 
-	for _, field := range fieldsToCompare {
-		argocdcommon.UpdateIfChanged(field.existing, field.desired, nil, &roleBindingChanged)
+	argocdcommon.UpdateIfChanged(fieldsToCompare, &rbChanged)
+
+	if !rbChanged {
+		return nil
 	}
 
-	if roleBindingChanged {
-		if err = permissions.UpdateRoleBinding(existingRoleBinding, nr.Client); err != nil {
-			nr.Logger.Error(err, "reconcileRoleBinding: failed to update roleBinding", "name", existingRoleBinding.Name, "namespace", existingRoleBinding.Namespace)
-			return err
-		}
+	if err = permissions.UpdateRoleBinding(existingRoleBinding, nr.Client); err != nil {
+		return errors.Wrapf(err, "reconcileRoleBinding: failed to update role %s", existingRoleBinding.Name)
 	}
 
-	nr.Logger.V(0).Info("reconcileRoleBinding: roleBinding updated", "name", existingRoleBinding.Name, "namespace", existingRoleBinding.Namespace)
+	nr.Logger.Info("rolebinding updated", "name", existingRoleBinding.Name, "namespace", existingRoleBinding.Namespace)
 
 	return nil
 }