From 3718fe47b45f7f57540438979a55f8f8f9cb7731 Mon Sep 17 00:00:00 2001 From: Michal Vala Date: Wed, 2 Sep 2020 15:05:27 +0200 Subject: [PATCH 1/5] make single-host gateway configmap labels configurable Signed-off-by: Michal Vala --- .../webapp/WEB-INF/classes/che/che.properties | 3 + .../provision/GatewayRouterProvisioner.java | 34 ++-------- .../provision/GatewayTlsProvisioner.java | 11 +++- .../external/GatewayRouteConfigGenerator.java | 7 +- .../server/external/GatewayServerExposer.java | 9 ++- .../TraefikGatewayRouteConfigGenerator.java | 12 +--- .../util/GatewayConfigmapLabels.java | 62 +++++++++++++++++ .../KubernetesInternalRuntimeTest.java | 3 - .../GatewayRouterProvisionerTest.java | 40 ++--------- .../provision/GatewayTlsProvisionerTest.java | 16 +++-- .../external/GatewayServerExposerTest.java | 27 +++++++- ...raefikGatewayRouteConfigGeneratorTest.java | 11 ---- .../util/GatewayConfigmapLabelsTest.java | 66 +++++++++++++++++++ 13 files changed, 200 insertions(+), 101 deletions(-) create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index db944f4c471..ad3941a4de6 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -256,6 +256,9 @@ che.infra.kubernetes.server_strategy=multi-host # - 'native': Exposes servers using k8s Ingresses. Works only on Kubernetes. che.infra.kubernetes.single_host.workspace.exposure=native +# Defines labels which will be set to ConfigMaps configuring single-host gateway. +che.infra.kubernetes.single_host.gateway.configmap.labels=app=che,component=che-gateway-config + # Used to generate domain for a server in a workspace in case property `che.infra.kubernetes.server_strategy` is set to `multi-host` che.infra.kubernetes.ingress.domain= diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisioner.java index e3c6b2fea0e..52cd57fa30d 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisioner.java @@ -13,9 +13,7 @@ import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_NAME_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_PORT_ATTRIBUTE; -import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.isLabeled; -import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.ConfigMap; import java.util.Map; import java.util.Map.Entry; @@ -27,6 +25,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.GatewayRouteConfigGenerator; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.GatewayRouteConfigGeneratorFactory; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.GatewayConfigmapLabels; /** * This provisioner finds {@link ConfigMap}s, that configures the single-host Gateway, generates @@ -36,25 +35,22 @@ */ public class GatewayRouterProvisioner implements ConfigurationProvisioner { - /** Configmap labeled with these holds the configuration of single-host gateway route */ - public static final Map GATEWAY_CONFIGMAP_LABELS = - ImmutableMap.builder() - .put("app", "che") - .put("role", "gateway-config") - .build(); - private final GatewayRouteConfigGeneratorFactory configGeneratorFactory; + private final GatewayConfigmapLabels configmapLabels; @Inject - public GatewayRouterProvisioner(GatewayRouteConfigGeneratorFactory configGeneratorFactory) { + public GatewayRouterProvisioner( + GatewayRouteConfigGeneratorFactory configGeneratorFactory, + GatewayConfigmapLabels configmapLabels) { this.configGeneratorFactory = configGeneratorFactory; + this.configmapLabels = configmapLabels; } @Override public void provision(KubernetesEnvironment k8sEnv, RuntimeIdentity identity) throws InfrastructureException { for (Entry configMapEntry : k8sEnv.getConfigMaps().entrySet()) { - if (isGatewayConfig(configMapEntry.getValue())) { + if (configmapLabels.isGatewayConfig(configMapEntry.getValue())) { ConfigMap gatewayConfigMap = configMapEntry.getValue(); Map servers = @@ -98,20 +94,4 @@ public void provision(KubernetesEnvironment k8sEnv, RuntimeIdentity identity) } } } - - /** - * Check whether configmap is gateway route configuration. That is defined by {@link - * GatewayRouterProvisioner#GATEWAY_CONFIGMAP_LABELS} labels. - * - * @param configMap to check - * @return `true` if ConfigMap is gateway route configuration, `false` otherwise - */ - public static boolean isGatewayConfig(ConfigMap configMap) { - for (Entry labelEntry : GATEWAY_CONFIGMAP_LABELS.entrySet()) { - if (!isLabeled(configMap, labelEntry.getKey(), labelEntry.getValue())) { - return false; - } - } - return true; - } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayTlsProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayTlsProvisioner.java index 882506f60a0..f40491b9fc3 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayTlsProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayTlsProvisioner.java @@ -24,6 +24,7 @@ import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.GatewayConfigmapLabels; /** * Enables Transport Layer Security (TLS) for external server implemented with gateway ConfigMaps. @@ -32,11 +33,15 @@ public class GatewayTlsProvisioner implements ConfigurationProvisioner, TlsProvisioner { - final boolean isTlsEnabled; + private final boolean isTlsEnabled; + private final GatewayConfigmapLabels configmapLabels; @Inject - public GatewayTlsProvisioner(@Named("che.infra.kubernetes.tls_enabled") boolean isTlsEnabled) { + public GatewayTlsProvisioner( + @Named("che.infra.kubernetes.tls_enabled") boolean isTlsEnabled, + GatewayConfigmapLabels configmapLabels) { this.isTlsEnabled = isTlsEnabled; + this.configmapLabels = configmapLabels; } @Override @@ -46,7 +51,7 @@ public void provision(T k8sEnv, RuntimeIdentity identity) throws InfrastructureE } for (ConfigMap configMap : k8sEnv.getConfigMaps().values()) { - if (GatewayRouterProvisioner.isGatewayConfig(configMap)) { + if (configmapLabels.isGatewayConfig(configMap)) { useSecureProtocolForGatewayConfigMap(configMap); } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayRouteConfigGenerator.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayRouteConfigGenerator.java index 5576cd7eafe..215762d34ee 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayRouteConfigGenerator.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayRouteConfigGenerator.java @@ -26,11 +26,12 @@ public interface GatewayRouteConfigGenerator { * Add prepared {@link ConfigMap},that will hold gateway route configuration, to the generator. So * it can be generated later with {@link GatewayRouteConfigGenerator#generate(String)}. * - *

