From 485efc5dcce4b4796e29b67195ebb8be125a8efa Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 4 Dec 2019 15:28:35 +0100 Subject: [PATCH 1/8] Experimental fix for #15283. The fix creates a ingress/route per public endpoint instead of merging the endpoints by port and only creating ingresses/routes per exposed port. Signed-off-by: Lukas Krejci --- ...ltHostExternalServiceExposureStrategy.java | 8 +-- .../external/ExternalServerExposer.java | 64 ++++++++++++------- .../ExternalServiceExposureStrategy.java | 5 +- ...tiHostExternalServiceExposureStrategy.java | 7 +- ...leHostExternalServiceExposureStrategy.java | 7 +- .../secure/jwtproxy/JwtProxyProvisioner.java | 42 +++++------- .../jwtproxy/JwtProxySecureServerExposer.java | 26 ++++++-- .../jwtproxy/JwtProxyProvisionerTest.java | 16 ++--- .../JwtProxySecureServerExposerTest.java | 5 +- .../OpenShiftExternalServerExposer.java | 34 ++++++---- .../OpenShiftServerExposureStrategy.java | 5 +- 11 files changed, 119 insertions(+), 100 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategy.java index 62850687d26..76f6e96b595 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategy.java @@ -11,8 +11,6 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; -import io.fabric8.kubernetes.api.model.ServicePort; - /** * Provides a path-based strategy for exposing service ports outside the cluster using Ingress * Ingresses will be created without an explicit host (defaulting to *). @@ -45,12 +43,12 @@ public class DefaultHostExternalServiceExposureStrategy implements ExternalServi public static final String DEFAULT_HOST_STRATEGY = "default-host"; @Override - public String getExternalHost(String serviceName, ServicePort servicePort) { + public String getExternalHost(String serviceName, String serverName) { return null; } @Override - public String getExternalPath(String serviceName, ServicePort servicePort) { - return "/" + serviceName + "/" + servicePort.getName(); + public String getExternalPath(String serviceName, String serverName) { + return "/" + serviceName + "/" + serverName; } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java index dcad785bbdf..b53dae57a61 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java @@ -11,9 +11,12 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; +import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.api.model.extensions.Ingress; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Named; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; @@ -60,41 +63,58 @@ public void expose( String serviceName, ServicePort servicePort, Map externalServers) { - Ingress ingress = generateIngress(machineName, serviceName, servicePort, externalServers); - k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); + List ingresses = + generateIngresses(machineName, serviceName, servicePort, externalServers); + for (Ingress ingress : ingresses) { + k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); + } } - private Ingress generateIngress( + private List generateIngresses( String machineName, String serviceName, ServicePort servicePort, Map ingressesServers) { - ExternalServerIngressBuilder ingressBuilder = new ExternalServerIngressBuilder(); - String host = strategy.getExternalHost(serviceName, servicePort); - if (host != null) { - ingressBuilder = ingressBuilder.withHost(host); - } + return ingressesServers + .entrySet() + .stream() + .map( + e -> { + String serverName = makeValidDnsName(e.getKey()); + ServerConfig serverConfig = e.getValue(); + + ExternalServerIngressBuilder ingressBuilder = new ExternalServerIngressBuilder(); + String host = strategy.getExternalHost(serviceName, serverName); + if (host != null) { + ingressBuilder = ingressBuilder.withHost(host); + } - return ingressBuilder - .withPath( - String.format( - pathTransformFmt, - ensureEndsWithSlash(strategy.getExternalPath(serviceName, servicePort)))) - .withName(getIngressName(serviceName, servicePort)) - .withMachineName(machineName) - .withServiceName(serviceName) - .withAnnotations(ingressAnnotations) - .withServicePort(servicePort.getName()) - .withServers(ingressesServers) - .build(); + return ingressBuilder + .withPath( + String.format( + pathTransformFmt, + ensureEndsWithSlash(strategy.getExternalPath(serviceName, serverName)))) + .withName(getIngressName(serviceName, serverName)) + .withMachineName(machineName) + .withServiceName(serviceName) + .withAnnotations(ingressAnnotations) + .withServicePort(servicePort.getName()) + .withServers(ImmutableMap.of(serverName, serverConfig)) + .build(); + }) + .collect(Collectors.toList()); } private static String ensureEndsWithSlash(String path) { return path.endsWith("/") ? path : path + '/'; } - private static String getIngressName(String serviceName, ServicePort servicePort) { - return serviceName + "-" + servicePort.getName(); + private static String getIngressName(String serviceName, String serverName) { + return serviceName + "-" + serverName; + } + + protected static String makeValidDnsName(String name) { + return name.replaceAll("/", "-"); } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServiceExposureStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServiceExposureStrategy.java index 01e3a6551df..61212a13135 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServiceExposureStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServiceExposureStrategy.java @@ -11,7 +11,6 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; -import io.fabric8.kubernetes.api.model.ServicePort; import org.eclipse.che.commons.annotation.Nullable; /** @@ -22,8 +21,8 @@ public interface ExternalServiceExposureStrategy { /** Returns a host that should be used to expose the service */ @Nullable - String getExternalHost(String serviceName, ServicePort servicePort); + String getExternalHost(String serviceName, String serverName); /** Returns the path on which the service should be exposed */ - String getExternalPath(String serviceName, ServicePort servicePort); + String getExternalPath(String serviceName, String serverName); } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategy.java index 4fdcba9a69d..070df9f2a5f 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategy.java @@ -15,7 +15,6 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressServiceExposureStrategyProvider.STRATEGY_PROPERTY; import com.google.common.base.Strings; -import io.fabric8.kubernetes.api.model.ServicePort; import javax.inject.Inject; import javax.inject.Named; import org.eclipse.che.inject.ConfigurationException; @@ -65,12 +64,12 @@ public MultiHostExternalServiceExposureStrategy( } @Override - public String getExternalHost(String serviceName, ServicePort servicePort) { - return serviceName + "-" + servicePort.getName() + "." + domain; + public String getExternalHost(String serviceName, String serverName) { + return serviceName + "-" + serverName + "." + domain; } @Override - public String getExternalPath(String serviceName, ServicePort servicePort) { + public String getExternalPath(String serviceName, String serverName) { return "/"; } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostExternalServiceExposureStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostExternalServiceExposureStrategy.java index f614ba4a8c0..69d7395684f 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostExternalServiceExposureStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostExternalServiceExposureStrategy.java @@ -11,7 +11,6 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; -import io.fabric8.kubernetes.api.model.ServicePort; import javax.inject.Inject; import javax.inject.Named; @@ -53,12 +52,12 @@ public SingleHostExternalServiceExposureStrategy(@Named("che.host") String cheHo } @Override - public String getExternalHost(String serviceName, ServicePort servicePort) { + public String getExternalHost(String serviceName, String serverName) { return cheHost; } @Override - public String getExternalPath(String serviceName, ServicePort servicePort) { - return "/" + serviceName + "/" + servicePort.getName(); + public String getExternalPath(String serviceName, String serverName) { + return "/" + serviceName + "/" + serverName; } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisioner.java index b9b7620093c..5a9d063da30 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisioner.java @@ -153,7 +153,8 @@ public JwtProxyProvisioner( * @param backendServiceName service name that will be exposed * @param backendServicePort service port that will be exposed * @param protocol protocol that will be used for exposed port - * @param secureServers secure servers to expose + * @param serverName the name of the exposed secure server + * @param secureServer secure server to expose * @return JWTProxy service port that expose the specified one * @throws InfrastructureException if any exception occurs during port exposing */ @@ -162,36 +163,27 @@ public ServicePort expose( String backendServiceName, ServicePort backendServicePort, String protocol, - Map secureServers) + String serverName, + ServerConfig secureServer) throws InfrastructureException { - Preconditions.checkArgument( - secureServers != null && !secureServers.isEmpty(), "Secure servers are missing"); + Preconditions.checkArgument(secureServer != null, "Secure server is missing"); ensureJwtProxyInjected(k8sEnv); - int listenPort = availablePort++; - Set excludes = new HashSet<>(); - Boolean cookiesAuthEnabled = null; - for (ServerConfig config : secureServers.values()) { - // accumulate unsecured paths - if (config.getAttributes().containsKey(UNSECURED_PATHS_ATTRIBUTE)) { - Collections.addAll( - excludes, config.getAttributes().get(UNSECURED_PATHS_ATTRIBUTE).split(",")); - } - // calculate `cookiesAuthEnabled` attributes - Boolean serverCookiesAuthEnabled = - parseBoolean(config.getAttributes().get(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE)); - if (cookiesAuthEnabled == null) { - cookiesAuthEnabled = serverCookiesAuthEnabled; - } else { - if (!cookiesAuthEnabled.equals(serverCookiesAuthEnabled)) { - throw new InfrastructureException( - "Secure servers which expose the same port should have the same `cookiesAuthEnabled` value."); - } - } + // accumulate unsecured paths + if (secureServer.getAttributes().containsKey(UNSECURED_PATHS_ATTRIBUTE)) { + Collections.addAll( + excludes, secureServer.getAttributes().get(UNSECURED_PATHS_ATTRIBUTE).split(",")); } + // calculate `cookiesAuthEnabled` attributes + Boolean cookiesAuthEnabled = + parseBoolean( + secureServer.getAttributes().get(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE)); + + int listenPort = availablePort++; + ServicePort exposedPort = new ServicePortBuilder() .withName("server-" + listenPort) @@ -208,7 +200,7 @@ public ServicePort expose( excludes, cookiesAuthEnabled, cookiePathStrategy.get(serviceName, exposedPort), - externalServiceExposureStrategy.getExternalPath(serviceName, exposedPort)); + externalServiceExposureStrategy.getExternalPath(serviceName, serverName)); k8sEnv .getConfigMaps() .get(getConfigMapName()) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java index ee15085a29a..58071bea6b4 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java @@ -12,6 +12,7 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; import com.google.inject.assistedinject.Assisted; import io.fabric8.kubernetes.api.model.ServicePort; import java.util.Map; @@ -68,11 +69,26 @@ public void expose( ServicePort servicePort, Map secureServers) throws InfrastructureException { - ServicePort exposedServicePort = - proxyProvisioner.expose( - k8sEnv, serviceName, servicePort, servicePort.getProtocol(), secureServers); - exposer.expose( - k8sEnv, machineName, proxyProvisioner.getServiceName(), exposedServicePort, secureServers); + for (Map.Entry e : secureServers.entrySet()) { + String serverName = e.getKey(); + ServerConfig serverConfig = e.getValue(); + + ServicePort exposedServicePort = + proxyProvisioner.expose( + k8sEnv, + serviceName, + servicePort, + servicePort.getProtocol(), + serverName, + serverConfig); + + exposer.expose( + k8sEnv, + machineName, + proxyProvisioner.getServiceName(), + exposedServicePort, + ImmutableMap.of(serverName, serverConfig)); + } } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisionerTest.java index bbef5d8b88e..6d8c296497a 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisionerTest.java @@ -129,8 +129,7 @@ public void shouldProvisionJwtProxyRelatedObjectsIntoKubernetesEnvironment() thr port.setTargetPort(new IntOrString(4401)); // when - jwtProxyProvisioner.expose( - k8sEnv, "terminal", port, "TCP", ImmutableMap.of("server", secureServer)); + jwtProxyProvisioner.expose(k8sEnv, "terminal", port, "TCP", "server", secureServer); // then InternalMachineConfig jwtProxyMachine = @@ -179,12 +178,7 @@ public void shouldThrowAnExceptionIsServersHaveDifferentValueForCookiesAuthEnabl port.setTargetPort(new IntOrString(4401)); // when - jwtProxyProvisioner.expose( - k8sEnv, - "terminal", - port, - "TCP", - ImmutableMap.of("server1", server1, "server2", server2, "server3", server3)); + jwtProxyProvisioner.expose(k8sEnv, "terminal", port, "TCP", "server1", server1); } @Test @@ -220,8 +214,7 @@ public void shouldUseCookiesAuthEnabledFromServersConfigs() throws Exception { port.setTargetPort(new IntOrString(4401)); // when - jwtProxyProvisioner.expose( - k8sEnv, "terminal", port, "TCP", ImmutableMap.of("server1", server1, "server2", server2)); + jwtProxyProvisioner.expose(k8sEnv, "terminal", port, "TCP", "server1", server1); // then verify(configBuilder).addVerifierProxy(any(), any(), any(), eq(true), any(), any()); @@ -250,8 +243,7 @@ public void shouldFalseValueAsDefaultForCookiesAuthEnabledAttribute() throws Exc port.setTargetPort(new IntOrString(4401)); // when - jwtProxyProvisioner.expose( - k8sEnv, "terminal", port, "TCP", ImmutableMap.of("server1", server1)); + jwtProxyProvisioner.expose(k8sEnv, "terminal", port, "TCP", "server1", server1); // then verify(configBuilder).addVerifierProxy(any(), any(), any(), eq(false), any(), any()); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java index 175e288d2c0..3ffdf51b9b5 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java @@ -12,7 +12,6 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; @@ -73,7 +72,7 @@ public void shouldExposeSecureServersWithNewJwtProxyServicePort() throws Excepti ServicePort jwtProxyServicePort = new ServicePort(); doReturn(jwtProxyServicePort) .when(jwtProxyProvisioner) - .expose(any(), anyString(), any(), anyString(), anyMap()); + .expose(any(), anyString(), any(), anyString(), any(), any()); when(jwtProxyProvisioner.getServiceName()).thenReturn(JWT_PROXY_SERVICE_NAME); @@ -84,7 +83,7 @@ public void shouldExposeSecureServersWithNewJwtProxyServicePort() throws Excepti // then verify(jwtProxyProvisioner) .expose( - eq(k8sEnv), eq(MACHINE_SERVICE_NAME), eq(machineServicePort), eq("TCP"), eq(servers)); + eq(k8sEnv), eq(MACHINE_SERVICE_NAME), eq(machineServicePort), eq("TCP"), any(), any()); verify(externalServerExposer) .expose( eq(k8sEnv), diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java index a63fdb42a10..9fc7ad69308 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java @@ -11,6 +11,7 @@ */ package org.eclipse.che.workspace.infrastructure.openshift.server; +import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServicePort; @@ -100,15 +101,18 @@ public void expose( String serviceName, ServicePort servicePort, Map externalServers) { - Route route = - new RouteBuilder() - .withName(Names.generateName("route")) - .withMachineName(machineName) - .withTargetPort(servicePort.getName()) - .withServers(externalServers) - .withTo(serviceName) - .build(); - openShiftEnvironment.getRoutes().put(route.getMetadata().getName(), route); + + for (Map.Entry e : externalServers.entrySet()) { + Route route = + new RouteBuilder() + .withName(Names.generateName("route")) + .withMachineName(machineName) + .withTargetPort(servicePort.getName()) + .withServer(makeValidDnsName(e.getKey()), e.getValue()) + .withTo(serviceName) + .build(); + openShiftEnvironment.getRoutes().put(route.getMetadata().getName(), route); + } } private static class RouteBuilder { @@ -116,7 +120,8 @@ private static class RouteBuilder { private String name; private String serviceName; private IntOrString targetPort; - private Map serversConfigs; + private String serverName; + private ServerConfig serverConfig; private String machineName; private RouteBuilder withName(String name) { @@ -134,8 +139,9 @@ private RouteBuilder withTargetPort(String targetPortName) { return this; } - private RouteBuilder withServers(Map serversConfigs) { - this.serversConfigs = serversConfigs; + private RouteBuilder withServer(String serverName, ServerConfig serverConfig) { + this.serverName = serverName; + this.serverConfig = serverConfig; return this; } @@ -153,13 +159,13 @@ private Route build() { .withName(name.replace("/", "-")) .withAnnotations( Annotations.newSerializer() - .servers(serversConfigs) + .servers(ImmutableMap.of(serverName, serverConfig)) .machineName(machineName) .annotations()) .endMetadata() .withNewSpec() .withNewTo() - .withName(serviceName) + .withName(serviceName + "-" + serverName) .endTo() .withNewPort() .withTargetPort(targetPort) diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerExposureStrategy.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerExposureStrategy.java index c3bbebff735..5ad6ac1fd27 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerExposureStrategy.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerExposureStrategy.java @@ -11,7 +11,6 @@ */ package org.eclipse.che.workspace.infrastructure.openshift.server; -import io.fabric8.kubernetes.api.model.ServicePort; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServiceExposureStrategy; /** @@ -21,12 +20,12 @@ */ public class OpenShiftServerExposureStrategy implements ExternalServiceExposureStrategy { @Override - public String getExternalHost(String serviceName, ServicePort servicePort) { + public String getExternalHost(String serviceName, String serverName) { return null; } @Override - public String getExternalPath(String serviceName, ServicePort servicePort) { + public String getExternalPath(String serviceName, String serverName) { return "/"; } } From 13c04b2d540b50e2d25bb7636b9cc86613072d1e Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Mon, 9 Dec 2019 10:02:53 +0100 Subject: [PATCH 2/8] Fix the openshift routes to point to the correct service. Signed-off-by: Lukas Krejci --- infrastructures/openshift/pom.xml | 4 ++++ .../server/OpenShiftExternalServerExposer.java | 11 ++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/infrastructures/openshift/pom.xml b/infrastructures/openshift/pom.xml index 8092bb3d002..a7a008cda1e 100644 --- a/infrastructures/openshift/pom.xml +++ b/infrastructures/openshift/pom.xml @@ -95,6 +95,10 @@ org.eclipse.che.core che-core-commons-inject + + org.eclipse.che.core + che-core-commons-lang + org.eclipse.che.core che-core-commons-tracing diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java index 9fc7ad69308..33fb57f947b 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java @@ -19,8 +19,8 @@ import java.util.Collections; import java.util.Map; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; +import org.eclipse.che.commons.lang.NameGenerator; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; -import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; @@ -105,7 +105,12 @@ public void expose( for (Map.Entry e : externalServers.entrySet()) { Route route = new RouteBuilder() - .withName(Names.generateName("route")) + .withName( + NameGenerator.generate("route", 3) + + "-" + + makeValidDnsName(machineName) + + "-" + + makeValidDnsName(e.getKey())) .withMachineName(machineName) .withTargetPort(servicePort.getName()) .withServer(makeValidDnsName(e.getKey()), e.getValue()) @@ -165,7 +170,7 @@ private Route build() { .endMetadata() .withNewSpec() .withNewTo() - .withName(serviceName + "-" + serverName) + .withName(serviceName) .endTo() .withNewPort() .withTargetPort(targetPort) From 2c525006716cf7ed0fa4c6cae87185273f8c4560 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 18 Dec 2019 15:43:05 +0100 Subject: [PATCH 3/8] Introduce a new server config attribute - "unique" - to require the server be exposed on a unique location, separate from the other endpoints sharing the same port. Signed-off-by: Lukas Krejci --- .../model/workspace/config/ServerConfig.java | 147 ++++++++++++++++++ .../core/model/workspace/runtime/Server.java | 6 +- ...ockerimageComponentToWorkspaceApplier.java | 2 +- .../server/KubernetesServerExposer.java | 5 +- .../kubernetes/server/PreviewUrlExposer.java | 23 ++- .../external/ExternalServerExposer.java | 96 ++++++++---- .../secure/jwtproxy/JwtProxyProvisioner.java | 38 +++-- .../jwtproxy/JwtProxySecureServerExposer.java | 31 +++- ...stExternalServiceExposureStrategyTest.java | 87 +++++++++-- ...stExternalServiceExposureStrategyTest.java | 52 ++++++- .../jwtproxy/JwtProxyProvisionerTest.java | 18 ++- .../JwtProxySecureServerExposerTest.java | 5 +- infrastructures/openshift/pom.xml | 4 - .../OpenShiftExternalServerExposer.java | 54 ++++--- .../OpenShiftExternalServerExposerTest.java | 2 + .../workspace/server/WorkspaceService.java | 3 +- .../workspace/server/spi/InternalRuntime.java | 4 +- 17 files changed, 465 insertions(+), 112 deletions(-) diff --git a/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/ServerConfig.java b/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/ServerConfig.java index 3c2f4bea8e6..8b0ee1a715a 100644 --- a/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/ServerConfig.java +++ b/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/ServerConfig.java @@ -11,6 +11,11 @@ */ package org.eclipse.che.api.core.model.workspace.config; +import static java.lang.String.join; +import static java.util.Collections.emptyList; + +import java.util.Arrays; +import java.util.List; import java.util.Map; import org.eclipse.che.api.core.model.workspace.runtime.Server; import org.eclipse.che.commons.annotation.Nullable; @@ -51,6 +56,12 @@ public interface ServerConfig { */ String SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE = "cookiesAuthEnabled"; + /** + * {@link ServerConfig} and {@link Server} attribute name which sets the server as unique, meaning + * that, if exposed, it has its own endpoint even if it shares the same port with other servers. + */ + String UNIQUE_SERVER_ATTRIBUTE = "unique"; + /** * Port used by server. * @@ -87,4 +98,140 @@ public interface ServerConfig { /** Attributes of the server */ Map getAttributes(); + + /** + * Determines whether the attributes configure the server to be internal. + * + * @param attributes the attributes with additional server configuration + * @see #INTERNAL_SERVER_ATTRIBUTE + */ + static boolean isInternal(Map attributes) { + return AttributesEvaluator.booleanAttr(attributes, INTERNAL_SERVER_ATTRIBUTE, false); + } + + /** + * Sets the "internal" flag in the provided attributes to the provided value. + * + * @param attributes the attributes with the additional server configuration + */ + static void setInternal(Map attributes, boolean value) { + attributes.put(INTERNAL_SERVER_ATTRIBUTE, Boolean.toString(value)); + } + + /** + * Determines whether the attributes configure the server to be secure. + * + * @param attributes the attributes with additional server configuration + * @see #SECURE_SERVER_ATTRIBUTE + */ + static boolean isSecure(Map attributes) { + return AttributesEvaluator.booleanAttr(attributes, SECURE_SERVER_ATTRIBUTE, false); + } + + /** + * Sets the "secure" flag in the provided attributes to the provided value. + * + * @param attributes the attributes with the additional server configuration + */ + static void setSecure(Map attributes, boolean value) { + attributes.put(SECURE_SERVER_ATTRIBUTE, Boolean.toString(value)); + } + + /** + * Determines whether the attributes configure the server to be unique. + * + * @param attributes the attributes with additional server configuration + * @see #UNIQUE_SERVER_ATTRIBUTE + */ + static boolean isUnique(Map attributes) { + return AttributesEvaluator.booleanAttr(attributes, UNIQUE_SERVER_ATTRIBUTE, false); + } + + /** + * Sets the "unique" flag in the provided attributes to the provided value. + * + * @param attributes the attributes with the additional server configuration + */ + static void setUnique(Map attributes, boolean value) { + attributes.put(UNIQUE_SERVER_ATTRIBUTE, Boolean.toString(value)); + } + + /** + * Determines whether the attributes configure the server to be authenticated using JWT cookies. + * + * @param attributes the attributes with additional server configuration + * @see #SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE + */ + static boolean isCookiesAuthEnabled(Map attributes) { + return AttributesEvaluator.booleanAttr( + attributes, SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE, false); + } + + /** + * Sets the "cookiesAuthEnabled" flag in the provided attributes to the provided value. + * + * @param attributes the attributes with the additional server configuration + */ + static void setCookiesAuthEnabled(Map attributes, boolean value) { + attributes.put(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE, Boolean.toString(value)); + } + + /** + * Finds the unsecured paths configuration in the provided attributes.s + * + * @param attributes the attributes with additional server configuration + * @see #UNSECURED_PATHS_ATTRIBUTE + */ + static List getUnsecuredPaths(Map attributes) { + if (attributes == null) { + return emptyList(); + } + + String paths = attributes.get(UNSECURED_PATHS_ATTRIBUTE); + if (paths == null) { + return emptyList(); + } + + return Arrays.asList(paths.split("\\s*,\\s*")); + } + + static void setUnsecuredPaths(Map attributes, List value) { + attributes.put(UNSECURED_PATHS_ATTRIBUTE, join(",", value)); + } + + default boolean isInternal() { + return isInternal(getAttributes()); + } + + default boolean isSecure() { + return isSecure(getAttributes()); + } + + default boolean isUnique() { + return isUnique(getAttributes()); + } + + default boolean isCookiesAuthEnabled() { + return isCookiesAuthEnabled(getAttributes()); + } + + default List getUnsecuredPaths() { + return getUnsecuredPaths(getAttributes()); + } +} + +// helper class for the default methods in the above interface +class AttributesEvaluator { + static boolean booleanAttr(Map attrs, String name, boolean defaultValue) { + if (attrs == null) { + return defaultValue; + } + + String attr = attrs.get(name); + if (attr == null) { + return defaultValue; + } + + return Boolean.parseBoolean(attr); + } } diff --git a/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/runtime/Server.java b/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/runtime/Server.java index e759230923d..72f21a3f4cb 100644 --- a/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/runtime/Server.java +++ b/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/runtime/Server.java @@ -26,6 +26,10 @@ public interface Server { /** @return the status */ ServerStatus getStatus(); - /** Returns attributes of the server with some metadata */ + /** + * Returns attributes of the server with some metadata. You can use static methods on {@link + * org.eclipse.che.api.core.model.workspace.config.ServerConfig} to evaluate attributes in this + * map easily. + */ Map getAttributes(); } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/DockerimageComponentToWorkspaceApplier.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/DockerimageComponentToWorkspaceApplier.java index 02f6c2461e8..eaa30130529 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/DockerimageComponentToWorkspaceApplier.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/DockerimageComponentToWorkspaceApplier.java @@ -229,7 +229,7 @@ private ServerConfigImpl toServerConfig(Endpoint endpoint) { String isPublic = attributes.remove(PUBLIC_ENDPOINT_ATTRIBUTE); if ("false".equals(isPublic)) { - attributes.put(ServerConfig.INTERNAL_SERVER_ATTRIBUTE, "true"); + ServerConfig.setInternal(attributes, true); } return new ServerConfigImpl(Integer.toString(endpoint.getPort()), protocol, path, attributes); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposer.java index 190605d38c2..9a54ed296f1 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposer.java @@ -13,7 +13,6 @@ import static java.lang.Integer.parseInt; import static java.util.stream.Collectors.toMap; -import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.INTERNAL_SERVER_ATTRIBUTE; import static org.eclipse.che.commons.lang.NameGenerator.generate; import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.CHE_ORIGINAL_NAME_LABEL; @@ -143,13 +142,13 @@ public void expose(Map servers) throws Infrastru servers.forEach( (key, value) -> { - if ("true".equals(value.getAttributes().get(INTERNAL_SERVER_ATTRIBUTE))) { + if (value.isInternal()) { // Server is internal. It doesn't make sense to make an it secure since // it is available only within workspace servers internalServers.put(key, value); } else { // Server is external. Check if it should be secure or not - if ("true".equals(value.getAttributes().get(ServerConfig.SECURE_SERVER_ATTRIBUTE))) { + if (value.isSecure()) { secureServers.put(key, value); } else { externalServers.put(key, value); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java index 1c7fe206694..379a228a8cb 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java @@ -16,6 +16,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_PREFIX; import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_UNIQUE_PART_SIZE; +import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServicePort; @@ -28,6 +29,7 @@ import javax.inject.Inject; import javax.inject.Singleton; import org.eclipse.che.api.workspace.server.model.impl.CommandImpl; +import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; import org.eclipse.che.api.workspace.server.spi.InternalInfrastructureException; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; @@ -78,7 +80,10 @@ public void expose(T env) throws InternalInfrastructureException { null, foundService.get().getMetadata().getName(), servicePort, - Collections.emptyMap()); + ImmutableMap.of( + command.getName(), + new ServerConfigImpl( + servicePort.getName(), "http", "/", Collections.emptyMap()))); } } else { portsToProvision.add(createServicePort(port)); @@ -86,16 +91,20 @@ public void expose(T env) throws InternalInfrastructureException { } if (!portsToProvision.isEmpty()) { + String serverName = generate(SERVER_PREFIX, SERVER_UNIQUE_PART_SIZE) + "-previewUrl"; Service service = - new ServerServiceBuilder() - .withName(generate(SERVER_PREFIX, SERVER_UNIQUE_PART_SIZE) + "-previewUrl") - .withPorts(portsToProvision) - .build(); - env.getServices().put(service.getMetadata().getName(), service); + new ServerServiceBuilder().withName(serverName).withPorts(portsToProvision).build(); + env.getServices().put(serverName, service); portsToProvision.forEach( port -> externalServerExposer.expose( - env, null, service.getMetadata().getName(), port, Collections.emptyMap())); + env, + null, + service.getMetadata().getName(), + port, + ImmutableMap.of( + serverName, + new ServerConfigImpl(port.getName(), "http", "/", Collections.emptyMap())))); } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java index b53dae57a61..55aa0754522 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java @@ -14,8 +14,11 @@ import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.api.model.extensions.Ingress; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Named; @@ -76,34 +79,71 @@ private List generateIngresses( ServicePort servicePort, Map ingressesServers) { - return ingressesServers - .entrySet() - .stream() - .map( - e -> { - String serverName = makeValidDnsName(e.getKey()); - ServerConfig serverConfig = e.getValue(); - - ExternalServerIngressBuilder ingressBuilder = new ExternalServerIngressBuilder(); - String host = strategy.getExternalHost(serviceName, serverName); - if (host != null) { - ingressBuilder = ingressBuilder.withHost(host); - } - - return ingressBuilder - .withPath( - String.format( - pathTransformFmt, - ensureEndsWithSlash(strategy.getExternalPath(serviceName, serverName)))) - .withName(getIngressName(serviceName, serverName)) - .withMachineName(machineName) - .withServiceName(serviceName) - .withAnnotations(ingressAnnotations) - .withServicePort(servicePort.getName()) - .withServers(ImmutableMap.of(serverName, serverConfig)) - .build(); - }) - .collect(Collectors.toList()); + // all the unique servers need their separate ingresses. all the other servers can get lumped + // under a common ingress corresponding to the server and port. + Map nonUniqueServers = new HashMap<>(); + + // let's go through the servers. Build the unique ingresses straight away and collect the + // servers to be put behind the common ingress. + List ingresses = + ingressesServers + .entrySet() + .stream() + .map( + e -> { + ServerConfig serverConfig = e.getValue(); + + if (!serverConfig.isUnique()) { + nonUniqueServers.put(e.getKey(), serverConfig); + return null; + } else { + return generateIngress( + machineName, + serviceName, + e.getKey(), + servicePort, + ImmutableMap.of(e.getKey(), serverConfig)); + } + }) + .filter(Objects::nonNull) + .collect(Collectors.toCollection(ArrayList::new)); + + // now generate the common ingress unconditionally, even if there are no servers + if (!nonUniqueServers.isEmpty()) { + ingresses.add( + generateIngress( + machineName, serviceName, servicePort.getName(), servicePort, nonUniqueServers)); + } + + return ingresses; + } + + private Ingress generateIngress( + String machineName, + String serviceName, + String serverId, + ServicePort servicePort, + Map servers) { + + String serverName = makeValidDnsName(serverId); + ExternalServerIngressBuilder ingressBuilder = new ExternalServerIngressBuilder(); + String host = strategy.getExternalHost(serviceName, serverName); + if (host != null) { + ingressBuilder = ingressBuilder.withHost(host); + } + + return ingressBuilder + .withPath( + String.format( + pathTransformFmt, + ensureEndsWithSlash(strategy.getExternalPath(serviceName, serverName)))) + .withName(getIngressName(serviceName, serverName)) + .withMachineName(machineName) + .withServiceName(serviceName) + .withAnnotations(ingressAnnotations) + .withServicePort(servicePort.getName()) + .withServers(servers) + .build(); } private static String ensureEndsWithSlash(String path) { diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisioner.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisioner.java index 5a9d063da30..ff89f89ac46 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisioner.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisioner.java @@ -153,8 +153,7 @@ public JwtProxyProvisioner( * @param backendServiceName service name that will be exposed * @param backendServicePort service port that will be exposed * @param protocol protocol that will be used for exposed port - * @param serverName the name of the exposed secure server - * @param secureServer secure server to expose + * @param secureServers secure servers to expose * @return JWTProxy service port that expose the specified one * @throws InfrastructureException if any exception occurs during port exposing */ @@ -163,25 +162,34 @@ public ServicePort expose( String backendServiceName, ServicePort backendServicePort, String protocol, - String serverName, - ServerConfig secureServer) + Map secureServers) throws InfrastructureException { - Preconditions.checkArgument(secureServer != null, "Secure server is missing"); + Preconditions.checkArgument( + secureServers != null && !secureServers.isEmpty(), "Secure servers are missing"); ensureJwtProxyInjected(k8sEnv); Set excludes = new HashSet<>(); + Boolean cookiesAuthEnabled = null; + for (ServerConfig config : secureServers.values()) { + // accumulate unsecured paths + if (config.getAttributes().containsKey(UNSECURED_PATHS_ATTRIBUTE)) { + Collections.addAll( + excludes, config.getAttributes().get(UNSECURED_PATHS_ATTRIBUTE).split(",")); + } - // accumulate unsecured paths - if (secureServer.getAttributes().containsKey(UNSECURED_PATHS_ATTRIBUTE)) { - Collections.addAll( - excludes, secureServer.getAttributes().get(UNSECURED_PATHS_ATTRIBUTE).split(",")); + // calculate `cookiesAuthEnabled` attributes + Boolean serverCookiesAuthEnabled = + parseBoolean(config.getAttributes().get(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE)); + if (cookiesAuthEnabled == null) { + cookiesAuthEnabled = serverCookiesAuthEnabled; + } else { + if (!cookiesAuthEnabled.equals(serverCookiesAuthEnabled)) { + throw new InfrastructureException( + "Secure servers which expose the same port should have the same `cookiesAuthEnabled` value."); + } + } } - // calculate `cookiesAuthEnabled` attributes - Boolean cookiesAuthEnabled = - parseBoolean( - secureServer.getAttributes().get(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE)); - int listenPort = availablePort++; ServicePort exposedPort = @@ -200,7 +208,7 @@ public ServicePort expose( excludes, cookiesAuthEnabled, cookiePathStrategy.get(serviceName, exposedPort), - externalServiceExposureStrategy.getExternalPath(serviceName, serverName)); + externalServiceExposureStrategy.getExternalPath(serviceName, exposedPort.getName())); k8sEnv .getConfigMaps() .get(getConfigMapName()) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java index 58071bea6b4..6a8a008ac3d 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java @@ -15,6 +15,7 @@ import com.google.common.collect.ImmutableMap; import com.google.inject.assistedinject.Assisted; import io.fabric8.kubernetes.api.model.ServicePort; +import java.util.HashMap; import java.util.Map; import javax.inject.Inject; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; @@ -70,25 +71,41 @@ public void expose( Map secureServers) throws InfrastructureException { + Map nonUniqueServers = new HashMap<>(); + for (Map.Entry e : secureServers.entrySet()) { String serverName = e.getKey(); ServerConfig serverConfig = e.getValue(); + if (serverConfig.isUnique()) { + Map serverMapping = ImmutableMap.of(serverName, serverConfig); + + ServicePort exposedServicePort = + proxyProvisioner.expose( + k8sEnv, serviceName, servicePort, servicePort.getProtocol(), serverMapping); + + exposer.expose( + k8sEnv, + machineName, + proxyProvisioner.getServiceName(), + exposedServicePort, + serverMapping); + } else { + nonUniqueServers.put(serverName, serverConfig); + } + } + + if (!nonUniqueServers.isEmpty()) { ServicePort exposedServicePort = proxyProvisioner.expose( - k8sEnv, - serviceName, - servicePort, - servicePort.getProtocol(), - serverName, - serverConfig); + k8sEnv, serviceName, servicePort, servicePort.getProtocol(), nonUniqueServers); exposer.expose( k8sEnv, machineName, proxyProvisioner.getServiceName(), exposedServicePort, - ImmutableMap.of(serverName, serverConfig)); + nonUniqueServers); } } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategyTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategyTest.java index 26071a203a7..611d043df77 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategyTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategyTest.java @@ -11,6 +11,7 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; +import static java.lang.String.format; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_PREFIX; @@ -32,6 +33,7 @@ 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.testng.Assert; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -39,6 +41,9 @@ public class DefaultHostExternalServiceExposureStrategyTest { private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); + private static final Map UNIQUE_SERVER_ATTRIBUTES = + ImmutableMap.of("key", "value", ServerConfig.UNIQUE_SERVER_ATTRIBUTE, "true"); + private static final String MACHINE_NAME = "pod/main"; private static final String SERVICE_NAME = SERVER_PREFIX + "12345678" + "-" + MACHINE_NAME; @@ -93,7 +98,7 @@ public void shouldCreateIngressForServer() { } @Test - public void shouldCreateIngressForServerWhenTwoServersHasTheSamePort() { + public void shouldCreateSingleIngressForTwoNonUniqueServersWithTheSamePort() { // given ServerConfigImpl httpServerConfig = new ServerConfigImpl("8080/tcp", "http", "/api", ATTRIBUTES_MAP); @@ -132,6 +137,46 @@ public void shouldCreateIngressForServerWhenTwoServersHasTheSamePort() { new ServerConfigImpl(wsServerConfig).withAttributes(ATTRIBUTES_MAP)); } + @Test + public void shouldCreateIngressPerUniqueServerWithTheSamePort() { + // given + ServerConfigImpl httpServerConfig = + new ServerConfigImpl("8080/tcp", "http", "/api", UNIQUE_SERVER_ATTRIBUTES); + ServerConfigImpl wsServerConfig = + new ServerConfigImpl("8080/tcp", "ws", "/connect", UNIQUE_SERVER_ATTRIBUTES); + ServicePort servicePort = + new ServicePortBuilder() + .withName("server-8080") + .withPort(8080) + .withProtocol("TCP") + .withTargetPort(new IntOrString(8080)) + .build(); + + Map serversToExpose = + ImmutableMap.of( + "http-server", httpServerConfig, + "ws-server", wsServerConfig); + + // when + externalServerExposer.expose( + kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, servicePort, serversToExpose); + + // then + assertEquals(kubernetesEnvironment.getIngresses().size(), 2); + assertThatExternalServerIsExposed( + MACHINE_NAME, + SERVICE_NAME, + "http-server", + servicePort, + new ServerConfigImpl(httpServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); + assertThatExternalServerIsExposed( + MACHINE_NAME, + SERVICE_NAME, + "ws-server", + servicePort, + new ServerConfigImpl(wsServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); + } + @SuppressWarnings("SameParameterValue") private void assertThatExternalServerIsExposed( String machineName, @@ -141,18 +186,32 @@ private void assertThatExternalServerIsExposed( ServerConfigImpl expected) { // ensure that required ingress is created - Ingress ingress = kubernetesEnvironment.getIngresses().values().iterator().next(); - IngressRule ingressRule = ingress.getSpec().getRules().get(0); - IngressBackend backend = ingressRule.getHttp().getPaths().get(0).getBackend(); - assertEquals(backend.getServiceName(), serviceName); - assertEquals(backend.getServicePort().getStrVal(), servicePort.getName()); - - Annotations.Deserializer ingressAnnotations = - Annotations.newDeserializer(ingress.getMetadata().getAnnotations()); - Map servers = ingressAnnotations.servers(); - ServerConfig serverConfig = servers.get(serverNameRegex); - assertEquals(serverConfig, expected); - - assertEquals(ingressAnnotations.machineName(), machineName); + for (Ingress ingress : kubernetesEnvironment.getIngresses().values()) { + IngressRule ingressRule = ingress.getSpec().getRules().get(0); + IngressBackend backend = ingressRule.getHttp().getPaths().get(0).getBackend(); + if (serviceName.equals(backend.getServiceName())) { + assertEquals(backend.getServicePort().getStrVal(), servicePort.getName()); + + Annotations.Deserializer ingressAnnotations = + Annotations.newDeserializer(ingress.getMetadata().getAnnotations()); + Map servers = ingressAnnotations.servers(); + ServerConfig serverConfig = servers.get(serverNameRegex); + + if (serverConfig == null) { + // ok, this ingress is not for this particular server + continue; + } + + assertEquals(serverConfig, expected); + + assertEquals(ingressAnnotations.machineName(), machineName); + return; + } + } + + Assert.fail( + format( + "Could not find an ingress for machine '%s' and service '%s'", + machineName, serviceName)); } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategyTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategyTest.java index ad4247e18bc..603c103fa0a 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategyTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategyTest.java @@ -40,6 +40,8 @@ public class MultiHostExternalServiceExposureStrategyTest { private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); + private static final Map UNIQUE_SERVER_ATTRIBUTES = + ImmutableMap.of("key", "value", ServerConfig.UNIQUE_SERVER_ATTRIBUTE, "true"); private static final String MACHINE_NAME = "pod/main"; private static final String SERVICE_NAME = SERVER_PREFIX + "12345678" + "-" + MACHINE_NAME; private static final String DOMAIN = "che.com"; @@ -145,6 +147,50 @@ public void shouldCreateIngressForServerWhenTwoServersHasTheSamePort() { new ServerConfigImpl(wsServerConfig).withAttributes(ATTRIBUTES_MAP)); } + @Test + public void shouldCreateIngressPerUniqueServerWithTheSamePort() { + // given + ServerConfigImpl httpServerConfig = + new ServerConfigImpl("8080/tcp", "http", "/api", UNIQUE_SERVER_ATTRIBUTES); + ServerConfigImpl wsServerConfig = + new ServerConfigImpl("8080/tcp", "ws", "/connect", UNIQUE_SERVER_ATTRIBUTES); + ServicePort servicePort = + new ServicePortBuilder() + .withName("server-8080") + .withPort(8080) + .withProtocol("TCP") + .withTargetPort(new IntOrString(8080)) + .build(); + + Map serversToExpose = + ImmutableMap.of( + "http-server", httpServerConfig, + "ws-server", wsServerConfig); + + // when + externalServerExposer.expose( + kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, servicePort, serversToExpose); + + // then + assertEquals(kubernetesEnvironment.getIngresses().size(), 2); + assertThatExternalServerIsExposed( + MACHINE_NAME, + SERVICE_NAME, + "http-server", + "tcp", + 8080, + servicePort, + new ServerConfigImpl(httpServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); + assertThatExternalServerIsExposed( + MACHINE_NAME, + SERVICE_NAME, + "ws-server", + "tcp", + 8080, + servicePort, + new ServerConfigImpl(wsServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); + } + private void assertThatExternalServerIsExposed( String machineName, String serviceName, @@ -155,9 +201,11 @@ private void assertThatExternalServerIsExposed( ServerConfigImpl expected) { // ensure that required ingress is created - Ingress ingress = kubernetesEnvironment.getIngresses().get(serviceName + "-server-" + port); + String ingressName = + serviceName + "-" + (expected.isUnique() ? serverNameRegex : servicePort.getName()); + Ingress ingress = kubernetesEnvironment.getIngresses().get(ingressName); IngressRule ingressRule = ingress.getSpec().getRules().get(0); - assertEquals(ingressRule.getHost(), serviceName + "-" + servicePort.getName() + "." + DOMAIN); + assertEquals(ingressRule.getHost(), ingressName + "." + DOMAIN); assertEquals(ingressRule.getHttp().getPaths().get(0).getPath(), "/"); IngressBackend backend = ingressRule.getHttp().getPaths().get(0).getBackend(); assertEquals(backend.getServiceName(), serviceName); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisionerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisionerTest.java index 6d8c296497a..d22f03cddcb 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisionerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxyProvisionerTest.java @@ -129,7 +129,8 @@ public void shouldProvisionJwtProxyRelatedObjectsIntoKubernetesEnvironment() thr port.setTargetPort(new IntOrString(4401)); // when - jwtProxyProvisioner.expose(k8sEnv, "terminal", port, "TCP", "server", secureServer); + jwtProxyProvisioner.expose( + k8sEnv, "terminal", port, "TCP", ImmutableMap.of("server", secureServer)); // then InternalMachineConfig jwtProxyMachine = @@ -157,7 +158,7 @@ public void shouldProvisionJwtProxyRelatedObjectsIntoKubernetesEnvironment() thr expectedExceptionsMessageRegExp = "Secure servers which expose the same port should have " + "the same `cookiesAuthEnabled` value\\.") - public void shouldThrowAnExceptionIsServersHaveDifferentValueForCookiesAuthEnabled() + public void shouldThrowAnExceptionIfServersHaveDifferentValueForCookiesAuthEnabled() throws Exception { // given ServerConfigImpl server1 = @@ -178,7 +179,12 @@ public void shouldThrowAnExceptionIsServersHaveDifferentValueForCookiesAuthEnabl port.setTargetPort(new IntOrString(4401)); // when - jwtProxyProvisioner.expose(k8sEnv, "terminal", port, "TCP", "server1", server1); + jwtProxyProvisioner.expose( + k8sEnv, + "terminal", + port, + "TCP", + ImmutableMap.of("server1", server1, "server2", server2, "server3", server3)); } @Test @@ -214,7 +220,8 @@ public void shouldUseCookiesAuthEnabledFromServersConfigs() throws Exception { port.setTargetPort(new IntOrString(4401)); // when - jwtProxyProvisioner.expose(k8sEnv, "terminal", port, "TCP", "server1", server1); + jwtProxyProvisioner.expose( + k8sEnv, "terminal", port, "TCP", ImmutableMap.of("server1", server1)); // then verify(configBuilder).addVerifierProxy(any(), any(), any(), eq(true), any(), any()); @@ -243,7 +250,8 @@ public void shouldFalseValueAsDefaultForCookiesAuthEnabledAttribute() throws Exc port.setTargetPort(new IntOrString(4401)); // when - jwtProxyProvisioner.expose(k8sEnv, "terminal", port, "TCP", "server1", server1); + jwtProxyProvisioner.expose( + k8sEnv, "terminal", port, "TCP", ImmutableMap.of("server1", server1)); // then verify(configBuilder).addVerifierProxy(any(), any(), any(), eq(false), any(), any()); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java index 3ffdf51b9b5..ca8a07944c9 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java @@ -72,7 +72,7 @@ public void shouldExposeSecureServersWithNewJwtProxyServicePort() throws Excepti ServicePort jwtProxyServicePort = new ServicePort(); doReturn(jwtProxyServicePort) .when(jwtProxyProvisioner) - .expose(any(), anyString(), any(), anyString(), any(), any()); + .expose(any(), anyString(), any(), anyString(), any()); when(jwtProxyProvisioner.getServiceName()).thenReturn(JWT_PROXY_SERVICE_NAME); @@ -82,8 +82,7 @@ public void shouldExposeSecureServersWithNewJwtProxyServicePort() throws Excepti // then verify(jwtProxyProvisioner) - .expose( - eq(k8sEnv), eq(MACHINE_SERVICE_NAME), eq(machineServicePort), eq("TCP"), any(), any()); + .expose(eq(k8sEnv), eq(MACHINE_SERVICE_NAME), eq(machineServicePort), eq("TCP"), any()); verify(externalServerExposer) .expose( eq(k8sEnv), diff --git a/infrastructures/openshift/pom.xml b/infrastructures/openshift/pom.xml index a7a008cda1e..8092bb3d002 100644 --- a/infrastructures/openshift/pom.xml +++ b/infrastructures/openshift/pom.xml @@ -95,10 +95,6 @@ org.eclipse.che.core che-core-commons-inject - - org.eclipse.che.core - che-core-commons-lang - org.eclipse.che.core che-core-commons-tracing diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java index 33fb57f947b..a9c1d3e671f 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java @@ -17,10 +17,11 @@ import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.openshift.api.model.Route; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; -import org.eclipse.che.commons.lang.NameGenerator; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; +import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; @@ -102,21 +103,34 @@ public void expose( ServicePort servicePort, Map externalServers) { + Map nonUniqueServers = new HashMap<>(); + for (Map.Entry e : externalServers.entrySet()) { - Route route = + if (e.getValue().isUnique()) { + Route route = + new RouteBuilder() + .withName(Names.generateName("route")) + .withMachineName(machineName) + .withTargetPort(servicePort.getName()) + .withServer(makeValidDnsName(e.getKey()), e.getValue()) + .withTo(serviceName) + .build(); + openShiftEnvironment.getRoutes().put(route.getMetadata().getName(), route); + } else { + nonUniqueServers.put(makeValidDnsName(e.getKey()), e.getValue()); + } + } + + if (!nonUniqueServers.isEmpty()) { + Route commonRoute = new RouteBuilder() - .withName( - NameGenerator.generate("route", 3) - + "-" - + makeValidDnsName(machineName) - + "-" - + makeValidDnsName(e.getKey())) + .withName(Names.generateName("route")) .withMachineName(machineName) .withTargetPort(servicePort.getName()) - .withServer(makeValidDnsName(e.getKey()), e.getValue()) + .withServers(nonUniqueServers) .withTo(serviceName) .build(); - openShiftEnvironment.getRoutes().put(route.getMetadata().getName(), route); + openShiftEnvironment.getRoutes().put(commonRoute.getMetadata().getName(), commonRoute); } } @@ -125,8 +139,7 @@ private static class RouteBuilder { private String name; private String serviceName; private IntOrString targetPort; - private String serverName; - private ServerConfig serverConfig; + private Map servers; private String machineName; private RouteBuilder withName(String name) { @@ -145,8 +158,16 @@ private RouteBuilder withTargetPort(String targetPortName) { } private RouteBuilder withServer(String serverName, ServerConfig serverConfig) { - this.serverName = serverName; - this.serverConfig = serverConfig; + return withServers(ImmutableMap.of(serverName, serverConfig)); + } + + private RouteBuilder withServers(Map servers) { + if (this.servers == null) { + this.servers = new HashMap<>(); + } + + this.servers.putAll(servers); + return this; } @@ -163,10 +184,7 @@ private Route build() { .withNewMetadata() .withName(name.replace("/", "-")) .withAnnotations( - Annotations.newSerializer() - .servers(ImmutableMap.of(serverName, serverConfig)) - .machineName(machineName) - .annotations()) + Annotations.newSerializer().servers(servers).machineName(machineName).annotations()) .endMetadata() .withNewSpec() .withNewTo() diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposerTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposerTest.java index 2890d0f003c..2b3b4d73243 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposerTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposerTest.java @@ -19,6 +19,7 @@ import java.util.HashMap; import java.util.Map; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; +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.Annotations.Deserializer; import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; @@ -39,6 +40,7 @@ public void shouldAddRouteToEnvForExposingSpecifiedServer() { // given OpenShiftEnvironment osEnv = OpenShiftEnvironment.builder().build(); Map servers = new HashMap<>(); + servers.put("server", new ServerConfigImpl()); // when osExternalServerExposer.expose( diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceService.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceService.java index 52619579241..1003548ecf4 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceService.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceService.java @@ -972,8 +972,7 @@ private WorkspaceDto filterServers(WorkspaceDto workspace, boolean includeIntern .getServers() .forEach( (name, server) -> { - if (!"true" - .equals(server.getAttributes().get(ServerConfig.INTERNAL_SERVER_ATTRIBUTE))) { + if (!ServerConfig.isInternal(server.getAttributes())) { filteredServers.put(name, server); } }); diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/InternalRuntime.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/InternalRuntime.java index b2cb702877e..4055a337f7e 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/InternalRuntime.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/InternalRuntime.java @@ -12,7 +12,6 @@ package org.eclipse.che.api.workspace.server.spi; import static java.util.stream.Collectors.toMap; -import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.INTERNAL_SERVER_ATTRIBUTE; import java.util.HashMap; import java.util.List; @@ -20,6 +19,7 @@ import org.eclipse.che.api.core.model.workspace.Warning; import org.eclipse.che.api.core.model.workspace.WorkspaceStatus; import org.eclipse.che.api.core.model.workspace.config.Command; +import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.api.core.model.workspace.runtime.Machine; import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; import org.eclipse.che.api.core.model.workspace.runtime.Server; @@ -219,7 +219,7 @@ private Map rewriteExternalServers( for (Map.Entry entry : incoming.entrySet()) { String name = entry.getKey(); Server incomingServer = entry.getValue(); - if (Boolean.parseBoolean(incomingServer.getAttributes().get(INTERNAL_SERVER_ATTRIBUTE))) { + if (ServerConfig.isInternal(incomingServer.getAttributes())) { outgoing.put(name, incomingServer); } else { try { From 93d85fde7f86d543f2c1e6bfc24ed3807ed0e9fa Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Mon, 20 Jan 2020 08:57:25 +0100 Subject: [PATCH 4/8] Factor out common server splitting logic by their exposure uniqueness. Signed-off-by: Lukas Krejci --- .../external/ExternalServerExposer.java | 97 ++++++++++--------- .../jwtproxy/JwtProxySecureServerExposer.java | 57 +++++------ .../OpenShiftExternalServerExposer.java | 44 +++------ 3 files changed, 86 insertions(+), 112 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java index 55aa0754522..ffbd982bb43 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java @@ -14,12 +14,9 @@ import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.api.model.extensions.Ingress; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.Objects; -import java.util.stream.Collectors; +import java.util.function.BiConsumer; import javax.inject.Inject; import javax.inject.Named; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; @@ -49,6 +46,36 @@ public ExternalServerExposer( this.pathTransformFmt = pathTransformFmt == null ? PATH_TRANSFORM_PATH_CATCH : pathTransformFmt; } + /** + * A helper method to split the servers to unique sets that should be exposed together. + * + *

