DrFaust92 / terraform-provider-bitbucket

Terraform Bitbucket Cloud provider.
https://registry.terraform.io/providers/drfaust92/bitbucket
Mozilla Public License 2.0
38 stars 30 forks source link

fix: make sure the authorization error is correctly shown to the end user #200

Closed Nkmol closed 5 months ago

Nkmol commented 6 months ago

This PR makes sure that an authorization error (401 Unauthorized) is at least shown to the end user.


Bitbucket responses, mainly headers and error codes, seem to have completely changed. For the unauthorized response, the Content-Type was changed to "text/plain" where no HTTP-body is returned any more.

Because of this, the client's decoding fails (see https://github.com/DrFaust92/bitbucket-go-client/blob/main/client.go#L387), and the original Bitbucket API error is no longer passed to the Terraform code.

To overcome this issue, I now rely on the raw HTTP Response and always log the original HTTP Code. The main files to review are the tests and the bitbucket/error.go file.

Authorization error

╷
│ Error: 401 Unauthorized: 
│ 
│   with bitbucket_repository.that,
│   on example.tf line 13, in resource "bitbucket_repository" "that":
│   13: resource "bitbucket_repository" "that" {
│ 

Other Bitbucket API error

╷
│ Error: 400 Bad Request: Repository with this Slug and Owner already exists.
│ 
│   with bitbucket_repository.that,
│   on example.tf line 13, in resource "bitbucket_repository" "that":
│   13: resource "bitbucket_repository" "that" {
│ 
╵

I ran the acceptance tests, but found some issues that also happen on the master branch.

An individual run, for example for the Bitbucket Repository resource, shows an error that is not caused by this PR:

?       github.com/terraform-providers/terraform-provider-bitbucket     [no test files]
=== RUN   TestAccBitbucketRepositoryGroupPermission_basic
--- PASS: TestAccBitbucketRepositoryGroupPermission_basic (19.98s)
=== RUN   TestAccBitbucketRepository_basic
--- PASS: TestAccBitbucketRepository_basic (9.63s)
=== RUN   TestAccBitbucketRepository_project
--- PASS: TestAccBitbucketRepository_project (10.74s)
=== RUN   TestAccBitbucketRepository_avatar
--- PASS: TestAccBitbucketRepository_avatar (9.26s)
=== RUN   TestAccBitbucketRepository_slug
--- PASS: TestAccBitbucketRepository_slug (8.16s)
=== RUN   TestAccBitbucketRepository_inherit
--- PASS: TestAccBitbucketRepository_inherit (22.73s)
=== RUN   TestAccBitbucketRepository_wrongCredential
--- PASS: TestAccBitbucketRepository_wrongCredential (0.71s)
=== RUN   TestAccBitbucketRepository_duplicate
--- PASS: TestAccBitbucketRepository_duplicate (5.49s)
=== RUN   TestAccBitbucketRepositoryUserPermission_basic
--- FAIL: TestAccBitbucketRepositoryUserPermission_basic (7.14s)
=== RUN   TestAccBitbucketRepositoryVariable_basic
--- PASS: TestAccBitbucketRepositoryVariable_basic (15.43s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-bitbucket/bitbucket   109.292s
FAIL

Manual tested a correct apply, authorization error and a Bitbucket API error.

act-mreeves commented 5 months ago

This looks useful. I've observed errors can be very opaque so bubbling it up from the bitbucket API seems helpful. https://github.com/DrFaust92/terraform-provider-bitbucket/issues/199

Example when I intentionally make my username (via BITBUCKET_USERNAME) incorrect.

╷
│ Error: Empty Summary: This is always a bug in the provider and should be reported to the provider developers.
│ 
│   with bitbucket_repository.foobar,
│   on foobar.tf line 1, in resource "bitbucket_repository" "foobar":
│    1: resource "bitbucket_repository" "foobar" {