boxcutter / windows

Virtual machine templates for Windows written in legacy JSON and Batch Scripting/JScript
Apache License 2.0
753 stars 266 forks source link

Proxy support for Initial install and shell provisioners #229

Open daxgames opened 4 years ago

daxgames commented 4 years ago

@arizvisa Merge my PRs in the order they were opened for best results. If merging all it should look like this when done.

They are all based on boxcutter/windows master branch.

The first 4 PRs do everything that was in #179

229

230

232

233

This is new based on issues encountered with New Packer 1.5.2

234

Proxy support for Initial install and shell provisioners

daxgames commented 4 years ago

@arizvisa Proxy support added.

daxgames commented 4 years ago

@arizvisa Still working on adding authenticated proxy.

daxgames commented 4 years ago

@arizvisa This is ready to go. Please ask if you have any questions.

arizvisa commented 4 years ago

Thanks for doing this work. My bad on merging the other ones before reading this. :-/

I just relocated, so I don't have all my equipment setup to simulate this properly. It's likely I'll be able to deploy a quick proxy and build more than a single VM sometime next week, though. However, I'll see what I can do over the weekend so you aren't stuck waiting too long.

daxgames commented 4 years ago

@arizvisa no worries. I oriiginally wrote a less capable version of the proxy support for this I think 3 years ago so I have been waiting for a while.

arizvisa commented 4 years ago

As semi-discussed in #83, the addition of allowing users to customize the configuration during the floppy step (which includes the update to all of the templates) is different from the consolidation of the download methods that you did to implement proxy support.

Can you isolate the configuration (along with the relevant paragraph of documentation) and all of the modifications to the templates into its own PR?

daxgames commented 4 years ago

@arizvisa It was done because it is absolutely required to configure proxy support during floppy step. So it is indeed part of the proxy support PR.

Edit: Let me explain. You don't want someone editing a file that is a part of this repo to add optional configuration. They should be able to add a file that is ignored by git as optional configuration.

arizvisa commented 4 years ago

Is it though? What's wrong with the pre-existing methodology of editing floppy/_packer_config.cmd?

daxgames commented 4 years ago

If you want to protect users and make it easy to use without making bad commits else its REALLY easy to mess up.

git add
git commit -m 'add proxy config'

I just don't see the features as a separate thing. In my opinion floppy/_packer_config*.cmd not being in the packer templates is an oversight that should have always been there.

Maybe DISABLE_BITS should be remove from floppy/_packer_config.cmd but I can see it becoming a default setting where proxy will never become a default setting. Which is why I put it in that file and suggest proxy config go in a user supplied ignored file.

arizvisa commented 4 years ago

Ok. No worries then. I'll do it for you.

daxgames commented 4 years ago

@arizvisa You'll leave it in or take it out? I am not trying to be difficult, just curious.

I mean, I am just pleading my case and trying to convince you. You are the maintainer and have the ultimate say. I just disagree it is a mixed feature PR because the scripts have always had the ability to run multiple floppy/_packer_config*.cmd scripts.

I can make it a separate PR if that is what you really want. Just let me know.

arizvisa commented 4 years ago

Can you re-base this? The differences should be a lot smaller as a result of PR #235.

daxgames commented 4 years ago

@arizvisa Rebased