Fix 1405 (#1406)

spring-cloud · Aug 10, 2023 · cea5b8e · cea5b8e
1 parent c046499
commit cea5b8e
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 22 deletions.
diff --git a/...ain/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtils.java b/...ain/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtils.java
@@ -151,12 +151,12 @@ public static ServiceInstance serviceInstance(@Nullable ServicePortSecureResolve
 					serviceMetadata.name(), serviceMetadata.labels(), serviceMetadata.annotations()));
 		}
 
-		Map<String, Map<String, String>> podMetadata = podMetadata(data.podName(), serviceInstanceMetadata,
-			properties, podLabelsAndMetadata);
+		Map<String, Map<String, String>> podMetadata = podMetadata(data.podName(), serviceInstanceMetadata, properties,
+				podLabelsAndMetadata);
 
-		return new DefaultKubernetesServiceInstance(data.instanceId(), serviceMetadata.name(),
-			data.host(), portData.portNumber(), serviceInstanceMetadata, secured,
-			serviceMetadata.namespace(), null, podMetadata);
+		return new DefaultKubernetesServiceInstance(data.instanceId(), serviceMetadata.name(), data.host(),
+				portData.portNumber(), serviceInstanceMetadata, secured, serviceMetadata.namespace(), null,
+				podMetadata);
 	}
 
 	/**

diff --git a/...ava/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtilsTests.java b/...ava/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtilsTests.java
@@ -661,8 +661,7 @@ void testServiceInstance() {
 		ServicePortSecureResolver resolver = new ServicePortSecureResolver(properties);
 
 		ServicePortNameAndNumber portData = new ServicePortNameAndNumber(8080, "http");
-		ServiceMetadata forServiceInstance = new ServiceMetadata("my-service", "k8s", "ClusterIP", Map.of(),
-				Map.of());
+		ServiceMetadata forServiceInstance = new ServiceMetadata("my-service", "k8s", "ClusterIP", Map.of(), Map.of());
 		InstanceIdHostPodName instanceIdHostPodName = new InstanceIdHostPodName("123", "127.0.0.1", null);
 		Map<String, String> serviceMetadata = Map.of("a", "b");
 
@@ -689,8 +688,7 @@ void testExternalNameServiceInstance() {
 				false, "", Set.of(), Map.of(), "", KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, false, false);
 
 		ServicePortNameAndNumber portData = new ServicePortNameAndNumber(-1, "http");
-		ServiceMetadata forServiceInstance = new ServiceMetadata("my-service", "k8s", "ClusterIP", Map.of(),
-				Map.of());
+		ServiceMetadata forServiceInstance = new ServiceMetadata("my-service", "k8s", "ClusterIP", Map.of(), Map.of());
 		InstanceIdHostPodName instanceIdHostPodName = new InstanceIdHostPodName("123", "spring.io", null);
 		Map<String, String> serviceMetadata = Map.of("a", "b");
 

diff --git a/...ngframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java b/...ngframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java
@@ -50,6 +50,8 @@
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 
+import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.UNSET_PORT_NAME;
+
 /**
  * @author wind57
  */
@@ -199,20 +201,12 @@ static Map<String, String> portsData(List<EndpointSubset> endpointSubsets) {
 
 	static LinkedHashMap<String, Integer> endpointSubsetPortsData(EndpointSubset endpointSubset) {
 		LinkedHashMap<String, Integer> result = new LinkedHashMap<>();
-		List<EndpointPort> endpointPorts = endpointSubset.getPorts();
-
-		// this is most probably not a needed if statement, but it preserves the
-		// previous logic before I refactored the code. In particular, this takes care of
-		// the fact that an EndpointsPort name could be missing.
-		if (endpointPorts.size() == 1) {
-			result.put(endpointPorts.get(0).getName(), endpointPorts.get(0).getPort());
-			return result;
-		}
-
 		endpointSubset.getPorts().forEach(port -> {
-			if (StringUtils.hasText(port.getName())) {
-				result.put(port.getName(), port.getPort());
-			}
+			// a service is allowed to not set a port name for a single entry.
+			// two ports without name can not be deployed, as this in an error
+			String portName = StringUtils.hasText(port.getName()) ? port.getName() : UNSET_PORT_NAME;
+			Integer portNumber = port.getPort();
+			result.put(portName, portNumber);
 		});
 		return result;
 	}

diff --git a/...mework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtilsTests.java b/...mework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtilsTests.java
@@ -16,6 +16,7 @@
 
 package org.springframework.cloud.kubernetes.fabric8.discovery;
 
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -36,6 +37,7 @@
 import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
 import org.springframework.mock.env.MockEnvironment;
 
+import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.endpointSubsetPortsData;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.services;
 
 /**
@@ -310,6 +312,34 @@ void testPortsDataTwo() {
 		Assertions.assertEquals(portsData.get("http"), "8081");
 	}
 
+	@Test
+	void endpointSubsetPortsDataWithoutPorts() {
+		EndpointSubset endpointSubset = new EndpointSubsetBuilder().build();
+		LinkedHashMap<String, Integer> result = endpointSubsetPortsData(endpointSubset);
+
+		Assertions.assertEquals(result.size(), 0);
+	}
+
+	@Test
+	void endpointSubsetPortsDataSinglePort() {
+		EndpointSubset endpointSubset = new EndpointSubsetBuilder()
+				.withPorts(new EndpointPortBuilder().withName("name").withPort(80).build()).build();
+		LinkedHashMap<String, Integer> result = endpointSubsetPortsData(endpointSubset);
+
+		Assertions.assertEquals(result.size(), 1);
+		Assertions.assertEquals(result.get("name"), 80);
+	}
+
+	@Test
+	void endpointSubsetPortsDataSinglePortNoName() {
+		EndpointSubset endpointSubset = new EndpointSubsetBuilder()
+				.withPorts(new EndpointPortBuilder().withPort(80).build()).build();
+		LinkedHashMap<String, Integer> result = endpointSubsetPortsData(endpointSubset);
+
+		Assertions.assertEquals(result.size(), 1);
+		Assertions.assertEquals(result.get("<unset>"), 80);
+	}
+
 	private void service(String name, String namespace, Map<String, String> labels) {
 		Service service = new ServiceBuilder().withNewMetadata().withName(name).withLabels(labels)
 				.withNamespace(namespace).and().build();