diff options
author | Stan Hu <stanhu@gmail.com> | 2020-10-15 13:40:46 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2020-10-15 13:40:46 +0300 |
commit | 05cd75fb57f06f29978e6cc0da3f7bc35d85859f (patch) | |
tree | bb582bb0ffd2e7bb2be841153b1798009eb3fd07 | |
parent | 42c4f256a282ed30300c546bbb3483a800474c8b (diff) | |
parent | dadc0c125e7b08fe38b1d0f8d575c9bda4cfcbf9 (diff) |
Merge branch 'sh-gitaly-lfs-smudge-activate' into 'master'
Activate LFS smudge filter in GetArchiveRequest
See merge request gitlab-org/gitaly!2621
-rw-r--r-- | changelogs/unreleased/sh-gitaly-lfs-smudge-activate.yml | 5 | ||||
-rw-r--r-- | internal/gitaly/maintenance/optimize_test.go | 5 | ||||
-rw-r--r-- | internal/gitaly/service/operations/testhelper_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/register.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/repository/archive.go | 27 | ||||
-rw-r--r-- | internal/gitaly/service/repository/archive_test.go | 119 | ||||
-rw-r--r-- | internal/gitaly/service/repository/fetch_remote_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/repository/replicate_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/repository/search_files_test.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/repository/server.go | 7 | ||||
-rw-r--r-- | internal/gitaly/service/repository/testhelper_test.go | 10 | ||||
-rw-r--r-- | internal/praefect/replicator_test.go | 2 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 15 | ||||
-rw-r--r-- | internal/testhelper/testserver/gitaly.go | 2 |
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(), |