Merge branch 'sh-gitaly-lfs-smudge-activate' into 'master'

Activate LFS smudge filter in GetArchiveRequest

See merge request gitlab-org/gitaly!2621

14 files changed, 184 insertions, 20 deletions

diff --git a/changelogs/unreleased/sh-gitaly-lfs-smudge-activate.yml b/changelogs/unreleased/sh-gitaly-lfs-smudge-activate.yml
new file mode 100644
index 000000000..98a64dbd4
--- /dev/null
+++ b/changelogs/unreleased/sh-gitaly-lfs-smudge-activate.yml
@@ -0,0 +1,5 @@
+---
+title: Add support for using LFS smudge filter
+merge_request: 2621
+author:
+type: added
diff --git a/internal/gitaly/maintenance/optimize_test.go b/internal/gitaly/maintenance/optimize_test.go
index 281c5efcd..9b7d082c0 100644
--- a/internal/gitaly/maintenance/optimize_test.go
+++ b/internal/gitaly/maintenance/optimize_test.go
@@ -23,8 +23,9 @@ type mockOptimizer struct {
 
 func (mo *mockOptimizer) OptimizeRepository(ctx context.Context, req *gitalypb.OptimizeRepositoryRequest, _ ...grpc.CallOption) (*gitalypb.OptimizeRepositoryResponse, error) {
 	mo.actual = append(mo.actual, req.Repository)
-	l := config.NewLocator(config.Cfg{Storages: mo.storages})
-	resp, err := repository.NewServer(nil, l, "").OptimizeRepository(ctx, req)
+	cfg := config.Cfg{Storages: mo.storages}
+	l := config.NewLocator(cfg)
+	resp, err := repository.NewServer(cfg, nil, l, "").OptimizeRepository(ctx, req)
 	assert.NoError(mo.t, err)
 	return resp, err
 }
diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go
index c453a7fa6..f1f505efa 100644
--- a/internal/gitaly/service/operations/testhelper_test.go
+++ b/internal/gitaly/service/operations/testhelper_test.go
@@ -94,7 +94,7 @@ func runOperationServiceServerWithRubyServer(t *testing.T, ruby *rubyserver.Serv
 
 	gitalypb.RegisterOperationServiceServer(srv.GrpcServer(), server)
 	gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hook.NewServer(hookManager))
-	gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), repository.NewServer(ruby, locator, internalSocket))
+	gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), repository.NewServer(config.Config, ruby, locator, internalSocket))
 	gitalypb.RegisterRefServiceServer(srv.GrpcServer(), ref.NewServer(locator))
 	gitalypb.RegisterCommitServiceServer(srv.GrpcServer(), commit.NewServer(locator))
 	gitalypb.RegisterSSHServiceServer(srv.GrpcServer(), ssh.NewServer(locator))
diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index 65a9a9906..34f732339 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -75,7 +75,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
 	gitalypb.RegisterNamespaceServiceServer(grpcServer, namespace.NewServer())
 	gitalypb.RegisterOperationServiceServer(grpcServer, operations.NewServer(cfg, rubyServer, hookManager, locator))
 	gitalypb.RegisterRefServiceServer(grpcServer, ref.NewServer(locator))
-	gitalypb.RegisterRepositoryServiceServer(grpcServer, repository.NewServer(rubyServer, locator, config.GitalyInternalSocketPath()))
+	gitalypb.RegisterRepositoryServiceServer(grpcServer, repository.NewServer(cfg, rubyServer, locator, config.GitalyInternalSocketPath()))
 	gitalypb.RegisterSSHServiceServer(grpcServer, ssh.NewServer(
 		locator,
 		ssh.WithPackfileNegotiationMetrics(sshPackfileNegotiationMetrics),
diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go
index 807f97ebd..79f4eb9ec 100644
--- a/internal/gitaly/service/repository/archive.go
+++ b/internal/gitaly/service/repository/archive.go
@@ -2,10 +2,12 @@ package repository
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 
 	"gitlab.com/gitlab-org/gitaly/internal/command"
@@ -64,7 +66,12 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo
 		return stream.Send(&gitalypb.GetArchiveResponse{Data: p})
 	})
 
