Get HomeDir from os/user.Current().HomeDir and fallback with $HOME

[HollowMan6]

Continuation of kubernetes/kubernetes#117165 as the conversation has been limited to collaborators.

Background

Since snap contaminates the $HOME out of their own considerations, software running under snap can't use the $HOME value to get the current user's real home directory. (Snap then introduced $SNAP_REAL_HOME to store the original $HOME value before their contamination, then I proposed that change to use $SNAP_REAL_HOME to get client-go working, but as you can see, that PR got strongly rejected)

Propose

Now I realize that os/user.Current().HomeDir actually returns the current user's real home directory even under the snap environment, so to fix this in a way that can never break anything normal, we simply rely on the os/user.Current().HomeDir to get the HomeDir since namely the client-fo function should have the same feature as the Go native one according to the comment (// HomeDir returns the home directory for the current user.). We will only fall back to $HOME blindly when there is something wrong with getting the current user (os/user.Current()) or the HomeDir value is empty.

I have created a patch for demoing my proposed change: https://github.com/kubescape/packaging/blob/main/snap_homedir.patch If you finally decide to accept this, I'm willing to realize it into a PR.

cc: @BenTheElder

[BenTheElder]

This is still a breaking change.

client-go is not the only client library in the project and clients are expected to have the same lookup behavior for KUBECONFIG and have for many years. even if client-go were the only client, skew between different versions is a problem.

should have the same feature as the Go native one according to the comment (// HomeDir returns the home directory for the current user.

That comment may be the intent, but the actual behavior is observable and well known and has additional implementations (many of which are not written in Go).

Changing behavior like this requires a KEP (Kubernetes Enhancement Proposal) to outline how we will safely roll out behavioral changes and formally have area owners accept a proposed change that is not a bug-fix.

As previously mentioned please see the existing discussion for XDG, which is at least a standard not specific to snap: kubernetes/enhancements#2111

Frankly, I would not recommend using a snap for kubectl. It's a single-file static go binary.

$HOME is in the POSIX portable operating systems standard that should be pointed at the user's home directory.
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html
Snap should stop violating POSIX.

Presumably snap packages could wrap kubectl with a shim that sets HOME=$SNAP_REAL_HOME without changing the client-go behavior.

[BenTheElder]

Also: Any disucussion here or in kubernetes/kubernetes doesn't change the fact that this would require a KEP similar to kubernetes/enhancements#2111, and that the owners of the client have previously rejected similar changes.

Even if I agreed that we should change the implementation, we could not do so without a formal accepted proposal like the one rejected in kubernetes/enhancements#2111.

As mentioned repeatedly, a detailed KEP could be considered, otherwise this behavior is not changing.

Until then I think you could just make the kubectl snap be:
kubectl:

#!/usr/bin/sh
HOME=$SNAP_REAL_HOME kubectl-real $@

kubectl-real: renamed kubectl binary.

[HollowMan6]

Although I'm still not happy, thankfully there's someone standing out explaining what it actually breaks finally. I admit that fixing the BUG itself is a breaking change, but I would still point out that it's not as easy as you mentioned to mitigate this issue.

Snap is designed to run in an isolated environment, which means they are not supposed to have any read or write access to any other programs' files (similar to the concept of the container but without cgroups). Since the majority of the programs just need to have their own config files and logs under $HOME, and nothing else, they choose to remap $HOME so that the majority of the programs can easily get packed as a snap without any code change. Now, things changed for a Kubernetes client, since reading the .kube config is violating their confinement policy (They can loose that policy by making a classic confinement request of course), then the only blocker is to find where the original $HOME actually locates.

As previously mentioned please see the existing discussion for XDG, which is at least a standard not specific to snap: kubernetes/enhancements#2111

I have already checked that PR a long time ago when you mention it there kubernetes/kubernetes#117165 (comment), and it has a completely different situation than the one I'm proposing. There it says: The goal is to make kubectl follow the XDG Spec and automatically migrate the configuration from $HOME/.kube to $HOME/.config/kube without stopping kubectl to work, and Deprecate any file under $HOME/.kube/ I do not have such kind of a goal for migrating any configuration files, I just want to fix the client-go's behavior only inside a snap package by using $SNAP_REAL_HOME when others use that library to build their own Kubernetes tools, so the thing you mentioned is completely off-topic, as well as everything you mentioned here:

Frankly, I would not recommend using a snap for kubectl. It's a single-file static go binary.

Presumably snap packages could wrap kubectl with a shim that sets HOME=$SNAP_REAL_HOME without changing the client-go behavior.

Until then I think you could just make the kubectl snap be: kubectl:

#!/usr/bin/sh
HOME=$SNAP_REAL_HOME kubectl-real $@

kubectl-real: renamed kubectl binary.

I'm in charge of packaging kubescape/kubescape as a snap package, and our own kubescape/k8s-interface depends on kubernetes-sigs/controller-runtime to get kubenetes config files. So I first went there and proposed the change kubernetes-sigs/controller-runtime#2266, but got rejected since the maintainer said "It doesn't make sense". While controller-runtime depends on the client-go, I go up here and propose the change again.

So as you can see, we don't rely on kubectl binaries but the client-go library. In addition, wrapping kubescape with a shim that sets HOME=$SNAP_REAL_HOME will not work as kubescape have its own config files to get stored in the snap remapped $HOME. Luckily, Golang is not Rust, and it still allows patching the vendors, so now the only way to fix this is to patch the code manually when compiled with go vendor mode.

Considering the words here kubernetes/kubernetes#117165 (comment) If some massive body of users chimes in saying that this is a pain point for them, I'll be happy to re-evaluate., I won't waste my time filing a KEP as suggested since it will eventually get rejected. If someone finds the issue and is interested, feel free to do so.

Also, if you think bugs can also be a feature and own style of behavior, and we shall fix no bugs and make no improvements to introduce no breaks at all, then I have nothing to say. I won't waste time looking into any possible solutions for you related to this anymore, as long as you think that fixing this bug is a breaking change.

[dims]

I won't waste time looking into any possible solutions for you related to this anymore, as long as you think that fixing this bug is a breaking change.

thank you!