Handle UserLogin properly when creating repositories

- GitHub/Gitea/GitLab providers:
  The `UserRepositoriesClient.Create` method's `UserRepositoryRef`
  parameter contains a `UserLogin` field which is not taken into
  account for this call. Instead, the repository is actually created
  in the namespace of the user that is tied to the token used by the
  respective `gitprovider.Client` implementation.

  If a user now passed in a `UserLogin` that is different from the one
  derived from the token, the return value of `Create` would still
  contain the wrong `UserLogin` passed in to `Create` and methods like
  `GetCloneURL` would return the wrong URL.

  With this commit, the returned `UserRepository` now points to the
  correct owner of the repository.
- Stash/Bitbucket Server:
  The Stash provider actually took the field into account and would
  fail to create the repository even when the token was correct. To
  align this provider's behaviour with the others, it now also creates
  the repository under the token user's account and ignores the
  provided `UserRepositoryRef.UserLogin` field.

Signed-off-by: Max Jonas Werner <[email protected]>

Author: Max Jonas Werner

gitea/client_repositories_org.go

@@ -77,7 +77,7 @@ func (c *OrgRepositoriesClient) List(ctx context.Context, ref gitprovider.Organi
 		// apiObj is already validated at ListOrgRepos
 		repos = append(repos, newOrgRepository(c.clientContext, apiObj, gitprovider.OrgRepositoryRef{
 			OrganizationRef: ref,
-			RepositoryName:  *&apiObj.Name,
+			RepositoryName:  apiObj.Name,
 		}))
 	}
 	return repos, nil

gitea/client_repositories_user.go

@@ -19,6 +19,7 @@ package gitea
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"code.gitea.io/sdk/gitea"
 	"github.com/fluxcd/go-git-providers/gitprovider"
@@ -111,6 +112,12 @@ func (c *UserRepositoriesClient) Create(ctx context.Context,
 	if err != nil {
 		return nil, err
 	}
+
+	if apiObj.Owner == nil {
+		return nil, fmt.Errorf("returned API object doesn't have an owner")
+	}
+	ref.UserLogin = apiObj.Owner.UserName
+
 	return newUserRepository(c.clientContext, apiObj, ref), nil
 }
 

gitea/client_repository_commit.go

@@ -24,9 +24,6 @@ import (
 	"github.com/fluxcd/go-git-providers/gitprovider"
 )
 
-var giteaNewFileMode = "100644"
-var giteaBlobTypeFile = "blob"
-
 // CommitClient implements the gitprovider.CommitClient interface.
 var _ gitprovider.CommitClient = &CommitClient{}
 

gitea/integration_repositories_org_test.go

