Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client validate return on 403 #282

Merged
merged 1 commit into from
Nov 11, 2019
Merged

Client validate return on 403 #282

merged 1 commit into from
Nov 11, 2019

Conversation

horjulf
Copy link
Contributor

@horjulf horjulf commented Nov 7, 2019

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
Copy link
Collaborator

bkabrda commented Nov 8, 2019

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
Copy link
Contributor Author

horjulf commented Nov 9, 2019

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
Copy link
Collaborator

bkabrda commented Nov 11, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants