Improve salt.utils.which to better support windows

[twangboy]

salt.utils.which only searches directories defined in the "PATH" environment variable which is very specific to DOS. Many required .exe's in windows reside in Program Files and Program Files (x86).

Possible solutions: 1 - Add a recursive function that will add every directory under Program Files and Program Files (x86) to the search 2 - Add an option to pass a specific location to search

This will fix a current bug where the gpg data in the pillar can't be decoded because the module can't find the local installation of gpg because it resides in the Program Files directory.

[twangboy]

@UtahDave @jfindlay

[rickh563]

ZD-271

[lorengordon]

Doesn't which on Linux also search only directories in the 'PATH'? I'd prefer option 2, but perhaps the option should be to add the specified location to the 'PATH' for the purposes of calling the function? That would preserve the semantics across platforms.

[twangboy]

Yes, but Linux hasn't deviated much in where it puts things like windows has since DOS with the Program Files directories. Option 2 may be the best, but windows gives the option of choosing an install location in many instances. Paths may change as well between versions and architectures, in which case Option 1 might be more likely to find the exe. Option 2 would be the easiest to implement and the least resource intensive though. I'd like to see how slow Option 1 is.

[lorengordon]

I'm not really following the DOS reference. All modern versions of Windows continue use the PATH variable exactly the way it is used on Linux. If an application should be available using just the executable filename, then the application should add itself to the PATH (or the user should do it themselves).

Also, Option 1 doesn't really fix the underlying concern. As you note, a user can choose to install an application anywhere, not just in the Program Files directory (and really, this goes for Linux, too).

[jfindlay]

This is a complex issue. I think we could support both mechanisms:

def which(exec, additional_paths=None, search_program_files=True):

or similar. Considering the variability challenges this must deal with, I think a pragmatic approach might be the best even if it is less than ideal. If we search where installers/users most typically put executable files, assuming that that can be programmatically targeted in a concise manner, then I think we will have covered most of the use cases.

As an easy zeroth order optimization, we could even only enable full searches of Program Files* if the executable isn't found by other means.

[twangboy]

After speaking with @thatch45 about this issue we're probably going to leave salt.utils.which the way it is, only searching locations designated in %PATH%. If your exe isn't in %PATH % you'll have to add the location to %PATH% either on the machine itself or from the master using cmd.run. @lorengordon is right in that it's impossible to account for every possibility. We're opting for the simplest solution.

That being said, this still doesn't fix the problem that started this issue. The gpg.py module seems to be written with linux in mind and doesn't work on windows even with the right directory in the %PATH%. We're going to need to make gpg.py work with windows or create a win_gpg.py. I'll talk to @UtahDave about what he thinks would be best.

