Fix wrong labels and annotations in kubernetes discovery native client

spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesDiscoveryClientUtils.java

@@ -16,16 +16,22 @@
 
 package org.springframework.cloud.kubernetes.client.discovery;
 
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 
 import io.kubernetes.client.openapi.models.V1ObjectMeta;
 import io.kubernetes.client.openapi.models.V1Service;
+import io.kubernetes.client.openapi.models.V1ServiceSpec;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
 import org.springframework.core.log.LogAccessor;
 
+import static org.springframework.cloud.kubernetes.commons.config.ConfigUtils.keysWithPrefix;
+import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.NAMESPACE_METADATA_KEY;
+import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.SERVICE_TYPE;
+
 /**
  * @author wind57
  */
@@ -61,4 +67,38 @@ static boolean matchesServiceLabels(V1Service service, KubernetesDiscoveryProper
 
 	}
 
+	/**
+	 * This adds the following metadata. <pre>
+	 *     - labels (if requested)
+	 *     - annotations (if requested)
+	 *     - metadata
+	 *     - service type
+	 * </pre>
+	 */
+	static Map<String, String> serviceMetadata(KubernetesDiscoveryProperties properties, V1Service service,
+			String serviceId) {
+
+		Map<String, String> serviceMetadata = new HashMap<>();
+		KubernetesDiscoveryProperties.Metadata metadataProps = properties.metadata();
+		if (metadataProps.addLabels()) {
+			Map<String, String> labelMetadata = keysWithPrefix(service.getMetadata().getLabels(),
+					metadataProps.labelsPrefix());
+			LOG.debug(() -> "Adding labels metadata: " + labelMetadata + " for serviceId: " + serviceId);
+			serviceMetadata.putAll(labelMetadata);
+		}
+		if (metadataProps.addAnnotations()) {
+			Map<String, String> annotationMetadata = keysWithPrefix(service.getMetadata().getAnnotations(),
+					metadataProps.annotationsPrefix());
+			LOG.debug(() -> "Adding annotations metadata: " + annotationMetadata + " for serviceId: " + serviceId);
+			serviceMetadata.putAll(annotationMetadata);
+		}
+
+		serviceMetadata.put(NAMESPACE_METADATA_KEY,
+				Optional.ofNullable(service.getMetadata()).map(V1ObjectMeta::getNamespace).orElse(null));
+		serviceMetadata.put(SERVICE_TYPE,
+				Optional.ofNullable(service.getSpec()).map(V1ServiceSpec::getType).orElse(null));
+
+		return serviceMetadata;
+	}
+
 }

spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClient.java

@@ -47,6 +47,7 @@
 import org.springframework.util.StringUtils;
 
 import static org.springframework.cloud.kubernetes.client.discovery.KubernetesDiscoveryClientUtils.matchesServiceLabels;
+import static org.springframework.cloud.kubernetes.client.discovery.KubernetesDiscoveryClientUtils.serviceMetadata;
 import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.HTTP;
 import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.HTTPS;
 import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.PRIMARY_PORT_NAME_LABEL_KEY;
@@ -114,27 +115,7 @@ public List<ServiceInstance> getInstances(String serviceId) {
 	}
 
 	private Stream<ServiceInstance> getServiceInstanceDetails(V1Service service, String serviceId) {
-		Map<String, String> svcMetadata = new HashMap<>();
-		if (properties.metadata() != null) {
-			if (properties.metadata().addLabels()) {
-				if (service.getMetadata() != null && service.getMetadata().getLabels() != null) {
-					String labelPrefix = properties.metadata().labelsPrefix() != null
-							? properties.metadata().labelsPrefix() : "";
-					service.getMetadata().getLabels().entrySet().stream()
-							.filter(e -> e.getKey().startsWith(labelPrefix))
-							.forEach(e -> svcMetadata.put(e.getKey(), e.getValue()));
-				}
-			}
-			if (properties.metadata().addAnnotations()) {
-				if (service.getMetadata() != null && service.getMetadata().getAnnotations() != null) {
-					String annotationPrefix = properties.metadata().annotationsPrefix() != null
-							? properties.metadata().annotationsPrefix() : "";
-					service.getMetadata().getAnnotations().entrySet().stream()
-							.filter(e -> e.getKey().startsWith(annotationPrefix))
-							.forEach(e -> svcMetadata.put(e.getKey(), e.getValue()));
-				}
-			}
-		}
+		Map<String, String> serviceMetadata = serviceMetadata(properties, service, serviceId);
 
 		V1Endpoints ep = endpointsLister.namespace(service.getMetadata().getNamespace())
 				.get(service.getMetadata().getName());
@@ -154,7 +135,7 @@ private Stream<ServiceInstance> getServiceInstanceDetails(V1Service service, Str
 
 		return ep.getSubsets().stream().filter(subset -> subset.getPorts() != null && subset.getPorts().size() > 0) // safeguard
 				.flatMap(subset -> {
-					Map<String, String> metadata = new HashMap<>(svcMetadata);
+					Map<String, String> metadata = new HashMap<>(serviceMetadata);
 					List<CoreV1EndpointPort> endpointPorts = subset.getPorts();
 					if (properties.metadata() != null && properties.metadata().addPorts()) {
 						endpointPorts.forEach(

spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesDiscoveryClientServiceMetadataTests.java

@@ -0,0 +1,233 @@
+/*
+ * 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.
+ * 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.client.discovery;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import io.kubernetes.client.openapi.models.V1Service;
+import io.kubernetes.client.openapi.models.V1ServiceSpec;
+import org.junit.jupiter.api.Assertions;
+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 org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
+
+/**
+ * @author wind57
+ */
+@ExtendWith(OutputCaptureExtension.class)
+class KubernetesDiscoveryClientServiceMetadataTests {
+
+	/**
+	 * <pre>
+	 *     - labels are not added
+	 *     - annotations are not added
+	 * </pre>
+	 */
+	@Test
+	void testServiceMetadataEmpty() {
+		boolean addLabels = false;
+		String labelsPrefix = "";
+		boolean addAnnotations = false;
+		String annotationsPrefix = "";
+		boolean addPorts = false;
+		String portsPrefix = "";
+
+		KubernetesDiscoveryProperties.Metadata metadata = new KubernetesDiscoveryProperties.Metadata(addLabels,
+				labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix);
+		KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
+				true, "", Set.of(), Map.of(), "", metadata, 0, false, false);
+		V1Service service = new V1Service().spec(new V1ServiceSpec().type("ClusterIP"))
+				.metadata(new V1ObjectMeta().namespace("default"));
+
+		Map<String, String> result = KubernetesDiscoveryClientUtils.serviceMetadata(properties, service, "my-service");
+		Assertions.assertEquals(result.size(), 2);
+		Assertions.assertEquals(result, Map.of("k8s_namespace", "default", "type", "ClusterIP"));
+	}
+
+	/**
+	 * <pre>
+	 *     - labels are added without a prefix
+	 *     - annotations are not added
+	 * </pre>
+	 */
+	@Test
+	void testServiceMetadataAddLabelsNoPrefix(CapturedOutput output) {
+		boolean addLabels = true;
+		String labelsPrefix = "";
+		boolean addAnnotations = false;
+		String annotationsPrefix = "";
+		boolean addPorts = false;
+		String portsPrefix = "";
+
+		KubernetesDiscoveryProperties.Metadata metadata = new KubernetesDiscoveryProperties.Metadata(addLabels,
+				labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix);
+		KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
+				true, "", Set.of(), Map.of(), "", metadata, 0, false, false);
+		V1Service service = new V1Service().spec(new V1ServiceSpec().type("ClusterIP"))
+				.metadata(new V1ObjectMeta().namespace("default").labels(Map.of("a", "b")));
+
+		Map<String, String> result = KubernetesDiscoveryClientUtils.serviceMetadata(properties, service, "my-service");
+		Assertions.assertEquals(result.size(), 3);
+		Assertions.assertEquals(result, Map.of("a", "b", "k8s_namespace", "default", "type", "ClusterIP"));
+		String labelsMetadata = filterOnK8sNamespaceAndType(result);
+		Assertions.assertTrue(
+				output.getOut().contains("Adding labels metadata: " + labelsMetadata + " for serviceId: my-service"));
+	}
+
+	/**
+	 * <pre>
+	 *     - labels are added with prefix
+	 *     - annotations are not added
+	 * </pre>
+	 */
+	@Test
+	void testServiceMetadataAddLabelsWithPrefix(CapturedOutput output) {
+		boolean addLabels = true;
+		String labelsPrefix = "prefix-";
+		boolean addAnnotations = false;
+		String annotationsPrefix = "";
+		boolean addPorts = false;
+		String portsPrefix = "";
+
+		KubernetesDiscoveryProperties.Metadata metadata = new KubernetesDiscoveryProperties.Metadata(addLabels,
+				labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix);
+		KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
+				true, "", Set.of(), Map.of(), "", metadata, 0, false, false);
+		V1Service service = new V1Service().spec(new V1ServiceSpec().type("ClusterIP"))
+				.metadata(new V1ObjectMeta().namespace("default").labels(Map.of("a", "b", "c", "d")));
+
+		Map<String, String> result = KubernetesDiscoveryClientUtils.serviceMetadata(properties, service, "my-service");
+		Assertions.assertEquals(result.size(), 4);
+		Assertions.assertEquals(result,
+				Map.of("prefix-a", "b", "prefix-c", "d", "k8s_namespace", "default", "type", "ClusterIP"));
+		// so that result is deterministic in assertion
+		String labelsMetadata = filterOnK8sNamespaceAndType(result);
+		Assertions.assertTrue(
+				output.getOut().contains("Adding labels metadata: " + labelsMetadata + " for serviceId: my-service"));
+	}
+
+	/**
+	 * <pre>
+	 *     - labels are not added
+	 *     - annotations are added without prefix
+	 * </pre>
+	 */
+	@Test
+	void testServiceMetadataAddAnnotationsNoPrefix(CapturedOutput output) {
+		boolean addLabels = false;
+		String labelsPrefix = "";
+		boolean addAnnotations = true;
+		String annotationsPrefix = "";
+		boolean addPorts = false;
+		String portsPrefix = "";
+
+		KubernetesDiscoveryProperties.Metadata metadata = new KubernetesDiscoveryProperties.Metadata(addLabels,
+				labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix);
+		KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
+				true, "", Set.of(), Map.of(), "", metadata, 0, false, false);
+		V1Service service = new V1Service().spec(new V1ServiceSpec().type("ClusterIP")).metadata(
+				new V1ObjectMeta().namespace("default").labels(Map.of("a", "b")).annotations(Map.of("aa", "bb")));
+
+		Map<String, String> result = KubernetesDiscoveryClientUtils.serviceMetadata(properties, service, "my-service");
+		Assertions.assertEquals(result.size(), 3);
+		Assertions.assertEquals(result, Map.of("aa", "bb", "k8s_namespace", "default", "type", "ClusterIP"));
+		Assertions
+				.assertTrue(output.getOut().contains("Adding annotations metadata: {aa=bb} for serviceId: my-service"));
+	}
+
+	/**
+	 * <pre>
+	 *     - labels are not added
+	 *     - annotations are added with prefix
+	 * </pre>
+	 */
+	@Test
+	void testServiceMetadataAddAnnotationsWithPrefix(CapturedOutput output) {
+		boolean addLabels = false;
+		String labelsPrefix = "";
+		boolean addAnnotations = true;
+		String annotationsPrefix = "prefix-";
+		boolean addPorts = false;
+		String portsPrefix = "";
+
+		KubernetesDiscoveryProperties.Metadata metadata = new KubernetesDiscoveryProperties.Metadata(addLabels,
+				labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix);
+		KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
+				true, "", Set.of(), Map.of(), "", metadata, 0, false, false);
+		V1Service service = new V1Service().spec(new V1ServiceSpec().type("ClusterIP")).metadata(new V1ObjectMeta()
+				.namespace("default").labels(Map.of("a", "b")).annotations(Map.of("aa", "bb", "cc", "dd")));
+
+		Map<String, String> result = KubernetesDiscoveryClientUtils.serviceMetadata(properties, service, "my-service");
+		Assertions.assertEquals(result.size(), 4);
+		Assertions.assertEquals(result,
+				Map.of("prefix-aa", "bb", "prefix-cc", "dd", "k8s_namespace", "default", "type", "ClusterIP"));
+		// so that result is deterministic in assertion
+		String annotations = filterOnK8sNamespaceAndType(result);
+		Assertions.assertTrue(
+				output.getOut().contains("Adding annotations metadata: " + annotations + " for serviceId: my-service"));
+	}
+
+	/**
+	 * <pre>
+	 *     - labels are added with prefix
+	 *     - annotations are added with prefix
+	 * </pre>
+	 */
+	@Test
+	void testServiceMetadataAddLabelsAndAnnotationsWithPrefix(CapturedOutput output) {
+		boolean addLabels = true;
+		String labelsPrefix = "label-";
+		boolean addAnnotations = true;
+		String annotationsPrefix = "annotation-";
+		boolean addPorts = false;
+		String portsPrefix = "";
+
+		KubernetesDiscoveryProperties.Metadata metadata = new KubernetesDiscoveryProperties.Metadata(addLabels,
+				labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix);
+		KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
+				true, "", Set.of(), Map.of(), "", metadata, 0, false, false);
+		V1Service service = new V1Service().spec(new V1ServiceSpec().type("ClusterIP")).metadata(new V1ObjectMeta()
+				.namespace("default").labels(Map.of("a", "b", "c", "d")).annotations(Map.of("aa", "bb", "cc", "dd")));
+
+		Map<String, String> result = KubernetesDiscoveryClientUtils.serviceMetadata(properties, service, "my-service");
+		Assertions.assertEquals(result.size(), 6);
+		Assertions.assertEquals(result, Map.of("annotation-aa", "bb", "annotation-cc", "dd", "label-a", "b", "label-c",
+				"d", "k8s_namespace", "default", "type", "ClusterIP"));
+		// so that result is deterministic in assertion
+		String labels = result.entrySet().stream().filter(en -> en.getKey().contains("label"))
+				.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)).toString();
+		String annotations = result.entrySet().stream().filter(en -> en.getKey().contains("annotation"))
+				.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)).toString();
+		Assertions.assertTrue(
+				output.getOut().contains("Adding labels metadata: " + labels + " for serviceId: my-service"));
+		Assertions.assertTrue(
+				output.getOut().contains("Adding annotations metadata: " + annotations + " for serviceId: my-service"));
+	}
+
+	private String filterOnK8sNamespaceAndType(Map<String, String> result) {
+		return result.entrySet().stream().filter(en -> !en.getKey().contains("k8s_namespace"))
+				.filter(en -> !en.getKey().equals("type"))
+				.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)).toString();
+	}
+
+}