Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

azurerm_windows_function_app, azurerm_linux_function_app - fix content share not being created automatically issue #18258

Merged
merged 19 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
31 changes: 31 additions & 0 deletions internal/services/appservice/helpers/service_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,34 @@ func ServicePlanInfoForApp(ctx context.Context, metadata sdk.ResourceMetaData, i

return osType, planSku, nil
}

func GetServicePlanSku(ctx context.Context, metadata sdk.ResourceMetaData, id interface{}, serviceFarmId string) (osType *string, planSku *string, err error) {
servicePlanClient := metadata.Client.AppService.ServicePlanClient

// serviceFarmId remains unchanged
if serviceFarmId == "" {
return nil, nil, nil
}

servicePlanId, err := parse.ServicePlanID(serviceFarmId)
if err != nil {
return nil, nil, err
}

sp, err := servicePlanClient.Get(ctx, servicePlanId.ResourceGroup, servicePlanId.ServerfarmName)
if err != nil || sp.Kind == nil {
return nil, nil, fmt.Errorf("reading Service Plan for %s: %+v", id, err)
}

osType = utils.String("windows")
if strings.Contains(strings.ToLower(*sp.Kind), "linux") {
osType = utils.String("linux")
}

planSku = utils.String("")
if sku := sp.Sku; sku != nil {
planSku = sku.Name
}

return osType, planSku, nil
}
76 changes: 66 additions & 10 deletions internal/services/appservice/linux_function_app_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,9 @@ func (r LinuxFunctionAppResource) Create() sdk.ResourceFunc {
if sku := servicePlan.Sku; sku != nil && sku.Name != nil {
planSKU = sku.Name
}
// Only send for ElasticPremium
sendContentSettings := helpers.PlanIsElastic(planSKU) && !functionApp.ForceDisableContentShare
// Only send for ElasticPremium and Consumption plan
elasticOrConsumptionPlan := helpers.PlanIsElastic(planSKU) || helpers.PlanIsConsumption(planSKU)
sendContentSettings := elasticOrConsumptionPlan && !functionApp.ForceDisableContentShare

existing, err := client.Get(ctx, id.ResourceGroup, id.SiteName)
if err != nil && !utils.ResponseWasNotFound(existing.Response) {
Expand Down Expand Up @@ -424,17 +425,28 @@ func (r LinuxFunctionAppResource) Create() sdk.ResourceFunc {
functionApp.AppSettings["AzureWebJobsDashboard__accountName"] = functionApp.StorageAccountName
}
}

// for function premium app with WEBSITE_CONTENTOVERVNET sets to 1, the user has to specify a predefined fire share.
// https://docs.microsoft.com/en-us/azure/azure-functions/functions-app-settings#website_contentovervnet
if sendContentSettings {
if functionApp.AppSettings == nil {
functionApp.AppSettings = make(map[string]string)
}
if !functionApp.StorageUsesMSI {
suffix := uuid.New().String()[0:4]
if _, present := functionApp.AppSettings["WEBSITE_CONTENTSHARE"]; !present {
_, contentOverVnetEnabled := functionApp.AppSettings["WEBSITE_CONTENTOVERVNET"]
_, contentSharePresent := functionApp.AppSettings["WEBSITE_CONTENTSHARE"]
_, contentShareConnectionString := functionApp.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"]

if !contentSharePresent {
if contentOverVnetEnabled {
return fmt.Errorf("the app_setting WEBSITE_CONTENTSHARE must be specified and set to a valid share when WEBSITE_CONTENTOVERVNET is specified")
}
functionApp.AppSettings["WEBSITE_CONTENTSHARE"] = fmt.Sprintf("%s-%s", strings.ToLower(functionApp.Name), suffix)
}
if _, present := functionApp.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"]; !present {
if !contentShareConnectionString {
if contentOverVnetEnabled && contentSharePresent {
return fmt.Errorf("WEBSITE_CONTENTAZUREFILECONNECTIONSTRING must be set when WEBSITE_CONTENTSHARE and WEBSITE_CONTENTOVERVNET are specified")
}
functionApp.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"] = storageString
}
} else {
Expand Down Expand Up @@ -751,7 +763,6 @@ func (r LinuxFunctionAppResource) Update() sdk.ResourceFunc {
}

client := metadata.Client.AppService.WebAppsClient

id, err := parse.FunctionAppID(metadata.ResourceData.Id())
if err != nil {
return err
Expand All @@ -767,19 +778,40 @@ func (r LinuxFunctionAppResource) Update() sdk.ResourceFunc {
return fmt.Errorf("reading Linux %s: %v", id, err)
}

_, planSKU, err := helpers.ServicePlanInfoForApp(ctx, metadata, *id)
// Some service plan updates are allowed - see customiseDiff for exceptions
var serviceFarmId string
if metadata.ResourceData.HasChange("service_plan_id") {
serviceFarmId = state.ServicePlanId
existing.SiteProperties.ServerFarmID = utils.String(serviceFarmId)
}

_, planSKU, err := helpers.GetServicePlanSku(ctx, metadata, *id, serviceFarmId)
if err != nil {
return err
}

// Only send for ElasticPremium
sendContentSettings := helpers.PlanIsElastic(planSKU) && !state.ForceDisableContentShare

// Some service plan updates are allowed - see customiseDiff for exceptions
if metadata.ResourceData.HasChange("service_plan_id") {
existing.SiteProperties.ServerFarmID = utils.String(state.ServicePlanId)
servicePlanId, err := parse.ServicePlanID(state.ServicePlanId)
if err != nil {
return err
}

servicePlanClient := metadata.Client.AppService.ServicePlanClient
servicePlan, err := servicePlanClient.Get(ctx, servicePlanId.ResourceGroup, servicePlanId.ServerfarmName)
if err != nil {
return fmt.Errorf("reading new service plan (%s) for Linux %s: %+v", servicePlanId, id, err)
}

if sku := servicePlan.Sku; sku != nil && sku.Name != nil {
planSKU = sku.Name
}
}

// Only send for ElasticPremium and consumption plan
sendContentSettings := (helpers.PlanIsConsumption(planSKU) || helpers.PlanIsElastic(planSKU)) && !state.ForceDisableContentShare

if metadata.ResourceData.HasChange("enabled") {
existing.SiteProperties.Enabled = utils.Bool(state.Enabled)
}
Expand Down Expand Up @@ -854,6 +886,30 @@ func (r LinuxFunctionAppResource) Update() sdk.ResourceFunc {
state.AppSettings = make(map[string]string)
}
state.AppSettings = helpers.ParseContentSettings(appSettingsResp, state.AppSettings)

if !state.StorageUsesMSI {
suffix := uuid.New().String()[0:4]
_, contentOverVnetEnabled := state.AppSettings["WEBSITE_CONTENTOVERVNET"]
_, contentSharePresent := state.AppSettings["WEBSITE_CONTENTSHARE"]
_, contentShareConnectionString := state.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"]

if !contentSharePresent {
if contentOverVnetEnabled {
return fmt.Errorf("the value of WEBSITE_CONTENTSHARE must be set to a predefined share when the storage account is restricted to a virtual network")
}
state.AppSettings["WEBSITE_CONTENTSHARE"] = fmt.Sprintf("%s-%s", strings.ToLower(state.Name), suffix)
}
if !contentShareConnectionString {
if contentOverVnetEnabled && contentSharePresent {
return fmt.Errorf("WEBSITE_CONTENTAZUREFILECONNECTIONSTRING must be set when WEBSITE_CONTENTSHARE and WEBSITE_CONTENTOVERVNET is specified")
}
state.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"] = storageString
}
} else {
if _, present := state.AppSettings["AzureWebJobsStorage__accountName"]; !present {
state.AppSettings["AzureWebJobsStorage__accountName"] = storageString
}
}
}

// Note: We process this regardless to give us a "clean" view of service-side app_settings, so we can reconcile the user-defined entries later
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,22 @@ func TestAccLinuxFunctionApp_withCustomContentShareElasticPremiumPlan(t *testing
})
}

func TestAccLinuxFunctionApp_withCustomContentShareVnetEnabled(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_linux_function_app", "test")
r := LinuxFunctionAppResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.appSettingsCustomContentShareWithVnetEnabled(data, SkuConsumptionPlan),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("kind").HasValue("functionapp,linux"),
),
},
data.ImportStep("app_settings.WEBSITE_CONTENTSHARE", "app_settings.WEBSITE_CONTENTAZUREFILECONNECTIONSTRING", "app_settings.%"),
xiaxyi marked this conversation as resolved.
Show resolved Hide resolved
})
}

