From be45d87762f83bc2b5f18e0e7d53f285d70647e1 Mon Sep 17 00:00:00 2001 From: jackofallops <11830746+jackofallops@users.noreply.github.com> Date: Tue, 5 Oct 2021 16:19:16 +0100 Subject: [PATCH] `azurerm_function_app` - fix regressions in function app storage (#13580) --- internal/services/web/function_app.go | 23 +++---- .../services/web/function_app_resource.go | 11 ++-- .../web/function_app_resource_test.go | 64 +++++++++++++++++++ 3 files changed, 81 insertions(+), 17 deletions(-) diff --git a/internal/services/web/function_app.go b/internal/services/web/function_app.go index b47ad9037206..5e9096de79b8 100644 --- a/internal/services/web/function_app.go +++ b/internal/services/web/function_app.go @@ -283,7 +283,7 @@ func schemaFunctionAppDataSourceSiteConfig() *pluginsdk.Schema { } } -func getBasicFunctionAppAppSettings(d *pluginsdk.ResourceData, appServiceTier, endpointSuffix string, existingSettings *[]web.NameValuePair) ([]web.NameValuePair, error) { +func getBasicFunctionAppAppSettings(d *pluginsdk.ResourceData, appServiceTier, endpointSuffix string, existingSettings map[string]*string) ([]web.NameValuePair, error) { // TODO: This is a workaround since there are no public Functions API // You may track the API request here: https://github.com/Azure/azure-rest-api-specs/issues/3750 dashboardPropName := "AzureWebJobsDashboard" @@ -323,17 +323,13 @@ func getBasicFunctionAppAppSettings(d *pluginsdk.ResourceData, appServiceTier, e functionVersion := d.Get("version").(string) var contentShare string - if existingSettings == nil { - // generate and use a new value + contentSharePreviouslySet := false + if currentContentShare, ok := existingSettings[contentSharePropName]; ok { + contentShare = *currentContentShare + contentSharePreviouslySet = true + } else { suffix := uuid.New().String()[0:4] contentShare = strings.ToLower(d.Get("name").(string)) + suffix - } else { - // find and re-use the current - for _, v := range *existingSettings { - if v.Name != nil && *v.Name == contentSharePropName { - contentShare = *v.Name - } - } } basicSettings := []web.NameValuePair{ @@ -353,8 +349,13 @@ func getBasicFunctionAppAppSettings(d *pluginsdk.ResourceData, appServiceTier, e {Name: &contentFileConnStringPropName, Value: &storageConnection}, } + // If there's an existing value for content, we need to send it. This can be the case for PremiumV2/PremiumV3 plans where the value has been previously configured. + if contentSharePreviouslySet { + return append(basicSettings, consumptionSettings...), nil + } + // On consumption and premium plans include WEBSITE_CONTENT components, unless it's a Linux consumption plan - // (see https://github.com/Azure/azure-functions-python-worker/issues/598) + // Note: The docs on this are misleading. Premium here refers explicitly to `ElasticPremium`, and not `PremiumV2` / `PremiumV3` etc. if !(strings.EqualFold(appServiceTier, "dynamic") && strings.EqualFold(d.Get("os_type").(string), "linux")) && (strings.EqualFold(appServiceTier, "dynamic") || strings.HasPrefix(strings.ToLower(appServiceTier), "elastic")) { return append(basicSettings, consumptionSettings...), nil diff --git a/internal/services/web/function_app_resource.go b/internal/services/web/function_app_resource.go index f4dce7ad4216..6084dec241a0 100644 --- a/internal/services/web/function_app_resource.go +++ b/internal/services/web/function_app_resource.go @@ -428,14 +428,13 @@ func resourceFunctionAppUpdate(d *pluginsdk.ResourceData, meta interface{}) erro return err } - existing, err := client.GetConfiguration(ctx, id.ResourceGroup, id.SiteName) + var currentAppSettings map[string]*string + appSettingsList, err := client.ListApplicationSettings(ctx, id.ResourceGroup, id.SiteName) if err != nil { - return fmt.Errorf("reading %s: %+v", id, err) + return fmt.Errorf("reading App Settings for %s: %+v", id, err) } - - var currentAppSettings *[]web.NameValuePair - if existing.AppSettings != nil { - currentAppSettings = existing.AppSettings + if appSettingsList.Properties != nil { + currentAppSettings = appSettingsList.Properties } basicAppSettings, err := getBasicFunctionAppAppSettings(d, appServiceTier, endpointSuffix, currentAppSettings) diff --git a/internal/services/web/function_app_resource_test.go b/internal/services/web/function_app_resource_test.go index 62a0c4dd5170..93238fa6afe5 100644 --- a/internal/services/web/function_app_resource_test.go +++ b/internal/services/web/function_app_resource_test.go @@ -224,6 +224,22 @@ func TestAccFunctionApp_linuxFxVersion(t *testing.T) { }) } +func TestAccFunctionApp_elasticPremiumPlanLinux(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_function_app", "test") + r := FunctionAppResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.linuxElasticPremium(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + acceptance.TestCheckResourceAttr(data.ResourceName, "kind", "functionapp,linux"), + ), + }, + data.ImportStep(), + }) +} + func TestAccFunctionApp_siteConfigVnetRouteAllEnabled(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_function_app", "test") r := FunctionAppResource{} @@ -1452,6 +1468,7 @@ resource "azurerm_function_app" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomString) } + func (r FunctionAppResource) appSettingsLinux(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { @@ -1689,6 +1706,53 @@ resource "azurerm_function_app" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomString) } +func (r FunctionAppResource) linuxElasticPremium(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%[1]d" + location = "%[2]s" +} + +resource "azurerm_storage_account" "test" { + name = "acctestsa%[3]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-%[1]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + kind = "elastic" + + sku { + tier = "ElasticPremium" + size = "EP1" + } + + reserved = true +} + +resource "azurerm_function_app" "test" { + name = "acctest-%[1]d-func" + location = azurerm_resource_group.test.location + version = "~1" + resource_group_name = azurerm_resource_group.test.name + app_service_plan_id = azurerm_app_service_plan.test.id + storage_account_name = azurerm_storage_account.test.name + storage_account_access_key = azurerm_storage_account.test.primary_access_key + os_type = "linux" + +} +`, data.RandomInteger, data.Locations.Primary, data.RandomString) +} + func (r FunctionAppResource) siteConfigVnetRouteAllEnabled(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" {