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

Fix some of the deprecated options in the schema for all of the templates. #220

Closed arizvisa closed 4 years ago

arizvisa commented 4 years ago

The packer validate command validates whether a template is using an invalid schema that results in an inability to build, but doesn't seem to throw out any warnings for keys that have been deprecated or replaced. It'd be pretty cool if packer had versioned their schemas at some point so we could do some trickery here, but unfortunately this is not the case. Because of this, we always lock tour schemas owards the most recent version of packer because they're always doing cool stuff over there anyways.

As a result, some keys are deprecated and might be changed/removed in the future. This PR renames the deprecated keys to the new superseded ones as recognized by the current version of packer at the time of the creation of this PR (1.5.1).

This closes issue #219.

arizvisa commented 4 years ago

First commit was made by doing the following:

$ sed -i 's/"ssh_wait_timeout":/"ssh_timeout":/' *.json wip/*.json
...
arizvisa commented 4 years ago

Here's the jq query that i have so far for grabbing the cores: (.builders[] | select(.type == "vmware-iso") | .vmx_data | ."cpuid.coresPerSocket" | tonumber) as $cores

Then for removing them: del(.builders[].vmx_data."cpuid.coresPerSocket")

Then to add them back, but prefixed: '.builders[] | select(.type == "vmware-iso") |= {cores: $cores} * .

arizvisa commented 4 years ago

All of the number of cores in each of the templates are set to 1, so I'ma likely place this under the number of cpus w/ sed.

arizvisa commented 4 years ago

Some of the hyperv-iso builders were missing the "ssh_timeout" option. This parameter isn't documented in packer's hyperv-iso documentation, but is still a valid parameter as its parsed by packer's ssh communicator.

The order for the ssh parameters was also placed at the end of each builder for all the ssh/cygwin templates so that when a user uses patch to fuzzy-patch these fields, it can be anchored against the vm_name.

I will likely create another PR that standardizes the order amongst all of the templates (by grouping) so that the patches look small when PRs are tampering with every single template.

arizvisa commented 4 years ago

Ok. This PR fixes the two things I mentioned in issue #219. This includes the renaming of "ssh_wait_timeout" to "ssh_timeout". The replacing of "vmx_data" used by the vmware-iso builders with "cores" which is the option that supersedes that, and then anchoring the "ssh_username", and "ssh_password" fields near the "vm_name".

arizvisa commented 4 years ago

Meh. Running the following emits validation issues in the wip/win2008r2-standardcore*.json templates.

$ for name in *.json wip/*.json; do packer validate $name 1>/dev/null || echo $name; done
wip/win2008r2-standardcore-cygwin.json
wip/win2008r2-standardcore.json
arizvisa commented 4 years ago

Okay. Other than that "update" user variable which needed to be a string (commit 49901c4), these validations are false alarms. Those templates are currently wip anyways. I'm not presently sure what their status is at the moment.

arizvisa commented 4 years ago

Ok. I'ma merge this.