diff --git a/azurerm/data_source_firewall.go b/azurerm/data_source_firewall.go index 1aa87178b712..d8969c80b2b0 100644 --- a/azurerm/data_source_firewall.go +++ b/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) } } diff --git a/azurerm/resource_arm_firewall.go b/azurerm/resource_arm_firewall.go index cc42f797d587..66a663207b75 100644 --- a/azurerm/resource_arm_firewall.go +++ b/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 in favour of the `public_ip_address_id` property to better match the Azure SDK.", }, "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, @@ -119,6 +116,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) @@ -215,8 +216,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) } } @@ -316,36 +316,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 @@ -415,3 +418,21 @@ 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 + + for _, configRaw := range configs { + data := configRaw.(map[string]interface{}) + if subnet, exist := data["subnet_id"].(string); exist && subnet != "" { + subnetNumber++ + } + } + + 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) + } + + return nil +} diff --git a/azurerm/resource_arm_firewall_test.go b/azurerm/resource_arm_firewall_test.go index 242c530d31fa..a8ae8cdcae66 100644 --- a/azurerm/resource_arm_firewall_test.go +++ b/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") @@ -402,6 +431,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(` diff --git a/website/docs/r/firewall.html.markdown b/website/docs/r/firewall.html.markdown index e0ff3e0b92dd..321cf86ae792 100644 --- a/website/docs/r/firewall.html.markdown +++ b/website/docs/r/firewall.html.markdown @@ -79,10 +79,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.