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

github_repository_collaborator unable to validate triage and maintain permissions #480

Closed
HorizonNet opened this issue May 31, 2020 · 11 comments
Labels
r/repository_collaborator Status: Needs info Full requirements are not yet known, so implementation should not be started Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented
Milestone

Comments

@HorizonNet
Copy link

Terraform Version

v0.12.24

Provider Version

v2.8.0

Affected Resource(s)

  • github_repository_collaborator

Terraform Configuration Files

data "github_membership" "test-user" {
  username = "testuser"
}

resource "github_repository" "my-repo" {
  name        = "my-repo"
  private     = true
  description = ""
}

resource "github_repository_collaborator" "my-repo-test-user" {
  repository = github_repository.my-repo.name
  username   = data.github_membership.test-user.username
  permission = "maintain"
}
terraform import github_repository_collaborator.my-repo-test-user my-repo:testuser

Expected Behavior

After importing an existing collaborator with Maintain permissions the collaborator should have maintain permissions in the state.

Actual Behavior

After importing an existing collaborator with Maintain permissions the collaborator gets push permissions in the state. Running terraform plan afterwards notes a replacement.

# github_repository_collaborator.my-repo-test-user must be replaced
-/+ resource "github_repository_collaborator" "my-repo-test-user" {
      ~ id            = "my-repo:testuser" -> (known after apply)
      + invitation_id = (known after apply)
      ~ permission    = "push" -> "maintain" # forces replacement
        repository    = "my-repo"
        username      = "testuser"
    }

Running terraform apply in the end leads to

Error: User testuser is already a collaborator

Overall, the error is similar to #469, but the actual problem seems to be the import to the state in this situation.

Steps to Reproduce

  1. terraform import github_repository_collaborator.my-repo-test-user my-repo:testuser
  2. terraform plan
  3. terraform apply
@anGie44
Copy link
Contributor

anGie44 commented Jun 1, 2020

hi @HorizonNet, thanks for writing-up this issue! I was able to reproduce and looking into this more, I think we're bound by the limitations of the github API (atleast in v3...in v4 I'm able to retrieve the "maintain" permissions for my user from the GraphQL explorer), such that the permission levels provided on request to create a github collaborator are not always equal to the permission levels returned from the github API.

while an invitation to collaborate in a repository can be read(aka pull in this provider), write (aka push in this provider), maintain, triage, or admin (and the invite's permission level can be read correctly with the List API call), once that user accepts that invitation (like in the case you've presented), the permission levels we can read from the github API (using the List Collaborators endpoint as done here in the provider) are only push, pull, or admin. thus, I believe we'll perpetually get this diff in cases where the intended permission is either maintain or triage. it's as if there's not a 1:1 mapping of repository invitation permission level to repository permission level from the github API's perspective.

with this being said, perhaps one way to suppress this diff will be to apply a DiffSuppressFunc to the permission attribute in the github_repository_collaborator resource? but only for these 2 permissions (maintain and triage) such that at terraform plan time, if the permission value determined from the Github API is push while we expected maintain or pull when we expected triage, then the diff will be suppressed, and a non-empty plan will be returned.

@anGie44
Copy link
Contributor

anGie44 commented Jun 1, 2020

hmm this issue also seems to bring up something we can further validate in the acceptance tests such that atm we're validating the cases where we invite a new collaborator and check against the invite's permissions (which can contain push, pull, maintain, triage, or admin), but we haven't simulated cases where a user accepts the invite and then refresh a plan accordingly @jcudit

@HorizonNet
Copy link
Author

I also check the collaborators endpoints for v3 and I also see an easy way to distinguish someone with Write permissions from someone with Maintain permissions. Are there any plans already for moving to v4?

@HorizonNet
Copy link
Author

Ah, just saw the discussion over in #83.

@emmahsax
Copy link

emmahsax commented Aug 7, 2020

This is something I've also run into. Very similar thing. Terraform wants to reapply this change every time, even though the user already has maintain access to the repository.

  # module.repository.github_repository_collaborator.users["example-user"] must be replaced
-/+ resource "github_repository_collaborator" "users" {
      ~ id            = "repository:example-user" -> (known after apply)
      + invitation_id = (known after apply)
      ~ permission    = "push" -> "maintain" # forces replacement
        repository    = "repository"
        username      = "example-user"
    }

@bharathkkb
Copy link

I am also getting a diff with triage. User does have triage access to repo.

      ~ permission    = "pull" -> "triage" # forces replacement

@jcudit jcudit added Type: Bug Something isn't working as documented r/repository_collaborator labels Nov 30, 2020
@nick-hf
Copy link

nick-hf commented Dec 18, 2020

This problem still makes every single one of our plans so noisy :(

@jcudit jcudit added this to the v4.1.2 milestone Jan 4, 2021
@jcudit
Copy link
Contributor

jcudit commented Jan 29, 2021

Took another pass at this and agree that:

  • The GitHub REST API is limiting us to only validating pull, push and admin 😢
  • The GraphQL API currently has no workarounds available for querying triage and maintain 😢
  • The interactive nature of this resource limits our testing 😢

Open to adding diff suppression here if that helps while we wait for a better API to become available. Aiming to get something up for review targeting our next release.

@jcudit jcudit changed the title github_repository_collaborator imports wrong state github_repository_collaborator unable to validate triage and maintain permissions Jan 29, 2021
@jcudit
Copy link
Contributor

jcudit commented Jan 29, 2021

☝🏾 could use some 👀 for anyone watching this.

digitalronin added a commit to ministryofjustice/github-collaborators that referenced this issue Feb 10, 2021
Also, remove mentions of "triage" and "admin" permissions from the
documentation and examples - they don't work, because of [this
bug](integrations/terraform-provider-github#480 (comment))
in the GitHub API.
@github-actions
Copy link

github-actions bot commented Dec 9, 2022

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Dec 9, 2022
@kfcampbell kfcampbell added Priority: Normal Status: Needs info Full requirements are not yet known, so implementation should not be started labels Dec 9, 2022
@github-actions github-actions bot removed the Status: Stale Used by stalebot to clean house label Dec 10, 2022
Copy link

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Apr 24, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r/repository_collaborator Status: Needs info Full requirements are not yet known, so implementation should not be started Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented
Projects
None yet
Development

No branches or pull requests

8 participants