From 0860edadd32aac261803e001787a5d2d2da28489 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Wed, 21 Jul 2021 22:59:00 +0300 Subject: [PATCH] fix: Workspace Roles and RoleBindings provision on each restart (#52) Signed-off-by: Max Shaposhnik --- .../AbstractWorkspaceServiceAccount.java | 32 ++++--- ...KubernetesWorkspaceServiceAccountTest.java | 86 +++++++++++++++++++ 2 files changed, 101 insertions(+), 17 deletions(-) create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesWorkspaceServiceAccountTest.java diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java index 9c9b6d2cad9..ce7a82e6842 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java @@ -85,9 +85,6 @@ 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 * - *

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 { @@ -95,9 +92,9 @@ public void prepare() throws InfrastructureException { 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 { * *

creates {@code -exec} and {@code -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 resources, List apiGroups, List 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 resources, List apiGroups, List 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 { diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesWorkspaceServiceAccountTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesWorkspaceServiceAccountTest.java new file mode 100644 index 00000000000..ac13d01c768 --- /dev/null +++ b/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 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); + } +}