diff --git a/utils/paths.go b/utils/paths.go index 24efd8581..d9e441bb8 100644 --- a/utils/paths.go +++ b/utils/paths.go @@ -301,25 +301,24 @@ func ToURI(path string) string { return u.String() } -func GetReleasesRemoteDetails(artifact, downloadPath, remoteRepo string, remoteServerDetails *config.ServerDetails) (server *config.ServerDetails, fullRemotePath string, err error) { +// For a configured remote repository, the resource is expected under '/artifactory/' (a remote that proxies https://releases.jfrog.io). +// fallbackRemotePath is '/' directly (a remote that proxies https://releases.jfrog.io/artifactory, or a repository holding the resources in its root). It is empty when there is no fallback. +func GetReleasesRemoteDetails(artifact, downloadPath, remoteRepo string, remoteServerDetails *config.ServerDetails) (server *config.ServerDetails, remotePath, fallbackRemotePath string, err error) { if remoteServerDetails != nil && remoteRepo != "" { // Config profile server and repo details are provided server = remoteServerDetails - fullRemotePath = path.Join(remoteRepo, downloadPath) - return - } - // Try to get releases remote details from environment variable - server, remoteRepo, err = dependencies.GetRemoteDetails(coreutils.ReleasesRemoteEnv) - if err != nil { - return + } else { + // Try to get releases remote details from environment variable + if server, remoteRepo, err = dependencies.GetRemoteDetails(coreutils.ReleasesRemoteEnv); err != nil { + return + } } - if remoteRepo != "" { - fullRemotePath = path.Join(remoteRepo, "artifactory", downloadPath) - return + if remoteRepo == "" { + log.Debug(fmt.Sprintf("'"+coreutils.ReleasesRemoteEnv+"' environment variable is not configured. The '%s' will be downloaded directly from releases.jfrog.io if needed.", artifact)) + // If not configured to download through a remote repository in Artifactory, download from releases.jfrog.io. + return &config.ServerDetails{ArtifactoryUrl: coreutils.JfrogReleasesUrl}, downloadPath, "", nil } - log.Debug(fmt.Sprintf("'"+coreutils.ReleasesRemoteEnv+"' environment variable is not configured. The '%s' will be downloaded directly from releases.jfrog.io if needed.", artifact)) - // If not configured to download through a remote repository in Artifactory, download from releases.jfrog.io. - return &config.ServerDetails{ArtifactoryUrl: coreutils.JfrogReleasesUrl}, downloadPath, nil + return server, path.Join(remoteRepo, "artifactory", downloadPath), path.Join(remoteRepo, downloadPath), nil } // RepositoryScansListLink is a deep link into Xray's Scans List for one artifact. diff --git a/utils/paths_test.go b/utils/paths_test.go index 95e568977..0fff2956b 100644 --- a/utils/paths_test.go +++ b/utils/paths_test.go @@ -5,8 +5,17 @@ import ( "path/filepath" "testing" + commonCommands "github.com/jfrog/jfrog-cli-core/v2/common/commands" + "github.com/jfrog/jfrog-cli-core/v2/utils/config" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-client-go/utils/log" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + downloadPath = "xray-scan-lib/xray-scan-lib-1.6.0-linux-amd64.tar.gz" + envServerId = "envServer" ) func TestIsPathExcludedFromRoot(t *testing.T) { @@ -163,3 +172,50 @@ func TestGetFullPathsWorkingDirs(t *testing.T) { }) } } + +func TestGetReleasesRemoteDetails(t *testing.T) { + profileServer := &config.ServerDetails{ArtifactoryUrl: "https://profile.example.com/artifactory/"} + envServer := &config.ServerDetails{ServerId: envServerId, Url: "https://env.example.com/", ArtifactoryUrl: "https://env.example.com/artifactory/"} + t.Setenv(coreutils.HomeDir, t.TempDir()) + require.NoError(t, commonCommands.NewConfigCommand(commonCommands.AddOrEdit, envServerId).SetDetails(envServer).SetInteractive(false).SetEncPassword(false).Run()) + testCases := []struct { + name string + remoteRepo string + remoteServer *config.ServerDetails + envValue string + expectedServerUrl string + expectedRemotePath string + expectedFallbackRemotePath string + }{ + { + name: "Config profile remote repository", + remoteRepo: "releases-remote", + remoteServer: profileServer, + expectedServerUrl: profileServer.ArtifactoryUrl, + expectedRemotePath: "releases-remote/artifactory/" + downloadPath, + expectedFallbackRemotePath: "releases-remote/" + downloadPath, + }, + { + name: "Env var remote repository", + envValue: envServerId + "/releases-remote", + expectedServerUrl: envServer.ArtifactoryUrl, + expectedRemotePath: "releases-remote/artifactory/" + downloadPath, + expectedFallbackRemotePath: "releases-remote/" + downloadPath, + }, + { + name: "Not configured, download from releases.jfrog.io", + expectedServerUrl: coreutils.JfrogReleasesUrl, + expectedRemotePath: downloadPath, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Setenv(coreutils.ReleasesRemoteEnv, testCase.envValue) + server, remotePath, fallbackRemotePath, err := GetReleasesRemoteDetails("Resource", downloadPath, testCase.remoteRepo, testCase.remoteServer) + require.NoError(t, err) + assert.Equal(t, testCase.expectedServerUrl, server.ArtifactoryUrl) + assert.Equal(t, testCase.expectedRemotePath, remotePath) + assert.Equal(t, testCase.expectedFallbackRemotePath, fallbackRemotePath) + }) + } +} diff --git a/utils/utils.go b/utils/utils.go index c73547612..9e3bebedf 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "net/http" "os" "path/filepath" "regexp" @@ -456,7 +457,7 @@ type RemoteResource struct { // Get the details of the resource from the JPD / Releases remote it is downloaded from. func GetRemoteResource(resourceName, downloadPath, remoteRepo string, remoteServerDetails *config.ServerDetails) (*RemoteResource, error) { - rtDetails, remotePath, err := GetReleasesRemoteDetails(resourceName, downloadPath, remoteRepo, remoteServerDetails) + rtDetails, remotePath, fallbackRemotePath, err := GetReleasesRemoteDetails(resourceName, downloadPath, remoteRepo, remoteServerDetails) if err != nil { return nil, err } @@ -466,12 +467,25 @@ func GetRemoteResource(resourceName, downloadPath, remoteRepo string, remoteServ } downloadUrl := rtDetails.ArtifactoryUrl + remotePath remoteFileDetails, _, err := client.GetRemoteFileDetails(downloadUrl, &httpClientDetails) - if err != nil { + if err != nil && fallbackRemotePath != "" && isLayoutMismatchError(err) { + fallbackUrl := rtDetails.ArtifactoryUrl + fallbackRemotePath + var fallbackErr error + if remoteFileDetails, _, fallbackErr = client.GetRemoteFileDetails(fallbackUrl, &httpClientDetails); fallbackErr != nil { + return nil, fmt.Errorf("couldn't get remote file details for %s: %w; and for %s: %w", downloadUrl, err, fallbackUrl, fallbackErr) + } + remotePath = fallbackRemotePath + } else if err != nil { return nil, fmt.Errorf("couldn't get remote file details for %s: %s", downloadUrl, err.Error()) } return &RemoteResource{name: resourceName, serverDetails: rtDetails, path: remotePath, details: remoteFileDetails}, nil } +// A wrong repository layout returns 404, or 403 when the user's permissions cover only specific folders of the repository. +func isLayoutMismatchError(err error) bool { + var httpErr *errorutils.HttpResponseError + return errors.As(err, &httpErr) && (httpErr.StatusCode == http.StatusNotFound || httpErr.StatusCode == http.StatusForbidden) +} + // Returns true when targetDir already holds the latest version of the resource. func (rr *RemoteResource) IsCached(targetDir string) (bool, error) { return isArtifactChecksumsMatch(rr.details, filepath.Join(targetDir, dependencies.ChecksumFileName), true) diff --git a/utils/utils_test.go b/utils/utils_test.go index 2fb55fa76..37d4246d7 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -11,8 +11,11 @@ import ( "net/http" "net/http/httptest" "os" + "path" "path/filepath" "slices" + "strings" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -299,6 +302,97 @@ func TestRemoteResourceDownloadEncrypted(t *testing.T) { } } +func TestGetRemoteResourceFallsBackToPathWithoutArtifactory(t *testing.T) { + const ( + repoPrefix = "/artifactory/" + resourcesRemoteRepo + "/" + withArtifactory = repoPrefix + "artifactory/" + encryptedArchivePath + withoutArtifactory = repoPrefix + encryptedArchivePath + ) + testCases := []struct { + name string + statusByPath map[string]int + expectedPath string + expectedRequests []string + expectedErrUrls []string + expectedErrCodes []string + }{ + { + name: "Found under artifactory", + statusByPath: map[string]int{withArtifactory: http.StatusOK}, + expectedPath: path.Join(resourcesRemoteRepo, "artifactory", encryptedArchivePath), + expectedRequests: []string{withArtifactory}, + }, + { + name: "Not found under artifactory, found in root", + statusByPath: map[string]int{withoutArtifactory: http.StatusOK}, + expectedPath: path.Join(resourcesRemoteRepo, encryptedArchivePath), + expectedRequests: []string{withArtifactory, withoutArtifactory}, + }, + { + name: "Not found in both", + statusByPath: map[string]int{}, + expectedRequests: []string{withArtifactory, withoutArtifactory}, + expectedErrUrls: []string{withArtifactory, withoutArtifactory}, + expectedErrCodes: []string{"404", "404"}, + }, + { + name: "Forbidden under artifactory, found in root", + statusByPath: map[string]int{withArtifactory: http.StatusForbidden, withoutArtifactory: http.StatusOK}, + expectedPath: path.Join(resourcesRemoteRepo, encryptedArchivePath), + expectedRequests: []string{withArtifactory, withoutArtifactory}, + }, + { + name: "Unauthorized under artifactory is not retried", + statusByPath: map[string]int{withArtifactory: http.StatusUnauthorized, withoutArtifactory: http.StatusOK}, + expectedRequests: []string{withArtifactory}, + expectedErrUrls: []string{withArtifactory}, + expectedErrCodes: []string{"401"}, + }, + { + name: "Both errors are reported when the fallback fails differently", + statusByPath: map[string]int{withoutArtifactory: http.StatusForbidden}, + expectedRequests: []string{withArtifactory, withoutArtifactory}, + expectedErrUrls: []string{withArtifactory, withoutArtifactory}, + expectedErrCodes: []string{"404", "403"}, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + var mutex sync.Mutex + var requests []string + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mutex.Lock() + requests = append(requests, r.URL.Path) + mutex.Unlock() + status, exists := testCase.statusByPath[r.URL.Path] + if !exists { + status = http.StatusNotFound + } + w.Header().Set("X-Checksum-Sha256", "checksum") + w.WriteHeader(status) + })) + defer mockServer.Close() + + resource, err := GetRemoteResource("Resource", encryptedArchivePath, resourcesRemoteRepo, &config.ServerDetails{ArtifactoryUrl: mockServer.URL + "/artifactory/"}) + + assert.Equal(t, testCase.expectedRequests, requests) + if len(testCase.expectedErrCodes) > 0 { + require.Error(t, err) + for _, expectedCode := range testCase.expectedErrCodes { + assert.Contains(t, err.Error(), expectedCode) + } + for _, expectedUrl := range testCase.expectedErrUrls { + assert.Contains(t, err.Error(), mockServer.URL+expectedUrl) + } + assert.Equal(t, len(testCase.expectedErrUrls), strings.Count(err.Error(), mockServer.URL)) + return + } + require.NoError(t, err) + assert.Equal(t, testCase.expectedPath, resource.path) + }) + } +} + func createZip(t *testing.T, fileName, content string) []byte { var buffer bytes.Buffer zipWriter := zip.NewWriter(&buffer)