PagerDuty / terraform-provider-pagerduty

Terraform PagerDuty provider
https://www.terraform.io/docs/providers/pagerduty/
Mozilla Public License 2.0
205 stars 210 forks source link

There is now way to apped an existing pagerduty_event_orchestration_router with rules dynamically #648

Open gthoren opened 1 year ago

gthoren commented 1 year ago

Hi,

Everytime I create a new Pagerduty service, I would like to add rules to an existing pagerduty_event_orchestration_router resource without having to manually specify the rule in the router. Previously, it was possible to create multiple pagerduty_event_rule and add it to a pagerduty_ruleset. I would like to do something similiar with event orchestration, but if I create multiple pagerduty_event_orchestration_routers and add it to an pagerduty_event_orchestration it will only take the latest route added.

Terraform Version

1.3.9

Affected Resource(s)

Please list the resources as a list, for example:

Terraform Configuration Files

resource "pagerduty_event_orchestration_router" "service_router" {
  event_orchestration = pagerduty_event_orchestration.monitor.id
  set {
    id = "start"
    rule {
      label = "Catch ${var.namespace} alerts in ${var.environment}"
      condition {
        expression = "event.custom_details.namespace_name matches '${var.namespace}' and event.custom_details.environment matches '${var.environment}'"
      }
      actions {
        route_to = var.service_id
      }
    }
  }
  catch_all {
    actions {
      route_to = "unrouted"
    }
  }
}

resource "pagerduty_event_orchestration_router" "service_router2" {
  event_orchestration = pagerduty_event_orchestration.monitor.id
  set {
    id = "start"
    rule {
      label = "Catch ${var.namespace} alerts in ${var.environment}"
      condition {
        expression = "event.custom_details.namespace_name matches '${var.namespace}' and event.custom_details.environment matches '${var.environment}'"
      }
      actions {
        route_to = var.service_id
      }
    }
  }
  catch_all {
    actions {
      route_to = "unrouted"
    }
  }
}

Expected Behavior

It should be possible to create multiple pagerduty_event_orchestration_router and add it to the event_orchestration, or be possible to add rule resources to the router.

Actual Behavior

Creating multiple pagerduty_event_orchestration_router and add it to the event_orchestration will result in only one of the routers beings set to the event_orchestration.

mrwacky42 commented 1 year ago

This is such a baffling failure of the provider, or worse, the PagerDuty API. We have hundreds of rules in our ruleset, created in many many Terraform modules. There is no way for us to know all the rules to create the orchestration at once.

TreverW commented 1 year ago

I'm running into the same problem.

richter-wgs commented 1 year ago

Please consider to fix this issue soon, due to PagerDuty shifting from rule sets to event orchestrations we created many orchestrations with 50+ routes. So this issue only having one route per event orchestration via terraform is a real blocker for us.

dkarl-wgs commented 1 year ago

Also running into the same problem, +1 for a fix! ;)

gsreynolds commented 1 year ago

Hi folks - this is possible by using a combination of for_each and dynamic blocks in Terraform.

There can only be 1 Event Orchestration Router per Event Orchestration. The router contains all of the service routes. See below, notice multiple rules inside the set with id = "start".

You have to build the entire set of EO service routes and update that in the API via Terraform in a single operation, rather than how Event Rulesets operated.

resource "pagerduty_event_orchestration_router" "router" {
  event_orchestration = pagerduty_event_orchestration.my_monitor.id
  set {
    id = "start"
    rule {
      label = "Events relating to our relational database"
      condition {
        expression = "event.summary matches part 'database'"
      }
      condition {
        expression = "event.source matches regex 'db[0-9]+-server'"
      }
      actions {
        route_to = pagerduty_service.database.id
      }
    }
    rule {
      condition {
        expression = "event.summary matches part 'www'"
      }
      actions {
        route_to = pagerduty_service.www.id
      }
    }
  }
  catch_all {
    actions {
      route_to = "unrouted"
    }
  }
}

To do this dynamically, you need to use for_each and dynamic blocks:

locals {
  acme_web_routing_rules = {
    "web_db" : { "name" : "relational database", "conditions" : ["event.source matches regex 'db[0-9]+-server'", "event.source matches regex 'db[0-9]+-server'"] },
    "web_app" : { "name" : "web app", "conditions" : ["event.summary matches part 'www'"] },
  }
}

