Gitaly: Refactor configurePackObjectsCache()

The 'configurePackObjectsCache()' method now returns
'ValidationErrors'.

2 files changed, 53 insertions, 24 deletions

diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index 369cc6e23..f36e484f7 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -722,20 +722,18 @@ func (cfg *Cfg) validateCgroups() error {
 	return nil
 }
 
-var (
-	errPackObjectsCacheNegativeMaxAge = errors.New("pack_objects_cache.max_age cannot be negative")
-	errPackObjectsCacheNoStorages     = errors.New("pack_objects_cache: cannot pick default cache directory: no storages")
-	errPackObjectsCacheRelativePath   = errors.New("pack_objects_cache: storage directory must be absolute path")
-)
-
 func (cfg *Cfg) configurePackObjectsCache() error {
 	poc := &cfg.PackObjectsCache
 	if !poc.Enabled {
 		return nil
 	}
 
+	var errs ValidationErrors
 	if poc.MaxAge < 0 {
-		return errPackObjectsCacheNegativeMaxAge
+		errs = append(errs, ValidationError{
+			Key:     []string{"pack_objects_cache", "max_age"},
+			Message: "cannot be negative",
+		})
 	}
 
 	if poc.MaxAge == 0 {
@@ -744,17 +742,23 @@ func (cfg *Cfg) configurePackObjectsCache() error {
 
 	if poc.Dir == "" {
 		if len(cfg.Storages) == 0 {
-			return errPackObjectsCacheNoStorages
+			errs = append(errs, ValidationError{
+				Key:     []string{"pack_objects_cache", "dir"},
+				Message: "cannot pick default cache directory: no storages",
+			})
+		} else {
+			poc.Dir = filepath.Join(cfg.Storages[0].Path, GitalyDataPrefix, "PackObjectsCache")
 		}
-
-		poc.Dir = filepath.Join(cfg.Storages[0].Path, GitalyDataPrefix, "PackObjectsCache")
 	}
 
-	if !filepath.IsAbs(poc.Dir) {
-		return errPackObjectsCacheRelativePath
+	if poc.Dir != "" && !filepath.IsAbs(poc.Dir) {
+		errs = append(errs, ValidationError{
+			Key:     []string{"pack_objects_cache", "dir"},
+			Message: "must be absolute path",
+		})
 	}
 
-	return nil
+	return errs
 }
 
 // SetupRuntimeDirectory creates a new runtime directory. Runtime directory contains internal
diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go
index aa2ef624d..b4197defe 100644
--- a/internal/gitaly/config/config_test.go
+++ b/internal/gitaly/config/config_test.go
@@ -1204,10 +1204,10 @@ path="/foobar"
 `
 
 	testCases := []struct {
-		desc string
-		in   string
-		out  StreamCacheConfig
-		err  error
+		desc        string
+		in          string
+		out         StreamCacheConfig
+		expectedErr error
 	}{
 		{desc: "empty"},
 		{
@@ -1231,15 +1231,22 @@ max_age = "10m"
 			in: `[pack_objects_cache]
 enabled = true
 `,
-			err: errPackObjectsCacheNoStorages,
+			expectedErr: ValidationErrors{{
+				Key:     []string{"pack_objects_cache", "dir"},
+				Message: "cannot pick default cache directory: no storages",
+			}},
 		},
 		{
 			desc: "enabled with negative max age",
 			in: `[pack_objects_cache]
 enabled = true
 max_age = "-5m"
+dir = "/"
 `,
-			err: errPackObjectsCacheNegativeMaxAge,
+			expectedErr: ValidationErrors{{
+				Key:     []string{"pack_objects_cache", "max_age"},
+				Message: "cannot be negative",
+			}},
 		},
 		{
 			desc: "enabled with relative path",
@@ -1247,7 +1254,25 @@ max_age = "-5m"
 enabled = true
 dir = "foobar"
 `,
-			err: errPackObjectsCacheRelativePath,
+			expectedErr: ValidationErrors{{
+				Key:     []string{"pack_objects_cache", "dir"},
+				Message: "must be absolute path",
+			}},
+		},
+		{
+			desc: "multiple validation errors",
+			in: `[pack_objects_cache]
+enabled = true
+max_age = "-5m"
+dir = "foobar"
+`,
+			expectedErr: ValidationErrors{{
+				Key:     []string{"pack_objects_cache", "max_age"},
+				Message: "cannot be negative",
+			}, {
+				Key:     []string{"pack_objects_cache", "dir"},
+				Message: "must be absolute path",
+			}},
 		},
 	}
 
@@ -1256,13 +1281,13 @@ dir = "foobar"
 			cfg, err := Load(strings.NewReader(tc.in))
 			require.NoError(t, err)
 
-			err = cfg.configurePackObjectsCache()
-			if tc.err != nil {
-				require.Equal(t, tc.err, err)
+			errs := cfg.configurePackObjectsCache()
+			if tc.expectedErr != nil {
+				require.Equal(t, tc.expectedErr, errs)
 				return
 			}
 
-			require.NoError(t, err)
+			require.Empty(t, errs)
 			require.Equal(t, tc.out, cfg.PackObjectsCache)
 		})
 	}