fix(appset): update gitlab SCM provider to search on parent folder (#…

…16253) (#21491) * (fix:appset) update gitlab SCM provider to search on parent folder fix #16253 Signed-off-by: Prune <[email protected]> * adding test-case that replicated the new Gitlab API behaviour Signed-off-by: Prune <[email protected]> * add comments to the case Signed-off-by: Prune <[email protected]> --------- Signed-off-by: Prune <[email protected]>
argoproj · Jan 15, 2025 · 37a7231 · 37a7231
1 parent 9a02f9b
commit 37a7231
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 26 deletions.
diff --git a/applicationset/services/scm_provider/gitlab.go b/applicationset/services/scm_provider/gitlab.go
@@ -131,37 +131,30 @@ func (g *GitlabProvider) RepoHasPath(_ context.Context, repo *Repository, path s
 	if err != nil {
 		return false, err
 	}
-	directories := []string{
-		path,
-		pathpkg.Dir(path),
+
+	options := gitlab.ListTreeOptions{
+		Path: gitlab.Ptr(pathpkg.Dir(path)), // search parent folder
+		Ref:  &repo.Branch,
 	}
-	for _, directory := range directories {
-		options := gitlab.ListTreeOptions{
-			Path: &directory,
-			Ref:  &repo.Branch,
+	for {
+		treeNode, resp, err := g.client.Repositories.ListTree(p.ID, &options)
+		if err != nil {
+			return false, err
 		}
-		for {
-			treeNode, resp, err := g.client.Repositories.ListTree(p.ID, &options)
-			if err != nil {
-				return false, err
-			}
-			if path == directory {
-				if resp.TotalItems > 0 {
-					return true, nil
-				}
-			}
-			for i := range treeNode {
-				if treeNode[i].Path == path {
-					return true, nil
-				}
-			}
-			if resp.NextPage == 0 {
-				// no future pages
-				break
+
+		// search for presence of the requested file in the parent folder
+		for i := range treeNode {
+			if treeNode[i].Path == path {
+				return true, nil
 			}
-			options.Page = resp.NextPage
 		}
+		if resp.NextPage == 0 {
+			// no future pages
+			break
+		}
+		options.Page = resp.NextPage
 	}
+
 	return false, nil
 }
 

diff --git a/applicationset/services/scm_provider/gitlab_test.go b/applicationset/services/scm_provider/gitlab_test.go
@@ -1040,6 +1040,12 @@ 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":
+			w.WriteHeader(http.StatusNotFound)
 		case "/api/v4/projects/27084533/repository/branches/foo":
 			w.WriteHeader(http.StatusNotFound)
 		default:
@@ -1194,6 +1200,11 @@ func TestGitlabHasPath(t *testing.T) {
 			path:   "argocd/notathing.yaml",
 			exists: false,
 		},
+		{
+			name:   "send a file path",
+			path:   "argocd/filepath.yaml",
+			exists: false,
+		},
 	}
 
 	for _, c := range cases {