Mastercard / terraform-provider-restapi

A terraform provider to manage objects in a RESTful API
Other
785 stars 215 forks source link

feat: add support for read_data #252

Closed detvdl closed 4 months ago

detvdl commented 7 months ago

Similar to https://github.com/Mastercard/terraform-provider-restapi/pull/182, added support for an argument read_data in order to support not-so-RESTful API designs.

Case study is Scalyr/DataSet, whose API for fetching configuration files is implement as a set of POST requests, with the hard requirement that there is a request body (even if it is empty):

Tested against this API with some logParser objects, and this helps to support a use case such as:

provider "restapi" {
  uri = "https://app.scalyr.com/api"
  headers = {
    "Authorization" = "Bearer ${local.api_token}"
    "Content-Type"  = "application/json"
  }
  debug                = false
  write_returns_object = false
}

resource "restapi_object" "scalyr_parsers" {
  for_each      = local.parsers
  object_id     = "/logParsers/${each.key}"
  path          = "/"
  create_method = "POST"
  update_method = "POST"
  read_method   = "POST"
  read_path     = "/getFile"
  read_data = jsonencode({
    path = "/logParsers/${each.key}"
  })
  create_path = "/putFile"
  data = jsonencode({
    path    = "/logParsers/${each.key}"
    content = each.value
  })
}
detvdl commented 7 months ago

Before I move ahead with also attempting to support this in data sources, is this something that would be useful?

Presently, I have no need to support this use-case from a data source perspective, but wouldn't mind adding it if this is deemed useful in any way

detvdl commented 7 months ago

Duplicate of #177 it seems, any advice on how to move forward with this?

It seems to highlight that there is a need for such a field, but I'm open to suggestions

DRuggeri commented 4 months ago

Thanks for the PR @detvdl ! I do think this makes sense and it would be generally useful, so ought to be brought into the provider. Thanks for including the test cases - this is g2g, IMO. I know you opened this a bit ago, but would you like to also add the Datasource support now before merge?

Also, thanks for the patience! As mentioned in the readme, we keep the provider up to date but features are ssssllllloooooowwwwww.

detvdl commented 4 months ago

Hi @DRuggeri,

I don't have a use-case for implementing this in data sources currently, and I think it might need a bit more of a re-work of the data source handling in general. The APIs for the Resource and Data source are quite different right now.

If it's okay, I would prefer to look at that separately, at a later date. If this is g2g for you, then feel free to merge and release as you see fit 🙂

DRuggeri commented 4 months ago

Right on - this will be released in 1.19.0 which is building now!