Skip to content

Commit

Permalink
Merge pull request #975 from cloudflare/handle-cache-key-fields-all-o…
Browse files Browse the repository at this point in the history
…ptions

cloudflare_page_rule: handle lists and string values for `cache_key_fields`
  • Loading branch information
jacobbednarz authored Mar 3, 2021
2 parents 7a92d7c + a8a428a commit 2ebced9
Show file tree
Hide file tree
Showing 2 changed files with 183 additions and 15 deletions.
24 changes: 15 additions & 9 deletions cloudflare/resource_cloudflare_page_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cloudflare
import (
"fmt"
"log"
"reflect"
"strconv"
"strings"

Expand Down Expand Up @@ -745,14 +746,14 @@ func transformFromCloudflarePageRuleAction(pageRuleAction *cloudflare.PageRuleAc
fieldOutput[fieldID] = fieldValue
}

if itemExistsInSlice(fieldOutput["exclude"], "*") {
fieldOutput["exclude"] = []interface{}{"*"}
if reflect.TypeOf(fieldOutput["exclude"]).Kind() == reflect.String && fieldOutput["exclude"] == "*" {
fieldOutput["ignore"] = true
fieldOutput["exclude"] = []interface{}{}
}

if itemExistsInSlice(fieldOutput["include"], "*") {
fieldOutput["include"] = []interface{}{}
if reflect.TypeOf(fieldOutput["include"]).Kind() == reflect.String && fieldOutput["include"] == "*" {
fieldOutput["ignore"] = false
fieldOutput["include"] = []interface{}{}
}

output[sectionID] = []interface{}{fieldOutput}
Expand Down Expand Up @@ -887,22 +888,27 @@ func transformToCloudflarePageRuleAction(id string, value interface{}, d *schema
}

if sectionOutput["ignore"].(bool) {
sectionOutput["exclude"] = []interface{}{"*"}
sectionOutput["exclude"] = "*"
}

delete(sectionOutput, "ignore")
}

exclude, ok1 := sectionOutput["exclude"]
include, ok2 := sectionOutput["include"]
ignore := sectionOutput["ignore"]

// Ensure that if no `include`, `exclude` or `ignore` attributes are
// set, we default to including all query string parameters in the
// cache key.
if (!ok1 || len(exclude.([]interface{})) == 0) && (!ok2 || len(include.([]interface{})) == 0) {
sectionOutput["include"] = []interface{}{"*"}
if ignore == nil || !ignore.(bool) {
if (!ok1 || len(exclude.([]interface{})) == 0) && (!ok2 || len(include.([]interface{})) == 0) {
sectionOutput["include"] = "*"
}
}

// Clean up the payload and ensure we don't send `ignore` property
// despite using it in the schema.
delete(sectionOutput, "ignore")

output[sectionID] = sectionOutput
default:
for fieldID, fieldValue := range sectionValue.([]interface{})[0].(map[string]interface{}) {
Expand Down
174 changes: 168 additions & 6 deletions cloudflare/resource_cloudflare_page_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,8 @@ func TestCacheKeyFieldsNilValue(t *testing.T) {
t.Fatalf("Unexpected error transforming page rule action: %s", err)
}

if !reflect.DeepEqual(pageRuleAction.Value.(map[string]interface{})["query_string"], map[string]interface{}{"include": []interface{}{"*"}}) {
t.Fatalf("Unexpected transformToCloudflarePageRuleAction result, expected %#v, got %#v", map[string]interface{}{"include": []interface{}{"*"}}, pageRuleAction.Value.(map[string]interface{})["query_string"])
if !reflect.DeepEqual(pageRuleAction.Value.(map[string]interface{})["query_string"], map[string]interface{}{"include": "*"}) {
t.Fatalf("Unexpected transformToCloudflarePageRuleAction result, expected %#v, got %#v", map[string]interface{}{"include": "*"}, pageRuleAction.Value.(map[string]interface{})["query_string"])
}
}

Expand Down Expand Up @@ -515,14 +515,68 @@ func TestAccCloudflarePageRuleCacheKeyFieldsExcludeAllQueryString(t *testing.T)
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.header.0.check_presence.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.header.0.include.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.host.0.resolved", "true"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.query_string.0.exclude.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.query_string.0.ignore", "true"),
),
},
},
})
}

func TestAccCloudflarePageRuleCacheKeyFields2(t *testing.T) {
func TestAccCloudflarePageRuleCacheKeyFieldsExcludeMultipleValuesQueryString(t *testing.T) {
var pageRule cloudflare.PageRule
domain := os.Getenv("CLOUDFLARE_DOMAIN")
zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
rnd := generateRandomResourceName()
pageRuleTarget := fmt.Sprintf("%s.%s", rnd, domain)
resourceName := fmt.Sprintf("cloudflare_page_rule.%s", rnd)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckCloudflarePageRuleDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckCloudflarePageRuleConfigCacheKeyFieldsExcludeMultipleValuesQueryString(zoneID, rnd, pageRuleTarget),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudflarePageRuleExists(resourceName, &pageRule),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.cookie.0.check_presence.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.cookie.0.include.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.header.0.check_presence.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.header.0.include.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.host.0.resolved", "true"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.query_string.0.exclude.#", "2"),
),
},
},
})
}

