adding --temp-path argument #4
Conversation
There was a problem hiding this comment.
Thank you for your contribution @louismariegaborit . Let improve the safety of that code a bit 😉 🙏
| end | ||
|
|
||
| # Runs everything | ||
| def self.run(options : Array(String)) | ||
| kubecontext = "_unset_" | ||
| spawn_shell = false | ||
|
|
||
| kubeconfigTemp = K8sVault::KUBECONFIG_TEMP; |
There was a problem hiding this comment.
Let's store it in config.kubeconfigTemp (record Config) instead 🙏 and use config.kubeconfigTemp in place of K8sVault::KUBECONFIG_TEMP in following code.
Deleting ENV["KUBECONFIG"] makes me uneasy 😅 especially, since the self.cleanup will fire before we pass the new KUBECONFIG to the Process.run on line 341
There was a problem hiding this comment.
Yes, I wasn't very convinced that using ENV var was a good idea.
But, how can I get the config value in the cleanup method ? It's the first time that I develop with Crystal lang.
There was a problem hiding this comment.
Could you advise me on retrieve config in the cleanup function ?
There was a problem hiding this comment.
@anapsix I tried to don't use ENV["KUBECONFIG"]. I chose to pass a param to cleanup function because config var might not be set.
Do you think this is correct ?
There was a problem hiding this comment.
Sorry Louis, kinda busy with work. Will poke at it next week.
There was a problem hiding this comment.
Do you think you will have time this week to read my PR ?
There was a problem hiding this comment.
kinda swamped at work at the moment, if your solution works for you, then that should hopefully serve the purpose while I get around to it
louismariegaborit
changed the title
louismariegaborit
force-pushed
the
add-temp-file-path-argument
branch
from
102060d to
43a3c41
Compare
Adding --path-temp argument to set a name for the generated kubeconfig file.