fix: Workspace Roles and RoleBindings provision on each restart (#52)

Signed-off-by: Max Shaposhnik <mshaposh@redhat.com>

infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java

@@ -85,19 +85,16 @@ protected AbstractWorkspaceServiceAccount(
    * Make sure that workspace service account exists and has `view` and `exec` role bindings, as
    * well as create workspace-view and exec roles in namespace scope
    *
-   * <p>Do NOT make any changes to the service account if it already exists in the namespace to
-   * preserve its configuration done by someone else.
-   *
    * @throws InfrastructureException when any exception occurred
    */
   public void prepare() throws InfrastructureException {
     Client k8sClient = clientFactory.create(workspaceId);
     if (k8sClient.serviceAccounts().inNamespace(namespace).withName(serviceAccountName).get()
         == null) {
       createWorkspaceServiceAccount(k8sClient);
-      createImplicitRolesWithBindings(k8sClient);
-      createExplicitClusterRoleBindings(k8sClient);
     }
+    ensureImplicitRolesWithBindings(k8sClient);
+    ensureExplicitClusterRoleBindings(k8sClient);
   }
 
   /**
@@ -106,9 +103,9 @@ public void prepare() throws InfrastructureException {
    *
    * <p>creates {@code <sa>-exec} and {@code <sa>-view}
    */
-  private void createImplicitRolesWithBindings(Client k8sClient) {
+  private void ensureImplicitRolesWithBindings(Client k8sClient) {
     // exec role
-    createRoleWithBinding(
+    ensureRoleWithBinding(
         k8sClient,
         EXEC_ROLE_NAME,
         singletonList("pods/exec"),
@@ -117,7 +114,7 @@ private void createImplicitRolesWithBindings(Client k8sClient) {
         serviceAccountName + "-exec");
 
     // view role
-    createRoleWithBinding(
+    ensureRoleWithBinding(
         k8sClient,
         VIEW_ROLE_NAME,
         Arrays.asList("pods", "services"),
@@ -126,7 +123,7 @@ private void createImplicitRolesWithBindings(Client k8sClient) {
         serviceAccountName + "-view");
 
     // metrics role
-    createRoleWithBinding(
+    ensureRoleWithBinding(
         k8sClient,
         METRICS_ROLE_NAME,
         Arrays.asList("pods", "nodes"),
@@ -135,14 +132,14 @@ private void createImplicitRolesWithBindings(Client k8sClient) {
         serviceAccountName + "-metrics");
   }
 
-  private void createRoleWithBinding(
+  private void ensureRoleWithBinding(
       Client k8sClient,
       String roleName,
       List<String> resources,
       List<String> apiGroups,
       List<String> verbs,
       String bindingName) {
-    createRole(k8sClient, roleName, resources, apiGroups, verbs);
+    ensureRole(k8sClient, roleName, resources, apiGroups, verbs);
     //noinspection unchecked
     roleBindings
         .apply(k8sClient)
@@ -158,7 +155,7 @@ private void createRoleWithBinding(
    *     boolean, String, String, KubernetesClientFactory, CheServerKubernetesClientFactory,
    *     UserManager, PreferenceManager, KubernetesSharedPool)
    */
-  private void createExplicitClusterRoleBindings(Client k8sClient) {
+  private void ensureExplicitClusterRoleBindings(Client k8sClient) {
     // If the user specified an additional cluster roles for the workspace,
     // create a role binding for them too
     int idx = 0;
@@ -212,16 +209,17 @@ private void createWorkspaceServiceAccount(Client k8sClient) {
                 .build());
   }
 
-  private void createRole(
+  private void ensureRole(
       Client k8sClient,
       String name,
       List<String> resources,
       List<String> apiGroups,
       List<String> verbs) {
-    if (roles.apply(k8sClient).inNamespace(namespace).withName(name).get() == null) {
-      R role = buildRole(name, resources, apiGroups, verbs);
-      roles.apply(k8sClient).inNamespace(namespace).create(role);
-    }
+    //noinspection unchecked
+    roles
+        .apply(k8sClient)
+        .inNamespace(namespace)
+        .createOrReplace(buildRole(name, resources, apiGroups, verbs));
   }
 
   public interface ClientFactory<C extends KubernetesClient> {

infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesWorkspaceServiceAccountTest.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2012-2021 Red Hat, Inc.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.workspace.infrastructure.kubernetes.namespace;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.when;
+
+import io.fabric8.kubernetes.api.model.ServiceAccountBuilder;
+import io.fabric8.kubernetes.api.model.rbac.RoleBindingBuilder;
+import io.fabric8.kubernetes.api.model.rbac.RoleBindingList;
+import io.fabric8.kubernetes.api.model.rbac.RoleBuilder;
+import io.fabric8.kubernetes.api.model.rbac.RoleList;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.server.mock.KubernetesServer;
+import java.util.Collections;
+import java.util.Set;
+import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory;
+import org.mockito.Mock;
+import org.mockito.testng.MockitoTestNGListener;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
+
+@Listeners(MockitoTestNGListener.class)
+public class KubernetesWorkspaceServiceAccountTest {
+
+  public static final String NAMESPACE = "testNamespace";
+  public static final String WORKSPACE_ID = "workspace123";
+  public static final String SA_NAME = "workspace-sa";
+  public static final Set<String> ROLE_NAMES = Collections.singleton("role-foo");
+
+  @Mock private KubernetesClientFactory clientFactory;
+  private KubernetesClient k8sClient;
+  private KubernetesServer serverMock;
+  private KubernetesWorkspaceServiceAccount serviceAccount;
+
+  @BeforeMethod
+  public void setUp() throws Exception {
+    this.serviceAccount =
+        new KubernetesWorkspaceServiceAccount(
+            WORKSPACE_ID, NAMESPACE, SA_NAME, ROLE_NAMES, clientFactory);
+
+    serverMock = new KubernetesServer(true, true);
+    serverMock.before();
+    k8sClient = serverMock.getClient();
+    when(clientFactory.create(anyString())).thenReturn(k8sClient);
+  }
+
+  @Test
+  public void shouldProvisionSARolesEvenIfItAlreadyExists() throws Exception {
+    ServiceAccountBuilder serviceAccountBuilder =
+        new ServiceAccountBuilder().withNewMetadata().withName(SA_NAME).endMetadata();
+    RoleBuilder roleBuilder = new RoleBuilder().withNewMetadata().withName("foo").endMetadata();
+    RoleBindingBuilder roleBindingBuilder =
+        new RoleBindingBuilder().withNewMetadata().withName("foo-builder").endMetadata();
+
+    // pre-create SA and some roles
+    k8sClient
+        .serviceAccounts()
+        .inNamespace(NAMESPACE)
+        .createOrReplace(serviceAccountBuilder.build());
+    k8sClient.rbac().roles().inNamespace(NAMESPACE).create(roleBuilder.build());
+    k8sClient.rbac().roleBindings().inNamespace(NAMESPACE).create(roleBindingBuilder.build());
+
+    // when
+    serviceAccount.prepare();
+
+    // then
+    // make sure more roles added
+    RoleList rl = k8sClient.rbac().roles().inNamespace(NAMESPACE).list();
+    assertTrue(rl.getItems().size() > 1);
+
+    RoleBindingList rbl = k8sClient.rbac().roleBindings().inNamespace(NAMESPACE).list();
+    assertTrue(rbl.getItems().size() > 1);
+  }
+}