Bug: azurerm_frontdoor fix for caching issue

azurerm/internal/services/frontdoor/helper.go

@@ -2,9 +2,11 @@ package frontdoor
 
 import (
 	"fmt"
+	"net/url"
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/services/frontdoor/mgmt/2019-04-01/frontdoor"
+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
 )
 
 func VerifyBackendPoolExists(backendPoolName string, backendPools []interface{}) error {
@@ -201,3 +203,92 @@ func FlattenFrontendEndpointLinkSlice(input *[]frontdoor.FrontendEndpointLink) [
 	}
 	return result
 }
+
+// ParseAzureResourceIDLowerPath converts a long-form Azure Resource Manager ID
+// into a ResourceID. We make assumptions about the structure of URLs,
+// which is obviously not good, but the best thing available given the
+// SDK. I had to normalize the key casing of Path because the Front Door API
+// via Portal does not have consistent casing within the resource, for example:
+//
+// In the backendPools block the casing of the HealthProbeSettings is (notice the lowercase 'h'):
+// portal-front-door/ -> healthProbeSettings/healthProbeSettings-1571100669337
+//
+// but in the HealthProbeSettings block the casing of the HealthProbeSettings is (notice the uppercase 'H')::
+// portal-front-door/ -> HealthProbeSettings/healthProbeSettings-1571100669337
+//
+// so if I need to parse the name of the resource from its ID string I would be
+// unable to do so with the current implementation so I normalize the key into
+// a known format so I can reliable parse the ID string.
+//
+// Link to issue: https://github.com/Azure/azure-sdk-for-go/issues/6762
+func ParseAzureResourceIDLowerPath(id string) (*azure.ResourceID, error) {
+	idURL, err := url.ParseRequestURI(id)
+	if err != nil {
+		return nil, fmt.Errorf("Cannot parse Azure ID: %s", err)
+	}
+
+	path := idURL.Path
+
+	path = strings.TrimPrefix(path, "/")
+	path = strings.TrimSuffix(path, "/")
+
+	components := strings.Split(path, "/")
+
+	// We should have an even number of key-value pairs.
+	if len(components)%2 != 0 {
+		return nil, fmt.Errorf("The number of path segments is not divisible by 2 in %q", path)
+	}
+
+	var subscriptionID string
+
+	// Put the constituent key-value pairs into a map
+	componentMap := make(map[string]string, len(components)/2)
+	for current := 0; current < len(components); current += 2 {
+		key := strings.ToLower(components[current])
+		value := components[current+1]
+
+		// Check key/value for empty strings.
+		if key == "" || value == "" {
+			return nil, fmt.Errorf("Key/Value cannot be empty strings. Key: '%s', Value: '%s'", key, value)
+		}
+
+		// Catch the subscriptionID before it can be overwritten by another "subscriptions"
+		// value in the ID which is the case for the Service Bus subscription resource
+		if key == "subscriptions" && subscriptionID == "" {
+			subscriptionID = value
+		} else {
+			componentMap[key] = value
+		}
+	}
+
+	// Build up a ResourceID from the map
+	idObj := &azure.ResourceID{}
+	idObj.Path = componentMap
+
+	if subscriptionID != "" {
+		idObj.SubscriptionID = subscriptionID
+	} else {
+		return nil, fmt.Errorf("No subscription ID found in: %q", path)
+	}
+
+	if resourceGroup, ok := componentMap["resourceGroups"]; ok {
+		idObj.ResourceGroup = resourceGroup
+		delete(componentMap, "resourceGroups")
+	} else {
+		// Some Azure APIs are weird and provide things in lower case...
+		// However it's not clear whether the casing of other elements in the URI
+		// matter, so we explicitly look for that case here.
+		if resourceGroup, ok := componentMap["resourcegroups"]; ok {
+			idObj.ResourceGroup = resourceGroup
+			delete(componentMap, "resourcegroups")
+		}
+	}
+
+	// It is OK not to have a provider in the case of a resource group
+	if provider, ok := componentMap["providers"]; ok {
+		idObj.Provider = provider
+		delete(componentMap, "providers")
+	}
+
+	return idObj, nil
+}