The consumer is responsible for doing the actual exposure and is supplied 2 pieces of data. + * The first is the server ID, which is non-null for any unique server from the input set and null + * for any compound set of servers that should be exposed together. The caller is responsible for + * figuring out an appropriate ID in such case. + * + * @param allServers all unique and non-unique servers mixed together + * @param consumer the consumer responsible for handling the split sets of servers + */ + public static void onEachExposableServerSet( + Map allServers, + BiConsumer> consumer) { + Map nonUniqueServers = new HashMap<>(); + + for (Map.Entry e : allServers.entrySet()) { + String serverId = makeValidDnsName(e.getKey()); + if (e.getValue().isUnique()) { + consumer.accept(serverId, ImmutableMap.of(serverId, e.getValue())); + } else { + nonUniqueServers.put(serverId, e.getValue()); + } + } + + if (!nonUniqueServers.isEmpty()) { + consumer.accept(null, nonUniqueServers); + } + } + /** * Exposes service ports on given service externally (outside kubernetes cluster). Each exposed * service port is associated with a specific Server configuration. Server configuration should be @@ -66,56 +93,32 @@ public void expose( String serviceName, ServicePort servicePort, Map externalServers) { - List ingresses = - generateIngresses(machineName, serviceName, servicePort, externalServers); - for (Ingress ingress : ingresses) { - k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); - } + + onEachExposableServerSet( + externalServers, + (serverId, servers) -> { + if (serverId == null) { + // this is the ID for non-unique servers + serverId = servicePort.getName(); + } + + exposeServers(k8sEnv, machineName, serviceName, serverId, servicePort, servers); + }); } - private List generateIngresses( + /** Exposes the given set of servers using a single ingress/route. */ + protected void exposeServers( + T k8sEnv, String machineName, String serviceName, + String serverId, ServicePort servicePort, - Map ingressesServers) { - - // all the unique servers need their separate ingresses. all the other servers can get lumped - // under a common ingress corresponding to the server and port. - Map nonUniqueServers = new HashMap<>(); + Map externalServers) { - // let's go through the servers. Build the unique ingresses straight away and collect the - // servers to be put behind the common ingress. - List ingresses = - ingressesServers - .entrySet() - .stream() - .map( - e -> { - ServerConfig serverConfig = e.getValue(); - - if (!serverConfig.isUnique()) { - nonUniqueServers.put(e.getKey(), serverConfig); - return null; - } else { - return generateIngress( - machineName, - serviceName, - e.getKey(), - servicePort, - ImmutableMap.of(e.getKey(), serverConfig)); - } - }) - .filter(Objects::nonNull) - .collect(Collectors.toCollection(ArrayList::new)); - - // now generate the common ingress unconditionally, even if there are no servers - if (!nonUniqueServers.isEmpty()) { - ingresses.add( - generateIngress( - machineName, serviceName, servicePort.getName(), servicePort, nonUniqueServers)); - } + Ingress ingress = + generateIngress(machineName, serviceName, serverId, servicePort, externalServers); - return ingresses; + k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); } private Ingress generateIngress( diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java index 6a8a008ac3d..c626c6b4a32 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java @@ -12,11 +12,10 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableMap; import com.google.inject.assistedinject.Assisted; import io.fabric8.kubernetes.api.model.ServicePort; -import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import javax.inject.Inject; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; @@ -71,41 +70,33 @@ public void expose( Map secureServers) throws InfrastructureException { - Map nonUniqueServers = new HashMap<>(); + AtomicReference exceptionHolder = new AtomicReference<>(); - for (Map.Entry e : secureServers.entrySet()) { - String serverName = e.getKey(); - ServerConfig serverConfig = e.getValue(); + ExternalServerExposer.onEachExposableServerSet( + secureServers, + (serverId, servers) -> { + if (exceptionHolder.get() != null) { + return; + } - if (serverConfig.isUnique()) { - Map serverMapping = ImmutableMap.of(serverName, serverConfig); + try { + ServicePort exposedServicePort = + proxyProvisioner.expose( + k8sEnv, serviceName, servicePort, servicePort.getProtocol(), servers); - ServicePort exposedServicePort = - proxyProvisioner.expose( - k8sEnv, serviceName, servicePort, servicePort.getProtocol(), serverMapping); + exposer.expose( + k8sEnv, + machineName, + proxyProvisioner.getServiceName(), + exposedServicePort, + servers); + } catch (InfrastructureException e) { + exceptionHolder.set(e); + } + }); - exposer.expose( - k8sEnv, - machineName, - proxyProvisioner.getServiceName(), - exposedServicePort, - serverMapping); - } else { - nonUniqueServers.put(serverName, serverConfig); - } - } - - if (!nonUniqueServers.isEmpty()) { - ServicePort exposedServicePort = - proxyProvisioner.expose( - k8sEnv, serviceName, servicePort, servicePort.getProtocol(), nonUniqueServers); - - exposer.expose( - k8sEnv, - machineName, - proxyProvisioner.getServiceName(), - exposedServicePort, - nonUniqueServers); + if (exceptionHolder.get() != null) { + throw exceptionHolder.get(); } } } diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java index a9c1d3e671f..4b4c585a4b7 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java @@ -96,42 +96,22 @@ public OpenShiftExternalServerExposer() { } @Override - public void expose( - OpenShiftEnvironment openShiftEnvironment, + protected void exposeServers( + OpenShiftEnvironment env, String machineName, String serviceName, + String serverId, ServicePort servicePort, Map externalServers) { - - Map nonUniqueServers = new HashMap<>(); - - for (Map.Entry e : externalServers.entrySet()) { - if (e.getValue().isUnique()) { - Route route = - new RouteBuilder() - .withName(Names.generateName("route")) - .withMachineName(machineName) - .withTargetPort(servicePort.getName()) - .withServer(makeValidDnsName(e.getKey()), e.getValue()) - .withTo(serviceName) - .build(); - openShiftEnvironment.getRoutes().put(route.getMetadata().getName(), route); - } else { - nonUniqueServers.put(makeValidDnsName(e.getKey()), e.getValue()); - } - } - - if (!nonUniqueServers.isEmpty()) { - Route commonRoute = - new RouteBuilder() - .withName(Names.generateName("route")) - .withMachineName(machineName) - .withTargetPort(servicePort.getName()) - .withServers(nonUniqueServers) - .withTo(serviceName) - .build(); - openShiftEnvironment.getRoutes().put(commonRoute.getMetadata().getName(), commonRoute); - } + Route commonRoute = + new RouteBuilder() + .withName(Names.generateName("route")) + .withMachineName(machineName) + .withTargetPort(servicePort.getName()) + .withServers(externalServers) + .withTo(serviceName) + .build(); + env.getRoutes().put(commonRoute.getMetadata().getName(), commonRoute); } private static class RouteBuilder { From d46eb20398472a8e2a751002b1ae0ac85b5f56ef Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Mon, 20 Jan 2020 17:26:48 +0100 Subject: [PATCH 5/8] Expose the preview URLs in the way consistent with the original functionality that is independent of the workspace config and is only based on the k8s objects deployed in the environment. Signed-off-by: Lukas Krejci --- .../kubernetes/server/PreviewUrlExposer.java | 21 ++++++------------- .../external/ExternalServerExposer.java | 6 +++--- .../OpenShiftExternalServerExposer.java | 2 +- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java index 379a228a8cb..5dad3b9b286 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java @@ -16,7 +16,6 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_PREFIX; import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_UNIQUE_PART_SIZE; -import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServicePort; @@ -29,7 +28,6 @@ import javax.inject.Inject; import javax.inject.Singleton; import org.eclipse.che.api.workspace.server.model.impl.CommandImpl; -import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; import org.eclipse.che.api.workspace.server.spi.InternalInfrastructureException; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; @@ -75,15 +73,9 @@ public void expose(T env) throws InternalInfrastructureException { String.format( "Port '%d' in service '%s' not found. This is not expected, please report a bug!", port, foundService.get().getMetadata().getName()))); - externalServerExposer.expose( - env, - null, - foundService.get().getMetadata().getName(), - servicePort, - ImmutableMap.of( - command.getName(), - new ServerConfigImpl( - servicePort.getName(), "http", "/", Collections.emptyMap()))); + String serviceName = foundService.get().getMetadata().getName(); + externalServerExposer.exposeAsSingle( + env, null, serviceName, serviceName, servicePort, Collections.emptyMap()); } } else { portsToProvision.add(createServicePort(port)); @@ -97,14 +89,13 @@ public void expose(T env) throws InternalInfrastructureException { env.getServices().put(serverName, service); portsToProvision.forEach( port -> - externalServerExposer.expose( + externalServerExposer.exposeAsSingle( env, null, service.getMetadata().getName(), + service.getMetadata().getName(), port, - ImmutableMap.of( - serverName, - new ServerConfigImpl(port.getName(), "http", "/", Collections.emptyMap())))); + Collections.emptyMap())); } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java index ffbd982bb43..31e3cde46b6 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java @@ -102,14 +102,14 @@ public void expose( serverId = servicePort.getName(); } - exposeServers(k8sEnv, machineName, serviceName, serverId, servicePort, servers); + exposeAsSingle(k8sEnv, machineName, serviceName, serverId, servicePort, servers); }); } /** Exposes the given set of servers using a single ingress/route. */ - protected void exposeServers( + public void exposeAsSingle( T k8sEnv, - String machineName, + @Nullable String machineName, String serviceName, String serverId, ServicePort servicePort, diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java index 4b4c585a4b7..413e6f60fe5 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java @@ -96,7 +96,7 @@ public OpenShiftExternalServerExposer() { } @Override - protected void exposeServers( + public void exposeAsSingle( OpenShiftEnvironment env, String machineName, String serviceName, From d2e7bc654a3a549a9e375acd9d419b91dc60ec17 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 22 Jan 2020 14:10:23 +0100 Subject: [PATCH 6/8] Don't call `forEachExposableServerSet` unnecessarily during JWT proxy exposure. Signed-off-by: Lukas Krejci --- .../server/secure/jwtproxy/JwtProxySecureServerExposer.java | 3 ++- .../secure/jwtproxy/JwtProxySecureServerExposerTest.java | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java index c626c6b4a32..2362a80f921 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java @@ -84,9 +84,10 @@ public void expose( proxyProvisioner.expose( k8sEnv, serviceName, servicePort, servicePort.getProtocol(), servers); - exposer.expose( + exposer.exposeAsSingle( k8sEnv, machineName, + serverId, proxyProvisioner.getServiceName(), exposedServicePort, servers); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java index ca8a07944c9..90ff0a6ddf2 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java @@ -14,6 +14,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -84,9 +85,10 @@ public void shouldExposeSecureServersWithNewJwtProxyServicePort() throws Excepti verify(jwtProxyProvisioner) .expose(eq(k8sEnv), eq(MACHINE_SERVICE_NAME), eq(machineServicePort), eq("TCP"), any()); verify(externalServerExposer) - .expose( + .exposeAsSingle( eq(k8sEnv), eq(MACHINE_NAME), + isNull(), eq(JWT_PROXY_SERVICE_NAME), eq(jwtProxyServicePort), eq(servers)); From a64818a68606dce2f77587d02afc9ccf42060d92 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Fri, 24 Jan 2020 17:19:43 +0100 Subject: [PATCH 7/8] Rework the flow such that the servers are split to sets only on a single location. Signed-off-by: Lukas Krejci --- .../server/KubernetesServerExposer.java | 58 +++++++++- .../kubernetes/server/PreviewUrlExposer.java | 4 +- .../external/ExternalServerExposer.java | 69 ++---------- .../secure/DefaultSecureServersFactory.java | 3 +- .../server/secure/SecureServerExposer.java | 3 + .../jwtproxy/JwtProxySecureServerExposer.java | 40 ++----- .../server/KubernetesServerExposerTest.java | 104 ++++++++++++++++-- ...stExternalServiceExposureStrategyTest.java | 46 +------- ...stExternalServiceExposureStrategyTest.java | 92 +--------------- .../JwtProxySecureServerExposerTest.java | 4 +- .../OpenShiftExternalServerExposer.java | 2 +- .../OpenShiftExternalServerExposerTest.java | 1 + 12 files changed, 183 insertions(+), 243 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposer.java index 9a54ed296f1..cccd0ef27c4 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposer.java @@ -16,6 +16,7 @@ import static org.eclipse.che.commons.lang.NameGenerator.generate; import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.CHE_ORIGINAL_NAME_LABEL; +import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.ContainerPort; import io.fabric8.kubernetes.api.model.ContainerPortBuilder; @@ -125,6 +126,41 @@ public KubernetesServerExposer( this.k8sEnv = k8sEnv; } + /** + * A helper method to split the servers to unique sets that should be exposed together. + * + *

The consumer is responsible for doing the actual exposure and is supplied 2 pieces of data. + * The first is the server ID, which is non-null for any unique server from the input set and null + * for any compound set of servers that should be exposed together. The caller is responsible for + * figuring out an appropriate ID in such case. + * + * @param allServers all unique and non-unique servers mixed together + * @param consumer the consumer responsible for handling the split sets of servers + */ + private static void onEachExposableServerSet( + Map allServers, ServerSetExposer consumer) + throws InfrastructureException { + Map nonUniqueServers = new HashMap<>(); + + for (Map.Entry e : allServers.entrySet()) { + String serverId = makeServerNameValidForDns(e.getKey()); + if (e.getValue().isUnique()) { + consumer.expose(serverId, ImmutableMap.of(serverId, e.getValue())); + } else { + nonUniqueServers.put(serverId, e.getValue()); + } + } + + if (!nonUniqueServers.isEmpty()) { + consumer.expose(null, nonUniqueServers); + } + } + + /** Replaces {@code /} with {@code -} in the provided name in an attempt to make it DNS safe. */ + public static String makeServerNameValidForDns(String name) { + return name.replaceAll("/", "-"); + } + /** * Exposes specified servers. * @@ -173,15 +209,23 @@ public void expose(Map servers) throws Infrastru // expose service port related external servers if exist Map matchedExternalServers = match(externalServers, servicePort); if (!matchedExternalServers.isEmpty()) { - externalServerExposer.expose( - k8sEnv, machineName, serviceName, servicePort, matchedExternalServers); + onEachExposableServerSet( + matchedExternalServers, + (serverId, srvrs) -> { + externalServerExposer.expose( + k8sEnv, machineName, serviceName, serverId, servicePort, srvrs); + }); } // expose service port related secure servers if exist Map matchedSecureServers = match(secureServers, servicePort); if (!matchedSecureServers.isEmpty()) { - secureServerExposer.expose( - k8sEnv, machineName, serviceName, servicePort, matchedSecureServers); + onEachExposableServerSet( + matchedSecureServers, + (serverId, srvrs) -> { + secureServerExposer.expose( + k8sEnv, machineName, serviceName, serverId, servicePort, matchedSecureServers); + }); } } } @@ -232,4 +276,10 @@ private Collection exposePorts(Collection s } return exposedPorts.values(); } + + @FunctionalInterface + private interface ServerSetExposer { + void expose(String serverId, Map serverSet) + throws InfrastructureException; + } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java index 5dad3b9b286..2cae39b2d5a 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/PreviewUrlExposer.java @@ -74,7 +74,7 @@ public void expose(T env) throws InternalInfrastructureException { "Port '%d' in service '%s' not found. This is not expected, please report a bug!", port, foundService.get().getMetadata().getName()))); String serviceName = foundService.get().getMetadata().getName(); - externalServerExposer.exposeAsSingle( + externalServerExposer.expose( env, null, serviceName, serviceName, servicePort, Collections.emptyMap()); } } else { @@ -89,7 +89,7 @@ public void expose(T env) throws InternalInfrastructureException { env.getServices().put(serverName, service); portsToProvision.forEach( port -> - externalServerExposer.exposeAsSingle( + externalServerExposer.expose( env, null, service.getMetadata().getName(), diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java index 31e3cde46b6..172421146cb 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java @@ -11,17 +11,15 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; -import com.google.common.collect.ImmutableMap; import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.api.model.extensions.Ingress; -import java.util.HashMap; import java.util.Map; -import java.util.function.BiConsumer; import javax.inject.Inject; import javax.inject.Named; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; public class ExternalServerExposer { @@ -47,67 +45,19 @@ public ExternalServerExposer( } /** - * A helper method to split the servers to unique sets that should be exposed together. - * - *

The consumer is responsible for doing the actual exposure and is supplied 2 pieces of data. - * The first is the server ID, which is non-null for any unique server from the input set and null - * for any compound set of servers that should be exposed together. The caller is responsible for - * figuring out an appropriate ID in such case. - * - * @param allServers all unique and non-unique servers mixed together - * @param consumer the consumer responsible for handling the split sets of servers - */ - public static void onEachExposableServerSet( - Map allServers, - BiConsumer> consumer) { - Map nonUniqueServers = new HashMap<>(); - - for (Map.Entry e : allServers.entrySet()) { - String serverId = makeValidDnsName(e.getKey()); - if (e.getValue().isUnique()) { - consumer.accept(serverId, ImmutableMap.of(serverId, e.getValue())); - } else { - nonUniqueServers.put(serverId, e.getValue()); - } - } - - if (!nonUniqueServers.isEmpty()) { - consumer.accept(null, nonUniqueServers); - } - } - - /** - * Exposes service ports on given service externally (outside kubernetes cluster). Each exposed + * Exposes service port on given service externally (outside kubernetes cluster). The exposed * service port is associated with a specific Server configuration. Server configuration should be * encoded in the exposing object's annotations, to be used by {@link KubernetesServerResolver}. * * @param k8sEnv Kubernetes environment * @param machineName machine containing servers * @param serviceName service associated with machine, mapping all machine server ports + * @param serverId non-null for a unique server, null for a compound set of servers that should be + * exposed together. * @param servicePort specific service port to be exposed externally * @param externalServers server configs of servers to be exposed externally */ public void expose( - T k8sEnv, - String machineName, - String serviceName, - ServicePort servicePort, - Map externalServers) { - - onEachExposableServerSet( - externalServers, - (serverId, servers) -> { - if (serverId == null) { - // this is the ID for non-unique servers - serverId = servicePort.getName(); - } - - exposeAsSingle(k8sEnv, machineName, serviceName, serverId, servicePort, servers); - }); - } - - /** Exposes the given set of servers using a single ingress/route. */ - public void exposeAsSingle( T k8sEnv, @Nullable String machineName, String serviceName, @@ -115,6 +65,11 @@ public void exposeAsSingle( ServicePort servicePort, Map externalServers) { + if (serverId == null) { + // this is the ID for non-unique servers + serverId = servicePort.getName(); + } + Ingress ingress = generateIngress(machineName, serviceName, serverId, servicePort, externalServers); @@ -128,7 +83,7 @@ private Ingress generateIngress( ServicePort servicePort, Map servers) { - String serverName = makeValidDnsName(serverId); + String serverName = KubernetesServerExposer.makeServerNameValidForDns(serverId); ExternalServerIngressBuilder ingressBuilder = new ExternalServerIngressBuilder(); String host = strategy.getExternalHost(serviceName, serverName); if (host != null) { @@ -156,8 +111,4 @@ private static String ensureEndsWithSlash(String path) { private static String getIngressName(String serviceName, String serverName) { return serviceName + "-" + serverName; } - - protected static String makeValidDnsName(String name) { - return name.replaceAll("/", "-"); - } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/DefaultSecureServersFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/DefaultSecureServersFactory.java index c2eb5dc6598..284c8d85bb4 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/DefaultSecureServersFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/DefaultSecureServersFactory.java @@ -46,9 +46,10 @@ public void expose( T k8sEnv, String machineName, String serviceName, + String serverId, ServicePort servicePort, Map secureServers) { - exposer.expose(k8sEnv, machineName, serviceName, servicePort, secureServers); + exposer.expose(k8sEnv, machineName, serviceName, serverId, servicePort, secureServers); } } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/SecureServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/SecureServerExposer.java index 4dd1c39c99f..0529827ed90 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/SecureServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/SecureServerExposer.java @@ -32,6 +32,8 @@ public interface SecureServerExposer { * @param k8sEnv Kubernetes environment that should be modified. * @param machineName machine name to which secure servers belong to * @param serviceName service name that exposes secure servers + * @param serverId non-null for a unique server, null for a compound set of servers that should be + * exposed together. * @param servicePort service port that exposes secure servers * @param secureServers secure servers to expose * @throws InfrastructureException when any exception occurs during servers exposing @@ -40,6 +42,7 @@ void expose( T k8sEnv, String machineName, String serviceName, + String serverId, ServicePort servicePort, Map secureServers) throws InfrastructureException; diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java index 2362a80f921..a6a83f6c22e 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java @@ -15,7 +15,6 @@ import com.google.inject.assistedinject.Assisted; import io.fabric8.kubernetes.api.model.ServicePort; import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; import javax.inject.Inject; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; @@ -66,38 +65,21 @@ public void expose( T k8sEnv, String machineName, String serviceName, + String serverId, ServicePort servicePort, Map secureServers) throws InfrastructureException { - AtomicReference exceptionHolder = new AtomicReference<>(); + ServicePort exposedServicePort = + proxyProvisioner.expose( + k8sEnv, serviceName, servicePort, servicePort.getProtocol(), secureServers); - ExternalServerExposer.onEachExposableServerSet( - secureServers, - (serverId, servers) -> { - if (exceptionHolder.get() != null) { - return; - } - - try { - ServicePort exposedServicePort = - proxyProvisioner.expose( - k8sEnv, serviceName, servicePort, servicePort.getProtocol(), servers); - - exposer.exposeAsSingle( - k8sEnv, - machineName, - serverId, - proxyProvisioner.getServiceName(), - exposedServicePort, - servers); - } catch (InfrastructureException e) { - exceptionHolder.set(e); - } - }); - - if (exceptionHolder.get() != null) { - throw exceptionHolder.get(); - } + exposer.expose( + k8sEnv, + machineName, + serverId, + proxyProvisioner.getServiceName(), + exposedServicePort, + secureServers); } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposerTest.java index 8bc2c94332f..c4c0009c761 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerExposerTest.java @@ -15,6 +15,8 @@ import static java.util.Collections.singletonMap; import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_PREFIX; import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_UNIQUE_PART_SIZE; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; @@ -24,10 +26,12 @@ import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.ContainerBuilder; import io.fabric8.kubernetes.api.model.ContainerPortBuilder; +import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodBuilder; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServicePort; +import io.fabric8.kubernetes.api.model.ServicePortBuilder; import java.util.ArrayList; import java.util.Map; import java.util.Map.Entry; @@ -35,6 +39,7 @@ import java.util.regex.Pattern; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; +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.environment.KubernetesEnvironment.PodData; @@ -67,6 +72,9 @@ public class KubernetesServerExposerTest { private static final Pattern SERVER_PREFIX_REGEX = Pattern.compile('^' + SERVER_PREFIX + "[A-z0-9]{" + SERVER_UNIQUE_PART_SIZE + "}-pod-main$"); private static final String MACHINE_NAME = "pod/main"; + private static final Map UNIQUE_SERVER_ATTRIBUTES = + ImmutableMap.of("key", "value", ServerConfig.UNIQUE_SERVER_ATTRIBUTE, "true"); + private static final String SERVICE_NAME = SERVER_PREFIX + "12345678" + "-" + MACHINE_NAME; private KubernetesServerExposer serverExposer; private KubernetesEnvironment kubernetesEnvironment; @@ -313,6 +321,80 @@ public void shouldExposeInternalAndExternalAndSecureServers() throws Exception { MACHINE_NAME, "tcp", 8282, "secure-server", new ServerConfigImpl(secureServerConfig)); } + @Test + public void shouldCreateIngressPerUniqueServerWithTheSamePort() throws Exception { + // given + ServerConfigImpl httpServerConfig = + new ServerConfigImpl("8080/tcp", "http", "/api", UNIQUE_SERVER_ATTRIBUTES); + ServerConfigImpl wsServerConfig = + new ServerConfigImpl("8080/tcp", "ws", "/connect", UNIQUE_SERVER_ATTRIBUTES); + ServicePort servicePort = + new ServicePortBuilder() + .withName("server-8080") + .withPort(8080) + .withProtocol("TCP") + .withTargetPort(new IntOrString(8080)) + .build(); + + Map serversToExpose = + ImmutableMap.of( + "http-server", httpServerConfig, + "ws-server", wsServerConfig); + + // when + serverExposer.expose(serversToExpose); + + // then + assertThatExternalServerIsExposed( + MACHINE_NAME, + "tcp", + 8080, + "http-server", + new ServerConfigImpl(httpServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); + assertThatExternalServerIsExposed( + MACHINE_NAME, + "tcp", + 8080, + "ws-server", + new ServerConfigImpl(wsServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); + } + + @Test + public void shouldCreateIngressForServerWhenTwoServersHasTheSamePort() + throws InfrastructureException { + // given + ServerConfigImpl httpServerConfig = + new ServerConfigImpl("8080/tcp", "http", "/api", ATTRIBUTES_MAP); + ServerConfigImpl wsServerConfig = + new ServerConfigImpl("8080/tcp", "ws", "/connect", ATTRIBUTES_MAP); + IntOrString targetPort = new IntOrString(8080); + + ServicePort servicePort = + new ServicePortBuilder() + .withName("server-8080") + .withPort(8080) + .withProtocol("TCP") + .withTargetPort(targetPort) + .build(); + + Map serversToExpose = + ImmutableMap.of( + "http-server", httpServerConfig, + "ws-server", wsServerConfig); + + // when + serverExposer.expose(serversToExpose); + + // then + assertThatExternalServersAreExposed( + MACHINE_NAME, + "tcp", + 8080, + ImmutableMap.of( + "http-server", new ServerConfigImpl(httpServerConfig).withAttributes(ATTRIBUTES_MAP), + "ws-server", new ServerConfigImpl(wsServerConfig).withAttributes(ATTRIBUTES_MAP))); + } + @SuppressWarnings("SameParameterValue") private void assertThatExternalServerIsExposed( String machineName, @@ -346,11 +428,12 @@ private void assertThatExternalServersAreExposed( verify(externalServerExposer) .expose( - kubernetesEnvironment, - machineName, - service.getMetadata().getName(), - servicePort, - expectedServers); + eq(kubernetesEnvironment), + eq(machineName), + eq(service.getMetadata().getName()), + any(), + eq(servicePort), + eq(expectedServers)); } @SuppressWarnings("SameParameterValue") @@ -377,11 +460,12 @@ private void assertThatSecureServerIsExposed( verify(secureServerExposer) .expose( - kubernetesEnvironment, - machineName, - service.getMetadata().getName(), - servicePort, - ImmutableMap.of(serverName, serverConfig)); + eq(kubernetesEnvironment), + eq(machineName), + eq(service.getMetadata().getName()), + any(), + eq(servicePort), + eq(ImmutableMap.of(serverName, serverConfig))); } @SuppressWarnings("SameParameterValue") diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategyTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategyTest.java index 611d043df77..a486f7f64a6 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategyTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostExternalServiceExposureStrategyTest.java @@ -41,8 +41,6 @@ public class DefaultHostExternalServiceExposureStrategyTest { private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); - private static final Map UNIQUE_SERVER_ATTRIBUTES = - ImmutableMap.of("key", "value", ServerConfig.UNIQUE_SERVER_ATTRIBUTE, "true"); private static final String MACHINE_NAME = "pod/main"; private static final String SERVICE_NAME = SERVER_PREFIX + "12345678" + "-" + MACHINE_NAME; @@ -86,7 +84,7 @@ public void shouldCreateIngressForServer() { // when externalServerExposer.expose( - kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, servicePort, serversToExpose); + kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, null, servicePort, serversToExpose); // then assertThatExternalServerIsExposed( @@ -119,7 +117,7 @@ public void shouldCreateSingleIngressForTwoNonUniqueServersWithTheSamePort() { // when externalServerExposer.expose( - kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, servicePort, serversToExpose); + kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, null, servicePort, serversToExpose); // then assertEquals(kubernetesEnvironment.getIngresses().size(), 1); @@ -137,46 +135,6 @@ public void shouldCreateSingleIngressForTwoNonUniqueServersWithTheSamePort() { new ServerConfigImpl(wsServerConfig).withAttributes(ATTRIBUTES_MAP)); } - @Test - public void shouldCreateIngressPerUniqueServerWithTheSamePort() { - // given - ServerConfigImpl httpServerConfig = - new ServerConfigImpl("8080/tcp", "http", "/api", UNIQUE_SERVER_ATTRIBUTES); - ServerConfigImpl wsServerConfig = - new ServerConfigImpl("8080/tcp", "ws", "/connect", UNIQUE_SERVER_ATTRIBUTES); - ServicePort servicePort = - new ServicePortBuilder() - .withName("server-8080") - .withPort(8080) - .withProtocol("TCP") - .withTargetPort(new IntOrString(8080)) - .build(); - - Map serversToExpose = - ImmutableMap.of( - "http-server", httpServerConfig, - "ws-server", wsServerConfig); - - // when - externalServerExposer.expose( - kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, servicePort, serversToExpose); - - // then - assertEquals(kubernetesEnvironment.getIngresses().size(), 2); - assertThatExternalServerIsExposed( - MACHINE_NAME, - SERVICE_NAME, - "http-server", - servicePort, - new ServerConfigImpl(httpServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); - assertThatExternalServerIsExposed( - MACHINE_NAME, - SERVICE_NAME, - "ws-server", - servicePort, - new ServerConfigImpl(wsServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); - } - @SuppressWarnings("SameParameterValue") private void assertThatExternalServerIsExposed( String machineName, diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategyTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategyTest.java index 603c103fa0a..7f67b0dec75 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategyTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostExternalServiceExposureStrategyTest.java @@ -88,7 +88,7 @@ public void shouldCreateIngressForServer() { // when externalServerExposer.expose( - kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, servicePort, serversToExpose); + kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, null, servicePort, serversToExpose); // then assertThatExternalServerIsExposed( @@ -101,96 +101,6 @@ public void shouldCreateIngressForServer() { new ServerConfigImpl(httpServerConfig).withAttributes(ATTRIBUTES_MAP)); } - @Test - public void shouldCreateIngressForServerWhenTwoServersHasTheSamePort() { - // given - ServerConfigImpl httpServerConfig = - new ServerConfigImpl("8080/tcp", "http", "/api", ATTRIBUTES_MAP); - ServerConfigImpl wsServerConfig = - new ServerConfigImpl("8080/tcp", "ws", "/connect", ATTRIBUTES_MAP); - IntOrString targetPort = new IntOrString(8080); - - ServicePort servicePort = - new ServicePortBuilder() - .withName("server-8080") - .withPort(8080) - .withProtocol("TCP") - .withTargetPort(targetPort) - .build(); - - Map serversToExpose = - ImmutableMap.of( - "http-server", httpServerConfig, - "ws-server", wsServerConfig); - - // when - externalServerExposer.expose( - kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, servicePort, serversToExpose); - - // then - assertEquals(kubernetesEnvironment.getIngresses().size(), 1); - assertThatExternalServerIsExposed( - MACHINE_NAME, - SERVICE_NAME, - "http-server", - "tcp", - 8080, - servicePort, - new ServerConfigImpl(httpServerConfig).withAttributes(ATTRIBUTES_MAP)); - assertThatExternalServerIsExposed( - MACHINE_NAME, - SERVICE_NAME, - "ws-server", - "tcp", - 8080, - servicePort, - new ServerConfigImpl(wsServerConfig).withAttributes(ATTRIBUTES_MAP)); - } - - @Test - public void shouldCreateIngressPerUniqueServerWithTheSamePort() { - // given - ServerConfigImpl httpServerConfig = - new ServerConfigImpl("8080/tcp", "http", "/api", UNIQUE_SERVER_ATTRIBUTES); - ServerConfigImpl wsServerConfig = - new ServerConfigImpl("8080/tcp", "ws", "/connect", UNIQUE_SERVER_ATTRIBUTES); - ServicePort servicePort = - new ServicePortBuilder() - .withName("server-8080") - .withPort(8080) - .withProtocol("TCP") - .withTargetPort(new IntOrString(8080)) - .build(); - - Map serversToExpose = - ImmutableMap.of( - "http-server", httpServerConfig, - "ws-server", wsServerConfig); - - // when - externalServerExposer.expose( - kubernetesEnvironment, MACHINE_NAME, SERVICE_NAME, servicePort, serversToExpose); - - // then - assertEquals(kubernetesEnvironment.getIngresses().size(), 2); - assertThatExternalServerIsExposed( - MACHINE_NAME, - SERVICE_NAME, - "http-server", - "tcp", - 8080, - servicePort, - new ServerConfigImpl(httpServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); - assertThatExternalServerIsExposed( - MACHINE_NAME, - SERVICE_NAME, - "ws-server", - "tcp", - 8080, - servicePort, - new ServerConfigImpl(wsServerConfig).withAttributes(UNIQUE_SERVER_ATTRIBUTES)); - } - private void assertThatExternalServerIsExposed( String machineName, String serviceName, diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java index 90ff0a6ddf2..45b86edcdaf 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java @@ -79,13 +79,13 @@ public void shouldExposeSecureServersWithNewJwtProxyServicePort() throws Excepti // when secureServerExposer.expose( - k8sEnv, MACHINE_NAME, MACHINE_SERVICE_NAME, machineServicePort, servers); + k8sEnv, MACHINE_NAME, MACHINE_SERVICE_NAME, null, machineServicePort, servers); // then verify(jwtProxyProvisioner) .expose(eq(k8sEnv), eq(MACHINE_SERVICE_NAME), eq(machineServicePort), eq("TCP"), any()); verify(externalServerExposer) - .exposeAsSingle( + .expose( eq(k8sEnv), eq(MACHINE_NAME), isNull(), diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java index 413e6f60fe5..cbb73fe2c26 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposer.java @@ -96,7 +96,7 @@ public OpenShiftExternalServerExposer() { } @Override - public void exposeAsSingle( + public void expose( OpenShiftEnvironment env, String machineName, String serviceName, diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposerTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposerTest.java index 2b3b4d73243..9f938bc6641 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposerTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftExternalServerExposerTest.java @@ -47,6 +47,7 @@ public void shouldAddRouteToEnvForExposingSpecifiedServer() { osEnv, "machine123", "service123", + null, new ServicePort("servicePort", null, null, "TCP", null), servers); From 33610ea476eba2a614f38988cc54e6e6d096e5d5 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Mon, 27 Jan 2020 23:22:29 +0100 Subject: [PATCH 8/8] The serverId and serverName were passed in swapped. d'oh! Signed-off-by: Lukas Krejci --- .../server/secure/jwtproxy/JwtProxySecureServerExposer.java | 2 +- .../server/secure/jwtproxy/JwtProxySecureServerExposerTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java index a6a83f6c22e..185e509ad3a 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java @@ -77,8 +77,8 @@ public void expose( exposer.expose( k8sEnv, machineName, - serverId, proxyProvisioner.getServiceName(), + serverId, exposedServicePort, secureServers); } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java index 45b86edcdaf..8518cfa10ca 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposerTest.java @@ -88,8 +88,8 @@ public void shouldExposeSecureServersWithNewJwtProxyServicePort() throws Excepti .expose( eq(k8sEnv), eq(MACHINE_NAME), - isNull(), eq(JWT_PROXY_SERVICE_NAME), + isNull(), eq(jwtProxyServicePort), eq(servers)); }