diff --git a/gitea/client_repositories_org.go b/gitea/client_repositories_org.go index f5bcba55..734745e9 100644 --- a/gitea/client_repositories_org.go +++ b/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 diff --git a/gitea/client_repositories_user.go b/gitea/client_repositories_user.go index f045d159..229968b2 100644 --- a/gitea/client_repositories_user.go +++ b/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 } diff --git a/gitea/client_repository_commit.go b/gitea/client_repository_commit.go index 625f6372..e8c2e735 100644 --- a/gitea/client_repository_commit.go +++ b/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{} diff --git a/gitea/integration_repositories_org_test.go b/gitea/integration_repositories_org_test.go index 9d82ce54..32a42321 100644 --- a/gitea/integration_repositories_org_test.go +++ b/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 diff --git a/gitea/integration_repositories_user_test.go b/gitea/integration_repositories_user_test.go index a8b648b0..85a324a8 100644 --- a/gitea/integration_repositories_user_test.go +++ b/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), diff --git a/gitea/integration_suite_test.go b/gitea/integration_suite_test.go index 950f26e7..0545f1fd 100644 --- a/gitea/integration_suite_test.go +++ b/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, } } diff --git a/gitea/resource_repository.go b/gitea/resource_repository.go index 59391599..1fecd9c3 100644 --- a/gitea/resource_repository.go +++ b/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")) diff --git a/gitea/util.go b/gitea/util.go index bbb4cb6f..7c900787 100644 --- a/gitea/util.go +++ b/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 diff --git a/github/client_repositories_user.go b/github/client_repositories_user.go index 42b853de..572bc58c 100644 --- a/github/client_repositories_user.go +++ b/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 } diff --git a/github/integration_test.go b/github/integration_test.go index c24689f5..ea0acc9d 100644 --- a/github/integration_test.go +++ b/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{}) diff --git a/gitlab/client_repositories_user.go b/gitlab/client_repositories_user.go index 3a643961..0f8ffc85 100644 --- a/gitlab/client_repositories_user.go +++ b/gitlab/client_repositories_user.go @@ -19,6 +19,7 @@ package gitlab import ( "context" "errors" + "fmt" "github.com/fluxcd/go-git-providers/gitprovider" ) @@ -82,6 +83,7 @@ func (c *UserRepositoriesClient) Create(ctx context.Context, req gitprovider.RepositoryInfo, opts ...gitprovider.RepositoryCreateOption, ) (gitprovider.UserRepository, error) { + // Make sure the RepositoryRef is valid if err := validateUserRepositoryRef(ref, c.domain); err != nil { return nil, err @@ -91,6 +93,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 newUserProject(c.clientContext, apiObj, ref), nil } diff --git a/gitlab/integration_test.go b/gitlab/integration_test.go index 71ec9033..bda91f36 100644 --- a/gitlab/integration_test.go +++ b/gitlab/integration_test.go @@ -796,6 +796,18 @@ var _ = Describe("GitLab Provider", func() { Expect(errors.Is(err, gitprovider.ErrAlreadyExists)).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(testBaseUrl, "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("%s/%s/%s.git", testBaseUrl, testUserName, repoName))) + }) + It("should update if the user repo already exists when reconciling", func() { repoRef := newUserRepoRef(testBaseUrl, testUserName, testRepoName) // No-op reconcile diff --git a/stash/client_repositories_user.go b/stash/client_repositories_user.go index 3a1b241e..4b7693dd 100644 --- a/stash/client_repositories_user.go +++ b/stash/client_repositories_user.go @@ -128,7 +128,7 @@ func (c *UserRepositoriesClient) Create(ctx context.Context, return nil, err } - apiObj, err := createRepository(ctx, c.client, addTilde(ref.UserLogin), ref, req, opts...) + apiObj, err := createRepository(ctx, c.client, addTilde(c.client.username), ref, req, opts...) if err != nil { if errors.Is(err, ErrAlreadyExists) { return nil, gitprovider.ErrAlreadyExists @@ -137,6 +137,7 @@ func (c *UserRepositoriesClient) Create(ctx context.Context, } ref.SetSlug(apiObj.Slug) + ref.UserLogin = apiObj.UserName return newUserRepository(c.clientContext, apiObj, ref), nil } diff --git a/stash/integration_repositories_user_test.go b/stash/integration_repositories_user_test.go index 4fa217f7..7c755026 100644 --- a/stash/integration_repositories_user_test.go +++ b/stash/integration_repositories_user_test.go @@ -107,6 +107,18 @@ var _ = Describe("Stash 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-yadda", repoName) + + repo, err := client.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{}) + + Expect(err).To(BeNil(), "repository with different UserLogin failed") + Expect( + repo.Repository().GetCloneURL(gitprovider.TransportTypeHTTPS)). + To(Equal(fmt.Sprintf("https://%s/%s/%s.git", stashDomain, stashUser, repoName))) + }) + It("should error at creation time if the user repo already does exist", func() { repoRef := newUserRepoRef(stashUser, testRepoName) _, err := client.UserRepositories().Get(ctx, repoRef) diff --git a/stash/repositories.go b/stash/repositories.go index f2ce67b7..1acaecdc 100644 --- a/stash/repositories.go +++ b/stash/repositories.go @@ -229,7 +229,7 @@ func (s *RepositoriesService) Create(ctx context.Context, projectKey string, rep return nil, fmt.Errorf("create respository failed: %w", err) } - if resp != nil && resp.StatusCode == http.StatusBadRequest { + if resp != nil && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("create repository failed: %s", resp.Status) }