@@ -61,9 +61,9 @@ var _ = Describe("Gitea Provider", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		// Generate a repository name which doesn't exist already
-		testOrgRepoName = fmt.Sprintf("test-repo-%03d", rand.Intn(1000))
+		testOrgRepoName = fmt.Sprintf("test-org-repo-%03d", rand.Intn(1000))
 		for findOrgRepo(repos, testOrgRepoName) != nil {
-			testOrgRepoName = fmt.Sprintf("test-repo-%03d", rand.Intn(1000))
+			testOrgRepoName = fmt.Sprintf("test-org-repo-%03d", rand.Intn(1000))
 		}
 
 		// We know that a repo with this name doesn't exist in the organization, let's verify we get an

gitea/integration_repositories_user_test.go

@@ -70,7 +70,7 @@ var _ = Describe("Gitea Provider", func() {
 		}
 
 		fmt.Print("Creating repository ", testRepoName, "...")
-		repoRef := newUserRepoRef(giteaUser, testRepoName)
+		repoRef := newUserRepoRef(testRepoName)
 
 		// Check that the repository doesn't exist
 		_, err = c.UserRepositories().Get(ctx, repoRef)
@@ -100,8 +100,21 @@ var _ = Describe("Gitea Provider", func() {
 		Expect(getSpec.Equals(postSpec)).To(BeTrue())
 	})
 
+	It("should return correct repo info when creating a repository with wrong UserLogin", func() {
+		repoName := fmt.Sprintf("test-user-repo-creation-%03d", rand.Intn(1000))
+		repoRef := newUserRepoRef(repoName)
+		repoRef.UserLogin = "yadda-yadda-yada"
+
+		repo, err := c.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{})
+
+		Expect(err).To(BeNil())
+		Expect(
+			repo.Repository().GetCloneURL(gitprovider.TransportTypeHTTPS)).
+			To(Equal(fmt.Sprintf("%s/%s/%s.git", giteaBaseUrl, giteaUser, repoName)))
+	})
+
 	It("should error at creation time if the repo already does exist", func() {
-		repoRef := newUserRepoRef(giteaUser, testRepoName)
+		repoRef := newUserRepoRef(testRepoName)
 		_, err := c.UserRepositories().Get(ctx, repoRef)
 		Expect(err).ToNot(HaveOccurred())
 
@@ -110,7 +123,7 @@ var _ = Describe("Gitea Provider", func() {
 	})
 
 	It("should update if the repository already exists when reconciling", func() {
-		repoRef := newUserRepoRef(giteaUser, testRepoName)
+		repoRef := newUserRepoRef(testRepoName)
 		// No-op reconcile
 		resp, actionTaken, err := c.UserRepositories().Reconcile(ctx, repoRef, gitprovider.RepositoryInfo{
 			Description:   gitprovider.StringVar(defaultDescription),
@@ -189,7 +202,7 @@ var _ = Describe("Gitea Provider", func() {
 
 	It("should be possible to create a pr for a user repository", func() {
 		testRepoName = fmt.Sprintf("test-user-repo2-%03d", rand.Intn(1000))
-		repoRef := newUserRepoRef(giteaUser, testRepoName)
+		repoRef := newUserRepoRef(testRepoName)
 		description := "test description"
 		// Create a new repo
 		userRepo, err := c.UserRepositories().Create(ctx, repoRef,
@@ -281,7 +294,7 @@ var _ = Describe("Gitea Provider", func() {
 
 	It("should be possible to download files from path and branch specified", func() {
 		testRepoName = fmt.Sprintf("test-repo-tree-%03d", rand.Intn(1000))
-		userRepoRef := newUserRepoRef(giteaUser, testRepoName)
+		userRepoRef := newUserRepoRef(testRepoName)
 		repo, err := c.UserRepositories().Create(ctx, userRepoRef, gitprovider.RepositoryInfo{
 			DefaultBranch: gitprovider.StringVar(defaultBranch),
 			Description:   gitprovider.StringVar(defaultDescription),

gitea/integration_suite_test.go

@@ -243,9 +243,9 @@ func newUserRef(userLogin string) gitprovider.UserRef {
 	}
 }
 
-func newUserRepoRef(userLogin, repoName string) gitprovider.UserRepositoryRef {
+func newUserRepoRef(repoName string) gitprovider.UserRepositoryRef {
 	return gitprovider.UserRepositoryRef{
-		UserRef:        newUserRef(userLogin),
+		UserRef:        newUserRef(giteaUser),
 		RepositoryName: repoName,
 	}
 }

gitea/resource_repository.go

@@ -164,7 +164,7 @@ func (r *userRepository) Update(ctx context.Context) error {
 		Archived:                  &r.r.Archived,
 		DefaultMergeStyle:         &r.r.DefaultMergeStyle,
 	}
-	if r.r.Mirror == true {
+	if r.r.Mirror {
 		opts.MirrorInterval = &r.r.MirrorInterval
 	}
 	apiObj, err := updateRepo(r.c, r.ref.GetIdentity(), r.ref.GetRepository(), &opts)
@@ -261,7 +261,7 @@ func validateRepositoryAPI(apiObj *gitea.Repository) error {
 			validator.Required("Name")
 		}
 		// Make sure visibility is valid if set
-		if apiObj.Private != true {
+		if !apiObj.Private {
 			v := gitprovider.RepositoryVisibility("public")
 			validator.Append(gitprovider.ValidateRepositoryVisibility(v), v, "Visibility")
 		} else {
@@ -276,7 +276,7 @@ func repositoryFromAPI(apiObj *gitea.Repository) gitprovider.RepositoryInfo {
 		Description:   &apiObj.Description,
 		DefaultBranch: &apiObj.DefaultBranch,
 	}
-	if apiObj.Private != true {
+	if !apiObj.Private {
 		repo.Visibility = gitprovider.RepositoryVisibilityVar(gitprovider.RepositoryVisibility("public"))
 	} else {
 		repo.Visibility = gitprovider.RepositoryVisibilityVar(gitprovider.RepositoryVisibility("private"))

gitea/util.go

@@ -26,13 +26,6 @@ import (
 	"github.com/fluxcd/go-git-providers/validation"
 )
 
-const (
-	alreadyExistsMagicString = "name already exists on this account"
-)
-
-// TODO: Guard better against nil pointer dereference panics in this package, also
-// validate data coming from the server
-
 // validateUserRepositoryRef makes sure the UserRepositoryRef is valid for Gitea's usage.
 func validateUserRepositoryRef(ref gitprovider.UserRepositoryRef, expectedDomain string) error {
 	// Make sure the RepositoryRef fields are valid

github/client_repositories_user.go

@@ -19,6 +19,7 @@ package github
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"github.com/fluxcd/go-git-providers/gitprovider"
 )
@@ -91,6 +92,13 @@ func (c *UserRepositoriesClient) Create(ctx context.Context,
 	if err != nil {
 		return nil, err
 	}
+
+	owner := apiObj.GetOwner()
+	if owner == nil {
+		return nil, fmt.Errorf("returned API object doesn't have an owner")
+	}
+	ref.UserLogin = *owner.Login
+
 	return newUserRepository(c.clientContext, apiObj, ref), nil
 }
 

github/integration_test.go

@@ -345,6 +345,18 @@ var _ = Describe("GitHub Provider", func() {
 		Expect(getSpec.Equals(postSpec)).To(BeTrue())
 	})
 
+	It("should return correct repo info when creating a repository with wrong UserLogin", func() {
+		repoName := fmt.Sprintf("test-user-repo-creation-%03d", rand.Intn(1000))
+		repoRef := newUserRepoRef("yadda-yadda-yada", repoName)
+
+		repo, err := c.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{})
+
+		Expect(err).To(BeNil())
+		Expect(
+			repo.Repository().GetCloneURL(gitprovider.TransportTypeHTTPS)).
+			To(Equal(fmt.Sprintf("https://%s/%s/%s.git", githubDomain, testUser, repoName)))
+	})
+
 	It("should error at creation time if the repo already does exist", func() {
 		repoRef := newOrgRepoRef(testOrgName, testOrgRepoName)
 		_, err := c.OrgRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{})