From bb2ca94bab72fd7c53f1024244158bba4836497f Mon Sep 17 00:00:00 2001 From: houkms Date: Wed, 16 Oct 2019 20:21:13 +0800 Subject: [PATCH 01/10] adjust ip_configurations in firewall --- azurerm/data_source_firewall.go | 6 +-- azurerm/data_source_firewall_test.go | 6 +-- azurerm/resource_arm_firewall.go | 77 ++++++++++++++------------- azurerm/resource_arm_firewall_test.go | 22 ++++---- website/docs/d/firewall.html.markdown | 6 +-- website/docs/r/firewall.html.markdown | 10 ++-- 6 files changed, 64 insertions(+), 63 deletions(-) diff --git a/azurerm/data_source_firewall.go b/azurerm/data_source_firewall.go index 1aa87178b712..7ba5b3e48b6e 100644 --- a/azurerm/data_source_firewall.go +++ b/azurerm/data_source_firewall.go @@ -32,7 +32,7 @@ func dataSourceArmFirewall() *schema.Resource { "resource_group_name": azure.SchemaResourceGroupNameForDataSource(), - "ip_configuration": { + "ip_configurations": { Type: schema.TypeList, Computed: true, MaxItems: 1, @@ -96,8 +96,8 @@ 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 { - return fmt.Errorf("Error setting `ip_configuration`: %+v", err) + if err := d.Set("ip_configurations", ipConfigs); err != nil { + return fmt.Errorf("Error setting `ip_configurations`: %+v", err) } } diff --git a/azurerm/data_source_firewall_test.go b/azurerm/data_source_firewall_test.go index a27c728f34ac..d70dbdbcc7a4 100644 --- a/azurerm/data_source_firewall_test.go +++ b/azurerm/data_source_firewall_test.go @@ -21,8 +21,8 @@ func TestAccDataSourceAzureRMFirewall_basic(t *testing.T) { { Config: testAccDataSourceFirewall_basic(rInt, location), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(dataSourceName, "ip_configuration.0.name", "configuration"), - resource.TestCheckResourceAttrSet(dataSourceName, "ip_configuration.0.private_ip_address"), + resource.TestCheckResourceAttr(dataSourceName, "ip_configurations.0.name", "configuration"), + resource.TestCheckResourceAttrSet(dataSourceName, "ip_configurations.0.private_ip_address"), ), }, }, @@ -63,7 +63,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configuration { + ip_configurations { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" diff --git a/azurerm/resource_arm_firewall.go b/azurerm/resource_arm_firewall.go index af6d97175c75..9add807d8f56 100644 --- a/azurerm/resource_arm_firewall.go +++ b/azurerm/resource_arm_firewall.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "regexp" + "strconv" "time" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" @@ -49,10 +50,9 @@ func resourceArmFirewall() *schema.Resource { "resource_group_name": azure.SchemaResourceGroupName(), - "ip_configuration": { + "ip_configurations": { Type: schema.TypeList, Required: true, - MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "name": { @@ -62,24 +62,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, @@ -212,8 +210,8 @@ 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 { - return fmt.Errorf("Error setting `ip_configuration`: %+v", err) + if err := d.Set("ip_configurations", ipConfigs); err != nil { + return fmt.Errorf("Error setting `ip_configurations`: %+v", err) } } @@ -292,12 +290,12 @@ func resourceArmFirewallDelete(d *schema.ResourceData, meta interface{}) error { } func expandArmFirewallIPConfigurations(d *schema.ResourceData) (*[]network.AzureFirewallIPConfiguration, *[]string, *[]string, error) { - configs := d.Get("ip_configuration").([]interface{}) + configs := d.Get("ip_configurations").([]interface{}) ipConfigs := make([]network.AzureFirewallIPConfiguration, 0) subnetNamesToLock := make([]string, 0) virtualNetworkNamesToLock := make([]string, 0) - for _, configRaw := range configs { + for index, configRaw := range configs { data := configRaw.(map[string]interface{}) name := data["name"].(string) subnetId := data["subnet_id"].(string) @@ -308,36 +306,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 `ip_configuration.%s.internal_public_ip_address_id` or `ip_configuration.%s.public_ip_address_id` must be set", strconv.Itoa(index), strconv.Itoa(index)) } 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 diff --git a/azurerm/resource_arm_firewall_test.go b/azurerm/resource_arm_firewall_test.go index 4f26ab2b825d..999441c1c0b6 100644 --- a/azurerm/resource_arm_firewall_test.go +++ b/azurerm/resource_arm_firewall_test.go @@ -62,16 +62,16 @@ func TestAccAzureRMFirewall_basicOld(t *testing.T) { Config: testAccAzureRMFirewall_basicOld(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_configurations.0.name", "configuration"), + resource.TestCheckResourceAttrSet(resourceName, "ip_configurations.0.private_ip_address"), ), }, { Config: testAccAzureRMFirewall_basic(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_configurations.0.name", "configuration"), + resource.TestCheckResourceAttrSet(resourceName, "ip_configurations.0.private_ip_address"), ), }, { @@ -97,8 +97,8 @@ func TestAccAzureRMFirewall_basic(t *testing.T) { Config: testAccAzureRMFirewall_basic(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_configurations.0.name", "configuration"), + resource.TestCheckResourceAttrSet(resourceName, "ip_configurations.0.private_ip_address"), ), }, { @@ -316,7 +316,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configuration { + ip_configurations { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" internal_public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -359,7 +359,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configuration { + ip_configurations { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -378,7 +378,7 @@ resource "azurerm_firewall" "import" { location = "${azurerm_firewall.test.location}" resource_group_name = "${azurerm_firewall.test.resource_group_name}" - ip_configuration { + ip_configurations { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -421,7 +421,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configuration { + ip_configurations { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -469,7 +469,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configuration { + ip_configurations { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" diff --git a/website/docs/d/firewall.html.markdown b/website/docs/d/firewall.html.markdown index 960cbaca3af8..8eb724fdfa10 100644 --- a/website/docs/d/firewall.html.markdown +++ b/website/docs/d/firewall.html.markdown @@ -20,7 +20,7 @@ data "azurerm_firewall" "test" { } output "firewall_private_ip" { - value = "${data.azurerm_firewall.test.ip_configuration.0.private_ip_address}" + value = "${data.azurerm_firewall.test.ip_configurations.0.private_ip_address}" } ``` @@ -36,11 +36,11 @@ The following attributes are exported: * `id` - The Resource ID of the Azure Firewall. -* `ip_configuration` - A `ip_configuration` block as defined below. +* `ip_configurations` - A `ip_configurations` block as defined below. --- -A `ip_configuration` block exports the following: +A `ip_configurations` block exports the following: * `subnet_id` - The Resource ID of the subnet where the Azure Firewall is deployed. diff --git a/website/docs/r/firewall.html.markdown b/website/docs/r/firewall.html.markdown index 78b258edeb94..f726d61e3209 100644 --- a/website/docs/r/firewall.html.markdown +++ b/website/docs/r/firewall.html.markdown @@ -46,7 +46,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configuration { + ip_configurations { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -64,13 +64,13 @@ The following arguments are supported: * `location` - (Required) Specifies the supported Azure location where the resource exists. Changing this forces a new resource to be created. -* `ip_configuration` - (Required) A `ip_configuration` block as documented below. +* `ip_configurations` - (Required) A `ip_configurations` block as documented below. * `tags` - (Optional) A mapping of tags to assign to the resource. --- -A `ip_configuration` block supports the following: +A `ip_configurations` block supports the following: * `name` - (Required) Specifies the name of the IP Configuration. @@ -88,11 +88,11 @@ The following attributes are exported: * `id` - The Resource ID of the Azure Firewall. -* `ip_configuration` - A `ip_configuration` block as defined below. +* `ip_configurations` - A `ip_configurations` block as defined below. --- -A `ip_configuration` block exports the following: +A `ip_configurations` block exports the following: * `private_ip_address` - The private IP address of the Azure Firewall. From 0fb0abe091b9bcd4d5f819dfc626348de66b342b Mon Sep 17 00:00:00 2001 From: houkms Date: Thu, 17 Oct 2019 12:23:48 +0800 Subject: [PATCH 02/10] adjsut ipconfigs in data_source_firewall --- azurerm/data_source_firewall.go | 1 - 1 file changed, 1 deletion(-) diff --git a/azurerm/data_source_firewall.go b/azurerm/data_source_firewall.go index 7ba5b3e48b6e..555be752c402 100644 --- a/azurerm/data_source_firewall.go +++ b/azurerm/data_source_firewall.go @@ -35,7 +35,6 @@ func dataSourceArmFirewall() *schema.Resource { "ip_configurations": { Type: schema.TypeList, Computed: true, - MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "name": { From de908578c915fff4484bc8499e29c5301be0c22e Mon Sep 17 00:00:00 2001 From: houkms Date: Fri, 18 Oct 2019 10:56:14 +0800 Subject: [PATCH 03/10] change ip_configurations back to ip_configuration --- azurerm/data_source_firewall.go | 7 +++---- azurerm/data_source_firewall_test.go | 6 +++--- azurerm/resource_arm_firewall.go | 9 ++++----- azurerm/resource_arm_firewall_test.go | 22 +++++++++++----------- website/docs/d/firewall.html.markdown | 6 +++--- website/docs/r/firewall.html.markdown | 10 +++++----- 6 files changed, 29 insertions(+), 31 deletions(-) diff --git a/azurerm/data_source_firewall.go b/azurerm/data_source_firewall.go index 555be752c402..d8969c80b2b0 100644 --- a/azurerm/data_source_firewall.go +++ b/azurerm/data_source_firewall.go @@ -32,7 +32,7 @@ func dataSourceArmFirewall() *schema.Resource { "resource_group_name": azure.SchemaResourceGroupNameForDataSource(), - "ip_configurations": { + "ip_configuration": { Type: schema.TypeList, Computed: true, Elem: &schema.Resource{ @@ -94,9 +94,8 @@ func dataSourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error { } if props := read.AzureFirewallPropertiesFormat; props != nil { - ipConfigs := flattenArmFirewallIPConfigurations(props.IPConfigurations) - if err := d.Set("ip_configurations", ipConfigs); err != nil { - return fmt.Errorf("Error setting `ip_configurations`: %+v", err) + if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil { + return fmt.Errorf("Error setting `ip_configuration`: %+v", err) } } diff --git a/azurerm/data_source_firewall_test.go b/azurerm/data_source_firewall_test.go index d70dbdbcc7a4..a27c728f34ac 100644 --- a/azurerm/data_source_firewall_test.go +++ b/azurerm/data_source_firewall_test.go @@ -21,8 +21,8 @@ func TestAccDataSourceAzureRMFirewall_basic(t *testing.T) { { Config: testAccDataSourceFirewall_basic(rInt, location), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(dataSourceName, "ip_configurations.0.name", "configuration"), - resource.TestCheckResourceAttrSet(dataSourceName, "ip_configurations.0.private_ip_address"), + resource.TestCheckResourceAttr(dataSourceName, "ip_configuration.0.name", "configuration"), + resource.TestCheckResourceAttrSet(dataSourceName, "ip_configuration.0.private_ip_address"), ), }, }, @@ -63,7 +63,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configurations { + ip_configuration { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" diff --git a/azurerm/resource_arm_firewall.go b/azurerm/resource_arm_firewall.go index 9add807d8f56..ff14d79b0215 100644 --- a/azurerm/resource_arm_firewall.go +++ b/azurerm/resource_arm_firewall.go @@ -50,7 +50,7 @@ func resourceArmFirewall() *schema.Resource { "resource_group_name": azure.SchemaResourceGroupName(), - "ip_configurations": { + "ip_configuration": { Type: schema.TypeList, Required: true, Elem: &schema.Resource{ @@ -209,9 +209,8 @@ func resourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error { } if props := read.AzureFirewallPropertiesFormat; props != nil { - ipConfigs := flattenArmFirewallIPConfigurations(props.IPConfigurations) - if err := d.Set("ip_configurations", ipConfigs); err != nil { - return fmt.Errorf("Error setting `ip_configurations`: %+v", err) + if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil { + return fmt.Errorf("Error setting `ip_configuration`: %+v", err) } } @@ -290,7 +289,7 @@ func resourceArmFirewallDelete(d *schema.ResourceData, meta interface{}) error { } func expandArmFirewallIPConfigurations(d *schema.ResourceData) (*[]network.AzureFirewallIPConfiguration, *[]string, *[]string, error) { - configs := d.Get("ip_configurations").([]interface{}) + configs := d.Get("ip_configuration").([]interface{}) ipConfigs := make([]network.AzureFirewallIPConfiguration, 0) subnetNamesToLock := make([]string, 0) virtualNetworkNamesToLock := make([]string, 0) diff --git a/azurerm/resource_arm_firewall_test.go b/azurerm/resource_arm_firewall_test.go index 999441c1c0b6..4f26ab2b825d 100644 --- a/azurerm/resource_arm_firewall_test.go +++ b/azurerm/resource_arm_firewall_test.go @@ -62,16 +62,16 @@ func TestAccAzureRMFirewall_basicOld(t *testing.T) { Config: testAccAzureRMFirewall_basicOld(ri, location), Check: resource.ComposeTestCheckFunc( testCheckAzureRMFirewallExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "ip_configurations.0.name", "configuration"), - resource.TestCheckResourceAttrSet(resourceName, "ip_configurations.0.private_ip_address"), + resource.TestCheckResourceAttr(resourceName, "ip_configuration.0.name", "configuration"), + resource.TestCheckResourceAttrSet(resourceName, "ip_configuration.0.private_ip_address"), ), }, { Config: testAccAzureRMFirewall_basic(ri, location), Check: resource.ComposeTestCheckFunc( testCheckAzureRMFirewallExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "ip_configurations.0.name", "configuration"), - resource.TestCheckResourceAttrSet(resourceName, "ip_configurations.0.private_ip_address"), + resource.TestCheckResourceAttr(resourceName, "ip_configuration.0.name", "configuration"), + resource.TestCheckResourceAttrSet(resourceName, "ip_configuration.0.private_ip_address"), ), }, { @@ -97,8 +97,8 @@ func TestAccAzureRMFirewall_basic(t *testing.T) { Config: testAccAzureRMFirewall_basic(ri, location), Check: resource.ComposeTestCheckFunc( testCheckAzureRMFirewallExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "ip_configurations.0.name", "configuration"), - resource.TestCheckResourceAttrSet(resourceName, "ip_configurations.0.private_ip_address"), + resource.TestCheckResourceAttr(resourceName, "ip_configuration.0.name", "configuration"), + resource.TestCheckResourceAttrSet(resourceName, "ip_configuration.0.private_ip_address"), ), }, { @@ -316,7 +316,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configurations { + ip_configuration { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" internal_public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -359,7 +359,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configurations { + ip_configuration { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -378,7 +378,7 @@ resource "azurerm_firewall" "import" { location = "${azurerm_firewall.test.location}" resource_group_name = "${azurerm_firewall.test.resource_group_name}" - ip_configurations { + ip_configuration { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -421,7 +421,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configurations { + ip_configuration { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -469,7 +469,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configurations { + ip_configuration { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" diff --git a/website/docs/d/firewall.html.markdown b/website/docs/d/firewall.html.markdown index 8eb724fdfa10..960cbaca3af8 100644 --- a/website/docs/d/firewall.html.markdown +++ b/website/docs/d/firewall.html.markdown @@ -20,7 +20,7 @@ data "azurerm_firewall" "test" { } output "firewall_private_ip" { - value = "${data.azurerm_firewall.test.ip_configurations.0.private_ip_address}" + value = "${data.azurerm_firewall.test.ip_configuration.0.private_ip_address}" } ``` @@ -36,11 +36,11 @@ The following attributes are exported: * `id` - The Resource ID of the Azure Firewall. -* `ip_configurations` - A `ip_configurations` block as defined below. +* `ip_configuration` - A `ip_configuration` block as defined below. --- -A `ip_configurations` block exports the following: +A `ip_configuration` block exports the following: * `subnet_id` - The Resource ID of the subnet where the Azure Firewall is deployed. diff --git a/website/docs/r/firewall.html.markdown b/website/docs/r/firewall.html.markdown index f726d61e3209..78b258edeb94 100644 --- a/website/docs/r/firewall.html.markdown +++ b/website/docs/r/firewall.html.markdown @@ -46,7 +46,7 @@ resource "azurerm_firewall" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" - ip_configurations { + ip_configuration { name = "configuration" subnet_id = "${azurerm_subnet.test.id}" public_ip_address_id = "${azurerm_public_ip.test.id}" @@ -64,13 +64,13 @@ The following arguments are supported: * `location` - (Required) Specifies the supported Azure location where the resource exists. Changing this forces a new resource to be created. -* `ip_configurations` - (Required) A `ip_configurations` block as documented below. +* `ip_configuration` - (Required) A `ip_configuration` block as documented below. * `tags` - (Optional) A mapping of tags to assign to the resource. --- -A `ip_configurations` block supports the following: +A `ip_configuration` block supports the following: * `name` - (Required) Specifies the name of the IP Configuration. @@ -88,11 +88,11 @@ The following attributes are exported: * `id` - The Resource ID of the Azure Firewall. -* `ip_configurations` - A `ip_configurations` block as defined below. +* `ip_configuration` - A `ip_configuration` block as defined below. --- -A `ip_configurations` block exports the following: +A `ip_configuration` block exports the following: * `private_ip_address` - The private IP address of the Azure Firewall. From 20b4aa6d9e896ee95a49458e8cae9be3cdbedca3 Mon Sep 17 00:00:00 2001 From: houkms Date: Tue, 22 Oct 2019 13:53:41 +0800 Subject: [PATCH 04/10] validate ip_cofigruation --- azurerm/data_source_firewall.go | 6 ++++-- azurerm/resource_arm_firewall.go | 36 ++++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/azurerm/data_source_firewall.go b/azurerm/data_source_firewall.go index d8969c80b2b0..a3ba7862725d 100644 --- a/azurerm/data_source_firewall.go +++ b/azurerm/data_source_firewall.go @@ -94,8 +94,10 @@ func dataSourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error { } if props := read.AzureFirewallPropertiesFormat; props != nil { - if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil { - return fmt.Errorf("Error setting `ip_configuration`: %+v", err) + if props.IPConfigurations != 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 ff14d79b0215..55a3d994635f 100644 --- a/azurerm/resource_arm_firewall.go +++ b/azurerm/resource_arm_firewall.go @@ -97,6 +97,10 @@ func resourceArmFirewallCreateUpdate(d *schema.ResourceData, meta interface{}) e ctx, cancel := timeouts.ForCreateUpdate(meta.(*ArmClient).StopContext, d) defer cancel() + if err := validateFirewallConfigurationSettings(d); err != nil { + return fmt.Errorf("Error creating Firewall %q (Resource Group %q): %+v", d.Get("name").(string), d.Get("resource_group_name").(string), err) + } + log.Printf("[INFO] preparing arguments for AzureRM Azure Firewall creation") name := d.Get("name").(string) @@ -209,8 +213,10 @@ func resourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error { } if props := read.AzureFirewallPropertiesFormat; props != nil { - if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil { - return fmt.Errorf("Error setting `ip_configuration`: %+v", err) + if props.IPConfigurations != nil { + if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil { + return fmt.Errorf("Error setting `ip_configuration`: %+v", err) + } } } @@ -407,3 +413,29 @@ 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 +} From 857e4fce270ac309b11ad570ac8b170fe0a1700d Mon Sep 17 00:00:00 2001 From: houkms Date: Tue, 22 Oct 2019 14:02:49 +0800 Subject: [PATCH 05/10] code refining --- azurerm/resource_arm_firewall.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/azurerm/resource_arm_firewall.go b/azurerm/resource_arm_firewall.go index 55a3d994635f..291065170391 100644 --- a/azurerm/resource_arm_firewall.go +++ b/azurerm/resource_arm_firewall.go @@ -419,23 +419,29 @@ func validateFirewallConfigurationSettings(d *schema.ResourceData) error { 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 } From 51d7bd30228cb413189457ab0389bceda463a8b1 Mon Sep 17 00:00:00 2001 From: houkms Date: Tue, 22 Oct 2019 14:21:08 +0800 Subject: [PATCH 06/10] update document for subnet_id in firewall --- website/docs/r/firewall.html.markdown | 2 ++ 1 file changed, 2 insertions(+) diff --git a/website/docs/r/firewall.html.markdown b/website/docs/r/firewall.html.markdown index 78b258edeb94..787e682506f0 100644 --- a/website/docs/r/firewall.html.markdown +++ b/website/docs/r/firewall.html.markdown @@ -78,6 +78,8 @@ A `ip_configuration` block supports the following: -> **NOTE** The Subnet used for the Firewall must have the name `AzureFirewallSubnet` and the subnet mask must be at least `/26`. +-> **NOTE** The `subnet_id` is shared among multiple `ip_configuration` blocks, make sure there is one and only one `subnet_id` in all `ip_configuration` blocks. + * `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. From 0234ed8593f55b403aa443284aacaa01e820f881 Mon Sep 17 00:00:00 2001 From: houkms Date: Mon, 28 Oct 2019 11:18:10 +0800 Subject: [PATCH 07/10] coding style adjustments for firewall --- azurerm/data_source_firewall.go | 6 ++---- azurerm/resource_arm_firewall.go | 19 ++++++++----------- website/docs/r/firewall.html.markdown | 4 ++-- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/azurerm/data_source_firewall.go b/azurerm/data_source_firewall.go index a3ba7862725d..d8969c80b2b0 100644 --- a/azurerm/data_source_firewall.go +++ b/azurerm/data_source_firewall.go @@ -94,10 +94,8 @@ func dataSourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error { } if props := read.AzureFirewallPropertiesFormat; props != nil { - if props.IPConfigurations != nil { - if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil { - return fmt.Errorf("Error setting `ip_configuration`: %+v", err) - } + 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 291065170391..76eff98250d4 100644 --- a/azurerm/resource_arm_firewall.go +++ b/azurerm/resource_arm_firewall.go @@ -4,7 +4,6 @@ import ( "fmt" "log" "regexp" - "strconv" "time" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" @@ -97,10 +96,6 @@ func resourceArmFirewallCreateUpdate(d *schema.ResourceData, meta interface{}) e ctx, cancel := timeouts.ForCreateUpdate(meta.(*ArmClient).StopContext, d) defer cancel() - if err := validateFirewallConfigurationSettings(d); err != nil { - return fmt.Errorf("Error creating Firewall %q (Resource Group %q): %+v", d.Get("name").(string), d.Get("resource_group_name").(string), err) - } - log.Printf("[INFO] preparing arguments for AzureRM Azure Firewall creation") name := d.Get("name").(string) @@ -119,6 +114,10 @@ func resourceArmFirewallCreateUpdate(d *schema.ResourceData, meta interface{}) e } } + if err := validateFirewallConfigurationSettings(d); err != nil { + return fmt.Errorf("Error creating 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) @@ -213,10 +212,8 @@ func resourceArmFirewallRead(d *schema.ResourceData, meta interface{}) error { } if props := read.AzureFirewallPropertiesFormat; props != nil { - if props.IPConfigurations != nil { - if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil { - return fmt.Errorf("Error setting `ip_configuration`: %+v", err) - } + if err := d.Set("ip_configuration", flattenArmFirewallIPConfigurations(props.IPConfigurations)); err != nil { + return fmt.Errorf("Error setting `ip_configuration`: %+v", err) } } @@ -300,7 +297,7 @@ func expandArmFirewallIPConfigurations(d *schema.ResourceData) (*[]network.Azure subnetNamesToLock := make([]string, 0) virtualNetworkNamesToLock := make([]string, 0) - for index, configRaw := range configs { + for _, configRaw := range configs { data := configRaw.(map[string]interface{}) name := data["name"].(string) subnetId := data["subnet_id"].(string) @@ -311,7 +308,7 @@ func expandArmFirewallIPConfigurations(d *schema.ResourceData) (*[]network.Azure } if !exist || pubID == "" { - return nil, nil, nil, fmt.Errorf("one of `ip_configuration.%s.internal_public_ip_address_id` or `ip_configuration.%s.public_ip_address_id` must be set", strconv.Itoa(index), strconv.Itoa(index)) + return nil, nil, nil, fmt.Errorf("one of `internal_public_ip_address_id` or `public_ip_address_id` must be set") } ipConfig := network.AzureFirewallIPConfiguration{ diff --git a/website/docs/r/firewall.html.markdown b/website/docs/r/firewall.html.markdown index 787e682506f0..52dc47604918 100644 --- a/website/docs/r/firewall.html.markdown +++ b/website/docs/r/firewall.html.markdown @@ -74,11 +74,11 @@ 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** The `subnet_id` is shared among multiple `ip_configuration` blocks, make sure there is one and only one `subnet_id` in all `ip_configuration` blocks. +-> **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. From 9965338df465e782f227f12109ff32ab467ca210 Mon Sep 17 00:00:00 2001 From: houkms Date: Mon, 4 Nov 2019 09:51:50 +0800 Subject: [PATCH 08/10] code refining --- azurerm/resource_arm_firewall.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/resource_arm_firewall.go b/azurerm/resource_arm_firewall.go index 76eff98250d4..5b2ed3915eb0 100644 --- a/azurerm/resource_arm_firewall.go +++ b/azurerm/resource_arm_firewall.go @@ -115,7 +115,7 @@ func resourceArmFirewallCreateUpdate(d *schema.ResourceData, meta interface{}) e } if err := validateFirewallConfigurationSettings(d); err != nil { - return fmt.Errorf("Error creating Firewall %q (Resource Group %q): %+v", name, resourceGroup, err) + return fmt.Errorf("Error validating Firewall %q (Resource Group %q): %+v", name, resourceGroup, err) } location := azure.NormalizeLocation(d.Get("location").(string)) From 71db80a6ee92af5be3cd421bb4dfd9833a501f8c Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Thu, 7 Nov 2019 10:42:25 +0000 Subject: [PATCH 09/10] azurerm_firewall: test for multiple public ip_configuration --- azurerm/resource_arm_firewall_test.go | 85 +++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/azurerm/resource_arm_firewall_test.go b/azurerm/resource_arm_firewall_test.go index 4f26ab2b825d..409b8791cf94 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") @@ -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(` From b8689f0347eb910bbf025e53a0d6b37c5314021d Mon Sep 17 00:00:00 2001 From: houkms Date: Tue, 12 Nov 2019 11:47:34 +0800 Subject: [PATCH 10/10] remove validation for deprecated properties --- azurerm/resource_arm_firewall.go | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/azurerm/resource_arm_firewall.go b/azurerm/resource_arm_firewall.go index 5b2ed3915eb0..bb0086390b77 100644 --- a/azurerm/resource_arm_firewall.go +++ b/azurerm/resource_arm_firewall.go @@ -70,7 +70,7 @@ func resourceArmFirewall() *schema.Resource { Optional: true, Computed: true, ValidateFunc: azure.ValidateResourceID, - Deprecated: "This field has been deprecated. Use `public_ip_address_id` instead.", + 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, @@ -414,31 +414,17 @@ func validateAzureFirewallSubnetName(v interface{}, k string) (warnings []string 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 }