Skip to content

Commit 6447eb4

Browse files
zmotsoSergK
zmotso
authored and
SergK
committed
fix: Deletion resources related to subgroup (#95)
Problem: If we delete the subgroup before deleting the resources that contain this subgroup (Example: KeycloakRealmGroup.spec.subGroup/KeycloakRealmUser.spec.groups) deletion of KeycloakRealmGroup/KeycloakRealmUser blocks with an error. This happens because we are syncing KeycloakRealmUser/KeycloakRealmGroup before deletion, and syncing can't be processed without a subgroup. Solution: Moved deletion of KeycloakRealmUser/KeycloakRealmGroup before syncing. We don't need to sync objects that are marked for deletion.
1 parent 5d22cc4 commit 6447eb4

File tree

4 files changed

+146
-28
lines changed

4 files changed

+146
-28
lines changed

‎controllers/keycloakrealmgroup/keycloakrealmgroup_controller.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,7 @@ func (r *ReconcileKeycloakRealmGroup) tryReconcile(ctx context.Context, keycloak
130130
return fmt.Errorf("unable to get keycloak realm from ref: %w", err)
131131
}
132132

133-
id, err := kClient.SyncRealmGroup(ctx, gocloak.PString(realm.Realm), &keycloakRealmGroup.Spec)
134-
if err != nil {
135-
return fmt.Errorf("unable to sync realm group: %w", err)
136-
}
137-
138-
keycloakRealmGroup.Status.ID = id
139-
140-
if _, err := r.helper.TryToDelete(
133+
deleted, err := r.helper.TryToDelete(
141134
ctx,
142135
keycloakRealmGroup,
143136
makeTerminator(
@@ -147,10 +140,22 @@ func (r *ReconcileKeycloakRealmGroup) tryReconcile(ctx context.Context, keycloak
147140
objectmeta.PreserveResourcesOnDeletion(keycloakRealmGroup),
148141
),
149142
keyCloakRealmGroupOperatorFinalizerName,
150-
); err != nil {
143+
)
144+
if err != nil {
151145
return fmt.Errorf("failed to delete keycloak realm group: %w", err)
152146
}
153147

148+
if deleted {
149+
return nil
150+
}
151+
152+
id, err := kClient.SyncRealmGroup(ctx, gocloak.PString(realm.Realm), &keycloakRealmGroup.Spec)
153+
if err != nil {
154+
return fmt.Errorf("unable to sync realm group: %w", err)
155+
}
156+
157+
keycloakRealmGroup.Status.ID = id
158+
154159
return nil
155160
}
156161

‎controllers/keycloakrealmgroup/keycloakrealmgroup_controller_integration_test.go

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ var _ = Describe("KeycloakRealmGroup controller", Ordered, func() {
3030
Expect(adapter.SkipAlreadyExistsErr(err)).ShouldNot(HaveOccurred())
3131

3232
By("Creating a KeycloakRealmGroup subgroup")
33-
group := &keycloakApi.KeycloakRealmGroup{
33+
subgroup := &keycloakApi.KeycloakRealmGroup{
3434
ObjectMeta: metav1.ObjectMeta{
3535
Name: "test-subgroup",
3636
Namespace: ns,
@@ -44,16 +44,39 @@ var _ = Describe("KeycloakRealmGroup controller", Ordered, func() {
4444
Path: "/test-subgroup",
4545
},
4646
}
47-
Expect(k8sClient.Create(ctx, group)).Should(Succeed())
47+
Expect(k8sClient.Create(ctx, subgroup)).Should(Succeed())
48+
Eventually(func(g Gomega) {
49+
createdGroup := &keycloakApi.KeycloakRealmGroup{}
50+
err = k8sClient.Get(ctx, types.NamespacedName{Name: subgroup.Name, Namespace: ns}, createdGroup)
51+
g.Expect(err).ShouldNot(HaveOccurred())
52+
g.Expect(createdGroup.Status.Value).Should(Equal(helper.StatusOK))
53+
}).WithTimeout(time.Second * 20).WithPolling(time.Second).Should(Succeed())
54+
55+
By("Creating a KeycloakRealmGroup subgroup2")
56+
subgroup2 := &keycloakApi.KeycloakRealmGroup{
57+
ObjectMeta: metav1.ObjectMeta{
58+
Name: "test-subgroup2",
59+
Namespace: ns,
60+
},
61+
Spec: keycloakApi.KeycloakRealmGroupSpec{
62+
Name: "test-subgroup2",
63+
RealmRef: common.RealmRef{
64+
Kind: keycloakApi.KeycloakRealmKind,
65+
Name: KeycloakRealmCR,
66+
},
67+
Path: "/test-subgroup2",
68+
},
69+
}
70+
Expect(k8sClient.Create(ctx, subgroup2)).Should(Succeed())
4871
Eventually(func(g Gomega) {
4972
createdGroup := &keycloakApi.KeycloakRealmGroup{}
50-
err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-subgroup", Namespace: ns}, createdGroup)
73+
err = k8sClient.Get(ctx, types.NamespacedName{Name: subgroup2.Name, Namespace: ns}, createdGroup)
5174
g.Expect(err).ShouldNot(HaveOccurred())
5275
g.Expect(createdGroup.Status.Value).Should(Equal(helper.StatusOK))
5376
}).WithTimeout(time.Second * 20).WithPolling(time.Second).Should(Succeed())
5477

5578
By("Creating a KeycloakRealmGroup")
56-
group = &keycloakApi.KeycloakRealmGroup{
79+
group := &keycloakApi.KeycloakRealmGroup{
5780
ObjectMeta: metav1.ObjectMeta{
5881
Name: groupCR,
5982
Namespace: ns,
@@ -83,7 +106,7 @@ var _ = Describe("KeycloakRealmGroup controller", Ordered, func() {
83106
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: ns, Name: groupCR}, group)).Should(Succeed())
84107

85108
By("Updating a parent KeycloakRealmGroup")
86-
group.Spec.SubGroups = []string{}
109+
group.Spec.SubGroups = []string{"test-subgroup2"}
87110

88111
Expect(k8sClient.Update(ctx, group)).Should(Succeed())
89112
Eventually(func(g Gomega) {
@@ -93,7 +116,7 @@ var _ = Describe("KeycloakRealmGroup controller", Ordered, func() {
93116
g.Expect(updatedGroup.Status.Value).Should(Equal(helper.StatusOK))
94117
}, time.Minute, time.Second*5).Should(Succeed())
95118
})
96-
It("Should delete KeycloakRealmGroup", func() {
119+
It("Should delete KeycloakRealmGroup and subgroup", func() {
97120
By("Getting KeycloakRealmGroup")
98121
group := &keycloakApi.KeycloakRealmGroup{}
99122
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: ns, Name: groupCR}, group)).Should(Succeed())
@@ -105,6 +128,82 @@ var _ = Describe("KeycloakRealmGroup controller", Ordered, func() {
105128

106129
g.Expect(k8sErrors.IsNotFound(err)).Should(BeTrue())
107130
}, timeout, interval).Should(Succeed())
131+
132+
By("Getting KeycloakRealmGroup subgroup")
133+
subgroup := &keycloakApi.KeycloakRealmGroup{}
134+
Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: ns, Name: "test-subgroup2"}, subgroup)).Should(Succeed())
135+
By("Deleting KeycloakRealmGroup subgroup")
136+
Expect(k8sClient.Delete(ctx, subgroup)).Should(Succeed())
137+
Eventually(func(g Gomega) {
138+
deletedSubGroup := &keycloakApi.KeycloakRealmGroup{}
139+
err := k8sClient.Get(ctx, types.NamespacedName{Name: subgroup.Name, Namespace: ns}, deletedSubGroup)
140+
141+
g.Expect(k8sErrors.IsNotFound(err)).Should(BeTrue())
142+
}, timeout, interval).Should(Succeed())
143+
})
144+
It("Should delete KeycloakRealmGroup if subgroup is deleted", func() {
145+
By("Creating a subgroup")
146+
subgroup := &keycloakApi.KeycloakRealmGroup{
147+
ObjectMeta: metav1.ObjectMeta{
148+
Name: "test-subgroup-for-deletion",
149+
Namespace: ns,
150+
},
151+
Spec: keycloakApi.KeycloakRealmGroupSpec{
152+
Name: "test-subgroup-for-deletion",
153+
RealmRef: common.RealmRef{
154+
Kind: keycloakApi.KeycloakRealmKind,
155+
Name: KeycloakRealmCR,
156+
},
157+
},
158+
}
159+
Expect(k8sClient.Create(ctx, subgroup)).Should(Succeed())
160+
Eventually(func(g Gomega) {
161+
createdSubGroup := &keycloakApi.KeycloakRealmGroup{}
162+
err := k8sClient.Get(ctx, types.NamespacedName{Name: subgroup.Name, Namespace: ns}, createdSubGroup)
163+
g.Expect(err).ShouldNot(HaveOccurred())
164+
g.Expect(createdSubGroup.Status.Value).Should(Equal(helper.StatusOK))
165+
}).WithTimeout(time.Second * 20).WithPolling(time.Second).Should(Succeed())
166+
167+
By("Creating a group with subgroup")
168+
group := &keycloakApi.KeycloakRealmGroup{
169+
ObjectMeta: metav1.ObjectMeta{
170+
Name: "test-group-for-deletion",
171+
Namespace: ns,
172+
},
173+
Spec: keycloakApi.KeycloakRealmGroupSpec{
174+
Name: "test-group-for-deletion",
175+
RealmRef: common.RealmRef{
176+
Kind: keycloakApi.KeycloakRealmKind,
177+
Name: KeycloakRealmCR,
178+
},
179+
SubGroups: []string{"test-subgroup-for-deletion"},
180+
},
181+
}
182+
Expect(k8sClient.Create(ctx, group)).Should(Succeed())
183+
Eventually(func(g Gomega) {
184+
createdGroup := &keycloakApi.KeycloakRealmGroup{}
185+
err := k8sClient.Get(ctx, types.NamespacedName{Name: group.Name, Namespace: ns}, createdGroup)
186+
g.Expect(err).ShouldNot(HaveOccurred())
187+
g.Expect(createdGroup.Status.Value).Should(Equal(helper.StatusOK))
188+
}).WithTimeout(time.Second * 20).WithPolling(time.Second).Should(Succeed())
189+
190+
By("Deleting subgroup")
191+
Expect(k8sClient.Delete(ctx, subgroup)).Should(Succeed())
192+
Eventually(func(g Gomega) {
193+
deletedSubGroup := &keycloakApi.KeycloakRealmGroup{}
194+
err := k8sClient.Get(ctx, types.NamespacedName{Name: subgroup.Name, Namespace: ns}, deletedSubGroup)
195+
196+
g.Expect(k8sErrors.IsNotFound(err)).Should(BeTrue())
197+
}, timeout, interval).Should(Succeed())
198+
199+
By("Deleting group")
200+
Expect(k8sClient.Delete(ctx, group)).Should(Succeed())
201+
Eventually(func(g Gomega) {
202+
deletedGroup := &keycloakApi.KeycloakRealmGroup{}
203+
err := k8sClient.Get(ctx, types.NamespacedName{Name: group.Name, Namespace: ns}, deletedGroup)
204+
205+
g.Expect(k8sErrors.IsNotFound(err)).Should(BeTrue())
206+
}, timeout, interval).Should(Succeed())
108207
})
109208
It("Should preserve group with annotation", func() {
110209
By("Creating a KeycloakRealmGroup")

‎controllers/keycloakrealmgroup/terminator.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
ctrl "sigs.k8s.io/controller-runtime"
88

99
"github.com/epam/edp-keycloak-operator/pkg/client/keycloak"
10+
"github.com/epam/edp-keycloak-operator/pkg/client/keycloak/adapter"
1011
)
1112

1213
type terminator struct {
@@ -26,6 +27,12 @@ func (t *terminator) DeleteResource(ctx context.Context) error {
2627
log.Info("Start deleting group")
2728

2829
if err := t.kClient.DeleteGroup(ctx, t.realmName, t.groupName); err != nil {
30+
if adapter.IsErrNotFound(err) {
31+
log.Info("Group not found, skipping deletion")
32+
33+
return nil
34+
}
35+
2936
return fmt.Errorf("unable to delete group %w", err)
3037
}
3138

‎controllers/keycloakrealmuser/keycloakrealmuser_controller.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,25 @@ func (r *Reconcile) tryReconcile(ctx context.Context, instance *keycloakApi.Keyc
132132
return fmt.Errorf("unable to get keycloak realm from ref: %w", err)
133133
}
134134

135+
if instance.Spec.KeepResource {
136+
deleted, err := r.helper.TryToDelete(ctx, instance,
137+
makeTerminator(
138+
gocloak.PString(realm.Realm),
139+
instance.Spec.Username,
140+
kClient,
141+
objectmeta.PreserveResourcesOnDeletion(instance),
142+
),
143+
finalizer,
144+
)
145+
if err != nil {
146+
return fmt.Errorf("failed to delete keycloak realm user: %w", err)
147+
}
148+
149+
if deleted {
150+
return nil
151+
}
152+
}
153+
135154
password, getPasswordErr := r.getPassword(ctx, instance)
136155
if getPasswordErr != nil {
137156
return fmt.Errorf("unable to get password: %w", getPasswordErr)
@@ -153,19 +172,7 @@ func (r *Reconcile) tryReconcile(ctx context.Context, instance *keycloakApi.Keyc
153172
return errors.Wrap(err, "unable to sync realm user")
154173
}
155174

156-
if instance.Spec.KeepResource {
157-
if _, err := r.helper.TryToDelete(ctx, instance,
158-
makeTerminator(
159-
gocloak.PString(realm.Realm),
160-
instance.Spec.Username,
161-
kClient,
162-
objectmeta.PreserveResourcesOnDeletion(instance),
163-
),
164-
finalizer,
165-
); err != nil {
166-
return errors.Wrap(err, "unable to set finalizers")
167-
}
168-
} else {
175+
if !instance.Spec.KeepResource {
169176
if err := r.client.Delete(ctx, instance); err != nil {
170177
return errors.Wrap(err, "unable to delete instance of keycloak realm user")
171178
}

0 commit comments

Comments
 (0)