Skip to content

Commit

Permalink
Refactor git cmd and error parser
Browse files Browse the repository at this point in the history
Reduce cyclic complexity in git_error_parser
  • Loading branch information
Baran Dalgic committed Oct 15, 2021
1 parent 8290272 commit 91ce776
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 67 deletions.
29 changes: 21 additions & 8 deletions cmd/git/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ func (e ExitError) Error() string {

type settings struct {
help bool
skipValidation bool
depth uint
url string
revision string
depth uint
target string
resultFileCommitSha string
resultFileCommitAuthor string
resultErrorMessage string
resultErrorReason string
secretPath string
skipValidation bool
}

var flagValues settings
Expand Down Expand Up @@ -112,6 +112,7 @@ func main() {
// Execute performs flag parsing, input validation and the Git clone
func Execute(ctx context.Context) error {
flagValues = settings{depth: 1}

pflag.Parse()

if flagValues.help {
Expand Down Expand Up @@ -419,10 +420,18 @@ func checkCredentials() (credentialType, error) {
return typePrivateKey, nil

case hasPrivateKey && !isSSHGitURL:
return typeUndef, &ExitError{Code: 110, Message: "Credential/URL inconsistency: SSH credentials provided, but URL is not a SSH Git URL", Reason: shpgit.AuthInvalidUserOrPass}
return typeUndef, &ExitError{
Code: 110,
Message: "Credential/URL inconsistency: SSH credentials provided, but URL is not a SSH Git URL",
Reason: shpgit.AuthInvalidUserOrPass,
}

case !hasPrivateKey && isSSHGitURL:
return typeUndef, &ExitError{Code: 110, Message: "Credential/URL inconsistency: No SSH credentials provided, but URL is a SSH Git URL", Reason: shpgit.AuthInvalidKey}
return typeUndef, &ExitError{
Code: 110,
Message: "Credential/URL inconsistency: No SSH credentials provided, but URL is a SSH Git URL",
Reason: shpgit.AuthInvalidKey,
}
}

// Checking whether mounted secret is of type `kubernetes.io/basic-auth`
Expand All @@ -435,8 +444,11 @@ func checkCredentials() (credentialType, error) {
return typeUsernamePassword, nil

case hasUsername && !hasPassword || !hasUsername && hasPassword:
return typeUndef, &ExitError{Code: 110, Message: "Basic Auth incomplete: Both username and password need to be configured", Reason: shpgit.AuthInvalidUserOrPass}

return typeUndef, &ExitError{
Code: 110,
Message: "Basic Auth incomplete: Both username and password need to be configured",
Reason: shpgit.AuthInvalidUserOrPass,
}
}

return typeUndef, &ExitError{Code: 110, Message: "Unsupported type of credentials provided, either SSH private key or username/password is supported"}
Expand All @@ -447,10 +459,11 @@ func writeErrorResults(failure *shpgit.ErrorResult) (err error) {
return nil
}

if err = ioutil.WriteFile(flagValues.resultErrorMessage, []byte(strings.TrimSpace(failure.Message)), 0644); err != nil {
if err = ioutil.WriteFile(flagValues.resultErrorMessage, []byte(strings.TrimSpace(failure.Message)), 0600); err != nil {
return err
}
if err = ioutil.WriteFile(flagValues.resultErrorReason, []byte(failure.Reason.String()), 0644); err != nil {

if err = ioutil.WriteFile(flagValues.resultErrorReason, []byte(failure.Reason.String()), 0600); err != nil {
return err
}

Expand Down
174 changes: 117 additions & 57 deletions pkg/git/git_error_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (
"strings"
)

type ErrorClass int
type Prefix int
type (
ErrorClass int
Prefix int
)

const (
General Prefix = iota
Expand All @@ -27,45 +29,32 @@ const (
AuthPrompted // caused when repo not found or private repo access with wrong/missing auth
)

var mapClassToMessage = map[ErrorClass]string{
RepositoryNotFound: "The source repository does not exist or you have insufficient rights.",
AuthInvalidUserOrPass: "Basic authentication has failed. Check your username or password. Note: Github requires a personal access token instead of your regular password.",
AuthPrompted: "The source repository does not exist or you have insufficient right. Provide authentication for more details.",
BranchNotFound: "The remote branch does not exist. Check your revision argument",
AuthInvalidKey: "The key is invalid for the specified target. Please make sure that the remote source exists, you have sufficient rights and the key is in the right format.",
}

type RawToken struct {
type rawToken struct {
raw string
}

type PrefixToken struct {
type prefixToken struct {
scope Prefix
RawToken
rawToken
}

type ErrorClassToken struct {
type errorClassToken struct {
class ErrorClass
RawToken
}

type ErrorToken struct {
Prefix PrefixToken
Error ErrorClassToken
rawToken
}

type ErrorWithCode struct {
ErrorCode int
ErrorMessage string
Error error
type errorToken struct {
Prefix prefixToken
Error errorClassToken
}

// ErrorResult is a representation of a runtime error of a git operation that presents a reason and a message
type ErrorResult struct {
Message string
Reason ErrorClass
}

func (rawToken RawToken) String() string {
func (rawToken rawToken) String() string {
return rawToken.raw
}

Expand All @@ -83,16 +72,33 @@ func (class ErrorClass) String() string {
// https based git operations against non-existing or private repo without authentication
case AuthPrompted:
return "git-remote-private"
default:
return "git-error"
}

return "git-error"
}

func (token ErrorToken) String() string {
func (class ErrorClass) toMessage() string {
switch class {
case RepositoryNotFound:
return "The source repository does not exist or you have insufficient rights."
case AuthInvalidUserOrPass:
return "Basic authentication has failed. Check your username or password. Note: Github requires a personal access token instead of your regular password."
case AuthPrompted:
return "The source repository does not exist or you have insufficient right. Provide authentication for more details."
case BranchNotFound:
return "The remote branch does not exist. Check your revision argument"
case AuthInvalidKey:
return "The key is invalid for the specified target. Please make sure that the remote source exists, you have sufficient rights and the key is in the right format."
}

return "Git encountered an unknown error."
}

func (token errorToken) String() string {
return token.Prefix.String() + ": " + token.Error.String()
}

func parse(message string) (tokenList []ErrorToken) {
func parse(message string) (tokenList []errorToken) {
reader := strings.NewReader(message)
scanner := bufio.NewScanner(reader)

Expand All @@ -105,30 +111,35 @@ func parse(message string) (tokenList []ErrorToken) {
return tokenList
}

func parseLine(line string) (*ErrorToken, error) {
var errWrongFormat = errors.New("not in the right format of 'prefix:message'")

func parseLine(line string) (*errorToken, error) {
var prefixBuilder strings.Builder

var errorMessage string

for i, char := range line {
if char == ':' {
errorMessage = line[i+1:]

break
}

prefixBuilder.WriteRune(char)
}

if len(strings.TrimSpace(errorMessage)) == 0 {
return nil, errors.New("not in the right format of 'prefix: message'")
return nil, errWrongFormat
}

prefixToken := parsePrefix(prefixBuilder.String())
errorClassToken := parseErrorMessage(errorMessage)

return &ErrorToken{Error: errorClassToken, Prefix: prefixToken}, nil
return &errorToken{Error: errorClassToken, Prefix: prefixToken}, nil
}

func parsePrefix(raw string) PrefixToken {
var prefix = General
func parsePrefix(raw string) prefixToken {
prefix := General

switch strings.ToLower(strings.TrimSpace(raw)) {
case "fatal":
Expand All @@ -141,37 +152,57 @@ func parsePrefix(raw string) PrefixToken {
prefix = Error
}

return PrefixToken{prefix, RawToken{raw}}
return prefixToken{prefix, rawToken{raw}}
}

func isAuthInvalidUserOrPass(raw string) bool {
return strings.Contains(raw, "authentication failed for") ||
strings.Contains(raw, "invalid username or password")
}

func isAuthPrompted(raw string) bool {
return strings.Contains(raw, "terminal prompts disabled")
}

func isAuthInvalidKey(raw string) bool {
return strings.Contains(raw, "could not read from remote")
}

func isRepositoryNotFound(raw string) bool {
isMatch, _ := regexp.Match("(repository|project) .* found", []byte(raw))

return isMatch
}

func parseErrorMessage(raw string) ErrorClassToken {
var errorClass = Unknown
func isBranchNotFound(raw string) bool {
return strings.Contains(raw, "remote branch") && strings.Contains(raw, "not found")
}

func parseErrorMessage(raw string) errorClassToken {
errorClass := Unknown
toCheck := strings.ToLower(strings.TrimSpace(raw))

// basic auth failed for given creds
if strings.Contains(toCheck, "authentication failed for") || strings.Contains(toCheck, "invalid username or password") {
switch {
case isAuthInvalidUserOrPass(toCheck):
errorClass = AuthInvalidUserOrPass
} else if strings.Contains(toCheck, "terminal prompts disabled") {
case isAuthPrompted(toCheck):
errorClass = AuthPrompted
} else if strings.Contains(toCheck, "could not read from remote") {
case isAuthInvalidKey(toCheck):
errorClass = AuthInvalidKey
} else if isMatch, _ := regexp.Match("(repository|project) .* found", []byte(toCheck)); isMatch {
case isRepositoryNotFound(toCheck):
errorClass = RepositoryNotFound
} else if strings.Contains(toCheck, "remote branch") && strings.Contains(toCheck, "not found") {
case isBranchNotFound(toCheck):
errorClass = BranchNotFound
}
return ErrorClassToken{errorClass, RawToken{

return errorClassToken{errorClass, rawToken{
raw,
}}
}

func classifyErrorFromTokens(tokens []ErrorToken) ErrorClass {
classifierMap := map[Prefix][]ErrorToken{}
for _, token := range tokens {
classifierMap[token.Prefix.scope] = append(classifierMap[token.Prefix.scope], token)
}

for _, remoteToken := range classifierMap[Remote] {
func classifyTokensWithRemotePrefix(tokens []errorToken) ErrorClass {
for _, remoteToken := range tokens {
switch remoteToken.Error.class {
case AuthInvalidUserOrPass:
return AuthInvalidUserOrPass
Expand All @@ -180,14 +211,21 @@ func classifyErrorFromTokens(tokens []ErrorToken) ErrorClass {
}
}

for _, remoteToken := range classifierMap[Error] {
switch remoteToken.Error.class {
case RepositoryNotFound:
return Unknown
}

func classifyTokensWithErrorPrefix(tokens []errorToken) ErrorClass {
for _, remoteToken := range tokens {
if remoteToken.Error.class == RepositoryNotFound {
return RepositoryNotFound
}
}

for _, fatalToken := range classifierMap[Fatal] {
return Unknown
}

func classifyTokensWithFatalPrefix(tokens []errorToken) ErrorClass {
for _, fatalToken := range tokens {
switch fatalToken.Error.class {
case AuthInvalidKey:
// either repo no exists or wrong key used
Expand All @@ -205,21 +243,43 @@ func classifyErrorFromTokens(tokens []ErrorToken) ErrorClass {
return Unknown
}

func extractResultsFromTokens(tokens []ErrorToken) *ErrorResult {
func classifyErrorFromTokens(tokens []errorToken) ErrorClass {
classifierMap := map[Prefix][]errorToken{}
for _, token := range tokens {
classifierMap[token.Prefix.scope] = append(classifierMap[token.Prefix.scope], token)
}

if errorClass := classifyTokensWithRemotePrefix(classifierMap[Remote]); errorClass != Unknown {
return errorClass
}

if errorClass := classifyTokensWithErrorPrefix(classifierMap[Error]); errorClass != Unknown {
return errorClass
}

if errorClass := classifyTokensWithFatalPrefix(classifierMap[Fatal]); errorClass != Unknown {
return errorClass
}

return Unknown
}

func extractResultsFromTokens(tokens []errorToken) *ErrorResult {
mainErrorClass := classifyErrorFromTokens(tokens)

if mainErrorClass == Unknown {
builder := strings.Builder{}
for _, token := range tokens {
builder.WriteString(token.String() + "\n")
}

return &ErrorResult{Message: builder.String(), Reason: Unknown}
}

return &ErrorResult{Message: mapClassToMessage[mainErrorClass], Reason: mainErrorClass}
return &ErrorResult{Message: mainErrorClass.toMessage(), Reason: mainErrorClass}
}

// NewErrorResultFromMessage parses a message, derives an error result and returns an instance of ErrorResult
// NewErrorResultFromMessage parses a message, derives an error result and returns an instance of ErrorResult.
func NewErrorResultFromMessage(message string) *ErrorResult {
return extractResultsFromTokens(parse(message))
}
4 changes: 2 additions & 2 deletions pkg/git/git_error_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
)

var _ = Describe("Parsing Git Error Messages", func() {
Context("parse raw to PrefixToken", func() {
Context("parse raw to prefixToken", func() {
It("should recognize and parse fatal", func() {
parsed := parsePrefix("fatal")

Expand All @@ -33,7 +33,7 @@ var _ = Describe("Parsing Git Error Messages", func() {
})
})

Context("Parse raw to ErrorToken", func() {
Context("Parse raw to errorToken", func() {
It("should recognize and parse unknown branch", func() {
parsed := parseErrorMessage("Remote branch not found")
Expect(parsed.class).To(Equal(BranchNotFound))
Expand Down

0 comments on commit 91ce776

Please sign in to comment.