containerapps - fix breaking schema behavioural change (#28639)

* fixup breaking behavioural change * fixup CustomizeDiff for computed value * add 5.0 feature flagging * update locations for testing as northcentralus does not support availability zones * remove unnecessary schema description entry * Apply suggestions from code review Co-authored-by: stephybun <[email protected]> * Update 5.0-upgrade-guide * expand deprecation description to cover default behaviour --------- Co-authored-by: stephybun <[email protected]>
hashicorp · Jan 30, 2025 · 083f474 · 083f474
1 parent 23433b6
commit 083f474
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 28 deletions.
diff --git a/.teamcity/components/settings.kt b/.teamcity/components/settings.kt
@@ -69,7 +69,7 @@ var serviceTestConfigurationOverrides = mapOf(
 
         // Container App Managed Environments are limited to 20 per location, using 10 as they can take some time to clear
         // Enable rotation test to mitigate resource burden in a single region
-        "containerapps" to testConfiguration(parallelism = 10, locationOverride = LocationConfiguration("eastus2","westus2","northcentralus", true)),
+        "containerapps" to testConfiguration(parallelism = 10, locationOverride = LocationConfiguration("eastus2","westus2","southcentralus", true)),
 
         // The AKS API has a low rate limit
         "containers" to testConfiguration(parallelism = 5, locationOverride = LocationConfiguration("eastus","westeurope","eastus2", false), timeout = 18),

diff --git a/internal/services/containerapps/container_app_environment_resource.go b/internal/services/containerapps/container_app_environment_resource.go
@@ -18,6 +18,7 @@ import (
 	"github.com/hashicorp/go-azure-helpers/resourcemanager/tags"
 	"github.com/hashicorp/go-azure-sdk/resource-manager/containerapps/2024-03-01/managedenvironments"
 	"github.com/hashicorp/go-azure-sdk/resource-manager/operationalinsights/2020-08-01/workspaces"
+	"github.com/hashicorp/terraform-provider-azurerm/internal/features"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/services/containerapps/helpers"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/services/containerapps/validate"
@@ -28,7 +29,7 @@ import (
 const (
 	LogsDestinationLogAnalytics string = "log-analytics"
 	LogsDestinationAzureMonitor string = "azure-monitor"
-	LogsDestinationAzureNone    string = ""
+	LogsDestinationNone         string = ""
 )
 
 type ContainerAppEnvironmentResource struct{}
@@ -74,7 +75,7 @@ func (r ContainerAppEnvironmentResource) IDValidationFunc() pluginsdk.SchemaVali
 }
 
 func (r ContainerAppEnvironmentResource) Arguments() map[string]*pluginsdk.Schema {
-	return map[string]*pluginsdk.Schema{
+	schema := map[string]*pluginsdk.Schema{
 		"name": {
 			Type:         pluginsdk.TypeString,
 			Required:     true,
@@ -106,12 +107,12 @@ func (r ContainerAppEnvironmentResource) Arguments() map[string]*pluginsdk.Schem
 		"logs_destination": {
 			Type:     pluginsdk.TypeString,
 			Optional: true,
-			Default:  LogsDestinationAzureNone,
+			Default:  LogsDestinationNone,
 			ValidateFunc: validation.StringInSlice([]string{
-				LogsDestinationLogAnalytics,
 				LogsDestinationAzureMonitor,
+				LogsDestinationLogAnalytics,
 			}, false),
-			Description: "The destination for the application logs. Possible values are `log-analytics` or `azure-monitor`.",
+			Description: "The destination for the application logs. Possible values include `log-analytics` and `azure-monitor`.  Omitting this value will result in logs being streamed only.",
 		},
 
 		"infrastructure_resource_group_name": {
@@ -170,6 +171,21 @@ func (r ContainerAppEnvironmentResource) Arguments() map[string]*pluginsdk.Schem
 
 		"tags": commonschema.Tags(),
 	}
+
+	if !features.FivePointOh() {
+		schema["logs_destination"] = &pluginsdk.Schema{
+			Type:     pluginsdk.TypeString,
+			Optional: true,
+			Computed: true, // NOTE: O+C as the introduction of this property is a behavioural change where we previously set it behind the scenes if `log_analytics_workspace_id` was set.
+			ValidateFunc: validation.StringInSlice([]string{
+				LogsDestinationAzureMonitor,
+				LogsDestinationNone,
+				LogsDestinationLogAnalytics,
+			}, false),
+		}
+	}
+
+	return schema
 }
 
 func (r ContainerAppEnvironmentResource) Attributes() map[string]*pluginsdk.Schema {
@@ -275,6 +291,8 @@ func (r ContainerAppEnvironmentResource) Create() sdk.ResourceFunc {
 					return fmt.Errorf("retrieving access keys to Log Analytics Workspace for %s: %+v", id, err)
 				}
 
+				managedEnvironment.Properties.AppLogsConfiguration.Destination = pointer.To(LogsDestinationLogAnalytics)
+
 				managedEnvironment.Properties.AppLogsConfiguration.LogAnalyticsConfiguration = &managedenvironments.LogAnalyticsConfiguration{
 					CustomerId: customerId,
 					SharedKey:  sharedKey,
@@ -424,24 +442,31 @@ func (r ContainerAppEnvironmentResource) Update() sdk.ResourceFunc {
 			}
 
 			if metadata.ResourceData.HasChanges("logs_destination", "log_analytics_workspace_id") {
+				// For 4.x we need to be compensate for the legacy behaviour of setting log destination based on the presence of log_analytics_workspace_id
+				if !features.FivePointOh() && metadata.ResourceData.GetRawConfig().AsValueMap()["logs_destination"].IsNull() && state.LogAnalyticsWorkspaceId == "" {
+					state.LogsDestination = LogsDestinationNone
+				}
+
 				switch state.LogsDestination {
 				case LogsDestinationAzureMonitor:
 					existing.Model.Properties.AppLogsConfiguration = &managedenvironments.AppLogsConfiguration{
 						Destination:               pointer.To(LogsDestinationAzureMonitor),
 						LogAnalyticsConfiguration: nil,
 					}
 				case LogsDestinationLogAnalytics:
-					customerId, sharedKey, err := getSharedKeyForWorkspace(ctx, metadata, state.LogAnalyticsWorkspaceId)
-					if err != nil {
-						return fmt.Errorf("retrieving access keys to Log Analytics Workspace for %s: %+v", id, err)
-					}
+					if state.LogAnalyticsWorkspaceId != "" {
+						customerId, sharedKey, err := getSharedKeyForWorkspace(ctx, metadata, state.LogAnalyticsWorkspaceId)
+						if err != nil {
+							return fmt.Errorf("retrieving access keys to Log Analytics Workspace for %s: %+v", id, err)
+						}
 
-					existing.Model.Properties.AppLogsConfiguration = &managedenvironments.AppLogsConfiguration{
-						Destination: pointer.To(LogsDestinationLogAnalytics),
-						LogAnalyticsConfiguration: &managedenvironments.LogAnalyticsConfiguration{
-							CustomerId: customerId,
-							SharedKey:  sharedKey,
-						},
+						existing.Model.Properties.AppLogsConfiguration = &managedenvironments.AppLogsConfiguration{
+							Destination: pointer.To(LogsDestinationLogAnalytics),
+							LogAnalyticsConfiguration: &managedenvironments.LogAnalyticsConfiguration{
+								CustomerId: customerId,
+								SharedKey:  sharedKey,
+							},
+						}
 					}
 
 				default:
@@ -498,20 +523,42 @@ func (r ContainerAppEnvironmentResource) CustomizeDiff() sdk.ResourceFunc {
 				}
 			}
 
-			if metadata.ResourceDiff.HasChanges("logs_destination", "log_analytics_workspace_id") {
-				logsDestination := metadata.ResourceDiff.Get("logs_destination").(string)
-				logAnalyticsWorkspaceID := metadata.ResourceDiff.Get("log_analytics_workspace_id").(string)
-				logAnalyticsWorkspaceIDIsNull := metadata.ResourceDiff.GetRawConfig().AsValueMap()["log_analytics_workspace_id"].IsNull()
+			if !features.FivePointOh() { // in 4.x `logs_destination` is Computed due to legacy code implying destination from presence of a valid id in `log_analytics_workspace_id` so we need to check explicit config values here
+				if metadata.ResourceDiff.HasChanges("logs_destination", "log_analytics_workspace_id") {
+					logsDestination := metadata.ResourceDiff.Get("logs_destination").(string)
+					logDestinationIsNull := metadata.ResourceDiff.GetRawConfig().AsValueMap()["logs_destination"].IsNull()
+					logAnalyticsWorkspaceID := metadata.ResourceDiff.Get("log_analytics_workspace_id").(string)
+					logAnalyticsWorkspaceIDIsNull := metadata.ResourceDiff.GetRawConfig().AsValueMap()["log_analytics_workspace_id"].IsNull()
+
+					if !logDestinationIsNull || !logAnalyticsWorkspaceIDIsNull {
+						switch logsDestination {
+						case LogsDestinationLogAnalytics:
+							if logAnalyticsWorkspaceIDIsNull {
+								return fmt.Errorf("`log_analytics_workspace_id` must be set when `logs_destination` is set to `log-analytics`")
+							}
 
-				switch logsDestination {
-				case LogsDestinationLogAnalytics:
-					if logAnalyticsWorkspaceIDIsNull {
-						return fmt.Errorf("`log_analytics_workspace_id` must be set when `logs_destination` is set to `log-analytics`")
+						case LogsDestinationAzureMonitor, LogsDestinationNone:
+							if (logAnalyticsWorkspaceID != "" || !logAnalyticsWorkspaceIDIsNull) && !logDestinationIsNull {
+								return fmt.Errorf("`log_analytics_workspace_id` can only be set when `logs_destination` is set to `log-analytics` or omitted")
+							}
+						}
 					}
+				}
+			} else {
+				if metadata.ResourceDiff.HasChanges("logs_destination", "log_analytics_workspace_id") {
+					logsDestination := metadata.ResourceDiff.Get("logs_destination").(string)
+					logAnalyticsWorkspaceID := metadata.ResourceDiff.Get("log_analytics_workspace_id").(string)
+
+					switch logsDestination {
+					case LogsDestinationLogAnalytics:
+						if logAnalyticsWorkspaceID == "" {
+							return fmt.Errorf("`log_analytics_workspace_id` must be set when `logs_destination` is set to `log-analytics`")
+						}
 
-				case LogsDestinationAzureMonitor, LogsDestinationAzureNone:
-					if logAnalyticsWorkspaceID != "" || !logAnalyticsWorkspaceIDIsNull {
-						return fmt.Errorf("`log_analytics_workspace_id` can only be set when `logs_destination` is set to `log-analytics`")
+					case LogsDestinationAzureMonitor, LogsDestinationNone:
+						if logAnalyticsWorkspaceID != "" {
+							return fmt.Errorf("`log_analytics_workspace_id` can only be set when `logs_destination` is set to `log-analytics` or omitted")
+						}
 					}
 				}
 			}

diff --git a/website/docs/5.0-upgrade-guide.html.markdown b/website/docs/5.0-upgrade-guide.html.markdown
@@ -97,6 +97,10 @@ Please follow the format in the example below for listing breaking changes in re
 
 * The `minimal_tls_version` property no longer accepts `Tls` or `Tls11` as a value.
 
+### `azurerm_container_app_environment`
+
+* The `logs_destination` property is no longer Computed and now must be set to `log-analytics` to be able to specify a value for `log_analytics_workspace_id`. It will now default to empty, meaning Streaming Only in the Azure Portal.
+
 ### `azurerm_eventhub`
 
 * The deprecated `namespace_name` property has been removed in favour of the `namespace_id` property.

diff --git a/website/docs/r/container_app_environment.html.markdown b/website/docs/r/container_app_environment.html.markdown
@@ -69,7 +69,7 @@ The following arguments are supported:
 
 ~> **Note:** required if `logs_destination` is set to `log-analytics`. Cannot be set if `logs_destination` is set to `azure-monitor`.
 
-* `logs_destination` - (Optional) Where the application logs will be saved for this Container Apps Managed Environment. Options are `log-analytics` or `azure-monitor`.
+* `logs_destination` - (Optional) Where the application logs will be saved for this Container Apps Managed Environment. Possible values include `log-analytics` and `azure-monitor`. Omitting this value will result in logs being streamed only.
 
 * `workload_profile` - (Optional) The profile of the workload to scope the container app execution. A `workload_profile` block as defined below.