RepositoryService::WriteRef

7 files changed, 208 insertions, 6 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fdd245505..3252d3b34 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,8 @@ UNRELEASED
 
 - Handle non-existent commits in ExtractCommitSignature
   https://gitlab.com/gitlab-org/gitaly/merge_requests/535
+- Implement RepositoryService::WriteRef
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/526
 
 v0.69.0
 
diff --git a/internal/git/helper.go b/internal/git/helper.go
index 30e9755ae..1904e9f86 100644
--- a/internal/git/helper.go
+++ b/internal/git/helper.go
@@ -26,7 +26,12 @@ func ValidateRevision(revision []byte) error {
 	if bytes.HasPrefix(revision, []byte("-")) {
 		return fmt.Errorf("revision can't start with '-'")
 	}
-
+	if bytes.Contains(revision, []byte(" ")) {
+		return fmt.Errorf("revision can't contain whitespace")
+	}
+	if bytes.Contains(revision, []byte("\x00")) {
+		return fmt.Errorf("revision can't contain NUL")
+	}
 	return nil
 }
 
diff --git a/internal/service/repository/write_ref.go b/internal/service/repository/write_ref.go
index 2369499fe..61726d3f1 100644
--- a/internal/service/repository/write_ref.go
+++ b/internal/service/repository/write_ref.go
@@ -1,13 +1,51 @@
 package repository
 
 import (
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"bytes"
+	"fmt"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
 
 	"golang.org/x/net/context"
 )
 
 func (s *server) WriteRef(ctx context.Context, req *pb.WriteRefRequest) (*pb.WriteRefResponse, error) {
-	return nil, helper.Unimplemented
+	if err := validateWriteRefRequest(req); err != nil {
+		return nil, grpc.Errorf(codes.InvalidArgument, "WriteRef: %v", err)
+	}
+
+	client, err := s.RepositoryServiceClient(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	clientCtx, err := rubyserver.SetHeaders(ctx, req.GetRepository())
+	if err != nil {
+		return nil, err
+	}
+
+	return client.WriteRef(clientCtx, req)
+}
+
+func validateWriteRefRequest(req *pb.WriteRefRequest) error {
+	if err := git.ValidateRevision(req.Ref); err != nil {
+		return fmt.Errorf("Validate Ref: %v", err)
+	}
+	if err := git.ValidateRevision(req.Revision); err != nil {
+		return fmt.Errorf("Validate Revision: %v", err)
+	}
+	if len(req.OldRevision) > 0 {
+		if err := git.ValidateRevision(req.OldRevision); err != nil {
+			return fmt.Errorf("Validate OldRevision: %v", err)
+		}
+	}
+
+	if !bytes.HasPrefix(req.Ref, []byte("refs/")) {
+		return fmt.Errorf("Ref has to be a full reference")
+	}
+	return nil
 }
diff --git a/internal/service/repository/write_ref_test.go b/internal/service/repository/write_ref_test.go
new file mode 100644
index 000000000..18196076a
--- /dev/null
+++ b/internal/service/repository/write_ref_test.go
@@ -0,0 +1,145 @@
+package repository
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"google.golang.org/grpc/codes"
+)
+
+func TestWriteRefSuccessful(t *testing.T) {
+	server, serverSocketPath := runRepoServer(t)
+	defer server.Stop()
+
+	client, conn := newRepositoryClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	testCases := []struct {
+		desc string
+		req  *pb.WriteRefRequest
+	}{
+		{
+			desc: "rugged update refs/heads/master",
+			req: &pb.WriteRefRequest{
+				Repository: testRepo,
+				Ref:        []byte("refs/heads/master"),
+				Revision:   []byte("4a24d82dbca5c11c61556f3b35ca472b7463187e"),
+				Shell:      false,
+			},
+		},
+		{
+			desc: "shell update refs/heads/master",
+			req: &pb.WriteRefRequest{
+				Repository: testRepo,
+				Ref:        []byte("refs/heads/master"),
+				Revision:   []byte("b83d6e391c22777fca1ed3012fce84f633d7fed0"),
+				Shell:      true,
+			},
+		},
+		{
+			desc: "shell update refs/heads/master w/ validation",
+			req: &pb.WriteRefRequest{
+				Repository:  testRepo,
+				Ref:         []byte("refs/heads/master"),
+				Revision:    []byte("498214de67004b1da3d820901307bed2a68a8ef6"),
+				OldRevision: []byte("b83d6e391c22777fca1ed3012fce84f633d7fed0"),
+				Shell:       true,
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+			_, err := client.WriteRef(ctx, tc.req)
+
+			require.NoError(t, err)
+
+			rev := testhelper.MustRunCommand(t, nil, "git", "--git-dir", testRepoPath, "log", "--pretty=%H", "-1", string(tc.req.Ref))
+
+			rev = bytes.Replace(rev, []byte("\n"), nil, 1)
+
+			require.Equal(t, string(tc.req.Revision), string(rev))
+		})
+	}
+}
+
+func TestWriteRefValidationError(t *testing.T) {
+	server, serverSocketPath := runRepoServer(t)
+	defer server.Stop()
+
+	client, conn := newRepositoryClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	testCases := []struct {
+		desc string
+		req  *pb.WriteRefRequest
+	}{
+		{
+			desc: "empty revision",
+			req: &pb.WriteRefRequest{
+				Repository: testRepo,
+				Ref:        []byte("refs/heads/master"),
+			},
+		},
+		{
+			desc: "empty ref name",
+			req: &pb.WriteRefRequest{
+				Repository: testRepo,
+				Revision:   []byte("498214de67004b1da3d820901307bed2a68a8ef6"),
+			},
+		},
+		{
+			desc: "non-prefixed ref name",
+			req: &pb.WriteRefRequest{
+				Repository: testRepo,
+				Ref:        []byte("master"),
+				Revision:   []byte("498214de67004b1da3d820901307bed2a68a8ef6"),
+			},
+		},
+		{
+			desc: "revision contains \\x00",
+			req: &pb.WriteRefRequest{
+				Repository: testRepo,
+				Ref:        []byte("refs/heads/master"),
+				Revision:   []byte("012301230123\x001243"),
+			},
+		},
+		{
+			desc: "ref contains \\x00",
+			req: &pb.WriteRefRequest{
+				Repository: testRepo,
+				Ref:        []byte("refs/head\x00s/master\x00"),
+				Revision:   []byte("0123012301231243"),
+			},
+		},
+		{
+			desc: "ref contains whitespace",
+			req: &pb.WriteRefRequest{
+				Repository: testRepo,
+				Ref:        []byte("refs/heads /master"),
+				Revision:   []byte("0123012301231243"),
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+			_, err := client.WriteRef(ctx, tc.req)
+
+			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+		})
+	}
+}
diff --git a/ruby/Gemfile b/ruby/Gemfile
index c4eda9e55..a6631f2f2 100644
--- a/ruby/Gemfile
+++ b/ruby/Gemfile
@@ -1,7 +1,7 @@
 source 'https://rubygems.org'
 
 gem 'github-linguist', '~> 4.7.0', require: 'linguist'
