Mastercard / terraform-provider-restapi

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

Read Data Support Breaks APIs that don't accept a body for GET method #262

Closed ryaison closed 4 months ago

ryaison commented 4 months ago

The addition of read_data support now results in errors when querying an API that doesn't support a body when using the GET method.

A working api_client.go sends

2024-03-05T16:06:29.809Z [INFO]  provider.terraform-provider-restapi_v1.18.2: 2024/03/05 16:06:29 api_client.go: BODY:: timestamp=2024-03-05T16:06:29.808Z
2024-03-05T16:06:29.809Z [INFO]  provider.terraform-provider-restapi_v1.18.2: 2024/03/05 16:06:29 <none>: timestamp=2024-03-05T16:06:29.808Z

A non-working api_client.go sends

2024-03-05T16:13:57.131Z [INFO]  provider.terraform-provider-restapi_v1.19.0: 2024/03/05 16:13:57 api_client.go: BODY:: timestamp=2024-03-05T16:13:57.131Z
2024-03-05T16:13:57.131Z [INFO]  provider.terraform-provider-restapi_v1.19.0: 2024/03/05 16:13:57 {}: timestamp=2024-03-05T16:13:57.131Z

As indicated in RFC 7231:

A payload within a GET request message has no defined semantics;
sending a payload body on a GET request might cause some existing
implementations to reject the request.

Can we add a default or argument to not send a body with a GET unless there's a read_data value?

Justin-DynamicD commented 4 months ago

In general, body is not universally needed for every method across various api providers, and it would be nice to be able set a bool true/false to be able to toggle its inclusion for read, create, etc.

create_include_data = false destroy_include_data = false

something like that (obviously name debatable)

that kind of thing. (maybe a better name,

DRuggeri commented 4 months ago

Oh dear - my apologies, folks. I missed a subtle bug during review of the new feature last week! I have corrected the bug (as well as made the check a bit less magic-y) in 7ffe6b3f40847505089380ad947dcbf5301bdb4e

Version v1.19.1 is building now