Terraform-provider-cloudflare: PageRule priority not honored

Created on 2 Jan 2019  路  12Comments  路  Source: cloudflare/terraform-provider-cloudflare

I'm not sure if this is an issue with this provider or with the Cloudflare API itself...

Terraform Version

0.11.11

Affected Resource(s)

Please list the resources as a list, for example:

  • cloudflare_page_rule

Terraform Configuration Files

variable "zone" {}

resource "cloudflare_page_rule" "3" {
  priority = 3
  zone     = "${var.zone}"
  target   = "*${var.zone}/3/*"
  status   = "active"

  actions = {
    cache_level = "cache_everything"
  }
}

resource "cloudflare_page_rule" "2" {
  priority = 2
  zone     = "${var.zone}"
  target   = "*${var.zone}/2/*"
  status   = "active"

  actions = {
    cache_level = "cache_everything"
  }
}

resource "cloudflare_page_rule" "1" {
  priority = 1
  zone     = "${var.zone}"
  target   = "*${var.zone}/1/*"
  status   = "active"

  actions = {
    cache_level = "cache_everything"
  }
}

Debug Output

https://gist.github.com/mbrowne/77057416f35854ba5dfb647d68d5cdfb

Expected Behavior

Page rules created with priority values matching the config

Actual Behavior

Priorities out of order. Raw JSON result from Cloudflare API showing state after running terraform apply:

[
  {
    "id": "2ae3824bfd411d9a4f47d66aa86be72f",
    "targets": [
      {
        "target": "url",
        "constraint": {
          "operator": "matches",
          "value": "*matthewhbrowne.com/2/*"
        }
      }
    ],
    "actions": [{ "id": "cache_level", "value": "cache_everything" }],
    "priority": 3,
    "status": "active",
    "modified_on": "2019-01-02T18:16:22Z",
    "created_on": "2019-01-02T17:19:26Z"
  },
  {
    "id": "cb89c29a3a3e87c2599b4d996943e995",
    "targets": [
      {
        "target": "url",
        "constraint": {
          "operator": "matches",
          "value": "*matthewhbrowne.com/3/*"
        }
      }
    ],
    "actions": [{ "id": "cache_level", "value": "cache_everything" }],
    "priority": 2,
    "status": "active",
    "modified_on": "2019-01-02T18:16:21Z",
    "created_on": "2019-01-02T17:03:03Z"
  },
  {
    "id": "f0048a5e8175abd9e069e7a514574056",
    "targets": [
      {
        "target": "url",
        "constraint": {
          "operator": "matches",
          "value": "*matthewhbrowne.com/1/*"
        }
      }
    ],
    "actions": [{ "id": "cache_level", "value": "cache_everything" }],
    "priority": 1,
    "status": "active",
    "modified_on": "2019-01-02T18:16:22Z",
    "created_on": "2019-01-02T17:03:03Z"
  }
]

Steps to Reproduce

Easiest way to reproduce:

  1. terraform apply
  2. terraform destroy
  3. terraform apply

It might be necessary to run destroy and apply one more time before the issue shows up.

Important Factoids

I created a brand new Cloudflare account to test this to verify the issue

kinbug upstream technical-debt

Most helpful comment

Thanks for reporting this @mbrowne!

If I understand your issue correctly, this is a known incompatibility where there are conflicting ordering processes between the Terraform provider and Cloudflare API. I know we have raised this with our Cloudflare folks but for posterity, I'll post a summary of it here.

The main issue here is that Terraform uses a declarative load order for resources and doesn't apply any control flow _unless_ one of the following conditions are required:

  • A variable is referenced in a resource
  • A depends_on attribute is defined within a resource

In contrast the Cloudflare API will reorder the page rules as they come in to ensure that they are sequential and there aren't gaps. For example, if you send in a rule with a priority of 3 yet don't have enough rules, it will become rule 1.

The way we've worked around this internally is by using a combination of the two, depends_on (for Terraform) and priority to meet the Cloudflare API requirements. Using your example above, this is how we have solved it.

(diff)

