Merge branch 'jc-simplify-git-object-dir-handling-13-1' into '13-1-stable'

Fix pre-receive hooks not working with symlinked paths (13-1-stable backport)

See merge request gitlab-org/gitaly!2387

4 files changed, 85 insertions, 40 deletions

diff --git a/changelogs/unreleased/jc-simplify-git-object-dir-handling.yml b/changelogs/unreleased/jc-simplify-git-object-dir-handling.yml
new file mode 100644
index 000000000..61573e277
--- /dev/null
+++ b/changelogs/unreleased/jc-simplify-git-object-dir-handling.yml
@@ -0,0 +1,6 @@
+---
+title: Fix pre-receive hooks not working with symlinked paths
+  variable field
+merge_request: 2381
+author:
+type: fixed
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go
index 0d59f334d..b6fc51ae6 100644
--- a/cmd/gitaly-hooks/hooks.go
+++ b/cmd/gitaly-hooks/hooks.go
@@ -103,6 +103,7 @@ func main() {
 		}
 
 		environment := glValues()
+		environment = append(environment, gitObjectDirs()...)
 
 		for _, key := range []string{metadata.PraefectEnvKey, metadata.TransactionEnvKey} {
 			if value, ok := os.LookupEnv(key); ok {
@@ -175,33 +176,6 @@ func repositoryFromEnv() (*gitalypb.Repository, error) {
 		return nil, fmt.Errorf("unmarshal JSON %q: %w", repoString, err)
 	}
 
-	pwd, err := os.Getwd()
-	if err != nil {
-		return nil, fmt.Errorf("can't define working directory: %w", err)
-	}
-
-	gitObjDirAbs, ok := os.LookupEnv("GIT_OBJECT_DIRECTORY")
-	if ok {
-		gitObjDir, err := filepath.Rel(pwd, gitObjDirAbs)
-		if err != nil {
-			return nil, fmt.Errorf("can't define rel path %q: %w", gitObjDirAbs, err)
-		}
-		repo.GitObjectDirectory = gitObjDir
-	}
-	gitAltObjDirsAbs, ok := os.LookupEnv("GIT_ALTERNATE_OBJECT_DIRECTORIES")
-	if ok {
-		var gitAltObjDirs []string
-		for _, gitAltObjDirAbs := range strings.Split(gitAltObjDirsAbs, ":") {
-			gitAltObjDir, err := filepath.Rel(pwd, gitAltObjDirAbs)
-			if err != nil {
-				return nil, fmt.Errorf("can't define rel path %q: %w", gitAltObjDirAbs, err)
-			}
-			gitAltObjDirs = append(gitAltObjDirs, gitAltObjDir)
-		}
-
-		repo.GitAlternateObjectDirectories = gitAltObjDirs
-	}
-
 	return &repo, nil
 }
 
@@ -241,6 +215,20 @@ func glValues() []string {
 	return glEnvVars
 }
 
+func gitObjectDirs() []string {
+	var objectDirs []string
+	gitObjectDirectory, ok := os.LookupEnv("GIT_OBJECT_DIRECTORY")
+	if ok {
+		objectDirs = append(objectDirs, "GIT_OBJECT_DIRECTORY="+gitObjectDirectory)
+	}
+	gitAlternateObjectDirectories, ok := os.LookupEnv("GIT_ALTERNATE_OBJECT_DIRECTORIES")
+	if ok {
+		objectDirs = append(objectDirs, "GIT_ALTERNATE_OBJECT_DIRECTORIES="+gitAlternateObjectDirectories)
+	}
+
+	return objectDirs
+}
+
 func gitPushOptions() []string {
 	var gitPushOptions []string
 
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index 2a17ce886..b3a2c6263 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -48,7 +48,27 @@ func testMain(m *testing.M) int {
 	return m.Run()
 }
 
+func TestHooksPrePostWithSymlinkedStoragePath(t *testing.T) {
+	tempDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
+
+	originalStoragePath := config.Config.Storages[0].Path
+	symlinkedStoragePath := filepath.Join(tempDir, "storage")
+	require.NoError(t, os.Symlink(originalStoragePath, symlinkedStoragePath))
+
+	defer func() {
+		config.Config.Storages[0].Path = originalStoragePath
+	}()
+	config.Config.Storages[0].Path = symlinkedStoragePath
+
+	testHooksPrePostReceive(t)
+}
+
 func TestHooksPrePostReceive(t *testing.T) {
+	testHooksPrePostReceive(t)
+}
+
+func testHooksPrePostReceive(t *testing.T) {
 	defer func(cfg config.Cfg) {
 		config.Config = cfg
 	}(config.Config)
diff --git a/internal/service/hook/pre_receive.go b/internal/service/hook/pre_receive.go
index d1410a268..9415c0779 100644
--- a/internal/service/hook/pre_receive.go
+++ b/internal/service/hook/pre_receive.go
@@ -13,7 +13,6 @@ import (
 	"time"
 
 	"gitlab.com/gitlab-org/gitaly/internal/config"
-	"gitlab.com/gitlab-org/gitaly/internal/git/alternates"
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
 	"gitlab.com/gitlab-org/gitaly/internal/gitlabshell"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
@@ -43,20 +42,14 @@ func hookRequestEnv(req hookRequest) ([]string, error) {
 }
 
 func preReceiveEnv(req prePostRequest) ([]string, error) {
-	_, env, err := alternates.PathAndEnv(req.GetRepository())
+	env, err := hookRequestEnv(req)
 	if err != nil {
 		return nil, err
 	}
 
-	hookEnv, err := hookRequestEnv(req)
-	if err != nil {
-		return nil, err
-	}
-
-	env = append(env, hookEnv...)
 	env = append(env, hooks.GitPushOptions(req.GetGitPushOptions())...)
 
-	return append(hookEnv, env...), nil
+	return env, nil
 }
 
 func gitlabShellHook(hookName string) string {
@@ -137,6 +130,31 @@ func (s *server) voteOnTransaction(stream gitalypb.HookService_PreReceiveHookSer
 	return nil
 }
 
+func getRelativeObjectDirs(repoPath, gitObjectDir, gitAlternateObjectDirs string) (string, []string, error) {
+	repoPathReal, err := filepath.EvalSymlinks(repoPath)
+	if err != nil {
+		return "", nil, err
+	}
+
+	gitObjDirRel, err := filepath.Rel(repoPathReal, gitObjectDir)
+	if err != nil {
+		return "", nil, err
+	}
+
+	var gitAltObjDirsRel []string
+
+	for _, gitAltObjDirAbs := range strings.Split(gitAlternateObjectDirs, ":") {
+		gitAltObjDirRel, err := filepath.Rel(repoPathReal, gitAltObjDirAbs)
+		if err != nil {
+			return "", nil, err
+		}
+
+		gitAltObjDirsRel = append(gitAltObjDirsRel, gitAltObjDirRel)
+	}
+
+	return gitObjDirRel, gitAltObjDirsRel, nil
+}
+
 func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer) error {
 	firstRequest, err := stream.Recv()
 	if err != nil {
@@ -156,6 +174,21 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
 	stdout := streamio.NewWriter(func(p []byte) error { return stream.Send(&gitalypb.PreReceiveHookResponse{Stdout: p}) })
 	stderr := streamio.NewWriter(func(p []byte) error { return stream.Send(&gitalypb.PreReceiveHookResponse{Stderr: p}) })
 
+	if gitObjDir, gitAltObjDirs := getEnvVar("GIT_OBJECT_DIRECTORY", reqEnvVars), getEnvVar("GIT_ALTERNATE_OBJECT_DIRECTORIES", reqEnvVars); gitObjDir != "" && gitAltObjDirs != "" {
+		repoPath, err := helper.GetRepoPath(repository)
+		if err != nil {
+			return helper.ErrInternalf("getting repo path: %v", err)
+		}
+
+		gitObjectDirRel, gitAltObjectDirRel, err := getRelativeObjectDirs(repoPath, gitObjDir, gitAltObjDirs)
+		if err != nil {
+			return helper.ErrInternalf("getting relative git object directories: %v", err)
+		}
+
+		repository.GitObjectDirectory = gitObjectDirRel
+		repository.GitAlternateObjectDirectories = gitAltObjectDirRel
+	}
+
 	stdin := streamio.NewReader(func() ([]byte, error) {
 		req, err := stream.Recv()
 		return req.GetStdin(), err
@@ -187,13 +220,11 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
 		return helper.ErrInternalf("creating custom hooks executor: %v", err)
 	}
 
-	_, gitObjectDirEnv, err := alternates.PathAndEnv(repository)
+	env, err := preReceiveEnv(firstRequest)
 	if err != nil {
-		return helper.ErrInternalf("getting git object dir from request %v", err)
+		return helper.ErrInternalf("getting pre receive env: %v", err)
 	}
 
-	env := append(reqEnvVars, gitObjectDirEnv...)
-
 	if err = executor(
 		stream.Context(),
 		nil,