From 2e2b2cea82e36e3d39788a7b58f1c6c5256e3fe2 Mon Sep 17 00:00:00 2001
From: Megan Marsh <megan@hashicorp.com>
Date: Fri, 9 Nov 2018 15:46:52 -0800
Subject: [PATCH] add a warning that checks for collisions between
 packer-generated values in the default vmx file and the vmx_data

---
 builder/vmware/iso/builder.go         | 44 +++++++++++++++++++++++++++
 builder/vmware/iso/builder_test.go    | 25 +++++++++++++++
 builder/vmware/iso/step_create_vmx.go |  2 +-
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/builder/vmware/iso/builder.go b/builder/vmware/iso/builder.go
index 1a6b66171..bc119de5c 100644
--- a/builder/vmware/iso/builder.go
+++ b/builder/vmware/iso/builder.go
@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
+	"strings"
 	"time"
 
 	vmwcommon "github.com/hashicorp/packer/builder/vmware/common"
@@ -162,6 +163,11 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 				errs, fmt.Errorf("vmx_template_path is invalid: %s", err))
 		}
 
+	} else {
+		warn := b.checkForVMXTemplateAndVMXDataCollisions()
+		if warn != "" {
+			warnings = append(warnings, warn)
+		}
 	}
 
 	if b.config.Network == "" {
@@ -399,6 +405,44 @@ func (b *Builder) Cancel() {
 	}
 }
 
+// Validate the vmx_data option against the default vmx template to warn
+// user if anything is being overridden.
+func (b *Builder) checkForVMXTemplateAndVMXDataCollisions() string {
+	if b.config.VMXTemplatePath != "" {
+		return ""
+	}
+
+	var overridden []string
+	tplLines := strings.Split(DefaultVMXTemplate, "\n")
+	tplLines = append(tplLines,
+		fmt.Sprintf("%s0:0.present", strings.ToLower(b.config.DiskAdapterType)),
+		fmt.Sprintf("%s0:0.fileName", strings.ToLower(b.config.DiskAdapterType)),
+		fmt.Sprintf("%s0:0.deviceType", strings.ToLower(b.config.DiskAdapterType)),
+		fmt.Sprintf("%s0:1.present", strings.ToLower(b.config.DiskAdapterType)),
+		fmt.Sprintf("%s0:1.fileName", strings.ToLower(b.config.DiskAdapterType)),
+		fmt.Sprintf("%s0:1.deviceType", strings.ToLower(b.config.DiskAdapterType)),
+	)
+
+	for _, line := range tplLines {
+		if strings.Contains(line, `{{`) {
+			key := line[:strings.Index(line, " =")]
+			if _, ok := b.config.VMXData[key]; ok {
+				overridden = append(overridden, key)
+			}
+		}
+	}
+
+	if len(overridden) > 0 {
+		warnings := fmt.Sprintf("Your vmx data contains the following "+
+			"variable(s), which Packer normally sets when it generates its "+
+			"own default vmx template. This may cause your build to fail or "+
+			"behave unpredictably: %s", strings.Join(overridden, ", "))
+		return warnings
+	}
+	return ""
+}
+
+// Make sure custom vmx template exists and that data can be read from it
 func (b *Builder) validateVMXTemplatePath() error {
 	f, err := os.Open(b.config.VMXTemplatePath)
 	if err != nil {
diff --git a/builder/vmware/iso/builder_test.go b/builder/vmware/iso/builder_test.go
index b17fb95ab..cd96488d0 100644
--- a/builder/vmware/iso/builder_test.go
+++ b/builder/vmware/iso/builder_test.go
@@ -441,6 +441,31 @@ func TestBuilderPrepare_VNCPort(t *testing.T) {
 	}
 }
 
+func TestBuilderCheckCollisions(t *testing.T) {
+	config := testConfig()
+	config["vmx_data"] = map[string]string{
+		"no.collision":    "awesomesauce",
+		"ide0:0.fileName": "is a collision",
+		"displayName":     "also a collision",
+	}
+	{
+		var b Builder
+		warns, _ := b.Prepare(config)
+		if len(warns) != 1 {
+			t.Fatalf("Should have warning about two collisions.")
+		}
+	}
+	{
+		config["vmx_template_path"] = "some/path.vmx"
+		var b Builder
+		warns, _ := b.Prepare(config)
+		if len(warns) != 0 {
+			t.Fatalf("Should not check for collisions with custom template.")
+		}
+	}
+
+}
+
 func TestBuilderPrepare_CommConfig(t *testing.T) {
 	// Test Winrm
 	{
diff --git a/builder/vmware/iso/step_create_vmx.go b/builder/vmware/iso/step_create_vmx.go
index d879cb3ef..44d6d5e86 100644
--- a/builder/vmware/iso/step_create_vmx.go
+++ b/builder/vmware/iso/step_create_vmx.go
@@ -435,7 +435,7 @@ func (s *stepCreateVMX) Run(_ context.Context, state multistep.StateBag) multist
 
 	/// Handle the cdrom adapter type. If the disk adapter type and the
 	//  cdrom adapter type are the same, then ensure that the cdrom is the
-	//  slave device on whatever bus the disk adapter is on.
+	//  secondary device on whatever bus the disk adapter is on.
 	cdromAdapterType := strings.ToLower(config.CdromAdapterType)
 	if cdromAdapterType == "" {
 		cdromAdapterType = templateData.CDROMType