boxcutter / windows

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

Add CM_LICENSED and restore backward compatibility to use free Chef components #202

Closed daxgames closed 4 years ago

daxgames commented 4 years ago

Ready to merge.

Adds

Usage

daxgames commented 4 years ago

@arizvisa Can you test this? The pieces of code I changed are working but the VM is failing to complete the build due to:

==> virtualbox-iso: Provisioning with shell script: /Users/ba13509/repos/bbb_github/pipedream/boxcutter_windows/script/ultradefrag.bat
==> virtualbox-iso: http://downloads.sourceforge.net/ultradefrag/ultradefrag-portable-7.0.2.bin.amd64.zip:
    virtualbox-iso: ==> Creating "C:\Users\vagrant\AppData\Local\Temp\ultradefrag"
    virtualbox-iso: ==> Downloading "http://downloads.sourceforge.net/ultradefrag/ultradefrag-portable-7.0.2.bin.amd64.zip" to "C:\Users\vagrant\AppData\Local\Temp\ultradefrag\ultradefrag-portable-7.0.2.bin.amd64.zip"
==> virtualbox-iso: 2020-01-07 13:03:52 ERROR 503: Service Unavailable.
    virtualbox-iso:
    virtualbox-iso: BITSADMIN version 3.0 [ 7.5.7600 ]
==> virtualbox-iso: Exception calling "DownloadFile" with "2" argument(s): "The remote server retur
==> virtualbox-iso: ned an error: (503) Server Unavailable."
    virtualbox-iso: BITS administration utility.
    virtualbox-iso: (C) Copyright 2000-2006 Microsoft Corp.
