[MS] provider/azurerm: fixes for bug #8227 and bug #11226 (#13877)

* Resolved merge conflicts

* Changes conforming to HashiCorp guidelines and additional bug fixes

* Rebase merge

* Rebase merge

* Merging changes

* Changes to tests and code constructs

builtin/providers/azurerm/import_arm_subnet_test.go

@@ -1,7 +1,6 @@
 package azurerm
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -12,7 +11,31 @@ func TestAccAzureRMSubnet_importBasic(t *testing.T) {
 	resourceName := "azurerm_subnet.test"
 
 	ri := acctest.RandInt()
-	config := fmt.Sprintf(testAccAzureRMSubnet_basic, ri, ri, ri, ri, ri)
+	config := testAccAzureRMSubnet_basic(ri)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMSubnetDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: config,
+			},
+
+			resource.TestStep{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccAzureRMSubnet_importWithRouteTable(t *testing.T) {
+	resourceName := "azurerm_subnet.test"
+
+	ri := acctest.RandInt()
+	config := testAccAzureRMSubnet_routeTable(ri)
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },

builtin/providers/azurerm/resource_arm_subnet_test.go

@@ -2,7 +2,9 @@ package azurerm
 
 import (
 	"fmt"
+	"log"
 	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -13,7 +15,7 @@ import (
 func TestAccAzureRMSubnet_basic(t *testing.T) {
 
 	ri := acctest.RandInt()
-	config := fmt.Sprintf(testAccAzureRMSubnet_basic, ri, ri, ri, ri, ri)
+	config := testAccAzureRMSubnet_basic(ri)
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -30,10 +32,38 @@ func TestAccAzureRMSubnet_basic(t *testing.T) {
 	})
 }
 
+func TestAccAzureRMSubnet_routeTableUpdate(t *testing.T) {
+
+	ri := acctest.RandInt()
+	initConfig := testAccAzureRMSubnet_routeTable(ri)
+	updatedConfig := testAccAzureRMSubnet_updatedRouteTable(ri)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMSubnetDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: initConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMSubnetExists("azurerm_subnet.test"),
+				),
+			},
+
+			resource.TestStep{
+				Config: updatedConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMSubnetRouteTableExists("azurerm_subnet.test", fmt.Sprintf("acctest-%d", ri)),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAzureRMSubnet_disappears(t *testing.T) {
 
 	ri := acctest.RandInt()
-	config := fmt.Sprintf(testAccAzureRMSubnet_basic, ri, ri, ri, ri, ri)
+	config := testAccAzureRMSubnet_basic(ri)
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -60,6 +90,8 @@ func testCheckAzureRMSubnetExists(name string) resource.TestCheckFunc {
 			return fmt.Errorf("Not found: %s", name)
 		}
 
+		log.Printf("[INFO] Checking Subnet addition.")
+
 		name := rs.Primary.Attributes["name"]
 		vnetName := rs.Primary.Attributes["virtual_network_name"]
 		resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]
@@ -78,6 +110,60 @@ func testCheckAzureRMSubnetExists(name string) resource.TestCheckFunc {
 			return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not exist", name, resourceGroup)
 		}
 
+		if resp.RouteTable == nil {
+			return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not contain route tables after add", name, resourceGroup)
+		}
+
+		return nil
+	}
+}
+
+func testCheckAzureRMSubnetRouteTableExists(subnetName string, routeTableId string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		// Ensure we have enough information in state to look up in API
+		rs, ok := s.RootModule().Resources[subnetName]
+		if !ok {
+			return fmt.Errorf("Not found: %s", subnetName)
+		}
+
+		log.Printf("[INFO] Checking Subnet update.")
+
+		name := rs.Primary.Attributes["name"]
+		vnetName := rs.Primary.Attributes["virtual_network_name"]
+		resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]
+		if !hasResourceGroup {
+			return fmt.Errorf("Bad: no resource group found in state for subnet: %s", name)
+		}
+
+		vnetConn := testAccProvider.Meta().(*ArmClient).vnetClient
+		vnetResp, vnetErr := vnetConn.Get(resourceGroup, vnetName, "")
+		if vnetErr != nil {
+			return fmt.Errorf("Bad: Get on vnetClient: %s", vnetErr)
+		}
+
+		if vnetResp.Subnets == nil {
+			return fmt.Errorf("Bad: Vnet %q (resource group: %q) does not have subnets after update", vnetName, resourceGroup)
+		}
+
+		conn := testAccProvider.Meta().(*ArmClient).subnetClient
+
+		resp, err := conn.Get(resourceGroup, vnetName, name, "")
+		if err != nil {
+			return fmt.Errorf("Bad: Get on subnetClient: %s", err)
+		}
+
+		if resp.StatusCode == http.StatusNotFound {
+			return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not exist", subnetName, resourceGroup)
+		}
+
+		if resp.RouteTable == nil {
+			return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not contain route tables after update", subnetName, resourceGroup)
+		}
+
+		if !strings.Contains(*resp.RouteTable.ID, routeTableId) {
+			return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not have route table %q", subnetName, resourceGroup, routeTableId)
+		}
+
 		return nil
 	}
 }
