gitaly/config: Introduce runtime directory configuration

In Gitaly, we're creating different kinds of files at runtime which are
required to operate correctly. These files are by default created in the
operating system's temporary directory, which is typically `/tmp`. While
it is clear that this directory can often be tmpfs and thus volatile,
this is perfectly fine: we regenerate the runtime data on every start
anyway.

Modern systems based on systemd use systemd-tmpfiles(8) though, which
also supports regular pruning of temporary files. So if the files we
create in `/tmp` aren't accessed for a specific grace period then the
daemon will clean those up. This problem becomes a lot worse though if
`/tmp` is mounted with the `noatime` mount option: even if files are
constantly used, systemd will eventually remove them anyway. Of course,
this completely breaks all parts of Gitaly which rely on these files:
hooks, the Git execution environment, and internal sockets.

The root cause for this problem is that Gitaly doesn't have a go-to
solution to host all such files, but instead it has ad-hoc solutions
for every new kind of file we need to exist at runtime. If we had that,
and if its location was configurable such that administrators can decide
themselves where to put them so that they don't get pruned, then this
problem wouldn't exist or at least be the responsibility of the admin.

This commit thus introduces a new runtime directory configuration into
Gitaly that is supposed to unify all current locations where we create
runtime files into a single well-defined location. This reduces the
problem we need to solve into a single one instead of creating the
problem anew for every new kind of runtime data.

By default, we're still kind of forced to create the runtime directory
in `/tmp`: except for the storage locations, it is the only location
known to be writeable by us. While we could try and abuse storage
locations, e.g. by just using the first storage as the location for the
runtime directory, this would put additional restrictions on the storage
paths which don't currently exist because we need to ensure short path
names so that sockets continue to work alright. But on systems where it
is known that `/tmp` will get regularly cleaned up, an administrator can
just point the new `runtime_dir` config to an arbitrary existing path,
which will then cover all runtime files.

Changelog: added

6 files changed, 149 insertions, 0 deletions

diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index 7d58bdd88..a0e231cb1 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -130,6 +130,12 @@ func run(cfg config.Cfg) error {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
+	defer func() {
+		if err := os.RemoveAll(cfg.RuntimeDir); err != nil {
+			log.Warn("could not clean up runtime dir")
+		}
+	}()
+
 	b, err := bootstrap.New(promauto.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "gitaly_connections_total",
diff --git a/config.toml.example b/config.toml.example
index eaa23fd8e..81df9f246 100644
--- a/config.toml.example
+++ b/config.toml.example
@@ -7,6 +7,11 @@ socket_path = "/home/git/gitlab/tmp/sockets/private/gitaly.socket"
 # The directory where Gitaly's executables are stored
 bin_dir = "/home/git/gitaly/_build/bin"
 
+# # Optional: The directory where Gitaly can create all files required to
+# # properly operate at runtime. If not set, Gitaly will create a directory in
+# # the global temporary directory. This directory must exist.
+# runtime_dir = "/home/git/gitaly/run"
+
 # # Optional: listen on a TCP socket. This is insecure (no authentication)
 # listen_addr = "localhost:9999"
 # tls_listen_addr = "localhost:8888"
diff --git a/internal/git/gittest/testhelper_test.go b/internal/git/gittest/testhelper_test.go
index 2085d22a0..4565301d2 100644
--- a/internal/git/gittest/testhelper_test.go
+++ b/internal/git/gittest/testhelper_test.go
@@ -45,6 +45,9 @@ func setup(t testing.TB) (config.Cfg, *gitalypb.Repository, string) {
 	cfg.BinDir = filepath.Join(rootDir, "bin.d")
 	require.NoError(t, os.Mkdir(cfg.BinDir, 0o755))
 
+	cfg.RuntimeDir = filepath.Join(rootDir, "run.d")
+	require.NoError(t, os.Mkdir(cfg.RuntimeDir, 0o700))
+
 	require.NoError(t, cfg.Validate())
 
 	repo, repoPath := CloneRepo(t, cfg, cfg.Storages[0])
diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index 9bd46f2df..9ff270c8c 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -48,6 +48,7 @@ type Cfg struct {
 	TLSListenAddr          string            `toml:"tls_listen_addr" split_words:"true"`
 	PrometheusListenAddr   string            `toml:"prometheus_listen_addr" split_words:"true"`
 	BinDir                 string            `toml:"bin_dir"`
+	RuntimeDir             string            `toml:"runtime_dir"`
 	Git                    Git               `toml:"git" envconfig:"git"`
 	Storages               []Storage         `toml:"storage" envconfig:"storage"`
 	Logging                Logging           `toml:"logging" envconfig:"logging"`
@@ -187,6 +188,7 @@ func (cfg *Cfg) Validate() error {
 		cfg.validateShell,
 		cfg.ConfigureRuby,
 		cfg.validateBinDir,
+		cfg.validateRuntimeDir,
 		cfg.validateInternalSocketDir,
 		cfg.validateMaintenance,
 		cfg.validateCgroups,
@@ -213,6 +215,46 @@ func (cfg *Cfg) setDefaults() error {
 		cfg.Hooks.CustomHooksDir = filepath.Join(cfg.GitlabShell.Dir, "hooks")
 	}
 
+	if cfg.RuntimeDir == "" {
+		// If there is no runtime directory configured we just use a temporary runtime
+		// directory. This may not always be an ideal choice given that it's typically
+		// created at `/tmp`, which may get periodically pruned if `noatime` is set.
+		runtimeDir, err := os.MkdirTemp("", "gitaly-")
+		if err != nil {
+			return fmt.Errorf("creating temporary runtime directory: %w", err)
+		}
+
+		cfg.RuntimeDir = runtimeDir
+	} else {
+		// Otherwise, we use the configured runtime directory. Note that we don't use the
+		// runtime directory directly, but instead create a subdirectory within it which is
+		// based on the process's PID. While we could use `MkdirTemp()` instead and don't
+		// bother with preexisting directories, the benefit of using the PID here is that we
+		// can determine whether the directory may still be in use by checking whether the
+		// PID exists. Furthermore, it allows easier debugging in case one wants to inspect
+		// the runtime directory of a running Gitaly node.
+
+		runtimeDir := filepath.Join(cfg.RuntimeDir, fmt.Sprintf("gitaly-%d", os.Getpid()))
+
+		if _, err := os.Stat(runtimeDir); err != nil && !os.IsNotExist(err) {
+			return fmt.Errorf("statting runtime directory: %w", err)
+		} else if err != nil {
+			// If the directory exists already then it must be from an old invocation of
+			// Gitaly. Because we use the PID as path component we know that the old
+			// instance cannot exist anymore though, so it's safe to remove this
+			// directory now.
+			if err := os.RemoveAll(runtimeDir); err != nil {
+				return fmt.Errorf("removing old runtime directory: %w", err)
+			}
+		}
+
+		if err := os.Mkdir(runtimeDir, 0o700); err != nil {
+			return fmt.Errorf("creating runtime directory: %w", err)
+		}
+
+		cfg.RuntimeDir = runtimeDir
+	}
+
 	if cfg.InternalSocketDir == "" {
 		// The socket path must be short-ish because listen(2) fails on long
 		// socket paths. We hope/expect that os.MkdirTemp creates a directory
@@ -350,6 +392,20 @@ func (cfg *Cfg) validateBinDir() error {
 	return err
 }
 
+func (cfg *Cfg) validateRuntimeDir() error {
+	if len(cfg.RuntimeDir) == 0 {
+		return fmt.Errorf("runtime_dir: is not set")
+	}
+
+	if err := validateIsDirectory(cfg.RuntimeDir, "runtime_dir"); err != nil {
+		return err
+	}
+
+	var err error
+	cfg.RuntimeDir, err = filepath.Abs(cfg.RuntimeDir)
+	return err
+}
+
 // validateGitConfigKey does a best-effort check whether or not a given git config key is valid. It
 // does not allow for assignments in keys, which is overly strict and does not allow some valid
 // keys. It does avoid misinterpretation of keys though and should catch many cases of
diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go
index 3546deae9..82c345c71 100644
--- a/internal/gitaly/config/config_test.go
+++ b/internal/gitaly/config/config_test.go
@@ -34,6 +34,12 @@ func TestLoadEmptyConfig(t *testing.T) {
 		InternalSocketDir: cfg.InternalSocketDir,
 	}
 	require.NoError(t, expectedCfg.setDefaults())
+
+	// The runtime directory is a temporary path, so we need to take the value from the loaded
+	// config. Furthermore, because `setDefaults()` would append the PID, we can't do so before
+	// calling that function.
+	expectedCfg.RuntimeDir = cfg.RuntimeDir
+
 	require.Equal(t, expectedCfg, cfg)
 }
 
@@ -1045,3 +1051,71 @@ func TestValidateBinDir(t *testing.T) {
 		})
 	}
 }
+
+func TestCfg_RuntimeDir(t *testing.T) {
+	t.Run("defaults", func(t *testing.T) {
+		t.Run("empty runtime directory", func(t *testing.T) {
+			cfg := Cfg{}
+			require.NoError(t, cfg.setDefaults())
+
+			require.Equal(t, os.TempDir(), filepath.Dir(cfg.RuntimeDir))
+			require.True(t, strings.HasPrefix(filepath.Base(cfg.RuntimeDir), "gitaly-"))
+			require.DirExists(t, cfg.RuntimeDir)
+		})
+
+		t.Run("non-existent runtime directory", func(t *testing.T) {
+			cfg := Cfg{
+				RuntimeDir: "/does/not/exist",
+			}
+
+			require.EqualError(t, cfg.setDefaults(), fmt.Sprintf("creating runtime directory: mkdir /does/not/exist/gitaly-%d: no such file or directory", os.Getpid()))
+		})
+
+		t.Run("existent runtime directory", func(t *testing.T) {
+			dir := testhelper.TempDir(t)
+			cfg := Cfg{
+				RuntimeDir: dir,
+			}
+			require.NoError(t, cfg.setDefaults())
+			require.Equal(t, filepath.Join(dir, fmt.Sprintf("gitaly-%d", os.Getpid())), cfg.RuntimeDir)
+			require.DirExists(t, cfg.RuntimeDir)
+		})
+	})
+
+	t.Run("validation", func(t *testing.T) {
+		dirPath := testhelper.TempDir(t)
+		filePath := filepath.Join(dirPath, "file")
+		require.NoError(t, os.WriteFile(filePath, nil, 0o644))
+
+		for _, tc := range []struct {
+			desc        string
+			runtimeDir  string
+			expectedErr error
+		}{
+			{
+				desc:       "valid runtime directory",
+				runtimeDir: dirPath,
+			},
+			{
+				desc:        "unset",
+				runtimeDir:  "",
+				expectedErr: fmt.Errorf("runtime_dir: is not set"),
+			},
+			{
+				desc:        "path doesn't exist",
+				runtimeDir:  "/does/not/exist",
+				expectedErr: fmt.Errorf("runtime_dir: path doesn't exist: %q", "/does/not/exist"),
+			},
+			{
+				desc:        "path is not a directory",
+				runtimeDir:  filePath,
+				expectedErr: fmt.Errorf(`runtime_dir: not a directory: %q`, filePath),
+			},
+		} {
+			t.Run(tc.desc, func(t *testing.T) {
+				err := (&Cfg{RuntimeDir: tc.runtimeDir}).validateRuntimeDir()
+				require.Equal(t, tc.expectedErr, err)
+			})
+		}
+	})
+}
diff --git a/internal/testhelper/testcfg/gitaly_builder.go b/internal/testhelper/testcfg/gitaly_builder.go
index 3f670ebcb..f89f0edc6 100644
--- a/internal/testhelper/testcfg/gitaly_builder.go
+++ b/internal/testhelper/testcfg/gitaly_builder.go
@@ -106,6 +106,11 @@ func (gc *GitalyCfgBuilder) Build(t testing.TB) config.Cfg {
 		require.NoError(t, os.Mkdir(cfg.GitlabShell.Dir, 0o755))
 	}
 
+	if cfg.RuntimeDir == "" {
+		cfg.RuntimeDir = filepath.Join(root, "runtime.d")
+		require.NoError(t, os.Mkdir(cfg.RuntimeDir, 0o700))
+	}
+
 	if cfg.InternalSocketDir == "" {
 		cfg.InternalSocketDir = filepath.Join(root, "internal_socks.d")
 		require.NoError(t, os.Mkdir(cfg.InternalSocketDir, 0o755))