Added support of running Helmsman on Windows #287
Conversation
PeterBrun
force-pushed
the
windows-support
branch
3 times, most recently
from
13e27e6 to
fd6c50e
Compare
PeterBrun
force-pushed
the
windows-support
branch
from
fd6c50e to
6c0e8b6
Compare
PeterBrun
force-pushed
the
windows-support
branch
2 times, most recently
from
6d947e1 to
6d77ad7
Compare
|
@luisdavim any reason why commands used to be call with |
|
@PeterBrun I have highlighted details that seemed to me that would change the behavior but it might be OK. I just wanted to highlight them for you to make sure that everything is intended by this patch and not a typo or a bug in a search & replace. |
PeterBrun
force-pushed
the
windows-support
branch
from
6d77ad7 to
8b2d8de
Compare
|
@luisdavim I have mostly tested on Windows. Using it there with some clusters. I have only tested the Linux build using the provided unit tests (not with a real cluster). So I cannot guarantee all use cases. So please go ahead and do a test drive 🙂 |
|
@sami-alajrami this LGTM but I wonder if there was any particular reason to start the helm and kubectl commands in a sub-shell instead of calling them directly. |
|
@mkubaczyk do you know what a sub-shell is being used? |
|
Hi can you take a look at the conflicts? Thanks. |
No, I don't think there was a special reason. Unless if it was to get error codes back .. but my memory is vague on it |
|
I that case I think we can merge this as soon as the conflicts are addressed. Thanks |
Any execution of a command using bash with the command parameter has been replaced with just the expected command executable (helm and kubectl). And instead of building the arguments as a single string, they are now built using string arrays. This solves the "quoting" problem, that is there when using a single string with all arguments. For example, before this change, a path had to be quoted on Windows to work. By just building string arrays with all the arguments, this problem is solved by Go' os/exec package. It also solves the problem of using a password protected Helm repo, where the username contains a '$' (for example Harbor creates robot accounts with this). This could be escaped using '$$', but then it still was being interpreted by bash as an environment variable. This could have been solved by adding quotes around in the old solution, but it is just simpler to just use the "native" string array arguments approach as described above. And still if the environment variable was intended as input, then it is already applied by the "substituteEnv" functionality.
PeterBrun
force-pushed
the
windows-support
branch
from
683c58c to
4814020
Compare
Added support of running Helmsman on Windows
Any execution of a command using bash with the command parameter has been replaced with just the expected command executable (helm and kubectl). And instead of building the arguments as a single string, they are now built using string arrays. This solves the "quoting" problem, that is there when using a single string with all arguments. For example, before this change, a path had to be quoted on Windows to work. By just building string arrays with all the arguments, this problem is solved by Go' os/exec package.
It also solves the problem of using a password protected Helm repo, where the username contains a '$' (for example Harbor creates robot accounts with this). This could be escaped using '$$', but then it still was being interpreted by bash as an environment variable. This could have been solved by adding quotes around in the old solution, but it is just simpler to just use the "native" string array arguments approach as described above. And still if the environment variable was intended as input, then it is already applied by the "substituteEnv" functionality.