From c3a51fc004b462bce3f379e2f03063b393a30f4c Mon Sep 17 00:00:00 2001 From: Michal Vala Date: Thu, 5 Nov 2020 11:33:24 +0100 Subject: [PATCH] best effort to label namespace when starting new workspace (#18248) --- .../webapp/WEB-INF/classes/che/che.properties | 4 +- .../CheServerKubernetesClientFactory.java | 6 +- .../namespace/KubernetesNamespace.java | 87 ++++++-- .../namespace/KubernetesNamespaceFactory.java | 9 +- .../KubernetesNamespaceFactoryTest.java | 39 +++- .../namespace/KubernetesNamespaceTest.java | 186 ++++++++++++++++-- .../openshift/project/OpenShiftProject.java | 15 +- .../project/OpenShiftProjectFactory.java | 10 +- .../project/OpenShiftProjectFactoryTest.java | 23 ++- .../project/OpenShiftProjectTest.java | 179 +++++++++++++++-- 10 files changed, 498 insertions(+), 60 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 6810a8cb65..1c35954816 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -261,7 +261,9 @@ che.infra.kubernetes.namespace.creation_allowed=true che.infra.kubernetes.namespace.default=-che # List of labels to find Namespaces/Projects that are used for Che Workspaces. -# They are used to find prepared Namespaces/Projects for users in combination with `che.infra.kubernetes.namespace.annotations`. +# They are used to: +# - find prepared Namespaces/Projects for users in combination with `che.infra.kubernetes.namespace.annotations`. +# - actively label namespaces with any workspace. che.infra.kubernetes.namespace.labels=app.kubernetes.io/part-of=che.eclipse.org,app.kubernetes.io/component=workspaces-namespace # List of annotations to find Namespaces/Projects prepared for Che users workspaces. diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/CheServerKubernetesClientFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/CheServerKubernetesClientFactory.java index 87171d2942..ab6beafa8d 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/CheServerKubernetesClientFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/CheServerKubernetesClientFactory.java @@ -21,8 +21,10 @@ import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.commons.annotation.Nullable; /** - * This {@link KubernetesClientFactory} is used to access Che installation namespace. It always - * provides client with default {@link Config}. + * This {@link KubernetesClientFactory} ensures that we use `che` ServiceAccount and not related to + * any workspace. It always provides client with default {@link Config}. It's useful for operations + * that needs permissions of `che` SA, such as operations inside `che` namespace (like creating a + * ConfigMaps for Gateway router) or some cluste-wide actions (like labeling the namespaces). */ @Singleton public class CheServerKubernetesClientFactory extends KubernetesClientFactory { diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java index a3f1fa3853..d6cf048759 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java @@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.DoneableServiceAccount; import io.fabric8.kubernetes.api.model.Namespace; +import io.fabric8.kubernetes.api.model.NamespaceBuilder; import io.fabric8.kubernetes.api.model.PersistentVolumeClaim; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.Secret; @@ -28,6 +29,8 @@ import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.Watch; import io.fabric8.kubernetes.client.Watcher; import io.fabric8.kubernetes.client.dsl.Resource; +import java.util.HashMap; +import java.util.Map; import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -69,13 +72,18 @@ public class KubernetesNamespace { private final KubernetesServices services; private final KubernetesPersistentVolumeClaims pvcs; private final KubernetesIngresses ingresses; + /** Factory for workspace related operations clients */ private final KubernetesClientFactory clientFactory; + /** Factory for cluster related operations clients (like labeling the namespaces) */ + private final KubernetesClientFactory cheSAClientFactory; + private final KubernetesSecrets secrets; private final KubernetesConfigsMaps configMaps; @VisibleForTesting protected KubernetesNamespace( KubernetesClientFactory clientFactory, + KubernetesClientFactory cheSAClientFactory, String workspaceId, String name, KubernetesDeployments deployments, @@ -85,6 +93,7 @@ public class KubernetesNamespace { KubernetesSecrets secrets, KubernetesConfigsMaps configMaps) { this.clientFactory = clientFactory; + this.cheSAClientFactory = cheSAClientFactory; this.workspaceId = workspaceId; this.name = name; this.deployments = deployments; @@ -96,8 +105,13 @@ public class KubernetesNamespace { } public KubernetesNamespace( - KubernetesClientFactory clientFactory, Executor executor, String name, String workspaceId) { + KubernetesClientFactory clientFactory, + KubernetesClientFactory cheSAClientFactory, + Executor executor, + String name, + String workspaceId) { this.clientFactory = clientFactory; + this.cheSAClientFactory = cheSAClientFactory; this.workspaceId = workspaceId; this.name = name; this.deployments = new KubernetesDeployments(name, workspaceId, clientFactory, executor); @@ -113,12 +127,17 @@ public class KubernetesNamespace { * *

Preparing includes creating if needed and waiting for default service account. * + *

The method will try to label the namespace with provided `labels`. It does not matter if the + * namespace already exists or we create new one. If update labels operation fail due to lack of + * permission, we do not fail completely. + * * @param canCreate defines what to do when the namespace is not found. The namespace is created * when {@code true}, otherwise an exception is thrown. + * @param labels labels that should be set to the namespace * @throws InfrastructureException if any exception occurs during namespace preparation or if the * namespace doesn't exist and {@code canCreate} is {@code false}. */ - void prepare(boolean canCreate) throws InfrastructureException { + void prepare(boolean canCreate, Map labels) throws InfrastructureException { KubernetesClient client = clientFactory.create(workspaceId); Namespace namespace = get(name, client); @@ -127,7 +146,51 @@ public class KubernetesNamespace { throw new InfrastructureException( format("Creating the namespace '%s' is not allowed, yet it was not found.", name)); } - create(name, client); + namespace = create(name, client); + } + label(namespace, labels); + } + + /** + * Applies given `ensureLabels` into given `namespace` and update the `namespace` in the + * Kubernetes. + * + *

If we do not have permissions to do so (code=403), this method does not throw any exception. + * + * @param namespace namespace to label + * @param ensureLabels these labels should be applied on given `namespace` + * @throws InfrastructureException if something goes wrong with update, except lack of permissions + */ + protected void label(Namespace namespace, Map ensureLabels) + throws InfrastructureException { + Map currentLabels = namespace.getMetadata().getLabels(); + Map newLabels = + currentLabels != null ? new HashMap<>(currentLabels) : new HashMap<>(); + + if (newLabels.entrySet().containsAll(ensureLabels.entrySet())) { + LOG.debug( + "Nothing to do, namespace [{}] already has all required labels.", + namespace.getMetadata().getName()); + return; + } + + try { + // update the namespace with new labels + cheSAClientFactory + .create() + .namespaces() + .createOrReplace( + new NamespaceBuilder(namespace) + .editMetadata() + .addToLabels(ensureLabels) + .endMetadata() + .build()); + } catch (KubernetesClientException kce) { + if (kce.getCode() == 403) { + LOG.debug("Can't label the namespace due to lack of permissions ¯\\_(ツ)_/¯"); + return; + } + throw new InfrastructureException(kce); } } @@ -231,17 +294,19 @@ public class KubernetesNamespace { } } - private void create(String namespaceName, KubernetesClient client) + private Namespace create(String namespaceName, KubernetesClient client) throws InfrastructureException { try { - client - .namespaces() - .createNew() - .withNewMetadata() - .withName(namespaceName) - .endMetadata() - .done(); + Namespace namespace = + client + .namespaces() + .createNew() + .withNewMetadata() + .withName(namespaceName) + .endMetadata() + .done(); waitDefaultServiceAccount(namespaceName, client); + return namespace; } catch (KubernetesClientException e) { if (e.getCode() == 403) { LOG.error( diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java index c3a5de67fc..353dd9c53f 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java @@ -57,6 +57,7 @@ import org.eclipse.che.commons.lang.NameGenerator; import org.eclipse.che.commons.lang.Pair; import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.inject.ConfigurationException; +import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.api.server.impls.KubernetesNamespaceMetaImpl; import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta; @@ -99,6 +100,7 @@ public class KubernetesNamespaceFactory { private final String serviceAccountName; private final Set clusterRoleNames; private final KubernetesClientFactory clientFactory; + private final KubernetesClientFactory cheClientFactory; private final boolean namespaceCreationAllowed; private final UserManager userManager; private final PreferenceManager preferenceManager; @@ -116,6 +118,7 @@ public class KubernetesNamespaceFactory { @Named("che.infra.kubernetes.namespace.labels") String namespaceLabels, @Named("che.infra.kubernetes.namespace.annotations") String namespaceAnnotations, KubernetesClientFactory clientFactory, + CheServerKubernetesClientFactory cheClientFactory, UserManager userManager, PreferenceManager preferenceManager, KubernetesSharedPool sharedPool) @@ -125,6 +128,7 @@ public class KubernetesNamespaceFactory { this.legacyNamespaceName = legacyNamespaceName; this.serviceAccountName = serviceAccountName; this.clientFactory = clientFactory; + this.cheClientFactory = cheClientFactory; this.defaultNamespaceName = defaultNamespaceName; this.allowUserDefinedNamespaces = allowUserDefinedNamespaces; this.preferenceManager = preferenceManager; @@ -168,7 +172,8 @@ public class KubernetesNamespaceFactory { @VisibleForTesting KubernetesNamespace doCreateNamespaceAccess(String workspaceId, String name) { - return new KubernetesNamespace(clientFactory, sharedPool.getExecutor(), name, workspaceId); + return new KubernetesNamespace( + clientFactory, cheClientFactory, sharedPool.getExecutor(), name, workspaceId); } /** @@ -386,7 +391,7 @@ public class KubernetesNamespaceFactory { public KubernetesNamespace getOrCreate(RuntimeIdentity identity) throws InfrastructureException { KubernetesNamespace namespace = get(identity); - namespace.prepare(canCreateNamespace(identity)); + namespace.prepare(canCreateNamespace(identity), namespaceLabels); if (!isNullOrEmpty(serviceAccountName)) { KubernetesWorkspaceServiceAccount workspaceServiceAccount = diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java index 7558c65d81..c145f76150 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java @@ -74,6 +74,7 @@ import org.eclipse.che.api.workspace.shared.Constants; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.SubjectImpl; import org.eclipse.che.inject.ConfigurationException; +import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; @@ -103,6 +104,7 @@ public class KubernetesNamespaceFactoryTest { @Mock private KubernetesSharedPool pool; @Mock private KubernetesClientFactory clientFactory; + @Mock private CheServerKubernetesClientFactory cheClientFactory; private KubernetesClient k8sClient; @Mock private UserManager userManager; @Mock private PreferenceManager preferenceManager; @@ -164,6 +166,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -186,6 +189,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -213,6 +217,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -238,6 +243,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -260,6 +266,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -310,6 +317,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -353,6 +361,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -383,6 +392,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -417,6 +427,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -445,6 +456,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -476,6 +488,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -502,6 +515,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -537,6 +551,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -573,6 +588,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -603,6 +619,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool)); @@ -617,7 +634,7 @@ public class KubernetesNamespaceFactoryTest { // then assertEquals(toReturnNamespace, namespace); verify(namespaceFactory, never()).doCreateServiceAccount(any(), any()); - verify(toReturnNamespace).prepare(eq(false)); + verify(toReturnNamespace).prepare(eq(false), any()); } @Test @@ -635,6 +652,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool)); @@ -649,7 +667,7 @@ public class KubernetesNamespaceFactoryTest { // then assertEquals(toReturnNamespace, namespace); verify(namespaceFactory, never()).doCreateServiceAccount(any(), any()); - verify(toReturnNamespace).prepare(eq(false)); + verify(toReturnNamespace).prepare(eq(false), any()); } @Test @@ -668,6 +686,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool)); @@ -705,6 +724,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool)); @@ -773,6 +793,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool)); @@ -829,6 +850,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -848,6 +870,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -871,6 +894,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -898,6 +922,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -928,6 +953,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -959,6 +985,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -990,6 +1017,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -1021,6 +1049,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -1048,6 +1077,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -1077,6 +1107,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -1128,6 +1159,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -1165,6 +1197,7 @@ public class KubernetesNamespaceFactoryTest { "try_placeholder_here=", NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -1187,6 +1220,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); @@ -1206,6 +1240,7 @@ public class KubernetesNamespaceFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, userManager, preferenceManager, pool); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceTest.java index 745824e044..4d52543cb2 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; import io.fabric8.kubernetes.api.model.DoneableNamespace; import io.fabric8.kubernetes.api.model.DoneableServiceAccount; @@ -31,6 +32,7 @@ import io.fabric8.kubernetes.api.model.Namespace; import io.fabric8.kubernetes.api.model.NamespaceBuilder; import io.fabric8.kubernetes.api.model.NamespaceFluent.MetadataNested; import io.fabric8.kubernetes.api.model.ServiceAccount; +import io.fabric8.kubernetes.api.model.Status; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.Watch; @@ -39,9 +41,12 @@ import io.fabric8.kubernetes.client.Watcher.Action; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; import io.fabric8.kubernetes.client.dsl.Resource; +import java.util.Map; import java.util.concurrent.Executor; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; +import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.stubbing.Answer; import org.mockito.testng.MockitoTestNGListener; @@ -67,6 +72,7 @@ public class KubernetesNamespaceTest { @Mock private KubernetesSecrets secrets; @Mock private KubernetesConfigsMaps configMaps; @Mock private KubernetesClientFactory clientFactory; + @Mock private CheServerKubernetesClientFactory cheClientFactory; @Mock private Executor executor; @Mock private KubernetesClient kubernetesClient; @Mock private NonNamespaceOperation namespaceOperation; @@ -90,6 +96,7 @@ public class KubernetesNamespaceTest { k8sNamespace = new KubernetesNamespace( clientFactory, + cheClientFactory, WORKSPACE_ID, NAMESPACE, deployments, @@ -103,14 +110,16 @@ public class KubernetesNamespaceTest { @Test public void testKubernetesNamespacePreparingWhenNamespaceExists() throws Exception { // given - MetadataNested namespaceMeta = prepareCreateNamespaceRequest(); + MetadataNested namespaceMeta = + prepareCreateNamespaceRequest( + new NamespaceBuilder().withNewMetadata().withName(NAMESPACE).endMetadata().build()); prepareNamespace(NAMESPACE); KubernetesNamespace namespace = - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); // when - namespace.prepare(true); + namespace.prepare(true, Map.of()); // then verify(namespaceMeta, never()).withName(NAMESPACE); @@ -119,15 +128,17 @@ public class KubernetesNamespaceTest { @Test public void testKubernetesNamespacePreparingCreationWhenNamespaceDoesNotExist() throws Exception { // given - MetadataNested namespaceMeta = prepareCreateNamespaceRequest(); + MetadataNested namespaceMeta = + prepareCreateNamespaceRequest( + new NamespaceBuilder().withNewMetadata().withName(NAMESPACE).endMetadata().build()); Resource resource = prepareNamespaceResource(NAMESPACE); doThrow(new KubernetesClientException("error", 403, null)).when(resource).get(); KubernetesNamespace namespace = - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); // when - namespace.prepare(true); + namespace.prepare(true, Map.of()); // then verify(namespaceMeta).withName(NAMESPACE); @@ -139,10 +150,10 @@ public class KubernetesNamespaceTest { Resource resource = prepareNamespaceResource(NAMESPACE); doThrow(new KubernetesClientException("error", 403, null)).when(resource).get(); KubernetesNamespace namespace = - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); // when - namespace.prepare(false); + namespace.prepare(false, Map.of()); // then // exception is thrown @@ -189,7 +200,8 @@ public class KubernetesNamespaceTest { doThrow(new KubernetesClientException("error", 403, null)).when(resource).get(); doThrow(KubernetesClientException.class).when(kubernetesClient).serviceAccounts(); - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID).prepare(false); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID) + .prepare(false, Map.of()); } @Test(expectedExceptions = InfrastructureException.class) @@ -200,7 +212,8 @@ public class KubernetesNamespaceTest { doThrow(new KubernetesClientException("error", 403, null)).when(resource).get(); when(serviceAccountResource.get()).thenReturn(null); - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID).prepare(false); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID) + .prepare(false, Map.of()); } @Test(expectedExceptions = InfrastructureException.class) @@ -219,12 +232,16 @@ public class KubernetesNamespaceTest { .when(serviceAccountResource) .watch(any()); - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID).prepare(false); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID) + .prepare(false, Map.of()); } @Test public void testStopsWaitingServiceAccountEventJustAfterEventReceived() throws Exception { prepareCreateNamespaceRequest(); + prepareCreateNamespaceRequest( + new NamespaceBuilder().withNewMetadata().withName(NAMESPACE).endMetadata().build()); + final Resource resource = prepareNamespaceResource(NAMESPACE); doThrow(new KubernetesClientException("error", 403, null)).when(resource).get(); when(serviceAccountResource.get()).thenReturn(null); @@ -237,7 +254,8 @@ public class KubernetesNamespaceTest { .when(serviceAccountResource) .watch(any()); - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID).prepare(true); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID) + .prepare(true, Map.of()); verify(serviceAccountResource).get(); verify(serviceAccountResource).watch(any()); @@ -247,7 +265,7 @@ public class KubernetesNamespaceTest { public void testDeletesExistingNamespace() throws Exception { // given KubernetesNamespace namespace = - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); Resource resource = prepareNamespaceResource(NAMESPACE); // when @@ -261,7 +279,7 @@ public class KubernetesNamespaceTest { public void testDoesntFailIfDeletedNamespaceDoesntExist() throws Exception { // given KubernetesNamespace namespace = - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); Resource resource = prepareNamespaceResource(NAMESPACE); when(resource.get()).thenThrow(new KubernetesClientException("err", 404, null)); when(resource.delete()).thenThrow(new KubernetesClientException("err", 404, null)); @@ -278,7 +296,7 @@ public class KubernetesNamespaceTest { public void testDoesntFailIfDeletedNamespaceIsBeingDeleted() throws Exception { // given KubernetesNamespace namespace = - new KubernetesNamespace(clientFactory, executor, NAMESPACE, WORKSPACE_ID); + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); Resource resource = prepareNamespaceResource(NAMESPACE); when(resource.delete()).thenThrow(new KubernetesClientException("err", 409, null)); @@ -290,21 +308,146 @@ public class KubernetesNamespaceTest { // and no exception is thrown } + @Test + public void testLabelNamespace() throws InfrastructureException { + // given + prepareNamespace(NAMESPACE); + KubernetesNamespace kubernetesNamespace = + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); + + KubernetesClient cheKubeClient = mock(KubernetesClient.class); + doReturn(cheKubeClient).when(cheClientFactory).create(); + + NonNamespaceOperation nonNamespaceOperation = mock(NonNamespaceOperation.class); + doReturn(nonNamespaceOperation).when(cheKubeClient).namespaces(); + + ArgumentCaptor namespaceArgumentCaptor = ArgumentCaptor.forClass(Namespace.class); + doAnswer(a -> a.getArgument(0)) + .when(nonNamespaceOperation) + .createOrReplace(namespaceArgumentCaptor.capture()); + + Map labels = Map.of("label.with.this", "yes", "and.this", "of courese"); + + // when + kubernetesNamespace.prepare(true, labels); + + // then + Namespace updatedNamespace = namespaceArgumentCaptor.getValue(); + assertTrue( + updatedNamespace.getMetadata().getLabels().entrySet().containsAll(labels.entrySet())); + assertEquals(updatedNamespace.getMetadata().getName(), NAMESPACE); + } + + @Test + public void testDontTryToLabelNamespaceIfAlreadyLabeled() throws InfrastructureException { + // given + Map labels = Map.of("label.with.this", "yes", "and.this", "of courese"); + + Namespace namespace = prepareNamespace(NAMESPACE); + namespace.getMetadata().setLabels(labels); + KubernetesNamespace kubernetesNamespace = + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); + + KubernetesClient cheKubeClient = mock(KubernetesClient.class); + lenient().doReturn(cheKubeClient).when(cheClientFactory).create(); + + NonNamespaceOperation nonNamespaceOperation = mock(NonNamespaceOperation.class); + lenient().doReturn(nonNamespaceOperation).when(cheKubeClient).namespaces(); + + lenient() + .doAnswer(a -> a.getArgument(0)) + .when(nonNamespaceOperation) + .createOrReplace(any(Namespace.class)); + + // when + kubernetesNamespace.prepare(true, labels); + + // then + assertTrue(namespace.getMetadata().getLabels().entrySet().containsAll(labels.entrySet())); + verify(nonNamespaceOperation, never()).createOrReplace(any()); + } + + @Test + public void testDoNotFailWhenNoPermissionsToUpdateNamespace() throws InfrastructureException { + // given + Map labels = Map.of("label.with.this", "yes", "and.this", "of courese"); + + prepareNamespace(NAMESPACE); + KubernetesNamespace kubernetesNamespace = + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); + + KubernetesClient cheKubeClient = mock(KubernetesClient.class); + lenient().doReturn(cheKubeClient).when(cheClientFactory).create(); + + NonNamespaceOperation nonNamespaceOperation = mock(NonNamespaceOperation.class); + lenient().doReturn(nonNamespaceOperation).when(cheKubeClient).namespaces(); + + ArgumentCaptor namespaceArgumentCaptor = ArgumentCaptor.forClass(Namespace.class); + lenient() + .doThrow(new KubernetesClientException("No permissions.", 403, new Status())) + .when(nonNamespaceOperation) + .createOrReplace(namespaceArgumentCaptor.capture()); + + // when + kubernetesNamespace.prepare(true, labels); + + // then + Namespace updatedNamespace = namespaceArgumentCaptor.getValue(); + assertTrue( + updatedNamespace.getMetadata().getLabels().entrySet().containsAll(labels.entrySet())); + assertEquals(updatedNamespace.getMetadata().getName(), NAMESPACE); + } + + @Test(expectedExceptions = InfrastructureException.class) + public void testFailWhenFailToUpdateNamespace() throws InfrastructureException { + // given + Map labels = Map.of("label.with.this", "yes", "and.this", "of courese"); + + Namespace namespace = prepareNamespace(NAMESPACE); + KubernetesNamespace kubernetesNamespace = + new KubernetesNamespace(clientFactory, cheClientFactory, executor, NAMESPACE, WORKSPACE_ID); + + KubernetesClient cheKubeClient = mock(KubernetesClient.class); + lenient().doReturn(cheKubeClient).when(cheClientFactory).create(); + + NonNamespaceOperation nonNamespaceOperation = mock(NonNamespaceOperation.class); + lenient().doReturn(nonNamespaceOperation).when(cheKubeClient).namespaces(); + + lenient() + .doThrow(new KubernetesClientException("Some other error", 500, new Status())) + .when(nonNamespaceOperation) + .createOrReplace(any(Namespace.class)); + + // when + kubernetesNamespace.prepare(true, labels); + + // then + verify(nonNamespaceOperation).createOrReplace(namespace); + } + private MetadataNested prepareCreateNamespaceRequest() { + return prepareCreateNamespaceRequest(new NamespaceBuilder().build()); + } + + private MetadataNested prepareCreateNamespaceRequest(Namespace ns) { DoneableNamespace namespace = mock(DoneableNamespace.class); MetadataNested metadataNested = mock(MetadataNested.class); - doReturn(namespace).when(namespaceOperation).createNew(); - doReturn(metadataNested).when(namespace).withNewMetadata(); - doReturn(metadataNested).when(metadataNested).withName(anyString()); - doReturn(namespace).when(metadataNested).endMetadata(); + lenient().doReturn(namespace).when(namespaceOperation).createNew(); + lenient().doReturn(metadataNested).when(namespace).withNewMetadata(); + lenient().doReturn(metadataNested).when(metadataNested).withName(anyString()); + lenient().doReturn(namespace).when(metadataNested).endMetadata(); + lenient().doReturn(ns).when(namespace).done(); return metadataNested; } private Resource prepareNamespaceResource(String namespaceName) { Resource namespaceResource = mock(Resource.class); doReturn(namespaceResource).when(namespaceOperation).withName(namespaceName); - doReturn(namespaceResource).when(namespaceResource).withPropagationPolicy(eq("Background")); + lenient() + .doReturn(namespaceResource) + .when(namespaceResource) + .withPropagationPolicy(eq("Background")); when(namespaceResource.get()) .thenReturn( new NamespaceBuilder().withNewMetadata().withName(namespaceName).endMetadata().build()); @@ -313,7 +456,8 @@ public class KubernetesNamespaceTest { } private Namespace prepareNamespace(String namespaceName) { - Namespace namespace = mock(Namespace.class); + Namespace namespace = + new NamespaceBuilder().withNewMetadata().withName(namespaceName).endMetadata().build(); Resource namespaceResource = prepareNamespaceResource(namespaceName); doReturn(namespace).when(namespaceResource).get(); return namespace; diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java index 7107da970b..4ca6cbda4b 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java @@ -19,9 +19,11 @@ import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.openshift.api.model.Project; import io.fabric8.openshift.api.model.Route; import io.fabric8.openshift.client.OpenShiftClient; +import java.util.Map; import java.util.concurrent.Executor; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.api.workspace.server.spi.InternalInfrastructureException; +import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesInfrastructureException; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesConfigsMaps; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesDeployments; @@ -49,6 +51,7 @@ public class OpenShiftProject extends KubernetesNamespace { @VisibleForTesting OpenShiftProject( OpenShiftClientFactory clientFactory, + KubernetesClientFactory cheClientFactory, String workspaceId, String name, KubernetesDeployments deployments, @@ -60,6 +63,7 @@ public class OpenShiftProject extends KubernetesNamespace { KubernetesConfigsMaps configMaps) { super( clientFactory, + cheClientFactory, workspaceId, name, deployments, @@ -73,8 +77,12 @@ public class OpenShiftProject extends KubernetesNamespace { } public OpenShiftProject( - OpenShiftClientFactory clientFactory, Executor executor, String name, String workspaceId) { - super(clientFactory, executor, name, workspaceId); + OpenShiftClientFactory clientFactory, + KubernetesClientFactory cheClientFactory, + Executor executor, + String name, + String workspaceId) { + super(clientFactory, cheClientFactory, executor, name, workspaceId); this.clientFactory = clientFactory; this.routes = new OpenShiftRoutes(name, workspaceId, clientFactory); } @@ -89,7 +97,7 @@ public class OpenShiftProject extends KubernetesNamespace { * @throws InfrastructureException if any exception occurs during project preparation or if the * project doesn't exist and {@code canCreate} is {@code false}. */ - void prepare(boolean canCreate) throws InfrastructureException { + void prepare(boolean canCreate, Map labels) throws InfrastructureException { String workspaceId = getWorkspaceId(); String projectName = getName(); @@ -109,6 +117,7 @@ public class OpenShiftProject extends KubernetesNamespace { create(projectName, osClient); waitDefaultServiceAccount(projectName, kubeClient); } + label(osClient.namespaces().withName(projectName).get(), labels); } /** diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java index 99c0aace8b..22315d61d4 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java @@ -35,6 +35,7 @@ import org.eclipse.che.api.user.server.UserManager; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.api.workspace.server.spi.NamespaceResolutionContext; import org.eclipse.che.commons.annotation.Nullable; +import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.api.server.impls.KubernetesNamespaceMetaImpl; import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory; @@ -56,6 +57,7 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory { private static final Logger LOG = LoggerFactory.getLogger(OpenShiftProjectFactory.class); private final OpenShiftClientFactory clientFactory; + private final CheServerKubernetesClientFactory cheClientFactory; private final OpenShiftStopWorkspaceRoleProvisioner stopWorkspaceRoleProvisioner; private final String oAuthIdentityProvider; @@ -72,6 +74,7 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory { @Named("che.infra.kubernetes.namespace.labels") String projectLabels, @Named("che.infra.kubernetes.namespace.annotations") String projectAnnotations, OpenShiftClientFactory clientFactory, + CheServerKubernetesClientFactory cheClientFactory, OpenShiftClientConfigFactory clientConfigFactory, OpenShiftStopWorkspaceRoleProvisioner stopWorkspaceRoleProvisioner, UserManager userManager, @@ -89,6 +92,7 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory { projectLabels, projectAnnotations, clientFactory, + cheClientFactory, userManager, preferenceManager, sharedPool); @@ -99,6 +103,7 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory { + "OAuth to personalize credentials that will be used for cluster access."); } this.clientFactory = clientFactory; + this.cheClientFactory = cheClientFactory; this.stopWorkspaceRoleProvisioner = stopWorkspaceRoleProvisioner; this.oAuthIdentityProvider = oAuthIdentityProvider; } @@ -106,7 +111,7 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory { public OpenShiftProject getOrCreate(RuntimeIdentity identity) throws InfrastructureException { OpenShiftProject osProject = get(identity); - osProject.prepare(canCreateNamespace(identity)); + osProject.prepare(canCreateNamespace(identity), namespaceLabels); if (!isNullOrEmpty(getServiceAccountName())) { OpenShiftWorkspaceServiceAccount osWorkspaceServiceAccount = @@ -156,7 +161,8 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory { @VisibleForTesting OpenShiftProject doCreateProjectAccess(String workspaceId, String name) { - return new OpenShiftProject(clientFactory, sharedPool.getExecutor(), name, workspaceId); + return new OpenShiftProject( + clientFactory, cheClientFactory, sharedPool.getExecutor(), name, workspaceId); } @VisibleForTesting diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java index e7387f62e4..ab6b3a668a 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java @@ -64,6 +64,7 @@ import org.eclipse.che.api.workspace.server.spi.NamespaceResolutionContext; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.SubjectImpl; import org.eclipse.che.inject.ConfigurationException; +import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.openshift.OpenShiftClientConfigFactory; @@ -94,6 +95,7 @@ public class OpenShiftProjectFactoryTest { @Mock private OpenShiftClientConfigFactory configFactory; @Mock private OpenShiftClientFactory clientFactory; + @Mock private CheServerKubernetesClientFactory cheClientFactory; @Mock private OpenShiftStopWorkspaceRoleProvisioner stopWorkspaceRoleProvisioner; @Mock private WorkspaceManager workspaceManager; @Mock private UserManager userManager; @@ -154,6 +156,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -179,6 +182,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -207,6 +211,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -234,6 +239,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -270,6 +276,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -307,6 +314,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -340,6 +348,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -383,6 +392,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -420,6 +430,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -457,6 +468,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -486,6 +498,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -531,6 +544,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -571,6 +585,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -603,6 +618,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -620,7 +636,7 @@ public class OpenShiftProjectFactoryTest { // then assertEquals(toReturnProject, project); verify(projectFactory, never()).doCreateServiceAccount(any(), any()); - verify(toReturnProject).prepare(eq(false)); + verify(toReturnProject).prepare(eq(false), any()); } @Test @@ -639,6 +655,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -677,6 +694,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -717,6 +735,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -769,6 +788,7 @@ public class OpenShiftProjectFactoryTest { NAMESPACE_LABELS, NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, @@ -802,6 +822,7 @@ public class OpenShiftProjectFactoryTest { "try_placeholder_here=", NAMESPACE_ANNOTATIONS, clientFactory, + cheClientFactory, configFactory, stopWorkspaceRoleProvisioner, userManager, diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectTest.java index e32925c4bb..08fa19f217 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectTest.java @@ -11,7 +11,9 @@ */ package org.eclipse.che.workspace.infrastructure.openshift.project; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; @@ -21,9 +23,13 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; import io.fabric8.kubernetes.api.model.DoneableServiceAccount; +import io.fabric8.kubernetes.api.model.Namespace; +import io.fabric8.kubernetes.api.model.NamespaceBuilder; import io.fabric8.kubernetes.api.model.ServiceAccount; +import io.fabric8.kubernetes.api.model.Status; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.dsl.MixedOperation; @@ -35,8 +41,10 @@ import io.fabric8.openshift.api.model.ProjectBuilder; import io.fabric8.openshift.api.model.ProjectRequestFluent.MetadataNested; import io.fabric8.openshift.client.OpenShiftClient; import io.fabric8.openshift.client.dsl.ProjectRequestOperation; +import java.util.Map; import java.util.concurrent.Executor; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; +import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesConfigsMaps; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesDeployments; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesIngresses; @@ -44,6 +52,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesP import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesSecrets; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesServices; import org.eclipse.che.workspace.infrastructure.openshift.OpenShiftClientFactory; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.BeforeMethod; @@ -69,6 +78,7 @@ public class OpenShiftProjectTest { @Mock private KubernetesSecrets secrets; @Mock private KubernetesConfigsMaps configsMaps; @Mock private OpenShiftClientFactory clientFactory; + @Mock private CheServerKubernetesClientFactory cheClientFactory; @Mock private Executor executor; @Mock private OpenShiftClient openShiftClient; @Mock private KubernetesClient kubernetesClient; @@ -93,6 +103,7 @@ public class OpenShiftProjectTest { openShiftProject = new OpenShiftProject( clientFactory, + cheClientFactory, WORKSPACE_ID, PROJECT_NAME, deployments, @@ -108,13 +119,14 @@ public class OpenShiftProjectTest { public void testOpenShiftProjectPreparingWhenProjectExists() throws Exception { // given MetadataNested projectMeta = prepareProjectRequest(); + prepareNamespaceGet(PROJECT_NAME); prepareProject(PROJECT_NAME); OpenShiftProject project = - new OpenShiftProject(clientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); // when - project.prepare(true); + project.prepare(true, Map.of()); // then verify(projectMeta, never()).withName(PROJECT_NAME); @@ -124,14 +136,15 @@ public class OpenShiftProjectTest { public void testOpenShiftProjectPreparingWhenProjectDoesNotExist() throws Exception { // given MetadataNested projectMetadata = prepareProjectRequest(); + prepareNamespaceGet(PROJECT_NAME); Resource resource = prepareProjectResource(PROJECT_NAME); doThrow(new KubernetesClientException("error", 403, null)).when(resource).get(); OpenShiftProject project = - new OpenShiftProject(clientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); // when - openShiftProject.prepare(true); + openShiftProject.prepare(true, Map.of()); // then verify(projectMetadata).withName(PROJECT_NAME); @@ -143,10 +156,10 @@ public class OpenShiftProjectTest { Resource resource = prepareProjectResource(PROJECT_NAME); doThrow(new KubernetesClientException("error", 403, null)).when(resource).get(); OpenShiftProject project = - new OpenShiftProject(clientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); // when - project.prepare(false); + project.prepare(false, Map.of()); // then // exception is thrown @@ -189,7 +202,7 @@ public class OpenShiftProjectTest { public void testDeletesExistingProject() throws Exception { // given OpenShiftProject project = - new OpenShiftProject(clientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); Resource resource = prepareProjectResource(PROJECT_NAME); // when @@ -203,7 +216,7 @@ public class OpenShiftProjectTest { public void testDoesntFailIfDeletedProjectDoesntExist() throws Exception { // given OpenShiftProject project = - new OpenShiftProject(clientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); Resource resource = prepareProjectResource(PROJECT_NAME); when(resource.get()).thenThrow(new KubernetesClientException("err", 404, null)); when(resource.delete()).thenThrow(new KubernetesClientException("err", 404, null)); @@ -220,7 +233,7 @@ public class OpenShiftProjectTest { public void testDoesntFailIfDeletedProjectIsBeingDeleted() throws Exception { // given OpenShiftProject project = - new OpenShiftProject(clientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); Resource resource = prepareProjectResource(PROJECT_NAME); when(resource.delete()).thenThrow(new KubernetesClientException("err", 409, null)); @@ -232,16 +245,136 @@ public class OpenShiftProjectTest { // and no exception is thrown } + @Test + public void testLabelNamespace() throws InfrastructureException { + // given + prepareProject(PROJECT_NAME); + prepareNamespaceGet(PROJECT_NAME); + OpenShiftProject openShiftProject = + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + + KubernetesClient cheKubeClient = mock(KubernetesClient.class); + doReturn(cheKubeClient).when(cheClientFactory).create(); + + NonNamespaceOperation nonNamespaceOperation = mock(NonNamespaceOperation.class); + doReturn(nonNamespaceOperation).when(cheKubeClient).namespaces(); + + ArgumentCaptor namespaceArgumentCaptor = ArgumentCaptor.forClass(Namespace.class); + doAnswer(a -> a.getArgument(0)) + .when(nonNamespaceOperation) + .createOrReplace(namespaceArgumentCaptor.capture()); + + Map labels = Map.of("label.with.this", "yes", "and.this", "of courese"); + + // when + openShiftProject.prepare(true, labels); + + // then + Namespace updatedNamespace = namespaceArgumentCaptor.getValue(); + assertTrue( + updatedNamespace.getMetadata().getLabels().entrySet().containsAll(labels.entrySet())); + assertEquals(updatedNamespace.getMetadata().getName(), PROJECT_NAME); + } + + @Test + public void testDontTryToLabelNamespaceIfAlreadyLabeled() throws InfrastructureException { + // given + Map labels = Map.of("label.with.this", "yes", "and.this", "of courese"); + + prepareProject(PROJECT_NAME); + Namespace namespace = prepareNamespaceGet(PROJECT_NAME); + namespace.getMetadata().setLabels(labels); + OpenShiftProject openShiftProject = + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + + KubernetesClient cheKubeClient = mock(KubernetesClient.class); + doReturn(cheKubeClient).when(cheClientFactory).create(); + + NonNamespaceOperation nonNamespaceOperation = mock(NonNamespaceOperation.class); + doReturn(nonNamespaceOperation).when(cheKubeClient).namespaces(); + + doAnswer(a -> a.getArgument(0)) + .when(nonNamespaceOperation) + .createOrReplace(any(Namespace.class)); + + // when + openShiftProject.prepare(true, labels); + + // then + assertTrue(namespace.getMetadata().getLabels().entrySet().containsAll(labels.entrySet())); + verify(nonNamespaceOperation, never()).createOrReplace(any()); + } + + @Test + public void testDoNotFailWhenNoPermissionsToUpdateNamespace() throws InfrastructureException { + // given + Map labels = Map.of("label.with.this", "yes", "and.this", "of courese"); + + prepareProject(PROJECT_NAME); + prepareNamespaceGet(PROJECT_NAME); + OpenShiftProject openShiftProject = + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + + KubernetesClient cheKubeClient = mock(KubernetesClient.class); + lenient().doReturn(cheKubeClient).when(cheClientFactory).create(); + + NonNamespaceOperation nonNamespaceOperation = mock(NonNamespaceOperation.class); + lenient().doReturn(nonNamespaceOperation).when(cheKubeClient).namespaces(); + + ArgumentCaptor namespaceArgumentCaptor = ArgumentCaptor.forClass(Namespace.class); + lenient() + .doThrow(new KubernetesClientException("No permissions.", 403, new Status())) + .when(nonNamespaceOperation) + .createOrReplace(namespaceArgumentCaptor.capture()); + + // when + openShiftProject.prepare(true, labels); + + // then + Namespace updatedNamespace = namespaceArgumentCaptor.getValue(); + assertTrue( + updatedNamespace.getMetadata().getLabels().entrySet().containsAll(labels.entrySet())); + assertEquals(updatedNamespace.getMetadata().getName(), PROJECT_NAME); + } + + @Test(expectedExceptions = InfrastructureException.class) + public void testFailWhenFailToUpdateNamespace() throws InfrastructureException { + // given + Map labels = Map.of("label.with.this", "yes", "and.this", "of courese"); + + prepareProject(PROJECT_NAME); + Namespace namespace = prepareNamespaceGet(PROJECT_NAME); + OpenShiftProject openShiftProject = + new OpenShiftProject(clientFactory, cheClientFactory, executor, PROJECT_NAME, WORKSPACE_ID); + + KubernetesClient cheKubeClient = mock(KubernetesClient.class); + lenient().doReturn(cheKubeClient).when(cheClientFactory).create(); + + NonNamespaceOperation nonNamespaceOperation = mock(NonNamespaceOperation.class); + lenient().doReturn(nonNamespaceOperation).when(cheKubeClient).namespaces(); + + lenient() + .doThrow(new KubernetesClientException("Some other error", 500, new Status())) + .when(nonNamespaceOperation) + .createOrReplace(any(Namespace.class)); + + // when + openShiftProject.prepare(true, labels); + + // then + verify(nonNamespaceOperation).createOrReplace(namespace); + } + private MetadataNested prepareProjectRequest() { ProjectRequestOperation projectRequestOperation = mock(ProjectRequestOperation.class); DoneableProjectRequest projectRequest = mock(DoneableProjectRequest.class); MetadataNested metadataNested = mock(MetadataNested.class); - doReturn(projectRequestOperation).when(openShiftClient).projectrequests(); - doReturn(projectRequest).when(projectRequestOperation).createNew(); - doReturn(metadataNested).when(projectRequest).withNewMetadata(); - doReturn(metadataNested).when(metadataNested).withName(anyString()); - doReturn(projectRequest).when(metadataNested).endMetadata(); + lenient().doReturn(projectRequestOperation).when(openShiftClient).projectrequests(); + lenient().doReturn(projectRequest).when(projectRequestOperation).createNew(); + lenient().doReturn(metadataNested).when(projectRequest).withNewMetadata(); + lenient().doReturn(metadataNested).when(metadataNested).withName(anyString()); + lenient().doReturn(projectRequest).when(metadataNested).endMetadata(); return metadataNested; } @@ -260,8 +393,24 @@ public class OpenShiftProjectTest { return projectResource; } + private Namespace prepareNamespaceGet(String namespaceName) { + Namespace namespace = + new NamespaceBuilder().withNewMetadata().withName(namespaceName).endMetadata().build(); + + NonNamespaceOperation nsOperation = mock(NonNamespaceOperation.class); + doReturn(nsOperation).when(openShiftClient).namespaces(); + + Resource nsResource = mock(Resource.class); + doReturn(nsResource).when(nsOperation).withName(namespaceName); + + doReturn(namespace).when(nsResource).get(); + + return namespace; + } + private Project prepareProject(String projectName) { - Project project = mock(Project.class); + Project project = + new ProjectBuilder().withNewMetadata().withName(projectName).endMetadata().build(); Resource projectResource = prepareProjectResource(projectName); doReturn(project).when(projectResource).get(); return project;