Bug Fix: azurerm_express_route_circuit (#3496)

* Fixing express route circuit

* UPpdate comment

azurerm/resource_arm_express_route_circuit.go

@@ -113,6 +113,9 @@ func resourceArmExpressRouteCircuitCreateUpdate(d *schema.ResourceData, meta int
 	name := d.Get("name").(string)
 	resGroup := d.Get("resource_group_name").(string)
 
+	azureRMLockByName(name, expressRouteCircuitResourceName)
+	defer azureRMUnlockByName(name, expressRouteCircuitResourceName)
+
 	if requireResourcesToBeImported && d.IsNewResource() {
 		existing, err := client.Get(ctx, resGroup, name)
 		if err != nil {
@@ -135,24 +138,53 @@ func resourceArmExpressRouteCircuitCreateUpdate(d *schema.ResourceData, meta int
 	tags := d.Get("tags").(map[string]interface{})
 	expandedTags := expandTags(tags)
 
-	erc := network.ExpressRouteCircuit{
-		Name:     &name,
-		Location: &location,
-		Sku:      sku,
-		ExpressRouteCircuitPropertiesFormat: &network.ExpressRouteCircuitPropertiesFormat{
+	// There is the potential for the express route circuit to become out of sync when the service provider updates
+	// the express route circuit. We'll get and update the resource in place as per https://aka.ms/erRefresh
+	// We also want to keep track of the resource obtained from the api and pass down any attributes not
+	// managed by Terraform.
+	erc := network.ExpressRouteCircuit{}
+	if !d.IsNewResource() {
+		existing, err := client.Get(ctx, resGroup, name)
+		if err != nil {
+			if !utils.ResponseWasNotFound(erc.Response) {
+				return fmt.Errorf("Error checking for presence of existing ExpressRoute Circuit %q (Resource Group %q): %s", name, resGroup, err)
+			}
+		}
+
+		future, err := client.CreateOrUpdate(ctx, resGroup, name, existing)
+		if err != nil {
+			return fmt.Errorf("Error Creating/Updating ExpressRouteCircuit %q (Resource Group %q): %+v", name, resGroup, err)
+		}
+
+		if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
+			return fmt.Errorf("Error Creating/Updating ExpressRouteCircuit %q (Resource Group %q): %+v", name, resGroup, err)
+		}
+		erc = existing
+	}
+
+	erc.Name = &name
+	erc.Location = &location
+	erc.Sku = sku
+	erc.Tags = expandedTags
+
+	if erc.ExpressRouteCircuitPropertiesFormat != nil {
+		erc.ExpressRouteCircuitPropertiesFormat.AllowClassicOperations = &allowRdfeOps
+		if erc.ExpressRouteCircuitPropertiesFormat.ServiceProviderProperties != nil {
+			erc.ExpressRouteCircuitPropertiesFormat.ServiceProviderProperties.ServiceProviderName = &serviceProviderName
+			erc.ExpressRouteCircuitPropertiesFormat.ServiceProviderProperties.PeeringLocation = &peeringLocation
+			erc.ExpressRouteCircuitPropertiesFormat.ServiceProviderProperties.BandwidthInMbps = &bandwidthInMbps
+		}
+	} else {
+		erc.ExpressRouteCircuitPropertiesFormat = &network.ExpressRouteCircuitPropertiesFormat{
 			AllowClassicOperations: &allowRdfeOps,
 			ServiceProviderProperties: &network.ExpressRouteCircuitServiceProviderProperties{
 				ServiceProviderName: &serviceProviderName,
 				PeeringLocation:     &peeringLocation,
 				BandwidthInMbps:     &bandwidthInMbps,
 			},
-		},
-		Tags: expandedTags,
+		}
 	}
 
-	azureRMLockByName(name, expressRouteCircuitResourceName)
-	defer azureRMUnlockByName(name, expressRouteCircuitResourceName)
-
 	future, err := client.CreateOrUpdate(ctx, resGroup, name, erc)
 	if err != nil {
 		return fmt.Errorf("Error Creating/Updating ExpressRouteCircuit %q (Resource Group %q): %+v", name, resGroup, err)

azurerm/resource_arm_express_route_circuit_authorization.go

@@ -56,6 +56,9 @@ func resourceArmExpressRouteCircuitAuthorizationCreate(d *schema.ResourceData, m
 	resourceGroup := d.Get("resource_group_name").(string)
 	circuitName := d.Get("express_route_circuit_name").(string)
 
+	azureRMLockByName(circuitName, expressRouteCircuitResourceName)
+	defer azureRMUnlockByName(circuitName, expressRouteCircuitResourceName)
+
 	if requireResourcesToBeImported && d.IsNewResource() {
 		existing, err := client.Get(ctx, resourceGroup, circuitName, name)
 		if err != nil {
@@ -73,9 +76,6 @@ func resourceArmExpressRouteCircuitAuthorizationCreate(d *schema.ResourceData, m
 		AuthorizationPropertiesFormat: &network.AuthorizationPropertiesFormat{},
 	}
 
-	azureRMLockByName(circuitName, expressRouteCircuitResourceName)
-	defer azureRMUnlockByName(circuitName, expressRouteCircuitResourceName)
-
 	future, err := client.CreateOrUpdate(ctx, resourceGroup, circuitName, name, properties)
 	if err != nil {
 		return fmt.Errorf("Error Creating/Updating Express Route Circuit Authorization %q (Circuit %q / Resource Group %q): %+v", name, circuitName, resourceGroup, err)

azurerm/resource_arm_express_route_circuit_peering.go

@@ -114,6 +114,9 @@ func resourceArmExpressRouteCircuitPeeringCreateUpdate(d *schema.ResourceData, m
 	circuitName := d.Get("express_route_circuit_name").(string)
 	resourceGroup := d.Get("resource_group_name").(string)
 
+	azureRMLockByName(circuitName, expressRouteCircuitResourceName)
+	defer azureRMUnlockByName(circuitName, expressRouteCircuitResourceName)
+
 	if requireResourcesToBeImported && d.IsNewResource() {
 		existing, err := client.Get(ctx, resourceGroup, circuitName, peeringType)
 		if err != nil {
@@ -156,9 +159,6 @@ func resourceArmExpressRouteCircuitPeeringCreateUpdate(d *schema.ResourceData, m
 		parameters.ExpressRouteCircuitPeeringPropertiesFormat.MicrosoftPeeringConfig = peeringConfig
 	}
 
-	azureRMLockByName(circuitName, expressRouteCircuitResourceName)
-	defer azureRMUnlockByName(circuitName, expressRouteCircuitResourceName)
-
 	future, err := client.CreateOrUpdate(ctx, resourceGroup, circuitName, peeringType, parameters)
 	if err != nil {
 		return err

azurerm/resource_arm_express_route_circuit_peering_test.go

@@ -96,6 +96,42 @@ func testAccAzureRMExpressRouteCircuitPeering_microsoftPeering(t *testing.T) {
 	})
 }
 
+func testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeeringWithCircuitUpdate(t *testing.T) {
+	resourceName := "azurerm_express_route_circuit_peering.test"
+	ri := tf.AccRandTimeInt()
+	location := testLocation()
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMExpressRouteCircuitPeeringDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAzureRMExpressRouteCircuitPeering_privatePeering(ri, location),
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMExpressRouteCircuitPeeringExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "peering_type", "AzurePrivatePeering"),
+					resource.TestCheckResourceAttr(resourceName, "microsoft_peering_config.#", "0"),
+				),
+			},
+			{
+				Config: testAccAzureRMExpressRouteCircuitPeering_privatePeeringWithCircuitUpdate(ri, location),
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMExpressRouteCircuitPeeringExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "peering_type", "AzurePrivatePeering"),
+					resource.TestCheckResourceAttr(resourceName, "microsoft_peering_config.#", "0"),
+				),
+			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"shared_key"}, //is not returned by the API
+			},
+		},
+	})
+}
+
 func testCheckAzureRMExpressRouteCircuitPeeringExists(resourceName string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[resourceName]
@@ -251,3 +287,42 @@ resource "azurerm_express_route_circuit_peering" "test" {
 }
 `, rInt, location, rInt)
 }
+
+func testAccAzureRMExpressRouteCircuitPeering_privatePeeringWithCircuitUpdate(rInt int, location string) string {
+	return fmt.Sprintf(`
+resource "azurerm_resource_group" "test" {
+  name     = "acctestRG-%d"
+  location = "%s"
+}
+
+resource "azurerm_express_route_circuit" "test" {
+  name                  = "acctest-erc-%d"
+  location              = "${azurerm_resource_group.test.location}"
+  resource_group_name   = "${azurerm_resource_group.test.name}"
+  service_provider_name = "Equinix"
+  peering_location      = "Silicon Valley"
+  bandwidth_in_mbps     = 50
+
+  sku {
+    tier   = "Standard"
+    family = "MeteredData"
+  }
+
+  tags = {
+    Environment = "prod"
+    Purpose     = "AcceptanceTests"
+  }
+}
+
+resource "azurerm_express_route_circuit_peering" "test" {
+  peering_type                  = "AzurePrivatePeering"
+  express_route_circuit_name    = "${azurerm_express_route_circuit.test.name}"
+  resource_group_name           = "${azurerm_resource_group.test.name}"
+  shared_key                    = "SSSSsssssshhhhhItsASecret"
+  peer_asn                      = 100
+  primary_peer_address_prefix   = "192.168.1.0/30"
+  secondary_peer_address_prefix = "192.168.2.0/30"
+  vlan_id                       = 100
+}
+`, rInt, location, rInt)
+}

azurerm/resource_arm_express_route_circuit_test.go

@@ -28,8 +28,9 @@ func TestAccAzureRMExpressRouteCircuit(t *testing.T) {
 			"data_basic":                   testAccDataSourceAzureRMExpressRoute_basicMetered,
 		},
 		"PrivatePeering": {
-			"azurePrivatePeering": testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeering,
-			"requiresImport":      testAccAzureRMExpressRouteCircuitPeering_requiresImport,
+			"azurePrivatePeering":           testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeering,
+			"azurePrivatePeeringWithUpdate": testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeeringWithCircuitUpdate,
+			"requiresImport":                testAccAzureRMExpressRouteCircuitPeering_requiresImport,
 		},
 		"MicrosoftPeering": {
 			"microsoftPeering": testAccAzureRMExpressRouteCircuitPeering_microsoftPeering,