Commits on Jan 31, 2024

1. resource/cloudflare_ruleset: improve unknown value handling …

   Within the rulesets schema, we have a handful of fields that are `Computed`.
   The expectation of this directive is that it is a value not known at run time
   and _may_ be provided by the remote. However, this premise will break down and
   not work correctly if the value is ever provided to the plan since the value is
   no longer "unknown". This subtle bug is one part of what is contributing to the
   additional output toil in #2690. To address this, I've removed the lines that
   modified these values and were being supplied to the plan operation.
   
   This takes the confusing and bloated output from looking like this:
   
   ```
   Terraform will perform the following actions:
   
     # cloudflare_ruleset.rate_limiting_example will be updated in-place
     ~ resource "cloudflare_ruleset" "rate_limiting_example" {
           id          = "87e1099f077f4e49bbfbe159217ff605"
           name        = "restrict API requests count"
           # (4 unchanged attributes hidden)
   
         ~ rules {
             ~ id           = "e3b97fdf354c4c24a7ac091c3537fbc5" -> (known after apply)
             ~ last_updated = "2024-01-31 04:02:55.651275 +0000 UTC" -> (known after apply)
             ~ ref          = "e3b97fdf354c4c24a7ac091c3537fbc5" -> (known after apply)
             ~ version      = "1" -> (known after apply)
               # (4 unchanged attributes hidden)
   
               # (1 unchanged block hidden)
           }
         + rules {
             + action       = "block"
             + description  = "rate limit for API"
             + enabled      = true
             + expression   = "(http.request.uri.path matches \"^/api0/\")"
             + id           = (known after apply)
             + last_updated = (known after apply)
             + ref          = (known after apply)
             + version      = (known after apply)
   
             + ratelimit {
                 + characteristics     = [
                     + "cf.colo.id",
                     + "ip.src",
                   ]
                 + mitigation_timeout  = 600
                 + period              = 60
                 + requests_per_period = 100
                 + requests_to_origin  = false
               }
           }
       }
   
   Plan: 0 to add, 1 to change, 0 to destroy.
   ```
   
   To a clearer and more concise output:
   
   ```
   Terraform will perform the following actions:
   
     # cloudflare_ruleset.rate_limiting_example will be updated in-place
     ~ resource "cloudflare_ruleset" "rate_limiting_example" {
           id          = "87e1099f077f4e49bbfbe159217ff605"
           name        = "restrict API requests count"
           # (4 unchanged attributes hidden)
   
         ~ rules {
               id           = "39ed6015367444e3905d838cadc1b9c2"
             + last_updated = (known after apply)
             + version      = (known after apply)
               # (5 unchanged attributes hidden)
   
               # (1 unchanged block hidden)
           }
         + rules {
             + action       = "block"
             + description  = "rate limit for API"
             + enabled      = true
             + expression   = "(http.request.uri.path matches \"^/api0/\")"
             + id           = (known after apply)
             + last_updated = (known after apply)
             + ref          = (known after apply)
             + version      = (known after apply)
   
             + ratelimit {
                 + characteristics     = [
                     + "cf.colo.id",
                     + "ip.src",
                   ]
                 + mitigation_timeout  = 600
                 + period              = 60
                 + requests_per_period = 100
                 + requests_to_origin  = false
               }
           }
       }
   
   Plan: 0 to add, 1 to change, 0 to destroy.
   ```
   
   The second part of this confusing output is the `last_updated` and `version`
   output. The `last_updated` is pretty self explanatory however, there are some
   minor but important details about the `version` field here. Within ERE, it
   captures and is tied to a particular state of the ruleset rule at any point and
   is incremented when changes are detected to any parts of the rule. The naunce
   here is that ERE does not perform a diff of the current state and what is
   proposed. Instead, it autoincrements this field **even if the payload is
   identical**. So why is this important for the Terraform resource? Well, since
   we use explicit ordering via [`ListNestedObject`][1] schema attribute, we are
   sending all rules to the API even when only a single one changes to ensure we
   maintain the correctness the end user expects. Doing this causes the
   `last_updated` and `version` fields to always show as unknown values and result
   in updates. There are some plans to potentially look at de-duping the payloads
   to prevent spurious versions being created however, that won't help here until
   such a time that we individually manage `rules` as their own entities.
   
   Closes #2690 by improving the output and marking the `version` and
   `last_updated` as known values that will always be changing due to the way we
   update all the resources.
   
   [1]: https://github.com/cloudflare/terraform-provider-cloudflare/blob/f7c05af8be05e0ef9da72c10baa5a5bf8440e5d7/internal/framework/service/rulesets/schema.go#L104

   

   jacobbednarz committed Jan 31, 2024
   Configuration menu

   • View commit details
   • Copy full SHA for 8d7389b
   • Browse repository at this point

   Copy the full SHA

   8d7389b View commit details

   Browse the repository at this point in the history