dynatrace-oss / terraform-provider-dynatrace

Apache License 2.0
71 stars 33 forks source link

Seperate resources for synthetic HTTP monitor requests #399

Closed lynnbongers closed 8 months ago

lynnbongers commented 9 months ago

Is your feature request related to a problem? Please describe. My team would like to automate the rollout of synthetic HTTP monitors in Terraform. To do this we use the resource "dynatrace_http_monitor" and a servicecatalog product from AWS that creates a clientid for the monitor to use to authenticate. We use the resource "aws_servicecatalog_provisioned_product" to create the service catalog product. The problem we face is that the servicecatalog product needs the HTTP check ID from the synthetic monitor and the synthetic monitor needs the clientID generated by the product to use in its POST request. This creates a circular dependency.

Describe the solution you'd like We would like separate resources for the requests in the HTTP monitor (ex. dynatrace_http_monitor_request). This would solve our circular dependency because the dynatrace_http_monitor would be created first 'without a request' supplying the monitor check id. The servicecatalog product can be created using that and finally the clientID can be used in the dynatrace_http_monitor_request resource.

Describe alternatives you've considered We have tried to work around the circular dependency by using different variables for the clientID and HTTP check ID, which did not work, and by using an api call to insert the values for the POST and GET requests. This is not part of infrastructure as code and therefore it is not a safe solution. We have also spoken with the team that created the AWS product and they explained that this is the only way to generate a safe way to authenticate systems.

Dynatrace-Reinhard-Pilz commented 9 months ago

Hello @lynnbongers,

Could you provide and example for how these requests would look like? I'm specifically interested in where this clientId comes into play - as part of the URL, as HTTP Header.

I'm asking because separating out the Requests of the Synthetic Monitors won't be particularly easy. Not impossible, but because these requests are internally in Dynatrace not accessible via unique identifiers, it will be challenging.

That's why I'd like to invest a bit of effort first into solving the circular dependency in a different way.

best regards, Reinhard

lynnbongers commented 9 months ago

Hi Reinhard,

We have tried a lot of things to work around this issue so new ideas are always welcome. The clientID is used in the request body of the OAuth2 authorization request. It comes from the output of the AWS service catalog product. This is an example of the monitor we use:

