resource/cloudflare_ruleset: fix ruleset category overrides in DDoS L7 phase by allowing the provider to omit the enabled flag, as required by Cloudflare's API

[Quentin-M]

In the DDoS L7 phase, category overrides do not support the enabled flag which must therefore be omitted. This is pretty closely related to #1405.

Terraform example

resource "cloudflare_ruleset" "example" {
  zone_id     = cloudflare_zone.example.id
  name        = "zone_managed_ddos"
  description = "Zone-level Cloudflare DDoS Managed Rulesets"
  kind        = "zone"
  phase       = "ddos_l7"

  // DDoS L7 ruleset
  rules {
    action      = "execute"
    description = "Override DDoS L7 Ruleset"
    expression  = "true"
    enabled     = true

    action_parameters {
      id = "4d21379b4f9f4bb088e0729962c8b3cf"

      overrides {
        // Enable all unusual request rules
        categories {
          category = "unusual-requests"
          action   = "challenge"
        }

API Request (sample) & error before this PR

                        "categories": [
                          {
                            "action": "challenge",
                            "category": "unusual-requests",
                            "enabled": false
                          },
                          {
                            "action": "challenge",
                            "category": "botnets",
                            "enabled": false
                          }
                        ],

Leading to the following 400 Bad Request error returned by the Cloudflare API (doubled, because two categories):

    DDoS L7 overrides cannot use the enabled field, DDoS L7 overrides cannot use the enabled field
    DDoS L7 overrides cannot use the enabled field, DDoS L7 overrides cannot use the enabled field

API Request (sample) after this PR

     "categories": [
      {
       "category": "unusual-requests",
       "action": "challenge"
      },
      {
       "category": "botnets",
       "action": "challenge"
      }
     ],

Notes

• This PR requires the following cloudflare-go schema: fix: make Enabled within RulesetRuleActionParametersCategories optional for DDoS L7 phase cloudflare-go#824
• This PR also includes a second commit to fix a panic encountered in resourceCloudflareRulesetStateUpgradeV0ToV1 during testing

[github-actions]

Oops! It looks like no changelog entry is attached to this PR. Please include a release note as described in https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/docs/changelog-process.md.

Example:

```release-note:TYPE
Release note
```

If you do not require a release note to be included, please add the workflow/skip-changelog-entry label.

[Quentin-M]

Resource creation now works appropriately, however, it appears that "enabled": false still gets written to the Terraform state despite being skipped in buildRulesetRulesFromResource, and never returned by the Cloudflare API - which makes updates break as the boolean ends up getting into the request during updates. I am not 100% sure how it gets written to state at this point.

---

Launching a debugger against the following state:

                        "categories": [
                          {
                            "action": "challenge",
                            "category": "unusual-requests"
                          },
                          {
                            "action": "block",
                            "category": "botnets"
                          }
                        ],

Results in resourceCloudflareRulesetUpdate getting a d.state.Attributes with rules.0.action_parameters.0.overrides.0.categories.0.enabled -> "false" rather than something representing nil, which subsequently will lead buildRulesetRulesFromResource to append the boolean wrongly into our rules object for submission against UpdateZoneRuleset. Same goes when setting enabled: null within the same, and same behavior can be observed when using d.RawState() regardless of whether the state contains false or null.

Ultimately, I feel like trying to use a tristate TypeBool with Terraform is bound to failure as cty/TF don't appear to be able to write/read the state file while maintaining the tri-state status. This leads to Create working properly but TF writing enabled into the state, and reading enabled: false regardless of whether the key is present in the state or not, and regardless of whether it's present but written as null.. which leads to Update breaking. Given DDoS_L7 is a special case of ruleset, I am not sure we can safely assume we can omit the field in the ruleset API calls if enabled: false... We might need a special case defined (ugh), or another bit..

I won't get too lost into writing some kind of fix implementation given I am not super-duper experienced writing providers, I don't have a solution that doesn't sound gross, there probably is a different issue/approach I'm not seeing).

[jacobbednarz]

FWIW, this isn’t the first time we’ve tangled with tristate booleans…I have a SDK bug open to address exactly this (hashicorp/terraform-plugin-sdk#817)

[Quentin-M]

Yeah, I've been reading that thread while trying to fix up the provider, looks fundamentally broken. I am not 100% sure how the SDK/cty ends up writing a *Bool{nil} as bool{false} but it did not seem like a great use of time to dive deep into it, I just assumed TypeBool is just a two-state bool and therefore we're losing information on state save/read.. making it basically impossible to determine later on whether we need to include it in our API requests or not, and thus breaking ddos_l7 updates. From my past experience facing Terraform core issues, it is most likely going to be significantly faster and probably much more robust to fix the problem within terraform-provider-cloudflare by patching up a TypeString<->*bool conversion between the state and the cloudflare-go API.

[jacobbednarz]

please remove this; it doesn't actually help the upgrade process. there was an update to the migrate in another PR.

[jacobbednarz]

looks like you'll need to update from master to resolve the conflicts. additionally, i recently landed some helpers (cloudflare/cloudflare-go#825) to swap between types easily. you'll probably want this for BoolPtr like we've done in other places recently.

[github-actions]

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!

[github-actions]

This PR was closed because it has been stalled for 7 days with no activity.