Increase the range of scenarios where merge request reports appear correctly in merge requests, and improve error messages/documentation when there are problems
Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.
Release notes
Problem to solve
As a creator or reviewer of merge requests, I want to reliably see all the merge request reports (e.g. security scans) relating specifically to the changes being reviewed so that I can merge changes confidently.
Intended users
Any users of merge requests (e.g. Cameron, Sasha, Sam,...)
User experience goal
The current AutoDevops merge request reports that are intended to show improvements or degredations caused by the changes being considered (e.g. SAST or License Management) should be displayed correctly in more circumstances and, when they are not available, there should be clearer messaging about what the user can do to enable the reports.
Proposal
Often some AutoDevops merge request reports do not appear in merge requests. Alternatively, sometimes they appear with error messages, or apear with warnings that they apply to the source branch only, and so are not as useful as a report that addresses the difference between source and target branches. I have identified two possible reasons for this below, but I am reluctant to create separate issues for each one because I might be wrong, and it might be easier to address all both issues together.
Additionally, I think the inconsistent behaviour between the different reports when one or both artifacts cannot be retrieved should be improved. Some reports don't appear at all (e.g. the Browser Performance Testing). I think this is particularly dangerous: someone scanning the MR sees all green ticks and remains unaware of a problem running the browser performance tests and inadvertently approves the MR. I think the report should appear with an error message explaining why it cannot be displayed. Code quality does this already, although in that case the message could be improved. The license scanning report appears even if it can only scan the source branch: I think the message should be made clearer that something has gone wrong and that it is falling back on showing the full list of licenses on the source branch rather than the usual diff.
Further details
I think the situations I have seen that cause reports not to be displayed correctly are due to two causes; these are given below with some suggested solutions:
1. The a job needed to populate a MR report is not included on the most recent version of the target (i.e. master, usually) pipeline
This happens frequently (in fact, most of the time) because in our team we run a scheduled pipeline every night on master that just runs the security jobs in order to populate the project and group security dashboards. This nightly master pipeline does not contain License Scanning, Code Quality or Browser Performance Testing (because we don't need to run these pipelines unless code has changed).
In this case:
- we get a version of the License Scanning report in the MR that only lists licences on the source branch, rather than new licences
- there is an error loading the Code Quality report
- the Browser Performance Testing report does not appear at all.
I believe the current GitLab algorithm is to get the target (i.e. master, usually) report artifact from the most recent pipeline on that target branch. I wonder if this can be made more sophisticated, so that it checks the most recent pipeline of the most recent master commit that possesses the relevant job. Another option would be to allow the user to configure which pipeline on the target branch (and possibly source branch too) should be used, either in the UI or in the .gitlab-ci.yml file. Another option might be to somehow be able to mark pipelines as exempt from the above algorithm, so GitLab can be told to ignore, for example, nightly scheduled pipelines.
Currently we plan to work around the limitations described above by adding all relevant jobs to our nightly pipeline, but obviously this is not ideal. And even if the behaviour cannot be improved along the lines I suggest above or some other way, I think there is scope to improve the messaging when the merge request reports when things go wrong, e.g. with a tooltip explaining why the report could not be loaded, perhaps with a link to the offending pipeline. Also, the documentation could be improved to explain the algorithm in more detail and highlight this potential pitfall. It took me quite a lot of experimentation to infer why reports weren't appearing.
2. The relevant job is included in the most recent pipelines of the source and target branches, but other jobs on one or both of those pipelines either fail or have not run yet because they are manual jobs
In branch pipelines, our team runs a custom manual job (for UI testing) that is not allowed to fail. The pipeline runs up to that job and stops with no failures. But because this pipeline has effectively not finished, the merge request cannot obtain the security report artifacts, for example, and we get errors loading those reports in the merge request. We can work around this limitation by marking the manual job as "allowed to fail".
I don't see the logic in not collecting artifacts from pipelines that have not completed. I would argue that security scans (for example) are independent of UI tests. If this behaviour must remain as it is though, the messages in the reports could be improved to explain what has happened, and the documentation should talk through these kinds of scenarios.
Permissions and Security
Documentation
Some proposed documentation changes are given in the Proposal.