From feb8ad180cfd7c3894b316cffb034a2402d203db Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 10 Jun 2020 08:15:09 +1000 Subject: [PATCH 1/2] argo: Allow settings to be individually customised There was an assumption in this resource that when you were applying one of the settings, that you would have entitlements to both and you would be explicitly setting both during the resource management. This isn't the case[1] and there are scenarios where someone will manage the `tiered_caching` setting without the entitlement to toggle `smart_routing` to the default value of "off". This splits the two API calls to be individually managed and only applied if there is a value provided for both (customised or default doesn't matter) Fixes #701 [1]: terraform-providers/terraform-provider-cloudflare#701 --- cloudflare/resource_cloudflare_argo.go | 51 ++++++++------- cloudflare/resource_cloudflare_argo_test.go | 70 +++++++++++++++++++++ 2 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 cloudflare/resource_cloudflare_argo_test.go diff --git a/cloudflare/resource_cloudflare_argo.go b/cloudflare/resource_cloudflare_argo.go index 2d97755eb6..a9823f5277 100644 --- a/cloudflare/resource_cloudflare_argo.go +++ b/cloudflare/resource_cloudflare_argo.go @@ -31,13 +31,11 @@ func resourceCloudflareArgo() *schema.Resource { "tiered_caching": { Type: schema.TypeString, ValidateFunc: validation.StringInSlice([]string{"on", "off"}, false), - Default: "off", Optional: true, }, "smart_routing": { Type: schema.TypeString, ValidateFunc: validation.StringInSlice([]string{"on", "off"}, false), - Default: "off", Optional: true, }, }, @@ -50,22 +48,27 @@ func resourceCloudflareArgoRead(d *schema.ResourceData, meta interface{}) error log.Printf("[DEBUG] zone ID: %s", zoneID) - tieredCaching, err := client.ArgoTieredCaching(zoneID) - if err != nil { - return errors.Wrap(err, "failed to get tiered caching setting") - } + checksum := stringChecksum(fmt.Sprintf("%s/argo", zoneID)) + d.SetId(checksum) + d.Set("zone_id", zoneID) + + if _, ok := d.GetOk("tiered_caching"); ok { + tieredCaching, err := client.ArgoTieredCaching(zoneID) + if err != nil { + return errors.Wrap(err, "failed to get tiered caching setting") + } - smartRouting, err := client.ArgoSmartRouting(zoneID) - if err != nil { - return errors.Wrap(err, "failed to get smart routing setting") + d.Set("tiered_caching", tieredCaching.Value) } - checksum := stringChecksum(fmt.Sprintf("%s/argo", zoneID)) - d.SetId(checksum) + if _, ok := d.GetOk("smart_routing"); ok { + smartRouting, err := client.ArgoSmartRouting(zoneID) + if err != nil { + return errors.Wrap(err, "failed to get smart routing setting") + } - d.Set("zone_id", zoneID) - d.Set("tiered_caching", tieredCaching.Value) - d.Set("smart_routing", smartRouting.Value) + d.Set("smart_routing", smartRouting.Value) + } return nil } @@ -76,17 +79,21 @@ func resourceCloudflareArgoUpdate(d *schema.ResourceData, meta interface{}) erro tieredCaching := d.Get("tiered_caching").(string) smartRouting := d.Get("smart_routing").(string) - argoSmartRouting, err := client.UpdateArgoSmartRouting(zoneID, smartRouting) - if err != nil { - return errors.Wrap(err, "failed to update smart routing setting") + if smartRouting != "" { + argoSmartRouting, err := client.UpdateArgoSmartRouting(zoneID, smartRouting) + if err != nil { + return errors.Wrap(err, "failed to update smart routing setting") + } + log.Printf("[DEBUG] Argo Smart Routing set to: %s", argoSmartRouting.Value) } - log.Printf("[DEBUG] Argo Smart Routing set to: %s", argoSmartRouting.Value) - argoTieredCaching, err := client.UpdateArgoTieredCaching(zoneID, tieredCaching) - if err != nil { - return errors.Wrap(err, "failed to update tiered caching setting") + if tieredCaching != "" { + argoTieredCaching, err := client.UpdateArgoTieredCaching(zoneID, tieredCaching) + if err != nil { + return errors.Wrap(err, "failed to update tiered caching setting") + } + log.Printf("[DEBUG] Argo Tiered Caching set to: %s", argoTieredCaching.Value) } - log.Printf("[DEBUG] Argo Tiered Caching set to: %s", argoTieredCaching.Value) return resourceCloudflareArgoRead(d, meta) } diff --git a/cloudflare/resource_cloudflare_argo_test.go b/cloudflare/resource_cloudflare_argo_test.go new file mode 100644 index 0000000000..4ba70a838d --- /dev/null +++ b/cloudflare/resource_cloudflare_argo_test.go @@ -0,0 +1,70 @@ +package cloudflare + +import ( + "fmt" + "os" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" +) + +func TestAccCloudflareArgoOnlySetTieredCaching(t *testing.T) { + zoneID := os.Getenv("CLOUDFLARE_ZONE_ID") + rnd := generateRandomResourceName() + name := fmt.Sprintf("cloudflare_argo.%s", rnd) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCheckCloudflareArgoTieredCachingConfig(zoneID, rnd), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(name, "tiered_caching", "on"), + resource.TestCheckNoResourceAttr(name, "smart_routing"), + ), + }, + }, + }) +} + +func TestAccCloudflareArgoSetTieredAndSmartRouting(t *testing.T) { + zoneID := os.Getenv("CLOUDFLARE_ZONE_ID") + rnd := generateRandomResourceName() + name := fmt.Sprintf("cloudflare_argo.%s", rnd) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCheckCloudflareArgoFullConfig(zoneID, rnd), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(name, "tiered_caching", "on"), + resource.TestCheckResourceAttr(name, "smart_routing", "on"), + ), + }, + }, + }) +} + +func testAccCheckCloudflareArgoTieredCachingConfig(zoneID, name string) string { + return fmt.Sprintf(` + resource "cloudflare_argo" "%[2]s" { + zone_id = "%[1]s" + tiered_caching = "on" + }`, zoneID, name) +} + +func testAccCheckCloudflareArgoFullConfig(zoneID, name string) string { + return fmt.Sprintf(` + resource "cloudflare_argo" "%[2]s" { + zone_id = "%[1]s" + tiered_caching = "on" + smart_routing = "on" + }`, zoneID, name) +} From a89e8c41f57bf604927e37a9ffc16eb78fed0487 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 10 Jun 2020 09:47:41 +1000 Subject: [PATCH 2/2] Update website documentation --- website/docs/r/argo.html.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/r/argo.html.markdown b/website/docs/r/argo.html.markdown index 0dc730b2c5..acdc0e8e8a 100644 --- a/website/docs/r/argo.html.markdown +++ b/website/docs/r/argo.html.markdown @@ -25,8 +25,8 @@ resource "cloudflare_argo" "example" { The following arguments are supported: * `zone_id` - (Required) The DNS zone ID that you wish to manage Argo on. -* `tiered_caching` - (Optional) Whether tiered caching is enabled. Valid values: `on` or `off`. Defaults to `off`. -* `smart_routing` - (Optional) Whether smart routing is enabled. Valid values: `on` or `off`. Defaults to `off`. +* `tiered_caching` - (Optional) Whether tiered caching is enabled. Valid values: `on` or `off`. +* `smart_routing` - (Optional) Whether smart routing is enabled. Valid values: `on` or `off`. ## Import