Skip to content

Commit 04d032d

Browse files
aaronschweigaaronschweig
authored
fix: enhance HelmRelease handling in realm subroutine and tests (#144)
* fix: enhance HelmRelease handling in realm subroutine and tests * chore: change the implementation to make the invitation controller more idempotent * fix: update GOLANGCI_LINT_VERSION to v2.5.0 and improve error handling in invite subroutine
1 parent 2d017f9 commit 04d032d

File tree

4 files changed

+164
-4
lines changed

4 files changed

+164
-4
lines changed

‎Taskfile.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ vars:
88
CRD_DIRECTORY: config/crd/bases
99
KCP_APIGEN_VERSION: v0.21.0
1010
KCP_VERSION: 0.26.1
11-
GOLANGCI_LINT_VERSION: v2.4.0
11+
GOLANGCI_LINT_VERSION: v2.5.0
1212
GOARCH:
1313
sh: go env GOARCH
1414
GOOS:

‎internal/subroutine/authorization_model_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func TestAuthorizationModelProcess(t *testing.T) {
220220
Contents: extensionModel,
221221
},
222222
{
223-
Name: "internal_core_types_namespaces.fga",
223+
Name: "internal_core_types_namespaces.fga",
224224
Contents: `module namespaces
225225
226226
extend type core_platform-mesh_io_account

‎internal/subroutine/invite/subroutine.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ type keycloakUser struct {
4343
Enabled bool `json:"enabled,omitempty"`
4444
}
4545

46+
type keycloakClient struct {
47+
ID string `json:"id,omitempty"`
48+
ClientID string `json:"clientId,omitempty"`
49+
}
50+
4651
func New(ctx context.Context, cfg *config.Config, mgr mcmanager.Manager, pwd string) (*subroutine, error) {
4752

4853
issuer := fmt.Sprintf("%s/realms/master", cfg.Invite.KeycloakBaseURL)
@@ -181,6 +186,33 @@ func (s *subroutine) Process(ctx context.Context, instance runtimeobject.Runtime
181186

182187
log.Debug().Str("email", invite.Spec.Email).Str("id", newUser.ID).Msg("User created")
183188

189+
clientQueryParams := url.Values{
190+
"clientId": {realm},
191+
}
192+
193+
res, err = s.keycloak.Get(fmt.Sprintf("%s/admin/realms/%s/clients?%s", s.keycloakBaseURL, realm, clientQueryParams.Encode()))
194+
if err != nil {
195+
log.Err(err).Msg("Failed to verify client exists")
196+
return ctrl.Result{}, errors.NewOperatorError(err, true, false)
197+
}
198+
defer res.Body.Close() //nolint:errcheck
199+
200+
if res.StatusCode != http.StatusOK {
201+
return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to verify client exists: %s", res.Status), true, false)
202+
}
203+
204+
var clients []keycloakClient
205+
if err = json.NewDecoder(res.Body).Decode(&clients); err != nil {
206+
return ctrl.Result{}, errors.NewOperatorError(err, true, false)
207+
}
208+
209+
if len(clients) == 0 {
210+
log.Info().Str("clientId", realm).Msg("Client does not exist yet, requeuing")
211+
return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("client %s does not exist yet", realm), true, false)
212+
}
213+
214+
log.Debug().Str("clientId", realm).Msg("Client verified")
215+
184216
queryParams := url.Values{
185217
"redirect_uri": {fmt.Sprintf("https://%s.%s/", realm, s.baseDomain)},
186218
"client_id": {realm},

‎internal/subroutine/invite/subroutine_test.go‎

Lines changed: 130 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,15 @@ func TestSubroutineProcess(t *testing.T) {
9898
user["id"] = "1234"
9999
users = append(users, user)
100100
})
101+
mux.HandleFunc("GET /admin/realms/acme/clients", func(w http.ResponseWriter, r *http.Request) {
102+
w.Header().Set("Content-Type", "application/json")
103+
w.WriteHeader(http.StatusOK)
104+
clients := []map[string]any{
105+
{"id": "client-uuid", "clientId": "acme"},
106+
}
107+
err := json.NewEncoder(w).Encode(&clients)
108+
assert.NoError(t, err)
109+
})
101110
mux.HandleFunc("PUT /admin/realms/acme/users/{id}/execute-actions-email", func(w http.ResponseWriter, r *http.Request) {
102111
w.Header().Set("Content-Type", "application/json")
103112
w.WriteHeader(http.StatusNoContent)
@@ -186,11 +195,130 @@ func TestSubroutineProcess(t *testing.T) {
186195
},
187196
expectErr: true,
188197
setupK8sMocks: func(m *mocks.MockClient) {
189-
// Simulate k8s Get failure (AccountInfo missing)
190198
m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(fmt.Errorf("accountinfo not found"))
191199
},
192200
setupKeycloakMocks: func(mux *http.ServeMux) {
193-
// No Keycloak calls expected when AccountInfo fetch fails.
201+
},
202+
},
203+
{
204+
desc: "error verifying client (500 from Keycloak)",
205+
obj: &v1alpha1.Invite{
206+
Spec: v1alpha1.InviteSpec{
207+
Email: "[email protected]",
208+
},
209+
},
210+
expectErr: true,
211+
setupK8sMocks: func(m *mocks.MockClient) {
212+
m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
213+
func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error {
214+
accountInfo := &accountsv1alpha1.AccountInfo{
215+
Spec: accountsv1alpha1.AccountInfoSpec{
216+
Organization: accountsv1alpha1.AccountLocation{
217+
Name: "acme",
218+
},
219+
},
220+
}
221+
*o.(*accountsv1alpha1.AccountInfo) = *accountInfo
222+
return nil
223+
},
224+
)
225+
},
226+
setupKeycloakMocks: func(mux *http.ServeMux) {
227+
users := []map[string]any{}
228+
mux.HandleFunc("GET /admin/realms/acme/users", func(w http.ResponseWriter, r *http.Request) {
229+
w.Header().Set("Content-Type", "application/json")
230+
w.WriteHeader(http.StatusOK)
231+
err := json.NewEncoder(w).Encode(&users)
232+
assert.NoError(t, err)
233+
})
234+
mux.HandleFunc("POST /admin/realms/acme/users", func(w http.ResponseWriter, r *http.Request) {
235+
w.Header().Set("Content-Type", "application/json")
236+
w.WriteHeader(http.StatusCreated)
237+
user := map[string]any{}
238+
err := json.NewDecoder(r.Body).Decode(&user)
239+
assert.NoError(t, err)
240+
user["id"] = "1234"
241+
users = append(users, user)
242+
})
243+
mux.HandleFunc("GET /admin/realms/acme/clients", func(w http.ResponseWriter, r *http.Request) {
244+
w.WriteHeader(http.StatusInternalServerError)
245+
_, _ = w.Write([]byte(`{"error":"boom"}`))
246+
})
247+
},
248+
},
249+
{
250+
desc: "client does not exist yet, should requeue",
251+
obj: &v1alpha1.Invite{
252+
Spec: v1alpha1.InviteSpec{
253+
Email: "[email protected]",
254+
},
255+
},
256+
expectErr: true,
257+
setupK8sMocks: func(m *mocks.MockClient) {
258+
m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
259+
func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error {
260+
accountInfo := &accountsv1alpha1.AccountInfo{
261+
Spec: accountsv1alpha1.AccountInfoSpec{
262+
Organization: accountsv1alpha1.AccountLocation{
263+
Name: "acme",
264+
},
265+
},
266+
}
267+
*o.(*accountsv1alpha1.AccountInfo) = *accountInfo
268+
return nil
269+
},
270+
)
271+
},
272+
setupKeycloakMocks: func(mux *http.ServeMux) {
273+
users := []map[string]any{}
274+
mux.HandleFunc("GET /admin/realms/acme/users", func(w http.ResponseWriter, r *http.Request) {
275+
w.Header().Set("Content-Type", "application/json")
276+
w.WriteHeader(http.StatusOK)
277+
err := json.NewEncoder(w).Encode(&users)
278+
assert.NoError(t, err)
279+
})
280+
mux.HandleFunc("POST /admin/realms/acme/users", func(w http.ResponseWriter, r *http.Request) {
281+
w.Header().Set("Content-Type", "application/json")
282+
w.WriteHeader(http.StatusCreated)
283+
user := map[string]any{}
284+
err := json.NewDecoder(r.Body).Decode(&user)
285+
assert.NoError(t, err)
286+
user["id"] = "1234"
287+
users = append(users, user)
288+
})
289+
mux.HandleFunc("GET /admin/realms/acme/clients", func(w http.ResponseWriter, r *http.Request) {
290+
w.Header().Set("Content-Type", "application/json")
291+
w.WriteHeader(http.StatusOK)
292+
clients := []map[string]any{}
293+
err := json.NewEncoder(w).Encode(&clients)
294+
assert.NoError(t, err)
295+
})
296+
},
297+
},
298+
{
299+
desc: "organization name is empty in AccountInfo",
300+
obj: &v1alpha1.Invite{
301+
Spec: v1alpha1.InviteSpec{
302+
Email: "[email protected]",
303+
},
304+
},
305+
expectErr: true,
306+
setupK8sMocks: func(m *mocks.MockClient) {
307+
m.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
308+
func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error {
309+
accountInfo := &accountsv1alpha1.AccountInfo{
310+
Spec: accountsv1alpha1.AccountInfoSpec{
311+
Organization: accountsv1alpha1.AccountLocation{
312+
Name: "",
313+
},
314+
},
315+
}
316+
*o.(*accountsv1alpha1.AccountInfo) = *accountInfo
317+
return nil
318+
},
319+
)
320+
},
321+
setupKeycloakMocks: func(mux *http.ServeMux) {
194322
},
195323
},
196324
}

0 commit comments

Comments
 (0)