diff --git a/applicationset/services/scm_provider/gitlab.go b/applicationset/services/scm_provider/gitlab.go index 28aec4d5d6033..e77455296bf9b 100644 --- a/applicationset/services/scm_provider/gitlab.go +++ b/applicationset/services/scm_provider/gitlab.go @@ -2,10 +2,10 @@ package scm_provider import ( "context" + "errors" "fmt" "net/http" "os" - pathpkg "path" "github.com/hashicorp/go-retryablehttp" "github.com/xanzy/go-gitlab" @@ -129,33 +129,31 @@ func (g *GitlabProvider) ListRepos(ctx context.Context, cloneProtocol string) ([ func (g *GitlabProvider) RepoHasPath(_ context.Context, repo *Repository, path string) (bool, error) { p, _, err := g.client.Projects.GetProject(repo.Organization+"/"+repo.Repository, nil) if err != nil { - return false, err + return false, fmt.Errorf("error getting Project Info: %w", err) } - options := gitlab.ListTreeOptions{ - Path: gitlab.Ptr(pathpkg.Dir(path)), // search parent folder - Ref: &repo.Branch, - } - for { - treeNode, resp, err := g.client.Repositories.ListTree(p.ID, &options) - if err != nil { - return false, err - } - - // search for presence of the requested file in the parent folder - for i := range treeNode { - if treeNode[i].Path == path { - return true, nil + // search if the path is a file and exists in the repo + fileOptions := gitlab.GetFileOptions{Ref: &repo.Branch} + _, _, err = g.client.RepositoryFiles.GetFile(p.ID, path, &fileOptions) + if err != nil { + if errors.Is(err, gitlab.ErrNotFound) { + // no file found, check for a directory + options := gitlab.ListTreeOptions{ + Path: &path, + Ref: &repo.Branch, } + _, _, err := g.client.Repositories.ListTree(p.ID, &options) + if err != nil { + if errors.Is(err, gitlab.ErrNotFound) { + return false, nil // no file or directory found + } + return false, err + } + return true, nil // directory found } - if resp.NextPage == 0 { - // no future pages - break - } - options.Page = resp.NextPage + return false, err } - - return false, nil + return true, nil // file found } func (g *GitlabProvider) listBranches(_ context.Context, repo *Repository) ([]gitlab.Branch, error) { diff --git a/applicationset/services/scm_provider/gitlab_test.go b/applicationset/services/scm_provider/gitlab_test.go index b781c655872b2..bf980538d5a8f 100644 --- a/applicationset/services/scm_provider/gitlab_test.go +++ b/applicationset/services/scm_provider/gitlab_test.go @@ -20,6 +20,7 @@ func gitlabMockHandler(t *testing.T) func(http.ResponseWriter, *http.Request) { t.Helper() return func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") + fmt.Println(r.RequestURI) switch r.RequestURI { case "/api/v4": fmt.Println("here1") @@ -1040,11 +1041,31 @@ func gitlabMockHandler(t *testing.T) func(http.ResponseWriter, *http.Request) { if err != nil { t.Fail() } - // Recent versions of the Gitlab API (v17.7+) return 404 not only when a file doesn't exist, but also - // when a path is to a file instead of a directory. Our code should not hit this path, because - // we should only send requests for parent directories. But we leave this handler in place - // to prevent regressions. - case "/api/v4/projects/27084533/repository/tree?path=argocd/filepath.yaml&ref=master": + // Recent versions of the Gitlab API (v17.7+) listTree return 404 not only when a file doesn't exist, but also + // when a path is to a file instead of a directory. Code was refactored to explicitly search for file then + // search for directory, catching 404 errors as "file not found". + case "/api/v4/projects/27084533/repository/files/argocd?ref=master": + w.WriteHeader(http.StatusNotFound) + case "/api/v4/projects/27084533/repository/files/argocd%2Finstall%2Eyaml?ref=master": + _, err := io.WriteString(w, `{"file_name":"install.yaml","file_path":"argocd/install.yaml","size":0,"encoding":"base64","content_sha256":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","ref":"main","blob_id":"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391","commit_id":"6d4c0f9d34534ccc73aa3f3180b25e2aebe630eb","last_commit_id":"b50eb63f9c0e09bfdb070db26fd32c7210291f52","execute_filemode":false,"content":""}`) + if err != nil { + t.Fail() + } + case "/api/v4/projects/27084533/repository/files/notathing?ref=master": + w.WriteHeader(http.StatusNotFound) + case "/api/v4/projects/27084533/repository/tree?path=notathing&ref=master": + w.WriteHeader(http.StatusNotFound) + case "/api/v4/projects/27084533/repository/files/argocd%2Fnotathing%2Eyaml?ref=master": + w.WriteHeader(http.StatusNotFound) + case "/api/v4/projects/27084533/repository/tree?path=argocd%2Fnotathing.yaml&ref=master": + w.WriteHeader(http.StatusNotFound) + case "/api/v4/projects/27084533/repository/files/notathing%2Fnotathing%2Eyaml?ref=master": + w.WriteHeader(http.StatusNotFound) + case "/api/v4/projects/27084533/repository/tree?path=notathing%2Fnotathing.yaml&ref=master": + w.WriteHeader(http.StatusNotFound) + case "/api/v4/projects/27084533/repository/files/notathing%2Fnotathing%2Fnotathing%2Eyaml?ref=master": + w.WriteHeader(http.StatusNotFound) + case "/api/v4/projects/27084533/repository/tree?path=notathing%2Fnotathing%2Fnotathing.yaml&ref=master": w.WriteHeader(http.StatusNotFound) case "/api/v4/projects/27084533/repository/branches/foo": w.WriteHeader(http.StatusNotFound) @@ -1201,8 +1222,13 @@ func TestGitlabHasPath(t *testing.T) { exists: false, }, { - name: "send a file path", - path: "argocd/filepath.yaml", + name: "noexistent file in noexistent directory", + path: "notathing/notathing.yaml", + exists: false, + }, + { + name: "noexistent file in nested noexistent directory", + path: "notathing/notathing/notathing.yaml", exists: false, }, }