Merge branch 'osw-skip-hooks-for-merge-to-ref' into 'master'

Skip hooks for UserMergeToRef RPC

See merge request gitlab-org/gitaly!1312

4 files changed, 23 insertions, 4 deletions

diff --git a/changelogs/unreleased/osw-skip-hooks-for-merge-to-ref.yml b/changelogs/unreleased/osw-skip-hooks-for-merge-to-ref.yml
new file mode 100644
index 000000000..25eab6c52
--- /dev/null
+++ b/changelogs/unreleased/osw-skip-hooks-for-merge-to-ref.yml
@@ -0,0 +1,5 @@
+---
+title: Skip hooks for UserMergeToRef RPC
+merge_request: 1312
+author:
+type: other
diff --git a/internal/service/operations/merge.go b/internal/service/operations/merge.go
index 60588c011..dd44a76be 100644
--- a/internal/service/operations/merge.go
+++ b/internal/service/operations/merge.go
@@ -3,6 +3,7 @@ package operations
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
@@ -107,6 +108,10 @@ func validateUserMergeToRefRequest(in *gitalypb.UserMergeToRefRequest) error {
 		return fmt.Errorf("empty target ref")
 	}
 
+	if !strings.HasPrefix(string(in.TargetRef), "refs/merge-requests") {
+		return fmt.Errorf("invalid target ref")
+	}
+
 	return nil
 }
 
diff --git a/internal/service/operations/merge_test.go b/internal/service/operations/merge_test.go
index 44e875b65..b9a0be004 100644
--- a/internal/service/operations/merge_test.go
+++ b/internal/service/operations/merge_test.go
@@ -616,6 +616,15 @@ func TestFailedUserMergeToRefRequest(t *testing.T) {
 			code:      codes.InvalidArgument,
 		},
 		{
+			desc:      "invalid target ref",
+			repo:      testRepo,
+			user:      mergeUser,
+			branch:    []byte(branchName),
+			sourceSha: commitToMerge,
+			targetRef: []byte("refs/heads/branch"),
+			code:      codes.InvalidArgument,
+		},
+		{
 			desc:      "non-existing branch",
 			repo:      testRepo,
 			user:      mergeUser,
@@ -644,7 +653,7 @@ func TestFailedUserMergeToRefRequest(t *testing.T) {
 	}
 }
 
-func TestUserMergeToRefFailedDueToHooksRequest(t *testing.T) {
+func TestUserMergeToRefIgnoreHooksRequest(t *testing.T) {
 	server, serverSocketPath := runOperationServiceServer(t)
 	defer server.Stop()
 
@@ -680,8 +689,8 @@ func TestUserMergeToRefFailedDueToHooksRequest(t *testing.T) {
 			defer cancel()
 
 			resp, err := client.UserMergeToRef(ctx, request)
-			require.Nil(t, err)
-			require.Contains(t, resp.PreReceiveError, "failure")
+			require.NoError(t, err)
+			require.Empty(t, resp.PreReceiveError)
 		})
 	}
 }
diff --git a/ruby/lib/gitlab/git/operation_service.rb b/ruby/lib/gitlab/git/operation_service.rb
index 48559d4e7..6530ef342 100644
--- a/ruby/lib/gitlab/git/operation_service.rb
+++ b/ruby/lib/gitlab/git/operation_service.rb
@@ -120,7 +120,7 @@ module Gitlab
 
         oldrev = from_branch.target
 
-        update_ref_in_hooks(ref, newrev, oldrev)
+        update_ref(ref, newrev, oldrev)
 
         newrev
       end