Provider crash due to conversion panic about terraform-provider-better-uptime HOT 6 CLOSED

Hello @fancybear-dev, thank you for the detailed report 🙌

While I can't provide any ETA on this, we'd like to fix the provider panicking in this case. The request_headers field should be easier to work with there. Thanks again for reporting it and all the details 👍

from terraform-provider-better-uptime.

Based on this issue (#19) and API docs (https://betterstack.com/docs/uptime/api/create-a-new-monitor/#example-curl) I suspect the provider does insufficient validation for the input value of request_headers of the betteruptime_monitor resource. We have a condition in place that results in a null value being passed, which is accepted - but seemingly not processed properly by the provider -> resulting in the conversion panic. Testing this now.

I find the API odd in general as well - never seen a data structure like this for this purpose;

"request_headers": [
  {
    "name": "X-Custom-Header",
    "value": "custom header value"
  }
]

This is overly complex to simply pass headers, something like;

"request_headers": {
	header_key: header_value,
	header_key2: header_value2
}

makes more sense to me personally - it also reads better in places like Terraform. I'm sure there are reasons for the existing solution, but thought I'd share regardless.

Right now in Terraform I do the conversion like this (to pass it 1:1 to the resource) - but this conversion in itself is the thing I find odd but it's what it is because the API expects it like this;

  headers = {
    "Authorization" = "whatever"
    "magic"         = "tree"
  }
  request_headers = [
    for header_key, header_value in local.headers: {
      name  = header_key
      value = header_value
    }
  ]

from terraform-provider-better-uptime.

Managed to work around the issue by ensuring the input value is as expected, and am now receiving the error that is correct - but should also be catched imo in the provider. If not catched in the provider at plan time, it creates overhead for developers when it fails at apply time. The provider should reflect what the API is validating on.

In any case, solved my issues - but will leave this issue open as I'm curious what your thoughts are.

from terraform-provider-better-uptime.

What I would like most, is if null is accepted to be passed to the provider - to allow for conditional request_headers. Right now it's either a provider panic or the API that does not accept it -> so I'm currently always passing a (bogus) header.

from terraform-provider-better-uptime.

Hi @fancybear-dev,

Thank you for your patience 🙏

I tried to replicate the conversion panic by using a map in request_header:

resource "betteruptime_monitor" "status" {
  url              = "https://example.com"
  monitor_type     = "status"
  request_headers  = { "X-Monitored-For" : "https://my_status_page.betteruptime.com" }
}

But it failed nicely in provider version 0.8.0, 0.9.0, 0.10.0, 0.11.0, and the latest 0.11.6:

╷
│ Error: Incorrect attribute value type
│ 
│   on main.tf line 40, in resource "betteruptime_monitor" "status":
│   40:   request_headers  = { "X-Monitored-For" : "https://${betteruptime_status_page.this.subdomain}.betteruptime.com" }
│ 
│ Inappropriate value for attribute "request_headers": list of map of string required.
╵

Providing array with map with different keys than name and value (such as request_headers = [ { "X-Monitored-For" : "https://${betteruptime_status_page.this.subdomain}.betteruptime.com" } ]) just resulted in being ignored.

This would result in unnecessary updates and unstable plan, but never panicking during my tests.

Do you by any chance have the original provider version and terraform config that produced the conversion panic?

I would love to prevent the provider from panicking, and it would be great to investigate and fix that...

As an alternative, I've decided to implement a validation for the request_headers fields, which would run in the plan phase and would help users to get the data in the format that's expected.

Hope this helps 🙌

from terraform-provider-better-uptime.

Hello @fancybear-dev

We've just released v0.11.7 where the request_headers are validated at plan time (if known, otherwise at apply time). I've also added an example of usage into examples/advanced so it can be looked up.

This should definitely provide better experience while using request_headers, and hopefully prevent running into conversion panic - even though I wasn't able to reproduce it.

Thanks again for reporting it in detail 🙌

from terraform-provider-better-uptime.