fix(appset): update gitlab SCM provider to search on parent folder (#16253) (#21491) (#21503)

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

fix #16253



* adding test-case that replicated the new Gitlab API behaviour



* add comments to the case



---------

Signed-off-by: Prune <prune@lecentre.net>
Co-authored-by: Prune Sebastien THOMAS <prune@lecentre.net>

Author: gcp-cherry-pick-bot[bot]

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
 }
 

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 {