diff --git a/docs/data-sources/membership.md b/docs/data-sources/membership.md index 4bb9284f27..c18aa48650 100644 --- a/docs/data-sources/membership.md +++ b/docs/data-sources/membership.md @@ -16,15 +16,31 @@ data "github_membership" "membership_for_some_user" { } ``` +### Lookup by stable user ID + +```terraform +# Look up a membership by the stable GitHub user ID. +# The numeric ID does not change when the user renames their account. +data "github_membership" "by_user_id" { + user_id = 1 +} +``` + ## Argument Reference -- `username` - (Required) The username to lookup in the organization. +Exactly one of the following must be set: + +- `username` - (Optional) The username (login) to lookup in the organization. +- `user_id` - (Optional) The GitHub numeric user ID. Stable across username changes; prefer this for lookups that should survive renames. + +Other arguments: -- `organization` - (Optional) The organization to check for the above username. +- `organization` - (Optional) The organization to check for the above user. ## Attributes Reference -- `username` - The username. +- `username` - The username (login). Always reflects the user's current login at refresh time. +- `user_id` - The GitHub numeric user ID. - `role` - `admin` or `member` -- the role the user has within the organization. - `etag` - An etag representing the membership object. - `state` - `active` or `pending` -- the state of membership within the organization. `active` if the member has accepted the invite, or `pending` if the invite is still pending. diff --git a/docs/data-sources/user.md b/docs/data-sources/user.md index 9d2fe0ba19..09e12890e0 100644 --- a/docs/data-sources/user.md +++ b/docs/data-sources/user.md @@ -26,13 +26,27 @@ output "current_github_login" { } ``` +### Lookup by stable user ID + +```terraform +# Retrieve information about a GitHub user by their stable numeric ID. +# Useful when the user may rename themselves: the lookup keeps working. +data "github_user" "by_id" { + user_id = 1 +} +``` + ## Argument Reference -- `username` - (Required) The username. Use an empty string `""` to retrieve information about the currently authenticated user. +Exactly one of the following must be set: + +- `username` - (Optional) The username (login). Use an empty string `""` to retrieve information about the currently authenticated user. +- `user_id` - (Optional) The GitHub numeric user ID. Stable across username changes, so prefer this if you need lookups that survive a rename. ## Attributes Reference - `id` - the ID of the user. +- `user_id` - the GitHub numeric user ID (same value as `id`, also settable as an input). - `node_id` - the Node ID of the user. - `login` - the user's login. - `avatar_url` - the user's avatar URL. diff --git a/docs/resources/membership.md b/docs/resources/membership.md index fef7355394..a2ab1b71bb 100644 --- a/docs/resources/membership.md +++ b/docs/resources/membership.md @@ -20,18 +20,46 @@ resource "github_membership" "membership_for_some_user" { } ``` +### Identifying the user by stable numeric ID + +Using `user_id` instead of `username` makes the membership resilient to the user renaming their GitHub account. After a rename, the next `terraform refresh` updates the `username` attribute in state with no diff, and the resource continues to manage the same membership. + +```terraform +# Manage organization membership by stable GitHub user ID. +# Recommended over `username` for production: if the user renames their +# account, the membership stays in sync without drift. +resource "github_membership" "membership_by_user_id" { + user_id = 1 + role = "member" +} +``` + ## Argument Reference The following arguments are supported: -- `username` - (Required) The user to add to the organization. +Exactly one of: + +- `username` - (Optional) The user (login) to add to the organization. Note: usernames can change; if the user renames themselves, the resource will recreate unless `user_id` is used instead. +- `user_id` - (Optional) The GitHub numeric user ID to add to the organization. Stable across username changes. Recommended for production use. + +Other arguments: + - `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'. +## Attributes Reference + +- `username` - The user's current login. When the resource is identified by `user_id`, this attribute tracks the user's live login at refresh time. +- `user_id` - The GitHub numeric user ID. +- `etag` - The etag of the membership object. + ## Import -GitHub Membership can be imported using an ID made up of `organization:username`, e.g. +GitHub Membership can be imported using an ID made up of `organization:user_id`, e.g. ```shell -terraform import github_membership.member hashicorp:someuser +terraform import github_membership.member hashicorp:12345 ``` + +Legacy IDs of the form `organization:username` are still accepted on import and will be migrated to the numeric form on the next refresh. diff --git a/examples/data-sources/membership/example_2.tf b/examples/data-sources/membership/example_2.tf new file mode 100644 index 0000000000..ae065069e9 --- /dev/null +++ b/examples/data-sources/membership/example_2.tf @@ -0,0 +1,5 @@ +# Look up a membership by the stable GitHub user ID. +# The numeric ID does not change when the user renames their account. +data "github_membership" "by_user_id" { + user_id = 1 +} diff --git a/examples/data-sources/user/example_2.tf b/examples/data-sources/user/example_2.tf new file mode 100644 index 0000000000..ed7214c0fb --- /dev/null +++ b/examples/data-sources/user/example_2.tf @@ -0,0 +1,5 @@ +# Retrieve information about a GitHub user by their stable numeric ID. +# Useful when the user may rename themselves: the lookup keeps working. +data "github_user" "by_id" { + user_id = 1 +} diff --git a/examples/resources/membership/example_2.tf b/examples/resources/membership/example_2.tf new file mode 100644 index 0000000000..2a1cd833a8 --- /dev/null +++ b/examples/resources/membership/example_2.tf @@ -0,0 +1,7 @@ +# Manage organization membership by stable GitHub user ID. +# Recommended over `username` for production: if the user renames their +# account, the membership stays in sync without drift. +resource "github_membership" "membership_by_user_id" { + user_id = 1 + role = "member" +} diff --git a/github/data_source_github_membership.go b/github/data_source_github_membership.go index 94c6c8a904..678324d2ca 100644 --- a/github/data_source_github_membership.go +++ b/github/data_source_github_membership.go @@ -13,8 +13,18 @@ func dataSourceGithubMembership() *schema.Resource { Schema: map[string]*schema.Schema{ "username": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ExactlyOneOf: []string{"username", "user_id"}, + Description: "The username (login) to lookup in the organization. Exactly one of `username` or `user_id` must be set.", + }, + "user_id": { + Type: schema.TypeInt, + Optional: true, + Computed: true, + ExactlyOneOf: []string{"username", "user_id"}, + Description: "The GitHub numeric user ID to lookup in the organization. Stable across username changes. Exactly one of `username` or `user_id` must be set.", }, "organization": { Type: schema.TypeString, @@ -37,8 +47,6 @@ func dataSourceGithubMembership() *schema.Resource { } func dataSourceGithubMembershipRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - username := d.Get("username").(string) - client := meta.(*Owner).v3client orgName := meta.(*Owner).name @@ -46,28 +54,41 @@ func dataSourceGithubMembershipRead(ctx context.Context, d *schema.ResourceData, orgName = configuredOrg } - membership, resp, err := client.Organizations.GetOrgMembership(ctx, - username, orgName) + // Resolve to username (login). If user_id is provided, resolve it via + // GET /user/{id} since GitHub's membership endpoints only accept the + // username. This makes the data source robust against username changes. + var username string + if v, ok := d.GetOk("user_id"); ok { + userID := int64(v.(int)) + user, _, err := client.Users.GetByID(ctx, userID) + if err != nil { + return diag.FromErr(err) + } + username = user.GetLogin() + } else { + username = d.Get("username").(string) + } + + membership, resp, err := client.Organizations.GetOrgMembership(ctx, username, orgName) if err != nil { return diag.FromErr(err) } d.SetId(buildTwoPartID(membership.GetOrganization().GetLogin(), membership.GetUser().GetLogin())) - err = d.Set("username", membership.GetUser().GetLogin()) - if err != nil { + if err = d.Set("username", membership.GetUser().GetLogin()); err != nil { return diag.FromErr(err) } - err = d.Set("role", membership.GetRole()) - if err != nil { + if err = d.Set("user_id", membership.GetUser().GetID()); err != nil { return diag.FromErr(err) } - err = d.Set("etag", resp.Header.Get("ETag")) - if err != nil { + if err = d.Set("role", membership.GetRole()); err != nil { return diag.FromErr(err) } - err = d.Set("state", membership.GetState()) - if err != nil { + if err = d.Set("etag", resp.Header.Get("ETag")); err != nil { + return diag.FromErr(err) + } + if err = d.Set("state", membership.GetState()); err != nil { return diag.FromErr(err) } return nil diff --git a/github/data_source_github_membership_test.go b/github/data_source_github_membership_test.go index 108aa01b83..212808349a 100644 --- a/github/data_source_github_membership_test.go +++ b/github/data_source_github_membership_test.go @@ -26,6 +26,7 @@ func TestAccGithubMembershipDataSource(t *testing.T) { resource.TestCheckResourceAttrSet("data.github_membership.test", "role"), resource.TestCheckResourceAttrSet("data.github_membership.test", "etag"), resource.TestCheckResourceAttrSet("data.github_membership.test", "state"), + resource.TestCheckResourceAttrSet("data.github_membership.test", "user_id"), ) resource.Test(t, resource.TestCase{ @@ -59,4 +60,104 @@ func TestAccGithubMembershipDataSource(t *testing.T) { }, }) }) + + t.Run("queries the membership for a user by user_id", func(t *testing.T) { + ctx := t.Context() + + meta, err := getTestMeta() + if err != nil { + t.Fatalf("failed to get test meta: %s", err) + } + + ghUser, _, err := meta.v3client.Users.Get(ctx, testAccConf.testOrgUser) + if err != nil { + t.Fatalf("failed to resolve org user id: %s", err) + } + + config := fmt.Sprintf(` + data "github_membership" "test" { + user_id = %d + organization = "%s" + } + `, ghUser.GetID(), testAccConf.owner) + + check := resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.github_membership.test", "username", testAccConf.testOrgUser), + resource.TestCheckResourceAttr("data.github_membership.test", "user_id", fmt.Sprintf("%d", ghUser.GetID())), + resource.TestCheckResourceAttrSet("data.github_membership.test", "role"), + resource.TestCheckResourceAttrSet("data.github_membership.test", "etag"), + resource.TestCheckResourceAttrSet("data.github_membership.test", "state"), + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + Check: check, + }, + }, + }) + }) + + t.Run("errors when querying with non-existent user_id", func(t *testing.T) { + config := fmt.Sprintf(` + data "github_membership" "test" { + user_id = 999999999999 + organization = "%s" + } + `, testAccConf.owner) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`Not Found`), + }, + }, + }) + }) + + t.Run("errors when neither username nor user_id is provided", func(t *testing.T) { + config := fmt.Sprintf(` + data "github_membership" "test" { + organization = "%s" + } + `, testAccConf.owner) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`one of (\x60username\x60,\x60user_id\x60|\x60user_id\x60,\x60username\x60) must be specified`), + }, + }, + }) + }) + + t.Run("errors when both username and user_id are provided", func(t *testing.T) { + config := fmt.Sprintf(` + data "github_membership" "test" { + username = "%s" + user_id = 1 + organization = "%s" + } + `, testAccConf.testOrgUser, testAccConf.owner) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`only one of (\x60user_id\x60,\x60username\x60|\x60username\x60,\x60user_id\x60) can be specified`), + }, + }, + }) + }) } diff --git a/github/data_source_github_user.go b/github/data_source_github_user.go index 2fd4cf3dc7..75bdd960d4 100644 --- a/github/data_source_github_user.go +++ b/github/data_source_github_user.go @@ -4,6 +4,7 @@ import ( "context" "strconv" + "github.com/google/go-github/v86/github" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -14,8 +15,18 @@ func dataSourceGithubUser() *schema.Resource { Schema: map[string]*schema.Schema{ "username": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ExactlyOneOf: []string{"username", "user_id"}, + Description: "The username (login) to lookup. Exactly one of `username` or `user_id` must be set.", + }, + "user_id": { + Type: schema.TypeInt, + Optional: true, + Computed: true, + ExactlyOneOf: []string{"username", "user_id"}, + Description: "The GitHub numeric user ID to lookup. Stable across username changes. Exactly one of `username` or `user_id` must be set.", }, "login": { Type: schema.TypeString, @@ -104,11 +115,20 @@ func dataSourceGithubUser() *schema.Resource { } func dataSourceGithubUserRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - username := d.Get("username").(string) - client := meta.(*Owner).v3client - user, _, err := client.Users.Get(ctx, username) + var ( + user *github.User + err error + ) + + if v, ok := d.GetOk("user_id"); ok { + userID := int64(v.(int)) + user, _, err = client.Users.GetByID(ctx, userID) + } else { + username := d.Get("username").(string) + user, _, err = client.Users.Get(ctx, username) + } if err != nil { return diag.FromErr(err) } @@ -133,6 +153,12 @@ func dataSourceGithubUserRead(ctx context.Context, d *schema.ResourceData, meta } d.SetId(strconv.FormatInt(user.GetID(), 10)) + if err = d.Set("username", user.GetLogin()); err != nil { + return diag.FromErr(err) + } + if err = d.Set("user_id", user.GetID()); err != nil { + return diag.FromErr(err) + } if err = d.Set("login", user.GetLogin()); err != nil { return diag.FromErr(err) } diff --git a/github/data_source_github_user_test.go b/github/data_source_github_user_test.go index 31b6f48204..6383a48fbe 100644 --- a/github/data_source_github_user_test.go +++ b/github/data_source_github_user_test.go @@ -55,4 +55,98 @@ func TestAccGithubUserDataSource(t *testing.T) { }, }) }) + + t.Run("queries an existing individual account by user_id", func(t *testing.T) { + ctx := t.Context() + + meta, err := getTestMeta() + if err != nil { + t.Fatalf("failed to get test meta: %s", err) + } + + ghUser, _, err := meta.v3client.Users.Get(ctx, testAccConf.testExternalUser) + if err != nil { + t.Fatalf("failed to resolve external user id: %s", err) + } + + config := fmt.Sprintf(` + data "github_user" "test" { + user_id = %d + } + `, ghUser.GetID()) + + check := resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("data.github_user.test", "login", testAccConf.testExternalUser), + resource.TestCheckResourceAttr("data.github_user.test", "username", testAccConf.testExternalUser), + resource.TestCheckResourceAttr("data.github_user.test", "id", fmt.Sprintf("%d", ghUser.GetID())), + resource.TestCheckResourceAttr("data.github_user.test", "user_id", fmt.Sprintf("%d", ghUser.GetID())), + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + Check: check, + }, + }, + }) + }) + + t.Run("errors when querying a non-existing user_id", func(t *testing.T) { + config := ` + data "github_user" "test" { + user_id = 999999999999 + } + ` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`Not Found`), + }, + }, + }) + }) + + t.Run("errors when neither username nor user_id is provided", func(t *testing.T) { + config := ` + data "github_user" "test" {} + ` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`one of (\x60username\x60,\x60user_id\x60|\x60user_id\x60,\x60username\x60) must be specified`), + }, + }, + }) + }) + + t.Run("errors when both username and user_id are provided", func(t *testing.T) { + config := fmt.Sprintf(` + data "github_user" "test" { + username = "%s" + user_id = 1 + } + `, testAccConf.testExternalUser) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`only one of (\x60user_id\x60,\x60username\x60|\x60username\x60,\x60user_id\x60) can be specified`), + }, + }, + }) + }) } diff --git a/github/resource_github_membership.go b/github/resource_github_membership.go index 9d5c7e409c..bf76ae741c 100644 --- a/github/resource_github_membership.go +++ b/github/resource_github_membership.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "strconv" "github.com/google/go-github/v86/github" "github.com/hashicorp/terraform-plugin-log/tflog" @@ -25,10 +26,20 @@ func resourceGithubMembership() *schema.Resource { Schema: map[string]*schema.Schema{ "username": { Type: schema.TypeString, - Required: true, + Optional: true, + Computed: true, ForceNew: true, DiffSuppressFunc: caseInsensitive(), - Description: "The user to add to the organization.", + ExactlyOneOf: []string{"username", "user_id"}, + Description: "The user (login) to add to the organization. Exactly one of `username` or `user_id` must be set.", + }, + "user_id": { + Type: schema.TypeInt, + Optional: true, + Computed: true, + ForceNew: true, + ExactlyOneOf: []string{"username", "user_id"}, + Description: "The GitHub numeric user ID to add to the organization. Stable across username changes; recommended over `username` for production usage. Exactly one of `username` or `user_id` must be set.", }, "role": { Type: schema.TypeString, @@ -58,14 +69,19 @@ func resourceGithubMembershipCreateOrUpdate(ctx context.Context, d *schema.Resou } client := meta.(*Owner).v3client - orgName := meta.(*Owner).name - username := d.Get("username").(string) - roleName := d.Get("role").(string) + if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxId, d.Id()) } + username, userID, err := resolveMembershipIdentity(ctx, client, d) + if err != nil { + return diag.FromErr(err) + } + + roleName := d.Get("role").(string) + _, resp, err := client.Organizations.EditOrgMembership(ctx, username, orgName, @@ -77,8 +93,14 @@ func resourceGithubMembershipCreateOrUpdate(ctx context.Context, d *schema.Resou return diag.FromErr(err) } - d.SetId(buildTwoPartID(orgName, username)) + d.SetId(buildTwoPartID(orgName, strconv.FormatInt(userID, 10))) + if err = d.Set("username", username); err != nil { + return diag.FromErr(err) + } + if err = d.Set("user_id", userID); err != nil { + return diag.FromErr(err) + } if err = d.Set("etag", resp.Header.Get("ETag")); err != nil { return diag.FromErr(err) } @@ -93,19 +115,38 @@ func resourceGithubMembershipRead(ctx context.Context, d *schema.ResourceData, m } client := meta.(*Owner).v3client - orgName := meta.(*Owner).name - _, username, err := parseID2(d.Id()) + + orgPart, secondPart, err := parseID2(d.Id()) if err != nil { return diag.FromErr(err) } + + username, userID, err := loginAndIDFromIDPart(ctx, client, secondPart) + if err != nil { + var ghErr *github.ErrorResponse + if errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusNotFound { + tflog.Info(ctx, fmt.Sprintf("Removing membership %s from state because the user no longer exists in GitHub", d.Id()), map[string]any{ + "membership_id": d.Id(), + }) + d.SetId("") + return nil + } + return diag.FromErr(err) + } + + // Lazily migrate legacy IDs of the form `org:username` to `org:user_id`. + // New resources are always created with the numeric form (see Create). + if secondPart != strconv.FormatInt(userID, 10) { + d.SetId(buildTwoPartID(orgPart, strconv.FormatInt(userID, 10))) + } + ctx = context.WithValue(ctx, ctxId, d.Id()) if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } - membership, resp, err := client.Organizations.GetOrgMembership(ctx, - username, orgName) + membership, resp, err := client.Organizations.GetOrgMembership(ctx, username, orgName) if err != nil { var ghErr *github.ErrorResponse if errors.As(err, &ghErr) { @@ -129,6 +170,9 @@ func resourceGithubMembershipRead(ctx context.Context, d *schema.ResourceData, m if err = d.Set("username", username); err != nil { return diag.FromErr(err) } + if err = d.Set("user_id", userID); err != nil { + return diag.FromErr(err) + } if err = d.Set("role", membership.GetRole()); err != nil { return diag.FromErr(err) } @@ -146,6 +190,8 @@ func resourceGithubMembershipDelete(ctx context.Context, d *schema.ResourceData, orgName := meta.(*Owner).name ctx = context.WithValue(ctx, ctxId, d.Id()) + // Username in state is kept fresh by Read, so it reflects the user's + // current login even after a rename. username := d.Get("username").(string) downgradeOnDestroy := d.Get("downgrade_on_destroy").(bool) downgradeTo := "member" @@ -212,3 +258,44 @@ func resourceGithubMembershipDelete(ctx context.Context, d *schema.ResourceData, return diag.FromErr(err) } + +// resolveMembershipIdentity returns the (login, numeric_id) pair for the +// configured membership, regardless of whether the user supplied `username` +// or `user_id`. The GitHub org membership endpoints only accept the login, so +// when `user_id` is provided we must resolve it via GET /user/{id} first. +func resolveMembershipIdentity(ctx context.Context, client *github.Client, d *schema.ResourceData) (string, int64, error) { + if v, ok := d.GetOk("user_id"); ok { + userID := int64(v.(int)) + user, _, err := client.Users.GetByID(ctx, userID) + if err != nil { + return "", 0, err + } + return user.GetLogin(), user.GetID(), nil + } + + username := d.Get("username").(string) + user, _, err := client.Users.Get(ctx, username) + if err != nil { + return "", 0, err + } + return user.GetLogin(), user.GetID(), nil +} + +// loginAndIDFromIDPart resolves the (login, numeric_id) pair from the second +// segment of a resource ID. New resources use `org:`; legacy +// resources use `org:` and are migrated on the next Read. +func loginAndIDFromIDPart(ctx context.Context, client *github.Client, idPart string) (string, int64, error) { + if userID, err := strconv.ParseInt(idPart, 10, 64); err == nil { + user, _, err := client.Users.GetByID(ctx, userID) + if err != nil { + return "", 0, err + } + return user.GetLogin(), user.GetID(), nil + } + + user, _, err := client.Users.Get(ctx, idPart) + if err != nil { + return "", 0, err + } + return user.GetLogin(), user.GetID(), nil +} diff --git a/github/resource_github_membership_test.go b/github/resource_github_membership_test.go index 47d33e02d7..b2c1d9ef20 100644 --- a/github/resource_github_membership_test.go +++ b/github/resource_github_membership_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "regexp" "testing" "github.com/google/go-github/v86/github" @@ -104,6 +105,176 @@ func TestAccGithubMembership(t *testing.T) { }, }) }) + + t.Run("creates organization membership by user_id", func(t *testing.T) { + ctx := t.Context() + + meta, err := getTestMeta() + if err != nil { + t.Fatalf("failed to get test meta: %s", err) + } + + ghUser, _, err := meta.v3client.Users.Get(ctx, testAccConf.testExternalUser) + if err != nil { + t.Fatalf("failed to resolve external user id: %s", err) + } + + var membership github.Membership + rn := "github_membership.test_org_membership" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubMembershipDestroy, + Steps: []resource.TestStep{ + { + Config: testAccGithubMembershipConfigByUserID(ghUser.GetID()), + Check: resource.ComposeTestCheckFunc( + testAccCheckGithubMembershipExists(ctx, rn, &membership), + testAccCheckGithubMembershipRoleState(ctx, rn, &membership), + resource.TestCheckResourceAttr(rn, "username", testAccConf.testExternalUser), + resource.TestCheckResourceAttr(rn, "user_id", fmt.Sprintf("%d", ghUser.GetID())), + ), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) + }) + + t.Run("errors when neither username nor user_id is provided", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: ` + resource "github_membership" "test_org_membership" { + role = "member" + } + `, + ExpectError: regexp.MustCompile(`one of (\x60username\x60,\x60user_id\x60|\x60user_id\x60,\x60username\x60) must be specified`), + }, + }, + }) + }) + + t.Run("errors when both username and user_id are provided", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(` + resource "github_membership" "test_org_membership" { + username = "%s" + user_id = 1 + role = "member" + } + `, testAccConf.testExternalUser), + ExpectError: regexp.MustCompile(`only one of (\x60user_id\x60,\x60username\x60|\x60username\x60,\x60user_id\x60) can be specified`), + }, + }, + }) + }) +} + +// TestAccGithubMembershipRenameResilience verifies that when a membership is +// created via user_id, a subsequent rename of the GitHub account does not +// produce drift: the resource Reads the current login by numeric id and +// silently updates the username attribute. Requires GH_TEST_EXTERNAL_USER_TOKEN +// since renaming the external user account requires that user's own PAT. +func TestAccGithubMembershipRenameResilience(t *testing.T) { + if len(testAccConf.testExternalUser) == 0 { + t.Skip("No external user provided") + } + if len(testAccConf.testExternalUserToken) == 0 { + t.Skip("No external user token provided (GH_TEST_EXTERNAL_USER_TOKEN) - skipping live-rename test") + } + + ctx := t.Context() + + meta, err := getTestMeta() + if err != nil { + t.Fatalf("failed to get test meta: %s", err) + } + + ghUser, _, err := meta.v3client.Users.Get(ctx, testAccConf.testExternalUser) + if err != nil { + t.Fatalf("failed to resolve external user id: %s", err) + } + + originalLogin := testAccConf.testExternalUser + renamedLogin := originalLogin + "-renamed" + + rn := "github_membership.test_org_membership" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubMembershipDestroy, + Steps: []resource.TestStep{ + { + Config: testAccGithubMembershipConfigByUserID(ghUser.GetID()), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(rn, "username", originalLogin), + resource.TestCheckResourceAttr(rn, "user_id", fmt.Sprintf("%d", ghUser.GetID())), + ), + }, + { + PreConfig: func() { + if err := renameExternalUser(ctx, renamedLogin); err != nil { + t.Fatalf("failed to rename external user before refresh step: %s", err) + } + t.Cleanup(func() { + if err := renameExternalUser(context.Background(), originalLogin); err != nil { + t.Logf("WARNING: failed to restore external user login back to %q: %s", originalLogin, err) + } + }) + }, + Config: testAccGithubMembershipConfigByUserID(ghUser.GetID()), + PlanOnly: true, + ExpectNonEmptyPlan: false, + }, + { + RefreshState: true, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(rn, "username", renamedLogin), + resource.TestCheckResourceAttr(rn, "user_id", fmt.Sprintf("%d", ghUser.GetID())), + ), + }, + }, + }) +} + +func testAccGithubMembershipConfigByUserID(userID int64) string { + return fmt.Sprintf(` + resource "github_membership" "test_org_membership" { + user_id = %d + role = "member" + } +`, userID) +} + +// renameExternalUser renames the GH_TEST_EXTERNAL_USER account using that +// user's own PAT (GH_TEST_EXTERNAL_USER_TOKEN). PATCH /user only works for the +// authenticated user, so the org owner's token cannot rename them. +func renameExternalUser(ctx context.Context, newLogin string) error { + cfg := Config{ + Token: testAccConf.testExternalUserToken, + BaseURL: testAccConf.baseURL, + } + owner, err := cfg.Meta() + if err != nil { + return fmt.Errorf("failed to build client for external user rename: %w", err) + } + client := owner.(*Owner).v3client + + _, _, err = client.Users.Edit(ctx, &github.User{Login: github.Ptr(newLogin)}) + return err } func testAccCheckGithubMembershipDestroy(s *terraform.State) error { diff --git a/templates/data-sources/membership.md.tmpl b/templates/data-sources/membership.md.tmpl index c034a8ee98..075cc7d191 100644 --- a/templates/data-sources/membership.md.tmpl +++ b/templates/data-sources/membership.md.tmpl @@ -12,15 +12,25 @@ Use this data source to find out if a user is a member of your organization, as {{ tffile "examples/data-sources/membership/example_1.tf" }} +### Lookup by stable user ID + +{{ tffile "examples/data-sources/membership/example_2.tf" }} + ## Argument Reference -- `username` - (Required) The username to lookup in the organization. +Exactly one of the following must be set: + +- `username` - (Optional) The username (login) to lookup in the organization. +- `user_id` - (Optional) The GitHub numeric user ID. Stable across username changes; prefer this for lookups that should survive renames. + +Other arguments: -- `organization` - (Optional) The organization to check for the above username. +- `organization` - (Optional) The organization to check for the above user. ## Attributes Reference -- `username` - The username. +- `username` - The username (login). Always reflects the user's current login at refresh time. +- `user_id` - The GitHub numeric user ID. - `role` - `admin` or `member` -- the role the user has within the organization. - `etag` - An etag representing the membership object. - `state` - `active` or `pending` -- the state of membership within the organization. `active` if the member has accepted the invite, or `pending` if the invite is still pending. diff --git a/templates/data-sources/user.md.tmpl b/templates/data-sources/user.md.tmpl index 0aeb7c9065..33d0f4e29b 100644 --- a/templates/data-sources/user.md.tmpl +++ b/templates/data-sources/user.md.tmpl @@ -12,13 +12,21 @@ Use this data source to retrieve information about a GitHub user. {{ tffile "examples/data-sources/user/example_1.tf" }} +### Lookup by stable user ID + +{{ tffile "examples/data-sources/user/example_2.tf" }} + ## Argument Reference -- `username` - (Required) The username. Use an empty string `""` to retrieve information about the currently authenticated user. +Exactly one of the following must be set: + +- `username` - (Optional) The username (login). Use an empty string `""` to retrieve information about the currently authenticated user. +- `user_id` - (Optional) The GitHub numeric user ID. Stable across username changes, so prefer this if you need lookups that survive a rename. ## Attributes Reference - `id` - the ID of the user. +- `user_id` - the GitHub numeric user ID (same value as `id`, also settable as an input). - `node_id` - the Node ID of the user. - `login` - the user's login. - `avatar_url` - the user's avatar URL. diff --git a/templates/resources/membership.md.tmpl b/templates/resources/membership.md.tmpl index 9b45be10c0..e6a9dd1e1d 100644 --- a/templates/resources/membership.md.tmpl +++ b/templates/resources/membership.md.tmpl @@ -14,18 +14,38 @@ This resource allows you to add/remove users from your organization. When applie {{ tffile "examples/resources/membership/example_1.tf" }} +### Identifying the user by stable numeric ID + +Using `user_id` instead of `username` makes the membership resilient to the user renaming their GitHub account. After a rename, the next `terraform refresh` updates the `username` attribute in state with no diff, and the resource continues to manage the same membership. + +{{ tffile "examples/resources/membership/example_2.tf" }} + ## Argument Reference The following arguments are supported: -- `username` - (Required) The user to add to the organization. +Exactly one of: + +- `username` - (Optional) The user (login) to add to the organization. Note: usernames can change; if the user renames themselves, the resource will recreate unless `user_id` is used instead. +- `user_id` - (Optional) The GitHub numeric user ID to add to the organization. Stable across username changes. Recommended for production use. + +Other arguments: + - `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'. +## Attributes Reference + +- `username` - The user's current login. When the resource is identified by `user_id`, this attribute tracks the user's live login at refresh time. +- `user_id` - The GitHub numeric user ID. +- `etag` - The etag of the membership object. + ## Import -GitHub Membership can be imported using an ID made up of `organization:username`, e.g. +GitHub Membership can be imported using an ID made up of `organization:user_id`, e.g. ```shell -terraform import github_membership.member hashicorp:someuser +terraform import github_membership.member hashicorp:12345 ``` + +Legacy IDs of the form `organization:username` are still accepted on import and will be migrated to the numeric form on the next refresh.