Refactoring of Azure Firewall:

- Locking on the Firewall Name
- Handling resources being deleted outside of Terraform
- Removing some crash points
- Making the Protocol and Action type case-sensitive
- Refactoring the virtual resource to allow for
- Parsing the ID rather than using the config for the delete and read functions (so delete's are successful when the config's gone)
- Rewriting some of the tests for the Network Rule Collections, to check the resource's state rather than the object
- Updating the documentation (and including Import support for Network Rule Collections)

azurerm/helpers/azure/firewall.go

@@ -0,0 +1,42 @@
+package azure
+
+import (
+	"fmt"
+
+	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-04-01/network"
+)
+
+// The API requires InternalPublicIPAddress to be set when for a CreateOrUpdate
+// operation, but Get operations return the property as PublicIPAddress
+// so we need to go through and copy the value to the correct property.
+func FixFirewallIPConfiguration(input *[]network.AzureFirewallIPConfiguration) (*[]network.AzureFirewallIPConfiguration, error) {
+	if input == nil {
+		return nil, fmt.Errorf("`input` was nil")
+	}
+
+	results := make([]network.AzureFirewallIPConfiguration, 0)
+	for _, config := range *input {
+		if config.Subnet == nil || config.Subnet.ID == nil {
+			return nil, fmt.Errorf("`config.Subnet.ID` was nil")
+		}
+
+		if config.PublicIPAddress == nil || config.PublicIPAddress.ID == nil {
+			return nil, fmt.Errorf("`config.PublicIPAddress.ID` was nil")
+		}
+
+		result := network.AzureFirewallIPConfiguration{
+			Name: config.Name,
+			AzureFirewallIPConfigurationPropertiesFormat: &network.AzureFirewallIPConfigurationPropertiesFormat{
+				Subnet: &network.SubResource{
+					ID: config.Subnet.ID,
+				},
+				InternalPublicIPAddress: &network.SubResource{
+					ID: config.PublicIPAddress.ID,
+				},
+			},
+		}
+		results = append(results, result)
+	}
+
+	return &results, nil
+}

azurerm/resource_arm_firewall.go

@@ -43,10 +43,6 @@ func resourceArmFirewall() *schema.Resource {
 							Type:     schema.TypeString,
 							Required: true,
 						},
-						"private_ip_address": {
-							Type:     schema.TypeString,
-							Computed: true,
-						},
 						"subnet_id": {
 							Type:         schema.TypeString,
 							Required:     true,
@@ -57,6 +53,10 @@ func resourceArmFirewall() *schema.Resource {
 							Required:     true,
 							ValidateFunc: azure.ValidateResourceID,
 						},
+						"private_ip_address": {
+							Type:     schema.TypeString,
+							Computed: true,
+						},
 					},
 				},
 			},
@@ -72,15 +72,18 @@ func resourceArmFirewallCreateUpdate(d *schema.ResourceData, meta interface{}) e
 
 	log.Printf("[INFO] preparing arguments for AzureRM Azure Firewall creation")
 
-	resourceGroup := d.Get("resource_group_name").(string)
 	name := d.Get("name").(string)
 	location := azureRMNormalizeLocation(d.Get("location").(string))
+	resourceGroup := d.Get("resource_group_name").(string)
 	tags := d.Get("tags").(map[string]interface{})
 	ipConfigs, subnetToLock, vnetToLock, err := expandArmFirewallIPConfigurations(d)
 	if err != nil {
 		return fmt.Errorf("Error Building list of Azure Firewall IP Configurations: %+v", err)
 	}
 
+	azureRMLockByName(name, azureFirewallResourceName)
+	defer azureRMUnlockByName(name, azureFirewallResourceName)
+
 	azureRMLockMultipleByName(subnetToLock, subnetResourceName)
 	defer azureRMUnlockMultipleByName(subnetToLock, subnetResourceName)
 