==> virtualbox-iso: At line:1 char:47
==> virtualbox-iso: + (New-Object System.Net.WebClient).DownloadFile <<<< ('http://downloads.source
==> virtualbox-iso: forge.net/ultradefrag/ultradefrag-portable-7.0.2.bin.amd64.zip', 'C:\Users\vagr
==> virtualbox-iso: ant\AppData\Local\Temp\ultradefrag\ultradefrag-portable-7.0.2.bin.amd64.zip')
    virtualbox-iso:
==> virtualbox-iso:     + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    virtualbox-iso: BITSAdmin is deprecated and is not guaranteed to be available in future versions of Windows.
==> virtualbox-iso:     + FullyQualifiedErrorId : DotNetMethodException
    virtualbox-iso: Administrative tools for the BITS service are now provided by BITS PowerShell cmdlets.
    virtualbox-iso:
==> virtualbox-iso:
    virtualbox-iso: Unable to create job - 0x80200014
    virtualbox-iso: ==> ERROR: Failed to download "http://downloads.sourceforge.net/ultradefrag/ultradefrag-portable-7.0.2.bin.amd64.zip" to "C:\Users\vagrant\AppData\Local\Temp\ultradefrag\ultradefrag-portable-7.0.2.bin.amd64.zip"
    virtualbox-iso:
    virtualbox-iso: Pinging 127.0.0.1 with 32 bytes of data:
    virtualbox-iso: Reply from 127.0.0.1: bytes=32 time<1ms TTL=128
    virtualbox-iso: Reply from 127.0.0.1: bytes=32 time<1ms TTL=128
    virtualbox-iso: Reply from 127.0.0.1: bytes=32 time<1ms TTL=128
    virtualbox-iso: Reply from 127.0.0.1: bytes=32 time<1ms TTL=128
    virtualbox-iso:
    virtualbox-iso: Ping statistics for 127.0.0.1:
    virtualbox-iso:     Packets: Sent = 4, Received = 4, Lost = 0 (0% loss),
    virtualbox-iso: Approximate round trip times in milli-seconds:
    virtualbox-iso:     Minimum = 0ms, Maximum = 0ms, Average = 0ms
    virtualbox-iso: ==> Script exiting with errorlevel 1
==> virtualbox-iso: Deregistering and deleting VM...
==> virtualbox-iso: Deleting output directory...
Build 'virtualbox-iso' errored: Script exited with non-zero exit status: 1.Allowed exit codes are: [0]

==> Some builds didn't complete successfully and had errors:
--> virtualbox-iso: Script exited with non-zero exit status: 1.Allowed exit codes are: [0]

==> Builds finished but no artifacts were created.
make: *** [box/virtualbox/eval-win7x64-enterprise-cheflatest-1.0.4.box] Error 1

And I didn't even touch that code.

arizvisa commented 4 years ago

sure.

arizvisa commented 4 years ago

So this PR seems to work with the eval-win81x64-enterprise.json template (it was just the .iso that I had already had previously downloaded which is why i initially chose it).

I'm testing eval-win7x64-enterprise.json right now, so that might be able to repro your exact error.

This is likely an issue with the win7 templates and how things are being downloaded in those. Salt-bootstrap has ssl/tls issues on win7 as well, so it's likely specific to those templates. I'll let you know if that's the case.

arizvisa commented 4 years ago

Hmm...

    vmware-iso: Ping statistics for 127.0.0.1:
    vmware-iso:     Packets: Sent = 4, Received = 4, Lost = 0 (0% loss),
    vmware-iso: Approximate round trip times in milli-seconds:
    vmware-iso:     Minimum = 0ms, Maximum = 0ms, Average = 0ms
    vmware-iso: ==> Script exiting with errorlevel 0
==> vmware-iso: Provisioning with shell script: C:\Users\user\work\lolmeter\build\boxcutter-windows/script/ultradefrag.bat
    vmware-iso: ==> Creating "C:\Users\vagrant\AppData\Local\Temp\ultradefrag"
    vmware-iso: ==> Downloading "http://downloads.sourceforge.net/ultradefrag/ultradefrag-portable-7.0.2.bin.amd64.zip" to "
C:\Users\vagrant\AppData\Local\Temp\ultradefrag\ultradefrag-portable-7.0.2.bin.amd64.zip"
==> vmware-iso: 2020-01-07 20:16:02 URL:https://phoenixnap.dl.sourceforge.net/project/ultradefrag/stable-release/7.0.2/ultra
defrag-portable-7.0.2.bin.amd64.zip [3252543/3252543] -> "C:/Users/vagrant/AppData/Local/Temp/ultradefrag/ultradefrag-portab
le-7.0.2.bin.amd64.zip" [1]
    vmware-iso: ==> Unzipping "C:\Users\vagrant\AppData\Local\Temp\ultradefrag\ultradefrag-portable-7.0.2.bin.amd64.zip" to
"C:\Users\vagrant\AppData\Local\Temp\ultradefrag"
    vmware-iso:
    vmware-iso: 7-Zip [64] 16.04 : Copyright (c) 1999-2016 Igor Pavlov : 2016-10-04
    vmware-iso:
    vmware-iso: Scanning the drive for archives:
    vmware-iso: 1 file, 3252543 bytes (3177 KiB)
    vmware-iso:
    vmware-iso: Extracting archive: C:\Users\vagrant\AppData\Local\Temp\ultradefrag\ultradefrag-portable-7.0.2.bin.amd64.zip

    vmware-iso: --
    vmware-iso: Path = C:\Users\vagrant\AppData\Local\Temp\ultradefrag\ultradefrag-portable-7.0.2.bin.amd64.zip
    vmware-iso: Type = zip
    vmware-iso: Physical Size = 3252543
    vmware-iso:
    vmware-iso: Everything is Ok
    vmware-iso:
    vmware-iso: Files: 4
    vmware-iso: Size:       2457088
    vmware-iso: Compressed: 3252543
    vmware-iso: ==> Running UltraDefrag on C:
    vmware-iso: UltraDefrag 7.0.2, Copyright (c) UltraDefrag Development Team, 2007-2016.
    vmware-iso: UltraDefrag comes with ABSOLUTELY NO WARRANTY. This is free software,
    vmware-iso: and you are welcome to redistribute it under certain conditions.
    vmware-iso:
arizvisa commented 4 years ago

Using eval-win7x64-enterprise.json didn't result in using bitsadmin to download ultradefrag though. Lemme see what I can do to force it to use bitsadmin.

This is one of the reasons why I'd like to eventually move towards self-hosting these deps with packer's httpd because for one it allows you to build them in a non-networked environment, and it can make these download issues act more consistently. And then, how do we know whether eternallybored.org isn't hosting a backdoored wget.exe for every 10th download, maybe he's a boxcutter employee but we don't know that for sure...

arizvisa commented 4 years ago

Ok, and definitely nailed it on eval-win7x64-enterprise.json once bitsadmin is used to contact the first external url. Same deal on eval-win81x64-enterprise.json. Might be a universal issue w/ bitsadmin being used to communicate w/ external urls.

daxgames commented 4 years ago

I had so many issues with bitsadmin i had a custom version if this repo that allowed it to be disabled as a download option. Once I did that I never had another issue.

arizvisa commented 4 years ago

That's a good idea, actually. I honestly have only used bitsadmin as a sort-of pentesting hack for staging the download of better tools. I don't really trust its capabilities in terms of being reliable for a production environment. Once I get through merging some of these older PRs, I'll add that idea to a TODO so that we can eventually add support for configuring that.

arizvisa commented 4 years ago

Ok. So just finished updating all of the cm tools w/ their latest urls. Thanks for your patience.

So...wrt this PR, how do you feel about either dual-using CM_VERSION with "unlicensed" vs "latest" to constrain the versions, or maybe allowing a user to specify the licensing version constraint within CM_OPTIONS? I'm just hesitant to add an explicit environment variable for each individual cmtool option as it can be arbitrary. Personally, I prefer to install the cmtools via winrm after-the-fact.

One issue with using CM_OPTIONS seems to be that if we want to expose the "ADDLOCAL=" parameter, then we'd need to manually parse that. I'm ok with that if a user wants it as that's what's being done with the salt cmtool.

If we use CM_VERSION and allow a user to specify "unlicensed" (or similar), then I'll just need to document it.

daxgames commented 4 years ago

In my opinion you can't use CM_VERSION because this breaks backward compatibility because anyone currently using CM_VERSION=latest will break.

This is why I chose cm_licensed because it can apply to one or all cm tools if/when the require a license acceptance now or in the future.

CM_OPTIONS is passed to the installer command line so this would not work without somehow pre-parsing the CM_OPTIONS and removing the license model which seems problematic.

Many times an image with a cm tool built in is desired so I am in favor of leaving the current behavior also I don't like removing existing functionality again because of backward compatibility. SOMEONE is using it and the WILL complain LOUDLY! :-)

arizvisa commented 4 years ago

Yeah, CM_OPTIONS doesn't need to be passed to the command-line. That's just the default that I chose. It's literally intended as a gateway to specify options to cmtools and I only introduced it 4 days ago. The only options that chef provides is that ADDLOCAL option which is just some cases. That's why I mentioned it.

(edited to mention lifetime of the CM_OPTIONS feature)

arizvisa commented 4 years ago

These are the docs for the chef installer commandline that I mentioned can be cased out: https://docs.chef.io/install_windows.html

So if you were to use it to build a template, it'd look just like packer ... -var "cm_options=licensed,ChefClientFeature,ChefSchTaskFeature" /path/to/your/template.

daxgames commented 4 years ago

It still breaks backward compatibility, or am I just missing something.

Anyone currently using CM_VERSION=latest will break.

arizvisa commented 4 years ago

I think you're missing something.

All I'm suggesting is instead of using -var "CM_LICENSED=false" you'd use -var "CM_OPTIONS=freeorwhatever", or "CM_OPTIONS=CM_LICENSED=false" if you want an explicit boolean. In essense this PR is only introducing a knob to change the semantics of what "latest" means. This is literally just a knob to choose a single explicit version.

Whether you use the newly introduced CM_OPTIONS or a different key for CM_VERSION won't affect backwards compatibility at all since another word for CM_VERSION never existed before (it's always been the version number or "latest"), and CM_OPTIONS has never ever existed either.

