cmd/gitaly-wrapper: Add tests for `isGitaly()`

When gitaly-wrapper tries to adopt a process, then it first verifies
that it is in fact the expected process. This is to guard against PID
wraparound, where the existing PID file contains a PID which has since
been taken by a completely unrelated command.

Add tests to verify this function works as expected. While at it, rename
it to `isExpectedProcess()`: the wrapper is not only used for Gitaly,
but also for Praefect and may thus not in fact check for Gitaly.

2 files changed, 36 insertions, 5 deletions

diff --git a/cmd/gitaly-wrapper/main.go b/cmd/gitaly-wrapper/main.go
index 7f3a93899..89b5be44d 100644
--- a/cmd/gitaly-wrapper/main.go
+++ b/cmd/gitaly-wrapper/main.go
@@ -53,7 +53,7 @@ func main() {
 		logger.WithError(err).Error("find process")
 	}
 
-	if process != nil && isGitaly(process, gitalyBin) {
+	if process != nil && isExpectedProcess(process, gitalyBin) {
 		logger.Info("adopting a process")
 	} else {
 		logger.Info("spawning a process")
@@ -164,13 +164,13 @@ func isAlive(p *os.Process) bool {
 	return p.Signal(syscall.Signal(0)) == nil
 }
 
-func isGitaly(p *os.Process, gitalyBin string) bool {
+func isExpectedProcess(p *os.Process, binary string) bool {
 	command, err := ps.Comm(p.Pid)
 	if err != nil {
 		return false
 	}
 
-	if filepath.Base(command) == filepath.Base(gitalyBin) {
+	if filepath.Base(command) == filepath.Base(binary) {
 		return true
 	}
 
diff --git a/cmd/gitaly-wrapper/main_test.go b/cmd/gitaly-wrapper/main_test.go
index 5895fd60d..0f053cb8f 100644
--- a/cmd/gitaly-wrapper/main_test.go
+++ b/cmd/gitaly-wrapper/main_test.go
@@ -39,11 +39,11 @@ func TestStolenPid(t *testing.T) {
 	require.Equal(t, cmd.Process.Pid, tail.Pid)
 
 	t.Run("stolen", func(t *testing.T) {
-		require.False(t, isGitaly(tail, "/path/to/gitaly"))
+		require.False(t, isExpectedProcess(tail, "/path/to/gitaly"))
 	})
 
 	t.Run("not stolen", func(t *testing.T) {
-		require.True(t, isGitaly(tail, "/path/to/tail"))
+		require.True(t, isExpectedProcess(tail, "/path/to/tail"))
 	})
 }
 
@@ -173,3 +173,34 @@ func TestReadPIDFile(t *testing.T) {
 		require.Equal(t, 12345, pid)
 	})
 }
+
+func TestIsExpectedProcess(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	executable := testhelper.WriteExecutable(t, filepath.Join(testhelper.TempDir(t), "noop"), []byte(
+		`#!/usr/bin/env bash
+		echo ready
+		read wait_until_killed
+	`))
+
+	cmd := exec.CommandContext(ctx, executable)
+	stdout, err := cmd.StdoutPipe()
+	require.NoError(t, err)
+	stdin, err := cmd.StdinPipe()
+	require.NoError(t, err)
+
+	require.NoError(t, cmd.Start())
+	defer func() {
+		testhelper.MustClose(t, stdin)
+		require.Error(t, cmd.Wait())
+	}()
+
+	// Wait for the process to be ready such that we know it's started up successfully
+	// and is executing the bash shell.
+	_, err = stdout.Read(make([]byte, 10))
+	require.NoError(t, err)
+
+	require.False(t, isExpectedProcess(cmd.Process, "does not match"))
+	require.True(t, isExpectedProcess(cmd.Process, "bash"))
+}