As far as the location of the gpg.exe, according to (https://pythonhosted.org/python-gnupg/) we may be able to just distribute the .exe and associated .dll with salt and then not have to worry about where gpg is installed. Again, I'll discuss all this with the platform team and come up with a course of action.

[lorengordon]

I hadn't really thought about it before, but it's interesting that Linux has one or more bin directories right in the 'PATH', where applications can drop the binary or a link so that they are easy to find and use, but that Windows does not. On Windows, the full path to the application must be in the PATH environment. Certainly complicates this issue. The joys of supporting multiple platforms! :/

If the gpg.py module is an internal salt dependency (presumably for a salt master on Windows, as I haven't needed it for minions or masterless setups...?), and it's the primary reason for this issue, then it makes sense to me to bundle gpg with the Windows installer for salt. Hopefully the 1.4.x versions are sufficient, as that link indicates it's the easiest to bundle.

But if push comes to shove, the approach described by @jfindlay seems reasonable. Though I might change the function definition slightly, as program_files seems too Windows-specific, and maybe use a lookup table of common program paths per distro to search through.

def which(exec, additional_paths=None, search_common_paths=True):

[ryancurrah]

Ryan here I opened the ZD ticket.

So after this amazing discussion, it got me thinking about the issue a little more and i did some more digging...

Here is the output of my PATH on a fresh Win2012r2 instance nothing installed... (there is chocolatey and full install of python baked in for cloud-init)

C:\Windows\system32>path
PATH=C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32
\WindowsPowerShell\v1.0\;C:\ProgramData\chocolatey\bin;C:\Python27;C:\Python27\Scripts

Now im going to install saltminion from chocolatey which we modified to have a dependency on gpg4win... and an output of my path from Windows UI in System > Advanced System Settings > Environment Variables > PATH...

C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\ProgramData\chocolatey\bin;C:\Python27;C:\Python27\Scripts;C:\Program Files (x86)\GNU\GnuPG\pub

GPG DOES GET ADDED TO THE PATH WHATTTT, IM CONFUSED WHY NO SHOW ON CMD PROMPT?

Now im going to open a brand spanking new command prompt and output the PATH after installing saltminion...

C:\Users\Administrator>path
PATH=C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32
\WindowsPowerShell\v1.0\;C:\ProgramData\chocolatey\bin;C:\Python27;C:\Python27\Scripts

WHAT THE 'Holy Priceless Collection of Etruscan Snoods Batman!' why isnt my PATH updating????

Ok whats going on here the PATH is correct in the registry but not on any cmd prompt or powershell prompt, time to google....

http://superuser.com/questions/593949/why-wont-my-windows-8-command-line-update-its-path

Oh snap this looks like the culprit... im going to give it a go, kill explorer.exe and relaunch cmd and print out my path.

C:\Users\Administrator>path
PATH=C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32
\WindowsPowerShell\v1.0\;C:\ProgramData\chocolatey\bin;C:\Python27;C:\Python27\Scripts

Dang it didnt work. I closed all windows, killed explorer multiple times, opened multiple new cmd prompts and the PATH is not updating still... time to refactor my google search criteria.

Ah looks like I need to log out and back in for the new PATH to take effect..

C:\Users\Administrator>path
PATH=C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32
\WindowsPowerShell\v1.0\;C:\ProgramData\chocolatey\bin;C:\Python27;C:\Python27\Scripts;C:\Program Files (x86)\GNU\GnuPG\pub

Holy shiznack, it worked! But not much use to us. Need a work around...

Looks like the solution is to broadcast something called a 'WM_SETTINGCHANGE' which will update all processes with the new environment variables. As referenced in these two serverfault / stack exchange questions...

http://serverfault.com/questions/8855/how-do-you-add-a-windows-environment-variable-without-rebooting

https://stackoverflow.com/questions/3636055/how-to-modify-the-path-variable-definitely-through-the-command-line-in-windows

I wonder if we could build that into salt to like do that before a highstate executes on windows and after a chocolatey.install command???

[ryancurrah]

Or salt could read the PATH directly from registry using this https://docs.python.org/2.7/library/_winreg.html.

http://stackoverflow.com/questions/573817/where-are-environment-variables-stored-in-registry

[ryancurrah]

Oh forgot to add result of rendering gpg pillar after loging out and back in...

c:\salt>salt-call.bat pillar.get env
local:
    ----------
    gpg:
        supersecr

[lorengordon]

@ryancurrah, as a workaround in the meantime, one of those links you sent had a PowerShell snippet to send the 'WM_SETTINGCHANGE' message. As part of your state run or cloud-init, you could execute the PowerShell snippet and see if it does what you need...

http://poshcode.org/2049:

#requires -version 2

if (-not ("win32.nativemethods" -as [type])) {
    # import sendmessagetimeout from win32
    add-type -Namespace Win32 -Name NativeMethods -MemberDefinition @"
[DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Auto)]
public static extern IntPtr SendMessageTimeout(
    IntPtr hWnd, uint Msg, UIntPtr wParam, string lParam,
    uint fuFlags, uint uTimeout, out UIntPtr lpdwResult);
"@
}

$HWND_BROADCAST = [intptr]0xffff;
$WM_SETTINGCHANGE = 0x1a;
$result = [uintptr]::zero

# notify all windows of environment block change
[win32.nativemethods]::SendMessageTimeout($HWND_BROADCAST, $WM_SETTINGCHANGE,
    [uintptr]::Zero, "Environment", 2, 5000, [ref]$result);

[twangboy]

Maybe we need to add something like this to call after we install something (on windows). Or maybe before we do a run.cmd or run.shell... or maybe when the minion starts... just thinking out loud here:

# this ensures that changes propagate immediately
def RefreshEnvironment():
    """ A method by Geoffrey Faivre-Malloy and Ronny Lipshitz """
    # broadcast settings change
    HWND_BROADCAST = 0xFFFF
    WM_SETTINGCHANGE = 0x001A
    SMTO_ABORTIFHUNG = 0x0002
    sParam = "Environment"

    res1, res2 = win32gui.SendMessageTimeout( HWND_BROADCAST, WM_SETTINGCHANGE, 0, sParam, SMTO_ABORTIFHUNG, 100 )
    if not res1:
        print( "result: %s, %s, from SendMessageTimeout" % (bool(res1), res2) )

from https://gist.github.com/apetrone/5937002

[lorengordon]

Makes sense to me do to it at least after package installs on Windows. Perhaps it could be a function in utils (or wherever), so that users could call it manually if they find the need?

[twangboy]

@lorengordon It looks like salt is already doing this via the win_path module. The function is called rehash. @ryancurrah I've tried to duplicate this issue. I've installed gpg4win manually and everything seems to work correctly. Meaning if I open a new cmd window and type path , the following is displayed as part of the path c:\program files (x86)\gnu\gnupg\pub. I've installed gpg4win via chocolatey with the same result. I've also tried opening a command prompt first and then adding a new directory to the path through windows. The new directory does NOT show up in the path in the open cmd prompt. I've tried sending a salt '<minion name>' win_path.rehash from the master as well as salt-call win_path.rehash from the minion inside the open cmd window. Neither refreshes the environment in the open cmd window. It appears the WM_SETTINGCHANGE does not effect open cmd windows. http://support.microsoft.com/kb/104011 If I open a new cmd window, the directory shows up in path as expected. I have not had to reboot. I don't know why you have to reboot.

[lorengordon]

@twangboy, one difference I see in the rehash code and the SendMessageTimeout calls we found elsewhere is that rehash sets the second to last parameter to 0 where others set it to 2. That's the flags parameter of SendMessageTimeout. Not sure that difference is significant, but it might be worth testing. Perhaps that's why the path wasn't refreshing in the open cmd window?

[twangboy]

I think 0 is the correct value for salt. 2 (SMTO_AbortIfHung) doesn't wait for the function to finish or timeout before returning. https://msdn.microsoft.com/en-us/library/windows/desktop/ms644952%28v=vs.85%29.aspx

[lorengordon]

Gotcha. I had a dev box running so I was in the middle of testing a flag of 2 anyway, but it doesn't seem to make a difference. My findings otherwise confirm yours -- an open cmd window won't update it's path, but a new cmd window will see the new path. No logoff or reboot required.

[twangboy]

@ryancurrah The problem I see is that a cmd prompt doesn't get environment variable updates until it has been restarted. This holds true for the salt-minion as well, meaning that if you install software that modifies environment variables, salt will not see the updated variables until it is restarted. Google is full of frustrated sysadmins who want to refresh environment variables within the cmd prompt without restarting the cmd prompt. It seems there is no solution other than to restart the cmd prompt. When using salt, that means restarting the salt-minion to see updates to the environment variables. If you're running salt in debug mode, that means stopping the minion, restarting the cmd prompt, and running salt again. In your case, for some reason that I can't reproduce, your machine required a reboot in order for cmd to see the updated environment variables. There is something unique in your environment that is causing that problem that I can't to replicate. I am closing this issue because it seems to be more of a windows thing than a salt thing.

[ryancurrah]

Your right, were calling salt using celeryd running in python. Even when we send out that refresh the path signal, the path still doesn't update until we restart the celeryd process. So what we did as a solution was to modify the glorious salt-call.bat and appended the path there.

Notice we added an extra line after the last Set Script:

@ echo off
:: Script for starting the Salt-Minion
:: Accepts all parameters that Salt-Minion Accepts

:: Define Variables
Set SaltDir=%~dp0
Set SaltDir=%SaltDir:~0,-1%
Set Python=%SaltDir%\bin\python.exe
Set Script=%SaltDir%\bin\Scripts\salt-call
PATH=%PATH%;C:\Program Files (x86)\GNU\GnuPG\pub

"%Python%" "%Script%" %*