Skip to content

Commit

Permalink
pull upstream
Browse files Browse the repository at this point in the history
  • Loading branch information
RobiNino committed Jul 29, 2024
2 parents 6580fa6 + a0d079c commit 478dd99
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 70 deletions.
16 changes: 0 additions & 16 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,6 @@ jobs:
- name: ${{ matrix.suite }} tests
run: go test -v github.com/jfrog/jfrog-client-go/tests --timeout 0 --test.${{ matrix.suite }} --rt.url=${{ secrets.PLATFORM_URL }}/artifactory --ds.url=${{ secrets.PLATFORM_URL }}/distribution --xr.url=${{ secrets.PLATFORM_URL }}/xray --xsc.url=${{ secrets.PLATFORM_URL }}/xsc --access.url=${{ secrets.PLATFORM_URL }}/access --rt.user=${{ secrets.PLATFORM_USER }} --rt.password=${{ secrets.PLATFORM_PASSWORD }} --access.token=${{ secrets.PLATFORM_ADMIN_TOKEN }} --ci.runId=${{ runner.os }}-${{ matrix.suite }}

JFrog-Client-Go-Pipelines-Tests:
needs: Pretest
name: pipelines ubuntu-latest
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}

- name: Setup Go with cache
uses: jfrog/.github/actions/install-go-with-cache@main

- name: pipelines tests
run: go test -v github.com/jfrog/jfrog-client-go/tests --timeout 0 --test.pipelines --rt.url=${{ secrets.PLATFORM_URL }}/artifactory --pipe.url=${{ secrets.PLATFORM_URL }}/pipelines --rt.user=${{ secrets.PLATFORM_USER }} --rt.password=${{ secrets.PLATFORM_PASSWORD }} --pipe.accessToken=${{ secrets.PLATFORM_ADMIN_TOKEN }} --pipe.vcsToken=${{ secrets.CLI_PIPE_VCS_TOKEN }} --pipe.vcsRepo=${{ secrets.CLI_PIPE_VCS_REPO }} --pipe.vcsBranch=${{ secrets.CLI_PIPE_VCS_BRANCH }} --ci.runId=${{ runner.os }}_pipe

JFrog-Client-Go-Repositories-Tests:
needs: Pretest
name: repositories ubuntu-latest
Expand Down
26 changes: 14 additions & 12 deletions artifactory/services/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package services

