azurerm_windows_function_app, azurerm_linux_function_app - fix content share not being created automatically issue
#18258
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -412,10 +412,20 @@ func (r WindowsFunctionAppResource) Create() sdk.ResourceFunc { | |
| } | ||
| if !functionApp.StorageUsesMSI { | ||
| suffix := uuid.New().String()[0:4] | ||
| _, contentOverVnetEnabled := functionApp.AppSettings["WEBSITE_CONTENTOVERVNET"] | ||
| _, contentSharePresent := functionApp.AppSettings["WEBSITE_CONTENTSHARE"] | ||
| _, contentShareConnectionString := functionApp.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") | ||
| } | ||
| functionApp.AppSettings["WEBSITE_CONTENTSHARE"] = fmt.Sprintf("%s-%s", strings.ToLower(functionApp.Name), suffix) | ||
| } | ||
| if !contentShareConnectionString { | ||
| if contentOverVnetEnabled && contentSharePresent { | ||
| return fmt.Errorf("WEBSITE_CONTENTAZUREFILECONNECTIONSTRING must be set when WEBSITE_CONTENTSHARE and WEBSITE_CONTENTOVERVNET is specified") | ||
| } | ||
| functionApp.AppSettings["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"] = storageString | ||
| } | ||
| } else { | ||
|
|
@@ -524,6 +534,7 @@ func (r WindowsFunctionAppResource) Read() sdk.ResourceFunc { | |
| Timeout: 5 * time.Minute, | ||
| Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error { | ||
| client := metadata.Client.AppService.WebAppsClient | ||
| servicePlanClient := metadata.Client.AppService.ServicePlanClient | ||
| id, err := parse.FunctionAppID(metadata.ResourceData.Id()) | ||
| if err != nil { | ||
| return err | ||
|
|
@@ -536,6 +547,22 @@ func (r WindowsFunctionAppResource) Read() sdk.ResourceFunc { | |
| return fmt.Errorf("reading Windows %s: %+v", id, err) | ||
| } | ||
|
|
||
| var planSku *string | ||
| if functionApp.ServerFarmID != nil { | ||
| servicePlanId, err := parse.ServicePlanID(*functionApp.ServerFarmID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| servicePlan, err := servicePlanClient.Get(ctx, servicePlanId.ResourceGroup, servicePlanId.ServerfarmName) | ||
| if err != nil { | ||
| return fmt.Errorf("reading %s: %+v", servicePlanId, err) | ||
| } | ||
| if sku := servicePlan.Sku; sku != nil && sku.Name != nil { | ||
| planSku = sku.Name | ||
| } | ||
| } | ||
| sendContentSettings := helpers.PlanIsElastic(planSku) | ||
|
|
||
xiaxyi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if functionApp.SiteProperties == nil { | ||
| return fmt.Errorf("reading properties of Windows %s", id) | ||
| } | ||
|
|
@@ -546,6 +573,43 @@ func (r WindowsFunctionAppResource) Read() sdk.ResourceFunc { | |
| return fmt.Errorf("reading App Settings for Windows %s: %+v", id, err) | ||
| } | ||
|
|
||
| var storageAccountName, storageFileShare string | ||
| if sendContentSettings || helpers.PlanIsConsumption(planSku) { | ||
| if appSettingsResp.Properties != nil { | ||
| if appSettingsResp.Properties["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"] != nil { | ||
| saConnectionString := strings.Split(*appSettingsResp.Properties["WEBSITE_CONTENTAZUREFILECONNECTIONSTRING"], ";") | ||
| for _, part := range saConnectionString { | ||
| if strings.Contains(part, "AccountName") { | ||
| storageAccountName = strings.TrimPrefix(part, "AccountName=") | ||
| break | ||
| } | ||
| } | ||
| } | ||
| if appSettingsResp.Properties["WEBSITE_CONTENTSHARE"] != nil { | ||
| storageFileShare = *appSettingsResp.Properties["WEBSITE_CONTENTSHARE"] | ||
| } | ||
| saClient := metadata.Client.Storage | ||
| account, err := saClient.FindAccount(ctx, storageAccountName) | ||
| if err != nil { | ||
| return fmt.Errorf("retrieving Account %q for Share %q: %s", storageAccountName, storageFileShare, err) | ||
| } | ||
| if account == nil { | ||
| return fmt.Errorf("unable to locate Storage Account %s", storageAccountName) | ||
| } | ||
| saFileShareClient, err := metadata.Client.Storage.FileSharesClient(ctx, *account) | ||
| if err != nil { | ||
| return fmt.Errorf("building File Share Client: %s", err) | ||
| } | ||
| exists, err := saFileShareClient.Exists(ctx, account.ResourceGroup, storageAccountName, storageFileShare) | ||
| if err != nil { | ||
| return fmt.Errorf("checking for existence of Storage Share %q (Account %q) : %+v", storageFileShare, storageAccountName, err) | ||
| } | ||
| if exists == nil { | ||
| return fmt.Errorf("the storage file share %s for function app %s does not exists, please specify an existing one", storageFileShare, id.ID()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. As above, this shouldn't be in the Read. There was a problem hiding this comment. Thanks for the review, this is to address the timeout issue. if the user doesn't specify a predefined file share and also set the WEBSITE_CONTENTOVERVNET in the app_setting, the provider will timeout by trying to access a non-existence file share. I can add the 4.0 condition to this check block. There was a problem hiding this comment. agreed - this shouldn't be in the Read function - we don't retrieve the file share in the Read, so this is unnecessary There was a problem hiding this comment. Thanks @tombuildsstuff for the comments. If we don't check the storage file share, shall we just let the read function timeout? There was a problem hiding this comment. @xiaxyi why will the read function timeout? it's calling the App Service ARM API and not interacting with the Storage Account in the read function? There was a problem hiding this comment. The function app is trying to connect to the file share specified in the If we don't check the file share in read function, then what's left is to add notes in docs and let the read function timeout. |
||
| connectionStrings, err := client.ListConnectionStrings(ctx, id.ResourceGroup, id.SiteName) | ||
| if err != nil { | ||
| return fmt.Errorf("reading Connection String information for Windows %s: %+v", id, err) | ||
|
|
@@ -702,7 +766,13 @@ 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, serviceFarmId) | ||
|
There was a problem hiding this comment. As above, the helper should not be repurposed, and the SKU should be read from the new service plan ID. A new helper for that would be acceptable. |
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -778,6 +848,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 | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is intended to retrieve the Service Plan ID from the App ID, it should not take the ID directly to circumvent this.