`azurerm_windows[linux]_web_app_[slot]` - add docker setting to `app_setting` block #22484

xiaxyi · 2023-07-12T08:02:39Z

Fixes

Since app_setting exists in two places, one is under site_config while the other is under site, current, when user sets the docker related settings in site_config such as docker_registry_url, docker_registry_username and docker_registry_password, these settings are not being updated correctly in app_setting under Site. The app_setting under site_config is not taking any effects...
handle provider crash due to nil map.

fix #22379
fix #22436
fix #22435
fix #22548

jackofallops

Hi @xiaxyi - Thanks for this PR, I've left some comments below to more cleanly resolve the reported bugs, if you can take a look?

Thanks!

jackofallops · 2023-08-10T13:43:39Z

internal/services/appservice/helpers/linux_web_app_schema.go

+				if appSettings == nil {
+					appSettings = map[string]string{}
+				}


This avoids the crash, but unfortunately doesn't correctly address the bug of the values not being set, as the line

expanded.AppSettings = ExpandAppSettingsForCreate(appSettings)

is missing from this function.

jackofallops · 2023-08-10T13:45:39Z

internal/services/appservice/helpers/web_app_slot_schema.go

+			if appSettings == nil {
+				appSettings = map[string]string{}
+			}


As above, the missing values bug is due to the app settings not being written to the expand via

expanded.AppSettings = ExpandAppSettingsForCreate(appSettings)

after the entries are added.

jackofallops · 2023-08-10T13:46:01Z

internal/services/appservice/helpers/web_app_slot_schema.go

+				if appSettings == nil {
+					appSettings = map[string]string{}
+				}


Same here, as above.

jackofallops · 2023-08-10T13:46:22Z

internal/services/appservice/helpers/web_app_slot_schema.go

+			if appSettings == nil {
+				appSettings = map[string]string{}
+			}


jackofallops · 2023-08-10T13:46:36Z

internal/services/appservice/helpers/web_app_slot_schema.go

+				if appSettings == nil {
+					appSettings = map[string]string{}
+				}


and again here

jackofallops · 2023-08-10T13:48:54Z

internal/services/appservice/linux_web_app_slot_resource.go

+			if metadata.ResourceData.HasChange("app_settings") || metadata.ResourceData.HasChange("site_config.0.health_check_eviction_time_in_min") || metadata.ResourceData.HasChanges("site_config.0.application_stack") {
+				if siteConfigAppSetting := existing.SiteConfig.AppSettings; siteConfigAppSetting != nil && len(*siteConfigAppSetting) > 0 {
+					if state.AppSettings == nil {
+						state.AppSettings = make(map[string]string)
+					}
+					for _, pair := range *siteConfigAppSetting {
+						if pair.Name == nil || pair.Value == nil {
+							continue
+						}
+						if *pair.Name == "DOCKER_REGISTRY_SERVER_URL" ||
+							*pair.Name == "DOCKER_REGISTRY_SERVER_USERNAME" ||
+							*pair.Name == "DOCKER_REGISTRY_SERVER_PASSWORD" {
+							state.AppSettings[*pair.Name] = *pair.Value
+						}
+					}
+				}


jackofallops · 2023-08-10T13:49:02Z

internal/services/appservice/windows_web_app_resource.go

+			if siteConfigAppSetting := siteConfig.AppSettings; siteConfigAppSetting != nil && len(*siteConfigAppSetting) > 0 {
+				if webApp.AppSettings == nil {
+					webApp.AppSettings = make(map[string]string)
+				}
+				for _, pair := range *siteConfigAppSetting {
+					if pair.Name == nil || pair.Value == nil {
+						continue
+					}
+					if *pair.Name == "DOCKER_REGISTRY_SERVER_URL" ||
+						*pair.Name == "DOCKER_REGISTRY_SERVER_USERNAME" ||
+						*pair.Name == "DOCKER_REGISTRY_SERVER_PASSWORD" {
+						webApp.AppSettings[*pair.Name] = *pair.Value
+					}
+				}
+			}
+


jackofallops · 2023-08-10T13:49:26Z

internal/services/appservice/windows_web_app_resource.go

+			if metadata.ResourceData.HasChange("app_settings") || metadata.ResourceData.HasChange("site_config.0.health_check_eviction_time_in_min") || metadata.ResourceData.HasChanges("site_config.0.application_stack") {
+				if siteConfigAppSetting := existing.SiteConfig.AppSettings; siteConfigAppSetting != nil && len(*siteConfigAppSetting) > 0 {
+					if state.AppSettings == nil {
+						state.AppSettings = make(map[string]string)
+					}
+					for _, pair := range *siteConfigAppSetting {
+						if pair.Name == nil || pair.Value == nil {
+							continue
+						}
+						if *pair.Name == "DOCKER_REGISTRY_SERVER_URL" ||
+							*pair.Name == "DOCKER_REGISTRY_SERVER_USERNAME" ||
+							*pair.Name == "DOCKER_REGISTRY_SERVER_PASSWORD" {
+							state.AppSettings[*pair.Name] = *pair.Value
+						}
+					}
+				}


jackofallops · 2023-08-10T13:49:36Z

internal/services/appservice/windows_web_app_slot_resource.go

+			if siteConfigAppSetting := siteConfig.AppSettings; siteConfigAppSetting != nil && len(*siteConfigAppSetting) > 0 {
+				if webAppSlot.AppSettings == nil {
+					webAppSlot.AppSettings = make(map[string]string)
+				}
+				for _, pair := range *siteConfigAppSetting {
+					if pair.Name == nil || pair.Value == nil {
+						continue
+					}
+					if *pair.Name == "DOCKER_REGISTRY_SERVER_URL" ||
+						*pair.Name == "DOCKER_REGISTRY_SERVER_USERNAME" ||
+						*pair.Name == "DOCKER_REGISTRY_SERVER_PASSWORD" {
+						webAppSlot.AppSettings[*pair.Name] = *pair.Value
+					}
+				}
+			}
+


and again here

jackofallops · 2023-08-10T13:49:47Z

internal/services/appservice/windows_web_app_slot_resource.go

+			if metadata.ResourceData.HasChange("app_settings") || metadata.ResourceData.HasChange("site_config.0.health_check_eviction_time_in_min") || metadata.ResourceData.HasChange("site_config.0.application_stack") {
+				if siteConfigAppSetting := existing.SiteConfig.AppSettings; siteConfigAppSetting != nil && len(*siteConfigAppSetting) > 0 {
+					if state.AppSettings == nil {
+						state.AppSettings = make(map[string]string)
+					}
+					for _, pair := range *siteConfigAppSetting {
+						if pair.Name == nil || pair.Value == nil {
+							continue
+						}
+						if *pair.Name == "DOCKER_REGISTRY_SERVER_URL" ||
+							*pair.Name == "DOCKER_REGISTRY_SERVER_USERNAME" ||
+							*pair.Name == "DOCKER_REGISTRY_SERVER_PASSWORD" {
+							state.AppSettings[*pair.Name] = *pair.Value
+						}
+					}
+				}


xiaxyi · 2023-08-21T07:24:34Z

updated the comment in thread~

james-mwakichako · 2023-08-30T16:55:38Z

Are we in a position to merge this PR @jackofallops ? We are waiting for this fix

oletolshaveics · 2023-09-04T15:10:44Z

Will it be possible to implement the changes suggested by jackofallops and get this merged? Docker based app services are pretty much broken for us at the moment due to this (after we upgraded to azurerm_linux_web_app from azure_app_service).

jgmchan · 2023-09-12T00:48:37Z

We're in the same position as well, this bug is causing production issues as it keeps removing the DOCKER_* environment variables and this means it won't pull down the latest docker containers.

oletolshaveics · 2023-09-12T04:52:24Z

My current work around look like this:

resource "azurerm_linux_web_app" "core_as" {
  name                						= // ...
  // ...

  site_config {
    // ...
	
	application_stack {
		docker_image_name                               = "(redacted)"		
		# I would like to enable these but currently do not
#              docker_registry_url                                  = "https://ghcr.io"
#		docker_registry_username                       = "(redacted)"
#		docker_registry_password                       = data.azurerm_key_vault_secret.ghdr.value
	}
  }

  app_settings = {
	// ...

    # Remove these when the docker settings no longer need to be in app_settings (https://github.com/hashicorp/terraform-provider-azurerm/pull/22484)
    "DOCKER_REGISTRY_SERVER_URL" 			= "https://ghcr.io"
    "DOCKER_REGISTRY_SERVER_USERNAME" 		= "(redacted)"
    "DOCKER_REGISTRY_SERVER_PASSWORD"		= data.azurerm_key_vault_secret.ghdr.value,
  }
}

The 3 app settings are detected as changes on every plan+apply, which is very annoying, but it does work. When I need to initialize an environment from scratch, I have to run Terraform 3(!?) times, before the app settings stabilize. Quite inconvenient, but at least it works.

jackofallops · 2023-09-18T14:20:12Z

Hi @xiaxyi I couldn't push changes to your fork to resolve this, so I've opened another PR to get the fixes in. I'm going to close this in favour of #23303

github-actions · 2024-05-14T02:07:55Z

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.

add docker setting in app setting block

2100167

github-actions bot added service/app-service size/L labels Jul 12, 2023

xiaxyi mentioned this pull request Jul 17, 2023

Web app service resource keeps detecting but not applying changes to 'docker_registry_url' attribute #22548

Closed

1 task

katbyte assigned jackofallops Jul 18, 2023

SebastianBalle mentioned this pull request Jul 31, 2023

DOCKER_REGISTRY_SERVER_URL re-applied in every run #22379

Closed

1 task

jackofallops requested changes Aug 10, 2023

View reviewed changes

github-actions bot added the waiting-response label Aug 10, 2023

xiaxyi mentioned this pull request Aug 18, 2023

Docker credentials definition on azurerm_linux_web_app #22996

Closed

1 task

github-actions bot removed the waiting-response label Aug 21, 2023

rcskosir linked an issue Aug 22, 2023 that may be closed by this pull request

Docker credentials definition on azurerm_linux_web_app #22996

Closed

1 task

jackofallops mentioned this pull request Sep 18, 2023

azurerm_[linux|windows]_web_app[_slot] fix docker app_stack update #23303

Merged

jackofallops closed this Sep 18, 2023

xiaxyi mentioned this pull request Dec 1, 2023

azurerm_linux_web_app, azurerm_linux_web_app_slot - fix docker setting not being saved to application setting issue #24086

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_windows[linux]_web_app_[slot]` - add docker setting to `app_setting` block #22484

`azurerm_windows[linux]_web_app_[slot]` - add docker setting to `app_setting` block #22484

xiaxyi commented Jul 12, 2023 •

edited

Loading

jackofallops left a comment

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

jackofallops Aug 10, 2023

xiaxyi commented Aug 21, 2023

james-mwakichako commented Aug 30, 2023

oletolshaveics commented Sep 4, 2023

jgmchan commented Sep 12, 2023

oletolshaveics commented Sep 12, 2023

jackofallops commented Sep 18, 2023

github-actions bot commented May 14, 2024

azurerm_windows[linux]_web_app_[slot] - add docker setting to app_setting block #22484

azurerm_windows[linux]_web_app_[slot] - add docker setting to app_setting block #22484

Conversation

xiaxyi commented Jul 12, 2023 • edited Loading

jackofallops left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiaxyi commented Aug 21, 2023

james-mwakichako commented Aug 30, 2023

oletolshaveics commented Sep 4, 2023

jgmchan commented Sep 12, 2023

oletolshaveics commented Sep 12, 2023

jackofallops commented Sep 18, 2023

github-actions bot commented May 14, 2024

`azurerm_windows[linux]_web_app_[slot]` - add docker setting to `app_setting` block #22484

`azurerm_windows[linux]_web_app_[slot]` - add docker setting to `app_setting` block #22484

xiaxyi commented Jul 12, 2023 •

edited

Loading