Better yet, this can even be fixed w/ documentation reminding the user that newer versions of chef require licensing which they should know about anyways if they're using it.

daxgames commented 4 years ago

When I say this breaks backward compatibility, what I mean is,:

  1. Somone is using a version of this repo.
  2. They sync their local with the remote.
  3. Try to build
  4. Boxcutter fails to build an image they can use without any other changes.

If a user syncs these changes, that require using cm_options to configure licensing to unlicensed so they end up with an image that includes a free version of Chef just like it used to they would be wrong. They would get an image with a version of Chef that would fail until they figured out we changed something.

daxgames commented 4 years ago

Now you could default cm_options to free as I did for cm_licensed and it would be backward compatible by my definition.

arizvisa commented 4 years ago

Now you could default cm_options to licensed=false as I did for cm_licensed and it would be backward compatible by my definition.

Yep.

In the first example I mentioned, existence would be truthy. Hence non-existence or default is false. It's literally moving a boolean around, the semantics don't need to change.

All I'm suggesting is instead of using -var "CM_LICENSED=false" you'd use -var "CM_OPTIONS=freeorwhatever", or "CM_OPTIONS=CM_LICENSED=false" if you want an explicit boolean.

Even if you don't use CM_OPTIONS and decide to use something like CM_VERSION=latest, you can even warn the user that the latest free version is being used and that they need to explicitly specify a later version or licensed for the most recently licensed version. Again, you're shuffling a boolean around. I offered CM_VERSION as an option because the only time you're testing CM_LICENSED is at the same time as CM_VERSION so they both fit neatly together.

Trust me. It's not like I want to merge or debate about this... I'm only doing it to try and appease your concerns. :-P

arizvisa commented 4 years ago

Would a PR make it more clear? It's literally only a few lines for both examples since you already did the hard work of finding the latest free versions.

arizvisa commented 4 years ago

Here we go, PRs #211 and #212. I'm a fan of CM_VERSION from #211 since we're only changing the semantics of latest to the most recent free version, and licensed to the most recent non-free version.

Installing the latest free version (default if you don't provide anything)

Installing the latest licensed version

Installing an explicit version (14.1.1)

arizvisa commented 4 years ago

Heh. Took more time writing up that table then it did writing the PR...

daxgames commented 4 years ago

I like #211

arizvisa commented 4 years ago

Gotcha. I'll merge that in a little bit then. Thanks your contribution and tracking down the versions good sir!

arizvisa commented 4 years ago

Marking this as a duplicate of #211 as this PR will be superseded by that one once it gets documented and properly reviewed as it was originally just an example.

arizvisa commented 4 years ago

Okay. Closing as PR #211 supersedes this one. Thanks again for your contribution and input.