url_map and dynamic "route_rules" ordering issue

wouterdegeus commented 2 years ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
If you are interested in working on this issue or have submitted a pull request, please leave a comment.
If an issue is assigned to the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

Terraform v1.0.11 on darwin_amd64

provider registry.terraform.io/hashicorp/google v4.7.0
provider registry.terraform.io/hashicorp/google-beta v4.7.0
provider registry.terraform.io/hashicorp/null v3.1.0

Affected Resource(s)

google_compute_url_map

Terraform Configuration Files

locals {
  backends = {
    default = {
      fqdn = "test.com"
      iap = true
      regex = "/.*"
    }
  }
}
resource "google_compute_url_map" "map" {
  name            = "map"
  default_service = google_compute_backend_service.bs["default"].self_link

  dynamic "host_rule" {
    for_each = try(local.backends, {})
    iterator = host
    content {
      hosts        = [host.value.fqdn]
      path_matcher = host.key
    }
  }

  dynamic "path_matcher" {
    for_each = local.backends
    iterator = backend
    content {
      name            = format("%s", backend.key)
      default_service = google_compute_backend_service.bs[backend.key].self_link

      dynamic "route_rules" {
        for_each = toset(try(backend.value.iap, false) ? [format("%s-wl", backend.key), format("%s-iap", backend.key)] : [format("%s", backend.key)])
        iterator = rr
        content {
          priority = index(sort(keys(local.backends)), backend.key) + (substr(rr.key, -4, -1) == "-iap" ? 200 : 100)
          service  = substr(rr.key, -4, -1) == "-iap" ? google_compute_backend_service.bs_iap[backend.key].self_link : google_compute_backend_service.bs[backend.key].self_link
          match_rules {
            regex_match = backend.value.regex
            dynamic "header_matches" {
              for_each = substr(rr.key, -3, -1) == "-wl" ? [1] :[]
              content {
                header_name = "x-testheader"
                exact_match = "true"
              }
            }
          }
        }
      }
    }
  }
}

In this case I was trying to create a url_map with multiple routing routes that have different priorities and matching rules, in this case one with a header match and one without. Both using a regex_match.

Expected Behavior

I expected the URL Map to be created properly, with multiple routing rules depending on the iap flag.

Actual Behavior

Plan output looked sane, multiple routing rules with different priorities are shown.

However, the apply errors with:

Error: Error updating UrlMap "projects/myproject/global/urlMaps/map": googleapi: Error 400: Invalid value for field 'resource.pathMatchers[0].routeRules[1].priority': '100'. Within a pathMatcher, a route rule must have a priority that's higher than the previous route rule's priority, invalid

Since I can not influence the order in which for_each iterates over the set, this can not be fixed easily by the user. The provider should do the sensible thing here and order the routing rules by priority as demanded.

Steps to Reproduce

terraform apply

Important Factoids

References

0000

b/312911334

rileykarson commented 2 years ago

Note: This may be fairly complicated- ordering objects in the provider correctly when they're not ordered that way in config pretty easily causes Terraform to have a permadiff

fancybear-dev commented 1 year ago

Has anyone managed to get this working with the current provider lacking the required functionality? I really need this, but every workaround thus far has been a dead end. Thought I'd at least ask here to potentially save a lot of time.

Honestly I don't think there is a viable workaround, also given this https://github.com/hashicorp/terraform/issues/30841

fancybear-dev commented 1 year ago

Hi - me again. Glad to tell you I've found a viable fix.

What you need to do, is that the dynamic blocks you use - will ALWAYS have an increasing priority in your plan. Terraform isn't executing parrallel API requests that is causing this issue, rather it's constructing a single request in which the order of the routing rules are in a wrong order. So if you fix the order, you fix the problem.

The reason you get the 100 prio error, is becausing the 200 prio routing rule is before it. Hence it errors. Switch it around and it will work.

Example of how your plan should look like;

A general (anonymized) example how how I did this (this ensure Terraform sorts based on prio correctly as well - hence the format);

locals {
  bypass_paths = distinct([
    "/log1",
    "/health1",
    "/log2",
    "/health2",
  ])
}
dynamic route_rules {
  for_each = { for idx, path in local.bypass_paths : format("%03d", idx) => path }
  content {
    priority = route_rules.key
    service = google_compute_backend_service.lb_backend_without_iap.id
    match_rules {
      full_path_match = route_rules.value
    }
  }
}

You can chain multiple dynamic blocks as well, by adding e.g. the length of a tuple as part of the for loop idx.

pawelJas commented 1 month ago

@rileykarson What is the suggested solution to this problem then? As you said a regular manual fix can lead to problems. @fancybear-dev proved there is a simple way to use priorities here. Gcloud also doesn't reorder given input. Is this just "as designed"?

hashicorp / terraform-provider-google