sidechannel: Convert to use runtime directory to store socketspks-sidechannel-migrate-to-runtime-dir

The sidechannel code is used to create sidechannels that circumvent gRPC
for efficiency's sake. This socket is currently created in the system's
temporary directory as returned by `os.MkdirTemp()`. This has the very
real downside that it easily leaks created files and directories in case
we have a logic bug anywhere.

We have recently introduced a new runtime directory that can help us in
this situation: the runtime directory is created once at a central place
and will be cleaned up when shutting down Gitaly. Consequentially, even
if we leaked and sidechannel files, we'd still remove them on shutdown.
And even if we didn't: the runtime directory is designed so that we can
check whether it's used because it has the current process's PID as part
of its component. So if a runtime directory exists whose PID doesn't
refer to any existing process it's safe to remove. While we don't have
any such logic yet, it can easily be added at a later point and have all
code which started to use the runtime directory benefit at the same
time.

Migrate the sidechannel code to create sockets in a subdirectory within
the runtime directory called "sidechannel.d" if the runtime directory is
set via the hooks payload.

Changelog: changed

3 files changed, 50 insertions, 0 deletions

diff --git a/internal/gitaly/service/hook/pack_objects_test.go b/internal/gitaly/service/hook/pack_objects_test.go
index 54deef710..e2ffb15b8 100644
--- a/internal/gitaly/service/hook/pack_objects_test.go
+++ b/internal/gitaly/service/hook/pack_objects_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/pktline"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
@@ -23,6 +24,18 @@ import (
 	"google.golang.org/grpc/codes"
 )
 
