Rewrite add_remote in Go

5 files changed, 232 insertions, 3 deletions

diff --git a/changelogs/unreleased/wc-go-add-remote.yml b/changelogs/unreleased/wc-go-add-remote.yml
new file mode 100644
index 000000000..aabf32288
--- /dev/null
+++ b/changelogs/unreleased/wc-go-add-remote.yml
@@ -0,0 +1,5 @@
+---
+title: Rewrite AddRemote in Go
+merge_request: 1572
+author:
+type: other
diff --git a/internal/git/remote/remote.go b/internal/git/remote/remote.go
index 41e83b187..37eebe9f1 100644
--- a/internal/git/remote/remote.go
+++ b/internal/git/remote/remote.go
@@ -8,6 +8,54 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 )
 
+var REFMAPS = map[string]string{
+	"allRefs": "+refs/*:refs/*",
+	"heads":   "+refs/heads/*:refs/heads/*",
+	"tags":    "+refs/tags/*:refs/tags/*",
+}
+
+//Add remote to repository
+func Add(ctx context.Context, repo repository.GitRepo, name string, url string, refmaps []string) error {
+	hasRemote, err := Exists(ctx, repo, name)
+	if err != nil {
+		return err
+	}
+
+	if hasRemote {
+		cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
+			Name: "remote",
+			Flags: []git.Option{
+				git.SubSubCmd{"set-url"},
+			},
+			Args: []string{name, url},
+		})
+		if err != nil {
+			return err
+		}
+		return cmd.Wait()
+	}
+
+	cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
+		Name: "remote",
+		Args: []string{"add", name, url},
+	})
+	if err != nil {
+		return err
+	}
+	if err := cmd.Wait(); err != nil {
+		return err
+	}
+
+	if len(refmaps) > 0 {
+		err = setMirror(ctx, repo, name, refmaps)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 //Remove removes the remote from repository
 func Remove(ctx context.Context, repo repository.GitRepo, name string) error {
 	cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
@@ -41,3 +89,71 @@ func Exists(ctx context.Context, repo repository.GitRepo, name string) (bool, er
 
 	return found, cmd.Wait()
 }
+
+func setMirror(ctx context.Context, repo repository.GitRepo, name string, refmaps []string) error {
+	for _, configOption := range []string{"mirror", "prune"} {
+		cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
+			Name: "config",
+			Flags: []git.Option{
+				git.Flag{"--add"},
+				git.ConfigPair{"remote." + name + "." + configOption, "true"},
+			},
+		})
+		if err != nil {
+			return err
+		}
+		if err := cmd.Wait(); err != nil {
+			return err
+		}
+	}
+
+	return setRefmaps(ctx, repo, name, refmaps)
+}
+
+func setRefmaps(ctx context.Context, repo repository.GitRepo, name string, refmaps []string) error {
+	parsedMaps := parseRefmaps(refmaps)
+
+	for i, refmap := range parsedMaps {
+		var flag git.Flag
+		if i == 0 {
+			flag = git.Flag{"--replace-all"}
+		} else {
+			flag = git.Flag{"--add"}
+		}
+
+		cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
+			Name: "config",
+			Flags: []git.Option{
+				flag,
+				git.ConfigPair{"remote." + name + ".fetch", refmap},
+			},
+		})
+		if err != nil {
+			return err
+		}
+		if err := cmd.Wait(); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func parseRefmaps(refmaps []string) []string {
+	parsedMaps := make([]string, len(refmaps))
+
+	for _, refmap := range refmaps {
+		if len(refmap) == 0 {
+			continue
+		}
+
+		expanded, ok := REFMAPS[refmap]
+		if ok {
+			parsedMaps = append(parsedMaps, expanded)
+		} else {
+			parsedMaps = append(parsedMaps, refmap)
+		}
+	}
+
+	return parsedMaps
+}
diff --git a/internal/git/remote/remote_test.go b/internal/git/remote/remote_test.go
index af4c56a9e..909491aec 100644
--- a/internal/git/remote/remote_test.go
+++ b/internal/git/remote/remote_test.go
@@ -9,6 +9,79 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
+func TestAddRemote(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	repoPath, err := helper.GetRepoPath(testRepo)
+	require.NoError(t, err)
+
+	testCases := []struct {
+		description           string
+		remoteName            string
+		url                   string
+		mirrorRefmaps         []string
+		resolvedMirrorRefmaps []string
+	}{
+		{
+			description: "creates remote with no refmaps",
+			remoteName:  "no-refmap",
+			url:         "https://gitlab.com/gitlab-org/git.git",
+		},
+		{
+			description: "updates url when remote exists",
+			remoteName:  "no-refmap",
+			url:         "https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git",
+		},
+		{
+			description:           "resolves abbreviated refmap",
+			remoteName:            "abbrev-refmap",
+			url:                   "https://gitlab.com/gitlab-org/git.git",
+			mirrorRefmaps:         []string{"heads"},
+			resolvedMirrorRefmaps: []string{"+refs/heads/*:refs/heads/*"},
+		},
+		{
+			description:           "adds multiple refmaps",
+			remoteName:            "multiple-refmaps",
+			url:                   "https://gitlab.com/gitlab-org/git.git",
+			mirrorRefmaps:         []string{"tags", "+refs/heads/*:refs/remotes/origin/*"},
+			resolvedMirrorRefmaps: []string{"+refs/tags/*:refs/tags/*", "+refs/heads/*:refs/remotes/origin/*"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			err := Add(ctx, testRepo, tc.remoteName, tc.url, tc.mirrorRefmaps)
+			require.NoError(t, err)
+
+			found, err := Exists(ctx, testRepo, tc.remoteName)
+			require.NoError(t, err)
+			require.True(t, found)
+
+			url := string(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "remote", "get-url", tc.remoteName))
+			require.Contains(t, url, tc.url)
+
+			mirrorRegex := "remote." + tc.remoteName
+			mirrorConfig := string(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--get-regexp", mirrorRegex))
+			if len(tc.resolvedMirrorRefmaps) > 0 {
+				require.Contains(t, mirrorConfig, "mirror true")
+				require.Contains(t, mirrorConfig, "prune true")
+			} else {
+				require.NotContains(t, mirrorConfig, "mirror true")
+			}
+
+			mirrorFetch := mirrorRegex + ".fetch"
+			fetch := string(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--get-all", mirrorFetch))
+			for _, resolvedMirrorRefmap := range tc.resolvedMirrorRefmaps {
+				require.Contains(t, fetch, resolvedMirrorRefmap)
+			}
+		})
+	}
+}
+
 func TestRemoveRemote(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index 7855b6333..32634796e 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -1,12 +1,16 @@
 package featureflag
 
 const (
-	// UploadPackFilter enables partial clones by sending uploadpack.allowFilter and uploadpack.allowAnySHA1InWant
-	// to upload-pack
-	UploadPackFilter = "upload_pack_filter"
+	// AddRemoteGo will cause the AddRemote RPC to use the go implementation when set
+	AddRemoteGo = "add_remote_go"
+
 	// GetAllLFSPointersGo will cause the GetAllLFSPointers RPC to use the go implementation when set
 	GetAllLFSPointersGo = "get_all_lfs_pointers_go"
 
 	// LinguistFileCountStats will invoke an additional git-linguist command to get the number of files per language
 	LinguistFileCountStats = "linguist_file_count_stats"
+
+	// UploadPackFilter enables partial clones by sending uploadpack.allowFilter and uploadpack.allowAnySHA1InWant
+	// to upload-pack
+	UploadPackFilter = "upload_pack_filter"
 )
diff --git a/internal/service/remote/remotes.go b/internal/service/remote/remotes.go
index 0cba15dfb..b5c69294e 100644
--- a/internal/service/remote/remotes.go
+++ b/internal/service/remote/remotes.go
@@ -11,19 +11,41 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/remote"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/chunk"
+	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
+var addRemoteRequests = prometheus.NewCounterVec(
+	prometheus.CounterOpts{
+		Name: "gitaly_add_remote_total",
+		Help: "Counter of go vs ruby implementation of AddRemote",
+	},
+	[]string{"implementation"},
+)
+
+func init() {
+	prometheus.MustRegister(addRemoteRequests)
+}
+
 // AddRemote adds a remote to the repository
 func (s *server) AddRemote(ctx context.Context, req *gitalypb.AddRemoteRequest) (*gitalypb.AddRemoteResponse, error) {
 	if err := validateAddRemoteRequest(req); err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "AddRemote: %v", err)
 	}
 
+	if featureflag.IsEnabled(ctx, featureflag.AddRemoteGo) {
+		addRemoteRequests.WithLabelValues("go").Inc()
+
+		return addRemote(ctx, req)
+	}
+
+	addRemoteRequests.WithLabelValues("ruby").Inc()
+
 	client, err := s.ruby.RemoteServiceClient(ctx)
 	if err != nil {
 		return nil, err
@@ -37,6 +59,15 @@ func (s *server) AddRemote(ctx context.Context, req *gitalypb.AddRemoteRequest)
 	return client.AddRemote(clientCtx, req)
 }
 
+func addRemote(ctx context.Context, req *gitalypb.AddRemoteRequest) (*gitalypb.AddRemoteResponse, error) {
+	err := remote.Add(ctx, req.GetRepository(), req.Name, req.Url, req.GetMirrorRefmaps())
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "AddRemote: %v", err)
+	}
+
+	return &gitalypb.AddRemoteResponse{}, nil
+}
+
 func validateAddRemoteRequest(req *gitalypb.AddRemoteRequest) error {
 	if strings.TrimSpace(req.GetName()) == "" {
 		return fmt.Errorf("empty remote name")