From 157f6572f22e47ed95e071785f1eecf7aa5fe6c3 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 25 Jun 2019 16:43:14 +0200 Subject: [PATCH 1/5] Remove duplication of receive-pack config --- internal/git/const.go | 8 ++++++++ internal/service/smarthttp/inforefs.go | 15 +++++++-------- internal/service/smarthttp/receive_pack.go | 19 +++---------------- internal/service/ssh/receive_pack.go | 22 +++------------------- 4 files changed, 21 insertions(+), 43 deletions(-) diff --git a/internal/git/const.go b/internal/git/const.go index 84796f4656..8293d99c30 100644 --- a/internal/git/const.go +++ b/internal/git/const.go @@ -6,4 +6,12 @@ const ( // NullSHA is the special value that Git uses to signal a ref or object does not exist NullSHA = "0000000000000000000000000000000000000000" + + // DisableAlternateRefsConfig is a config option for git-receive-pack. In + // case the repository belongs to an object pool, we want to prevent Git + // from including the pool's refs in the ref advertisement. We do this by + // rigging core.alternateRefsCommand to produce no output. Because Git + // itself will append the pool repository directory, the command ends + // with a "#". The end result is that Git runs `/bin/sh -c 'exit 0 # /path/to/pool.git`. + DisableAlternateRefsConfig = "core.alternateRefsCommand=exit 0 #" ) diff --git a/internal/service/smarthttp/inforefs.go b/internal/service/smarthttp/inforefs.go index 7b00186796..dcfb8b94db 100644 --- a/internal/service/smarthttp/inforefs.go +++ b/internal/service/smarthttp/inforefs.go @@ -42,14 +42,13 @@ func handleInfoRefs(ctx context.Context, service string, req *gitalypb.InfoRefsR return err } - // In case the repository belongs to an object pool, we want to prevent - // Git from including the pool's refs in the ref advertisement. We do - // this by rigging core.alternateRefsCommand to produce no output. - // Because Git itself will append the pool repository directory, the - // command ends with a "#". The end result is that Git runs - // `/bin/sh -c 'exit 0 # /path/to/pool.git`. - args := []string{"-c", "core.alternateRefsCommand=exit 0 #"} - for _, params := range req.GitConfigOptions { + opts := req.GitConfigOptions + if service == "receive-pack" { + opts = append(git.ReceivePackConfig(), opts...) + } + + var args []string + for _, params := range opts { args = append(args, "-c", params) } diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go index fd214f98c2..9b4d6e6560 100644 --- a/internal/service/smarthttp/receive_pack.go +++ b/internal/service/smarthttp/receive_pack.go @@ -1,15 +1,11 @@ package smarthttp import ( - "fmt" - grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/git" - "gitlab.com/gitlab-org/gitaly/internal/git/hooks" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/streamio" "google.golang.org/grpc/codes" @@ -41,17 +37,8 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac stdout := streamio.NewWriter(func(p []byte) error { return stream.Send(&gitalypb.PostReceivePackResponse{Data: p}) }) - env := []string{ - fmt.Sprintf("GL_ID=%s", req.GlId), - "GL_PROTOCOL=http", - fmt.Sprintf("GITLAB_SHELL_DIR=%s", config.Config.GitlabShell.Dir), - } - if req.GlRepository != "" { - env = append(env, fmt.Sprintf("GL_REPOSITORY=%s", req.GlRepository)) - } - if req.GlUsername != "" { - env = append(env, fmt.Sprintf("GL_USERNAME=%s", req.GlUsername)) - } + + env := append(git.ReceivePackEnv(req), "GL_PROTOCOL=http") repoPath, err := helper.GetRepoPath(req.Repository) if err != nil { @@ -61,7 +48,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac env = git.AddGitProtocolEnv(ctx, req, env) env = append(env, command.GitEnv...) - opts := append([]string{fmt.Sprintf("core.hooksPath=%s", hooks.Path())}, req.GitConfigOptions...) + opts := append(git.ReceivePackConfig(), req.GitConfigOptions...) gitOptions := git.BuildGitOptions(opts, "receive-pack", "--stateless-rpc", repoPath) cmd, err := git.BareCommand(ctx, stdin, stdout, nil, env, gitOptions...) diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go index e8224808c7..b94513a269 100644 --- a/internal/service/ssh/receive_pack.go +++ b/internal/service/ssh/receive_pack.go @@ -7,9 +7,7 @@ import ( log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/git" - "gitlab.com/gitlab-org/gitaly/internal/git/hooks" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/streamio" ) @@ -51,15 +49,8 @@ func sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, req *gitaly stderr := streamio.NewWriter(func(p []byte) error { return stream.Send(&gitalypb.SSHReceivePackResponse{Stderr: p}) }) - env := []string{ - fmt.Sprintf("GL_ID=%s", req.GlId), - fmt.Sprintf("GL_USERNAME=%s", req.GlUsername), - "GL_PROTOCOL=ssh", - fmt.Sprintf("GITLAB_SHELL_DIR=%s", config.Config.GitlabShell.Dir), - } - if req.GlRepository != "" { - env = append(env, fmt.Sprintf("GL_REPOSITORY=%s", req.GlRepository)) - } + + env := append(git.ReceivePackEnv(req), "GL_PROTOCOL=ssh") repoPath, err := helper.GetRepoPath(req.Repository) if err != nil { @@ -69,15 +60,8 @@ func sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, req *gitaly env = git.AddGitProtocolEnv(ctx, req, env) env = append(env, command.GitEnv...) - opts := append([]string{fmt.Sprintf("core.hooksPath=%s", hooks.Path())}, req.GitConfigOptions...) + opts := append(git.ReceivePackConfig(), req.GitConfigOptions...) - // In case the repository belongs to an object pool, we want to prevent - // Git from including the pool's refs in the ref advertisement. We do - // this by rigging core.alternateRefsCommand to produce no output. - // Because Git itself will append the pool repository directory, the - // command ends with a "#". The end result is that Git runs - // `/bin/sh -c 'exit 0 # /path/to/pool.git`. - opts = append(opts, "core.alternateRefsCommand=exit 0 #") gitOptions := git.BuildGitOptions(opts, "receive-pack", repoPath) cmd, err := git.BareCommand(ctx, stdin, stdout, stderr, env, gitOptions...) -- GitLab From c0e64a9e81a5297f963d5d5488a52410662c335d Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 25 Jun 2019 16:44:33 +0200 Subject: [PATCH 2/5] Remove unused constant --- internal/git/const.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/git/const.go b/internal/git/const.go index 8293d99c30..84796f4656 100644 --- a/internal/git/const.go +++ b/internal/git/const.go @@ -6,12 +6,4 @@ const ( // NullSHA is the special value that Git uses to signal a ref or object does not exist NullSHA = "0000000000000000000000000000000000000000" - - // DisableAlternateRefsConfig is a config option for git-receive-pack. In - // case the repository belongs to an object pool, we want to prevent Git - // from including the pool's refs in the ref advertisement. We do this by - // rigging core.alternateRefsCommand to produce no output. Because Git - // itself will append the pool repository directory, the command ends - // with a "#". The end result is that Git runs `/bin/sh -c 'exit 0 # /path/to/pool.git`. - DisableAlternateRefsConfig = "core.alternateRefsCommand=exit 0 #" ) -- GitLab From 4c0022397aed2d1b7e6f2d42770f8c4755f372c5 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 25 Jun 2019 16:45:45 +0200 Subject: [PATCH 3/5] Add missing file --- internal/git/receivepack.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 internal/git/receivepack.go diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go new file mode 100644 index 0000000000..19d1d8a7e9 --- /dev/null +++ b/internal/git/receivepack.go @@ -0,0 +1,36 @@ +package git + +import ( + "fmt" + + "gitlab.com/gitlab-org/gitaly/internal/config" + "gitlab.com/gitlab-org/gitaly/internal/git/hooks" +) + +type ReceivePackRequest interface { + GetGlId() string + GetGlUsername() string + GetGlRepository() string +} + +func ReceivePackEnv(req ReceivePackRequest) []string { + return []string{ + fmt.Sprintf("GL_ID=%s", req.GetGlId()), + fmt.Sprintf("GL_USERNAME=%s", req.GetGlUsername()), + fmt.Sprintf("GITLAB_SHELL_DIR=%s", config.Config.GitlabShell.Dir), + fmt.Sprintf("GL_REPOSITORY=%s", req.GetGlRepository()), + } +} + +func ReceivePackConfig() []string { + return []string{ + fmt.Sprintf("core.hooksPath=%s", hooks.Path()), + + // In case the repository belongs to an object pool, we want to prevent + // Git from including the pool's refs in the ref advertisement. We do + // this by rigging core.alternateRefsCommand to produce no output. + // Because Git itself will append the pool repository directory, the + // command ends with a "#". The end result is that Git runs `/bin/sh -c 'exit 0 # /path/to/pool.git`. + "core.alternateRefsCommand=exit 0 #", + } +} -- GitLab From 29144b29e79df60d17a6c51b71cb8916dda686ff Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 25 Jun 2019 16:46:05 +0200 Subject: [PATCH 4/5] changelog --- changelogs/unreleased/jv-dry-alternate-refs.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/jv-dry-alternate-refs.yml diff --git a/changelogs/unreleased/jv-dry-alternate-refs.yml b/changelogs/unreleased/jv-dry-alternate-refs.yml new file mode 100644 index 0000000000..83023f8811 --- /dev/null +++ b/changelogs/unreleased/jv-dry-alternate-refs.yml @@ -0,0 +1,5 @@ +--- +title: Remove duplication of receive-pack config +merge_request: 1332 +author: +type: other -- GitLab From 66b5e3d0e3673d5929ea286a02be043fa05fa936 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 25 Jun 2019 16:51:59 +0200 Subject: [PATCH 5/5] Add doc comments --- internal/git/receivepack.go | 8 +++++++- internal/service/smarthttp/receive_pack.go | 2 +- internal/service/ssh/receive_pack.go | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go index 19d1d8a7e9..8e1f2e54d4 100644 --- a/internal/git/receivepack.go +++ b/internal/git/receivepack.go @@ -7,13 +7,17 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/hooks" ) +// ReceivePackRequest abstracts away the different requests that end up +// spawning git-receive-pack. type ReceivePackRequest interface { GetGlId() string GetGlUsername() string GetGlRepository() string } -func ReceivePackEnv(req ReceivePackRequest) []string { +// HookEnv is information we pass down to the Git hooks during +// git-receive-pack. +func HookEnv(req ReceivePackRequest) []string { return []string{ fmt.Sprintf("GL_ID=%s", req.GetGlId()), fmt.Sprintf("GL_USERNAME=%s", req.GetGlUsername()), @@ -22,6 +26,8 @@ func ReceivePackEnv(req ReceivePackRequest) []string { } } +// ReceivePackConfig contains config options we want to enforce when +// receiving a push with git-receive-pack. func ReceivePackConfig() []string { return []string{ fmt.Sprintf("core.hooksPath=%s", hooks.Path()), diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go index 9b4d6e6560..75734a9c61 100644 --- a/internal/service/smarthttp/receive_pack.go +++ b/internal/service/smarthttp/receive_pack.go @@ -38,7 +38,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac return stream.Send(&gitalypb.PostReceivePackResponse{Data: p}) }) - env := append(git.ReceivePackEnv(req), "GL_PROTOCOL=http") + env := append(git.HookEnv(req), "GL_PROTOCOL=http") repoPath, err := helper.GetRepoPath(req.Repository) if err != nil { diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go index b94513a269..2deaa62b7a 100644 --- a/internal/service/ssh/receive_pack.go +++ b/internal/service/ssh/receive_pack.go @@ -50,7 +50,7 @@ func sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, req *gitaly return stream.Send(&gitalypb.SSHReceivePackResponse{Stderr: p}) }) - env := append(git.ReceivePackEnv(req), "GL_PROTOCOL=ssh") + env := append(git.HookEnv(req), "GL_PROTOCOL=ssh") repoPath, err := helper.GetRepoPath(req.Repository) if err != nil { -- GitLab