feat: plugin.yaml support platformHooks (closes #7117)

[dimabru]

What this PR does / why we need it:
This pr adds platformHook behavior for plugin installations. This is mainly to support plugin installation for windows, since at this point, helm plugins are not able to run on windows.
Usage is very similar to current platformCommand. The user can specify specific os to run hooks on, and if missing, default to root hooks key in plugin yaml

closes #7117

Special notes for your reviewer:
For env vars, there was an issue with using built-in os.ExpandEnv function on windows.
Hence, there's no support for $var in windows scripts. What is working though is windows env var syntax %VAR% - mainly used for %HELM_PLUGIN_DIR%

If applicable:

• this PR contains documentation
• this PR contains unit tests
• this PR has been tested for backwards compatibility

[dimabru]

closes #7117

[dimabru]

Hi @bacongobbler, any chance you can have a look at the code?
Thanks a lot :)

[TBBle]

With a quick look, this also changes the behaviour of hooks, which I don't think is desirable, as it will break existing working setups, e.g. running helm plugin install under MSYS2 which is the work-around I've used for #7117.

since at this point, helm plugins are not able to run on windows.

Helm plugins run on Windows, as long as their installer bash script recognises Windows and installs a Windows binary as, e.g., https://github.com/helm/helm-2to3/blob/main/scripts/install_plugin.sh, does.

Also, it continues to hard-code a per-platform shell. At least, by my suggestion in #7117, platformHooks would be up to the platform hook itself to specify sh -c or cmd /c or whatever the appropriate shell is, e.g. on Windows, PowerShell would be a reasonable choice for hook implementation. This would mean that the existing hooks are equivalent to a "default" platformHook of sh -c <existing content> which is reasonably non-surprising.

I'm not sure if I'd group the platform hooks by OS as done here, or by event as the current hooks are. That's assuming I'm reading this correctly: lack of tests means I have to imagine what the config file looks like with these changes.

It might even be handy to prepare a draft PR against a plugin or two (that already support WIndows) to see how your changes look in the real world, or ask the maintainers of those plugins to do so. That would be important ecosystem feedback anyway.

[dimabru]

Hi @TBBle
Thanks a lot for the detailed comment.

In your suggestion in #7117 (comment) you suggest to deprecate hooks and keep only the new platformHooks behavior. Why is that? Why not keep hooks and use sh -c as fallback for backwards compatibility? Also, I think its not far-fetched to assume that helm defaults to shell executable in case platformHook is missing.

I think I understand what you mean when you say this change will break existing behavior. I will change the script to not assume exec script by checking os, but rather add an option for the user to explicitly declare the preffered way of execution.

As you suggested, I will add unit-tests and a draft pr in another plugin to give a clear example on how this behavior might look.

[TBBle]

Deprecating hooks would in-effect be keeping it as backwards-compat behaviour: Existing plugins would install exactly as they do now, including assuming sh -c and hence not working on Windows outside a specific user setup. That's also the reason to deprecate it, because it assumes something that isn't a great assumption across platforms, so people shouldn't be writing new install hooks using it. If they mean to only support Linux, they can do that explicitly in platformHooks (and then Helm would be able to detect that and refuse to install the plugin, rather than failing mysteriously as it does now).

Basically, I was mirroring the behaviour of platformCommand, and that does not prepend a shell in the command. I think having platformHooks act differently to platformCommand in this regard would be a surprising mistake.