Skip to content

Commit d520549

Browse files
authored
incorrect services returned due to wrong label filtering (#1388)
1 parent 9b8872b commit d520549

File tree

3 files changed

+52
-7
lines changed

3 files changed

+52
-7
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ static boolean matchesServiceLabels(V1Service service, KubernetesDiscoveryProper
7575
return false;
7676
}
7777

78-
LOG.debug(() -> "Service labels from properties : " + properties.serviceLabels());
79-
LOG.debug(() -> "Service labels from service : " + service.getMetadata().getLabels());
78+
LOG.debug(() -> "Service labels from properties : " + propertiesServiceLabels);
79+
LOG.debug(() -> "Service labels from service : " + serviceLabels);
8080

81-
return serviceLabels.keySet().containsAll(propertiesServiceLabels.keySet());
81+
return serviceLabels.entrySet().containsAll(propertiesServiceLabels.entrySet());
8282

8383
}
8484

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,7 @@ public List<ServiceInstance> getInstances(String serviceId) {
132132

133133
List<V1Service> services = serviceListers.stream().flatMap(x -> x.list().stream())
134134
.filter(scv -> scv.getMetadata() != null).filter(svc -> serviceId.equals(svc.getMetadata().getName()))
135-
.filter(filter).toList();
136-
if (services.size() == 0 || services.stream().noneMatch(service -> matchesServiceLabels(service, properties))) {
137-
return List.of();
138-
}
135+
.filter(scv -> matchesServiceLabels(scv, properties)).filter(filter).toList();
139136
return services.stream().flatMap(service -> getServiceInstanceDetails(service, serviceId)).toList();
140137
}
141138

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,54 @@ void testOneServiceMatchesFilter() {
448448

449449
}
450450

451+
@Test
452+
void testServicesWithDifferentMetadataLabels() {
453+
V1Service serviceA = service("serviceX", "namespaceA", Map.of("shape", "round"));
454+
V1Service serviceB = service("serviceX", "namespaceB", Map.of("shape", "triangle"));
455+
456+
V1Endpoints endpointsA = endpointsReadyAddress("serviceX", "namespaceA");
457+
V1Endpoints endpointsB = endpointsReadyAddress("serviceX", "namespaceB");
458+
459+
Lister<V1Service> serviceLister = setupServiceLister(serviceA, serviceB);
460+
Lister<V1Endpoints> endpointsLister = setupEndpointsLister(endpointsA, endpointsB);
461+
462+
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(false, true, Set.of(), true, 60L,
463+
false, null, Set.of(), Map.of("shape", "round"), null, KubernetesDiscoveryProperties.Metadata.DEFAULT,
464+
0, false);
465+
466+
KubernetesInformerDiscoveryClient discoveryClient = new KubernetesInformerDiscoveryClient(
467+
SHARED_INFORMER_FACTORY, serviceLister, endpointsLister, null, null, properties);
468+
469+
List<ServiceInstance> serviceInstances = discoveryClient.getInstances("serviceX");
470+
assertThat(serviceInstances.size()).isEqualTo(1);
471+
assertThat(serviceInstances.get(0).getMetadata().get("k8s_namespace")).isEqualTo("namespaceA");
472+
}
473+
474+
@Test
475+
void testServicesWithSameMetadataLabels() {
476+
V1Service serviceA = service("serviceX", "namespaceA", Map.of("shape", "round"));
477+
V1Service serviceB = service("serviceX", "namespaceB", Map.of("shape", "round"));
478+
479+
V1Endpoints endpointsA = endpointsReadyAddress("serviceX", "namespaceA");
480+
V1Endpoints endpointsB = endpointsReadyAddress("serviceX", "namespaceB");
481+
482+
Lister<V1Service> serviceLister = setupServiceLister(serviceA, serviceB);
483+
Lister<V1Endpoints> endpointsLister = setupEndpointsLister(endpointsA, endpointsB);
484+
485+
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(false, true, Set.of(), true, 60L,
486+
false, null, Set.of(), Map.of("shape", "round"), null, KubernetesDiscoveryProperties.Metadata.DEFAULT,
487+
0, false);
488+
489+
KubernetesInformerDiscoveryClient discoveryClient = new KubernetesInformerDiscoveryClient(
490+
SHARED_INFORMER_FACTORY, serviceLister, endpointsLister, null, null, properties);
491+
492+
List<ServiceInstance> serviceInstances = discoveryClient.getInstances("serviceX").stream()
493+
.sorted(Comparator.comparing(x -> x.getMetadata().get("k8s_namespace"))).toList();
494+
assertThat(serviceInstances.size()).isEqualTo(2);
495+
assertThat(serviceInstances.get(0).getMetadata().get("k8s_namespace")).isEqualTo("namespaceA");
496+
assertThat(serviceInstances.get(1).getMetadata().get("k8s_namespace")).isEqualTo("namespaceB");
497+
}
498+
451499
private Lister<V1Service> setupServiceLister(V1Service... services) {
452500
Cache<V1Service> serviceCache = new Cache<>();
453501
Lister<V1Service> serviceLister = new Lister<>(serviceCache);

0 commit comments

Comments
 (0)