Rate limit for zone resource #30
Conversation
There was a problem hiding this comment.
Hi there! The size of this pull request makes it very difficult to review. If possible please extract the changes to the included vendor folder into it's own Pull Request.
The same applies to https://github.com/terraform-providers/terraform-provider-cloudflare/pull/31
Please let me know if that is not possible.
benjvi
force-pushed
the
zone-rate-limits
branch
from
5112b0c to
1856988
Compare
benjvi
force-pushed
the
zone-rate-limits
branch
from
829bc80 to
4daa72c
Compare
| d.Set("threshold", rateLimit.Threshold) | ||
| d.Set("period", rateLimit.Period) | ||
| d.Set("match", flattenRateLimitTrafficMatcher(rateLimit.Match)) | ||
| d.Set("action", flattenRateLimitAction(rateLimit.Action)) |
There was a problem hiding this comment.
Errors need to be checked when setting non-scalar attributes such as action and match:
if err := d.Set("action", ...); err != nil {
log.Printf("[WARN] Error setting action: %s", err)
}
|
@catsby I updated with your comments. Also I changed the ID a bit so it could be consistent with the existing DNS record resource + a bit more intuitive ( it also seems that underscore is relatively common in subdomains so I changed the separator to "/" on import ) |
Adds the new resource and updates the cloudflare-go library to a recent version
Partially due to the way structured maps are (not) handled in terraform the expanding/flattening logic is pretty long for the nested attributes. I've split things out so hopefully its a bit clearer. The only other change from the go client should be that bypass_url_patterns was specified as a key-value struct, here its simplified as a string list.
Since we need both zone + resource id to do lookups the ID is a amalgamation of the two, and we generate two computed fields for the separate zone + resource ids which is what I refer to in any operations after read. Hopefully this makes sense - this approach not common since normally resource id is sufficient for lookups but there is some prior art in the AWS provider for this