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

bitbucket_branch_restriction doesn't handle "users" changes properly for "restrict_merges" #53

Closed jimmyR closed 6 months ago

jimmyR commented 2 years ago

bitbucket_branch_restriction doesn't handle "users" changes properly, whereas bitbucket_default_reviewers does.

This is caused by the fact that bitbucket_default_reviewers uses account "uuid", whereas bitbucket_branch_restriction uses account "username".

The "username" isn't returned in the user list associated with a branch restrictions using the bitbucket API. So it's not possible for terraform to correlate the users associated with a branch restriction to the users defined in the terraform configuration, as one is a list of usernames and the other a list of uuids.

Terraform Version

Terraform v1.0.4 on darwin_amd64 provider registry.terraform.io/DrFaust92/bitbucket v2.13.1

Affected Resource(s)

bitbucket_branch_restriction

Terraform Configuration Files

resource "bitbucket_branch_restriction" "foo_restrict_merges" {
  owner      = "foo"
  repository = bitbucket_repository.foo.name
  kind = "restrict_merges"
  pattern = "main"
  users = ["john","paul","ringo","george"]
  branch_match_kind = "glob"
  value = 0
}

Expected Behavior

terraform handles user changes to branch restrictions in the same manner as default reviewers.

Adding and removing users, both in terraform and behind terraforms back, results in no errors.

Removing users from a branch restriction behind terraforms back, followed by running a terraform apply, reverts the user list to the correct state as defined in the terraform configuration

Actual Behavior

terraform throws an error, caused by pushing a list of bitbucket.Account types, where a list of strings should be

❯ TF_LOG=DEBUG terraform plan 2>&1 | grep ERROR
2022-03-29T18:45:07.710+0100 [INFO]  provider.terraform-provider-bitbucket_v2.13.1: 2022/03/29 18:45:07 [ERROR] setting state: users.0: '' expected type 'string', got unconvertible type 'bitbucket.Account', value: '{user 0xc0001d2bd0  Bob Fox  0001-01-01 00:00:00 +0000 UTC {abcdef10-2345-6789-1234-1234567890ab} false}': timestamp=2022-03-29T18:45:07.710+0100

If users are added or removed to a branch restriction behind terraforms back using the bitbucket web UI, terraform will not add or remove users added to the branch restriction list.

Steps to Reproduce

Create a branch restriction on any repo in bitbucket that requires a users list

Apply the change using terraform using TFLOG=DEBUG

An error is thrown indicating that a bitbucket.Account cannot be returned when a string was expected

Important Factoids

"users" is defined as schema.TypeSet, which I believe is a []string but "users" is passed []bitbucket.Account not []string, resulting in a consistent error.

Converting "users" to a list of uuids (strings), resolves the error:

       users := make([]string, 0, len(brRes.Users))

       for _, item := range brRes.Users {
               users = append(users, item.Uuid)
       }

        d.SetId(string(fmt.Sprintf("%v", brRes.Id)))
        d.Set("kind", brRes.Kind)
        d.Set("pattern", brRes.Pattern)
        d.Set("value", brRes.Value)
        d.Set("users", users)

However, the impact on this is that the users field in terraform config now needs to be of type uuid, and not username. However, a list of usernames is impossible (as far as I can see) to correlate anyway, so only uuids make sense (I think).

As such I changed from bitbucket.Account.Username to bitbucket.Account.Uuid

        for _, item := range d.Get("users").(*schema.Set).List() {
                account := bitbucket.Account{
                       Uuid: item.(string),
                }

with

 type User struct {
       Uuid string `json:"username,omitempty"`
 }

Using this code, the error is no more, and user list management on branch restrictions starts working correctly, the same as default reviewers.

blbradley commented 2 years ago

can you make a PR? please

blbradley commented 2 years ago

@DrFaust92 how do you feel about this solution?

blbradley commented 2 years ago

i was able to get this to work with the username, so i don't really feel like this is an issue.

Nkmol commented 6 months ago

I tried to reproduce this issue, but was unable to reproduce it:

All seems to work.

The reason for the mentioned code, is that the Bitbucket API accepts a user object that can contain the uuid and/or the username property. The code transform the input of a string array to this object, so the Bitbucket API correctly accepts this value.

The current shortcomings of this resource are:

  1. it does not support UUID → See https://github.com/DrFaust92/terraform-provider-bitbucket/issues/173
  2. It does not support an array of groups, currently only one group can be applied to a restriction.

The only action I can think of is to add the username part to the example documentation, so it is clear no UUID are used, but the Bitbucket username.