diff --git a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/DefaultIsServicePortSecureResolver.java b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/DefaultIsServicePortSecureResolver.java new file mode 100644 index 00000000..016b5fbd --- /dev/null +++ b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/DefaultIsServicePortSecureResolver.java @@ -0,0 +1,120 @@ +/* + * Copyright 2013-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.springframework.cloud.kubernetes.discovery; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +/** + * TODO break up into delegates if the implementation get's more complicated + *

+ * Returns true if one of the following conditions apply: + *

+ * spring.cloud.kubernetes.discovery.secured has been set to true + * the service contains a label or an annotation named 'secured' that is truthy + * the port is one of the known ports used for secure communication + */ +class DefaultIsServicePortSecureResolver { + + private static final Log log = LogFactory.getLog(DefaultIsServicePortSecureResolver.class); + + + private static final Set TRUTHY_STRINGS = new HashSet() {{ + add("true"); + add("on"); + add("yes"); + add("1"); + }}; + + private final KubernetesDiscoveryProperties properties; + + public DefaultIsServicePortSecureResolver(KubernetesDiscoveryProperties properties) { + this.properties = properties; + } + + boolean resolve(Input input) { + final String securedLabelValue = input.getServiceLabels().getOrDefault("secured", "false"); + if (TRUTHY_STRINGS.contains(securedLabelValue)) { + if (log.isDebugEnabled()) { + log.debug("Considering service with name: " + input.getServiceName() + " and port " + input.getPort() + + " is secure since the service contains a true value for the 'secured' label"); + } + return true; + } + + final String securedAnnotationValue = input.getServiceAnnotations().getOrDefault("secured", "false"); + if (TRUTHY_STRINGS.contains(securedAnnotationValue)) { + if (log.isDebugEnabled()) { + log.debug("Considering service with name: " + input.getServiceName() + " and port " + input.getPort() + + " is secure since the service contains a true value for the 'secured' annotation"); + } + return true; + } + + if (input.getPort() != null && properties.getKnownSecurePorts().contains(input.getPort())) { + if (log.isDebugEnabled()) { + log.debug("Considering service with name: " + input.getServiceName() + " and port " + input.getPort() + + " is secure due to the port being a known https port"); + } + return true; + } + + return false; + } + + static class Input { + private final Integer port; + private final String serviceName; + private final Map serviceLabels; + private final Map serviceAnnotations; + + //used only for testing + Input(Integer port, String serviceName) { + this(port, serviceName, null, null); + } + + Input(Integer port, String serviceName, + Map serviceLabels, Map serviceAnnotations) { + this.port = port; + this.serviceName = serviceName; + this.serviceLabels = serviceLabels == null ? new HashMap<>() : serviceLabels; + this.serviceAnnotations = serviceAnnotations == null ? new HashMap<>() : serviceAnnotations; + } + + public String getServiceName() { + return serviceName; + } + + public Map getServiceLabels() { + return serviceLabels; + } + + public Map getServiceAnnotations() { + return serviceAnnotations; + } + + public Integer getPort() { + return port; + } + } +} diff --git a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClient.java b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClient.java index 67f0b389..7fdf024d 100644 --- a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClient.java +++ b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClient.java @@ -48,6 +48,8 @@ public class KubernetesDiscoveryClient implements DiscoveryClient { private KubernetesClient client; private final KubernetesDiscoveryProperties properties; + private final DefaultIsServicePortSecureResolver isServicePortSecureResolver; + private final SpelExpressionParser parser = new SpelExpressionParser(); private final SimpleEvaluationContext evalCtxt = SimpleEvaluationContext .forReadOnlyDataBinding() @@ -55,10 +57,18 @@ public class KubernetesDiscoveryClient implements DiscoveryClient { .build(); public KubernetesDiscoveryClient(KubernetesClient client, - KubernetesDiscoveryProperties kubernetesDiscoveryProperties) { + KubernetesDiscoveryProperties kubernetesDiscoveryProperties) { + + this(client, kubernetesDiscoveryProperties, new DefaultIsServicePortSecureResolver(kubernetesDiscoveryProperties)); + } + + KubernetesDiscoveryClient(KubernetesClient client, + KubernetesDiscoveryProperties kubernetesDiscoveryProperties, + DefaultIsServicePortSecureResolver isServicePortSecureResolver) { this.client = client; this.properties = kubernetesDiscoveryProperties; + this.isServicePortSecureResolver = isServicePortSecureResolver; } public KubernetesClient getClient() { @@ -120,12 +130,21 @@ public class KubernetesDiscoveryClient implements DiscoveryClient { } List addresses = s.getAddresses(); - for (EndpointAddress a : addresses) { + for (EndpointAddress endpointAddress : addresses) { + final EndpointPort endpointPort = + s.getPorts().stream().findFirst().orElseThrow(IllegalStateException::new); instances.add(new KubernetesServiceInstance(serviceId, - a, - s.getPorts().stream().findFirst().orElseThrow(IllegalStateException::new), + endpointAddress, + endpointPort, endpointMetadata, - false)); + isServicePortSecureResolver.resolve( + new DefaultIsServicePortSecureResolver.Input( + endpointPort.getPort(), + service.getMetadata().getName(), + service.getMetadata().getLabels(), + service.getMetadata().getAnnotations() + ) + ))); } } } diff --git a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientAutoConfiguration.java b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientAutoConfiguration.java index 3b444e27..45adcb40 100644 --- a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientAutoConfiguration.java +++ b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientAutoConfiguration.java @@ -18,7 +18,6 @@ package org.springframework.cloud.kubernetes.discovery; import io.fabric8.kubernetes.client.KubernetesClient; - import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -35,12 +34,19 @@ import org.springframework.context.annotation.Configuration; CommonsClientAutoConfiguration.class, }) public class KubernetesDiscoveryClientAutoConfiguration { + @Bean + @ConditionalOnMissingBean + public DefaultIsServicePortSecureResolver isServicePortSecureResolver(KubernetesDiscoveryProperties properties) { + return new DefaultIsServicePortSecureResolver(properties); + } + @Bean @ConditionalOnMissingBean @ConditionalOnProperty(name = "spring.cloud.kubernetes.discovery.enabled",matchIfMissing = true) - public KubernetesDiscoveryClient kubernetesDiscoveryClient(KubernetesClient client, - KubernetesDiscoveryProperties properties) { - return new KubernetesDiscoveryClient(client, properties); + public KubernetesDiscoveryClient kubernetesDiscoveryClient( + KubernetesClient client, KubernetesDiscoveryProperties properties, + DefaultIsServicePortSecureResolver isServicePortSecureResolver) { + return new KubernetesDiscoveryClient(client, properties, isServicePortSecureResolver); } @Bean diff --git a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryProperties.java b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryProperties.java index bf99106f..55692b8b 100644 --- a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryProperties.java +++ b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryProperties.java @@ -21,6 +21,11 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.core.style.ToStringCreator; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + @ConfigurationProperties("spring.cloud.kubernetes.discovery") public class KubernetesDiscoveryProperties { @@ -34,6 +39,13 @@ public class KubernetesDiscoveryProperties { /** SpEL expression to filter services. */ private String filter; + + /** Set the port numbers that are considered secure and use HTTPS */ + private Set knownSecurePorts = new HashSet() {{ + add(443); + add(8443); + }}; + private Metadata metadata = new Metadata(); public boolean isEnabled() { @@ -60,6 +72,14 @@ public class KubernetesDiscoveryProperties { this.filter = filter; } + public Set getKnownSecurePorts() { + return knownSecurePorts; + } + + public void setKnownSecurePorts(Set knownSecurePorts) { + this.knownSecurePorts = knownSecurePorts; + } + public Metadata getMetadata() { return metadata; } diff --git a/spring-cloud-kubernetes-discovery/src/test/groovy/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientTest.groovy b/spring-cloud-kubernetes-discovery/src/test/groovy/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientTest.groovy index 923c1f9f..f786a787 100644 --- a/spring-cloud-kubernetes-discovery/src/test/groovy/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientTest.groovy +++ b/spring-cloud-kubernetes-discovery/src/test/groovy/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientTest.groovy @@ -28,75 +28,38 @@ import spock.lang.Specification class KubernetesDiscoveryClientTest extends Specification { - private static KubernetesMockServer mockServer = new KubernetesMockServer() - private static KubernetesClient mockClient + private static KubernetesMockServer mockServer = new KubernetesMockServer() + private static KubernetesClient mockClient - def setupSpec() { - mockServer.init() - mockClient = mockServer.createClient() + def setupSpec() { + mockServer.init() + mockClient = mockServer.createClient() - //Configure the kubernetes master url to point to the mock server - System.setProperty(Config.KUBERNETES_MASTER_SYSTEM_PROPERTY, mockClient.getConfiguration().getMasterUrl()) - System.setProperty(Config.KUBERNETES_TRUST_CERT_SYSTEM_PROPERTY, "true") - System.setProperty(Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false") - System.setProperty(Config.KUBERNETES_AUTH_TRYSERVICEACCOUNT_SYSTEM_PROPERTY, "false") - } + //Configure the kubernetes master url to point to the mock server + System.setProperty(Config.KUBERNETES_MASTER_SYSTEM_PROPERTY, mockClient.getConfiguration().getMasterUrl()) + System.setProperty(Config.KUBERNETES_TRUST_CERT_SYSTEM_PROPERTY, "true") + System.setProperty(Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false") + System.setProperty(Config.KUBERNETES_AUTH_TRYSERVICEACCOUNT_SYSTEM_PROPERTY, "false") + } - def cleanupSpec() { - mockServer.destroy(); - } + def cleanupSpec() { + mockServer.destroy() + } - def "Should be able to handle endpoints single address"() { - given: - mockServer.expect().get().withPath("/api/v1/namespaces/test/endpoints/endpoint").andReturn(200, new EndpointsBuilder() - .withNewMetadata() - .withName("endpoint") - .endMetadata() - .addNewSubset() - .addNewAddress() - .withIp("ip1") - .endAddress() - .addNewPort("http",80,"TCP") - .endSubset() - .build()).once() - and: - mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint").andReturn(200, new ServiceBuilder() - .withNewMetadata() - .withName("endpoint") - .withLabels(new HashMap() {{ - put("l", "v") - }}) - .endMetadata() - .build()).once() - - DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, new KubernetesDiscoveryProperties()) - when: - List instances = discoveryClient.getInstances("endpoint") - then: - instances != null - instances.size() == 1 - instances.find({s -> s.host == "ip1"}) - } - - - - def "Should be able to handle endpoints multiple addresses"() { - given: - mockServer.expect().get().withPath("/api/v1/namespaces/test/endpoints/endpoint").andReturn(200, new EndpointsBuilder() - .withNewMetadata() - .withName("endpoint") - .endMetadata() - .addNewSubset() - .addNewAddress() - .withIp("ip1") - .endAddress() - .addNewAddress() - .withIp("ip2") - .endAddress() - .addNewPort("http",80,"TCP") - .endSubset() - .build()).once() + def "Should be able to handle endpoints single address"() { + given: + mockServer.expect().get().withPath("/api/v1/namespaces/test/endpoints/endpoint").andReturn(200, new EndpointsBuilder() + .withNewMetadata() + .withName("endpoint") + .endMetadata() + .addNewSubset() + .addNewAddress() + .withIp("ip1") + .endAddress() + .addNewPort("http",80,"TCP") + .endSubset() + .build()).once() and: mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint").andReturn(200, new ServiceBuilder() @@ -108,14 +71,60 @@ class KubernetesDiscoveryClientTest extends Specification { .endMetadata() .build()).once() - DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, new KubernetesDiscoveryProperties()) - when: - List instances = discoveryClient.getInstances("endpoint") - then: - instances != null - instances.size() == 2 - instances.find({s -> s.host == "ip1"}) - instances.find({s -> s.host == "ip2"}) + final properties = new KubernetesDiscoveryProperties() + DiscoveryClient discoveryClient = new KubernetesDiscoveryClient( + mockClient, properties, new DefaultIsServicePortSecureResolver(properties)) - } + when: + List instances = discoveryClient.getInstances("endpoint") + + then: + instances != null + instances.size() == 1 + instances.find({s -> s.host == "ip1" && !s.secure}) + } + + + + def "Should be able to handle endpoints multiple addresses"() { + given: + mockServer.expect().get().withPath("/api/v1/namespaces/test/endpoints/endpoint").andReturn(200, new EndpointsBuilder() + .withNewMetadata() + .withName("endpoint") + .endMetadata() + .addNewSubset() + .addNewAddress() + .withIp("ip1") + .endAddress() + .addNewAddress() + .withIp("ip2") + .endAddress() + .addNewPort("https",443,"TCP") + .endSubset() + .build()).once() + + and: + mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint").andReturn(200, new ServiceBuilder() + .withNewMetadata() + .withName("endpoint") + .withLabels(new HashMap() {{ + put("l", "v") + }}) + .endMetadata() + .build()).once() + + final properties = new KubernetesDiscoveryProperties() + DiscoveryClient discoveryClient = new KubernetesDiscoveryClient( + mockClient, properties, new DefaultIsServicePortSecureResolver(properties)) + + when: + List instances = discoveryClient.getInstances("endpoint") + + then: + instances != null + instances.size() == 2 + instances.find({s -> s.host == "ip1" && s.secure}) + instances.find({s -> s.host == "ip2" && s.secure}) + + } } diff --git a/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/DefaultIsServicePortSecureResolverTest.java b/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/DefaultIsServicePortSecureResolverTest.java new file mode 100644 index 00000000..bcb4b4d5 --- /dev/null +++ b/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/DefaultIsServicePortSecureResolverTest.java @@ -0,0 +1,81 @@ +/* + * Copyright 2013-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.springframework.cloud.kubernetes.discovery; + +import org.junit.Test; + +import java.util.HashMap; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class DefaultIsServicePortSecureResolverTest { + + + + @Test + public void testPortNumbersOnly() { + final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(); + properties.getKnownSecurePorts().add(12345); + + final DefaultIsServicePortSecureResolver sut = new DefaultIsServicePortSecureResolver(properties); + + assertFalse(sut.resolve(new DefaultIsServicePortSecureResolver.Input(null, "dummy"))); + assertFalse(sut.resolve(new DefaultIsServicePortSecureResolver.Input(8080, "dummy"))); + assertFalse(sut.resolve(new DefaultIsServicePortSecureResolver.Input(1234, "dummy"))); + + assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(443, "dummy"))); + assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(8443, "dummy"))); + assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(12345, "dummy"))); + } + + @Test + public void testLabelsAndAnnotations() { + final DefaultIsServicePortSecureResolver sut + = new DefaultIsServicePortSecureResolver(new KubernetesDiscoveryProperties()); + + assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input( + 8080, + "dummy", + new HashMap() {{ + put("secured", "true"); + put("other", "value"); + }}, + new HashMap<>())) + ); + assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input( + 1234, + "dummy", + new HashMap() {{ + put("other", "value"); + put("secured", "1"); + }}, + new HashMap<>())) + ); + assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input( + 4321, + "dummy", + new HashMap<>(), + new HashMap() {{ + put("other1", "value1"); + put("secured", "yes"); + put("other2", "value2"); + }})) + ); + } +} diff --git a/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java b/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java index dbd20c7d..74fa84c6 100644 --- a/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java +++ b/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java @@ -58,6 +58,9 @@ public class KubernetesDiscoveryClientFilterMetadataTest { @Mock private KubernetesDiscoveryProperties properties; + @Mock + private DefaultIsServicePortSecureResolver isServicePortSecureResolver; + @Mock private KubernetesDiscoveryProperties.Metadata metadata;