Remove ACR WebHook in order to fix slot behavior

* Remove ACR webhook code
* Refactor app service module to not require a container to be specified
  for an app service
* Refactor app service module to not care about changes to specified
  Docker contianer upon subsequent terraform apply commands
* Refactor isolated template to leave the docker container unspecified
* Update tests to reflect new behavior

infra/modules/providers/azure/app-service/main.tf

@@ -6,6 +6,16 @@ locals {
   acr_webhook_name               = "cdhook"
   app_names                      = keys(var.app_service_config)
   app_configs                    = values(var.app_service_config)
+
+  app_linux_fx_versions = [
+    for config in values(var.app_service_config) :
+    // Without specifyin a `linux_fx_version` the webapp created by the `azurerm_app_service` resource
+    // will be a non-container webapp.
+    //
+    // The value of "DOCKER" is a stand-in value that can be used to force the webapp created to be
+    // container compatible without explicitly specifying the image that the app should run.
+    config.image == "" ? "DOCKER" : format("DOCKER|%s/%s", var.docker_registry_server_url, config.image)
+  ]
 }
 
 data "azurerm_resource_group" "appsvc" {
@@ -32,40 +42,25 @@ resource "azurerm_app_service" "appsvc" {
     DOCKER_REGISTRY_SERVER_PASSWORD     = var.docker_registry_server_password
     APPINSIGHTS_INSTRUMENTATIONKEY      = var.app_insights_instrumentation_key
     KEYVAULT_URI                        = var.vault_uri
-    DOCKER_ENABLE_CI                    = var.docker_enable_ci
   }
 
   site_config {
-    linux_fx_version     = format("DOCKER|%s/%s", var.docker_registry_server_url, local.app_configs[count.index].image)
+    linux_fx_version     = local.app_linux_fx_versions[count.index]
     always_on            = var.site_config_always_on
     virtual_network_name = var.vnet_name
   }
 
   identity {
     type = "SystemAssigned"
   }
-}
-
-resource "null_resource" "acr_webhook_creation" {
-  count      = var.docker_enable_ci == true && var.uses_acr ? length(local.app_names) : 0
-  depends_on = [azurerm_app_service.appsvc]
-
-  triggers = {
-    images_to_deploy = "${join(",", [for config in local.app_configs : config.image])}"
-    uses_acr         = var.uses_acr
-  }
-
-  provisioner "local-exec" {
-    command = "az acr webhook create --registry \"$ACRNAME\" --name \"$APPNAME$WEBHOOKNAME\" --actions push --uri $(az webapp deployment container show-cd-url -n $APPNAME_URL -g $APPSVCNAME --query CI_CD_URL -o tsv)"
-
-    environment = {
-      ACRNAME     = var.azure_container_registry_name
-      APPNAME     = replace(lower("${var.app_service_name_prefix}${local.app_names[count.index]}"), "-", "")
-      APPNAME_URL = "${var.app_service_name_prefix}-${local.app_names[count.index]}"
-      WEBHOOKNAME = local.acr_webhook_name
-      APPSVCNAME  = data.azurerm_resource_group.appsvc.name
-    }
 
+  lifecycle {
+    # This stanza will prevent terraform from reverting changes to the application container settings.
+    # These settings are how application teams deploy new containers to the app service and should not
+    # be overridden by Terraform deployments.
+    ignore_changes = [
+      "site_config[0].linux_fx_version"
+    ]
   }
 }
 
@@ -77,6 +72,34 @@ resource "azurerm_app_service_slot" "appsvc_staging_slot" {
   resource_group_name = data.azurerm_resource_group.appsvc.name
   app_service_plan_id = data.azurerm_app_service_plan.appsvc.id
   depends_on          = [azurerm_app_service.appsvc]
+
+  app_settings = {
+    DOCKER_REGISTRY_SERVER_URL          = format("https://%s", var.docker_registry_server_url)
+    WEBSITES_ENABLE_APP_SERVICE_STORAGE = false
+    DOCKER_REGISTRY_SERVER_USERNAME     = var.docker_registry_server_username
+    DOCKER_REGISTRY_SERVER_PASSWORD     = var.docker_registry_server_password
+    APPINSIGHTS_INSTRUMENTATIONKEY      = var.app_insights_instrumentation_key
+    KEYVAULT_URI                        = var.vault_uri
+  }
+
+  site_config {
+    linux_fx_version     = local.app_linux_fx_versions[count.index]
+    always_on            = var.site_config_always_on
+    virtual_network_name = var.vnet_name
+  }
+
+  identity {
+    type = "SystemAssigned"
+  }
+
+  lifecycle {
+    # This stanza will prevent terraform from reverting changes to the application container settings.
+    # These settings are how application teams deploy new containers to the app service and should not
+    # be overridden by Terraform deployments.
+    ignore_changes = [
+      "site_config[0].linux_fx_version"
+    ]
+  }
 }
 
 data "azurerm_app_service" "all" {

infra/modules/providers/azure/app-service/variables.tf

@@ -34,6 +34,7 @@ variable "resource_tags" {
 variable "app_service_config" {
   description = "Metadata about the app services to be created."
   type = map(object({
+    // If "", no container configuration will be set. Otherwise, this will be used to set the container configuration for the app service.
     image = string
   }))
   default = {}
@@ -58,7 +59,7 @@ variable "app_insights_instrumentation_key" {
 }
 
 variable "site_config_always_on" {
-  description = "Should the app be loaded at all times? Defaults to false."
+  description = "Should the app be loaded at all times? Defaults to true."
   type        = string
   default     = true
 }
@@ -98,9 +99,3 @@ variable "docker_registry_server_password" {
   type        = string
   default     = ""
 }
-
-variable "docker_enable_ci" {
-  description = "Enable's continuous deployment for the app service image."
-  type        = bool
-  default     = true
-}

infra/modules/providers/azure/provider/main.tf

@@ -1,5 +1,5 @@
 provider "azurerm" {
-  version = "~>1.32.0"
+  version = "~>1.34.0"
 }
 
 provider "null" {

infra/templates/az-isolated-service-single-region/ase.tf

@@ -65,7 +65,7 @@ module "app_service" {
   app_service_config = {
     for target in var.unauthn_deployment_targets :
     target.app_name => {
-      image = "${target.image_name}:${target.image_release_tag_prefix}"
+      image = ""
     }
   }
   providers = {
@@ -98,7 +98,7 @@ module "authn_app_service" {
   app_service_config = {
     for target in var.authn_deployment_targets :
     target.app_name => {
-      image = "${target.image_name}:${target.image_release_tag_prefix}"
+      image = ""
     }
   }
   providers = {

infra/templates/az-isolated-service-single-region/tests/unit/unit_test.go

@@ -59,7 +59,6 @@ func asMap(t *testing.T, jsonString string) map[string]interface{} {
 }
 
 func TestTemplate(t *testing.T) {
-	expectedStagingSlot := asMap(t, `{"name":"staging"}`)
 	expectedAppDevResourceGroup := asMap(t, `{
 		"location": "`+region+`"
 	}`)
@@ -191,19 +190,24 @@ func TestTemplate(t *testing.T) {
 		}]
 	}`)
 	expectedAppServiceSchema := `{
-		"identity": [{ "type": "SystemAssigned" }],
+		"identity":    [{ "type": "SystemAssigned" }],
+		"enabled":     true,
 		"site_config": [{
-			"always_on": true
+			"always_on":         true,
+			"linux_fx_version": "DOCKER"
 		}]
 	}`
-	expectedAppServiceSchema2 := `{
-		"identity": [{ "type": "SystemAssigned" }],
+	expectedAppService := asMap(t, expectedAppServiceSchema)
+
+	expectedAppServiceSlot := asMap(t, `{
+		"name":        "staging",
+		"identity":    [{ "type": "SystemAssigned" }],
+		"enabled":     true,
 		"site_config": [{
-			"always_on": true
+			"always_on":         true,
+			"linux_fx_version": "DOCKER"
 		}]
-	}`
-	expectedAppService1 := asMap(t, expectedAppServiceSchema)
-	expectedAppService2 := asMap(t, expectedAppServiceSchema2)
+	}`)
 
 	expectedAppServiceKVPolicies := asMap(t, `{
 		"certificate_permissions": ["get", "list"],
@@ -222,26 +226,22 @@ func TestTemplate(t *testing.T) {
 		TfOptions:             tfOptions,
 		Workspace:             workspace,
 		PlanAssertions:        nil,
-		ExpectedResourceCount: 62,
+		ExpectedResourceCount: 60,
 		ExpectedResourceAttributeValues: infratests.ResourceDescription{
 			// "module.keyvault.azurerm_key_vault.keyvault":                                   expectedKeyVault,
 			// "module.container_registry.azurerm_container_registry.container_registry":      expectedAzureContainerRegistry,
 			"azurerm_resource_group.app_rg":                                                                                            expectedAppDevResourceGroup,
 			"azurerm_resource_group.admin_rg":                                                                                          expectedAdminResourceGroup,
 			"module.service_plan.azurerm_app_service_plan.svcplan":                                                                     expectedAppServicePlan,
 			"module.app_insights.azurerm_application_insights.appinsights":                                                             expectedAppInsights,
-			"module.app_service.azurerm_app_service.appsvc[0]":                                                                         expectedAppService1,
-			"module.authn_app_service.azurerm_app_service.appsvc[0]":                                                                   expectedAppService2,
-			"module.app_service.azurerm_app_service_slot.appsvc_staging_slot[0]":                                                       expectedStagingSlot,
-			"module.authn_app_service.azurerm_app_service_slot.appsvc_staging_slot[0]":                                                 expectedStagingSlot,
+			"module.app_service.azurerm_app_service.appsvc[0]":                                                                         expectedAppService,
+			"module.authn_app_service.azurerm_app_service.appsvc[0]":                                                                   expectedAppService,
+			"module.app_service.azurerm_app_service_slot.appsvc_staging_slot[0]":                                                       expectedAppServiceSlot,
+			"module.authn_app_service.azurerm_app_service_slot.appsvc_staging_slot[0]":                                                 expectedAppServiceSlot,
 			"module.service_plan.azurerm_monitor_autoscale_setting.app_service_auto_scale":                                             expectedAutoScalePlan,
 			"module.app_service_keyvault_access_policy.azurerm_key_vault_access_policy.keyvault[0]":                                    expectedAppServiceKVPolicies,
 			"module.authn_app_service_keyvault_access_policy.azurerm_key_vault_access_policy.keyvault[0]":                              expectedAppServiceKVPolicies,
 			"module.keyvault.module.deployment_service_principal_keyvault_access_policies.azurerm_key_vault_access_policy.keyvault[0]": expectedDeploymentServicePrincipalKVPolicies,
-
-			// These are basically existence checks. Nothing interesting to inspect for the resources
-			"module.app_service.null_resource.acr_webhook_creation[0]":       nil,
-			"module.authn_app_service.null_resource.acr_webhook_creation[0]": nil,
 		},
 	}
 

infra/templates/az-isolated-service-single-region/variables.tf

@@ -48,18 +48,14 @@ variable "ase_vnet_name" {
 variable "unauthn_deployment_targets" {
   description = "Metadata about apps to deploy, such as repository location, docker file metadata and image names"
   type = list(object({
-    app_name                 = string
-    image_name               = string
-    image_release_tag_prefix = string
+    app_name = string
   }))
 }
 
 variable "authn_deployment_targets" {
   description = "Metadata about apps to deploy that also require authentication."
   type = list(object({
-    app_name                 = string
-    image_name               = string
-    image_release_tag_prefix = string
+    app_name = string
   }))
 }
 

infra/templates/az-service-single-region/tests/unit/template_unit_test.go

@@ -10,7 +10,6 @@ import (
 )
 
 var region = "eastus"
-var workspace = "azsingleregion"
 
 var tfOptions = &terraform.Options{
 	TerraformDir: "../../",
@@ -35,6 +34,8 @@ func asMap(t *testing.T, jsonString string) map[string]interface{} {
 
 func TestTemplate(t *testing.T) {
 	expectedAppServicePlan := asMap(t, `{
+		"enabled": true,
+		"site_config": [{ "always_on": true }]
 	}`)
 
 	expectedAppGatewayPlan := asMap(t, `{
@@ -100,17 +101,20 @@ func TestTemplate(t *testing.T) {
 	testFixture := infratests.UnitTestFixture{
 		GoTest:                t,
 		TfOptions:             tfOptions,
-		Workspace:             workspace,
 		PlanAssertions:        nil,
-		ExpectedResourceCount: 37,
+		ExpectedResourceCount: 36,
 		ExpectedResourceAttributeValues: infratests.ResourceDescription{
 			"azurerm_resource_group.svcplan": map[string]interface{}{
 				"location": region,
 			},
 			"module.app_gateway.data.azurerm_resource_group.appgateway":  map[string]interface{}{},
 			"module.app_insight.data.azurerm_resource_group.appinsights": map[string]interface{}{},
 			"module.app_service.azurerm_app_service_slot.appsvc_staging_slot[0]": map[string]interface{}{
-				"name": "staging",
+				"name":    "staging",
+				"enabled": true,
+				"site_config": []interface{}{
+					map[string]interface{}{"always_on": true},
+				},
 			},
 			"module.app_service.azurerm_app_service.appsvc[0]":                                                                         expectedAppServicePlan,
 			"module.app_gateway.azurerm_application_gateway.appgateway":                                                                expectedAppGatewayPlan,