Provided {@link ConfigMap} must be properly labeled and must be annotated with {@link - * org.eclipse.che.api.core.model.workspace.config.ServerConfig} annotations. + *

Provided {@link ConfigMap} must be annotated with {@link + * org.eclipse.che.api.core.model.workspace.config.ServerConfig} annotations. It's responsibility + * of the caller to ensure that. The {@link GatewayRouteConfigGenerator} fails on {@link + * GatewayRouteConfigGenerator#generate(String)} when invalid {@link ConfigMap}s are added to it. * * @param routeConfig config to add - * @throws InfrastructureException when passed ConfigMap is not gateway configuration ConfigMap */ void addRouteConfig(String name, ConfigMap routeConfig) throws InfrastructureException; diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayServerExposer.java index 60b1387eb79..69a5124d97c 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayServerExposer.java @@ -27,8 +27,8 @@ import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; -import org.eclipse.che.workspace.infrastructure.kubernetes.provision.GatewayRouterProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.GatewayConfigmapLabels; /** * Uses gateway configured with ConfigMaps to expose servers. @@ -39,10 +39,13 @@ public class GatewayServerExposer implements ExternalServerExposer { private final ExternalServiceExposureStrategy strategy; + private final GatewayConfigmapLabels configmapLabels; @Inject - public GatewayServerExposer(ExternalServiceExposureStrategy strategy) { + public GatewayServerExposer( + ExternalServiceExposureStrategy strategy, GatewayConfigmapLabels configmapLabels) { this.strategy = strategy; + this.configmapLabels = configmapLabels; } /** @@ -115,7 +118,7 @@ private ConfigMap createGatewayRouteConfigmap( new ConfigMapBuilder() .withNewMetadata() .withName(name) - .withLabels(GatewayRouterProvisioner.GATEWAY_CONFIGMAP_LABELS) + .withLabels(configmapLabels.getLabels()) .withAnnotations(annotations) .endMetadata(); return gatewayConfigMap.build(); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/TraefikGatewayRouteConfigGenerator.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/TraefikGatewayRouteConfigGenerator.java index fd3e342ecbd..8a6061e7725 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/TraefikGatewayRouteConfigGenerator.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/TraefikGatewayRouteConfigGenerator.java @@ -14,7 +14,6 @@ import static com.fasterxml.jackson.dataformat.yaml.YAMLGenerator.Feature.WRITE_DOC_START_MARKER; import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_NAME_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_PORT_ATTRIBUTE; -import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.GatewayRouterProvisioner.isGatewayConfig; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; @@ -65,15 +64,8 @@ public class TraefikGatewayRouteConfigGenerator implements GatewayRouteConfigGen private final Map routeConfigs = new HashMap<>(); @Override - public void addRouteConfig(String name, ConfigMap routeConfig) throws InfrastructureException { - if (isGatewayConfig(routeConfig)) { - this.routeConfigs.put(name, routeConfig); - } else { - throw new InfrastructureException( - "Not a gateway configuration ConfigMap '" - + routeConfig.getMetadata().getName() - + "'. This is a bug, please report."); - } + public void addRouteConfig(String name, ConfigMap routeConfig) { + this.routeConfigs.put(name, routeConfig); } /** diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java new file mode 100644 index 00000000000..49ca8862de8 --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2012-2018 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/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.util; + +import static java.util.stream.Collectors.toMap; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.isLabeled; + +import com.google.common.collect.ImmutableMap; +import io.fabric8.kubernetes.api.model.ConfigMap; +import java.util.Arrays; +import java.util.Map; +import java.util.Map.Entry; +import javax.inject.Inject; +import javax.inject.Named; +import javax.inject.Singleton; + +/** Little utility bean helping with Gateway ConfigMaps labels. */ +@Singleton +public class GatewayConfigmapLabels { + + private final Map labels; + + @Inject + public GatewayConfigmapLabels( + @Named("che.infra.kubernetes.single_host.gateway.configmap.labels") String[] labelsProperty) { + // TODO: use Collectors.toUnmodifiableMap when JDK11 features are allowed + this.labels = + ImmutableMap.copyOf( + Arrays.stream(labelsProperty) + .map(label -> label.split("=", 2)) + .collect(toMap(l -> l[0], l -> l[1]))); + } + + public Map getLabels() { + return labels; + } + + /** + * Check whether configmap is gateway route configuration. That is defined with labels provided by + * `che.infra.kubernetes.single_host.gateway.configmap.labels` configuration property. + * + * @param configMap to check + * @return `true` if ConfigMap is gateway route configuration, `false` otherwise + */ + public boolean isGatewayConfig(ConfigMap configMap) { + for (Entry labelEntry : labels.entrySet()) { + if (!isLabeled(configMap, labelEntry.getKey(), labelEntry.getValue())) { + return false; + } + } + return true; + } +} diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java index d02b19692c3..a6e7b4f9660 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java @@ -26,7 +26,6 @@ import static org.eclipse.che.dto.server.DtoFactory.newDto; import static org.eclipse.che.workspace.infrastructure.kubernetes.Annotations.CREATE_IN_CHE_INSTALLATION_NAMESPACE; import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.CHE_ORIGINAL_NAME_LABEL; -import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.GatewayRouterProvisioner.GATEWAY_CONFIGMAP_LABELS; import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostExternalServiceExposureStrategy.MULTI_HOST_STRATEGY; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -1136,7 +1135,6 @@ public void testGatewayRouteConfigsAreCreatedAsConfigmapsInCheNamespace() new ConfigMapBuilder() .withNewMetadata() .withName("route1") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(ImmutableMap.of(CREATE_IN_CHE_INSTALLATION_NAMESPACE, "true")) .endMetadata() .build(); @@ -1144,7 +1142,6 @@ public void testGatewayRouteConfigsAreCreatedAsConfigmapsInCheNamespace() new ConfigMapBuilder() .withNewMetadata() .withName("route2") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(ImmutableMap.of(CREATE_IN_CHE_INSTALLATION_NAMESPACE, "true")) .endMetadata() .build(); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisionerTest.java index c2200b0641c..ad33a10516e 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisionerTest.java @@ -14,7 +14,7 @@ import static java.util.Collections.emptyMap; import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_NAME_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_PORT_ATTRIBUTE; -import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.GatewayRouterProvisioner.GATEWAY_CONFIGMAP_LABELS; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -33,10 +33,10 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.GatewayRouteConfigGenerator; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.GatewayRouteConfigGeneratorFactory; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.GatewayConfigmapLabels; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.BeforeMethod; -import org.testng.annotations.DataProvider; import org.testng.annotations.Listeners; import org.testng.annotations.Test; @@ -47,6 +47,7 @@ public class GatewayRouterProvisionerTest { @Mock private GatewayRouteConfigGeneratorFactory configGeneratorFactory; @Mock private GatewayRouteConfigGenerator gatewayRouteConfigGenerator; + @Mock private GatewayConfigmapLabels gatewayConfigmapLabels; @Mock private KubernetesEnvironment env; @Mock private RuntimeIdentity identity; @@ -65,19 +66,16 @@ public void setUp() { lenient().when(configGeneratorFactory.create()).thenReturn(gatewayRouteConfigGenerator); lenient().when(identity.getInfrastructureNamespace()).thenReturn(NAMESPACE); - gatewayRouterProvisioner = new GatewayRouterProvisioner(configGeneratorFactory); + when(gatewayConfigmapLabels.isGatewayConfig(any(ConfigMap.class))).thenReturn(true); + gatewayRouterProvisioner = + new GatewayRouterProvisioner(configGeneratorFactory, gatewayConfigmapLabels); } @Test(expectedExceptions = InfrastructureException.class) public void testFailWhenNoServersInConfigmapAnnotations() throws InfrastructureException { // given ConfigMap gatewayRouteConfigMap = - new ConfigMapBuilder() - .withNewMetadata() - .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) - .endMetadata() - .build(); + new ConfigMapBuilder().withNewMetadata().withName("route").endMetadata().build(); when(env.getConfigMaps()).thenReturn(Collections.singletonMap("route", gatewayRouteConfigMap)); // when @@ -99,7 +97,6 @@ public void testFailWhenMoreThanOneServerInConfigmapAnnotations() throws Infrast new ConfigMapBuilder() .withNewMetadata() .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(annotationsWith2Servers) .endMetadata() .build(); @@ -121,7 +118,6 @@ public void testFailWhenServerHasNotAllNeededAttributes() throws InfrastructureE new ConfigMapBuilder() .withNewMetadata() .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(annotationsWith2Servers) .endMetadata() .build(); @@ -143,7 +139,6 @@ public void testProvision() throws InfrastructureException { new ConfigMapBuilder() .withNewMetadata() .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(annotationsWith2Servers) .endMetadata() .build(); @@ -176,25 +171,4 @@ public void testProvision() throws InfrastructureException { Map actualData = gatewayRouteConfigMap.getData(); assertEquals(actualData, expectedData); } - - @Test(dataProvider = "isGatewayConfigData") - public void testIsGatewayConfig(Map labels, boolean isGatewayConfigExpected) { - ConfigMap cm = - new ConfigMapBuilder().withNewMetadata().withLabels(labels).endMetadata().build(); - assertEquals(GatewayRouterProvisioner.isGatewayConfig(cm), isGatewayConfigExpected); - } - - @DataProvider - public Object[][] isGatewayConfigData() { - return new Object[][] { - {GATEWAY_CONFIGMAP_LABELS, true}, - {ImmutableMap.builder().putAll(GATEWAY_CONFIGMAP_LABELS).put("other", "value").build(), true}, - {emptyMap(), false}, - {ImmutableMap.of("one", "two"), false}, - {ImmutableMap.of(), false}, - {ImmutableMap.of("app", "yes", "role", "no"), false}, - {ImmutableMap.of("app", GATEWAY_CONFIGMAP_LABELS.get("app"), "role", "no"), false}, - {ImmutableMap.of("app", "no", "role", GATEWAY_CONFIGMAP_LABELS.get("role")), false}, - }; - } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayTlsProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayTlsProvisionerTest.java index 302301ebb70..ddd4eb74a6c 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayTlsProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayTlsProvisionerTest.java @@ -13,7 +13,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; -import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.GatewayRouterProvisioner.GATEWAY_CONFIGMAP_LABELS; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -26,8 +26,10 @@ import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.GatewayConfigmapLabels; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.DataProvider; import org.testng.annotations.Listeners; import org.testng.annotations.Test; @@ -38,6 +40,7 @@ public class GatewayTlsProvisionerTest { public static final String WORKSPACE_ID = "workspace123"; @Mock private KubernetesEnvironment k8sEnv; @Mock private RuntimeIdentity runtimeIdentity; + @Mock private GatewayConfigmapLabels gatewayConfigmapLabels; private final ServerConfigImpl httpServer = new ServerConfigImpl("8080/tpc", "http", "/api", emptyMap()); @@ -47,6 +50,11 @@ public class GatewayTlsProvisionerTest { singletonMap("annotation-key", "annotation-value"); private final String machine = "machine"; + @BeforeMethod + public void setUp() { + when(gatewayConfigmapLabels.isGatewayConfig(any(ConfigMap.class))).thenReturn(true); + } + @Test(dataProvider = "tlsProvisionData") public void provisionTlsForGatewayRouteConfigmaps( ServerConfigImpl server, boolean tlsEnabled, String expectedProtocol) throws Exception { @@ -58,13 +66,12 @@ public void provisionTlsForGatewayRouteConfigmaps( new ConfigMapBuilder() .withNewMetadata() .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(composedAnnotations) .endMetadata() .build(); GatewayTlsProvisioner gatewayTlsProvisioner = - new GatewayTlsProvisioner<>(tlsEnabled); + new GatewayTlsProvisioner<>(tlsEnabled, gatewayConfigmapLabels); when(k8sEnv.getConfigMaps()).thenReturn(singletonMap("route", routeConfigMap)); @@ -102,14 +109,13 @@ public void throwExceptionWhenMultipleServersInGatewayRouteConfigAnnotations() new ConfigMapBuilder() .withNewMetadata() .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(composedAnnotations) .endMetadata() .build(); when(k8sEnv.getConfigMaps()).thenReturn(singletonMap("route", routeConfigMap)); GatewayTlsProvisioner gatewayTlsProvisioner = - new GatewayTlsProvisioner<>(true); + new GatewayTlsProvisioner<>(true, gatewayConfigmapLabels); // when gatewayTlsProvisioner.provision(k8sEnv, runtimeIdentity); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayServerExposerTest.java index c50b08106a8..039ee52d929 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/GatewayServerExposerTest.java @@ -11,10 +11,11 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; -import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.GatewayRouterProvisioner.GATEWAY_CONFIGMAP_LABELS; +import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; +import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.ServicePort; @@ -24,9 +25,22 @@ import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.GatewayConfigmapLabels; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; import org.testng.annotations.Test; +@Listeners(MockitoTestNGListener.class) public class GatewayServerExposerTest { + private static final Map GATEWAY_CONFIGMAP_LABELS = + ImmutableMap.builder() + .put("app", "che") + .put("role", "gateway-config") + .build(); + + @Mock private GatewayConfigmapLabels gatewayConfigmapLabels; private final String machineName = "machine"; private final String serviceName = "service"; @@ -39,8 +53,15 @@ public class GatewayServerExposerTest { private final Map servers = Collections.singletonMap("serverOne", new ServerConfigImpl("1111", "ws", null, s1attrs)); - private final ExternalServerExposer serverExposer = - new GatewayServerExposer<>(new SingleHostExternalServiceExposureStrategy("che-host")); + private ExternalServerExposer serverExposer; + + @BeforeMethod + public void setUp() { + when(gatewayConfigmapLabels.getLabels()).thenReturn(GATEWAY_CONFIGMAP_LABELS); + serverExposer = + new GatewayServerExposer<>( + new SingleHostExternalServiceExposureStrategy("che-host"), gatewayConfigmapLabels); + } @Test public void testExposeServiceWithGatewayConfigmap() { diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/TraefikGatewayRouteConfigGeneratorTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/TraefikGatewayRouteConfigGeneratorTest.java index 7158a10ff1e..39b34994ec8 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/TraefikGatewayRouteConfigGeneratorTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/TraefikGatewayRouteConfigGeneratorTest.java @@ -13,7 +13,6 @@ import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_NAME_ATTRIBUTE; import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_PORT_ATTRIBUTE; -import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.GatewayRouterProvisioner.GATEWAY_CONFIGMAP_LABELS; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; @@ -75,7 +74,6 @@ public void testGenerateGatewayConfig() throws InfrastructureException { new ConfigMapBuilder() .withNewMetadata() .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(annotations) .endMetadata() .build(); @@ -102,7 +100,6 @@ public void testMultipleRouteConfigsAreGeneratedAsMultipleMapEntries() new ConfigMapBuilder() .withNewMetadata() .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(annotations) .endMetadata() .build(); @@ -131,7 +128,6 @@ public void failWhenMultipleServersInConfigmapAnnotations() throws Infrastructur new ConfigMapBuilder() .withNewMetadata() .withName("route") - .withLabels(GATEWAY_CONFIGMAP_LABELS) .withAnnotations(annotations) .endMetadata() .build(); @@ -139,11 +135,4 @@ public void failWhenMultipleServersInConfigmapAnnotations() throws Infrastructur gatewayConfigGenerator.generate("che-namespace"); } - - @Test(expectedExceptions = InfrastructureException.class) - public void failWhenAddConfigmapWithoutLabels() throws InfrastructureException { - ConfigMap routeConfig = - new ConfigMapBuilder().withNewMetadata().withName("route").endMetadata().build(); - gatewayConfigGenerator.addRouteConfig("c1", routeConfig); - } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java new file mode 100644 index 00000000000..a37e1807ac5 --- /dev/null +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2012-2018 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/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.util; + +import static org.testng.Assert.assertEquals; + +import com.google.common.collect.ImmutableMap; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import java.util.Map; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class GatewayConfigmapLabelsTest { + + @Test(dataProvider = "isGatewayConfigData") + public void testIsGatewayConfig( + String[] labelsProperty, Map labels, boolean isGatewayConfigExpected) { + GatewayConfigmapLabels gatewayConfigmapLabels = new GatewayConfigmapLabels(labelsProperty); + ConfigMap cm = + new ConfigMapBuilder().withNewMetadata().withLabels(labels).endMetadata().build(); + assertEquals(gatewayConfigmapLabels.isGatewayConfig(cm), isGatewayConfigExpected); + } + + @DataProvider + public Object[][] isGatewayConfigData() { + return new Object[][] { + { + new String[] {"app=che", "component=che-gateway-config"}, + ImmutableMap.of("app", "che", "component", "che-gateway-config"), + true + }, + { + new String[] {"app=che"}, + ImmutableMap.of("app", "che", "component", "che-gateway-config"), + true + }, + {new String[] {}, ImmutableMap.of("any", "label"), true}, + {new String[] {}, ImmutableMap.of(), true}, + { + new String[] {"app=che", "component=che-gateway-config"}, + ImmutableMap.of("app", "cheche", "component", "che-gateway-config"), + false + }, + { + new String[] {"app=che", "component=che-gateway-config"}, + ImmutableMap.of("app", "cheche"), + false + }, + { + new String[] {"app=che", "component=che-gateway-config"}, + ImmutableMap.of("component", "che-gateway-config"), + false + }, + }; + } +} From a967162c89fc31d299d069b3fadf95286698dd51 Mon Sep 17 00:00:00 2001 From: Michal Vala Date: Wed, 2 Sep 2020 15:18:37 +0200 Subject: [PATCH 2/5] test gateway provisioning with no matching labels Signed-off-by: Michal Vala --- .../GatewayRouterProvisionerTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisionerTest.java index ad33a10516e..93c0c4910fd 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/GatewayRouterProvisionerTest.java @@ -16,6 +16,7 @@ import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SERVICE_PORT_ATTRIBUTE; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -129,6 +130,30 @@ public void testFailWhenServerHasNotAllNeededAttributes() throws InfrastructureE // then exception } + @Test + public void testNoProvisionWhenNoMatchingLabels() throws InfrastructureException { + // given + Map annotationsWith2Servers = + new Annotations.Serializer().server("s1", serverConfig).annotations(); + + ConfigMap gatewayRouteConfigMap = + new ConfigMapBuilder() + .withNewMetadata() + .withName("route") + .withAnnotations(annotationsWith2Servers) + .endMetadata() + .build(); + when(env.getConfigMaps()).thenReturn(Collections.singletonMap("route", gatewayRouteConfigMap)); + + when(gatewayConfigmapLabels.isGatewayConfig(gatewayRouteConfigMap)).thenReturn(false); + + // when + gatewayRouterProvisioner.provision(env, identity); + + // then + verify(configGeneratorFactory, never()).create(); + } + @Test public void testProvision() throws InfrastructureException { // given From db6b08f22016289479ee7fa5f55e5f88b99b6b00 Mon Sep 17 00:00:00 2001 From: Michal Vala Date: Thu, 3 Sep 2020 09:26:53 +0200 Subject: [PATCH 3/5] parse single-host gateway configmap labels property from pure string Signed-off-by: Michal Vala --- .../util/GatewayConfigmapLabels.java | 25 +++++++------ .../util/GatewayConfigmapLabelsTest.java | 36 ++++++++++++------- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java index 49ca8862de8..387e1826d6f 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java @@ -11,17 +11,16 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.util; -import static java.util.stream.Collectors.toMap; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.isLabeled; -import com.google.common.collect.ImmutableMap; +import com.google.common.base.Splitter; import io.fabric8.kubernetes.api.model.ConfigMap; -import java.util.Arrays; import java.util.Map; import java.util.Map.Entry; import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; +import org.eclipse.che.api.workspace.server.spi.InfrastructureException; /** Little utility bean helping with Gateway ConfigMaps labels. */ @Singleton @@ -31,13 +30,19 @@ public class GatewayConfigmapLabels { @Inject public GatewayConfigmapLabels( - @Named("che.infra.kubernetes.single_host.gateway.configmap.labels") String[] labelsProperty) { - // TODO: use Collectors.toUnmodifiableMap when JDK11 features are allowed - this.labels = - ImmutableMap.copyOf( - Arrays.stream(labelsProperty) - .map(label -> label.split("=", 2)) - .collect(toMap(l -> l[0], l -> l[1]))); + @Named("che.infra.kubernetes.single_host.gateway.configmap.labels") String labelsProperty) + throws InfrastructureException { + if (labelsProperty == null || labelsProperty.isEmpty()) { + throw new InfrastructureException( + "for gateway single-host, 'che.infra.kubernetes.single_host.gateway.configmap.labels' property must be defined"); + } + try { + this.labels = Splitter.on(",").trimResults().withKeyValueSeparator("=").split(labelsProperty); + } catch (IllegalArgumentException iae) { + throw new InfrastructureException( + "'che.infra.kubernetes.single_host.gateway.configmap.labels' is set to invalid value. It must be in format `name1=value1,name2=value2`. Check the documentation for further details.", + iae); + } } public Map getLabels() { diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java index a37e1807ac5..c7524162a4b 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java @@ -17,6 +17,7 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import java.util.Map; +import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -24,40 +25,51 @@ public class GatewayConfigmapLabelsTest { @Test(dataProvider = "isGatewayConfigData") public void testIsGatewayConfig( - String[] labelsProperty, Map labels, boolean isGatewayConfigExpected) { + String labelsProperty, Map labels, boolean isGatewayConfigExpected) + throws InfrastructureException { GatewayConfigmapLabels gatewayConfigmapLabels = new GatewayConfigmapLabels(labelsProperty); ConfigMap cm = new ConfigMapBuilder().withNewMetadata().withLabels(labels).endMetadata().build(); assertEquals(gatewayConfigmapLabels.isGatewayConfig(cm), isGatewayConfigExpected); } + @Test(expectedExceptions = InfrastructureException.class) + public void failsToConstructWhenLabelsAreNull() throws InfrastructureException { + new GatewayConfigmapLabels(null); + } + + @Test(expectedExceptions = InfrastructureException.class) + public void failsToConstructWhenLabelsAreEmpty() throws InfrastructureException { + new GatewayConfigmapLabels(""); + } + + @Test(expectedExceptions = InfrastructureException.class) + public void failsToConstructWhenLabelsAreNotValid() throws InfrastructureException { + new GatewayConfigmapLabels("badvalue"); + } + @DataProvider public Object[][] isGatewayConfigData() { return new Object[][] { { - new String[] {"app=che", "component=che-gateway-config"}, + "app=che,component=che-gateway-config", ImmutableMap.of("app", "che", "component", "che-gateway-config"), true }, { - new String[] {"app=che"}, + "app=che, component=che-gateway-config", ImmutableMap.of("app", "che", "component", "che-gateway-config"), true }, - {new String[] {}, ImmutableMap.of("any", "label"), true}, - {new String[] {}, ImmutableMap.of(), true}, + {"app=che", ImmutableMap.of("app", "che", "component", "che-gateway-config"), true}, { - new String[] {"app=che", "component=che-gateway-config"}, + "app=che,component=che-gateway-config", ImmutableMap.of("app", "cheche", "component", "che-gateway-config"), false }, + {"app=che,component=che-gateway-config", ImmutableMap.of("app", "cheche"), false}, { - new String[] {"app=che", "component=che-gateway-config"}, - ImmutableMap.of("app", "cheche"), - false - }, - { - new String[] {"app=che", "component=che-gateway-config"}, + "app=che,component=che-gateway-config", ImmutableMap.of("component", "che-gateway-config"), false }, From 9966f9af872f8d00204ecd4c1a26fafa8045db06 Mon Sep 17 00:00:00 2001 From: Michal Vala Date: Thu, 3 Sep 2020 10:09:20 +0200 Subject: [PATCH 4/5] rename single-host config properties Signed-off-by: Michal Vala --- .../src/main/webapp/WEB-INF/classes/che/che.properties | 5 +++-- .../kubernetes/provision/TlsProvisionerProvider.java | 4 ++-- .../server/AbstractExposureStrategyAwareProvider.java | 2 +- .../server/external/ExternalServerExposerProvider.java | 4 ++-- .../server/resolver/AbstractServerResolverFactory.java | 2 +- .../server/resolver/KubernetesServerResolverFactory.java | 2 +- .../kubernetes/util/GatewayConfigmapLabels.java | 8 ++++---- .../openshift/server/OpenShiftServerResolverFactory.java | 2 +- 8 files changed, 15 insertions(+), 14 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index ad3941a4de6..abaad2cf81c 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -254,10 +254,11 @@ che.infra.kubernetes.server_strategy=multi-host # Defines the way in which the workspace plugins and editors are exposed in the single-host mode. # Supported exposures: # - 'native': Exposes servers using k8s Ingresses. Works only on Kubernetes. -che.infra.kubernetes.single_host.workspace.exposure=native +# - 'gateway': Exposes servers using reverse-proxy gateway. +che.infra.kubernetes.singlehost.workspace.exposure=native # Defines labels which will be set to ConfigMaps configuring single-host gateway. -che.infra.kubernetes.single_host.gateway.configmap.labels=app=che,component=che-gateway-config +che.infra.kubernetes.singlehost.gateway.configmap_labels=app=che,component=che-gateway-config # Used to generate domain for a server in a workspace in case property `che.infra.kubernetes.server_strategy` is set to `multi-host` che.infra.kubernetes.ingress.domain= diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/TlsProvisionerProvider.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/TlsProvisionerProvider.java index 393b08eb4b2..36b7ae03310 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/TlsProvisionerProvider.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/TlsProvisionerProvider.java @@ -20,7 +20,7 @@ /** * Provides {@link TlsProvisioner} based on `che.infra.kubernetes.server_strategy` and - * `che.infra.kubernetes.single_host.workspace.exposure` properties. + * `che.infra.kubernetes.singlehost.workspace.exposure` properties. * * @param type of environment */ @@ -30,7 +30,7 @@ public class TlsProvisionerProvider @Inject public TlsProvisionerProvider( @Named("che.infra.kubernetes.server_strategy") String exposureStrategy, - @Named("che.infra.kubernetes.single_host.workspace.exposure") String wsExposureType, + @Named("che.infra.kubernetes.singlehost.workspace.exposure") String wsExposureType, Map> mapping) { super( exposureStrategy, diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/AbstractExposureStrategyAwareProvider.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/AbstractExposureStrategyAwareProvider.java index 53c7c7afa7a..dc036c0cc02 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/AbstractExposureStrategyAwareProvider.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/AbstractExposureStrategyAwareProvider.java @@ -45,7 +45,7 @@ public abstract class AbstractExposureStrategyAwareProvider implements Provid */ protected AbstractExposureStrategyAwareProvider( @Named("che.infra.kubernetes.server_strategy") String exposureStrategy, - @Named("che.infra.kubernetes.single_host.workspace.exposure") String wsExposureType, + @Named("che.infra.kubernetes.singlehost.workspace.exposure") String wsExposureType, Map mapping, String errorMessageTemplate) { if (exposureStrategy.equals(SingleHostExternalServiceExposureStrategy.SINGLE_HOST_STRATEGY)) { diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerProvider.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerProvider.java index 349b2d55ace..7206a9453ab 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerProvider.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerProvider.java @@ -21,7 +21,7 @@ /** * Provides {@link ExternalServerExposer} based on `che.infra.kubernetes.server_strategy` and - * `che.infra.kubernetes.single_host.workspace.exposure` properties. + * `che.infra.kubernetes.singlehost.workspace.exposure` properties. * * @param type of environment */ @@ -31,7 +31,7 @@ public class ExternalServerExposerProvider @Inject public ExternalServerExposerProvider( @Named("che.infra.kubernetes.server_strategy") String exposureStrategy, - @Named("che.infra.kubernetes.single_host.workspace.exposure") String exposureType, + @Named("che.infra.kubernetes.singlehost.workspace.exposure") String exposureType, Map> exposers) { super( diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/resolver/AbstractServerResolverFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/resolver/AbstractServerResolverFactory.java index df97191d673..bf24fc7bee5 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/resolver/AbstractServerResolverFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/resolver/AbstractServerResolverFactory.java @@ -32,7 +32,7 @@ public class AbstractServerResolverFactory { protected AbstractServerResolverFactory( @Named("che.infra.kubernetes.server_strategy") String exposureStrategy, - @Named("che.infra.kubernetes.single_host.workspace.exposure") String wsExposureType, + @Named("che.infra.kubernetes.singlehost.workspace.exposure") String wsExposureType, Map> mapping, String errorMessageTemplate) { constructorProvider = diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/resolver/KubernetesServerResolverFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/resolver/KubernetesServerResolverFactory.java index 8bb050b8edd..d31058fdd7c 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/resolver/KubernetesServerResolverFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/resolver/KubernetesServerResolverFactory.java @@ -32,7 +32,7 @@ public KubernetesServerResolverFactory( IngressPathTransformInverter pathTransformInverter, @Named("che.host") String cheHost, @Named("che.infra.kubernetes.server_strategy") String exposureStrategy, - @Named("che.infra.kubernetes.single_host.workspace.exposure") String wsExposureType) { + @Named("che.infra.kubernetes.singlehost.workspace.exposure") String wsExposureType) { super( exposureStrategy, diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java index 387e1826d6f..b49f3be844b 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java @@ -30,17 +30,17 @@ public class GatewayConfigmapLabels { @Inject public GatewayConfigmapLabels( - @Named("che.infra.kubernetes.single_host.gateway.configmap.labels") String labelsProperty) + @Named("che.infra.kubernetes.singlehost.gateway.configmap_labels") String labelsProperty) throws InfrastructureException { if (labelsProperty == null || labelsProperty.isEmpty()) { throw new InfrastructureException( - "for gateway single-host, 'che.infra.kubernetes.single_host.gateway.configmap.labels' property must be defined"); + "for gateway single-host, 'che.infra.kubernetes.singlehost.gateway.configmap_labels' property must be defined"); } try { this.labels = Splitter.on(",").trimResults().withKeyValueSeparator("=").split(labelsProperty); } catch (IllegalArgumentException iae) { throw new InfrastructureException( - "'che.infra.kubernetes.single_host.gateway.configmap.labels' is set to invalid value. It must be in format `name1=value1,name2=value2`. Check the documentation for further details.", + "'che.infra.kubernetes.singlehost.gateway.configmap_labels' is set to invalid value. It must be in format `name1=value1,name2=value2`. Check the documentation for further details.", iae); } } @@ -51,7 +51,7 @@ public Map getLabels() { /** * Check whether configmap is gateway route configuration. That is defined with labels provided by - * `che.infra.kubernetes.single_host.gateway.configmap.labels` configuration property. + * `che.infra.kubernetes.singlehost.gateway.configmap_labels` configuration property. * * @param configMap to check * @return `true` if ConfigMap is gateway route configuration, `false` otherwise diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolverFactory.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolverFactory.java index 572578a3184..40a397b33d2 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolverFactory.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolverFactory.java @@ -32,7 +32,7 @@ public class OpenShiftServerResolverFactory extends AbstractServerResolverFactor public OpenShiftServerResolverFactory( @Named("che.host") String cheHost, @Named("che.infra.kubernetes.server_strategy") String exposureStrategy, - @Named("che.infra.kubernetes.single_host.workspace.exposure") String wsExposureType) { + @Named("che.infra.kubernetes.singlehost.workspace.exposure") String wsExposureType) { super( exposureStrategy, wsExposureType, From 5d2c6c096e7b26171ea6d9271b7044c515f3f60b Mon Sep 17 00:00:00 2001 From: Michal Vala Date: Thu, 3 Sep 2020 12:47:59 +0200 Subject: [PATCH 5/5] throw ConfigurationException in case singlehost configmap labels are not set correctly + minor refactor Signed-off-by: Michal Vala --- .../kubernetes/util/GatewayConfigmapLabels.java | 12 ++++++------ .../util/GatewayConfigmapLabelsTest.java | 17 ++++++++--------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java index b49f3be844b..c76a6cac9c3 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabels.java @@ -11,6 +11,7 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.util; +import static com.google.common.base.Strings.isNullOrEmpty; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.isLabeled; import com.google.common.base.Splitter; @@ -20,7 +21,7 @@ import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; -import org.eclipse.che.api.workspace.server.spi.InfrastructureException; +import org.eclipse.che.inject.ConfigurationException; /** Little utility bean helping with Gateway ConfigMaps labels. */ @Singleton @@ -30,16 +31,15 @@ public class GatewayConfigmapLabels { @Inject public GatewayConfigmapLabels( - @Named("che.infra.kubernetes.singlehost.gateway.configmap_labels") String labelsProperty) - throws InfrastructureException { - if (labelsProperty == null || labelsProperty.isEmpty()) { - throw new InfrastructureException( + @Named("che.infra.kubernetes.singlehost.gateway.configmap_labels") String labelsProperty) { + if (isNullOrEmpty(labelsProperty)) { + throw new ConfigurationException( "for gateway single-host, 'che.infra.kubernetes.singlehost.gateway.configmap_labels' property must be defined"); } try { this.labels = Splitter.on(",").trimResults().withKeyValueSeparator("=").split(labelsProperty); } catch (IllegalArgumentException iae) { - throw new InfrastructureException( + throw new ConfigurationException( "'che.infra.kubernetes.singlehost.gateway.configmap_labels' is set to invalid value. It must be in format `name1=value1,name2=value2`. Check the documentation for further details.", iae); } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java index c7524162a4b..370246fd11f 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/GatewayConfigmapLabelsTest.java @@ -17,7 +17,7 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import java.util.Map; -import org.eclipse.che.api.workspace.server.spi.InfrastructureException; +import org.eclipse.che.inject.ConfigurationException; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -25,26 +25,25 @@ public class GatewayConfigmapLabelsTest { @Test(dataProvider = "isGatewayConfigData") public void testIsGatewayConfig( - String labelsProperty, Map labels, boolean isGatewayConfigExpected) - throws InfrastructureException { + String labelsProperty, Map labels, boolean isGatewayConfigExpected) { GatewayConfigmapLabels gatewayConfigmapLabels = new GatewayConfigmapLabels(labelsProperty); ConfigMap cm = new ConfigMapBuilder().withNewMetadata().withLabels(labels).endMetadata().build(); assertEquals(gatewayConfigmapLabels.isGatewayConfig(cm), isGatewayConfigExpected); } - @Test(expectedExceptions = InfrastructureException.class) - public void failsToConstructWhenLabelsAreNull() throws InfrastructureException { + @Test(expectedExceptions = ConfigurationException.class) + public void failsToConstructWhenLabelsAreNull() { new GatewayConfigmapLabels(null); } - @Test(expectedExceptions = InfrastructureException.class) - public void failsToConstructWhenLabelsAreEmpty() throws InfrastructureException { + @Test(expectedExceptions = ConfigurationException.class) + public void failsToConstructWhenLabelsAreEmpty() { new GatewayConfigmapLabels(""); } - @Test(expectedExceptions = InfrastructureException.class) - public void failsToConstructWhenLabelsAreNotValid() throws InfrastructureException { + @Test(expectedExceptions = ConfigurationException.class) + public void failsToConstructWhenLabelsAreNotValid() { new GatewayConfigmapLabels("badvalue"); }