Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Namespace filter for service discovery #1000

Closed
vchegwidden opened this issue Apr 4, 2022 · 9 comments · Fixed by #1113
Closed

Namespace filter for service discovery #1000

vchegwidden opened this issue Apr 4, 2022 · 9 comments · Fixed by #1113

Comments

@vchegwidden
Copy link

This request is to add the ability to filter/limit namespaces to use for service discovery in the DiscoveryClient through a property

When working as a tenant on a shared cluster we don't have permissions to view resources in all namespaces. We also don't have permissions to create cluster roles and cluster rolebindings.
In these situations you can't use the spring.cloud.kubernetes.discovery.all-namespaces=true property since it will throw an exception when trying to get resources in namespaces you don't have access to.
Not having the option to list namespaces to take into consideration means you would be limited to a single namespace for all your deployments, which isn't great...

I've been trying to run Spring boot admin in a cluster where my access is limited to only the namespaces I own. When trying to use the DiscoveryClient and setting all-namespaces=true the following exception is thrown since the service account doesn't have the correct permissions

"stack_trace":"i.f.k.c.KubernetesClientException: Failure executing: GET at: https://<nodeIP>/api/v1/namespaces/some-namespace/services. 
    Message: Forbidden!Configured service account doesn't have access. Service account may have been revoked. services is forbidden: 
    User \"system:serviceaccount:sba:spring-boot-admin\" cannot list resource \"services\" in API group \"\" in the namespace \"some-namespace\".

The work-around I've used is to implement my own DiscoveryClient that takes a list of namespaces from a property, then overriding the behaviour of getServices and getEndpointList to only look in the namespaces supplied (Note I've only tested this with the fabric8 client)

It would however be great if this was included out of the box.

@Value("#{'${spring.cloud.kubernetes.discovery.filter-namespaces:}'.split(',')}")
private List<String> filteredNamespaces;

@Override
public List<String> getServices(Predicate<Service> filter) {
    if (filteredNamespaces.isEmpty()) {
        return super.getServices(filter);
    }

    List<String> services = new ArrayList<>();
    for (String ns : filteredNamespaces) {
        services.addAll(
            this.client.services()
                .inNamespace(ns).list().getItems().stream().filter(filter).map(s -> s.getMetadata().getName().toList());
    }
}

@Override
public List<Endpoints> getEndPointsList(String serviceId) {
    if (filteredNamespaces.isEmpty()) {
        return super.getEndpointList(serviceId);
    }

    List<Endpoints> endpoints = new ArrayList<>();
    for (String ns : filteredNamespaces) {
        endpoints.addAll(
            this.client.endpoints()
                .inNamespace(ns).withField("metadata.name", seviceId).withLabels(properties.getServiceLabels().list().getItems());
    }
}

@wind57
Copy link
Contributor

wind57 commented Jul 8, 2022

this is for sure doable, and it is on my radar, just dropping a note here.

@mehdikhelif
Copy link

That would be really a nice improvement

mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 16, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 16, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 17, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 17, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 17, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 17, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 18, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 19, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 19, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 19, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 19, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 19, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 19, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 19, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 19, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 20, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 20, 2022
mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Oct 27, 2022
@bj-1795
Copy link

bj-1795 commented Nov 1, 2022

hi @mbialkowski1 is there any progress on this one?

mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Nov 13, 2022
@vbose
Copy link

vbose commented Nov 14, 2022

We badly need this feature. Can someone provide an update on this issue? We are still in Java 8 and we would really appreciate it if the fix is backward compatible with Java 8.

@wind57
Copy link
Contributor

wind57 commented Nov 14, 2022

The PR is in review, I assume a few weeks and it will be part of main. No gurantees whatsoever. The thing is, its going to be part of main branch, jdk-17 only.

@vbose
Copy link

vbose commented Nov 14, 2022

Also, spring.cloud.kubernetes.discover.filter: "metadata.labels['spring-boot']" does not have any effect. It discovers all services regardless. Any workaround for this issue as well is available? We use spring-cloud-starter-kubernetes-fabric8 dependency in the pom.

@wind57
Copy link
Contributor

wind57 commented Nov 14, 2022

Looks like a diff issue, please raise a separate issue. Thank you.

mbialkowski1 added a commit to mbialkowski1/spring-cloud-kubernetes that referenced this issue Nov 24, 2022
@ryanjbaxter ryanjbaxter linked a pull request Dec 1, 2022 that will close this issue
@ryanjbaxter ryanjbaxter added this to the 3.0.0-RC3 milestone Dec 1, 2022
@luisalves00
Copy link

luisalves00 commented Dec 20, 2022

Also, spring.cloud.kubernetes.discover.filter: "metadata.labels['spring-boot']" does not have any effect. It discovers all services regardless. Any workaround for this issue as well is available? We use spring-cloud-starter-kubernetes-fabric8 dependency in the pom.

This seems to ("almost") work:

spring:
    cloud:
        kubernetes:
            discovery:
                all-namespaces: true
                filter: "#root.metadata.namespace matches '^uat.*$'"

On the health endpoint (show-details: always) it seems to work as there the filter seems to be applied, but my openfeign client still finds services that are not in the list and I cannot figure out why.

Any ideas why openfeign might ignore the filter?

For the ones that are not using fabric8 this might help: 990

@luisalves00
Copy link

Opened a new ticket as this one is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants