From 3b398546859ec31e93af98cc2b2c6fd1cd792085 Mon Sep 17 00:00:00 2001 From: Ilya Buziuk Date: Tue, 31 Jan 2023 12:24:06 +0100 Subject: [PATCH] fix: Simplification of the 'canCreateNamespace' logic Signed-off-by: Ilya Buziuk --- .../namespace/KubernetesNamespaceFactory.java | 30 +++---------------- .../KubernetesNamespaceFactoryTest.java | 11 ++----- .../project/OpenShiftProjectFactory.java | 5 ++-- .../project/OpenShiftProjectFactoryTest.java | 6 ++-- 4 files changed, 12 insertions(+), 40 deletions(-) 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 4ef931be3e..68637831a4 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2022 Red Hat, Inc. + * Copyright (c) 2012-2023 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/ @@ -265,33 +265,11 @@ public class KubernetesNamespaceFactory { * Tells the caller whether the namespace that is being prepared for the provided workspace * runtime identity can be created or is expected to already be present. * - *

Note that this method cannot be reduced to merely checking if user-defined namespaces are - * allowed or not (and depending on prior validation using the {@link - * #checkIfNamespaceIsAllowed(String)} method during the workspace creation) because workspace - * start is a) async from workspace creation and the underlying namespaces might have disappeared - * and b) can be called during workspace recovery, where we don't even have the current user in - * the context. - * - * @param identity the identity of the workspace runtime - * @param userName the user name * @return true if the namespace can be created, false if the namespace is expected to already * exist - * @throws InfrastructureException on failure */ - protected boolean canCreateNamespace(RuntimeIdentity identity, String userName) - throws InfrastructureException { - if (!namespaceCreationAllowed) { - return false; - } - - String requiredNamespace = identity.getInfrastructureNamespace(); - - NamespaceResolutionContext resolutionContext = - new NamespaceResolutionContext(identity.getWorkspaceId(), identity.getOwnerId(), userName); - - String resolvedDefaultNamespace = evaluateNamespaceName(resolutionContext); - - return resolvedDefaultNamespace.equals(requiredNamespace); + protected boolean canCreateNamespace() { + return namespaceCreationAllowed; } /** @@ -316,7 +294,7 @@ public class KubernetesNamespaceFactory { evaluateAnnotationPlaceholders(resolutionCtx); namespace.prepare( - canCreateNamespace(identity, userName), + canCreateNamespace(), labelNamespaces ? namespaceLabels : emptyMap(), annotateNamespaces ? namespaceAnnotationsEvaluated : emptyMap()); 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 5e01b4bbf2..d5b1dea3ad 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2023 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/ @@ -505,8 +505,6 @@ public class KubernetesNamespaceFactoryTest { when(k8sClient.secrets()).thenReturn(mixedOperation); when(mixedOperation.inNamespace(anyString())).thenReturn(namespaceOperation); when(namespaceResource.get()).thenReturn(null); - when(cheClientFactory.create()).thenReturn(k8sClient); - when(clientFactory.create()).thenReturn(k8sClient); // when RuntimeIdentity identity = @@ -753,7 +751,7 @@ public class KubernetesNamespaceFactoryTest { // then assertEquals(toReturnNamespace, namespace); - verify(toReturnNamespace).prepare(eq(false), any(), any()); + verify(toReturnNamespace).prepare(eq(true), any(), any()); } @Test @@ -853,7 +851,6 @@ public class KubernetesNamespaceFactoryTest { when(toReturnNamespace.getName()).thenReturn("workspace123"); doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any()); when(k8sClient.supportsApiPath(eq("/apis/metrics.k8s.io"))).thenReturn(true); - when(cheClientFactory.create()).thenReturn(k8sClient); when(clientFactory.create(any())).thenReturn(k8sClient); // pre-create the cluster roles @@ -930,7 +927,6 @@ public class KubernetesNamespaceFactoryTest { when(toReturnNamespace.getName()).thenReturn("workspace123"); doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any()); when(clientFactory.create(any())).thenReturn(k8sClient); - when(cheClientFactory.create()).thenReturn(k8sClient); // when RuntimeIdentity identity = @@ -978,7 +974,6 @@ public class KubernetesNamespaceFactoryTest { doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any()); when(k8sClient.supportsApiPath(eq("/apis/metrics.k8s.io"))).thenReturn(true); when(clientFactory.create(any())).thenReturn(k8sClient); - when(cheClientFactory.create()).thenReturn(k8sClient); // when RuntimeIdentity identity = @@ -1452,7 +1447,7 @@ public class KubernetesNamespaceFactoryTest { // then assertEquals(toReturnNamespace, namespace); verify(toReturnNamespace) - .prepare(eq(false), any(), eq(Map.of("try_placeholder_here", "jondoe"))); + .prepare(eq(true), any(), eq(Map.of("try_placeholder_here", "jondoe"))); } @Test(dataProvider = "invalidUsernames") 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 3ba2051aca..935989c27d 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2022 Red Hat, Inc. + * Copyright (c) 2012-2023 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/ @@ -104,7 +104,6 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory { public OpenShiftProject getOrCreate(RuntimeIdentity identity) throws InfrastructureException { OpenShiftProject osProject = get(identity); - var subject = EnvironmentContext.getCurrent().getSubject(); var userName = subject.getUserName(); NamespaceResolutionContext resolutionCtx = @@ -113,7 +112,7 @@ public class OpenShiftProjectFactory extends KubernetesNamespaceFactory { evaluateAnnotationPlaceholders(resolutionCtx); osProject.prepare( - canCreateNamespace(identity, userName), + canCreateNamespace(), initWithCheServerSa && !isNullOrEmpty(oAuthIdentityProvider), labelNamespaces ? namespaceLabels : emptyMap(), annotateNamespaces ? namespaceAnnotationsEvaluated : emptyMap()); 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 5182d2c945..fe4c8e578a 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2023 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/ @@ -535,7 +535,7 @@ public class OpenShiftProjectFactoryTest { // then assertEquals(toReturnProject, project); - verify(toReturnProject).prepare(eq(false), eq(false), any(), any()); + verify(toReturnProject).prepare(eq(true), eq(false), any(), any()); } @Test @@ -854,7 +854,7 @@ public class OpenShiftProjectFactoryTest { // then assertEquals(toReturnProject, project); verify(toReturnProject) - .prepare(eq(false), eq(false), any(), eq(Map.of("try_placeholder_here", "jondoe"))); + .prepare(eq(true), eq(false), any(), eq(Map.of("try_placeholder_here", "jondoe"))); } @Test