import (
"errors"
"fmt"
"github.com/jfrog/build-info-go/entities"
biutils "github.com/jfrog/build-info-go/utils"
ioutils "github.com/jfrog/gofrog/io"
Expand Down Expand Up @@ -359,7 +360,7 @@ func addCreateDirsTasks(directoriesDataKeys []string, alreadyCreatedDirs map[str
sort.Sort(sort.Reverse(sort.StringSlice(directoriesDataKeys)))
for index, v := range directoriesDataKeys {
// In order to avoid duplication we need to check the path wasn't already created by the previous action.
if v != "." && // For some files the returned path can be the root path, ".", in that case we doing need to create any directory.
if v != "." && // For some files the returned path can be the root path, ".", in that case we don't need to create any directory.
(index == 0 || !utils.IsSubPath(directoriesDataKeys, index, "/")) { // directoriesDataKeys store all the path which might needed to be created, that's include duplicated paths.
// By sorting the directoriesDataKeys we can assure that the longest path was created and therefore no need to create all it's sub paths.

Expand Down Expand Up @@ -426,7 +427,7 @@ func (ds *DownloadService) downloadFile(downloadFileDetails *httpclient.Download
ds.Progress.IncrementGeneralProgress()
}
httpClientsDetails := ds.GetArtifactoryDetails().CreateHttpClientDetails()
bulkDownload := downloadParams.SplitCount == 0 || downloadParams.MinSplitSize < 0 || downloadParams.MinSplitSize*1000 > downloadFileDetails.Size
bulkDownload := downloadParams.SplitCount == 0 || downloadParams.MinSplitSizeKb < 0 || downloadParams.MinSplitSizeKb*1000 > downloadFileDetails.Size
if !bulkDownload {
acceptRange, err := ds.isFileAcceptRange(downloadFileDetails)
if err != nil {
Expand Down Expand Up @@ -522,7 +523,7 @@ func createLocalSymlink(localPath, localFileName, symlinkArtifact string, symlin
if errorutils.CheckError(err) != nil {
return err
}
log.Debug(logMsgPrefix + "Creating symlink file.")
log.Debug(fmt.Sprintf("%sCreated symlink: %q -> %q", logMsgPrefix, localSymlinkPath, symlinkArtifact))
return nil
}

Expand Down Expand Up @@ -553,7 +554,6 @@ func (ds *DownloadService) createFileHandlerFunc(downloadParams DownloadParams,
if err != nil {
return err
}
log.Info(logMsgPrefix+"Downloading", downloadData.Dependency.GetItemRelativePath())
if ds.DryRun {
successCounters[threadId]++
return nil
Expand All @@ -563,17 +563,19 @@ func (ds *DownloadService) createFileHandlerFunc(downloadParams DownloadParams,
return err
}
localPath, localFileName := fileutils.GetLocalPathAndFile(downloadData.Dependency.Name, downloadData.Dependency.Path, target, downloadData.Flat, placeholdersUsed)
localFullPath := filepath.Join(localPath, localFileName)
if downloadData.Dependency.Type == string(utils.Folder) {
return createDir(localPath, localFileName, logMsgPrefix)
return createDir(localFullPath, logMsgPrefix)
}
if err = removeIfSymlink(filepath.Join(localPath, localFileName)); err != nil {
if err = removeIfSymlink(localFullPath); err != nil {
return err
}
if downloadParams.IsSymlink() {
if isSymlink, err := ds.createSymlinkIfNeeded(ds.GetArtifactoryDetails().GetUrl(), localPath, localFileName, logMsgPrefix, downloadData, successCounters, threadId, downloadParams); isSymlink {
return err
}
}
log.Info(fmt.Sprintf("%sDownloading %q to %q", logMsgPrefix, downloadData.Dependency.GetItemRelativePath(), localFullPath))
if err = ds.downloadFileIfNeeded(downloadPath, localPath, localFileName, logMsgPrefix, downloadData, downloadParams); err != nil {
log.Error(logMsgPrefix, "Received an error: "+err.Error())
return err
Expand All @@ -586,12 +588,13 @@ func (ds *DownloadService) createFileHandlerFunc(downloadParams DownloadParams,
}

func (ds *DownloadService) downloadFileIfNeeded(downloadPath, localPath, localFileName, logMsgPrefix string, downloadData DownloadData, downloadParams DownloadParams) error {
isEqual, err := fileutils.IsEqualToLocalFile(filepath.Join(localPath, localFileName), downloadData.Dependency.Actual_Md5, downloadData.Dependency.Actual_Sha1)
localFilePath := filepath.Join(localPath, localFileName)
isEqual, err := fileutils.IsEqualToLocalFile(localFilePath, downloadData.Dependency.Actual_Md5, downloadData.Dependency.Actual_Sha1)
if err != nil {
return err
}
if isEqual {
log.Debug(logMsgPrefix + "File already exists locally.")
log.Debug(logMsgPrefix+"File already exists locally:", localFilePath)
if ds.Progress != nil {
ds.Progress.IncrementGeneralProgress()
}
Expand All @@ -604,8 +607,7 @@ func (ds *DownloadService) downloadFileIfNeeded(downloadPath, localPath, localFi
return ds.downloadFile(downloadFileDetails, logMsgPrefix, downloadParams)
}

func createDir(localPath, localFileName, logMsgPrefix string) error {
folderPath := filepath.Join(localPath, localFileName)
func createDir(folderPath, logMsgPrefix string) error {
if err := fileutils.CreateDirIfNotExist(folderPath); err != nil {
return err
}
Expand Down Expand Up @@ -642,7 +644,7 @@ type DownloadParams struct {
Flat bool
Explode bool
BypassArchiveInspection bool
MinSplitSize int64
MinSplitSizeKb int64
SplitCount int
PublicGpgKey string
SkipChecksum bool
Expand Down Expand Up @@ -684,5 +686,5 @@ func (ds *DownloadParams) GetPublicGpgKey() string {
}

func NewDownloadParams() DownloadParams {
return DownloadParams{CommonParams: &utils.CommonParams{}, MinSplitSize: 5120, SplitCount: 3}
return DownloadParams{CommonParams: &utils.CommonParams{}, MinSplitSizeKb: 5120, SplitCount: 3}
}
27 changes: 16 additions & 11 deletions artifactory/services/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ const (
defaultUploadMinSplit = utils.SizeMiB * 200
// The default maximum number of parts that can be concurrently uploaded per file during a multipart upload
defaultUploadSplitCount = 5
// Minimal file size to show progress bar
minFileSizeForProgressInKb = 250 * utils.SizeKib
)

type UploadService struct {
Expand Down Expand Up @@ -92,7 +94,7 @@ func (us *UploadService) UploadFiles(uploadParams ...UploadParams) (summary *uti
errorsQueue := clientutils.NewErrorsQueue(1)
if us.saveSummary {
us.resultsManager, err = newResultManager()
if err != nil {
if err != nil || us.resultsManager == nil {
return nil, err
}
defer func() {
Expand Down Expand Up @@ -528,13 +530,13 @@ func (us *UploadService) uploadFileFromReader(getReaderFunc func() (io.Reader, e
var resp *http.Response
var body []byte
var checksumDeployed = false
var e error
var err error
httpClientsDetails := us.ArtDetails.CreateHttpClientDetails()
if !us.DryRun {
if us.shouldTryChecksumDeploy(details.Size, uploadParams) {
resp, body, e = us.doChecksumDeploy(details, targetUrlWithProps, httpClientsDetails, us.client)
if e != nil {
return false, e
resp, body, err = us.doChecksumDeploy(details, targetUrlWithProps, httpClientsDetails, us.client)
if err != nil {
return false, err
}
checksumDeployed = isSuccessfulUploadStatusCode(resp.StatusCode)
}
Expand Down Expand Up @@ -568,9 +570,9 @@ func (us *UploadService) uploadFileFromReader(getReaderFunc func() (io.Reader, e
},
}

e = retryExecutor.Execute()
if e != nil {
return false, e
err = retryExecutor.Execute()
if err != nil {
return false, err
}
}
}
Expand Down Expand Up @@ -647,7 +649,8 @@ func (us *UploadService) doUploadFromReader(fileReader io.Reader, targetUrlWithP
if us.Progress != nil {
progressReader := us.Progress.NewProgressReader(details.Size, "Uploading", targetUrlWithProps)
reader = progressReader.ActionWithProgress(fileReader)
defer us.Progress.RemoveProgress(progressReader.GetId())
progressId := progressReader.GetId()
defer us.Progress.RemoveProgress(progressId)
} else {
reader = fileReader
}
Expand Down Expand Up @@ -969,10 +972,12 @@ func (us *UploadService) addFileToZip(artifact *clientutils.Artifact, progressPr
err = errors.Join(err, errorutils.CheckError(file.Close()))
}
}()
if us.Progress != nil {
// Show progress bar only for files larger than 250Kb to avoid polluting the terminal with endless progress bars.
if us.Progress != nil && info.Size() > minFileSizeForProgressInKb {
progressReader := us.Progress.NewProgressReader(info.Size(), progressPrefix, localPath)
reader = progressReader.ActionWithProgress(file)
defer us.Progress.RemoveProgress(progressReader.GetId())
progressId := progressReader.GetId()
defer us.Progress.RemoveProgress(progressId)
} else {
reader = file
}
Expand Down
22 changes: 10 additions & 12 deletions artifactory/services/utils/multipartupload.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ const (
aborted completionStatus = "ABORTED"

// API constants
uploadsApi = "/api/v1/uploads/"
routeToHeader = "X-JFrog-Route-To"
artifactoryNodeId = "X-Artifactory-Node-Id"
uploadsApi = "/api/v1/uploads/"

// Sizes and limits constants
MaxMultipartUploadFileSize = SizeTiB * 5
Expand Down Expand Up @@ -144,7 +142,8 @@ func (mu *MultipartUpload) UploadFileConcurrently(localPath, targetPath string,
var progressReader ioutils.Progress
if progress != nil {
progressReader = progress.NewProgressReader(fileSize, "Multipart upload", targetPath)
defer progress.RemoveProgress(progressReader.GetId())
progressId := progressReader.GetId()
defer progress.RemoveProgress(progressId)
}

defer func() {
Expand Down Expand Up @@ -304,18 +303,17 @@ type urlPartResponse struct {
}

func (mu *MultipartUpload) completeAndPollForStatus(logMsgPrefix string, completionAttemptsLeft uint, sha1 string, multipartUploadClient *httputils.HttpClientDetails, progressReader ioutils.Progress) (err error) {
nodeId, err := mu.completeMultipartUpload(logMsgPrefix, sha1, multipartUploadClient)
err = mu.completeMultipartUpload(logMsgPrefix, sha1, multipartUploadClient)
if err != nil {
return
}

err = mu.pollCompletionStatus(logMsgPrefix, completionAttemptsLeft, sha1, nodeId, multipartUploadClient, progressReader)
err = mu.pollCompletionStatus(logMsgPrefix, completionAttemptsLeft, sha1, multipartUploadClient, progressReader)
return
}

func (mu *MultipartUpload) pollCompletionStatus(logMsgPrefix string, completionAttemptsLeft uint, sha1, nodeId string, multipartUploadClient *httputils.HttpClientDetails, progressReader ioutils.Progress) error {
func (mu *MultipartUpload) pollCompletionStatus(logMsgPrefix string, completionAttemptsLeft uint, sha1 string, multipartUploadClient *httputils.HttpClientDetails, progressReader ioutils.Progress) error {
multipartUploadClientWithNodeId := multipartUploadClient.Clone()
multipartUploadClientWithNodeId.Headers = map[string]string{routeToHeader: nodeId}

lastMergeLog := time.Now()
pollingExecutor := &utils.RetryExecutor{
Expand Down Expand Up @@ -359,22 +357,22 @@ func (mu *MultipartUpload) pollCompletionStatus(logMsgPrefix string, completionA
return pollingExecutor.Execute()
}

func (mu *MultipartUpload) completeMultipartUpload(logMsgPrefix, sha1 string, multipartUploadClient *httputils.HttpClientDetails) (string, error) {
func (mu *MultipartUpload) completeMultipartUpload(logMsgPrefix, sha1 string, multipartUploadClient *httputils.HttpClientDetails) error {
url := fmt.Sprintf("%s%scomplete?sha1=%s", mu.artifactoryUrl, uploadsApi, sha1)
resp, body, err := mu.client.GetHttpClient().SendPost(url, []byte{}, *multipartUploadClient, logMsgPrefix)
if err != nil {
return "", err
return err
}
log.Debug("Artifactory response:", string(body), resp.Status)
return resp.Header.Get(artifactoryNodeId), errorutils.CheckResponseStatusWithBody(resp, body, http.StatusAccepted)
return errorutils.CheckResponseStatusWithBody(resp, body, http.StatusAccepted)
}

func (mu *MultipartUpload) status(logMsgPrefix string, multipartUploadClientWithNodeId *httputils.HttpClientDetails) (status statusResponse, err error) {
url := fmt.Sprintf("%s%sstatus", mu.artifactoryUrl, uploadsApi)
resp, body, err := mu.client.GetHttpClient().SendPost(url, []byte{}, *multipartUploadClientWithNodeId, logMsgPrefix)
// If the Artifactory node returns a "Service unavailable" error (status 503), attempt to retry the upload completion process on a different node.
if resp != nil && resp.StatusCode == http.StatusServiceUnavailable {
unavailableNodeErr := fmt.Sprintf(logMsgPrefix + fmt.Sprintf("The Artifactory node ID '%s' is unavailable.", multipartUploadClientWithNodeId.Headers[routeToHeader]))
unavailableNodeErr := fmt.Sprintf(logMsgPrefix + "Artifactory is unavailable.")
return statusResponse{Status: retryableError, Error: unavailableNodeErr}, nil
}
if err != nil {
Expand Down
17 changes: 4 additions & 13 deletions artifactory/services/utils/multipartupload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,6 @@ func TestCompleteMultipartUpload(t *testing.T) {
assert.Equal(t, "/api/v1/uploads/complete", r.URL.Path)
assert.Equal(t, fmt.Sprintf("sha1=%s", sha1), r.URL.RawQuery)

// Add the "X-Artifactory-Node-Id" header to the response
w.Header().Add(artifactoryNodeId, nodeId)

// Send response 202 Accepted
w.WriteHeader(http.StatusAccepted)
})
Expand All @@ -198,9 +195,8 @@ func TestCompleteMultipartUpload(t *testing.T) {
defer cleanUp()

// Execute completeMultipartUpload
actualNodeId, err := multipartUpload.completeMultipartUpload("", sha1, &httputils.HttpClientDetails{})
err := multipartUpload.completeMultipartUpload("", sha1, &httputils.HttpClientDetails{})
assert.NoError(t, err)
assert.Equal(t, nodeId, actualNodeId)
}

func TestStatus(t *testing.T) {
Expand All @@ -211,9 +207,6 @@ func TestStatus(t *testing.T) {
// Check URL
assert.Equal(t, "/api/v1/uploads/status", r.URL.Path)

// Check "X-JFrog-Route-To" header
assert.Equal(t, nodeId, r.Header.Get(routeToHeader))

// Send response 200 OK
w.WriteHeader(http.StatusOK)
response, err := json.Marshal(statusResponse{Status: finished, Progress: utils.Pointer(100)})
Expand All @@ -227,8 +220,7 @@ func TestStatus(t *testing.T) {
defer cleanUp()

// Execute status
clientDetails := &httputils.HttpClientDetails{Headers: map[string]string{routeToHeader: nodeId}}
status, err := multipartUpload.status("", clientDetails)
status, err := multipartUpload.status("", &httputils.HttpClientDetails{})
assert.NoError(t, err)
assert.Equal(t, statusResponse{Status: finished, Progress: utils.Pointer(100)}, status)
}
Expand All @@ -252,10 +244,9 @@ func TestStatusServiceUnavailable(t *testing.T) {
defer cleanUp()

// Execute status
clientDetails := &httputils.HttpClientDetails{Headers: map[string]string{routeToHeader: nodeId}}
status, err := multipartUpload.status("", clientDetails)
status, err := multipartUpload.status("", &httputils.HttpClientDetails{})
assert.NoError(t, err)
assert.Equal(t, statusResponse{Status: retryableError, Error: "The Artifactory node ID 'nodeId' is unavailable."}, status)
assert.Equal(t, statusResponse{Status: retryableError, Error: "Artifactory is unavailable."}, status)
}

func TestAbort(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/golang-jwt/jwt/v4 v4.5.0
github.com/gookit/color v1.5.4
github.com/jfrog/archiver/v3 v3.6.1
github.com/jfrog/build-info-go v1.9.29
github.com/jfrog/build-info-go v1.9.30
github.com/jfrog/gofrog v1.7.4
github.com/minio/sha256-simd v1.0.1
github.com/stretchr/testify v1.9.0
Expand Down
10 changes: 6 additions & 4 deletions http/httpclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,8 @@ func (jc *HttpClient) doUploadFile(localPath, url string, httpClientsDetails htt
if file != nil && progress != nil {
progressReader := progress.NewProgressReader(size, "Uploading", url)
reader = progressReader.ActionWithProgress(reqContent)
defer progress.RemoveProgress(progressReader.GetId())
progressId := progressReader.GetId()
defer progress.RemoveProgress(progressId)
} else {
reader = reqContent
}
Expand Down Expand Up @@ -468,9 +469,10 @@ func saveToFile(downloadFileDetails *DownloadFileDetails, resp *http.Response, p

var reader io.Reader
if progress != nil {
readerProgress := progress.NewProgressReader(resp.ContentLength, "", downloadFileDetails.RelativePath)
reader = readerProgress.ActionWithProgress(resp.Body)
defer progress.RemoveProgress(readerProgress.GetId())
progressReader := progress.NewProgressReader(resp.ContentLength, "", downloadFileDetails.RelativePath)
reader = progressReader.ActionWithProgress(resp.Body)
progressId := progressReader.GetId()
defer progress.RemoveProgress(progressId)
} else {
reader = resp.Body
}
Expand Down
2 changes: 1 addition & 1 deletion utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
const (
Development = "development"
Agent = "jfrog-client-go"
Version = "1.42.0"
Version = "1.43.0"
)

type MinVersionProduct string
Expand Down

0 comments on commit 478dd99

Please sign in to comment.