Fix rub owner deletion bug + remove unused fields from RemoteSyncer

api/v2alpha2/remotesyncer_types.go

@@ -26,8 +26,6 @@ import (
 )
 
 type RemoteSyncerSpec struct {
-	CommitMode CommitMode `json:"commitMode"`
-
 	// +kubebuilder:validation:MinItems=1
 	// +kubebuilder:validation:MaxItems=3
 	Operations []admissionregistrationv1.OperationType `json:"operations"`
@@ -51,7 +49,7 @@ type RemoteSyncerSpec struct {
 	DefaultUnauthorizedUserMode DefaultUnauthorizedUserMode `json:"defaultUnauthorizedUserMode"`
 
 	// +optional
-	DefaultUserBind *corev1.ObjectReference `json:"defaultUserBind,omitempty"` // Ref to a RemoteUserBinding object
+	DefaultUserBind *corev1.ObjectReference `json:"defaultUserBind,omitempty"` // Ref to a RemoteUser object
 
 	// +optional
 	IncludedResources []NamespaceScopedResourcesPath `json:"includedResources,omitempty"`

api/v2alpha2/remoteuser_webhook.go

@@ -45,7 +45,7 @@ func (r *RemoteUser) SetupWebhookWithManager(mgr ctrl.Manager) error {
 }
 
 //+kubebuilder:webhook:path=/validate-syngit-syngit-io-v2alpha2-remoteuser,mutating=false,failurePolicy=fail,sideEffects=None,groups=syngit.syngit.io,resources=remoteusers,verbs=create;update,versions=v2alpha2,name=vremoteuser.kb.io,admissionReviewVersions=v1
-//+kubebuilder:webhook:path=/reconcile-syngit-remoteuser-owner,mutating=false,failurePolicy=fail,sideEffects=None,groups=syngit.syngit.io,resources=remoteusers,verbs=create,versions=v2alpha2,admissionReviewVersions=v1,name=vremoteusers-owner.kb.io
+//+kubebuilder:webhook:path=/reconcile-syngit-remoteuser-owner,mutating=false,failurePolicy=fail,sideEffects=None,groups=syngit.syngit.io,resources=remoteusers,verbs=create;delete,versions=v2alpha2,admissionReviewVersions=v1,name=vremoteusers-owner.kb.io
 
 var _ webhook.Validator = &RemoteUser{}
 
@@ -102,6 +102,66 @@ type RemoteUserWebhookHandler struct {
 }
 
 func (ruwh *RemoteUserWebhookHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
+
+	username := req.DeepCopy().UserInfo.Username
+	name := rubPrefix + username
+
+	if string(req.Operation) == "DELETE" {
+		ru := &RemoteUser{}
+		err := ruwh.Decoder.DecodeRaw(req.OldObject, ru)
+		if err != nil {
+			return admission.Errored(http.StatusBadRequest, err)
+		}
+
+		rub := &RemoteUserBinding{}
+		webhookNamespacedName := &types.NamespacedName{
+			Name:      name,
+			Namespace: req.Namespace,
+		}
+		rubErr := ruwh.Client.Get(ctx, *webhookNamespacedName, rub)
+		if rubErr != nil {
+			return admission.Errored(http.StatusBadRequest, rubErr)
+		}
+
+		isControlled := false
+		ownerRefs := rub.ObjectMeta.DeepCopy().OwnerReferences
+		newOwnerRefs := []v1.OwnerReference{}
+		for _, owner := range ownerRefs {
+			if string(owner.UID) != string(ru.UID) {
+				newOwnerRefs = append(newOwnerRefs, owner)
+			} else {
+				isControlled = true
+			}
+		}
+		rub.ObjectMeta.OwnerReferences = newOwnerRefs
+
+		remoteRefs := rub.Spec.DeepCopy().RemoteRefs
+		newRemoteRefs := []corev1.ObjectReference{}
+		for _, rm := range remoteRefs {
+			if (rm.Name != ru.Name && isControlled) || !isControlled {
+				newRemoteRefs = append(newRemoteRefs, rm)
+			}
+		}
+		rub.Spec.RemoteRefs = newRemoteRefs
+
+		if len(newRemoteRefs) != 0 {
+
+			deleteErr := ruwh.Client.Update(ctx, rub)
+			if deleteErr != nil {
+				return admission.Errored(http.StatusInternalServerError, deleteErr)
+			}
+
+		} else {
+
+			deleteErr := ruwh.Client.Delete(ctx, rub)
+			if deleteErr != nil {
+				return admission.Errored(http.StatusInternalServerError, deleteErr)
+			}
+		}
+
+		return admission.Allowed("This object has been removed from the " + name + " RemoteUserBinding owners")
+	}
+
 	ru := &RemoteUser{}
 	err := ruwh.Decoder.Decode(req, ru)
 	if err != nil {
@@ -112,9 +172,7 @@ func (ruwh *RemoteUserWebhookHandler) Handle(ctx context.Context, req admission.
 		return admission.Allowed("This object does not own a RemoteUserBinding")
 	}
 
-	username := req.DeepCopy().UserInfo.Username
 	objRef := corev1.ObjectReference{Name: ru.Name}
-	name := rubPrefix + username
 	rub := &RemoteUserBinding{}
 	webhookNamespacedName := &types.NamespacedName{
 		Name:      name,
@@ -128,13 +186,11 @@ func (ruwh *RemoteUserWebhookHandler) Handle(ctx context.Context, req admission.
 		rub.Name = name
 		rub.Namespace = req.Namespace
 
-		isControlled := true
 		ownerRef := v1.OwnerReference{
 			Name:       ru.Name,
 			APIVersion: ru.APIVersion,
 			Kind:       ru.GroupVersionKind().Kind,
 			UID:        ru.GetUID(),
-			Controller: &isControlled,
 		}
 		ownerRefs := make([]v1.OwnerReference, 0)
 		ownerRefs = append(ownerRefs, ownerRef)
@@ -158,15 +214,13 @@ func (ruwh *RemoteUserWebhookHandler) Handle(ctx context.Context, req admission.
 		// The RemoteUserBinding already exists
 
 		// Update the list of the RemoteUserBinding object
-		isControlled := true
 		ownerRef := v1.OwnerReference{
 			Name:       ru.Name,
 			APIVersion: ru.APIVersion,
 			Kind:       ru.GroupVersionKind().Kind,
 			UID:        ru.GetUID(),
-			Controller: &isControlled,
 		}
-		ownerRefs := make([]v1.OwnerReference, 0)
+		ownerRefs := rub.ObjectMeta.DeepCopy().OwnerReferences
 		ownerRefs = append(ownerRefs, ownerRef)
 		rub.ObjectMeta.OwnerReferences = ownerRefs
 
@@ -178,6 +232,7 @@ func (ruwh *RemoteUserWebhookHandler) Handle(ctx context.Context, req admission.
 		if updateErr != nil {
 			return admission.Errored(http.StatusInternalServerError, updateErr)
 		}
+
 	}
 
 	return admission.Allowed("This object owns the " + name + " RemoteUserBinding")

chart/0.0.2/templates/crd/syngit.syngit.io_remotesyncer.yaml

@@ -571,8 +571,6 @@ spec:
                   type: object
                   x-kubernetes-map-type: atomic
                 type: array
-              commitMode:
-                type: string
               commitProcess:
                 type: string
               defaultBlockAppliedMessage:
@@ -693,7 +691,6 @@ spec:
             required:
             - authorizedUsers
             - branch
-            - commitMode
             - commitProcess
             - defaultUnauthorizedUserMode
             - operations

config/crd/bases/syngit.syngit.io_remotesyncers.yaml

@@ -618,8 +618,6 @@ spec:
                   type: object
                   x-kubernetes-map-type: atomic
                 type: array
-              commitMode:
-                type: string
               commitProcess:
                 type: string
               defaultBlockAppliedMessage:
@@ -756,7 +754,6 @@ spec:
             required:
             - authorizedUsers
             - branch
-            - commitMode
             - commitProcess
             - defaultUnauthorizedUserMode
             - operations

internal/controller/remoteuser_controller.go

@@ -35,7 +35,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -101,58 +100,6 @@ func (r *RemoteUserReconciler) setServerConfiguration(ctx context.Context, remot
 	return *gpc, nil
 }
 
-func (r *RemoteUserReconciler) finalizer(ctx context.Context, remoteUser *syngit.RemoteUser) error {
-
-	finalizerName := "syngit.io/remote-user-bindings"
-
-	// examine DeletionTimestamp to determine if object is under deletion
-	if remoteUser.ObjectMeta.DeletionTimestamp.IsZero() {
-		// The object is not being deleted, so if it does not have our finalizer,
-		// then lets add the finalizer and update the object. This is equivalent
-		// to registering our finalizer.
-		if !controllerutil.ContainsFinalizer(remoteUser, finalizerName) {
-			controllerutil.AddFinalizer(remoteUser, finalizerName)
-			if err := r.Update(ctx, remoteUser); err != nil {
-				return err
-			}
-		}
-	} else {
-		// The object is being deleted
-		if controllerutil.ContainsFinalizer(remoteUser, finalizerName) {
-
-			allRemoteUserBindings := syngit.RemoteUserBindingList{}
-			if err := r.List(ctx, &allRemoteUserBindings, client.InNamespace(remoteUser.Namespace)); err != nil {
-				// does not exists -> deleted
-				return client.IgnoreNotFound(err)
-			}
-			remoteUserBindings := make([]syngit.RemoteUserBinding, 0)
-			for _, rub := range allRemoteUserBindings.Items {
-				if len(rub.OwnerReferences) != 0 {
-					owner := rub.OwnerReferences[0]
-					if owner.UID == remoteUser.GetUID() {
-						remoteUserBindings = append(remoteUserBindings, rub)
-					}
-				}
-			}
-
-			for _, rub := range remoteUserBindings {
-				err := r.Delete(ctx, &rub)
-				if err != nil {
-					return err
-				}
-			}
-
-			// remove our finalizer from the list and update it.
-			controllerutil.RemoveFinalizer(remoteUser, finalizerName)
-			if err := r.Update(ctx, remoteUser); err != nil {
-				return err
-			}
-		}
-	}
-
-	return nil
-}
-
 // +kubebuilder:rbac:groups=syngit.syngit.io,resources=remoteusers,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=syngit.syngit.io,resources=remoteusers/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=syngit.syngit.io,resources=remoteusers/finalizers,verbs=update
@@ -180,11 +127,6 @@ func (r *RemoteUserReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		Status:             "False",
 	}
 
-	finalizerError := r.finalizer(ctx, &remoteUser)
-	if finalizerError != nil {
-		return ctrl.Result{}, finalizerError
-	}
-
 	// Get the referenced Secret
 	var secret corev1.Secret
 	namespacedNameSecret := types.NamespacedName{Namespace: req.Namespace, Name: remoteUser.Spec.SecretRef.Name}