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

[sinbai]

Two purposes of this PR:

• Application stack is incorrectly updated

Symptom:

1. Creating azurerm_windows_web_app with site_config as shown below.

site_config {
health_check_path           = "/health"
load_balancing_mode="LeastRequests"
  application_stack {
      current_stack                = "java"
    java_embedded_server_enabled = true
      java_version                 = "11"
    } 
}

2. Update azurerm_windows_web_app and only change health_check_eviction_time_in_min from null to 5 as follows.

site_config {
        health_check_eviction_time_in_min = 5
	health_check_path           = "/health"
	load_balancing_mode="LeastRequests"
        application_stack {
           current_stack                = "java"
	    java_embedded_server_enabled = true
            java_version                 = "11"
        }
}

Actual:

Expected:

Per the following code in the existing in Read(), I assume this should also be added to Update() function to solve the above issue.

// 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)
}

• I assume that we should only set JavaVersion in Read() function when currentStack is java since the API will return the value of JavaVersion that was once set.

Test result:
PASS: TestAccWindowsWebApp_updateAppStack (611.57s)

[jackofallops]

Hi @sinbai - Thanks for this change and apologies for the delayed review as we've been discussing this internally trying to find a better way to work with the app_stack values for Windows apps. I've left one comment for an additional change, since the current_stack property is Optional we should not rely on it. Given that there is other reliance on the value already present, we're going to accept it, but we'll need the comment adding to the docs for now, and we'll be taking a look at addressing it appropriately for 4.0 as we believe it will require a breaking change in the longer term.
Thanks!

[jackofallops]

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.

[sinbai]

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

[jackofallops]

Hi @sinbai - Thanks for adding the requested change. During testing I've found that this is actually doesn't work as expected due to a slightly larger problem / bug. I'm investigating now and I may need to push some changes to your branch to fix the broader problem. If that happens, I'll get another maintainer to review my additions and we'll get this in as soon as it's all good and testing works as expected.

outdated

[tombuildsstuff]

LGTM 👍

[jackofallops]

Test look good, failure transient and unrelated to this change

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.