protect storages config field with mutexjc-protect-storages

9 files changed, 32 insertions, 18 deletions

diff --git a/internal/cache/walker_test.go b/internal/cache/walker_test.go
index b3d3044d6..ea4b2e644 100644
--- a/internal/cache/walker_test.go
+++ b/internal/cache/walker_test.go
@@ -100,17 +100,17 @@ func setupDiskCacheWalker(t testing.TB) func() {
 	require.NoError(t, err)
 
 	oldStorages := config.Config.Storages
-	config.Config.Storages = []config.Storage{
+	config.ModifyStorages([]config.Storage{
 		{
 			Name: t.Name(),
 			Path: tmpPath,
 		},
-	}
+	})
 
 	satisfyConfigValidation(tmpPath)
 
 	cleanup := func() {
-		config.Config.Storages = oldStorages
+		config.ModifyStorages(oldStorages)
 		require.NoError(t, os.RemoveAll(tmpPath))
 	}
 
diff --git a/internal/config/config.go b/internal/config/config.go
index fb711adda..03d302649 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -7,6 +7,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/BurntSushi/toml"
@@ -257,6 +258,9 @@ func SetGitPath() error {
 // StoragePath looks up the base path for storageName. The second boolean
 // return value indicates if anything was found.
 func StoragePath(storageName string) (string, bool) {
+	storageMutex.RLock()
+	defer storageMutex.RUnlock()
+
 	for _, storage := range Config.Storages {
 		if storage.Name == storageName {
 			return storage.Path, true
@@ -276,3 +280,13 @@ func validateBinDir() error {
 	Config.BinDir, err = filepath.Abs(Config.BinDir)
 	return err
 }
+
+var storageMutex sync.RWMutex
+
+// ModifyStorages is used in tests to modify the storages in the config in a thread safe way
+func ModifyStorages(storages []Storage) {
+	storageMutex.Lock()
+	defer storageMutex.Unlock()
+
+	Config.Storages = storages
+}
diff --git a/internal/helper/repo_test.go b/internal/helper/repo_test.go
index c6e9c921b..4caf34051 100644
--- a/internal/helper/repo_test.go
+++ b/internal/helper/repo_test.go
@@ -14,7 +14,7 @@ import (
 
 func TestGetRepoPath(t *testing.T) {
 	defer func(oldStorages []config.Storage) {
-		config.Config.Storages = oldStorages
+		config.ModifyStorages(oldStorages)
 	}(config.Config.Storages)
 
 	testRepo := testhelper.TestRepository()
@@ -120,7 +120,7 @@ func TestGetRepoPath(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			config.Config.Storages = tc.storages
+			config.ModifyStorages(tc.storages)
 			path, err := GetRepoPath(tc.repo)
 
 			if tc.err != codes.OK {
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index b669e6875..27f344f82 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -148,10 +148,10 @@ func TestReplicate(t *testing.T) {
 
 	oldStorages := gitaly_config.Config.Storages
 	defer func() {
-		gitaly_config.Config.Storages = oldStorages
+		gitaly_config.ModifyStorages(oldStorages)
 	}()
 
-	gitaly_config.Config.Storages = append(gitaly_config.Config.Storages, gitaly_config.Storage{
+	gitaly_config.ModifyStorages(append(gitaly_config.Config.Storages, gitaly_config.Storage{
 		Name: backupStorageName,
 		Path: backupDir,
 	},
@@ -159,7 +159,7 @@ func TestReplicate(t *testing.T) {
 			Name: "default",
 			Path: testhelper.GitlabTestStoragePath(),
 		},
-	)
+	))
 
 	ctx, cancel := testhelper.Context()
 	defer cancel()
diff --git a/internal/service/namespace/namespace_test.go b/internal/service/namespace/namespace_test.go
index f21dc0174..968fbb27f 100644
--- a/internal/service/namespace/namespace_test.go
+++ b/internal/service/namespace/namespace_test.go
@@ -23,11 +23,11 @@ func testMain(m *testing.M) int {
 	defer os.Remove(storageOtherDir)
 
 	oldStorages := config.Config.Storages
-	config.Config.Storages = []config.Storage{
+	config.ModifyStorages([]config.Storage{
 		{Name: "default", Path: testhelper.GitlabTestStoragePath()},
 		{Name: "other", Path: storageOtherDir},
-	}
-	defer func() { config.Config.Storages = oldStorages }()
+	})
+	defer func() { config.ModifyStorages(oldStorages) }()
 
 	return m.Run()
 }
diff --git a/internal/service/repository/repository_test.go b/internal/service/repository/repository_test.go
index ea6e91763..fa7bfa219 100644
--- a/internal/service/repository/repository_test.go
+++ b/internal/service/repository/repository_test.go
@@ -34,9 +34,9 @@ func TestRepositoryExists(t *testing.T) {
 	}
 
 	defer func(oldStorages []config.Storage) {
-		config.Config.Storages = oldStorages
+		config.ModifyStorages(oldStorages)
 	}(config.Config.Storages)
-	config.Config.Storages = testStorages
+	config.ModifyStorages(testStorages)
 
 	queries := []struct {
 		desc      string
diff --git a/internal/service/server/info_test.go b/internal/service/server/info_test.go
index d1b287285..37ba0de13 100644
--- a/internal/service/server/info_test.go
+++ b/internal/service/server/info_test.go
@@ -34,9 +34,9 @@ func TestGitalyServerInfo(t *testing.T) {
 		{Name: "broken", Path: "/does/not/exist"},
 	}
 	defer func(oldStorages []config.Storage) {
-		config.Config.Storages = oldStorages
+		config.ModifyStorages(oldStorages)
 	}(config.Config.Storages)
-	config.Config.Storages = testStorages
+	config.ModifyStorages(testStorages)
 
 	tempDir, err := ioutil.TempDir("", "gitaly-bin")
 	require.NoError(t, err)
diff --git a/internal/service/storage/listdirectories_test.go b/internal/service/storage/listdirectories_test.go
index 115d17444..0e03143ad 100644
--- a/internal/service/storage/listdirectories_test.go
+++ b/internal/service/storage/listdirectories_test.go
@@ -26,9 +26,9 @@ func TestListDirectories(t *testing.T) {
 	testStorages := []config.Storage{{Name: "default", Path: testDir}}
 
 	defer func(oldStorages []config.Storage) {
-		config.Config.Storages = oldStorages
+		config.ModifyStorages(oldStorages)
 	}(config.Config.Storages)
-	config.Config.Storages = testStorages
+	config.ModifyStorages(testStorages)
 
 	repoPaths := []string{"foo", "bar", "bar/baz", "bar/baz/foo/buz"}
 	for _, p := range repoPaths {
diff --git a/internal/service/storage/testhelper_test.go b/internal/service/storage/testhelper_test.go
index 63da71a65..335d41882 100644
--- a/internal/service/storage/testhelper_test.go
+++ b/internal/service/storage/testhelper_test.go
@@ -36,7 +36,7 @@ func configureTestStorage() {
 
 	testStorage = config.Storage{Name: "storage-will-be-deleted", Path: storagePath}
 
-	config.Config.Storages = []config.Storage{testStorage}
+	config.ModifyStorages([]config.Storage{testStorage})
 }
 
 func runStorageServer(t *testing.T) (*grpc.Server, string) {