Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--internal/errors/cfgerror/testhelper_test.go11
-rw-r--r--internal/errors/cfgerror/validate.go48
-rw-r--r--internal/errors/cfgerror/validate_test.go35
-rw-r--r--internal/gitaly/config/config.go57
-rw-r--r--internal/gitaly/config/config_test.go131
5 files changed, 252 insertions, 30 deletions
diff --git a/internal/errors/cfgerror/testhelper_test.go b/internal/errors/cfgerror/testhelper_test.go
new file mode 100644
index 000000000..be6675514
--- /dev/null
+++ b/internal/errors/cfgerror/testhelper_test.go
@@ -0,0 +1,11 @@
+package cfgerror
+
+import (
+ "testing"
+
+ "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
+)
+
+func TestMain(m *testing.M) {
+ testhelper.Run(m)
+}
diff --git a/internal/errors/cfgerror/validate.go b/internal/errors/cfgerror/validate.go
index 54a31123b..935caf905 100644
--- a/internal/errors/cfgerror/validate.go
+++ b/internal/errors/cfgerror/validate.go
@@ -3,11 +3,22 @@ package cfgerror
import (
"errors"
"fmt"
+ "os"
"strings"
)
-// ErrNotSet should be used when the value is not set, but it is required.
-var ErrNotSet = errors.New("not set")
+var (
+ // ErrNotSet should be used when the value is not set, but it is required.
+ ErrNotSet = errors.New("not set")
+ // ErrBlankOrEmpty should be used when non-blank/non-empty string is expected.
+ ErrBlankOrEmpty = errors.New("blank or empty")
+ // ErrDoesntExist should be used when resource doesn't exist.
+ ErrDoesntExist = errors.New("doesn't exist")
+ // ErrNotDir should be used when path on the file system exists, but it is not a directory.
+ ErrNotDir = errors.New("not a dir")
+ // ErrNotUnique should be used when the value must be unique, but there are duplicates.
+ ErrNotUnique = errors.New("not unique")
+)
// ValidationError represents an issue with provided configuration.
type ValidationError struct {
@@ -96,3 +107,36 @@ func (vs ValidationErrors) Error() string {
func New() ValidationErrors {
return nil
}
+
+// NotEmpty checks if value is empty.
+func NotEmpty(val string) error {
+ if val == "" {
+ return NewValidationError(ErrNotSet)
+ }
+ return nil
+}
+
+// NotBlank checks the value is not empty or blank.
+func NotBlank(val string) error {
+ if strings.TrimSpace(val) == "" {
+ return NewValidationError(ErrBlankOrEmpty)
+ }
+ return nil
+}
+
+// DirExists checks the value points to an existing directory on the disk.
+func DirExists(path string) error {
+ fs, err := os.Stat(path)
+ if err != nil {
+ if errors.Is(err, os.ErrNotExist) {
+ return NewValidationError(fmt.Errorf("%w: %q", ErrDoesntExist, path))
+ }
+ return err
+ }
+
+ if !fs.IsDir() {
+ return NewValidationError(fmt.Errorf("%w: %q", ErrNotDir, path))
+ }
+
+ return nil
+}
diff --git a/internal/errors/cfgerror/validate_test.go b/internal/errors/cfgerror/validate_test.go
index 847a62cc6..c129a17d7 100644
--- a/internal/errors/cfgerror/validate_test.go
+++ b/internal/errors/cfgerror/validate_test.go
@@ -2,10 +2,15 @@ package cfgerror
import (
"errors"
+ "fmt"
+ "os"
+ "path/filepath"
"testing"
"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/testhelper"
)
func TestValidationError_Error(t *testing.T) {
@@ -99,3 +104,33 @@ func TestNewValidationError(t *testing.T) {
err = NewValidationError(assert.AnError, "outer", "inner")
require.Equal(t, ValidationError{Cause: assert.AnError, Key: []string{"outer", "inner"}}, err)
}
+
+func TestNotEmpty(t *testing.T) {
+ t.Parallel()
+ require.NoError(t, NotEmpty("value"))
+ require.Equal(t, NewValidationError(ErrNotSet), NotEmpty(""))
+}
+
+func TestNotBlank(t *testing.T) {
+ t.Parallel()
+ require.NoError(t, NotBlank("value"))
+ require.Equal(t, NewValidationError(ErrBlankOrEmpty), NotBlank(""))
+ require.Equal(t, NewValidationError(ErrBlankOrEmpty), NotBlank(" \t \n "))
+}
+
+func TestDirExists(t *testing.T) {
+ t.Parallel()
+
+ filePath := filepath.Join(testhelper.TempDir(t), "tmp-file")
+ require.NoError(t, os.WriteFile(filePath, []byte{}, perm.PublicFile))
+ existing := testhelper.TempDir(t)
+ notExisting := filepath.Join(existing, "bad")
+
+ require.NoError(t, DirExists(existing))
+
+ expectedNotExisting := NewValidationError(fmt.Errorf("%w: %q", ErrDoesntExist, notExisting))
+ require.Equal(t, expectedNotExisting, DirExists(notExisting))
+
+ expectedNotDir := NewValidationError(fmt.Errorf("%w: %q", ErrNotDir, filePath))
+ require.Equal(t, expectedNotDir, DirExists(filePath))
+}
diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index 7cdaf4cc6..6a39483ac 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -194,8 +194,61 @@ func (cfg GitConfig) GlobalArgs() ([]string, error) {
// Storage contains a single storage-shard
type Storage struct {
- Name string
- Path string
+ Name string `toml:"name"`
+ Path string `toml:"path"`
+}
+
+// Validate runs validation on all fields and compose all found errors.
+func (s Storage) Validate() error {
+ return cfgerror.New().
+ Append(cfgerror.NotEmpty(s.Name), "name").
+ Append(cfgerror.DirExists(s.Path), "path").
+ AsError()
+}
+
+func validateStorages(storages []Storage) error {
+ if len(storages) == 0 {
+ return cfgerror.NewValidationError(cfgerror.ErrNotSet)
+ }
+
+ var errs cfgerror.ValidationErrors
+ for i, s := range storages {
+ errs = errs.Append(s.Validate(), fmt.Sprintf("[%d]", i))
+ }
+
+ for i, storage := range storages {
+ for _, other := range storages[:i] {
+ if other.Name == storage.Name {
+ err := fmt.Errorf("%w: %q", cfgerror.ErrNotUnique, storage.Name)
+ cause := cfgerror.NewValidationError(err, "name")
+ errs = errs.Append(cause, fmt.Sprintf("[%d]", i))
+ }
+
+ if storage.Path == other.Path {
+ // This is weird, but we allow it for legacy gitlab.com reasons.
+ continue
+ }
+
+ if storage.Path == "" || other.Path == "" {
+ // If one of Path-s is not set the code below will produce an error
+ // that only confuses, so we stop here.
+ continue
+ }
+
+ if strings.HasPrefix(storage.Path, other.Path) || strings.HasPrefix(other.Path, storage.Path) {
+ // If storages have the same subdirectory, that is allowed.
+ if filepath.Dir(storage.Path) == filepath.Dir(other.Path) {
+ continue
+ }
+
+ cause := fmt.Errorf("can't nest: %q and %q", storage.Path, other.Path)
+ err := cfgerror.NewValidationError(cause, "path")
+ errs = errs.Append(err, fmt.Sprintf("[%d]", i))
+ }
+ }
+ }
+
+ return errs.AsError()
}
// Sentry is a sentry.Config. We redefine this type to a different name so
diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go
index f36f3f300..d09ea0b36 100644
--- a/internal/gitaly/config/config_test.go
+++ b/internal/gitaly/config/config_test.go
@@ -471,16 +471,16 @@ func TestValidateStorages(t *testing.T) {
filePath := filepath.Join(testhelper.TempDir(t), "temporary-file")
require.NoError(t, os.WriteFile(filePath, []byte{}, perm.PublicFile))
- invalidDir := filepath.Join(repositories, t.Name())
+ invalidDir := filepath.Join(filepath.Dir(repositories), t.Name())
testCases := []struct {
- desc string
- storages []Storage
- expErrMsg string
+ desc string
+ storages []Storage
+ expectedErr error
}{
{
- desc: "no storages",
- expErrMsg: "no storage configurations found. Are you using the right format? https://gitlab.com/gitlab-org/gitaly/issues/397",
+ desc: "no storages",
+ expectedErr: cfgerror.NewValidationError(cfgerror.ErrNotSet),
},
{
desc: "just 1 storage",
@@ -510,7 +510,13 @@ func TestValidateStorages(t *testing.T) {
{Name: "other", Path: repositories},
{Name: "third", Path: nestedRepositories},
},
- expErrMsg: `storage paths may not nest: "third" and "default"`,
+ expectedErr: cfgerror.ValidationErrors{{
+ Key: []string{"[2]", "path"},
+ Cause: fmt.Errorf(`can't nest: %q and %q`, nestedRepositories, repositories),
+ }, {
+ Key: []string{"[2]", "path"},
+ Cause: fmt.Errorf(`can't nest: %q and %q`, nestedRepositories, repositories),
+ }},
},
{
desc: "nested paths 2",
@@ -519,7 +525,13 @@ func TestValidateStorages(t *testing.T) {
{Name: "other", Path: repositories},
{Name: "third", Path: repositories},
},
- expErrMsg: `storage paths may not nest: "other" and "default"`,
+ expectedErr: cfgerror.ValidationErrors{{
+ Key: []string{"[1]", "path"},
+ Cause: fmt.Errorf(`can't nest: %q and %q`, repositories, nestedRepositories),
+ }, {
+ Key: []string{"[2]", "path"},
+ Cause: fmt.Errorf(`can't nest: %q and %q`, repositories, nestedRepositories),
+ }},
},
{
desc: "duplicate definition",
@@ -527,7 +539,10 @@ func TestValidateStorages(t *testing.T) {
{Name: "default", Path: repositories},
{Name: "default", Path: repositories},
},
- expErrMsg: `storage "default" is defined more than once`,
+ expectedErr: cfgerror.ValidationErrors{cfgerror.NewValidationError(
+ fmt.Errorf(`%w: "default"`, cfgerror.ErrNotUnique),
+ "[1]", "name",
+ )},
},
{
desc: "re-definition",
@@ -535,7 +550,12 @@ func TestValidateStorages(t *testing.T) {
{Name: "default", Path: repositories},
{Name: "default", Path: repositories2},
},
- expErrMsg: `storage "default" is defined more than once`,
+ expectedErr: cfgerror.ValidationErrors{
+ cfgerror.NewValidationError(
+ fmt.Errorf(`%w: "default"`, cfgerror.ErrNotUnique),
+ "[1]", "name",
+ ),
+ },
},
{
desc: "empty name",
@@ -543,15 +563,12 @@ func TestValidateStorages(t *testing.T) {
{Name: "some", Path: repositories},
{Name: "", Path: repositories},
},
- expErrMsg: `empty storage name at declaration 2`,
- },
- {
- desc: "empty path",
- storages: []Storage{
- {Name: "some", Path: repositories},
- {Name: "default", Path: ""},
+ expectedErr: cfgerror.ValidationErrors{
+ cfgerror.NewValidationError(
+ cfgerror.ErrNotSet,
+ "[1]", "name",
+ ),
},
- expErrMsg: `empty storage path for storage "default"`,
},
{
desc: "non existing directory",
@@ -559,7 +576,12 @@ func TestValidateStorages(t *testing.T) {
{Name: "default", Path: repositories},
{Name: "nope", Path: invalidDir},
},
- expErrMsg: fmt.Sprintf(`storage path %q for storage "nope" doesn't exist`, invalidDir),
+ expectedErr: cfgerror.ValidationErrors{
+ cfgerror.NewValidationError(
+ fmt.Errorf("%w: %q", cfgerror.ErrDoesntExist, invalidDir),
+ "[1]", "path",
+ ),
+ },
},
{
desc: "path points to the regular file",
@@ -567,21 +589,41 @@ func TestValidateStorages(t *testing.T) {
{Name: "default", Path: repositories},
{Name: "is_file", Path: filePath},
},
- expErrMsg: fmt.Sprintf(`storage path %q for storage "is_file" is not a dir`, filePath),
+ expectedErr: cfgerror.ValidationErrors{
+ cfgerror.NewValidationError(
+ fmt.Errorf("%w: %q", cfgerror.ErrNotDir, filePath),
+ "[1]", "path",
+ ),
+ },
+ },
+ {
+ desc: "multiple errors",
+ storages: []Storage{
+ {Name: "", Path: repositories},
+ {Name: "default", Path: "somewhat"},
+ },
+ expectedErr: cfgerror.ValidationErrors{
+ cfgerror.NewValidationError(
+ cfgerror.ErrNotSet,
+ "[0]", "name",
+ ),
+ cfgerror.NewValidationError(
+ fmt.Errorf("%w: %q", cfgerror.ErrDoesntExist, "somewhat"),
+ "[1]", "path",
+ ),
+ },
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
- cfg := Cfg{Storages: tc.storages}
-
- err := cfg.validateStorages()
- if tc.expErrMsg != "" {
- assert.EqualError(t, err, tc.expErrMsg, "%+v", tc.storages)
+ err := validateStorages(tc.storages)
+ if tc.expectedErr != nil {
+ assert.Equalf(t, tc.expectedErr, err, "%+v", tc.storages)
return
}
- assert.NoError(t, err, "%+v", tc.storages)
+ assert.NoErrorf(t, err, "%+v", tc.storages)
})
}
}
@@ -1696,3 +1738,40 @@ func TestPackObjectsLimiting(t *testing.T) {
})
}
}
+
+func TestStorage_Validate(t *testing.T) {
+ t.Parallel()
+
+ dirPath := testhelper.TempDir(t)
+ filePath := filepath.Join(dirPath, "file")
+ require.NoError(t, os.WriteFile(filePath, nil, perm.SharedFile))
+ for _, tc := range []struct {
+ name string
+ storage Storage
+ expectedErr error
+ }{
+ {
+ name: "valid",
+ storage: Storage{Name: "name", Path: dirPath},
+ },
+ {
+ name: "invalid",
+ storage: Storage{Name: "", Path: filePath},
+ expectedErr: cfgerror.ValidationErrors{
+ cfgerror.NewValidationError(
+ cfgerror.ErrNotSet,
+ "name",
+ ),
+ cfgerror.NewValidationError(
+ fmt.Errorf("%w: %q", cfgerror.ErrNotDir, filePath),
+ "path",
+ ),
+ },
+ },
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ err := tc.storage.Validate()
+ require.Equal(t, tc.expectedErr, err)
+ })
+ }
+}
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-14 22:58:44 +0300