+func runTestsWithRuntimeDir(t *testing.T, testFunc func(*testing.T, string)) {
+	t.Helper()
+
+	t.Run("no runtime dir", func(t *testing.T) {
+		testFunc(t, "")
+	})
+
+	t.Run("with runtime dir", func(t *testing.T) {
+		testFunc(t, testhelper.TempDir(t))
+	})
+}
+
 func cfgWithCache(t *testing.T) config.Cfg {
 	cfg := testcfg.Build(t)
 	cfg.PackObjectsCache.Enabled = true
@@ -59,6 +72,11 @@ func TestParsePackObjectsArgs(t *testing.T) {
 }
 
 func TestServer_PackObjectsHook_separateContext(t *testing.T) {
+	t.Parallel()
+	runTestsWithRuntimeDir(t, testServerPackObjectsHookSeparateContextWithRuntimeDir)
+}
+
+func testServerPackObjectsHookSeparateContextWithRuntimeDir(t *testing.T, runtimeDir string) {
 	cfg := cfgWithCache(t)
 	cfg.SocketPath = runHooksServer(t, cfg, nil)
 
@@ -85,6 +103,9 @@ func TestServer_PackObjectsHook_separateContext(t *testing.T) {
 
 	ctx1, wt1, err := hookPkg.SetupSidechannel(
 		ctx1,
+		git.HooksPayload{
+			RuntimeDir: runtimeDir,
+		},
 		func(c *net.UnixConn) error {
 			defer close(start2)
 			<-start1
@@ -121,6 +142,9 @@ func TestServer_PackObjectsHook_separateContext(t *testing.T) {
 	var stdout2 []byte
 	ctx2, wt2, err := hookPkg.SetupSidechannel(
 		ctx2,
+		git.HooksPayload{
+			RuntimeDir: runtimeDir,
+		},
 		func(c *net.UnixConn) error {
 			<-start2
 			if _, err := io.WriteString(c, stdin); err != nil {
@@ -161,6 +185,11 @@ func TestServer_PackObjectsHook_separateContext(t *testing.T) {
 }
 
 func TestServer_PackObjectsHook_usesCache(t *testing.T) {
+	t.Parallel()
+	runTestsWithRuntimeDir(t, testServerPackObjectsHookUsesCache)
+}
+
+func testServerPackObjectsHookUsesCache(t *testing.T, runtimeDir string) {
 	cfg := cfgWithCache(t)
 
 	tlc := &streamcache.TestLoggingCache{}
@@ -178,6 +207,9 @@ func TestServer_PackObjectsHook_usesCache(t *testing.T) {
 		var stdout []byte
 		ctx, wt, err := hookPkg.SetupSidechannel(
 			ctx,
+			git.HooksPayload{
+				RuntimeDir: runtimeDir,
+			},
 			func(c *net.UnixConn) error {
 				if _, err := io.WriteString(c, "3dd08961455abf80ef9115f4afdc1c6f968b503c\n--not\n\n"); err != nil {
 					return err
@@ -236,7 +268,10 @@ func TestServer_PackObjectsHook_usesCache(t *testing.T) {
 
 func TestServer_PackObjectsHookWithSidechannel(t *testing.T) {
 	t.Parallel()
+	runTestsWithRuntimeDir(t, testServerPackObjectsHookWithSidechannelWithRuntimeDir)
+}
 
+func testServerPackObjectsHookWithSidechannelWithRuntimeDir(t *testing.T, runtimeDir string) {
 	testCases := []struct {
 		desc  string
 		stdin string
@@ -268,6 +303,9 @@ func TestServer_PackObjectsHookWithSidechannel(t *testing.T) {
 			var packets []string
 			ctx, wt, err := hookPkg.SetupSidechannel(
 				ctx,
+				git.HooksPayload{
+					RuntimeDir: runtimeDir,
+				},
 				func(c *net.UnixConn) error {
 					if _, err := io.WriteString(c, tc.stdin); err != nil {
 						return err
@@ -351,6 +389,8 @@ func TestServer_PackObjectsHookWithSidechannel(t *testing.T) {
 }
 
 func TestServer_PackObjectsHookWithSidechannel_invalidArgument(t *testing.T) {
+	t.Parallel()
+
 	cfg := testcfg.Build(t)
 	cfg.SocketPath = runHooksServer(t, cfg, nil)
 	ctx := testhelper.Context(t)
@@ -389,11 +429,19 @@ func TestServer_PackObjectsHookWithSidechannel_invalidArgument(t *testing.T) {
 }
 
 func TestServer_PackObjectsHookWithSidechannel_Canceled(t *testing.T) {
+	t.Parallel()
+	runTestsWithRuntimeDir(t, testServerPackObjectsHookWithSidechannelCanceledWithRuntimeDir)
+}
+
+func testServerPackObjectsHookWithSidechannelCanceledWithRuntimeDir(t *testing.T, runtimeDir string) {
 	cfg := cfgWithCache(t)
 	ctx := testhelper.Context(t)
 
 	ctx, wt, err := hookPkg.SetupSidechannel(
 		ctx,
+		git.HooksPayload{
+			RuntimeDir: runtimeDir,
+		},
 		func(c *net.UnixConn) error {
 			// Simulate a client that successfully initiates a request, but hangs up
 			// before fully consuming the response.
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index 122defe7e..7eb070b4d 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -101,6 +101,7 @@ func TestSuccessfulReceivePackRequest(t *testing.T) {
 	payload.Transaction = nil
 
 	require.Equal(t, git.HooksPayload{
+		RuntimeDir:          cfg.RuntimeDir,
 		InternalSocket:      cfg.GitalyInternalSocketPath(),
 		InternalSocketToken: cfg.Auth.Token,
 		ReceiveHooksPayload: &git.ReceiveHooksPayload{
diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go
index cda2fa4aa..90797458d 100644
--- a/internal/gitaly/service/ssh/receive_pack_test.go
+++ b/internal/gitaly/service/ssh/receive_pack_test.go
@@ -158,6 +158,7 @@ func TestReceivePackPushSuccess(t *testing.T) {
 	}
 
 	require.Equal(t, git.HooksPayload{
+		RuntimeDir:          cfg.RuntimeDir,
 		InternalSocket:      cfg.GitalyInternalSocketPath(),
 		InternalSocketToken: cfg.Auth.Token,
 		ReceiveHooksPayload: &git.ReceiveHooksPayload{