feat: Remove ownerReference from Keycloak resources (#71)

Setting ownerReference with controller:true for Keycloak resources has disadvantages: - `controller: true` should be set for resources created by a controller. For example, we create a Deployment, and the controller creates a ReplicaSet with an `ownerReference` to the Deployment. In our case, we create resources separately, and the Keycloak controller isn't responsible for their creation. - ArgoCD doesn't remove resources with `controller: true`. It considers another controller responsible for them. If we remove the ArgoCD environment that contains KeycloakClient, this CR won't be removed by either ArgoCD or the Keycloak operator. This change break automatic deletion of CRs if related Keycloak/KeycloakRealm is removed. Now it isn't responsibility of the keycloak operator.
epam · Jul 26, 2024 · 67a3563 · 67a3563
1 parent a4331c6
commit 67a3563
Show file tree

Hide file tree

Showing 16 changed files with 8 additions and 352 deletions.
diff --git a/README.md b/README.md
@@ -125,6 +125,14 @@ To prevent the operator from deleting resources from Keycloak, add the `edp.epam
        kind: Keycloak
    ```
 
+#### Resources deletion
+
+To avoid resources getting stuck during deletion, it is important to delete them in the correct order:
+
+1. **First**, remove realm resources `KeycloakClient`, `KeycloakRealmUser`, etc.
+2. **Then**, remove `KeycloakRealm`/`ClusterKeycloakRealm`.
+3. **Finally**, remove `Keycloak`/`ClusterKeycloak`.
+
 ## Local Development
 
 To develop the operator, first set up a local environment, and refer to the [Local Development](https://epam.github.io/edp-install/developer-guide/local-development/) page.

diff --git a/controllers/clusterkeycloakrealm/clusterkeycloakrealm_controller.go b/controllers/clusterkeycloakrealm/clusterkeycloakrealm_controller.go
@@ -23,7 +23,6 @@ type Helper interface {
 	SetFailureCount(fc helper.FailureCountable) time.Duration
 	TryToDelete(ctx context.Context, obj client.Object, terminator helper.Terminator, finalizer string) (isDeleted bool, resultErr error)
 	CreateKeycloakClientFromClusterRealm(ctx context.Context, realm *keycloakAlpha.ClusterKeycloakRealm) (keycloak.Client, error)
-	SetKeycloakOwnerRef(ctx context.Context, object helper.ObjectWithKeycloakRef) error
 	InvalidateKeycloakClientTokenSecret(ctx context.Context, namespace, rootKeycloakName string) error
 }
 
@@ -61,10 +60,6 @@ func (r *ClusterKeycloakRealmReconciler) Reconcile(ctx context.Context, req ctrl
 		return ctrl.Result{}, fmt.Errorf("unable to get cluster realm: %w", err)
 	}
 
-	if err := r.helper.SetKeycloakOwnerRef(ctx, clusterRealm); err != nil {
-		return ctrl.Result{}, fmt.Errorf("unable to set keycloak owner ref: %w", err)
-	}
-
 	kClient, err := r.helper.CreateKeycloakClientFromClusterRealm(ctx, clusterRealm)
 	if err != nil {
 		if errors.Is(err, helper.ErrKeycloakIsNotAvailable) {

diff --git a/controllers/helper/controller_helper.go b/controllers/helper/controller_helper.go
@@ -10,7 +10,6 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/go-resty/resty/v2"
 	"github.com/pkg/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -61,8 +60,6 @@ type adapterBuilder func(
 //
 //go:generate mockery --name ControllerHelper --filename helper_mock.go
 type ControllerHelper interface {
-	SetKeycloakOwnerRef(ctx context.Context, object ObjectWithKeycloakRef) error
-	SetRealmOwnerRef(ctx context.Context, object ObjectWithRealmRef) error
 	SetFailureCount(fc FailureCountable) time.Duration
 	TryToDelete(ctx context.Context, obj client.Object, terminator Terminator, finalizer string) (isDeleted bool, resultErr error)
 	GetKeycloakRealmFromRef(ctx context.Context, object ObjectWithRealmRef, kcClient keycloak.Client) (*gocloak.RealmRepresentation, error)
@@ -115,114 +112,6 @@ func MakeHelper(client client.Client, scheme *runtime.Scheme, operatorNamespace
 	}
 }
 
-// SetKeycloakOwnerRef sets owner reference for object.
-//
-//nolint:dupl,cyclop
-func (h *Helper) SetKeycloakOwnerRef(ctx context.Context, object ObjectWithKeycloakRef) error {
-	if metav1.GetControllerOf(object) != nil {
-		return nil
-	}
-
-	kind := object.GetKeycloakRef().Kind
-	name := object.GetKeycloakRef().Name
-
-	switch kind {
-	case keycloakApi.KeycloakKind:
-		kc := &keycloakApi.Keycloak{}
-		if err := h.client.Get(ctx, types.NamespacedName{
-			Namespace: object.GetNamespace(),
-			Name:      name,
-		}, kc); err != nil {
-			return fmt.Errorf("failed to get Keycloak: %w", err)
-		}
-
-		if err := controllerutil.SetControllerReference(kc, object, h.scheme); err != nil {
-			return fmt.Errorf("failed to set controller reference for %s: %w", object.GetName(), err)
-		}
-
-		if err := h.client.Update(ctx, object); err != nil {
-			return fmt.Errorf("failed to update keycloak owner reference %s: %w", kc.GetName(), err)
-		}
-
-		return nil
-
-	case keycloakAlpha.ClusterKeycloakKind:
-		clusterKc := &keycloakAlpha.ClusterKeycloak{}
-		if err := h.client.Get(ctx, types.NamespacedName{
-			Name: name,
-		}, clusterKc); err != nil {
-			return fmt.Errorf("failed to get ClusterKeycloak: %w", err)
-		}
-
-		if err := controllerutil.SetControllerReference(clusterKc, object, h.scheme); err != nil {
-			return fmt.Errorf("failed to set controller reference for %s: %w", object.GetName(), err)
-		}
-
-		if err := h.client.Update(ctx, object); err != nil {
-			return fmt.Errorf("failed to update keycloak owner reference %s: %w", clusterKc.GetName(), err)
-		}
-
-		return nil
-
-	default:
-		return fmt.Errorf("unknown keycloak kind: %s", kind)
-	}
-}
-
-// SetRealmOwnerRef sets owner reference for object.
-//
-//nolint:dupl,cyclop
-func (h *Helper) SetRealmOwnerRef(ctx context.Context, object ObjectWithRealmRef) error {
-	if metav1.GetControllerOf(object) != nil {
-		return nil
-	}
-
-	kind := object.GetRealmRef().Kind
-	name := object.GetRealmRef().Name
-
-	switch kind {
-	case keycloakApi.KeycloakRealmKind:
-		realm := &keycloakApi.KeycloakRealm{}
-		if err := h.client.Get(ctx, types.NamespacedName{
-			Namespace: object.GetNamespace(),
-			Name:      name,
-		}, realm); err != nil {
-			return fmt.Errorf("failed to get KeycloakRealm: %w", err)
-		}
-
-		if err := controllerutil.SetControllerReference(realm, object, h.scheme); err != nil {
-			return fmt.Errorf("failed to set controller reference for %s: %w", object.GetName(), err)
-		}
-
-		if err := h.client.Update(ctx, object); err != nil {
-			return fmt.Errorf("failed to update realm owner reference %s: %w", realm.GetName(), err)
-		}
-
-		return nil
-
-	case keycloakAlpha.ClusterKeycloakRealmKind:
-		clusterRealm := &keycloakAlpha.ClusterKeycloakRealm{}
-		if err := h.client.Get(ctx, types.NamespacedName{
-			Name: name,
-		}, clusterRealm); err != nil {
-			return fmt.Errorf("failed to get ClusterKeycloakRealm: %w", err)
-		}
-
-		if err := controllerutil.SetControllerReference(clusterRealm, object, h.scheme); err != nil {
-			return fmt.Errorf("unable to set controller reference for %s: %w", object.GetName(), err)
-		}
-
-		if err := h.client.Update(ctx, object); err != nil {
-			return fmt.Errorf("failed to update realm owner reference %s: %w", clusterRealm.GetName(), err)
-		}
-
-		return nil
-
-	default:
-		return fmt.Errorf("unknown realm kind: %s", kind)
-	}
-}
-
 func (h *Helper) TryToDelete(ctx context.Context, obj client.Object, terminator Terminator, finalizer string) (isDeleted bool, resultErr error) {
 	logger := ctrl.LoggerFrom(ctx)
 

diff --git a/controllers/helper/controller_helper_test.go b/controllers/helper/controller_helper_test.go
@@ -8,139 +8,17 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/go-resty/resty/v2"
 	"github.com/pkg/errors"
-	"github.com/stretchr/testify/assert"
-	testifymock "github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
-	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
-	"github.com/epam/edp-keycloak-operator/api/common"
 	keycloakApi "github.com/epam/edp-keycloak-operator/api/v1"
 	"github.com/epam/edp-keycloak-operator/pkg/client/keycloak/adapter"
 	"github.com/epam/edp-keycloak-operator/pkg/client/keycloak/mock"
 	"github.com/epam/edp-keycloak-operator/pkg/fakehttp"
 )
 
-func TestHelper_GetOrCreateRealmOwnerRef(t *testing.T) {
-	mc := K8SClientMock{}
-
-	sch := runtime.NewScheme()
-	utilruntime.Must(keycloakApi.AddToScheme(sch))
-
-	helper := MakeHelper(&mc, sch, "default")
-
-	kcGroup := keycloakApi.KeycloakRealmGroup{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "test",
-		},
-		Spec: keycloakApi.KeycloakRealmGroupSpec{
-			RealmRef: common.RealmRef{
-				Kind: keycloakApi.KeycloakRealmKind,
-				Name: "realm",
-			},
-		},
-	}
-
-	mc.On("Get", types.NamespacedName{
-		Namespace: "test",
-		Name:      "realm",
-	}, &keycloakApi.KeycloakRealm{}).Return(nil)
-	mc.On("Update", testifymock.Anything, testifymock.Anything).Return(nil)
-
-	err := helper.SetRealmOwnerRef(context.Background(), &kcGroup)
-	require.NoError(t, err)
-
-	kcGroup = keycloakApi.KeycloakRealmGroup{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "test",
-		},
-		Spec: keycloakApi.KeycloakRealmGroupSpec{
-			Realm: "foo13",
-			RealmRef: common.RealmRef{
-				Kind: keycloakApi.KeycloakRealmKind,
-				Name: "realm",
-			},
-		},
-	}
-
-	mc.On("Get", types.NamespacedName{
-		Namespace: "test",
-		Name:      "foo13",
-	}, &keycloakApi.KeycloakRealm{}).Return(nil)
-
-	err = helper.SetRealmOwnerRef(context.Background(), &kcGroup)
-	require.NoError(t, err)
-}
-
-func TestHelper_GetOrCreateRealmOwnerRef_Failure(t *testing.T) {
-	mc := K8SClientMock{}
-
-	sch := runtime.NewScheme()
-	utilruntime.Must(keycloakApi.AddToScheme(sch))
-
-	helper := MakeHelper(&mc, sch, "default")
-
-	kcGroup := keycloakApi.KeycloakRealmGroup{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "test",
-			OwnerReferences: []metav1.OwnerReference{
-				{
-					Name: "foo",
-					Kind: "KeycloakRealm",
-				},
-			},
-		},
-		Spec: keycloakApi.KeycloakRealmGroupSpec{
-			RealmRef: common.RealmRef{
-				Kind: keycloakApi.KeycloakRealmKind,
-				Name: "realm",
-			},
-		},
-	}
-
-	mockErr := errors.New("mock error")
-
-	mc.On("Get", types.NamespacedName{
-		Namespace: "test",
-		Name:      kcGroup.Spec.RealmRef.Name,
-	}, &keycloakApi.KeycloakRealm{}).Return(mockErr)
-
-	err := helper.SetRealmOwnerRef(context.Background(), &kcGroup)
-	if err == nil {
-		t.Fatal("no error on k8s client get fatal")
-	}
-
-	assert.ErrorIs(t, err, mockErr)
-
-	kcGroup = keycloakApi.KeycloakRealmGroup{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "test",
-		},
-		Spec: keycloakApi.KeycloakRealmGroupSpec{
-			RealmRef: common.RealmRef{
-				Kind: keycloakApi.KeycloakRealmKind,
-				Name: "realm",
-			},
-		},
-	}
-
-	mc.On("Get", types.NamespacedName{
-		Namespace: "test",
-		Name:      kcGroup.Spec.RealmRef.Name,
-	}, &keycloakApi.KeycloakRealm{}).Return(mockErr)
-
-	err = helper.SetRealmOwnerRef(context.Background(), &kcGroup)
-	if err == nil {
-		t.Fatal("no error on k8s client get fatal")
-	}
-
-	assert.ErrorIs(t, err, mockErr)
-}
-
 func TestMakeHelper(t *testing.T) {
 	rCl := resty.New()
 

diff --git a/controllers/keycloakauthflow/keycloakauthflow_controller.go b/controllers/keycloakauthflow/keycloakauthflow_controller.go
@@ -30,7 +30,6 @@ type Helper interface {
 	SetFailureCount(fc helper.FailureCountable) time.Duration
 	TryToDelete(ctx context.Context, obj client.Object, terminator helper.Terminator, finalizer string) (isDeleted bool, resultErr error)
 	CreateKeycloakClientFromRealmRef(ctx context.Context, object helper.ObjectWithRealmRef) (keycloak.Client, error)
-	SetRealmOwnerRef(ctx context.Context, object helper.ObjectWithRealmRef) error
 	GetKeycloakRealmFromRef(ctx context.Context, object helper.ObjectWithRealmRef, kcClient keycloak.Client) (*gocloak.RealmRepresentation, error)
 }
 
@@ -127,10 +126,6 @@ func (r *Reconcile) Reconcile(ctx context.Context, request reconcile.Request) (r
 }
 
 func (r *Reconcile) tryReconcile(ctx context.Context, instance *keycloakApi.KeycloakAuthFlow) error {
-	if err := r.helper.SetRealmOwnerRef(ctx, instance); err != nil {
-		return fmt.Errorf("unable to set realm owner ref: %w", err)
-	}
-
 	kClient, err := r.helper.CreateKeycloakClientFromRealmRef(ctx, instance)
 	if err != nil {
 		return fmt.Errorf("unable to create keycloak client from realm ref: %w", err)

diff --git a/controllers/keycloakclient/keycloakclient_controller.go b/controllers/keycloakclient/keycloakclient_controller.go
@@ -26,7 +26,6 @@ import (
 type Helper interface {
 	SetFailureCount(fc helper.FailureCountable) time.Duration
 	TryToDelete(ctx context.Context, obj client.Object, terminator helper.Terminator, finalizer string) (isDeleted bool, resultErr error)
-	SetRealmOwnerRef(ctx context.Context, object helper.ObjectWithRealmRef) error
 	CreateKeycloakClientFromRealmRef(ctx context.Context, object helper.ObjectWithRealmRef) (keycloak.Client, error)
 	GetKeycloakRealmFromRef(ctx context.Context, object helper.ObjectWithRealmRef, kcClient keycloak.Client) (*gocloak.RealmRepresentation, error)
 }
@@ -118,11 +117,6 @@ func (r *ReconcileKeycloakClient) Reconcile(ctx context.Context, request reconci
 }
 
 func (r *ReconcileKeycloakClient) tryReconcile(ctx context.Context, keycloakClient *keycloakApi.KeycloakClient) error {
-	err := r.helper.SetRealmOwnerRef(ctx, keycloakClient)
-	if err != nil {
-		return fmt.Errorf("unable to set realm owner ref: %w", err)
-	}
-
 	kClient, err := r.helper.CreateKeycloakClientFromRealmRef(ctx, keycloakClient)
 	if err != nil {
 		return fmt.Errorf("unable to create keycloak client from realm ref: %w", err)

diff --git a/controllers/keycloakclientscope/keycloakclientscope_controller.go b/controllers/keycloakclientscope/keycloakclientscope_controller.go
@@ -30,7 +30,6 @@ const finalizerName = "keycloak.clientscope.operator.finalizer.name"
 type Helper interface {
 	SetFailureCount(fc helper.FailureCountable) time.Duration
 	TryToDelete(ctx context.Context, obj client.Object, terminator helper.Terminator, finalizer string) (isDeleted bool, resultErr error)
-	SetRealmOwnerRef(ctx context.Context, object helper.ObjectWithRealmRef) error
 	GetKeycloakRealmFromRef(ctx context.Context, object helper.ObjectWithRealmRef, kcClient keycloak.Client) (*gocloak.RealmRepresentation, error)
 	CreateKeycloakClientFromRealmRef(ctx context.Context, object helper.ObjectWithRealmRef) (keycloak.Client, error)
 }
@@ -131,11 +130,6 @@ func (r *Reconcile) Reconcile(ctx context.Context, request reconcile.Request) (r
 }
 
 func (r *Reconcile) tryReconcile(ctx context.Context, instance *keycloakApi.KeycloakClientScope) (string, error) {
-	err := r.helper.SetRealmOwnerRef(ctx, instance)
-	if err != nil {
-		return "", fmt.Errorf("unable to set realm owner ref: %w", err)
-	}
-
 	cl, err := r.helper.CreateKeycloakClientFromRealmRef(ctx, instance)
 	if err != nil {
 		return "", fmt.Errorf("unable to create keycloak client from realm ref: %w", err)

diff --git a/controllers/keycloakrealm/keycloakrealm_controller.go b/controllers/keycloakrealm/keycloakrealm_controller.go
@@ -29,7 +29,6 @@ type Helper interface {
 	SetFailureCount(fc helper.FailureCountable) time.Duration
 	TryToDelete(ctx context.Context, obj client.Object, terminator helper.Terminator, finalizer string) (isDeleted bool, resultErr error)
 	CreateKeycloakClientFromRealm(ctx context.Context, realm *keycloakApi.KeycloakRealm) (keycloak.Client, error)
-	SetKeycloakOwnerRef(ctx context.Context, object helper.ObjectWithKeycloakRef) error
 	InvalidateKeycloakClientTokenSecret(ctx context.Context, namespace, rootKeycloakName string) error
 }
 
@@ -123,10 +122,6 @@ func (r *ReconcileKeycloakRealm) Reconcile(ctx context.Context, request reconcil
 }
 
 func (r *ReconcileKeycloakRealm) tryReconcile(ctx context.Context, realm *keycloakApi.KeycloakRealm) error {
-	if err := r.helper.SetKeycloakOwnerRef(ctx, realm); err != nil {
-		return fmt.Errorf("failed to set keycloak owner reference: %w", err)
-	}
-
 	kClient, err := r.helper.CreateKeycloakClientFromRealm(ctx, realm)
 	if err != nil {
 		return fmt.Errorf("failed to create keycloak client for realm: %w", err)