Ensure that the xenorchestra_vm resource formats mac addresses proper…

…ly before making requests to the XO api (#115)

* Ensure that the xenorchestra_vm resource formats mac addresses properly before making requests to the XO api

* Add test for vm creating with dashed mac address

* Remove parts that were due to testing environment

* Fix it for real

* Update docs to highlight how mac addresses are parsed and stored in state

docs/resources/vm.md

@@ -70,7 +70,7 @@ resource "xenorchestra_vm" "bar" {
 * affinity_host - (Optional) The preferred host you would like the VM to run on. If changed on an existing VM it will require a reboot for the VM to be rescheduled.
 * network - (Required) The network the VM will use
     * network_id - (Required) The ID of the network the VM will be on.
-    * mac_address - (Optional) The mac address of the network interaface
+    * mac_address - (Optional) The mac address of the network interface. This must be parsable by go's [net.ParseMAC function](https://golang.org/pkg/net/#ParseMAC). All mac addresses are stored in Terraform's state with [HardwareAddr's string representation](https://golang.org/pkg/net/#HardwareAddr.String) i.e. 00:00:5e:00:53:01
 * disk - (Required) The disk the VM will have access to.
     * sr_id - (Required) The storage repository ID to use.
     * name_label - (Required) The name for the disk.

xoa/resource_xenorchestra_vm.go

@@ -123,6 +123,15 @@ func resourceRecord() *schema.Resource {
 							Type:     schema.TypeString,
 							Optional: true,
 							Computed: true,
+							StateFunc: func(val interface{}) string {
+								unformattedMac := val.(string)
+								mac, err := net.ParseMAC(unformattedMac)
+								if err != nil {
+									panic(fmt.Sprintf("Mac address `%s` was not parsable. This should never happened because Terraform's validation should happen before this is stored into state", unformattedMac))
+								}
+								return mac.String()
+
+							},
 							ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
 								mac_address := val.(string)
 								if _, err := net.ParseMAC(mac_address); err != nil {
@@ -188,11 +197,11 @@ func resourceVmCreate(d *schema.ResourceData, m interface{}) error {
 	networks := d.Get("network").([]interface{})
 
 	for _, network := range networks {
-		net, _ := network.(map[string]interface{})
+		netMap, _ := network.(map[string]interface{})
 
 		network_maps = append(network_maps, map[string]string{
-			"network": net["network_id"].(string),
-			"mac":     net["mac_address"].(string),
+			"network": netMap["network_id"].(string),
+			"mac":     getFormattedMac(netMap["mac_address"].(string)),
 		})
 	}
 
@@ -545,7 +554,7 @@ func expandNetworks(networks []interface{}) []*client.VIF {
 		attached := data["attached"].(bool)
 		device := data["device"].(string)
 		networkId := data["network_id"].(string)
-		macAddress := data["mac_address"].(string)
+		macAddress := getFormattedMac(data["mac_address"].(string))
 		vifs = append(vifs, &client.VIF{
 			Attached:   attached,
 			Device:     device,
@@ -759,3 +768,15 @@ func performDiskUpdateAction(c *client.Client, action updateDiskActions, d clien
 
 	return errors.New(fmt.Sprintf("disk update action '%d' not handled", action))
 }
+
+func getFormattedMac(macAddress string) string {
+	if macAddress == "" {
+		return macAddress
+	}
+	mac, err := net.ParseMAC(macAddress)
+
+	if err != nil {
+		panic(fmt.Sprintf("Mac address `%s` was not parsable. This is a bug in the provider and this value should have been properly formatted", macAddress))
+	}
+	return mac.String()
+}

xoa/resource_xenorchestra_vm_test.go

@@ -417,10 +417,35 @@ func TestAccXenorchestraVm_createWithCloudInitNetworkConfig(t *testing.T) {
 	})
 }
 
+func TestAccXenorchestraVm_createWithDashedMacAddress(t *testing.T) {
+	resourceName := "xenorchestra_vm.bar"
+	macWithDashes := "00-0a-83-b1-c0-01"
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckXenorchestraVmDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccVmConfigWithMacAddress(macWithDashes),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccVmExists(resourceName),
+					resource.TestCheckResourceAttrSet(resourceName, "id"),
+					resource.TestCheckResourceAttr(resourceName, "network.#", "1"),
+					internal.TestCheckTypeSetElemNestedAttrs(resourceName, "network.*", map[string]string{
+						// All mac addresses should be formatted to use colons
+						"mac_address": getFormattedMac(macWithDashes),
+					}),
+					internal.TestCheckTypeSetElemAttrPair(resourceName, "network.*.*", "data.xenorchestra_network.network", "id")),
+			},
+		},
+	})
+}
+
 func TestAccXenorchestraVm_createAndUpdateWithMacAddress(t *testing.T) {
 	resourceName := "xenorchestra_vm.bar"
 	macAddress := "00:0a:83:b1:c0:83"
 	otherMacAddress := "00:0a:83:b1:c0:00"
+	macWithDashes := "00-0a-83-b1-c0-01"
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
@@ -448,6 +473,18 @@ func TestAccXenorchestraVm_createAndUpdateWithMacAddress(t *testing.T) {
 					}),
 					internal.TestCheckTypeSetElemAttrPair(resourceName, "network.*.*", "data.xenorchestra_network.network", "id")),
 			},
+			{
+				Config: testAccVmConfigWithMacAddress(macWithDashes),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccVmExists(resourceName),
+					resource.TestCheckResourceAttrSet(resourceName, "id"),
+					resource.TestCheckResourceAttr(resourceName, "network.#", "1"),
+					internal.TestCheckTypeSetElemNestedAttrs(resourceName, "network.*", map[string]string{
+						// All mac addresses should be formatted to use colons
+						"mac_address": getFormattedMac(macWithDashes),
+					}),
+					internal.TestCheckTypeSetElemAttrPair(resourceName, "network.*.*", "data.xenorchestra_network.network", "id")),
+			},
 		},
 	})
 }