Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: azurerm_frontdoor fix the way backend_pool_load_balancing/backend_pool_health_probe are saved to state file, exposed new attributes in backend_pool_health_probe block enabled and probe_method #5924

Merged
merged 13 commits into from
Mar 3, 2020
Merged
63 changes: 54 additions & 9 deletions azurerm/internal/services/frontdoor/resource_arm_frontdoor.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package frontdoor
import (
"fmt"
"log"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/frontdoor/mgmt/2019-11-01/frontdoor"
Expand Down Expand Up @@ -261,6 +262,11 @@ func resourceArmFrontDoor() *schema.Resource {
Required: true,
ValidateFunc: ValidateBackendPoolRoutingRuleName,
},
"enabled": {
Type: schema.TypeBool,
Optional: true,
Default: true,
},
"path": {
Type: schema.TypeString,
Optional: true,
Expand All @@ -275,6 +281,15 @@ func resourceArmFrontDoor() *schema.Resource {
}, false),
Default: string(frontdoor.HTTP),
},
"probe_method": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{
string(frontdoor.GET),
string(frontdoor.HEAD),
}, false),
Default: string(frontdoor.GET),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor aesthetics/consistency, Could we put the default above validatefunc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

},
"interval_in_seconds": {
Type: schema.TypeInt,
Optional: true,
Expand Down Expand Up @@ -484,6 +499,7 @@ func resourceArmFrontDoorCreateUpdate(d *schema.ResourceData, meta interface{})
frontendEndpoints := d.Get("frontend_endpoint").([]interface{})
backendPoolsSettings := d.Get("enforce_backend_pools_certificate_name_check").(bool)
enabledState := d.Get("load_balancer_enabled").(bool)

t := d.Get("tags").(map[string]interface{})

frontDoorParameters := frontdoor.FrontDoor{
Expand Down Expand Up @@ -862,6 +878,18 @@ func expandArmFrontDoorHealthProbeSettingsModel(input []interface{}, frontDoorPa
protocol := v["protocol"].(string)
intervalInSeconds := int32(v["interval_in_seconds"].(int))
name := v["name"].(string)
probeMethod := v["probe_method"].(string)
enabled := v["enabled"].(bool)

healthProbeEnabled := frontdoor.HealthProbeEnabledEnabled
if !enabled {
healthProbeEnabled = frontdoor.HealthProbeEnabledDisabled
}

healthProbeMethod := frontdoor.GET
if probeMethod == string(frontdoor.HEAD) {
healthProbeMethod = frontdoor.HEAD
}

result := frontdoor.HealthProbeSettingsModel{
ID: utils.String(frontDoorPath + "/HealthProbeSettings/" + name),
Expand All @@ -870,6 +898,8 @@ func expandArmFrontDoorHealthProbeSettingsModel(input []interface{}, frontDoorPa
IntervalInSeconds: utils.Int32(intervalInSeconds),
Path: utils.String(path),
Protocol: frontdoor.Protocol(protocol),
HealthProbeMethod: healthProbeMethod,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just do

Suggested change
HealthProbeMethod: healthProbeMethod,
HealthProbeMethod: frontdoor.HealthProbeMethol(v ["probe_method"].(string))

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, should work... fixed.

EnabledState: healthProbeEnabled,
},
}

Expand Down Expand Up @@ -1076,7 +1106,7 @@ func expandArmFrontDoorForwardingConfiguration(input []interface{}, frontDoorPat

if cacheQueryParameterStripDirective == "" {
// Set Default Value for strip directive is not in the key slice and cache is enabled
cacheQueryParameterStripDirective = string(frontdoor.StripNone)
cacheQueryParameterStripDirective = string(frontdoor.StripAll)
}

forwardingConfiguration.CacheConfiguration = &frontdoor.CacheConfiguration{
Expand Down Expand Up @@ -1276,12 +1306,13 @@ func flattenArmFrontDoorFrontendEndpoint(d *schema.ResourceData, input *[]frontd
}

func flattenArmFrontDoorHealthProbeSettingsModel(input *[]frontdoor.HealthProbeSettingsModel) []interface{} {
results := make([]interface{}, 0)
if input == nil {
return make([]interface{}, 0)
return results
}
result := make(map[string]interface{})

for _, v := range *input {
result := make(map[string]interface{})
if id := v.ID; id != nil {
result["id"] = *id
}
Expand All @@ -1295,21 +1326,31 @@ func flattenArmFrontDoorHealthProbeSettingsModel(input *[]frontdoor.HealthProbeS
if path := properties.Path; path != nil {
result["path"] = *path
}
if healthProbeMethod := properties.HealthProbeMethod; healthProbeMethod != "" {
// I have to upper this as the frontdoor.GET and frondoor.HEAD types are uppercased
// but Azure stores them in the resource as pascal cased (e.g. "Get" and "Head")
result["probe_method"] = strings.ToUpper(string(healthProbeMethod))
}
if enabled := properties.EnabledState; enabled != "" {
result["enabled"] = (enabled == frontdoor.HealthProbeEnabledEnabled)
}
result["protocol"] = string(properties.Protocol)
}

results = append(results, result)
}

return []interface{}{result}
return results
}

func flattenArmFrontDoorLoadBalancingSettingsModel(input *[]frontdoor.LoadBalancingSettingsModel) []interface{} {
results := make([]interface{}, 0)
if input == nil {
return make([]interface{}, 0)
return results
}

result := make(map[string]interface{})

for _, v := range *input {
result := make(map[string]interface{})
if id := v.ID; id != nil {
result["id"] = *id
}
Expand All @@ -1327,8 +1368,11 @@ func flattenArmFrontDoorLoadBalancingSettingsModel(input *[]frontdoor.LoadBalanc
result["successful_samples_required"] = *successfulSamplesRequired
}
}

results = append(results, result)
}
return []interface{}{result}

return results
}

func flattenArmFrontDoorRoutingRule(input *[]frontdoor.RoutingRule, oldBlocks interface{}) []interface{} {
Expand Down Expand Up @@ -1387,7 +1431,8 @@ func flattenArmFrontDoorRoutingRule(input *[]frontdoor.RoutingRule, oldBlocks in
if stripDirective := cacheConfiguration.QueryParameterStripDirective; stripDirective != "" {
c["cache_query_parameter_strip_directive"] = string(stripDirective)
} else {
c["cache_query_parameter_strip_directive"] = string(frontdoor.StripNone)
// Default value if not set
c["cache_query_parameter_strip_directive"] = string(frontdoor.StripAll)
}

if dynamicCompression := cacheConfiguration.DynamicCompression; dynamicCompression != "" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ func TestAccAzureRMFrontDoor_basic(t *testing.T) {
Config: testAccAzureRMFrontDoor_basic(data),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMFrontDoorExists(data.ResourceName),
resource.TestCheckResourceAttr(data.ResourceName, "backend_pool_health_probe.0.enabled", "true"),
resource.TestCheckResourceAttr(data.ResourceName, "backend_pool_health_probe.0.probe_method", "GET"),
),
},
{
Config: testAccAzureRMFrontDoor_basicDisabled(data),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMFrontDoorExists(data.ResourceName),
resource.TestCheckResourceAttr(data.ResourceName, "backend_pool_health_probe.0.enabled", "false"),
resource.TestCheckResourceAttr(data.ResourceName, "backend_pool_health_probe.0.probe_method", "HEAD"),
),
},
data.ImportStep(),
Expand Down Expand Up @@ -134,7 +144,7 @@ func TestAccAzureRMFrontDoor_EnableDisableCache(t *testing.T) {
testCheckAzureRMFrontDoorExists(data.ResourceName),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_enabled", "true"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_use_dynamic_compression", "false"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_query_parameter_strip_directive", "StripNone"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_query_parameter_strip_directive", "StripAll"),
),
},
{
Expand All @@ -143,7 +153,7 @@ func TestAccAzureRMFrontDoor_EnableDisableCache(t *testing.T) {
testCheckAzureRMFrontDoorExists(data.ResourceName),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_enabled", "false"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_use_dynamic_compression", "false"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_query_parameter_strip_directive", "StripNone"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_query_parameter_strip_directive", "StripAll"),
),
},
{
Expand All @@ -152,7 +162,7 @@ func TestAccAzureRMFrontDoor_EnableDisableCache(t *testing.T) {
testCheckAzureRMFrontDoorExists(data.ResourceName),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_enabled", "true"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_use_dynamic_compression", "false"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_query_parameter_strip_directive", "StripNone"),
resource.TestCheckResourceAttr(data.ResourceName, "routing_rule.0.forwarding_configuration.0.cache_query_parameter_strip_directive", "StripAll"),
),
},
data.ImportStep(),
Expand Down Expand Up @@ -299,6 +309,69 @@ resource "azurerm_frontdoor" "test" {
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
}

func testAccAzureRMFrontDoor_basicDisabled(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a provior block up here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

name = "acctestRG-%d"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ad frontdoor here

Suggested change
name = "acctestRG-%d"
name = "acctestRG-frontdoor-%d"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

location = "%s"
}

locals {
backend_name = "backend-bing"
endpoint_name = "frontend-endpoint"
health_probe_name = "health-probe"
load_balancing_name = "load-balancing-setting"
}

resource "azurerm_frontdoor" "test" {
name = "acctestfd-%d"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uppercase where supported

Suggested change
name = "acctestfd-%d"
name = "acctest-FD-%d"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
enforce_backend_pools_certificate_name_check = false

routing_rule {
name = "routing-rule"
accepted_protocols = ["Http", "Https"]
patterns_to_match = ["/*"]
frontend_endpoints = [local.endpoint_name]
forwarding_configuration {
forwarding_protocol = "MatchRequest"
backend_pool_name = local.backend_name
}
}

backend_pool_load_balancing {
name = local.load_balancing_name
}

backend_pool_health_probe {
name = local.health_probe_name
enabled = false
probe_method = "HEAD"
}

backend_pool {
name = local.backend_name
backend {
host_header = "www.bing.com"
address = "www.bing.com"
http_port = 80
https_port = 443
}

load_balancing_name = local.load_balancing_name
health_probe_name = local.health_probe_name
}

frontend_endpoint {
name = local.endpoint_name
host_name = "acctestfd-%d.azurefd.net"
custom_https_provisioning_enabled = false
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
}

func testAccAzureRMFrontDoor_requiresImport(data acceptance.TestData) string {
template := testAccAzureRMFrontDoor_basic(data)
return fmt.Sprintf(`
Expand Down Expand Up @@ -508,7 +581,6 @@ resource "azurerm_frontdoor" "test" {
forwarding_configuration {
forwarding_protocol = "MatchRequest"
backend_pool_name = local.backend_name
cache_enabled = false
}
}

Expand Down Expand Up @@ -571,6 +643,7 @@ resource "azurerm_frontdoor" "test" {
forwarding_configuration {
forwarding_protocol = "MatchRequest"
backend_pool_name = local.backend_name
cache_enabled = true
}
}

Expand Down
6 changes: 6 additions & 0 deletions website/docs/r/frontdoor.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,16 @@ The `backend_pool_health_probe` block supports the following:

* `name` - (Required) Specifies the name of the Health Probe.

* `enabled` - (Optional) Is this health probe enabled? Dafaults to `true`.

* `path` - (Optional) The path to use for the Health Probe. Default is `/`.

* `protocol` - (Optional) Protocol scheme to use for the Health Probe. Defaults to `Http`.

* `probe_method` - (Optional) Specifies HTTP method the health probe uses when querying the backend pool instances. Possible values include: `Get` and `Head`. Defaults to `Get`.

-> **NOTE:** Use the `Head` method if you do not need to check the response body of your health probe.

* `interval_in_seconds` - (Optional) The number of seconds between each Health Probe. Defaults to `120`.

---
Expand Down