Support gitlab subgroup and subsubgroups etc when creating PRs

cmd/clusters-service/pkg/git/git.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"path/filepath"
 	"strings"
 	"time"
@@ -15,7 +14,6 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/spf13/viper"
 	go_git "gopkg.in/src-d/go-git.v4"
-	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport/http"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -33,7 +31,6 @@ var DefaultBackoff = wait.Backoff{
 
 type Provider interface {
 	WriteFilesToBranchAndCreatePullRequest(ctx context.Context, req WriteFilesToBranchAndCreatePullRequestRequest) (*WriteFilesToBranchAndCreatePullRequestResponse, error)
-	CloneRepoToTempDir(req CloneRepoToTempDirRequest) (*CloneRepoToTempDirResponse, error)
 	GetRepository(ctx context.Context, gp GitProvider, url string) (gitprovider.OrgRepository, error)
 	GetTreeList(ctx context.Context, gp GitProvider, repoUrl string, sha string, path string, recursive bool) ([]*gitprovider.TreeEntry, error)
 	ListPullRequests(ctx context.Context, gp GitProvider, url string) ([]gitprovider.PullRequest, error)
@@ -72,17 +69,6 @@ type WriteFilesToBranchAndCreatePullRequestResponse struct {
 	WebURL string
 }
 
-type CloneRepoToTempDirRequest struct {
-	GitProvider   GitProvider
-	RepositoryURL string
-	BaseBranch    string
-	ParentDir     string
-}
-
-type CloneRepoToTempDirResponse struct {
-	Repo *GitRepo
-}
-
 // WriteFilesToBranchAndCreatePullRequest writes a set of provided files
 // to a new branch and creates a new pull request for that branch.
 // It returns the URL of the pull request.
@@ -92,7 +78,7 @@ func (s *GitProviderService) WriteFilesToBranchAndCreatePullRequest(ctx context.
 
 	repoURL, err := GetGitProviderUrl(req.RepositoryURL)
 	if err != nil {
-		return nil, fmt.Errorf("unable to get git porivder url: %w", err)
+		return nil, fmt.Errorf("unable to get git provider url: %w", err)
 	}
 
 	repo, err := s.GetRepository(ctx, req.GitProvider, repoURL)
@@ -148,47 +134,6 @@ func (s *GitProviderService) WriteFilesToBranchAndCreatePullRequest(ctx context.
 	}, nil
 }
 
-func (s *GitProviderService) CloneRepoToTempDir(req CloneRepoToTempDirRequest) (*CloneRepoToTempDirResponse, error) {
-	s.log.Info("Creating a temp directory...")
-	gitDir, err := os.MkdirTemp(req.ParentDir, "git-")
-	if err != nil {
-		return nil, err
-	}
-	s.log.Info("Temp directory created.", "dir", gitDir)
-
-	s.log.Info("Cloning the Git repository...", "repository", req.RepositoryURL, "dir", gitDir)
-
-	repo, err := go_git.PlainClone(gitDir, false, &go_git.CloneOptions{
-		URL: req.RepositoryURL,
-		Auth: &http.BasicAuth{
-			Username: "abc123",
-			Password: req.GitProvider.Token,
-		},
-		ReferenceName: plumbing.NewBranchReferenceName(req.BaseBranch),
-
-		SingleBranch: true,
-		Tags:         go_git.NoTags,
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	s.log.Info("Cloned repository", "repository", req.RepositoryURL)
-
-	gitRepo := &GitRepo{
-		WorktreeDir: gitDir,
-		Repo:        repo,
-		Auth: &http.BasicAuth{
-			Username: "abc123",
-			Password: req.GitProvider.Token,
-		},
-	}
-
-	return &CloneRepoToTempDirResponse{
-		Repo: gitRepo,
-	}, nil
-}
-
 type GitRepo struct {
 	WorktreeDir string
 	Repo        *go_git.Repository
@@ -207,6 +152,7 @@ func (s *GitProviderService) GetRepository(ctx context.Context, gp GitProvider,
 	}
 
 	ref.Domain = addSchemeToDomain(ref.Domain)
+	ref = WithCombinedSubGroups(*ref)
 
 	var repo gitprovider.OrgRepository
 	err = retry.OnError(DefaultBackoff,
@@ -227,6 +173,15 @@ func (s *GitProviderService) GetRepository(ctx context.Context, gp GitProvider,
 	return repo, nil
 }
 
+// WithCombinedSubGroups combines the subgroups into the organization field of the reference
+// This is to work around a bug in the go-git-providers library where it doesn't handle subgroups correctly.
+// https://github.com/fluxcd/go-git-providers/issues/183
+func WithCombinedSubGroups(ref gitprovider.OrgRepositoryRef) *gitprovider.OrgRepositoryRef {
+	ref.Organization = strings.Join(append([]string{ref.Organization}, ref.SubOrganizations...), "/")
+	ref.SubOrganizations = nil
+	return &ref
+}
+
 // GetTreeList retrieves list of tree files from gitprovider given the sha/branch
 func (s *GitProviderService) GetTreeList(ctx context.Context, gp GitProvider, repoUrl string, sha string, path string, recursive bool) ([]*gitprovider.TreeEntry, error) {
 	repo, err := s.GetRepository(ctx, gp, repoUrl)

cmd/clusters-service/pkg/git/git_test.go

@@ -183,26 +183,52 @@ func TestCreatePullRequestInGitLab(t *testing.T) {
 	gitlabHost := fmt.Sprintf("https://%s", os.Getenv("GIT_PROVIDER_HOSTNAME"))
 	client, err := gitlab.NewClient(os.Getenv("GITLAB_TOKEN"), gitlab.WithBaseURL(gitlabHost))
 	require.NoError(t, err)
-	// Create a repository using a name that doesn't exist already
-	repoName := fmt.Sprintf("%s-%03d", TestRepositoryNamePrefix, rand.Intn(1000))
-	repos, _, err := client.Projects.ListProjects(&gitlab.ListProjectsOptions{
-		Owned: gitlab.Bool(true),
-	})
+
+	repoName := TestRepositoryNamePrefix + "-group-test"
+
+	// Create a group using a name that doesn't exist already
+	groupName := fmt.Sprintf("%s-%03d", TestRepositoryNamePrefix, rand.Intn(1000))
+	groups, _, err := client.Groups.ListGroups(&gitlab.ListGroupsOptions{})
 	assert.NoError(t, err)
-	for findGitLabRepo(repos, repoName) != nil {
-		repoName = fmt.Sprintf("%s-%03d", TestRepositoryNamePrefix, rand.Intn(1000))
+	for findGitLabGroup(groups, groupName) != nil {
+		groupName = fmt.Sprintf("%s-%03d", TestRepositoryNamePrefix, rand.Intn(1000))
 	}
+
+	parentGroup, _, err := client.Groups.CreateGroup(&gitlab.CreateGroupOptions{
+		Path:       gitlab.String(groupName),
+		Name:       gitlab.String(groupName),
+		Visibility: gitlab.Visibility(gitlab.PrivateVisibility),
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		_, err = client.Groups.DeleteGroup(parentGroup.ID)
+		require.NoError(t, err)
+	})
+
+	fooGroup, _, err := client.Groups.CreateGroup(&gitlab.CreateGroupOptions{
+		Path:       gitlab.String("foo"),
+		Name:       gitlab.String("foo group"),
+		ParentID:   gitlab.Int(parentGroup.ID),
+		Visibility: gitlab.Visibility(gitlab.PrivateVisibility),
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		_, err = client.Groups.DeleteGroup(fooGroup.ID)
+		require.NoError(t, err)
+	})
+
 	repo, _, err := client.Projects.CreateProject(&gitlab.CreateProjectOptions{
 		Name:                 gitlab.String(repoName),
 		MergeRequestsEnabled: gitlab.Bool(true),
 		Visibility:           gitlab.Visibility(gitlab.PrivateVisibility),
 		InitializeWithReadme: gitlab.Bool(true),
+		NamespaceID:          gitlab.Int(fooGroup.ID),
 	})
 	require.NoError(t, err)
-	defer func() {
+	t.Cleanup(func() {
 		_, err = client.Projects.DeleteProject(repo.ID)
 		require.NoError(t, err)
-	}()
+	})
 
 	s := git.NewGitProviderService(logr.Discard())
 	path := "management/cluster-01.yaml"
@@ -384,6 +410,29 @@ func TestGetGitProviderUrl(t *testing.T) {
 	assert.Equal(t, expected, repoURL)
 }
 
+func TestCombineSubGroups(t *testing.T) {
+	ref, err := gitprovider.ParseOrgRepositoryURL("https://gitlab.com/org/sub1/sub2/sub3/repo")
+	assert.NoError(t, err)
+	newRef := git.WithCombinedSubGroups(*ref)
+
+	// Where are the still the same
+	assert.Equal(t, "repo", ref.RepositoryName)
+	assert.Equal(t, "repo", newRef.RepositoryName)
+
+	assert.Equal(t, "https://gitlab.com/org/sub1/sub2/sub3/repo.git", ref.GetCloneURL("https"))
+	assert.Equal(t, "https://gitlab.com/org/sub1/sub2/sub3/repo.git", newRef.GetCloneURL("https"))
+
+	assert.Equal(t, "https://gitlab.com/org/sub1/sub2/sub3/repo", ref.String())
+	assert.Equal(t, "https://gitlab.com/org/sub1/sub2/sub3/repo", newRef.String())
+
+	// Where they now differ
+	assert.Equal(t, "org", ref.Organization)
+	assert.Equal(t, "org/sub1/sub2/sub3", newRef.Organization)
+
+	assert.Equal(t, []string{"sub1", "sub2", "sub3"}, ref.SubOrganizations)
+	assert.Equal(t, []string(nil), newRef.SubOrganizations)
+}
+
 func findGitHubRepo(repos []*github.Repository, name string) *github.Repository {
 	if name == "" {
 		return nil
@@ -407,3 +456,15 @@ func findGitLabRepo(repos []*gitlab.Project, name string) *gitlab.Project {
 	}
 	return nil
 }
+
+func findGitLabGroup(repos []*gitlab.Group, name string) *gitlab.Group {
+	if name == "" {
+		return nil
+	}
+	for _, repo := range repos {
+		if repo.Name == name {
+			return repo
+		}
+	}
+	return nil
+}

cmd/clusters-service/pkg/git/gitfakes/git_fake.go

@@ -37,13 +37,6 @@ func (p *FakeGitProvider) WriteFilesToBranchAndCreatePullRequest(ctx context.Con
 	return &git.WriteFilesToBranchAndCreatePullRequestResponse{WebURL: p.url}, nil
 }
 
-func (p *FakeGitProvider) CloneRepoToTempDir(req git.CloneRepoToTempDirRequest) (*git.CloneRepoToTempDirResponse, error) {
-	if p.err != nil {
-		return nil, p.err
-	}
-	return &git.CloneRepoToTempDirResponse{Repo: p.repo}, nil
-}
-
 func (p *FakeGitProvider) GetRepository(ctx context.Context, gp git.GitProvider, url string) (gitprovider.OrgRepository, error) {
 	if p.err != nil {
 		return nil, p.err