resource "pagerduty_event_orchestration_router" "router" {
  event_orchestration = pagerduty_event_orchestration.my_monitor.id
  set {
    id = "start"
    dynamic "rule" {
      for_each = local.acme_web_routing_rules
      content {
        label = "Events relating to ${rule.value.name}"
        dynamic "condition" {
          for_each = rule.value.conditions
          content {
            expression = condition.value
          }
        }
        actions {
          route_to = pagerduty_service.acme_web[rule.key].id
        }
      }
    }
  }
  catch_all {
    actions {
      route_to = "unrouted"
    }
  }
}

https://registry.terraform.io/providers/PagerDuty/pagerduty/latest/docs/resources/event_orchestration_router

From https://github.com/PagerDuty/terraform-provider-pagerduty/issues/627

Having a single Event Orchestration router Terraform resource is one of the significant improvements over how Event Rule resources operated. There is a single POST operation required to update the EO Router, rather than tens/hundreds/thousands of operations if you had many rules in a Event Ruleset.

gsreynolds commented 1 year ago

@gthoren @mrwacky42 @TreverW @richter-wgs @dkarl-wgs please see my comment above regarding using for_each and dynamic blocks with the pagerduty_event_orchestration_router resource.

There is only one router per Event Orchestration and it contains the set of all service routes.

mrwacky42 commented 1 year ago

Hi @gsreynolds - Like I said above, this does not work for us because several Terraform modules create the rules in different places in our Terraform code. There is no place to create a single resource with "knowledge" of every service that would be receiving alerts.

That said, I'm no longer attached to that project, so my interest level here is rated: indifferrent.

bschaeffer commented 1 year ago

One problem with using dynamic rule blocks with routers that have lots of rules is that it creates an enormous diff if you are generating large rule sets and you end up removing one item.

For instance, take the following:

resource "pagerduty_event_orchestration_router" "router" {
  for_each = local.environments
  event_orchestration = pagerduty_event_orchestration.orchestrator[each.key].id
  set {
    id = "start"
    dynamic "rule" {
      for_each = each.value.rules
      content {   # ... }
    }
  }
  # ...
}

A developer recently removed one item from a local.environments[production].services that drives dynamic rule block above, and this is the diff they ended up with:

# pagerduty_event_orchestration_router.router["production"] will be updated in-place
~ resource "pagerduty_event_orchestration_router" "router" {
        id                  = "foo"
        # (1 unchanged attribute hidden)

      ~ set {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {
          ~ rule {

Pretty unintelligible and probably due to the way this provider uses list blocks

RyanW8 commented 9 months ago

This is also a problem for us. We've used dynamic rule & condition blocks but we have event orchestrations with hundreds of routes in. Meaning a tiny change leads to a huge incomprehensible diff.

Being able to define an EO router rule as a single resource would lead to many more API operations but a much better UX; a worthwhile compromise IMO.

jmjr-uk commented 8 months ago

This is a significant blocker to our adoption of PagerDuty as well. As others have experienced, we do not have a single source-of-truth which defines all of our services (~300 or so and growing). This makes it very hard to configure service routing for alerts from Datadog into PagerDuty without having to also maintain ~300 different integrations and Orchestrations alongside it which is not desirable for a number of reasons.

Needing to (or having the option to) make more API calls is a worthwhile trade-off to make the management of these orchestrations easier using IaC, particularly for a setup like ours.

martinohansen commented 7 months ago

As others have noted: we don't have a single place where we define our routing, we call a module for each environment which in turn adds a set of rules based on the module variables. With the current state of things, we're unable to migrate to the new orchestration without reworking our entire setup.

Daniel-Vaz commented 7 months ago

Plus one to see a better implementation of this in order to support Event orchestration Rules to be dynamically appended to a single main orchestration router.

The proposed solution by @gsreynolds is nice and clean when everything is managed in a single place, but doesn't address use-cases where different terraform executions\states just want to add a extra route to a already existing Orchestration Router.

lucasnery-hotmart commented 1 month ago

I'm running into the same problem, has a solution been found yet?

I found

    dynamic_route_to {
      lookup_by = "service_id"
      source = "event.custom_details.pd_service_id"
      regex = "(.*)"
    }
  }

but the solution needs to hire pagerduty AIOps

image