From a1d074e7b23fde9edd0eba52e393bb2aecaeb605 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Thu, 28 Oct 2021 17:25:40 +0300 Subject: [PATCH 1/3] feat: Add a predefined secret to store theia preferences Signed-off-by: Igor Vinokur --- .../AbstractWorkspaceServiceAccount.java | 3 +- .../namespace/KubernetesNamespaceFactory.java | 19 +++++++ .../KubernetesNamespaceFactoryTest.java | 54 ++++++++++++++++-- .../project/OpenShiftProjectFactory.java | 21 +++++++ .../project/OpenShiftProjectFactoryTest.java | 57 +++++++++++++++++-- 5 files changed, 144 insertions(+), 10 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java index f2a14c188bd..3f7d95d3f14 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java @@ -51,6 +51,7 @@ public abstract class AbstractWorkspaceServiceAccount< public static final String METRICS_ROLE_NAME = "workspace-metrics"; public static final String SECRETS_ROLE_NAME = "workspace-secrets"; public static final String CREDENTIALS_SECRET_NAME = "workspace-credentials-secret"; + public static final String THEIA_SECRET_NAME = "workspace-theia-secret"; protected final String namespace; protected final String serviceAccountName; @@ -159,7 +160,7 @@ private void ensureImplicitRolesWithBindings(Client k8sClient) { buildRole( SECRETS_ROLE_NAME, singletonList("secrets"), - singletonList(CREDENTIALS_SECRET_NAME), + Arrays.asList(CREDENTIALS_SECRET_NAME, THEIA_SECRET_NAME), singletonList(""), Arrays.asList("get", "patch")), serviceAccountName + "-secrets"); 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 3e0e533f36a..526e4658756 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 @@ -21,6 +21,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.THEIA_SECRET_NAME; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.NamespaceNameValidator.METADATA_NAME_MAX_LENGTH; import com.google.common.annotations.VisibleForTesting; @@ -361,6 +362,24 @@ public KubernetesNamespace getOrCreate(RuntimeIdentity identity) throws Infrastr .inNamespace(identity.getInfrastructureNamespace()) .create(secret); } + if (namespace + .secrets() + .get() + .stream() + .noneMatch(s -> s.getMetadata().getName().equals(THEIA_SECRET_NAME))) { + Secret secret = + new SecretBuilder() + .withType("opaque") + .withNewMetadata() + .withName(THEIA_SECRET_NAME) + .endMetadata() + .build(); + clientFactory + .create() + .secrets() + .inNamespace(identity.getInfrastructureNamespace()) + .create(secret); + } 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 47ed7f0fb69..30b818a2f18 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 @@ -19,6 +19,7 @@ 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.SECRETS_ROLE_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.THEIA_SECRET_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.anyMap; @@ -30,6 +31,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -517,12 +519,53 @@ public void shouldCreateCredentialsSecretIfNotExists() throws Exception { // then ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); - verify(namespaceOperation).create(secretsCaptor.capture()); - Secret secret = secretsCaptor.getValue(); + verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); + Secret secret = secretsCaptor.getAllValues().get(0); Assert.assertEquals(secret.getMetadata().getName(), CREDENTIALS_SECRET_NAME); Assert.assertEquals(secret.getType(), "opaque"); } + @Test + public void shouldCreateTheiaSecretIfNotExists() throws Exception { + // given + namespaceFactory = + spy( + new KubernetesNamespaceFactory( + "", + "", + "-che", + true, + true, + true, + NAMESPACE_LABELS, + NAMESPACE_ANNOTATIONS, + clientFactory, + cheClientFactory, + userManager, + preferenceManager, + pool)); + KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class); + 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); + lenient().when(mixedOperation.inNamespace(anyString())).thenReturn(namespaceOperation); + + // when + RuntimeIdentity identity = + new RuntimeIdentityImpl("workspace123", null, USER_ID, "workspace123"); + namespaceFactory.getOrCreate(identity); + + // then + ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); + verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); + Secret secret = secretsCaptor.getAllValues().get(1); + Assert.assertEquals(secret.getMetadata().getName(), THEIA_SECRET_NAME); + Assert.assertEquals(secret.getType(), "opaque"); + } + @Test public void shouldNotCreateCredentialsSecretIfExists() throws Exception { // given @@ -797,7 +840,7 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception { } @Test - public void shouldCreateAndBindCredentialsSecretRole() throws Exception { + public void shouldCreateAndBindCredentialsAndTheiaSecretsRole() throws Exception { // given namespaceFactory = spy( @@ -841,7 +884,8 @@ public void shouldCreateAndBindCredentialsSecretRole() throws Exception { assertTrue(roleOptional.isPresent()); PolicyRule rule = roleOptional.get().getRules().get(0); assertEquals(rule.getResources(), singletonList("secrets")); - assertEquals(rule.getResourceNames(), singletonList(CREDENTIALS_SECRET_NAME)); + assertEquals( + rule.getResourceNames(), Arrays.asList(CREDENTIALS_SECRET_NAME, THEIA_SECRET_NAME)); assertEquals(rule.getApiGroups(), singletonList("")); assertEquals(rule.getVerbs(), Arrays.asList("get", "patch")); assertTrue( @@ -1523,7 +1567,7 @@ private void prepareNamespace(KubernetesNamespace namespace) throws Infrastructu when(namespace.secrets()).thenReturn(secrets); Secret secretMock = mock(Secret.class); ObjectMeta objectMeta = mock(ObjectMeta.class); - when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME); + when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME, THEIA_SECRET_NAME); when(secretMock.getMetadata()).thenReturn(objectMeta); when(secrets.get()).thenReturn(Collections.singletonList(secretMock)); } 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 547f55c02ac..41ce4bd90d6 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 @@ -17,6 +17,7 @@ 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.THEIA_SECRET_NAME; import com.google.common.annotations.VisibleForTesting; import com.google.inject.Inject; @@ -143,6 +144,26 @@ public OpenShiftProject getOrCreate(RuntimeIdentity identity) throws Infrastruct .create(secret); } + // create theia secret + if (osProject + .secrets() + .get() + .stream() + .noneMatch(s -> s.getMetadata().getName().equals(THEIA_SECRET_NAME))) { + Secret secret = + new SecretBuilder() + .withType("opaque") + .withNewMetadata() + .withName(THEIA_SECRET_NAME) + .endMetadata() + .build(); + clientFactory + .createOC() + .secrets() + .inNamespace(identity.getInfrastructureNamespace()) + .create(secret); + } + if (!isNullOrEmpty(getServiceAccountName())) { OpenShiftWorkspaceServiceAccount osWorkspaceServiceAccount = doCreateServiceAccount(osProject.getWorkspaceId(), osProject.getName()); 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 910adc758c2..637f792d181 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 @@ -17,11 +17,14 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.THEIA_SECRET_NAME; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DESCRIPTION_ANNOTATION; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DESCRIPTION_ATTRIBUTE; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DISPLAY_NAME_ANNOTATION; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DISPLAY_NAME_ATTRIBUTE; -import static org.mockito.ArgumentMatchers.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; @@ -591,12 +594,58 @@ public void shouldCreateCredentialsSecretIfNotExists() throws Exception { // then ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); - verify(namespaceOperation).create(secretsCaptor.capture()); - Secret secret = secretsCaptor.getValue(); + verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); + Secret secret = secretsCaptor.getAllValues().get(0); Assert.assertEquals(secret.getMetadata().getName(), CREDENTIALS_SECRET_NAME); Assert.assertEquals(secret.getType(), "opaque"); } + @Test + public void shouldCreateTheiaPreferencesSecretIfNotExists() throws Exception { + // given + projectFactory = + spy( + new OpenShiftProjectFactory( + "", + null, + "-che", + true, + true, + true, + NAMESPACE_LABELS, + NAMESPACE_ANNOTATIONS, + true, + clientFactory, + cheClientFactory, + cheServerOpenshiftClientFactory, + stopWorkspaceRoleProvisioner, + userManager, + preferenceManager, + pool, + NO_OAUTH_IDENTITY_PROVIDER)); + OpenShiftProject toReturnProject = mock(OpenShiftProject.class); + doReturn(toReturnProject).when(projectFactory).doCreateProjectAccess(any(), any()); + NonNamespaceOperation namespaceOperation = mock(NonNamespaceOperation.class); + MixedOperation mixedOperation = mock(MixedOperation.class); + KubernetesSecrets secrets = mock(KubernetesSecrets.class); + when(toReturnProject.secrets()).thenReturn(secrets); + when(secrets.get()).thenReturn(Collections.emptyList()); + lenient().when(osClient.secrets()).thenReturn(mixedOperation); + lenient().when(mixedOperation.inNamespace(anyString())).thenReturn(namespaceOperation); + + // when + RuntimeIdentity identity = + new RuntimeIdentityImpl("workspace123", null, USER_ID, "workspace123"); + projectFactory.getOrCreate(identity); + + // then + ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); + verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); + Secret secret = secretsCaptor.getAllValues().get(1); + Assert.assertEquals(secret.getMetadata().getName(), THEIA_SECRET_NAME); + Assert.assertEquals(secret.getType(), "opaque"); + } + @Test public void shouldNotCreateCredentialsSecretIfExist() throws Exception { // given @@ -910,7 +959,7 @@ private void prepareProject(OpenShiftProject project) throws InfrastructureExcep when(project.secrets()).thenReturn(secrets); Secret secretMock = mock(Secret.class); ObjectMeta objectMeta = mock(ObjectMeta.class); - when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME); + when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME, THEIA_SECRET_NAME); when(secretMock.getMetadata()).thenReturn(objectMeta); when(secrets.get()).thenReturn(Collections.singletonList(secretMock)); } From df089db26958e2b508066e850c3eff607d62542d Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Tue, 2 Nov 2021 16:08:54 +0200 Subject: [PATCH 2/3] fixup! feat: Add a predefined secret to store theia preferences --- .../namespace/AbstractWorkspaceServiceAccount.java | 4 ++-- .../namespace/KubernetesNamespaceFactory.java | 7 ++++--- .../namespace/KubernetesNamespaceFactoryTest.java | 12 ++++++------ .../openshift/project/OpenShiftProjectFactory.java | 8 ++++---- .../project/OpenShiftProjectFactoryTest.java | 8 ++++---- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java index 3f7d95d3f14..47e29c136d5 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java @@ -51,7 +51,7 @@ public abstract class AbstractWorkspaceServiceAccount< public static final String METRICS_ROLE_NAME = "workspace-metrics"; public static final String SECRETS_ROLE_NAME = "workspace-secrets"; public static final String CREDENTIALS_SECRET_NAME = "workspace-credentials-secret"; - public static final String THEIA_SECRET_NAME = "workspace-theia-secret"; + public static final String PREFERENCES_SECRET_NAME = "workspace-preferences-secret"; protected final String namespace; protected final String serviceAccountName; @@ -160,7 +160,7 @@ private void ensureImplicitRolesWithBindings(Client k8sClient) { buildRole( SECRETS_ROLE_NAME, singletonList("secrets"), - Arrays.asList(CREDENTIALS_SECRET_NAME, THEIA_SECRET_NAME), + Arrays.asList(CREDENTIALS_SECRET_NAME, PREFERENCES_SECRET_NAME), singletonList(""), Arrays.asList("get", "patch")), serviceAccountName + "-secrets"); 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 526e4658756..32b4218a267 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 @@ -21,7 +21,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.THEIA_SECRET_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_SECRET_NAME; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.NamespaceNameValidator.METADATA_NAME_MAX_LENGTH; import com.google.common.annotations.VisibleForTesting; @@ -362,16 +362,17 @@ public KubernetesNamespace getOrCreate(RuntimeIdentity identity) throws Infrastr .inNamespace(identity.getInfrastructureNamespace()) .create(secret); } + if (namespace .secrets() .get() .stream() - .noneMatch(s -> s.getMetadata().getName().equals(THEIA_SECRET_NAME))) { + .noneMatch(s -> s.getMetadata().getName().equals(PREFERENCES_SECRET_NAME))) { Secret secret = new SecretBuilder() .withType("opaque") .withNewMetadata() - .withName(THEIA_SECRET_NAME) + .withName(PREFERENCES_SECRET_NAME) .endMetadata() .build(); clientFactory 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 30b818a2f18..586d844b34b 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 @@ -18,8 +18,8 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_SECRET_NAME; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.SECRETS_ROLE_NAME; -import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.THEIA_SECRET_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.anyMap; @@ -526,7 +526,7 @@ public void shouldCreateCredentialsSecretIfNotExists() throws Exception { } @Test - public void shouldCreateTheiaSecretIfNotExists() throws Exception { + public void shouldCreatePreferencesSecretIfNotExists() throws Exception { // given namespaceFactory = spy( @@ -562,7 +562,7 @@ public void shouldCreateTheiaSecretIfNotExists() throws Exception { ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); Secret secret = secretsCaptor.getAllValues().get(1); - Assert.assertEquals(secret.getMetadata().getName(), THEIA_SECRET_NAME); + Assert.assertEquals(secret.getMetadata().getName(), PREFERENCES_SECRET_NAME); Assert.assertEquals(secret.getType(), "opaque"); } @@ -840,7 +840,7 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception { } @Test - public void shouldCreateAndBindCredentialsAndTheiaSecretsRole() throws Exception { + public void shouldCreateAndBindCredentialsAndPreferencesSecretsRole() throws Exception { // given namespaceFactory = spy( @@ -885,7 +885,7 @@ public void shouldCreateAndBindCredentialsAndTheiaSecretsRole() throws Exception PolicyRule rule = roleOptional.get().getRules().get(0); assertEquals(rule.getResources(), singletonList("secrets")); assertEquals( - rule.getResourceNames(), Arrays.asList(CREDENTIALS_SECRET_NAME, THEIA_SECRET_NAME)); + rule.getResourceNames(), Arrays.asList(CREDENTIALS_SECRET_NAME, PREFERENCES_SECRET_NAME)); assertEquals(rule.getApiGroups(), singletonList("")); assertEquals(rule.getVerbs(), Arrays.asList("get", "patch")); assertTrue( @@ -1567,7 +1567,7 @@ private void prepareNamespace(KubernetesNamespace namespace) throws Infrastructu when(namespace.secrets()).thenReturn(secrets); Secret secretMock = mock(Secret.class); ObjectMeta objectMeta = mock(ObjectMeta.class); - when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME, THEIA_SECRET_NAME); + when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME, PREFERENCES_SECRET_NAME); when(secretMock.getMetadata()).thenReturn(objectMeta); when(secrets.get()).thenReturn(Collections.singletonList(secretMock)); } 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 41ce4bd90d6..6dc7a4aa9de 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 @@ -17,7 +17,7 @@ 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.THEIA_SECRET_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_SECRET_NAME; import com.google.common.annotations.VisibleForTesting; import com.google.inject.Inject; @@ -144,17 +144,17 @@ public OpenShiftProject getOrCreate(RuntimeIdentity identity) throws Infrastruct .create(secret); } - // create theia secret + // create preferences secret if (osProject .secrets() .get() .stream() - .noneMatch(s -> s.getMetadata().getName().equals(THEIA_SECRET_NAME))) { + .noneMatch(s -> s.getMetadata().getName().equals(PREFERENCES_SECRET_NAME))) { Secret secret = new SecretBuilder() .withType("opaque") .withNewMetadata() - .withName(THEIA_SECRET_NAME) + .withName(PREFERENCES_SECRET_NAME) .endMetadata() .build(); clientFactory 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 637f792d181..1bad47859ce 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 @@ -17,7 +17,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.THEIA_SECRET_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_SECRET_NAME; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DESCRIPTION_ANNOTATION; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DESCRIPTION_ATTRIBUTE; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DISPLAY_NAME_ANNOTATION; @@ -601,7 +601,7 @@ public void shouldCreateCredentialsSecretIfNotExists() throws Exception { } @Test - public void shouldCreateTheiaPreferencesSecretIfNotExists() throws Exception { + public void shouldCreatePreferencesSecretIfNotExists() throws Exception { // given projectFactory = spy( @@ -642,7 +642,7 @@ public void shouldCreateTheiaPreferencesSecretIfNotExists() throws Exception { ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); Secret secret = secretsCaptor.getAllValues().get(1); - Assert.assertEquals(secret.getMetadata().getName(), THEIA_SECRET_NAME); + Assert.assertEquals(secret.getMetadata().getName(), PREFERENCES_SECRET_NAME); Assert.assertEquals(secret.getType(), "opaque"); } @@ -959,7 +959,7 @@ private void prepareProject(OpenShiftProject project) throws InfrastructureExcep when(project.secrets()).thenReturn(secrets); Secret secretMock = mock(Secret.class); ObjectMeta objectMeta = mock(ObjectMeta.class); - when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME, THEIA_SECRET_NAME); + when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME, PREFERENCES_SECRET_NAME); when(secretMock.getMetadata()).thenReturn(objectMeta); when(secrets.get()).thenReturn(Collections.singletonList(secretMock)); } From 2fb5920ce64ddc431578616153fda0909b4c75e9 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Thu, 18 Nov 2021 15:15:43 +0200 Subject: [PATCH 3/3] fixup! feat: Add a predefined secret to store theia preferences --- .../AbstractWorkspaceServiceAccount.java | 16 +- .../namespace/KubernetesNamespaceFactory.java | 21 ++- .../KubernetesNamespaceFactoryTest.java | 151 ++++++++---------- ...KubernetesWorkspaceServiceAccountTest.java | 68 ++++++++ .../project/OpenShiftProjectFactory.java | 23 ++- .../project/OpenShiftProjectFactoryTest.java | 81 ++++++++-- 6 files changed, 240 insertions(+), 120 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java index 47e29c136d5..39c097ef6bd 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/AbstractWorkspaceServiceAccount.java @@ -50,8 +50,9 @@ public abstract class AbstractWorkspaceServiceAccount< public static final String VIEW_ROLE_NAME = "workspace-view"; public static final String METRICS_ROLE_NAME = "workspace-metrics"; public static final String SECRETS_ROLE_NAME = "workspace-secrets"; + public static final String CONFIGMAPS_ROLE_NAME = "workspace-configmaps"; public static final String CREDENTIALS_SECRET_NAME = "workspace-credentials-secret"; - public static final String PREFERENCES_SECRET_NAME = "workspace-preferences-secret"; + public static final String PREFERENCES_CONFIGMAP_NAME = "workspace-preferences-configmap"; protected final String namespace; protected final String serviceAccountName; @@ -160,10 +161,21 @@ private void ensureImplicitRolesWithBindings(Client k8sClient) { buildRole( SECRETS_ROLE_NAME, singletonList("secrets"), - Arrays.asList(CREDENTIALS_SECRET_NAME, PREFERENCES_SECRET_NAME), + singletonList(CREDENTIALS_SECRET_NAME), singletonList(""), Arrays.asList("get", "patch")), serviceAccountName + "-secrets"); + + // preferences-configmap role + ensureRoleWithBinding( + k8sClient, + buildRole( + CONFIGMAPS_ROLE_NAME, + singletonList("configmaps"), + singletonList(PREFERENCES_CONFIGMAP_NAME), + singletonList(""), + Arrays.asList("get", "patch")), + serviceAccountName + "-configmaps"); } private void ensureRoleWithBinding(Client k8sClient, R role, String bindingName) { 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 32b4218a267..79acb00ccf5 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 @@ -21,7 +21,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_SECRET_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_CONFIGMAP_NAME; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.NamespaceNameValidator.METADATA_NAME_MAX_LENGTH; import com.google.common.annotations.VisibleForTesting; @@ -30,6 +30,8 @@ import com.google.common.collect.Sets; import com.google.inject.Inject; import com.google.inject.Singleton; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Namespace; import io.fabric8.kubernetes.api.model.Secret; @@ -363,23 +365,18 @@ public KubernetesNamespace getOrCreate(RuntimeIdentity identity) throws Infrastr .create(secret); } - if (namespace - .secrets() - .get() - .stream() - .noneMatch(s -> s.getMetadata().getName().equals(PREFERENCES_SECRET_NAME))) { - Secret secret = - new SecretBuilder() - .withType("opaque") + if (namespace.configMaps().get(PREFERENCES_CONFIGMAP_NAME).isEmpty()) { + ConfigMap configMap = + new ConfigMapBuilder() .withNewMetadata() - .withName(PREFERENCES_SECRET_NAME) + .withName(PREFERENCES_CONFIGMAP_NAME) .endMetadata() .build(); clientFactory .create() - .secrets() + .configMaps() .inNamespace(identity.getInfrastructureNamespace()) - .create(secret); + .create(configMap); } if (!isNullOrEmpty(serviceAccountName)) { 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 586d844b34b..ab379d0a331 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 @@ -14,12 +14,12 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; +import static java.util.Optional.empty; import static org.eclipse.che.api.workspace.shared.Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE; import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_SECRET_NAME; -import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.SECRETS_ROLE_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_CONFIGMAP_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.anyMap; @@ -31,7 +31,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -43,6 +42,7 @@ import ch.qos.logback.classic.spi.LoggingEvent; import ch.qos.logback.core.Appender; import com.google.common.collect.ImmutableMap; +import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.Namespace; import io.fabric8.kubernetes.api.model.NamespaceBuilder; import io.fabric8.kubernetes.api.model.NamespaceList; @@ -51,7 +51,6 @@ import io.fabric8.kubernetes.api.model.ServiceAccountList; import io.fabric8.kubernetes.api.model.Status; import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBuilder; -import io.fabric8.kubernetes.api.model.rbac.PolicyRule; import io.fabric8.kubernetes.api.model.rbac.Role; import io.fabric8.kubernetes.api.model.rbac.RoleBindingList; import io.fabric8.kubernetes.api.model.rbac.RoleList; @@ -506,6 +505,7 @@ public void shouldCreateCredentialsSecretIfNotExists() throws Exception { KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class); KubernetesSecrets secrets = mock(KubernetesSecrets.class); when(toReturnNamespace.secrets()).thenReturn(secrets); + when(toReturnNamespace.configMaps()).thenReturn(mock(KubernetesConfigsMaps.class)); when(secrets.get()).thenReturn(Collections.emptyList()); doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any()); MixedOperation mixedOperation = mock(MixedOperation.class); @@ -519,14 +519,14 @@ public void shouldCreateCredentialsSecretIfNotExists() throws Exception { // then ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); - verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); - Secret secret = secretsCaptor.getAllValues().get(0); + verify(namespaceOperation).create(secretsCaptor.capture()); + Secret secret = secretsCaptor.getValue(); Assert.assertEquals(secret.getMetadata().getName(), CREDENTIALS_SECRET_NAME); Assert.assertEquals(secret.getType(), "opaque"); } @Test - public void shouldCreatePreferencesSecretIfNotExists() throws Exception { + public void shouldCreatePreferencesConfigmapIfNotExists() throws Exception { // given namespaceFactory = spy( @@ -545,12 +545,13 @@ public void shouldCreatePreferencesSecretIfNotExists() throws Exception { preferenceManager, pool)); KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class); - KubernetesSecrets secrets = mock(KubernetesSecrets.class); - when(toReturnNamespace.secrets()).thenReturn(secrets); - when(secrets.get()).thenReturn(Collections.emptyList()); + KubernetesConfigsMaps configsMaps = mock(KubernetesConfigsMaps.class); + when(toReturnNamespace.secrets()).thenReturn(mock(KubernetesSecrets.class)); + when(toReturnNamespace.configMaps()).thenReturn(configsMaps); + when(configsMaps.get(eq(PREFERENCES_CONFIGMAP_NAME))).thenReturn(empty()); doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any()); MixedOperation mixedOperation = mock(MixedOperation.class); - lenient().when(k8sClient.secrets()).thenReturn(mixedOperation); + lenient().when(k8sClient.configMaps()).thenReturn(mixedOperation); lenient().when(mixedOperation.inNamespace(anyString())).thenReturn(namespaceOperation); // when @@ -559,11 +560,10 @@ public void shouldCreatePreferencesSecretIfNotExists() throws Exception { namespaceFactory.getOrCreate(identity); // then - ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); - verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); - Secret secret = secretsCaptor.getAllValues().get(1); - Assert.assertEquals(secret.getMetadata().getName(), PREFERENCES_SECRET_NAME); - Assert.assertEquals(secret.getType(), "opaque"); + ArgumentCaptor configMapCaptor = ArgumentCaptor.forClass(ConfigMap.class); + verify(namespaceOperation).create(configMapCaptor.capture()); + ConfigMap configmap = configMapCaptor.getValue(); + Assert.assertEquals(configmap.getMetadata().getName(), PREFERENCES_CONFIGMAP_NAME); } @Test @@ -601,6 +601,41 @@ public void shouldNotCreateCredentialsSecretIfExists() throws Exception { verify(namespaceOperation, never()).create(any()); } + @Test + public void shouldNotCreatePreferencesConfigmapIfExists() throws Exception { + // given + namespaceFactory = + spy( + new KubernetesNamespaceFactory( + "", + "", + "-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.configMaps()).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 = @@ -822,7 +857,12 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception { RoleList roles = k8sClient.rbac().roles().inNamespace("workspace123").list(); assertEquals( roles.getItems().stream().map(r -> r.getMetadata().getName()).collect(Collectors.toSet()), - Sets.newHashSet("workspace-view", "workspace-metrics", "workspace-secrets", "exec")); + Sets.newHashSet( + "workspace-configmaps", + "workspace-view", + "workspace-metrics", + "workspace-secrets", + "exec")); RoleBindingList bindings = k8sClient.rbac().roleBindings().inNamespace("workspace123").list(); assertEquals( bindings @@ -834,71 +874,12 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception { "serviceAccount-metrics", "serviceAccount-cluster0", "serviceAccount-cluster1", + "serviceAccount-configmaps", "serviceAccount-view", "serviceAccount-exec", "serviceAccount-secrets")); } - @Test - public void shouldCreateAndBindCredentialsAndPreferencesSecretsRole() throws Exception { - // given - namespaceFactory = - spy( - new KubernetesNamespaceFactory( - "serviceAccount", - "cr2, cr3", - "-che", - true, - true, - true, - NAMESPACE_LABELS, - NAMESPACE_ANNOTATIONS, - clientFactory, - cheClientFactory, - userManager, - 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()); - when(clientFactory.create(any())).thenReturn(k8sClient); - - // when - RuntimeIdentity identity = - new RuntimeIdentityImpl("workspace123", null, USER_ID, "workspace123"); - namespaceFactory.getOrCreate(identity); - - // then - Optional roleOptional = - k8sClient - .rbac() - .roles() - .inNamespace("workspace123") - .list() - .getItems() - .stream() - .filter(r -> r.getMetadata().getName().equals(SECRETS_ROLE_NAME)) - .findAny(); - assertTrue(roleOptional.isPresent()); - PolicyRule rule = roleOptional.get().getRules().get(0); - assertEquals(rule.getResources(), singletonList("secrets")); - assertEquals( - rule.getResourceNames(), Arrays.asList(CREDENTIALS_SECRET_NAME, PREFERENCES_SECRET_NAME)); - assertEquals(rule.getApiGroups(), singletonList("")); - assertEquals(rule.getVerbs(), Arrays.asList("get", "patch")); - assertTrue( - k8sClient - .rbac() - .roleBindings() - .inNamespace("workspace123") - .list() - .getItems() - .stream() - .anyMatch(rb -> rb.getMetadata().getName().equals("serviceAccount-secrets"))); - } - @Test public void shouldCreateExecAndViewRolesAndBindings() throws Exception { // given @@ -941,7 +922,12 @@ public void shouldCreateExecAndViewRolesAndBindings() throws Exception { RoleList roles = k8sClient.rbac().roles().inNamespace("workspace123").list(); assertEquals( roles.getItems().stream().map(r -> r.getMetadata().getName()).collect(Collectors.toSet()), - Sets.newHashSet("workspace-view", "workspace-metrics", "workspace-secrets", "exec")); + Sets.newHashSet( + "workspace-configmaps", + "workspace-view", + "workspace-metrics", + "workspace-secrets", + "exec")); Role role1 = roles.getItems().get(0); Role role2 = roles.getItems().get(1); @@ -961,6 +947,7 @@ public void shouldCreateExecAndViewRolesAndBindings() throws Exception { "serviceAccount-metrics", "serviceAccount-view", "serviceAccount-exec", + "serviceAccount-configmaps", "serviceAccount-secrets")); } @@ -1146,7 +1133,7 @@ public void testEvalNamespaceKubeAdmin() throws Exception { userManager, preferenceManager, pool)); - doReturn(Optional.empty()).when(namespaceFactory).fetchNamespace(anyString()); + doReturn(empty()).when(namespaceFactory).fetchNamespace(anyString()); String namespace = namespaceFactory.evaluateNamespaceName( @@ -1329,7 +1316,7 @@ public void shouldFailToProvisionIfNotAbleToFindNamespace() throws Infrastructur KubernetesNamespaceMetaImpl namespaceMeta = new KubernetesNamespaceMetaImpl( "jondoe-cha-cha-cha", ImmutableMap.of("phase", "active", "default", "true")); - doReturn(Optional.empty()).when(namespaceFactory).fetchNamespace(eq("jondoe-cha-cha-cha")); + doReturn(empty()).when(namespaceFactory).fetchNamespace(eq("jondoe-cha-cha-cha")); // when NamespaceResolutionContext context = @@ -1564,10 +1551,12 @@ private void throwOnTryToGetNamespacesList(Throwable e) throws Exception { private void prepareNamespace(KubernetesNamespace namespace) throws InfrastructureException { KubernetesSecrets secrets = mock(KubernetesSecrets.class); + KubernetesConfigsMaps configmaps = mock(KubernetesConfigsMaps.class); when(namespace.secrets()).thenReturn(secrets); + when(namespace.configMaps()).thenReturn(configmaps); Secret secretMock = mock(Secret.class); ObjectMeta objectMeta = mock(ObjectMeta.class); - when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME, PREFERENCES_SECRET_NAME); + when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME); when(secretMock.getMetadata()).thenReturn(objectMeta); when(secrets.get()).thenReturn(Collections.singletonList(secretMock)); } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesWorkspaceServiceAccountTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesWorkspaceServiceAccountTest.java index a66099298e9..47d1c4c215a 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesWorkspaceServiceAccountTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesWorkspaceServiceAccountTest.java @@ -11,21 +11,31 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.namespace; +import static java.util.Collections.singletonList; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.CONFIGMAPS_ROLE_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.CREDENTIALS_SECRET_NAME; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.METRICS_ROLE_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_CONFIGMAP_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.SECRETS_ROLE_NAME; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; import io.fabric8.kubernetes.api.model.ServiceAccountBuilder; +import io.fabric8.kubernetes.api.model.rbac.PolicyRule; +import io.fabric8.kubernetes.api.model.rbac.Role; import io.fabric8.kubernetes.api.model.rbac.RoleBindingBuilder; import io.fabric8.kubernetes.api.model.rbac.RoleBindingList; import io.fabric8.kubernetes.api.model.rbac.RoleBuilder; import io.fabric8.kubernetes.api.model.rbac.RoleList; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.server.mock.KubernetesServer; +import java.util.Arrays; import java.util.Collections; +import java.util.Optional; import java.util.Set; import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory; import org.mockito.Mock; @@ -131,4 +141,62 @@ public void shouldNotCreateMetricsRoleIfAPINotEnabledOnServer() throws Exception .stream() .noneMatch(rb -> rb.getMetadata().getName().equals(SA_NAME + "-metrics"))); } + + @Test + public void shouldCreateCredentialsSecretRole() throws Exception { + KubernetesClient localK8sClient = spy(serverMock.getClient()); + when(clientFactory.create(anyString())).thenReturn(localK8sClient); + + // when + serviceAccount.prepare(); + + // then + RoleList rl = k8sClient.rbac().roles().inNamespace(NAMESPACE).list(); + Optional roleOptional = + rl.getItems() + .stream() + .filter(r -> r.getMetadata().getName().equals(SECRETS_ROLE_NAME)) + .findFirst(); + assertTrue(roleOptional.isPresent()); + PolicyRule rule = roleOptional.get().getRules().get(0); + assertEquals(rule.getResources(), singletonList("secrets")); + assertEquals(rule.getResourceNames(), singletonList(CREDENTIALS_SECRET_NAME)); + assertEquals(rule.getApiGroups(), singletonList("")); + assertEquals(rule.getVerbs(), Arrays.asList("get", "patch")); + + RoleBindingList rbl = k8sClient.rbac().roleBindings().inNamespace(NAMESPACE).list(); + assertTrue( + rbl.getItems() + .stream() + .anyMatch(rb -> rb.getMetadata().getName().equals(SA_NAME + "-secrets"))); + } + + @Test + public void shouldCreatePreferencesConfigmapRole() throws Exception { + KubernetesClient localK8sClient = spy(serverMock.getClient()); + when(clientFactory.create(anyString())).thenReturn(localK8sClient); + + // when + serviceAccount.prepare(); + + // then + RoleList rl = k8sClient.rbac().roles().inNamespace(NAMESPACE).list(); + Optional roleOptional = + rl.getItems() + .stream() + .filter(r -> r.getMetadata().getName().equals(CONFIGMAPS_ROLE_NAME)) + .findFirst(); + assertTrue(roleOptional.isPresent()); + PolicyRule rule = roleOptional.get().getRules().get(0); + assertEquals(rule.getResources(), singletonList("configmaps")); + assertEquals(rule.getResourceNames(), singletonList(PREFERENCES_CONFIGMAP_NAME)); + assertEquals(rule.getApiGroups(), singletonList("")); + assertEquals(rule.getVerbs(), Arrays.asList("get", "patch")); + + RoleBindingList rbl = k8sClient.rbac().roleBindings().inNamespace(NAMESPACE).list(); + assertTrue( + rbl.getItems() + .stream() + .anyMatch(rb -> rb.getMetadata().getName().equals(SA_NAME + "-configmaps"))); + } } 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 6dc7a4aa9de..604c011dd87 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 @@ -17,11 +17,13 @@ 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_SECRET_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_CONFIGMAP_NAME; import com.google.common.annotations.VisibleForTesting; import com.google.inject.Inject; import com.google.inject.Singleton; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.kubernetes.api.model.SecretBuilder; @@ -144,24 +146,19 @@ public OpenShiftProject getOrCreate(RuntimeIdentity identity) throws Infrastruct .create(secret); } - // create preferences secret - if (osProject - .secrets() - .get() - .stream() - .noneMatch(s -> s.getMetadata().getName().equals(PREFERENCES_SECRET_NAME))) { - Secret secret = - new SecretBuilder() - .withType("opaque") + // create preferences configmap + if (osProject.configMaps().get(PREFERENCES_CONFIGMAP_NAME).isEmpty()) { + ConfigMap configMap = + new ConfigMapBuilder() .withNewMetadata() - .withName(PREFERENCES_SECRET_NAME) + .withName(PREFERENCES_CONFIGMAP_NAME) .endMetadata() .build(); clientFactory .createOC() - .secrets() + .configMaps() .inNamespace(identity.getInfrastructureNamespace()) - .create(secret); + .create(configMap); } if (!isNullOrEmpty(getServiceAccountName())) { 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 1bad47859ce..e7dfa272339 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 @@ -14,10 +14,11 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; +import static java.util.Optional.empty; import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE; 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 static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_SECRET_NAME; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_CONFIGMAP_NAME; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DESCRIPTION_ANNOTATION; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DESCRIPTION_ATTRIBUTE; import static org.eclipse.che.workspace.infrastructure.openshift.Constants.PROJECT_DISPLAY_NAME_ANNOTATION; @@ -38,6 +39,7 @@ import static org.testng.Assert.assertNull; import com.google.common.collect.ImmutableMap; +import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.kubernetes.api.model.Status; @@ -56,6 +58,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import org.eclipse.che.api.core.ValidationException; import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; import org.eclipse.che.api.user.server.PreferenceManager; @@ -71,6 +74,7 @@ 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.namespace.KubernetesConfigsMaps; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesSecrets; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.openshift.CheServerOpenshiftClientFactory; @@ -582,8 +586,11 @@ public void shouldCreateCredentialsSecretIfNotExists() throws Exception { NonNamespaceOperation namespaceOperation = mock(NonNamespaceOperation.class); MixedOperation mixedOperation = mock(MixedOperation.class); KubernetesSecrets secrets = mock(KubernetesSecrets.class); + KubernetesConfigsMaps configsMaps = mock(KubernetesConfigsMaps.class); when(toReturnProject.secrets()).thenReturn(secrets); + when(toReturnProject.configMaps()).thenReturn(configsMaps); when(secrets.get()).thenReturn(Collections.emptyList()); + when(configsMaps.get(anyString())).thenReturn(Optional.of(mock(ConfigMap.class))); lenient().when(osClient.secrets()).thenReturn(mixedOperation); lenient().when(mixedOperation.inNamespace(anyString())).thenReturn(namespaceOperation); @@ -594,14 +601,14 @@ public void shouldCreateCredentialsSecretIfNotExists() throws Exception { // then ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); - verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); - Secret secret = secretsCaptor.getAllValues().get(0); + verify(namespaceOperation).create(secretsCaptor.capture()); + Secret secret = secretsCaptor.getValue(); Assert.assertEquals(secret.getMetadata().getName(), CREDENTIALS_SECRET_NAME); Assert.assertEquals(secret.getType(), "opaque"); } @Test - public void shouldCreatePreferencesSecretIfNotExists() throws Exception { + public void shouldCreatePreferencesConfigmapIfNotExists() throws Exception { // given projectFactory = spy( @@ -628,9 +635,17 @@ public void shouldCreatePreferencesSecretIfNotExists() throws Exception { NonNamespaceOperation namespaceOperation = mock(NonNamespaceOperation.class); MixedOperation mixedOperation = mock(MixedOperation.class); KubernetesSecrets secrets = mock(KubernetesSecrets.class); + Secret secret = mock(Secret.class); + ObjectMeta objectMeta = mock(ObjectMeta.class); + when(secret.getMetadata()).thenReturn(objectMeta); + when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME); when(toReturnProject.secrets()).thenReturn(secrets); - when(secrets.get()).thenReturn(Collections.emptyList()); + when(secrets.get()).thenReturn(singletonList(secret)); lenient().when(osClient.secrets()).thenReturn(mixedOperation); + KubernetesConfigsMaps configsMaps = mock(KubernetesConfigsMaps.class); + when(toReturnProject.configMaps()).thenReturn(configsMaps); + when(configsMaps.get(eq(PREFERENCES_CONFIGMAP_NAME))).thenReturn(empty()); + lenient().when(osClient.configMaps()).thenReturn(mixedOperation); lenient().when(mixedOperation.inNamespace(anyString())).thenReturn(namespaceOperation); // when @@ -639,11 +654,10 @@ public void shouldCreatePreferencesSecretIfNotExists() throws Exception { projectFactory.getOrCreate(identity); // then - ArgumentCaptor secretsCaptor = ArgumentCaptor.forClass(Secret.class); - verify(namespaceOperation, times(2)).create(secretsCaptor.capture()); - Secret secret = secretsCaptor.getAllValues().get(1); - Assert.assertEquals(secret.getMetadata().getName(), PREFERENCES_SECRET_NAME); - Assert.assertEquals(secret.getType(), "opaque"); + ArgumentCaptor configMapCaptor = ArgumentCaptor.forClass(ConfigMap.class); + verify(namespaceOperation).create(configMapCaptor.capture()); + ConfigMap configmap = configMapCaptor.getValue(); + Assert.assertEquals(configmap.getMetadata().getName(), PREFERENCES_CONFIGMAP_NAME); } @Test @@ -686,6 +700,46 @@ public void shouldNotCreateCredentialsSecretIfExist() throws Exception { verify(namespaceOperation, never()).create(any()); } + @Test + public void shouldNotCreatePreferencesConfigmapIfExist() throws Exception { + // given + projectFactory = + spy( + new OpenShiftProjectFactory( + "", + null, + "-che", + true, + true, + true, + NAMESPACE_LABELS, + NAMESPACE_ANNOTATIONS, + true, + clientFactory, + cheClientFactory, + cheServerOpenshiftClientFactory, + stopWorkspaceRoleProvisioner, + userManager, + preferenceManager, + pool, + NO_OAUTH_IDENTITY_PROVIDER)); + OpenShiftProject toReturnProject = mock(OpenShiftProject.class); + prepareProject(toReturnProject); + doReturn(toReturnProject).when(projectFactory).doCreateProjectAccess(any(), any()); + NonNamespaceOperation namespaceOperation = mock(NonNamespaceOperation.class); + MixedOperation mixedOperation = mock(MixedOperation.class); + lenient().when(osClient.configMaps()).thenReturn(mixedOperation); + lenient().when(mixedOperation.inNamespace(anyString())).thenReturn(namespaceOperation); + + // when + RuntimeIdentity identity = + new RuntimeIdentityImpl("workspace123", null, USER_ID, "workspace123"); + projectFactory.getOrCreate(identity); + + // then + verify(namespaceOperation, never()).create(any()); + } + @Test public void shouldPrepareWorkspaceServiceAccountIfItIsConfiguredAndProjectIsNotPredefined() throws Exception { @@ -956,12 +1010,15 @@ private void prepareListedProjects(List projects) throws Exception { private void prepareProject(OpenShiftProject project) throws InfrastructureException { KubernetesSecrets secrets = mock(KubernetesSecrets.class); + KubernetesConfigsMaps configsMaps = mock(KubernetesConfigsMaps.class); when(project.secrets()).thenReturn(secrets); + when(project.configMaps()).thenReturn(configsMaps); + when(configsMaps.get(anyString())).thenReturn(Optional.of(mock(ConfigMap.class))); Secret secretMock = mock(Secret.class); ObjectMeta objectMeta = mock(ObjectMeta.class); - when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME, PREFERENCES_SECRET_NAME); + when(objectMeta.getName()).thenReturn(CREDENTIALS_SECRET_NAME); when(secretMock.getMetadata()).thenReturn(objectMeta); - when(secrets.get()).thenReturn(Collections.singletonList(secretMock)); + when(secrets.get()).thenReturn(singletonList(secretMock)); } private void throwOnTryToGetProjectsList(Throwable e) throws Exception {