fix: Simplification of the 'canCreateNamespace' logic

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
pull/435/head
Ilya Buziuk 2023-01-31 12:24:06 +01:00
parent 7323f4776f
commit 3b39854685
4 changed files with 12 additions and 40 deletions

View File

@ -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.
*
* <p>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());

View File

@ -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")

View File

@ -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());

View File

@ -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