@@ -5,6 +5,7 @@
   zone     = "${var.zone}"
   target   = "*${var.zone}/3/*"
   status   = "active"
+  depends_on = ["cloudflare_page_rule.2"]

   actions = {
     cache_level = "cache_everything"
@@ -16,6 +17,7 @@
   zone     = "${var.zone}"
   target   = "*${var.zone}/2/*"
   status   = "active"
+  depends_on = ["cloudflare_page_rule.1"]

   actions = {
     cache_level = "cache_everything"

(full)

variable "zone" {}

resource "cloudflare_page_rule" "3" {
  priority   = 3
  zone       = "${var.zone}"
  target     = "*${var.zone}/3/*"
  status     = "active"
  depends_on = ["cloudflare_page_rule.2"]

  actions = {
    cache_level = "cache_everything"
  }
}

resource "cloudflare_page_rule" "2" {
  priority   = 2
  zone       = "${var.zone}"
  target     = "*${var.zone}/2/*"
  status     = "active"
  depends_on = ["cloudflare_page_rule.1"]

  actions = {
    cache_level = "cache_everything"
  }
}

resource "cloudflare_page_rule" "1" {
  priority = 1
  zone     = "${var.zone}"
  target   = "*${var.zone}/1/*"
  status   = "active"

  actions = {
    cache_level = "cache_everything"
  }
}

This ensures that Terraform evaluates these resources in the order we want _and_ Cloudflare's priority stays correct and doesn't flap as mentioned in #65.

I know this is something that is being looked at from Cloudflare's side and I don't think the workaround in the provider would be manageable if implemented due to the additional overhead on every operation as you'd need to fetch all resources and re-evaluate everytime a change was initiated.

All 12 comments

Thanks for reporting this @mbrowne!

If I understand your issue correctly, this is a known incompatibility where there are conflicting ordering processes between the Terraform provider and Cloudflare API. I know we have raised this with our Cloudflare folks but for posterity, I'll post a summary of it here.

The main issue here is that Terraform uses a declarative load order for resources and doesn't apply any control flow _unless_ one of the following conditions are required:

  • A variable is referenced in a resource
  • A depends_on attribute is defined within a resource

In contrast the Cloudflare API will reorder the page rules as they come in to ensure that they are sequential and there aren't gaps. For example, if you send in a rule with a priority of 3 yet don't have enough rules, it will become rule 1.

The way we've worked around this internally is by using a combination of the two, depends_on (for Terraform) and priority to meet the Cloudflare API requirements. Using your example above, this is how we have solved it.

(diff)

@@ -5,6 +5,7 @@
   zone     = "${var.zone}"
   target   = "*${var.zone}/3/*"
   status   = "active"
+  depends_on = ["cloudflare_page_rule.2"]

   actions = {
     cache_level = "cache_everything"
@@ -16,6 +17,7 @@
   zone     = "${var.zone}"
   target   = "*${var.zone}/2/*"
   status   = "active"
+  depends_on = ["cloudflare_page_rule.1"]

   actions = {
     cache_level = "cache_everything"

(full)

variable "zone" {}

resource "cloudflare_page_rule" "3" {
  priority   = 3
  zone       = "${var.zone}"
  target     = "*${var.zone}/3/*"
  status     = "active"
  depends_on = ["cloudflare_page_rule.2"]

  actions = {
    cache_level = "cache_everything"
  }
}

resource "cloudflare_page_rule" "2" {
  priority   = 2
  zone       = "${var.zone}"
  target     = "*${var.zone}/2/*"
  status     = "active"
  depends_on = ["cloudflare_page_rule.1"]

  actions = {
    cache_level = "cache_everything"
  }
}

resource "cloudflare_page_rule" "1" {
  priority = 1
  zone     = "${var.zone}"
  target   = "*${var.zone}/1/*"
  status   = "active"

  actions = {
    cache_level = "cache_everything"
  }
}

This ensures that Terraform evaluates these resources in the order we want _and_ Cloudflare's priority stays correct and doesn't flap as mentioned in #65.

I know this is something that is being looked at from Cloudflare's side and I don't think the workaround in the provider would be manageable if implemented due to the additional overhead on every operation as you'd need to fetch all resources and re-evaluate everytime a change was initiated.

Thanks very much for suggesting this workaround. The first time I tried it on our real Cloudflare account, the ordering of the rules still didn't match the order in our Terraform file, but then I removed all the rules and recreated them and it seems to be working fine now (I did it twice to be sure). If this workaround will continue to be necessary for a long time, perhaps it should be mentioned in the documentation? It wouldn't be a big deal if different orderings had the same effect, but different priority order can actually cause rules to be applied differently...

I'm not sure why the ordering would have changed but I'm glad you got there in the end. If you've already got resources (of any type) created within Cloudflare, it's generally a better idea to import them instead. All of the Cloudflare Terraform resources should implement this and if they don't, it should be considered a bug. Taking this approach will ensure that what is already there will be composed exactly as-is without messing with existing properties. With page rules you would still have needed to do the depends_on/priority check but that is only for that one resource type.

Regarding documentation, I'm still holding out for the Cloudflare folks to get this resolved as the issue is with the API (or perhaps the underlying system) which means this will be impacting anyone who is using that. I'm assuming Cloudflare themselves don't hit this issue as they handle the ordering logic before sending it to the endpoint.

While I'm hesitant to share our workaround due to it's fragility and naivety, this is the script we use to automate the depends_on additions where ever a page rule priority is defined.

#!/usr/bin/env ruby

# The Cloudflare API requires contiguous priorities to be provided in the Terraform
# configuration for page rules. This script uses naive string matching to ensure that
# each line containing `priority = X` specifies numbers in descending order, and
# that each `depends_on` directive refers to the previous rule.

file = ARGV[0] or abort("Usage: #{$0} <file>")
input = File.readlines(file)
next_priority = 1
previous_rule = nil
output = input.reverse.map do |line|
  depends_on = /^(\s*depends_on *= *\[) *"[^"]*" *\] *$/.match(line)
  if depends_on
    line = "#{depends_on[1]} \"cloudflare_page_rule.#{previous_rule}\" ]"
  end
  priority = /^( *priority *= *)\d+ *$/.match(line)
  if priority
    line = "#{priority[1]}#{next_priority}"
    next_priority += 1
  end
  resource = /resource "cloudflare_page_rule" "([^"]+)"/.match(line)
  if resource
    previous_rule = resource[1]
  end
  line
end
File.open(file, "w+") do |f|
  f.puts(output.reverse)
end

It's not the prettiest thing out there but it has worked flawlessly in our cases however, like all things you find on the internet, YMMV.

Thanks. I actually already ran terraform import for each page rule (via a script I wrote, since it would be tedious to do it manually for all 20 of our page rules), so I'm not sure what was going on. But of course things will be much easier once we're fully migrated to Terraform.

Thanks for sharing the workaround. Is there a way to do something similar with modules? I have several modules which define page rules and I use some of them multiple times in my project. Modules are not 3rd party, so I can modify them.

I don't think it would be possible to use a workaround like this and use modules as the ordering isn't reusable. Happy to be proved wrong here but I can't see a way where that would work and get any benefit from the modules.

Well you could create a separate module for each page rule and pass in the priority number as a parameter. And rather than trying to automate the whole thing you could write a test to just verify that the human-entered numbers were entered correctly in sequential order. That's what I ended up doing for validation...wrote the test script in Go since it has the official HCL library (just the raw config values...it doesn't do variable interpolation like Terraform does internally, but that's irrelevant in the case of hard-coded priority numbers).

@mbrowne I think the issue would still be present as you need the priority and the depends_on meta attribute which requires context of what should be applied. At that point, modules seem pretty redundant as you would be rewriting most of it when you reference it.

Thanks for your answers. Because of my time constrains, I ended up with a quite fragile solution which basically ignores priority. It "solves" the issue from https://github.com/terraform-providers/terraform-provider-cloudflare/issues/65 and allows me to set priority directly in Cloudflare. After setting the correct priority manually in Cloudflare I can modify page rules without modifying priority via terraform.

I added lifecycle with ignore_changes to my every rule:

resource "cloudflare_page_rule" "name" {
  # ...

  lifecycle {
    # Ignore priority. We can't modify it via Terraform, at the moment,
    # so priority should be set in Cloudflare manually.
    # Ref: https://github.com/terraform-providers/terraform-provider-cloudflare/issues/187
    ignore_changes = "priority"
  }
}

It kinda works for me because I only manage 3 zones and I'm not going to change page rules often, but I do not recommend this workaround. if Cloudflare decide to change the way they handle priority in API requests like this, you will highly likely have your page rules silently broken (it's not documented anywhere, so It seems to be safe to assume that backward compatibility is not guaranteed).

hey there @mbrowne, not sure if this can help but we solved the issue by using a configuration like this

variable "pagerule_targets" {
  type = "map"
  default = {
    // special URIs
    "822" = "test.domain.com/ui/index.html"
    "821" = "test.domain.com/ui/"

    // wildcard paths
    "73" = "cms.domain.com/*html"
    "72" = "cms.domain.com/etc/*"

    // defaults
    "12" = "test.domain.com/*"
    "11" = "cms.domain.com/*"
  }


variable "pagerule_actions" {
  type = "map"
  default = {
    // special URIs
    "822" = {
      always_use_https = "true"
      cache_level = "cache_everything"
      edge_cache_ttl = "10"
    }
    "821" = {
      always_use_https = "true"
      cache_level = "cache_everything"
      edge_cache_ttl = "10"
    }
    // wildcard paths
    "73" = {
      cache_level = "cache_everything"
      edge_cache_ttl = "7200"
    }
    "72" = {
      cache_level = "cache_everything"
      edge_cache_ttl = "2419200"
    // defaults
    "12" = {
      always_use_https = "true"
      cache_level = "cache_everything"
      edge_cache_ttl = "86400"
    }
    "11" = {
      always_use_https = "true"
      cache_level = "cache_everything"
      edge_cache_ttl = "86400"
    }
  }


resource "cloudflare_page_rule" "rules" {
   count = "${length(keys(var.pagerule_targets))}"
   lifecycle {
    create_before_destroy = true
  }

  zone_id = "${cloudflare_zone.zone.id}"
  target = "${var.pagerule_targets[element(keys(var.pagerule_targets),count.index)]}"
  actions = "${list(var.pagerule_actions[element(keys(var.pagerule_targets),count.index)])}"
  // because priority in the web UI is ordered in the inverse compared to priority (the higher number in the UI, the lower priority)
  // we use the key value with a numbered convention
  //  * 10-19: defaults, lowest priority, more general settings
  //  * 30-79: special wildcards for URIs
  //  * 800+: special single URIs with peculiar settings
  // because the ordering is preserved when using lexicographica listing from the keys
  // we can use the index of the array
  priority = "${count.index + 1}"
}

What happens with this is the ordering does not change when applying, next time you will only have changes related to rules insertion/deletion

the issue is still happening for us even on latest TF and CF provider versions. the workaround we have for now is to just run the apply on the page rules module multiple times with -parallelism=1, empricially we see that all collisions are resolved and we achieve correct order of the page rules after 4 runs on a set of 40 rules. does anyone know if cloudflare is planning to provide a fix for this issue?

This isn鈥檛 something the provider can fix as mentioned earlier in this issue. You鈥檒l need to use one of the workarounds in this thread.

Cloudflare have mentioned this isn鈥檛 something they will be fixing as it is more likely they would replace the mechanism used instead.

I鈥檓 going to lock this issue as nothing will change from the provider side however the workarounds should stay visible to others.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

brucedvgw picture brucedvgw  路  3Comments

AgrimPrasad picture AgrimPrasad  路  3Comments

DingGGu picture DingGGu  路  3Comments

azhurbilo picture azhurbilo  路  6Comments

jacobbednarz picture jacobbednarz  路  5Comments