Skip to content

Reporting build status to Gitlab Merge Request doesn't work for forks #7370

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

Open
maxking opened this issue Aug 7, 2020 · 14 comments
Open

Reporting build status to Gitlab Merge Request doesn't work for forks #7370

maxking opened this issue Aug 7, 2020 · 14 comments
Labels
Bug A bug Needed: design decision A core team decision is required

Comments

@maxking
Copy link

maxking commented Aug 7, 2020

Details

I have enabled building PRs for our Gitlab project (https://gitlab.com/mailman/mailman) and have gone through all the steps to connect the Gitlab account in my Readthedocs account. However, the projects that I have imported are using 'mailman' gitlab gruop, that I have admin rights on.

However, I always get

 Could not send GitLab build status report for Mailman Core. Make sure you have the correct GitLab repository permissions and your GitLab account is connected to ReadtheDocs. 

When I visit readthedocs.org.

Expected Result

I expected the build results to be reported via Pipelines.

Actual Result

Nothing is reported in the pipelines page, although, the Build does get triggerred on Readthedocs and documentation is available to browse if you manually visit the builds page and get the URL to the PR docs.

@stsewd
Copy link
Member

stsewd commented Aug 13, 2020

Are GitLab groups the same as organizations in GitHub? or are they a subgroup of organizations?

@stsewd
Copy link
Member

stsewd commented Aug 13, 2020

Ok, I think this is bug from our side, we received the merge request event, but we processed it like a push event instead of a merge request event.

@stsewd stsewd added the Bug A bug label Aug 13, 2020
@stsewd
Copy link
Member

stsewd commented Aug 13, 2020

My above comment isn't right, the merge request was closed, so it was processed according to that. Looks like builds are working, but the status isn't being reported. I'll dig more into this.

@maxking
Copy link
Author

maxking commented Aug 13, 2020

Are GitLab groups the same as organizations in GitHub? or are they a subgroup of organizations?

They are like organizations on Github. You can see the one we are using at https://gitlab.com/mailman

And yes, the builds are indeed working. It is just the status that isn’t being reported.

@stsewd
Copy link
Member

stsewd commented Aug 13, 2020

btw, your project is building from https://gitlab.com/mailman/mailman-suite-doc/-/merge_requests, the MR you linked seems from another project.

@stsewd
Copy link
Member

stsewd commented Aug 13, 2020

I checked the logs, and we are sending the status for some projects https://gitlab.com/daltonproject/daltonproject/-/merge_requests/140. Your project is failing here

if resp.status_code in [401, 403, 404]:
log.info(
'GitLab project does not exist or user does not have '
'permissions: project=%s',
project,
)
return False

@maxking
Copy link
Author

maxking commented Aug 13, 2020

Well, the project is Mailman Core, which is a sub-project of Mailman Suite. All the builds for Mailman Core project are in RTD at https://readthedocs.org/projects/mailman/builds/ and the corresponding merge requests are at https://gitlab.com/mailman/mailman/-/merge_requests.

I am sorry I gave you a wrong project url, it should be https://readthedocs.org/projects/mailman/. (I’ll edit the issue description too).

Also, thanks for the pointer to the code where it fails. Seems like Gitlab is retuning permission errors indeed, and I am not sure if that is because the scope of the token requested by RTD doesn’t cover the project that it is trying to send status for or something else. I do have owner permissions on all the projects under Mailman group on Gitlab. I’ll also look up some of the Gitlab docs to see if there is scope an issue.

@maxking
Copy link
Author

maxking commented Aug 13, 2020

It seems like the api access level, which RTD requests and has access to, should grant it ability to work with the groups according to their docs.

@maxking
Copy link
Author

maxking commented Aug 13, 2020

It would be good to know the exact status code of the response between 401, 403 and 404. My suspicion is more on 404 and not 403 because looking a the code, the URL that you POST the status to url}/api/v4/projects/{repo_id}/statuses/{commit} would only work if the PR was created using the branch on the main repo.

For forks, which is true in my case, repo_id would be different and POSTing the status would result in 404 since the {commit} doesn’t exist in the main repo that RTD is configured with.

There are two ways to go about this:

  • You try to get the repo_id of the fork the MR was created from and try to POST the status to that repo, which would fail in most cases since the user whose access token RTD holds, will not have access to any other forks other than their own.
  • You simply do not send build status for the MRs created from forks, which you can do by comparing the source and target in the webhook event that you get from Gitlab.

It would also be nice to not show a warning in RTD in this particular situation that it failed to POST a response due to auth errors since that simply leads the users to believe it can be fixed by re-linking Gitlab account and syncing webhooks.

stsewd added a commit that referenced this issue Aug 13, 2020
* External providers: better logging for GitLab

Ref #7370
@stsewd
Copy link
Member

stsewd commented Aug 13, 2020

So, the message you're seeing in the dashboard is created when you sync/create the webhook, it disappears by clicking on it.

For forks, which is true in my case, repo_id would be different and POSTing the status would result in 404 since the {commit} doesn’t exist in the main repo that RTD is configured with.

Ok, that's interesting I'll look more into that.

About the status code, I just merged a PR to have more information in the logs #7385, it will be live by next week.

@maxking
Copy link
Author

maxking commented Aug 13, 2020

I sort of verified my theory by simply create a PR from the same main repo and seeing if RTD is able to POST the status back to Gitlab and it does.

See MR: https://gitlab.com/mailman/mailman/-/merge_requests/683 and RTD build at: https://readthedocs.org/projects/mailman/builds/11658674/

@maxking
Copy link
Author

maxking commented Aug 13, 2020


IMG_0075

And the warning that I was talking about is this, which is created after every build that it couldn’t POST the response to when you login to your RTD account. It would be nice if this happened only when the status was 403.

@stsewd
Copy link
Member

stsewd commented Aug 13, 2020

Thanks for the additional info! So, when a project isn't public a 404 can be returned, so that's why we catch those too.

@stsewd stsewd changed the title Reporting build status to Gitlab Merge Request doesn't work for group projects Reporting build status to Gitlab Merge Request doesn't work for forks Aug 13, 2020
@saadmk11
Copy link
Member

saadmk11 commented Aug 15, 2020

I think this is a issue with GitLab. More details can be found here:
https://gitlab.com/gitlab-org/gitlab/-/issues/27636

@stsewd stsewd added the Needed: design decision A core team decision is required label Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug Needed: design decision A core team decision is required
Projects
None yet
Development

No branches or pull requests

3 participants