hooks: Fix incomplete test setups

When invoking hooks, the code assumes several environment variables to
exist. Most importantly, we need to provide information about where to
find the gitaly-hooks binary, but also meta information like the user
who caused the hook execution and the repository it was triggered in.
While many tests already set this up correctly, others don't. This
worked fine until now, but this is mostly by luck only. And as soon as
we're going to expect the HooksPayload to always exist in the
environment, those tests will fail.

Let's prepare for this change by adjusting our tests to always inject a
HooksPayload.

8 files changed, 102 insertions, 15 deletions

diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index ded2f639c..3f49e5b83 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -480,6 +480,13 @@ func TestHooksPostReceiveFailed(t *testing.T) {
 			}.Env()
 			require.NoError(t, err)
 
+			praefect := &metadata.PraefectServer{
+				SocketPath: "/path/to/socket",
+				Token:      "secret",
+			}
+			praefectEnv, err := praefect.Env()
+			require.NoError(t, err)
+
 			env := envForHooks(t, tempGitlabShellDir, testRepo,
 				glHookValues{
 					GLID:       glID,
@@ -489,7 +496,7 @@ func TestHooksPostReceiveFailed(t *testing.T) {
 				},
 				proxyValues{},
 			)
-			env = append(env, transactionEnv)
+			env = append(env, transactionEnv, praefectEnv)
 
 			cmd := exec.Command(postReceiveHookPath)
 			cmd.Env = env
diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go
index 1a213f1be..1700b4828 100644
--- a/internal/gitaly/hook/postreceive_test.go
+++ b/internal/gitaly/hook/postreceive_test.go
@@ -96,6 +96,13 @@ func TestPostReceive_customHook(t *testing.T) {
 	}.Env()
 	require.NoError(t, err)
 
+	praefect := &metadata.PraefectServer{
+		SocketPath: "/path/to/socket",
+		Token:      "secret",
+	}
+	praefectEnv, err := praefect.Env()
+	require.NoError(t, err)
+
 	testCases := []struct {
 		desc           string
 		env            []string
@@ -166,14 +173,14 @@ func TestPostReceive_customHook(t *testing.T) {
 		},
 		{
 			desc:           "hook is executed on primary",
-			env:            append(standardEnv, primaryEnv),
+			env:            append(standardEnv, primaryEnv, praefectEnv),
 			stdin:          "changes\n",
 			hook:           "#!/bin/sh\necho foo\n",
 			expectedStdout: "foo\n",
 		},
 		{
 			desc:  "hook is not executed on secondary",
-			env:   append(standardEnv, secondaryEnv),
+			env:   append(standardEnv, secondaryEnv, praefectEnv),
 			stdin: "changes\n",
 			hook:  "#!/bin/sh\necho foo\n",
 		},
diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go
index 0412e5570..ee43904e5 100644
--- a/internal/gitaly/hook/prereceive_test.go
+++ b/internal/gitaly/hook/prereceive_test.go
@@ -45,6 +45,13 @@ func TestPrereceive_customHooks(t *testing.T) {
 	}.Env()
 	require.NoError(t, err)
 
+	praefect := &metadata.PraefectServer{
+		SocketPath: "/path/to/socket",
+		Token:      "secret",
+	}
+	praefectEnv, err := praefect.Env()
+	require.NoError(t, err)
+
 	testCases := []struct {
 		desc           string
 		env            []string
@@ -99,14 +106,14 @@ func TestPrereceive_customHooks(t *testing.T) {
 		},
 		{
 			desc:           "hook is executed on primary",
-			env:            append(standardEnv, primaryEnv),
+			env:            append(standardEnv, primaryEnv, praefectEnv),
 			hook:           "#!/bin/sh\necho foo\n",
 			stdin:          "change\n",
 			expectedStdout: "foo\n",
 		},
 		{
 			desc:  "hook is not executed on secondary",
-			env:   append(standardEnv, secondaryEnv),
+			env:   append(standardEnv, secondaryEnv, praefectEnv),
 			hook:  "#!/bin/sh\necho foo\n",
 			stdin: "change\n",
 		},
diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go
index b219e7d3b..a7281787a 100644
--- a/internal/gitaly/hook/update_test.go
+++ b/internal/gitaly/hook/update_test.go
@@ -42,6 +42,13 @@ func TestUpdate_customHooks(t *testing.T) {
 	}.Env()
 	require.NoError(t, err)
 
+	praefect := &metadata.PraefectServer{
+		SocketPath: "/path/to/socket",
+		Token:      "secret",
+	}
+	praefectEnv, err := praefect.Env()
+	require.NoError(t, err)
+
 	hash1 := strings.Repeat("1", 40)
 	hash2 := strings.Repeat("2", 40)
 
@@ -123,7 +130,7 @@ func TestUpdate_customHooks(t *testing.T) {
 		},
 		{
 			desc:           "hook is executed on primary",
-			env:            append(standardEnv, primaryEnv),
+			env:            append(standardEnv, primaryEnv, praefectEnv),
 			reference:      "refs/heads/master",
 			oldHash:        hash1,
 			newHash:        hash2,
@@ -132,7 +139,7 @@ func TestUpdate_customHooks(t *testing.T) {
 		},
 		{
 			desc:      "hook is not executed on secondary",
-			env:       append(standardEnv, secondaryEnv),
+			env:       append(standardEnv, secondaryEnv, praefectEnv),
 			reference: "refs/heads/master",
 			oldHash:   hash1,
 			newHash:   hash2,
diff --git a/internal/gitaly/service/hook/post_receive_test.go b/internal/gitaly/service/hook/post_receive_test.go
index 726798e5c..26921e005 100644
--- a/internal/gitaly/service/hook/post_receive_test.go
+++ b/internal/gitaly/service/hook/post_receive_test.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
@@ -36,7 +37,7 @@ func TestPostReceiveInvalidArgument(t *testing.T) {
 	testhelper.RequireGrpcError(t, err, codes.InvalidArgument)
 }
 
-func transactionEnv(t *testing.T, primary bool) string {
+func transactionEnv(t *testing.T, primary bool) []string {
 	t.Helper()
 
 	transaction := metadata.Transaction{
@@ -44,11 +45,17 @@ func transactionEnv(t *testing.T, primary bool) string {
 		Node:    "node-1",
 		Primary: primary,
 	}
+	transactionEnv, err := transaction.Env()
+	require.NoError(t, err)
 
-	env, err := transaction.Env()
+	praefect := &metadata.PraefectServer{
+		SocketPath: "/path/to/socket",
+		Token:      "secret",
+	}
+	praefectEnv, err := praefect.Env()
 	require.NoError(t, err)
 
-	return env
+	return []string{transactionEnv, praefectEnv}
 }
 
 func TestHooksMissingStdin(t *testing.T) {
@@ -95,7 +102,7 @@ func TestHooksMissingStdin(t *testing.T) {
 
 	testCases := []struct {
 		desc string
-		env  string
+		env  []string
 		fail bool
 	}{
 		{
@@ -120,15 +127,20 @@ func TestHooksMissingStdin(t *testing.T) {
 			ctx, cancel := testhelper.Context()
 			defer cancel()
 
+			hooksPayload, err := git.NewHooksPayload(config.Config, testRepo).Env()
+			require.NoError(t, err)
+
 			stream, err := client.PostReceiveHook(ctx)
 			require.NoError(t, err)
 			require.NoError(t, stream.Send(&gitalypb.PostReceiveHookRequest{
 				Repository: testRepo,
-				EnvironmentVariables: []string{
+				EnvironmentVariables: append([]string{
+					hooksPayload,
 					"GL_ID=key_id",
 					"GL_USERNAME=username",
 					"GL_PROTOCOL=protocol",
-					"GL_REPOSITORY=repository", tc.env},
+					"GL_REPOSITORY=repository",
+				}, tc.env...),
 			}))
 
 			go func() {
@@ -245,7 +257,11 @@ To create a merge request for okay, visit:
 			stream, err := client.PostReceiveHook(ctx)
 			require.NoError(t, err)
 
+			hooksPayload, err := git.NewHooksPayload(config.Config, testRepo).Env()
+			require.NoError(t, err)
+
 			envVars := []string{
+				hooksPayload,
 				"GL_ID=key_id",
 				"GL_USERNAME=username",
 				"GL_PROTOCOL=protocol",
@@ -254,7 +270,8 @@ To create a merge request for okay, visit:
 
 			require.NoError(t, stream.Send(&gitalypb.PostReceiveHookRequest{
 				Repository:           testRepo,
-				EnvironmentVariables: envVars}))
+				EnvironmentVariables: envVars,
+			}))
 
 			go func() {
 				writer := streamio.NewWriter(func(p []byte) error {
diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go
index 8ddf82dab..512bb487f 100644
--- a/internal/gitaly/service/hook/pre_receive_test.go
+++ b/internal/gitaly/service/hook/pre_receive_test.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
@@ -144,10 +145,14 @@ func TestPreReceiveHook_GitlabAPIAccess(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
+	hooksPayload, err := git.NewHooksPayload(config.Config, testRepo).Env()
+	require.NoError(t, err)
+
 	stdin := bytes.NewBufferString(changes)
 	req := gitalypb.PreReceiveHookRequest{
 		Repository: testRepo,
 		EnvironmentVariables: []string{
+			hooksPayload,
 			"GL_ID=" + glID,
 			"GL_PROTOCOL=" + protocol,
 			"GL_USERNAME=username",
@@ -247,11 +252,15 @@ func TestPreReceive_APIErrors(t *testing.T) {
 			ctx, cancel := testhelper.Context()
 			defer cancel()
 
+			hooksPayload, err := git.NewHooksPayload(config.Config, testRepo).Env()
+			require.NoError(t, err)
+
 			stream, err := client.PreReceiveHook(ctx)
 			require.NoError(t, err)
 			require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{
 				Repository: testRepo,
 				EnvironmentVariables: []string{
+					hooksPayload,
 					"GL_ID=key-123",
 					"GL_PROTOCOL=web",
 					"GL_USERNAME=username",
@@ -310,11 +319,15 @@ exit %d
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
+	hooksPayload, err := git.NewHooksPayload(config.Config, testRepo).Env()
+	require.NoError(t, err)
+
 	stream, err := client.PreReceiveHook(ctx)
 	require.NoError(t, err)
 	require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{
 		Repository: testRepo,
 		EnvironmentVariables: []string{
+			hooksPayload,
 			"GL_ID=key-123",
 			"GL_PROTOCOL=web",
 			"GL_USERNAME=username",
@@ -438,12 +451,24 @@ func TestPreReceiveHook_Primary(t *testing.T) {
 			transactionEnv, err := transaction.Env()
 			require.NoError(t, err)
 
+			praefect := &metadata.PraefectServer{
+				SocketPath: "/path/to/socket",
+				Token:      "secret",
+			}
+			praefectEnv, err := praefect.Env()
+			require.NoError(t, err)
+
+			hooksPayload, err := git.NewHooksPayload(config.Config, testRepo).Env()
+			require.NoError(t, err)
+
 			environment := []string{
+				hooksPayload,
 				"GL_ID=key-123",
 				"GL_PROTOCOL=web",
 				"GL_USERNAME=username",
 				"GL_REPOSITORY=repository",
 				transactionEnv,
+				praefectEnv,
 			}
 
 			stream, err := client.PreReceiveHook(ctx)
diff --git a/internal/gitaly/service/hook/reference_transaction_test.go b/internal/gitaly/service/hook/reference_transaction_test.go
index c7052f4fc..2e9241632 100644
--- a/internal/gitaly/service/hook/reference_transaction_test.go
+++ b/internal/gitaly/service/hook/reference_transaction_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/metadata"
@@ -136,7 +137,11 @@ func TestReferenceTransactionHook(t *testing.T) {
 			transactionEnv, err := transaction.Env()
 			require.NoError(t, err)
 
+			hooksPayload, err := git.NewHooksPayload(config.Config, testRepo).Env()
+			require.NoError(t, err)
+
 			environment := []string{
+				hooksPayload,
 				transactionEnv,
 				transactionServerEnv,
 			}
diff --git a/internal/gitaly/service/hook/update_test.go b/internal/gitaly/service/hook/update_test.go
index 61abb13e1..76515b97c 100644
--- a/internal/gitaly/service/hook/update_test.go
+++ b/internal/gitaly/service/hook/update_test.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -43,6 +44,17 @@ func TestUpdate_CustomHooks(t *testing.T) {
 	client, conn := newHooksClient(t, serverSocketPath)
 	defer conn.Close()
 
+	hooksPayload, err := git.NewHooksPayload(config.Config, testRepo).Env()
+	require.NoError(t, err)
+
+	envVars := []string{
+		hooksPayload,
+		"GL_ID=key-123",
+		"GL_USERNAME=username",
+		"GL_PROTOCOL=protocol",
+		"GL_REPOSITORY=repository",
+	}
+
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 	req := gitalypb.UpdateHookRequest{
@@ -50,7 +62,7 @@ func TestUpdate_CustomHooks(t *testing.T) {
 		Ref:                  []byte("master"),
 		OldValue:             strings.Repeat("a", 40),
 		NewValue:             strings.Repeat("b", 40),
-		EnvironmentVariables: []string{"GL_ID=key-123", "GL_USERNAME=username", "GL_PROTOCOL=protocol", "GL_REPOSITORY=repository"},
+		EnvironmentVariables: envVars,
 	}
 
 	errorMsg := "error123"