Closed ghost closed 6 years ago
nicklan
commented
6 years ago Thanks much for the PR! Would it be possible to not have all the changes from rustfmt? It makes it really hard to figure out what's new and what's just format change. I'll make another PR after I get this merged to format everything so this isn't a problem in the future.
I also agree on the lack of testing. It would be good to get some unit/integration tests and I'd welcome PRs to that effect.
ghost
commented
6 years ago @nicklan Good to hear I am getting a nudge in the right direction - running rustfmt afterwards certainly is a better idea! The latest commit should be what you need to be able to merge this one. Next I would be looking into allow non-certificate authentication modes - we have plenty of credentials based authentication here, which makes it impossible to use click at this time.
Do you think it's something one could contribute relatively swiftly? Or does it come with major architectural changes? Maybe the best thing would be to wait, given you were looking into using a k8s client library anyway.
ghost
commented
6 years ago Oh, I just checked the code to see that credentials based authentication is already implemented. However, I get [Warning] Couldn't find/load context <context-name>, now no current context. Error: Kube Error: Invalid Cluster Name . Even though the configuration is perfectly valid.
ghost
commented
6 years ago I found the culprit, and put the fix in another commit. Would be great if that could land, as I would think that doesn't break anything else. However, without tests anything goes, really :D.
nicklan
commented
6 years ago Awesome, looks good thanks!
kubectl uses this environment variable to find its configuration file, but uses the one in the default ($HOME) location otherwise.
Click now does the same, which makes it comply with what people would expect.
The reason I am making this PR is that we do have a tool which configures K8s on the fly, and produces a k8s configuration in a temporary location.
Please let me know what should be changed I might push this one over the finishing line :)
On another note: it appears there are not tests at all, and I think this will eventually be a problem for quality and/or longevity. There I would offer some help if desired.