diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserPreconditionService.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserPreconditionService.java index 67ab8df13d9c06b5077fefbd6437c705c912c9bb..19b753164060e5e298865eeb66b064043b41af04 100644 --- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserPreconditionService.java +++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserPreconditionService.java @@ -38,20 +38,20 @@ class KeycloakUserPreconditionService { @Autowired private KeycloakGenericRemoteService keycloakGenericRemoteService; - Optional<String> getReconcilePreconditionErrors(OzgKeycloakUser user) { + public Optional<String> getReconcilePreconditionErrors(OzgKeycloakUser user) { - String namespace = user.getMetadata().getNamespace(); + var namespace = user.getMetadata().getNamespace(); if (!keycloakGenericRemoteService.realmExists(namespace)) { - return Optional.of("Realm " + namespace + " does not yet exist"); + return Optional.of(String.format("Realm %s does not yet exist", namespace)); } - Optional<String> clientErorr = clientsExists(user, namespace); + var clientErorr = clientsExists(user, namespace); if (clientErorr.isPresent()) { return clientErorr; } - Optional<String> groupError = groupsExists(user, namespace); + var groupError = groupsExists(user, namespace); if (groupError.isPresent()) { return groupError; } @@ -64,7 +64,7 @@ class KeycloakUserPreconditionService { .map(KeycloakUserSpecClientRole::getClientId) .map(clientId -> keycloakGenericRemoteService.getByClientId(clientId, realm)) .filter(Optional::isEmpty) - .map(clientId -> "Client " + clientId + " does not yet exist") + .map(clientId -> String.format("Client %s does not yet exist", clientId)) .findAny(); } @@ -72,7 +72,7 @@ class KeycloakUserPreconditionService { return user.getSpec().getKeycloakUser().getGroups().stream() .map(KeycloakUserSpecUserGroup::getName) .filter(groupName -> !keycloakGenericRemoteService.groupExists(groupName, realm)) - .map(groupName -> "Group " + groupName + " for realm " + realm + " does not yet exist") + .map(groupName -> String.format("Group %s for realm %s does not exist yet", groupName, realm)) .findAny(); } } diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconciler.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconciler.java index 9a80f94338d9317989b79e3c3760b07e184c230b..182ffa06b64bb26a8534a349e9b0387de46c7711 100644 --- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconciler.java +++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconciler.java @@ -24,7 +24,6 @@ package de.ozgcloud.operator.keycloak.user; import java.time.Duration; -import java.util.Optional; import java.util.logging.Level; import org.springframework.beans.factory.annotation.Autowired; @@ -40,9 +39,9 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import lombok.extern.java.Log; +@Log @ControllerConfiguration @Component -@Log public class KeycloakUserReconciler implements Reconciler<OzgKeycloakUser>, Cleaner<OzgKeycloakUser> { @Autowired @@ -53,54 +52,67 @@ public class KeycloakUserReconciler implements Reconciler<OzgKeycloakUser>, Clea @Override public UpdateControl<OzgKeycloakUser> reconcile(OzgKeycloakUser resource, Context<OzgKeycloakUser> context) { - try { - log.info("Reconciling KeycloakUser " + resource.getMetadata().getName()); - String namespace = resource.getMetadata().getNamespace(); + log.info(String.format("Reconciling user %s...", resource.getMetadata().getName())); - Optional<String> preconditionError = preconditionService.getReconcilePreconditionErrors(resource); + var preconditionError = preconditionService.getReconcilePreconditionErrors(resource); if (preconditionError.isPresent()) { return buildStatusInProgress(resource, preconditionError.get()); } - keycloakUserService.createOrUpdateUser(resource.getSpec(), namespace); + keycloakUserService.createOrUpdateUser(resource.getSpec(), resource.getMetadata().getNamespace()); - return buildStatusOk(resource); + resource.setStatus(buildOzgKeycloakUserOkStatus()); + return UpdateControl.updateStatus(resource); } catch (Exception e) { - log.log(Level.SEVERE, - "Could not reconcile user " + resource.getMetadata().getName() + " in namespace " + resource.getMetadata().getNamespace() + ": " - + e.getMessage(), - e); - resource.setStatus(OzgKeycloakUserStatus.builder().status(OzgCustomResourceStatus.ERROR).message(e.getMessage()).build()); - return UpdateControl.updateStatus(resource).rescheduleAfter(Duration.ofSeconds(Config.RECONCILER_RETRY_SECONDS)); + log.log(Level.SEVERE, String.format("Could not reconcile user %s for namespace %s: %s", resource.getMetadata().getName(), + resource.getMetadata().getNamespace(), e.getMessage()), e); + + resource.setStatus(buildOzgKeycloakUserErrorStatus(e.getMessage())); + return createRescheduleUpdateControl(resource); } } - private UpdateControl<OzgKeycloakUser> buildStatusOk(OzgKeycloakUser resource) { - resource.setStatus(OzgKeycloakUserStatus.builder().status(OzgCustomResourceStatus.OK).message(null).build()); - return UpdateControl.updateStatus(resource); + private OzgKeycloakUserStatus buildOzgKeycloakUserOkStatus() { + return buildOzgKeycloakUserStatus(OzgCustomResourceStatus.OK, null); + } + + private OzgKeycloakUserStatus buildOzgKeycloakUserErrorStatus(String message) { + return buildOzgKeycloakUserStatus(OzgCustomResourceStatus.ERROR, message); } private UpdateControl<OzgKeycloakUser> buildStatusInProgress(OzgKeycloakUser resource, String errorMessage) { - log.log(Level.INFO, - "Could not yet reconcile user " + resource.getMetadata().getName() + " in namespace " + resource.getMetadata().getNamespace() + ": " - + errorMessage); - resource.setStatus(OzgKeycloakUserStatus.builder().status(OzgCustomResourceStatus.IN_PROGRESS).message(errorMessage).build()); - return UpdateControl.updateStatus(resource).rescheduleAfter(Duration.ofSeconds(Config.RECONCILER_RETRY_SECONDS)); + log.log(Level.WARNING, String.format("Could not reconcile user %s in namespace %s: %s", resource.getMetadata().getName(), + resource.getMetadata().getNamespace(), errorMessage)); + + resource.setStatus(buildOzgKeycloakUserInProgressStatus(errorMessage)); + return createRescheduleUpdateControl(resource); } - @Override - public DeleteControl cleanup(OzgKeycloakUser crd, Context<OzgKeycloakUser> context) { + private OzgKeycloakUserStatus buildOzgKeycloakUserInProgressStatus(String message) { + return buildOzgKeycloakUserStatus(OzgCustomResourceStatus.IN_PROGRESS, message); + } + + private OzgKeycloakUserStatus buildOzgKeycloakUserStatus(OzgCustomResourceStatus status, String message) { + return OzgKeycloakUserStatus.builder().status(status).message(message).build(); + } + private UpdateControl<OzgKeycloakUser> createRescheduleUpdateControl(OzgKeycloakUser userResource) { + return UpdateControl.updateStatus(userResource).rescheduleAfter(Duration.ofSeconds(Config.RECONCILER_RETRY_SECONDS)); + } + + @Override + public DeleteControl cleanup(OzgKeycloakUser userResource, Context<OzgKeycloakUser> context) { try { - log.info("Deleting KeycloakUser " + crd.getMetadata().getName()); + log.info(String.format("Deleting KeycloakUser %s", userResource.getMetadata().getName())); - keycloakUserService.deleteUser(crd.getSpec(), crd.getMetadata().getNamespace()); + keycloakUserService.deleteUser(userResource.getSpec(), userResource.getMetadata().getNamespace()); return DeleteControl.defaultDelete(); } catch (Exception e) { - log.log(Level.SEVERE, "Could not delete user " + crd.getMetadata().getName() + " in namespace " + crd.getMetadata().getNamespace(), e); + log.log(Level.SEVERE, String.format("Could not delete user %s in namespace %s", userResource.getMetadata().getName(), + userResource.getMetadata().getNamespace()), e); return DeleteControl.defaultDelete(); } } diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java index a4cff8d15c97320cc72707c80deb9e310dd438e2..4e1d2f02fbfb611f550e9682ef5b0dedd7224d66 100644 --- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java +++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java @@ -26,8 +26,6 @@ package de.ozgcloud.operator.keycloak.user; import java.util.Arrays; import java.util.Optional; -import javax.ws.rs.core.Response; - import org.keycloak.admin.client.CreatedResponseUtil; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.RealmResource; @@ -50,30 +48,46 @@ class KeycloakUserRemoteService { @Autowired private KeycloakGenericRemoteService keycloakGenericRemoteService; - void createUser(UserRepresentation user, String realm) { - - RealmResource realmResource = keycloak.realm(realm); + public void createUser(UserRepresentation user, String namespace) { + var realmResource = getRealm(namespace); - Response response = realmResource.users().create(user); + var response = realmResource.users().create(user); KeycloakResultParser.parseCreatedResponse(response); - String userId = CreatedResponseUtil.getCreatedId(response); + var userId = CreatedResponseUtil.getCreatedId(response); + + addClientRoles(userId, realmResource, namespace, user); + } + + public Optional<UserRepresentation> getUserByName(String userName, String namespace) { + return getRealm(namespace).users().search(userName, true).stream().findFirst(); + } - addClientRoles(userId, realmResource, realm, user); + public void deleteUser(String userId, String namespace) { + getRealm(namespace).users().delete(userId); } - void addClientRoles(String userId, RealmResource realmResource, String realm, UserRepresentation user) { + public void updateUser(UserRepresentation user, String namespace) { + var realmResource = getRealm(namespace); + + realmResource.users().get(user.getId()).update(user); + + addClientRoles(user.getId(), realmResource, namespace, user); + } - user.getClientRoles().keySet() - .forEach(clientId -> { - ClientRepresentation appClient = getRealmClient(realmResource, clientId); - user.getClientRoles().get(clientId).stream() - .map(clientRoleName -> keycloakGenericRemoteService.getClientRole(clientRoleName, appClient.getId(), realm) - .orElseThrow(() -> new KeycloakException( - "Role " + clientRoleName + " not found for client with clientId " + clientId + " in realm " + realm))) - .forEach(clientRole -> addClientRoleToUser(clientRole, realmResource, userId, appClient)); - }); + private RealmResource getRealm(String namespace) { + return keycloak.realm(namespace); + } + void addClientRoles(String userId, RealmResource realmResource, String namespace, UserRepresentation user) { + user.getClientRoles().keySet().forEach(clientId -> { + var realmClient = getRealmClient(realmResource, clientId); + user.getClientRoles().get(clientId).stream() + .map(clientRoleName -> keycloakGenericRemoteService.getClientRole(clientRoleName, realmClient.getId(), namespace) + .orElseThrow(() -> new KeycloakException( + "Role " + clientRoleName + " not found for client with clientId " + clientId + " in realm " + namespace))) + .forEach(clientRole -> addClientRoleToUser(clientRole, realmResource, userId, realmClient)); + }); } ClientRepresentation getRealmClient(RealmResource realmResource, String clientId) { @@ -85,16 +99,4 @@ class KeycloakUserRemoteService { void addClientRoleToUser(RoleRepresentation clientRole, RealmResource realmResource, String userId, ClientRepresentation appClient) { realmResource.users().get(userId).roles().clientLevel(appClient.getId()).add(Arrays.asList(clientRole)); } - - Optional<UserRepresentation> getUserByName(String username, String realm) { - return keycloak.realm(realm).users().search(username, true).stream().findFirst(); - } - - void deleteUser(String userId, String realm) { - keycloak.realm(realm).users().delete(userId); - } - - void updateUser(UserRepresentation user, String realm) { - keycloak.realm(realm).users().get(user.getId()).update(user); - } -} +} \ No newline at end of file diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserService.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserService.java index 6760f4cfbd025fb86387d39a21efdf42b5312072..1f33da68e3371d2b7d6265572796ff2bdad4fa96 100644 --- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserService.java +++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserService.java @@ -37,13 +37,13 @@ class KeycloakUserService { @Autowired private KeycloakUserMapper userMapper; - void createOrUpdateUser(OzgKeycloakUserSpec userSpec, String namespace) { + public void createOrUpdateUser(OzgKeycloakUserSpec userSpec, String namespace) { remoteService.getUserByName(userSpec.getKeycloakUser().getUsername(), namespace) .ifPresentOrElse(existingUser -> remoteService.updateUser(userMapper.update(existingUser, userSpec), namespace), () -> remoteService.createUser(userMapper.map(userSpec), namespace)); } - void deleteUser(OzgKeycloakUserSpec userSpec, String namespace) { + public void deleteUser(OzgKeycloakUserSpec userSpec, String namespace) { Optional.of(userSpec) .map(userMapper::map) .flatMap(keycloakUser -> remoteService.getUserByName(keycloakUser.getUsername(), namespace)) diff --git a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java index a3e3997eaa6c761be35587d5873adda9fa9abd20..4f3c6c2fd5a6afe15abcb331d5bf6a2124201cf1 100644 --- a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java +++ b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java @@ -35,16 +35,14 @@ import java.util.Optional; import javax.ws.rs.core.Response; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.keycloak.admin.client.Keycloak; -import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.RoleMappingResource; -import org.keycloak.admin.client.resource.RoleResource; import org.keycloak.admin.client.resource.RoleScopeResource; -import org.keycloak.admin.client.resource.RolesResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.admin.client.resource.UsersResource; import org.keycloak.representations.idm.ClientRepresentation; @@ -84,16 +82,10 @@ class KeycloakUserRemoteServiceTest { @Mock private ClientsResource clientsResource; @Mock - private ClientResource clientResource; - @Mock private ClientRepresentation clientRepresentation; @Mock private RoleRepresentation roleRepresentation; @Mock - private RolesResource rolesResource; - @Mock - private RoleResource roleResource; - @Mock private RoleScopeResource roleScopeResource; @Mock private RoleMappingResource roleMappingResource; @@ -113,8 +105,6 @@ class KeycloakUserRemoteServiceTest { @Test void shouldThrowOnResponseError() { - - when(response.getStatusInfo()).thenReturn(Response.Status.INTERNAL_SERVER_ERROR); assertThrows(KeycloakException.class, () -> userRemoteService.createUser(USER_REPRESENTATION, REALM)); @@ -123,7 +113,7 @@ class KeycloakUserRemoteServiceTest { @Test void shouldCallAddClientRoles() { when(response.getStatusInfo()).thenReturn(Response.Status.CREATED); - doNothing().when(userRemoteService).addClientRoles(any(), any(),eq(REALM), any()); + doNothing().when(userRemoteService).addClientRoles(any(), any(), eq(REALM), any()); userRemoteService.createUser(USER_REPRESENTATION, REALM); @@ -209,6 +199,7 @@ class KeycloakUserRemoteServiceTest { } } + @DisplayName("Update user") @Nested class TestUpdateUser { @@ -218,7 +209,9 @@ class KeycloakUserRemoteServiceTest { void init() { when(keycloak.realm(REALM)).thenReturn(realmResource); when(realmResource.users()).thenReturn(usersResource); - when(usersResource.get(USERID)).thenReturn(userResource); + when(usersResource.get(any())).thenReturn(userResource); + + doNothing().when(userRemoteService).addClientRoles(any(), any(), eq(REALM), any()); } @Test @@ -230,6 +223,13 @@ class KeycloakUserRemoteServiceTest { verify(userResource).update(user); } + + @Test + void shouldCallAddClientRoles() { + userRemoteService.updateUser(USER_REPRESENTATION, REALM); + + verify(userRemoteService).addClientRoles(isNull(), eq(realmResource), eq(REALM), eq(USER_REPRESENTATION)); + } } @Nested