Merge branch 'main' of https://github.com/spring-cloud/spring-cloud-k…

…ubernetes
spring-cloud · Mar 19, 2024 · a600c69 · a600c69
2 parents dcf4e62 + 47f4fe1
commit a600c69
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 44 deletions.
diff --git a/...8-autoconfig/src/main/java/org/springframework/cloud/kubernetes/fabric8/Fabric8Utils.java b/...8-autoconfig/src/main/java/org/springframework/cloud/kubernetes/fabric8/Fabric8Utils.java
@@ -16,12 +16,16 @@
 
 package org.springframework.cloud.kubernetes.fabric8;
 
+import io.fabric8.kubernetes.api.model.ObjectMeta;
+import io.fabric8.kubernetes.api.model.Service;
+import io.fabric8.kubernetes.api.model.ServiceSpec;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import jakarta.annotation.Nullable;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider;
 import org.springframework.cloud.kubernetes.commons.config.NamespaceResolutionFailedException;
+import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadata;
 import org.springframework.core.log.LogAccessor;
 import org.springframework.util.StringUtils;
 
@@ -37,6 +41,13 @@ private Fabric8Utils() {
 
 	}
 
+	public static ServiceMetadata serviceMetadata(Service service) {
+		ObjectMeta metadata = service.getMetadata();
+		ServiceSpec serviceSpec = service.getSpec();
+		return new ServiceMetadata(metadata.getName(), metadata.getNamespace(), serviceSpec.getType(),
+				metadata.getLabels(), metadata.getAnnotations());
+	}
+
 	private static final LogAccessor LOG = new LogAccessor(LogFactory.getLog(Fabric8Utils.class));
 
 	/**

diff --git a/...ngframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java b/...ngframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java
@@ -29,10 +29,8 @@
 import io.fabric8.kubernetes.api.model.EndpointSubset;
 import io.fabric8.kubernetes.api.model.Endpoints;
 import io.fabric8.kubernetes.api.model.EndpointsList;
-import io.fabric8.kubernetes.api.model.ObjectMeta;
 import io.fabric8.kubernetes.api.model.Service;
 import io.fabric8.kubernetes.api.model.ServiceList;
-import io.fabric8.kubernetes.api.model.ServiceSpec;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.dsl.FilterNested;
 import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable;
@@ -43,7 +41,6 @@
 
 import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider;
 import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
-import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadata;
 import org.springframework.cloud.kubernetes.fabric8.Fabric8Utils;
 import org.springframework.core.log.LogAccessor;
 import org.springframework.util.CollectionUtils;
@@ -198,13 +195,6 @@ static Map<String, Integer> endpointSubsetsPortData(List<EndpointSubset> endpoin
 						EndpointPort::getPort));
 	}
 
-	static ServiceMetadata serviceMetadata(Service service) {
-		ObjectMeta metadata = service.getMetadata();
-		ServiceSpec serviceSpec = service.getSpec();
-		return new ServiceMetadata(metadata.getName(), metadata.getNamespace(), serviceSpec.getType(),
-				metadata.getLabels(), metadata.getAnnotations());
-	}
-
 	/**
 	 * serviceName can be null, in which case, such a filter will not be applied.
 	 */

diff --git a/...ava/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java b/...ava/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java
@@ -44,12 +44,12 @@
 import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.serviceInstance;
 import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.serviceInstanceMetadata;
 import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.EXTERNAL_NAME;
+import static org.springframework.cloud.kubernetes.fabric8.Fabric8Utils.serviceMetadata;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8InstanceIdHostPodNameSupplier.externalName;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8InstanceIdHostPodNameSupplier.nonExternalName;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.addresses;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.endpointSubsetsPortData;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.endpoints;
-import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.serviceMetadata;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.services;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8PodLabelsAndAnnotationsSupplier.externalName;
 import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8PodLabelsAndAnnotationsSupplier.nonExternalName;

diff --git a/...g/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapper.java b/...g/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapper.java
@@ -16,7 +16,6 @@
 
 package org.springframework.cloud.kubernetes.fabric8.loadbalancer;
 
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -27,10 +26,13 @@
 import io.fabric8.kubernetes.client.utils.Utils;
 
 import org.springframework.cloud.kubernetes.commons.discovery.DefaultKubernetesServiceInstance;
+import org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils;
 import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
 import org.springframework.cloud.kubernetes.commons.discovery.KubernetesServiceInstance;
+import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadata;
 import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesLoadBalancerProperties;
 import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesServiceInstanceMapper;
