mirror of https://github.com/harness/drone.git
Update CODEOWNERS to match github (#2069)
Johannes Batzill
Harness
parent
a6894572a4
commit
d35db42583
|
|
@ -76,6 +76,7 @@ func mapCodeOwnerEvaluation(ownerEvaluation *codeowners.Evaluation) []types.Code
|
|||
}
|
||||
}
|
||||
codeOwnerEvaluationEntries[i] = types.CodeOwnerEvaluationEntry{
|
||||
LineNumber: entry.LineNumber,
|
||||
Pattern: entry.Pattern,
|
||||
OwnerEvaluations: ownerEvaluations,
|
||||
UserGroupOwnerEvaluations: userGroupOwnerEvaluations,
|
||||
|
|
|
|||
|
|
@ -107,7 +107,15 @@ func Translate(ctx context.Context, err error) *Error {
|
|||
case errors.As(err, &codeOwnersTooLargeError):
|
||||
return UnprocessableEntityf(codeOwnersTooLargeError.Error())
|
||||
case errors.As(err, &codeOwnersFileParseError):
|
||||
return UnprocessableEntityf(codeOwnersFileParseError.Error())
|
||||
return NewWithPayload(
|
||||
http.StatusUnprocessableEntity,
|
||||
codeOwnersFileParseError.Error(),
|
||||
map[string]any{
|
||||
"line_number": codeOwnersFileParseError.LineNumber,
|
||||
"line": codeOwnersFileParseError.Line,
|
||||
"err": codeOwnersFileParseError.Err.Error(),
|
||||
},
|
||||
)
|
||||
// lock errors
|
||||
case errors.As(err, &lockError):
|
||||
return errorFromLockError(lockError)
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import (
|
|||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/harness/gitness/app/services/usergroup"
|
||||
|
|
@ -31,6 +32,7 @@ import (
|
|||
|
||||
"github.com/bmatcuk/doublestar/v4"
|
||||
"github.com/rs/zerolog/log"
|
||||
"golang.org/x/exp/slices"
|
||||
)
|
||||
|
||||
const (
|
||||
|
|
@ -44,6 +46,19 @@ const (
|
|||
|
||||
var (
|
||||
ErrNotFound = errors.New("file not found")
|
||||
|
||||
// escapableControlCharactersInPattern are control characters that are used to
|
||||
// control the parsing of the codeowners file.
|
||||
escapableControlCharactersInPattern = []rune{' ', '\t', '#'}
|
||||
// escapableSpecialCharactersInPattern are special characters that are available in the pattern syntax
|
||||
// to allow for more complex pattern matching.
|
||||
escapableSpecialCharactersInPattern = []rune{'*', '?', '[', ']', '{', '}', '-', '!', '^'}
|
||||
ErrFileParseInvalidEscapingInPattern = errors.New(
|
||||
"a pattern requires '\\' to be escaped with another '\\', or used to escape control characters " +
|
||||
"[space, tab, '#'] or any of the available special characters " +
|
||||
"['*', '?', '[', ']', '{', '}', '-', '!', '^']",
|
||||
)
|
||||
ErrFileParseTrailingBackslashInPattern = errors.New("a pattern can't end with a trailing '\\'")
|
||||
)
|
||||
|
||||
// TooLargeError represents an error if codeowners file is too large.
|
||||
|
|
@ -67,15 +82,21 @@ func (e *TooLargeError) Is(target error) bool {
|
|||
|
||||
// FileParseError represents an error if codeowners file is not parsable.
|
||||
type FileParseError struct {
|
||||
line string
|
||||
LineNumber int64
|
||||
Line string
|
||||
Err error
|
||||
}
|
||||
|
||||
func (e *FileParseError) Error() string {
|
||||
return fmt.Sprintf(
|
||||
"The repository's CODEOWNERS file has an invalid line: %s", e.line,
|
||||
"The repository's CODEOWNERS file has an error at line %d: %s", e.LineNumber, e.Err,
|
||||
)
|
||||
}
|
||||
|
||||
func (e *FileParseError) Unwrap() error {
|
||||
return e.Err
|
||||
}
|
||||
|
||||
func (e *FileParseError) Is(target error) bool {
|
||||
_, ok := target.(*FileParseError)
|
||||
return ok
|
||||
|
|
@ -105,8 +126,19 @@ type CodeOwners struct {
|
|||
}
|
||||
|
||||
type Entry struct {
|
||||
// LineNumber is the line number of the code owners entry.
|
||||
LineNumber int64
|
||||
|
||||
// Pattern is a glob star pattern used to match the entry against a given file path.
|
||||
Pattern string
|
||||
Owners []string
|
||||
// Owners is the list of owners for the given pattern.
|
||||
// NOTE: Could be empty in case of an entry that clears previously defined ownerships.
|
||||
Owners []string
|
||||
}
|
||||
|
||||
// IsOwnershipReset returns true iff the entry resets any previously defined ownerships.
|
||||
func (e *Entry) IsOwnershipReset() bool {
|
||||
return len(e.Owners) == 0
|
||||
}
|
||||
|
||||
type Evaluation struct {
|
||||
|
|
@ -115,6 +147,7 @@ type Evaluation struct {
|
|||
}
|
||||
|
||||
type EvaluationEntry struct {
|
||||
LineNumber int64
|
||||
Pattern string
|
||||
OwnerEvaluations []OwnerEvaluation
|
||||
UserGroupOwnerEvaluations []UserGroupOwnerEvaluation
|
||||
|
|
@ -174,29 +207,72 @@ func (s *Service) get(
|
|||
}
|
||||
|
||||
func (s *Service) parseCodeOwner(codeOwnersContent string) ([]Entry, error) {
|
||||
var lineNumber int64
|
||||
var codeOwners []Entry
|
||||
scanner := bufio.NewScanner(strings.NewReader(codeOwnersContent))
|
||||
for scanner.Scan() {
|
||||
line := scanner.Text()
|
||||
line = strings.TrimSpace(line)
|
||||
lineNumber++
|
||||
originalLine := scanner.Text()
|
||||
|
||||
line := strings.TrimSpace(originalLine)
|
||||
if line == "" || strings.HasPrefix(line, "#") {
|
||||
continue
|
||||
}
|
||||
|
||||
parts := strings.Split(line, " ")
|
||||
if len(parts) < 2 {
|
||||
return nil, &FileParseError{line}
|
||||
isSeparator := func(r rune) bool { return r == ' ' || r == '\t' }
|
||||
lineAsRunes := []rune(line)
|
||||
pattern := strings.Builder{}
|
||||
|
||||
// important to iterate over runes and not bytes to support utf-8 encoding.
|
||||
for len(lineAsRunes) > 0 {
|
||||
if isSeparator(lineAsRunes[0]) || lineAsRunes[0] == '#' {
|
||||
break
|
||||
}
|
||||
|
||||
if lineAsRunes[0] == '\\' {
|
||||
// ensure pattern doesn't end with trailing backslash.
|
||||
if len(lineAsRunes) == 1 {
|
||||
return nil, &FileParseError{
|
||||
LineNumber: lineNumber,
|
||||
Line: originalLine,
|
||||
Err: ErrFileParseTrailingBackslashInPattern,
|
||||
}
|
||||
}
|
||||
|
||||
switch {
|
||||
// escape character and special characters need to stay escaped ("\\", "\*", ...)
|
||||
case lineAsRunes[1] == '\\' || slices.Contains(escapableSpecialCharactersInPattern, lineAsRunes[1]):
|
||||
pattern.WriteRune('\\')
|
||||
lineAsRunes = lineAsRunes[1:]
|
||||
|
||||
// control characters aren't special characters in pattern syntax, so escaping should be removed.
|
||||
case slices.Contains(escapableControlCharactersInPattern, lineAsRunes[1]):
|
||||
lineAsRunes = lineAsRunes[1:]
|
||||
|
||||
default:
|
||||
return nil, &FileParseError{
|
||||
LineNumber: lineNumber,
|
||||
Line: originalLine,
|
||||
Err: ErrFileParseInvalidEscapingInPattern,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pattern.WriteRune(lineAsRunes[0])
|
||||
lineAsRunes = lineAsRunes[1:]
|
||||
}
|
||||
|
||||
pattern := parts[0]
|
||||
owners := parts[1:]
|
||||
|
||||
codeOwner := Entry{
|
||||
Pattern: pattern,
|
||||
Owners: owners,
|
||||
// remove inline comment (can't be escaped in owners, only pattern supports escaping)
|
||||
if i := slices.Index(lineAsRunes, '#'); i >= 0 {
|
||||
lineAsRunes = lineAsRunes[:i]
|
||||
}
|
||||
|
||||
codeOwners = append(codeOwners, codeOwner)
|
||||
codeOwners = append(codeOwners, Entry{
|
||||
LineNumber: lineNumber,
|
||||
Pattern: pattern.String(),
|
||||
// could be empty list in case of removing ownership
|
||||
Owners: strings.FieldsFunc(string(lineAsRunes), isSeparator),
|
||||
})
|
||||
}
|
||||
if err := scanner.Err(); err != nil {
|
||||
return nil, fmt.Errorf("error reading input: %w", err)
|
||||
|
|
@ -290,7 +366,6 @@ func (s *Service) getApplicableCodeOwnersForPR(
|
|||
return nil, err
|
||||
}
|
||||
|
||||
var filteredEntries []Entry
|
||||
diffFileStats, err := s.git.DiffFileNames(ctx, &git.DiffParams{
|
||||
ReadParams: git.CreateReadParams(repo),
|
||||
BaseRef: pr.MergeBaseSHA,
|
||||
|
|
@ -300,15 +375,33 @@ func (s *Service) getApplicableCodeOwnersForPR(
|
|||
return nil, fmt.Errorf("failed to get diff file stat: %w", err)
|
||||
}
|
||||
|
||||
for _, entry := range codeOwners.Entries {
|
||||
ok, err := contains(entry.Pattern, diffFileStats.Files)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if ok {
|
||||
filteredEntries = append(filteredEntries, entry)
|
||||
entryIDs := map[int]struct{}{}
|
||||
for _, file := range diffFileStats.Files {
|
||||
// last rule that matches wins (hence simply go in reverse order)
|
||||
for i := len(codeOwners.Entries) - 1; i >= 0; i-- {
|
||||
pattern := codeOwners.Entries[i].Pattern
|
||||
if ok, err := match(pattern, file); err != nil {
|
||||
return nil, fmt.Errorf("failed to match pattern %q for file %q: %w", pattern, file, err)
|
||||
} else if ok {
|
||||
entryIDs[i] = struct{}{}
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
filteredEntries := make([]Entry, 0, len(entryIDs))
|
||||
for i := range entryIDs {
|
||||
if !codeOwners.Entries[i].IsOwnershipReset() {
|
||||
filteredEntries = append(filteredEntries, codeOwners.Entries[i])
|
||||
}
|
||||
}
|
||||
|
||||
// sort output to match order of occurrence in source CODEOWNERS file
|
||||
sort.Slice(
|
||||
filteredEntries,
|
||||
func(i, j int) bool { return filteredEntries[i].LineNumber <= filteredEntries[j].LineNumber },
|
||||
)
|
||||
|
||||
return &CodeOwners{
|
||||
FileSHA: codeOwners.FileSHA,
|
||||
Entries: filteredEntries,
|
||||
|
|
@ -367,6 +460,7 @@ func (s *Service) Evaluate(
|
|||
}
|
||||
if len(ownerEvaluations) != 0 || len(userGroupOwnerEvaluations) != 0 {
|
||||
evaluationEntries = append(evaluationEntries, EvaluationEntry{
|
||||
LineNumber: entry.LineNumber,
|
||||
Pattern: entry.Pattern,
|
||||
OwnerEvaluations: ownerEvaluations,
|
||||
UserGroupOwnerEvaluations: userGroupOwnerEvaluations,
|
||||
|
|
@ -493,24 +587,64 @@ func findReviewerInList(email string, uid string, reviewers []*types.PullReqRevi
|
|||
return nil
|
||||
}
|
||||
|
||||
// We match a pattern list against a target
|
||||
// doubleStar match allows to match / separated path wisely.
|
||||
// A path foo/bar will match against pattern ** or foo/*
|
||||
// Also, for a directory ending with / we have to return true for all files in that directory,
|
||||
// hence we append ** for it.
|
||||
func contains(pattern string, targets []string) (bool, error) {
|
||||
for _, target := range targets {
|
||||
// in case of / ending rule, owner owns the whole directory hence append **
|
||||
if strings.HasSuffix(pattern, "/") {
|
||||
pattern += "**"
|
||||
}
|
||||
match, err := doublestar.PathMatch(pattern, target)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to match pattern due to error: %w", err)
|
||||
}
|
||||
if match {
|
||||
return true, nil
|
||||
}
|
||||
// Match matches a file path against the provided CODEOWNERS pattern.
|
||||
// The code follows the .gitignore syntax closely (similar to github):
|
||||
// https://git-scm.com/docs/gitignore#_pattern_format
|
||||
//
|
||||
// IMPORTANT: It seems that doublestar has a bug, as `*k/**` matches `k` but `k*/**` doesnt (incorrect)'.
|
||||
// Because of that, we currently match patterns like `test*` only partially:
|
||||
// - `test2`, `test/abc`, `test2/abc` are matching
|
||||
// - `test` is not matching
|
||||
// As a workaround, the user will have to add the same rule without a trailing `*` for now.
|
||||
func match(pattern string, path string) (bool, error) {
|
||||
if pattern == "" {
|
||||
return false, fmt.Errorf("empty pattern not allowed")
|
||||
}
|
||||
return false, nil
|
||||
if path == "" {
|
||||
return false, fmt.Errorf("empty path not allowed")
|
||||
}
|
||||
|
||||
// catch easy cases immediately to simplify code
|
||||
if pattern == "/" || pattern == "*" || pattern == "**" {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// cleanup path to simplify matching (always start with "/" and remove trailing "/")
|
||||
if path[0] != '/' {
|
||||
path = "/" + path
|
||||
}
|
||||
if path[len(path)-1] == '/' {
|
||||
path = path[0 : len(path)-1]
|
||||
}
|
||||
|
||||
// if the pattern contains a slash anywhere but at the end, it's treated as an absolute path.
|
||||
// Otherwise, the pattern can match on any level.
|
||||
if !strings.Contains(pattern[:len(pattern)-1], "/") {
|
||||
pattern = "**/" + pattern
|
||||
} else if pattern[0] != '/' {
|
||||
pattern = "/" + pattern
|
||||
}
|
||||
|
||||
// if the pattern ends with "/**", then it matches everything inside.
|
||||
// Since doublestar matches pattern "x/**" with target "x", we replace it with "x/*/**".
|
||||
if strings.HasSuffix(pattern, "/**") {
|
||||
pattern = pattern[:len(pattern)-3] + "/*/**"
|
||||
}
|
||||
|
||||
// If CODEOWNERS matches a file, it also matches a folder with the same name, and anything inside that folder.
|
||||
// Special case is a rule ending with "/", it only matches files inside the folder, not the folder itself.
|
||||
// Since doublestar matches pattern "x/**" with target "x", we extend the pattern with "*/**" in such a case.
|
||||
// Another special case is "/*", where the user explicitly stops nested matching.
|
||||
if pattern[len(pattern)-1] == '/' {
|
||||
pattern += "*/**"
|
||||
} else if !strings.HasSuffix(pattern, "/**") && !strings.HasSuffix(pattern, "/*") {
|
||||
pattern += "/**"
|
||||
}
|
||||
|
||||
match, err := doublestar.PathMatch(pattern, path)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed doublestar path match: %w", err)
|
||||
}
|
||||
|
||||
return match, nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,7 +15,6 @@
|
|||
package codeowners
|
||||
|
||||
import (
|
||||
"context"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
|
|
@ -24,11 +23,6 @@ import (
|
|||
)
|
||||
|
||||
func TestService_ParseCodeOwner(t *testing.T) {
|
||||
content1 := "**/contracts/openapi/v1/ mankrit.singh@harness.io ashish.sanodia@harness.io\n"
|
||||
content2 := "**/contracts/openapi/v1/ mankrit.singh@harness.io ashish.sanodia@harness.io\n" +
|
||||
"/scripts/api mankrit.singh@harness.io ashish.sanodia@harness.io"
|
||||
content3 := "# codeowner file \n**/contracts/openapi/v1/ mankrit.singh@harness.io ashish.sanodia@harness.io\n" +
|
||||
"#\n/scripts/api mankrit.singh@harness.io ashish.sanodia@harness.io"
|
||||
type fields struct {
|
||||
repoStore store.RepoStore
|
||||
git git.Interface
|
||||
|
|
@ -46,36 +40,211 @@ func TestService_ParseCodeOwner(t *testing.T) {
|
|||
}{
|
||||
{
|
||||
name: "Code owners Single",
|
||||
args: args{codeOwnersContent: content1},
|
||||
want: []Entry{{
|
||||
Pattern: "**/contracts/openapi/v1/",
|
||||
Owners: []string{"mankrit.singh@harness.io", "ashish.sanodia@harness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Code owners Multiple",
|
||||
args: args{codeOwnersContent: content2},
|
||||
want: []Entry{{
|
||||
Pattern: "**/contracts/openapi/v1/",
|
||||
Owners: []string{"mankrit.singh@harness.io", "ashish.sanodia@harness.io"},
|
||||
},
|
||||
args: args{`**/contracts/openapi/v1/ user1@harness.io user2@harness.io`},
|
||||
want: []Entry{
|
||||
{
|
||||
Pattern: "/scripts/api",
|
||||
Owners: []string{"mankrit.singh@harness.io", "ashish.sanodia@harness.io"},
|
||||
LineNumber: 1,
|
||||
Pattern: "**/contracts/openapi/v1/",
|
||||
Owners: []string{"user1@harness.io", "user2@harness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Code owners With comments",
|
||||
args: args{codeOwnersContent: content3},
|
||||
want: []Entry{{
|
||||
Pattern: "**/contracts/openapi/v1/",
|
||||
Owners: []string{"mankrit.singh@harness.io", "ashish.sanodia@harness.io"},
|
||||
},
|
||||
name: "Code owners Multiple",
|
||||
args: args{`
|
||||
**/contracts/openapi/v1/ user1@harness.io user2@harness.io
|
||||
/scripts/api user3@harness.io user4@harness.io
|
||||
`},
|
||||
want: []Entry{
|
||||
{
|
||||
Pattern: "/scripts/api",
|
||||
Owners: []string{"mankrit.singh@harness.io", "ashish.sanodia@harness.io"},
|
||||
LineNumber: 2,
|
||||
Pattern: "**/contracts/openapi/v1/",
|
||||
Owners: []string{"user1@harness.io", "user2@harness.io"},
|
||||
},
|
||||
{
|
||||
LineNumber: 3,
|
||||
Pattern: "/scripts/api",
|
||||
Owners: []string{"user3@harness.io", "user4@harness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: " Code owners with full line comments",
|
||||
args: args{`
|
||||
# codeowner file
|
||||
**/contracts/openapi/v1/ user1@harness.io user2@harness.io
|
||||
#
|
||||
/scripts/api user1@harness.io user2@harness.io
|
||||
`},
|
||||
want: []Entry{
|
||||
{
|
||||
LineNumber: 3,
|
||||
Pattern: "**/contracts/openapi/v1/",
|
||||
Owners: []string{"user1@harness.io", "user2@harness.io"},
|
||||
},
|
||||
{
|
||||
LineNumber: 5,
|
||||
Pattern: "/scripts/api",
|
||||
Owners: []string{"user1@harness.io", "user2@harness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: " Code owners with reset",
|
||||
args: args{`
|
||||
* user1@harness.io
|
||||
/scripts/api
|
||||
`},
|
||||
want: []Entry{
|
||||
{
|
||||
LineNumber: 2,
|
||||
Pattern: "*",
|
||||
Owners: []string{"user1@harness.io"},
|
||||
},
|
||||
{
|
||||
LineNumber: 3,
|
||||
Pattern: "/scripts/api",
|
||||
Owners: []string{},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: " Code owners with escaped characters in pattern",
|
||||
args: args{`
|
||||
# escaped escape character
|
||||
\\ user1@harness.io
|
||||
# escaped control characters (are unescaped)
|
||||
\ \ \# user2@harness.io
|
||||
# escaped special pattern syntax characters (stay escaped)
|
||||
\*\?\[\]\{\}\-\!\^ user3@harness.io
|
||||
# mix of escapes
|
||||
\\\ \*\\\\\? user4@harness.io
|
||||
`},
|
||||
want: []Entry{
|
||||
{
|
||||
|
||||
LineNumber: 3,
|
||||
Pattern: "\\\\",
|
||||
Owners: []string{"user1@harness.io"},
|
||||
},
|
||||
{
|
||||
|
||||
LineNumber: 5,
|
||||
Pattern: " #",
|
||||
Owners: []string{"user2@harness.io"},
|
||||
},
|
||||
{
|
||||
LineNumber: 7,
|
||||
Pattern: "\\*\\?\\[\\]\\{\\}\\-\\!\\^",
|
||||
Owners: []string{"user3@harness.io"},
|
||||
},
|
||||
{
|
||||
LineNumber: 9,
|
||||
Pattern: "\\\\ \\*\\\\\\\\\\?",
|
||||
Owners: []string{"user4@harness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: " Code owners with multiple spaces as divider",
|
||||
args: args{`
|
||||
* user1@harness.io user2@harness.io
|
||||
`},
|
||||
want: []Entry{
|
||||
{
|
||||
LineNumber: 2,
|
||||
Pattern: "*",
|
||||
Owners: []string{"user1@harness.io", "user2@harness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: " Code owners with invalid escaping standalone '\\'",
|
||||
args: args{`
|
||||
\
|
||||
`},
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: " Code owners with invalid escaping unsupported char",
|
||||
args: args{`
|
||||
\a
|
||||
`},
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: " Code owners with utf8",
|
||||
args: args{`
|
||||
D∆NCE user@h∆rness.io
|
||||
`},
|
||||
want: []Entry{
|
||||
{
|
||||
LineNumber: 2,
|
||||
Pattern: "D∆NCE",
|
||||
Owners: []string{"user@h∆rness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: " Code owners with tabs and spaces",
|
||||
args: args{`
|
||||
a\ user1@harness.io user2@harness.io user3@harness.io
|
||||
|
||||
`},
|
||||
want: []Entry{
|
||||
{
|
||||
LineNumber: 2,
|
||||
Pattern: "a ",
|
||||
Owners: []string{"user1@harness.io", "user2@harness.io", "user3@harness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: " Code owners with inline comments",
|
||||
args: args{`
|
||||
a #user1@harness.io
|
||||
b # user1@harness.io
|
||||
c #
|
||||
d#
|
||||
e# user1@harness.io
|
||||
f user1@harness.io#user2@harness.io
|
||||
g user1@harness.io # user2@harness.io
|
||||
`},
|
||||
want: []Entry{
|
||||
{
|
||||
LineNumber: 2,
|
||||
Pattern: "a",
|
||||
Owners: []string{},
|
||||
},
|
||||
{
|
||||
LineNumber: 3,
|
||||
Pattern: "b",
|
||||
Owners: []string{},
|
||||
},
|
||||
{
|
||||
LineNumber: 4,
|
||||
Pattern: "c",
|
||||
Owners: []string{},
|
||||
},
|
||||
{
|
||||
LineNumber: 5,
|
||||
Pattern: "d",
|
||||
Owners: []string{},
|
||||
},
|
||||
{
|
||||
LineNumber: 6,
|
||||
Pattern: "e",
|
||||
Owners: []string{},
|
||||
},
|
||||
{
|
||||
LineNumber: 7,
|
||||
Pattern: "f",
|
||||
Owners: []string{"user1@harness.io"},
|
||||
},
|
||||
{
|
||||
LineNumber: 8,
|
||||
Pattern: "g",
|
||||
Owners: []string{"user1@harness.io"},
|
||||
},
|
||||
},
|
||||
},
|
||||
|
|
@ -99,183 +268,406 @@ func TestService_ParseCodeOwner(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func Test_contains(t *testing.T) {
|
||||
target1 := []string{"random"}
|
||||
target2 := []string{"random/xyz"}
|
||||
target3 := []string{"abhinav/path"}
|
||||
target4 := []string{"abc/xyz"}
|
||||
target5 := []string{"abc/xyz", "random"}
|
||||
target6 := []string{"doc/frotz"}
|
||||
target7 := []string{"filename"}
|
||||
target8 := []string{"as/foo"}
|
||||
target9 := []string{"foo/bar"}
|
||||
target10 := []string{"a/x/y/b"}
|
||||
target11 := []string{"foo/getting-started.md"}
|
||||
target12 := []string{"foo.txt"}
|
||||
target13 := []string{"/scripts/filename.txt"}
|
||||
target14 := []string{"path/to/file.txt"}
|
||||
target15 := []string{"path/to/foo"}
|
||||
target16 := []string{"foo/build-app/troubleshooting.md"}
|
||||
|
||||
func Test_match(t *testing.T) {
|
||||
type args struct {
|
||||
ctx context.Context
|
||||
pattern string
|
||||
target []string
|
||||
pattern string
|
||||
matchingTargets []string
|
||||
nonMatchingTargets []string
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "Test * pattern",
|
||||
name: "root",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "*",
|
||||
target: target1,
|
||||
pattern: "/",
|
||||
matchingTargets: []string{"a.txt", "x/a.txt", "x/y/a.txt"},
|
||||
nonMatchingTargets: []string{},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test ** pattern",
|
||||
name: "file exact match",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "**",
|
||||
target: target2,
|
||||
pattern: "/a.txt",
|
||||
matchingTargets: []string{"a.txt", "a.txt/b.go", "a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt2", "b.txt", "a.go", "x/a.txt"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test ** pattern on fixed path",
|
||||
name: "file exact match with directory",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "abc/xyz",
|
||||
target: target3,
|
||||
pattern: "/x/a.txt",
|
||||
matchingTargets: []string{"x/a.txt", "x/a.txt/b.txt", "x/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt", "x/a.txt2", "x/b.txt", "x/a.go"},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "Test abc/xyz pattern",
|
||||
name: "file relative match",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "abc/xyz",
|
||||
target: target4,
|
||||
pattern: "a.txt",
|
||||
matchingTargets: []string{"a.txt", "x/a.txt", "x/y/a.txt", "x/y/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt2", "b.txt", "a.go", "x/a.txt2"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test incorrect pattern negative",
|
||||
name: "file relative match with directory",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "random/xyz",
|
||||
target: target5,
|
||||
pattern: "x/a.txt",
|
||||
matchingTargets: []string{"x/a.txt", "x/a.txt/b.go", "x/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt2", "b.txt", "a.go", "x/a.txt2", "v/x/a.txt", "y/a.txt"},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "Test file path with **",
|
||||
name: "folder exact match",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "**",
|
||||
target: target14,
|
||||
pattern: "/x/",
|
||||
matchingTargets: []string{"x/a.txt", "x/b.go", "x/y/a.txt"},
|
||||
nonMatchingTargets: []string{"x", "a.txt", "y/a.txt", "w/x/a.txt"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test / pattern",
|
||||
name: "folder relative match",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "doc/frotz",
|
||||
target: target6,
|
||||
pattern: "x/",
|
||||
matchingTargets: []string{"x/a.txt", "x/b.txt", "w/x/a.txt", "w/x/y/a.txt"},
|
||||
nonMatchingTargets: []string{"x", "w/x", "a.txt", "y/a.txt"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test ? pattern",
|
||||
name: "match-all",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "?ilename",
|
||||
target: target7,
|
||||
pattern: "*",
|
||||
matchingTargets: []string{"a", "a.txt", "x/a.txt", "x/y/a.txt"},
|
||||
nonMatchingTargets: []string{},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test /** pattern",
|
||||
name: "match-all in relative dir",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "**/foo",
|
||||
target: target8,
|
||||
pattern: "x/*",
|
||||
matchingTargets: []string{"x/a.txt"},
|
||||
nonMatchingTargets: []string{"x", "y/a.txt", "w/x/b.go", "x/a.txt/b.txt"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test **/ with deep nesting",
|
||||
name: "match-all in absolute dir",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "**/foo",
|
||||
target: target15,
|
||||
pattern: "/x/*",
|
||||
matchingTargets: []string{"x/a.txt", "x/b.go"},
|
||||
nonMatchingTargets: []string{"x", "y/a.txt", "w/x/a.txt", "x/a.txt/b.go"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test **/ pattern",
|
||||
name: "file match-all type",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "foo/**",
|
||||
target: target9,
|
||||
pattern: "*.txt",
|
||||
matchingTargets: []string{"a.txt", "x/a.txt", "x/y/a.txt", "x/y/a.txt/b.go", "x/y/a.txt/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt2", "a.go"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test a/**/b pattern",
|
||||
name: "file match-all type in root folder",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "a/x/y/b",
|
||||
target: target10,
|
||||
pattern: "/*.txt",
|
||||
matchingTargets: []string{"a.txt", "a.txt/b.go", "a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt2", "a.go", "x/a.txt", "x/y/a.txt"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test /* pattern positive",
|
||||
name: "file match-all type in absolute sub folder",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "foo/*",
|
||||
target: target11,
|
||||
pattern: "/x/*.txt",
|
||||
matchingTargets: []string{"x/a.txt", "x/a.txt/b.go", "x/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt", "x/a.txt2", "x/a.go", "w/x/a.txt", "y/a.txt"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test /* pattern negative",
|
||||
name: "file match-all types in relative sub folder",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "foo/*",
|
||||
target: target16,
|
||||
pattern: "x/*.txt",
|
||||
matchingTargets: []string{"x/a.txt", "x/a.txt/b.go", "x/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt", "x/a.txt2", "x/a.go", "w/x/a.txt", "y/a.txt"},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "Test * for files",
|
||||
name: "inner match-all",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "*.txt",
|
||||
target: target12,
|
||||
pattern: "/x/*/a.txt",
|
||||
matchingTargets: []string{"x/y/a.txt", "x/y/a.txt/b.go", "x/y/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt", "x/a.txt", "w/x/y/a.txt", "x/y/z/a.txt"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Test /a/",
|
||||
name: "escaped match-all",
|
||||
args: args{
|
||||
ctx: nil,
|
||||
pattern: "/scripts/",
|
||||
target: target13,
|
||||
pattern: "\\*",
|
||||
matchingTargets: []string{"*", "x/y/*", "x/y/*/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt"},
|
||||
},
|
||||
},
|
||||
/*
|
||||
TODO: Fix bug in doublestar library, currently doesn't match `a.` ...
|
||||
{
|
||||
name: "trailing match-all on string",
|
||||
args: args{
|
||||
pattern: "a.*",
|
||||
matchingTargets: []string{"a.", "a.txt", "x/a.txt", "x/a.txt/b.go", "x/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"atxt", "b.txt"},
|
||||
},
|
||||
},
|
||||
*/
|
||||
{
|
||||
name: "globstar",
|
||||
args: args{
|
||||
pattern: "**",
|
||||
matchingTargets: []string{"a", "a.txt", "x/a.txt", "x/y/a.txt"},
|
||||
nonMatchingTargets: []string{},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "trailing globstar absolute path",
|
||||
args: args{
|
||||
pattern: "/x/**",
|
||||
matchingTargets: []string{"x/a.txt", "x/b.txt", "x/y/a.txt"},
|
||||
nonMatchingTargets: []string{"a.txt", "x", "y/a.txt", "w/x/a.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "trailing globstar relative path",
|
||||
args: args{
|
||||
pattern: "x/**",
|
||||
matchingTargets: []string{"x/a.txt", "x/b.txt", "x/y/a.txt"},
|
||||
nonMatchingTargets: []string{"a.txt", "x", "y/a.txt", "w/x/a.txt"},
|
||||
},
|
||||
},
|
||||
/*
|
||||
TODO: Fix bug in doublestar library, currently doesn't match `a.` ...
|
||||
{
|
||||
name: "trailing globstar on string",
|
||||
args: args{
|
||||
pattern: "a.**",
|
||||
matchingTargets: []string{"a.", "a.txt", "x/a.txt", "x/a.txt/b.go", "x/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"atxt", "b.txt"},
|
||||
},
|
||||
},
|
||||
*/
|
||||
{
|
||||
name: "leading globstar",
|
||||
args: args{
|
||||
pattern: "**/a.txt",
|
||||
matchingTargets: []string{"a.txt", "x/a.txt", "x/y/a.txt", "x/y/a.txt/b.go", "x/y/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"b.txt", "a.txt2"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "surrounding globstar",
|
||||
args: args{
|
||||
pattern: "**/x/**",
|
||||
matchingTargets: []string{"x/a.txt", "w/x/a.txt", "x/y/a.txt", "w/x/y/a.txt"},
|
||||
nonMatchingTargets: []string{"a.txt", "x", "w/x"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "inner globstar",
|
||||
args: args{
|
||||
pattern: "/x/**/a.txt",
|
||||
matchingTargets: []string{
|
||||
"x/a.txt", "x/y/a.txt", "x/y/z/a.txt", "x/y/z/a.txt/b.go", "x/y/z/a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.txt", "w/x/a.txt", "y/a.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "multi-inner globstar",
|
||||
args: args{
|
||||
pattern: "/x/**/z/**/a.txt",
|
||||
matchingTargets: []string{
|
||||
"x/z/a.txt",
|
||||
"x/y/z/l/a.txt",
|
||||
"x/y/yy/z/l/ll/a.txt",
|
||||
"x/y/yy/z/l/ll/a.txt/b.go",
|
||||
"x/y/yy/z/l/ll/a.txt/b.go/c.ar",
|
||||
},
|
||||
nonMatchingTargets: []string{"a.txt", "x/a.txt", "z/a.txt", "w/x/a.txt", "y/a.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "dirty globstar",
|
||||
args: args{
|
||||
pattern: "/a**.txt",
|
||||
matchingTargets: []string{"a.txt", "abc.txt", "a.txt/b.go", "a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a/b/.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "escaped globstar",
|
||||
args: args{
|
||||
pattern: "\\*\\*",
|
||||
matchingTargets: []string{"**", "x/**", "**/y", "x/**/y"},
|
||||
nonMatchingTargets: []string{"x"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "partially escaped globstar",
|
||||
args: args{
|
||||
pattern: "*\\*",
|
||||
matchingTargets: []string{"*", "**", "a*", "x/*", "x/a*", "*/y", "a*/", "x/*/y", "x/a*/y"},
|
||||
nonMatchingTargets: []string{"x"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "single wildchar",
|
||||
args: args{
|
||||
pattern: "/a.?xt",
|
||||
matchingTargets: []string{"a.txt", "a.xxt/b.go", "a.xxt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"x/a.txt", "z/a.txt", "w/x/a.txt", "y/a.txt", "a./xt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "escaped single wildchar",
|
||||
args: args{
|
||||
pattern: "/a.\\?xt",
|
||||
matchingTargets: []string{"a.?xt", "a.?xt/b.go", "a.?xt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{"a.\\?xt", "a.txt", "x/a.?xt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "class",
|
||||
args: args{
|
||||
pattern: "/[abc].txt",
|
||||
matchingTargets: []string{"a.txt", "b.txt", "c.txt"},
|
||||
nonMatchingTargets: []string{"[a-c].txt", "d.txt", "A.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "range class",
|
||||
args: args{
|
||||
pattern: "/[a-c].txt",
|
||||
matchingTargets: []string{"a.txt", "b.txt", "c.txt"},
|
||||
nonMatchingTargets: []string{"[a-c].txt", "d.txt", "A.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "escaped class",
|
||||
args: args{
|
||||
pattern: "/\\[a-c\\].txt",
|
||||
matchingTargets: []string{"[a-c].txt"},
|
||||
nonMatchingTargets: []string{"\\[a-c\\].txt", "a.txt", "b.txt", "c.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "class escaped control chars",
|
||||
args: args{
|
||||
pattern: "/[\\!\\^\\-a-c].txt",
|
||||
matchingTargets: []string{"a.txt", "b.txt", "c.txt", "^.txt", "!.txt", "-.txt"},
|
||||
nonMatchingTargets: []string{"d.txt", "[\\!\\^\\-a-c].txt", "[!^-a-c].txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "inverted class ^",
|
||||
args: args{
|
||||
pattern: "/[^a-c].txt",
|
||||
matchingTargets: []string{"d.txt", "B.txt"},
|
||||
nonMatchingTargets: []string{"a.txt", "b.txt", "c.txt", "[^a-c].txt", "[a-c].txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "escaped inverted class ^",
|
||||
args: args{
|
||||
pattern: "/\\[^a-c\\].txt",
|
||||
matchingTargets: []string{"[^a-c].txt"},
|
||||
nonMatchingTargets: []string{"\\[^a-c\\].txt", "a.txt", "b.txt", "c.txt", "d.txt", "[a-c].txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "inverted class !",
|
||||
args: args{
|
||||
pattern: "/[!a-c].txt",
|
||||
matchingTargets: []string{"d.txt", "B.txt"},
|
||||
nonMatchingTargets: []string{"a.txt", "b.txt", "c.txt", "[!a-c].txt", "[a-c].txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "escaped inverted class !",
|
||||
args: args{
|
||||
pattern: "/\\[!a-c\\].txt",
|
||||
matchingTargets: []string{"[!a-c].txt"},
|
||||
nonMatchingTargets: []string{"\\[!a-c\\].txt", "a.txt", "b.txt", "c.txt", "d.txt", "[a-c].txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "alternate matches",
|
||||
args: args{
|
||||
pattern: "/{a,b,[c-d],e?,f\\*}.txt",
|
||||
matchingTargets: []string{
|
||||
"a.txt", "b.txt", "c.txt", "d.txt", "e2.txt", "f*.txt", "a.txt/b.go", "a.txt/b.go/c.ar"},
|
||||
nonMatchingTargets: []string{
|
||||
"{a,b,[c-d],e?,f\\*}.txt", "{a,b,[c-d],e?,f*}.txt", "e.txt", "f.txt", "g.txt", "ab.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "space",
|
||||
args: args{
|
||||
pattern: "/a b.txt",
|
||||
matchingTargets: []string{"a b.txt"},
|
||||
nonMatchingTargets: []string{"a.txt", "b.txt", "ab.txt", "a b.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "tab",
|
||||
args: args{
|
||||
pattern: "/a b.txt",
|
||||
matchingTargets: []string{"a b.txt"},
|
||||
nonMatchingTargets: []string{"a.txt", "b.txt", "ab.txt", "a b.txt", "a b.txt"},
|
||||
},
|
||||
},
|
||||
{
|
||||
// Note: it's debatable which behavior is correct - for now keep doublestar default behavior on this.
|
||||
// Keeping UT to ensure we don't accidentally change behavior.
|
||||
name: "escaped backslash",
|
||||
args: args{
|
||||
pattern: "/a\\\\/b.txt",
|
||||
matchingTargets: []string{"a\\/b.txt"},
|
||||
nonMatchingTargets: []string{"a\\\\/b.txt", "a/b.txt", "a/b.txt/c.ar"},
|
||||
},
|
||||
},
|
||||
{
|
||||
// Note: it's debatable which behavior is correct - for now keep doublestar default behavior on this.
|
||||
// Keeping UT to ensure we don't accidentally change behavior.
|
||||
name: "escaped path separator",
|
||||
args: args{
|
||||
pattern: "/a\\/b.txt",
|
||||
matchingTargets: []string{"a/b.txt", "a/b.txt/c.ar"},
|
||||
nonMatchingTargets: []string{"a\\/b.txt"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
}
|
||||
testMatch := func(pattern string, target string, want bool) {
|
||||
got, err := match(pattern, target)
|
||||
if err != nil {
|
||||
t.Errorf("failed with error: %s", err)
|
||||
} else if got != want {
|
||||
t.Errorf("match(%q, %q) = %t but wanted %t)", pattern, target, got, want)
|
||||
}
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got, _ := contains(tt.args.pattern, tt.args.target); got != tt.want {
|
||||
t.Errorf("contains() = %v, want %v", got, tt.want)
|
||||
for _, target := range tt.args.matchingTargets {
|
||||
if len(target) > 0 && target[0] == '/' {
|
||||
t.Errorf("target shouldn't start with leading '/'")
|
||||
}
|
||||
|
||||
testMatch(tt.args.pattern, target, true)
|
||||
testMatch(tt.args.pattern, "/"+target, true)
|
||||
}
|
||||
for _, target := range tt.args.nonMatchingTargets {
|
||||
if len(target) > 0 && target[0] == '/' {
|
||||
t.Errorf("target shouldn't start with leading '/'")
|
||||
}
|
||||
|
||||
testMatch(tt.args.pattern, target, false)
|
||||
testMatch(tt.args.pattern, "/"+target, false)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ type CodeOwnerEvaluation struct {
|
|||
}
|
||||
|
||||
type CodeOwnerEvaluationEntry struct {
|
||||
LineNumber int64 `json:"line_number"`
|
||||
Pattern string `json:"pattern"`
|
||||
OwnerEvaluations []OwnerEvaluation `json:"owner_evaluations"`
|
||||
UserGroupOwnerEvaluations []UserGroupOwnerEvaluation `json:"user_group_owner_evaluations"`
|
||||
|
|
|
|||
Loading…
Reference in New Issue