azurerm_firewall: allow multiple ip_configuration blocks

azurerm/data_source_firewall.go

@@ -35,7 +35,6 @@ func dataSourceArmFirewall() *schema.Resource {
 			"ip_configuration": {
 				Type:     schema.TypeList,
 				Computed: true,
-				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"name": {
@@ -95,8 +94,7 @@ func dataSourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	if props := read.AzureFirewallPropertiesFormat; props != nil {
-		ipConfigs := flattenArmFirewallIPConfigurations(props.IPConfigurations)
-		if err := d.Set("ip_configuration", ipConfigs); err != nil {
+		if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil {
 			return fmt.Errorf("Error setting `ip_configuration`: %+v", err)
 		}
 	}

azurerm/resource_arm_firewall.go

@@ -52,7 +52,6 @@ func resourceArmFirewall() *schema.Resource {
 			"ip_configuration": {
 				Type:     schema.TypeList,
 				Required: true,
-				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"name": {
@@ -62,24 +61,22 @@ func resourceArmFirewall() *schema.Resource {
 						},
 						"subnet_id": {
 							Type:         schema.TypeString,
-							Required:     true,
+							Optional:     true,
 							ForceNew:     true,
 							ValidateFunc: validateAzureFirewallSubnetName,
 						},
 						"internal_public_ip_address_id": {
-							Type:          schema.TypeString,
-							Optional:      true,
-							Computed:      true,
-							ValidateFunc:  azure.ValidateResourceID,
-							Deprecated:    "This field has been deprecated. Use `public_ip_address_id` instead.",
-							ConflictsWith: []string{"ip_configuration.0.public_ip_address_id"},
+							Type:         schema.TypeString,
+							Optional:     true,
+							Computed:     true,
+							ValidateFunc: azure.ValidateResourceID,
+							Deprecated:   "This field has been deprecated. Use `public_ip_address_id` instead.",
 						},
 						"public_ip_address_id": {
-							Type:          schema.TypeString,
-							Optional:      true,
-							Computed:      true,
-							ValidateFunc:  azure.ValidateResourceID,
-							ConflictsWith: []string{"ip_configuration.0.internal_public_ip_address_id"},
+							Type:         schema.TypeString,
+							Optional:     true,
+							Computed:     true,
+							ValidateFunc: azure.ValidateResourceID,
 						},
 						"private_ip_address": {
 							Type:     schema.TypeString,
@@ -117,6 +114,10 @@ func resourceArmFirewallCreateUpdate(d *schema.ResourceData, meta interface{}) e
 		}
 	}
 
+	if err := validateFirewallConfigurationSettings(d); err != nil {
+		return fmt.Errorf("Error validating Firewall %q (Resource Group %q): %+v", name, resourceGroup, err)
+	}
+
 	location := azure.NormalizeLocation(d.Get("location").(string))
 	t := d.Get("tags").(map[string]interface{})
 	ipConfigs, subnetToLock, vnetToLock, err := expandArmFirewallIPConfigurations(d)
@@ -211,8 +212,7 @@ func resourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	if props := read.AzureFirewallPropertiesFormat; props != nil {
-		ipConfigs := flattenArmFirewallIPConfigurations(props.IPConfigurations)
-		if err := d.Set("ip_configuration", ipConfigs); err != nil {
+		if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil {
 			return fmt.Errorf("Error setting `ip_configuration`: %+v", err)
 		}
 	}
@@ -308,36 +308,39 @@ func expandArmFirewallIPConfigurations(d *schema.ResourceData) (*[]network.Azure
 		}
 
 		if !exist || pubID == "" {
-			return nil, nil, nil, fmt.Errorf("one of `ip_configuration.0.internal_public_ip_address_id` or `ip_configuration.0.public_ip_address_id` must be set")
-		}
-
-		subnetID, err := azure.ParseAzureResourceID(subnetId)
-		if err != nil {
-			return nil, nil, nil, err
-		}
-
-		subnetName := subnetID.Path["subnets"]
-		virtualNetworkName := subnetID.Path["virtualNetworks"]
-
-		if !sliceContainsValue(subnetNamesToLock, subnetName) {
-			subnetNamesToLock = append(subnetNamesToLock, subnetName)
-		}
-
-		if !sliceContainsValue(virtualNetworkNamesToLock, virtualNetworkName) {
-			virtualNetworkNamesToLock = append(virtualNetworkNamesToLock, virtualNetworkName)
+			return nil, nil, nil, fmt.Errorf("one of `internal_public_ip_address_id` or `public_ip_address_id` must be set")
 		}
 
 		ipConfig := network.AzureFirewallIPConfiguration{
 			Name: utils.String(name),
 			AzureFirewallIPConfigurationPropertiesFormat: &network.AzureFirewallIPConfigurationPropertiesFormat{
-				Subnet: &network.SubResource{
-					ID: utils.String(subnetId),
-				},
 				PublicIPAddress: &network.SubResource{
 					ID: utils.String(pubID),
 				},
 			},
 		}
+
+		if subnetId != "" {
+			subnetID, err := azure.ParseAzureResourceID(subnetId)
+			if err != nil {
+				return nil, nil, nil, err
+			}
+
+			subnetName := subnetID.Path["subnets"]
+			virtualNetworkName := subnetID.Path["virtualNetworks"]
+
+			if !sliceContainsValue(subnetNamesToLock, subnetName) {
+				subnetNamesToLock = append(subnetNamesToLock, subnetName)
+			}
+
+			if !sliceContainsValue(virtualNetworkNamesToLock, virtualNetworkName) {
+				virtualNetworkNamesToLock = append(virtualNetworkNamesToLock, virtualNetworkName)
+			}
+
+			ipConfig.AzureFirewallIPConfigurationPropertiesFormat.Subnet = &network.SubResource{
+				ID: utils.String(subnetId),
+			}
+		}
 		ipConfigs = append(ipConfigs, ipConfig)
 	}
 	return &ipConfigs, &subnetNamesToLock, &virtualNetworkNamesToLock, nil
@@ -407,3 +410,35 @@ func validateAzureFirewallSubnetName(v interface{}, k string) (warnings []string
 
 	return warnings, errors
 }
+
+func validateFirewallConfigurationSettings(d *schema.ResourceData) error {
+	configs := d.Get("ip_configuration").([]interface{})
+	subnetNumber := 0
+	hasInternal := false
+	hasPublic := false
+
+	for _, configRaw := range configs {
+		data := configRaw.(map[string]interface{})
+		if subnet, exist := data["subnet_id"].(string); exist && subnet != "" {
+			subnetNumber++
+		}
+
+		if internal, exist := data["internal_public_ip_address_id"].(string); exist && internal != "" {
+			hasInternal = true
+		}
+
+		if public, exist := data["public_ip_address_id"].(string); exist && public != "" {
+			hasPublic = true
+		}
+	}
+
+	if subnetNumber != 1 {
+		return fmt.Errorf(`The "ip_configuration" is invalid, %d "subnet_id" have been set, one "subnet_id" should be set among all "ip_configuration" blocks`, subnetNumber)
+	}
+
+	if hasInternal && hasPublic {
+		return fmt.Errorf(`The "ip_configuration" is invalid, both "public_ip_address_id" and "internal_public_ip_address_id" have been set, all defined "ip_configuration" blocks must use the same attribute name.`)
+	}
+
+	return nil
+}

azurerm/resource_arm_firewall_test.go

@@ -110,6 +110,35 @@ func TestAccAzureRMFirewall_basic(t *testing.T) {
 	})
 }
 
+func TestAccAzureRMFirewall_withMultiplePublicIPs(t *testing.T) {
+	resourceName := "azurerm_firewall.test"
+	ri := tf.AccRandTimeInt()
+	location := testLocation()
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMFirewallDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAzureRMFirewall_multiplePublicIps(ri, location),
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMFirewallExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "ip_configuration.0.name", "configuration"),
+					resource.TestCheckResourceAttrSet(resourceName, "ip_configuration.0.private_ip_address"),
+					resource.TestCheckResourceAttr(resourceName, "ip_configuration.1.name", "configuration_2"),
+					resource.TestCheckResourceAttrSet(resourceName, "ip_configuration.1.public_ip_address_id"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccAzureRMFirewall_requiresImport(t *testing.T) {
 	if !features.ShouldResourcesBeImported() {
 		t.Skip("Skipping since resources aren't required to be imported")
@@ -368,6 +397,62 @@ resource "azurerm_firewall" "test" {
 `, rInt, location, rInt, rInt, rInt)
 }
 
+func testAccAzureRMFirewall_multiplePublicIps(rInt int, location string) string {
+	return fmt.Sprintf(`
+resource "azurerm_resource_group" "test" {
+  name     = "acctestRG-%d"
+  location = "%s"
+}
+
+resource "azurerm_virtual_network" "test" {
+  name                = "acctestvirtnet%d"
+  address_space       = ["10.0.0.0/16"]
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+}
+
+resource "azurerm_subnet" "test" {
+  name                 = "AzureFirewallSubnet"
+  resource_group_name  = "${azurerm_resource_group.test.name}"
+  virtual_network_name = "${azurerm_virtual_network.test.name}"
+  address_prefix       = "10.0.1.0/24"
+}
+
+resource "azurerm_public_ip" "test" {
+  name                = "acctestpip%d"
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+  allocation_method   = "Static"
+  sku                 = "Standard"
+}
+
+resource "azurerm_public_ip" "test_2" {
+  name                = "acctestpip2%d"
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+  allocation_method   = "Static"
+  sku                 = "Standard"
+}
+
+resource "azurerm_firewall" "test" {
+  name                = "acctestfirewall%d"
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+
+  ip_configuration {
+    name                 = "configuration"
+    subnet_id            = "${azurerm_subnet.test.id}"
+    public_ip_address_id = "${azurerm_public_ip.test.id}"
+  }
+
+  ip_configuration {
+    name                 = "configuration_2"
+    public_ip_address_id = "${azurerm_public_ip.test_2.id}"
+  }
+}
+`, rInt, location, rInt, rInt, rInt, rInt)
+}
+
 func testAccAzureRMFirewall_requiresImport(rInt int, location string) string {
 	template := testAccAzureRMFirewall_basic(rInt, location)
 	return fmt.Sprintf(`

website/docs/r/firewall.html.markdown

@@ -74,10 +74,12 @@ A `ip_configuration` block supports the following:
 
 * `name` - (Required) Specifies the name of the IP Configuration.
 
-* `subnet_id` - (Required) Reference to the subnet associated with the IP Configuration.
+* `subnet_id` - (Optional) Reference to the subnet associated with the IP Configuration.
 
 -> **NOTE** The Subnet used for the Firewall must have the name `AzureFirewallSubnet` and the subnet mask must be at least `/26`.
 
+-> **NOTE** At least one and only one `ip_configuration` block may contain a `subnet_id`.
+
 * `public_ip_address_id` - (Required) The Resource ID of the Public IP Address associated with the firewall.
 
 -> **NOTE** The Public IP must have a `Static` allocation and `Standard` sku.