`resource "dynatrace_http_monitor" "Nameless_monitor" {
  name      = "Name of the monitor"
  enabled   = true
  frequency = 0
  locations = locations
  anomaly_detection {
    loading_time_thresholds {
      enabled = true
    }
    outage_handling {
      global_outage    = true
      # local_outage   = false
      # retry_on_error = false
      global_outage_policy {
        consecutive_runs = 1
      }
    }
  }
  script {
    request {
      description     = "nameless-monitor"
      body            = jsonencode({
              "client_id": "clientID",
              "grant_type": "client_credentials",
              "scope": "api://this-is-a-test.com/.default"
        })
      method          = "POST"
      post_processing =<<-EOT
        var responseBody = response.getResponseBody();
        var jsonData = JSON.parse(responseBody);
        api.setValue("bearerToken", jsonData.access_token);
      EOT
      url             = "https://authorization-url.com"
      configuration {
        accept_any_certificate = true
        client_certificate     = "certificate"
        follow_redirects       = true
      }
      validation {
        rule {
          type            = "httpStatusesList"
          # pass_if_found = false
          value           = ">=400"
        }
      }
    }
    request {
      description    = "endpoint"
      method         = "GET"
      pre_processing =<<-EOT
        request.addHeader("Authorization", "Bearer " + api.getValue("bearerToken"));
      EOT
      url            = "https://endpoint.com"
      configuration {
        accept_any_certificate = true
        follow_redirects       = true
      }
      validation {
        rule {
          type            = "httpStatusesList"
          # pass_if_found = false
          value           = ">=400"
        }
      }
    }
  }
}

The service catalog product is created with this resource:

resource "aws_servicecatalog_provisioned_product" "dynatrace_synthetic_client" {
  for_each = { for endpoint in var.dynatrace_http_monitor_endpoints : endpoint.name_prefix => endpoint if endpoint.use_auth }

  name                     = "dynatrace-synthetic-client-${var.cluster_name}"
  product_id               = "prod-xxxxxxxxxx"
  provisioning_artifact_id = "pa-xxxxxxxxxxxx"
  path_id                  = "lxxx-xxxxxxxxxxxx"

  provisioning_parameters {
    key   = "SyntheticId"
    value = "HTTP_check_id"
  }

Where the HTTP_check_id is needed as a parameter. Because of this the monitor needs to exist and terraform does not have an option to update the monitor with the clientID from the product.

We discussed the issue and we have an idea for a solution: example solution:

//monitor with empty script
resource "dynatrace_http_monitor" "monitor_1" {
"
script{}
"
}
resource "dynatrace_http_monitor_script" "monitor_1_request_1" {
monitor_id = dynatrace_http_monitor.monitor_1.id
...
}
resource "dynatrace_http_monitor_script" "monitor_1_request_2" {
monitor_id = dynatrace_http_monitor.monitor_1.id
....
}

We would like to hear your thoughts on this.

Kind regards, Lynn

Dynatrace-Reinhard-Pilz commented 8 months ago

Hello @lynnbongers,

Yes, you're right, this is difficult to solve. I've played around a bit with a few ideas I had in mind. But at best I would be able to come up with a solution that decouples things via data sources. And here the tradeoff would be that you'd have to run apply twice before all the settings are consistent.

The example solution you've posted looks pretty similar to what we have discussed internally. It just makes things easier to manage if you don't have a separate resource block per request - like in this example.

# monitor with empty script
resource "dynatrace_http_monitor" "monitor_1" {
...
   # a missing script block signals that requests are not getting manged within this resource
...
}

resource "dynatrace_http_monitor_script" "monitor_1_script" {
   monitor_id = dynatrace_http_monitor.monitor_1.id
   script {
      request {
      }
      request {
      }
   }
}

Otherwise you wouldn't be able to define the order or the requests properly.

One limitation of this solution will be that a dynatrace_http_monitor that has initially been defined without a script block cannot manage any requests from that point on. You'll have to destroy the resource first and then recreate it. Reason for that is simply that the Provider will have to store a flag within the state that signals that requests are getting maintained somewhere else.

AND there is unfortunately one additional caveat to consider: The REST API (and also the WebUI) doesn't allow for creating HTTP Monitors without any requests. The REST API would simply reject the payload. I'm pretty sure that it's technically possible to change that, but that's a task we'd have to reach out to Dynatrace R&D for that - that's separate from the dev team that develops the Terraform Provider. And I also cannot promise an ETA for that.

So here's the alternative I could offer that doesn't depend on any code changes within Dynatrace: In case the script block is missing, the Terraform Provider sends some sort of "dummy" request to the REST API (alongside all the other settings for the HTTP Monitor). The user doesn't have to specify it within the HCL code, but the request would look like this:

...
script {
    request {
        description    = "--terraform-auto-generated--"
        method         = "OPTIONS"
        url            = "http://localhost"
    }
}
...

Even if Dynatrace attempts to execute the HTTP Monitor right away, it won't fail - this dummy request doesn't contain any validation steps. And once the client_id is available and Terraform creates this resource

resource "dynatrace_http_monitor_script" "monitor_1_script" {
   monitor_id = dynatrace_http_monitor.monitor_1.id
   script {
      request {
      }
      request {
      }
   }
}

the dummy request will get replaced by the actual requests.

Let us know if you're ok with the above mentioned limitations.

lynnbongers commented 8 months ago

Hi Reinhard,

Thank you for getting back to me. I have discussed your suggestions with the team and we have some questions. Unfortunately it is not workable for us to use apply twice.

If I understand this correctly, you have 2 proposals: 1) The resource dynatrace_http_monitor with no script block, which will be flagged to never manage requests. For this to work the internal team will need to change the API and we won't know how long that will take. 2) The resource dynatrace_http_monitor with no script block, a dummy script block will be added by the provider to be compatible with API and the creation of the dynatrace_http_monitor_script resource, which your team would be able to do, to overwrite the dummy script block.

But here we need 2 apply's right? Where the dynatrace_http_monitor_script resource will overwrite the script? Also, after applying the dynatrace_http_monitor_script, will a new plan for dynatrace_http_monitor try to change stuff back?

Perhaps it would be possible to introduce dynatrace_http_monitor_request instead of dynatrace_http_monitor_script? It might be difficult to manage dynatrace_http_monitor_script as one can have multiple requests and you have to manage items in 1 terraform resource. Would it be possible to manage the order of the requests by dependencies?

dynatrace_http_monitor_request req1{
monitor_id = dynatrace_http_monitor.monitor_1.id
}

dynatrace_http_monitor_request req2{
monitor_id = dynatrace_http_monitor.monitor_1.id
...
depends_on = [
    dynatrace_http_monitor_request.req1,
  ]
}

Kind regards, Lynn

Dynatrace-Reinhard-Pilz commented 8 months ago

Hi Lynn,

Looks like I've confused the hell out of you with the way I've explained things.

No, there won't be a need for running apply twice. Those would have been the workarounds I experimented with initially. And I also don't consider that a solution.

The 2 proposals you've mentioned are actually just a single proposal.

Your Terraform users just need to know about this:

From here on, this is an implementation detail, nothing more. Dynatrace R&D may be able to make our lives easier here, but like I said, it's hard to say when we'd get that API change. This "dummy request" is a necessity, because the REST API won't allow the Provider to create a Monitor without Requests. If you're configuring a dynatrace_http_monitor without a script block, the Provider will nevertheless configure this one request - that has no effect at all, even if Dynatrace executes it. And that dummy request will vanish as soon as a resource dynatrace_http_monitor_script that configures the requests for that monitor is getting created by Terraform.

Can you explain in detail why you'd prefer to have separate resource blocks per request? I'm asking because the solution with dynatrace_http_monitor_script would contain all of them in a well ordered manner - without room for misconceptions. Your proposed solution with depends_on would in principle represent a hint to the Terraform Provider when the Request are getting CREATED. But in the event that a change was made in ONE of of these dynatrace_http_monitor_request resources, the Provider wouldn't know which one it is. Also, if one of the requests gets deleted, the Provider wouldn't know about it.

lynnbongers commented 8 months ago

Hi Reinhard,

Thank you for explaining!

It sounds like a really good solution for our problem.

The idea behind the separate request resources was that in case anything changes that not the entire script would need to be reapplied, just the one request but honestly the script resource sounds a lot better. The dynatrace_http_monitor_script resource would contain all the requests in a well ordered manner, without room for misconceptions.

We would like to work with the solution of creating the dynatrace_http_monitor_script resource. And if it is possible to eventually make the change to the API that would be even better.

What would be the ETA for this solution?

Kind regards, Lynn

Dynatrace-Reinhard-Pilz commented 8 months ago

We should be able to look into this within the next two weeks. The actual implementation definitely won't take that long. I'm just a bit careful because our team occasionally gets pulled into other topics than Terraform - which causes delays. But you can expect something within the first week of March.

lynnbongers commented 8 months ago

The first week of March sounds great, thank you for your help!

kishikawa12 commented 8 months ago

Hi @lynnbongers,

We've created a new resource dynatrace_http_monitor_script that will allow you to configure the script block of the HTTP monitor separately from the main dynatrace_http_monitor resource. In order to utilize the secondary resource, in your dynatrace_http_monitor configuration please omit the script block and set no_script=true. Additional details will be included in the documentation with the release of the enhancement.

We'll plan on deploying a new release at the end of the week. I hope that helps!

lynnbongers commented 8 months ago

Thank you for the update! We'll get started on working with the new resource

lynnbongers commented 8 months ago

Final update: We implemented the resource and everything works perfectly. Thank you so much for your work!