From d52054988476dc68f41fd72fccd54f058d4c70da Mon Sep 17 00:00:00 2001 From: erabii Date: Tue, 25 Jul 2023 02:51:00 +0300 Subject: [PATCH] incorrect services returned due to wrong label filtering (#1388) --- .../KubernetesDiscoveryClientUtils.java | 6 +-- .../KubernetesInformerDiscoveryClient.java | 5 +- ...ubernetesInformerDiscoveryClientTests.java | 48 +++++++++++++++++++ 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesDiscoveryClientUtils.java b/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesDiscoveryClientUtils.java index 9b50ebb2..9e0a88ae 100644 --- a/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesDiscoveryClientUtils.java +++ b/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesDiscoveryClientUtils.java @@ -75,10 +75,10 @@ final class KubernetesDiscoveryClientUtils { return false; } - LOG.debug(() -> "Service labels from properties : " + properties.serviceLabels()); - LOG.debug(() -> "Service labels from service : " + service.getMetadata().getLabels()); + LOG.debug(() -> "Service labels from properties : " + propertiesServiceLabels); + LOG.debug(() -> "Service labels from service : " + serviceLabels); - return serviceLabels.keySet().containsAll(propertiesServiceLabels.keySet()); + return serviceLabels.entrySet().containsAll(propertiesServiceLabels.entrySet()); } diff --git a/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClient.java b/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClient.java index bcc069d8..572ecd35 100644 --- a/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClient.java +++ b/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClient.java @@ -132,10 +132,7 @@ public class KubernetesInformerDiscoveryClient implements DiscoveryClient { List services = serviceListers.stream().flatMap(x -> x.list().stream()) .filter(scv -> scv.getMetadata() != null).filter(svc -> serviceId.equals(svc.getMetadata().getName())) - .filter(filter).toList(); - if (services.size() == 0 || services.stream().noneMatch(service -> matchesServiceLabels(service, properties))) { - return List.of(); - } + .filter(scv -> matchesServiceLabels(scv, properties)).filter(filter).toList(); return services.stream().flatMap(service -> getServiceInstanceDetails(service, serviceId)).toList(); } diff --git a/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClientTests.java b/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClientTests.java index 6042dc89..626009fa 100644 --- a/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClientTests.java +++ b/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/KubernetesInformerDiscoveryClientTests.java @@ -448,6 +448,54 @@ class KubernetesInformerDiscoveryClientTests { } + @Test + void testServicesWithDifferentMetadataLabels() { + V1Service serviceA = service("serviceX", "namespaceA", Map.of("shape", "round")); + V1Service serviceB = service("serviceX", "namespaceB", Map.of("shape", "triangle")); + + V1Endpoints endpointsA = endpointsReadyAddress("serviceX", "namespaceA"); + V1Endpoints endpointsB = endpointsReadyAddress("serviceX", "namespaceB"); + + Lister serviceLister = setupServiceLister(serviceA, serviceB); + Lister endpointsLister = setupEndpointsLister(endpointsA, endpointsB); + + KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(false, true, Set.of(), true, 60L, + false, null, Set.of(), Map.of("shape", "round"), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, + 0, false); + + KubernetesInformerDiscoveryClient discoveryClient = new KubernetesInformerDiscoveryClient( + SHARED_INFORMER_FACTORY, serviceLister, endpointsLister, null, null, properties); + + List serviceInstances = discoveryClient.getInstances("serviceX"); + assertThat(serviceInstances.size()).isEqualTo(1); + assertThat(serviceInstances.get(0).getMetadata().get("k8s_namespace")).isEqualTo("namespaceA"); + } + + @Test + void testServicesWithSameMetadataLabels() { + V1Service serviceA = service("serviceX", "namespaceA", Map.of("shape", "round")); + V1Service serviceB = service("serviceX", "namespaceB", Map.of("shape", "round")); + + V1Endpoints endpointsA = endpointsReadyAddress("serviceX", "namespaceA"); + V1Endpoints endpointsB = endpointsReadyAddress("serviceX", "namespaceB"); + + Lister serviceLister = setupServiceLister(serviceA, serviceB); + Lister endpointsLister = setupEndpointsLister(endpointsA, endpointsB); + + KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(false, true, Set.of(), true, 60L, + false, null, Set.of(), Map.of("shape", "round"), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, + 0, false); + + KubernetesInformerDiscoveryClient discoveryClient = new KubernetesInformerDiscoveryClient( + SHARED_INFORMER_FACTORY, serviceLister, endpointsLister, null, null, properties); + + List serviceInstances = discoveryClient.getInstances("serviceX").stream() + .sorted(Comparator.comparing(x -> x.getMetadata().get("k8s_namespace"))).toList(); + assertThat(serviceInstances.size()).isEqualTo(2); + assertThat(serviceInstances.get(0).getMetadata().get("k8s_namespace")).isEqualTo("namespaceA"); + assertThat(serviceInstances.get(1).getMetadata().get("k8s_namespace")).isEqualTo("namespaceB"); + } + private Lister setupServiceLister(V1Service... services) { Cache serviceCache = new Cache<>(); Lister serviceLister = new Lister<>(serviceCache);