-gem 'gitaly-proto', '~> 0.71.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.72.0', require: 'gitaly'
 gem 'activesupport'
 gem 'rdoc', '~> 4.2'
 gem 'gollum-lib', '~> 4.2', require: false
diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock
index e697501a9..821ecd1f0 100644
--- a/ruby/Gemfile.lock
+++ b/ruby/Gemfile.lock
@@ -17,7 +17,7 @@ GEM
       multipart-post (>= 1.2, < 3)
     gemojione (3.3.0)
       json
-    gitaly-proto (0.71.0)
+    gitaly-proto (0.72.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -136,7 +136,7 @@ PLATFORMS
 
 DEPENDENCIES
   activesupport
-  gitaly-proto (~> 0.71.0)
+  gitaly-proto (~> 0.72.0)
   github-linguist (~> 4.7.0)
   gitlab-styles (~> 2.0.0)
   gollum-lib (~> 4.2)
diff --git a/ruby/lib/gitaly_server/repository_service.rb b/ruby/lib/gitaly_server/repository_service.rb
index bcdcbd763..b9dfd5705 100644
--- a/ruby/lib/gitaly_server/repository_service.rb
+++ b/ruby/lib/gitaly_server/repository_service.rb
@@ -80,5 +80,17 @@ module GitalyServer
         Gitaly::IsRebaseInProgressResponse.new(in_progress: result)
       end
     end
+
+    def write_ref(request, call)
+      bridge_exceptions do
+        repo = Gitlab::Git::Repository.from_gitaly(request.repository, call)
+
+        # We ignore the output since the shell-version returns the output
+        #  while the rugged-version returns true. But both throws expections on errors
+        repo.write_ref(request.ref, request.revision, old_ref: request.old_revision, shell: request.shell)
+
+        Gitaly::WriteRefResponse.new
+      end
+    end
   end
 end