@@ -135,7 +221,8 @@ func testCheckAzureRMSubnetDestroy(s *terraform.State) error {
 	return nil
 }
 
-var testAccAzureRMSubnet_basic = `
+func testAccAzureRMSubnet_basic(rInt int) string {
+	return fmt.Sprintf(`
 resource "azurerm_resource_group" "test" {
     name = "acctestRG-%d"
     location = "West US"
@@ -171,4 +258,114 @@ resource "azurerm_route" "test" {
 	next_hop_type = "VirtualAppliance" 
 	next_hop_in_ip_address = "10.10.1.1" 
 }
-`
+`, rInt, rInt, rInt, rInt, rInt)
+}
+
+func testAccAzureRMSubnet_routeTable(rInt int) string {
+	return fmt.Sprintf(`
+resource "azurerm_resource_group" "test" {
+    name = "acctestRG-%d"
+    location = "West US"
+}
+
+resource "azurerm_virtual_network" "test" {
+    name = "acctestvirtnet%d"
+    address_space = ["10.0.0.0/16"]
+    location = "West US"
+    resource_group_name = "${azurerm_resource_group.test.name}"
+}
+
+resource "azurerm_subnet" "test" {
+    name = "acctestsubnet%d"
+    resource_group_name = "${azurerm_resource_group.test.name}"
+    virtual_network_name = "${azurerm_virtual_network.test.name}"
+    address_prefix = "10.0.2.0/24"
+	route_table_id       = "${azurerm_route_table.test.id}"
+}
+
+resource "azurerm_route_table" "test" {
+  name                = "acctest-%d"
+  location            = "West US"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+}
+
+resource "azurerm_route" "route_a" {
+  name                = "acctest-%d"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+  route_table_name    = "${azurerm_route_table.test.name}"
+
+  address_prefix         = "10.100.0.0/14"
+  next_hop_type          = "VirtualAppliance"
+  next_hop_in_ip_address = "10.10.1.1"
+}`, rInt, rInt, rInt, rInt, rInt)
+}
+
+func testAccAzureRMSubnet_updatedRouteTable(rInt int) string {
+	return fmt.Sprintf(`
+resource "azurerm_resource_group" "test" {
+    name = "acctestRG-%d"
+    location = "West US"
+	tags {
+		environment = "Testing"
+	}
+}
+
+resource "azurerm_network_security_group" "test_secgroup" {
+    name = "acctest-%d"
+    location = "West US"
+    resource_group_name = "${azurerm_resource_group.test.name}"
+
+    security_rule {
+        name = "acctest-%d"
+        priority = 100
+        direction = "Inbound"
+        access = "Allow"
+        protocol = "Tcp"
+        source_port_range = "*"
+        destination_port_range = "*"
+        source_address_prefix = "*"
+        destination_address_prefix = "*"
+    }
+
+    tags {
+        environment = "Testing"
+    }
+}
+
+resource "azurerm_virtual_network" "test" {
+    name = "acctestvirtnet%d"
+    address_space = ["10.0.0.0/16"]
+    location = "West US"
+    resource_group_name = "${azurerm_resource_group.test.name}"
+	tags {
+		environment = "Testing"
+	}
+}
+
+resource "azurerm_subnet" "test" {
+    name = "acctestsubnet%d"
+    resource_group_name = "${azurerm_resource_group.test.name}"
+    virtual_network_name = "${azurerm_virtual_network.test.name}"
+    address_prefix = "10.0.2.0/24"
+	route_table_id       = "${azurerm_route_table.test.id}"
+}
+
+resource "azurerm_route_table" "test" {
+  name                = "acctest-%d"
+  location            = "West US"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+  tags {
+    environment = "Testing"
+  }
+}
+
+resource "azurerm_route" "route_a" {
+  name                = "acctest-%d"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+  route_table_name    = "${azurerm_route_table.test.name}"
+
+  address_prefix         = "10.100.0.0/14"
+  next_hop_type          = "VirtualAppliance"
+  next_hop_in_ip_address = "10.10.1.1"
+}`, rInt, rInt, rInt, rInt, rInt, rInt, rInt)
+}

builtin/providers/azurerm/resource_arm_virtual_network.go

@@ -1,6 +1,7 @@
 package azurerm
 
 import (
+	"bytes"
 	"fmt"
 	"log"
 	"net/http"
@@ -89,11 +90,15 @@ func resourceArmVirtualNetworkCreate(d *schema.ResourceData, meta interface{}) e
 	location := d.Get("location").(string)
 	resGroup := d.Get("resource_group_name").(string)
 	tags := d.Get("tags").(map[string]interface{})
+	vnetProperties, vnetPropsErr := getVirtualNetworkProperties(d, meta)
+	if vnetPropsErr != nil {
+		return vnetPropsErr
+	}
 
 	vnet := network.VirtualNetwork{
 		Name:                           &name,
 		Location:                       &location,
-		VirtualNetworkPropertiesFormat: getVirtualNetworkProperties(d),
+		VirtualNetworkPropertiesFormat: vnetProperties,
 		Tags: expandTags(tags),
 	}
 
@@ -212,7 +217,7 @@ func resourceArmVirtualNetworkDelete(d *schema.ResourceData, meta interface{}) e
 	return err
 }
 
-func getVirtualNetworkProperties(d *schema.ResourceData) *network.VirtualNetworkPropertiesFormat {
+func getVirtualNetworkProperties(d *schema.ResourceData, meta interface{}) (*network.VirtualNetworkPropertiesFormat, error) {
 	// first; get address space prefixes:
 	prefixes := []string{}
 	for _, prefix := range d.Get("address_space").([]interface{}) {
@@ -232,26 +237,41 @@ func getVirtualNetworkProperties(d *schema.ResourceData) *network.VirtualNetwork
 			subnet := subnet.(map[string]interface{})
 
 			name := subnet["name"].(string)
+			log.Printf("[INFO] setting subnets inside vNet, processing %q", name)
+			//since subnets can also be created outside of vNet definition (as root objects)
+			// do a GET on subnet properties from the server before setting them
+			resGroup := d.Get("resource_group_name").(string)
+			vnetName := d.Get("name").(string)
+			subnetObj, err := getExistingSubnet(resGroup, vnetName, name, meta)
+			if err != nil {
+				return nil, err
+			}
+			log.Printf("[INFO] Completed GET of Subnet props ")
+
 			prefix := subnet["address_prefix"].(string)
 			secGroup := subnet["security_group"].(string)
 
-			var subnetObj network.Subnet
+			//set the props from config and leave the rest intact
 			subnetObj.Name = &name
-			subnetObj.SubnetPropertiesFormat = &network.SubnetPropertiesFormat{}
+			if subnetObj.SubnetPropertiesFormat == nil {
+				subnetObj.SubnetPropertiesFormat = &network.SubnetPropertiesFormat{}
+			}
+
 			subnetObj.SubnetPropertiesFormat.AddressPrefix = &prefix
 
 			if secGroup != "" {
 				subnetObj.SubnetPropertiesFormat.NetworkSecurityGroup = &network.SecurityGroup{
 					ID: &secGroup,
 				}
+			} else {
+				subnetObj.SubnetPropertiesFormat.NetworkSecurityGroup = nil
 			}
 
-			subnets = append(subnets, subnetObj)
+			subnets = append(subnets, *subnetObj)
 		}
 	}
 
-	// finally; return the struct:
+	properties := &network.VirtualNetworkPropertiesFormat{
-	return &network.VirtualNetworkPropertiesFormat{
 		AddressSpace: &network.AddressSpace{
 			AddressPrefixes: &prefixes,
 		},
@@ -260,15 +280,56 @@ func getVirtualNetworkProperties(d *schema.ResourceData) *network.VirtualNetwork
 		},
 		Subnets: &subnets,
 	}
+	// finally; return the struct:
+	return properties, nil
 }
 
 func resourceAzureSubnetHash(v interface{}) int {
+	var buf bytes.Buffer
 	m := v.(map[string]interface{})
-	subnet := m["name"].(string) + m["address_prefix"].(string)
+	buf.WriteString(fmt.Sprintf("%s", m["name"].(string)))
-	if securityGroup, present := m["security_group"]; present {
+	buf.WriteString(fmt.Sprintf("%s", m["address_prefix"].(string)))
-		subnet = subnet + securityGroup.(string)
+	if v, ok := m["security_group"]; ok {
+		buf.WriteString(v.(string))
 	}
-	return hashcode.String(subnet)
+	return hashcode.String(buf.String())
+}
+
+func getExistingSubnet(resGroup string, vnetName string, subnetName string, meta interface{}) (*network.Subnet, error) {
+	//attempt to retrieve existing subnet from the server
+	existingSubnet := network.Subnet{}
+	subnetClient := meta.(*ArmClient).subnetClient
+	resp, err := subnetClient.Get(resGroup, vnetName, subnetName, "")
+
+	if err != nil {
+		if resp.StatusCode == http.StatusNotFound {
+			return &existingSubnet, nil
+		}
+		//raise an error if there was an issue other than 404 in getting subnet properties
+		return nil, err
+	}
+
+	existingSubnet.SubnetPropertiesFormat = &network.SubnetPropertiesFormat{}
+	existingSubnet.SubnetPropertiesFormat.AddressPrefix = resp.SubnetPropertiesFormat.AddressPrefix
+
+	if resp.SubnetPropertiesFormat.NetworkSecurityGroup != nil {
+		existingSubnet.SubnetPropertiesFormat.NetworkSecurityGroup = resp.SubnetPropertiesFormat.NetworkSecurityGroup
+	}
+
+	if resp.SubnetPropertiesFormat.RouteTable != nil {
+		existingSubnet.SubnetPropertiesFormat.RouteTable = resp.SubnetPropertiesFormat.RouteTable
+	}
+
+	if resp.SubnetPropertiesFormat.IPConfigurations != nil {
+		ips := make([]string, 0, len(*resp.SubnetPropertiesFormat.IPConfigurations))
+		for _, ip := range *resp.SubnetPropertiesFormat.IPConfigurations {
+			ips = append(ips, *ip.ID)
+		}
+
+		existingSubnet.SubnetPropertiesFormat.IPConfigurations = resp.SubnetPropertiesFormat.IPConfigurations
+	}
+
+	return &existingSubnet, nil
 }
 
 func expandAzureRmVirtualNetworkVirtualNetworkSecurityGroupNames(d *schema.ResourceData) ([]string, error) {

builtin/providers/azurerm/resource_arm_virtual_network_test.go

@@ -209,8 +209,8 @@ resource "azurerm_virtual_network" "test" {
     }
 
     tags {
-	environment = "Production"
+		environment = "Production"
-	cost_center = "MSFT"
+		cost_center = "MSFT"
     }
 }
 `
@@ -233,7 +233,7 @@ resource "azurerm_virtual_network" "test" {
     }
 
     tags {
-	environment = "staging"
+		environment = "staging"
     }
 }
 `