From 113a7541fbc08b138da029e4b826c09a8231be1f Mon Sep 17 00:00:00 2001 From: Parth Patel Date: Sun, 12 Nov 2023 08:52:08 +1300 Subject: [PATCH 1/5] Fix: add collaborator when repo name contains the org name --- ...resource_github_repository_collaborator.go | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/github/resource_github_repository_collaborator.go b/github/resource_github_repository_collaborator.go index 9b758bee63..197f2e035e 100644 --- a/github/resource_github_repository_collaborator.go +++ b/github/resource_github_repository_collaborator.go @@ -69,14 +69,16 @@ func resourceGithubRepositoryCollaborator() *schema.Resource { func resourceGithubRepositoryCollaboratorCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*Owner).v3client - owner := meta.(*Owner).name username := d.Get("username").(string) repoName := d.Get("repository").(string) + + owner, repoNameWithoutOwner := parseRepoName(repoName, meta.(*Owner).name) + ctx := context.Background() _, _, err := client.Repositories.AddCollaborator(ctx, owner, - repoName, + repoNameWithoutOwner, username, &github.RepositoryAddCollaboratorOptions{ Permission: d.Get("permission").(string), @@ -94,15 +96,15 @@ func resourceGithubRepositoryCollaboratorCreate(d *schema.ResourceData, meta int func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta interface{}) error { client := meta.(*Owner).v3client - owner := meta.(*Owner).name repoName, username, err := parseTwoPartID(d.Id(), "repository", "username") + owner, repoNameWithoutOwner := parseRepoName(repoName, meta.(*Owner).name) if err != nil { return err } ctx := context.WithValue(context.Background(), ctxId, d.Id()) // First, check if the user has been invited but has not yet accepted - invitation, err := findRepoInvitation(client, ctx, owner, repoName, username) + invitation, err := findRepoInvitation(client, ctx, owner, repoNameWithoutOwner, username) if err != nil { if ghErr, ok := err.(*github.ErrorResponse); ok { if ghErr.Response.StatusCode == http.StatusNotFound { @@ -135,7 +137,7 @@ func resourceGithubRepositoryCollaboratorRead(d *schema.ResourceData, meta inter for { collaborators, resp, err := client.Repositories.ListCollaborators(ctx, - owner, repoName, opt) + owner, repoNameWithoutOwner, opt) if err != nil { return err } @@ -170,22 +172,23 @@ func resourceGithubRepositoryCollaboratorUpdate(d *schema.ResourceData, meta int func resourceGithubRepositoryCollaboratorDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*Owner).v3client - owner := meta.(*Owner).name username := d.Get("username").(string) repoName := d.Get("repository").(string) + owner, repoNameWithoutOwner := parseRepoName(repoName, meta.(*Owner).name) + ctx := context.WithValue(context.Background(), ctxId, d.Id()) // Delete any pending invitations - invitation, err := findRepoInvitation(client, ctx, owner, repoName, username) + invitation, err := findRepoInvitation(client, ctx, owner, repoNameWithoutOwner, username) if err != nil { return err } else if invitation != nil { - _, err = client.Repositories.DeleteInvitation(ctx, owner, repoName, invitation.GetID()) + _, err = client.Repositories.DeleteInvitation(ctx, owner, repoNameWithoutOwner, invitation.GetID()) return err } - _, err = client.Repositories.RemoveCollaborator(ctx, owner, repoName, username) + _, err = client.Repositories.RemoveCollaborator(ctx, owner, repoNameWithoutOwner, username) return err } @@ -210,3 +213,14 @@ func findRepoInvitation(client *github.Client, ctx context.Context, owner, repo, } return nil, nil } + +func parseRepoName(repoName string, defaultOwner string) (string, string) { + // GitHub replaces '/' with '-' for a repo name, so it is safe to assume that if repo name contains '/' + // then first part will be the owner name and second part will be the repo name + if strings.Contains(repoName, "/") { + parts := strings.Split(repoName, "/") + return parts[0], parts[1] + } else { + return defaultOwner, repoName + } +} From 3083cfaed334108c2c18a662c0ed30171d289251 Mon Sep 17 00:00:00 2001 From: Parth Patel Date: Sun, 12 Nov 2023 08:52:33 +1300 Subject: [PATCH 2/5] Tests for the fix --- ...rce_github_repository_collaborator_test.go | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index 64cc8de347..89219842c2 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -2,6 +2,7 @@ package github import ( "fmt" + "os" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" @@ -62,4 +63,92 @@ func TestAccGithubRepositoryCollaborator(t *testing.T) { }) }) + t.Run("creates invitations when repository contains the org name", func(t *testing.T) { + + orgName := os.Getenv("GITHUB_ORGANIZATION") + + if orgName == "" { + t.Skip("Set GITHUB_ORGANIZATION to unskip this test run") + } + + configWithOwner := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-%s" + auto_init = true + } + + resource "github_repository_collaborator" "test_repo_collaborator_2" { + repository = "%s/${github_repository.test.name}" + username = "parthpatel9216" + permission = "triage" + } + `, randomID, orgName) + + checkWithOwner := resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "github_repository_collaborator.test_repo_collaborator_2", "permission", + "triage", + ), + ) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: configWithOwner, + Check: checkWithOwner, + }, + }, + }) + } + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + testCase(t, individual) + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) +} + +func TestParseRepoName(t *testing.T) { + tests := []struct { + name string + repoName string + defaultOwner string + wantOwner string + wantRepoName string + }{ + { + name: "Repo name without owner", + repoName: "example-repo", + defaultOwner: "default-owner", + wantOwner: "default-owner", + wantRepoName: "example-repo", + }, + { + name: "Repo name with owner", + repoName: "owner-name/example-repo", + defaultOwner: "default-owner", + wantOwner: "owner-name", + wantRepoName: "example-repo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotOwner, gotRepoName := parseRepoName(tt.repoName, tt.defaultOwner) + if gotOwner != tt.wantOwner || gotRepoName != tt.wantRepoName { + t.Errorf("parseRepoName(%q, %q) = %q, %q, want %q, %q", + tt.repoName, tt.defaultOwner, gotOwner, gotRepoName, tt.wantOwner, tt.wantRepoName) + } + }) + } } From c40c7eeb783a7499365c23426650cf308aac4716 Mon Sep 17 00:00:00 2001 From: Parth Patel Date: Sun, 12 Nov 2023 08:52:48 +1300 Subject: [PATCH 3/5] Update documentation --- website/docs/r/repository_collaborator.html.markdown | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/docs/r/repository_collaborator.html.markdown b/website/docs/r/repository_collaborator.html.markdown index 276e32c89d..06392b3346 100644 --- a/website/docs/r/repository_collaborator.html.markdown +++ b/website/docs/r/repository_collaborator.html.markdown @@ -48,6 +48,9 @@ resource "github_repository_collaborator" "a_repo_collaborator" { The following arguments are supported: * `repository` - (Required) The GitHub repository + +~> Note: The owner of the repository can be passed as part fo the repository name e.g. `owner-org-name/repo-name`. If owner is not supplied as part of the repository name, can also be supplied by setting the environment variable `GITHUB_OWNER`. + * `username` - (Required) The user to add to the repository as a collaborator. * `permission` - (Optional) The permission of the outside collaborator for the repository. Must be one of `pull`, `push`, `maintain`, `triage` or `admin` or the name of an existing [custom repository role](https://docs.github.com/en/enterprise-cloud@latest/organizations/managing-peoples-access-to-your-organization-with-roles/managing-custom-repository-roles-for-an-organization) within the organization for organization-owned repositories. From 61446f67ef97c2f04a81f5356e30984af6737583 Mon Sep 17 00:00:00 2001 From: Parth Patel Date: Mon, 13 Nov 2023 15:01:36 +1300 Subject: [PATCH 4/5] Removed username from test --- github/resource_github_repository_collaborator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/resource_github_repository_collaborator_test.go b/github/resource_github_repository_collaborator_test.go index 89219842c2..22bf713ff8 100644 --- a/github/resource_github_repository_collaborator_test.go +++ b/github/resource_github_repository_collaborator_test.go @@ -79,7 +79,7 @@ func TestAccGithubRepositoryCollaborator(t *testing.T) { resource "github_repository_collaborator" "test_repo_collaborator_2" { repository = "%s/${github_repository.test.name}" - username = "parthpatel9216" + username = "" permission = "triage" } `, randomID, orgName) From bcd8ce8b76115af9936a95ea78ce4557f3403648 Mon Sep 17 00:00:00 2001 From: Parth Patel Date: Wed, 22 Nov 2023 11:03:38 +1300 Subject: [PATCH 5/5] Update website/docs/r/repository_collaborator.html.markdown Co-authored-by: Keegan Campbell --- website/docs/r/repository_collaborator.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/repository_collaborator.html.markdown b/website/docs/r/repository_collaborator.html.markdown index 06392b3346..fd7bedaa30 100644 --- a/website/docs/r/repository_collaborator.html.markdown +++ b/website/docs/r/repository_collaborator.html.markdown @@ -49,7 +49,7 @@ The following arguments are supported: * `repository` - (Required) The GitHub repository -~> Note: The owner of the repository can be passed as part fo the repository name e.g. `owner-org-name/repo-name`. If owner is not supplied as part of the repository name, can also be supplied by setting the environment variable `GITHUB_OWNER`. +~> Note: The owner of the repository can be passed as part of the repository name e.g. `owner-org-name/repo-name`. If owner is not supplied as part of the repository name, it may also be supplied by setting the environment variable `GITHUB_OWNER`. * `username` - (Required) The user to add to the repository as a collaborator. * `permission` - (Optional) The permission of the outside collaborator for the repository.