From 5030c89cfec9e80cb12f6c230d0f042fc268679a Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Mon, 5 Aug 2019 21:41:39 +0200 Subject: [PATCH 01/12] Generalize the ingress configuration to support single-host kubernetes deployments better. Signed-off-by: Lukas Krejci --- .../webapp/WEB-INF/classes/che/che.properties | 26 +++++++-- .../helm/che/templates/configmap.yaml | 5 +- .../kubernetes/KubernetesInternalRuntime.java | 7 ++- .../server/KubernetesServerResolver.java | 16 +++++- .../ExternalServerExposerStrategy.java | 20 +++++++ ...ingleHostIngressExternalServerExposer.java | 51 +++++++++++++++++- .../KubernetesInternalRuntimeTest.java | 3 ++ .../server/KubernetesServerResolverTest.java | 53 +++++++++++++------ ...eHostIngressExternalServerExposerTest.java | 30 +++++++++++ .../openshift/OpenShiftInternalRuntime.java | 6 ++- .../server/OpenShiftServerResolver.java | 6 ++- .../OpenShiftInternalRuntimeTest.java | 3 ++ .../OpenShiftServerResolverTest.java | 23 +++++--- 13 files changed, 214 insertions(+), 35 deletions(-) create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposerTest.java diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index ed78111007d..ab072a85f07 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -479,13 +479,33 @@ che.infra.kubernetes.installer_server_min_port=10000 che.infra.kubernetes.installer_server_max_port=20000 # Defines annotations for ingresses which are used for servers exposing. Value depends on ingress controller. -# For example for nginx ingress controller 0.9.0-beta17 the following value is recommended: -# {"ingress.kubernetes.io/rewrite-target": "/","ingress.kubernetes.io/ssl-redirect": "false",\ -# "ingress.kubernetes.io/proxy-connect-timeout": "3600","ingress.kubernetes.io/proxy-read-timeout": "3600"} # # OpenShift infrastructure ignores this property because it uses Routes instead of ingresses. +# +# Note that for a single-host deployment strategy to work, a controller supporting URL rewriting has to be +# used (so that URLs can point to different servers while the servers don't need to support changing the app root). +# See the che.infra.kubernetes.ingress.path.rewrite_transform property for further details on how to achieve +# this. +# +# For example for nginx ingress controller 0.22.0 and later the following value is recommended: +# {"ingress.kubernetes.io/rewrite-target": "/$1","ingress.kubernetes.io/ssl-redirect": "false",\ +# "ingress.kubernetes.io/proxy-connect-timeout": "3600","ingress.kubernetes.io/proxy-read-timeout": "3600"} +# and the che.infra.kubernetes.ingress.path.rewrite_transform should be set to "%s(/?.*)" +# +# For nginx ingress controller older than 0.22.0, the rewrite-target should be set to merely "/" and the path transform +# to "%s" (see the the che.infra.kubernetes.ingress.path.rewrite_transform property). che.infra.kubernetes.ingress.annotations_json=NULL +# Defines a "recipe" on how to declare the path of the ingress that should expose a server. +# The "%s" represents the base public URL of the server. In the concrete example for nginx the "(/?.*)" part makes sure +# that any potential further sub-path is captured and can be used in the rewrite-target rule specified in +# the che.infra.kubernetes.ingress.annotations_json property (as is shown in the example value of that property). This +# property must be a valid input to the String.format() method and contain exactly one reference to "%s". +# +# Note that this property is only taken into the account when Che is deployed on Kubernetes with the single-host +# server strategy. +che.infra.kubernetes.ingress.path_transform=NULL + # Defines security context for pods that will be created by Kubernetes Infra # # This is ignored by OpenShift infra diff --git a/deploy/kubernetes/helm/che/templates/configmap.yaml b/deploy/kubernetes/helm/che/templates/configmap.yaml index d5aabc7adc6..1214fe15cee 100644 --- a/deploy/kubernetes/helm/che/templates/configmap.yaml +++ b/deploy/kubernetes/helm/che/templates/configmap.yaml @@ -68,10 +68,11 @@ data: JAVA_OPTS: "-XX:MaxRAMFraction=2 -XX:+UseParallelGC -XX:MinHeapFreeRatio=10 -XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -XX:+UnlockExperimentalVMOptions -XX:+UseCGroupMemoryLimitForHeap -Dsun.zip.disableMemoryMapping=true -Xms20m " CHE_WORKSPACE_AUTO_START: "false" {{- if .Values.global.tls.enabled }} - CHE_INFRA_KUBERNETES_INGRESS_ANNOTATIONS__JSON: '{"kubernetes.io/ingress.class": "nginx", "kubernetes.io/tls-acme": "true", "{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/rewrite-target": "/","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/ssl-redirect": "true","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-connect-timeout": "3600","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-read-timeout": "3600"}' + CHE_INFRA_KUBERNETES_INGRESS_ANNOTATIONS__JSON: '{"kubernetes.io/ingress.class": "nginx", "kubernetes.io/tls-acme": "true", "{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/rewrite-target": "/$1","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/ssl-redirect": "true","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-connect-timeout": "3600","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-read-timeout": "3600"}' {{- else }} - CHE_INFRA_KUBERNETES_INGRESS_ANNOTATIONS__JSON: '{"kubernetes.io/ingress.class": "nginx", "{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/rewrite-target": "/","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/ssl-redirect": "false","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-connect-timeout": "3600","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-read-timeout": "3600"}' + CHE_INFRA_KUBERNETES_INGRESS_ANNOTATIONS__JSON: '{"kubernetes.io/ingress.class": "nginx", "{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/rewrite-target": "/$1","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/ssl-redirect": "false","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-connect-timeout": "3600","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-read-timeout": "3600"}' {{- end }} + CHE_INFRA_KUBERNETES_INGRESS_PATH__TRANSFORM: '%s(/?.*)' CHE_INFRA_KUBERNETES_SERVER__STRATEGY: {{ .Values.global.serverStrategy }} CHE_LOGGER_CONFIG: {{ .Values.global.log.loggerConfig | quote}} CHE_LOGS_APPENDERS_IMPL: {{ .Values.global.log.appenderName }} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java index 9b48b0e9949..9b7c075d22a 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java @@ -85,6 +85,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.event.PodEvent; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; import org.eclipse.che.workspace.infrastructure.kubernetes.util.UnrecoverablePodEventListenerFactory; @@ -118,6 +119,7 @@ public class KubernetesInternalRuntime private final Set internalEnvironmentProvisioners; private final KubernetesEnvironmentProvisioner kubernetesEnvironmentProvisioner; private final SidecarToolingProvisioner toolingProvisioner; + protected final ExternalServerExposerStrategy externalServerExposerStrategy; private final RuntimeHangingDetector runtimeHangingDetector; protected final Tracer tracer; @@ -140,6 +142,7 @@ public KubernetesInternalRuntime( Set internalEnvironmentProvisioners, KubernetesEnvironmentProvisioner kubernetesEnvironmentProvisioner, SidecarToolingProvisioner toolingProvisioner, + ExternalServerExposerStrategy externalServerExposerStrategy, RuntimeHangingDetector runtimeHangingDetector, Tracer tracer, @Assisted KubernetesRuntimeContext context, @@ -161,6 +164,7 @@ public KubernetesInternalRuntime( this.toolingProvisioner = toolingProvisioner; this.kubernetesEnvironmentProvisioner = kubernetesEnvironmentProvisioner; this.internalEnvironmentProvisioners = internalEnvironmentProvisioners; + this.externalServerExposerStrategy = externalServerExposerStrategy; this.runtimeHangingDetector = runtimeHangingDetector; this.startSynchronizer = startSynchronizerFactory.create(context.getIdentity()); this.tracer = tracer; @@ -645,7 +649,8 @@ protected void startMachines() throws InfrastructureException { listenEvents(); final KubernetesServerResolver serverResolver = - new KubernetesServerResolver(createdServices, readyIngresses); + new KubernetesServerResolver( + externalServerExposerStrategy, createdServices, readyIngresses); doStartMachine(serverResolver); } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java index de4f304079c..d5645e0038a 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java @@ -24,6 +24,7 @@ import org.eclipse.che.api.workspace.server.model.impl.ServerImpl; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; /** * Helps to resolve {@link ServerImpl servers} by machine name according to specified {@link Ingress @@ -40,8 +41,11 @@ public class KubernetesServerResolver { private final Multimap services; private final Multimap ingresses; + private ExternalServerExposerStrategy demangler; - public KubernetesServerResolver(List services, List ingresses) { + public KubernetesServerResolver( + ExternalServerExposerStrategy demangler, List services, List ingresses) { + this.demangler = demangler; this.services = ArrayListMultimap.create(); for (Service service : services) { String machineName = @@ -109,7 +113,10 @@ private void fillIngressServers(Ingress ingress, Map servers .forEach( (name, config) -> { String path = - buildPath(ingressRule.getHttp().getPaths().get(0).getPath(), config.getPath()); + buildPath( + demangler.demanglePath( + ingress, ingressRule.getHttp().getPaths().get(0).getPath()), + config.getPath()); servers.put( name, new RuntimeServerBuilder() @@ -137,6 +144,11 @@ private String buildPath(String fragment1, @Nullable String fragment2) { } } + // always end server URLs with a slash, so that they can be safely sub-path'd.. + if (sb.charAt(sb.length() - 1) != '/') { + sb.append('/'); + } + return sb.toString(); } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategy.java index 3e2d4f3a29b..baeb659ba2f 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategy.java @@ -11,6 +11,7 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; +import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.ServicePort; import java.util.Map; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; @@ -41,4 +42,23 @@ void expose( String serviceName, ServicePort servicePort, Map externalServers); + + /** + * Sometimes, the exposer needs to modify the path contained in the object exposing the server + * (ingress or route). Namely, this is needed to make the URL rewriting work for single-host + * strategy where the path needs to contain a regular expression match group to retain some of the + * path. + * + *

