Skip to content

Ensure MergeRequestDiff is valid after creating it

In https://gitlab.com/gitlab-org/gitlab-ce/commit/ebf16ada856efb85424a98848c141f21e609886a we introduced a SHA validator to ensure that the data provided in merge request diffs when importing, was legit. But, in https://gitlab.com/gitlab-org/gitlab-ce/issues/58804 we realized that, even if the MergeRequestDiff validation fails, a default ("valid") record was added to a MergeRequest when it was created.

"Valid" doesn't mean that the values are right, it only means that the validation process doesn't fail. For example, a "valid" MergeRequestDiff can be a record with base_commit_sha, head_commit_sha, start_commit_sha, or commits_count set to nil. These attributes are basic and we use them in several places assuming they are not nil.

We should replace the save method in save_git_content with save!, and then handle that exception in all those places where it can be raised. Besides, we should check the importers, because if the MergeRequestDiff is not valid we should abort the creation of the MergeRequest. For example, in the Bitbucket importer we first create the MergeRequest but, if any exception is raised, the MergeRequest is not deleted.

/cc @DouweM

Edited by Francisco Javier López