func TestAccLinuxFunctionApp_withAppSettingsPremiumPlan(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_linux_function_app", "test")
r := LinuxFunctionAppResource{}
Expand Down Expand Up @@ -1791,6 +1807,40 @@ resource "azurerm_linux_function_app" "test" {
`, r.template(data, planSku), data.RandomInteger)
}

func (r LinuxFunctionAppResource) appSettingsCustomContentShareWithVnetEnabled(data acceptance.TestData, planSku string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

%s

resource "azurerm_storage_share" "test" {
name = "test"
storage_account_name = azurerm_storage_account.test.name
quota = 1
}

resource "azurerm_linux_function_app" "test" {
name = "acctest-LFA-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
service_plan_id = azurerm_service_plan.test.id

storage_account_name = azurerm_storage_account.test.name
storage_account_access_key = azurerm_storage_account.test.primary_access_key

app_settings = {
WEBSITE_CONTENTOVERVNET = 1
WEBSITE_CONTENTSHARE = azurerm_storage_share.test.name
WEBSITE_CONTENTAZUREFILECONNECTIONSTRING = azurerm_storage_account.test.primary_connection_string
}

site_config {}
}
`, r.template(data, planSku), data.RandomInteger)
}

func (r LinuxFunctionAppResource) stickySettings(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
Expand Down
65 changes: 62 additions & 3 deletions internal/services/appservice/windows_function_app_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,20 @@ func (r WindowsFunctionAppResource) Create() sdk.ResourceFunc {
}
if !functionApp.StorageUsesMSI {
suffix := uuid.New().String()[0:4]
if _, present := functionApp.AppSettings["WEBSITE_CONTENTSHARE"]; !present {
_, contentOverVnetEnabled := functionApp.AppSettings["WEBSITE_CONTENTOVERVNET"]
_, contentSharePresent := functionApp.AppSettings["WEBSITE_CONTENTSHARE"]
_, contentShareConnectionString := functionApp.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"]

if !contentSharePresent {
if contentOverVnetEnabled {
return fmt.Errorf("the app_setting WEBSITE_CONTENTSHARE must be specified and set to a valid share when WEBSITE_CONTENTOVERVNET is specified")
}
functionApp.AppSettings["WEBSITE_CONTENTSHARE"] = fmt.Sprintf("%s-%s", strings.ToLower(functionApp.Name), suffix)
}
if _, present := functionApp.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"]; !present {
if !contentShareConnectionString {
if contentOverVnetEnabled && contentSharePresent {
return fmt.Errorf("WEBSITE_CONTENTAZUREFILECONNECTIONSTRING must be set when WEBSITE_CONTENTSHARE and WEBSITE_CONTENTOVERVNET are specified")
}
functionApp.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"] = storageString
}
} else {
Expand Down Expand Up @@ -767,12 +777,37 @@ func (r WindowsFunctionAppResource) Update() sdk.ResourceFunc {
return fmt.Errorf("reading Windows %s: %v", id, err)
}

var serviceFarmId string
if metadata.ResourceData.HasChange("service_plan_id") {
serviceFarmId = state.ServicePlanId
existing.SiteProperties.ServerFarmID = utils.String(serviceFarmId)
}

_, planSKU, err := helpers.ServicePlanInfoForApp(ctx, metadata, *id)
if err != nil {
return err
}

// Only send for Dynamic and ElasticPremium
// Some service plan updates are allowed - see customiseDiff for exceptions
if metadata.ResourceData.HasChange("service_plan_id") {
existing.SiteProperties.ServerFarmID = utils.String(state.ServicePlanId)
servicePlanId, err := parse.ServicePlanID(state.ServicePlanId)
if err != nil {
return err
}

servicePlanClient := metadata.Client.AppService.ServicePlanClient
servicePlan, err := servicePlanClient.Get(ctx, servicePlanId.ResourceGroup, servicePlanId.ServerfarmName)
if err != nil {
return fmt.Errorf("reading new service plan (%s) for Windows %s: %+v", servicePlanId, id, err)
}

if sku := servicePlan.Sku; sku != nil && sku.Name != nil {
planSKU = sku.Name
}
}

// Only send for ElasticPremium and consumption plan
sendContentSettings := (helpers.PlanIsConsumption(planSKU) || helpers.PlanIsElastic(planSKU)) && !state.ForceDisableContentShare

// Some service plan updates are allowed - see customiseDiff for exceptions
Expand Down Expand Up @@ -854,6 +889,30 @@ func (r WindowsFunctionAppResource) Update() sdk.ResourceFunc {
state.AppSettings = make(map[string]string)
}
state.AppSettings = helpers.ParseContentSettings(appSettingsResp, state.AppSettings)

if !state.StorageUsesMSI {
suffix := uuid.New().String()[0:4]
_, contentOverVnetEnabled := state.AppSettings["WEBSITE_CONTENTOVERVNET"]
_, contentSharePresent := state.AppSettings["WEBSITE_CONTENTSHARE"]
_, contentShareConnectionString := state.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"]

if !contentSharePresent {
if contentOverVnetEnabled {
return fmt.Errorf("the value of WEBSITE_CONTENTSHARE must be set to a predefined share when the storage account is restricted to a virtual network")
}
state.AppSettings["WEBSITE_CONTENTSHARE"] = fmt.Sprintf("%s-%s", strings.ToLower(state.Name), suffix)
}
if !contentShareConnectionString {
if contentOverVnetEnabled && contentSharePresent {
return fmt.Errorf("WEBSITE_CONTENTAZUREFILECONNECTIONSTRING must be set when WEBSITE_CONTENTSHARE and WEBSITE_CONTENTOVERVNET is specified")
}
state.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"] = storageString
}
} else {
if _, present := state.AppSettings["AzureWebJobsStorage__accountName"]; !present {
state.AppSettings["AzureWebJobsStorage__accountName"] = storageString
}
}
}

// Note: We process this regardless to give us a "clean" view of service-side app_settings, so we can reconcile the user-defined entries later
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,22 @@ func TestAccWindowsFunctionApp_withCustomContentShareElasticPremiumPlan(t *testi
})
}

