Make format to resolve verify error

An unrelated linting error was detected. This corrected the formatting.

6 files changed, 184 insertions, 16 deletions

diff --git a/changelogs/unreleased/po-ref-hook-opt.yml b/changelogs/unreleased/po-ref-hook-opt.yml
new file mode 100644
index 000000000..4b1a9b012
--- /dev/null
+++ b/changelogs/unreleased/po-ref-hook-opt.yml
@@ -0,0 +1,5 @@
+---
+title: Reference hook option for Git command DSL
+merge_request: 2596
+author:
+type: other
diff --git a/internal/git/command.go b/internal/git/command.go
index 6fb682913..f89ec74a2 100644
--- a/internal/git/command.go
+++ b/internal/git/command.go
@@ -23,12 +23,14 @@ func unsafeCmdWithEnv(ctx context.Context, extraEnv []string, repo repository.Gi
 
 // unsafeStdinCmd creates a git.Command with the given args and Repository that is
 // suitable for Write()ing to
-func unsafeStdinCmd(ctx context.Context, repo repository.GitRepo, args ...string) (*command.Command, error) {
+func unsafeStdinCmd(ctx context.Context, extraEnv []string, repo repository.GitRepo, args ...string) (*command.Command, error) {
 	args, env, err := argsAndEnv(repo, args...)
 	if err != nil {
 		return nil, err
 	}
 
+	env = append(env, extraEnv...)
+
 	return unsafeBareCmd(ctx, CmdStream{In: command.SetupStdin}, env, args...)
 }
 
diff --git a/internal/git/hooks.go b/internal/git/hooks.go
new file mode 100644
index 000000000..03f465535
--- /dev/null
+++ b/internal/git/hooks.go
@@ -0,0 +1,40 @@
+package git
+
+import (
+	"context"
+	"fmt"
+
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
+	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
+)
+
+// WithRefTxHook returns an option that populates the safe command with the
+// environment variables necessary to properly execute a reference hook for
+// repository changes that may possibly update references
+func WithRefTxHook(ctx context.Context, repo *gitalypb.Repository, cfg config.Cfg) CmdOpt {
+	return func(cc *cmdCfg) error {
+		rfEnvs, err := refHookEnv(ctx, repo, cfg)
+		if err != nil {
+			return fmt.Errorf("ref hook env var: %w", err)
+		}
+
+		cc.env = append(cc.env, rfEnvs...)
+		return nil
+	}
+}
+
+// refHookEnv returns all env vars required by the reference transaction hook
+func refHookEnv(ctx context.Context, repo *gitalypb.Repository, cfg config.Cfg) ([]string, error) {
+	repoJSON, err := jsonpbMarshaller.MarshalToString(repo)
+	if err != nil {
+		return nil, err
+	}
+
+	return []string{
+		"GITALY_SOCKET=" + config.GitalyInternalSocketPath(),
+		fmt.Sprintf("GITALY_REPO=%s", repoJSON),
+		fmt.Sprintf("GITALY_TOKEN=%s", cfg.Auth.Token),
+		fmt.Sprintf("%s=%v", featureflag.ReferenceTransactionHookEnvVar, featureflag.IsEnabled(ctx, featureflag.ReferenceTransactionHook)),
+	}, nil
+}
diff --git a/internal/git/hooks_test.go b/internal/git/hooks_test.go
new file mode 100644
index 000000000..a2a3726a3
--- /dev/null
+++ b/internal/git/hooks_test.go
@@ -0,0 +1,89 @@
+package git_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestWithRefHook(t *testing.T) {
+	testRepo, _, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	const token = "my-super-secure-token"
+	defer func(oldToken string) { config.Config.Auth.Token = oldToken }(config.Config.Auth.Token)
+	config.Config.Auth.Token = token
+
+	opt := git.WithRefTxHook(ctx, testRepo, config.Config)
+	subCmd := git.SubCmd{Name: "log", Flags: []git.Option{git.Flag{"-0"}}}
+
+	for _, tt := range []struct {
+		name string
+		fn   func() (*command.Command, error)
+	}{
+		{
+			name: "SafeCmd",
+			fn: func() (*command.Command, error) {
+				return git.SafeCmd(ctx, testRepo, nil, subCmd, opt)
+			},
+		},
+		{
+			name: "SafeCmdWithEnv",
+			fn: func() (*command.Command, error) {
+				return git.SafeCmdWithEnv(ctx, nil, testRepo, nil, subCmd, opt)
+			},
+		},
+		{
+			name: "SafeBareCmd",
+			fn: func() (*command.Command, error) {
+				return git.SafeBareCmd(ctx, git.CmdStream{}, nil, nil, subCmd, opt)
+			},
+		},
+		{
+			name: "SafeStdinCmd",
+			fn: func() (*command.Command, error) {
+				return git.SafeStdinCmd(ctx, testRepo, nil, subCmd, opt)
+			},
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			expectEnvVars := map[string]struct{}{
+				"GITALY_SOCKET":                     struct{}{},
+				"GITALY_REPO":                       struct{}{},
+				"GITALY_TOKEN":                      struct{}{},
+				"GITALY_REFERENCE_TRANSACTION_HOOK": struct{}{},
+			}
+
+			cmd, err := tt.fn()
+			require.NoError(t, err)
+
+			var actualEnvVars []string
+			for _, env := range cmd.Env() {
+				kv := strings.SplitN(env, "=", 2)
+				if len(kv) < 2 {
+					continue
+				}
+				key, val := kv[0], kv[1]
+
+				if _, ok := expectEnvVars[key]; ok {
+					assert.NotEmptyf(t, strings.TrimSpace(val),
+						"env var %s value should not be empty string", key)
+				}
+				actualEnvVars = append(actualEnvVars, key)
+			}
+
+			for k := range expectEnvVars {
+				assert.Contains(t, actualEnvVars, k)
+			}
+		})
+	}
+}
diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go
index f75afd55a..e50e36404 100644
--- a/internal/git/receivepack.go
+++ b/internal/git/receivepack.go
@@ -29,26 +29,24 @@ var jsonpbMarshaller = &jsonpb.Marshaler{}
 // ReceivePackHookEnv is information we pass down to the Git hooks during
 // git-receive-pack.
 func ReceivePackHookEnv(ctx context.Context, req ReceivePackRequest) ([]string, error) {
-	repo, err := jsonpbMarshaller.MarshalToString(req.GetRepository())
+	gitlabshellEnv, err := gitlabshell.Env()
 	if err != nil {
 		return nil, err
 	}
 
-	gitlabshellEnv, err := gitlabshell.Env()
+	env, err := refHookEnv(ctx, req.GetRepository(), config.Config)
 	if err != nil {
 		return nil, err
 	}
 
-	env := append([]string{
+	env = append(env,
 		fmt.Sprintf("GL_ID=%s", req.GetGlId()),
 		fmt.Sprintf("GL_USERNAME=%s", req.GetGlUsername()),
 		fmt.Sprintf("GL_REPOSITORY=%s", req.GetGlRepository()),
 		fmt.Sprintf("GL_PROJECT_PATH=%s", req.GetRepository().GetGlProjectPath()),
-		fmt.Sprintf("GITALY_SOCKET=" + config.GitalyInternalSocketPath()),
-		fmt.Sprintf("GITALY_REPO=%s", repo),
-		fmt.Sprintf("GITALY_TOKEN=%s", config.Config.Auth.Token),
 		fmt.Sprintf("%s=%s", featureflag.ReferenceTransactionHookEnvVar, strconv.FormatBool(featureflag.IsEnabled(ctx, featureflag.ReferenceTransactionHook))),
-	}, gitlabshellEnv...)
+	)
+	env = append(env, gitlabshellEnv...)
 
 	transaction, err := metadata.TransactionFromContext(ctx)
 	if err == nil {
diff --git a/internal/git/safecmd.go b/internal/git/safecmd.go
index c45aa3a8f..f305fbfe3 100644
--- a/internal/git/safecmd.go
+++ b/internal/git/safecmd.go
@@ -213,44 +213,78 @@ func ConvertGlobalOptions(options *gitalypb.GlobalOptions) []Option {
 	return globals
 }
 
+type cmdCfg struct {
+	env []string
+}
+
+// CmdOpt is an option for running a command
+type CmdOpt func(*cmdCfg) error
+
+func handleOpts(cc *cmdCfg, opts []CmdOpt) error {
+	for _, opt := range opts {
+		if err := opt(cc); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // SafeCmd creates a command.Command with the given args and Repository. It
 // validates the arguments in the command before executing.
-func SafeCmd(ctx context.Context, repo repository.GitRepo, globals []Option, sc Cmd) (*command.Command, error) {
-	return SafeCmdWithEnv(ctx, nil, repo, globals, sc)
+func SafeCmd(ctx context.Context, repo repository.GitRepo, globals []Option, sc Cmd, opts ...CmdOpt) (*command.Command, error) {
+	return SafeCmdWithEnv(ctx, nil, repo, globals, sc, opts...)
 }
 
 // SafeCmdWithEnv creates a command.Command with the given args, environment, and Repository. It
 // validates the arguments in the command before executing.
-func SafeCmdWithEnv(ctx context.Context, env []string, repo repository.GitRepo, globals []Option, sc Cmd) (*command.Command, error) {
+func SafeCmdWithEnv(ctx context.Context, env []string, repo repository.GitRepo, globals []Option, sc Cmd, opts ...CmdOpt) (*command.Command, error) {
+	cc := &cmdCfg{}
+
+	if err := handleOpts(cc, opts); err != nil {
+		return nil, err
+	}
+
 	args, err := combineArgs(globals, sc)
 	if err != nil {
 		return nil, err
 	}
 
-	return unsafeCmdWithEnv(ctx, env, repo, args...)
+	return unsafeCmdWithEnv(ctx, append(env, cc.env...), repo, args...)
 }
 
 // SafeBareCmd creates a git.Command with the given args, stream, and env. It
 // validates the arguments in the command before executing.
-func SafeBareCmd(ctx context.Context, stream CmdStream, env []string, globals []Option, sc Cmd) (*command.Command, error) {
+func SafeBareCmd(ctx context.Context, stream CmdStream, env []string, globals []Option, sc Cmd, opts ...CmdOpt) (*command.Command, error) {
+	cc := &cmdCfg{}
+
+	if err := handleOpts(cc, opts); err != nil {
+		return nil, err
+	}
+
 	args, err := combineArgs(globals, sc)
 	if err != nil {
 		return nil, err
 	}
 
-	return unsafeBareCmd(ctx, stream, env, args...)
+	return unsafeBareCmd(ctx, stream, append(env, cc.env...), args...)
 }
 
 // SafeStdinCmd creates a git.Command with the given args and Repository that is
 // suitable for Write()ing to. It validates the arguments in the command before
 // executing.
-func SafeStdinCmd(ctx context.Context, repo repository.GitRepo, globals []Option, sc SubCmd) (*command.Command, error) {
+func SafeStdinCmd(ctx context.Context, repo repository.GitRepo, globals []Option, sc SubCmd, opts ...CmdOpt) (*command.Command, error) {
+	cc := &cmdCfg{}
+
+	if err := handleOpts(cc, opts); err != nil {
+		return nil, err
+	}
+
 	args, err := combineArgs(globals, sc)
 	if err != nil {
 		return nil, err
 	}
 
-	return unsafeStdinCmd(ctx, repo, args...)
+	return unsafeStdinCmd(ctx, cc.env, repo, args...)
 }
 
 // SafeCmdWithoutRepo works like Command but without a git repository. It