Please document how to upgrade cloudflare_ruleset past 4.1.0

[hp197]

Confirmation

• My issue isn't already found on the issue tracker.
• I have replicated my issue using the latest version of the provider and it is still present.

Terraform and Cloudflare provider version

4.6.0

Affected resource(s)

cloudflare_ruleset

Terraform configuration files

Gist to tfstate file (v4.1.0): https://gist.github.com/hp197/82dfda7e3743e4bbad8ccea5734b6bf7

Link to debug output

https://gist.github.com/hp197/c1d13c72e861f60d7263282f566b5613

Panic output

/ See debug log /

Expected output

Something working

Actual output

Non working

Steps to reproduce

1. Having a older (4.1.0) ruleset.
2. Upgrade terraform provider to 4.6.0
3. terraform state rm old ruleset
4. import existing ruleset

Additional factoids

Please update the documentation on how to get from from v4.1.0 onwards with rulesets.
The warning in the v4.2.0 wont cut it here, it's too brief.

Searched in https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/docs/guides/version-4-upgrade.md
But couldn't find any info there.

We are CF business/premium plan user.
I can contact our account manager if that would help for you to get extra prio.

References

Might be related to #2452

[github-actions]

Community Note

Voting for Prioritization

• Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

• If you are interested in working on this issue, please leave a comment.
• If this would be your first contribution, please review the contribution guide.

[jacobbednarz]

the v4.2.0 release notes are all that is required to go from 4.1.0 to 4.2.0. this is a process I've walked through for my personal configuration as well as two internal zones without issues so there is either something about your configuration that isn't accounted for here or an edge case that wasn't considered.

can you provide a specific and minimal reproduction test case to what you are seeing to diagnose? the general steps included in the reproduction steps here are not enough to trigger the issue.

[hp197]

@jacobbednarz can you give me an email from your CF account to tps-infra-cloud-and-hosting [at] mediahuis . (be)lgium

I would be happy to share the full statefile once I've verified you're a CF employee ;-)

[jacobbednarz]

i don't need your state file; i'm after the reproduction steps which should work irrespective of your state file as the steps will build the reproduction case.

[hp197]

Let me try to give you pointers how we got here, not sure if you'll have the same outcome as this can very well also happen because the domain was once created against an older CF api with different behaivior.

Following code is a exact copy, except the domain names.
Domain was created 1 Oct 2019 against TF v0.12.9 and CF plugin version v1.18.
The ruleset was created in CF using plugin v3.35.0 (thank got CI/CD pipelines for historical traceability :))

Our TF part:

resource "cloudflare_ruleset" domain_tld-origin-ruleset{
  zone_id     = cloudflare_zone.domain_tld.id
  name        = "Origin ruleset (Gitlab managed)"
  description = "Origin ruleset (Gitlab managed)"
  kind        = "zone"
  phase       = "http_request_origin"
  rules {
    expression = "(http.host matches \"(testm|test).domain.tld\" and http.request.uri matches \"^/path\" and not http.request.uri matches \"^/path/api\")"
    description = "Send /path traffic to test s3 bucket website"
    enabled     = true
    action = "route"
    action_parameters {
      host_header = "test-bucket.s3-website-eu-west-1.amazonaws.com"
      
      origin {
        host = "test-application.domain.tld"
        
          port = 0
        
      }
      
      
    }
  }

  rules {
    expression = "(http.host matches \"(previewm|preview).domain.tld\" and http.request.uri matches \"^/path\" and not http.request.uri matches \"^/path/api\")"
    description = "Send /path traffic to acceptance s3 bucket website"
    enabled     = true
    action = "route"
    action_parameters {
      host_header = "acc-bucket.s3-website-eu-west-1.amazonaws.com"
      
      origin {
        host = "acc-application.domain.tld"
        
          port = 0
        
      }
      
      
    }
  }

  rules {
    expression = "(http.host matches \"(www.|m.).domain.tld\" and http.request.uri matches \"^/path\" and not http.request.uri matches \"^/path/api\")"
    description = "Send /path traffic to production s3 bucket website"
    enabled     = true
    action = "route"
    action_parameters {
      host_header = "prod-bucket.s3-website-eu-west-1.amazonaws.com"
      
      origin {
        host = "prod-application.domain.tld"
        
          port = 0
        
      }
      
      
    }
  }

}

Do you need more?

[jacobbednarz]

other than port = 0 in your rule, this looks to be working for me on:

terraform 1.4.6
terraform-provider-cloudflare e5a05e1

did you follow the guidance on the upgrade guide about removing from state and reimporting to make sure you were getting the full benefits?

[hp197]

Yes, see my report:

Steps to reproduce
Having a older (4.1.0) ruleset.
Upgrade terraform provider to 4.6.0
terraform state rm old ruleset
import existing ruleset

Note that we have around 6000 domains in CF (and a few 100 premiums).
While not all of them have rulesets, these kind of hard to automate operations aren't very nice to some of your customers ;-)

[tutunak]

Agree with previous comment, this broken change was introduced in a minor release. Documentation says nothing about this, only a warning in release notes.

[jacobbednarz]

While not all of them have rulesets, these kind of hard to automate operations aren't very nice to some of your customers ;-)

the choice to migrate wasn't made lightly as not only was this problematic for customers but it was quite an engineering effort on our side too. the migration involved swapping the internals of our provider to an experimental backend which no other provider of our size had yet attempted. you can see the full context in #2170 and #2271.

