Merge branch 'pks-user-merge-branch-post-receive-hook' into 'master'

operations: Fix PostReceive hook receiving no input

See merge request gitlab-org/gitaly!2653

3 files changed, 27 insertions, 7 deletions

diff --git a/changelogs/unreleased/pks-user-merge-branch-post-receive-hook.yml b/changelogs/unreleased/pks-user-merge-branch-post-receive-hook.yml
new file mode 100644
index 000000000..7dd7030f4
--- /dev/null
+++ b/changelogs/unreleased/pks-user-merge-branch-post-receive-hook.yml
@@ -0,0 +1,5 @@
+---
+title: 'operations: Fix PostReceive hook receiving no input'
+merge_request: 2653
+author:
+type: fixed
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index f5ab8a346..5568872b8 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -125,10 +125,10 @@ func (s *server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Re
 		fmt.Sprintf("GITALY_TOKEN=%s", s.cfg.Auth.Token),
 	}, gitlabshellEnv...)
 
-	stdin := bytes.NewBufferString(fmt.Sprintf("%s %s %s\n", oldrev, newrev, reference))
+	changes := fmt.Sprintf("%s %s %s\n", oldrev, newrev, reference)
 	var stdout, stderr bytes.Buffer
 
-	if err := s.hookManager.PreReceiveHook(ctx, repo, env, stdin, &stdout, &stderr); err != nil {
+	if err := s.hookManager.PreReceiveHook(ctx, repo, env, strings.NewReader(changes), &stdout, &stderr); err != nil {
 		return preReceiveError{message: stdout.String()}
 	}
 	if err := s.hookManager.UpdateHook(ctx, repo, reference, oldrev, newrev, env, &stdout, &stderr); err != nil {
@@ -148,7 +148,7 @@ func (s *server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Re
 		return updateRefError{reference: reference}
 	}
 
-	if err := s.hookManager.PostReceiveHook(ctx, repo, nil, env, stdin, &stdout, &stderr); err != nil {
+	if err := s.hookManager.PostReceiveHook(ctx, repo, nil, env, strings.NewReader(changes), &stdout, &stderr); err != nil {
 		return err
 	}
 
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index 8e0cb3faa..eb9e01d32 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
+	"os"
 	"os/exec"
 	"strings"
 	"testing"
@@ -65,10 +66,18 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) {
 
 	hooks := GitlabHooks
 	hookTempfiles := make([]string, len(hooks))
-	for i, h := range hooks {
-		var cleanup func()
-		hookTempfiles[i], cleanup = testhelper.WriteEnvToCustomHook(t, testRepoPath, h)
+	for i, hook := range hooks {
+		outputFile, err := ioutil.TempFile("", "")
+		require.NoError(t, err)
+		require.NoError(t, outputFile.Close())
+		defer os.Remove(outputFile.Name())
+
+		script := fmt.Sprintf("#!/bin/sh\n(cat && env) >%s \n", outputFile.Name())
+		cleanup, err := testhelper.WriteCustomHook(testRepoPath, hook, []byte(script))
+		require.NoError(t, err)
 		defer cleanup()
+
+		hookTempfiles[i] = outputFile.Name()
 	}
 
 	mergeCommitMessage := "Merged by Gitaly"
@@ -117,7 +126,13 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) {
 	for i, h := range hooks {
 		hookEnv, err := ioutil.ReadFile(hookTempfiles[i])
 		require.NoError(t, err)
-		require.Contains(t, strings.Split(string(hookEnv), "\n"), expectedGlID, "expected env of hook %q to contain %q", h, expectedGlID)
+
+		lines := strings.Split(string(hookEnv), "\n")
+		require.Contains(t, lines, expectedGlID, "expected env of hook %q to contain %q", h, expectedGlID)
+
+		if h == "pre-receive" || h == "post-receive" {
+			require.Regexp(t, mergeBranchHeadBefore+" .* refs/heads/"+mergeBranchName, lines[0], "expected env of hook %q to contain reference change", h)
+		}
 	}
 }