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

use GetRoleName() instead of permission mapping func #1192

Merged
merged 2 commits into from
Jul 15, 2022
Merged

use GetRoleName() instead of permission mapping func #1192

merged 2 commits into from
Jul 15, 2022

Conversation

joshua-hancox
Copy link
Contributor

@joshua-hancox joshua-hancox commented Jun 16, 2022

Requires go-github version bump: #1205

fixes: #1183

@kfcampbell kfcampbell marked this pull request as ready for review July 15, 2022 21:45
Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this! Apologies for the delay.

@kfcampbell kfcampbell merged commit 754d73f into integrations:main Jul 15, 2022
@csainty
Copy link
Contributor

csainty commented Jul 17, 2022

This breaks the resource for non-custom roles as the naming is different. read vs pull, write vs push etc.

@Tenzer
Copy link

Tenzer commented Jul 18, 2022

I'm seeing the same as @csainty. Confusingly, around half of the repositories we manage seem to use "push" and the other half "write". It could look like the oldest repositories we have wants to use "push", so I'm guessing it might have something to do with when the repository is created.

Not having it mapped to the same name makes it difficult to use for_each to assign a list of repositories to a group with the correct permissions.

@schans
Copy link

schans commented Jul 18, 2022

Same here. This is a breaking change for use. If we keep it as is some 300 repos will be changed on every run, if we change push to write, some 450 repos will be changed every run.

We should revert or create some option to have aliases or something.

If somebody finds a way to "rename" the roles in Github, please share. That might be a solution as well.

@schans
Copy link

schans commented Jul 18, 2022

For us some of the differences seem to come from roles/permissions assigned to groups (teams), org members (collaborators) and outside collaborators.

That seems to be the difference between in the v3 api:

https://api.github.com/repos/<org>/<repo>/teams
https://api.github.com/repos/<org>/<repo>/collaborators (works only for org members)
https://api.github.com/repos/<org>/<repo>/collaborators/<user>/permission (works for outside collaborators)

Note that https://api.github.com/repos/<org>/<repo>/collaborators/<user> doesn't return anything for outside collaborators.

(fyi we only use the default roles)

Some information here https://git.luolix.topmunity/t/is-the-api-insane-with-respect-to-collaborators-or-am-i-confused/253696

@schans
Copy link

schans commented Jul 18, 2022

Created a new ticket in #1222 as comments here might get unnoticed.

@bpaquet bpaquet mentioned this pull request Jul 19, 2022
kfcampbell added a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 19, 2022
Created by running git revert -n
754d73f HEAD.
kfcampbell added a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
kfcampbell added a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 27, 2022
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
kazaker pushed a commit to auto1-oss/terraform-provider-github that referenced this pull request Dec 28, 2022
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
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.

resource_github_team_repository cannot handle custom role after creation
5 participants