+import org.springframework.cloud.kubernetes.fabric8.Fabric8Utils;
 
 /**
  * Class for mapping Kubernetes Service object into {@link KubernetesServiceInstance}.
@@ -39,6 +41,11 @@
  */
 public class Fabric8ServiceInstanceMapper implements KubernetesServiceInstanceMapper<Service> {
 
+	/**
+	 * empty on purpose, load balancer implementation does not need them.
+	 */
+	private static final Map<String, Integer> PORTS_DATA = Map.of();
+
 	private final KubernetesLoadBalancerProperties properties;
 
 	private final KubernetesDiscoveryProperties discoveryProperties;
@@ -57,7 +64,7 @@ public KubernetesServiceInstance map(Service service) {
 		if (ports.size() == 1) {
 			port = ports.get(0);
 		}
-		else if (ports.size() > 1 && Utils.isNotNullOrEmpty(this.properties.getPortName())) {
+		else if (ports.size() > 1 && Utils.isNotNullOrEmpty(properties.getPortName())) {
 			Optional<ServicePort> optPort = ports.stream().filter(it -> properties.getPortName().endsWith(it.getName()))
 					.findAny();
 			if (optPort.isPresent()) {
@@ -72,24 +79,12 @@ else if (ports.size() > 1 && Utils.isNotNullOrEmpty(this.properties.getPortName(
 		boolean secure = KubernetesServiceInstanceMapper.isSecure(service.getMetadata().getLabels(),
 				service.getMetadata().getAnnotations(), port.getName(), port.getPort());
 		return new DefaultKubernetesServiceInstance(meta.getUid(), meta.getName(), host, port.getPort(),
-				getServiceMetadata(service), secure);
+				serviceMetadata(service), secure);
 	}
 
-	private Map<String, String> getServiceMetadata(Service service) {
-		Map<String, String> serviceMetadata = new HashMap<>();
-		KubernetesDiscoveryProperties.Metadata metadataProps = this.discoveryProperties.metadata();
-		if (metadataProps.addLabels()) {
-			Map<String, String> labelMetadata = KubernetesServiceInstanceMapper
-					.getMapWithPrefixedKeys(service.getMetadata().getLabels(), metadataProps.labelsPrefix());
-			serviceMetadata.putAll(labelMetadata);
-		}
-		if (metadataProps.addAnnotations()) {
-			Map<String, String> annotationMetadata = KubernetesServiceInstanceMapper
-					.getMapWithPrefixedKeys(service.getMetadata().getAnnotations(), metadataProps.annotationsPrefix());
-			serviceMetadata.putAll(annotationMetadata);
-		}
-
-		return serviceMetadata;
+	Map<String, String> serviceMetadata(Service service) {
+		ServiceMetadata serviceMetadata = Fabric8Utils.serviceMetadata(service);
+		return DiscoveryClientUtils.serviceInstanceMetadata(PORTS_DATA, serviceMetadata, discoveryProperties);
 	}
 
 }
diff --git a/...ingframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapperTests.java b/...ingframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapperTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2023 the original author or authors.
+ * Copyright 2013-2024 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.
@@ -38,7 +38,7 @@ class Fabric8ServiceInstanceMapperTests {
 	@Test
 	void testMapperSimple() {
 		KubernetesLoadBalancerProperties properties = new KubernetesLoadBalancerProperties();
-		Service service = buildService("test", "abc", 8080, null, Map.of());
+		Service service = buildService("test", "test-namespace", "abc", 8080, null, Map.of());
 		KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties,
 				KubernetesDiscoveryProperties.DEFAULT).map(service);
 		Assertions.assertNotNull(instance);
@@ -53,7 +53,7 @@ void testMapperMultiplePorts() {
 		List<ServicePort> ports = new ArrayList<>();
 		ports.add(new ServicePortBuilder().withPort(8080).withName("web").build());
 		ports.add(new ServicePortBuilder().withPort(9000).withName("http").build());
-		Service service = buildService("test", "abc", ports, Map.of());
+		Service service = buildService("test", "test-namespace", "abc", ports, Map.of());
 		KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties,
 				KubernetesDiscoveryProperties.DEFAULT).map(service);
 		Assertions.assertNotNull(instance);
@@ -65,7 +65,7 @@ void testMapperMultiplePorts() {
 	@Test
 	void testMapperSecure() {
 		KubernetesLoadBalancerProperties properties = new KubernetesLoadBalancerProperties();
-		Service service = buildService("test", "abc", 443, null, Map.of());
+		Service service = buildService("test", "test-namespace", "abc", 443, null, Map.of());
 		KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties,
 				KubernetesDiscoveryProperties.DEFAULT).map(service);
 		Assertions.assertNotNull(instance);
@@ -82,7 +82,7 @@ void testMapperSecureNullLabelsAndAnnotations() {
 				false);
 		List<ServicePort> ports = new ArrayList<>();
 		ports.add(new ServicePortBuilder().withPort(443).build());
-		Service service = buildService("test", "abc", ports, null, null);
+		Service service = buildService("test", "test-namespace", "abc", ports, null, null);
 		KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties, discoveryProperties)
 				.map(service);
 		Assertions.assertNotNull(instance);
@@ -95,29 +95,56 @@ void testMapperSecureNullLabelsAndAnnotations() {
 	void testMapperSecureWithLabels() {
 		KubernetesLoadBalancerProperties properties = new KubernetesLoadBalancerProperties();
 		Map<String, String> labels = Map.of("secured", "true", "label1", "123");
-		Service service = buildService("test", "abc", 8080, null, labels);
+		Service service = buildService("test", "test-namespace", "abc", 8080, null, labels);
 		KubernetesServiceInstance instance = new Fabric8ServiceInstanceMapper(properties,
 				KubernetesDiscoveryProperties.DEFAULT).map(service);
 		Assertions.assertNotNull(instance);
 		Assertions.assertEquals("test", instance.getServiceId());
 		Assertions.assertEquals("abc", instance.getInstanceId());
 		Assertions.assertTrue(instance.isSecure());
-		Assertions.assertEquals(2, instance.getMetadata().keySet().size());
+		Assertions.assertEquals(4, instance.getMetadata().keySet().size());
 	}
 
-	private Service buildService(String name, String uid, int port, String portName, Map<String, String> labels) {
+	@Test
+	void serviceMetadataTest() {
+
+		KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties();
+		KubernetesDiscoveryProperties discoveryProperties = new KubernetesDiscoveryProperties(true, false, Set.of(),
+				true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0,
+				true);
+
+		List<ServicePort> ports = new ArrayList<>();
+		ports.add(new ServicePortBuilder().withPort(443).build());
+
+		Map<String, String> labels = Map.of("one", "1");
+		Map<String, String> annotations = Map.of("two", "2");
+
+		Service service = buildService("test", "test-namespace", "abc", ports, labels, annotations);
+		Map<String, String> result = new Fabric8ServiceInstanceMapper(loadBalancerProperties, discoveryProperties)
+				.serviceMetadata(service);
+		Assertions.assertEquals(result.size(), 4);
+		Assertions.assertEquals(result.get("k8s_namespace"), "test-namespace");
+		Assertions.assertEquals(result.get("type"), "ClusterIP");
+		Assertions.assertEquals(result.get("one"), "1");
+		Assertions.assertEquals(result.get("two"), "2");
+	}
+
+	private Service buildService(String name, String namespace, String uid, int port, String portName,
+			Map<String, String> labels) {
 		ServicePort servicePort = new ServicePortBuilder().withPort(port).withName(portName).build();
-		return buildService(name, uid, Collections.singletonList(servicePort), labels);
+		return buildService(name, namespace, uid, Collections.singletonList(servicePort), labels);
 	}
 
-	private Service buildService(String name, String uid, List<ServicePort> ports, Map<String, String> labels,
-			Map<String, String> annotations) {
-		return new ServiceBuilder().withNewMetadata().withName(name).withUid(uid).addToLabels(labels)
-				.withAnnotations(annotations).endMetadata().withNewSpec().addAllToPorts(ports).endSpec().build();
+	private Service buildService(String name, String namespace, String uid, List<ServicePort> ports,
+			Map<String, String> labels, Map<String, String> annotations) {
+		return new ServiceBuilder().withNewMetadata().withNamespace(namespace).withName(name).withUid(uid)
+				.addToLabels(labels).withAnnotations(annotations).endMetadata().withNewSpec().addAllToPorts(ports)
+				.withType("ClusterIP").endSpec().build();
 	}
 
-	private Service buildService(String name, String uid, List<ServicePort> ports, Map<String, String> labels) {
-		return buildService(name, uid, ports, labels, Map.of());
+	private Service buildService(String name, String namespace, String uid, List<ServicePort> ports,
+			Map<String, String> labels) {
+		return buildService(name, namespace, uid, ports, labels, Map.of());
 	}
 
 }