azurerm/internal/services/frontdoor/resource_arm_front_door.go

@@ -3,6 +3,7 @@ package frontdoor
 import (
 	"fmt"
 	"log"
+	"strings"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/services/frontdoor/mgmt/2019-04-01/frontdoor"
@@ -172,24 +173,25 @@ func resourceArmFrontDoor() *schema.Resource {
 										Required:     true,
 										ValidateFunc: ValidateBackendPoolRoutingRuleName,
 									},
+									// Remove default value for #4461
 									"cache_use_dynamic_compression": {
 										Type:     schema.TypeBool,
 										Optional: true,
-										Default:  false,
 									},
+									// Remove default value for #4461
 									"cache_query_parameter_strip_directive": {
 										Type:     schema.TypeString,
 										Optional: true,
 										ValidateFunc: validation.StringInSlice([]string{
 											string(frontdoor.StripAll),
 											string(frontdoor.StripNone),
 										}, false),
-										Default: string(frontdoor.StripNone),
 									},
 									"custom_forwarding_path": {
 										Type:     schema.TypeString,
 										Optional: true,
 									},
+									// Added Portal Default value for #4627
 									"forwarding_protocol": {
 										Type:     schema.TypeString,
 										Optional: true,
@@ -198,7 +200,7 @@ func resourceArmFrontDoor() *schema.Resource {
 											string(frontdoor.HTTPSOnly),
 											string(frontdoor.MatchRequest),
 										}, false),
-										Default: string(frontdoor.MatchRequest),
+										Default: string(frontdoor.HTTPSOnly),
 									},
 								},
 							},
@@ -596,7 +598,7 @@ func resourceArmFrontDoorRead(d *schema.ResourceData, meta interface{}) error {
 	ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
 	defer cancel()
 
-	id, err := azure.ParseAzureResourceID(d.Id())
+	id, err := ParseAzureResourceIDLowerPath(d.Id())
 	if err != nil {
 		return err
 	}
@@ -668,7 +670,7 @@ func resourceArmFrontDoorDelete(d *schema.ResourceData, meta interface{}) error
 	ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d)
 	defer cancel()
 
-	id, err := azure.ParseAzureResourceID(d.Id())
+	id, err := ParseAzureResourceIDLowerPath(d.Id())
 	if err != nil {
 		return err
 	}
