diff --git a/pkg/api/controller.go b/pkg/api/controller.go index fac53f83152..39986649b31 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -1620,7 +1620,7 @@ func handleAPIError(w http.ResponseWriter, err error) bool { errors.Is(err, permissions.ErrInvalidAction), errors.Is(err, model.ErrValidationError), errors.Is(err, graveler.ErrInvalidRef), - errors.Is(err, catalog.ErrInvalidValue): + errors.Is(err, graveler.ErrInvalidValue): writeError(w, http.StatusBadRequest, err) case errors.Is(err, graveler.ErrNotUnique): diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go index 3349f2344e5..e0c9abe96b2 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -29,6 +29,7 @@ import ( "github.com/treeverse/lakefs/pkg/logging" "github.com/treeverse/lakefs/pkg/pyramid" "github.com/treeverse/lakefs/pkg/pyramid/params" + "github.com/treeverse/lakefs/pkg/validator" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -212,9 +213,9 @@ func (c *Catalog) CreateRepository(ctx context.Context, repository string, stora repositoryID := graveler.RepositoryID(repository) storageNS := graveler.StorageNamespace(storageNamespace) branchID := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"storageNamespace", storageNS, ValidateStorageNamespace}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "name", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "storageNamespace", Value: storageNS, Fn: graveler.ValidateStorageNamespace}, }); err != nil { return nil, err } @@ -236,9 +237,9 @@ func (c *Catalog) CreateBareRepository(ctx context.Context, repository string, s repositoryID := graveler.RepositoryID(repository) storageNS := graveler.StorageNamespace(storageNamespace) branchID := graveler.BranchID(defaultBranchID) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"storageNamespace", storageNS, ValidateStorageNamespace}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "name", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "storageNamespace", Value: storageNS, Fn: graveler.ValidateStorageNamespace}, }); err != nil { return nil, err } @@ -258,8 +259,8 @@ func (c *Catalog) CreateBareRepository(ctx context.Context, repository string, s // GetRepository get repository information func (c *Catalog) GetRepository(ctx context.Context, repository string) (*Repository, error) { repositoryID := graveler.RepositoryID(repository) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, }); err != nil { return nil, err } @@ -279,8 +280,8 @@ func (c *Catalog) GetRepository(ctx context.Context, repository string) (*Reposi // DeleteRepository delete a repository func (c *Catalog) DeleteRepository(ctx context.Context, repository string) error { repositoryID := graveler.RepositoryID(repository) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, }); err != nil { return err } @@ -348,9 +349,9 @@ func (c *Catalog) ListRepositories(ctx context.Context, limit int, prefix, after func (c *Catalog) GetStagingToken(ctx context.Context, repository string, branch string) (*string, error) { repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, }); err != nil { return nil, err } @@ -369,10 +370,10 @@ func (c *Catalog) CreateBranch(ctx context.Context, repository string, branch st repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) sourceRef := graveler.Ref(sourceBranch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, - {"ref", sourceRef, ValidateRef}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, + {Name: "ref", Value: sourceRef, Fn: graveler.ValidateRef}, }); err != nil { return nil, err } @@ -399,9 +400,9 @@ func (c *Catalog) CreateBranch(ctx context.Context, repository string, branch st func (c *Catalog) DeleteBranch(ctx context.Context, repository string, branch string) error { repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "name", Value: branchID, Fn: graveler.ValidateBranchID}, }); err != nil { return err } @@ -412,8 +413,8 @@ func (c *Catalog) ListBranches(ctx context.Context, repository string, prefix st repositoryID := graveler.RepositoryID(repository) afterBranch := graveler.BranchID(after) prefixBranch := graveler.BranchID(prefix) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, }); err != nil { return nil, false, err } @@ -466,9 +467,9 @@ func (c *Catalog) ListBranches(ctx context.Context, repository string, prefix st func (c *Catalog) BranchExists(ctx context.Context, repository string, branch string) (bool, error) { repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "name", Value: branchID, Fn: graveler.ValidateBranchID}, }); err != nil { return false, err } @@ -485,9 +486,9 @@ func (c *Catalog) BranchExists(ctx context.Context, repository string, branch st func (c *Catalog) GetBranchReference(ctx context.Context, repository string, branch string) (string, error) { repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, }); err != nil { return "", err } @@ -501,9 +502,9 @@ func (c *Catalog) GetBranchReference(ctx context.Context, repository string, bra func (c *Catalog) ResetBranch(ctx context.Context, repository string, branch string) error { repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, }); err != nil { return err } @@ -513,9 +514,9 @@ func (c *Catalog) ResetBranch(ctx context.Context, repository string, branch str func (c *Catalog) CreateTag(ctx context.Context, repository string, tagID string, ref string) (string, error) { repositoryID := graveler.RepositoryID(repository) tag := graveler.TagID(tagID) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"tagID", tag, ValidateTagID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "tagID", Value: tag, Fn: graveler.ValidateTagID}, }); err != nil { return "", err } @@ -533,9 +534,9 @@ func (c *Catalog) CreateTag(ctx context.Context, repository string, tagID string func (c *Catalog) DeleteTag(ctx context.Context, repository string, tagID string) error { repositoryID := graveler.RepositoryID(repository) tag := graveler.TagID(tagID) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"tagID", tag, ValidateTagID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "name", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "tagID", Value: tag, Fn: graveler.ValidateTagID}, }); err != nil { return err } @@ -547,8 +548,8 @@ func (c *Catalog) ListTags(ctx context.Context, repository string, prefix string limit = ListTagsLimitMax } repositoryID := graveler.RepositoryID(repository) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "name", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, }); err != nil { return nil, false, err } @@ -597,9 +598,9 @@ func (c *Catalog) ListTags(ctx context.Context, repository string, prefix string func (c *Catalog) GetTag(ctx context.Context, repository string, tagID string) (string, error) { repositoryID := graveler.RepositoryID(repository) tag := graveler.TagID(tagID) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"tagID", tag, ValidateTagID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "name", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "tagID", Value: tag, Fn: graveler.ValidateTagID}, }); err != nil { return "", err } @@ -615,10 +616,10 @@ func (c *Catalog) GetTag(ctx context.Context, repository string, tagID string) ( func (c *Catalog) GetEntry(ctx context.Context, repository string, reference string, path string, _ GetEntryParams) (*DBEntry, error) { repositoryID := graveler.RepositoryID(repository) refToGet := graveler.Ref(reference) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"ref", refToGet, ValidateRef}, - {"path", Path(path), ValidatePath}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "ref", Value: refToGet, Fn: graveler.ValidateRef}, + {Name: "path", Value: Path(path), Fn: ValidatePath}, }); err != nil { return nil, err } @@ -678,10 +679,10 @@ func (c *Catalog) CreateEntry(ctx context.Context, repository string, branch str branchID := graveler.BranchID(branch) ent := newEntryFromCatalogEntry(entry) path := Path(entry.Path) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, - {"path", path, ValidatePath}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, + {Name: "path", Value: path, Fn: ValidatePath}, }); err != nil { return err } @@ -697,10 +698,10 @@ func (c *Catalog) DeleteEntry(ctx context.Context, repository string, branch str repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) p := Path(path) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, - {"path", p, ValidatePath}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, + {Name: "path", Value: p, Fn: ValidatePath}, }); err != nil { return err } @@ -718,11 +719,11 @@ func (c *Catalog) ListEntries(ctx context.Context, repository string, reference delimiterPath := Path(delimiter) repositoryID := graveler.RepositoryID(repository) refToList := graveler.Ref(reference) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"ref", refToList, ValidateRef}, - {"prefix", prefixPath, ValidatePathOptional}, - {"delimiter", delimiterPath, ValidatePathOptional}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "ref", Value: refToList, Fn: graveler.ValidateRef}, + {Name: "prefix", Value: prefixPath, Fn: ValidatePathOptional}, + {Name: "delimiter", Value: delimiterPath, Fn: ValidatePathOptional}, }); err != nil { return nil, false, err } @@ -762,10 +763,10 @@ func (c *Catalog) ResetEntry(ctx context.Context, repository string, branch stri repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) entryPath := Path(path) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, - {"path", entryPath, ValidatePath}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, + {Name: "path", Value: entryPath, Fn: ValidatePath}, }); err != nil { return err } @@ -777,9 +778,9 @@ func (c *Catalog) ResetEntries(ctx context.Context, repository string, branch st repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) prefixPath := Path(prefix) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, }); err != nil { return err } @@ -790,9 +791,9 @@ func (c *Catalog) ResetEntries(ctx context.Context, repository string, branch st func (c *Catalog) Commit(ctx context.Context, repository, branch, message, committer string, metadata Metadata, date *int64) (*CommitLog, error) { repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, }); err != nil { return nil, err } @@ -825,8 +826,8 @@ func (c *Catalog) Commit(ctx context.Context, repository, branch, message, commi func (c *Catalog) GetCommit(ctx context.Context, repository string, reference string) (*CommitLog, error) { repositoryID := graveler.RepositoryID(repository) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, }); err != nil { return nil, err } @@ -855,9 +856,9 @@ func (c *Catalog) GetCommit(ctx context.Context, repository string, reference st func (c *Catalog) ListCommits(ctx context.Context, repository string, branch string, params LogParams) ([]*CommitLog, bool, error) { repositoryID := graveler.RepositoryID(repository) branchRef := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branch", branchRef, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchRef, Fn: graveler.ValidateBranchID}, }); err != nil { return nil, false, err } @@ -976,13 +977,13 @@ func (c *Catalog) Revert(ctx context.Context, repository string, branch string, Message: fmt.Sprintf("Revert %s", params.Reference), } parentNumber := params.ParentNumber - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, - {"ref", reference, ValidateRef}, - {"committer", commitParams.Committer, ValidateRequiredString}, - {"message", commitParams.Message, ValidateRequiredString}, - {"parentNumber", parentNumber, ValidateNonNegativeInt}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, + {Name: "ref", Value: reference, Fn: graveler.ValidateRef}, + {Name: "committer", Value: commitParams.Committer, Fn: validator.ValidateRequiredString}, + {Name: "message", Value: commitParams.Message, Fn: validator.ValidateRequiredString}, + {Name: "parentNumber", Value: parentNumber, Fn: validator.ValidateNonNegativeInt}, }); err != nil { return err } @@ -994,10 +995,10 @@ func (c *Catalog) Diff(ctx context.Context, repository string, leftReference str repositoryID := graveler.RepositoryID(repository) left := graveler.Ref(leftReference) right := graveler.Ref(rightReference) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"left", left, ValidateRef}, - {"right", right, ValidateRef}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "left", Value: left, Fn: graveler.ValidateRef}, + {Name: "right", Value: right, Fn: graveler.ValidateRef}, }); err != nil { return nil, false, err } @@ -1014,10 +1015,10 @@ func (c *Catalog) Compare(ctx context.Context, repository, leftReference string, repositoryID := graveler.RepositoryID(repository) left := graveler.Ref(leftReference) right := graveler.Ref(rightReference) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"left", left, ValidateRef}, - {"right", right, ValidateRef}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repositoryName", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "left", Value: left, Fn: graveler.ValidateRef}, + {Name: "right", Value: right, Fn: graveler.ValidateRef}, }); err != nil { return nil, false, err } @@ -1033,9 +1034,9 @@ func (c *Catalog) Compare(ctx context.Context, repository, leftReference string, func (c *Catalog) DiffUncommitted(ctx context.Context, repository, branch, prefix, delimiter string, limit int, after string) (Differences, bool, error) { repositoryID := graveler.RepositoryID(repository) branchID := graveler.BranchID(branch) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"branchID", branchID, ValidateBranchID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "branch", Value: branchID, Fn: graveler.ValidateBranchID}, }); err != nil { return nil, false, err } @@ -1150,12 +1151,12 @@ func (c *Catalog) Merge(ctx context.Context, repository string, destinationBranc if commitParams.Message == "" { commitParams.Message = fmt.Sprintf("Merge '%s' into '%s'", source, destination) } - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, - {"destination", destination, ValidateBranchID}, - {"source", source, ValidateRef}, - {"committer", commitParams.Committer, ValidateRequiredString}, - {"message", commitParams.Message, ValidateRequiredString}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, + {Name: "destination", Value: destination, Fn: graveler.ValidateBranchID}, + {Name: "source", Value: source, Fn: graveler.ValidateRef}, + {Name: "committer", Value: commitParams.Committer, Fn: validator.ValidateRequiredString}, + {Name: "message", Value: commitParams.Message, Fn: validator.ValidateRequiredString}, }); err != nil { return nil, err } @@ -1251,8 +1252,8 @@ func (c *Catalog) CreateBranchProtectionRule(ctx context.Context, repositoryID s func (c *Catalog) PrepareExpiredCommits(ctx context.Context, repository string, previousRunID string) (*graveler.GarbageCollectionRunMetadata, error) { repositoryID := graveler.RepositoryID(repository) - if err := Validate([]ValidateArg{ - {"repositoryID", repositoryID, ValidateRepositoryID}, + if err := validator.Validate([]validator.ValidateArg{ + {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, }); err != nil { return nil, err } diff --git a/pkg/catalog/errors.go b/pkg/catalog/errors.go index 9e7ead5f228..d4b6afe68b5 100644 --- a/pkg/catalog/errors.go +++ b/pkg/catalog/errors.go @@ -8,14 +8,17 @@ import ( ) var ( - ErrNotFound = db.ErrNotFound ErrInvalid = errors.New("validation error") + ErrInvalidType = fmt.Errorf("invalid type: %w", ErrInvalid) + ErrRequiredValue = fmt.Errorf("required value: %w", ErrInvalid) + ErrPathRequiredValue = fmt.Errorf("missing path: %w", ErrRequiredValue) + ErrInvalidValue = fmt.Errorf("invalid value: %w", ErrInvalid) + ErrNotFound = db.ErrNotFound ErrInvalidMetadataSrcFormat = errors.New("invalid metadata src format") ErrExpired = errors.New("expired from storage") ErrFeatureNotSupported = errors.New("feature not supported") ErrBranchNotFound = fmt.Errorf("branch %w", ErrNotFound) ErrRepositoryNotFound = fmt.Errorf("repository %w", ErrNotFound) - ErrInvalidValue = fmt.Errorf("invalid value: %w", ErrInvalid) ErrNoDifferenceWasFound = errors.New("no difference was found") ErrConflictFound = errors.New("conflict found") ErrInvalidRef = errors.New("invalid ref") diff --git a/pkg/catalog/validate.go b/pkg/catalog/validate.go index a26e4eb431f..6fbf9f0277a 100644 --- a/pkg/catalog/validate.go +++ b/pkg/catalog/validate.go @@ -2,175 +2,21 @@ package catalog import ( "fmt" - "regexp" - "strings" - "github.com/treeverse/lakefs/pkg/graveler" - "github.com/treeverse/lakefs/pkg/ident" + "github.com/treeverse/lakefs/pkg/validator" ) const ( MaxPathLength = 1024 ) -var ( - reValidRef = regexp.MustCompile(`^[^\s]+$`) - reValidBranchID = regexp.MustCompile(`^\w[-\w]*$`) - reValidRepositoryID = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{2,62}$`) -) - -var ( - ErrInvalidType = fmt.Errorf("invalid type: %w", ErrInvalid) - ErrRequiredValue = fmt.Errorf("required value: %w", ErrInvalid) - ErrPathRequiredValue = fmt.Errorf("missing path: %w", ErrRequiredValue) -) - -type ValidateFunc func(v interface{}) error - -type ValidateArg struct { - Name string - Value interface{} - Fn ValidateFunc -} - -func Validate(args []ValidateArg) error { - for _, arg := range args { - err := arg.Fn(arg.Value) - if err != nil { - return fmt.Errorf("argument %s: %w", arg.Name, err) - } - } - return nil -} - -func MakeValidateOptional(fn ValidateFunc) ValidateFunc { - return func(v interface{}) error { - switch s := v.(type) { - case string: - if len(s) == 0 { - return nil - } - case fmt.Stringer: - if len(s.String()) == 0 { - return nil - } - case nil: - return nil - } - return fn(v) - } -} - -func ValidateStorageNamespace(v interface{}) error { - s, ok := v.(graveler.StorageNamespace) - if !ok { - panic(ErrInvalidType) - } - if len(s) == 0 { - return ErrRequiredValue - } - return nil -} - -func ValidateRef(v interface{}) error { - s, ok := v.(graveler.Ref) - if !ok { - panic(ErrInvalidType) - } - if len(s) == 0 { - return ErrRequiredValue - } - if !reValidRef.MatchString(s.String()) { - return ErrInvalidValue - } - return nil -} - -func ValidateBranchID(v interface{}) error { - s, ok := v.(graveler.BranchID) - if !ok { - panic(ErrInvalidType) - } - if len(s) == 0 { - return ErrRequiredValue - } - branchName := s.String() - if !reValidBranchID.MatchString(branchName) { - return ErrInvalidValue - } - return nil -} - -func ValidateTagID(v interface{}) error { - s, ok := v.(graveler.TagID) - if !ok { - panic(ErrInvalidType) - } - // https://git-scm.com/docs/git-check-ref-format - tag := string(s) - if len(tag) == 0 { - return ErrRequiredValue - } - if tag == "@" { - return ErrInvalidValue - } - if strings.HasSuffix(tag, ".") || strings.HasSuffix(tag, ".lock") || strings.HasSuffix(tag, "/") { - return ErrInvalidValue - } - if strings.Contains(tag, "..") || strings.Contains(tag, "//") || strings.Contains(tag, "@{") { - return ErrInvalidValue - } - // Unlike git, we do allow '~'. That supports migration from our previous ref format where commits started with a tilde. - if strings.ContainsAny(tag, "^:?*[\\") { - return ErrInvalidValue - } - for _, r := range tag { - if isControlCodeOrSpace(r) { - return ErrInvalidValue - } - } - return nil -} - -func isControlCodeOrSpace(r rune) bool { - const space = 0x20 - return r <= space -} - -func ValidateCommitID(v interface{}) error { - s, ok := v.(graveler.CommitID) - if !ok { - panic(ErrInvalidType) - } - if len(s) == 0 { - return ErrRequiredValue - } - if !ident.IsContentAddress(s.String()) { - return ErrInvalidValue - } - return nil -} - -func ValidateRepositoryID(v interface{}) error { - s, ok := v.(graveler.RepositoryID) - if !ok { - panic(ErrInvalidType) - } - if len(s) == 0 { - return ErrRequiredValue - } - if !reValidRepositoryID.MatchString(s.String()) { - return ErrInvalidValue - } - return nil -} - func ValidatePath(v interface{}) error { s, ok := v.(Path) if !ok { panic(ErrInvalidType) } - l := len(s.String()) + + l := len(s) if l == 0 { return ErrPathRequiredValue } @@ -180,27 +26,4 @@ func ValidatePath(v interface{}) error { return nil } -func ValidateRequiredString(v interface{}) error { - s, ok := v.(string) - if !ok { - panic(ErrInvalidType) - } - if len(s) == 0 { - return ErrRequiredValue - } - return nil -} - -func ValidateNonNegativeInt(v interface{}) error { - i, ok := v.(int) - if !ok { - panic(ErrInvalidType) - } - if i < 0 { - return ErrInvalidValue - } - return nil -} - -var ValidatePathOptional = MakeValidateOptional(ValidatePath) -var ValidateTagIDOptional = MakeValidateOptional(ValidateTagID) +var ValidatePathOptional = validator.MakeValidateOptional(ValidatePath) diff --git a/pkg/catalog/validate_test.go b/pkg/catalog/validate_test.go deleted file mode 100644 index 19d7e6ec7d6..00000000000 --- a/pkg/catalog/validate_test.go +++ /dev/null @@ -1,51 +0,0 @@ -package catalog - -import ( - "errors" - "testing" - - "github.com/treeverse/lakefs/pkg/graveler" -) - -func TestValidateTagID(t *testing.T) { - tests := []struct { - name string - tag graveler.TagID - wantErr error - }{ - {name: "empty", tag: "", wantErr: ErrRequiredValue}, - {name: "tilda", tag: "~tag", wantErr: nil}, - {name: "valid1", tag: "tag", wantErr: nil}, - {name: "valid2", tag: "v1.0", wantErr: nil}, - {name: "ends with dot", tag: "tag.", wantErr: ErrInvalidValue}, - {name: "ends with dot", tag: "tag.", wantErr: ErrInvalidValue}, - {name: "ends with lock", tag: "tag.lock", wantErr: ErrInvalidValue}, - {name: "space", tag: "a tag", wantErr: ErrInvalidValue}, - {name: "invalid control", tag: "a\x01tag", wantErr: ErrInvalidValue}, - {name: "double slash", tag: "more//tags", wantErr: ErrInvalidValue}, - {name: "double dot", tag: "more..tags", wantErr: ErrInvalidValue}, - {name: "template", tag: "more@{tags}", wantErr: ErrInvalidValue}, - {name: "invalid value", tag: "@", wantErr: ErrInvalidValue}, - {name: "question mark", tag: "tag?", wantErr: ErrInvalidValue}, - {name: "column", tag: "tag:tag", wantErr: ErrInvalidValue}, - {name: "back slash", tag: "tag\\tag", wantErr: ErrInvalidValue}, - {name: "open square brackets", tag: "ta[g]", wantErr: ErrInvalidValue}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := ValidateTagID(tt.tag) - if !errors.Is(err, tt.wantErr) { - t.Errorf("ValidateTagID() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} - -func TestValidateTagID_Type(t *testing.T) { - defer func() { - if r := recover(); r == nil { - t.Errorf("ValidateTagID should panic on invalid type") - } - }() - _ = ValidateTagID("tag") -} diff --git a/pkg/graveler/errors.go b/pkg/graveler/errors.go index 6891283b00e..ada9ba08255 100644 --- a/pkg/graveler/errors.go +++ b/pkg/graveler/errors.go @@ -18,11 +18,17 @@ var ( ErrPreconditionFailed = errors.New("precondition failed") ErrWriteToProtectedBranch = wrapError(ErrUserVisible, "cannot write to protected branch") ErrCommitToProtectedBranch = wrapError(ErrUserVisible, "cannot commit to protected branch") - ErrInvalidValue = errors.New("invalid value") + ErrInvalidValue = fmt.Errorf("invalid value: %w", ErrInvalid) ErrInvalidMergeBase = fmt.Errorf("only 2 commits allowed in FindMergeBase: %w", ErrInvalidValue) ErrNoMergeBase = errors.New("no merge base") ErrInvalidRef = fmt.Errorf("ref: %w", ErrInvalidValue) ErrInvalidCommitID = fmt.Errorf("commit id: %w", ErrInvalidValue) + ErrInvalidBranchID = fmt.Errorf("branch id: %w", ErrInvalidValue) + ErrInvalidTagID = fmt.Errorf("tag id: %w", ErrInvalidValue) + ErrInvalid = errors.New("validation error") + ErrInvalidType = fmt.Errorf("invalid type: %w", ErrInvalid) + ErrInvalidRepositoryID = fmt.Errorf("repository id: %w", ErrInvalidValue) + ErrRequiredValue = fmt.Errorf("required value: %w", ErrInvalid) ErrCommitNotFound = fmt.Errorf("commit %w", ErrNotFound) ErrCreateBranchNoCommit = fmt.Errorf("can't create a branch without commit") ErrRepositoryNotFound = fmt.Errorf("repository %w", ErrNotFound) diff --git a/pkg/graveler/validate.go b/pkg/graveler/validate.go new file mode 100644 index 00000000000..e2fe8bf5933 --- /dev/null +++ b/pkg/graveler/validate.go @@ -0,0 +1,118 @@ +package graveler + +import ( + "strings" + + "github.com/treeverse/lakefs/pkg/ident" + "github.com/treeverse/lakefs/pkg/validator" +) + +func ValidateStorageNamespace(v interface{}) error { + s, ok := v.(StorageNamespace) + if !ok { + panic(ErrInvalidType) + } + + if len(s) == 0 { + return ErrRequiredValue + } + return nil +} + +func ValidateRef(v interface{}) error { + s, ok := v.(Ref) + if !ok { + panic(ErrInvalidType) + } + if len(s) == 0 { + return ErrRequiredValue + } + if !validator.ReValidRef.MatchString(s.String()) { + return ErrInvalidRef + } + return nil +} + +func ValidateBranchID(v interface{}) error { + s, ok := v.(BranchID) + if !ok { + panic(ErrInvalidType) + } + if len(s) == 0 { + return ErrRequiredValue + } + if !validator.ReValidBranchID.MatchString(s.String()) { + return ErrInvalidBranchID + } + return nil +} + +func ValidateTagID(v interface{}) error { + s, ok := v.(TagID) + if !ok { + panic(ErrInvalidType) + } + + // https://git-scm.com/docs/git-check-ref-format + if len(s) == 0 { + return ErrRequiredValue + } + + tagID := s.String() + if tagID == "@" { + return ErrInvalidTagID + } + if strings.HasSuffix(tagID, ".") || strings.HasSuffix(tagID, ".lock") || strings.HasSuffix(tagID, "/") { + return ErrInvalidTagID + } + if strings.Contains(tagID, "..") || strings.Contains(tagID, "//") || strings.Contains(tagID, "@{") { + return ErrInvalidTagID + } + // Unlike git, we do allow '~'. That supports migration from our previous ref format where commits started with a tilde. + if strings.ContainsAny(tagID, "^:?*[\\") { + return ErrInvalidTagID + } + for _, r := range tagID { + if isControlCodeOrSpace(r) { + return ErrInvalidTagID + } + } + return nil +} + +func isControlCodeOrSpace(r rune) bool { + const space = 0x20 + return r <= space +} + +func ValidateCommitID(v interface{}) error { + s, ok := v.(CommitID) + if !ok { + panic(ErrInvalidType) + } + + if len(s) == 0 { + return ErrRequiredValue + } + if !ident.IsContentAddress(s.String()) { + return ErrInvalidCommitID + } + return nil +} + +func ValidateRepositoryID(v interface{}) error { + s, ok := v.(RepositoryID) + if !ok { + panic(ErrInvalidType) + } + + if len(s) == 0 { + return ErrRequiredValue + } + if !validator.ReValidRepositoryID.MatchString(s.String()) { + return ErrInvalidRepositoryID + } + return nil +} + +var ValidateTagIDOptional = validator.MakeValidateOptional(ValidateTagID) diff --git a/pkg/graveler/validate_test.go b/pkg/graveler/validate_test.go new file mode 100644 index 00000000000..dcc37aaaed0 --- /dev/null +++ b/pkg/graveler/validate_test.go @@ -0,0 +1,85 @@ +package graveler + +import ( + "errors" + "testing" +) + +func TestValidateTagID(t *testing.T) { + tests := []struct { + name string + tag TagID + wantErr error + }{ + {name: "empty", tag: "", wantErr: ErrRequiredValue}, + {name: "tilda", tag: "~tag", wantErr: nil}, + {name: "valid1", tag: "tag", wantErr: nil}, + {name: "valid2", tag: "v1.0", wantErr: nil}, + {name: "ends with dot", tag: "tag.", wantErr: ErrInvalidValue}, + {name: "ends with dot", tag: "tag.", wantErr: ErrInvalidValue}, + {name: "ends with lock", tag: "tag.lock", wantErr: ErrInvalidValue}, + {name: "space", tag: "a tag", wantErr: ErrInvalidValue}, + {name: "invalid control", tag: "a\x01tag", wantErr: ErrInvalidValue}, + {name: "double slash", tag: "more//tags", wantErr: ErrInvalidValue}, + {name: "double dot", tag: "more..tags", wantErr: ErrInvalidValue}, + {name: "template", tag: "more@{tags}", wantErr: ErrInvalidValue}, + {name: "invalid value", tag: "@", wantErr: ErrInvalidValue}, + {name: "question mark", tag: "tag?", wantErr: ErrInvalidValue}, + {name: "column", tag: "tag:tag", wantErr: ErrInvalidValue}, + {name: "back slash", tag: "tag\\tag", wantErr: ErrInvalidValue}, + {name: "open square brackets", tag: "ta[g]", wantErr: ErrInvalidValue}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTagID(tt.tag) + if !errors.Is(err, tt.wantErr) { + t.Errorf("ValidateTagID() error = %v, wantErr %v (%v)", err, tt.wantErr, tt.name) + } + }) + } +} + +func TestValidateTagID_Type(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Errorf("ValidateTagID should panic on invalid type") + } + }() + _ = ValidateTagID("tag") +} + +func TestValidateBranchID(t *testing.T) { + tests := []struct { + name string + branchID BranchID + wantErr error + }{ + {name: "alpha dash", branchID: "valid-branch", wantErr: nil}, + {name: "alpha underscore", branchID: "valid_branch", wantErr: nil}, + {name: "alpha numeric", branchID: "valid123", wantErr: nil}, + {name: "capital alpha numeric", branchID: "Valid123", wantErr: nil}, + {name: "alpha underscore numeric", branchID: "valid_123", wantErr: nil}, + {name: "alpha dash numeric", branchID: "valid-123", wantErr: nil}, + {name: "alpha numeric", branchID: "123valid", wantErr: nil}, + {name: "char1", branchID: "invalid~char", wantErr: ErrInvalidValue}, + {name: "alpha dot numeric", branchID: "invalid1.0", wantErr: ErrInvalidValue}, + {name: "alpha two dots", branchID: "invalid..branch", wantErr: ErrInvalidValue}, + {name: "ends with dot", branchID: "invalid.", wantErr: ErrInvalidValue}, + {name: "alpha slash", branchID: "invalid/branch", wantErr: ErrInvalidValue}, + {name: "alpha double slash", branchID: "invalid//branch", wantErr: ErrInvalidValue}, + {name: "alpha question mark", branchID: "invalid?branch", wantErr: ErrInvalidValue}, + {name: "alpha at", branchID: "invalid@branch", wantErr: ErrInvalidValue}, + {name: "alpha column", branchID: "invalid:branch", wantErr: ErrInvalidValue}, + {name: "alpha backslash", branchID: "invalid\\branch", wantErr: ErrInvalidValue}, + {name: "alpha square brackets", branchID: "invalid[branch]", wantErr: ErrInvalidValue}, + {name: "alpha curly brackets", branchID: "invalid{branch}", wantErr: ErrInvalidValue}, + } + for _, tb := range tests { + t.Run(tb.name, func(t *testing.T) { + err := ValidateBranchID(tb.branchID) + if !errors.Is(err, tb.wantErr) { + t.Errorf("ValidateBranchID() error = %v, wantErr %v", err, tb.wantErr) + } + }) + } +} diff --git a/pkg/uri/parser.go b/pkg/uri/parser.go index d4a322efb05..51e89700d8b 100644 --- a/pkg/uri/parser.go +++ b/pkg/uri/parser.go @@ -5,6 +5,8 @@ import ( "fmt" "net/url" "strings" + + "github.com/treeverse/lakefs/pkg/validator" ) const ( @@ -32,15 +34,15 @@ type URI struct { } func (u *URI) IsRepository() bool { - return len(u.Repository) > 0 && len(u.Ref) == 0 && u.Path == nil + return len(u.Repository) > 0 && len(u.Ref) == 0 && u.Path == nil && validator.ReValidRepositoryID.MatchString(u.Repository) } func (u *URI) IsRef() bool { - return len(u.Repository) > 0 && len(u.Ref) > 0 && u.Path == nil + return len(u.Repository) > 0 && len(u.Ref) > 0 && u.Path == nil && validator.ReValidRepositoryID.MatchString(u.Repository) && validator.ReValidBranchID.MatchString(u.Ref) } func (u *URI) IsFullyQualified() bool { - return len(u.Repository) > 0 && len(u.Ref) > 0 && u.Path != nil + return len(u.Repository) > 0 && len(u.Ref) > 0 && u.Path != nil && validator.ReValidRepositoryID.MatchString(u.Repository) && validator.ReValidBranchID.MatchString(u.Ref) } func (u *URI) GetPath() string { diff --git a/pkg/validator/validate.go b/pkg/validator/validate.go new file mode 100644 index 00000000000..a4582983f60 --- /dev/null +++ b/pkg/validator/validate.go @@ -0,0 +1,79 @@ +package validator + +import ( + "errors" + "fmt" + "regexp" +) + +var ( + ReValidRef = regexp.MustCompile(`^[^\s]+$`) + ReValidBranchID = regexp.MustCompile(`^\w[-\w]*$`) + ReValidRepositoryID = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{2,62}$`) +) + +var ( + ErrInvalid = errors.New("validation error") + ErrInvalidType = fmt.Errorf("invalid type: %w", ErrInvalid) + ErrRequiredValue = fmt.Errorf("required value: %w", ErrInvalid) + ErrInvalidValue = fmt.Errorf("invalid value: %w", ErrInvalid) + ErrPathRequiredValue = fmt.Errorf("missing path: %w", ErrRequiredValue) +) + +type ValidateFunc func(v interface{}) error + +type ValidateArg struct { + Name string + Value interface{} + Fn ValidateFunc +} + +func Validate(args []ValidateArg) error { + for _, arg := range args { + err := arg.Fn(arg.Value) + if err != nil { + return fmt.Errorf("argument %s: %w", arg.Name, err) + } + } + return nil +} + +func MakeValidateOptional(fn ValidateFunc) ValidateFunc { + return func(v interface{}) error { + switch s := v.(type) { + case string: + if len(s) == 0 { + return nil + } + case fmt.Stringer: + if len(s.String()) == 0 { + return nil + } + case nil: + return nil + } + return fn(v) + } +} + +func ValidateRequiredString(v interface{}) error { + s, ok := v.(string) + if !ok { + panic(ErrInvalidType) + } + if len(s) == 0 { + return ErrRequiredValue + } + return nil +} + +func ValidateNonNegativeInt(v interface{}) error { + i, ok := v.(int) + if !ok { + panic(ErrInvalidType) + } + if i < 0 { + return ErrInvalidValue + } + return nil +}