Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jamiejackson opened this issue Jun 4, 2024 · 3 comments

Comments

@jamiejackson
Copy link

jamiejackson commented Jun 4, 2024

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:

  • I must remember to modify the relationship on both sides, otherwise I get a problematic terraform plan.
  • There is a circular dependency between the two, so I am forced to hardcode an ID on one of the sides. For instance:
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
Copy link
Author

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

@amirkkn
Copy link

amirkkn commented Jun 13, 2024

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
Copy link

amirkkn commented Jun 13, 2024

I could temporary bypass it, hopefully

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

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants