diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/DefaultIsServicePortSecureResolver.java b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/DefaultIsServicePortSecureResolver.java deleted file mode 100644 index 9b69b66a..00000000 --- a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/DefaultIsServicePortSecureResolver.java +++ /dev/null @@ -1,128 +0,0 @@ -/* - * Copyright 2013-2019 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 - * - * https://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.fabric8.discovery; - -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - -import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties; - -/** - * 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; - - 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 && this.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 this.serviceName; - } - - public Map getServiceLabels() { - return this.serviceLabels; - } - - public Map getServiceAnnotations() { - return this.serviceAnnotations; - } - - public Integer getPort() { - return this.port; - } - - } - -} diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java index 472fbd74..8741e1ff 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java @@ -57,7 +57,7 @@ public class KubernetesDiscoveryClient implements DiscoveryClient { private final KubernetesDiscoveryProperties properties; - private final DefaultIsServicePortSecureResolver isServicePortSecureResolver; + private final ServicePortSecureResolver servicePortSecureResolver; private final KubernetesClientServicesFunction kubernetesClientServicesFunction; @@ -73,17 +73,17 @@ public class KubernetesDiscoveryClient implements DiscoveryClient { KubernetesClientServicesFunction kubernetesClientServicesFunction) { this(client, kubernetesDiscoveryProperties, kubernetesClientServicesFunction, - new DefaultIsServicePortSecureResolver(kubernetesDiscoveryProperties)); + new ServicePortSecureResolver(kubernetesDiscoveryProperties)); } KubernetesDiscoveryClient(KubernetesClient client, KubernetesDiscoveryProperties kubernetesDiscoveryProperties, KubernetesClientServicesFunction kubernetesClientServicesFunction, - DefaultIsServicePortSecureResolver isServicePortSecureResolver) { + ServicePortSecureResolver servicePortSecureResolver) { this.client = client; this.properties = kubernetesDiscoveryProperties; this.kubernetesClientServicesFunction = kubernetesClientServicesFunction; - this.isServicePortSecureResolver = isServicePortSecureResolver; + this.servicePortSecureResolver = servicePortSecureResolver; } public KubernetesClient getClient() { @@ -171,7 +171,7 @@ public class KubernetesDiscoveryClient implements DiscoveryClient { EndpointPort endpointPort = findEndpointPort(s); instances.add(new KubernetesServiceInstance(instanceId, serviceId, endpointAddress.getIp(), endpointPort.getPort(), endpointMetadata, - this.isServicePortSecureResolver.resolve(new DefaultIsServicePortSecureResolver.Input( + this.servicePortSecureResolver.resolve(new ServicePortSecureResolver.Input( endpointPort.getPort(), service.getMetadata().getName(), service.getMetadata().getLabels(), service.getMetadata().getAnnotations())))); } diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientAutoConfiguration.java b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientAutoConfiguration.java index 51b3110a..744bb612 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientAutoConfiguration.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientAutoConfiguration.java @@ -46,12 +46,6 @@ import org.springframework.context.annotation.Configuration; @AutoConfigureAfter({ Fabric8AutoConfiguration.class }) public class KubernetesDiscoveryClientAutoConfiguration { - @Bean - @ConditionalOnMissingBean - public DefaultIsServicePortSecureResolver isServicePortSecureResolver(KubernetesDiscoveryProperties properties) { - return new DefaultIsServicePortSecureResolver(properties); - } - @Bean public KubernetesClientServicesFunction servicesFunction(KubernetesDiscoveryProperties properties) { if (properties.getServiceLabels().isEmpty()) { @@ -96,10 +90,9 @@ public class KubernetesDiscoveryClientAutoConfiguration { @ConditionalOnMissingBean public KubernetesDiscoveryClient kubernetesDiscoveryClient(KubernetesClient client, KubernetesDiscoveryProperties properties, - KubernetesClientServicesFunction kubernetesClientServicesFunction, - DefaultIsServicePortSecureResolver isServicePortSecureResolver) { + KubernetesClientServicesFunction kubernetesClientServicesFunction) { return new KubernetesDiscoveryClient(client, properties, kubernetesClientServicesFunction, - isServicePortSecureResolver); + new ServicePortSecureResolver(properties)); } } diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/ServicePortSecureResolver.java b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/ServicePortSecureResolver.java new file mode 100644 index 00000000..07a71333 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/ServicePortSecureResolver.java @@ -0,0 +1,103 @@ +/* + * Copyright 2013-2019 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 + * + * https://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.fabric8.discovery; + +import java.util.Collections; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties; + +class ServicePortSecureResolver { + + private static final Log LOG = LogFactory.getLog(ServicePortSecureResolver.class); + + private static final Set TRUTHY_STRINGS = Stream.of("true", "on", "yes", "1").collect(Collectors.toSet()); + + private final KubernetesDiscoveryProperties properties; + + ServicePortSecureResolver(KubernetesDiscoveryProperties properties) { + this.properties = properties; + } + + /** + *

+ * Returns true if any of the following conditions apply. + *

+ *

+ * + */ + boolean resolve(Input input) { + + String securedLabelValue = input.serviceLabels.getOrDefault("secured", "false"); + if (TRUTHY_STRINGS.contains(securedLabelValue)) { + LOG.debug("Considering service with name: " + input.serviceName + " and port " + input.port + + " is secure since the service contains a true value for the 'secured' label"); + return true; + } + + String securedAnnotationValue = input.serviceAnnotations.getOrDefault("secured", "false"); + if (TRUTHY_STRINGS.contains(securedAnnotationValue)) { + LOG.debug("Considering service with name: " + input.serviceName + " and port " + input.port + + " is secure since the service contains a true value for the 'secured' annotation"); + return true; + } + + if (input.port != null && this.properties.getKnownSecurePorts().contains(input.port)) { + LOG.debug("Considering service with name: " + input.serviceName + " and port " + input.port + + " is secure due to the port being a known https port"); + return true; + } + + return false; + } + + static final 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 ? Collections.emptyMap() : serviceLabels; + this.serviceAnnotations = serviceAnnotations == null ? Collections.emptyMap() : serviceAnnotations; + } + + } + +} diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/DefaultIsServicePortSecureResolverTest.java b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/DefaultIsServicePortSecureResolverTest.java deleted file mode 100644 index d41c412f..00000000 --- a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/DefaultIsServicePortSecureResolverTest.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright 2013-2019 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 - * - * https://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.fabric8.discovery; - -import java.util.HashMap; - -import org.junit.Test; - -import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties; - -import static org.assertj.core.api.Assertions.assertThat; - -public class DefaultIsServicePortSecureResolverTest { - - @Test - public void testPortNumbersOnly() { - final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(); - properties.getKnownSecurePorts().add(12345); - - final DefaultIsServicePortSecureResolver sut = new DefaultIsServicePortSecureResolver(properties); - - assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(null, "dummy"))).isFalse(); - assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(8080, "dummy"))).isFalse(); - assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(1234, "dummy"))).isFalse(); - - assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(443, "dummy"))).isTrue(); - assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(8443, "dummy"))).isTrue(); - assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(12345, "dummy"))).isTrue(); - } - - @Test - public void testLabelsAndAnnotations() { - final DefaultIsServicePortSecureResolver sut = new DefaultIsServicePortSecureResolver( - new KubernetesDiscoveryProperties()); - - assertThat( - sut.resolve(new DefaultIsServicePortSecureResolver.Input(8080, "dummy", new HashMap() { - { - put("secured", "true"); - put("other", "value"); - } - }, new HashMap<>()))).isTrue(); - assertThat( - sut.resolve(new DefaultIsServicePortSecureResolver.Input(1234, "dummy", new HashMap() { - { - put("other", "value"); - put("secured", "1"); - } - }, new HashMap<>()))).isTrue(); - assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(4321, "dummy", new HashMap<>(), - new HashMap() { - { - put("other1", "value1"); - put("secured", "yes"); - put("other2", "value2"); - } - }))).isTrue(); - } - -} diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientFilterMetadataTest.java b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientFilterMetadataTest.java index ec127553..442ca459 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientFilterMetadataTest.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientFilterMetadataTest.java @@ -69,7 +69,7 @@ public class KubernetesDiscoveryClientFilterMetadataTest { private KubernetesDiscoveryProperties properties; @Mock - private DefaultIsServicePortSecureResolver isServicePortSecureResolver; + private ServicePortSecureResolver isServicePortSecureResolver; @Mock private KubernetesDiscoveryProperties.Metadata metadata; diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientTest.java b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientTest.java index 96947716..d7e8f73b 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientTest.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientTest.java @@ -100,7 +100,7 @@ public class KubernetesDiscoveryClientTest { properties.getMetadata().setAddAnnotations(false); final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties, - KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties)); + KubernetesClient::services, new ServicePortSecureResolver(properties)); final List instances = discoveryClient.getInstances("endpoint"); @@ -141,7 +141,7 @@ public class KubernetesDiscoveryClientTest { properties.setPrimaryPortName("http_tcp"); final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties, - KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties)); + KubernetesClient::services, new ServicePortSecureResolver(properties)); final List instances = discoveryClient.getInstances("endpoint"); @@ -175,7 +175,7 @@ public class KubernetesDiscoveryClientTest { properties.setServiceLabels(labels); final KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties, - KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties)); + KubernetesClient::services, new ServicePortSecureResolver(properties)); final List result_endpoints = discoveryClient.getEndPointsList("endpoint"); @@ -218,7 +218,7 @@ public class KubernetesDiscoveryClientTest { properties.getMetadata().setAddLabels(false); final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties, - KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties)); + KubernetesClient::services, new ServicePortSecureResolver(properties)); final List instances = discoveryClient.getInstances("endpoint"); @@ -244,7 +244,7 @@ public class KubernetesDiscoveryClientTest { final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(); final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties, - KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties)); + KubernetesClient::services, new ServicePortSecureResolver(properties)); final List services = discoveryClient.getServices(); @@ -274,7 +274,7 @@ public class KubernetesDiscoveryClientTest { { put("label", "value"); } - }), new DefaultIsServicePortSecureResolver(properties)); + }), new ServicePortSecureResolver(properties)); final List services = discoveryClient.getServices(); @@ -341,7 +341,7 @@ public class KubernetesDiscoveryClientTest { properties.setAllNamespaces(true); final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties, - KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties)); + KubernetesClient::services, new ServicePortSecureResolver(properties)); final List instances = discoveryClient.getInstances("endpoint"); diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/ServicePortSecureResolverTest.java b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/ServicePortSecureResolverTest.java new file mode 100644 index 00000000..b2d29c82 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/ServicePortSecureResolverTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2013-2019 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 + * + * https://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.fabric8.discovery; + +import java.util.Collections; +import java.util.Map; + +import org.junit.Test; + +import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ServicePortSecureResolverTest { + + private static final Map SECURED_TRUE_MAP = Collections.singletonMap("secured", "true"); + + private static final Map SECURED_1_MAP = Collections.singletonMap("secured", "1"); + + private static final Map SECURED_YES_MAP = Collections.singletonMap("secured", "yes"); + + private static final Map SECURED_ON_MAP = Collections.singletonMap("secured", "on"); + + private static final ServicePortSecureResolver.Input SECURED_TRUE = new ServicePortSecureResolver.Input(8080, + "dummy", SECURED_TRUE_MAP, Collections.emptyMap()); + + private static final ServicePortSecureResolver.Input SECURED_1 = new ServicePortSecureResolver.Input(1234, "dummy", + SECURED_1_MAP, Collections.emptyMap()); + + private static final ServicePortSecureResolver.Input SECURED_YES = new ServicePortSecureResolver.Input(4321, + "dummy", SECURED_YES_MAP, Collections.emptyMap()); + + private static final ServicePortSecureResolver.Input SECURED_ON = new ServicePortSecureResolver.Input(4321, "dummy", + SECURED_ON_MAP, Collections.emptyMap()); + + @Test + public void testPortNumbersOnly() { + KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(); + properties.getKnownSecurePorts().add(12345); + + ServicePortSecureResolver secureResolver = new ServicePortSecureResolver(properties); + + assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(null, "dummy"))).isFalse(); + assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(8080, "dummy"))).isFalse(); + assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(1234, "dummy"))).isFalse(); + + assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(443, "dummy"))).isTrue(); + assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(8443, "dummy"))).isTrue(); + assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(12345, "dummy"))).isTrue(); + } + + @Test + public void testLabelsAndAnnotations() { + ServicePortSecureResolver secureResolver = new ServicePortSecureResolver(new KubernetesDiscoveryProperties()); + + assertThat(secureResolver.resolve(SECURED_TRUE)).isTrue(); + assertThat(secureResolver.resolve(SECURED_1)).isTrue(); + assertThat(secureResolver.resolve(SECURED_YES)).isTrue(); + assertThat(secureResolver.resolve(SECURED_ON)).isTrue(); + } + +}