From f8c8fcf32a6f079f3bd2d75d4dbe1b630a8ae97f Mon Sep 17 00:00:00 2001
From: OZGCloud <ozgcloud@mgm-tp.com>
Date: Tue, 8 Aug 2023 19:26:54 +0200
Subject: [PATCH] OZG-3961 code improvements
---
.../operator/keycloak/KeycloakException.java | 4 +-
.../client/KeycloakClientRemoteService.java | 28 +++---
.../realm/KeycloakRealmRemoteService.java | 1 -
.../user/KeycloakUserRemoteService.java | 49 ++++-------
.../user/KubernetesRemoteService.java | 30 +++++++
.../user/KeycloakUserRemoteServiceTest.java | 82 +++++++----------
.../user/KubernetesRemoteServiceTest.java | 87 +++++++++++++++++++
7 files changed, 181 insertions(+), 100 deletions(-)
create mode 100644 src/main/java/de/ozgcloud/operator/keycloak/user/KubernetesRemoteService.java
create mode 100644 src/test/java/de/ozgcloud/operator/keycloak/user/KubernetesRemoteServiceTest.java
diff --git a/src/main/java/de/ozgcloud/operator/keycloak/KeycloakException.java b/src/main/java/de/ozgcloud/operator/keycloak/KeycloakException.java
index 4cc82bd..ac7de16 100644
--- a/src/main/java/de/ozgcloud/operator/keycloak/KeycloakException.java
+++ b/src/main/java/de/ozgcloud/operator/keycloak/KeycloakException.java
@@ -26,7 +26,7 @@ package de.ozgcloud.operator.keycloak;
@SuppressWarnings("serial")
public class KeycloakException extends RuntimeException {
- public KeycloakException(String string) {
- super(string);
+ public KeycloakException(String message) {
+ super(message);
}
}
diff --git a/src/main/java/de/ozgcloud/operator/keycloak/client/KeycloakClientRemoteService.java b/src/main/java/de/ozgcloud/operator/keycloak/client/KeycloakClientRemoteService.java
index 825cec3..7c3d4c1 100644
--- a/src/main/java/de/ozgcloud/operator/keycloak/client/KeycloakClientRemoteService.java
+++ b/src/main/java/de/ozgcloud/operator/keycloak/client/KeycloakClientRemoteService.java
@@ -25,10 +25,10 @@ package de.ozgcloud.operator.keycloak.client;
import java.util.logging.Level;
-import javax.ws.rs.core.Response;
-
import org.keycloak.admin.client.CreatedResponseUtil;
import org.keycloak.admin.client.Keycloak;
+import org.keycloak.admin.client.resource.ClientResource;
+import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
import org.springframework.beans.factory.annotation.Autowired;
@@ -44,22 +44,30 @@ class KeycloakClientRemoteService {
@Autowired
private Keycloak keycloak;
- void updateClient(ClientRepresentation client, String realm) {
- keycloak.realm(realm).clients().get(client.getId()).update(client);
+ public void updateClient(ClientRepresentation client, String realm) {
+ getClientResource(realm, client.getId()).update(client);
}
- String createClient(ClientRepresentation client, String realm) {
+ public String createClient(ClientRepresentation client, String realm) {
log.log(Level.FINE, "Creating client {0} in realm {1}", new String[] { client.getId(), realm });
- Response response = keycloak.realm(realm).clients().create(client);
+ var response = getRealm(realm).clients().create(client);
KeycloakResultParser.parseCreatedResponse(response);
return CreatedResponseUtil.getCreatedId(response);
}
- void updateClientRole(RoleRepresentation role, String clientId, String realm) {
- keycloak.realm(realm).clients().get(clientId).roles().get(role.getName()).update(role);
+ public void updateClientRole(RoleRepresentation role, String clientId, String realm) {
+ getClientResource(realm, clientId).roles().get(role.getName()).update(role);
+ }
+
+ public void addClientRole(RoleRepresentation role, String clientId, String realm) {
+ getClientResource(realm, clientId).roles().create(role);
+ }
+
+ private ClientResource getClientResource(String realm, String clientId) {
+ return getRealm(realm).clients().get(clientId);
}
- void addClientRole(RoleRepresentation role, String clientId, String realm) {
- keycloak.realm(realm).clients().get(clientId).roles().create(role);
+ private RealmResource getRealm(String realm) {
+ return keycloak.realm(realm);
}
}
diff --git a/src/main/java/de/ozgcloud/operator/keycloak/realm/KeycloakRealmRemoteService.java b/src/main/java/de/ozgcloud/operator/keycloak/realm/KeycloakRealmRemoteService.java
index 656e88f..97801f7 100644
--- a/src/main/java/de/ozgcloud/operator/keycloak/realm/KeycloakRealmRemoteService.java
+++ b/src/main/java/de/ozgcloud/operator/keycloak/realm/KeycloakRealmRemoteService.java
@@ -35,7 +35,6 @@ class KeycloakRealmRemoteService {
Keycloak keycloak;
void createRealm(RealmRepresentation realm) {
-
keycloak.realms().create(realm);
}
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 64919b2..ab4aaf6 100644
--- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java
+++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java
@@ -47,9 +47,7 @@ import de.ozgcloud.operator.keycloak.user.OzgKeycloakUserSpec.KeycloakUserSpecUs
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.SecretBuilder;
-import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.dsl.Resource;
-import io.fabric8.kubernetes.client.extension.ResourceAdapter;
import lombok.extern.java.Log;
@Log
@@ -63,7 +61,7 @@ class KeycloakUserRemoteService {
@Autowired
private Keycloak keycloak;
@Autowired
- private KubernetesClient kubernetesClient;
+ private KubernetesRemoteService kubernetesRemoteService;
@Autowired
private KeycloakGenericRemoteService keycloakGenericRemoteService;
@@ -120,27 +118,18 @@ class KeycloakUserRemoteService {
realmResource.users().get(userId).roles().clientLevel(appClient.getId()).add(Arrays.asList(clientRole));
}
- public boolean existSecret(OzgKeycloakUserSpec userSpec, String createdNamespace) {
- log.log(Level.INFO, "existSecret...");
- var namespace = "keycloak";// TODO durch den namespace ersetzen, wenn die Helm Charts passen
- log.log(Level.INFO, "for user: " + userSpec.getKeycloakUser().getUsername() + " and namespace: " + namespace);
- var secret = getUserSecret(userSpec, namespace);
-
- log.log(Level.INFO, "Found Secret: " + secret.get());
- return Objects.nonNull(secret.get());
+ public boolean existSecret(OzgKeycloakUserSpec userSpec, String namespace) {
+ var namespaceTemp = "keycloak";// TODO durch den namespace ersetzen, wenn die Helm Charts passen
+ return Objects.nonNull(getUserSecret(userSpec, namespaceTemp).get());
}
- public void createSecret(OzgKeycloakUserSpec userSpec, String createdNamespace) {
- log.log(Level.INFO, "Create secret for user: " + userSpec.getKeycloakUser().getUsername());
- var namespace = "keycloak";// TODO durch den namespace ersetzen, wenn die Helm Charts passen
-
- var credentialsSecret = createUserSecret(userSpec.getKeycloakUser(), namespace);
+ public void createSecret(OzgKeycloakUserSpec userSpec, String namespace) {
+ var namespaceTemp = "keycloak";// TODO durch den namespace ersetzen, wenn die Helm Charts passen
-// kubernetesClient.secrets().inNamespace(namespace).create(credentialsSecret);
- var resourceAdapter = new ResourceAdapter<Secret>();
- resourceAdapter.create(credentialsSecret);
+ log.log(Level.INFO, "Create secret for user: " + userSpec.getKeycloakUser().getUsername());
+ var credentialsSecret = createUserSecret(userSpec.getKeycloakUser(), namespaceTemp);
- log.log(Level.INFO, "Secret successful created in namespace: " + namespace);
+ kubernetesRemoteService.createSecret(namespaceTemp, credentialsSecret);
}
Secret createUserSecret(KeycloakUserSpecUser userSpec, String namespace) {
@@ -173,31 +162,23 @@ class KeycloakUserRemoteService {
return upperCaseCharacter + randomString;
}
- public String getPasswordFromSecret(OzgKeycloakUserSpec userSpec, String createdNamespace) {
- var namespace = "keycloak";// TODO durch den namespace ersetzen, wenn die Helm Charts passen
-
- var secret = getUserSecret(userSpec, namespace);
- return getPasswordFromSecret(secret);
+ public String getPasswordFromSecret(OzgKeycloakUserSpec userSpec, String namespace) {
+ var namespaceTemp = "keycloak";// TODO durch den namespace ersetzen, wenn die Helm Charts passen
+ return getPasswordFromSecret(getUserSecret(userSpec, namespaceTemp));
}
- public Resource<Secret> getUserSecret(OzgKeycloakUserSpec userSpec, String createdNamespace) {
- var namespace = "keycloak";// TODO durch den namespace ersetzen, wenn die Helm Charts passen
+ public Resource<Secret> getUserSecret(OzgKeycloakUserSpec userSpec, String namespace) {
+ var namespaceTemp = "keycloak";// TODO durch den namespace ersetzen, wenn die Helm Charts passen
var secretName = buildCredentialSecretName(userSpec.getKeycloakUser());
log.log(Level.INFO, "Get User secret: " + secretName);
- return getUserSecret(secretName, namespace);
+ return kubernetesRemoteService.getSecret(namespaceTemp, secretName);
}
private String buildCredentialSecretName(KeycloakUserSpecUser userSpec) {
return userSpec.getUsername().toLowerCase() + "-credentials";
}
- private Resource<Secret> getUserSecret(String secretName, String namespace) {
- log.log(Level.INFO, "Get secret by name: " + secretName);
- log.log(Level.INFO, "Get secret in namespace: " + namespace);
- return kubernetesClient.secrets().inNamespace(namespace).withName(secretName);
- }
-
private String getPasswordFromSecret(Resource<Secret> secret) {
return decodeBase64(secret.get().getData().get(SECRET_PASSWORD_FIELD));
}
diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KubernetesRemoteService.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KubernetesRemoteService.java
new file mode 100644
index 0000000..16809d1
--- /dev/null
+++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KubernetesRemoteService.java
@@ -0,0 +1,30 @@
+package de.ozgcloud.operator.keycloak.user;
+
+import java.util.logging.Level;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import io.fabric8.kubernetes.api.model.Secret;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.dsl.Resource;
+import lombok.extern.java.Log;
+
+@Log
+@Component
+class KubernetesRemoteService {
+
+ @Autowired
+ private KubernetesClient kubernetesClient;
+
+ public Resource<Secret> getSecret(String namespace, String name) {
+ log.log(Level.INFO, "Get " + name + "secret from " + namespace + " namespace.");
+ return kubernetesClient.secrets().inNamespace(namespace).withName(name);
+ }
+
+ public void createSecret(String namespace, Secret secret) {
+ log.log(Level.INFO, "Create " + secret.getFullResourceName() + " secret in " + namespace + " namespace.");
+ kubernetesClient.secrets().inNamespace(namespace).create(secret);
+ log.log(Level.INFO, "Secret successful created.");
+ }
+}
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 202bf88..dec33eb 100644
--- a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java
+++ b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java
@@ -60,9 +60,6 @@ import de.ozgcloud.operator.keycloak.KeycloakGenericRemoteService;
import de.ozgcloud.operator.keycloak.user.OzgKeycloakUserSpec.KeycloakUserSpecUser;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.SecretBuilder;
-import io.fabric8.kubernetes.api.model.SecretList;
-import io.fabric8.kubernetes.client.KubernetesClient;
-import io.fabric8.kubernetes.client.dsl.MixedOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
class KeycloakUserRemoteServiceTest {
@@ -72,6 +69,8 @@ class KeycloakUserRemoteServiceTest {
private final static String USERID = "UserId";
private final static String CLIENT_ID = "ClientId";
+ private final static String NAMESPACE = "dummyNamespace";
+
@Spy
@InjectMocks
private KeycloakUserRemoteService userRemoteService;
@@ -102,7 +101,7 @@ class KeycloakUserRemoteServiceTest {
@Mock
private UserRepresentation userRepresentation;
@Mock
- private KubernetesClient kubernetesClient;
+ private KubernetesRemoteService kubernetesRemoteService;
@Nested
class TestCreateUser {
@@ -302,16 +301,16 @@ class KeycloakUserRemoteServiceTest {
void shouldGetUserSecret() {
when(resourceMock.get()).thenReturn(new Secret());
- userRemoteService.existSecret(userSpec, REALM);
+ userRemoteService.existSecret(userSpec, NAMESPACE);
- verify(userRemoteService).getUserSecret(userSpec, REALM);
+ verify(userRemoteService).getUserSecret(userSpec, NAMESPACE);
}
@Test
void shouldReturnTrueIfExists() {
when(resourceMock.get()).thenReturn(new Secret());
- var exists = userRemoteService.existSecret(userSpec, REALM);
+ var exists = userRemoteService.existSecret(userSpec, NAMESPACE);
assertThat(exists).isTrue();
}
@@ -320,7 +319,7 @@ class KeycloakUserRemoteServiceTest {
void shouldReturnFalseIfNotExists() {
when(resourceMock.get()).thenReturn(null);
- var exists = userRemoteService.existSecret(userSpec, REALM);
+ var exists = userRemoteService.existSecret(userSpec, NAMESPACE);
assertThat(exists).isFalse();
}
@@ -333,33 +332,26 @@ class KeycloakUserRemoteServiceTest {
private final OzgKeycloakUserSpec userSpec = OzgKeycloakUserSpecTestFactory.create();
@Mock
- private Secret credentialSecret;
-
- @Mock
- private MixedOperation<Secret, SecretList, Resource<Secret>> secretsMock;
-
- @BeforeEach
- void mock() {
- when(kubernetesClient.secrets()).thenReturn(secretsMock);
- when(secretsMock.inNamespace(any())).thenReturn(secretsMock);
- }
+ private Secret createSecret;
- @Disabled
@Test
void shouldBuildUserSecret() {
- userRemoteService.createSecret(userSpec, REALM);
+ userRemoteService.createSecret(userSpec, NAMESPACE);
- verify(userRemoteService).createUserSecret(eq(userSpec.getKeycloakUser()), any());
// TODO Enable, wenn die Helm Charts passen
+ verify(userRemoteService).createUserSecret(eq(userSpec.getKeycloakUser()), any());
// verify(userRemoteService).createUserSecret(userSpec.getKeycloakUser(), REALM);
}
- @Disabled
@Test
void shouldCreateSecret() {
- userRemoteService.createSecret(userSpec, REALM);
+ doReturn(createSecret).when(userRemoteService).createUserSecret(any(), any());
+
+ userRemoteService.createSecret(userSpec, NAMESPACE);
- verify(secretsMock).create(any());
+ // TODO Enable, wenn die Helm Charts passen
+ verify(kubernetesRemoteService).createSecret(any(), eq(createSecret));
+// verify(kubernetesRemoteService).createSecret(NAMESPACE, createSecret);
}
}
@@ -371,14 +363,14 @@ class KeycloakUserRemoteServiceTest {
@Test
void shouldHaveType() {
- var secret = userRemoteService.createUserSecret(userSpec, REALM);
+ var secret = userRemoteService.createUserSecret(userSpec, NAMESPACE);
assertThat(secret.getType()).isEqualTo("Opaque");
}
@Test
void shouldHaveUserName() {
- var secret = userRemoteService.createUserSecret(userSpec, REALM);
+ var secret = userRemoteService.createUserSecret(userSpec, NAMESPACE);
assertThat(secret.getStringData()).containsEntry(KeycloakUserRemoteService.SECRET_NAME_FIELD, KeycloakUserSpecUserTestFactory.USERNAME);
}
@@ -387,7 +379,7 @@ class KeycloakUserRemoteServiceTest {
void shouldHavePassword() {
doReturn(KeycloakUserSpecUserTestFactory.PASSWORD).when(userRemoteService).getPassword(any());
- var secret = userRemoteService.createUserSecret(userSpec, REALM);
+ var secret = userRemoteService.createUserSecret(userSpec, NAMESPACE);
assertThat(secret.getStringData()).containsEntry(KeycloakUserRemoteService.SECRET_PASSWORD_FIELD,
KeycloakUserSpecUserTestFactory.PASSWORD);
@@ -399,23 +391,23 @@ class KeycloakUserRemoteServiceTest {
@Test
void shouldHaveName() {
- var secret = userRemoteService.createUserSecret(userSpec, REALM);
+ var secret = userRemoteService.createUserSecret(userSpec, NAMESPACE);
assertThat(secret.getMetadata().getName()).isEqualTo(userSpec.getUsername() + "-credentials");
}
@Test
void shouldHaveGeneratedName() {
- var secret = userRemoteService.createUserSecret(userSpec, REALM);
+ var secret = userRemoteService.createUserSecret(userSpec, NAMESPACE);
assertThat(secret.getMetadata().getGenerateName()).isEqualTo(userSpec.getUsername() + "-credentials");
}
@Test
void shouldHaveNamespace() {
- var secret = userRemoteService.createUserSecret(userSpec, REALM);
+ var secret = userRemoteService.createUserSecret(userSpec, NAMESPACE);
- assertThat(secret.getMetadata().getNamespace()).isEqualTo(REALM);
+ assertThat(secret.getMetadata().getNamespace()).isEqualTo(NAMESPACE);
}
}
}
@@ -457,7 +449,7 @@ class KeycloakUserRemoteServiceTest {
doReturn(resource).when(userRemoteService).getUserSecret(any(), any());
when(resource.get()).thenReturn(secret);
- userRemoteService.getPasswordFromSecret(userSpec, REALM);
+ userRemoteService.getPasswordFromSecret(userSpec, NAMESPACE);
verify(userRemoteService).getUserSecret(eq(userSpec), any());
// TODO Enable, wenn die Helm Charts passen
@@ -469,7 +461,7 @@ class KeycloakUserRemoteServiceTest {
doReturn(resource).when(userRemoteService).getUserSecret(any(), any());
when(resource.get()).thenReturn(secret);
- var password = userRemoteService.getPasswordFromSecret(userSpec, REALM);
+ var password = userRemoteService.getPasswordFromSecret(userSpec, NAMESPACE);
assertThat(password).isEqualTo("dummyPassword");
}
@@ -479,29 +471,13 @@ class KeycloakUserRemoteServiceTest {
@Nested
class TestGetUserSecret {
- @Mock
- private MixedOperation<Secret, SecretList, Resource<Secret>> secretsMock;
-
- @BeforeEach
- void mock() {
- when(kubernetesClient.secrets()).thenReturn(secretsMock);
- when(secretsMock.inNamespace(any())).thenReturn(secretsMock);
- }
-
@Test
- void shouldGetFromNamespace() {
- userRemoteService.getUserSecret(OzgKeycloakUserSpecTestFactory.create(), REALM);
+ void shouldGetSecret() {
+ userRemoteService.getUserSecret(OzgKeycloakUserSpecTestFactory.create(), NAMESPACE);
- verify(secretsMock).inNamespace(any());
// TODO Enable, wenn die Helm Charts passen
-// verify(secretsMock).inNamespace(REALM);
- }
-
- @Test
- void shouldGetWithName() {
- userRemoteService.getUserSecret(OzgKeycloakUserSpecTestFactory.create(), REALM);
-
- verify(secretsMock).withName(KeycloakUserSpecUserTestFactory.USERNAME + "-credentials");
+ verify(kubernetesRemoteService).getSecret(any(), eq(KeycloakUserSpecUserTestFactory.USERNAME + "-credentials"));
+// verify(kubernetesRemoteService).getSecret(NAMESPACE, KeycloakUserSpecUserTestFactory.USERNAME + "-credentials");
}
}
}
\ No newline at end of file
diff --git a/src/test/java/de/ozgcloud/operator/keycloak/user/KubernetesRemoteServiceTest.java b/src/test/java/de/ozgcloud/operator/keycloak/user/KubernetesRemoteServiceTest.java
new file mode 100644
index 0000000..443e458
--- /dev/null
+++ b/src/test/java/de/ozgcloud/operator/keycloak/user/KubernetesRemoteServiceTest.java
@@ -0,0 +1,87 @@
+package de.ozgcloud.operator.keycloak.user;
+
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.*;
+
+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.mockito.InjectMocks;
+import org.mockito.Mock;
+
+import io.fabric8.kubernetes.api.model.Secret;
+import io.fabric8.kubernetes.api.model.SecretList;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.dsl.MixedOperation;
+import io.fabric8.kubernetes.client.dsl.Resource;
+
+class KubernetesRemoteServiceTest {
+
+ private static final String NAMESPACE = "dummyNamespaceName";
+
+ @InjectMocks
+ private KubernetesRemoteService service;
+ @Mock
+ private KubernetesClient kubernetesClient;
+
+ @DisplayName("Get secret")
+ @Nested
+ class TestGetSecret {
+
+ private static final String SECRET_NAME = "dummySecretName";
+
+ @Mock
+ private MixedOperation<Secret, SecretList, Resource<Secret>> secretsMock;
+
+ @BeforeEach
+ void mock() {
+ when(kubernetesClient.secrets()).thenReturn(secretsMock);
+ when(secretsMock.inNamespace(any())).thenReturn(secretsMock);
+ }
+
+ @Test
+ void shouldGetFromNamespace() {
+ service.getSecret(NAMESPACE, SECRET_NAME);
+
+ verify(secretsMock).inNamespace(NAMESPACE);
+ }
+
+ @Test
+ void shouldGetWithName() {
+ service.getSecret(NAMESPACE, SECRET_NAME);
+
+ verify(secretsMock).withName(SECRET_NAME);
+ }
+ }
+
+ @DisplayName("Create secret")
+ @Nested
+ class TestCreateSecret {
+
+ @Mock
+ private Secret secret;
+ @Mock
+ private MixedOperation<Secret, SecretList, Resource<Secret>> secretsMock;
+
+ @BeforeEach
+ void mock() {
+ when(kubernetesClient.secrets()).thenReturn(secretsMock);
+ when(secretsMock.inNamespace(any())).thenReturn(secretsMock);
+ }
+
+ @Test
+ void shouldGetFromNamespace() {
+ service.createSecret(NAMESPACE, secret);
+
+ verify(secretsMock).create(secret);
+ }
+
+ @Test
+ void shouldCreateByGivenSecret() {
+ service.createSecret(NAMESPACE, secret);
+
+ verify(secretsMock).create(any());
+ }
+ }
+}
--
GitLab