Fix wrong labels and annotations in kubernetes discovery native client #1282
Conversation
Configure Renovate
| @@ -112,27 +113,7 @@ public List<ServiceInstance> getInstances(String serviceId) { | |||
| } | |||
|
|
|||
| private Stream<ServiceInstance> getServiceInstanceDetails(V1Service service, String serviceId) { | |||
| Map<String, String> svcMetadata = new HashMap<>(); | |||
There was a problem hiding this comment.
this PR fixes an issue, imo. if you look at this piece of code:
service.getMetadata().getAnnotations().entrySet().stream()
.filter(e -> e.getKey().startsWith(annotationPrefix))
.forEach(e -> svcMetadata.put(e.getKey(), e.getValue()));
it says that we are going to take only annotations that start with annotationPrefix. In the fabric8 implementation we do it differently: we prefix existing values with it. As a matter of fact we do this in configmaps and secrets property sources also.
As such, I removed this code and fixed the bug in the utils class, where the code is the same as in the fabric8 implementation. Also added tests.
| serviceMetadata.putAll(annotationMetadata); | ||
| } | ||
|
|
||
| serviceMetadata.put(NAMESPACE_METADATA_KEY, |
There was a problem hiding this comment.
metadata now includes two more fields, just like we do in the fabric8 implementation.
wind57
changed the title
| Assertions.assertEquals(serviceInstance.getServiceId(), "spring-cloud-kubernetes-client-discovery-it"); | ||
| Assertions.assertNotNull(serviceInstance.getHost()); | ||
| Assertions.assertEquals(serviceInstance.getMetadata(), | ||
| Map.of("http", "8080", "k8s_namespace", "default", "type", "ClusterIP", |
There was a problem hiding this comment.
an integration test that asserts for proper labels and annotations with prefix
|
@ryanjbaxter this one can be looked at too. I started to fix some issues as I dive into the implementation and have fabric8 context in my head |
No description provided.