Rate Limit plan invalid when correlate not set

[sodabrew]

In version 1.12, since PR #204 was merged, terraform plan against a cloudflare_rate_limit stanza that does not have correlate (it is listed as optional after all) and for which no correlate is set in the Console, always reports a pending change:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ cloudflare_rate_limit.my_domain_name
      correlate.#: "1" => "0"

Plan: 0 to add, 1 to change, 0 to destroy.

Neither "1" nor "0" would seem to be valid values for correlate, given that by = "nat" is the only option shown in the provider docs - though I cannot even find the documentation for this feature - so I'm not sure what's a correct value here.

[13rac1]

Additional information. The correlate field is listed as optional in the documentation: https://www.terraform.io/docs/providers/cloudflare/r/rate_limit.html#correlate and this provider:
https://github.com/terraform-providers/terraform-provider-cloudflare/blob/3969f39fa172863b0836edd5197230f5962e8f38/cloudflare/resource_cloudflare_rate_limit.go#L180-L182
But is always set (so not optional?):
https://github.com/terraform-providers/terraform-provider-cloudflare/blob/3969f39fa172863b0836edd5197230f5962e8f38/cloudflare/resource_cloudflare_rate_limit.go#L230

And is required (or at least will never be nil or missing) in the Cloudflare supplied structs: https://github.com/cloudflare/cloudflare-go/blob/9837a599c0ba49ff8208dd679b4500e8ece407a5/rate_limiting.go#L21

	Correlate   RateLimitCorrelate      `json:"correlate"`

Workaround to make Terraform idempotent is an empty correlate in the tf file:

correlate {}

[jacobbednarz]

@sodabrew The reason it is showing up like that is because it's a List type, it keeps track of the number of elements in the type. A setting with correlate defined is:

correlate.#: "1"
correlate.0.by: "nat"

which translates to:

correlate {
  by = "nat"
}

I'll have to do some more digging as I'm not seeing this with the test configurations. Did you previously have a broken correlate setting in your Terraform state file? Could you try removing the rate limit using terraform state rm (or fully delete it from your terraform file) and then re-adding/import it?

[jacobbednarz]

There is also the Read method which could be causing this as it's setting the value.

https://github.com/terraform-providers/terraform-provider-cloudflare/blob/3969f39fa172863b0836edd5197230f5962e8f38/cloudflare/resource_cloudflare_rate_limit.go#L449

[jacobbednarz]

Ok, so I've narrowed this down to the Read method and it unnecessarily trying to sync with the remote side of things. I'm now just trying to work out the exact type to wrap an if check in to ensure that it still works when importing (the initial issue in #204) and plan/apply don't trigger extra changes.

[jacobbednarz]

The most sensible approach I can come up with is the following:

diff --git a/cloudflare/resource_cloudflare_rate_limit.go b/cloudflare/resource_cloudflare_rate_limit.go
index dfc531d..fc239b5 100644
--- a/cloudflare/resource_cloudflare_rate_limit.go
+++ b/cloudflare/resource_cloudflare_rate_limit.go
@@ -227,7 +227,10 @@ func resourceCloudflareRateLimitCreate(d *schema.ResourceData, meta interface{})

+       if rateLimit.Correlate != nil {
+               d.Set("correlate", flattenRateLimitCorrelate(*rateLimit.Correlate))
+       }
+
        d.Set("description", rateLimit.Description)
        d.Set("disabled", rateLimit.Disabled)

diff --git a/vendor/github.com/cloudflare/cloudflare-go/rate_limiting.go b/vendor/github.com/cloudflare/cloudflare-go/rate_limiting.go
index 253c44b..2b3690c 100644
--- a/vendor/github.com/cloudflare/cloudflare-go/rate_limiting.go
+++ b/vendor/github.com/cloudflare/cloudflare-go/rate_limiting.go
@@ -18,7 +18,7 @@ type RateLimit struct {
        Threshold   int                     `json:"threshold"`
        Period      int                     `json:"period"`
        Action      RateLimitAction         `json:"action"`
-       Correlate   RateLimitCorrelate      `json:"correlate"`
+       Correlate   *RateLimitCorrelate     `json:"correlate"`
 }

 // RateLimitTrafficMatcher contains the rules that will be used to apply a rate limit to traffic

This does mean though, we'll need to address this in the underlying library first but it does cover all of our use cases and prevents the need to do weird things with the built in Terraform types.

[jacobbednarz]

Have opened cloudflare/cloudflare-go#281 to get this addressed upstream and then I can fix it here too.

[jacobbednarz]

This has landed so I'll be taking a pass at this in the coming days.

[13rac1]

Awesome! Thanks!