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.

Sign up for GitHub

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

Function app consumption plan bug #1515

Merged
merged 6 commits into from
Jul 16, 2018
Merged

Function app consumption plan bug #1515

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dc57466
Do not load WEBSITE_CONTENT app settings when on a consumption plan
ranieuwe Jul 7, 2018
dafae77
Merge branch 'master' into bug-appservice-function-consumption
ranieuwe Jul 7, 2018
cb19e80
Inverse the dynamic plan check, thanks @APErebus
ranieuwe Jul 11, 2018
c71f674
'%s' -> %q for safety
ranieuwe Jul 11, 2018
b683149
Add a nil check
ranieuwe Jul 11, 2018
342665d
Added tests to ensure the settings are removed:
tombuildsstuff Jul 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 48 additions & 7 deletions azurerm/resource_arm_function_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package azurerm

import (
"context"
"fmt"
"log"
"strings"

"github.com/Azure/azure-sdk-for-go/services/web/mgmt/2016-09-01/web"
"github.com/hashicorp/terraform/helper/schema"
Expand Down Expand Up @@ -214,7 +216,14 @@ func resourceArmFunctionAppCreate(d *schema.ResourceData, meta interface{}) erro
clientAffinityEnabled := d.Get("client_affinity_enabled").(bool)
httpsOnly := d.Get("https_only").(bool)
tags := d.Get("tags").(map[string]interface{})
basicAppSettings := getBasicFunctionAppAppSettings(d)
appServiceTier, err := getFunctionAppServiceTier(ctx, appServicePlanID, meta)

if err != nil {
return err
}

basicAppSettings := getBasicFunctionAppAppSettings(d, appServiceTier)

siteConfig := expandFunctionAppSiteConfig(d)
siteConfig.AppSettings = &basicAppSettings

Expand Down Expand Up @@ -279,7 +288,13 @@ func resourceArmFunctionAppUpdate(d *schema.ResourceData, meta interface{}) erro
clientAffinityEnabled := d.Get("client_affinity_enabled").(bool)
httpsOnly := d.Get("https_only").(bool)
tags := d.Get("tags").(map[string]interface{})
basicAppSettings := getBasicFunctionAppAppSettings(d)

appServiceTier, err := getFunctionAppServiceTier(ctx, appServicePlanID, meta)

if err != nil {
return err
}
basicAppSettings := getBasicFunctionAppAppSettings(d, appServiceTier)
siteConfig := expandFunctionAppSiteConfig(d)
siteConfig.AppSettings = &basicAppSettings

Expand Down Expand Up @@ -312,7 +327,7 @@ func resourceArmFunctionAppUpdate(d *schema.ResourceData, meta interface{}) erro
return err
}

appSettings := expandFunctionAppAppSettings(d)
appSettings := expandFunctionAppAppSettings(d, appServiceTier)
settings := web.StringDictionary{
Properties: appSettings,
}
Expand Down Expand Up @@ -458,7 +473,7 @@ func resourceArmFunctionAppDelete(d *schema.ResourceData, meta interface{}) erro
return nil
}

func getBasicFunctionAppAppSettings(d *schema.ResourceData) []web.NameValuePair {
func getBasicFunctionAppAppSettings(d *schema.ResourceData, appServiceTier string) []web.NameValuePair {
dashboardPropName := "AzureWebJobsDashboard"
storagePropName := "AzureWebJobsStorage"
functionVersionPropName := "FUNCTIONS_EXTENSION_VERSION"
Expand All @@ -469,19 +484,45 @@ func getBasicFunctionAppAppSettings(d *schema.ResourceData) []web.NameValuePair
functionVersion := d.Get("version").(string)
contentShare := d.Get("name").(string) + "-content"

return []web.NameValuePair{
basicSettings := []web.NameValuePair{
{Name: &dashboardPropName, Value: &storageConnection},
{Name: &storagePropName, Value: &storageConnection},
{Name: &functionVersionPropName, Value: &functionVersion},
}

consumptionSettings := []web.NameValuePair{
{Name: &contentSharePropName, Value: &contentShare},
{Name: &contentFileConnStringPropName, Value: &storageConnection},
}

// If the application plan is dynamic (consumption), we do not want to include WEBSITE_CONTENT components
if strings.EqualFold(appServiceTier, "dynamic") {
Copy link

@APErebus APErebus Jul 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is incorrect. We want the extra app settings if the app service tier is dynamic. We do not want these settings for standard/non-consumption app services.

Just missing an invert on this boolean check 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are absolute right. D'oh. Thanks for spotting this. Will fix

return basicSettings
}
return append(basicSettings, consumptionSettings...)
}

func getFunctionAppServiceTier(ctx context.Context, appServicePlanId string, meta interface{}) (string, error) {
id, err := parseAzureResourceID(appServicePlanId)
if err != nil {
return "", fmt.Errorf("[ERROR] Unable to parse App Service Plan ID '%s': %+v", appServicePlanId, err)
}

log.Printf("[DEBUG] Retrieving App Server Plan %s", id.Path["serverfarms"])

appServicePlansClient := meta.(*ArmClient).appServicePlansClient
appServicePlan, err := appServicePlansClient.Get(ctx, id.ResourceGroup, id.Path["serverfarms"])
if err != nil {
return "", fmt.Errorf("[ERROR] Could not retrieve App Service Plan ID '%s': %+v", appServicePlanId, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor these two '%s' can become %q

}

return *appServicePlan.Sku.Tier, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a crash here if appServicePlan.Sku or appServicePlan.Sku.Tier is nil (which happens for older resources); can we make this:

if sku := appServicePlan.Sku; sku != nil {
 if tier := sku.Tier; tier != nil {
    return *tier, nil
  }
}
return fmt.Errorf("No `sku` block was returned for App Service Plan %q (Resource Group %q)", resourceGroup, name)

}

func expandFunctionAppAppSettings(d *schema.ResourceData) map[string]*string {
func expandFunctionAppAppSettings(d *schema.ResourceData, appServiceTier string) map[string]*string {
output := expandAppServiceAppSettings(d)

basicAppSettings := getBasicFunctionAppAppSettings(d)
basicAppSettings := getBasicFunctionAppAppSettings(d, appServiceTier)
for _, p := range basicAppSettings {
output[*p.Name] = p.Value
}
Expand Down