func TestAccCloudflarePageRuleCacheKeyFieldsNoQueryStringValuesDefined(t *testing.T) {
var pageRule cloudflare.PageRule
domain := os.Getenv("CLOUDFLARE_DOMAIN")
zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
target := fmt.Sprintf("test-cache-key-fields.%s", domain)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckCloudflarePageRuleDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckCloudflarePageRuleConfigCacheKeyFieldsNoQueryStringValuesDefined(zoneID, target),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.header.0.exclude.#", "1"),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.host.0.resolved", "false"),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.user.0.device_type", "true"),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.user.0.geo", "true"),
),
},
},
})
}

func TestAccCloudflarePageRuleCacheKeyFieldsIncludeAllQueryStringValues(t *testing.T) {
var pageRule cloudflare.PageRule
domain := os.Getenv("CLOUDFLARE_DOMAIN")
zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
Expand All @@ -534,13 +588,43 @@ func TestAccCloudflarePageRuleCacheKeyFields2(t *testing.T) {
CheckDestroy: testAccCheckCloudflarePageRuleDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckCloudflarePageRuleConfigCacheKeyFields2(zoneID, target),
Config: testAccCheckCloudflarePageRuleConfigCacheKeyFieldsIncludeAllQueryStringValues(zoneID, target),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.header.0.exclude.#", "1"),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.host.0.resolved", "false"),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.user.0.device_type", "true"),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.user.0.geo", "true"),
resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.cache_key_fields.0.query_string.0.ignore", "false"),
),
},
},
})
}

func TestAccCloudflarePageRuleCacheKeyFieldsIncludeMultipleValuesQueryString(t *testing.T) {
var pageRule cloudflare.PageRule
domain := os.Getenv("CLOUDFLARE_DOMAIN")
zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
rnd := generateRandomResourceName()
pageRuleTarget := fmt.Sprintf("%s.%s", rnd, domain)
resourceName := fmt.Sprintf("cloudflare_page_rule.%s", rnd)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckCloudflarePageRuleDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckCloudflarePageRuleConfigCacheKeyFieldsIncludeMultipleValuesQueryString(zoneID, rnd, pageRuleTarget),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudflarePageRuleExists(resourceName, &pageRule),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.cookie.0.check_presence.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.cookie.0.include.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.header.0.check_presence.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.header.0.include.#", "1"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.host.0.resolved", "true"),
resource.TestCheckResourceAttr(resourceName, "actions.0.cache_key_fields.0.query_string.0.include.#", "2"),
),
},
},
Expand Down Expand Up @@ -940,7 +1024,34 @@ resource "cloudflare_page_rule" "%[3]s" {
}`, zoneID, target, rnd)
}

func testAccCheckCloudflarePageRuleConfigCacheKeyFields2(zoneID, target string) string {
func testAccCheckCloudflarePageRuleConfigCacheKeyFieldsExcludeMultipleValuesQueryString(zoneID, rnd, target string) string {
return fmt.Sprintf(`
resource "cloudflare_page_rule" "%[3]s" {
zone_id = "%[1]s"
target = "%[3]s"
actions {
cache_key_fields {
cookie {
check_presence = ["cookie_presence"]
include = ["cookie_include"]
}
header {
check_presence = ["header_presence"]
include = ["header_include"]
}
host {
resolved = true
}
query_string {
exclude = ["query1", "query2"]
}
user {}
}
}
}`, zoneID, target, rnd)
}

func testAccCheckCloudflarePageRuleConfigCacheKeyFieldsNoQueryStringValuesDefined(zoneID, target string) string {
return fmt.Sprintf(`
resource "cloudflare_page_rule" "test" {
zone_id = "%s"
Expand All @@ -962,6 +1073,57 @@ resource "cloudflare_page_rule" "test" {
}`, zoneID, target)
}

func testAccCheckCloudflarePageRuleConfigCacheKeyFieldsIncludeAllQueryStringValues(zoneID, target string) string {
return fmt.Sprintf(`
resource "cloudflare_page_rule" "test" {
zone_id = "%s"
target = "%s"
actions {
cache_key_fields {
cookie {}
header {
exclude = ["origin"]
}
host {}
query_string {
ignore = false
}
user {
device_type = true
geo = true
}
}
}
}`, zoneID, target)
}

func testAccCheckCloudflarePageRuleConfigCacheKeyFieldsIncludeMultipleValuesQueryString(zoneID, rnd, target string) string {
return fmt.Sprintf(`
resource "cloudflare_page_rule" "%[3]s" {
zone_id = "%[1]s"
target = "%[3]s"
actions {
cache_key_fields {
cookie {
check_presence = ["cookie_presence"]
include = ["cookie_include"]
}
header {
check_presence = ["header_presence"]
include = ["header_include"]
}
host {
resolved = true
}
query_string {
include = ["query1", "query2"]
}
user {}
}
}
}`, zoneID, target, rnd)
}

func testAccCheckCloudflarePageRuleConfigCacheTTLByStatus(zoneID, target, rnd string) string {
return fmt.Sprintf(`
resource "cloudflare_page_rule" "%[3]s" {
Expand Down

0 comments on commit 2ebced9

Please sign in to comment.