Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_firewall: allow multiple ip_configuration blocks #4639

Merged
merged 11 commits into from
Nov 15, 2019
4 changes: 1 addition & 3 deletions azurerm/data_source_firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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)
}
houkms marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down
91 changes: 56 additions & 35 deletions azurerm/resource_arm_firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -62,24 +61,22 @@ func resourceArmFirewall() *schema.Resource {
},
"subnet_id": {
Type: schema.TypeString,
Required: true,
Optional: true,
houkms marked this conversation as resolved.
Show resolved Hide resolved
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.",
},
houkms marked this conversation as resolved.
Show resolved Hide resolved
"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,
},
houkms marked this conversation as resolved.
Show resolved Hide resolved
"private_ip_address": {
Type: schema.TypeString,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
houkms marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -407,3 +410,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 {
houkms marked this conversation as resolved.
Show resolved Hide resolved
data := configRaw.(map[string]interface{})
if subnet, exist := data["subnet_id"].(string); exist && subnet != "" {
subnetNumber++
}
houkms marked this conversation as resolved.
Show resolved Hide resolved
}

if subnetNumber != 1 {
houkms marked this conversation as resolved.
Show resolved Hide resolved
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
}
85 changes: 85 additions & 0 deletions azurerm/resource_arm_firewall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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(`
Expand Down
4 changes: 3 additions & 1 deletion website/docs/r/firewall.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down