choosing a namespace in a consistent fashion across fabric8 and k8s-native clients

[wind57]

We can set the namespace property for a certain secret/config map at this level:

spring.cloud.kubernetes.secrets.namespace or spring.cloud.kubernetes.secrets.sources[].namespace. This is how a NormalizedSource::namespace is populated (same logic is applied for config maps) This is easy.

Suppose that I do not set it in neither of the above.

In such a case, we will reach KubernetesClientSecretsPropertySourceLocator.

Which says that if normalized source is not set, we will use fallbackNamespace and this is where I would like to look more closely. This property is :

String fallbackNamespace = kubernetesNamespaceProvider != null ? kubernetesNamespaceProvider.getNamespace()
				: kubernetesClientProperties.getNamespace();

It is understandable why we use both kubernetesNamespaceProvider and kubernetesClientProperties to find the namespace. There are two constructors in KubernetesClientSecretsPropertySourceLocator, and one takes a property or the other.

Now the first (1) nitpick is that the constructor that takes a KubernetesClientProperties as input is not used by us - so let's deprecate it (I would very much rather prefer we delete it). Moreover internally it uses spring.cloud.kubernetes.client.namespace property, one that is documented in general, but it is not documented in our project. That is a nitpick/question (2).

And a final (3) question here is : fabric8 client does not read such a property at all (spring.cloud.kubernetes.client.namespace), so this seems a bit odd as there is a discrepancy between the two clients that we use. This is either a bug in fabric8 implementation or it needs to be clearly stipulated in the documentation.

The other constructor that takes KubernetesNamespaceProvider as input, we do use it.

Now let's get back to that :

String fallbackNamespace = kubernetesNamespaceProvider != null ? kubernetesNamespaceProvider.getNamespace()
				: kubernetesClientProperties.getNamespace();

So if provider is present, let's read the namespace from :

• spring.cloud.kubernetes.client.namespace, if not present:
• spring.cloud.kubernetes.client.serviceAccountNamespacePath, if not present:
• /var/run/secrets/kubernetes.io/serviceaccount/namespace

So we say that if it is not present in spring.cloud.kubernetes.client.namespace, let's do two more checks for a path. But, we do not do that when provider is null, and we defer to kubernetesClientProperties. So in one case, we check also paths, but not in the other. Also nothing close to this is done in fabric8 client.

---

Now, what are we going to do to properly solve this? I have been thinking about this for quite some days, and this is the PR that I propose: #860

The overall idea is this:

• clearly deprecate the constructors that we do not use internally and only exist for backward compatibility. Document the limitation that they might impose and generally "discourage" their usage.
• make fabric8 and k8s clients work in the same exact manner when searching for a namespace. The order is explained above.
• k8s-client does not need structural changes, fabric8 does. As such this is a breaking change - in the sense that if someone relies on beans of the locator of fabric8 (or those classes, etc), their code will break. The chances are very slim and we can document that in the upgrade of the next version clearly.
• throw an Exception when we can't find a namespace in the cases above (this is not done in the PR that I have mentioned), as it needs a confirmation here/talk here first
• add proper notes on how a namespace is chosen in the documentation. Again, not done, as I would like to get some input first.

[wind57]

@ryanjbaxter not going to nag on this one, just a friendly reminder that I am still "active" and waiting on these for further talk/resolution. no rush what-so-ever, thank you.

[ryanjbaxter]

I merged #860 let me know if there are other PRs related to this I need to look at.

[wind57]

nice. thank you for merging and thus agreeing on the proposal. the nitpick is that the PR was more of a proposal PR, it needs more things added, mainly documentation and some tests. A PR is coming soon

[wind57]

@ryanjbaxter the final PR that should close this namespace subject. #865

[wind57]

this can be closed as fixed.