@@ -130,29 +133,31 @@ func resourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error {
 	resourceGroup := id.ResourceGroup
 	name := id.Path["azureFirewalls"]
 
-	firewall, err := client.Get(ctx, resourceGroup, name)
+	read, err := client.Get(ctx, resourceGroup, name)
 	if err != nil {
-		if utils.ResponseWasNotFound(firewall.Response) {
+		if utils.ResponseWasNotFound(read.Response) {
+			log.Printf("[DEBUG] Firewall %q was not found in Resource Group %q - removing from state!", name, resourceGroup)
 			d.SetId("")
 			return nil
 		}
+
 		return fmt.Errorf("Error making Read request on Azure Firewall %q (Resource Group %q): %+v", name, resourceGroup, err)
 	}
 
-	if props := firewall.AzureFirewallPropertiesFormat; props != nil {
+	d.Set("name", read.Name)
+	d.Set("resource_group_name", resourceGroup)
+	if location := read.Location; location != nil {
+		d.Set("location", azureRMNormalizeLocation(*location))
+	}
+
+	if props := read.AzureFirewallPropertiesFormat; props != nil {
 		ipConfigs := flattenArmFirewallIPConfigurations(props.IPConfigurations)
 		if err := d.Set("ip_configuration", ipConfigs); err != nil {
 			return fmt.Errorf("Error setting `ip_configuration`: %+v", err)
 		}
 	}
 
-	d.Set("name", name)
-	d.Set("resource_group_name", resourceGroup)
-	if location := firewall.Location; location != nil {
-		d.Set("location", azureRMNormalizeLocation(*location))
-	}
-
-	flattenAndSetTags(d, firewall.Tags)
+	flattenAndSetTags(d, read.Tags)
 
 	return nil
 }
@@ -168,29 +173,47 @@ func resourceArmFirewallDelete(d *schema.ResourceData, meta interface{}) error {
 	resourceGroup := id.ResourceGroup
 	name := id.Path["azureFirewalls"]
 
-	configs := d.Get("ip_configuration").([]interface{})
-	subnetNamesToLock := make([]string, 0)
-	virtualNetworkNamesToLock := make([]string, 0)
-
-	for _, configRaw := range configs {
-		data := configRaw.(map[string]interface{})
-
-		subnet_id := data["subnet_id"].(string)
-		subnetID, err := parseAzureResourceID(subnet_id)
-		if err != nil {
-			return err
-		}
-		subnetName := subnetID.Path["subnets"]
-		if !sliceContainsValue(subnetNamesToLock, subnetName) {
-			subnetNamesToLock = append(subnetNamesToLock, subnetName)
+	read, err := client.Get(ctx, resourceGroup, name)
+	if err != nil {
+		if utils.ResponseWasNotFound(read.Response) {
+			// deleted outside of TF
+			log.Printf("[DEBUG] Firewall %q was not found in Resource Group %q - assuming removed!", name, resourceGroup)
+			return nil
 		}
 
-		virtualNetworkName := subnetID.Path["virtualNetworks"]
-		if !sliceContainsValue(virtualNetworkNamesToLock, virtualNetworkName) {
-			virtualNetworkNamesToLock = append(virtualNetworkNamesToLock, virtualNetworkName)
+		return fmt.Errorf("Error retrieving Firewall %q (Resource Group %q): %+v", name, resourceGroup, err)
+	}
+
+	subnetNamesToLock := make([]string, 0)
+	virtualNetworkNamesToLock := make([]string, 0)
+	if props := read.AzureFirewallPropertiesFormat; props != nil {
+		if configs := props.IPConfigurations; configs != nil {
+			for _, config := range *configs {
+				if config.Subnet == nil || config.Subnet.ID == nil {
+					continue
+				}
+
+				parsedSubnetId, err := parseAzureResourceID(*config.Subnet.ID)
+				if err != nil {
+					return err
+				}
+				subnetName := parsedSubnetId.Path["subnets"]
+
+				if !sliceContainsValue(subnetNamesToLock, subnetName) {
+					subnetNamesToLock = append(subnetNamesToLock, subnetName)
+				}
+
+				virtualNetworkName := parsedSubnetId.Path["virtualNetworks"]
+				if !sliceContainsValue(virtualNetworkNamesToLock, virtualNetworkName) {
+					virtualNetworkNamesToLock = append(virtualNetworkNamesToLock, virtualNetworkName)
+				}
+			}
 		}
 	}
 
+	azureRMLockByName(name, azureFirewallResourceName)
+	defer azureRMUnlockByName(name, azureFirewallResourceName)
+
 	azureRMLockMultipleByName(&subnetNamesToLock, subnetResourceName)
 	defer azureRMUnlockMultipleByName(&subnetNamesToLock, subnetResourceName)
 
@@ -212,26 +235,17 @@ func resourceArmFirewallDelete(d *schema.ResourceData, meta interface{}) error {
 
 func expandArmFirewallIPConfigurations(d *schema.ResourceData) (*[]network.AzureFirewallIPConfiguration, *[]string, *[]string, error) {
 	configs := d.Get("ip_configuration").([]interface{})
-	ipConfigs := make([]network.AzureFirewallIPConfiguration, 0, len(configs))
+	ipConfigs := make([]network.AzureFirewallIPConfiguration, 0)
 	subnetNamesToLock := make([]string, 0)
 	virtualNetworkNamesToLock := make([]string, 0)
 
 	for _, configRaw := range configs {
 		data := configRaw.(map[string]interface{})
 		name := data["name"].(string)
-		subnet_id := data["subnet_id"].(string)
+		subnetId := data["subnet_id"].(string)
 		intPubID := data["internal_public_ip_address_id"].(string)
 
-		properties := network.AzureFirewallIPConfigurationPropertiesFormat{
-			Subnet: &network.SubResource{
-				ID: &subnet_id,
-			},
-			InternalPublicIPAddress: &network.SubResource{
-				ID: &intPubID,
-			},
-		}
-
-		subnetID, err := parseAzureResourceID(subnet_id)
+		subnetID, err := parseAzureResourceID(subnetId)
 		if err != nil {
 			return nil, nil, nil, err
 		}
@@ -248,27 +262,35 @@ func expandArmFirewallIPConfigurations(d *schema.ResourceData) (*[]network.Azure
 		}
 
 		ipConfig := network.AzureFirewallIPConfiguration{
-			Name: &name,
-			AzureFirewallIPConfigurationPropertiesFormat: &properties,
+			Name: utils.String(name),
+			AzureFirewallIPConfigurationPropertiesFormat: &network.AzureFirewallIPConfigurationPropertiesFormat{
+				Subnet: &network.SubResource{
+					ID: utils.String(subnetId),
+				},
+				InternalPublicIPAddress: &network.SubResource{
+					ID: utils.String(intPubID),
+				},
+			},
 		}
 		ipConfigs = append(ipConfigs, ipConfig)
 	}
 	return &ipConfigs, &subnetNamesToLock, &virtualNetworkNamesToLock, nil
 }
 
-func flattenArmFirewallIPConfigurations(ipConfigs *[]network.AzureFirewallIPConfiguration) []interface{} {
+func flattenArmFirewallIPConfigurations(input *[]network.AzureFirewallIPConfiguration) []interface{} {
 	result := make([]interface{}, 0)
-	if ipConfigs == nil {
+	if input == nil {
 		return result
 	}
-	for _, ipConfig := range *ipConfigs {
+
+	for _, v := range *input {
 		afIPConfig := make(map[string]interface{})
-		props := ipConfig.AzureFirewallIPConfigurationPropertiesFormat
+		props := v.AzureFirewallIPConfigurationPropertiesFormat
 		if props == nil {
 			continue
 		}
 
-		if name := ipConfig.Name; name != nil {
+		if name := v.Name; name != nil {
 			afIPConfig["name"] = *name
 		}