diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-01-11 13:51:04 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-01-14 17:25:03 +0300 |
commit | 8cab4f8b7423dccc64211265d7d55a1028f86af1 (patch) | |
tree | 9e3df7a7d40d27dc84bdb2238ea54abbf8bccac2 /cmd/gitaly-wrapper | |
parent | 55ce2628b6795bf5f80b1fad4916568b049c6533 (diff) |
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.
Diffstat (limited to 'cmd/gitaly-wrapper')
-rw-r--r-- | cmd/gitaly-wrapper/main.go | 6 | ||||
-rw-r--r-- | cmd/gitaly-wrapper/main_test.go | 35 |
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")) +} |