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

azurerm_windows_web_app : fix the issue of incorrect application stack setting after updating the web app without changing the application_stack configuration #23372

Merged
merged 4 commits into from
Nov 22, 2023
Merged

azurerm_windows_web_app : fix the issue of incorrect application stack setting after updating the web app without changing the application_stack configuration #23372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
db97f08
fix siteconfig could not be updated correctly issue
sinbai Sep 22, 2023
8dae90f
update code
sinbai Sep 25, 2023
9669515
update doc
sinbai Nov 20, 2023
fa7932f
add delta detection for app_stack items and update multistack test fr…
jackofallops Nov 22, 2023
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
70 changes: 44 additions & 26 deletions internal/services/appservice/helpers/windows_web_app_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,34 +617,46 @@ func (s *SiteConfigWindows) ExpandForUpdate(metadata sdk.ResourceMetaData, exist
if metadata.ResourceData.HasChange("site_config.0.application_stack") {
if len(s.ApplicationStack) == 1 {
winAppStack := s.ApplicationStack[0]
if winAppStack.NetFrameworkVersion != "" {
expanded.NetFrameworkVersion = pointer.To(winAppStack.NetFrameworkVersion)
}
if winAppStack.NetCoreVersion != "" {
expanded.NetFrameworkVersion = pointer.To(winAppStack.NetCoreVersion)
if metadata.ResourceData.HasChanges("site_config.0.application_stack.0.dotnet_version", "site_config.0.application_stack.0.dotnet_core_version") {
switch {
case winAppStack.NetFrameworkVersion != "":
expanded.NetFrameworkVersion = pointer.To(winAppStack.NetFrameworkVersion)
case winAppStack.NetCoreVersion != "":
expanded.NetFrameworkVersion = pointer.To(winAppStack.NetCoreVersion)
default:
expanded.NetFrameworkVersion = nil
}
}
if winAppStack.PhpVersion != "" {
if winAppStack.PhpVersion != PhpVersionOff {
expanded.PhpVersion = pointer.To(winAppStack.PhpVersion)
} else {
expanded.PhpVersion = pointer.To("")
if metadata.ResourceData.HasChange("site_config.0.application_stack.0.php_version") {
if winAppStack.PhpVersion != "" {
if winAppStack.PhpVersion != PhpVersionOff {
expanded.PhpVersion = pointer.To(winAppStack.PhpVersion)
} else {
expanded.PhpVersion = pointer.To("")
}
}
}
if winAppStack.PythonVersion != "" || winAppStack.Python {
expanded.PythonVersion = pointer.To(winAppStack.PythonVersion)
}
if winAppStack.JavaVersion != "" {
expanded.JavaVersion = pointer.To(winAppStack.JavaVersion)
switch {
case winAppStack.JavaEmbeddedServer:
expanded.JavaContainer = pointer.To(JavaContainerEmbeddedServer)
expanded.JavaContainerVersion = pointer.To(JavaContainerEmbeddedServerVersion)
case winAppStack.TomcatVersion != "":
expanded.JavaContainer = pointer.To(JavaContainerTomcat)
expanded.JavaContainerVersion = pointer.To(winAppStack.TomcatVersion)
case winAppStack.JavaContainer != "":
expanded.JavaContainer = pointer.To(winAppStack.JavaContainer)
expanded.JavaContainerVersion = pointer.To(winAppStack.JavaContainerVersion)
if metadata.ResourceData.HasChange("site_config.0.application_stack.0.java_version") {
if winAppStack.JavaVersion != "" {
expanded.JavaVersion = pointer.To(winAppStack.JavaVersion)
switch {
case winAppStack.JavaEmbeddedServer:
expanded.JavaContainer = pointer.To(JavaContainerEmbeddedServer)
expanded.JavaContainerVersion = pointer.To(JavaContainerEmbeddedServerVersion)
case winAppStack.TomcatVersion != "":
expanded.JavaContainer = pointer.To(JavaContainerTomcat)
expanded.JavaContainerVersion = pointer.To(winAppStack.TomcatVersion)
case winAppStack.JavaContainer != "":
expanded.JavaContainer = pointer.To(winAppStack.JavaContainer)
expanded.JavaContainerVersion = pointer.To(winAppStack.JavaContainerVersion)
}
} else {
expanded.JavaVersion = nil
expanded.JavaContainer = nil
expanded.JavaContainerVersion = nil
}
}
if !features.FourPointOhBeta() {
Expand Down Expand Up @@ -807,22 +819,28 @@ func (s *SiteConfigWindows) Flatten(appSiteConfig *web.SiteConfig, currentStack
var winAppStack ApplicationStackWindows
if currentStack == CurrentStackDotNetCore {
winAppStack.NetCoreVersion = pointer.From(appSiteConfig.NetFrameworkVersion)
}
if currentStack == CurrentStackDotNet {
} else {
winAppStack.NetFrameworkVersion = pointer.From(appSiteConfig.NetFrameworkVersion)
}

winAppStack.PhpVersion = pointer.From(appSiteConfig.PhpVersion)
if winAppStack.PhpVersion == "" {
winAppStack.PhpVersion = PhpVersionOff
}
winAppStack.PythonVersion = pointer.From(appSiteConfig.PythonVersion) // This _should_ always be `""`
winAppStack.Python = currentStack == CurrentStackPython
winAppStack.JavaVersion = pointer.From(appSiteConfig.JavaVersion)

// we should only set JavaVersion when currentStack is java since the API will return the value of JavaVersion that was once set
if currentStack == "java" {
Copy link
Member

Choose a reason for hiding this comment

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

If the optional property current_stack is not set, this will result in a diff. Since this is also true for the other cases where this is used, can we add a note to the docs for this property to call this out to users?, perhaps something like:

~> **NOTE:** Windows Web apps can configure multiple `app_stack` properties, it is recommended to always configure this `Optional` value and set it to the primary application stack of your app to ensure correct operation of this resource and display the correct metadata in the Azure Portal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jackofallops thanks for your feedback. The doc has been updated. Could you please take another look?

winAppStack.JavaVersion = pointer.From(appSiteConfig.JavaVersion)
}
switch pointer.From(appSiteConfig.JavaContainer) {
case JavaContainerTomcat:
winAppStack.TomcatVersion = *appSiteConfig.JavaContainerVersion
winAppStack.TomcatVersion = pointer.From(appSiteConfig.JavaContainerVersion)
winAppStack.JavaVersion = pointer.From(appSiteConfig.JavaVersion)
case JavaContainerEmbeddedServer:
winAppStack.JavaEmbeddedServer = true
winAppStack.JavaVersion = pointer.From(appSiteConfig.JavaVersion)
}

s.WindowsFxVersion = pointer.From(appSiteConfig.WindowsFxVersion)
Expand Down
7 changes: 7 additions & 0 deletions internal/services/appservice/windows_web_app_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,13 @@ func (r WindowsWebAppResource) Update() sdk.ResourceFunc {
return fmt.Errorf("reading Windows %s: %v", id, err)
}

// Despite being part of the defined `Get` response model, site_config is always nil so we get it explicitly
webAppSiteConfig, err := client.GetConfiguration(ctx, id.ResourceGroup, id.SiteName)
if err != nil {
return fmt.Errorf("reading Site Config for Windows %s: %+v", id, err)
}
existing.SiteConfig = webAppSiteConfig.SiteConfig

var serviceFarmId string
servicePlanChange := false
if existing.SiteProperties.ServerFarmID != nil {
Expand Down
48 changes: 41 additions & 7 deletions internal/services/appservice/windows_web_app_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,13 @@ func TestAccWindowsWebApp_updateAppStack(t *testing.T) {
),
},
data.ImportStep(),
{
Config: r.healthCheckUpdate(data, "11"),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
{
Config: r.dotNet(data, "v5.0"),
Check: acceptance.ComposeTestCheckFunc(
Expand Down Expand Up @@ -2569,6 +2576,34 @@ resource "azurerm_windows_web_app" "test" {
`, r.baseTemplate(data), data.RandomInteger, javaVersion)
}

//nolint:unparam
func (r WindowsWebAppResource) healthCheckUpdate(data acceptance.TestData, javaVersion string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

%s

resource "azurerm_windows_web_app" "test" {
name = "acctestWA-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
service_plan_id = azurerm_service_plan.test.id

site_config {
health_check_path = "/health"
health_check_eviction_time_in_min = 5
application_stack {
current_stack = "java"
java_version = "%s"
java_embedded_server_enabled = "true"
}
}
}
`, r.baseTemplate(data), data.RandomInteger, javaVersion)
}

func (r WindowsWebAppResource) javaTomcat(data acceptance.TestData, javaVersion string, tomcatVersion string) string {
return fmt.Sprintf(`
provider "azurerm" {
Expand Down Expand Up @@ -2610,18 +2645,17 @@ resource "azurerm_windows_web_app" "test" {

site_config {
application_stack {
dotnet_version = "%s"
php_version = "%s"
python = "%t"
java_version = "%s"
java_container = "%s"
java_container_version = "%s"
dotnet_version = "%s"
php_version = "%s"
python = "%t"
java_version = "%s"
tomcat_version = "%s"

current_stack = "python"
}
}
}
`, r.baseTemplate(data), data.RandomInteger, "v4.0", "7.4", true, "1.8", "TOMCAT", "9.0")
`, r.baseTemplate(data), data.RandomInteger, "v4.0", "7.4", true, "1.8", "9.0")
}

func (r WindowsWebAppResource) withLogsHttpBlob(data acceptance.TestData) string {
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/windows_web_app.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ An `application_stack` block supports the following:

~> **NOTE:** Whilst this property is Optional omitting it can cause unexpected behaviour, in particular for display of settings in the Azure Portal.

~> **NOTE:** Windows Web apps can configure multiple `app_stack` properties, it is recommended to always configure this `Optional` value and set it to the primary application stack of your app to ensure correct operation of this resource and display the correct metadata in the Azure Portal.

* `docker_image_name` - (Optional) The docker image, including tag, to be used. e.g. `azure-app-service/windows/parkingpage:latest`.

* `docker_registry_url` - (Optional) The URL of the container registry where the `docker_image_name` is located. e.g. `https://index.docker.io` or `https://mcr.microsoft.com`. This value is required with `docker_image_name`.
Expand Down
Loading