review changes pt2

internal/services/appservice/helpers/app_stack.go

@@ -55,7 +55,7 @@ func windowsApplicationStackSchema() *pluginsdk.Schema {
 					Type:     pluginsdk.TypeString,
 					Optional: true,
 					Computed: true,
-					ValidateFunc: validation.StringInSlice([]string{
+					ValidateFunc: validation.StringInSlice([]string{ // Note: DotNet versions are abstracted between API and Portal displayed values, so do not match 1:1. A table of the converted values is provided in the resource doc.
 						"v2.0",
 						"v3.0",
 						"v4.0",
@@ -508,7 +508,7 @@ func linuxApplicationStackSchema() *pluginsdk.Schema {
 					Optional: true,
 					ValidateFunc: validation.StringInSlice([]string{ // TODO replace with major.minor regex?
 						"2.6", // Deprecated - accepted but not offered in the portal. Remove in 4.0
-						"2.7",
+						"2.7", // EOL 31/03/2023 https://github.com/Azure/app-service-linux-docs/blob/master/Runtime_Support/ruby_support.md Remove Ruby support in 4.0?
 					}, false),
 					ExactlyOneOf: []string{
 						"site_config.0.application_stack.0.docker_image",

internal/services/appservice/helpers/auto_heal.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/services/web/mgmt/2021-02-01/web" // nolint: staticcheck
 	"github.com/hashicorp/go-azure-helpers/lang/pointer"
+	"github.com/hashicorp/terraform-provider-azurerm/internal/services/appservice/validate"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
 )
@@ -194,9 +195,9 @@ func autoHealTriggerSchemaWindows() *pluginsdk.Schema {
 							},
 
 							"interval": {
-								Type:     pluginsdk.TypeString,
-								Required: true,
-								// ValidateFunc: validation.IsRFC3339Time, // TODO should be hh:mm:ss - This is too loose, need to improve
+								Type:         pluginsdk.TypeString,
+								Required:     true,
+								ValidateFunc: validate.AutoHealInterval, // TODO should be hh:mm:ss - This is too loose, need to improve
 							},
 						},
 					},
@@ -216,7 +217,7 @@ func autoHealTriggerSchemaWindows() *pluginsdk.Schema {
 							"status_code_range": {
 								Type:         pluginsdk.TypeString,
 								Required:     true,
-								ValidateFunc: nil, // TODO - status code range validation
+								ValidateFunc: validate.StatusCodeRange,
 							},
 
 							"count": {
@@ -226,21 +227,19 @@ func autoHealTriggerSchemaWindows() *pluginsdk.Schema {
 							},
 
 							"interval": {
-								Type:     pluginsdk.TypeString,
-								Required: true,
-								// ValidateFunc: validation.IsRFC3339Time,
+								Type:         pluginsdk.TypeString,
+								Required:     true,
+								ValidateFunc: validate.AutoHealInterval,
 							},
 
 							"sub_status": {
-								Type:         pluginsdk.TypeInt,
-								Optional:     true,
-								ValidateFunc: nil, // TODO - no docs on this, needs investigation
+								Type:     pluginsdk.TypeInt,
+								Optional: true,
 							},
 
 							"win32_status": {
-								Type:         pluginsdk.TypeString,
-								Optional:     true,
-								ValidateFunc: nil, // TODO - no docs on this, needs investigation
+								Type:     pluginsdk.TypeString,
+								Optional: true,
 							},
 
 							"path": {
@@ -259,15 +258,15 @@ func autoHealTriggerSchemaWindows() *pluginsdk.Schema {
 					Elem: &pluginsdk.Resource{
 						Schema: map[string]*pluginsdk.Schema{
 							"time_taken": {
-								Type:     pluginsdk.TypeString,
-								Required: true,
-								// ValidateFunc: validation.IsRFC3339Time,
+								Type:         pluginsdk.TypeString,
+								Required:     true,
+								ValidateFunc: validate.AutoHealInterval,
 							},
 
 							"interval": {
-								Type:     pluginsdk.TypeString,
-								Required: true,
-								// ValidateFunc: validation.IsRFC3339Time,
+								Type:         pluginsdk.TypeString,
+								Required:     true,
+								ValidateFunc: validate.AutoHealInterval,
 							},
 
 							"count": {

internal/services/appservice/helpers/common_web_app_schema.go

@@ -1,6 +1,7 @@
 package helpers
 
 import (
+	"fmt"
 	"strconv"
 	"time"
 
@@ -344,10 +345,11 @@ func BackupSchema() *pluginsdk.Schema {
 							},
 
 							"start_time": {
-								Type:        pluginsdk.TypeString,
-								Optional:    true,
-								Computed:    true,
-								Description: "When the schedule should start working in RFC-3339 format.",
+								Type:         pluginsdk.TypeString,
+								Optional:     true,
+								Computed:     true,
+								Description:  "When the schedule should start working in RFC-3339 format.",
+								ValidateFunc: validation.IsRFC3339Time,
 							},
 
 							"last_execution_time": {
@@ -872,10 +874,10 @@ func ExpandLogsConfig(config []LogsConfig) *web.SiteLogsConfig {
 	return result
 }
 
-func ExpandBackupConfig(backupConfigs []Backup) *web.BackupRequest {
+func ExpandBackupConfig(backupConfigs []Backup) (*web.BackupRequest, error) {
 	result := &web.BackupRequest{}
 	if len(backupConfigs) == 0 {
-		return result
+		return result, nil
 	}
 
 	backupConfig := backupConfigs[0]
@@ -893,11 +895,14 @@ func ExpandBackupConfig(backupConfigs []Backup) *web.BackupRequest {
 	}
 
 	if backupSchedule.StartTime != "" {
-		dateTimeToStart, _ := time.Parse(time.RFC3339, backupSchedule.StartTime)
+		dateTimeToStart, err := time.Parse(time.RFC3339, backupSchedule.StartTime)
+		if err != nil {
+			return nil, fmt.Errorf("parsing back up start_time: %+v", err)
+		}
 		result.BackupRequestProperties.BackupSchedule.StartTime = &date.Time{Time: dateTimeToStart}
 	}
 
-	return result
+	return result, nil
 }
 
 func ExpandStorageConfig(storageConfigs []StorageAccount) *web.AzureStoragePropertyDictionaryResource {
@@ -1008,7 +1013,7 @@ func expandVirtualApplicationsForUpdate(virtualApplicationConfig []VirtualApplic
 
 func FlattenBackupConfig(backupRequest web.BackupRequest) []Backup {
 	if backupRequest.BackupRequestProperties == nil {
-		return nil
+		return []Backup{}
 	}
 	props := *backupRequest.BackupRequestProperties
 	backup := Backup{}
@@ -1056,7 +1061,7 @@ func FlattenBackupConfig(backupRequest web.BackupRequest) []Backup {
 
 func FlattenLogsConfig(logsConfig web.SiteLogsConfig) []LogsConfig {
 	if logsConfig.SiteLogsConfigProperties == nil {
-		return nil
+		return []LogsConfig{}
 	}
 	props := *logsConfig.SiteLogsConfigProperties
 	if onlyDefaultLoggingConfig(props) {
@@ -1163,7 +1168,7 @@ func onlyDefaultLoggingConfig(props web.SiteLogsConfigProperties) bool {
 
 func FlattenStorageAccounts(appStorageAccounts web.AzureStoragePropertyDictionaryResource) []StorageAccount {
 	if len(appStorageAccounts.Properties) == 0 {
-		return nil
+		return []StorageAccount{}
 	}
 	var storageAccounts []StorageAccount
 	for k, v := range appStorageAccounts.Properties {
@@ -1195,7 +1200,7 @@ func FlattenStorageAccounts(appStorageAccounts web.AzureStoragePropertyDictionar
 
 func FlattenConnectionStrings(appConnectionStrings web.ConnectionStringDictionary) []ConnectionString {
 	if len(appConnectionStrings.Properties) == 0 {
-		return nil
+		return []ConnectionString{}
 	}
 	var connectionStrings []ConnectionString
 	for k, v := range appConnectionStrings.Properties {
@@ -1237,7 +1242,7 @@ func ExpandAppSettingsForCreate(settings map[string]string) *[]web.NameValuePair
 	return nil
 }
 
-func FlattenAppSettings(input web.StringDictionary) (map[string]string, *int) {
+func FlattenAppSettings(input web.StringDictionary) (map[string]string, *int, error) {
 	maxPingFailures := "WEBSITE_HEALTHCHECK_MAXPINGFAILURES"
 	unmanagedSettings := []string{
 		"DIAGNOSTICS_AZUREBLOBCONTAINERSASURL",
@@ -1254,7 +1259,10 @@ func FlattenAppSettings(input web.StringDictionary) (map[string]string, *int) {
 	var healthCheckCount *int
 	appSettings := FlattenWebStringDictionary(input)
 	if v, ok := appSettings[maxPingFailures]; ok {
-		h, _ := strconv.Atoi(v)
+		h, err := strconv.Atoi(v)
+		if err != nil {
+			return nil, nil, fmt.Errorf("could not convert max ping failures to int")
+		}
 		healthCheckCount = &h
 	}
 
@@ -1263,7 +1271,7 @@ func FlattenAppSettings(input web.StringDictionary) (map[string]string, *int) {
 		delete(appSettings, v)
 	}
 
-	return appSettings, healthCheckCount
+	return appSettings, healthCheckCount, nil
 }
 
 func flattenVirtualApplications(appVirtualApplications *[]web.VirtualApplication) []VirtualApplication {

internal/services/appservice/helpers/function_app_slot_schema.go

@@ -921,7 +921,7 @@ func FlattenSiteConfigWindowsFunctionAppSlot(functionAppSlotSiteConfig *web.Site
 		NodeVersion:           "",    // Note: this will be set from app_settings later in unpackWindowsFunctionAppSettings
 		JavaVersion:           pointer.From(functionAppSlotSiteConfig.JavaVersion),
 		PowerShellCoreVersion: powershellVersion,
-		CustomHandler:         false, // set this later from app_settings
+		CustomHandler:         false, // Note: this is set later from app_settings
 	}}
 
 	return result, nil

internal/services/appservice/helpers/web_app_slot_schema.go

@@ -846,7 +846,7 @@ func ExpandSiteConfigWindowsWebAppSlot(siteConfig []SiteConfigWindowsWebAppSlot,
 				}
 			}
 			if winAppStack.NetCoreVersion != "" {
-				expanded.NetFrameworkVersion = pointer.To(winAppStack.NetFrameworkVersion)
+				expanded.NetFrameworkVersion = pointer.To(winAppStack.NetCoreVersion)
 				if currentStack == "" {
 					currentStack = CurrentStackDotNetCore
 				}

internal/services/appservice/helpers/windows_web_app_schema.go

@@ -7,6 +7,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/web/mgmt/2021-02-01/web" // nolint: staticcheck
 	"github.com/hashicorp/go-azure-helpers/lang/pointer"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"
+	"github.com/hashicorp/terraform-provider-azurerm/internal/services/apimanagement/parse"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/services/apimanagement/validate"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
@@ -461,6 +462,8 @@ func ExpandSiteConfigWindows(siteConfig []SiteConfigWindows, existing *web.SiteC
 				return nil, nil, fmt.Errorf("always_on cannot be set to true when using Free, F1, D1 Sku")
 			}
 			if expanded.AlwaysOn != nil && *expanded.AlwaysOn {
+				// TODO - This code will not work as expected due to the service plan always being updated before the App, so the apply will always error out on the Service Plan change if `always_on` is incorrectly set.
+				// Need to investigate if there's a way to avoid users needing to run 2 applies for this.
 				return nil, nil, fmt.Errorf("always_on feature has to be turned off before switching to a free/shared Sku")
 			}
 		}
@@ -486,7 +489,6 @@ func ExpandSiteConfigWindows(siteConfig []SiteConfigWindows, existing *web.SiteC
 	if metadata.ResourceData.HasChange("site_config.0.application_stack") {
 		if len(winSiteConfig.ApplicationStack) == 1 {
 			winAppStack := winSiteConfig.ApplicationStack[0]
-			// TODO - only one of these should be non-nil?
 			if winAppStack.NetFrameworkVersion != "" {
 				expanded.NetFrameworkVersion = pointer.To(winAppStack.NetFrameworkVersion)
 				if currentStack == "" {
@@ -572,7 +574,7 @@ func ExpandSiteConfigWindows(siteConfig []SiteConfigWindows, existing *web.SiteC
 	if metadata.ResourceData.HasChange("site_config.0.ip_restriction") {
 		ipRestrictions, err := ExpandIpRestrictions(winSiteConfig.IpRestriction)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, fmt.Errorf("expanding IP Restrictions: %+v", err)
 		}
 		expanded.IPSecurityRestrictions = ipRestrictions
 	}
@@ -582,7 +584,7 @@ func ExpandSiteConfigWindows(siteConfig []SiteConfigWindows, existing *web.SiteC
 	if metadata.ResourceData.HasChange("site_config.0.scm_ip_restriction") {
 		scmIpRestrictions, err := ExpandIpRestrictions(winSiteConfig.ScmIpRestriction)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, fmt.Errorf("expanding SCM IP Restrictions: %+v", err)
 		}
 		expanded.ScmIPSecurityRestrictions = scmIpRestrictions
 	}
@@ -652,9 +654,9 @@ func ExpandSiteConfigWindows(siteConfig []SiteConfigWindows, existing *web.SiteC
 	return expanded, &currentStack, nil
 }
 
-func FlattenSiteConfigWindows(appSiteConfig *web.SiteConfig, currentStack string, healthCheckCount *int) []SiteConfigWindows {
+func FlattenSiteConfigWindows(appSiteConfig *web.SiteConfig, currentStack string, healthCheckCount *int) ([]SiteConfigWindows, error) {
 	if appSiteConfig == nil {
-		return nil
+		return nil, nil
 	}
 
 	siteConfig := SiteConfigWindows{
@@ -688,7 +690,11 @@ func FlattenSiteConfigWindows(appSiteConfig *web.SiteConfig, currentStack string
 	}
 
 	if appSiteConfig.APIManagementConfig != nil && appSiteConfig.APIManagementConfig.ID != nil {
-		siteConfig.ApiManagementConfigId = *appSiteConfig.APIManagementConfig.ID
+		apiId, err := parse.ApiIDInsensitively(*appSiteConfig.APIManagementConfig.ID)
+		if err != nil {
+			return nil, fmt.Errorf("could not parse API Management ID: %+v", err)
+		}
+		siteConfig.ApiManagementConfigId = apiId.ID()
 	}
 
 	if appSiteConfig.APIDefinition != nil && appSiteConfig.APIDefinition.URL != nil {
@@ -763,5 +769,5 @@ func FlattenSiteConfigWindows(appSiteConfig *web.SiteConfig, currentStack string
 		}
 	}
 
-	return []SiteConfigWindows{siteConfig}
+	return []SiteConfigWindows{siteConfig}, nil
 }

internal/services/appservice/linux_function_app_resource.go

@@ -499,7 +499,10 @@ func (r LinuxFunctionAppResource) Create() sdk.ResourceFunc {
 				}
 			}
 
-			backupConfig := helpers.ExpandBackupConfig(functionApp.Backup)
+			backupConfig, err := helpers.ExpandBackupConfig(functionApp.Backup)
+			if err != nil {
+				return fmt.Errorf("expanding backup configuration for Linux %s: %+v", id, err)
+			}
 			if backupConfig.BackupRequestProperties != nil {
 				if _, err := client.UpdateBackupConfiguration(ctx, id.ResourceGroup, id.SiteName, *backupConfig); err != nil {
 					return fmt.Errorf("adding Backup Settings for Linux %s: %+v", id, err)
@@ -903,7 +906,11 @@ func (r LinuxFunctionAppResource) Update() sdk.ResourceFunc {
 			}
 
 			if metadata.ResourceData.HasChange("backup") {
-				backupUpdate := helpers.ExpandBackupConfig(state.Backup)
+				backupUpdate, err := helpers.ExpandBackupConfig(state.Backup)
+				if err != nil {
+					return fmt.Errorf("expanding backup configuration for Linux %s: %+v", *id, err)
+				}
+
 				if backupUpdate.BackupRequestProperties == nil {
 					if _, err := client.DeleteBackupConfiguration(ctx, id.ResourceGroup, id.SiteName); err != nil {
 						return fmt.Errorf("removing Backup Settings for Linux %s: %+v", id, err)

internal/services/appservice/linux_function_app_slot_resource.go

@@ -484,7 +484,11 @@ func (r LinuxFunctionAppSlotResource) Create() sdk.ResourceFunc {
 				return fmt.Errorf("waiting for creation of Linux %s: %+v", id, err)
 			}
 
-			backupConfig := helpers.ExpandBackupConfig(functionAppSlot.Backup)
+			backupConfig, err := helpers.ExpandBackupConfig(functionAppSlot.Backup)
+			if err != nil {
+				return fmt.Errorf("expanding backup configuration for Linux %s: %+v", id, err)
+			}
+
 			if backupConfig.BackupRequestProperties != nil {
 				if _, err := client.UpdateBackupConfigurationSlot(ctx, id.ResourceGroup, id.SiteName, *backupConfig, id.SlotName); err != nil {
 					return fmt.Errorf("adding Backup Settings for Linux %s: %+v", id, err)
@@ -839,7 +843,10 @@ func (r LinuxFunctionAppSlotResource) Update() sdk.ResourceFunc {
 			}
 
 			if metadata.ResourceData.HasChange("backup") {
-				backupUpdate := helpers.ExpandBackupConfig(state.Backup)
+				backupUpdate, err := helpers.ExpandBackupConfig(state.Backup)
+				if err != nil {
+					return fmt.Errorf("expanding backup configuration for Linux %s: %+v", *id, err)
+				}
 				if backupUpdate.BackupRequestProperties == nil {
 					if _, err := client.DeleteBackupConfigurationSlot(ctx, id.ResourceGroup, id.SiteName, id.SlotName); err != nil {
 						return fmt.Errorf("removing Backup Settings for Linux %s: %+v", id, err)

internal/services/appservice/linux_web_app_data_source.go

@@ -282,7 +282,10 @@ func (r LinuxWebAppDataSource) Read() sdk.ResourceFunc {
 			}
 
 			var healthCheckCount *int
-			webApp.AppSettings, healthCheckCount = helpers.FlattenAppSettings(appSettings)
+			webApp.AppSettings, healthCheckCount, err = helpers.FlattenAppSettings(appSettings)
+			if err != nil {
+				return fmt.Errorf("flattening app settings for Linux %s: %+v", id, err)
+			}
 			webApp.Kind = utils.NormalizeNilableString(existing.Kind)
 			webApp.Location = location.NormalizeNilable(existing.Location)
 			webApp.Tags = tags.ToTypedObject(existing.Tags)