From fcdb3f8907fc70f0492a17d5be4c28211da96971 Mon Sep 17 00:00:00 2001 From: asafa Date: Wed, 7 Aug 2024 17:02:20 +0300 Subject: [PATCH 1/6] Add Mvn install stderr to error --- build/maven.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/build/maven.go b/build/maven.go index 5c9c890c..734e943b 100644 --- a/build/maven.go +++ b/build/maven.go @@ -342,7 +342,8 @@ func (config *mvnRunConfig) SetOutputWriter(outputWriter io.Writer) *mvnRunConfi func (config *mvnRunConfig) runCmd() error { command := config.GetCmd() - command.Stderr = os.Stderr + errBuffer := bytes.NewBuffer([]byte{}) + command.Stderr = errBuffer if config.outputWriter == nil { command.Stdout = os.Stderr } else { @@ -350,5 +351,10 @@ func (config *mvnRunConfig) runCmd() error { } command.Dir = config.workspace config.logger.Info("Running mvn command:", strings.Join(command.Args, " ")) + err := command.Run() + if err != nil { + errResult := errBuffer.Bytes() + return fmt.Errorf("error while running '%s %s': %s\n%s", "mvn", strings.Join(command.Args, " "), err.Error(), strings.TrimSpace(string(errResult))) + } return command.Run() } From f8c5bed8210b388fb357ca7480f7db5e7a1ddb4a Mon Sep 17 00:00:00 2001 From: asafa Date: Wed, 7 Aug 2024 18:24:03 +0300 Subject: [PATCH 2/6] Fix. --- build/maven.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/maven.go b/build/maven.go index 734e943b..06d9c3e9 100644 --- a/build/maven.go +++ b/build/maven.go @@ -356,5 +356,5 @@ func (config *mvnRunConfig) runCmd() error { errResult := errBuffer.Bytes() return fmt.Errorf("error while running '%s %s': %s\n%s", "mvn", strings.Join(command.Args, " "), err.Error(), strings.TrimSpace(string(errResult))) } - return command.Run() + return nil } From 99b983082a789726c723f8650d453326f497dce6 Mon Sep 17 00:00:00 2001 From: asafa Date: Mon, 12 Aug 2024 17:36:50 +0300 Subject: [PATCH 3/6] Fix. --- build/maven.go | 33 +++++++++++++++++++++++++++------ go.mod | 3 ++- go.sum | 6 ++++-- utils/error.go | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 utils/error.go diff --git a/build/maven.go b/build/maven.go index 06d9c3e9..608c7119 100644 --- a/build/maven.go +++ b/build/maven.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/jfrog/build-info-go/utils" + "golang.org/x/term" ) const ( @@ -340,21 +341,41 @@ func (config *mvnRunConfig) SetOutputWriter(outputWriter io.Writer) *mvnRunConfi return config } -func (config *mvnRunConfig) runCmd() error { +func (config *mvnRunConfig) runCmd() (err error) { command := config.GetCmd() errBuffer := bytes.NewBuffer([]byte{}) - command.Stderr = errBuffer + multiWriter := io.MultiWriter(os.Stderr, errBuffer) + command.Stderr = multiWriter if config.outputWriter == nil { command.Stdout = os.Stderr } else { command.Stdout = config.outputWriter } command.Dir = config.workspace + addColorToCmdOutput(command) config.logger.Info("Running mvn command:", strings.Join(command.Args, " ")) - err := command.Run() + + err = command.Run() if err != nil { - errResult := errBuffer.Bytes() - return fmt.Errorf("error while running '%s %s': %s\n%s", "mvn", strings.Join(command.Args, " "), err.Error(), strings.TrimSpace(string(errResult))) + if utils.IsForbiddenOutput("maven", errBuffer.String()) { + err = errors.Join(utils.NewForbiddenError(), err) + } + } + return +} + +// To always have color in Maven's output, add "-Dstyle.color=always" to the command line arguments +func addColorToCmdOutput(command *exec.Cmd) { + if term.IsTerminal(int(os.Stderr.Fd())) { + shouldAddColor := true + for _, arg := range command.Args { + if strings.Contains(arg, "-Dstyle.color") { + shouldAddColor = false + break + } + } + if shouldAddColor { + command.Args = append(command.Args, "-Dstyle.color=always") + } } - return nil } diff --git a/go.mod b/go.mod index 3d61ff95..b342778b 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/urfave/cli/v2 v2.27.2 github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/exp v0.0.0-20240707233637-46b078467d37 + golang.org/x/term v0.23.0 ) require ( @@ -26,6 +27,6 @@ require ( github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778 // indirect github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913 // indirect golang.org/x/sync v0.7.0 // indirect - golang.org/x/sys v0.17.0 // indirect + golang.org/x/sys v0.23.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 77516e03..56a3ea55 100644 --- a/go.sum +++ b/go.sum @@ -46,8 +46,10 @@ golang.org/x/exp v0.0.0-20240707233637-46b078467d37/go.mod h1:M4RDyNAINzryxdtnbR golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.23.0 h1:YfKFowiIMvtgl1UERQoTPPToxltDeZfbj4H7dVUCwmM= +golang.org/x/sys v0.23.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.23.0 h1:F6D4vR+EHoL9/sWAWgAR1H2DcHr4PareCbAaCo1RpuU= +golang.org/x/term v0.23.0/go.mod h1:DgV24QBUrK6jhZXl+20l6UWznPlwAHm1Q1mGHtydmSk= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/utils/error.go b/utils/error.go new file mode 100644 index 00000000..e05175db --- /dev/null +++ b/utils/error.go @@ -0,0 +1,40 @@ +package utils + +import ( + "fmt" + "strings" +) + +// ForbiddenError represents a 403 Forbidden error. +type ForbiddenError struct { + Message string +} + +// Error implements the error interface for ForbiddenError. +func (e *ForbiddenError) Error() string { + return fmt.Sprintf("403 Forbidden") +} + +// NewForbiddenError creates a new ForbiddenError with the given message. +func NewForbiddenError() *ForbiddenError { + return &ForbiddenError{} +} + +// IsForbiddenOutput verify the output is forbidden, each tech have its own forbidden output. +func IsForbiddenOutput(tech string, cmdOutput string) bool { + switch tech { + case "npm": + return strings.Contains(strings.ToLower(cmdOutput), "403 forbidden") + case "maven": + return strings.Contains(cmdOutput, "status code: 403") || + strings.Contains(strings.ToLower(cmdOutput), "403 forbidden") || + // In some cases mvn returns 500 status code even though it got 403 from artifactory. + strings.Contains(cmdOutput, "status code: 500") + case "pip": + return strings.Contains(strings.ToLower(cmdOutput), "http error 403") + case "go": + return strings.Contains(strings.ToLower(cmdOutput), "403 forbidden") || + strings.Contains(strings.ToLower(cmdOutput), " 403") + } + return false +} From 2cc938affc4c6e8bdc673bc5cb8ce2f71be5a86e Mon Sep 17 00:00:00 2001 From: asafa Date: Tue, 13 Aug 2024 13:30:22 +0300 Subject: [PATCH 4/6] Add test. --- build/maven_test.go | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/build/maven_test.go b/build/maven_test.go index 75d514f2..a6cb8f60 100644 --- a/build/maven_test.go +++ b/build/maven_test.go @@ -8,7 +8,9 @@ import ( "github.com/jfrog/build-info-go/tests" "github.com/jfrog/build-info-go/utils" "os" + "os/exec" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -127,3 +129,54 @@ func TestGetExecutableName(t *testing.T) { assert.Equal(t, result, mvnHome) } } + +func TestAddColorToCmdOutput(t *testing.T) { + testCases := []struct { + name string + initialArgs []string + expectedResult string + colorArgExist bool + }{ + { + name: "Not a terminal, shouldn't add color", + initialArgs: []string{"mvn"}, + colorArgExist: false, + }, + { + name: "Terminal supports color and existing color argument", + initialArgs: []string{"mvn", "-Dstyle.color=always"}, + expectedResult: "Dstyle.color=always", + colorArgExist: true, + }, + { + name: "Terminal supports color and existing color argument", + initialArgs: []string{"mvn", "-Dstyle.color=never"}, + expectedResult: "Dstyle.color=never", + colorArgExist: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Mock terminal support + + // Create a mock exec.Cmd object + cmd := exec.Command(tc.initialArgs[0], tc.initialArgs[1:]...) + + // Call the function to test + addColorToCmdOutput(cmd) + + // Check if the argument was added + containsColorArg := false + for _, arg := range cmd.Args { + if strings.Contains(arg, "Dstyle.color") { + if strings.Contains(arg, tc.expectedResult) { + containsColorArg = true + break + } + } + } + assert.Equal(t, tc.colorArgExist, containsColorArg) + }) + } +} From 0e0107ad1cb682fd378a3deb2bb6d0ecd4409454 Mon Sep 17 00:00:00 2001 From: asafa Date: Tue, 13 Aug 2024 15:08:50 +0300 Subject: [PATCH 5/6] Fix. --- utils/error.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/utils/error.go b/utils/error.go index e05175db..6d6298f3 100644 --- a/utils/error.go +++ b/utils/error.go @@ -1,7 +1,6 @@ package utils import ( - "fmt" "strings" ) @@ -12,7 +11,7 @@ type ForbiddenError struct { // Error implements the error interface for ForbiddenError. func (e *ForbiddenError) Error() string { - return fmt.Sprintf("403 Forbidden") + return "403 Forbidden" } // NewForbiddenError creates a new ForbiddenError with the given message. From c3f52700927d0adc6615b2bc5df5fbe20660d6e0 Mon Sep 17 00:00:00 2001 From: asafa Date: Mon, 19 Aug 2024 16:31:17 +0300 Subject: [PATCH 6/6] Fix. --- build/maven.go | 2 +- utils/error.go | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/build/maven.go b/build/maven.go index 608c7119..ffe6852e 100644 --- a/build/maven.go +++ b/build/maven.go @@ -357,7 +357,7 @@ func (config *mvnRunConfig) runCmd() (err error) { err = command.Run() if err != nil { - if utils.IsForbiddenOutput("maven", errBuffer.String()) { + if utils.IsForbiddenOutput(utils.Maven, errBuffer.String()) { err = errors.Join(utils.NewForbiddenError(), err) } } diff --git a/utils/error.go b/utils/error.go index 6d6298f3..8d14cbd1 100644 --- a/utils/error.go +++ b/utils/error.go @@ -4,6 +4,15 @@ import ( "strings" ) +type PackageManager string + +const ( + Npm PackageManager = "npm" + Maven PackageManager = "maven" + Pip PackageManager = "pip" + Go PackageManager = "go" +) + // ForbiddenError represents a 403 Forbidden error. type ForbiddenError struct { Message string @@ -19,8 +28,8 @@ func NewForbiddenError() *ForbiddenError { return &ForbiddenError{} } -// IsForbiddenOutput verify the output is forbidden, each tech have its own forbidden output. -func IsForbiddenOutput(tech string, cmdOutput string) bool { +// IsForbiddenOutput checks whether the provided output includes a 403 Forbidden. The various package managers have their own forbidden output formats. +func IsForbiddenOutput(tech PackageManager, cmdOutput string) bool { switch tech { case "npm": return strings.Contains(strings.ToLower(cmdOutput), "403 forbidden")