diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapper.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapper.java index f77f85cc..57f693c8 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapper.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2023 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. @@ -16,52 +16,69 @@ package org.springframework.cloud.kubernetes.commons.loadbalancer; -import java.util.HashMap; import java.util.Map; +import java.util.StringJoiner; + +import org.apache.commons.logging.LogFactory; import org.springframework.cloud.kubernetes.commons.discovery.KubernetesServiceInstance; +import org.springframework.core.log.LogAccessor; import org.springframework.util.StringUtils; +import static org.springframework.cloud.kubernetes.commons.config.ConfigUtils.keysWithPrefix; + /** * @author Ryan Baxter */ public interface KubernetesServiceInstanceMapper { + /** + * Logger instance. + */ + LogAccessor LOG = new LogAccessor(LogFactory.getLog(KubernetesServiceInstanceMapper.class)); + KubernetesServiceInstance map(T service); static String createHost(String serviceName, String namespace, String clusterDomain) { - return String.format("%s.%s.svc.%s", serviceName, StringUtils.hasText(namespace) ? namespace : "default", - clusterDomain); + String namespaceToUse = StringUtils.hasText(namespace) ? namespace : "default"; + return new StringJoiner(".").add(serviceName).add(namespaceToUse).add("svc").add(clusterDomain).toString(); } static boolean isSecure(Map labels, Map annotations, String servicePortName, Integer servicePort) { - if (labels != null) { - final String securedLabelValue = labels.getOrDefault("secured", "false"); - if (securedLabelValue.equals("true")) { - return true; - } + + if (hasTrueSecuredValue(labels)) { + LOG.debug(() -> "Service has a true 'secured' label"); + return true; } - if (annotations != null) { - final String securedAnnotationValue = annotations.getOrDefault("secured", "false"); - if (securedAnnotationValue.equals("true")) { - return true; - } + if (hasTrueSecuredValue(annotations)) { + LOG.debug(() -> "Service has a true 'secured' annotation"); + return true; } - return (servicePortName != null && servicePortName.endsWith("https")) || servicePort.toString().endsWith("443"); + + if (servicePortName != null && servicePortName.endsWith("https")) { + LOG.debug(() -> "Service port name ends with 'https'"); + return true; + } + + if (servicePort != null && servicePort.toString().endsWith("443")) { + LOG.debug(() -> "Service port ends with '443'"); + return true; + } + + return false; } static Map getMapWithPrefixedKeys(Map map, String prefix) { - if (map == null) { - return new HashMap<>(); + return keysWithPrefix(map, prefix); + } + + private static boolean hasTrueSecuredValue(Map input) { + if (input != null) { + return "true".equals(input.get("secured")); } - if (!StringUtils.hasText(prefix)) { - return map; - } - final Map result = new HashMap<>(); - map.forEach((k, v) -> result.put(prefix + k, v)); - return result; + return false; } } diff --git a/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapperTests.java b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapperTests.java new file mode 100644 index 00000000..95895efe --- /dev/null +++ b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapperTests.java @@ -0,0 +1,194 @@ +/* + * Copyright 2013-2020 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.commons.loadbalancer; + +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author wind57 + */ +@ExtendWith(OutputCaptureExtension.class) +class KubernetesServiceInstanceMapperTests { + + private static final String SECURED_LABEL_MESSAGE = "Service has a true 'secured' label"; + + private static final String SECURED_ANNOTATION_MESSAGE = "Service has a true 'secured' annotation"; + + private static final String NAME_ENDS_IN_HTTPS_MESSAGE = "Service port name ends with 'https'"; + + private static final String PORT_ENDS_IN_443_MESSAGE = "Service port ends with '443'"; + + @Test + void testCreateHostWithNamespace() { + String namespace = "customNamespace"; + String host = KubernetesServiceInstanceMapper.createHost("serviceName", namespace, "clusterDomain"); + assertThat(host).isEqualTo("serviceName.customNamespace.svc.clusterDomain"); + } + + @Test + void testCreateHostWithEmptyNamespace() { + String host = KubernetesServiceInstanceMapper.createHost("serviceName", "", "clusterDomain"); + assertThat(host).isEqualTo("serviceName.default.svc.clusterDomain"); + } + + @Test + void testCreateHostWithNullNamespace() { + String host = KubernetesServiceInstanceMapper.createHost("serviceName", null, "clusterDomain"); + assertThat(host).isEqualTo("serviceName.default.svc.clusterDomain"); + } + + @Test + void testIsSecureWithTrueLabel(CapturedOutput output) { + Map labels = Map.of("secured", "true"); + Map annotations = Map.of(); + String servicePortName = null; + Integer servicePort = null; + assertThat(KubernetesServiceInstanceMapper.isSecure(labels, annotations, servicePortName, servicePort)) + .isTrue(); + + assertThat(output.getOut().contains(SECURED_LABEL_MESSAGE)).isTrue(); + assertThat(output.getOut().contains(SECURED_ANNOTATION_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(NAME_ENDS_IN_HTTPS_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(PORT_ENDS_IN_443_MESSAGE)).isFalse(); + } + + @Test + void testIsSecureWithTrueAnnotation(CapturedOutput output) { + // empty labels + Map labels = Map.of(); + Map annotations = Map.of("secured", "true"); + String servicePortName = null; + Integer servicePort = null; + assertThat(KubernetesServiceInstanceMapper.isSecure(labels, annotations, servicePortName, servicePort)) + .isTrue(); + + assertThat(output.getOut().contains(SECURED_LABEL_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(SECURED_ANNOTATION_MESSAGE)).isTrue(); + assertThat(output.getOut().contains(NAME_ENDS_IN_HTTPS_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(PORT_ENDS_IN_443_MESSAGE)).isFalse(); + } + + @Test + void testIsSecureWithTrueAnnotationNullLabels(CapturedOutput output) { + // null labels + Map labels = null; + Map annotations = Map.of("secured", "true"); + String servicePortName = null; + Integer servicePort = null; + assertThat(KubernetesServiceInstanceMapper.isSecure(labels, annotations, servicePortName, servicePort)) + .isTrue(); + + assertThat(output.getOut().contains(SECURED_LABEL_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(SECURED_ANNOTATION_MESSAGE)).isTrue(); + assertThat(output.getOut().contains(NAME_ENDS_IN_HTTPS_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(PORT_ENDS_IN_443_MESSAGE)).isFalse(); + } + + @Test + void testIsNotSecureServicePortNameAndServicePortAreNull(CapturedOutput output) { + // null labels + Map labels = null; + // null annotations + Map annotations = null; + String servicePortName = null; + Integer servicePort = null; + assertThat(KubernetesServiceInstanceMapper.isSecure(labels, annotations, servicePortName, servicePort)) + .isFalse(); + + assertThat(output.getOut().contains(SECURED_LABEL_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(SECURED_ANNOTATION_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(NAME_ENDS_IN_HTTPS_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(PORT_ENDS_IN_443_MESSAGE)).isFalse(); + } + + @Test + void testIsNotSecureServicePortNameDoesNotMatch(CapturedOutput output) { + // null labels + Map labels = null; + // null annotations + Map annotations = null; + String servicePortName = "abc_https_def"; + Integer servicePort = null; + assertThat(KubernetesServiceInstanceMapper.isSecure(labels, annotations, servicePortName, servicePort)) + .isFalse(); + + assertThat(output.getOut().contains(SECURED_LABEL_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(SECURED_ANNOTATION_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(NAME_ENDS_IN_HTTPS_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(PORT_ENDS_IN_443_MESSAGE)).isFalse(); + } + + @Test + void testIsSecureServicePortNameMatches(CapturedOutput output) { + // null labels + Map labels = null; + // null annotations + Map annotations = null; + String servicePortName = "abc_https"; + Integer servicePort = null; + assertThat(KubernetesServiceInstanceMapper.isSecure(labels, annotations, servicePortName, servicePort)) + .isTrue(); + + assertThat(output.getOut().contains(SECURED_LABEL_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(SECURED_ANNOTATION_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(NAME_ENDS_IN_HTTPS_MESSAGE)).isTrue(); + assertThat(output.getOut().contains(PORT_ENDS_IN_443_MESSAGE)).isFalse(); + } + + @Test + void testIsNotSecureServicePortDoesNotMatch(CapturedOutput output) { + // null labels + Map labels = null; + // null annotations + Map annotations = null; + String servicePortName = null; + Integer servicePort = 444; + assertThat(KubernetesServiceInstanceMapper.isSecure(labels, annotations, servicePortName, servicePort)) + .isFalse(); + + assertThat(output.getOut().contains(SECURED_LABEL_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(SECURED_ANNOTATION_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(NAME_ENDS_IN_HTTPS_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(PORT_ENDS_IN_443_MESSAGE)).isFalse(); + } + + @Test + void testIsSecureServicePortMatches(CapturedOutput output) { + // null labels + Map labels = null; + // null annotations + Map annotations = null; + String servicePortName = null; + Integer servicePort = 443; + assertThat(KubernetesServiceInstanceMapper.isSecure(labels, annotations, servicePortName, servicePort)) + .isTrue(); + + assertThat(output.getOut().contains(SECURED_LABEL_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(SECURED_ANNOTATION_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(NAME_ENDS_IN_HTTPS_MESSAGE)).isFalse(); + assertThat(output.getOut().contains(PORT_ENDS_IN_443_MESSAGE)).isTrue(); + } + +} diff --git a/spring-cloud-kubernetes-commons/src/test/resources/logback-test.xml b/spring-cloud-kubernetes-commons/src/test/resources/logback-test.xml index 13afbf0c..3d389bca 100644 --- a/spring-cloud-kubernetes-commons/src/test/resources/logback-test.xml +++ b/spring-cloud-kubernetes-commons/src/test/resources/logback-test.xml @@ -14,5 +14,6 @@ +