resource/cloudflare_ruleset: improve unknown value handling

[jacobbednarz]

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 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.

A further improvement consideration here is if we don't have active uses for last_updated and version, consider removing them entirely from the schema. This will have downstream impacts but may clear up the output.

Closes #3082 as it is superseded by this PR.

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.

[github-actions[bot]]

changelog detected :white_check_mark:

[jacobbednarz]

need to dig into the test that handles preserving the references which is currently failing.

TF_ACC=1 go test ./internal/framework/service/rulesets -v -run "^TestAccCloudflareRuleset_PreserveRuleRefs" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareRuleset_PreserveRuleRefs
    resource_test.go:868: Step 2/12 error: Check failed: Check 1/2 error: cloudflare_ruleset.wvwfudwszz: Attribute "rules.0.ref" value: expected '4ac8be4f5f4944e4a15933e3823e2ef7' got 'ca9a6c1240a04c83898080e8b82252cb'
--- FAIL: TestAccCloudflareRuleset_PreserveRuleRefs (12.13s)
FAIL
FAIL	github.com/cloudflare/terraform-provider-cloudflare/internal/framework/service/rulesets	13.884s
FAIL
make: *** [testacc] Error 1

right now, this approach isn't feasible for all use cases of rulesets. we'll need to work with the internal teams in order to address some of the underlying architecture/design decisions to see why we can do here.

[pranit-p]

When will this feature be available?

[Nmishin]

I tested this change yesterday, and looks like everything works as expected - rules updated in CF and have correct ref (the same as id). And in the state the same. Need to deep into the test itself.

[github-actions[bot]]

Marking this pull request as stale due to 14 days of inactivity. This helps our maintainers find and focus on the active pull requests. If this pull request receives no comments in the next 7 days it will automatically be closed. Maintainers can also remove the lifecycle/stale label. If this pull request was automatically closed and you feel this pull request should be reopened, we encourage creating a new pull request linking back to this one for added context. Thank you!