-	return handleArchive(ctx, writer, in, compressCmd, format, path, exclude)
+	gitlabConfig, err := json.Marshal(s.cfg)
+	if err != nil {
+		return err
+	}
+
+	return handleArchive(ctx, writer, in, compressCmd, format, path, exclude, gitlabConfig, s.binDir)
 }
 
 func parseArchiveFormat(format gitalypb.GetArchiveRequest_Format) (*exec.Cmd, string) {
@@ -132,16 +139,16 @@ func findGetArchivePath(f *commit.TreeEntryFinder, commitID, path string) (ok bo
 	return true, nil
 }
 
-func handleArchive(ctx context.Context, writer io.Writer, in *gitalypb.GetArchiveRequest, compressCmd *exec.Cmd, format string, path string, exclude []string) error {
+func handleArchive(ctx context.Context, writer io.Writer, in *gitalypb.GetArchiveRequest, compressCmd *exec.Cmd, format string, archivePath string, exclude []string, internalCfg []byte, binDir string) error {
 	var args []string
 	pathspecs := make([]string, 0, len(exclude)+1)
 	if !in.GetElidePath() {
 		// git archive [options] <commit ID> -- <path> [exclude*]
 		args = []string{in.GetCommitId()}
-		pathspecs = append(pathspecs, path)
-	} else if path != "." {
+		pathspecs = append(pathspecs, archivePath)
+	} else if archivePath != "." {
 		// git archive [options] <commit ID>:<path> -- [exclude*]
-		args = []string{in.GetCommitId() + ":" + path}
+		args = []string{in.GetCommitId() + ":" + archivePath}
 	} else {
 		// git archive [options] <commit ID> -- [exclude*]
 		args = []string{in.GetCommitId()}
@@ -154,9 +161,17 @@ func handleArchive(ctx context.Context, writer io.Writer, in *gitalypb.GetArchiv
 	env := []string{
 		fmt.Sprintf("GL_REPOSITORY=%s", in.GetRepository().GetGlRepository()),
 		fmt.Sprintf("GL_PROJECT_PATH=%s", in.GetRepository().GetGlProjectPath()),
+		fmt.Sprintf("GL_INTERNAL_CONFIG=%s", internalCfg),
+	}
+
+	var globals []git.Option
+
+	if in.GetIncludeLfsBlobs() {
+		binary := filepath.Join(binDir, "gitaly-lfs-smudge")
+		globals = append(globals, git.ValueFlag{"-c", fmt.Sprintf("filter.lfs.smudge=%s", binary)})
 	}
 
-	archiveCommand, err := git.SafeCmdWithEnv(ctx, env, in.GetRepository(), nil, git.SubCmd{
+	archiveCommand, err := git.SafeCmdWithEnv(ctx, env, in.GetRepository(), globals, git.SubCmd{
 		Name:        "archive",
 		Flags:       []git.Option{git.ValueFlag{"--format", format}, git.ValueFlag{"--prefix", in.GetPrefix() + "/"}},
 		Args:        args,
diff --git a/internal/gitaly/service/repository/archive_test.go b/internal/gitaly/service/repository/archive_test.go
index 4f600d836..84cc7d983 100644
--- a/internal/gitaly/service/repository/archive_test.go
+++ b/internal/gitaly/service/repository/archive_test.go
@@ -1,6 +1,9 @@
 package repository
 
 import (
+	"archive/zip"
+	"bytes"
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -16,6 +19,11 @@ import (
 	"google.golang.org/grpc/codes"
 )
 
+const (
+	secretToken = "topsecret"
+	lfsBody     = "hello world\n"
+)
+
 func TestGetArchiveSuccess(t *testing.T) {
 	serverSocketPath, stop := runRepoServer(t)
 	defer stop()
@@ -162,6 +170,113 @@ func TestGetArchiveSuccess(t *testing.T) {
 	}
 }
 
+func TestGetArchiveWithLfsSuccess(t *testing.T) {
+	testhelper.ConfigureGitalyLfsSmudge()
+
+	defaultOptions := testhelper.GitlabTestServerOptions{
+		SecretToken: secretToken,
+		LfsBody:     lfsBody,
+	}
+
+	url, cleanup := testhelper.SetupAndStartGitlabServer(t, &defaultOptions)
+	defer cleanup()
+
+	cfg := config.Config
+	cfg.Gitlab.URL = url
+	cfg.Gitlab.SecretFile = filepath.Join(cfg.GitlabShell.Dir, ".gitlab_shell_secret")
+	serverSocketPath, stop := runRepoServerWithConfig(t, cfg)
+	defer stop()
+
+	client, conn := newRepositoryClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	// lfs-moar branch SHA
+	sha := "46abbb087fcc0fd02c340f0f2f052bd2c7708da3"
+
+	testCases := []struct {
+		desc            string
+		prefix          string
+		path            []byte
+		includeLfsBlobs bool
+	}{
+		{
+			desc:            "without prefix and with LFS blobs",
+			prefix:          "",
+			includeLfsBlobs: true,
+		},
+		{
+			desc:            "without prefix and without LFS blobs",
+			prefix:          "",
+			includeLfsBlobs: false,
+		},
+		{
+			desc:            "with prefix and with LFS blobs",
+			prefix:          "my-prefix",
+			includeLfsBlobs: true,
+		},
+		{
+			desc:            "with prefix and without LFS blobs",
+			prefix:          "my-prefix",
+			includeLfsBlobs: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+
+			req := &gitalypb.GetArchiveRequest{
+				Repository:      testRepo,
+				CommitId:        sha,
+				Prefix:          tc.prefix,
+				Format:          gitalypb.GetArchiveRequest_ZIP,
+				Path:            tc.path,
+				IncludeLfsBlobs: tc.includeLfsBlobs,
+			}
+			stream, err := client.GetArchive(ctx, req)
+			require.NoError(t, err)
+
+			data, err := consumeArchive(stream)
+			require.NoError(t, err)
+			reader := bytes.NewReader(data)
+
+			zipReader, err := zip.NewReader(reader, int64(reader.Len()))
+			require.NoError(t, err)
+
+			lfsFiles := []string{"/30170.lfs", "/another.lfs"}
+			for _, lfsFile := range lfsFiles {
+				found := false
+				for _, f := range zipReader.File {
+					if f.Name != tc.prefix+lfsFile {
+						continue
+					}
+
+					found = true
+
+					fc, err := f.Open()
+					require.NoError(t, err)
+					defer fc.Close()
+
+					data, err := ioutil.ReadAll(fc)
+					require.NoError(t, err)
+
+					if tc.includeLfsBlobs {
+						require.Equal(t, lfsBody, string(data))
+					} else {
+						require.Contains(t, string(data), "oid sha256:")
+					}
+				}
+
+				require.True(t, found, "expected to find LFS file")
+			}
+		})
+	}
+}
+
 func TestGetArchiveFailure(t *testing.T) {
 	serverSocketPath, stop := runRepoServer(t)
 	defer stop()
@@ -405,6 +520,9 @@ env | grep ^GL_`))
 	config.Config.Git.BinPath = tmpFile.Name()
 	defer func() { config.Config.Git.BinPath = oldBinPath }()
 
+	cfgData, err := json.Marshal(config.Config.Gitlab)
+	require.NoError(t, err)
+
 	stream, err := client.GetArchive(ctx, req)
 	require.NoError(t, err)
 
@@ -412,6 +530,7 @@ env | grep ^GL_`))
 	require.NoError(t, err)
 	require.Contains(t, string(data), "GL_REPOSITORY="+testhelper.GlRepository)
 	require.Contains(t, string(data), "GL_PROJECT_PATH="+testhelper.GlProjectPath)
+	require.Contains(t, string(data), "GL_INTERNAL_CONFIG="+string(cfgData))
 }
 
 func compressedFileContents(t *testing.T, format gitalypb.GetArchiveRequest_Format, name string) []byte {
diff --git a/internal/gitaly/service/repository/fetch_remote_test.go b/internal/gitaly/service/repository/fetch_remote_test.go
index 7e0f0ad3c..29a56229c 100644
--- a/internal/gitaly/service/repository/fetch_remote_test.go
+++ b/internal/gitaly/service/repository/fetch_remote_test.go
@@ -68,7 +68,7 @@ func TestFetchRemoteSuccess(t *testing.T) {
 }
 
 func TestFetchRemoteFailure(t *testing.T) {
-	server := NewServer(RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath())
+	server := NewServer(config.Config, RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath())
 
 	tests := []struct {
 		desc string
diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go
index 8e79664e5..c08de75b6 100644
--- a/internal/gitaly/service/repository/replicate_test.go
+++ b/internal/gitaly/service/repository/replicate_test.go
@@ -317,7 +317,7 @@ func runServerWithBadFetchInternalRemote(t *testing.T) (*grpc.Server, string) {
 	internalListener, err := net.Listen("unix", config.GitalyInternalSocketPath())
 	require.NoError(t, err)
 
-	gitalypb.RegisterRepositoryServiceServer(server, repository.NewServer(repository.RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath()))
+	gitalypb.RegisterRepositoryServiceServer(server, repository.NewServer(config.Config, repository.RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath()))
 	gitalypb.RegisterRemoteServiceServer(server, &mockRemoteServer{})
 	reflection.Register(server)
 
diff --git a/internal/gitaly/service/repository/search_files_test.go b/internal/gitaly/service/repository/search_files_test.go
index aa625a272..fa0611e73 100644
--- a/internal/gitaly/service/repository/search_files_test.go
+++ b/internal/gitaly/service/repository/search_files_test.go
@@ -210,7 +210,7 @@ func TestSearchFilesByContentLargeFile(t *testing.T) {
 }
 
 func TestSearchFilesByContentFailure(t *testing.T) {
-	server := NewServer(RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath())
+	server := NewServer(config.Config, RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath())
 
 	testRepo, _, cleanupRepo := testhelper.NewTestRepo(t)
 	defer cleanupRepo()
@@ -331,7 +331,7 @@ func TestSearchFilesByNameSuccessful(t *testing.T) {
 }
 
 func TestSearchFilesByNameFailure(t *testing.T) {
-	server := NewServer(RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath())
+	server := NewServer(config.Config, RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath())
 
 	testCases := []struct {
 		desc   string
diff --git a/internal/gitaly/service/repository/server.go b/internal/gitaly/service/repository/server.go
index 5a52d1eaa..cd0a4f4a5 100644
--- a/internal/gitaly/service/repository/server.go
+++ b/internal/gitaly/service/repository/server.go
@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"gitlab.com/gitlab-org/gitaly/client"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
@@ -15,15 +16,19 @@ type server struct {
 	conns                *client.Pool
 	internalGitalySocket string
 	locator              storage.Locator
+	cfg                  config.Gitlab
+	binDir               string
 }
 
 // NewServer creates a new instance of a gRPC repo server
-func NewServer(rs *rubyserver.Server, locator storage.Locator, internalGitalySocket string) gitalypb.RepositoryServiceServer {
+func NewServer(cfg config.Cfg, rs *rubyserver.Server, locator storage.Locator, internalGitalySocket string) gitalypb.RepositoryServiceServer {
 	return &server{
 		ruby:                 rs,
 		locator:              locator,
 		conns:                client.NewPool(),
 		internalGitalySocket: internalGitalySocket,
+		cfg:                  cfg.Gitlab,
+		binDir:               cfg.BinDir,
 	}
 }
 
diff --git a/internal/gitaly/service/repository/testhelper_test.go b/internal/gitaly/service/repository/testhelper_test.go
index 43dbe466e..e6b32798e 100644
--- a/internal/gitaly/service/repository/testhelper_test.go
+++ b/internal/gitaly/service/repository/testhelper_test.go
@@ -65,7 +65,7 @@ func newSecureRepoClient(t *testing.T, serverSocketPath string, pool *x509.CertP
 
 var NewSecureRepoClient = newSecureRepoClient
 
-func runRepoServer(t *testing.T, opts ...testhelper.TestServerOpt) (string, func()) {
+func runRepoServerWithConfig(t *testing.T, cfg config.Cfg, opts ...testhelper.TestServerOpt) (string, func()) {
 	streamInt := []grpc.StreamServerInterceptor{
 		mcache.StreamInvalidator(dcache.LeaseKeyer{}, protoregistry.GitalyProtoPreregistered),
 	}
@@ -73,9 +73,9 @@ func runRepoServer(t *testing.T, opts ...testhelper.TestServerOpt) (string, func
 		mcache.UnaryInvalidator(dcache.LeaseKeyer{}, protoregistry.GitalyProtoPreregistered),
 	}
 
-	srv := testhelper.NewServerWithAuth(t, streamInt, unaryInt, config.Config.Auth.Token, opts...)
+	srv := testhelper.NewServerWithAuth(t, streamInt, unaryInt, cfg.Auth.Token, opts...)
 
-	gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), NewServer(RubyServer, config.NewLocator(config.Config), config.GitalyInternalSocketPath()))
+	gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), NewServer(cfg, RubyServer, config.NewLocator(cfg), config.GitalyInternalSocketPath()))
 	reflection.Register(srv.GrpcServer())
 
 	require.NoError(t, srv.Start())
@@ -83,6 +83,10 @@ func runRepoServer(t *testing.T, opts ...testhelper.TestServerOpt) (string, func
 	return "unix://" + srv.Socket(), srv.Stop
 }
 
+func runRepoServer(t *testing.T, opts ...testhelper.TestServerOpt) (string, func()) {
+	return runRepoServerWithConfig(t, config.Config, opts...)
+}
+
 func TestRepoNoAuth(t *testing.T) {
 	socket, stop := runRepoServer(t)
 	defer stop()
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 0e531e151..db515eeb4 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -976,7 +976,7 @@ func newReplicationService(tb testing.TB) (*grpc.Server, string) {
 	svr := testhelper.NewTestGrpcServer(tb, nil, nil)
 
 	locator := gitaly_config.NewLocator(gitaly_config.Config)
-	gitalypb.RegisterRepositoryServiceServer(svr, repository.NewServer(RubyServer, locator, internalSocketName))
+	gitalypb.RegisterRepositoryServiceServer(svr, repository.NewServer(gitaly_config.Config, RubyServer, locator, internalSocketName))
 	gitalypb.RegisterObjectPoolServiceServer(svr, objectpoolservice.NewServer(locator))
 	gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(RubyServer, locator))
 	gitalypb.RegisterSSHServiceServer(svr, ssh.NewServer(locator))
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 5719cea86..a296d34fa 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -724,6 +724,21 @@ func ConfigureGitalyGit2Go() {
 	MustRunCommand(nil, nil, "go", goBuildArgs...)
 }
 
+// ConfigureGitalyLfsSmudge configures the gitaly-lfs-smudge command for tests
+func ConfigureGitalyLfsSmudge() {
+	if config.Config.BinDir == "" {
+		log.Fatal("config.Config.BinDir must be set")
+	}
+
+	goBuildArgs := []string{
+		"build",
+		"-o",
+		filepath.Join(config.Config.BinDir, "gitaly-lfs-smudge"),
+		"gitlab.com/gitlab-org/gitaly/cmd/gitaly-lfs-smudge",
+	}
+	MustRunCommand(nil, nil, "go", goBuildArgs...)
+}
+
 // GetRepositoryRefs gives a list of each repository ref as a string
 func GetRepositoryRefs(t testing.TB, repoPath string) string {
 	refs := MustRunCommand(t, nil, "git", "-C", repoPath, "for-each-ref")
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index be38eba67..5f3e950d1 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -51,7 +51,7 @@ func RealGitaly(storages []config.Storage, authToken, internalSocketPath string)
 		healthpb.HealthServer
 	}{
 		gitalyserver.NewServer(storages),
-		repository.NewServer(RubyServer, config.NewLocator(config.Config), internalSocketPath),
+		repository.NewServer(config.Config, RubyServer, config.NewLocator(config.Config), internalSocketPath),
 		internalgitaly.NewServer(config.Config.Storages),
 		commit.NewServer(config.NewLocator(config.Config)),
 		health.NewServer(),