search

zorkian / go-datadog-api

A Go implementation of the Datadog API.
BSD 3-Clause "New" or "Revised" License
183 stars 156 forks source link

Client validate return on 403 #282

Closed horjulf closed 4 years ago

horjulf commented 4 years ago

Datadog now returns HTML with a 403 which causes the JSON unmarshal to fail. We now check for the status code before attempting to parse JSON.

2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: 2019/11/07 14:29:06 [DEBUG] Datadog API Request Details:
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: ---[ REQUEST ]---------------------------------------
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: GET /api/v1/validate?api_key=&application_key= HTTP/1.1
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Host: api.datadoghq.com
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: User-Agent: Go-http-client/1.1
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Content-Type: application/json
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Accept-Encoding: gzip
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1:
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1:
2019-11-07T14:29:06.388+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: -----------------------------------------------------
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: 2019/11/07 14:29:07 [DEBUG] Datadog API Response Details:
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: ---[ RESPONSE ]--------------------------------------
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: HTTP/1.1 403 Forbidden
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Connection: close
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Transfer-Encoding: chunked
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Cache-Control: no-cache
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Content-Type: text/html
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Date: Thu, 07 Nov 2019 01:29:07 GMT
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1:
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: 5d
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: <html><body><h1>403 Forbidden</h1>
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: Request forbidden by administrative rules.
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: </body></html>
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1:
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: 0
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1:
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1:
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: -----------------------------------------------------
2019-11-07T14:29:07.681+1300 [DEBUG] plugin.terraform-provider-datadog_v2.5.1: 2019/11/07 14:29:07 [ERROR] Datadog Client validation error: invalid character '<' looking for beginning of value
bkabrda commented 4 years ago

Hey thanks for the PR. IIUC this should actually return a non-nil error, because 403 means the key is not valid. Is that right?

horjulf commented 4 years ago

Hey @bkabrda, I used the same logic as the last return: return out.IsValid, nil

Since we get a 403, we know that the credentials are not valid, theres no error to return, the consumer expects the bool to indicate if they are valid or not. I think the error here should only be used if we fail to verify that the credentials are valid or not.

bkabrda commented 4 years ago

@horjulf you're absolutely correct, thanks for the explanation. I'm merging this, thanks a lot for the PR!