azurerm_windows_function_app_slot - prevent removing application_stack related settings when stack block is not changed

[cek-cek]

The PR removes only the first redundant condition and adjusts line padding.

The reasoning is, that the condition prevents merging ApplicationStack fetched from existing appservice the to upcoming apply as application_stack block is usually not changed. When the condition is removed, the necessary existing stack settings will always be merged for the settings update. Linux variant of the code correctly omits this condition already.

Fixes #18947

[xiaxyi]

Thanks @cek-cek for raising this pr, I checked the code change and have some personal understandings as below. Feel free to let me know if it makes sense to you and I'm happy to have a discussion with you.

I see you are making changes to the FUNCTIONS_WORKER_RUNTIME in app_setting. This key is handled properly in current provider buy setting it in each runtime case, for example, powershell runtime:

appSettings = updateOrAppendAppSettings(appSettings, "FUNCTIONS_WORKER_RUNTIME", "powershell", false)

The issue is the runtime property in app's site config, for example, we also need to set the powershell version instead of just setting the windowsFxVersion and FUNCTIONS_WORKER_RUNTIME

expanded.JavaVersion = utils.String(windowsAppStack.JavaVersion)

For the detailed changes, feel free to check the pr: #18076

Thanks, feel free to let me know if you have any concern or question.

[cek-cek]

Hi @xiaxyi, thank you for looking into my PR. I've looked at your PR #18076 and I believe they fix different issues. I've tested both PRs through locally built provider on our issue and while this PR does fix the #18947, your PR does not have any effect on that particular misbehavior.

To explain in more depth, let me first attach the screenshot of my fix:

GitHub by default shows diffs including whitespace, which probably caused the confusion that I am changing FUNCTIONS_WORKER_RUNTIME etc. You need to explicitly turn off this behavior in settings to see the diff as on the screenshot. As I explained in PR's description, I've only removed this wrapping condition and therefore had to adjust whitespace of all of the lines in between, which breaks the default diff view.

The issue itself is that whenever we update our custom appsettings, the auto-managed settings such as FUNCTIONS_WORKER_RUNTIME or WEBSITE_NODE_DEFAULT_VERSION get lost. This is because the problematic to-be-removed condition propagates auto-managed settings to new appsettings only if application_block has changes. And update of application_block does not happen very often, but the auto-managed settings are essential to be kept.

The problematic condition is only present for Windows configuration. The "Linux sibling" function ExpandSiteConfigLinuxFunctionAppSlot() does not contain this optimization and works correctly. Also the main non-slot variant of the code is not affected.

I hope my explanation makes sense and this fix does not have any side effects, that I am not aware of. Honestly, to me, it just looks like the condition was there only as an optimization attempt. Should you have any other feedback or believe the #18947 should be fixed in any other way, feel free to let me know. Thank you.

[KenticoMartinS]

Hi, is there any ETA for this PR to be merged? It is really inconvenient as for nearly every terraform apply we need to update both FUNCTIONS_WORKER_RUNTIME and WEBSITE_NODE_DEFAULT_VERSION on most of the azure functions we use.

[jackofallops]

Hi @cek-cek - Thanks for this PR, and apologies for taking a while to get to it. I've had to address this issue in a broader change for application stacks and as a result I'm going to have to close this in favour of that. We appreciated the contribution, and apologies for the delay that has meant this needed to be superseded by #19685.

[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.