@@ -1050,32 +1052,29 @@ func expandArmFrontDoorForwardingConfiguration(input []interface{}, frontDoorPat
 
 	customForwardingPath := v["custom_forwarding_path"].(string)
 	forwardingProtocol := v["forwarding_protocol"].(string)
+	backendPoolName := v["backend_pool_name"].(string)
 	cacheUseDynamicCompression := v["cache_use_dynamic_compression"].(bool)
 	cacheQueryParameterStripDirective := v["cache_query_parameter_strip_directive"].(string)
-	backendPoolName := v["backend_pool_name"].(string)
-
-	useDynamicCompression := frontdoor.DynamicCompressionEnabledDisabled
-
-	if cacheUseDynamicCompression {
-		useDynamicCompression = frontdoor.DynamicCompressionEnabledEnabled
-	}
-
-	cacheConfiguration := &frontdoor.CacheConfiguration{
-		QueryParameterStripDirective: frontdoor.Query(cacheQueryParameterStripDirective),
-		DynamicCompression:           useDynamicCompression,
-	}
 
 	backend := &frontdoor.SubResource{
 		ID: utils.String(frontDoorPath + "/BackendPools/" + backendPoolName),
 	}
 
 	forwardingConfiguration := frontdoor.ForwardingConfiguration{
 		ForwardingProtocol: frontdoor.ForwardingProtocol(forwardingProtocol),
-		CacheConfiguration: cacheConfiguration,
 		BackendPool:        backend,
 		OdataType:          frontdoor.OdataTypeMicrosoftAzureFrontDoorModelsFrontdoorForwardingConfiguration,
 	}
 
+	// Per the portal, if you enable the cache the cache_query_parameter_strip_directive
+	// is then a required attribute else the CacheConfiguration type is null
+	if cacheUseDynamicCompression {
+		forwardingConfiguration.CacheConfiguration = &frontdoor.CacheConfiguration{
+			DynamicCompression:           frontdoor.DynamicCompressionEnabledEnabled,
+			QueryParameterStripDirective: frontdoor.Query(cacheQueryParameterStripDirective),
+		}
+	}
+
 	if customForwardingPath != "" {
 		forwardingConfiguration.CustomForwardingPath = utils.String(customForwardingPath)
 	}
@@ -1409,11 +1408,11 @@ func flattenArmFrontDoorSubResource(input *frontdoor.SubResource, resourceType s
 	name := ""
 
 	if id := input.ID; id != nil {
-		aid, err := azure.ParseAzureResourceID(*id)
+		aid, err := ParseAzureResourceIDLowerPath(*id)
 		if err != nil {
 			return ""
 		}
-		name = aid.Path[resourceType]
+		name = aid.Path[strings.ToLower(resourceType)]
 	}
 
 	return name

azurerm/internal/services/frontdoor/validate.go

@@ -65,9 +65,17 @@ func ValidateFrontdoorSettings(d *schema.ResourceDiff) error {
 			if err := VerifyBackendPoolExists(fc["backend_pool_name"].(string), backendPools); err != nil {
 				return fmt.Errorf(`"routing_rule":%q is invalid. %+v`, routingRuleName, err)
 			}
+
+			// Check 3. validate if the cache_query_parameter_strip_directive is defined
+			//          that the cache_use_dynamic_compression is set to true
+			if cacheQueryParameterStripDirective := fc["cache_query_parameter_strip_directive"].(string); cacheQueryParameterStripDirective != "" {
+				if !fc["cache_use_dynamic_compression"].(bool) {
+					return fmt.Errorf(`"routing_rule": %q is invalid. "cache_use_dynamic_compression" must be set to "true" if the "cache_query_parameter_strip_directive" attribute is defined`, routingRuleName)
+				}
+			}
 		}
 
-		// Check 3. validate that each routing rule frontend_endpoints are actually defined in the resource schema
+		// Check 4. validate that each routing rule frontend_endpoints are actually defined in the resource schema
 		if routingRuleFrontends := routingRule["frontend_endpoints"].([]interface{}); len(routingRuleFrontends) > 0 {
 			if err := VerifyRoutingRuleFrontendEndpoints(routingRuleFrontends, configFrontendEndpoints); err != nil {
 				return fmt.Errorf(`"routing_rule":%q %+v`, routingRuleName, err)

website/docs/r/front_door.html.markdown

@@ -70,41 +70,41 @@ resource "azurerm_frontdoor" "example" {
 
 The following arguments are supported:
 
-* `name` - (Required) Name of the Front Door which is globally unique. Changing this forces a new resource to be created.
+* `name` - (Required) Specifies the name of the `Front Door` service. Changing this forces a new resource to be created.
 
-* `resource_group_name` - (Required) Name of the Resource group within the Azure subscription. Changing this forces a new resource to be created.
+* `resource_group_name` - (Required) Specifies the name of the Resource Group in which the `Front Door` service should exist. Changing this forces a new resource to be created.
 
-* `location` - (Required) Resource location. Changing this forces a new resource to be created.
+* `location` - (Required) Specifies the supported Azure location where the resource exists. Changing this forces a new resource to be created.
 
 * `backend_pool` - (Required) A `backend_pool` block as defined below.
 
 * `backend_pool_health_probe` - (Required) A `backend_pool_health_probe` block as defined below.
 
 * `backend_pool_load_balancing` - (Required) A `backend_pool_load_balancing` block as defined below.
 
-* `enforce_backend_pools_certificate_name_check` - (Required) Whether to enforce certificate name check on HTTPS requests to all backend pools. No effect on non-HTTPS requests. Permitted values are `true` or `false`.
+* `enforce_backend_pools_certificate_name_check` - (Required) Enforce certificate name check on `HTTPS` requests to all backend pools, this setting will have no effect on `HTTP` requests. Permitted values are `true` or `false`.
 
-* `load_balancer_enabled` - (Optional) Operational status of the Front Door load balancer. Permitted values are `true` or `false` Defaults to `true`.
+* `load_balancer_enabled` - (Optional) Operational status of the `Front Door` load balancer. Permitted values are `true` or `false` Defaults to `true`.
 
-* `friendly_name` - (Optional) A friendly name for the Front Door service.
+* `friendly_name` - (Optional) A friendly name for the `Front Door` service.
 
 * `frontend_endpoint` - (Required) A `frontend_endpoint` block as defined below.
 
 * `routing_rule` - (Required) A `routing_rule` block as defined below.
 
-* `tags` - (Optional) Resource tags.
+* `tags` - (Optional) A mapping of tags to assign to the resource.
 
 ---
 
 The `backend_pool` block supports the following:
 
-* `name` - (Required) The name of the `Backend Pool`.
+* `name` - (Required) Specifies the name of the name of the `Backend Pool`.
 
 * `backend` - (Required) A `backend` block as defined below.
 
-* `load_balancing_name` - (Required) The name property of the `backend_pool_load_balancing` block whithin this resource to use for the `Backend Pool`.
+* `load_balancing_name` - (Required) Specifies the name of the `backend_pool_load_balancing` block whithin this resource to use for this `Backend Pool`.
 
-* `health_probe_name` - (Required) The name property of a `backend_pool_health_probe` block whithin this resource to use for the `Backend Pool`.
+* `health_probe_name` - (Required) Specifies the name of the `backend_pool_health_probe` block whithin this resource to use for this `Backend Pool`.
 
 ---
 
@@ -126,35 +126,37 @@ The `backend` block supports the following:
 
 The `frontend_endpoint` block supports the following:
 
-* `name` - (Required) The name of the Frontend Endpoint.
+* `name` - (Required) Specifies the name of the `frontend_endpoint`.
 
-* `host_name` - (Required) The host name of the Frontend Endpoint. Must be a domain name.
-
-* `custom_https_provisioning_enabled` - (Required) Whether to allow HTTPS protocol for a custom domain that's associated with Front Door to ensure sensitive data is delivered securely via TLS/SSL encryption when sent across the internet. Valid options are `true` or `false`.
+* `host_name` - (Required) Specifies the host name of the `frontend_endpoint`. Must be a domain name.
 
 * `session_affinity_enabled` - (Optional) Whether to allow session affinity on this host. Valid options are `true` or `false` Defaults to `false`.
 
 * `session_affinity_ttl_seconds` - (Optional) The TTL to use in seconds for session affinity, if applicable. Defaults to `0`.
 
+* `custom_https_provisioning_enabled` - (Required) Whether to allow HTTPS protocol for a custom domain that's associated with Front Door to ensure sensitive data is delivered securely via TLS/SSL encryption when sent across the internet. Valid options are `true` or `false`.
+
+* `custom_https_configuration` - (Optional) A `custom_https_configuration` block as defined below. This block is required if the `custom_https_provisioning_enabled` is set to `true`.
+
 * `web_application_firewall_policy_link_id` - (Optional) Defines the Web Application Firewall policy `ID` for each host.
 
 ---
 
 The `backend_pool_health_probe` block supports the following:
 
-* `name` - (Required) The name of the Azure Front Door Backend Health Probe.
+* `name` - (Required) Specifies the name of the `backend_pool_health_probe`.
 
-* `path` - (Optional) The path to use for the Backend Health Probe. Default is `/`.
+* `path` - (Optional) The path to use for the `backend_pool_health_probe`. Default is `/`.
 
-* `protocol` - (Optional) Protocol scheme to use for the Backend Health Probe. Defaults to `Http`.
+* `protocol` - (Optional) Protocol scheme to use for the `backend_pool_health_probe`. Defaults to `Http`.
 
-* `interval_in_seconds` - (Optional) The number of seconds between health probes. Defaults to `120`.
+* `interval_in_seconds` - (Optional) The number of seconds between the `backend_pool_health_probe` probes. Defaults to `120`.
 
 ---
 
 The `backend_pool_load_balancing` block supports the following:
 
-* `name` - (Required) The name of the Azure Front Door Backend Load Balancer.
+* `name` - (Required) Specifies the name of the `backend_pool_load_balancing`.
 
 * `sample_size` - (Optional) The number of samples to consider for load balancing decisions. Defaults to `4`.
 
@@ -166,7 +168,7 @@ The `backend_pool_load_balancing` block supports the following:
 
 The `routing_rule` block supports the following:
 
-* `name` - (Required) The name of the Front Door Backend Routing Rule.
+* `name` - (Required) Specifies the name of the `routing_rule`.
 
 * `frontend_endpoints` - (Required) The names of the `frontend_endpoint` blocks whithin this resource to associate with this `routing_rule`.
 
@@ -184,23 +186,23 @@ The `routing_rule` block supports the following:
 
 The `forwarding_configuration` block supports the following:
 
-* `backend_pool_name` - (Required) The name of the Front Door Backend Pool. 
+* `backend_pool_name` - (Required) Specifies the name of the `backend_pool` to forward the incoming traffic to. 
 
-* `cache_use_dynamic_compression` - (Optional) Whether to use dynamic compression when caching. Valid options are `true` or `false`. Defaults to `true`.
+* `cache_use_dynamic_compression` - (Optional) Whether to use dynamic compression when caching. Valid options are `true` or `false`. Defaults to `false`.
 
-* `cache_query_parameter_strip_directive` - (Optional) Defines cache behavior in releation to query string parameters. Valid options are `StripAll` or `StripNone`. Defaults to `StripNone`
+* `cache_query_parameter_strip_directive` - (Optional) Defines cache behavior in releation to query string parameters. Valid options are `StripAll` or `StripNone`.
 
 * `custom_forwarding_path` - (Optional) Path to use when constructing the request to forward to the backend. This functions as a URL Rewrite. Default behavior preserves the URL path.
 
-* `forwarding_protocol` - (Optional) Protocol to use when redirecting. Valid options are `HttpOnly`, `HttpsOnly`, or `MatchRequest`. Defaults to `MatchRequest`.
+* `forwarding_protocol` - (Optional) Protocol to use when redirecting. Valid options are `HTTPOnly`, `HTTPSOnly`, or `MatchRequest`. Defaults to `HTTPSOnly`.
 
 ---
 
 The `redirect_configuration` block supports the following:
 
 * `custom_host` - (Optional)  Set this to change the URL for the redirection. 
 
-* `redirect_protocol` - (Optional) Protocol to use when redirecting. Valid options are `HttpOnly`, `HttpsOnly`, `MatchRequest`. Defaults to `MatchRequest`
+* `redirect_protocol` - (Optional) Protocol to use when redirecting. Valid options are `HTTPOnly`, `HTTPSOnly`, `MatchRequest`. Defaults to `MatchRequest`
 
 * `redirect_type` - (Optional) Status code for the redirect. Valida options are `Moved`, `Found`, `TemporaryRedirect`, `PermanentRedirect`. Defaults to `Found`
 
@@ -218,7 +220,7 @@ The `custom_https_configuration` block supports the following:
 
 The following attributes are only valid if `certificate_source` is set to `AzureKeyVault`:
 
-* `azure_key_vault_certificate_vault_id` - (Required) The `id` of the Key Vault containing the SSL certificate.
+* `azure_key_vault_certificate_vault_id` - (Required) The `ID` of the Key Vault containing the SSL certificate.
 
 * `azure_key_vault_certificate_secret_name` - (Required) The name of the Key Vault secret representing the full certificate PFX.