Merge branch '663-server-implementation-repositoryservice-changestorage' into 'master'

Resolve "Server implementation RemoteService.FetchInternalRemote"

Closes #663

See merge request gitlab-org/gitaly!508

11 files changed, 236 insertions, 36 deletions

diff --git a/.gitignore b/.gitignore
index fb5c8eca5..6fd3edea3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,13 +1,12 @@
 /_build
 /gitaly
-/gitaly-ssh
+/**/gitaly-ssh
 /*.deb
 /_support/package/bin
 /_support/bin
 /internal/service/smarthttp/testdata
 /internal/testhelper/testdata
 /config.toml
-/internal/service/ssh/gitaly-ssh
 /.ruby-bundle
 /ruby/vendor/bundle
 /ruby/.bundle
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6901295ab..79b265b7f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 v0.65.0
 
+- Implement RemoteService.FetchInternalRemote RPC
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/508
 - Add support for MaxCount in CountCommits RPC
   https://gitlab.com/gitlab-org/gitaly/merge_requests/507
 - Implement CreateFork RPC
diff --git a/internal/service/ref/delete_refs_test.go b/internal/service/ref/delete_refs_test.go
index 0305b3e5f..2b817a012 100644
--- a/internal/service/ref/delete_refs_test.go
+++ b/internal/service/ref/delete_refs_test.go
@@ -37,14 +37,13 @@ func TestSuccessfulDeleteRefs(t *testing.T) {
 	_, err := client.DeleteRefs(ctx, rpcRequest)
 	require.NoError(t, err)
 
-	refs := testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "for-each-ref")
-	refsStr := string(refs)
-
-	require.NotContains(t, refsStr, "refs/delete/a")
-	require.NotContains(t, refsStr, "refs/also-delete/b")
-	require.Contains(t, refsStr, "refs/keep/c")
-	require.Contains(t, refsStr, "refs/also-keep/d")
-	require.Contains(t, refsStr, "refs/heads/master")
+	refs := testhelper.GetRepositoryRefs(t, repoPath)
+
+	require.NotContains(t, refs, "refs/delete/a")
+	require.NotContains(t, refs, "refs/also-delete/b")
+	require.Contains(t, refs, "refs/keep/c")
+	require.Contains(t, refs, "refs/also-keep/d")
+	require.Contains(t, refs, "refs/heads/master")
 }
 
 func TestFailedDeleteRefsDueToValidation(t *testing.T) {
diff --git a/internal/service/remote/fetch_internal_remote.go b/internal/service/remote/fetch_internal_remote.go
index 0d16d9dfe..ed69697e3 100644
--- a/internal/service/remote/fetch_internal_remote.go
+++ b/internal/service/remote/fetch_internal_remote.go
@@ -1,13 +1,43 @@
 package remote
 
 import (
+	"fmt"
+
 	"golang.org/x/net/context"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 )
 
 // FetchInternalRemote fetches another Gitaly repository set as a remote
-func (s *server) FetchInternalRemote(context.Context, *pb.FetchInternalRemoteRequest) (*pb.FetchInternalRemoteResponse, error) {
-	return nil, helper.Unimplemented
+func (s *server) FetchInternalRemote(ctx context.Context, req *pb.FetchInternalRemoteRequest) (*pb.FetchInternalRemoteResponse, error) {
+	if err := validateFetchInternalRemoteRequest(req); err != nil {
+		return nil, grpc.Errorf(codes.InvalidArgument, "FetchInternalRemote: %v", err)
+	}
+
+	client, err := s.RemoteServiceClient(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	clientCtx, err := rubyserver.SetHeaders(ctx, req.GetRepository())
+	if err != nil {
+		return nil, err
+	}
+
+	return client.FetchInternalRemote(clientCtx, req)
+}
+
+func validateFetchInternalRemoteRequest(req *pb.FetchInternalRemoteRequest) error {
+	if req.GetRepository() == nil {
+		return fmt.Errorf("empty Repository")
+	}
+
+	if req.GetRemoteRepository() == nil {
+		return fmt.Errorf("empty Remote Repository")
+	}
+
+	return nil
 }
diff --git a/internal/service/remote/fetch_internal_remote_test.go b/internal/service/remote/fetch_internal_remote_test.go
new file mode 100644
index 000000000..d9697a171
--- /dev/null
+++ b/internal/service/remote/fetch_internal_remote_test.go
@@ -0,0 +1,144 @@
+package remote_test
+
+import (
+	"context"
+	"net"
+	"os"
+	"testing"
+
+	"gitlab.com/gitlab-org/gitaly/internal/service/remote"
+
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/metadata"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	serverPkg "gitlab.com/gitlab-org/gitaly/internal/server"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestSuccessfulFetchInternalRemote(t *testing.T) {
+	server, serverSocketPath := runFullServer(t)
+	defer server.Stop()
+
+	client, conn := remote.NewRemoteClient(t, serverSocketPath)
+	defer conn.Close()
+
+	remoteRepo, remoteRepoPath, remoteCleanupFn := testhelper.NewTestRepo(t)
+	defer remoteCleanupFn()
+
+	repo, repoPath, cleanupFn := initRepo(t)
+	defer cleanupFn()
+
+	ctxOuter, cancel := testhelper.Context()
+	defer cancel()
+
+	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
+	ctx := metadata.NewOutgoingContext(ctxOuter, md)
+
+	request := &pb.FetchInternalRemoteRequest{
+		Repository:       repo,
+		RemoteRepository: remoteRepo,
+	}
+
+	c, err := client.FetchInternalRemote(ctx, request)
+	require.NoError(t, err)
+	require.True(t, c.GetResult())
+
+	remoteRefs := testhelper.GetRepositoryRefs(t, remoteRepoPath)
+	refs := testhelper.GetRepositoryRefs(t, repoPath)
+	require.Equal(t, remoteRefs, refs)
+}
+
+func TestFailedFetchInternalRemote(t *testing.T) {
+	server, serverSocketPath := runFullServer(t)
+	defer server.Stop()
+
+	client, conn := remote.NewRemoteClient(t, serverSocketPath)
+	defer conn.Close()
+
+	repo, _, cleanupFn := initRepo(t)
+	defer cleanupFn()
+
+	ctxOuter, cancel := testhelper.Context()
+	defer cancel()
+
+	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
+	ctx := metadata.NewOutgoingContext(ctxOuter, md)
+
+	// Non-existing remote repo
+	remoteRepo := &pb.Repository{StorageName: "default", RelativePath: "fake.git"}
+
+	request := &pb.FetchInternalRemoteRequest{
+		Repository:       repo,
+		RemoteRepository: remoteRepo,
+	}
+
+	c, err := client.FetchInternalRemote(ctx, request)
+	require.NoError(t, err)
+	require.False(t, c.GetResult())
+}
+
+func TestFailedFetchInternalRemoteDueToValidations(t *testing.T) {
+	server, serverSocketPath := runFullServer(t)
+	defer server.Stop()
+
+	client, conn := remote.NewRemoteClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	repo := &pb.Repository{StorageName: "default", RelativePath: "repo.git"}
+
+	testCases := []struct {
+		desc    string
+		request *pb.FetchInternalRemoteRequest
+	}{
+		{
+			desc:    "empty Repository",
+			request: &pb.FetchInternalRemoteRequest{RemoteRepository: repo},
+		},
+		{
+			desc:    "empty Remote Repository",
+			request: &pb.FetchInternalRemoteRequest{Repository: repo},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			_, err := client.FetchInternalRemote(ctx, tc.request)
+
+			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, tc.desc)
+		})
+	}
+}
+
+func initRepo(t *testing.T) (*pb.Repository, string, func()) {
+	testhelper.ConfigureTestStorage()
+
+	repo := &pb.Repository{StorageName: "default", RelativePath: "repo.git"}
+	repoPath, err := helper.GetPath(repo)
+	require.NoError(t, err)
+
+	testhelper.MustRunCommand(t, nil, "git", "init", "--bare", repoPath)
+
+	return repo, repoPath, func() { os.RemoveAll(repoPath) }
+}
+
+func runFullServer(t *testing.T) (*grpc.Server, string) {
+	server := serverPkg.New(remote.RubyServer)
+	serverSocketPath := testhelper.GetTemporaryGitalySocketFileName()
+
+	listener, err := net.Listen("unix", serverSocketPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	go server.Serve(listener)
+
+	return server, serverSocketPath
+}
diff --git a/internal/service/remote/remotes_test.go b/internal/service/remote/remotes_test.go
index f8cc926d2..fc8a92dfa 100644
--- a/internal/service/remote/remotes_test.go
+++ b/internal/service/remote/remotes_test.go
@@ -19,7 +19,7 @@ func TestSuccessfulAddRemote(t *testing.T) {
 	server, serverSocketPath := runRemoteServiceServer(t)
 	defer server.Stop()
 
-	client, conn := newRemoteClient(t, serverSocketPath)
+	client, conn := NewRemoteClient(t, serverSocketPath)
 	defer conn.Close()
 
 	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
@@ -94,7 +94,7 @@ func TestFailedAddRemoteDueToValidation(t *testing.T) {
 	server, serverSocketPath := runRemoteServiceServer(t)
 	defer server.Stop()
 
-	client, conn := newRemoteClient(t, serverSocketPath)
+	client, conn := NewRemoteClient(t, serverSocketPath)
 	defer conn.Close()
 
 	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
@@ -142,7 +142,7 @@ func TestSuccessfulRemoveRemote(t *testing.T) {
 	server, serverSocketPath := runRemoteServiceServer(t)
 	defer server.Stop()
 
-	client, conn := newRemoteClient(t, serverSocketPath)
+	client, conn := NewRemoteClient(t, serverSocketPath)
 	defer conn.Close()
 
 	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
@@ -192,7 +192,7 @@ func TestFailedRemoveRemoteDueToValidation(t *testing.T) {
 	server, serverSocketPath := runRemoteServiceServer(t)
 	defer server.Stop()
 
-	client, conn := newRemoteClient(t, serverSocketPath)
+	client, conn := NewRemoteClient(t, serverSocketPath)
 	defer conn.Close()
 
 	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
diff --git a/internal/service/remote/testhelper_test.go b/internal/service/remote/testhelper_test.go
index 9843b46eb..ece4482c7 100644
--- a/internal/service/remote/testhelper_test.go
+++ b/internal/service/remote/testhelper_test.go
@@ -14,7 +14,7 @@ import (
 	"google.golang.org/grpc/reflection"
 )
 
-var rubyServer *rubyserver.Server
+var RubyServer *rubyserver.Server
 
 func TestMain(m *testing.M) {
 	os.Exit(testMain(m))
@@ -26,11 +26,13 @@ func testMain(m *testing.M) int {
 	var err error
 
 	testhelper.ConfigureRuby()
-	rubyServer, err = rubyserver.Start()
+	testhelper.ConfigureGitalySSH()
+
+	RubyServer, err = rubyserver.Start()
 	if err != nil {
 		log.Fatal(err)
 	}
-	defer rubyServer.Stop()
+	defer RubyServer.Stop()
 
 	return m.Run()
 }
@@ -44,7 +46,7 @@ func runRemoteServiceServer(t *testing.T) (*grpc.Server, string) {
 		t.Fatal(err)
 	}
 
-	pb.RegisterRemoteServiceServer(grpcServer, &server{rubyServer})
+	pb.RegisterRemoteServiceServer(grpcServer, &server{RubyServer})
 	reflection.Register(grpcServer)
 
 	go grpcServer.Serve(listener)
@@ -52,7 +54,7 @@ func runRemoteServiceServer(t *testing.T) (*grpc.Server, string) {
 	return grpcServer, serverSocketPath
 }
 
-func newRemoteClient(t *testing.T, serverSocketPath string) (pb.RemoteServiceClient, *grpc.ClientConn) {
+func NewRemoteClient(t *testing.T, serverSocketPath string) (pb.RemoteServiceClient, *grpc.ClientConn) {
 	connOpts := []grpc.DialOption{
 		grpc.WithInsecure(),
 		grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) {
diff --git a/internal/service/repository/testhelper_test.go b/internal/service/repository/testhelper_test.go
index e6bcd04d2..d41be6fc0 100644
--- a/internal/service/repository/testhelper_test.go
+++ b/internal/service/repository/testhelper_test.go
@@ -4,7 +4,6 @@ import (
 	"log"
 	"net"
 	"os"
-	"path"
 	"path/filepath"
 	"testing"
 	"time"
@@ -98,18 +97,7 @@ func testMain(m *testing.M) int {
 		log.Fatal(err)
 	}
 
-	config.Config.BinDir, err = filepath.Abs("testdata/gitaly-libexec")
-	if err != nil {
-		log.Fatal(err)
-	}
-
-	goBuildArgs := []string{
-		"build",
-		"-o",
-		path.Join(config.Config.BinDir, "gitaly-ssh"),
-		"gitlab.com/gitlab-org/gitaly/cmd/gitaly-ssh",
-	}
-	testhelper.MustRunCommand(nil, nil, "go", goBuildArgs...)
+	testhelper.ConfigureGitalySSH()
 
 	RubyServer, err = rubyserver.Start()
 	if err != nil {
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index e0e6251c4..3107824e5 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -360,3 +360,28 @@ func cloneTestRepo(t *testing.T, bare bool) (repo *pb.Repository, repoPath strin
 
 	return repo, repoPath, func() { os.RemoveAll(repoPath) }
 }
+
+// ConfigureGitalySSH configures the gitaly-ssh command for tests
+func ConfigureGitalySSH() {
+	var err error
+
+	config.Config.BinDir, err = filepath.Abs("testdata/gitaly-libexec")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	goBuildArgs := []string{
+		"build",
+		"-o",
+		path.Join(config.Config.BinDir, "gitaly-ssh"),
+		"gitlab.com/gitlab-org/gitaly/cmd/gitaly-ssh",
+	}
+	MustRunCommand(nil, nil, "go", goBuildArgs...)
+}
+
+// GetRepositoryRefs gives a list of each repository ref as a string
+func GetRepositoryRefs(t *testing.T, repoPath string) string {
+	refs := MustRunCommand(t, nil, "git", "-C", repoPath, "for-each-ref")
+
+	return string(refs)
+}
diff --git a/ruby/lib/gitaly_server/remote_service.rb b/ruby/lib/gitaly_server/remote_service.rb
index 545735c87..3b8837b36 100644
--- a/ruby/lib/gitaly_server/remote_service.rb
+++ b/ruby/lib/gitaly_server/remote_service.rb
@@ -24,6 +24,17 @@ module GitalyServer
       end
     end
 
+    def fetch_internal_remote(request, call)
+      bridge_exceptions do
+        repo = Gitlab::Git::Repository.from_gitaly(request.repository, call)
+        remote_repo = Gitlab::Git::GitalyRemoteRepository.new(request.remote_repository, call)
+
+        result = repo.fetch_repository_as_mirror(remote_repo)
+
+        Gitaly::FetchInternalRemoteResponse.new(result: result)
+      end
+    end
+
     private
 
     def parse_refmap(refmap)
diff --git a/ruby/lib/gitlab/gitaly_client.rb b/ruby/lib/gitlab/gitaly_client.rb
index 49d6a3120..20fafcb8c 100644
--- a/ruby/lib/gitlab/gitaly_client.rb
+++ b/ruby/lib/gitlab/gitaly_client.rb
@@ -10,7 +10,7 @@ module Gitlab
       # In case we hit a method that tries to do a Gitaly RPC, we want to
       # prevent this most of the time.
       def migrate(*args)
-        whitelist = [:fetch_ref]
+        whitelist = [:fetch_ref, :fetch_internal]
         yield whitelist.include?(args.first)
       end
     end