From b9ef2efd0eee06fd18c371f32a626ba4c3f8de3b Mon Sep 17 00:00:00 2001 From: Sergii Leshchenko Date: Thu, 5 Jul 2018 11:49:09 +0300 Subject: [PATCH] fixup! Add SecureServerExposer --- .../model/workspace/config/ServerConfig.java | 8 ++ .../server/KubernetesServerExposer.java | 2 +- .../server/KubernetesServerExposerTest.java | 130 +++++++++++------- 3 files changed, 87 insertions(+), 53 deletions(-) diff --git a/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/ServerConfig.java b/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/ServerConfig.java index 732322a7e4f7..000642708098 100644 --- a/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/ServerConfig.java +++ b/core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/ServerConfig.java @@ -28,6 +28,14 @@ public interface ServerConfig { */ String INTERNAL_SERVER_ATTRIBUTE = "internal"; + /** + * {@link ServerConfig} and {@link Server} attribute name which can identify server as secure or + * non-secure. Requests to secure servers will be authenticated and must contain machine token. + * Attribute value {@code true} makes a server secure, any other value or lack of the + * attribute makes the server non-secure. + */ + String SECURE_SERVER_ATTRIBUTE = "secure"; + /** * Port used by server. * 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 81f1acae91f1..e0bea4a59f03 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 @@ -147,7 +147,7 @@ public void expose(Map servers) throws Infrastru internalServers.put(key, value); } else { // Server is external. Check if it should be secure or not - if ("true".equals(value.getAttributes().get("secure"))) { + if ("true".equals(value.getAttributes().get(ServerConfig.SECURE_SERVER_ATTRIBUTE))) { secureServers.put(key, value); } else { externalServers.put(key, value); 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 623cf6716a37..a68b79558d65 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 @@ -53,13 +53,15 @@ public class KubernetesServerExposerTest { @Mock private ExternalServerExposerStrategy externalServerExposerStrategy; - // TODO Add tests @Mock private SecureServerExposer secureServerExposer; private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); private static final Map INTERNAL_SERVER_ATTRIBUTE_MAP = singletonMap(ServerConfig.INTERNAL_SERVER_ATTRIBUTE, Boolean.TRUE.toString()); + private static final Map SECURE_SERVER_ATTRIBUTE_MAP = + singletonMap(ServerConfig.SECURE_SERVER_ATTRIBUTE, Boolean.TRUE.toString()); + private static final Pattern SERVER_PREFIX_REGEX = Pattern.compile('^' + SERVER_PREFIX + "[A-z0-9]{" + SERVER_UNIQUE_PART_SIZE + "}-pod-main$"); private static final String MACHINE_NAME = "pod/main"; @@ -278,33 +280,36 @@ public void shouldExposeContainerPortAndCreateServiceForInternalServer() throws } @Test - public void shouldExposeInternalAndExternalServers() throws Exception { + public void shouldExposeInternalAndExternalAndSecureServers() throws Exception { // given + ServerConfigImpl secureServerConfig = + new ServerConfigImpl("8282/tcp", "http", "/api", SECURE_SERVER_ATTRIBUTE_MAP); ServerConfigImpl internalServerConfig = new ServerConfigImpl("8080/tcp", "http", "/api", INTERNAL_SERVER_ATTRIBUTE_MAP); ServerConfigImpl externalServerConfig = new ServerConfigImpl("9090/tcp", "http", "/api", ATTRIBUTES_MAP); Map serversToExpose = - ImmutableMap.of("int-server", internalServerConfig, "ext-server", externalServerConfig); + ImmutableMap.of( + "int-server", + internalServerConfig, + "ext-server", + externalServerConfig, + "secure-server", + secureServerConfig); // when serverExposer.expose(serversToExpose); // then assertThatInternalServerIsExposed( - MACHINE_NAME, - "int-server", - "tcp", - 8080, - new ServerConfigImpl(internalServerConfig).withAttributes(INTERNAL_SERVER_ATTRIBUTE_MAP)); + MACHINE_NAME, "int-server", "tcp", 8080, new ServerConfigImpl(internalServerConfig)); assertThatExternalServerIsExposed( - MACHINE_NAME, - "tcp", - 9090, - "ext-server", - new ServerConfigImpl(externalServerConfig).withAttributes(ATTRIBUTES_MAP)); + MACHINE_NAME, "tcp", 9090, "ext-server", new ServerConfigImpl(externalServerConfig)); + assertThatSecureServerIsExposed( + MACHINE_NAME, "tcp", 8282, "secure-server", new ServerConfigImpl(secureServerConfig)); } + @SuppressWarnings("SameParameterValue") private void assertThatExternalServerIsExposed( String machineName, String portProtocol, @@ -322,38 +327,14 @@ private void assertThatExternalServersAreExposed( Integer port, Map expectedServers) { // then - assertTrue( - container - .getPorts() - .stream() - .anyMatch( - p -> - p.getContainerPort().equals(port) - && p.getProtocol().equals(portProtocol.toUpperCase()))); + assertThatContainerPortIsExposed(portProtocol, port); // ensure that service is created - Service service = null; - for (Entry entry : kubernetesEnvironment.getServices().entrySet()) { - if (SERVER_PREFIX_REGEX.matcher(entry.getKey()).matches()) { - service = entry.getValue(); - break; - } - } + Service service = findContainerRelatedService(); assertNotNull(service); // ensure that required service port is exposed - Optional servicePortOpt = - service - .getSpec() - .getPorts() - .stream() - .filter(p -> p.getTargetPort().getIntVal().equals(port)) - .findAny(); - assertTrue(servicePortOpt.isPresent()); - ServicePort servicePort = servicePortOpt.get(); - assertEquals(servicePort.getTargetPort().getIntVal(), port); - assertEquals(servicePort.getPort(), port); - assertEquals(servicePort.getName(), SERVER_PREFIX + "-" + port); + ServicePort servicePort = assertThatServicePortIsExposed(port, service); Annotations.Deserializer serviceAnnotations = Annotations.newDeserializer(service.getMetadata().getAnnotations()); @@ -368,6 +349,37 @@ private void assertThatExternalServersAreExposed( expectedServers); } + @SuppressWarnings("SameParameterValue") + private void assertThatSecureServerIsExposed( + String machineName, + String portProtocol, + Integer port, + String serverName, + ServerConfig serverConfig) + throws Exception { + // then + assertThatContainerPortIsExposed(portProtocol, port); + // ensure that service is created + + Service service = findContainerRelatedService(); + assertNotNull(service); + + // ensure that required service port is exposed + ServicePort servicePort = assertThatServicePortIsExposed(port, service); + + Annotations.Deserializer serviceAnnotations = + Annotations.newDeserializer(service.getMetadata().getAnnotations()); + assertEquals(serviceAnnotations.machineName(), machineName); + + verify(secureServerExposer) + .expose( + kubernetesEnvironment, + machineName, + service.getMetadata().getName(), + servicePort, + ImmutableMap.of(serverName, serverConfig)); + } + @SuppressWarnings("SameParameterValue") private void assertThatInternalServerIsExposed( String machineName, @@ -375,7 +387,26 @@ private void assertThatInternalServerIsExposed( String portProtocol, Integer port, ServerConfigImpl expected) { - // then + assertThatContainerPortIsExposed(portProtocol, port); + + // ensure that service is created + + Service service = findContainerRelatedService(); + assertNotNull(service); + + // ensure that required service port is exposed + assertThatServicePortIsExposed(port, service); + + Annotations.Deserializer serviceAnnotations = + Annotations.newDeserializer(service.getMetadata().getAnnotations()); + assertEquals(serviceAnnotations.machineName(), machineName); + + Map servers = serviceAnnotations.servers(); + ServerConfig serverConfig = servers.get(serverNameRegex); + assertEquals(serverConfig, expected); + } + + private void assertThatContainerPortIsExposed(String portProtocol, Integer port) { assertTrue( container .getPorts() @@ -384,8 +415,9 @@ private void assertThatInternalServerIsExposed( p -> p.getContainerPort().equals(port) && p.getProtocol().equals(portProtocol.toUpperCase()))); - // ensure that service is created + } + private Service findContainerRelatedService() { Service service = null; for (Entry entry : kubernetesEnvironment.getServices().entrySet()) { if (SERVER_PREFIX_REGEX.matcher(entry.getKey()).matches()) { @@ -393,9 +425,10 @@ private void assertThatInternalServerIsExposed( break; } } - assertNotNull(service); + return service; + } - // ensure that required service port is exposed + private ServicePort assertThatServicePortIsExposed(Integer port, Service service) { Optional servicePortOpt = service .getSpec() @@ -408,13 +441,6 @@ private void assertThatInternalServerIsExposed( assertEquals(servicePort.getTargetPort().getIntVal(), port); assertEquals(servicePort.getPort(), port); assertEquals(servicePort.getName(), SERVER_PREFIX + "-" + port); - - Annotations.Deserializer serviceAnnotations = - Annotations.newDeserializer(service.getMetadata().getAnnotations()); - assertEquals(serviceAnnotations.machineName(), machineName); - - Map servers = serviceAnnotations.servers(); - ServerConfig serverConfig = servers.get(serverNameRegex); - assertEquals(serverConfig, expected); + return servicePort; } }