From 22130487f7214481e3f3ca6bc0b594e22e931643 Mon Sep 17 00:00:00 2001 From: Grant Tibbey Date: Tue, 11 Jun 2019 14:34:44 +1000 Subject: [PATCH 01/15] Fix page rule browser cache ttl when set to 0 --- cloudflare/resource_cloudflare_page_rule.go | 19 +-- .../resource_cloudflare_page_rule_test.go | 119 +++++++++++++++++- 2 files changed, 124 insertions(+), 14 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule.go b/cloudflare/resource_cloudflare_page_rule.go index 6af0bcdca1..34be5083c7 100644 --- a/cloudflare/resource_cloudflare_page_rule.go +++ b/cloudflare/resource_cloudflare_page_rule.go @@ -3,7 +3,7 @@ package cloudflare import ( "fmt" "log" - + "strconv" "strings" cloudflare "github.com/cloudflare/cloudflare-go" @@ -206,9 +206,9 @@ func resourceCloudflarePageRule() *schema.Resource { }, "browser_cache_ttl": { - Type: schema.TypeInt, - Optional: true, - ValidateFunc: validation.IntAtMost(31536000), + Type: schema.TypeString, + Optional: true, + //ValidateFunc: validation.IntAtMost(31536000), }, "edge_cache_ttl": { @@ -596,7 +596,12 @@ func transformToCloudflarePageRuleAction(id string, value interface{}, d *schema changed := d.HasChange(fmt.Sprintf("actions.0.%s", id)) if strValue, ok := value.(string); ok { - if strValue == "" && !changed { + if id == "browser_cache_ttl" && changed { + intValue, err := strconv.Atoi(strValue) + if err == nil { + pageRuleAction.Value = intValue + } + } else if strValue == "" && !changed { pageRuleAction.Value = nil } else { pageRuleAction.Value = strValue @@ -616,9 +621,7 @@ func transformToCloudflarePageRuleAction(id string, value interface{}, d *schema } } } else if intValue, ok := value.(int); ok { - if id == "browser_cache_ttl" && changed { - pageRuleAction.Value = intValue - } else if id == "edge_cache_ttl" && intValue > 0 && changed { + if id == "edge_cache_ttl" && intValue > 0 && changed { pageRuleAction.Value = intValue } else { pageRuleAction.Value = nil diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 5e5c063655..3bc205cf3a 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -2,17 +2,124 @@ package cloudflare import ( "fmt" - "os" - "testing" - - "reflect" - "regexp" - "github.com/cloudflare/cloudflare-go" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + "os" + "reflect" + "regexp" + "testing" ) +func TestCreatesBrowserCacheTTLIntegerValues(t *testing.T) { + runTestSteps(t, []resource.TestStep{ + { + Config: buildPageRule(`browser_cache_ttl = 1`), + Check: assertActionExists("browser_cache_ttl", "1", float64(1)), + }, + }) +} + +func TestCreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + runTestSteps(t, []resource.TestStep{ + { + Config: buildPageRule(`browser_cache_ttl = 0`), + Check: assertActionExists("browser_cache_ttl", "0", float64(0)), + }, + }) +} + +func TestUpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + runTestSteps(t, []resource.TestStep{ + { + Config: buildPageRule(`browser_cache_ttl = 1`), + }, + { + Config: buildPageRule(`browser_cache_ttl = 0`), + Check: assertActionExists("browser_cache_ttl", "0", float64(0)), + }, + }) +} + +func TestDeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + runTestSteps(t, []resource.TestStep{ + { + Config: buildPageRule(`browser_cache_ttl = 0`), + }, + { + Config: buildPageRule(`browser_check = "on"`), + Check: assertActionNotExists("browser_cache_ttl"), + }, + }) +} + +func buildPageRule(actions string) string { + zone := os.Getenv("CLOUDFLARE_DOMAIN") + target := fmt.Sprintf("terraform-test.%s", zone) + + return fmt.Sprintf(` + resource "cloudflare_page_rule" "terraform-test" { + zone = "%s" + target = "%s" + actions = { + %s + } + }`, + zone, + target, + actions) +} + +func runTestSteps(t *testing.T, testSteps []resource.TestStep) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudflarePageRuleDestroy, + Steps: testSteps, + }) +} + +func assertActionExists(key string, stateValue string, cloudflareValue interface{}) resource.TestCheckFunc { + resourceName := "cloudflare_page_rule.terraform-test" + return resource.ComposeTestCheckFunc( + assertStatePageRuleActionExists(resourceName, key, stateValue), + assertCloudflarePageRuleActionExists(resourceName, key, cloudflareValue), + ) +} + +func assertActionNotExists(key string) resource.TestCheckFunc { + resourceName := "cloudflare_page_rule.terraform-test" + return resource.ComposeTestCheckFunc( + assertStatePageRuleActionExists(resourceName, key, ""), + ) +} + +func assertStatePageRuleActionExists(resourceName string, key string, value string) resource.TestCheckFunc { + return resource.TestCheckResourceAttr( + resourceName, + fmt.Sprintf("actions.0.%s", key), + value) +} + +func assertCloudflarePageRuleActionExists(resourceName string, key string, value interface{}) resource.TestCheckFunc { + return func(state *terraform.State) error { + rs, _ := state.RootModule().Resources[resourceName] + client := testAccProvider.Meta().(*cloudflare.API) + pageRule, err := client.PageRule(rs.Primary.Attributes["zone_id"], rs.Primary.ID) + if err != nil { + return fmt.Errorf("cloudflare page rule not found: %s", err) + } + for _, pageRuleAction := range pageRule.Actions { + if pageRuleAction.ID == key && pageRuleAction.Value == value { + return nil + } + } + return fmt.Errorf("cloudflare page rule action not found %#v:%#v\nAction State\n%#v", key, value, pageRule.Actions) + } +} + +//################################################################# + func TestAccCloudflarePageRule_Basic(t *testing.T) { var pageRule cloudflare.PageRule zone := os.Getenv("CLOUDFLARE_DOMAIN") From e6db968d3cfbb2db17b2a7ce8123a7668b130029 Mon Sep 17 00:00:00 2001 From: Grant Tibbey Date: Tue, 11 Jun 2019 15:14:47 +1000 Subject: [PATCH 02/15] Fix testing resource format --- cloudflare/resource_cloudflare_page_rule_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 3bc205cf3a..03f43a8342 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -61,7 +61,7 @@ func buildPageRule(actions string) string { resource "cloudflare_page_rule" "terraform-test" { zone = "%s" target = "%s" - actions = { + actions { %s } }`, From b1dbaa88a803d4b069bb77c2b6c702ba2c507253 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Thu, 13 Jun 2019 14:56:18 +0400 Subject: [PATCH 03/15] Remove ValidateFunc now that browser_cache_ttl is a string --- cloudflare/resource_cloudflare_page_rule.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cloudflare/resource_cloudflare_page_rule.go b/cloudflare/resource_cloudflare_page_rule.go index 34be5083c7..9fbc13cf24 100644 --- a/cloudflare/resource_cloudflare_page_rule.go +++ b/cloudflare/resource_cloudflare_page_rule.go @@ -208,7 +208,6 @@ func resourceCloudflarePageRule() *schema.Resource { "browser_cache_ttl": { Type: schema.TypeString, Optional: true, - //ValidateFunc: validation.IntAtMost(31536000), }, "edge_cache_ttl": { From e2ccc1b58f93d5610cb216cfde7c7edc17ab19ab Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 12:06:38 +0400 Subject: [PATCH 04/15] Format import line --- cloudflare/resource_cloudflare_page_rule_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 03f43a8342..24aa3960bc 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -2,13 +2,14 @@ package cloudflare import ( "fmt" - "github.com/cloudflare/cloudflare-go" - "github.com/hashicorp/terraform/helper/resource" - "github.com/hashicorp/terraform/terraform" "os" "reflect" "regexp" "testing" + + "github.com/cloudflare/cloudflare-go" + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" ) func TestCreatesBrowserCacheTTLIntegerValues(t *testing.T) { From 46eae5c78458dd2045d40a40414c38eebd20e9b0 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 12:07:17 +0400 Subject: [PATCH 05/15] Rename tests according to conventions --- cloudflare/resource_cloudflare_page_rule_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 24aa3960bc..a3ef7bfb3d 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -12,7 +12,7 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestCreatesBrowserCacheTTLIntegerValues(t *testing.T) { +func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) { runTestSteps(t, []resource.TestStep{ { Config: buildPageRule(`browser_cache_ttl = 1`), @@ -21,7 +21,7 @@ func TestCreatesBrowserCacheTTLIntegerValues(t *testing.T) { }) } -func TestCreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { +func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { runTestSteps(t, []resource.TestStep{ { Config: buildPageRule(`browser_cache_ttl = 0`), @@ -30,7 +30,7 @@ func TestCreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { }) } -func TestUpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { +func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { runTestSteps(t, []resource.TestStep{ { Config: buildPageRule(`browser_cache_ttl = 1`), @@ -42,7 +42,7 @@ func TestUpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { }) } -func TestDeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { +func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { runTestSteps(t, []resource.TestStep{ { Config: buildPageRule(`browser_cache_ttl = 0`), From cff5aa18a0941a422b0954766cfcc4f0e600d6da Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 12:15:42 +0400 Subject: [PATCH 06/15] Rename method --- cloudflare/resource_cloudflare_page_rule_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index a3ef7bfb3d..062b175463 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -15,7 +15,7 @@ import ( func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) { runTestSteps(t, []resource.TestStep{ { - Config: buildPageRule(`browser_cache_ttl = 1`), + Config: buildPageRuleConfig(`browser_cache_ttl = 1`), Check: assertActionExists("browser_cache_ttl", "1", float64(1)), }, }) @@ -24,7 +24,7 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { runTestSteps(t, []resource.TestStep{ { - Config: buildPageRule(`browser_cache_ttl = 0`), + Config: buildPageRuleConfig(`browser_cache_ttl = 0`), Check: assertActionExists("browser_cache_ttl", "0", float64(0)), }, }) @@ -33,10 +33,10 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { runTestSteps(t, []resource.TestStep{ { - Config: buildPageRule(`browser_cache_ttl = 1`), + Config: buildPageRuleConfig(`browser_cache_ttl = 1`), }, { - Config: buildPageRule(`browser_cache_ttl = 0`), + Config: buildPageRuleConfig(`browser_cache_ttl = 0`), Check: assertActionExists("browser_cache_ttl", "0", float64(0)), }, }) @@ -45,16 +45,16 @@ func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { runTestSteps(t, []resource.TestStep{ { - Config: buildPageRule(`browser_cache_ttl = 0`), + Config: buildPageRuleConfig(`browser_cache_ttl = 0`), }, { - Config: buildPageRule(`browser_check = "on"`), + Config: buildPageRuleConfig(`browser_check = "on"`), Check: assertActionNotExists("browser_cache_ttl"), }, }) } -func buildPageRule(actions string) string { +func buildPageRuleConfig(actions string) string { zone := os.Getenv("CLOUDFLARE_DOMAIN") target := fmt.Sprintf("terraform-test.%s", zone) From d0f1eddafda4a43bed65c80c3a10902af0ac10c8 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 12:18:06 +0400 Subject: [PATCH 07/15] Rename method --- cloudflare/resource_cloudflare_page_rule_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 062b175463..64d6ad993d 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -13,7 +13,7 @@ import ( ) func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) { - runTestSteps(t, []resource.TestStep{ + testAccRunResourceTestSteps(t, []resource.TestStep{ { Config: buildPageRuleConfig(`browser_cache_ttl = 1`), Check: assertActionExists("browser_cache_ttl", "1", float64(1)), @@ -22,7 +22,7 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) } func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { - runTestSteps(t, []resource.TestStep{ + testAccRunResourceTestSteps(t, []resource.TestStep{ { Config: buildPageRuleConfig(`browser_cache_ttl = 0`), Check: assertActionExists("browser_cache_ttl", "0", float64(0)), @@ -31,7 +31,7 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders } func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { - runTestSteps(t, []resource.TestStep{ + testAccRunResourceTestSteps(t, []resource.TestStep{ { Config: buildPageRuleConfig(`browser_cache_ttl = 1`), }, @@ -43,7 +43,7 @@ func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders } func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { - runTestSteps(t, []resource.TestStep{ + testAccRunResourceTestSteps(t, []resource.TestStep{ { Config: buildPageRuleConfig(`browser_cache_ttl = 0`), }, @@ -71,7 +71,7 @@ func buildPageRuleConfig(actions string) string { actions) } -func runTestSteps(t *testing.T, testSteps []resource.TestStep) { +func testAccRunResourceTestSteps(t *testing.T, testSteps []resource.TestStep) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, From 48f34281ffa4139da40efcb8086619824b5d5d99 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 12:29:23 +0400 Subject: [PATCH 08/15] Rename test methods --- cloudflare/resource_cloudflare_page_rule_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 64d6ad993d..50211b9355 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -83,26 +83,26 @@ func testAccRunResourceTestSteps(t *testing.T, testSteps []resource.TestStep) { func assertActionExists(key string, stateValue string, cloudflareValue interface{}) resource.TestCheckFunc { resourceName := "cloudflare_page_rule.terraform-test" return resource.ComposeTestCheckFunc( - assertStatePageRuleActionExists(resourceName, key, stateValue), - assertCloudflarePageRuleActionExists(resourceName, key, cloudflareValue), + testAccCheckStatePageRuleExistsWithAction(resourceName, key, stateValue), + testAccCheckCloudflarePageRuleExistsWithAction(resourceName, key, cloudflareValue), ) } func assertActionNotExists(key string) resource.TestCheckFunc { resourceName := "cloudflare_page_rule.terraform-test" return resource.ComposeTestCheckFunc( - assertStatePageRuleActionExists(resourceName, key, ""), + testAccCheckStatePageRuleExistsWithAction(resourceName, key, ""), ) } -func assertStatePageRuleActionExists(resourceName string, key string, value string) resource.TestCheckFunc { +func testAccCheckStatePageRuleExistsWithAction(resourceName string, key string, value string) resource.TestCheckFunc { return resource.TestCheckResourceAttr( resourceName, fmt.Sprintf("actions.0.%s", key), value) } -func assertCloudflarePageRuleActionExists(resourceName string, key string, value interface{}) resource.TestCheckFunc { +func testAccCheckCloudflarePageRuleExistsWithAction(resourceName string, key string, value interface{}) resource.TestCheckFunc { return func(state *terraform.State) error { rs, _ := state.RootModule().Resources[resourceName] client := testAccProvider.Meta().(*cloudflare.API) From 6eab1f03d3d109f6db5b4a823740071adacb9b9a Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 13:38:36 +0400 Subject: [PATCH 09/15] Remove higher level composer functions to make things more explicit --- .../resource_cloudflare_page_rule_test.go | 58 +++++++++---------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 50211b9355..8f981a59a3 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -15,8 +15,11 @@ import ( func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) { testAccRunResourceTestSteps(t, []resource.TestStep{ { - Config: buildPageRuleConfig(`browser_cache_ttl = 1`), - Check: assertActionExists("browser_cache_ttl", "1", float64(1)), + Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "1"), + testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(1)), + ), }, }) } @@ -24,8 +27,11 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { testAccRunResourceTestSteps(t, []resource.TestStep{ { - Config: buildPageRuleConfig(`browser_cache_ttl = 0`), - Check: assertActionExists("browser_cache_ttl", "0", float64(0)), + Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), + Check: resource.ComposeTestCheckFunc( + testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "0"), + testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(0)), + ), }, }) } @@ -33,11 +39,14 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { testAccRunResourceTestSteps(t, []resource.TestStep{ { - Config: buildPageRuleConfig(`browser_cache_ttl = 1`), + Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), }, { - Config: buildPageRuleConfig(`browser_cache_ttl = 0`), - Check: assertActionExists("browser_cache_ttl", "0", float64(0)), + Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), + Check: resource.ComposeTestCheckFunc( + testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "0"), + testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(0)), + ), }, }) } @@ -45,27 +54,28 @@ func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { testAccRunResourceTestSteps(t, []resource.TestStep{ { - Config: buildPageRuleConfig(`browser_cache_ttl = 0`), + Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), }, { - Config: buildPageRuleConfig(`browser_check = "on"`), - Check: assertActionNotExists("browser_cache_ttl"), + Config: buildPageRuleConfig("test", `browser_check = "on"`), + Check: testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", ""), }, }) } -func buildPageRuleConfig(actions string) string { +func buildPageRuleConfig(resourceName string, actions string) string { zone := os.Getenv("CLOUDFLARE_DOMAIN") target := fmt.Sprintf("terraform-test.%s", zone) return fmt.Sprintf(` - resource "cloudflare_page_rule" "terraform-test" { + resource "cloudflare_page_rule" "%s" { zone = "%s" target = "%s" actions { %s } - }`, + }`, + resourceName, zone, target, actions) @@ -80,31 +90,19 @@ func testAccRunResourceTestSteps(t *testing.T, testSteps []resource.TestStep) { }) } -func assertActionExists(key string, stateValue string, cloudflareValue interface{}) resource.TestCheckFunc { - resourceName := "cloudflare_page_rule.terraform-test" - return resource.ComposeTestCheckFunc( - testAccCheckStatePageRuleExistsWithAction(resourceName, key, stateValue), - testAccCheckCloudflarePageRuleExistsWithAction(resourceName, key, cloudflareValue), - ) -} - -func assertActionNotExists(key string) resource.TestCheckFunc { - resourceName := "cloudflare_page_rule.terraform-test" - return resource.ComposeTestCheckFunc( - testAccCheckStatePageRuleExistsWithAction(resourceName, key, ""), - ) -} - func testAccCheckStatePageRuleExistsWithAction(resourceName string, key string, value string) resource.TestCheckFunc { return resource.TestCheckResourceAttr( - resourceName, + fmt.Sprintf("cloudflare_page_rule.%s", resourceName), fmt.Sprintf("actions.0.%s", key), value) } func testAccCheckCloudflarePageRuleExistsWithAction(resourceName string, key string, value interface{}) resource.TestCheckFunc { return func(state *terraform.State) error { - rs, _ := state.RootModule().Resources[resourceName] + rs, ok := state.RootModule().Resources[fmt.Sprintf("cloudflare_page_rule.%s", resourceName)] + if !ok { + return fmt.Errorf("Not found: %s", resourceName) + } client := testAccProvider.Meta().(*cloudflare.API) pageRule, err := client.PageRule(rs.Primary.Attributes["zone_id"], rs.Primary.ID) if err != nil { From 01dae6ede958e176c75e49bedf437da0e19f18a7 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 13:41:38 +0400 Subject: [PATCH 10/15] Move tests around --- .../resource_cloudflare_page_rule_test.go | 212 +++++++++--------- 1 file changed, 105 insertions(+), 107 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 8f981a59a3..224e36a85c 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -12,113 +12,6 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) { - testAccRunResourceTestSteps(t, []resource.TestStep{ - { - Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), - Check: resource.ComposeTestCheckFunc( - testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "1"), - testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(1)), - ), - }, - }) -} - -func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { - testAccRunResourceTestSteps(t, []resource.TestStep{ - { - Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), - Check: resource.ComposeTestCheckFunc( - testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "0"), - testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(0)), - ), - }, - }) -} - -func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { - testAccRunResourceTestSteps(t, []resource.TestStep{ - { - Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), - }, - { - Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), - Check: resource.ComposeTestCheckFunc( - testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "0"), - testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(0)), - ), - }, - }) -} - -func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { - testAccRunResourceTestSteps(t, []resource.TestStep{ - { - Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), - }, - { - Config: buildPageRuleConfig("test", `browser_check = "on"`), - Check: testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", ""), - }, - }) -} - -func buildPageRuleConfig(resourceName string, actions string) string { - zone := os.Getenv("CLOUDFLARE_DOMAIN") - target := fmt.Sprintf("terraform-test.%s", zone) - - return fmt.Sprintf(` - resource "cloudflare_page_rule" "%s" { - zone = "%s" - target = "%s" - actions { - %s - } - }`, - resourceName, - zone, - target, - actions) -} - -func testAccRunResourceTestSteps(t *testing.T, testSteps []resource.TestStep) { - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckCloudflarePageRuleDestroy, - Steps: testSteps, - }) -} - -func testAccCheckStatePageRuleExistsWithAction(resourceName string, key string, value string) resource.TestCheckFunc { - return resource.TestCheckResourceAttr( - fmt.Sprintf("cloudflare_page_rule.%s", resourceName), - fmt.Sprintf("actions.0.%s", key), - value) -} - -func testAccCheckCloudflarePageRuleExistsWithAction(resourceName string, key string, value interface{}) resource.TestCheckFunc { - return func(state *terraform.State) error { - rs, ok := state.RootModule().Resources[fmt.Sprintf("cloudflare_page_rule.%s", resourceName)] - if !ok { - return fmt.Errorf("Not found: %s", resourceName) - } - client := testAccProvider.Meta().(*cloudflare.API) - pageRule, err := client.PageRule(rs.Primary.Attributes["zone_id"], rs.Primary.ID) - if err != nil { - return fmt.Errorf("cloudflare page rule not found: %s", err) - } - for _, pageRuleAction := range pageRule.Actions { - if pageRuleAction.ID == key && pageRuleAction.Value == value { - return nil - } - } - return fmt.Errorf("cloudflare page rule action not found %#v:%#v\nAction State\n%#v", key, value, pageRule.Actions) - } -} - -//################################################################# - func TestAccCloudflarePageRule_Basic(t *testing.T) { var pageRule cloudflare.PageRule zone := os.Getenv("CLOUDFLARE_DOMAIN") @@ -376,6 +269,57 @@ func TestTranformForwardingURL(t *testing.T) { } } +func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) { + testAccRunResourceTestSteps(t, []resource.TestStep{ + { + Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "1"), + testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(1)), + ), + }, + }) +} + +func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + testAccRunResourceTestSteps(t, []resource.TestStep{ + { + Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), + Check: resource.ComposeTestCheckFunc( + testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "0"), + testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(0)), + ), + }, + }) +} + +func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + testAccRunResourceTestSteps(t, []resource.TestStep{ + { + Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), + }, + { + Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), + Check: resource.ComposeTestCheckFunc( + testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "0"), + testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(0)), + ), + }, + }) +} + +func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + testAccRunResourceTestSteps(t, []resource.TestStep{ + { + Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), + }, + { + Config: buildPageRuleConfig("test", `browser_check = "on"`), + Check: testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", ""), + }, + }) +} + func testAccCheckCloudflarePageRuleRecreated(before, after *cloudflare.PageRule) resource.TestCheckFunc { return func(s *terraform.State) error { if before.ID == after.ID { @@ -629,3 +573,57 @@ resource "cloudflare_page_rule" "test" { } }`, zone, target) } + +func buildPageRuleConfig(resourceName string, actions string) string { + zone := os.Getenv("CLOUDFLARE_DOMAIN") + target := fmt.Sprintf("terraform-test.%s", zone) + + return fmt.Sprintf(` + resource "cloudflare_page_rule" "%s" { + zone = "%s" + target = "%s" + actions { + %s + } + }`, + resourceName, + zone, + target, + actions) +} + +func testAccRunResourceTestSteps(t *testing.T, testSteps []resource.TestStep) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudflarePageRuleDestroy, + Steps: testSteps, + }) +} + +func testAccCheckStatePageRuleExistsWithAction(resourceName string, key string, value string) resource.TestCheckFunc { + return resource.TestCheckResourceAttr( + fmt.Sprintf("cloudflare_page_rule.%s", resourceName), + fmt.Sprintf("actions.0.%s", key), + value) +} + +func testAccCheckCloudflarePageRuleExistsWithAction(resourceName string, key string, value interface{}) resource.TestCheckFunc { + return func(state *terraform.State) error { + rs, ok := state.RootModule().Resources[fmt.Sprintf("cloudflare_page_rule.%s", resourceName)] + if !ok { + return fmt.Errorf("Not found: %s", resourceName) + } + client := testAccProvider.Meta().(*cloudflare.API) + pageRule, err := client.PageRule(rs.Primary.Attributes["zone_id"], rs.Primary.ID) + if err != nil { + return fmt.Errorf("cloudflare page rule not found: %s", err) + } + for _, pageRuleAction := range pageRule.Actions { + if pageRuleAction.ID == key && pageRuleAction.Value == value { + return nil + } + } + return fmt.Errorf("cloudflare page rule action not found %#v:%#v\nAction State\n%#v", key, value, pageRule.Actions) + } +} From 1b3d608349b688c21e3594de4f2a4ff95d1068a6 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 14:13:54 +0400 Subject: [PATCH 11/15] More consistent resource name and add exists check --- .../resource_cloudflare_page_rule_test.go | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 224e36a85c..123b20f056 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -270,30 +270,35 @@ func TestTranformForwardingURL(t *testing.T) { } func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) { + var pageRule cloudflare.PageRule testAccRunResourceTestSteps(t, []resource.TestStep{ { Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), Check: resource.ComposeTestCheckFunc( - testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "1"), - testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(1)), + testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), + testAccCheckStatePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", "1"), + testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(1)), ), }, }) } func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + var pageRule cloudflare.PageRule testAccRunResourceTestSteps(t, []resource.TestStep{ { Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), Check: resource.ComposeTestCheckFunc( - testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "0"), - testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(0)), + testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), + testAccCheckStatePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", "0"), + testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(0)), ), }, }) } func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + var pageRule cloudflare.PageRule testAccRunResourceTestSteps(t, []resource.TestStep{ { Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), @@ -301,21 +306,26 @@ func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders { Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), Check: resource.ComposeTestCheckFunc( - testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", "0"), - testAccCheckCloudflarePageRuleExistsWithAction("test", "browser_cache_ttl", float64(0)), + testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), + testAccCheckStatePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", "0"), + testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(0)), ), }, }) } func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders(t *testing.T) { + var pageRule cloudflare.PageRule testAccRunResourceTestSteps(t, []resource.TestStep{ { Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), }, { Config: buildPageRuleConfig("test", `browser_check = "on"`), - Check: testAccCheckStatePageRuleExistsWithAction("test", "browser_cache_ttl", ""), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), + testAccCheckStatePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", ""), + ), }, }) } @@ -603,14 +613,14 @@ func testAccRunResourceTestSteps(t *testing.T, testSteps []resource.TestStep) { func testAccCheckStatePageRuleExistsWithAction(resourceName string, key string, value string) resource.TestCheckFunc { return resource.TestCheckResourceAttr( - fmt.Sprintf("cloudflare_page_rule.%s", resourceName), + resourceName, fmt.Sprintf("actions.0.%s", key), value) } func testAccCheckCloudflarePageRuleExistsWithAction(resourceName string, key string, value interface{}) resource.TestCheckFunc { return func(state *terraform.State) error { - rs, ok := state.RootModule().Resources[fmt.Sprintf("cloudflare_page_rule.%s", resourceName)] + rs, ok := state.RootModule().Resources[resourceName] if !ok { return fmt.Errorf("Not found: %s", resourceName) } From a9f5377201d4ffe37d88566ebf56c1e53bdb4fd7 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 14:16:40 +0400 Subject: [PATCH 12/15] Consistency in check calls --- cloudflare/resource_cloudflare_page_rule_test.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 123b20f056..8a15402460 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -276,7 +276,7 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), Check: resource.ComposeTestCheckFunc( testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), - testAccCheckStatePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", "1"), + resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.browser_cache_ttl", "1"), testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(1)), ), }, @@ -290,7 +290,7 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), Check: resource.ComposeTestCheckFunc( testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), - testAccCheckStatePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", "0"), + resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.browser_cache_ttl", "0"), testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(0)), ), }, @@ -307,7 +307,7 @@ func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), Check: resource.ComposeTestCheckFunc( testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), - testAccCheckStatePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", "0"), + resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.browser_cache_ttl", "0"), testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(0)), ), }, @@ -324,7 +324,7 @@ func TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders Config: buildPageRuleConfig("test", `browser_check = "on"`), Check: resource.ComposeTestCheckFunc( testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), - testAccCheckStatePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", ""), + resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.browser_cache_ttl", ""), ), }, }) @@ -611,13 +611,6 @@ func testAccRunResourceTestSteps(t *testing.T, testSteps []resource.TestStep) { }) } -func testAccCheckStatePageRuleExistsWithAction(resourceName string, key string, value string) resource.TestCheckFunc { - return resource.TestCheckResourceAttr( - resourceName, - fmt.Sprintf("actions.0.%s", key), - value) -} - func testAccCheckCloudflarePageRuleExistsWithAction(resourceName string, key string, value interface{}) resource.TestCheckFunc { return func(state *terraform.State) error { rs, ok := state.RootModule().Resources[resourceName] From 1553e5300b30e7e1bc5dc4ecb95443ceb7a302d2 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Tue, 18 Jun 2019 14:25:49 +0400 Subject: [PATCH 13/15] Refactor check method to make use of previously fetched pageRule --- .../resource_cloudflare_page_rule_test.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 8a15402460..5c764138cb 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -276,8 +276,8 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues(t *testing.T) Config: buildPageRuleConfig("test", "browser_cache_ttl = 1"), Check: resource.ComposeTestCheckFunc( testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), + testAccCheckCloudflarePageRuleHasAction(&pageRule, "browser_cache_ttl", float64(1)), resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.browser_cache_ttl", "1"), - testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(1)), ), }, }) @@ -291,7 +291,7 @@ func TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders Check: resource.ComposeTestCheckFunc( testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.browser_cache_ttl", "0"), - testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(0)), + testAccCheckCloudflarePageRuleHasAction(&pageRule, "browser_cache_ttl", float64(0)), ), }, }) @@ -307,8 +307,8 @@ func TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders Config: buildPageRuleConfig("test", "browser_cache_ttl = 0"), Check: resource.ComposeTestCheckFunc( testAccCheckCloudflarePageRuleExists("cloudflare_page_rule.test", &pageRule), + testAccCheckCloudflarePageRuleHasAction(&pageRule, "browser_cache_ttl", float64(0)), resource.TestCheckResourceAttr("cloudflare_page_rule.test", "actions.0.browser_cache_ttl", "0"), - testAccCheckCloudflarePageRuleExistsWithAction("cloudflare_page_rule.test", "browser_cache_ttl", float64(0)), ), }, }) @@ -611,17 +611,8 @@ func testAccRunResourceTestSteps(t *testing.T, testSteps []resource.TestStep) { }) } -func testAccCheckCloudflarePageRuleExistsWithAction(resourceName string, key string, value interface{}) resource.TestCheckFunc { +func testAccCheckCloudflarePageRuleHasAction(pageRule *cloudflare.PageRule, key string, value interface{}) resource.TestCheckFunc { return func(state *terraform.State) error { - rs, ok := state.RootModule().Resources[resourceName] - if !ok { - return fmt.Errorf("Not found: %s", resourceName) - } - client := testAccProvider.Meta().(*cloudflare.API) - pageRule, err := client.PageRule(rs.Primary.Attributes["zone_id"], rs.Primary.ID) - if err != nil { - return fmt.Errorf("cloudflare page rule not found: %s", err) - } for _, pageRuleAction := range pageRule.Actions { if pageRuleAction.ID == key && pageRuleAction.Value == value { return nil From 2fd98b919cc605b638e8e54feb600dc6b10f47f2 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Wed, 19 Jun 2019 14:27:48 +0400 Subject: [PATCH 14/15] Additional import tests & represent browser_cache_ttl as string internally This warning was being emitted, and the main import test would fail if `browser_cache_ttl = 0` (or any value) was added. Error setting actions in page rule "8f8d71918b97a24ed742e037596b5e1c": actions.0.browser_cache_ttl: '' expected type 'string', got unconvertible type 'float64 --- .../import_cloudflare_page_rule_test.go | 46 +++++++++++++++++++ cloudflare/resource_cloudflare_page_rule.go | 8 +++- .../resource_cloudflare_page_rule_test.go | 1 + 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/cloudflare/import_cloudflare_page_rule_test.go b/cloudflare/import_cloudflare_page_rule_test.go index a4a47f9f25..cb9a089c5b 100644 --- a/cloudflare/import_cloudflare_page_rule_test.go +++ b/cloudflare/import_cloudflare_page_rule_test.go @@ -40,3 +40,49 @@ func TestAccCloudflarePageRule_Import(t *testing.T) { }, }) } + +func TestAccCloudflarePageRule_ImportWithBrowserCacheTTL30(t *testing.T) { + var pageRule cloudflare.PageRule + zone := os.Getenv("CLOUDFLARE_DOMAIN") + name := "cloudflare_page_rule.test" + testAccRunResourceTestSteps(t, []resource.TestStep{ + { + Config: buildPageRuleConfig("test", "browser_cache_ttl = 30"), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudflarePageRuleExists(name, &pageRule), + ), + }, + { + ResourceName: name, + ImportStateIdPrefix: fmt.Sprintf("%s/", zone), + ImportState: true, + ImportStateVerify: true, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudflarePageRuleExists(name, &pageRule), + ), + }, + }) +} + +func TestAccCloudflarePageRule_ImportWithoutBrowserCacheTTL(t *testing.T) { + var pageRule cloudflare.PageRule + zone := os.Getenv("CLOUDFLARE_DOMAIN") + name := "cloudflare_page_rule.test" + testAccRunResourceTestSteps(t, []resource.TestStep{ + { + Config: buildPageRuleConfig("test", `browser_check = "on"`), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudflarePageRuleExists(name, &pageRule), + ), + }, + { + ResourceName: name, + ImportStateIdPrefix: fmt.Sprintf("%s/", zone), + ImportState: true, + ImportStateVerify: true, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudflarePageRuleExists(name, &pageRule), + ), + }, + }) +} diff --git a/cloudflare/resource_cloudflare_page_rule.go b/cloudflare/resource_cloudflare_page_rule.go index 9fbc13cf24..7fcfc3b8bd 100644 --- a/cloudflare/resource_cloudflare_page_rule.go +++ b/cloudflare/resource_cloudflare_page_rule.go @@ -541,9 +541,11 @@ var pageRuleAPINilFields = []string{ "disable_security", } var pageRuleAPIFloatFields = []string{ - "browser_cache_ttl", "edge_cache_ttl", } +var pageRuleAPIFloatAsStringFields = []string{ + "browser_cache_ttl", +} var pageRuleAPIStringFields = []string{ "bypass_cache_on_cookie", "cache_key", @@ -577,6 +579,10 @@ func transformFromCloudflarePageRuleAction(pageRuleAction *cloudflare.PageRuleAc value = pageRuleAction.Value.(string) break + case contains(pageRuleAPIFloatAsStringFields, pageRuleAction.ID): + value = fmt.Sprintf("%.0f", pageRuleAction.Value.(float64)) + break + case pageRuleAction.ID == "forwarding_url" || pageRuleAction.ID == "minify": value = []interface{}{pageRuleAction.Value.(map[string]interface{})} break diff --git a/cloudflare/resource_cloudflare_page_rule_test.go b/cloudflare/resource_cloudflare_page_rule_test.go index 5c764138cb..a17e52cb6f 100644 --- a/cloudflare/resource_cloudflare_page_rule_test.go +++ b/cloudflare/resource_cloudflare_page_rule_test.go @@ -541,6 +541,7 @@ resource "cloudflare_page_rule" "test" { actions { always_online = "on" browser_check = "on" + browser_cache_ttl = 0 email_obfuscation = "on" ip_geolocation = "on" server_side_exclude = "on" From 5d53cfc129c9079a5be3177df2d0f21bd204b436 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Thu, 20 Jun 2019 08:56:08 +0400 Subject: [PATCH 15/15] Check for page rule action id inline for clarity/simplicity --- cloudflare/resource_cloudflare_page_rule.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/cloudflare/resource_cloudflare_page_rule.go b/cloudflare/resource_cloudflare_page_rule.go index 7fcfc3b8bd..774c09cef2 100644 --- a/cloudflare/resource_cloudflare_page_rule.go +++ b/cloudflare/resource_cloudflare_page_rule.go @@ -540,12 +540,6 @@ var pageRuleAPINilFields = []string{ "disable_railgun", "disable_security", } -var pageRuleAPIFloatFields = []string{ - "edge_cache_ttl", -} -var pageRuleAPIFloatAsStringFields = []string{ - "browser_cache_ttl", -} var pageRuleAPIStringFields = []string{ "bypass_cache_on_cookie", "cache_key", @@ -571,15 +565,15 @@ func transformFromCloudflarePageRuleAction(pageRuleAction *cloudflare.PageRuleAc value = true break - case contains(pageRuleAPIFloatFields, pageRuleAction.ID): - value = pageRuleAction.Value.(float64) // we use TypeInt but terraform seems to do the right thing converting from float - break - case contains(pageRuleAPIStringFields, pageRuleAction.ID): value = pageRuleAction.Value.(string) break - case contains(pageRuleAPIFloatAsStringFields, pageRuleAction.ID): + case pageRuleAction.ID == "edge_cache_ttl": + value = pageRuleAction.Value.(float64) // we use TypeInt but terraform seems to do the right thing converting from float + break + + case pageRuleAction.ID == "browser_cache_ttl": value = fmt.Sprintf("%.0f", pageRuleAction.Value.(float64)) break