From e2f7d850ce1133d62df5f649f95ca1a63eca9b8d Mon Sep 17 00:00:00 2001 From: Eran Turgeman <81029514+eranturgeman@users.noreply.github.com> Date: Tue, 31 Oct 2023 17:18:42 +0200 Subject: [PATCH 1/3] Enhancements to Artifactory Integration and NPM dependency tree building (#1011) --- artifactory/utils/projectconfig.go | 38 +++++++++++++++++++--- go.mod | 2 +- go.sum | 4 +-- xray/commands/audit/auditparams.go | 5 +++ xray/commands/audit/sca/npm/npm.go | 43 ++++++++++++++++++++++++- xray/commands/audit/sca/npm/npm_test.go | 3 +- xray/commands/audit/scarunner.go | 5 +++ xray/utils/auditbasicparams.go | 10 ++++++ 8 files changed, 100 insertions(+), 10 deletions(-) diff --git a/artifactory/utils/projectconfig.go b/artifactory/utils/projectconfig.go index 4ab09d4be..95072e5a3 100644 --- a/artifactory/utils/projectconfig.go +++ b/artifactory/utils/projectconfig.go @@ -2,16 +2,15 @@ package utils import ( "fmt" - "path/filepath" - "reflect" - - "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" - "github.com/jfrog/jfrog-cli-core/v2/utils/config" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" + xrayutils "github.com/jfrog/jfrog-cli-core/v2/xray/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" "github.com/spf13/viper" + "path/filepath" + "reflect" ) const ( @@ -38,6 +37,11 @@ const ( Terraform ) +// Associates a technology with another of a different type in the structure. +// Docker is not present, as there is no docker-config command and, consequently, no docker.yaml file we need to operate on. +var techType = map[coreutils.Technology]ProjectType{coreutils.Maven: Maven, coreutils.Gradle: Gradle, coreutils.Npm: Npm, coreutils.Yarn: Yarn, coreutils.Go: Go, coreutils.Pip: Pip, + coreutils.Pipenv: Pipenv, coreutils.Poetry: Poetry, coreutils.Nuget: Nuget, coreutils.Dotnet: Dotnet} + var ProjectTypes = []string{ "go", "pip", @@ -187,3 +191,27 @@ func ReadResolutionOnlyConfiguration(confFilePath string) (*RepositoryConfig, er } return GetRepoConfigByPrefix(confFilePath, ProjectConfigResolverPrefix, vConfig) } + +// Verifies the existence of depsRepo. If it doesn't exist, it searches for a configuration file based on the technology type. If found, it assigns depsRepo in the AuditParams. +func SetResolutionRepoIfExists(params xrayutils.AuditParams, tech coreutils.Technology) (err error) { + if params.DepsRepo() != "" { + return + } + configFilePath, exists, err := GetProjectConfFilePath(techType[tech]) + if err != nil { + err = fmt.Errorf("failed while searching for %s.yaml config file: %s", tech.String(), err.Error()) + return + } + if !exists { + log.Debug(fmt.Sprintf("No %s.yaml configuration file was found. Resolving dependencies from %s default registry", tech.String(), tech.String())) + return + } + + repoConfig, err := ReadResolutionOnlyConfiguration(configFilePath) + if err != nil { + err = fmt.Errorf("failed while reading %s.yaml config file: %s", tech.String(), err.Error()) + return + } + params.SetDepsRepo(repoConfig.targetRepo) + return +} diff --git a/go.mod b/go.mod index 7065e6f93..22f60750f 100644 --- a/go.mod +++ b/go.mod @@ -101,4 +101,4 @@ require ( // replace github.com/jfrog/jfrog-client-go => github.com/jfrog/jfrog-client-go v1.28.1-0.20231003120621-90e9d7ea05e9 -// replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.8.9-0.20231019085746-e1b192457664 +replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.8.9-0.20231031143744-13f94ab07bbc diff --git a/go.sum b/go.sum index 369bfedd0..457c753f7 100644 --- a/go.sum +++ b/go.sum @@ -196,8 +196,8 @@ 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.8 h1:HiNzyMSEpsBaduKhmK+CwcpulEeBrTmxutz4oX/oWkg= github.com/jedib0t/go-pretty/v6 v6.4.8/go.mod h1:Ndk3ase2CkQbXLLNf5QDHoYb6J9WtVfmHZu9n8rk2xs= -github.com/jfrog/build-info-go v1.9.14 h1:xVezJ16Vpm/boRBn3lI1THCQmkylm+6R4zYWxOQ0NSM= -github.com/jfrog/build-info-go v1.9.14/go.mod h1:ujJ8XQZMdT2tMkLSMJNyDd1pCY+duwHdjV+9or9FLIg= +github.com/jfrog/build-info-go v1.8.9-0.20231031143744-13f94ab07bbc h1:MFejgCB90z7nA/KP48lF1t04tYuXAAQc53cBaFd9zcw= +github.com/jfrog/build-info-go v1.8.9-0.20231031143744-13f94ab07bbc/go.mod h1:ujJ8XQZMdT2tMkLSMJNyDd1pCY+duwHdjV+9or9FLIg= github.com/jfrog/gofrog v1.3.1 h1:QqAwQXCVReT724uga1AYqG/ZyrNQ6f+iTxmzkb+YFQk= github.com/jfrog/gofrog v1.3.1/go.mod h1:IFMc+V/yf7rA5WZ74CSbXe+Lgf0iApEQLxRZVzKRUR0= github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYLipdsOFMY= diff --git a/xray/commands/audit/auditparams.go b/xray/commands/audit/auditparams.go index 9dc42e3c2..5aa78156a 100644 --- a/xray/commands/audit/auditparams.go +++ b/xray/commands/audit/auditparams.go @@ -82,3 +82,8 @@ func (params *AuditParams) SetThirdPartyApplicabilityScan(includeThirdPartyDeps params.thirdPartyApplicabilityScan = includeThirdPartyDeps return params } + +func (params *AuditParams) SetDepsRepo(depsRepo string) *AuditParams { + params.AuditBasicParams.SetDepsRepo(depsRepo) + return params +} diff --git a/xray/commands/audit/sca/npm/npm.go b/xray/commands/audit/sca/npm/npm.go index 3574a3065..7b0c0cdc9 100644 --- a/xray/commands/audit/sca/npm/npm.go +++ b/xray/commands/audit/sca/npm/npm.go @@ -1,8 +1,11 @@ package npm import ( + "errors" + "fmt" biutils "github.com/jfrog/build-info-go/build/utils" buildinfo "github.com/jfrog/build-info-go/entities" + "github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/npm" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/commands/audit/sca" "github.com/jfrog/jfrog-cli-core/v2/xray/utils" @@ -31,6 +34,17 @@ func BuildDependencyTree(params utils.AuditParams) (dependencyTrees []*xrayUtils treeDepsParam := createTreeDepsParam(params) + restoreNpmrcFunc, err := configNpmResolutionServerIfNeeded(params) + if err != nil { + err = fmt.Errorf("failed while configuring a resolution server") + return + } + defer func() { + if restoreNpmrcFunc != nil { + err = errors.Join(err, restoreNpmrcFunc()) + } + }() + // Calculate npm dependencies dependenciesMap, err := biutils.CalculateDependenciesMap(npmExecutablePath, currentDir, packageInfo.BuildInfoModuleId(), treeDepsParam, log.Logger) if err != nil { @@ -47,6 +61,32 @@ func BuildDependencyTree(params utils.AuditParams) (dependencyTrees []*xrayUtils return } +// Generates a .npmrc file to configure an Artifactory server as the resolver server. +func configNpmResolutionServerIfNeeded(params utils.AuditParams) (restoreNpmrcFunc func() error, err error) { + if params == nil { + err = fmt.Errorf("got empty params upon configuring resolution server") + return + } + serverDetails, err := params.ServerDetails() + if err != nil || serverDetails == nil { + return + } + depsRepo := params.DepsRepo() + if depsRepo == "" { + return + } + + npmCmd := npm.NewNpmCommand("install", false).SetServerDetails(serverDetails) + if err = npmCmd.PreparePrerequisites(depsRepo); err != nil { + return + } + if err = npmCmd.CreateTempNpmrc(); err != nil { + return + } + restoreNpmrcFunc = npmCmd.RestoreNpmrcFunc() + return +} + func createTreeDepsParam(params utils.AuditParams) biutils.NpmTreeDepListParam { if params == nil { return biutils.NpmTreeDepListParam{ @@ -54,7 +94,8 @@ func createTreeDepsParam(params utils.AuditParams) biutils.NpmTreeDepListParam { } } npmTreeDepParam := biutils.NpmTreeDepListParam{ - Args: addIgnoreScriptsFlag(params.Args()), + Args: addIgnoreScriptsFlag(params.Args()), + InstallCommandArgs: params.InstallCommandArgs(), } if npmParams, ok := params.(utils.AuditNpmParams); ok { npmTreeDepParam.IgnoreNodeModules = npmParams.NpmIgnoreNodeModules() diff --git a/xray/commands/audit/sca/npm/npm_test.go b/xray/commands/audit/sca/npm/npm_test.go index 36871d29a..ad1591238 100644 --- a/xray/commands/audit/sca/npm/npm_test.go +++ b/xray/commands/audit/sca/npm/npm_test.go @@ -116,6 +116,7 @@ func TestIgnoreScripts(t *testing.T) { // The package.json file contain a postinstall script running an "exit 1" command. // Without the "--ignore-scripts" flag, the test will fail. - _, _, err := BuildDependencyTree(nil) + params := &utils.AuditBasicParams{} + _, _, err := BuildDependencyTree(params) assert.NoError(t, err) } diff --git a/xray/commands/audit/scarunner.go b/xray/commands/audit/scarunner.go index ae4a25b1a..0cf8cf116 100644 --- a/xray/commands/audit/scarunner.go +++ b/xray/commands/audit/scarunner.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/jfrog/build-info-go/utils/pythonutils" "github.com/jfrog/gofrog/datastructures" + "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-cli-core/v2/xray/commands/audit/sca" _go "github.com/jfrog/jfrog-cli-core/v2/xray/commands/audit/sca/go" @@ -143,6 +144,10 @@ func GetTechDependencyTree(params xrayutils.AuditParams, tech coreutils.Technolo if err != nil { return } + err = utils.SetResolutionRepoIfExists(params, tech) + if err != nil { + return + } var uniqueDeps []string startTime := time.Now() switch tech { diff --git a/xray/utils/auditbasicparams.go b/xray/utils/auditbasicparams.go index e5c739517..997c09006 100644 --- a/xray/utils/auditbasicparams.go +++ b/xray/utils/auditbasicparams.go @@ -23,6 +23,7 @@ type AuditParams interface { Progress() ioUtils.ProgressMgr SetProgress(progress ioUtils.ProgressMgr) Args() []string + InstallCommandArgs() []string SetNpmScope(depType string) *AuditBasicParams OutputFormat() OutputFormat DepsRepo() string @@ -44,6 +45,7 @@ type AuditBasicParams struct { args []string depsRepo string ignoreConfigFile bool + installCommandArgs []string } func (abp *AuditBasicParams) DirectDependencies() []string { @@ -64,6 +66,10 @@ func (abp *AuditBasicParams) SetServerDetails(serverDetails *config.ServerDetail return abp } +func (abp *AuditBasicParams) SetInstallCommandArgs(installCommandArgs []string) *AuditBasicParams { + abp.installCommandArgs = installCommandArgs + return abp +} func (abp *AuditBasicParams) PipRequirementsFile() string { return abp.pipRequirementsFile } @@ -121,6 +127,10 @@ func (abp *AuditBasicParams) Args() []string { return abp.args } +func (abp *AuditBasicParams) InstallCommandArgs() []string { + return abp.installCommandArgs +} + func (abp *AuditBasicParams) SetNpmScope(depType string) *AuditBasicParams { switch depType { case "devOnly": From 57aed941540e430386edf1ecca1a6266368a6991 Mon Sep 17 00:00:00 2001 From: Yahav Itzhak Date: Tue, 31 Oct 2023 18:38:00 +0200 Subject: [PATCH 2/3] Transfer - Interruption may causes data loss (#1010) --- .../transferfiles/delayedartifactshandler.go | 31 +++++++++--------- .../commands/transferfiles/fileserror.go | 32 +++++++------------ artifactory/commands/transferfiles/utils.go | 8 +++++ 3 files changed, 34 insertions(+), 37 deletions(-) diff --git a/artifactory/commands/transferfiles/delayedartifactshandler.go b/artifactory/commands/transferfiles/delayedartifactshandler.go index 8e04b740a..a5a20cfc9 100644 --- a/artifactory/commands/transferfiles/delayedartifactshandler.go +++ b/artifactory/commands/transferfiles/delayedartifactshandler.go @@ -89,19 +89,25 @@ type DelayedArtifactsFile struct { } // Collect all the delayed artifact files that were created up to this point for the repository and transfer their artifacts using handleDelayedArtifactsFiles -func consumeAllDelayFiles(base phaseBase) error { +func consumeAllDelayFiles(base phaseBase) (err error) { filesToConsume, err := getDelayFiles([]string{base.repoKey}) - if err != nil { - return err + if err != nil || len(filesToConsume) == 0 { + return } delayFunctions := getDelayUploadComparisonFunctions(base.repoSummary.PackageType) - if len(filesToConsume) > 0 && len(delayFunctions) > 0 { - log.Info("Starting to handle delayed artifacts uploads...") - if err = handleDelayedArtifactsFiles(filesToConsume, base, delayFunctions[1:]); err == nil { - log.Info("Done handling delayed artifacts uploads.") - } + if len(delayFunctions) == 0 { + return + } + + log.Info("Starting to handle delayed artifacts uploads...") + // Each delay function causes the transfer to skip a specific group of files. + // Within the handleDelayedArtifactsFiles function, we recursively remove the first delay function from the slice to transfer the first set of files every time. + if err = handleDelayedArtifactsFiles(filesToConsume, base, delayFunctions[1:]); err != nil { + return } - return err + + log.Info("Done handling delayed artifacts uploads.") + return deleteAllFiles(filesToConsume) } // Call consumeAllDelayFiles only if there are no failed transferred files for the repository up to this point. @@ -182,13 +188,6 @@ func consumeDelayedArtifactsFiles(pcWrapper *producerConsumerWrapper, filesToCon if err = base.stateManager.ChangeDelayedFilesCountBy(uint64(len(delayedArtifactsFile.DelayedArtifacts)), false); err != nil { log.Warn("Couldn't decrease the delayed files counter", err.Error()) } - - // Remove the file, so it won't be consumed again. - if err = os.Remove(filePath); err != nil { - return errorutils.CheckError(err) - } - - log.Debug("Done handling delayed artifacts file: '" + filePath + "'. Deleting it...") } return nil } diff --git a/artifactory/commands/transferfiles/fileserror.go b/artifactory/commands/transferfiles/fileserror.go index 0a74b5e75..47f0b867a 100644 --- a/artifactory/commands/transferfiles/fileserror.go +++ b/artifactory/commands/transferfiles/fileserror.go @@ -3,13 +3,11 @@ package transferfiles import ( "errors" "fmt" - "os" "strings" "time" "github.com/jfrog/gofrog/parallel" "github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/transferfiles/api" - "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/log" ) @@ -33,7 +31,8 @@ func (e *errorsRetryPhase) run() error { // Does so by creating and uploading by chunks, and polling on status. // Consumed errors files are deleted, new failures are written to new files. func (e *errorsRetryPhase) handlePreviousUploadFailures() error { - if len(e.errorsFilesToHandle) == 0 { + errorsFilesToHandle := e.errorsFilesToHandle + if len(errorsFilesToHandle) == 0 { return nil } log.Info("Starting to handle previous upload failures...") @@ -46,11 +45,13 @@ func (e *errorsRetryPhase) handlePreviousUploadFailures() error { delayAction := func(phase phaseBase, addedDelayFiles []string) error { return consumeAllDelayFiles(phase) } - err := e.transferManager.doTransferWithProducerConsumer(action, delayAction) - if err == nil { - log.Info("Done handling previous upload failures.") + + if err := e.transferManager.doTransferWithProducerConsumer(action, delayAction); err != nil { + return err } - return err + + log.Info("Done handling previous upload failures.") + return deleteAllFiles(errorsFilesToHandle) } func convertUploadStatusToFileRepresentation(statuses []ExtendedFileUploadStatusResponse) (files []api.FileRepresentation) { @@ -83,24 +84,13 @@ func (e *errorsRetryPhase) handleErrorsFile(errFilePath string, pcWrapper *produ } // Upload - shouldStop, err := uploadByChunks(convertUploadStatusToFileRepresentation(failedFiles.Errors), uploadChunkChan, e.phaseBase, delayHelper, errorsChannelMng, pcWrapper) - if err != nil || shouldStop { - return err - } - - // Remove the file, so it won't be consumed again. - err = os.Remove(errFilePath) - if err != nil { - return errorutils.CheckError(err) - } - - log.Debug("Done handling errors file: '", errFilePath, "'. Deleting it...") - return nil + _, err = uploadByChunks(convertUploadStatusToFileRepresentation(failedFiles.Errors), uploadChunkChan, e.phaseBase, delayHelper, errorsChannelMng, pcWrapper) + return err } func (e *errorsRetryPhase) createErrorFilesHandleFunc(pcWrapper *producerConsumerWrapper, uploadChunkChan chan UploadedChunk, delayHelper delayUploadHelper, errorsChannelMng *ErrorsChannelMng) errorFileHandlerFunc { return func() parallel.TaskFunc { - return func(threadId int) error { + return func(int) error { var errList []string var err error for _, errFile := range e.errorsFilesToHandle { diff --git a/artifactory/commands/transferfiles/utils.go b/artifactory/commands/transferfiles/utils.go index 14ebd3380..e41f50d3a 100644 --- a/artifactory/commands/transferfiles/utils.go +++ b/artifactory/commands/transferfiles/utils.go @@ -735,3 +735,11 @@ func getUniqueErrorOrDelayFilePath(dirPath string, getFileNamePrefix func() stri } return } + +func deleteAllFiles(filesToDelete []string) (err error) { + for _, fileToDelete := range filesToDelete { + log.Debug("Deleting:", fileToDelete, "...") + err = errors.Join(err, errorutils.CheckError(os.Remove(fileToDelete))) + } + return +} From 095debb9975ef3d0b25c310df92a6f6918bc404f Mon Sep 17 00:00:00 2001 From: Eran Turgeman <81029514+eranturgeman@users.noreply.github.com> Date: Wed, 1 Nov 2023 11:41:08 +0200 Subject: [PATCH 3/3] added info to an error message (#1014) --- xray/commands/audit/sca/npm/npm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xray/commands/audit/sca/npm/npm.go b/xray/commands/audit/sca/npm/npm.go index 7b0c0cdc9..3e9520ebe 100644 --- a/xray/commands/audit/sca/npm/npm.go +++ b/xray/commands/audit/sca/npm/npm.go @@ -36,7 +36,7 @@ func BuildDependencyTree(params utils.AuditParams) (dependencyTrees []*xrayUtils restoreNpmrcFunc, err := configNpmResolutionServerIfNeeded(params) if err != nil { - err = fmt.Errorf("failed while configuring a resolution server") + err = fmt.Errorf("failed while configuring a resolution server: %s", err.Error()) return } defer func() {