func TestAccWindowsFunctionApp_withCustomContentShareVnetEnabled(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_windows_function_app", "test")
r := WindowsFunctionAppResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.appSettingsCustomContentShareVnetEnabled(data, SkuConsumptionPlan),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("kind").HasValue("functionapp"),
),
},
data.ImportStep("app_settings.WEBSITE_CONTENTSHARE", "app_settings.WEBSITE_CONTENTAZUREFILECONNECTIONSTRING", "app_settings.%"),
xiaxyi marked this conversation as resolved.
Show resolved Hide resolved
})
}

func TestAccWindowsFunctionApp_withAppSettingsPremiumPlan(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_windows_function_app", "test")
r := WindowsFunctionAppResource{}
Expand Down Expand Up @@ -1557,6 +1573,40 @@ resource "azurerm_windows_function_app" "test" {
`, r.template(data, planSku), data.RandomInteger)
}

func (r WindowsFunctionAppResource) appSettingsCustomContentShareVnetEnabled(data acceptance.TestData, planSku string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

%s

resource "azurerm_storage_share" "test" {
name = "test"
storage_account_name = azurerm_storage_account.test.name
quota = 1
}

resource "azurerm_windows_function_app" "test" {
name = "acctest-WFA-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
service_plan_id = azurerm_service_plan.test.id

storage_account_name = azurerm_storage_account.test.name
storage_account_access_key = azurerm_storage_account.test.primary_access_key

app_settings = {
WEBSITE_CONTENTOVERVNET = 1
WEBSITE_CONTENTSHARE = azurerm_storage_share.test.name
WEBSITE_CONTENTAZUREFILECONNECTIONSTRING = azurerm_storage_account.test.primary_connection_string
}

site_config {}
}
`, r.template(data, planSku), data.RandomInteger)
}

func (r WindowsFunctionAppResource) stickySettings(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
Expand Down
Loading