terraform-provider-cloudflare icon indicating copy to clipboard operation
terraform-provider-cloudflare copied to clipboard

Drift in `cloudflare_ruleset` for `http_request_firewall_custom` phase for computed args `id`, `last_updated`, `ref`, `version`

Open nitrocode opened this issue 1 year ago • 46 comments

Confirmation

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

Terraform and Cloudflare provider version

Terraform 1.5.5 Cloudflare provider 4.12.0 latest

Affected resource(s)

  • cloudflare_ruleset

Terraform configuration files

resource "cloudflare_ruleset" "custom" {
  kind    = "zone"
  name    = "default"
  phase   = "http_request_firewall_custom"
  zone_id = local.zone_id

  # ... 30 rules ...

  # e.g. pre-existing rule
  rules {
    enabled     = true
    action      = "log"
    description = "<snip>"
    expression  = <<-EOT
              <snip>
            EOT
  }

  # added additional rule which caused a drift. could be any rule
  rules {
    enabled     = true
    action      = "log"
    description = "<snip>"
    expression  = <<-EOT
              any(http.request.headers["custom-header"][*] matches ".?")
            EOT
  }

Link to debug output

none

Panic output

N/A

Expected output

No perma drift

Actual output

Perma drift

Steps to reproduce

  1. Add a new rule or modify an existing one
  2. terraform plan

Additional factoids

This happened after we bumped the provider from 3.x to 4.x

We deleted the cloudflare_ruleset and reimported.

If we do not make a change, we see No changes as expected. Any change we make, and we see a bunch of drift between rules, all affecting arguments that we do not set such as id and version

We have 30 something rules and we see these computed arguments changing for every rule.

in 4.12.0 cloudflare provider

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cloudflare_ruleset.custom will be updated in-place
  ~ resource "cloudflare_ruleset" "custom" {
        id      = "<snip>"
        name    = "default"
        # (3 unchanged attributes hidden)

      ~ rules {
          ~ id           = "<snip>" -> (known after apply)
          ~ last_updated = "2023-08-18 16:36:15.176813 +0000 UTC" -> (known after apply)
          ~ ref          = "<snip>" -> (known after apply)
          ~ version      = "3" -> (known after apply)
            # (4 unchanged attributes hidden)
        }
      ~ rules {
          ~ id           = "<snip>" -> (known after apply)
          ~ last_updated = "2023-08-18 16:36:15.176813 +0000 UTC" -> (known after apply)
          ~ ref          = "<snip>" -> (known after apply)
          ~ version      = "15" -> (known after apply)
            # (4 unchanged attributes hidden)

          ~ action_parameters {
              + version  = (known after apply)
                # (3 unchanged attributes hidden)
            }

            # (1 unchanged block hidden)
        }
      + rules {
          + action       = "log"
          + description  = "log traffic with headers"
          + enabled      = true
          + expression   = <<-EOT
                any(http.request.headers["custom-header"][*] matches ".?")
            EOT
          + id           = (known after apply)
          + last_updated = (known after apply)
          + ref          = (known after apply)
          + version      = (known after apply)
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

In 4.7.1 cloudflare provider

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

          ~ action_parameters {
              + version  = (known after apply)
                # (1 unchanged attribute hidden)
            }

            # (1 unchanged block hidden)
        }
      + rules {
          + action       = "log"
          + description  = "log traffic with headers"
          + enabled      = true
          + expression   = <<-EOT
                any(http.request.headers["custom-header"][*] matches ".?")
            EOT
          + id           = (known after apply)
          + last_updated = (known after apply)
          + ref          = (known after apply)
          + version      = (known after apply)
        }

We also cannot set a lifecycle ignore rule because this does not work for us

  lifecycle {
    ignore_changes = [
      "rules.*.id",
      "rules.*.version",
      "rules.*.last_updated",
      "rules.*.ref",
      # "rules[*].id",
      # "rules.id",
    ]
  }

I'm told this might work but it's not scaleable. We have over 30 rules. I tried this and still see the drift!

  lifecycle {
    ignore_changes = [
      rules[0].id,
      rules[0].version,
      rules[0].last_updated,
      rules[0].ref,
      rules[1].id,
      rules[1].version,
      rules[1].last_updated,
      rules[1].ref,
      rules[2].id,
      rules[2].version,
      rules[2].last_updated,
      rules[2].ref,
      rules[3].id,
      rules[3].version,
      rules[3].last_updated,
      rules[3].ref,
      # ...
      rules[n].id,
      rules[n].version,
      rules[n].last_updated,
      rules[n].ref,
    ]
  }

We did not see this issue in 3.x cloudflare provider

I don't see any DiffSuppressFunc for any of the above drifted computed arguments. Unsure if the drift needs to be suppressed or if it needs to be investigated.

https://github.com/cloudflare/terraform-provider-cloudflare/blob/adbc0accaafe0df39e78fcfd61d092573e13a25d/internal/framework/service/rulesets/resource.go

References

Related issues

  • https://github.com/cloudflare/terraform-provider-cloudflare/issues/2606
  • https://github.com/cloudflare/terraform-provider-cloudflare/issues/2445

Related pr

  • https://github.com/cloudflare/terraform-provider-cloudflare/pull/2511 released in 4.8.0

From change log

BREAKING CHANGES:

  • resource/cloudflare_ruleset: Prevent the rule ID, version and last updated attributes from being set (https://github.com/cloudflare/terraform-provider-cloudflare/issues/2511)

nitrocode avatar Aug 16 '23 22:08 nitrocode

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.

github-actions[bot] avatar Aug 16 '23 22:08 github-actions[bot]

Thank you for reporting this issue! For maintainers to dig into issues it is required that all issues include the entirety of TF_LOG=DEBUG output to be provided. The only parts that should be redacted are your user credentials in the X-Auth-Key, X-Auth-Email and Authorization HTTP headers. Details such as zone or account identifiers are not considered sensitive but can be redacted if you are very cautious. This log file provides additional context from Terraform, the provider and the Cloudflare API that helps in debugging issues. Without it, maintainers are very limited in what they can do and may hamper diagnosis efforts.

This issue has been marked with triage/needs-information and is unlikely to receive maintainer attention until the log file is provided making this a complete bug report.

github-actions[bot] avatar Aug 16 '23 22:08 github-actions[bot]

I can provide this, but I'm not comfortable posting that here in a public forum. I'm an enterprise customer, can I create a case?

Rules, IPs, etc. would take a long while to redact.

Log uploaded to case id: #2901904

jessegonzalez-life360 avatar Aug 19 '23 06:08 jessegonzalez-life360

Thanks for the lifecycle hint. It can help with perma drift until a proper solution is identified.

jessegonzalez-life360 avatar Aug 25 '23 21:08 jessegonzalez-life360

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

github-actions[bot] avatar Sep 25 '23 00:09 github-actions[bot]

Marking this issue as unstale

nitrocode avatar Sep 25 '23 14:09 nitrocode

cc: @jacobbednarz

nitrocode avatar Sep 25 '23 14:09 nitrocode

Confirming this bug as well, any time there's a change to 1 rule, all the other rules in the ruleset get their mentioned fields updated in diff

image

DDuarte avatar Oct 10 '23 08:10 DDuarte

Thanks for confirming @DDuarte.

If you get a chance @jacobbednarz please take a look at this.

It's hard enough that all of our rules have to be in a single resource and it's even harder to parse through the noisy plan to see the real differences without having to pipe our plans through some post-processing.

nitrocode avatar Oct 11 '23 22:10 nitrocode

@DDuarte and @nitrocode if you define a unique ref for the rules block, you should no longer see permanent drift.

Edit: Spoke too soon, the issue persists.

jessegonzalez-life360 avatar Nov 01 '23 17:11 jessegonzalez-life360

+1 dealing with this issue. given that we're migrating from multiple deprecated cloudflare_firewall_rules into a single new cloudflare_ruleset, this makes the plan outputs become really hard to read. currently using a list of 300 attributes being ignored by lifecycle rules 😅

gabrielqs avatar Nov 14 '23 15:11 gabrielqs

I tried to fix this something like:

    lifecycle {
      ignore_changes = ["rules[*].ref", "rules[*].last_updated"]
    }

However there is nothing like [*] in Terraform syntax, so this couldn't work.

vojkny avatar Nov 27 '23 08:11 vojkny

@gabrielqs could you share what you use? I tried setting the ignore_changes and I could not get it to work.

nitrocode avatar Nov 28 '23 05:11 nitrocode

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

github-actions[bot] avatar Dec 29 '23 00:12 github-actions[bot]

This is still a problem

gabrielqs avatar Dec 29 '23 06:12 gabrielqs

@nitrocode sorry, only seeing your message now. here's what we had to do to get around this:

  lifecycle {
    ignore_changes = [
      rules[0].id,
      rules[0].version,
      rules[0].last_updated,
      rules[0].ref,
      rules[0].action_parameters[0].version,
      rules[1].id,
      rules[1].version,
      rules[1].last_updated,
      rules[1].ref,
      rules[1].action_parameters[0].version,
      rules[2].id,
      rules[2].version,
      rules[2].last_updated,
      rules[2].ref,
      rules[2].action_parameters[0].version,
      ...

It is terrible, and we need to remind ourselves to add this every time we add a new rule, but at least the drift doesn't show up on every plan

gabrielqs avatar Dec 29 '23 13:12 gabrielqs

I'm happy it works for you but that doesn't work for us. We still see the drift even with explicitly specifying each individual attributes per rule in ignore_changes.

Since these rules are all in a single terraform resource and we cannot get the plan in a reviewable state, we may have to consider moving these rules out of terraform and having them managed with a script that supplants all the rules upon pr merge.

cc @jacobbednarz friendly ping to get cloudflare's feedback here. This is the 2nd highest voted issue and ideally we should be able to manage and review changes within terraform without so much noise.

nitrocode avatar Dec 30 '23 16:12 nitrocode

I am providing a full TF_LOG=DEBUG for 2 scenarios, both of which are problematic when working with the Cloudflare ruleset resource. Please note that while there is a workaround (manually verifying them), this is a major issue for my team with a hundred rules in a zone and could result in a bad rule change making its way into the WAF.

Scenarios:

  1. A rule is added in the middle of existing rules. Note that it looks like rules are drastically changing and a 'new' rule is being added to the bottom of the list. However, that 'new' rule is just an existing rule at the end. The rules are correctly being shown, but it is extremely confusing. When there are more than a couple rules, it becomes almost impossible to parse through the changes to make sure no mistakes were made. Also, all rules show changes to id, ref, version, etc, further complicating the issue. Link to gist

  2. A rule is modified (ie name, expression, etc) but no other changes are made. This results in the plan showing all rules having changed. Again, if there are more than a couple rules, it makes it almost impossible to verify that the changes desired are what is shown in the plan. Link to gist

troymjones avatar Jan 22 '24 17:01 troymjones

Please respond @jacobbednarz . This is the second highest voted issue now.

https://github.com/cloudflare/terraform-provider-cloudflare/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc

nitrocode avatar Jan 22 '24 21:01 nitrocode

@jacobbednarz , Hello,is this bug expected to be fixed?

saintsinnerr avatar Jan 23 '24 15:01 saintsinnerr

When will this feature be available?

pranit-p avatar Feb 13 '24 05:02 pranit-p

I tried to fix this something like:

    lifecycle {
      ignore_changes = ["rules[*].ref", "rules[*].last_updated"]
    }

However there is nothing like [*] in Terraform syntax, so this couldn't work.

Related https://github.com/hashicorp/terraform/issues/24188

eephillip avatar Feb 13 '24 16:02 eephillip

Yes @eephillip

But, this code is not workable

I tried to fix this something like:

    lifecycle {
      ignore_changes = ["rules[*].ref", "rules[*].last_updated"]
    }

However there is nothing like [*] in Terraform syntax, so this couldn't work.

Related hashicorp/terraform#24188

pranit-p avatar Feb 13 '24 17:02 pranit-p

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

github-actions[bot] avatar Mar 15 '24 00:03 github-actions[bot]

this is still an issue

fcoelho avatar Mar 15 '24 00:03 fcoelho

Still an issue

gabrielqs avatar Mar 15 '24 01:03 gabrielqs

Still an Issue

Anticmos avatar Mar 26 '24 16:03 Anticmos

@nitrocode can you provide the TF_LOG=DEBUG output so that they can remove needs-information tag by chance?

Anticmos avatar Mar 26 '24 16:03 Anticmos

@Anticmos please see this comment where the debug log was attached to an internal cloudflare case #2901904 (see https://github.com/cloudflare/terraform-provider-cloudflare/issues/2690#issuecomment-1684856165 in Aug 2023)

nitrocode avatar Mar 26 '24 20:03 nitrocode

@Anticmos feel free to add another one. 😄

nitrocode avatar Mar 26 '24 20:03 nitrocode