Skip to content

Commit

Permalink
Ignore ghost user (#53)
Browse files Browse the repository at this point in the history
  • Loading branch information
miguelslemos committed Oct 31, 2020
1 parent 0b6c2ef commit 97fb795
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 29 deletions.
38 changes: 9 additions & 29 deletions internal/check/valid_owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package check
import (
"context"
"net/http"
"net/mail"
"strings"

"github.com/mszostok/codeowners-validator/internal/ctxutil"
Expand Down Expand Up @@ -82,11 +81,11 @@ func (v *ValidOwner) Check(ctx context.Context, in Input) (Output, error) {

func (v *ValidOwner) selectValidateFn(name string) func(context.Context, string) *validateError {
switch {
case isGithubUser(name):
case IsGithubUser(name):
return v.validateGithubUser
case isGithubTeam(name):
case IsGithubTeam(name):
return v.validateTeam
case isEmailAddress(name):
case IsEmailAddress(name):
// TODO(mszostok): try to check if e-mail really exists
return func(context.Context, string) *validateError { return nil }
default:
Expand Down Expand Up @@ -162,6 +161,12 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr
}

func (v *ValidOwner) validateGithubUser(ctx context.Context, name string) *validateError {
// Ignore @ghost user
// https://github.community/t5/How-to-use-Git-and-GitHub/CODEOWNERS-file-with-a-NOT-file-type-condition/m-p/31013/highlight/true#M8523
if IsGithubGhostUser(name) {
return nil
}

if v.orgMembers == nil { //TODO(mszostok): lazy init, make it more robust.
if err := v.initOrgListMembers(ctx); err != nil {
return newValidateError("Cannot initialize organization member list: %v", err).AsPermanent()
Expand Down Expand Up @@ -222,31 +227,6 @@ func (v *ValidOwner) initOrgListMembers(ctx context.Context) error {
return nil
}

func isEmailAddress(s string) bool {
_, err := mail.ParseAddress(s)

return err == nil
}

func isGithubTeam(s string) bool {
hasPrefix := strings.HasPrefix(s, "@")
containsSlash := strings.Contains(s, "/")
split := strings.SplitN(s, "/", 3) // 3 is enough to confirm that is invalid + will not overflow the buffer

if hasPrefix && containsSlash && len(split) == 2 {
return true
}

return false
}

func isGithubUser(s string) bool {
if strings.HasPrefix(s, "@") && !strings.Contains(s, "/") {
return true
}
return false
}

// Name returns human readable name of the validator
func (ValidOwner) Name() string {
return "Valid Owner Checker"
Expand Down
26 changes: 26 additions & 0 deletions internal/check/valid_owner_check.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package check

import (
"net/mail"
"strings"
)

func IsEmailAddress(s string) bool {
_, err := mail.ParseAddress(s)
return err == nil
}

func IsGithubTeam(s string) bool {
hasPrefix := strings.HasPrefix(s, "@")
containsSlash := strings.Contains(s, "/")
split := strings.SplitN(s, "/", 3) // 3 is enough to confirm that is invalid + will not overflow the buffer
return hasPrefix && containsSlash && len(split) == 2 && len(split[1]) > 0
}

func IsGithubUser(s string) bool {
return !strings.Contains(s, "/") && strings.HasPrefix(s, "@")
}

func IsGithubGhostUser(s string) bool {
return s == "@ghost"
}
52 changes: 52 additions & 0 deletions internal/check/valid_owner_check_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package check_test

import (
"testing"

"github.com/mszostok/codeowners-validator/internal/check"

"github.com/stretchr/testify/assert"
)

func TestValidOwnerChecker(t *testing.T) {
tests := map[string]struct {
user string
isValid bool
}{
"Invalid Email": {
user: `asda.comm`,
isValid: false,
},
"Valid Email": {
user: `gmail@gmail.com`,
isValid: true,
},
"Invalid Team": {
user: `@org/`,
isValid: false,
},
"Valid Team": {
user: `@org/user`,
isValid: true,
},
"Invalid User": {
user: `user`,
isValid: false,
},
"Valid User": {
user: `@user`,
isValid: true,
},
}
for tn, tc := range tests {
t.Run(tn, func(t *testing.T) {
// when
result := isValidUser(tc.user)
assert.Equal(t, tc.isValid, result)
})
}
}

func isValidUser(user string) bool {
return check.IsEmailAddress(user) || check.IsGithubUser(user) || check.IsGithubTeam(user)
}

0 comments on commit 97fb795

Please sign in to comment.