this broken change was introduced in a minor release

correct - and this was due to the broken nature of the resource prior to this change (see the PR links above). if you were using the resource in older versions, you were already running a broken resource that would potentially break if you provided it the right combination of values and there was no way to manually recover due to the internal. additionally at the time, 4.1.0 had < 10 active installations, all of them internal to Cloudflare teams so the remainder of traffic should have got a newer version out of the box unless they specifically asked for that version.

as for the issue here, i'm still not sure why you have port = 0 and that is the only thing here that I can see wouldn't work cleanly. other than that, my test case is working from 4.0 -> master from earlier today. without more information or pointers, i don't really have much else to go off here. @tutunak what does your reproduction case look like for this issue?

[tutunak]

@jacobbednarz it's not possible to upgrade from 3.35 provider directly to 4.6. Only through 3.35 -> 4.1 -> 4.6. Existing issue with this problem #2452. I managed it in that way

1. Upgrade 3.35
2. Upgrade 4.1
3. remove rulesets from state
4. Upgrade 4.6
5. import rulesets

Fortunately, I have only 22 rulesets across all zones, and everything, with checking, manipulating state etc. took only one work day

[marcel-puchol-jt]

The process described in #2464 (comment) worked in order to update the version. However, when I made any change on an existing rule, seems that it wants to update all the rule IDs:

  # module.zone.cloudflare_ruleset.ratelimiting_rules["test"] will be updated in-place
  ~ resource "cloudflare_ruleset" "ratelimiting_rules" {
        id          = "34567186311149fda17dc7721e248552"
        name        = "jt-stg-platform-ruleset-zone-ratelimiting-rules"
        # (4 unchanged attributes hidden)

      ~ rules {
          ~ id           = "efa92f51a42f410cb08797cda051924c" -> (known after apply)
          + last_updated = (known after apply)
          ~ version      = "8" -> (known after apply)
            # (4 unchanged attributes hidden)

            # (1 unchanged block hidden)
        }
      ~ rules {
          ~ expression   = "{expr-from}" -> "{expr-to}"
          ~ id           = "13c19992b24e474e8c8d54854d08fd33" -> (known after apply)
          + last_updated = (known after apply)
          ~ version      = "4" -> (known after apply)
            # (3 unchanged attributes hidden)

            # (1 unchanged block hidden)
        }
      ~ rules {
          ~ id           = "0f6de47a26c04b2db8343916edc74b66" -> (known after apply)
          + last_updated = (known after apply)
          ~ version      = "10" -> (known after apply)
            # (4 unchanged attributes hidden)

            # (1 unchanged block hidden)
        }
      ~ rules {
          ~ id           = "3bd0f57149b44b7cb9c759db162c25be" -> (known after apply)
          + last_updated = (known after apply)
          ~ version      = "1" -> (known after apply)
            # (4 unchanged attributes hidden)

            # (1 unchanged block hidden)
        }
      ~ rules {
          ~ id           = "5692fe6ee32448b7b408c16188e5cef3" -> (known after apply)
          + last_updated = (known after apply)
          ~ version      = "3" -> (known after apply)
            # (4 unchanged attributes hidden)

            # (1 unchanged block hidden)
        }
    }

which seems the same problem present in #2172 . Seems that changes done in #2271 made that this problem re-appear (also the comment It's not 100% the same so we'll need to do a bit of investigation how to best approach it. helped to detect this regression problem).

[jacobbednarz]

@marcel-puchol-jt that output looks correct since those fields are now marked as computed. the tricky bit here is that you are using a module too; not just the resource directly which may store the resource with different values through abstraction however, it looks as expected.

as for the investigation, that was completed and it was the same in the end. the exact test suite used in the initial PR was used to determine the correctness following the upgrade.

[marcel-puchol-jt]

@marcel-puchol-jt that output looks correct since those fields are now marked as computed. the tricky bit here is that you are using a module too; not just the resource directly which may store the resource with different values through abstraction however, it looks as expected.

as for the investigation, that was completed and it was the same in the end. the exact test suite used in the initial PR was used to determine the correctness following the upgrade.

@jacobbednarz the output represents a ruleset in which only one rule is changed. In this case, the IDs should not change when a single rule is updated. Even for the changed rule, the ID should remain the same, as only the expression in changing. In fact, I tested to apply the previous change, and all IDs remain (so the plan is not accurate on showing that IDs are going to change) for those unchanged rules. However, the ID in which the expression changes, it really change the ID for this rule (which shouldn't, or at least, this was not the behaviour before 4.1.0).

Finally, I don't understand the tricky part related to using a module. Which is the problem on using the code inside a module?

[jacobbednarz]

the plan is accurate. it is not telling you the ID value is changing but that something inside the structure has changed and because the value is optional and computed, it will be calculated for that run but it doesn't yet know the final value. once the last_updated at is saved to state that difference won't be shown. if you don't have any other values changing in the inner structure, this output is not shown but still runs behind the scenes.

regarding using modules: a module is an abstraction over one or more resources and and these abstractions can hold their own logic bugs which are outside of the provider and not able to be diagnosed. for example, in the module you have handling for your desired defaults if they are not provided. if you typo the value, it may miss the key and not be set at all which is something that won't show up in debug logs like this since it is a client side bug in your definition.

[jacobbednarz]

#2481 pulls the release note into the upgrade guide for anyone who is performing the upgrade.

[github-actions]

This functionality has been released in v4.7.0 of the Terraform Cloudflare Provider.

Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!