diff --git a/artifactory/commands/transferfiles/fileserror.go b/artifactory/commands/transferfiles/fileserror.go index 47f0b867a..faa1aad52 100644 --- a/artifactory/commands/transferfiles/fileserror.go +++ b/artifactory/commands/transferfiles/fileserror.go @@ -77,7 +77,7 @@ func (e *errorsRetryPhase) handleErrorsFile(errFilePath string, pcWrapper *produ // Since we're about to handle the transfer retry of the failed files, // we should now decrement the failures counter view. e.progressBar.changeNumberOfFailuresBy(-1 * len(failedFiles.Errors)) - err = e.stateManager.ChangeTransferFailureCountBy(uint(len(failedFiles.Errors)), false) + err = e.stateManager.ChangeTransferFailureCountBy(uint64(len(failedFiles.Errors)), false) if err != nil { return err } diff --git a/artifactory/commands/transferfiles/state/runstatus.go b/artifactory/commands/transferfiles/state/runstatus.go index 5454aa8bb..2c6924f6c 100644 --- a/artifactory/commands/transferfiles/state/runstatus.go +++ b/artifactory/commands/transferfiles/state/runstatus.go @@ -40,7 +40,7 @@ type TransferRunStatus struct { WorkingThreads int `json:"working_threads,omitempty"` VisitedFolders uint64 `json:"visited_folders,omitempty"` DelayedFiles uint64 `json:"delayed_files,omitempty"` - TransferFailures uint `json:"transfer_failures,omitempty"` + TransferFailures uint64 `json:"transfer_failures,omitempty"` TimeEstimationManager `json:"time_estimation,omitempty"` StaleChunks []StaleChunks `json:"stale_chunks,omitempty"` } diff --git a/artifactory/commands/transferfiles/state/statemanager.go b/artifactory/commands/transferfiles/state/statemanager.go index 273c9d28b..962d10910 100644 --- a/artifactory/commands/transferfiles/state/statemanager.go +++ b/artifactory/commands/transferfiles/state/statemanager.go @@ -136,18 +136,19 @@ func (ts *TransferStateManager) SetRepoFullTransferCompleted() error { // Increasing Transferred Diff files (modified files) and SizeByBytes value in suitable repository progress state func (ts *TransferStateManager) IncTransferredSizeAndFilesPhase1(chunkTotalFiles, chunkTotalSizeInBytes int64) error { err := ts.TransferState.Action(func(state *TransferState) error { - state.CurrentRepo.Phase1Info.TransferredSizeBytes += chunkTotalSizeInBytes - state.CurrentRepo.Phase1Info.TransferredUnits += chunkTotalFiles + atomicallyAddInt64(&state.CurrentRepo.Phase1Info.TransferredSizeBytes, chunkTotalSizeInBytes) + atomicallyAddInt64(&state.CurrentRepo.Phase1Info.TransferredUnits, chunkTotalFiles) return nil }) if err != nil { return err } return ts.TransferRunStatus.action(func(transferRunStatus *TransferRunStatus) error { - transferRunStatus.OverallTransfer.TransferredSizeBytes += chunkTotalSizeInBytes - transferRunStatus.OverallTransfer.TransferredUnits += chunkTotalFiles + atomicallyAddInt64(&transferRunStatus.OverallTransfer.TransferredSizeBytes, chunkTotalSizeInBytes) + atomicallyAddInt64(&transferRunStatus.OverallTransfer.TransferredUnits, chunkTotalFiles) + if transferRunStatus.BuildInfoRepo { - transferRunStatus.OverallBiFiles.TransferredUnits += chunkTotalFiles + atomicallyAddInt64(&transferRunStatus.OverallBiFiles.TransferredUnits, chunkTotalFiles) } return nil }) @@ -155,16 +156,16 @@ func (ts *TransferStateManager) IncTransferredSizeAndFilesPhase1(chunkTotalFiles func (ts *TransferStateManager) IncTransferredSizeAndFilesPhase2(chunkTotalFiles, chunkTotalSizeInBytes int64) error { return ts.TransferState.Action(func(state *TransferState) error { - state.CurrentRepo.Phase2Info.TransferredSizeBytes += chunkTotalSizeInBytes - state.CurrentRepo.Phase2Info.TransferredUnits += chunkTotalFiles + atomicallyAddInt64(&state.CurrentRepo.Phase2Info.TransferredSizeBytes, chunkTotalSizeInBytes) + atomicallyAddInt64(&state.CurrentRepo.Phase2Info.TransferredUnits, chunkTotalFiles) return nil }) } func (ts *TransferStateManager) IncTotalSizeAndFilesPhase2(filesNumber, totalSize int64) error { return ts.TransferState.Action(func(state *TransferState) error { - state.CurrentRepo.Phase2Info.TotalSizeBytes += totalSize - state.CurrentRepo.Phase2Info.TotalUnits += filesNumber + atomicallyAddInt64(&state.CurrentRepo.Phase2Info.TotalSizeBytes, totalSize) + atomicallyAddInt64(&state.CurrentRepo.Phase2Info.TotalUnits, filesNumber) return nil }) } @@ -172,8 +173,10 @@ func (ts *TransferStateManager) IncTotalSizeAndFilesPhase2(filesNumber, totalSiz // Set relevant information of files and storage we need to transfer in phase3 func (ts *TransferStateManager) SetTotalSizeAndFilesPhase3(filesNumber, totalSize int64) error { return ts.TransferState.Action(func(state *TransferState) error { - state.CurrentRepo.Phase3Info.TotalSizeBytes = totalSize - state.CurrentRepo.Phase3Info.TotalUnits = filesNumber + state.CurrentRepo.Phase3Info.TransferredUnits = 0 + state.CurrentRepo.Phase3Info.TransferredSizeBytes = 0 + atomicallyAddInt64(&state.CurrentRepo.Phase3Info.TotalSizeBytes, totalSize) + atomicallyAddInt64(&state.CurrentRepo.Phase3Info.TotalUnits, filesNumber) return nil }) } @@ -181,8 +184,8 @@ func (ts *TransferStateManager) SetTotalSizeAndFilesPhase3(filesNumber, totalSiz // Increase transferred storage and files in phase 3 func (ts *TransferStateManager) IncTransferredSizeAndFilesPhase3(chunkTotalFiles, chunkTotalSizeInBytes int64) error { return ts.TransferState.Action(func(state *TransferState) error { - state.CurrentRepo.Phase3Info.TransferredSizeBytes += chunkTotalSizeInBytes - state.CurrentRepo.Phase3Info.TransferredUnits += chunkTotalFiles + atomicallyAddInt64(&state.CurrentRepo.Phase3Info.TransferredSizeBytes, chunkTotalSizeInBytes) + atomicallyAddInt64(&state.CurrentRepo.Phase3Info.TransferredUnits, chunkTotalFiles) return nil }) } @@ -288,29 +291,21 @@ func (ts *TransferStateManager) GetDiffHandlingRange() (start, end time.Time, er func (ts *TransferStateManager) IncVisitedFolders() error { return ts.action(func(transferRunStatus *TransferRunStatus) error { - transferRunStatus.VisitedFolders++ + atomicallyAddUint64(&transferRunStatus.VisitedFolders, 1, true) return nil }) } func (ts *TransferStateManager) ChangeDelayedFilesCountBy(count uint64, increase bool) error { return ts.TransferRunStatus.action(func(transferRunStatus *TransferRunStatus) error { - if increase { - transferRunStatus.DelayedFiles += count - } else { - transferRunStatus.DelayedFiles -= count - } + atomicallyAddUint64(&transferRunStatus.DelayedFiles, count, increase) return nil }) } -func (ts *TransferStateManager) ChangeTransferFailureCountBy(count uint, increase bool) error { +func (ts *TransferStateManager) ChangeTransferFailureCountBy(count uint64, increase bool) error { return ts.TransferRunStatus.action(func(transferRunStatus *TransferRunStatus) error { - if increase { - transferRunStatus.TransferFailures += count - } else { - transferRunStatus.TransferFailures -= count - } + atomicallyAddUint64(&transferRunStatus.TransferFailures, count, increase) return nil }) } diff --git a/artifactory/commands/transferfiles/state/statemanager_test.go b/artifactory/commands/transferfiles/state/statemanager_test.go index 62db05c9e..2db3f9760 100644 --- a/artifactory/commands/transferfiles/state/statemanager_test.go +++ b/artifactory/commands/transferfiles/state/statemanager_test.go @@ -1,6 +1,7 @@ package state import ( + "sync" "testing" "time" @@ -181,11 +182,11 @@ func TestChangeTransferFailureCountBy(t *testing.T) { // Increase failures count assert.NoError(t, stateManager.ChangeTransferFailureCountBy(2, true)) assert.NoError(t, stateManager.ChangeTransferFailureCountBy(4, true)) - assert.Equal(t, uint(6), stateManager.TransferFailures) + assert.Equal(t, uint64(6), stateManager.TransferFailures) // Decrease failures count assert.NoError(t, stateManager.ChangeTransferFailureCountBy(3, false)) - assert.Equal(t, uint(3), stateManager.TransferFailures) + assert.Equal(t, uint64(3), stateManager.TransferFailures) } func assertReposTransferredSize(t *testing.T, stateManager *TransferStateManager, expectedSize int64, repoKeys ...string) { @@ -310,3 +311,51 @@ func TestGetRunningTimeString(t *testing.T) { }) } } + +func TestStateConcurrency(t *testing.T) { + stateManager, cleanUp := InitStateTest(t) + defer cleanUp() + + // Concurrently increment variables in the state + var wg sync.WaitGroup + for i := 0; i < 1000; i++ { + wg.Add(1) + go func() { + defer wg.Done() + assert.NoError(t, stateManager.IncTransferredSizeAndFilesPhase1(1, 1)) + assert.NoError(t, stateManager.IncTransferredSizeAndFilesPhase2(1, 1)) + assert.NoError(t, stateManager.IncTransferredSizeAndFilesPhase3(1, 1)) + assert.NoError(t, stateManager.IncVisitedFolders()) + assert.NoError(t, stateManager.ChangeDelayedFilesCountBy(1, true)) + assert.NoError(t, stateManager.ChangeTransferFailureCountBy(1, true)) + }() + } + wg.Wait() + + // Assert 1000 in all values + assert.Equal(t, 1000, int(stateManager.CurrentRepo.Phase1Info.TransferredSizeBytes)) + assert.Equal(t, 1000, int(stateManager.CurrentRepo.Phase1Info.TransferredUnits)) + assert.Equal(t, 1000, int(stateManager.CurrentRepo.Phase2Info.TransferredSizeBytes)) + assert.Equal(t, 1000, int(stateManager.CurrentRepo.Phase2Info.TransferredUnits)) + assert.Equal(t, 1000, int(stateManager.CurrentRepo.Phase3Info.TransferredSizeBytes)) + assert.Equal(t, 1000, int(stateManager.CurrentRepo.Phase3Info.TransferredUnits)) + assert.Equal(t, 1000, int(stateManager.OverallTransfer.TransferredSizeBytes)) + assert.Equal(t, 1000, int(stateManager.VisitedFolders)) + assert.Equal(t, 1000, int(stateManager.DelayedFiles)) + assert.Equal(t, 1000, int(stateManager.TransferFailures)) + + // Concurrently decrement delayed artifacts and transfer failures + for i := 0; i < 500; i++ { + wg.Add(1) + go func() { + defer wg.Done() + assert.NoError(t, stateManager.ChangeDelayedFilesCountBy(1, false)) + assert.NoError(t, stateManager.ChangeTransferFailureCountBy(1, false)) + }() + } + wg.Wait() + + // Assert 500 in delayed artifacts and transfer failures + assert.Equal(t, 500, int(stateManager.DelayedFiles)) + assert.Equal(t, 500, int(stateManager.TransferFailures)) +} diff --git a/artifactory/commands/transferfiles/state/utils.go b/artifactory/commands/transferfiles/state/utils.go index 6f87a5711..8ccc129e4 100644 --- a/artifactory/commands/transferfiles/state/utils.go +++ b/artifactory/commands/transferfiles/state/utils.go @@ -5,6 +5,7 @@ import ( "path/filepath" "strconv" "strings" + "sync/atomic" "time" "github.com/jfrog/build-info-go/utils" @@ -121,3 +122,22 @@ func GetOldTransferDirectoryStructureError() error { } return errorutils.CheckErrorf(oldTransferDirectoryStructureErrorFormat, transferDir) } + +// Atomically add to an int64 variable. +// addr - Pointer to int64 variable +// delta - The change to do +func atomicallyAddInt64(addr *int64, delta int64) { + atomic.AddInt64(addr, delta) +} + +// Atomically add to an uint64 variable. +// addr - Pointer to uint64 variable +// delta - The change to do +// increase - True to increment, false to decrement +func atomicallyAddUint64(addr *uint64, delta uint64, increase bool) { + if increase { + atomic.AddUint64(addr, delta) + } else { + atomic.AddUint64(addr, ^(delta - 1)) + } +} diff --git a/artifactory/commands/transferfiles/status.go b/artifactory/commands/transferfiles/status.go index b5880e32e..7bbd3dcd7 100644 --- a/artifactory/commands/transferfiles/status.go +++ b/artifactory/commands/transferfiles/status.go @@ -80,7 +80,7 @@ func addOverallStatus(stateManager *state.TransferStateManager, output *strings. addString(output, "🧵", "Working threads", strconv.Itoa(stateManager.WorkingThreads), 2) addString(output, "⚡", "Transfer speed", stateManager.GetSpeedString(), 2) addString(output, "⌛", "Estimated time remaining", stateManager.GetEstimatedRemainingTimeString(), 1) - failureTxt := strconv.FormatUint(uint64(stateManager.TransferFailures), 10) + failureTxt := strconv.FormatUint(stateManager.TransferFailures, 10) if stateManager.TransferFailures > 0 { failureTxt += " (" + progressbar.RetryFailureContentNote + ")" } diff --git a/artifactory/commands/transferfiles/transfer.go b/artifactory/commands/transferfiles/transfer.go index 03ecd3848..8053ea707 100644 --- a/artifactory/commands/transferfiles/transfer.go +++ b/artifactory/commands/transferfiles/transfer.go @@ -236,7 +236,7 @@ func (tdc *TransferFilesCommand) initStateManager(allSourceLocalRepos, sourceBui if e != nil { return e } - tdc.stateManager.TransferFailures = uint(numberInitialErrors) + tdc.stateManager.TransferFailures = uint64(numberInitialErrors) numberInitialDelays, e := getDelayedFilesCount(allSourceLocalRepos) if e != nil { @@ -516,6 +516,10 @@ func (tdc *TransferFilesCommand) handleStop(srcUpService *srcUserPluginService) // The stopSignal channel is closed return } + // Before interrupting the process, do a thread dump + if err := doThreadDump(); err != nil { + log.Error(err) + } tdc.cancelFunc() if newPhase != nil { newPhase.StopGracefully() @@ -809,3 +813,14 @@ func parseErrorsFromLogFiles(logPaths []string) (allErrors FilesErrors, err erro func assertSupportedTransferDirStructure() error { return state.VerifyTransferRunStatusVersion() } + +func doThreadDump() error { + log.Info("Starting thread dumping...") + threadDump, err := coreutils.NewProfiler().ThreadDump() + if err != nil { + return err + } + log.Info(threadDump) + log.Info("Thread dump ended successfully") + return nil +} diff --git a/artifactory/utils/weblogin.go b/artifactory/utils/weblogin.go index 085c8e99c..6f630c7d9 100644 --- a/artifactory/utils/weblogin.go +++ b/artifactory/utils/weblogin.go @@ -32,7 +32,7 @@ func DoWebLogin(serverDetails *config.ServerDetails) (token auth.CommonTokenPara "Make sure the details you entered are correct and that Artifactory meets the version requirement.")) return } - log.Info("After logging in via your web browser, please enter the code if prompted: "+uuidStr[len(uuidStr)-4:]) + log.Info("After logging in via your web browser, please enter the code if prompted: " + uuidStr[len(uuidStr)-4:]) if err = browser.OpenURL(clientUtils.AddTrailingSlashIfNeeded(serverDetails.Url) + "ui/login?jfClientSession=" + uuidStr + "&jfClientName=JFrog-CLI&jfClientCode=1"); err != nil { return } diff --git a/go.mod b/go.mod index bcffd9e04..4df4432eb 100644 --- a/go.mod +++ b/go.mod @@ -13,10 +13,10 @@ require ( github.com/gocarina/gocsv v0.0.0-20230616125104-99d496ca653d github.com/google/uuid v1.3.1 github.com/gookit/color v1.5.4 - github.com/jfrog/build-info-go v1.9.16 + github.com/jfrog/build-info-go v1.9.17 github.com/jfrog/gofrog v1.3.2 github.com/jfrog/jfrog-apps-config v1.0.1 - github.com/jfrog/jfrog-client-go v1.35.0 + github.com/jfrog/jfrog-client-go v1.35.1 github.com/magiconair/properties v1.8.7 github.com/manifoldco/promptui v0.9.0 github.com/owenrumney/go-sarif/v2 v2.3.0 diff --git a/go.sum b/go.sum index c425c52a8..8745e5d31 100644 --- a/go.sum +++ b/go.sum @@ -196,14 +196,14 @@ github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOl github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= github.com/jedib0t/go-pretty/v6 v6.4.0 h1:YlI/2zYDrweA4MThiYMKtGRfT+2qZOO65ulej8GTcVI= github.com/jedib0t/go-pretty/v6 v6.4.0/go.mod h1:MgmISkTWDSFu0xOqiZ0mKNntMQ2mDgOcwOkwBEkMDJI= -github.com/jfrog/build-info-go v1.9.16 h1:zMNxUXve4CZndhlbaEGwgayWPY8CRyPzSobvTKYRPcg= -github.com/jfrog/build-info-go v1.9.16/go.mod h1:/5VZXH2Ud0IK3cOFwPykjwPOaEcHhzzbjnRiou+YKpM= +github.com/jfrog/build-info-go v1.9.17 h1:sUA6V3P8i+awYlK7dkwm4l6IuLE2W964F5Pb18x95HA= +github.com/jfrog/build-info-go v1.9.17/go.mod h1:/5VZXH2Ud0IK3cOFwPykjwPOaEcHhzzbjnRiou+YKpM= github.com/jfrog/gofrog v1.3.2 h1:TktKP+PdZdxjkYZxWWIq4DkTGSYtr9Slsy+egZpEhUY= github.com/jfrog/gofrog v1.3.2/go.mod h1:AQo5Fq0G9nDEF6icH7MYQK0iohR4HuEAXl8jaxRuT6Q= github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYLipdsOFMY= github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w= -github.com/jfrog/jfrog-client-go v1.35.0 h1:VTyrR/jFlWInRdGYswa2fwFc1Thsh6eGMnHuqhDVh7s= -github.com/jfrog/jfrog-client-go v1.35.0/go.mod h1:cG0vdKXbyfupKgSYmwA5FZPco6zSfyJi3eEYOxuqm/k= +github.com/jfrog/jfrog-client-go v1.35.1 h1:T5HXyRykSCx4cZ//9VCnvHs5rvsPCuNsTU4+CRvkVWk= +github.com/jfrog/jfrog-client-go v1.35.1/go.mod h1:XF/PXHuKILfB1sN3n903yWaWJKJX5VYofDGvO9cJ3+g= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk= github.com/kevinburke/ssh_config v1.2.0 h1:x584FjTGwHzMwvHx18PXxbBVzfnxogHaAReU4gf13a4= diff --git a/utils/coreutils/profiler.go b/utils/coreutils/profiler.go new file mode 100644 index 000000000..94626e6a4 --- /dev/null +++ b/utils/coreutils/profiler.go @@ -0,0 +1,90 @@ +package coreutils + +import ( + "errors" + "fmt" + "os" + "runtime/pprof" + "time" + + "github.com/jfrog/jfrog-client-go/utils/errorutils" + "github.com/jfrog/jfrog-client-go/utils/io/fileutils" +) + +const ( + // The default interval between 2 profiling actions + defaultInterval = time.Second + // The default number of profilings + defaultRepetitions = 3 +) + +// This struct wraps the "pprof" profiler in Go. +// It is used for thread dumping. +type Profiler struct { + interval time.Duration + repetitions uint +} + +type ProfilerOption func(*Profiler) + +func NewProfiler(opts ...ProfilerOption) *Profiler { + profiler := &Profiler{ + interval: defaultInterval, + repetitions: defaultRepetitions, + } + for _, opt := range opts { + opt(profiler) + } + return profiler +} + +func WithInterval(interval time.Duration) ProfilerOption { + return func(p *Profiler) { + p.interval = interval + } +} + +func WithRepetitions(repetitions uint) ProfilerOption { + return func(p *Profiler) { + p.repetitions = repetitions + } +} + +func (p *Profiler) ThreadDump() (output string, err error) { + var outputFilePath string + if outputFilePath, err = p.threadDumpToFile(); err != nil { + return + } + defer func() { + err = errors.Join(err, errorutils.CheckError(os.Remove(outputFilePath))) + }() + return p.convertFileToString(outputFilePath) +} + +func (p *Profiler) threadDumpToFile() (outputFilePath string, err error) { + outputFile, err := fileutils.CreateTempFile() + if err != nil { + return + } + defer func() { + err = errors.Join(err, errorutils.CheckError(outputFile.Close())) + }() + + for i := 0; i < int(p.repetitions); i++ { + fmt.Fprintf(outputFile, "========== Thread dump #%d ==========\n", i) + prof := pprof.Lookup("goroutine") + if err = errorutils.CheckError(prof.WriteTo(outputFile, 1)); err != nil { + return + } + time.Sleep(p.interval) + } + return outputFile.Name(), nil +} + +func (p *Profiler) convertFileToString(outputFilePath string) (string, error) { + if outputBytes, err := os.ReadFile(outputFilePath); err != nil { + return "", errorutils.CheckError(err) + } else { + return string(outputBytes), nil + } +} diff --git a/utils/coreutils/profiler_test.go b/utils/coreutils/profiler_test.go new file mode 100644 index 000000000..88bb5e75b --- /dev/null +++ b/utils/coreutils/profiler_test.go @@ -0,0 +1,58 @@ +package coreutils + +import ( + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestThreadDump(t *testing.T) { + // Create default profiler + profiler := NewProfiler() + + // Start a thread that sleeps + go func() { + dummyZzzz() + }() + + // Run thread dump + output, err := profiler.ThreadDump() + assert.NoError(t, err) + + // Check results + assert.Contains(t, output, "Thread dump #0") + assert.Contains(t, output, "Thread dump #1") + assert.Contains(t, output, "Thread dump #2") + assert.Contains(t, output, "dummyZzzz") +} + +func TestThreadInterval(t *testing.T) { + // Create profiler with 10 repetitions and 10ms intervals + var expectedRepetitions uint = 10 + var expectedInterval = 10 * time.Millisecond + profiler := NewProfiler(WithInterval(expectedInterval), WithRepetitions(expectedRepetitions)) + + // Check that the required values are set + assert.Equal(t, profiler.interval, expectedInterval) + assert.Equal(t, profiler.repetitions, expectedRepetitions) + + // start measure the time + start := time.Now() + + // Run thread dump + output, err := profiler.ThreadDump() + assert.NoError(t, err) + + // Ensure duration less than 1 second + assert.WithinDuration(t, start, time.Now(), time.Second) + + // Ensure 10 repetitions + assert.Contains(t, output, "Thread dump #"+strconv.FormatUint(uint64(expectedRepetitions)-1, 10)) +} + +// In the thread dump test, we look for this function name in the output to ensure that functions executed in goroutines are recorded. +func dummyZzzz() { + time.Sleep(2 * time.Second) +} diff --git a/xray/commands/audit/sca/java/deptreemanager.go b/xray/commands/audit/sca/java/deptreemanager.go index c1f8412bd..e6add9fd2 100644 --- a/xray/commands/audit/sca/java/deptreemanager.go +++ b/xray/commands/audit/sca/java/deptreemanager.go @@ -60,8 +60,8 @@ type depTreeNode struct { // getGraphFromDepTree reads the output files of the gradle-dep-tree and maven-dep-tree plugins and returns them as a slice of GraphNodes. // It takes the output of the plugin's run (which is a byte representation of a list of paths of the output files, separated by newlines) as input. -func getGraphFromDepTree(depTreeOutput []byte) (depsGraph []*xrayUtils.GraphNode, uniqueDeps []string, err error) { - modules, err := parseDepTreeFiles(depTreeOutput) +func getGraphFromDepTree(outputFilePaths string) (depsGraph []*xrayUtils.GraphNode, uniqueDeps []string, err error) { + modules, err := parseDepTreeFiles(outputFilePaths) if err != nil { return } @@ -97,8 +97,8 @@ func populateDependencyTree(currNode *xrayUtils.GraphNode, currNodeId string, mo } } -func parseDepTreeFiles(jsonFilePaths []byte) ([]*moduleDepTree, error) { - outputFilePaths := strings.Split(strings.TrimSpace(string(jsonFilePaths)), "\n") +func parseDepTreeFiles(jsonFilePaths string) ([]*moduleDepTree, error) { + outputFilePaths := strings.Split(strings.TrimSpace(jsonFilePaths), "\n") var modules []*moduleDepTree for _, path := range outputFilePaths { results, err := parseDepTreeFile(path) diff --git a/xray/commands/audit/sca/java/gradle.go b/xray/commands/audit/sca/java/gradle.go index 3f50e010b..013a2f3a6 100644 --- a/xray/commands/audit/sca/java/gradle.go +++ b/xray/commands/audit/sca/java/gradle.go @@ -66,11 +66,11 @@ func buildGradleDependencyTree(params *DepTreeParams) (dependencyTree []*xrayUti return } -func (gdt *gradleDepTreeManager) runGradleDepTree() ([]byte, error) { +func (gdt *gradleDepTreeManager) runGradleDepTree() (string, error) { // Create the script file in the repository depTreeDir, err := gdt.createDepTreeScriptAndGetDir() if err != nil { - return nil, err + return "", err } defer func() { err = errors.Join(err, fileutils.RemoveTempDir(depTreeDir)) @@ -79,11 +79,15 @@ func (gdt *gradleDepTreeManager) runGradleDepTree() ([]byte, error) { if gdt.useWrapper { gdt.useWrapper, err = isGradleWrapperExist() if err != nil { - return nil, err + return "", err } } - return gdt.execGradleDepTree(depTreeDir) + output, err := gdt.execGradleDepTree(depTreeDir) + if err != nil { + return "", err + } + return string(output), nil } func (gdt *gradleDepTreeManager) createDepTreeScriptAndGetDir() (tmpDir string, err error) { diff --git a/xray/commands/audit/sca/java/mvn.go b/xray/commands/audit/sca/java/mvn.go index dbf3e689c..6757de5ba 100644 --- a/xray/commands/audit/sca/java/mvn.go +++ b/xray/commands/audit/sca/java/mvn.go @@ -4,6 +4,7 @@ import ( _ "embed" "errors" "fmt" + "github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/utils/log" @@ -23,6 +24,8 @@ const ( settingsXmlFile = "settings.xml" ) +var mavenConfigPath = filepath.Join(".mvn", "maven.config") + type MavenDepTreeCmd string const ( @@ -57,19 +60,19 @@ func NewMavenDepTreeManager(params *DepTreeParams, cmdName MavenDepTreeCmd, isDe func buildMavenDependencyTree(params *DepTreeParams, isDepTreeInstalled bool) (dependencyTree []*xrayUtils.GraphNode, uniqueDeps []string, err error) { manager := NewMavenDepTreeManager(params, Tree, isDepTreeInstalled) - outputFileContent, err := manager.RunMavenDepTree() + outputFilePaths, err := manager.RunMavenDepTree() if err != nil { return } - dependencyTree, uniqueDeps, err = getGraphFromDepTree(outputFileContent) + dependencyTree, uniqueDeps, err = getGraphFromDepTree(outputFilePaths) return } -func (mdt *MavenDepTreeManager) RunMavenDepTree() ([]byte, error) { +func (mdt *MavenDepTreeManager) RunMavenDepTree() (string, error) { // Create a temp directory for all the files that are required for the maven-dep-tree run depTreeExecDir, err := fileutils.CreateTempDir() if err != nil { - return nil, err + return "", err } defer func() { err = errors.Join(err, fileutils.RemoveTempDir(depTreeExecDir)) @@ -78,13 +81,18 @@ func (mdt *MavenDepTreeManager) RunMavenDepTree() ([]byte, error) { // Create a settings.xml file that sets the dependency resolution from the given server and repository if mdt.depsRepo != "" { if err = mdt.createSettingsXmlWithConfiguredArtifactory(depTreeExecDir); err != nil { - return nil, err + return "", err } } if err = mdt.installMavenDepTreePlugin(depTreeExecDir); err != nil { - return nil, err + return "", err + } + + depTreeOutput, err := mdt.execMavenDepTree(depTreeExecDir) + if err != nil { + return "", err } - return mdt.execMavenDepTree(depTreeExecDir) + return depTreeOutput, nil } func (mdt *MavenDepTreeManager) installMavenDepTreePlugin(depTreeExecDir string) error { @@ -104,44 +112,80 @@ func GetMavenPluginInstallationGoals(pluginPath string) []string { return []string{"org.apache.maven.plugins:maven-install-plugin:3.1.1:install-file", "-Dfile=" + pluginPath, "-B"} } -func (mdt *MavenDepTreeManager) execMavenDepTree(depTreeExecDir string) ([]byte, error) { +func (mdt *MavenDepTreeManager) execMavenDepTree(depTreeExecDir string) (string, error) { if mdt.cmdName == Tree { return mdt.runTreeCmd(depTreeExecDir) } return mdt.runProjectsCmd() } -func (mdt *MavenDepTreeManager) runTreeCmd(depTreeExecDir string) ([]byte, error) { +func (mdt *MavenDepTreeManager) runTreeCmd(depTreeExecDir string) (string, error) { mavenDepTreePath := filepath.Join(depTreeExecDir, mavenDepTreeOutputFile) goals := []string{"com.jfrog:maven-dep-tree:" + mavenDepTreeVersion + ":" + string(Tree), "-DdepsTreeOutputFile=" + mavenDepTreePath, "-B"} if _, err := mdt.RunMvnCmd(goals); err != nil { - return nil, err + return "", err } mavenDepTreeOutput, err := os.ReadFile(mavenDepTreePath) - err = errorutils.CheckError(err) - return mavenDepTreeOutput, err + if err != nil { + return "", errorutils.CheckError(err) + } + return string(mavenDepTreeOutput), nil } -func (mdt *MavenDepTreeManager) runProjectsCmd() ([]byte, error) { +func (mdt *MavenDepTreeManager) runProjectsCmd() (string, error) { goals := []string{"com.jfrog:maven-dep-tree:" + mavenDepTreeVersion + ":" + string(Projects), "-q"} - return mdt.RunMvnCmd(goals) + output, err := mdt.RunMvnCmd(goals) + if err != nil { + return "", err + } + return string(output), nil } -func (mdt *MavenDepTreeManager) RunMvnCmd(goals []string) ([]byte, error) { +func (mdt *MavenDepTreeManager) RunMvnCmd(goals []string) (cmdOutput []byte, err error) { + restoreMavenConfig, err := removeMavenConfig() + if err != nil { + return + } + + defer func() { + if restoreMavenConfig != nil { + err = errors.Join(err, restoreMavenConfig()) + } + }() + if mdt.settingsXmlPath != "" { goals = append(goals, "-s", mdt.settingsXmlPath) } //#nosec G204 - cmdOutput, err := exec.Command("mvn", goals...).CombinedOutput() + cmdOutput, err = exec.Command("mvn", goals...).CombinedOutput() if err != nil { if len(cmdOutput) > 0 { log.Info(string(cmdOutput)) } err = fmt.Errorf("failed running command 'mvn %s': %s", strings.Join(goals, " "), err.Error()) } - return cmdOutput, err + return +} + +func removeMavenConfig() (func() error, error) { + mavenConfigExists, err := fileutils.IsFileExists(mavenConfigPath, false) + if err != nil { + return nil, err + } + if !mavenConfigExists { + return nil, nil + } + restoreMavenConfig, err := utils.BackupFile(mavenConfigPath, "maven.config.bkp") + if err != nil { + return nil, err + } + err = os.Remove(mavenConfigPath) + if err != nil { + err = errorutils.CheckErrorf("failed to remove %s while building the maven dependencies tree. Error received:\n%s", mavenConfigPath, err.Error()) + } + return restoreMavenConfig, err } // Creates a new settings.xml file configured with the provided server and repository from the current MavenDepTreeManager instance. diff --git a/xray/commands/audit/sca/java/mvn_test.go b/xray/commands/audit/sca/java/mvn_test.go index 9f326ffd3..364edd407 100644 --- a/xray/commands/audit/sca/java/mvn_test.go +++ b/xray/commands/audit/sca/java/mvn_test.go @@ -3,6 +3,8 @@ package java import ( "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-cli-core/v2/xray/commands/audit/sca" + "github.com/jfrog/jfrog-client-go/utils/io/fileutils" + "github.com/jfrog/jfrog-client-go/utils/tests" "github.com/stretchr/testify/assert" "os" "path/filepath" @@ -224,6 +226,33 @@ func TestRunProjectsCmd(t *testing.T) { mvnDepTreeManager := NewMavenDepTreeManager(&DepTreeParams{}, Projects, false) output, err := mvnDepTreeManager.RunMavenDepTree() assert.NoError(t, err) - pomPathOccurrences := strings.Count(string(output), "pomPath") + pomPathOccurrences := strings.Count(output, "pomPath") assert.Equal(t, 4, pomPathOccurrences) } + +func TestRemoveMavenConfig(t *testing.T) { + tmpDir := t.TempDir() + currentDir, err := os.Getwd() + assert.NoError(t, err) + restoreDir := tests.ChangeDirWithCallback(t, currentDir, tmpDir) + defer restoreDir() + + // No maven.config exists + restoreFunc, err := removeMavenConfig() + assert.Nil(t, restoreFunc) + assert.Nil(t, err) + + // Create maven.config + err = fileutils.CreateDirIfNotExist(".mvn") + assert.NoError(t, err) + file, err := os.Create(mavenConfigPath) + assert.NoError(t, err) + err = file.Close() + assert.NoError(t, err) + restoreFunc, err = removeMavenConfig() + assert.NoError(t, err) + assert.NoFileExists(t, mavenConfigPath) + err = restoreFunc() + assert.NoError(t, err) + assert.FileExists(t, mavenConfigPath) +}