Packit-service: Members of a GH organization with write access can't use packit

[bocekm]

@fellipeh has created this PR https://github.com/oamg/convert2rhel/pull/39 but packit says Only users with write or admin permissions to the repository can trigger Packit-as-a-Service even though he has the write access to the repository.

You can see on the following screenshot that 21 members (of the oamg organization) have the write access:

When I click on the Manage button on the page above, I see that the members of oamg org have really the write permissions:

Here's a screenshot showing that @fellipeh is a member of the oamg org:

[csomh]

Thanks @bocekm for filing this. Here is what I've found with some debugging.

Using Packit's GitHub credentials, I've created a project using ogr's GithubProject class. Packit Service currently is using an object like this to tell who can merge PRs in a project. Calling the same method gave:

>>> len(project.who_can_merge_pr())
7

But as we discussed, this is wrong.

Calling PyGithub's API, or the GitHub API directly (with the same credentials) gave the same results.

But calling get_collaborator_permission() from PyGithub tells that @fellipeh has indeed write access to the repo:

>>> project.github_repo.get_collaborator_permission("fellipeh")
'write'

:exploding_head:

I would say, that checking if the user is in the list of collaborators (and retrieving the list for that) is a little bit overcomplicated, especially if there is an API to tell the permission of the user :slightly_smiling_face:

As for why the list of collaborators returned by the API is not complete: :man_shrugging: . I would say this is a feature to prevent GitHub Apps scraping for usernames, but this is just a guess.

Anyways, we'll look into fixing this next week.

[bocekm]

Thanks for looking into that. The number 7 is really strange. There're 2 users I've invited manually to collaborate on the project (not members of the oamg group), and then those 21 members of the oamg org who all have write access to the project. So, I would expect the number to be 2, or 23. Not 7.

There's the workaround of adding Fellipe manually to the list of collaborators specifically invited to the project. That should work. But I guess it'd be better for packit to resolve this as it might happen to other projects as well.

[bocekm]

@fellipeh, please try commenting \packit test under the https://github.com/oamg/convert2rhel/pull/39 again.

[fellipeh]

@bocekm same error:

Starting to think that blue robot doesn't like me.. ;)

[TomasTomecek]

@bocekm @fellipeh even though this seems to be fixed in our codebase, I'm pretty sure the change is not deployed to production. It will be up on Monday May 18th, unless @csomh decides to deploy it sooner.

[csomh]

Yeah, that's right, new deployments usually happen on Monday morning, but we might have one sooner today. Anyways, I did not forget about this issue and I'll keep you posted.

(We will think about how to make this more transparent for you guys.)

[bocekm]

Ah, ok, thanks for letting us know. No hurry.

[jpopelka]

Production has just been redeployed and should include the fix/change now.

[fellipeh]

Production has just been redeployed and should include the fix/change now.

@jpopelka Still failing..

[jpopelka]

Still failing..

Indeed, I was just told (@csomh) that we hadn't pulled in the ogr change so the production is not yet fixed, sorry for the noise.

Ah, ok, thanks for letting us know. No hurry.

Given it's Thursday already, let's wait for the regular weekend's re-deployment.
EDIT: @csomh is going to do the re-deployment today

[csomh]

@fellipeh please give it another try :crossed_fingers:

[fellipeh]

@csomh Now it's failing only on 2 actions

One question: the packit action depends of pylint actions?

[csomh]

@fellipeh That's weird and probably is related to #535.

I think the failures are left-overs from previous runs. Could you try /packit build and see if they are fixed? Pushing to the branch would have a similar effect (that would re-trigger both builds and tests).

[fellipeh]

@csomh Works fine now.. I could run without issue:

[bocekm]

@csomh, thanks!