Merge branch 'centralise_dir_perms' into 'master'

Centralise directory permissions within the gitaly project

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5334

Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>

6 files changed, 19 insertions, 13 deletions

diff --git a/internal/testhelper/configure.go b/internal/testhelper/configure.go
index ad5df7ad9..76a751082 100644
--- a/internal/testhelper/configure.go
+++ b/internal/testhelper/configure.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	log "github.com/sirupsen/logrus"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
 	gitalylog "gitlab.com/gitlab-org/gitaly/v15/internal/log"
 )
 
@@ -185,7 +186,7 @@ func configureTestDirectory() (_ func(), returnedErr error) {
 	// around after our tests. To avoid this, we thus set the TMPDIR environment variable to
 	// point into a directory inside of out test directory.
 	globalTempDir := filepath.Join(testDirectory, "tmp")
-	if err := os.Mkdir(globalTempDir, 0o755); err != nil {
+	if err := os.Mkdir(globalTempDir, perm.SharedDir); err != nil {
 		return nil, fmt.Errorf("creating global temporary directory: %w", err)
 	}
 	if err := os.Setenv("TMPDIR", globalTempDir); err != nil {
diff --git a/internal/testhelper/logger.go b/internal/testhelper/logger.go
index 68dfc3b00..c654f959d 100644
--- a/internal/testhelper/logger.go
+++ b/internal/testhelper/logger.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
 )
 
 // NewDiscardingLogger creates a logger that discards everything.
@@ -60,7 +61,7 @@ func CreateTestLogDir(tb testing.TB) string {
 
 	logDir := filepath.Join(testLogDir, tb.Name())
 
-	require.NoError(tb, os.MkdirAll(logDir, 0o755))
+	require.NoError(tb, os.MkdirAll(logDir, perm.SharedDir))
 
 	return logDir
 }
diff --git a/internal/testhelper/testcfg/binaries.go b/internal/testhelper/testcfg/binaries.go
index 0eb03031d..fc4ee9529 100644
--- a/internal/testhelper/testcfg/binaries.go
+++ b/internal/testhelper/testcfg/binaries.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 )
 
@@ -123,7 +124,7 @@ func BuildBinary(tb testing.TB, targetDir, sourcePath string) string {
 	require.FileExists(tb, sharedBinaryPath, "%s does not exist", executableName)
 	require.NoFileExists(tb, targetPath, "%s exists already -- do you try to build it twice?", executableName)
 
-	require.NoError(tb, os.MkdirAll(targetDir, os.ModePerm))
+	require.NoError(tb, os.MkdirAll(targetDir, perm.PublicDir))
 
 	// We hard-link the file into place instead of copying it because copying used to cause
 	// ETXTBSY errors in CI. This is likely caused by a bug in the overlay filesystem used by
diff --git a/internal/testhelper/testcfg/gitaly.go b/internal/testhelper/testcfg/gitaly.go
index faef5e2b6..e3bbf3823 100644
--- a/internal/testhelper/testcfg/gitaly.go
+++ b/internal/testhelper/testcfg/gitaly.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 )
 
@@ -77,7 +78,7 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg {
 
 	if cfg.BinDir == "" {
 		cfg.BinDir = filepath.Join(root, "bin.d")
-		require.NoError(tb, os.Mkdir(cfg.BinDir, 0o755))
+		require.NoError(tb, os.Mkdir(cfg.BinDir, perm.SharedDir))
 	}
 
 	if cfg.Ruby.Dir == "" {
@@ -92,19 +93,19 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg {
 			cfg.Logging.Dir = logDir
 		} else {
 			cfg.Logging.Dir = filepath.Join(root, "log.d")
-			require.NoError(tb, os.Mkdir(cfg.Logging.Dir, 0o755))
+			require.NoError(tb, os.Mkdir(cfg.Logging.Dir, perm.SharedDir))
 		}
 	}
 
 	if cfg.GitlabShell.Dir == "" {
 		cfg.GitlabShell.Dir = filepath.Join(root, "shell.d")
-		require.NoError(tb, os.Mkdir(cfg.GitlabShell.Dir, 0o755))
+		require.NoError(tb, os.Mkdir(cfg.GitlabShell.Dir, perm.SharedDir))
 	}
 
 	if cfg.RuntimeDir == "" {
 		cfg.RuntimeDir = filepath.Join(root, "runtime.d")
-		require.NoError(tb, os.Mkdir(cfg.RuntimeDir, 0o700))
-		require.NoError(tb, os.Mkdir(cfg.InternalSocketDir(), 0o755))
+		require.NoError(tb, os.Mkdir(cfg.RuntimeDir, perm.PrivateDir))
+		require.NoError(tb, os.Mkdir(cfg.InternalSocketDir(), perm.SharedDir))
 	}
 
 	if len(cfg.Storages) != 0 && len(gc.storages) != 0 {
@@ -113,7 +114,7 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg {
 
 	if len(cfg.Storages) == 0 {
 		storagesDir := filepath.Join(root, "storages.d")
-		require.NoError(tb, os.Mkdir(storagesDir, 0o755))
+		require.NoError(tb, os.Mkdir(storagesDir, perm.SharedDir))
 
 		if len(gc.storages) == 0 {
 			gc.storages = []string{"default"}
@@ -123,7 +124,7 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg {
 		cfg.Storages = make([]config.Storage, len(gc.storages))
 		for i, storageName := range gc.storages {
 			storagePath := filepath.Join(storagesDir, storageName)
-			require.NoError(tb, os.MkdirAll(storagePath, 0o755))
+			require.NoError(tb, os.MkdirAll(storagePath, perm.SharedDir))
 			cfg.Storages[i].Name = storageName
 			cfg.Storages[i].Path = storagePath
 		}
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 3ff1ae7bc..50dd122a1 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -26,6 +26,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 )
 
@@ -212,7 +213,7 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 func CreateGlobalDirectory(tb testing.TB, name string) string {
 	require.NotEmpty(tb, testDirectory, "global temporary directory does not exist")
 	path := filepath.Join(testDirectory, name)
-	require.NoError(tb, os.Mkdir(path, 0o777))
+	require.NoError(tb, os.Mkdir(path, perm.PublicDir))
 	return path
 }
 
@@ -240,7 +241,7 @@ type Cleanup func()
 // executable.
 func WriteExecutable(tb testing.TB, path string, content []byte) string {
 	dir := filepath.Dir(path)
-	require.NoError(tb, os.MkdirAll(dir, 0o755))
+	require.NoError(tb, os.MkdirAll(dir, perm.SharedDir))
 	tb.Cleanup(func() {
 		assert.NoError(tb, os.RemoveAll(dir))
 	})
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index d2a377710..12da30497 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -30,6 +30,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitlab"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler"
 	praefectconfig "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/streamcache"
@@ -180,7 +181,7 @@ func runGitaly(tb testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, reg
 		registrar(internalServer, deps)
 		registerHealthServerIfNotRegistered(internalServer)
 
-		require.NoError(tb, os.MkdirAll(cfg.InternalSocketDir(), 0o700))
+		require.NoError(tb, os.MkdirAll(cfg.InternalSocketDir(), perm.PrivateDir))
 		tb.Cleanup(func() { require.NoError(tb, os.RemoveAll(cfg.InternalSocketDir())) })
 
 		internalListener, err := net.Listen("unix", cfg.InternalSocketPath())