Pipeline display is broken on pull requests from forks

Summary

In the following situation:

• A pull request is sent from a fork of a project.
• The repo has not enabled the build pipeline for forks.

Steps to reproduce

Go to the pull request, you'd see a broken build:

What is the current bug behavior?

clicking on the pipeline icon will result in a 404 page, clicking on the pipeline details will open an empty dropdown and print a red banner on top of the page: "Something went wrong on our side."

What is the expected correct behavior?

It should not show a build, as there is none, as builds are disabled for forks.

Output of checks

This bug happens on GitLab.com. E.g. here: https://gitlab.com/uTox/uTox/merge_requests/790/diffs

/cc @filipa

added frontend ~12980 forking ~13921 reproduced on GitLab.com labels

@cebe @TheGrayHatter Thanks for the detailed report!

added Next Patch Release regression labels

mentioned in issue #27926 (closed)

I see 2 bugs:

• dropdown must be closed when we receive an error
• we should only show the dropdown if we have builds.

Although point 2 is still possible in the MR view since it's still in haml, I believe it's not that straight forward in the tables

@grzesiek is it possible we have this info for each stage? (I believe the original design covered this scenario, where only some of the stages would have a dropdown - @dimitrieh )

It appears that we have again a target_project / source_project mismatch somewhere. We probably need pipelines for merge requests to solve that once and for all.

We probably need a backend developer to fix that /cc @ayufan

changed milestone to %9.0

This problem exists on both merge request and commit details pages. The reason is probably that we only check for the existence of a pipeline, which should not be there is pipelines are disabled for a project. In this case, someone probably had pipelines enabled, so there was a pipeline created for given commit, and this is why we show info about the pipeline in the widget. Pipeline is in the database, but pipelines are disabled in th fork, so we should not show pipeline info in merge request widget.

assigned to @psimyn

@grzesiek should that apply for past pipelines as well? e.g. if I have a pipeline run on a branch, and then disable pipelines for the project. If that's the case, checking the project for builds_enabled in the template should be enough.

For commit detail pages

Before:

After:

There is also a link (the tick) in the commit list view. Should that be hidden if disabled, since it'll always 403/404?

If I haven't grossly misunderstood issue, I'll update description to match this

@psimyn This is an interesting solution, but there is one thing we should clarify - commit can also have an external status. According to our current approach user can have internal pipelines disabled, but still use external commit status API. This effectively adds a internal pipeline object, which we then check in the partial (if @pipeline). This is confusing, and - as we can see - quite error prone.

I think we should keep this logic for now, and it is probably okay to have pipelines disabled and still use external commit status API (please correct me if I'm wrong @markpundsack).

We currently do not have a way to check if pipeline is "external". I wonder if this would make sense to add Ci::Pipeline#external? method but it might be a little cumbersome to implement it, since our business rules are not completely clear:

1. Pipeline can have both internal and external jobs.
2. There is no distinction between internal and external pipeline yet.
3. User can have internal pipelines disabled, yet create a pipeline using an external commit status API.

I wonder if we just should mark pipeline as external when it is created using external commit status API. We do not do this now. What do you think @ayufan ?

@grzesiek I like the idea of checking for the existence of pipelines rather than if CI/CD is enabled because, as you point out, people can have external CI status even with it disabled. Also, disabling CI/CD doesn't erase old pipelines, so continuing to show links to old pipelines makes sense. I can see the other side of it though, where it's confusing to see a Pipelines tab when it's disabled. Especially for the project-level Pipelines tab (as opposed to the MR pipelines tab).

I'm not sure what the long-term solution is though. Marking pipelines/jobs as external might be part of it, but that feels deficient. Today, an external job run with a GitLab CI/CD pipeline shows them together in the same pipeline. Maybe we should explicitly separate them, and then you can mark the entire pipeline as external or not. Even so, where do they show up? If they're still showing up in the Pipelines tab, then we're back to checking for existence of external pipelines rather than a single boolean. Seems sloppy. Well, unless applied consistently.

removed assignee

@psimyn Perhaps MR.has_ci? is what we need. It was recently updated to handle internal and external builds, https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9245/diffs#ef51b30fcc35462117070f3fe498d3949c279cb9_686_686

assigned to @psimyn

I wasn't able to replicate this with a fork. previously I was using existing MRs, and then disabling Pipelines for that repo. I think the underlying issue is the same

@markpundsack while disabling doesn't erase old pipelines, it can change permissions of who can view them(?). For example, changing Pipelines setting from "Everyone with access" to "Only team members" or "disabled". The Pipeline still exists on /pipelines/, will 403 for this user. I think we'd want that same logic for the MR widget header - don't display the status at all for people without permission (or if pipeline shouldn't exist).

@MadLittleMods MR.has_ci? is in the template already, pretty sure it is also affected by this. Unless it can be used in conjunction with the if @pipeline to get more info.

removed assignee

Another somewhat related issue we've come across is how to run and test bulids from external users?

I have a CI system set up, one of them using a shell runner set to manual only. For small trivial patches, I can't see any reason we shouldn't be able to run it though our pipeline. This could be as simple as adding a button, Run on internal pipline. The only problem with this is if the user changes the manual job to auto. Then they'd be able to get access to the shell runner. So while ideally, that button would be a toggle, so further pushes will automatically run on our CI systems, if they then change the .gitlab-ci.yaml the could do damage. So perhaps a button that would run the CI using ONLY the gitlab-ci config from the default branch?

I was searching for a similar issue and found #23902 (moved) which brought me back here. But because this is somewhat related to both, I figured I'd start here, and see if you'd rather see another issue for this feature request?

mentioned in issue #30038 (closed)

added ~19173 and removed frontend labels

mentioned in issue #2456 (moved)

mentioned in issue #31335 (closed)

@fatihacet Has the new MR widget fixed this?

changed milestone to %9.2

assigned to @fatihacet

@fatihacet @filipa Is that fixed?

@ayufan @fatihacet I don't think this was fixed already.

@fatihacet are you working on this?

@filipa nope, this is not something on my radar.

removed assignee

@grzesiek is there any way we can get this info on the API? So we don't render this?

@filipa Whether we have stages to render? This seems that it should be handled already. Also I have a MR that improves that support for existing pipelines.

@filipa @fatihacet I'm not working on that now. This will be part of External CI services MR.

changed milestone to %9.3

removed Next Patch Release label

removed regression label

added Deliverable label

assigned to @ayufan

mentioned in merge request !11176 (merged)

closed via merge request !11176 (merged)

mentioned in commit da66c90b

mentioned in commit e5e2d90a

mentioned in commit 188abebb