Skip to content

Commit

Permalink
fixup! fix: Add a predefined secret to store credentials
Browse files Browse the repository at this point in the history
  • Loading branch information
@vinokurig
vinokurig committed Aug 10, 2021
1 parent cd092ce commit 79ba1e5
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 70 deletions.
7 changes: 1 addition & 6 deletions ...va/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,13 @@ public KubernetesNamespace(
* @param annotations annotations 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}.
* @return {@code true} if the namespace didn't exist and namespace creation was invoked, {@code
* false} if the namespace was already created in the previous calls.
*/
boolean prepare(boolean canCreate, Map<String, String> labels, Map<String, String> annotations)
void prepare(boolean canCreate, Map<String, String> labels, Map<String, String> annotations)
throws InfrastructureException {
KubernetesClient client = clientFactory.create(workspaceId);
Namespace namespace = get(name, client);
boolean needToCreateNewNamespace = false;

if (namespace == null) {
needToCreateNewNamespace = true;
if (!canCreate) {
throw new InfrastructureException(
format("Creating the namespace '%s' is not allowed, yet it was not found.", name));
Expand All @@ -161,7 +157,6 @@ boolean prepare(boolean canCreate, Map<String, String> labels, Map<String, Strin
}
label(namespace, labels);
annotate(namespace, annotations);
return needToCreateNewNamespace;
}

/**
Expand Down
16 changes: 10 additions & 6 deletions ...eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,12 +339,16 @@ public KubernetesNamespace getOrCreate(RuntimeIdentity identity) throws Infrastr
Map<String, String> namespaceAnnotationsEvaluated =
evaluateAnnotationPlaceholders(resolutionCtx);

boolean newNamespace =
namespace.prepare(
canCreateNamespace(identity),
labelNamespaces ? namespaceLabels : emptyMap(),
annotateNamespaces ? namespaceAnnotationsEvaluated : emptyMap());
if (newNamespace) {
namespace.prepare(
canCreateNamespace(identity),
labelNamespaces ? namespaceLabels : emptyMap(),
annotateNamespaces ? namespaceAnnotationsEvaluated : emptyMap());

if (namespace
.secrets()
.get()
.stream()
.noneMatch(s -> s.getMetadata().getName().equals(CREDENTIALS_SECRET_NAME))) {
Secret secret =
new SecretBuilder()
.withType("opaque")
Expand Down
14 changes: 14 additions & 0 deletions ...java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesSecrets.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,20 @@ public List<Secret> get(LabelSelector labelSelector) throws InfrastructureExcept
}
}

/**
* Get all secrets.
*
* @return namespace secrets list
* @throws InfrastructureException when any exception occurs
*/
public List<Secret> get() throws InfrastructureException {
try {
return clientFactory.create(workspaceId).secrets().inNamespace(namespace).list().getItems();
} catch (KubernetesClientException e) {
throw new KubernetesInfrastructureException(e);
}
}

/**
* Deletes all existing secrets.
*
Expand Down
63 changes: 60 additions & 3 deletions ...pse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.SECRETS_ROLE_NAME;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory.NAMESPACE_TEMPLATE_ATTRIBUTE;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
Expand All @@ -44,6 +43,7 @@
import io.fabric8.kubernetes.api.model.Namespace;
import io.fabric8.kubernetes.api.model.NamespaceBuilder;
import io.fabric8.kubernetes.api.model.NamespaceList;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.ServiceAccountList;
import io.fabric8.kubernetes.api.model.Status;
Expand Down Expand Up @@ -460,7 +460,7 @@ public void shouldReturnDefaultNamespaceWhenItDoesNotExistAndUserDefinedIsNotAll
}

@Test
public void shouldCreateCredentialsSecretWhenNamespaceDoesNotExist() throws Exception {
public void shouldCreateCredentialsSecretIfExists() throws Exception {
// given
namespaceFactory =
spy(
Expand All @@ -479,7 +479,9 @@ public void shouldCreateCredentialsSecretWhenNamespaceDoesNotExist() throws Exce
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
when(toReturnNamespace.prepare(anyBoolean(), anyMap(), anyMap())).thenReturn(true);
KubernetesSecrets secrets = mock(KubernetesSecrets.class);
when(toReturnNamespace.secrets()).thenReturn(secrets);
when(secrets.get()).thenReturn(Collections.emptyList());
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
MixedOperation mixedOperation = mock(MixedOperation.class);
lenient().when(k8sClient.secrets()).thenReturn(mixedOperation);
Expand All @@ -498,6 +500,41 @@ public void shouldCreateCredentialsSecretWhenNamespaceDoesNotExist() throws Exce
Assert.assertEquals(secret.getType(), "opaque");
}

@Test
public void shouldNotCreateCredentialsSecretIfNotExists() throws Exception {
// given
namespaceFactory =
spy(
new KubernetesNamespaceFactory(
"",
"",
"<username>-che",
true,
true,
true,
NAMESPACE_LABELS,
NAMESPACE_ANNOTATIONS,
clientFactory,
cheClientFactory,
userManager,
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
MixedOperation mixedOperation = mock(MixedOperation.class);
lenient().when(k8sClient.secrets()).thenReturn(mixedOperation);
lenient().when(mixedOperation.inNamespace(anyString())).thenReturn(namespaceOperation);

// when
RuntimeIdentity identity =
new RuntimeIdentityImpl("workspace123", null, USER_ID, "workspace123");
namespaceFactory.getOrCreate(identity);

// then
verify(namespaceOperation, never()).create(any());
}

@Test(
expectedExceptions = InfrastructureException.class,
expectedExceptionsMessageRegExp =
Expand Down Expand Up @@ -576,6 +613,7 @@ public void shouldRequireNamespacePriorExistenceIfDifferentFromDefaultAndUserDef
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());

// when
Expand Down Expand Up @@ -608,6 +646,7 @@ public void shouldReturnDefaultNamespaceWhenCreatingIsNotIsNotAllowed() throws E
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());

// when
Expand Down Expand Up @@ -642,6 +681,7 @@ public void shouldPrepareWorkspaceServiceAccountIfItIsConfiguredAndNamespaceIsNo
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
when(toReturnNamespace.getWorkspaceId()).thenReturn("workspace123");
when(toReturnNamespace.getName()).thenReturn("workspace123");
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
Expand Down Expand Up @@ -680,6 +720,7 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception {
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
when(toReturnNamespace.getWorkspaceId()).thenReturn("workspace123");
when(toReturnNamespace.getName()).thenReturn("workspace123");
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
Expand Down Expand Up @@ -751,6 +792,7 @@ public void shouldCreateAndBindCredentialsSecretRole() throws Exception {
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
when(toReturnNamespace.getWorkspaceId()).thenReturn("workspace123");
when(toReturnNamespace.getName()).thenReturn("workspace123");
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
Expand Down Expand Up @@ -809,6 +851,7 @@ public void shouldCreateExecAndViewRolesAndBindings() throws Exception {
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
when(toReturnNamespace.getWorkspaceId()).thenReturn("workspace123");
when(toReturnNamespace.getName()).thenReturn("workspace123");
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
Expand Down Expand Up @@ -1143,6 +1186,7 @@ public void shouldHandleProvision() throws InfrastructureException {
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
when(toReturnNamespace.getName()).thenReturn("jondoe-che");
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
KubernetesNamespaceMetaImpl namespaceMeta =
Expand Down Expand Up @@ -1182,6 +1226,7 @@ public void shouldFailToProvisionIfNotAbleToFindNamespace() throws Infrastructur
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
when(toReturnNamespace.getName()).thenReturn("jondoe-cha-cha-cha");
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
KubernetesNamespaceMetaImpl namespaceMeta =
Expand Down Expand Up @@ -1220,6 +1265,7 @@ public void shouldFail2ProvisionIfNotAbleToFindNamespace() throws Infrastructure
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
when(toReturnNamespace.getName()).thenReturn("jondoe-cha-cha-cha");
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
KubernetesNamespaceMetaImpl namespaceMeta =
Expand Down Expand Up @@ -1296,6 +1342,7 @@ public void testUsernamePlaceholderInAnnotationsIsEvaluated() throws Infrastruct
pool));
EnvironmentContext.getCurrent().setSubject(new SubjectImpl("jondoe", "123", null, false));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
prepareNamespace(toReturnNamespace);
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());

// when
Expand Down Expand Up @@ -1417,6 +1464,16 @@ private void throwOnTryToGetNamespacesList(Throwable e) throws Exception {
when(namespaceList.getItems()).thenThrow(e);
}

private void prepareNamespace(KubernetesNamespace namespace) throws InfrastructureException {
KubernetesSecrets secrets = mock(KubernetesSecrets.class);
when(namespace.secrets()).thenReturn(secrets);
Secret secretMock = mock(Secret.class);
ObjectMeta objectMeta = mock(ObjectMeta.class);
when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME);
when(secretMock.getMetadata()).thenReturn(objectMeta);
when(secrets.get()).thenReturn(Collections.singletonList(secretMock));
}

private Namespace createNamespace(String name, String phase) {
return new NamespaceBuilder()
.withNewMetadata()
Expand Down
14 changes: 1 addition & 13 deletions ...ain/java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,8 @@
package org.eclipse.che.workspace.infrastructure.openshift.project;

import static java.lang.String.format;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.CREDENTIALS_SECRET_NAME;

import com.google.common.annotations.VisibleForTesting;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.SecretBuilder;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.openshift.api.model.Project;
Expand Down Expand Up @@ -104,8 +101,7 @@ public OpenShiftProject(
/**
* Prepare a project for using.
*
* <p>Preparing includes creating if needed and waiting for default service account, then it
* creates a secret for storing credentials.
* <p>Preparing includes creating if needed and waiting for default service account.
*
* @param canCreate defines what to do when the project is not found. The project is created when
* {@code true}, otherwise an exception is thrown.
Expand Down Expand Up @@ -216,14 +212,6 @@ private void create(String projectName, OpenShiftClient osClient) throws Infrast
.withName(projectName)
.endMetadata()
.build());
Secret secret =
new SecretBuilder()
.withType("opaque")
.withNewMetadata()
.withName(CREDENTIALS_SECRET_NAME)
.endMetadata()
.build();
osClient.secrets().inNamespace(projectName).create(secret);
} catch (KubernetesClientException e) {
if (e.getCode() == 403) {
LOG.error(
Expand Down
23 changes: 23 additions & 0 deletions ...a/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.PHASE_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.CREDENTIALS_SECRET_NAME;

import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.api.model.SecretBuilder;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.openshift.api.model.Project;
import java.util.HashMap;
Expand Down Expand Up @@ -120,6 +123,26 @@ public OpenShiftProject getOrCreate(RuntimeIdentity identity) throws Infrastruct
labelNamespaces ? namespaceLabels : emptyMap(),
annotateNamespaces ? namespaceAnnotationsEvaluated : emptyMap());

// create credentials secret
if (osProject
.secrets()
.get()
.stream()
.noneMatch(s -> s.getMetadata().getName().equals(CREDENTIALS_SECRET_NAME))) {
Secret secret =
new SecretBuilder()
.withType("opaque")
.withNewMetadata()
.withName(CREDENTIALS_SECRET_NAME)
.endMetadata()
.build();
clientFactory
.createOC()
.secrets()
.inNamespace(identity.getInfrastructureNamespace())
.create(secret);
}

if (!isNullOrEmpty(getServiceAccountName())) {
OpenShiftWorkspaceServiceAccount osWorkspaceServiceAccount =
doCreateServiceAccount(osProject.getWorkspaceId(), osProject.getName());
Expand Down
Loading

0 comments on commit 79ba1e5

Please sign in to comment.