k8s-client might read an incorrect secret from a different namespace

[wind57]

This is more of a question, then a bug; but it's a bit of both...

I am looking at this line. Notice the logic here:

• if there is a namespace defined, use it via listNamespacedSecret.

• if there is not, read all namespaces via listSecretForAllNamespaces.

This is an interesting choice. We can obviously read the wrong thing easily and expose data from a secret that users never intended for (this is also easily reproducible).

On the other hand, if we take a step back, we only call listSecretForAllNamespaces because we think the namespace might be null. The future of this bug and discussion should really happen after this one.

For the time being, I am leaving it here as a place-holder and will get back to it.

[ryanjbaxter]

I think the reason why I wrote it that way was because if we didn't know the namespace we were out of luck. I suppose we could throw and error at that point. You are right it could mean we expose something we shouldn't be.

[wind57]

You're right and I totally understand that, but it's tricky. Let's take each at a time.

• For the fabric8 client, we think we have a namespace, always, because we default as a last resort to KubernetesClient::getNamespace and hey, the client is really suppose to know where it runs, right? Well, not really.

Basically fabric8 says : I am going to try and read a few properties and env variables, if not set, I will try to read /var/run/secrets/kubernetes.io/serviceaccount/namespace. Not present? Oh well...

I should have raised a defect in the fabric8 implementation too and at least ask for proper documentation that clearly stipulates that it might not get a namespace or will it. I will do that, now that I think about it.

• k8s-client does not have such a method to find out the namespace from the current client. I have asked about this, no response yet. Judging by the python client issue, I am 99% of the answer I am going to get.

So neither of the clients is really safe, it's just that for the fabric8 because it has dedicated method, we trusted it. And it seems over the years no one complained. As said in that issue/discussion I have no idea how it would be possible to have a pod without that path. And if we ever get a bug report around that - sure, let's analyze how that is possible, first.

As such, if we agree on this, neither the k8s-client is going to have that chance to get a null namespace; because we query /var/run/secrets/kubernetes.io/serviceaccount/namespace via KubernetesNamespaceProvider anyway.

I hope this makes sense.

[ryanjbaxter]

Yeah that is part of the reason why I created KubernetesNamespaceProvider.

I agree in a Pod we should never run into the case where we can't get the namespace, but we should probably account for that condition and handle it appropriately here.

[wind57]

I see, most probably throwing an Exception would be the most appropriate thing to do. I could easily fix this, as I am already working on some re-factoring simplifications, but before that I need also your input on the other issue.

[yue9944882]

I agree in a Pod we should never run into the case where we can't get the namespace, but we should probably account for that condition and handle it appropriately here.

just for clarification, mounting service-account is almost mandatory in a kubernetes cluster for a pod, while it can still opt-out from mounting service-account in a few cases:

• static pod wont mount service-account
• kube-apiserver can explicitly disable the ServiceAccount admission controller then mounting service-account will be optional for a pod
• the service is deployed out of a kubernetes cluster.