From 8de30b88f38688bdbd721838f5e1470b747a08c9 Mon Sep 17 00:00:00 2001
From: Payam Hekmat <980614+phekmat@users.noreply.github.com>
Date: Tue, 3 Sep 2019 19:09:03 -0500
Subject: [PATCH] Fix app service logging with app settings

- Update the log settings after updating the app settings and whenever
the app settings change if there's a `logs` block. Azure adds app settings
for the logging configuration, so changing the app settings would otherwise
overwrite them
- Update test case for above
---
 azurerm/resource_arm_app_service.go      | 33 +++++++++-------
 azurerm/resource_arm_app_service_test.go | 48 ++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/azurerm/resource_arm_app_service.go b/azurerm/resource_arm_app_service.go
index ce1c63edd56f..75ea3e708722 100644
--- a/azurerm/resource_arm_app_service.go
+++ b/azurerm/resource_arm_app_service.go
@@ -378,19 +378,6 @@ func resourceArmAppServiceUpdate(d *schema.ResourceData, meta interface{}) error
 		}
 	}
 
-	if d.HasChange("logs") {
-		logs := azure.ExpandAppServiceLogs(d.Get("logs"))
-		id := d.Id()
-		logsResource := web.SiteLogsConfig{
-			ID:                       &id,
-			SiteLogsConfigProperties: &logs,
-		}
-
-		if _, err := client.UpdateDiagnosticLogsConfig(ctx, resGroup, name, logsResource); err != nil {
-			return fmt.Errorf("Error updating Diagnostics Logs for App Service %q: %+v", name, err)
-		}
-	}
-
 	if d.HasChange("backup") {
 		backupRaw := d.Get("backup").([]interface{})
 		if backup := azure.ExpandAppServiceBackup(backupRaw); backup != nil {
@@ -422,6 +409,7 @@ func resourceArmAppServiceUpdate(d *schema.ResourceData, meta interface{}) error
 		}
 	}
 
+	// app settings updates have a side effect on logging settings. See the note below
 	if d.HasChange("app_settings") {
 		// update the AppSettings
 		appSettings := expandAppServiceAppSettings(d)
@@ -434,6 +422,25 @@ func resourceArmAppServiceUpdate(d *schema.ResourceData, meta interface{}) error
 		}
 	}
 
+	// the logging configuration has a dependency on the app settings in Azure
+	// e.g. configuring logging to blob storage will add the DIAGNOSTICS_AZUREBLOBCONTAINERSASURL
+	// and DIAGNOSTICS_AZUREBLOBRETENTIONINDAYS app settings to the app service.
+	// If the app settings are updated, also update the logging configuration if it exists, otherwise
+	// updating the former will clobber the log settings
+	_, hasLogs := d.GetOkExists("logs")
+	if d.HasChange("logs") || (hasLogs && d.HasChange("app_settings")) {
+		logs := azure.ExpandAppServiceLogs(d.Get("logs"))
+		id := d.Id()
+		logsResource := web.SiteLogsConfig{
+			ID:                       &id,
+			SiteLogsConfigProperties: &logs,
+		}
+
+		if _, err := client.UpdateDiagnosticLogsConfig(ctx, resGroup, name, logsResource); err != nil {
+			return fmt.Errorf("Error updating Diagnostics Logs for App Service %q: %+v", name, err)
+		}
+	}
+
 	if d.HasChange("storage_account") {
 		storageAccounts := azure.ExpandAppServiceStorageAccounts(d)
 		properties := web.AzureStoragePropertyDictionaryResource{
diff --git a/azurerm/resource_arm_app_service_test.go b/azurerm/resource_arm_app_service_test.go
index 55caca416f16..4a27fc747389 100644
--- a/azurerm/resource_arm_app_service_test.go
+++ b/azurerm/resource_arm_app_service_test.go
@@ -957,6 +957,7 @@ func TestAccAzureRMAppService_applicationBlobStorageLogs(t *testing.T) {
 	resourceName := "azurerm_app_service.test"
 	ri := tf.AccRandTimeInt()
 	config := testAccAzureRMAppService_applicationBlobStorageLogs(ri, testLocation())
+	updated := testAccAzureRMAppService_applicationBlobStorageLogsWithAppSettings(ri, testLocation())
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -972,6 +973,16 @@ func TestAccAzureRMAppService_applicationBlobStorageLogs(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "logs.0.application_logs.0.azure_blob_storage.0.retention_in_days", "3"),
 				),
 			},
+			{
+				Config: updated,
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMAppServiceExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "logs.0.application_logs.0.azure_blob_storage.0.level", "Information"),
+					resource.TestCheckResourceAttr(resourceName, "logs.0.application_logs.0.azure_blob_storage.0.sas_url", "http://x.com/"),
+					resource.TestCheckResourceAttr(resourceName, "logs.0.application_logs.0.azure_blob_storage.0.retention_in_days", "3"),
+					resource.TestCheckResourceAttr(resourceName, "app_settings.foo", "bar"),
+				),
+			},
 			{
 				ResourceName:      resourceName,
 				ImportState:       true,
@@ -3307,7 +3318,44 @@ resource "azurerm_app_service" "test" {
 }
 `, rInt, location, rInt, rInt)
 }
+func testAccAzureRMAppService_applicationBlobStorageLogsWithAppSettings(rInt int, location string) string {
+	return fmt.Sprintf(`
+resource "azurerm_resource_group" "test" {
+  name     = "acctestRG-%d"
+  location = "%s"
+}
+
+resource "azurerm_app_service_plan" "test" {
+  name                = "acctestASP-%d"
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+
+  sku {
+    tier = "Standard"
+    size = "S1"
+  }
+}
 
+resource "azurerm_app_service" "test" {
+  name                = "acctestAS-%d"
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+  app_service_plan_id = "${azurerm_app_service_plan.test.id}"
+  app_settings = {
+	  foo = "bar"
+  }
+  logs {
+    application_logs {
+      azure_blob_storage {
+        level             = "Information"
+        sas_url           = "http://x.com/"
+        retention_in_days = 3
+      }
+    }
+  }
+}
+`, rInt, location, rInt, rInt)
+}
 func testAccAzureRMAppService_httpFileSystemLogs(rInt int, location string) string {
 	return fmt.Sprintf(`
 resource "azurerm_resource_group" "test" {