site24x7 / terraform-provider-site24x7

Terraform provider for Site24x7
MIT License
22 stars 33 forks source link

Provider Shouldn't Require Definition of Both Sides of Integration/Monitor Relationship (Circular Dependency) #269

Open jamiejackson opened 2 months ago

jamiejackson commented 2 months ago

Provider version 1.0.84 forces both sides of the Integration/Monitor† relationship to be defined, which is inappropriate. Other providers do this via association resources or they allow the relationship to be set on one side of the relationship.

†There may be other circular resource dependencies in the provider that need to be worked out, as well, but I'm starting with this one.

I'd suggest studying how some of the big providers do many-to-many relationships and pick/implement an option that makes sense.

There are problems with the current behavior. Here are two of them:

resource "site24x7_slack_integration" "experimental" {
  ...
  # i have to do this:
  monitors = ["207348000022635025"]
  # because i cannot do this, due to the circular dependency.
  monitors = [site24x7_website_monitor.home_page__stage__jaj_test.id]
}

resource "site24x7_website_monitor" "home_page__stage__jaj_test" {
  ...
  third_party_service_ids     = [
    site24x7_slack_integration.experimental.id
  ]
}

At first blush, I think it probably makes the most sense to define the integration relationship from the monitor side, which would look like this:

resource "site24x7_slack_integration" "experimental" {
  ...
  # _no_ monitors array specified!
}

resource "site24x7_website_monitor" "home_page__stage__jaj_test" {
  ...
  # monitor defines which integrations it has
  third_party_service_ids     = [
    site24x7_slack_integration.experimental.id
  ]
}

Here is the fuller example of my current, problematic use case, as I am forced to implement it now.

resource "site24x7_slack_integration" "experimental" {
  alert_tags_id  = []
  critical_alert = true
  down_alert     = true
  monitors = ["207348000022635025"]
  name           = "JAJ Test Default (#experimental) Webhook)"
  selection_type = 2
  sender_name    = "Site24x7"
  tags           = []
  title          = "$MONITORNAME is $STATUS"
  trouble_alert  = true
  url            = data.aws_ssm_parameter.slack_webhook_url__default.value
}

# I'm using this to experiment with https://github.com/site24x7/terraform-provider-site24x7/issues/261
resource "site24x7_website_monitor" "home_page__stage__jaj_test" {
  display_name                = "JAJ test"
  website                     = "https://stage.example.com/"
  check_frequency             = "1"
  credential_profile_id       = local.credentials.stage_basic_auth_id
  ignore_cert_err             = false
  location_profile_id         = site24x7_location_profile.united_states.id
  match_regex_severity        = 2
  matching_keyword_severity   = 0
  matching_keyword_value      = "need housing assistance"
  timeout                     = 30
  unmatching_keyword_severity = 2
  use_name_server             = false
  monitor_groups              = []
  third_party_service_ids     = [
    site24x7_slack_integration.experimental.id
  ]
}
jamiejackson commented 2 months ago

(#246 might be another example of an inappropriate circular relationship.)

amirkkn commented 2 months ago

We wanted to start using terraform for setup and we got the same issue with webhook integration. Hopefully someone work on this bug. We use version 1.0.90

amirkkn commented 2 months ago

I could temporary bypass it, hopefully

resource "site24x7_webhook_integration" "webhook_integration" {
  ...
  monitors                        = []
  lifecycle {
    ignore_changes = [monitors]
  }

}