Merge pull request hashicorp#3997 from boumenot/pr-issue-3730

azure: handle os_type errors more gracefully
agamdua · Oct 13, 2016 · ae2b31c · ae2b31c
2 parents ca3b456 + bba643d
commit ae2b31c
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 3 deletions.
diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go
@@ -101,7 +101,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 	b.setTemplateParameters(b.stateBag)
 	var steps []multistep.Step
 
-	if strings.EqualFold(b.config.OSType, constants.Target_Linux) {
+	if b.config.OSType == constants.Target_Linux {
 		steps = []multistep.Step{
 			NewStepCreateResourceGroup(azureClient, ui),
 			NewStepValidateTemplate(azureClient, ui, b.config, GetVirtualMachineDeployment),
@@ -119,7 +119,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			NewStepDeleteResourceGroup(azureClient, ui),
 			NewStepDeleteOSDisk(azureClient, ui),
 		}
-	} else if strings.EqualFold(b.config.OSType, constants.Target_Windows) {
+	} else if b.config.OSType == constants.Target_Windows {
 		steps = []multistep.Step{
 			NewStepCreateResourceGroup(azureClient, ui),
 			NewStepValidateTemplate(azureClient, ui, b.config, GetKeyVaultDeployment),

diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go
@@ -477,7 +477,13 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) {
 
 	/////////////////////////////////////////////
 	// OS
-	if c.OSType != constants.Target_Linux && c.OSType != constants.Target_Windows {
+	if strings.EqualFold(c.OSType, constants.Target_Linux) {
+		c.OSType = constants.Target_Linux
+	} else if strings.EqualFold(c.OSType, constants.Target_Windows) {
+		c.OSType = constants.Target_Windows
+	} else if c.OSType == "" {
 		errs = packer.MultiErrorAppend(errs, fmt.Errorf("An os_type must be specified"))
+	} else {
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("The os_type %q is invalid", c.OSType))
 	}
 }
diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go
@@ -119,6 +119,47 @@ func TestConfigShouldNotDefaultImageVersionIfCustomImage(t *testing.T) {
 	}
 }
 
+func TestConfigShouldNormalizeOSTypeCase(t *testing.T) {
+	config := map[string]string{
+		"capture_name_prefix":    "ignore",
+		"capture_container_name": "ignore",
+		"location":               "ignore",
+		"image_url":              "ignore",
+		"storage_account":        "ignore",
+		"resource_group_name":    "ignore",
+		"subscription_id":        "ignore",
+		"communicator":           "none",
+	}
+
+	os_types := map[string][]string{
+		constants.Target_Linux:   {"linux", "LiNuX"},
+		constants.Target_Windows: {"windows", "WiNdOWs"},
+	}
+
+	for k, v := range os_types {
+		for _, os_type := range v {
+			config["os_type"] = os_type
+			c, _, err := newConfig(config, getPackerConfiguration())
+			if err != nil {
+				t.Fatalf("Expected config to accept the value %q, but it did not", os_type)
+			}
+
+			if c.OSType != k {
+				t.Fatalf("Expected config to normalize the value %q to %q, but it did not", os_type, constants.Target_Linux)
+			}
+		}
+	}
+
+	bad_os_types := []string{"", "does-not-exist"}
+	for _, os_type := range bad_os_types {
+		config["os_type"] = os_type
+		_, _, err := newConfig(config, getPackerConfiguration())
+		if err == nil {
+			t.Fatalf("Expected config to not accept the value %q, but it did", os_type)
+		}
+	}
+}
+
 func TestConfigShouldRejectCustomImageAndMarketPlace(t *testing.T) {
 	config := map[string]string{
 		"capture_name_prefix":    "ignore",