Skip to content

Pre-receive error messages in /internal/allowed API check are not propagated in UserMergeBranch

From gitlab#30141 (comment 481566895), when a file is locked and a user attempts to merge a locked file/directory, we no longer give proper feedback to the user. Instead, the user sees the incorrect message, Conflicts detected during merge:

image

This is actually been a serious problem we worked out in the Ruby implementation because for numerous customers, this mysterious failure costed a lot of engineering time to troubleshoot.

In https://gitlab.com/gitlab-org/gitaly/blob/b0bc161d3f4faa3c4a23da73e4f29632519b8586/internal/gitaly/hook/prereceive.go#L113-116, the GitLab API allowed call receives a message, and this is returned with a clean error message:

GitLab: The path 'test' is locked by Administrator

However, in https://gitlab.com/gitlab-org/gitaly/blob/8e8146470716158e12cf06d7997c47dcb2b96bb9/internal/gitaly/service/operations/update_with_hooks.go#L65-68, we look for the error in the stdout and stderr stream, but these are empty because the error came from the network response, not any external command.

I verified this patch fixes the issue:

diff --git a/internal/gitaly/service/operations/update_with_hooks.go b/internal/gitaly/service/operations/update_with_hooks.go
index 55021714b..299f05516 100644
--- a/internal/gitaly/service/operations/update_with_hooks.go
+++ b/internal/gitaly/service/operations/update_with_hooks.go
@@ -64,6 +64,9 @@ func (s *Server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Re
 
 	if err := s.hookManager.PreReceiveHook(ctx, repo, nil, env, strings.NewReader(changes), &stdout, &stderr); err != nil {
 		msg := hookErrorFromStdoutAndStderr(stdout.String(), stderr.String())
+		if msg == "" {
+			msg = err.Error()
+		}
 		return preReceiveError{message: msg}
 	}
 	if err := s.hookManager.UpdateHook(ctx, repo, reference, oldrev, newrev, env, &stdout, &stderr); err != nil {

This is okay because GitLab Rails will filter any messages that don't start with GitLab:.

Another option may be to create a special error, NotAllowedError, and then treat that as a special case.

Edited by Stan Hu
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information