From 7b68276d7d3e771214eb97b59687b0e688c024e2 Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 19 Jan 2020 18:28:07 +0900 Subject: [PATCH 1/9] azurerm_function_app - support for ip_restriction --- .../services/web/resource_arm_function_app.go | 125 +++++++- .../tests/resource_arm_function_app_test.go | 298 ++++++++++++++++++ website/docs/r/function_app.html.markdown | 14 + 3 files changed, 431 insertions(+), 6 deletions(-) diff --git a/azurerm/internal/services/web/resource_arm_function_app.go b/azurerm/internal/services/web/resource_arm_function_app.go index bad7fc3517a2..4a6850ce9bbd 100644 --- a/azurerm/internal/services/web/resource_arm_function_app.go +++ b/azurerm/internal/services/web/resource_arm_function_app.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "net" "strings" "time" @@ -12,6 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/validation" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" @@ -225,6 +227,36 @@ func resourceArmFunctionApp() *schema.Resource { Optional: true, Default: false, }, + "ip_restriction": { + Type: schema.TypeList, + Optional: true, + Computed: true, + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "ip_address": { + Type: schema.TypeString, + Optional: true, + }, + "virtual_network_subnet_id": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validate.NoEmptyStrings, + }, + "subnet_mask": { + Type: schema.TypeString, + Optional: true, + Computed: true, + // TODO we should fix this in 2.0 + // This attribute was made with the assumption that `ip_address` was the only valid option + // but `virtual_network_subnet_id` is being added and doesn't need a `subnet_mask`. + // We'll assume a default of "255.255.255.255" in the expand code when `ip_address` is specified + // and `subnet_mask` is not. + // Default: "255.255.255.255", + }, + }, + }, + }, "min_tls_version": { Type: schema.TypeString, Optional: true, @@ -324,7 +356,11 @@ func resourceArmFunctionAppCreate(d *schema.ResourceData, meta interface{}) erro basicAppSettings := getBasicFunctionAppAppSettings(d, appServiceTier) - siteConfig := expandFunctionAppSiteConfig(d) + siteConfig, err := expandFunctionAppSiteConfig(d) + if err != nil { + return fmt.Errorf("Error expanding `site_config` for Function App %q (Resource Group %q): %s", name, resourceGroup, err) + } + siteConfig.AppSettings = &basicAppSettings siteEnvelope := web.Site{ @@ -407,7 +443,10 @@ func resourceArmFunctionAppUpdate(d *schema.ResourceData, meta interface{}) erro return err } basicAppSettings := getBasicFunctionAppAppSettings(d, appServiceTier) - siteConfig := expandFunctionAppSiteConfig(d) + siteConfig, err := expandFunctionAppSiteConfig(d) + if err != nil { + return fmt.Errorf("Error expanding `site_config` for Function App %q (Resource Group %q): %s", name, resGroup, err) + } siteConfig.AppSettings = &basicAppSettings @@ -450,7 +489,10 @@ func resourceArmFunctionAppUpdate(d *schema.ResourceData, meta interface{}) erro } if d.HasChange("site_config") { - siteConfig := expandFunctionAppSiteConfig(d) + siteConfig, err := expandFunctionAppSiteConfig(d) + if err != nil { + return fmt.Errorf("Error expanding `site_config` for Function App %q (Resource Group %q): %s", name, resGroup, err) + } siteConfigResource := web.SiteConfigResource{ SiteConfig: &siteConfig, } @@ -706,12 +748,12 @@ func expandFunctionAppAppSettings(d *schema.ResourceData, appServiceTier string) return output } -func expandFunctionAppSiteConfig(d *schema.ResourceData) web.SiteConfig { +func expandFunctionAppSiteConfig(d *schema.ResourceData) (web.SiteConfig, error) { configs := d.Get("site_config").([]interface{}) siteConfig := web.SiteConfig{} if len(configs) == 0 { - return siteConfig + return siteConfig, nil } config := configs[0].(map[string]interface{}) @@ -746,6 +788,51 @@ func expandFunctionAppSiteConfig(d *schema.ResourceData) web.SiteConfig { siteConfig.HTTP20Enabled = utils.Bool(v.(bool)) } + if v, ok := config["ip_restriction"]; ok { + ipSecurityRestrictions := v.([]interface{}) + restrictions := make([]web.IPSecurityRestriction, 0) + for i, ipSecurityRestriction := range ipSecurityRestrictions { + restriction := ipSecurityRestriction.(map[string]interface{}) + + ipAddress := restriction["ip_address"].(string) + vNetSubnetID := restriction["virtual_network_subnet_id"].(string) + if vNetSubnetID != "" && ipAddress != "" { + return siteConfig, fmt.Errorf(fmt.Sprintf("only one of `ip_address` or `virtual_network_subnet_id` can set set for `site_config.0.ip_restriction.%d`", i)) + } + + if vNetSubnetID == "" && ipAddress == "" { + return siteConfig, fmt.Errorf(fmt.Sprintf("one of `ip_address` or `virtual_network_subnet_id` must be set set for `site_config.0.ip_restriction.%d`", i)) + } + + ipSecurityRestriction := web.IPSecurityRestriction{} + if ipAddress != "" { + mask := restriction["subnet_mask"].(string) + if mask == "" { + mask = "255.255.255.255" + } + // the 2018-02-01 API expects a blank subnet mask and an IP address in CIDR format: a.b.c.d/x + // so translate the IP and mask if necessary + restrictionMask := "" + cidrAddress := ipAddress + if mask != "" { + ipNet := net.IPNet{IP: net.ParseIP(ipAddress), Mask: net.IPMask(net.ParseIP(mask))} + cidrAddress = ipNet.String() + } else if !strings.Contains(ipAddress, "/") { + cidrAddress += "/32" + } + ipSecurityRestriction.IPAddress = &cidrAddress + ipSecurityRestriction.SubnetMask = &restrictionMask + } + + if vNetSubnetID != "" { + ipSecurityRestriction.VnetSubnetResourceID = &vNetSubnetID + } + + restrictions = append(restrictions, ipSecurityRestriction) + } + siteConfig.IPSecurityRestrictions = &restrictions + } + if v, ok := config["min_tls_version"]; ok { siteConfig.MinTLSVersion = web.SupportedTLSVersions(v.(string)) } @@ -754,7 +841,7 @@ func expandFunctionAppSiteConfig(d *schema.ResourceData) web.SiteConfig { siteConfig.FtpsState = web.FtpsState(v.(string)) } - return siteConfig + return siteConfig, nil } func flattenFunctionAppSiteConfig(input *web.SiteConfig) []interface{} { @@ -790,6 +877,32 @@ func flattenFunctionAppSiteConfig(input *web.SiteConfig) []interface{} { result["http2_enabled"] = *input.HTTP20Enabled } + restrictions := make([]interface{}, 0) + if vs := input.IPSecurityRestrictions; vs != nil { + for _, v := range *vs { + block := make(map[string]interface{}) + if ip := v.IPAddress; ip != nil { + // the 2018-02-01 API uses CIDR format (a.b.c.d/x), so translate that back to IP and mask + if strings.Contains(*ip, "/") { + ipAddr, ipNet, _ := net.ParseCIDR(*ip) + block["ip_address"] = ipAddr.String() + mask := net.IP(ipNet.Mask) + block["subnet_mask"] = mask.String() + } else { + block["ip_address"] = *ip + } + } + if subnet := v.SubnetMask; subnet != nil { + block["subnet_mask"] = *subnet + } + if vNetSubnetID := v.VnetSubnetResourceID; vNetSubnetID != nil { + block["virtual_network_subnet_id"] = *vNetSubnetID + } + restrictions = append(restrictions, block) + } + } + result["ip_restriction"] = restrictions + result["min_tls_version"] = string(input.MinTLSVersion) result["ftps_state"] = string(input.FtpsState) diff --git a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go index 77bd3494a632..a2940af6fbba 100644 --- a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go +++ b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go @@ -628,6 +628,109 @@ func TestAccAzureRMFunctionApp_ftpsState(t *testing.T) { }) } +func TestAccAzureRMFunctionApp_oneIpRestriction(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_function_app", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFunctionAppDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFunctionApp_oneIpRestriction(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFunctionAppExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.ip_address", "10.10.10.10"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.subnet_mask", "255.255.255.255"), + ), + }, + data.ImportStep(), + }, + }) +} + +func TestAccAzureRMFunctionApp_oneVNetSubnetIpRestriction(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_function_app", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFunctionAppDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFunctionApp_oneVNetSubnetIpRestriction(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFunctionAppExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + +func TestAccAzureRMFunctionApp_zeroedIpRestriction(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_function_app", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFunctionAppDestroy, + Steps: []resource.TestStep{ + { + // This configuration includes a single explicit ip_restriction + Config: testAccAzureRMFunctionApp_oneIpRestriction(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFunctionAppExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.#", "1"), + ), + }, + { + // This configuration has no site_config blocks at all. + Config: testAccAzureRMFunctionApp_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFunctionAppExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.#", "1"), + ), + }, + { + // This configuration explicitly sets ip_restriction to [] using attribute syntax. + Config: testAccAzureRMFunctionApp_zeroedIpRestriction(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFunctionAppExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.#", "0"), + ), + }, + }, + }) +} + +func TestAccAzureRMFunctionApp_manyIpRestrictions(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_function_app", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFunctionAppDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFunctionApp_manyIpRestrictions(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFunctionAppExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.ip_address", "10.10.10.10"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.subnet_mask", "255.255.255.255"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.1.ip_address", "20.20.20.0"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.1.subnet_mask", "255.255.255.0"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.2.ip_address", "30.30.0.0"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.2.subnet_mask", "255.255.0.0"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.3.ip_address", "192.168.1.2"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.3.subnet_mask", "255.255.255.0"), + ), + }, + data.ImportStep(), + }, + }) +} + func testCheckAzureRMFunctionAppDestroy(s *terraform.State) error { client := acceptance.AzureProvider.Meta().(*clients.Client).Web.AppServicesClient ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext @@ -1729,3 +1832,198 @@ resource "azurerm_function_app" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger, data.RandomInteger) } + +func testAccAzureRMFunctionApp_oneIpRestriction(data acceptance.TestData) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_storage_account" "test" { + name = "acctestsa%s" + resource_group_name = "${azurerm_resource_group.test.name}" + location = "${azurerm_resource_group.test.location}" + account_tier = "Standard" + account_replication_type = "LRS" +} + +resource "azurerm_app_service_plan" "test" { + name = "acctestASP-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + + sku { + tier = "Standard" + size = "S1" + } +} + +resource "azurerm_function_app" "test" { + name = "acctest-%d-func" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + app_service_plan_id = "${azurerm_app_service_plan.test.id}" + storage_connection_string = "${azurerm_storage_account.test.primary_connection_string}" + + site_config { + ip_restriction { + ip_address = "10.10.10.10" + } + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger, data.RandomInteger) +} + +func testAccAzureRMFunctionApp_oneVNetSubnetIpRestriction(data acceptance.TestData) 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 = "acctestsubnet%d" + resource_group_name = "${azurerm_resource_group.test.name}" + virtual_network_name = "${azurerm_virtual_network.test.name}" + address_prefix = "10.0.2.0/24" +} + +resource "azurerm_storage_account" "test" { + name = "acctestsa%s" + resource_group_name = "${azurerm_resource_group.test.name}" + location = "${azurerm_resource_group.test.location}" + account_tier = "Standard" + account_replication_type = "LRS" +} + +resource "azurerm_app_service_plan" "test" { + name = "acctestASP-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + + sku { + tier = "Standard" + size = "S1" + } +} + +resource "azurerm_function_app" "test" { + name = "acctest-%d-func" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + app_service_plan_id = "${azurerm_app_service_plan.test.id}" + storage_connection_string = "${azurerm_storage_account.test.primary_connection_string}" + + site_config { + ip_restriction { + virtual_network_subnet_id = "${azurerm_subnet.test.id}" + } + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomString, data.RandomInteger, data.RandomInteger) +} + +func testAccAzureRMFunctionApp_manyIpRestrictions(data acceptance.TestData) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_storage_account" "test" { + name = "acctestsa%s" + resource_group_name = "${azurerm_resource_group.test.name}" + location = "${azurerm_resource_group.test.location}" + account_tier = "Standard" + account_replication_type = "LRS" +} + +resource "azurerm_app_service_plan" "test" { + name = "acctestASP-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + + sku { + tier = "Standard" + size = "S1" + } +} + +resource "azurerm_function_app" "test" { + name = "acctest-%d-func" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + app_service_plan_id = "${azurerm_app_service_plan.test.id}" + storage_connection_string = "${azurerm_storage_account.test.primary_connection_string}" + + site_config { + ip_restriction { + ip_address = "10.10.10.10" + } + + ip_restriction { + ip_address = "20.20.20.0" + subnet_mask = "255.255.255.0" + } + + ip_restriction { + ip_address = "30.30.0.0" + subnet_mask = "255.255.0.0" + } + + ip_restriction { + ip_address = "192.168.1.2" + subnet_mask = "255.255.255.0" + } + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger, data.RandomInteger) +} + +func testAccAzureRMFunctionApp_zeroedIpRestriction(data acceptance.TestData) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_storage_account" "test" { + name = "acctestsa%s" + resource_group_name = "${azurerm_resource_group.test.name}" + location = "${azurerm_resource_group.test.location}" + account_tier = "Standard" + account_replication_type = "LRS" +} + +resource "azurerm_app_service_plan" "test" { + name = "acctestASP-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + + sku { + tier = "Standard" + size = "S1" + } +} + +resource "azurerm_function_app" "test" { + name = "acctest-%d-func" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + app_service_plan_id = "${azurerm_app_service_plan.test.id}" + storage_connection_string = "${azurerm_storage_account.test.primary_connection_string}" + + site_config { + ip_restriction = [] + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger, data.RandomInteger) +} \ No newline at end of file diff --git a/website/docs/r/function_app.html.markdown b/website/docs/r/function_app.html.markdown index abb6cfa646a9..c94f87a24c9e 100644 --- a/website/docs/r/function_app.html.markdown +++ b/website/docs/r/function_app.html.markdown @@ -154,6 +154,8 @@ The following arguments are supported: * `cors` - (Optional) A `cors` block as defined below. +* `ip_restriction` - (Optional) A [List of objects](/docs/configuration/attr-as-blocks.html) representing ip restrictions as defined below. + --- A `cors` block supports the following: @@ -242,6 +244,18 @@ A `microsoft` block supports the following: * `oauth_scopes` (Optional) The OAuth 2.0 scopes that will be requested as part of Microsoft Account authentication. https://msdn.microsoft.com/en-us/library/dn631845.aspx +--- + +A `ip_restriction` block supports the following: + +* `ip_address` - (Optional) The IP Address used for this IP Restriction. + +* `subnet_mask` - (Optional) The Subnet mask used for this IP Restriction. Defaults to `255.255.255.255`. + +* `virtual_network_subnet_id` - (Optional.The Virtual Network Subnet ID used for this IP Restriction. + +-> **NOTE:** One of either `ip_address` or `virtual_network_subnet_id` must be specified + ## Attributes Reference The following attributes are exported: From 3971b882d14802b5375299b61f5ea4947013ff6f Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 19 Jan 2020 18:47:00 +0900 Subject: [PATCH 2/9] Fix code formatting issue --- azurerm/internal/services/web/resource_arm_function_app.go | 2 +- .../services/web/tests/resource_arm_function_app_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/azurerm/internal/services/web/resource_arm_function_app.go b/azurerm/internal/services/web/resource_arm_function_app.go index 4a6850ce9bbd..9a815fb5d247 100644 --- a/azurerm/internal/services/web/resource_arm_function_app.go +++ b/azurerm/internal/services/web/resource_arm_function_app.go @@ -13,8 +13,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/validation" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" diff --git a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go index a2940af6fbba..a4d65b361289 100644 --- a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go +++ b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go @@ -2026,4 +2026,4 @@ resource "azurerm_function_app" "test" { } } `, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger, data.RandomInteger) -} \ No newline at end of file +} From 9593ff8786410ce64c5f5af9eebafdbbf362ead5 Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 26 Jan 2020 02:36:31 +0900 Subject: [PATCH 3/9] Changed test case method name --- .../services/web/tests/resource_arm_function_app_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go index a4d65b361289..327cd73e1043 100644 --- a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go +++ b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go @@ -668,7 +668,7 @@ func TestAccAzureRMFunctionApp_oneVNetSubnetIpRestriction(t *testing.T) { }) } -func TestAccAzureRMFunctionApp_zeroedIpRestriction(t *testing.T) { +func TestAccAzureRMFunctionApp_ipRestrictionRemoved(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_function_app", "test") resource.ParallelTest(t, resource.TestCase{ @@ -694,7 +694,7 @@ func TestAccAzureRMFunctionApp_zeroedIpRestriction(t *testing.T) { }, { // This configuration explicitly sets ip_restriction to [] using attribute syntax. - Config: testAccAzureRMFunctionApp_zeroedIpRestriction(data), + Config: testAccAzureRMFunctionApp_ipRestrictionRemoved(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMFunctionAppExists(data.ResourceName), resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.#", "0"), @@ -1988,7 +1988,7 @@ resource "azurerm_function_app" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger, data.RandomInteger) } -func testAccAzureRMFunctionApp_zeroedIpRestriction(data acceptance.TestData) string { +func testAccAzureRMFunctionApp_ipRestrictionRemoved(data acceptance.TestData) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { name = "acctestRG-%d" From 7df4387f15dde3dcdbf4ab1d61cf83bf1d2b2951 Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 26 Jan 2020 02:52:50 +0900 Subject: [PATCH 4/9] Changed `virtual_network_subnet_id` property name --- .../services/web/resource_arm_function_app.go | 12 ++++++------ .../web/tests/resource_arm_function_app_test.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/azurerm/internal/services/web/resource_arm_function_app.go b/azurerm/internal/services/web/resource_arm_function_app.go index 9a815fb5d247..d9d4ad52e3c8 100644 --- a/azurerm/internal/services/web/resource_arm_function_app.go +++ b/azurerm/internal/services/web/resource_arm_function_app.go @@ -238,7 +238,7 @@ func resourceArmFunctionApp() *schema.Resource { Type: schema.TypeString, Optional: true, }, - "virtual_network_subnet_id": { + "subnet_id": { Type: schema.TypeString, Optional: true, ValidateFunc: validate.NoEmptyStrings, @@ -249,7 +249,7 @@ func resourceArmFunctionApp() *schema.Resource { Computed: true, // TODO we should fix this in 2.0 // This attribute was made with the assumption that `ip_address` was the only valid option - // but `virtual_network_subnet_id` is being added and doesn't need a `subnet_mask`. + // but `subnet_id` is being added and doesn't need a `subnet_mask`. // We'll assume a default of "255.255.255.255" in the expand code when `ip_address` is specified // and `subnet_mask` is not. // Default: "255.255.255.255", @@ -795,13 +795,13 @@ func expandFunctionAppSiteConfig(d *schema.ResourceData) (web.SiteConfig, error) restriction := ipSecurityRestriction.(map[string]interface{}) ipAddress := restriction["ip_address"].(string) - vNetSubnetID := restriction["virtual_network_subnet_id"].(string) + vNetSubnetID := restriction["subnet_id"].(string) if vNetSubnetID != "" && ipAddress != "" { - return siteConfig, fmt.Errorf(fmt.Sprintf("only one of `ip_address` or `virtual_network_subnet_id` can set set for `site_config.0.ip_restriction.%d`", i)) + return siteConfig, fmt.Errorf(fmt.Sprintf("only one of `ip_address` or `subnet_id` can set set for `site_config.0.ip_restriction.%d`", i)) } if vNetSubnetID == "" && ipAddress == "" { - return siteConfig, fmt.Errorf(fmt.Sprintf("one of `ip_address` or `virtual_network_subnet_id` must be set set for `site_config.0.ip_restriction.%d`", i)) + return siteConfig, fmt.Errorf(fmt.Sprintf("one of `ip_address` or `subnet_id` must be set set for `site_config.0.ip_restriction.%d`", i)) } ipSecurityRestriction := web.IPSecurityRestriction{} @@ -896,7 +896,7 @@ func flattenFunctionAppSiteConfig(input *web.SiteConfig) []interface{} { block["subnet_mask"] = *subnet } if vNetSubnetID := v.VnetSubnetResourceID; vNetSubnetID != nil { - block["virtual_network_subnet_id"] = *vNetSubnetID + block["subnet_id"] = *vNetSubnetID } restrictions = append(restrictions, block) } diff --git a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go index 327cd73e1043..d1209e152b57 100644 --- a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go +++ b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go @@ -1924,7 +1924,7 @@ resource "azurerm_function_app" "test" { site_config { ip_restriction { - virtual_network_subnet_id = "${azurerm_subnet.test.id}" + subnet_id = "${azurerm_subnet.test.id}" } } } From 1b462a4cba83da6a818dc4b6fdb3bedf35e6ecbc Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 26 Jan 2020 04:11:20 +0900 Subject: [PATCH 5/9] Remove `subnet_mask` property --- .../services/web/resource_arm_function_app.go | 46 ++----------------- .../tests/resource_arm_function_app_test.go | 28 ++++------- 2 files changed, 14 insertions(+), 60 deletions(-) diff --git a/azurerm/internal/services/web/resource_arm_function_app.go b/azurerm/internal/services/web/resource_arm_function_app.go index d9d4ad52e3c8..aa0a3fe2f6c4 100644 --- a/azurerm/internal/services/web/resource_arm_function_app.go +++ b/azurerm/internal/services/web/resource_arm_function_app.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "log" - "net" "strings" "time" @@ -243,17 +242,6 @@ func resourceArmFunctionApp() *schema.Resource { Optional: true, ValidateFunc: validate.NoEmptyStrings, }, - "subnet_mask": { - Type: schema.TypeString, - Optional: true, - Computed: true, - // TODO we should fix this in 2.0 - // This attribute was made with the assumption that `ip_address` was the only valid option - // but `subnet_id` is being added and doesn't need a `subnet_mask`. - // We'll assume a default of "255.255.255.255" in the expand code when `ip_address` is specified - // and `subnet_mask` is not. - // Default: "255.255.255.255", - }, }, }, }, @@ -797,31 +785,16 @@ func expandFunctionAppSiteConfig(d *schema.ResourceData) (web.SiteConfig, error) ipAddress := restriction["ip_address"].(string) vNetSubnetID := restriction["subnet_id"].(string) if vNetSubnetID != "" && ipAddress != "" { - return siteConfig, fmt.Errorf(fmt.Sprintf("only one of `ip_address` or `subnet_id` can set set for `site_config.0.ip_restriction.%d`", i)) + return siteConfig, fmt.Errorf(fmt.Sprintf("only one of `ip_address` or `subnet_id` can set for `site_config.0.ip_restriction.%d`", i)) } if vNetSubnetID == "" && ipAddress == "" { - return siteConfig, fmt.Errorf(fmt.Sprintf("one of `ip_address` or `subnet_id` must be set set for `site_config.0.ip_restriction.%d`", i)) + return siteConfig, fmt.Errorf(fmt.Sprintf("one of `ip_address` or `subnet_id` must be set for `site_config.0.ip_restriction.%d`", i)) } ipSecurityRestriction := web.IPSecurityRestriction{} if ipAddress != "" { - mask := restriction["subnet_mask"].(string) - if mask == "" { - mask = "255.255.255.255" - } - // the 2018-02-01 API expects a blank subnet mask and an IP address in CIDR format: a.b.c.d/x - // so translate the IP and mask if necessary - restrictionMask := "" - cidrAddress := ipAddress - if mask != "" { - ipNet := net.IPNet{IP: net.ParseIP(ipAddress), Mask: net.IPMask(net.ParseIP(mask))} - cidrAddress = ipNet.String() - } else if !strings.Contains(ipAddress, "/") { - cidrAddress += "/32" - } - ipSecurityRestriction.IPAddress = &cidrAddress - ipSecurityRestriction.SubnetMask = &restrictionMask + ipSecurityRestriction.IPAddress = &ipAddress } if vNetSubnetID != "" { @@ -882,18 +855,7 @@ func flattenFunctionAppSiteConfig(input *web.SiteConfig) []interface{} { for _, v := range *vs { block := make(map[string]interface{}) if ip := v.IPAddress; ip != nil { - // the 2018-02-01 API uses CIDR format (a.b.c.d/x), so translate that back to IP and mask - if strings.Contains(*ip, "/") { - ipAddr, ipNet, _ := net.ParseCIDR(*ip) - block["ip_address"] = ipAddr.String() - mask := net.IP(ipNet.Mask) - block["subnet_mask"] = mask.String() - } else { - block["ip_address"] = *ip - } - } - if subnet := v.SubnetMask; subnet != nil { - block["subnet_mask"] = *subnet + block["ip_address"] = *ip } if vNetSubnetID := v.VnetSubnetResourceID; vNetSubnetID != nil { block["subnet_id"] = *vNetSubnetID diff --git a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go index d1209e152b57..915e16e50e71 100644 --- a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go +++ b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go @@ -640,8 +640,7 @@ func TestAccAzureRMFunctionApp_oneIpRestriction(t *testing.T) { Config: testAccAzureRMFunctionApp_oneIpRestriction(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMFunctionAppExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.ip_address", "10.10.10.10"), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.subnet_mask", "255.255.255.255"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.ip_address", "10.10.10.10/32"), ), }, data.ImportStep(), @@ -716,14 +715,10 @@ func TestAccAzureRMFunctionApp_manyIpRestrictions(t *testing.T) { Config: testAccAzureRMFunctionApp_manyIpRestrictions(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMFunctionAppExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.ip_address", "10.10.10.10"), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.subnet_mask", "255.255.255.255"), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.1.ip_address", "20.20.20.0"), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.1.subnet_mask", "255.255.255.0"), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.2.ip_address", "30.30.0.0"), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.2.subnet_mask", "255.255.0.0"), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.3.ip_address", "192.168.1.2"), - resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.3.subnet_mask", "255.255.255.0"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.0.ip_address", "10.10.10.10/32"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.1.ip_address", "20.20.20.0/24"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.2.ip_address", "30.30.0.0/16"), + resource.TestCheckResourceAttr(data.ResourceName, "site_config.0.ip_restriction.3.ip_address", "192.168.1.2/24"), ), }, data.ImportStep(), @@ -1868,7 +1863,7 @@ resource "azurerm_function_app" "test" { site_config { ip_restriction { - ip_address = "10.10.10.10" + ip_address = "10.10.10.10/32" } } } @@ -1966,22 +1961,19 @@ resource "azurerm_function_app" "test" { site_config { ip_restriction { - ip_address = "10.10.10.10" + ip_address = "10.10.10.10/32" } ip_restriction { - ip_address = "20.20.20.0" - subnet_mask = "255.255.255.0" + ip_address = "20.20.20.0/24" } ip_restriction { - ip_address = "30.30.0.0" - subnet_mask = "255.255.0.0" + ip_address = "30.30.0.0/16" } ip_restriction { - ip_address = "192.168.1.2" - subnet_mask = "255.255.255.0" + ip_address = "192.168.1.2/24" } } } From 2d4be9339932a7789aa1fb2396aba267ad064e8d Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 26 Jan 2020 04:12:42 +0900 Subject: [PATCH 6/9] Update document --- website/docs/r/function_app.html.markdown | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/website/docs/r/function_app.html.markdown b/website/docs/r/function_app.html.markdown index c94f87a24c9e..68334c7d2525 100644 --- a/website/docs/r/function_app.html.markdown +++ b/website/docs/r/function_app.html.markdown @@ -248,13 +248,11 @@ A `microsoft` block supports the following: A `ip_restriction` block supports the following: -* `ip_address` - (Optional) The IP Address used for this IP Restriction. +* `ip_address` - (Optional) The IP Address CIDR notation used for this IP Restriction. -* `subnet_mask` - (Optional) The Subnet mask used for this IP Restriction. Defaults to `255.255.255.255`. +* `subnet_id` - (Optional) The Subnet ID used for this IP Restriction. -* `virtual_network_subnet_id` - (Optional.The Virtual Network Subnet ID used for this IP Restriction. - --> **NOTE:** One of either `ip_address` or `virtual_network_subnet_id` must be specified +-> **NOTE:** One of either `ip_address` or `subnet_id` must be specified ## Attributes Reference From c9603dca0c768f92dcbb27a1f5a53542de5edeac Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 26 Jan 2020 20:56:11 +0900 Subject: [PATCH 7/9] Fix tflint error --- .../services/web/tests/resource_arm_function_app_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go index 915e16e50e71..88575276525f 100644 --- a/azurerm/internal/services/web/tests/resource_arm_function_app_test.go +++ b/azurerm/internal/services/web/tests/resource_arm_function_app_test.go @@ -1965,15 +1965,15 @@ resource "azurerm_function_app" "test" { } ip_restriction { - ip_address = "20.20.20.0/24" + ip_address = "20.20.20.0/24" } ip_restriction { - ip_address = "30.30.0.0/16" + ip_address = "30.30.0.0/16" } ip_restriction { - ip_address = "192.168.1.2/24" + ip_address = "192.168.1.2/24" } } } From 7cea183ee248aabd0201cfa187e0af71c8fbe5d5 Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Mon, 27 Jan 2020 02:51:38 +0900 Subject: [PATCH 8/9] Extract `ip_restriction` expand and flatten methods --- .../services/web/resource_arm_function_app.go | 103 +++++++++++------- 1 file changed, 62 insertions(+), 41 deletions(-) diff --git a/azurerm/internal/services/web/resource_arm_function_app.go b/azurerm/internal/services/web/resource_arm_function_app.go index aa0a3fe2f6c4..631688142a5f 100644 --- a/azurerm/internal/services/web/resource_arm_function_app.go +++ b/azurerm/internal/services/web/resource_arm_function_app.go @@ -234,8 +234,9 @@ func resourceArmFunctionApp() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "ip_address": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Optional: true, + ValidateFunc: validate.NoEmptyStrings, }, "subnet_id": { Type: schema.TypeString, @@ -777,31 +778,10 @@ func expandFunctionAppSiteConfig(d *schema.ResourceData) (web.SiteConfig, error) } if v, ok := config["ip_restriction"]; ok { - ipSecurityRestrictions := v.([]interface{}) - restrictions := make([]web.IPSecurityRestriction, 0) - for i, ipSecurityRestriction := range ipSecurityRestrictions { - restriction := ipSecurityRestriction.(map[string]interface{}) - - ipAddress := restriction["ip_address"].(string) - vNetSubnetID := restriction["subnet_id"].(string) - if vNetSubnetID != "" && ipAddress != "" { - return siteConfig, fmt.Errorf(fmt.Sprintf("only one of `ip_address` or `subnet_id` can set for `site_config.0.ip_restriction.%d`", i)) - } - - if vNetSubnetID == "" && ipAddress == "" { - return siteConfig, fmt.Errorf(fmt.Sprintf("one of `ip_address` or `subnet_id` must be set for `site_config.0.ip_restriction.%d`", i)) - } - - ipSecurityRestriction := web.IPSecurityRestriction{} - if ipAddress != "" { - ipSecurityRestriction.IPAddress = &ipAddress - } - - if vNetSubnetID != "" { - ipSecurityRestriction.VnetSubnetResourceID = &vNetSubnetID - } - - restrictions = append(restrictions, ipSecurityRestriction) + ipSecurityRestrictions := v.(interface{}) + restrictions, err := expandFunctionAppIpRestriction(ipSecurityRestrictions) + if err != nil { + return siteConfig, err } siteConfig.IPSecurityRestrictions = &restrictions } @@ -850,20 +830,7 @@ func flattenFunctionAppSiteConfig(input *web.SiteConfig) []interface{} { result["http2_enabled"] = *input.HTTP20Enabled } - restrictions := make([]interface{}, 0) - if vs := input.IPSecurityRestrictions; vs != nil { - for _, v := range *vs { - block := make(map[string]interface{}) - if ip := v.IPAddress; ip != nil { - block["ip_address"] = *ip - } - if vNetSubnetID := v.VnetSubnetResourceID; vNetSubnetID != nil { - block["subnet_id"] = *vNetSubnetID - } - restrictions = append(restrictions, block) - } - } - result["ip_restriction"] = restrictions + result["ip_restriction"] = flattenFunctionAppIpRestriction(input.IPSecurityRestrictions) result["min_tls_version"] = string(input.MinTLSVersion) result["ftps_state"] = string(input.FtpsState) @@ -894,6 +861,39 @@ func expandFunctionAppConnectionStrings(d *schema.ResourceData) map[string]*web. return output } +func expandFunctionAppIpRestriction(input interface{}) ([]web.IPSecurityRestriction, error) { + ipSecurityRestrictions := input.([]interface{}) + restrictions := make([]web.IPSecurityRestriction, 0) + + for i, ipSecurityRestriction := range ipSecurityRestrictions { + restriction := ipSecurityRestriction.(map[string]interface{}) + + ipAddress := restriction["ip_address"].(string) + vNetSubnetID := restriction["subnet_id"].(string) + + if vNetSubnetID != "" && ipAddress != "" { + return nil, fmt.Errorf(fmt.Sprintf("only one of `ip_address` or `subnet_id` can set for `site_config.0.ip_restriction.%d`", i)) + } + + if vNetSubnetID == "" && ipAddress == "" { + return nil, fmt.Errorf(fmt.Sprintf("one of `ip_address` or `subnet_id` must be set for `site_config.0.ip_restriction.%d`", i)) + } + + ipSecurityRestriction := web.IPSecurityRestriction{} + if ipAddress != "" { + ipSecurityRestriction.IPAddress = &ipAddress + } + + if vNetSubnetID != "" { + ipSecurityRestriction.VnetSubnetResourceID = &vNetSubnetID + } + + restrictions = append(restrictions, ipSecurityRestriction) + } + + return restrictions, nil +} + func flattenFunctionAppConnectionStrings(input map[string]*web.ConnStringValueTypePair) interface{} { results := make([]interface{}, 0) @@ -945,3 +945,24 @@ func flattenFunctionAppSiteCredential(input *web.UserProperties) []interface{} { return append(results, result) } + +func flattenFunctionAppIpRestriction(input *[]web.IPSecurityRestriction) []interface{} { + restrictions := make([]interface{}, 0) + + if input == nil { + return restrictions + } + + for _, v := range *input { + block := make(map[string]interface{}) + if ip := v.IPAddress; ip != nil { + block["ip_address"] = *ip + } + if vNetSubnetID := v.VnetSubnetResourceID; vNetSubnetID != nil { + block["subnet_id"] = *vNetSubnetID + } + restrictions = append(restrictions, block) + } + + return restrictions +} From a6d72276d41197ff9462be04b68c73d6f3ff0369 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Tue, 11 Feb 2020 10:55:11 +0100 Subject: [PATCH 9/9] r/function_app: ensuring the `ip_address` and `subnet_id` address fields are always set --- .../services/web/resource_arm_function_app.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/azurerm/internal/services/web/resource_arm_function_app.go b/azurerm/internal/services/web/resource_arm_function_app.go index 631688142a5f..2f1e5a298d98 100644 --- a/azurerm/internal/services/web/resource_arm_function_app.go +++ b/azurerm/internal/services/web/resource_arm_function_app.go @@ -954,14 +954,20 @@ func flattenFunctionAppIpRestriction(input *[]web.IPSecurityRestriction) []inter } for _, v := range *input { - block := make(map[string]interface{}) - if ip := v.IPAddress; ip != nil { - block["ip_address"] = *ip + ipAddress := "" + if v.IPAddress != nil { + ipAddress = *v.IPAddress } - if vNetSubnetID := v.VnetSubnetResourceID; vNetSubnetID != nil { - block["subnet_id"] = *vNetSubnetID + + subnetId := "" + if v.VnetSubnetResourceID != nil { + subnetId = *v.VnetSubnetResourceID } - restrictions = append(restrictions, block) + + restrictions = append(restrictions, map[string]interface{}{ + "ip_address": ipAddress, + "subnet_id": subnetId, + }) } return restrictions