Ramilito / kubesess

Kubectl plugin managing sessions
MIT License
205 stars 5 forks source link

Do not expect KUBECONFIG to be always set #42

Closed morozov closed 1 year ago

morozov commented 1 year ago

Fixes #40.

Ramilito commented 1 year ago

I'm planning on doing something similar later in main.rs but this fixes the immediate issue, nice work!

    static ref KUBECONFIG: String = {
        match env::var("KUBECONFIG") {
            Ok(val) => val,
            Err(_e) => format!("{}/.kube/config", dirs::home_dir().unwrap().display()),
        }
    };

And then everywhere we reference this value we would use the global KUBECONFIG variable

morozov commented 1 year ago

Another issue I stumbled upon is that this call: https://github.com/Ramilito/kubesess/blob/75f073108c6b6d00e4afd8026714500886601225/src/config.rs#L95

Can panic if the open() fails (e.g. because the file doesn't exist or it's not readable by the current user). This is way less likely than the issue in question but it may still make sense to handle it properly.

Ramilito commented 1 year ago

hmm good point, what would you suggest we do there, create the file?

morozov commented 1 year ago

I would just exit with a meaningful error message. Ideally, the one returned by the operating system. E.g. "No such file or directory":

$ cat non-existing-file.txt
cat: non-existing-file.txt: No such file or directory

The shell wrappers should properly interpret the non-zero exit code and passthrough STDERR to the terminal.

Ramilito commented 1 year ago

That's a good suggestion, I created an issue for it!