This method reverts such mangling and returns to the user a path that can be used by the + * HTTP clients. + * + * @param exposingObject a Kubernetes object in charge of actual exposure of the server (i.e. + * ingress or route) + * @param path the path contained within the configuration of the object that needs to be + * demangled + * @return the path demangled such that it can be used in an externally reachable URL + */ + default String demanglePath(HasMetadata exposingObject, String path) { + return path; + } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposer.java index b89f87a0f6f..58e692a1df5 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposer.java @@ -11,12 +11,16 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; +import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.api.model.extensions.Ingress; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; 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; /** @@ -52,13 +56,46 @@ public class SingleHostIngressExternalServerExposer public static final String SINGLE_HOST_STRATEGY = "single-host"; private final Map ingressAnnotations; private final String cheHost; + private final String pathTransformFmt; + private final Pattern pathTransformInverse; @Inject public SingleHostIngressExternalServerExposer( @Named("infra.kubernetes.ingress.annotations") Map ingressAnnotations, - @Named("che.host") String cheHost) { + @Named("che.host") String cheHost, + @Nullable @Named("che.infra.kubernetes.ingress.path_transform") String pathTransformFmt) { this.ingressAnnotations = ingressAnnotations; this.cheHost = cheHost; + this.pathTransformFmt = pathTransformFmt == null ? "%s" : pathTransformFmt; + this.pathTransformInverse = extractPathFromFmt(this.pathTransformFmt); + } + + private static Pattern extractPathFromFmt(String pathTransformFmt) { + int refIdx = pathTransformFmt.indexOf("%s"); + String matchPath = "(.*)"; + + String transformed; + if (refIdx < 0) { + transformed = Pattern.quote(pathTransformFmt); + } else { + if (refIdx == 0) { + if (pathTransformFmt.length() > 2) { + transformed = matchPath + Pattern.quote(pathTransformFmt.substring(2)); + } else { + transformed = matchPath; + } + } else { + String prefix = Pattern.quote(pathTransformFmt.substring(0, refIdx)); + String suffix = + refIdx < pathTransformFmt.length() - 2 + ? Pattern.quote(pathTransformFmt.substring(refIdx + 2)) + : ""; + + transformed = prefix + matchPath + suffix; + } + } + + return Pattern.compile("^" + transformed + "$"); } @Override @@ -72,6 +109,16 @@ public void expose( k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); } + @Override + public String demanglePath(HasMetadata exposingObject, String path) { + Matcher matcher = pathTransformInverse.matcher(path); + if (matcher.matches()) { + return matcher.group(1); + } else { + return path; + } + } + private Ingress generateIngress( String machineName, String serviceName, @@ -94,6 +141,6 @@ private String generateExternalServerIngressName(String serviceName, ServicePort } private String generateExternalServerIngressPath(String serviceName, ServicePort servicePort) { - return "/" + serviceName + "/" + servicePort.getName(); + return String.format(pathTransformFmt, "/" + serviceName + "/" + servicePort.getName()); } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java index 579fd578bf7..8a560f9b97b 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java @@ -126,6 +126,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.event.PodEvent; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.PodEvents; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; @@ -195,6 +196,7 @@ public class KubernetesInternalRuntimeTest { @Mock private WorkspaceProbes workspaceProbes; @Mock private KubernetesServerResolver kubernetesServerResolver; @Mock private InternalEnvironmentProvisioner internalEnvironmentProvisioner; + @Mock private ExternalServerExposerStrategy serverExposerStrategy; @Mock private RuntimeHangingDetector runtimeHangingDetector; @Mock @@ -260,6 +262,7 @@ public void setup() throws Exception { ImmutableSet.of(internalEnvironmentProvisioner), kubernetesEnvironmentProvisioner, toolingProvisioner, + serverExposerStrategy, runtimeHangingDetector, tracer, context, diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java index 723699b1683..2fd7fe4771a 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java @@ -20,6 +20,7 @@ import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceBuilder; +import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.api.model.ServicePortBuilder; import io.fabric8.kubernetes.api.model.extensions.HTTPIngressPath; import io.fabric8.kubernetes.api.model.extensions.HTTPIngressRuleValue; @@ -37,6 +38,8 @@ import org.eclipse.che.commons.lang.Pair; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations.Serializer; +import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; import org.testng.annotations.Test; /** @@ -54,7 +57,7 @@ public class KubernetesServerResolverTest { @Test public void - testResolvingServersWhenThereIsNoTheCorrespondingServiceAndingressForTheSpecifiedMachine() { + testResolvingServersWhenThereIsNoTheCorrespondingServiceAndIngressForTheSpecifiedMachine() { // given Service nonMatchedByPodService = createService("nonMatched", "foreignMachine", CONTAINER_PORT, null); @@ -65,7 +68,10 @@ public class KubernetesServerResolverTest { Pair.of("http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(singletonList(nonMatchedByPodService), singletonList(ingress)); + new KubernetesServerResolver( + new NoopExternalServerExposureStrategy<>(), + singletonList(nonMatchedByPodService), + singletonList(ingress)); // when Map resolved = serverResolver.resolve("machine"); @@ -75,15 +81,16 @@ public class KubernetesServerResolverTest { } @Test - public void testResolvingServersWhenThereIsMatchedingressForTheSpecifiedMachine() { + public void testResolvingServersWhenThereIsMatchedIngressForTheSpecifiedMachine() { Ingress ingress = createIngress( "matched", "machine", - Pair.of("http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); + Pair.of("http-server", new ServerConfigImpl("3054", "http", "/api/", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(emptyList(), singletonList(ingress)); + new KubernetesServerResolver( + new NoopExternalServerExposureStrategy<>(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -91,7 +98,7 @@ public void testResolvingServersWhenThereIsMatchedingressForTheSpecifiedMachine( assertEquals( resolved.get("http-server"), new ServerImpl() - .withUrl("http://" + INGRESS_IP + INGRESS_RULE_PATH_PREFIX + "/api") + .withUrl("http://" + INGRESS_IP + INGRESS_RULE_PATH_PREFIX + "/api/") .withStatus(ServerStatus.UNKNOWN) .withAttributes(defaultAttributeAnd(Constants.SERVER_PORT_ATTRIBUTE, "3054"))); } @@ -105,7 +112,8 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", null, ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(emptyList(), singletonList(ingress)); + new KubernetesServerResolver( + new NoopExternalServerExposureStrategy<>(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -113,7 +121,7 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath assertEquals( resolved.get("http-server"), new ServerImpl() - .withUrl("http://" + INGRESS_IP + INGRESS_RULE_PATH_PREFIX) + .withUrl("http://" + INGRESS_IP + INGRESS_RULE_PATH_PREFIX + "/") .withStatus(ServerStatus.UNKNOWN) .withAttributes(defaultAttributeAnd(Constants.SERVER_PORT_ATTRIBUTE, "3054"))); } @@ -127,7 +135,8 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", "", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(emptyList(), singletonList(ingress)); + new KubernetesServerResolver( + new NoopExternalServerExposureStrategy<>(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -135,13 +144,13 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath assertEquals( resolved.get("http-server"), new ServerImpl() - .withUrl("http://" + INGRESS_IP + INGRESS_RULE_PATH_PREFIX) + .withUrl("http://" + INGRESS_IP + INGRESS_RULE_PATH_PREFIX + "/") .withStatus(ServerStatus.UNKNOWN) .withAttributes(defaultAttributeAnd(Constants.SERVER_PORT_ATTRIBUTE, "3054"))); } @Test - public void testResolvingServersWhenThereIsMatchedingressForMachineAndServerPathIsRelative() { + public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPathIsRelative() { Ingress ingress = createIngress( "matched", @@ -149,7 +158,8 @@ public void testResolvingServersWhenThereIsMatchedingressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(emptyList(), singletonList(ingress)); + new KubernetesServerResolver( + new NoopExternalServerExposureStrategy<>(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -157,7 +167,7 @@ public void testResolvingServersWhenThereIsMatchedingressForMachineAndServerPath assertEquals( resolved.get("http-server"), new ServerImpl() - .withUrl("http://" + INGRESS_IP + INGRESS_RULE_PATH_PREFIX + "/api") + .withUrl("http://" + INGRESS_IP + INGRESS_RULE_PATH_PREFIX + "/api/") .withStatus(ServerStatus.UNKNOWN) .withAttributes(defaultAttributeAnd(Constants.SERVER_PORT_ATTRIBUTE, "3054"))); } @@ -173,7 +183,8 @@ public void testResolvingInternalServers() { "http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(singletonList(service), emptyList()); + new KubernetesServerResolver( + new NoopExternalServerExposureStrategy<>(), singletonList(service), emptyList()); Map resolved = serverResolver.resolve("machine"); @@ -197,7 +208,8 @@ public void testResolvingInternalServersWithPortWithTransportProtocol() { "http-server", new ServerConfigImpl("3054/udp", "xxx", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(singletonList(service), emptyList()); + new KubernetesServerResolver( + new NoopExternalServerExposureStrategy<>(), singletonList(service), emptyList()); Map resolved = serverResolver.resolve("machine"); @@ -271,4 +283,15 @@ private Map defaultAttributeAnd(String key, String value) { attributes.put(key, value); return attributes; } + + private static final class NoopExternalServerExposureStrategy + implements ExternalServerExposerStrategy { + @Override + public void expose( + T k8sEnv, + String machineName, + String serviceName, + ServicePort servicePort, + Map externalServers) {} + } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposerTest.java new file mode 100644 index 00000000000..003195034a3 --- /dev/null +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposerTest.java @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; + +import static org.testng.Assert.*; + +import java.util.Collections; +import org.testng.annotations.Test; + +public class SingleHostIngressExternalServerExposerTest { + + @Test + public void testDemanglePath() { + SingleHostIngressExternalServerExposer exposer = + new SingleHostIngressExternalServerExposer(Collections.emptyMap(), null, "%s(/?.*)"); + + String demangledPath = exposer.demanglePath(null, "/path/subpath(/?.*)"); + + assertEquals(demangledPath, "/path/subpath"); + } +} diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java index 2a9b84c2de4..7157659c097 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java @@ -38,6 +38,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.cache.KubernetesMachineCache; import org.eclipse.che.workspace.infrastructure.kubernetes.cache.KubernetesRuntimeStateCache; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; import org.eclipse.che.workspace.infrastructure.kubernetes.util.UnrecoverablePodEventListenerFactory; @@ -73,6 +74,7 @@ public OpenShiftInternalRuntime( Set internalEnvironmentProvisioners, OpenShiftEnvironmentProvisioner kubernetesEnvironmentProvisioner, SidecarToolingProvisioner toolingProvisioner, + ExternalServerExposerStrategy serverExposerStrategy, RuntimeHangingDetector runtimeHangingDetector, Tracer tracer, @Assisted OpenShiftRuntimeContext context, @@ -95,6 +97,7 @@ public OpenShiftInternalRuntime( internalEnvironmentProvisioners, kubernetesEnvironmentProvisioner, toolingProvisioner, + serverExposerStrategy, runtimeHangingDetector, tracer, context, @@ -115,7 +118,8 @@ protected void startMachines() throws InfrastructureException { listenEvents(); - doStartMachine(new OpenShiftServerResolver(createdServices, createdRoutes)); + doStartMachine( + new OpenShiftServerResolver(externalServerExposerStrategy, createdServices, createdRoutes)); } @Traced diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolver.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolver.java index a52fe15cd3a..4737b44ca60 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolver.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolver.java @@ -22,6 +22,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; import org.eclipse.che.workspace.infrastructure.kubernetes.server.RuntimeServerBuilder; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; /** * Helps to resolve {@link ServerImpl servers} by machine name according to specified {@link Route @@ -39,8 +40,9 @@ public class OpenShiftServerResolver extends KubernetesServerResolver { private final Multimap routes; - public OpenShiftServerResolver(List services, List routes) { - super(services, Collections.emptyList()); + public OpenShiftServerResolver( + ExternalServerExposerStrategy demangler, List services, List routes) { + super(demangler, services, Collections.emptyList()); this.routes = ArrayListMultimap.create(); for (Route route : routes) { diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java index d728159bfe4..b1f156215b8 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java @@ -80,6 +80,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesSecrets; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesServices; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; import org.eclipse.che.workspace.infrastructure.kubernetes.util.UnrecoverablePodEventListenerFactory; @@ -145,6 +146,7 @@ public class OpenShiftInternalRuntimeTest { @Mock private OpenShiftEnvironmentProvisioner kubernetesEnvironmentProvisioner; @Mock private SidecarToolingProvisioner toolingProvisioner; @Mock private UnrecoverablePodEventListenerFactory unrecoverablePodEventListenerFactory; + @Mock private ExternalServerExposerStrategy serverExposerStrategy; @Mock private RuntimeHangingDetector runtimeHangingDetector; @Mock(answer = Answers.RETURNS_MOCKS) @@ -182,6 +184,7 @@ public void setup() throws Exception { ImmutableSet.of(internalEnvironmentProvisioner), kubernetesEnvironmentProvisioner, toolingProvisioner, + serverExposerStrategy, runtimeHangingDetector, tracer, context, diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java index 929b47e62b9..b0c7f870dac 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java @@ -31,7 +31,12 @@ import org.eclipse.che.api.workspace.shared.Constants; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations.Serializer; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; import org.eclipse.che.workspace.infrastructure.openshift.server.OpenShiftServerResolver; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.Listeners; import org.testng.annotations.Test; /** @@ -39,12 +44,15 @@ * * @author Sergii Leshchenko */ +@Listeners(MockitoTestNGListener.class) public class OpenShiftServerResolverTest { private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); private static final int CONTAINER_PORT = 3054; private static final String ROUTE_HOST = "localhost"; + @Mock private ExternalServerExposerStrategy exposer; + @Test public void testResolvingServersWhenThereIsNoTheCorrespondingServiceAndRouteForTheSpecifiedMachine() { @@ -59,7 +67,8 @@ public class OpenShiftServerResolverTest { "http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(singletonList(nonMatchedByPodService), singletonList(route)); + new OpenShiftServerResolver( + exposer, singletonList(nonMatchedByPodService), singletonList(route)); // when Map resolved = serverResolver.resolve("machine"); @@ -78,7 +87,7 @@ public void testResolvingServersWhenThereIsMatchedRouteForTheSpecifiedMachine() "http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(emptyList(), singletonList(route)); + new OpenShiftServerResolver(exposer, emptyList(), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -101,7 +110,7 @@ public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIs "http-server", new ServerConfigImpl("3054", "http", null, ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(emptyList(), singletonList(route)); + new OpenShiftServerResolver(exposer, emptyList(), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -123,7 +132,7 @@ public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIs singletonMap("http-server", new ServerConfigImpl("3054", "http", "", ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(emptyList(), singletonList(route)); + new OpenShiftServerResolver(exposer, emptyList(), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -146,7 +155,7 @@ public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIs "http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(emptyList(), singletonList(route)); + new OpenShiftServerResolver(exposer, emptyList(), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -171,7 +180,7 @@ public void testResolvingInternalServers() { Route route = createRoute("matched", "machine", null); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(singletonList(service), singletonList(route)); + new OpenShiftServerResolver(exposer, singletonList(service), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -196,7 +205,7 @@ public void testResolvingInternalServersWithPortWithTransportProtocol() { Route route = createRoute("matched", "machine", null); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(singletonList(service), singletonList(route)); + new OpenShiftServerResolver(exposer, singletonList(service), singletonList(route)); Map resolved = serverResolver.resolve("machine"); From a69ed767c23abdfa8c75eff7a9d19cbd503ccade Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 7 Aug 2019 13:49:05 +0200 Subject: [PATCH 02/12] Fixed the path transform regex and update the description in the che.properties to make more sense. Signed-off-by: Lukas Krejci --- .../main/webapp/WEB-INF/classes/che/che.properties | 12 +++++------- deploy/kubernetes/helm/che/templates/configmap.yaml | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index ab072a85f07..26dcc8c76a1 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -490,20 +490,18 @@ che.infra.kubernetes.installer_server_max_port=20000 # For example for nginx ingress controller 0.22.0 and later the following value is recommended: # {"ingress.kubernetes.io/rewrite-target": "/$1","ingress.kubernetes.io/ssl-redirect": "false",\ # "ingress.kubernetes.io/proxy-connect-timeout": "3600","ingress.kubernetes.io/proxy-read-timeout": "3600"} -# and the che.infra.kubernetes.ingress.path.rewrite_transform should be set to "%s(/?.*)" +# and the che.infra.kubernetes.ingress.path.rewrite_transform should be set to "%s/(.*)" # # For nginx ingress controller older than 0.22.0, the rewrite-target should be set to merely "/" and the path transform # to "%s" (see the the che.infra.kubernetes.ingress.path.rewrite_transform property). che.infra.kubernetes.ingress.annotations_json=NULL # Defines a "recipe" on how to declare the path of the ingress that should expose a server. -# The "%s" represents the base public URL of the server. In the concrete example for nginx the "(/?.*)" part makes sure -# that any potential further sub-path is captured and can be used in the rewrite-target rule specified in -# the che.infra.kubernetes.ingress.annotations_json property (as is shown in the example value of that property). This -# property must be a valid input to the String.format() method and contain exactly one reference to "%s". +# The "%s" represents the base public URL of the server. This property must be a valid input to the String.format() +# method and contain exactly one reference to "%s". # -# Note that this property is only taken into the account when Che is deployed on Kubernetes with the single-host -# server strategy. +# Please see the description of the che.infra.kubernetes.ingress.annotations_json property to see how these two +# properties interplay when specifying the ingress annotations and path. che.infra.kubernetes.ingress.path_transform=NULL # Defines security context for pods that will be created by Kubernetes Infra diff --git a/deploy/kubernetes/helm/che/templates/configmap.yaml b/deploy/kubernetes/helm/che/templates/configmap.yaml index 1214fe15cee..39828def911 100644 --- a/deploy/kubernetes/helm/che/templates/configmap.yaml +++ b/deploy/kubernetes/helm/che/templates/configmap.yaml @@ -72,7 +72,7 @@ data: {{- else }} CHE_INFRA_KUBERNETES_INGRESS_ANNOTATIONS__JSON: '{"kubernetes.io/ingress.class": "nginx", "{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/rewrite-target": "/$1","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/ssl-redirect": "false","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-connect-timeout": "3600","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-read-timeout": "3600"}' {{- end }} - CHE_INFRA_KUBERNETES_INGRESS_PATH__TRANSFORM: '%s(/?.*)' + CHE_INFRA_KUBERNETES_INGRESS_PATH__TRANSFORM: '%s/(.*)' CHE_INFRA_KUBERNETES_SERVER__STRATEGY: {{ .Values.global.serverStrategy }} CHE_LOGGER_CONFIG: {{ .Values.global.log.loggerConfig | quote}} CHE_LOGS_APPENDERS_IMPL: {{ .Values.global.log.appenderName }} From 824b4399c35d5e865fd958cb349d670b070e4f75 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 7 Aug 2019 13:49:44 +0200 Subject: [PATCH 03/12] Reduced the repetition in the impls of server exposure strategy. Signed-off-by: Lukas Krejci --- .../kubernetes/KubernetesInfraModule.java | 39 ++--- .../kubernetes/KubernetesInternalRuntime.java | 15 +- .../provision/server/ServersConverter.java | 10 +- .../server/KubernetesServerExposer.java | 6 +- .../server/KubernetesServerResolver.java | 6 +- ...faultHostIngressExternalServerExposer.java | 96 ------------ .../DefaultHostIngressNamingStrategy.java | 62 ++++++++ .../external/ExternalServerExposer.java | 82 ++++++++++ ...ExternalServerExposerStrategyProvider.java | 54 ------- .../external/ExternalServerPathDemangler.java | 81 ++++++++++ .../external/IngressNamingStrategy.java | 25 +++ .../IngressNamingStrategyProvider.java | 46 ++++++ ...va => MultiHostIngressNamingStrategy.java} | 57 ++----- ...ingleHostIngressExternalServerExposer.java | 146 ------------------ .../SingleHostIngressNamingStrategy.java | 69 +++++++++ .../secure/DefaultSecureServersFactory.java | 10 +- .../jwtproxy/JwtProxySecureServerExposer.java | 16 +- .../KubernetesInternalRuntimeTest.java | 9 +- .../server/KubernetesServerExposerTest.java | 8 +- .../server/KubernetesServerResolverTest.java | 39 ++--- ...DefaultHostIngressNamingStrategyTest.java} | 7 +- ...> MultiHostIngressNamingStrategyTest.java} | 9 +- ...eHostIngressExternalServerExposerTest.java | 30 ---- .../JwtProxySecureServerExposerTest.java | 4 +- .../openshift/OpenShiftInfraModule.java | 4 +- .../openshift/OpenShiftInternalRuntime.java | 8 +- .../OpenShiftExternalServerExposer.java | 10 +- .../server/OpenShiftServerResolver.java | 7 +- .../OpenShiftInternalRuntimeTest.java | 5 +- .../OpenShiftServerResolverTest.java | 20 ++- 30 files changed, 481 insertions(+), 499 deletions(-) delete mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressExternalServerExposer.java create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategy.java create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java delete mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategyProvider.java create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategy.java create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategyProvider.java rename infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{MultiHostIngressExternalServerExposer.java => MultiHostIngressNamingStrategy.java} (51%) delete mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposer.java create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressNamingStrategy.java rename infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{DefaultHostIngressExternalServerExposerTest.java => DefaultHostIngressNamingStrategyTest.java} (95%) rename infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{MultiHostIngressExternalServerExposerTest.java => MultiHostIngressNamingStrategyTest.java} (94%) delete mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposerTest.java diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInfraModule.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInfraModule.java index 269aa24a063..59d6e45ba23 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInfraModule.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInfraModule.java @@ -17,9 +17,9 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.CommonPVCStrategy.COMMON_STRATEGY; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.PerWorkspacePVCStrategy.PER_WORKSPACE_STRATEGY; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.UniqueWorkspacePVCStrategy.UNIQUE_STRATEGY; -import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostIngressExternalServerExposer.DEFAULT_HOST_STRATEGY; -import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressExternalServerExposer.MULTI_HOST_STRATEGY; -import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostIngressExternalServerExposer.SINGLE_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostIngressNamingStrategy.DEFAULT_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressNamingStrategy.MULTI_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostIngressNamingStrategy.SINGLE_HOST_STRATEGY; import com.google.inject.AbstractModule; import com.google.inject.TypeLiteral; @@ -60,11 +60,11 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.LogsRootEnvVariableProvider; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.kubernetes.server.IngressAnnotationsProvider; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostIngressExternalServerExposer; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategyProvider; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressExternalServerExposer; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostIngressExternalServerExposer; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostIngressNamingStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressNamingStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressNamingStrategyProvider; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressNamingStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostIngressNamingStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.DefaultSecureServersFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposerFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposerFactoryProvider; @@ -117,23 +117,12 @@ protected void configure() { .addBinding() .to(KubernetesClientTermination.class); - MapBinder> ingressStrategies = - MapBinder.newMapBinder( - binder(), - new TypeLiteral() {}, - new TypeLiteral>() {}); - ingressStrategies - .addBinding(MULTI_HOST_STRATEGY) - .to(MultiHostIngressExternalServerExposer.class); - ingressStrategies - .addBinding(SINGLE_HOST_STRATEGY) - .to(SingleHostIngressExternalServerExposer.class); - ingressStrategies - .addBinding(DEFAULT_HOST_STRATEGY) - .to(DefaultHostIngressExternalServerExposer.class); - bind(new TypeLiteral>() {}) - .toProvider( - new TypeLiteral>() {}); + MapBinder ingressStrategies = + MapBinder.newMapBinder(binder(), String.class, IngressNamingStrategy.class); + ingressStrategies.addBinding(MULTI_HOST_STRATEGY).to(MultiHostIngressNamingStrategy.class); + ingressStrategies.addBinding(SINGLE_HOST_STRATEGY).to(SingleHostIngressNamingStrategy.class); + ingressStrategies.addBinding(DEFAULT_HOST_STRATEGY).to(DefaultHostIngressNamingStrategy.class); + bind(IngressNamingStrategy.class).toProvider(IngressNamingStrategyProvider.class); bind(ServersConverter.class).to(new TypeLiteral>() {}); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java index 9b7c075d22a..4c5c7b76e9b 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java @@ -85,7 +85,8 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.event.PodEvent; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; import org.eclipse.che.workspace.infrastructure.kubernetes.util.UnrecoverablePodEventListenerFactory; @@ -119,7 +120,8 @@ public class KubernetesInternalRuntime private final Set internalEnvironmentProvisioners; private final KubernetesEnvironmentProvisioner kubernetesEnvironmentProvisioner; private final SidecarToolingProvisioner toolingProvisioner; - protected final ExternalServerExposerStrategy externalServerExposerStrategy; + protected final ExternalServerExposer externalServerExposer; + protected final ExternalServerPathDemangler externalServerPathDemangler; private final RuntimeHangingDetector runtimeHangingDetector; protected final Tracer tracer; @@ -142,7 +144,8 @@ public KubernetesInternalRuntime( Set internalEnvironmentProvisioners, KubernetesEnvironmentProvisioner kubernetesEnvironmentProvisioner, SidecarToolingProvisioner toolingProvisioner, - ExternalServerExposerStrategy externalServerExposerStrategy, + ExternalServerExposer externalServerExposer, + ExternalServerPathDemangler externalServerPathDemangler, RuntimeHangingDetector runtimeHangingDetector, Tracer tracer, @Assisted KubernetesRuntimeContext context, @@ -164,7 +167,8 @@ public KubernetesInternalRuntime( this.toolingProvisioner = toolingProvisioner; this.kubernetesEnvironmentProvisioner = kubernetesEnvironmentProvisioner; this.internalEnvironmentProvisioners = internalEnvironmentProvisioners; - this.externalServerExposerStrategy = externalServerExposerStrategy; + this.externalServerExposer = externalServerExposer; + this.externalServerPathDemangler = externalServerPathDemangler; this.runtimeHangingDetector = runtimeHangingDetector; this.startSynchronizer = startSynchronizerFactory.create(context.getIdentity()); this.tracer = tracer; @@ -649,8 +653,7 @@ protected void startMachines() throws InfrastructureException { listenEvents(); final KubernetesServerResolver serverResolver = - new KubernetesServerResolver( - externalServerExposerStrategy, createdServices, readyIngresses); + new KubernetesServerResolver(externalServerPathDemangler, createdServices, readyIngresses); doStartMachine(serverResolver); } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/server/ServersConverter.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/server/ServersConverter.java index 9967cfa3aa2..a509b5aab3e 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/server/ServersConverter.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/server/ServersConverter.java @@ -27,7 +27,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.ConfigurationProvisioner; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposerFactoryProvider; @@ -44,14 +44,14 @@ public class ServersConverter implements ConfigurationProvisioner { - private final ExternalServerExposerStrategy externalServerExposerStrategy; + private final ExternalServerExposer externalServerExposer; private final SecureServerExposerFactoryProvider secureServerExposerFactoryProvider; @Inject public ServersConverter( - ExternalServerExposerStrategy externalServerExposerStrategy, + ExternalServerExposer externalServerExposer, SecureServerExposerFactoryProvider secureServerExposerFactoryProvider) { - this.externalServerExposerStrategy = externalServerExposerStrategy; + this.externalServerExposer = externalServerExposer; this.secureServerExposerFactoryProvider = secureServerExposerFactoryProvider; } @@ -72,7 +72,7 @@ public void provision(T k8sEnv, RuntimeIdentity identity) throws InfrastructureE if (!machineConfig.getServers().isEmpty()) { KubernetesServerExposer kubernetesServerExposer = new KubernetesServerExposer<>( - externalServerExposerStrategy, + externalServerExposer, secureServerExposer, machineName, podConfig, 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 3fdd06203d1..c9d065b7aac 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 @@ -39,7 +39,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.UniqueNamesProvisioner; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposer; /** @@ -103,7 +103,7 @@ public class KubernetesServerExposer { public static final int SERVER_UNIQUE_PART_SIZE = 8; public static final String SERVER_PREFIX = "server"; - private final ExternalServerExposerStrategy externalServerExposer; + private final ExternalServerExposer externalServerExposer; private final SecureServerExposer secureServerExposer; private final String machineName; private final Container container; @@ -111,7 +111,7 @@ public class KubernetesServerExposer { private final T k8sEnv; public KubernetesServerExposer( - ExternalServerExposerStrategy externalServerExposer, + ExternalServerExposer externalServerExposer, SecureServerExposer secureServerExposer, String machineName, PodData pod, diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java index d5645e0038a..ea4abfb8fba 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java @@ -24,7 +24,7 @@ import org.eclipse.che.api.workspace.server.model.impl.ServerImpl; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; /** * Helps to resolve {@link ServerImpl servers} by machine name according to specified {@link Ingress @@ -41,10 +41,10 @@ public class KubernetesServerResolver { private final Multimap services; private final Multimap ingresses; - private ExternalServerExposerStrategy demangler; + private ExternalServerPathDemangler demangler; public KubernetesServerResolver( - ExternalServerExposerStrategy demangler, List services, List ingresses) { + ExternalServerPathDemangler demangler, List services, List ingresses) { this.demangler = demangler; this.services = ArrayListMultimap.create(); for (Service service : services) { diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressExternalServerExposer.java deleted file mode 100644 index 26c48808e10..00000000000 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressExternalServerExposer.java +++ /dev/null @@ -1,96 +0,0 @@ -/* - * Copyright (c) 2012-2018 Red Hat, Inc. - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; - -import io.fabric8.kubernetes.api.model.ServicePort; -import io.fabric8.kubernetes.api.model.extensions.Ingress; -import java.util.Map; -import javax.inject.Inject; -import javax.inject.Named; -import org.eclipse.che.api.core.model.workspace.config.ServerConfig; -import org.eclipse.che.workspace.infrastructure.kubernetes.Names; -import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; - -/** - * Provides a path-based strategy for exposing service ports outside the cluster using Ingress - * Ingresses will be created without an explicit host (defaulting to *). - * - *

This strategy uses different Ingress path entries
- * Each external server is exposed with a unique path prefix. - * - *

This strategy imposes limitation on user-developed applications.
- * It should only be used for local development with a single IP address - * - * 

- *   Path-Based Ingress exposing service's port:
- * Ingress
- * ...
- * spec:
- *   rules:
- *     - http:
- *         paths:
- *           - path: service123/webapp        ---->> Service.metadata.name + / + Service.spec.ports[0].name
- *             backend:
- *               serviceName: service123      ---->> Service.metadata.name
- *               servicePort: [8080|web-app]  ---->> Service.spec.ports[0].[port|name]
- *
- * - * @author Sergii Leshchenko - * @author Guy Daich - */ -public class DefaultHostIngressExternalServerExposer - implements ExternalServerExposerStrategy { - - public static final String DEFAULT_HOST_STRATEGY = "default-host"; - private final Map ingressAnnotations; - - @Inject - public DefaultHostIngressExternalServerExposer( - @Named("infra.kubernetes.ingress.annotations") Map ingressAnnotations) { - this.ingressAnnotations = ingressAnnotations; - } - - @Override - public void expose( - KubernetesEnvironment k8sEnv, - String machineName, - String serviceName, - ServicePort servicePort, - Map externalServers) { - Ingress ingress = generateIngress(machineName, serviceName, servicePort, externalServers); - k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); - } - - private Ingress generateIngress( - String machineName, - String serviceName, - ServicePort servicePort, - Map ingressesServers) { - return new ExternalServerIngressBuilder() - .withPath(generateExternalServerIngressPath(serviceName, servicePort)) - .withName(generateExternalServerIngressName()) - .withMachineName(machineName) - .withServiceName(serviceName) - .withAnnotations(ingressAnnotations) - .withServicePort(servicePort.getName()) - .withServers(ingressesServers) - .build(); - } - - private String generateExternalServerIngressName() { - return Names.generateName("ingress"); - } - - private String generateExternalServerIngressPath(String serviceName, ServicePort servicePort) { - return "/" + serviceName + "/" + servicePort.getName(); - } -} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategy.java new file mode 100644 index 00000000000..5140823c8c9 --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategy.java @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; + +import io.fabric8.kubernetes.api.model.ServicePort; +import org.eclipse.che.workspace.infrastructure.kubernetes.Names; + +/** + * Provides a path-based strategy for exposing service ports outside the cluster using Ingress + * Ingresses will be created without an explicit host (defaulting to *). + * + *

This strategy uses different Ingress path entries
+ * Each external server is exposed with a unique path prefix. + * + *

This strategy imposes limitation on user-developed applications.
+ * It should only be used for local development with a single IP address + * + * 

+ *   Path-Based Ingress exposing service's port:
+ * Ingress
+ * ...
+ * spec:
+ *   rules:
+ *     - http:
+ *         paths:
+ *           - path: service123/webapp        ---->> Service.metadata.name + / + Service.spec.ports[0].name
+ *             backend:
+ *               serviceName: service123      ---->> Service.metadata.name
+ *               servicePort: [8080|web-app]  ---->> Service.spec.ports[0].[port|name]
+ *
+ * + * @author Sergii Leshchenko + * @author Guy Daich + */ +public class DefaultHostIngressNamingStrategy implements IngressNamingStrategy { + + public static final String DEFAULT_HOST_STRATEGY = "default-host"; + + @Override + public String getIngressHost(String serviceName, ServicePort servicePort) { + return null; + } + + @Override + public String getIngressName(String serviceName, ServicePort servicePort) { + return Names.generateName("ingress"); + } + + @Override + public String getIngressPath(String serviceName, ServicePort servicePort) { + return "/" + serviceName + "/" + servicePort.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 new file mode 100644 index 00000000000..717c8283d25 --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; + +import io.fabric8.kubernetes.api.model.ServicePort; +import io.fabric8.kubernetes.api.model.extensions.Ingress; +import java.util.Map; +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.KubernetesServerResolver; + +public class ExternalServerExposer { + private final IngressNamingStrategy strategy; + private final Map ingressAnnotations; + private final String pathTransformFmt; + + @Inject + public ExternalServerExposer( + IngressNamingStrategy strategy, + @Named("infra.kubernetes.ingress.annotations") Map annotations, + @Nullable @Named("che.infra.kubernetes.ingress.path_transform") String pathTransformFmt) { + this.strategy = strategy; + this.ingressAnnotations = annotations; + this.pathTransformFmt = pathTransformFmt == null ? "%s" : pathTransformFmt; + } + + /** + * 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 + * 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 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) { + Ingress ingress = generateIngress(machineName, serviceName, servicePort, externalServers); + k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); + } + + private Ingress generateIngress( + String machineName, + String serviceName, + ServicePort servicePort, + Map ingressesServers) { + + ExternalServerIngressBuilder bld = new ExternalServerIngressBuilder(); + String host = strategy.getIngressHost(serviceName, servicePort); + if (host != null) { + bld = bld.withHost(host); + } + + return bld.withPath( + String.format(pathTransformFmt, strategy.getIngressPath(serviceName, servicePort))) + .withName(strategy.getIngressName(serviceName, servicePort)) + .withMachineName(machineName) + .withServiceName(serviceName) + .withAnnotations(ingressAnnotations) + .withServicePort(servicePort.getName()) + .withServers(ingressesServers) + .build(); + } +} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategyProvider.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategyProvider.java deleted file mode 100644 index 79a9a8588eb..00000000000 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategyProvider.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright (c) 2012-2018 Red Hat, Inc. - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; - -import static java.lang.String.format; - -import java.util.Map; -import javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Provider; -import javax.inject.Singleton; -import org.eclipse.che.inject.ConfigurationException; -import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; - -/** - * Provides implementation of {@link ExternalServerExposerStrategy} for configured value. - * - * @author Guy Daich - */ -@Singleton -public class ExternalServerExposerStrategyProvider - implements Provider> { - - public static final String STRATEGY_PROPERTY = "che.infra.kubernetes.server_strategy"; - - private final ExternalServerExposerStrategy externalServerExposerStrategy; - - @Inject - public ExternalServerExposerStrategyProvider( - @Named(STRATEGY_PROPERTY) String strategy, - Map> strategies) { - final ExternalServerExposerStrategy externalServerExposerStrategy = strategies.get(strategy); - if (externalServerExposerStrategy != null) { - this.externalServerExposerStrategy = externalServerExposerStrategy; - } else { - throw new ConfigurationException( - format("Unsupported Ingress strategy '%s' configured", strategy)); - } - } - - @Override - public ExternalServerExposerStrategy get() { - return externalServerExposerStrategy; - } -} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java new file mode 100644 index 00000000000..776f05e7d1d --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import javax.inject.Inject; +import javax.inject.Named; +import org.eclipse.che.commons.annotation.Nullable; + +public class ExternalServerPathDemangler { + private final Pattern pathTransformInverse; + + @Inject + public ExternalServerPathDemangler( + @Nullable @Named("che.infra.kubernetes.ingress.path_transform") String pathTransformFmt) { + this.pathTransformInverse = extractPathFromFmt(pathTransformFmt); + } + + private static Pattern extractPathFromFmt(String pathTransformFmt) { + int refIdx = pathTransformFmt.indexOf("%s"); + String matchPath = "(.*)"; + + String transformed; + if (refIdx < 0) { + transformed = Pattern.quote(pathTransformFmt); + } else { + if (refIdx == 0) { + if (pathTransformFmt.length() > 2) { + transformed = matchPath + Pattern.quote(pathTransformFmt.substring(2)); + } else { + transformed = matchPath; + } + } else { + String prefix = Pattern.quote(pathTransformFmt.substring(0, refIdx)); + String suffix = + refIdx < pathTransformFmt.length() - 2 + ? Pattern.quote(pathTransformFmt.substring(refIdx + 2)) + : ""; + + transformed = prefix + matchPath + suffix; + } + } + + return Pattern.compile("^" + transformed + "$"); + } + + /** + * Sometimes, the exposer needs to modify the path contained in the object exposing the server + * (ingress or route). Namely, this is needed to make the URL rewriting work for single-host + * strategy where the path needs to contain a regular expression match group to retain some of the + * path. + * + *

This method reverts such mangling and returns to the user a path that can be used by the + * HTTP clients. + * + * @param exposingObject a Kubernetes object in charge of actual exposure of the server (i.e. + * ingress or route) + * @param path the path contained within the configuration of the object that needs to be + * demangled + * @return the path demangled such that it can be used in an externally reachable URL + */ + public String demanglePath(HasMetadata exposingObject, String path) { + Matcher matcher = pathTransformInverse.matcher(path); + if (matcher.matches()) { + return matcher.group(1); + } else { + return path; + } + } +} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategy.java new file mode 100644 index 00000000000..82a5ac6e696 --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategy.java @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; + +import io.fabric8.kubernetes.api.model.ServicePort; +import org.eclipse.che.commons.annotation.Nullable; + +public interface IngressNamingStrategy { + + @Nullable + String getIngressHost(String serviceName, ServicePort servicePort); + + String getIngressName(String serviceName, ServicePort servicePort); + + String getIngressPath(String serviceName, ServicePort servicePort); +} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategyProvider.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategyProvider.java new file mode 100644 index 00000000000..738c5c5ce82 --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategyProvider.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; + +import static java.lang.String.format; + +import java.util.Map; +import javax.inject.Inject; +import javax.inject.Named; +import javax.inject.Provider; +import javax.inject.Singleton; +import org.eclipse.che.inject.ConfigurationException; + +@Singleton +public class IngressNamingStrategyProvider implements Provider { + + static final String STRATEGY_PROPERTY = "che.infra.kubernetes.server_strategy"; + + private final IngressNamingStrategy namingStrategy; + + @Inject + public IngressNamingStrategyProvider( + @Named(STRATEGY_PROPERTY) String strategy, Map strategies) { + + namingStrategy = strategies.get(strategy); + + if (namingStrategy == null) { + throw new ConfigurationException( + format("Unsupported server naming strategy '%s' configured", strategy)); + } + } + + @Override + public IngressNamingStrategy get() { + return namingStrategy; + } +} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategy.java similarity index 51% rename from infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressExternalServerExposer.java rename to infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategy.java index 8e2c67946dd..2b0d677ef19 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressExternalServerExposer.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategy.java @@ -12,17 +12,13 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; import static java.lang.String.format; -import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategyProvider.STRATEGY_PROPERTY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressNamingStrategyProvider.STRATEGY_PROPERTY; import com.google.common.base.Strings; import io.fabric8.kubernetes.api.model.ServicePort; -import io.fabric8.kubernetes.api.model.extensions.Ingress; -import java.util.Map; import javax.inject.Inject; import javax.inject.Named; -import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.inject.ConfigurationException; -import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; /** * Provides a host-based strategy for exposing service ports outside the cluster using Ingress @@ -48,20 +44,16 @@ * @author Sergii Leshchenko * @author Guy Daich */ -public class MultiHostIngressExternalServerExposer - implements ExternalServerExposerStrategy { +public class MultiHostIngressNamingStrategy implements IngressNamingStrategy { public static final String MULTI_HOST_STRATEGY = "multi-host"; private static final String INGRESS_DOMAIN_PROPERTY = "che.infra.kubernetes.ingress.domain"; private final String domain; - private final Map ingressAnnotations; @Inject - public MultiHostIngressExternalServerExposer( - @Named("infra.kubernetes.ingress.annotations") Map ingressAnnotations, - @Named(INGRESS_DOMAIN_PROPERTY) String domain, - @Named(STRATEGY_PROPERTY) String strategy) { + public MultiHostIngressNamingStrategy( + @Named(INGRESS_DOMAIN_PROPERTY) String domain, @Named(STRATEGY_PROPERTY) String strategy) { if (Strings.isNullOrEmpty(domain) && MULTI_HOST_STRATEGY.equals(strategy)) { throw new ConfigurationException( format( @@ -69,48 +61,21 @@ public MultiHostIngressExternalServerExposer( + "but property '%s' is not set", MULTI_HOST_STRATEGY, INGRESS_DOMAIN_PROPERTY)); } - this.ingressAnnotations = ingressAnnotations; this.domain = domain; } @Override - public void expose( - KubernetesEnvironment k8sEnv, - String machineName, - String serviceName, - ServicePort servicePort, - Map externalServers) { - Ingress ingress = generateIngress(machineName, serviceName, servicePort, externalServers); - k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); - } - - private Ingress generateIngress( - String machineName, - String serviceName, - ServicePort servicePort, - Map ingressesServers) { - return new ExternalServerIngressBuilder() - .withHost(generateExternalServerIngressHostname(serviceName, servicePort)) - .withPath(generateExternalServerIngressPath()) - .withName(generateExternalServerIngressName(serviceName, servicePort)) - .withMachineName(machineName) - .withServiceName(serviceName) - .withAnnotations(ingressAnnotations) - .withServicePort(servicePort.getName()) - .withServers(ingressesServers) - .build(); - } - - private String generateExternalServerIngressPath() { - return "/"; + public String getIngressHost(String serviceName, ServicePort servicePort) { + return serviceName + "-" + servicePort.getName() + "." + domain; } - private String generateExternalServerIngressName(String serviceName, ServicePort servicePort) { + @Override + public String getIngressName(String serviceName, ServicePort servicePort) { return serviceName + '-' + servicePort.getName(); } - private String generateExternalServerIngressHostname( - String serviceName, ServicePort servicePort) { - return serviceName + "-" + servicePort.getName() + "." + domain; + @Override + public String getIngressPath(String serviceName, ServicePort servicePort) { + return "/"; } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposer.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposer.java deleted file mode 100644 index 58e692a1df5..00000000000 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposer.java +++ /dev/null @@ -1,146 +0,0 @@ -/* - * Copyright (c) 2012-2018 Red Hat, Inc. - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.api.model.ServicePort; -import io.fabric8.kubernetes.api.model.extensions.Ingress; -import java.util.Map; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -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; - -/** - * Provides a path-based strategy for exposing service ports outside the cluster using Ingress. - * Ingresses will be created with a common host name for all workspaces. - * - *

This strategy uses different Ingress path entries
- * Each external server is exposed with a unique path prefix. - * - *

This strategy imposes limitation on user-developed applications.
- * - * 

- *   Path-Based Ingress exposing service's port:
- * Ingress
- * ...
- * spec:
- *   rules:
- *     - host: CHE_HOST
- *       http:
- *         paths:
- *           - path: service123/webapp        ---->> Service.metadata.name + / + Service.spec.ports[0].name
- *             backend:
- *               serviceName: service123      ---->> Service.metadata.name
- *               servicePort: [8080|web-app]  ---->> Service.spec.ports[0].[port|name]
- *
- * - * @author Sergii Leshchenko - * @author Guy Daich - */ -public class SingleHostIngressExternalServerExposer - implements ExternalServerExposerStrategy { - - public static final String SINGLE_HOST_STRATEGY = "single-host"; - private final Map ingressAnnotations; - private final String cheHost; - private final String pathTransformFmt; - private final Pattern pathTransformInverse; - - @Inject - public SingleHostIngressExternalServerExposer( - @Named("infra.kubernetes.ingress.annotations") Map ingressAnnotations, - @Named("che.host") String cheHost, - @Nullable @Named("che.infra.kubernetes.ingress.path_transform") String pathTransformFmt) { - this.ingressAnnotations = ingressAnnotations; - this.cheHost = cheHost; - this.pathTransformFmt = pathTransformFmt == null ? "%s" : pathTransformFmt; - this.pathTransformInverse = extractPathFromFmt(this.pathTransformFmt); - } - - private static Pattern extractPathFromFmt(String pathTransformFmt) { - int refIdx = pathTransformFmt.indexOf("%s"); - String matchPath = "(.*)"; - - String transformed; - if (refIdx < 0) { - transformed = Pattern.quote(pathTransformFmt); - } else { - if (refIdx == 0) { - if (pathTransformFmt.length() > 2) { - transformed = matchPath + Pattern.quote(pathTransformFmt.substring(2)); - } else { - transformed = matchPath; - } - } else { - String prefix = Pattern.quote(pathTransformFmt.substring(0, refIdx)); - String suffix = - refIdx < pathTransformFmt.length() - 2 - ? Pattern.quote(pathTransformFmt.substring(refIdx + 2)) - : ""; - - transformed = prefix + matchPath + suffix; - } - } - - return Pattern.compile("^" + transformed + "$"); - } - - @Override - public void expose( - KubernetesEnvironment k8sEnv, - String machineName, - String serviceName, - ServicePort servicePort, - Map externalServers) { - Ingress ingress = generateIngress(machineName, serviceName, servicePort, externalServers); - k8sEnv.getIngresses().put(ingress.getMetadata().getName(), ingress); - } - - @Override - public String demanglePath(HasMetadata exposingObject, String path) { - Matcher matcher = pathTransformInverse.matcher(path); - if (matcher.matches()) { - return matcher.group(1); - } else { - return path; - } - } - - private Ingress generateIngress( - String machineName, - String serviceName, - ServicePort servicePort, - Map ingressesServers) { - return new ExternalServerIngressBuilder() - .withHost(cheHost) - .withPath(generateExternalServerIngressPath(serviceName, servicePort)) - .withName(generateExternalServerIngressName(serviceName, servicePort)) - .withMachineName(machineName) - .withServiceName(serviceName) - .withAnnotations(ingressAnnotations) - .withServicePort(servicePort.getName()) - .withServers(ingressesServers) - .build(); - } - - private String generateExternalServerIngressName(String serviceName, ServicePort servicePort) { - return serviceName + '-' + servicePort.getName(); - } - - private String generateExternalServerIngressPath(String serviceName, ServicePort servicePort) { - return String.format(pathTransformFmt, "/" + serviceName + "/" + servicePort.getName()); - } -} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressNamingStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressNamingStrategy.java new file mode 100644 index 00000000000..fd00623525e --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressNamingStrategy.java @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; + +import io.fabric8.kubernetes.api.model.ServicePort; +import javax.inject.Inject; +import javax.inject.Named; + +/** + * Provides a path-based strategy for exposing service ports outside the cluster using Ingress. + * Ingresses will be created with a common host name for all workspaces. + * + *

This strategy uses different Ingress path entries
+ * Each external server is exposed with a unique path prefix. + * + *

This strategy imposes limitation on user-developed applications.
+ * + * 

+ *   Path-Based Ingress exposing service's port:
+ * Ingress
+ * ...
+ * spec:
+ *   rules:
+ *     - host: CHE_HOST
+ *       http:
+ *         paths:
+ *           - path: service123/webapp        ---->> Service.metadata.name + / + Service.spec.ports[0].name
+ *             backend:
+ *               serviceName: service123      ---->> Service.metadata.name
+ *               servicePort: [8080|web-app]  ---->> Service.spec.ports[0].[port|name]
+ *
+ * + * @author Sergii Leshchenko + * @author Guy Daich + */ +public class SingleHostIngressNamingStrategy implements IngressNamingStrategy { + + public static final String SINGLE_HOST_STRATEGY = "single-host"; + private final String cheHost; + + @Inject + public SingleHostIngressNamingStrategy(@Named("che.host") String cheHost) { + this.cheHost = cheHost; + } + + @Override + public String getIngressHost(String serviceName, ServicePort servicePort) { + return cheHost; + } + + @Override + public String getIngressName(String serviceName, ServicePort servicePort) { + return serviceName + '-' + servicePort.getName(); + } + + @Override + public String getIngressPath(String serviceName, ServicePort servicePort) { + return "/" + serviceName + "/" + servicePort.getName(); + } +} 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 faebf43d158..c2eb5dc6598 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 @@ -17,7 +17,7 @@ import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; /** * Default implementation of {@link SecureServerExposerFactory} that creates instances of {@link @@ -28,11 +28,11 @@ */ public class DefaultSecureServersFactory implements SecureServerExposerFactory { - private final ExternalServerExposerStrategy exposerStrategy; + private final ExternalServerExposer exposer; @Inject - public DefaultSecureServersFactory(ExternalServerExposerStrategy exposerStrategy) { - this.exposerStrategy = exposerStrategy; + public DefaultSecureServersFactory(ExternalServerExposer exposer) { + this.exposer = exposer; } @Override @@ -48,7 +48,7 @@ public void expose( String serviceName, ServicePort servicePort, Map secureServers) { - exposerStrategy.expose(k8sEnv, machineName, serviceName, servicePort, secureServers); + exposer.expose(k8sEnv, machineName, serviceName, servicePort, secureServers); } } } 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 72f0c62e4f7..daddbaaa33d 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 @@ -20,7 +20,7 @@ import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy.factory.JwtProxyProvisionerFactory; @@ -29,7 +29,7 @@ * *

To expose secure servers it provisions JwtProxy objects into environment with {@link * JwtProxyProvisioner}. Then JwtProxy service port is made public accessible by {@link - * ExternalServerExposerStrategy}. + * ExternalServerExposer}. * *

In this way, requests to exposed secure servers will be routed via JwtProxy pod that is added * one per workspace. And it will be impossible to requests secure servers if there is no machine @@ -41,13 +41,13 @@ public class JwtProxySecureServerExposer implements SecureServerExposer { - private final ExternalServerExposerStrategy exposerStrategy; + private final ExternalServerExposer exposer; private final JwtProxyProvisioner proxyProvisioner; @VisibleForTesting JwtProxySecureServerExposer( - JwtProxyProvisioner jwtProxyProvisioner, ExternalServerExposerStrategy exposerStrategy) { - this.exposerStrategy = exposerStrategy; + JwtProxyProvisioner jwtProxyProvisioner, ExternalServerExposer exposer) { + this.exposer = exposer; this.proxyProvisioner = jwtProxyProvisioner; } @@ -55,8 +55,8 @@ public class JwtProxySecureServerExposer public JwtProxySecureServerExposer( @Assisted RuntimeIdentity identity, JwtProxyProvisionerFactory jwtProxyProvisionerFactory, - ExternalServerExposerStrategy exposerStrategy) { - this.exposerStrategy = exposerStrategy; + ExternalServerExposer exposer) { + this.exposer = exposer; this.proxyProvisioner = jwtProxyProvisionerFactory.create(identity); } @@ -76,7 +76,7 @@ public void expose( servicePort.getProtocol(), secureServers); - exposerStrategy.expose( + exposer.expose( k8sEnv, machineName, proxyProvisioner.getServiceName(), exposedServicePort, secureServers); } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java index 8a560f9b97b..30ece9ce169 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java @@ -126,7 +126,8 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.event.PodEvent; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.PodEvents; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; @@ -196,7 +197,8 @@ public class KubernetesInternalRuntimeTest { @Mock private WorkspaceProbes workspaceProbes; @Mock private KubernetesServerResolver kubernetesServerResolver; @Mock private InternalEnvironmentProvisioner internalEnvironmentProvisioner; - @Mock private ExternalServerExposerStrategy serverExposerStrategy; + @Mock private ExternalServerExposer serverExposer; + @Mock private ExternalServerPathDemangler pathDemangler; @Mock private RuntimeHangingDetector runtimeHangingDetector; @Mock @@ -262,7 +264,8 @@ public void setup() throws Exception { ImmutableSet.of(internalEnvironmentProvisioner), kubernetesEnvironmentProvisioner, toolingProvisioner, - serverExposerStrategy, + serverExposer, + pathDemangler, runtimeHangingDetector, tracer, context, 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 bc777ea0c4f..8bc2c94332f 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 @@ -38,7 +38,7 @@ 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; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposer; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; @@ -54,7 +54,7 @@ @Listeners(MockitoTestNGListener.class) public class KubernetesServerExposerTest { - @Mock private ExternalServerExposerStrategy externalServerExposerStrategy; + @Mock private ExternalServerExposer externalServerExposer; @Mock private SecureServerExposer secureServerExposer; private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); @@ -91,7 +91,7 @@ public void setUp() throws Exception { PodData podData = new PodData(pod.getSpec(), pod.getMetadata()); this.serverExposer = new KubernetesServerExposer<>( - externalServerExposerStrategy, + externalServerExposer, secureServerExposer, MACHINE_NAME, podData, @@ -344,7 +344,7 @@ private void assertThatExternalServersAreExposed( Annotations.newDeserializer(service.getMetadata().getAnnotations()); assertEquals(serviceAnnotations.machineName(), machineName); - verify(externalServerExposerStrategy) + verify(externalServerExposer) .expose( kubernetesEnvironment, machineName, diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java index 2fd7fe4771a..4b727afd3e8 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java @@ -20,7 +20,6 @@ import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceBuilder; -import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.api.model.ServicePortBuilder; import io.fabric8.kubernetes.api.model.extensions.HTTPIngressPath; import io.fabric8.kubernetes.api.model.extensions.HTTPIngressRuleValue; @@ -38,8 +37,7 @@ import org.eclipse.che.commons.lang.Pair; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations.Serializer; -import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; import org.testng.annotations.Test; /** @@ -69,9 +67,7 @@ public class KubernetesServerResolverTest { KubernetesServerResolver serverResolver = new KubernetesServerResolver( - new NoopExternalServerExposureStrategy<>(), - singletonList(nonMatchedByPodService), - singletonList(ingress)); + new NoopPathDemangler(), singletonList(nonMatchedByPodService), singletonList(ingress)); // when Map resolved = serverResolver.resolve("machine"); @@ -89,8 +85,7 @@ public void testResolvingServersWhenThereIsMatchedIngressForTheSpecifiedMachine( Pair.of("http-server", new ServerConfigImpl("3054", "http", "/api/", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver( - new NoopExternalServerExposureStrategy<>(), emptyList(), singletonList(ingress)); + new KubernetesServerResolver(new NoopPathDemangler(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -112,8 +107,7 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", null, ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver( - new NoopExternalServerExposureStrategy<>(), emptyList(), singletonList(ingress)); + new KubernetesServerResolver(new NoopPathDemangler(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -135,8 +129,7 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", "", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver( - new NoopExternalServerExposureStrategy<>(), emptyList(), singletonList(ingress)); + new KubernetesServerResolver(new NoopPathDemangler(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -158,8 +151,7 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver( - new NoopExternalServerExposureStrategy<>(), emptyList(), singletonList(ingress)); + new KubernetesServerResolver(new NoopPathDemangler(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -183,8 +175,7 @@ public void testResolvingInternalServers() { "http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver( - new NoopExternalServerExposureStrategy<>(), singletonList(service), emptyList()); + new KubernetesServerResolver(new NoopPathDemangler(), singletonList(service), emptyList()); Map resolved = serverResolver.resolve("machine"); @@ -208,8 +199,7 @@ public void testResolvingInternalServersWithPortWithTransportProtocol() { "http-server", new ServerConfigImpl("3054/udp", "xxx", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver( - new NoopExternalServerExposureStrategy<>(), singletonList(service), emptyList()); + new KubernetesServerResolver(new NoopPathDemangler(), singletonList(service), emptyList()); Map resolved = serverResolver.resolve("machine"); @@ -284,14 +274,9 @@ private Map defaultAttributeAnd(String key, String value) { return attributes; } - private static final class NoopExternalServerExposureStrategy - implements ExternalServerExposerStrategy { - @Override - public void expose( - T k8sEnv, - String machineName, - String serviceName, - ServicePort servicePort, - Map externalServers) {} + private static final class NoopPathDemangler extends ExternalServerPathDemangler { + NoopPathDemangler() { + super("%s"); + } } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressExternalServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategyTest.java similarity index 95% rename from infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressExternalServerExposerTest.java rename to infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategyTest.java index bbe57022da4..820cc0158b9 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressExternalServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategyTest.java @@ -36,13 +36,13 @@ import org.testng.annotations.Test; /** @author Guy Daich */ -public class DefaultHostIngressExternalServerExposerTest { +public class DefaultHostIngressNamingStrategyTest { private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); private static final String MACHINE_NAME = "pod/main"; private static final String SERVICE_NAME = SERVER_PREFIX + "12345678" + "-" + MACHINE_NAME; - private DefaultHostIngressExternalServerExposer externalServerExposer; + private ExternalServerExposer externalServerExposer; private KubernetesEnvironment kubernetesEnvironment; @BeforeMethod @@ -60,7 +60,8 @@ public void setUp() throws Exception { kubernetesEnvironment = KubernetesEnvironment.builder().setPods(ImmutableMap.of("pod", pod)).build(); - externalServerExposer = new DefaultHostIngressExternalServerExposer(emptyMap()); + externalServerExposer = + new ExternalServerExposer<>(new DefaultHostIngressNamingStrategy(), emptyMap(), "%s"); } @Test diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressExternalServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategyTest.java similarity index 94% rename from infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressExternalServerExposerTest.java rename to infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategyTest.java index b7b9c888d78..f759d1332b5 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressExternalServerExposerTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategyTest.java @@ -14,7 +14,7 @@ import static java.util.Collections.emptyMap; 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.external.MultiHostIngressExternalServerExposer.MULTI_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressNamingStrategy.MULTI_HOST_STRATEGY; import static org.testng.Assert.assertEquals; import com.google.common.collect.ImmutableMap; @@ -37,14 +37,14 @@ import org.testng.annotations.Test; /** @author Guy Daich */ -public class MultiHostIngressExternalServerExposerTest { +public class MultiHostIngressNamingStrategyTest { private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); 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"; - private MultiHostIngressExternalServerExposer externalServerExposer; + private ExternalServerExposer externalServerExposer; private KubernetesEnvironment kubernetesEnvironment; @BeforeMethod @@ -63,7 +63,8 @@ public void setUp() throws Exception { kubernetesEnvironment = KubernetesEnvironment.builder().setPods(ImmutableMap.of("pod", pod)).build(); externalServerExposer = - new MultiHostIngressExternalServerExposer(emptyMap(), DOMAIN, MULTI_HOST_STRATEGY); + new ExternalServerExposer<>( + new MultiHostIngressNamingStrategy(DOMAIN, MULTI_HOST_STRATEGY), emptyMap(), "%s"); } @Test diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposerTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposerTest.java deleted file mode 100644 index 003195034a3..00000000000 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressExternalServerExposerTest.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (c) 2012-2018 Red Hat, Inc. - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; - -import static org.testng.Assert.*; - -import java.util.Collections; -import org.testng.annotations.Test; - -public class SingleHostIngressExternalServerExposerTest { - - @Test - public void testDemanglePath() { - SingleHostIngressExternalServerExposer exposer = - new SingleHostIngressExternalServerExposer(Collections.emptyMap(), null, "%s(/?.*)"); - - String demangledPath = exposer.demanglePath(null, "/path/subpath(/?.*)"); - - assertEquals(demangledPath, "/path/subpath"); - } -} 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 b72fc4203d2..4e8675d8a0f 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 @@ -26,7 +26,7 @@ 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.environment.KubernetesEnvironment; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.BeforeMethod; @@ -47,7 +47,7 @@ public class JwtProxySecureServerExposerTest { @Mock private KubernetesEnvironment k8sEnv; @Mock private JwtProxyProvisioner jwtProxyProvisioner; - @Mock private ExternalServerExposerStrategy externalServerExposer; + @Mock private ExternalServerExposer externalServerExposer; private JwtProxySecureServerExposer secureServerExposer; diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInfraModule.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInfraModule.java index 35e8bd77cfd..bc7293aeb96 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInfraModule.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInfraModule.java @@ -59,7 +59,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.KubernetesCheApiInternalEnvVarProvider; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.LogsRootEnvVariableProvider; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.DefaultSecureServersFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposerFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposerFactoryProvider; @@ -111,7 +111,7 @@ protected void configure() { volumesStrategies.addBinding(UNIQUE_STRATEGY).to(UniqueWorkspacePVCStrategy.class); bind(WorkspaceVolumesStrategy.class).toProvider(WorkspaceVolumeStrategyProvider.class); - bind(new TypeLiteral>() {}) + bind(new TypeLiteral>() {}) .to(OpenShiftExternalServerExposer.class); bind(ServersConverter.class).to(new TypeLiteral>() {}); diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java index 7157659c097..7841ea5b352 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java @@ -38,7 +38,6 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.cache.KubernetesMachineCache; import org.eclipse.che.workspace.infrastructure.kubernetes.cache.KubernetesRuntimeStateCache; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; import org.eclipse.che.workspace.infrastructure.kubernetes.util.UnrecoverablePodEventListenerFactory; @@ -74,7 +73,6 @@ public OpenShiftInternalRuntime( Set internalEnvironmentProvisioners, OpenShiftEnvironmentProvisioner kubernetesEnvironmentProvisioner, SidecarToolingProvisioner toolingProvisioner, - ExternalServerExposerStrategy serverExposerStrategy, RuntimeHangingDetector runtimeHangingDetector, Tracer tracer, @Assisted OpenShiftRuntimeContext context, @@ -97,7 +95,8 @@ public OpenShiftInternalRuntime( internalEnvironmentProvisioners, kubernetesEnvironmentProvisioner, toolingProvisioner, - serverExposerStrategy, + null, + null, runtimeHangingDetector, tracer, context, @@ -118,8 +117,7 @@ protected void startMachines() throws InfrastructureException { listenEvents(); - doStartMachine( - new OpenShiftServerResolver(externalServerExposerStrategy, createdServices, createdRoutes)); + doStartMachine(new OpenShiftServerResolver(createdServices, createdRoutes)); } @Traced 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 2dee31dd9c9..a63fdb42a10 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 @@ -15,11 +15,12 @@ import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.openshift.api.model.Route; +import java.util.Collections; import java.util.Map; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; 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.ExternalServerExposerStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; /** @@ -86,8 +87,11 @@ * @author Alexander Garagatyi * @see Annotations */ -public class OpenShiftExternalServerExposer - implements ExternalServerExposerStrategy { +public class OpenShiftExternalServerExposer extends ExternalServerExposer { + + public OpenShiftExternalServerExposer() { + super(null, Collections.emptyMap(), "%s"); + } @Override public void expose( diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolver.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolver.java index 4737b44ca60..b76097d6a4a 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolver.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/server/OpenShiftServerResolver.java @@ -22,7 +22,6 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; import org.eclipse.che.workspace.infrastructure.kubernetes.server.RuntimeServerBuilder; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; /** * Helps to resolve {@link ServerImpl servers} by machine name according to specified {@link Route @@ -33,16 +32,14 @@ * * @author Sergii Leshchenko * @author Alexander Garagatyi - * @see OpenShiftExternalServerExposer * @see Annotations */ public class OpenShiftServerResolver extends KubernetesServerResolver { private final Multimap routes; - public OpenShiftServerResolver( - ExternalServerExposerStrategy demangler, List services, List routes) { - super(demangler, services, Collections.emptyList()); + public OpenShiftServerResolver(List services, List routes) { + super(null, services, Collections.emptyList()); this.routes = ArrayListMultimap.create(); for (Route route : routes) { diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java index b1f156215b8..0ebf6e96e24 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java @@ -80,7 +80,6 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesSecrets; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesServices; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; import org.eclipse.che.workspace.infrastructure.kubernetes.util.UnrecoverablePodEventListenerFactory; @@ -88,6 +87,7 @@ import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; import org.eclipse.che.workspace.infrastructure.openshift.project.OpenShiftProject; import org.eclipse.che.workspace.infrastructure.openshift.project.OpenShiftRoutes; +import org.eclipse.che.workspace.infrastructure.openshift.server.OpenShiftExternalServerExposer; import org.mockito.Answers; import org.mockito.ArgumentCaptor; import org.mockito.Captor; @@ -146,7 +146,7 @@ public class OpenShiftInternalRuntimeTest { @Mock private OpenShiftEnvironmentProvisioner kubernetesEnvironmentProvisioner; @Mock private SidecarToolingProvisioner toolingProvisioner; @Mock private UnrecoverablePodEventListenerFactory unrecoverablePodEventListenerFactory; - @Mock private ExternalServerExposerStrategy serverExposerStrategy; + @Mock private OpenShiftExternalServerExposer serverExposerStrategy; @Mock private RuntimeHangingDetector runtimeHangingDetector; @Mock(answer = Answers.RETURNS_MOCKS) @@ -184,7 +184,6 @@ public void setup() throws Exception { ImmutableSet.of(internalEnvironmentProvisioner), kubernetesEnvironmentProvisioner, toolingProvisioner, - serverExposerStrategy, runtimeHangingDetector, tracer, context, diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java index b0c7f870dac..a6ffe6f0a75 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java @@ -31,8 +31,7 @@ import org.eclipse.che.api.workspace.shared.Constants; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations.Serializer; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposerStrategy; -import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; +import org.eclipse.che.workspace.infrastructure.openshift.server.OpenShiftExternalServerExposer; import org.eclipse.che.workspace.infrastructure.openshift.server.OpenShiftServerResolver; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; @@ -51,7 +50,7 @@ public class OpenShiftServerResolverTest { private static final int CONTAINER_PORT = 3054; private static final String ROUTE_HOST = "localhost"; - @Mock private ExternalServerExposerStrategy exposer; + @Mock private OpenShiftExternalServerExposer exposer; @Test public void @@ -67,8 +66,7 @@ public class OpenShiftServerResolverTest { "http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver( - exposer, singletonList(nonMatchedByPodService), singletonList(route)); + new OpenShiftServerResolver(singletonList(nonMatchedByPodService), singletonList(route)); // when Map resolved = serverResolver.resolve("machine"); @@ -87,7 +85,7 @@ public void testResolvingServersWhenThereIsMatchedRouteForTheSpecifiedMachine() "http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(exposer, emptyList(), singletonList(route)); + new OpenShiftServerResolver(emptyList(), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -110,7 +108,7 @@ public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIs "http-server", new ServerConfigImpl("3054", "http", null, ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(exposer, emptyList(), singletonList(route)); + new OpenShiftServerResolver(emptyList(), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -132,7 +130,7 @@ public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIs singletonMap("http-server", new ServerConfigImpl("3054", "http", "", ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(exposer, emptyList(), singletonList(route)); + new OpenShiftServerResolver(emptyList(), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -155,7 +153,7 @@ public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIs "http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(exposer, emptyList(), singletonList(route)); + new OpenShiftServerResolver(emptyList(), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -180,7 +178,7 @@ public void testResolvingInternalServers() { Route route = createRoute("matched", "machine", null); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(exposer, singletonList(service), singletonList(route)); + new OpenShiftServerResolver(singletonList(service), singletonList(route)); Map resolved = serverResolver.resolve("machine"); @@ -205,7 +203,7 @@ public void testResolvingInternalServersWithPortWithTransportProtocol() { Route route = createRoute("matched", "machine", null); OpenShiftServerResolver serverResolver = - new OpenShiftServerResolver(exposer, singletonList(service), singletonList(route)); + new OpenShiftServerResolver(singletonList(service), singletonList(route)); Map resolved = serverResolver.resolve("machine"); From 00b052c4ef5f1205564ecb301a5e6db6992d44a8 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 7 Aug 2019 18:49:28 +0200 Subject: [PATCH 04/12] Fix the multi-host exposure strategy. Signed-off-by: Lukas Krejci --- .../main/webapp/WEB-INF/classes/che/che.properties | 6 +++--- deploy/kubernetes/helm/che/templates/configmap.yaml | 2 +- .../server/external/ExternalServerExposer.java | 12 +++++++++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index 26dcc8c76a1..727c5261eb1 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -490,15 +490,15 @@ che.infra.kubernetes.installer_server_max_port=20000 # For example for nginx ingress controller 0.22.0 and later the following value is recommended: # {"ingress.kubernetes.io/rewrite-target": "/$1","ingress.kubernetes.io/ssl-redirect": "false",\ # "ingress.kubernetes.io/proxy-connect-timeout": "3600","ingress.kubernetes.io/proxy-read-timeout": "3600"} -# and the che.infra.kubernetes.ingress.path.rewrite_transform should be set to "%s/(.*)" +# and the che.infra.kubernetes.ingress.path.rewrite_transform should be set to "%s(.*)" # # For nginx ingress controller older than 0.22.0, the rewrite-target should be set to merely "/" and the path transform # to "%s" (see the the che.infra.kubernetes.ingress.path.rewrite_transform property). che.infra.kubernetes.ingress.annotations_json=NULL # Defines a "recipe" on how to declare the path of the ingress that should expose a server. -# The "%s" represents the base public URL of the server. This property must be a valid input to the String.format() -# method and contain exactly one reference to "%s". +# The "%s" represents the base public URL of the server and is guaranteed to end with a forward slash. This property +# must be a valid input to the String.format() method and contain exactly one reference to "%s". # # Please see the description of the che.infra.kubernetes.ingress.annotations_json property to see how these two # properties interplay when specifying the ingress annotations and path. diff --git a/deploy/kubernetes/helm/che/templates/configmap.yaml b/deploy/kubernetes/helm/che/templates/configmap.yaml index 39828def911..1d1cde8ce70 100644 --- a/deploy/kubernetes/helm/che/templates/configmap.yaml +++ b/deploy/kubernetes/helm/che/templates/configmap.yaml @@ -72,7 +72,7 @@ data: {{- else }} CHE_INFRA_KUBERNETES_INGRESS_ANNOTATIONS__JSON: '{"kubernetes.io/ingress.class": "nginx", "{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/rewrite-target": "/$1","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/ssl-redirect": "false","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-connect-timeout": "3600","{{ .Values.global.ingressAnnotationsPrefix }}ingress.kubernetes.io/proxy-read-timeout": "3600"}' {{- end }} - CHE_INFRA_KUBERNETES_INGRESS_PATH__TRANSFORM: '%s/(.*)' + CHE_INFRA_KUBERNETES_INGRESS_PATH__TRANSFORM: '%s(.*)' CHE_INFRA_KUBERNETES_SERVER__STRATEGY: {{ .Values.global.serverStrategy }} CHE_LOGGER_CONFIG: {{ .Values.global.log.loggerConfig | quote}} CHE_LOGS_APPENDERS_IMPL: {{ .Values.global.log.appenderName }} 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 717c8283d25..210c9f4982b 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 @@ -70,7 +70,9 @@ private Ingress generateIngress( } return bld.withPath( - String.format(pathTransformFmt, strategy.getIngressPath(serviceName, servicePort))) + String.format( + pathTransformFmt, + ensureEndsWithSlash(strategy.getIngressPath(serviceName, servicePort)))) .withName(strategy.getIngressName(serviceName, servicePort)) .withMachineName(machineName) .withServiceName(serviceName) @@ -79,4 +81,12 @@ private Ingress generateIngress( .withServers(ingressesServers) .build(); } + + private static String ensureEndsWithSlash(String path) { + if (path.charAt(path.length() - 1) == '/') { + return path; + } else { + return path + '/'; + } + } } From f1bcf69fea379cd086c94770ccb523a379a2e0f4 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Mon, 12 Aug 2019 15:40:02 +0200 Subject: [PATCH 05/12] Remove unused fields in the the tests. Signed-off-by: Lukas Krejci --- .../infrastructure/openshift/OpenShiftInternalRuntimeTest.java | 1 - .../infrastructure/openshift/OpenShiftServerResolverTest.java | 2 -- 2 files changed, 3 deletions(-) diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java index 0ebf6e96e24..f71009c7ce2 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java @@ -146,7 +146,6 @@ public class OpenShiftInternalRuntimeTest { @Mock private OpenShiftEnvironmentProvisioner kubernetesEnvironmentProvisioner; @Mock private SidecarToolingProvisioner toolingProvisioner; @Mock private UnrecoverablePodEventListenerFactory unrecoverablePodEventListenerFactory; - @Mock private OpenShiftExternalServerExposer serverExposerStrategy; @Mock private RuntimeHangingDetector runtimeHangingDetector; @Mock(answer = Answers.RETURNS_MOCKS) diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java index a6ffe6f0a75..114cc836b4d 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java @@ -50,8 +50,6 @@ public class OpenShiftServerResolverTest { private static final int CONTAINER_PORT = 3054; private static final String ROUTE_HOST = "localhost"; - @Mock private OpenShiftExternalServerExposer exposer; - @Test public void testResolvingServersWhenThereIsNoTheCorrespondingServiceAndRouteForTheSpecifiedMachine() { From 6cf61b1912d3cb3c71acf0e3226db6d2575c4714 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Mon, 12 Aug 2019 15:56:28 +0200 Subject: [PATCH 06/12] Removed unused imports, 8-| Signed-off-by: Lukas Krejci --- .../infrastructure/openshift/OpenShiftInternalRuntimeTest.java | 1 - .../infrastructure/openshift/OpenShiftServerResolverTest.java | 2 -- 2 files changed, 3 deletions(-) diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java index f71009c7ce2..d728159bfe4 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntimeTest.java @@ -87,7 +87,6 @@ import org.eclipse.che.workspace.infrastructure.openshift.environment.OpenShiftEnvironment; import org.eclipse.che.workspace.infrastructure.openshift.project.OpenShiftProject; import org.eclipse.che.workspace.infrastructure.openshift.project.OpenShiftRoutes; -import org.eclipse.che.workspace.infrastructure.openshift.server.OpenShiftExternalServerExposer; import org.mockito.Answers; import org.mockito.ArgumentCaptor; import org.mockito.Captor; diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java index 114cc836b4d..51f963c20fa 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftServerResolverTest.java @@ -31,9 +31,7 @@ import org.eclipse.che.api.workspace.shared.Constants; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations.Serializer; -import org.eclipse.che.workspace.infrastructure.openshift.server.OpenShiftExternalServerExposer; import org.eclipse.che.workspace.infrastructure.openshift.server.OpenShiftServerResolver; -import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.Listeners; import org.testng.annotations.Test; From d49b11fb8260ee0de666c19a65811cd31d0dc588 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Tue, 13 Aug 2019 21:43:38 +0200 Subject: [PATCH 07/12] Remove unused constructor parameter. Signed-off-by: Lukas Krejci --- .../kubernetes/KubernetesInternalRuntime.java | 6 +----- .../kubernetes/KubernetesInternalRuntimeTest.java | 3 --- .../infrastructure/openshift/OpenShiftInternalRuntime.java | 1 - 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java index 4c5c7b76e9b..28a620fced3 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java @@ -85,7 +85,6 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.event.PodEvent; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; @@ -120,8 +119,7 @@ public class KubernetesInternalRuntime private final Set internalEnvironmentProvisioners; private final KubernetesEnvironmentProvisioner kubernetesEnvironmentProvisioner; private final SidecarToolingProvisioner toolingProvisioner; - protected final ExternalServerExposer externalServerExposer; - protected final ExternalServerPathDemangler externalServerPathDemangler; + private final ExternalServerPathDemangler externalServerPathDemangler; private final RuntimeHangingDetector runtimeHangingDetector; protected final Tracer tracer; @@ -144,7 +142,6 @@ public KubernetesInternalRuntime( Set internalEnvironmentProvisioners, KubernetesEnvironmentProvisioner kubernetesEnvironmentProvisioner, SidecarToolingProvisioner toolingProvisioner, - ExternalServerExposer externalServerExposer, ExternalServerPathDemangler externalServerPathDemangler, RuntimeHangingDetector runtimeHangingDetector, Tracer tracer, @@ -167,7 +164,6 @@ public KubernetesInternalRuntime( this.toolingProvisioner = toolingProvisioner; this.kubernetesEnvironmentProvisioner = kubernetesEnvironmentProvisioner; this.internalEnvironmentProvisioners = internalEnvironmentProvisioners; - this.externalServerExposer = externalServerExposer; this.externalServerPathDemangler = externalServerPathDemangler; this.runtimeHangingDetector = runtimeHangingDetector; this.startSynchronizer = startSynchronizerFactory.create(context.getIdentity()); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java index 30ece9ce169..ec400e307d1 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java @@ -126,7 +126,6 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.event.PodEvent; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer; import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.PodEvents; @@ -197,7 +196,6 @@ public class KubernetesInternalRuntimeTest { @Mock private WorkspaceProbes workspaceProbes; @Mock private KubernetesServerResolver kubernetesServerResolver; @Mock private InternalEnvironmentProvisioner internalEnvironmentProvisioner; - @Mock private ExternalServerExposer serverExposer; @Mock private ExternalServerPathDemangler pathDemangler; @Mock private RuntimeHangingDetector runtimeHangingDetector; @@ -264,7 +262,6 @@ public void setup() throws Exception { ImmutableSet.of(internalEnvironmentProvisioner), kubernetesEnvironmentProvisioner, toolingProvisioner, - serverExposer, pathDemangler, runtimeHangingDetector, tracer, diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java index 7841ea5b352..66b8d701751 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java @@ -96,7 +96,6 @@ public OpenShiftInternalRuntime( kubernetesEnvironmentProvisioner, toolingProvisioner, null, - null, runtimeHangingDetector, tracer, context, From ac7178face4408ebfd1bd721aee826ad0689767b Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Tue, 13 Aug 2019 23:41:44 +0200 Subject: [PATCH 08/12] * Improve the descriptions in che.properties * Remove the no longer used ExternalServerExposerStrategy interface * Small improvements in naming Signed-off-by: Lukas Krejci --- .../webapp/WEB-INF/classes/che/che.properties | 15 ++++- .../external/ExternalServerExposer.java | 10 ++- .../ExternalServerExposerStrategy.java | 64 ------------------- .../external/ExternalServerPathDemangler.java | 12 ++-- 4 files changed, 23 insertions(+), 78 deletions(-) delete mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategy.java diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index cf4ee9e8b2e..6b54914641f 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -478,14 +478,17 @@ che.infra.kubernetes.pvc.wait_bound=true che.infra.kubernetes.installer_server_min_port=10000 che.infra.kubernetes.installer_server_max_port=20000 -# Defines annotations for ingresses which are used for servers exposing. Value depends on ingress controller. +# Defines annotations for ingresses which are used for servers exposing. Value depends on the kind of ingress +# controller. # # OpenShift infrastructure ignores this property because it uses Routes instead of ingresses. # # Note that for a single-host deployment strategy to work, a controller supporting URL rewriting has to be # used (so that URLs can point to different servers while the servers don't need to support changing the app root). -# See the che.infra.kubernetes.ingress.path.rewrite_transform property for further details on how to achieve -# this. +# The che.infra.kubernetes.ingress.path.rewrite_transform property defines how the path of the ingress should be +# transformed to support the URL rewriting and this property defines the set of annotations on the ingress itself +# that instruct the chosen ingress controller to actually do the URL rewriting, potentially building on the path +# transformation (if required by the chosen ingress controller). # # For example for nginx ingress controller 0.22.0 and later the following value is recommended: # {"ingress.kubernetes.io/rewrite-target": "/$1","ingress.kubernetes.io/ssl-redirect": "false",\ @@ -494,6 +497,9 @@ che.infra.kubernetes.installer_server_max_port=20000 # # For nginx ingress controller older than 0.22.0, the rewrite-target should be set to merely "/" and the path transform # to "%s" (see the the che.infra.kubernetes.ingress.path.rewrite_transform property). +# +# Please consult the nginx ingress controller documentation for the explanation of how the ingress controller uses +# the regular expression present in the ingress path and how it achieves the URL rewriting. che.infra.kubernetes.ingress.annotations_json=NULL # Defines a "recipe" on how to declare the path of the ingress that should expose a server. @@ -502,6 +508,9 @@ che.infra.kubernetes.ingress.annotations_json=NULL # # Please see the description of the che.infra.kubernetes.ingress.annotations_json property to see how these two # properties interplay when specifying the ingress annotations and path. +# +# If not defined, this property defaults to "%s" (without the quotes) which means that the path is not transformed in +# any way for use with the ingress controller. che.infra.kubernetes.ingress.path_transform=NULL # Defines security context for pods that will be created by Kubernetes Infra 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 210c9f4982b..d7e945539e9 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 @@ -22,6 +22,8 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; public class ExternalServerExposer { + static final String PATH_TRANSFORM_PATH_CATCH = "%s"; + private final IngressNamingStrategy strategy; private final Map ingressAnnotations; private final String pathTransformFmt; @@ -33,7 +35,7 @@ public ExternalServerExposer( @Nullable @Named("che.infra.kubernetes.ingress.path_transform") String pathTransformFmt) { this.strategy = strategy; this.ingressAnnotations = annotations; - this.pathTransformFmt = pathTransformFmt == null ? "%s" : pathTransformFmt; + this.pathTransformFmt = pathTransformFmt == null ? PATH_TRANSFORM_PATH_CATCH : pathTransformFmt; } /** @@ -83,10 +85,6 @@ private Ingress generateIngress( } private static String ensureEndsWithSlash(String path) { - if (path.charAt(path.length() - 1) == '/') { - return path; - } else { - return path + '/'; - } + return path.endsWith("/") ? path : path + '/'; } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategy.java deleted file mode 100644 index baeb659ba2f..00000000000 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposerStrategy.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright (c) 2012-2018 Red Hat, Inc. - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.api.model.ServicePort; -import java.util.Map; -import org.eclipse.che.api.core.model.workspace.config.ServerConfig; -import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; - -/** - * Defines a basic set of operations for exposing external servers. - * - * @author Guy Daich - */ -public interface ExternalServerExposerStrategy { - - /** - * 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 - * 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 servicePort specific service port to be exposed externally - * @param externalServers server configs of servers to be exposed externally - */ - void expose( - T k8sEnv, - String machineName, - String serviceName, - ServicePort servicePort, - Map externalServers); - - /** - * Sometimes, the exposer needs to modify the path contained in the object exposing the server - * (ingress or route). Namely, this is needed to make the URL rewriting work for single-host - * strategy where the path needs to contain a regular expression match group to retain some of the - * path. - * - *

This method reverts such mangling and returns to the user a path that can be used by the - * HTTP clients. - * - * @param exposingObject a Kubernetes object in charge of actual exposure of the server (i.e. - * ingress or route) - * @param path the path contained within the configuration of the object that needs to be - * demangled - * @return the path demangled such that it can be used in an externally reachable URL - */ - default String demanglePath(HasMetadata exposingObject, String path) { - return path; - } -} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java index 776f05e7d1d..840b93e17b3 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java @@ -11,6 +11,8 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer.PATH_TRANSFORM_PATH_CATCH; + import io.fabric8.kubernetes.api.model.HasMetadata; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -28,8 +30,8 @@ public ExternalServerPathDemangler( } private static Pattern extractPathFromFmt(String pathTransformFmt) { - int refIdx = pathTransformFmt.indexOf("%s"); - String matchPath = "(.*)"; + int refIdx = pathTransformFmt.indexOf(PATH_TRANSFORM_PATH_CATCH); + String pathMatchingFragment = "(.*)"; String transformed; if (refIdx < 0) { @@ -37,9 +39,9 @@ private static Pattern extractPathFromFmt(String pathTransformFmt) { } else { if (refIdx == 0) { if (pathTransformFmt.length() > 2) { - transformed = matchPath + Pattern.quote(pathTransformFmt.substring(2)); + transformed = pathMatchingFragment + Pattern.quote(pathTransformFmt.substring(2)); } else { - transformed = matchPath; + transformed = pathMatchingFragment; } } else { String prefix = Pattern.quote(pathTransformFmt.substring(0, refIdx)); @@ -48,7 +50,7 @@ private static Pattern extractPathFromFmt(String pathTransformFmt) { ? Pattern.quote(pathTransformFmt.substring(refIdx + 2)) : ""; - transformed = prefix + matchPath + suffix; + transformed = prefix + pathMatchingFragment + suffix; } } From ae7f8c1466abbe989e90426912002be3f1e07156 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Fri, 23 Aug 2019 12:47:38 +0200 Subject: [PATCH 09/12] Better naming and simplifications Signed-off-by: Lukas Krejci --- .../kubernetes/KubernetesInfraModule.java | 35 +++++++++++-------- .../kubernetes/KubernetesInternalRuntime.java | 10 +++--- .../server/KubernetesServerResolver.java | 14 ++++---- ...ltHostIngressServiceExposureStrategy.java} | 8 +---- .../external/ExternalServerExposer.java | 10 ++++-- ...java => IngressPathTransformInverter.java} | 11 +++--- ...va => IngressServiceExposureStrategy.java} | 10 ++++-- ...gressServiceExposureStrategyProvider.java} | 12 ++++--- ...tiHostIngressServiceExposureStrategy.java} | 11 ++---- ...leHostIngressServiceExposureStrategy.java} | 9 ++--- .../KubernetesInternalRuntimeTest.java | 6 ++-- .../server/KubernetesServerResolverTest.java | 4 +-- ...stIngressServiceExposureStrategyTest.java} | 5 +-- ...stIngressServiceExposureStrategyTest.java} | 8 +++-- 14 files changed, 78 insertions(+), 75 deletions(-) rename infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{DefaultHostIngressNamingStrategy.java => DefaultHostIngressServiceExposureStrategy.java} (86%) rename infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{ExternalServerPathDemangler.java => IngressPathTransformInverter.java} (88%) rename infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{IngressNamingStrategy.java => IngressServiceExposureStrategy.java} (67%) rename infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{IngressNamingStrategyProvider.java => IngressServiceExposureStrategyProvider.java} (72%) rename infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{MultiHostIngressNamingStrategy.java => MultiHostIngressServiceExposureStrategy.java} (87%) rename infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{SingleHostIngressNamingStrategy.java => SingleHostIngressServiceExposureStrategy.java} (85%) rename infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{DefaultHostIngressNamingStrategyTest.java => DefaultHostIngressServiceExposureStrategyTest.java} (97%) rename infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/{MultiHostIngressNamingStrategyTest.java => MultiHostIngressServiceExposureStrategyTest.java} (95%) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInfraModule.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInfraModule.java index 59d6e45ba23..74318f9c042 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInfraModule.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInfraModule.java @@ -17,9 +17,9 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.CommonPVCStrategy.COMMON_STRATEGY; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.PerWorkspacePVCStrategy.PER_WORKSPACE_STRATEGY; import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.UniqueWorkspacePVCStrategy.UNIQUE_STRATEGY; -import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostIngressNamingStrategy.DEFAULT_HOST_STRATEGY; -import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressNamingStrategy.MULTI_HOST_STRATEGY; -import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostIngressNamingStrategy.SINGLE_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostIngressServiceExposureStrategy.DEFAULT_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressServiceExposureStrategy.MULTI_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostIngressServiceExposureStrategy.SINGLE_HOST_STRATEGY; import com.google.inject.AbstractModule; import com.google.inject.TypeLiteral; @@ -60,11 +60,11 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.env.LogsRootEnvVariableProvider; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.server.ServersConverter; import org.eclipse.che.workspace.infrastructure.kubernetes.server.IngressAnnotationsProvider; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostIngressNamingStrategy; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressNamingStrategy; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressNamingStrategyProvider; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressNamingStrategy; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostIngressNamingStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.DefaultHostIngressServiceExposureStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressServiceExposureStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressServiceExposureStrategyProvider; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressServiceExposureStrategy; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostIngressServiceExposureStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.DefaultSecureServersFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposerFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.SecureServerExposerFactoryProvider; @@ -117,12 +117,19 @@ protected void configure() { .addBinding() .to(KubernetesClientTermination.class); - MapBinder ingressStrategies = - MapBinder.newMapBinder(binder(), String.class, IngressNamingStrategy.class); - ingressStrategies.addBinding(MULTI_HOST_STRATEGY).to(MultiHostIngressNamingStrategy.class); - ingressStrategies.addBinding(SINGLE_HOST_STRATEGY).to(SingleHostIngressNamingStrategy.class); - ingressStrategies.addBinding(DEFAULT_HOST_STRATEGY).to(DefaultHostIngressNamingStrategy.class); - bind(IngressNamingStrategy.class).toProvider(IngressNamingStrategyProvider.class); + MapBinder ingressStrategies = + MapBinder.newMapBinder(binder(), String.class, IngressServiceExposureStrategy.class); + ingressStrategies + .addBinding(MULTI_HOST_STRATEGY) + .to(MultiHostIngressServiceExposureStrategy.class); + ingressStrategies + .addBinding(SINGLE_HOST_STRATEGY) + .to(SingleHostIngressServiceExposureStrategy.class); + ingressStrategies + .addBinding(DEFAULT_HOST_STRATEGY) + .to(DefaultHostIngressServiceExposureStrategy.class); + bind(IngressServiceExposureStrategy.class) + .toProvider(IngressServiceExposureStrategyProvider.class); bind(ServersConverter.class).to(new TypeLiteral>() {}); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java index 28a620fced3..979379e49b5 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntime.java @@ -85,7 +85,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.event.PodEvent; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressPathTransformInverter; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; import org.eclipse.che.workspace.infrastructure.kubernetes.util.UnrecoverablePodEventListenerFactory; @@ -119,7 +119,7 @@ public class KubernetesInternalRuntime private final Set internalEnvironmentProvisioners; private final KubernetesEnvironmentProvisioner kubernetesEnvironmentProvisioner; private final SidecarToolingProvisioner toolingProvisioner; - private final ExternalServerPathDemangler externalServerPathDemangler; + private final IngressPathTransformInverter ingressPathTransformInverter; private final RuntimeHangingDetector runtimeHangingDetector; protected final Tracer tracer; @@ -142,7 +142,7 @@ public KubernetesInternalRuntime( Set internalEnvironmentProvisioners, KubernetesEnvironmentProvisioner kubernetesEnvironmentProvisioner, SidecarToolingProvisioner toolingProvisioner, - ExternalServerPathDemangler externalServerPathDemangler, + IngressPathTransformInverter ingressPathTransformInverter, RuntimeHangingDetector runtimeHangingDetector, Tracer tracer, @Assisted KubernetesRuntimeContext context, @@ -164,7 +164,7 @@ public KubernetesInternalRuntime( this.toolingProvisioner = toolingProvisioner; this.kubernetesEnvironmentProvisioner = kubernetesEnvironmentProvisioner; this.internalEnvironmentProvisioners = internalEnvironmentProvisioners; - this.externalServerPathDemangler = externalServerPathDemangler; + this.ingressPathTransformInverter = ingressPathTransformInverter; this.runtimeHangingDetector = runtimeHangingDetector; this.startSynchronizer = startSynchronizerFactory.create(context.getIdentity()); this.tracer = tracer; @@ -649,7 +649,7 @@ protected void startMachines() throws InfrastructureException { listenEvents(); final KubernetesServerResolver serverResolver = - new KubernetesServerResolver(externalServerPathDemangler, createdServices, readyIngresses); + new KubernetesServerResolver(ingressPathTransformInverter, createdServices, readyIngresses); doStartMachine(serverResolver); } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java index ea4abfb8fba..f5848b0c3c4 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolver.java @@ -24,7 +24,7 @@ import org.eclipse.che.api.workspace.server.model.impl.ServerImpl; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressPathTransformInverter; /** * Helps to resolve {@link ServerImpl servers} by machine name according to specified {@link Ingress @@ -41,11 +41,13 @@ public class KubernetesServerResolver { private final Multimap services; private final Multimap ingresses; - private ExternalServerPathDemangler demangler; + private IngressPathTransformInverter pathTransformInverter; public KubernetesServerResolver( - ExternalServerPathDemangler demangler, List services, List ingresses) { - this.demangler = demangler; + IngressPathTransformInverter pathTransformInverter, + List services, + List ingresses) { + this.pathTransformInverter = pathTransformInverter; this.services = ArrayListMultimap.create(); for (Service service : services) { String machineName = @@ -114,8 +116,8 @@ private void fillIngressServers(Ingress ingress, Map servers (name, config) -> { String path = buildPath( - demangler.demanglePath( - ingress, ingressRule.getHttp().getPaths().get(0).getPath()), + pathTransformInverter.undoPathTransformation( + ingressRule.getHttp().getPaths().get(0).getPath()), config.getPath()); servers.put( name, diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressServiceExposureStrategy.java similarity index 86% rename from infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategy.java rename to infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressServiceExposureStrategy.java index 5140823c8c9..aac53c52d62 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressServiceExposureStrategy.java @@ -12,7 +12,6 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; import io.fabric8.kubernetes.api.model.ServicePort; -import org.eclipse.che.workspace.infrastructure.kubernetes.Names; /** * Provides a path-based strategy for exposing service ports outside the cluster using Ingress @@ -41,7 +40,7 @@ * @author Sergii Leshchenko * @author Guy Daich */ -public class DefaultHostIngressNamingStrategy implements IngressNamingStrategy { +public class DefaultHostIngressServiceExposureStrategy implements IngressServiceExposureStrategy { public static final String DEFAULT_HOST_STRATEGY = "default-host"; @@ -50,11 +49,6 @@ public String getIngressHost(String serviceName, ServicePort servicePort) { return null; } - @Override - public String getIngressName(String serviceName, ServicePort servicePort) { - return Names.generateName("ingress"); - } - @Override public String getIngressPath(String serviceName, ServicePort servicePort) { return "/" + serviceName + "/" + servicePort.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 d7e945539e9..72b3e2811b7 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 @@ -24,13 +24,13 @@ public class ExternalServerExposer { static final String PATH_TRANSFORM_PATH_CATCH = "%s"; - private final IngressNamingStrategy strategy; + private final IngressServiceExposureStrategy strategy; private final Map ingressAnnotations; private final String pathTransformFmt; @Inject public ExternalServerExposer( - IngressNamingStrategy strategy, + IngressServiceExposureStrategy strategy, @Named("infra.kubernetes.ingress.annotations") Map annotations, @Nullable @Named("che.infra.kubernetes.ingress.path_transform") String pathTransformFmt) { this.strategy = strategy; @@ -75,7 +75,7 @@ private Ingress generateIngress( String.format( pathTransformFmt, ensureEndsWithSlash(strategy.getIngressPath(serviceName, servicePort)))) - .withName(strategy.getIngressName(serviceName, servicePort)) + .withName(getIngressName(serviceName, servicePort)) .withMachineName(machineName) .withServiceName(serviceName) .withAnnotations(ingressAnnotations) @@ -87,4 +87,8 @@ private Ingress generateIngress( private static String ensureEndsWithSlash(String path) { return path.endsWith("/") ? path : path + '/'; } + + private static String getIngressName(String serviceName, ServicePort servicePort) { + return serviceName + "-" + servicePort.getName(); + } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java similarity index 88% rename from infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java rename to infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java index 840b93e17b3..f465d8858c3 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerPathDemangler.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java @@ -13,18 +13,17 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer.PATH_TRANSFORM_PATH_CATCH; -import io.fabric8.kubernetes.api.model.HasMetadata; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.inject.Inject; import javax.inject.Named; import org.eclipse.che.commons.annotation.Nullable; -public class ExternalServerPathDemangler { +public class IngressPathTransformInverter { private final Pattern pathTransformInverse; @Inject - public ExternalServerPathDemangler( + public IngressPathTransformInverter( @Nullable @Named("che.infra.kubernetes.ingress.path_transform") String pathTransformFmt) { this.pathTransformInverse = extractPathFromFmt(pathTransformFmt); } @@ -61,18 +60,16 @@ private static Pattern extractPathFromFmt(String pathTransformFmt) { * Sometimes, the exposer needs to modify the path contained in the object exposing the server * (ingress or route). Namely, this is needed to make the URL rewriting work for single-host * strategy where the path needs to contain a regular expression match group to retain some of the - * path. + * path (at least in the case of the nginx ingress controller). * *

This method reverts such mangling and returns to the user a path that can be used by the * HTTP clients. * - * @param exposingObject a Kubernetes object in charge of actual exposure of the server (i.e. - * ingress or route) * @param path the path contained within the configuration of the object that needs to be * demangled * @return the path demangled such that it can be used in an externally reachable URL */ - public String demanglePath(HasMetadata exposingObject, String path) { + public String undoPathTransformation(String path) { Matcher matcher = pathTransformInverse.matcher(path); if (matcher.matches()) { return matcher.group(1); diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressServiceExposureStrategy.java similarity index 67% rename from infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategy.java rename to infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressServiceExposureStrategy.java index 82a5ac6e696..b9caa9f9827 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressServiceExposureStrategy.java @@ -14,12 +14,16 @@ import io.fabric8.kubernetes.api.model.ServicePort; import org.eclipse.che.commons.annotation.Nullable; -public interface IngressNamingStrategy { +/** + * Implementations of this strategy are used by the {@link ExternalServerExposer} to compose an + * Ingress rule that exposes the services. + */ +public interface IngressServiceExposureStrategy { + /** Returns a host that should be used to expose the service */ @Nullable String getIngressHost(String serviceName, ServicePort servicePort); - String getIngressName(String serviceName, ServicePort servicePort); - + /** Returns the path on which the service should be exposed */ String getIngressPath(String serviceName, ServicePort servicePort); } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategyProvider.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressServiceExposureStrategyProvider.java similarity index 72% rename from infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategyProvider.java rename to infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressServiceExposureStrategyProvider.java index 738c5c5ce82..d281bc0db8a 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressNamingStrategyProvider.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressServiceExposureStrategyProvider.java @@ -21,15 +21,17 @@ import org.eclipse.che.inject.ConfigurationException; @Singleton -public class IngressNamingStrategyProvider implements Provider { +public class IngressServiceExposureStrategyProvider + implements Provider { static final String STRATEGY_PROPERTY = "che.infra.kubernetes.server_strategy"; - private final IngressNamingStrategy namingStrategy; + private final IngressServiceExposureStrategy namingStrategy; @Inject - public IngressNamingStrategyProvider( - @Named(STRATEGY_PROPERTY) String strategy, Map strategies) { + public IngressServiceExposureStrategyProvider( + @Named(STRATEGY_PROPERTY) String strategy, + Map strategies) { namingStrategy = strategies.get(strategy); @@ -40,7 +42,7 @@ public IngressNamingStrategyProvider( } @Override - public IngressNamingStrategy get() { + public IngressServiceExposureStrategy get() { return namingStrategy; } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressServiceExposureStrategy.java similarity index 87% rename from infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategy.java rename to infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressServiceExposureStrategy.java index 2b0d677ef19..07565ef950c 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressServiceExposureStrategy.java @@ -12,7 +12,7 @@ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; import static java.lang.String.format; -import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressNamingStrategyProvider.STRATEGY_PROPERTY; +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; @@ -44,7 +44,7 @@ * @author Sergii Leshchenko * @author Guy Daich */ -public class MultiHostIngressNamingStrategy implements IngressNamingStrategy { +public class MultiHostIngressServiceExposureStrategy implements IngressServiceExposureStrategy { public static final String MULTI_HOST_STRATEGY = "multi-host"; private static final String INGRESS_DOMAIN_PROPERTY = "che.infra.kubernetes.ingress.domain"; @@ -52,7 +52,7 @@ public class MultiHostIngressNamingStrategy implements IngressNamingStrategy { private final String domain; @Inject - public MultiHostIngressNamingStrategy( + public MultiHostIngressServiceExposureStrategy( @Named(INGRESS_DOMAIN_PROPERTY) String domain, @Named(STRATEGY_PROPERTY) String strategy) { if (Strings.isNullOrEmpty(domain) && MULTI_HOST_STRATEGY.equals(strategy)) { throw new ConfigurationException( @@ -69,11 +69,6 @@ public String getIngressHost(String serviceName, ServicePort servicePort) { return serviceName + "-" + servicePort.getName() + "." + domain; } - @Override - public String getIngressName(String serviceName, ServicePort servicePort) { - return serviceName + '-' + servicePort.getName(); - } - @Override public String getIngressPath(String serviceName, ServicePort servicePort) { return "/"; diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressNamingStrategy.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressServiceExposureStrategy.java similarity index 85% rename from infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressNamingStrategy.java rename to infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressServiceExposureStrategy.java index fd00623525e..1415be590ae 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressNamingStrategy.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/SingleHostIngressServiceExposureStrategy.java @@ -42,13 +42,13 @@ * @author Sergii Leshchenko * @author Guy Daich */ -public class SingleHostIngressNamingStrategy implements IngressNamingStrategy { +public class SingleHostIngressServiceExposureStrategy implements IngressServiceExposureStrategy { public static final String SINGLE_HOST_STRATEGY = "single-host"; private final String cheHost; @Inject - public SingleHostIngressNamingStrategy(@Named("che.host") String cheHost) { + public SingleHostIngressServiceExposureStrategy(@Named("che.host") String cheHost) { this.cheHost = cheHost; } @@ -57,11 +57,6 @@ public String getIngressHost(String serviceName, ServicePort servicePort) { return cheHost; } - @Override - public String getIngressName(String serviceName, ServicePort servicePort) { - return serviceName + '-' + servicePort.getName(); - } - @Override public String getIngressPath(String serviceName, ServicePort servicePort) { return "/" + serviceName + "/" + servicePort.getName(); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java index ec400e307d1..ef039d5f8c1 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesInternalRuntimeTest.java @@ -126,7 +126,7 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.event.PodEvent; import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.pvc.WorkspaceVolumesStrategy; import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressPathTransformInverter; import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSharedPool; import org.eclipse.che.workspace.infrastructure.kubernetes.util.PodEvents; import org.eclipse.che.workspace.infrastructure.kubernetes.util.RuntimeEventsPublisher; @@ -196,7 +196,7 @@ public class KubernetesInternalRuntimeTest { @Mock private WorkspaceProbes workspaceProbes; @Mock private KubernetesServerResolver kubernetesServerResolver; @Mock private InternalEnvironmentProvisioner internalEnvironmentProvisioner; - @Mock private ExternalServerPathDemangler pathDemangler; + @Mock private IngressPathTransformInverter pathTransformInverter; @Mock private RuntimeHangingDetector runtimeHangingDetector; @Mock @@ -262,7 +262,7 @@ public void setup() throws Exception { ImmutableSet.of(internalEnvironmentProvisioner), kubernetesEnvironmentProvisioner, toolingProvisioner, - pathDemangler, + pathTransformInverter, runtimeHangingDetector, tracer, context, diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java index 4b727afd3e8..d09d8f7cb85 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java @@ -37,7 +37,7 @@ import org.eclipse.che.commons.lang.Pair; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations; import org.eclipse.che.workspace.infrastructure.kubernetes.Annotations.Serializer; -import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerPathDemangler; +import org.eclipse.che.workspace.infrastructure.kubernetes.server.external.IngressPathTransformInverter; import org.testng.annotations.Test; /** @@ -274,7 +274,7 @@ private Map defaultAttributeAnd(String key, String value) { return attributes; } - private static final class NoopPathDemangler extends ExternalServerPathDemangler { + private static final class NoopPathDemangler extends IngressPathTransformInverter { NoopPathDemangler() { super("%s"); } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategyTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressServiceExposureStrategyTest.java similarity index 97% rename from infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategyTest.java rename to infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressServiceExposureStrategyTest.java index 820cc0158b9..87e30a05c28 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressNamingStrategyTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/DefaultHostIngressServiceExposureStrategyTest.java @@ -36,7 +36,7 @@ import org.testng.annotations.Test; /** @author Guy Daich */ -public class DefaultHostIngressNamingStrategyTest { +public class DefaultHostIngressServiceExposureStrategyTest { private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); private static final String MACHINE_NAME = "pod/main"; @@ -61,7 +61,8 @@ public void setUp() throws Exception { kubernetesEnvironment = KubernetesEnvironment.builder().setPods(ImmutableMap.of("pod", pod)).build(); externalServerExposer = - new ExternalServerExposer<>(new DefaultHostIngressNamingStrategy(), emptyMap(), "%s"); + new ExternalServerExposer<>( + new DefaultHostIngressServiceExposureStrategy(), emptyMap(), "%s"); } @Test diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategyTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressServiceExposureStrategyTest.java similarity index 95% rename from infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategyTest.java rename to infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressServiceExposureStrategyTest.java index f759d1332b5..500288c3fb2 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressNamingStrategyTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/MultiHostIngressServiceExposureStrategyTest.java @@ -14,7 +14,7 @@ import static java.util.Collections.emptyMap; 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.external.MultiHostIngressNamingStrategy.MULTI_HOST_STRATEGY; +import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostIngressServiceExposureStrategy.MULTI_HOST_STRATEGY; import static org.testng.Assert.assertEquals; import com.google.common.collect.ImmutableMap; @@ -37,7 +37,7 @@ import org.testng.annotations.Test; /** @author Guy Daich */ -public class MultiHostIngressNamingStrategyTest { +public class MultiHostIngressServiceExposureStrategyTest { private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); private static final String MACHINE_NAME = "pod/main"; @@ -64,7 +64,9 @@ public void setUp() throws Exception { KubernetesEnvironment.builder().setPods(ImmutableMap.of("pod", pod)).build(); externalServerExposer = new ExternalServerExposer<>( - new MultiHostIngressNamingStrategy(DOMAIN, MULTI_HOST_STRATEGY), emptyMap(), "%s"); + new MultiHostIngressServiceExposureStrategy(DOMAIN, MULTI_HOST_STRATEGY), + emptyMap(), + "%s"); } @Test From 7de04c695f4d740d8e3290239d29eb75f2b1acfc Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Fri, 23 Aug 2019 13:55:10 +0200 Subject: [PATCH 10/12] Better variable name. Signed-off-by: Lukas Krejci --- .../kubernetes/server/external/ExternalServerExposer.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 72b3e2811b7..8f153231a0c 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 @@ -65,13 +65,14 @@ private Ingress generateIngress( ServicePort servicePort, Map ingressesServers) { - ExternalServerIngressBuilder bld = new ExternalServerIngressBuilder(); + ExternalServerIngressBuilder ingressBuilder = new ExternalServerIngressBuilder(); String host = strategy.getIngressHost(serviceName, servicePort); if (host != null) { - bld = bld.withHost(host); + ingressBuilder = ingressBuilder.withHost(host); } - return bld.withPath( + return ingressBuilder + .withPath( String.format( pathTransformFmt, ensureEndsWithSlash(strategy.getIngressPath(serviceName, servicePort)))) From 0659105fec9c1f850cdc38279802a482ff9e8cb6 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Fri, 23 Aug 2019 16:05:48 +0200 Subject: [PATCH 11/12] Rephrase the confusing javadoc. Signed-off-by: Lukas Krejci --- .../server/external/IngressPathTransformInverter.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java index f465d8858c3..b35aeb1d7f0 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java @@ -58,9 +58,9 @@ private static Pattern extractPathFromFmt(String pathTransformFmt) { /** * Sometimes, the exposer needs to modify the path contained in the object exposing the server - * (ingress or route). Namely, this is needed to make the URL rewriting work for single-host - * strategy where the path needs to contain a regular expression match group to retain some of the - * path (at least in the case of the nginx ingress controller). + * (i.e. ingress in this case). Namely, this is needed to make the URL rewriting work for + * single-host strategy where the path needs to contain a regular expression match group to retain + * some of the path (at least in the case of the nginx ingress controller). * *

This method reverts such mangling and returns to the user a path that can be used by the * HTTP clients. From ea4121fae3bf8051d10091fdfa2fa680fffa1115 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Mon, 26 Aug 2019 13:07:50 +0200 Subject: [PATCH 12/12] Some javadocs, better names, code simplification and tests. Signed-off-by: Lukas Krejci --- .../external/ExternalServerExposer.java | 5 ++ .../IngressPathTransformInverter.java | 61 +++++++++++-------- .../server/KubernetesServerResolverTest.java | 26 +++++--- .../IngressPathTransformInverterTest.java | 39 ++++++++++++ 4 files changed, 98 insertions(+), 33 deletions(-) create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverterTest.java 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 8f153231a0c..89053b61a28 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 @@ -22,6 +22,11 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerResolver; public class ExternalServerExposer { + /** + * A string to look for in the value of the "che.infra.kubernetes.ingress.path_transform" + * configuration property that marks the location where the generated public path of the service + * should be put in the final string representing the ingress path. + */ static final String PATH_TRANSFORM_PATH_CATCH = "%s"; private final IngressServiceExposureStrategy strategy; diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java index b35aeb1d7f0..527c65449da 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java @@ -11,6 +11,7 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; +import static java.lang.String.format; import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.ExternalServerExposer.PATH_TRANSFORM_PATH_CATCH; import java.util.regex.Matcher; @@ -18,8 +19,21 @@ import javax.inject.Inject; import javax.inject.Named; import org.eclipse.che.commons.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +/** + * This class is used to undo the effect of the "che.infra.kubernetes.ingress.path_transform" + * configuration on the ingress paths. I.e. use this to get the path-part of the URL exposed by an + * ingress given an ingress path. + * + *

This is usually a noop, apart from the single-host mode. + */ public class IngressPathTransformInverter { + private static final Logger LOGGER = LoggerFactory.getLogger(IngressPathTransformInverter.class); + private static final Pattern PATH_FORMAT_DECONSTRUCTION_REGEX = + Pattern.compile("(.*)" + PATH_TRANSFORM_PATH_CATCH + "(.*)"); + private final Pattern pathTransformInverse; @Inject @@ -28,32 +42,30 @@ public IngressPathTransformInverter( this.pathTransformInverse = extractPathFromFmt(pathTransformFmt); } + /** + * Given the ingress path transformation from the configuration, this method constructs a regex + * that is able to extract the original path from a value transformed by the path transformation. + * E.g. when applied to the transformed path, the regex will extract the substring corresponding + * to the original "%s" in the path transformation. + * + * @param pathTransformFmt the path transformation format + * @return the regex that essentially reverts the effect of the path transformation + */ private static Pattern extractPathFromFmt(String pathTransformFmt) { - int refIdx = pathTransformFmt.indexOf(PATH_TRANSFORM_PATH_CATCH); - String pathMatchingFragment = "(.*)"; - - String transformed; - if (refIdx < 0) { - transformed = Pattern.quote(pathTransformFmt); + Matcher m = PATH_FORMAT_DECONSTRUCTION_REGEX.matcher(pathTransformFmt); + if (m.matches() && m.groupCount() == 2) { + String prefix = Pattern.quote(m.group(1)); + String suffix = Pattern.quote(m.group(2)); + return Pattern.compile("^" + prefix + "(.*)" + suffix + "$"); } else { - if (refIdx == 0) { - if (pathTransformFmt.length() > 2) { - transformed = pathMatchingFragment + Pattern.quote(pathTransformFmt.substring(2)); - } else { - transformed = pathMatchingFragment; - } - } else { - String prefix = Pattern.quote(pathTransformFmt.substring(0, refIdx)); - String suffix = - refIdx < pathTransformFmt.length() - 2 - ? Pattern.quote(pathTransformFmt.substring(refIdx + 2)) - : ""; - - transformed = prefix + pathMatchingFragment + suffix; - } + LOGGER.warn( + format( + "Invalid path transformation format '%s' could not be successfully matched by the" + + " deconstruction regex '%s'. Using the path transformation as is which can result in malfunctioning" + + " ingresses.", + pathTransformFmt, PATH_FORMAT_DECONSTRUCTION_REGEX)); + return Pattern.compile(Pattern.quote(pathTransformFmt)); } - - return Pattern.compile("^" + transformed + "$"); } /** @@ -67,7 +79,8 @@ private static Pattern extractPathFromFmt(String pathTransformFmt) { * * @param path the path contained within the configuration of the object that needs to be * demangled - * @return the path demangled such that it can be used in an externally reachable URL + * @return the path demangled such that it can be used in an externally reachable URL or the + * untouched path if the path doesn't match the configured path format. */ public String undoPathTransformation(String path) { Matcher matcher = pathTransformInverse.matcher(path); diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java index d09d8f7cb85..1e2b48c2487 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java @@ -67,7 +67,9 @@ public class KubernetesServerResolverTest { KubernetesServerResolver serverResolver = new KubernetesServerResolver( - new NoopPathDemangler(), singletonList(nonMatchedByPodService), singletonList(ingress)); + new NoopPathTransformInverter(), + singletonList(nonMatchedByPodService), + singletonList(ingress)); // when Map resolved = serverResolver.resolve("machine"); @@ -85,7 +87,8 @@ public void testResolvingServersWhenThereIsMatchedIngressForTheSpecifiedMachine( Pair.of("http-server", new ServerConfigImpl("3054", "http", "/api/", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(new NoopPathDemangler(), emptyList(), singletonList(ingress)); + new KubernetesServerResolver( + new NoopPathTransformInverter(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -107,7 +110,8 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", null, ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(new NoopPathDemangler(), emptyList(), singletonList(ingress)); + new KubernetesServerResolver( + new NoopPathTransformInverter(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -129,7 +133,8 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", "", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(new NoopPathDemangler(), emptyList(), singletonList(ingress)); + new KubernetesServerResolver( + new NoopPathTransformInverter(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -151,7 +156,8 @@ public void testResolvingServersWhenThereIsMatchedIngressForMachineAndServerPath Pair.of("http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(new NoopPathDemangler(), emptyList(), singletonList(ingress)); + new KubernetesServerResolver( + new NoopPathTransformInverter(), emptyList(), singletonList(ingress)); Map resolved = serverResolver.resolve("machine"); @@ -175,7 +181,8 @@ public void testResolvingInternalServers() { "http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(new NoopPathDemangler(), singletonList(service), emptyList()); + new KubernetesServerResolver( + new NoopPathTransformInverter(), singletonList(service), emptyList()); Map resolved = serverResolver.resolve("machine"); @@ -199,7 +206,8 @@ public void testResolvingInternalServersWithPortWithTransportProtocol() { "http-server", new ServerConfigImpl("3054/udp", "xxx", "api", ATTRIBUTES_MAP))); KubernetesServerResolver serverResolver = - new KubernetesServerResolver(new NoopPathDemangler(), singletonList(service), emptyList()); + new KubernetesServerResolver( + new NoopPathTransformInverter(), singletonList(service), emptyList()); Map resolved = serverResolver.resolve("machine"); @@ -274,8 +282,8 @@ private Map defaultAttributeAnd(String key, String value) { return attributes; } - private static final class NoopPathDemangler extends IngressPathTransformInverter { - NoopPathDemangler() { + private static final class NoopPathTransformInverter extends IngressPathTransformInverter { + NoopPathTransformInverter() { super("%s"); } } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverterTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverterTest.java new file mode 100644 index 00000000000..f52c5aa442c --- /dev/null +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverterTest.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.server.external; + +import static org.testng.Assert.assertEquals; + +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class IngressPathTransformInverterTest { + + @Test(dataProvider = "transformationTestCases") + public void testUndoPathTransformationWithBasicTransform( + String format, String transformedPath, String originalPath) { + IngressPathTransformInverter inverter = new IngressPathTransformInverter(format); + assertEquals(inverter.undoPathTransformation(transformedPath), originalPath); + } + + @DataProvider + public static Object[][] transformationTestCases() { + return new Object[][] { + {"%s", "path", "path"}, + {"invalid", "path", "path"}, + {"%ssuffix", "pathsuffix", "path"}, + {"prefix%s", "prefixpath", "path"}, + {"prefix%ssuffix", "prefixpathsuffix", "path"}, + {"prefix%s", "non-matching", "non-matching"} + }; + } +}