Windows Proxy

[lawsonj2019]

I've added the capability for Windows Guests to have their http_proxy, https_proxy, ftp_proxy, and no_proxy environment variables set via this plugin and the use of explicit config.win_proxy Vagrantfile entries, or through the use of the VAGRANT_WIN_HTTP_PROXY style environment variables.

[tmatilai]

Hi James,

This looks totally awesome, thanks!

I was thinking if instead of adding a new config, we could just use env_proxy. What do you think?

Even better would be to move configuration logic from the action class into capabilities and use env_proxy_conf for both windows and linux. But that would require much more intrusive refactoring, and that's out of scope for this PR.

[johnbellone]

@lawsonj2019 Nice job!

I second using env_proxy_conf here.

[lawsonj2019]

The env_proxy_conf would probably be the best in the long run. I wanted to keep the configuration separate to not trample on work you were doing, but if we could still use the env_proxy and keep the windows action for this then that would work very well in the short term.

[johnbellone]

Ha, I spoke without actually looking at the code. The refactoring is definitely some effort. @tmatilai Perhaps that something to tackle for #58 as well? I have been meaning to get going on that.

[johnbellone]

That is we merge this and refactor along with adding support to get us around the bug presented in #58. Still early here and not enough coffee :D.

[tmatilai]

I'm fine using a separate action for now, but would really like to avoid the extra config class. We already have quite a few...

I have many things and refactoring planned, but been way too busy at work lately. Hope to allocate some love for this project in a couple of weeks. So if @lawsonj2019 can change the code to use env_proxy config, I would merge this and prepare a release next weekend.

[lawsonj2019]

I refactored the code to use the env_proxy config but ran into a couple of issues. The main one being that both the Windows and the Linux capabilities were being executed for the env config (on both OSes), probably due to the env_proxy config being enabled. This didn't appear to be too much of a problem on Windows, although a file was being written to the disk, but on Linux instances the Windows command to create the proxies raised an exception, as it wouldn't be able to execute CMD. In the end I refactored again so that there is no separate action, but a single configure_env_proxy action that decides whether the vagrant machine is a Windows client and executes one path, or another OS and executes the existing path. It's a little more intrusive to the original code than the first pull request, but hopefully lines up a little better with your expectations. I've tested this with Linux (Ubuntu) and Windows guests (from a Windows host machine with a proxy) with Vagrant 1.5.4 and it appears to be stable, but I'm sure you'll want to do a fair bit of consideration on this.

[tmatilai]

@lawsonj2019 thanks! I'll take a closer look on weekend.

It seems that in the first refactoring you renamed the capability to env_proxy_conf. So as that capability is thus supported on both linux and windows, both of the actions were happy to do their magic.

But I'll test and review later. Thanks again!

[tmatilai]

Squashed and merged! I don't have time to set up a Windows guest right now, but looks good to me.

Btw, could we also add support for npm, git, etc. too? Would the commands work as is?

[lawsonj2019]

Support for git, npm, etc.should be mostly fine with the addition of the environment variables. I think they both honour the environment variables. If not the same principles as the changes in the configure_env_proxy action could be applied to determine whether it is a Windows machine and apply any specific configuration.

Getting the git/npm executable path as part of the capability would need to be looked at, as this would be different on Windows - the util class in the capabilities would need to cater for Windows searching (unless you just assume git/npm/etc. are on the PATH).

[tmatilai]

IIRC the vagrant-windows plugin modifies which commands to be compatible. And at least pear won't support env vars. I think npm had some issues too.

Needs more investocation. Would you happen to know any trusted Packer templates to build a Windows box, preferably for VMware?

[lawsonj2019]

Not on the VMWare front. We're looking at packer templates right now for Windows, so if we come up with anything relevant I will let you know.