From 41bfa36ecf368b35c2a422176724c58e15800833 Mon Sep 17 00:00:00 2001 From: galargh Date: Tue, 11 Aug 2026 20:21:34 +0200 Subject: [PATCH] feat: Add downgrade_to option to github_membership resource --- docs/resources/membership.md | 16 +- examples/resources/membership/example_2.tf | 8 + github/resource_github_membership.go | 42 ++- github/resource_github_membership_test.go | 409 +++++++++++++++++++++ templates/resources/membership.md.tmpl | 7 +- 5 files changed, 474 insertions(+), 8 deletions(-) create mode 100644 examples/resources/membership/example_2.tf diff --git a/docs/resources/membership.md b/docs/resources/membership.md index fef7355394..ff54f60b9d 100644 --- a/docs/resources/membership.md +++ b/docs/resources/membership.md @@ -20,13 +20,27 @@ resource "github_membership" "membership_for_some_user" { } ``` +## Example Usage with Downgrade on Destroy + +```terraform +# Downgrade a member to an outside collaborator when the resource is destroyed +resource "github_membership" "outside_collaborator_on_destroy" { + username = "SomeUser" + role = "member" + + downgrade_on_destroy = true + downgrade_to = "outside_collaborator" +} +``` + ## Argument Reference The following arguments are supported: - `username` - (Required) The user to add to the organization. - `role` - (Optional) The role of the user within the organization. Must be one of `member` or `admin`. Defaults to `member`. `admin` role represents the `owner` role available via GitHub UI. -- `downgrade_on_destroy` - (Optional) Defaults to `false`. If set to true, when this resource is destroyed, the member will not be removed from the organization. Instead, the member's role will be downgraded to 'member'. +- `downgrade_on_destroy` - (Optional) Defaults to `false`. Instead of removing the member from the org, you can choose to downgrade their membership when this resource is destroyed. This is useful when wanting to downgrade admins while keeping them in the organization, or to downgrade members while keeping their access to public repositories intact. +- `downgrade_to` - (Optional) The target membership state when `downgrade_on_destroy` is true. Must be one of `member` or `outside_collaborator`. Defaults to `member`. ## Import diff --git a/examples/resources/membership/example_2.tf b/examples/resources/membership/example_2.tf new file mode 100644 index 0000000000..3f9ec2d071 --- /dev/null +++ b/examples/resources/membership/example_2.tf @@ -0,0 +1,8 @@ +# Downgrade a member to an outside collaborator when the resource is destroyed +resource "github_membership" "outside_collaborator_on_destroy" { + username = "SomeUser" + role = "member" + + downgrade_on_destroy = true + downgrade_to = "outside_collaborator" +} diff --git a/github/resource_github_membership.go b/github/resource_github_membership.go index 5f643f0b96..2d071f32b3 100644 --- a/github/resource_github_membership.go +++ b/github/resource_github_membership.go @@ -12,6 +12,12 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) +const ( + membershipDowngradeToMember = "member" + membershipDowngradeToOutsideCollaborator = "outside_collaborator" + membershipStateActive = "active" +) + func resourceGithubMembership() *schema.Resource { return &schema.Resource{ CreateContext: resourceGithubMembershipCreateOrUpdate, @@ -45,7 +51,14 @@ func resourceGithubMembership() *schema.Resource { Type: schema.TypeBool, Optional: true, Default: false, - Description: "Instead of removing the member from the org, you can choose to downgrade their membership to 'member' when this resource is destroyed. This is useful when wanting to downgrade admins while keeping them in the organization", + Description: "Instead of removing the member from the org, you can choose to downgrade their membership when this resource is destroyed. This is useful when wanting to downgrade admins while keeping them in the organization, or to downgrade members while keeping their access to public repositories intact.", + }, + "downgrade_to": { + Type: schema.TypeString, + Optional: true, + ValidateDiagFunc: validateValueFunc([]string{membershipDowngradeToMember, membershipDowngradeToOutsideCollaborator}), + Default: membershipDowngradeToMember, + Description: "The target membership state when downgrade_on_destroy is true. Must be one of 'member' or 'outside_collaborator'.", }, }, } @@ -64,6 +77,9 @@ func resourceGithubMembershipCreateOrUpdate(ctx context.Context, d *schema.Resou roleName := d.Get("role").(string) if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxId, d.Id()) + if !d.HasChange("role") { + return resourceGithubMembershipRead(ctx, d, meta) + } } _, resp, err := client.Organizations.EditOrgMembership(ctx, @@ -148,7 +164,10 @@ func resourceGithubMembershipDelete(ctx context.Context, d *schema.ResourceData, username := d.Get("username").(string) downgradeOnDestroy := d.Get("downgrade_on_destroy").(bool) - downgradeTo := "member" + downgradeTo, ok := d.Get("downgrade_to").(string) + if !ok || downgradeTo == "" { + downgradeTo = membershipDowngradeToMember + } if downgradeOnDestroy { tflog.Info(ctx, fmt.Sprintf("Downgrading '%s' membership for '%s' to '%s'", orgName, username, downgradeTo), map[string]any{ @@ -176,7 +195,7 @@ func resourceGithubMembershipDelete(ctx context.Context, d *schema.ResourceData, return diag.FromErr(err) } - if *membership.Role == downgradeTo { + if downgradeTo == membershipDowngradeToMember && membership.GetRole() == downgradeTo { tflog.Info(ctx, fmt.Sprintf("Not downgrading '%s' membership for '%s' because they are already '%s'", orgName, username, downgradeTo), map[string]any{ "org_name": orgName, "username": username, @@ -185,9 +204,20 @@ func resourceGithubMembershipDelete(ctx context.Context, d *schema.ResourceData, return nil } - _, _, err = client.Organizations.EditOrgMembership(ctx, username, orgName, &github.Membership{ - Role: new(downgradeTo), - }) + if downgradeTo == membershipDowngradeToOutsideCollaborator && membership.GetState() != membershipStateActive { + return diag.Errorf("cannot downgrade %q to outside collaborator because organization membership is %q, not active; the user must accept the organization invitation first", username, membership.GetState()) + } + + switch downgradeTo { + case membershipDowngradeToMember: + _, _, err = client.Organizations.EditOrgMembership(ctx, username, orgName, &github.Membership{ + Role: new(downgradeTo), + }) + case membershipDowngradeToOutsideCollaborator: + _, err = client.Organizations.ConvertMemberToOutsideCollaborator(ctx, orgName, username) + default: + return diag.Errorf("%s is an invalid value for argument downgrade_to", downgradeTo) + } } else { tflog.Info(ctx, fmt.Sprintf("Revoking '%s' membership for '%s'", orgName, username), map[string]any{ "org_name": orgName, diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index 93d216ff50..8441b122b7 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -4,9 +4,16 @@ import ( "context" "errors" "fmt" + "net/http" + "strings" "testing" + "time" "github.com/google/go-github/v89/github" + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" ) @@ -42,6 +49,10 @@ func TestAccGithubMembership(t *testing.T) { ResourceName: rn, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "downgrade_on_destroy", + "downgrade_to", + }, }, }, }) @@ -75,6 +86,54 @@ func TestAccGithubMembership(t *testing.T) { }) }) + t.Run("downgrades organization membership to outside collaborator", func(t *testing.T) { + // IMPORTANT: Do not run this sub test in parallel is it uses shared state. + + if len(testAccConf.testExternalUser1Token) == 0 { + t.Skip("No external user token provided") + } + + ctx := t.Context() + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + repoName := fmt.Sprintf("%smembership-downgrade-%s", testResourcePrefix, randomID) + username := testAccConf.testExternalUser1 + rn := "github_membership.test_org_membership" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + skipUnlessHasOrgs(t) + t.Cleanup(func() { + _, _ = testAccConf.meta.v3client.Organizations.RemoveOutsideCollaborator(context.Background(), testAccConf.owner, username) + }) + }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubMembershipDestroy, + Steps: []resource.TestStep{ + { + Config: testAccGithubMembershipConfigOutsideCollaboratorDowngradePending(username, repoName), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubMembershipState(t, ctx, rn, "pending"), + ), + }, + { + PreConfig: func() { + testAccGithubMembershipAcceptOrgInvitation(t, testAccConf.testExternalUser1Token) + testAccGithubMembershipAddRepositoryCollaborator(t, repoName, username) + }, + Config: testAccGithubMembershipConfigOutsideCollaboratorDowngrade(username, repoName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(rn, "downgrade_to", membershipDowngradeToOutsideCollaborator), + testAccCheckGithubMembershipState(t, ctx, rn, membershipStateActive), + ), + }, + { + Config: testAccGithubMembershipConfigOutsideCollaboratorDowngradeCleanup(username, repoName), + Check: testAccCheckGithubMembershipOutsideCollaborator(t, ctx, username), + }, + }, + }) + }) + t.Run("creates organization membership with case insensitivity", func(t *testing.T) { // IMPORTANT: Do not run this sub test in parallel is it uses shared state. @@ -108,6 +167,10 @@ func TestAccGithubMembership(t *testing.T) { ResourceName: rn, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "downgrade_on_destroy", + "downgrade_to", + }, }, }, }) @@ -129,10 +192,36 @@ func testAccCheckGithubMembershipDestroy(s *terraform.State) error { } downgradedOnDestroy := rs.Primary.Attributes["downgrade_on_destroy"] == "true" + downgradeTo := rs.Primary.Attributes["downgrade_to"] membership, resp, err := conn.Organizations.GetOrgMembership(ctx, username, orgName) responseIsSuccessful := err == nil && membership != nil && buildTwoPartID(orgName, username) == rs.Primary.ID if downgradedOnDestroy { + if downgradeTo == membershipDowngradeToOutsideCollaborator { + if responseIsSuccessful { + return fmt.Errorf("organization membership %q still exists", rs.Primary.ID) + } + if resp == nil || resp.StatusCode != http.StatusNotFound { + return err + } + + isOutsideCollaborator, err := testAccGithubMembershipIsOutsideCollaborator(ctx, conn, orgName, username) + if err != nil { + return err + } + if !isOutsideCollaborator { + return fmt.Errorf("organization membership %q was not converted to an outside collaborator", rs.Primary.ID) + } + + // Now actually remove them from the org to clean up + _, removeErr := conn.Organizations.RemoveOutsideCollaborator(ctx, orgName, username) + if removeErr != nil { + return fmt.Errorf("outside collaborator %q could not be removed during membership downgrade test case cleanup: %w", rs.Primary.ID, removeErr) + } + + return nil + } + if !responseIsSuccessful { return fmt.Errorf("could not load organization membership for %q", rs.Primary.ID) } @@ -157,6 +246,141 @@ func testAccCheckGithubMembershipDestroy(s *terraform.State) error { return nil } +func testAccCheckGithubMembershipOutsideCollaborator(t *testing.T, ctx context.Context, username string) resource.TestCheckFunc { + t.Helper() + + return func(s *terraform.State) error { + conn := testAccConf.meta.v3client + orgName := testAccConf.owner + + return retry.RetryContext(ctx, 30*time.Second, func() *retry.RetryError { + membership, resp, err := conn.Organizations.GetOrgMembership(ctx, username, orgName) + if err == nil && membership != nil { + return retry.RetryableError(fmt.Errorf("organization membership %q still exists", buildTwoPartID(orgName, username))) + } + if resp == nil || resp.StatusCode != http.StatusNotFound { + return retry.NonRetryableError(err) + } + + isOutsideCollaborator, err := testAccGithubMembershipIsOutsideCollaborator(ctx, conn, orgName, username) + if err != nil { + return retry.NonRetryableError(err) + } + if !isOutsideCollaborator { + return retry.RetryableError(fmt.Errorf("%q is not an outside collaborator for organization %q", username, orgName)) + } + + return nil + }) + } +} + +func testAccGithubMembershipAcceptOrgInvitation(t *testing.T, inviteeToken string) { + t.Helper() + + client, err := testAccGithubMembershipInviteeClient(inviteeToken) + if err != nil { + t.Fatalf("failed to create invitee GitHub client: %s", err) + } + + err = retry.RetryContext(t.Context(), 30*time.Second, func() *retry.RetryError { + _, resp, err := client.Organizations.EditOrgMembership(t.Context(), "", testAccConf.owner, &github.Membership{ + State: new(membershipStateActive), + }) + if err != nil { + statusCode := 0 + if resp != nil { + statusCode = resp.StatusCode + } + if statusCode == http.StatusForbidden || statusCode == http.StatusNotFound || statusCode == http.StatusUnauthorized { + return retry.NonRetryableError(fmt.Errorf("failed to accept organization invitation: %w", err)) + } + return retry.RetryableError(fmt.Errorf("failed to accept organization invitation: %w", err)) + } + return nil + }) + if err != nil { + t.Fatal(err) + } +} + +func testAccGithubMembershipAddRepositoryCollaborator(t *testing.T, repoName, username string) { + t.Helper() + + _, _, err := testAccConf.meta.v3client.Repositories.AddCollaborator(t.Context(), + testAccConf.owner, + repoName, + username, + &github.RepositoryAddCollaboratorOptions{Permission: "push"}, + ) + if err != nil { + t.Fatalf("failed to add repository collaborator %q on %q: %s", username, repoName, err) + } +} + +func testAccGithubMembershipInviteeClient(inviteeToken string) (*github.Client, error) { + config := &Config{ + BaseURL: testAccConf.baseURL, + IsGHES: testAccConf.isGHES, + Token: inviteeToken, + } + return config.NewRESTClient(config.AuthenticatedHTTPClient()) +} + +func testAccGithubMembershipIsOutsideCollaborator(ctx context.Context, conn *github.Client, orgName, username string) (bool, error) { + opts := &github.ListOutsideCollaboratorsOptions{ + ListOptions: github.ListOptions{PerPage: 100}, + } + for { + users, resp, err := conn.Organizations.ListOutsideCollaborators(ctx, orgName, opts) + if err != nil { + return false, err + } + for _, user := range users { + if strings.EqualFold(user.GetLogin(), username) { + return true, nil + } + } + if resp == nil || resp.NextPage == 0 { + return false, nil + } + opts.Page = resp.NextPage + } +} + +func testAccCheckGithubMembershipState(t *testing.T, ctx context.Context, n, expectedState string) resource.TestCheckFunc { + t.Helper() + + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("not Found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("no membership ID is set") + } + + conn := testAccConf.meta.v3client + + orgName, username, err := parseID2(rs.Primary.ID) + if err != nil { + return err + } + + membership, _, err := conn.Organizations.GetOrgMembership(ctx, username, orgName) + if err != nil { + return err + } + + if membership.GetState() != expectedState { + return fmt.Errorf("expected membership state %q, got %q", expectedState, membership.GetState()) + } + + return nil + } +} + func testAccCheckGithubMembershipExists(ctx context.Context, n string, membership *github.Membership) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -237,6 +461,45 @@ func testAccGithubMembershipConfigDowngradable(username string) string { `, username, true) } +func testAccGithubMembershipConfigOutsideCollaboratorDowngradePending(username, repoName string) string { + return fmt.Sprintf(` + resource "github_membership" "test_org_membership" { + username = "%[1]s" + role = "member" + } + + resource "github_repository" "test" { + name = "%[2]s" + auto_init = true + } +`, username, repoName) +} + +func testAccGithubMembershipConfigOutsideCollaboratorDowngrade(username, repoName string) string { + return fmt.Sprintf(` + resource "github_membership" "test_org_membership" { + username = "%[1]s" + role = "member" + downgrade_on_destroy = true + downgrade_to = "%[3]s" + } + + resource "github_repository" "test" { + name = "%[2]s" + auto_init = true + } +`, username, repoName, membershipDowngradeToOutsideCollaborator) +} + +func testAccGithubMembershipConfigOutsideCollaboratorDowngradeCleanup(username, repoName string) string { + return fmt.Sprintf(` + resource "github_repository" "test" { + name = "%[2]s" + auto_init = true + } +`, username, repoName) +} + func testAccGithubMembershipTheSame(orig, other *github.Membership) resource.TestCheckFunc { return func(s *terraform.State) error { if orig.GetURL() != other.GetURL() { @@ -246,3 +509,149 @@ func testAccGithubMembershipTheSame(orig, other *github.Membership) resource.Tes return nil } } + +func Test_resourceGithubMembershipDelete(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + downgradeOnDestroy bool + downgradeTo string + setDowngradeTo bool + expectError string + responses []*mockResponse + }{ + { + name: "removes membership", + responses: []*mockResponse{ + { + ExpectedMethod: http.MethodDelete, + ExpectedUri: "/orgs/test-org/memberships/octocat", + StatusCode: http.StatusNoContent, + }, + }, + }, + { + name: "member default", + downgradeOnDestroy: true, + responses: []*mockResponse{ + { + ExpectedMethod: http.MethodGet, + ExpectedUri: "/orgs/test-org/memberships/octocat", + StatusCode: http.StatusOK, + ResponseBody: `{"role":"admin"}`, + }, + { + ExpectedMethod: http.MethodPut, + ExpectedUri: "/orgs/test-org/memberships/octocat", + StatusCode: http.StatusOK, + ResponseBody: `{"role":"member"}`, + }, + }, + }, + { + name: "outside collaborator", + downgradeOnDestroy: true, + downgradeTo: membershipDowngradeToOutsideCollaborator, + setDowngradeTo: true, + responses: []*mockResponse{ + { + ExpectedMethod: http.MethodGet, + ExpectedUri: "/orgs/test-org/memberships/octocat", + StatusCode: http.StatusOK, + ResponseBody: `{"role":"member","state":"active"}`, + }, + { + ExpectedMethod: http.MethodPut, + ExpectedUri: "/orgs/test-org/outside_collaborators/octocat", + StatusCode: http.StatusNoContent, + }, + }, + }, + { + name: "outside collaborator pending invitation", + downgradeOnDestroy: true, + downgradeTo: membershipDowngradeToOutsideCollaborator, + setDowngradeTo: true, + expectError: `cannot downgrade "octocat" to outside collaborator because organization membership is "pending", not active; the user must accept the organization invitation first`, + responses: []*mockResponse{ + { + ExpectedMethod: http.MethodGet, + ExpectedUri: "/orgs/test-org/memberships/octocat", + StatusCode: http.StatusOK, + ResponseBody: `{"role":"member","state":"pending"}`, + }, + }, + }, + { + name: "membership not found", + downgradeOnDestroy: true, + downgradeTo: membershipDowngradeToOutsideCollaborator, + setDowngradeTo: true, + responses: []*mockResponse{ + { + ExpectedMethod: http.MethodGet, + ExpectedUri: "/orgs/test-org/memberships/octocat", + StatusCode: http.StatusNotFound, + ResponseBody: `{"message":"Not Found"}`, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ts := githubApiMock(tt.responses) + defer ts.Close() + + raw := map[string]any{ + "username": "octocat", + "downgrade_on_destroy": tt.downgradeOnDestroy, + } + if tt.setDowngradeTo { + raw["downgrade_to"] = tt.downgradeTo + } + d := schema.TestResourceDataRaw(t, resourceGithubMembership().Schema, raw) + d.SetId(buildTwoPartID("test-org", "octocat")) + + meta := &Owner{ + name: "test-org", + v3client: mustCreateTestGitHubClient(t, ts.URL), + IsOrganization: true, + } + + diags := resourceGithubMembershipDelete(t.Context(), d, meta) + if tt.expectError != "" { + if !diags.HasError() { + t.Fatalf("expected diagnostics containing %q, got none", tt.expectError) + } + if !strings.Contains(diags[0].Summary, tt.expectError) { + t.Fatalf("expected diagnostics containing %q, got: %v", tt.expectError, diags) + } + return + } + if diags.HasError() { + t.Fatalf("expected no diagnostics, got: %v", diags) + } + }) + } +} + +func Test_resourceGithubMembershipDowngradeToValidation(t *testing.T) { + t.Parallel() + + validate := resourceGithubMembership().Schema["downgrade_to"].ValidateDiagFunc + for _, value := range []string{membershipDowngradeToMember, membershipDowngradeToOutsideCollaborator} { + if diags := validate(value, cty.Path{cty.GetAttrStep{Name: "downgrade_to"}}); diags.HasError() { + t.Fatalf("expected %q to be valid, got: %v", value, diags) + } + } + + for _, value := range []string{"outside-collaborator", "outside collaborator"} { + if diags := validate(value, cty.Path{cty.GetAttrStep{Name: "downgrade_to"}}); !diags.HasError() { + t.Fatalf("expected %q to be invalid", value) + } + } +} diff --git a/templates/resources/membership.md.tmpl b/templates/resources/membership.md.tmpl index 9b45be10c0..ea52089ba8 100644 --- a/templates/resources/membership.md.tmpl +++ b/templates/resources/membership.md.tmpl @@ -14,13 +14,18 @@ This resource allows you to add/remove users from your organization. When applie {{ tffile "examples/resources/membership/example_1.tf" }} +## Example Usage with Downgrade on Destroy + +{{ tffile "examples/resources/membership/example_2.tf" }} + ## Argument Reference The following arguments are supported: - `username` - (Required) The user to add to the organization. - `role` - (Optional) The role of the user within the organization. Must be one of `member` or `admin`. Defaults to `member`. `admin` role represents the `owner` role available via GitHub UI. -- `downgrade_on_destroy` - (Optional) Defaults to `false`. If set to true, when this resource is destroyed, the member will not be removed from the organization. Instead, the member's role will be downgraded to 'member'. +- `downgrade_on_destroy` - (Optional) Defaults to `false`. Instead of removing the member from the org, you can choose to downgrade their membership when this resource is destroyed. This is useful when wanting to downgrade admins while keeping them in the organization, or to downgrade members while keeping their access to public repositories intact. +- `downgrade_to` - (Optional) The target membership state when `downgrade_on_destroy` is true. Must be one of `member` or `outside_collaborator`. Defaults to `member`. ## Import