diff --git a/github/resource_github_team_settings.go b/github/resource_github_team_settings.go index eae86ace2a..a4ca31ccb1 100644 --- a/github/resource_github_team_settings.go +++ b/github/resource_github_team_settings.go @@ -2,9 +2,9 @@ package github import ( "context" - "errors" - "strconv" + "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/shurcooL/githubv4" @@ -12,12 +12,12 @@ import ( func resourceGithubTeamSettings() *schema.Resource { return &schema.Resource{ - Create: resourceGithubTeamSettingsCreate, - Read: resourceGithubTeamSettingsRead, - Update: resourceGithubTeamSettingsUpdate, - Delete: resourceGithubTeamSettingsDelete, + CreateContext: resourceGithubTeamSettingsCreate, + ReadContext: resourceGithubTeamSettingsRead, + UpdateContext: resourceGithubTeamSettingsUpdate, + DeleteContext: resourceGithubTeamSettingsDelete, Importer: &schema.ResourceImporter{ - State: resourceGithubTeamSettingsImport, + StateContext: resourceGithubTeamSettingsImport, }, Schema: map[string]*schema.Schema{ "team_id": { @@ -36,6 +36,13 @@ func resourceGithubTeamSettings() *schema.Resource { Computed: true, Description: "The unique ID of the Team on GitHub. Corresponds to the ID of the 'github_team_settings' resource.", }, + "notify": { + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "Whether to notify the entire team when at least one member is also assigned to the pull request.", + ConflictsWith: []string{"review_request_delegation.0.notify"}, + }, "review_request_delegation": { Type: schema.TypeList, MaxItems: 1, @@ -51,19 +58,21 @@ func resourceGithubTeamSettings() *schema.Resource { ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{string(githubv4.TeamReviewAssignmentAlgorithmRoundRobin), string(githubv4.TeamReviewAssignmentAlgorithmLoadBalance)}, false)), }, "member_count": { - Type: schema.TypeInt, - Optional: true, - RequiredWith: []string{"review_request_delegation"}, - Description: "The number of team members to assign to a pull request.", + Type: schema.TypeInt, + Optional: true, + Default: 1, + Description: "The number of team members to assign to a pull request.", ValidateDiagFunc: validation.ToDiagFunc(validation.All( validation.IntAtLeast(1), )), }, "notify": { - Type: schema.TypeBool, - Optional: true, - Default: false, - Description: "whether to notify the entire team when at least one member is also assigned to the pull request.", + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "whether to notify the entire team when at least one member is also assigned to the pull request.", + Deprecated: "Use the top-level notify attribute instead.", + ConflictsWith: []string{"notify"}, }, }, }, @@ -72,36 +81,96 @@ func resourceGithubTeamSettings() *schema.Resource { } } -func resourceGithubTeamSettingsCreate(d *schema.ResourceData, meta any) error { - err := checkOrganization(meta) - if err != nil { - return err +func resourceGithubTeamSettingsCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + if err := checkOrganization(m); err != nil { + return diag.FromErr(err) } + graphql := meta.v4client + teamIDString := d.Get("team_id").(string) + + tflog.Debug(ctx, "Resolving team_id to Team node_id and slug", map[string]any{ + "team_id": teamIDString, + }) // Given a string that is either a team id or team slug, return the // get the basic details of the team including node_id and slug - ctx := context.Background() - - teamIDString, _ := d.Get("team_id").(string) - - nodeId, slug, err := resolveTeamIDs(teamIDString, meta.(*Owner), ctx) + nodeId, slug, err := resolveTeamIDs(ctx, meta, teamIDString) if err != nil { - return err + return diag.FromErr(err) } + tflog.Trace(ctx, "Resolved team_id to Team node_id and slug", map[string]any{ + "node_id": nodeId, + "slug": slug, + }) d.SetId(nodeId) if err = d.Set("team_slug", slug); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("team_uid", nodeId); err != nil { - return err + return diag.FromErr(err) + } + + reviewRequestDelegation := d.Get("review_request_delegation").([]any) + + var mutation struct { + UpdateTeamReviewAssignment struct { + ClientMutationId githubv4.ID `graphql:"clientMutationId"` + } `graphql:"updateTeamReviewAssignment(input:$input)"` } - return resourceGithubTeamSettingsUpdate(d, meta) + + tflog.Debug(ctx, "Review request delegation settings", map[string]any{ + "team_id": d.Id(), + "team_slug": slug, + "review_request_delegation": reviewRequestDelegation, + "length_of_settings": len(reviewRequestDelegation), + }) + + notify := resolveNotify(ctx, d) + + if len(reviewRequestDelegation) == 0 { + tflog.Debug(ctx, "No review request delegation settings provided, disabling review request delegation", map[string]any{ + "team_id": d.Id(), + "team_slug": slug, + "notify": notify, + }) + + input := defaultTeamReviewAssignmentSettings(d.Id()) + input.NotifyTeam = githubv4.NewBoolean(githubv4.Boolean(notify)) + + err := graphql.Mutate(ctx, &mutation, input, nil) + if err != nil { + return diag.FromErr(err) + } + } else { + tflog.Debug(ctx, "Review request delegation settings provided, setting according to provided configuration", map[string]any{ + "team_id": d.Id(), + "team_slug": slug, + "review_request_delegation": reviewRequestDelegation, + "notify": notify, + }) + settings := reviewRequestDelegation[0].(map[string]any) + + teamReviewAlgorithm := githubv4.TeamReviewAssignmentAlgorithm(settings["algorithm"].(string)) + updateTeamReviewAssignmentInput := githubv4.UpdateTeamReviewAssignmentInput{ + ID: d.Id(), + Enabled: githubv4.Boolean(true), + Algorithm: &teamReviewAlgorithm, + TeamMemberCount: githubv4.NewInt(githubv4.Int(settings["member_count"].(int))), + NotifyTeam: githubv4.NewBoolean(githubv4.Boolean(notify)), + } + + err := graphql.Mutate(ctx, &mutation, updateTeamReviewAssignmentInput, nil) + if err != nil { + return diag.FromErr(err) + } + } + return nil } -func resourceGithubTeamSettingsRead(d *schema.ResourceData, meta any) error { - err := checkOrganization(meta) - if err != nil { - return err +func resourceGithubTeamSettingsRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + if err := checkOrganization(meta); err != nil { + return diag.FromErr(err) } graphql := meta.(*Owner).v4client @@ -115,66 +184,95 @@ func resourceGithubTeamSettingsRead(d *schema.ResourceData, meta any) error { "login": githubv4.String(orgName), } - e := graphql.Query(meta.(*Owner).StopContext, &query, variables) - if e != nil { - return e + err := graphql.Query(ctx, &query, variables) + if err != nil { + return diag.FromErr(err) + } + + notifyValue := query.Organization.Team.ReviewRequestDelegationNotifyAll + + // Set notify in the location matching the user's config: top-level or + // deprecated nested field inside review_request_delegation. + _, usesDeprecatedNotify := d.GetOk("review_request_delegation.0.notify") + tflog.Debug(ctx, "Uses deprecated notify", map[string]any{ + "uses_deprecated_notify": usesDeprecatedNotify, + "notify_value": notifyValue, + }) + + if !usesDeprecatedNotify { + if err = d.Set("notify", notifyValue); err != nil { + return diag.FromErr(err) + } } if query.Organization.Team.ReviewRequestDelegation { reviewRequestDelegation := make(map[string]any) reviewRequestDelegation["algorithm"] = query.Organization.Team.ReviewRequestDelegationAlgorithm reviewRequestDelegation["member_count"] = query.Organization.Team.ReviewRequestDelegationCount - reviewRequestDelegation["notify"] = query.Organization.Team.ReviewRequestDelegationNotifyAll + if usesDeprecatedNotify { + reviewRequestDelegation["notify"] = notifyValue + } if err = d.Set("review_request_delegation", []any{reviewRequestDelegation}); err != nil { - return err + return diag.FromErr(err) } } else { if err = d.Set("review_request_delegation", []any{}); err != nil { - return err + return diag.FromErr(err) } } return nil } -func resourceGithubTeamSettingsUpdate(d *schema.ResourceData, meta any) error { - if d.HasChange("review_request_delegation") || d.IsNewResource() { +func resourceGithubTeamSettingsUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + if d.HasChange("review_request_delegation") || d.HasChange("notify") { + meta := m.(*Owner) + graphql := meta.v4client + reviewRequestDelegation := d.Get("review_request_delegation").([]any) + notify := resolveNotify(ctx, d) - ctx := context.WithValue(context.Background(), ctxId, d.Id()) - graphql := meta.(*Owner).v4client - if setting := d.Get("review_request_delegation").([]any); len(setting) == 0 { - var mutation struct { - UpdateTeamReviewAssignment struct { - ClientMutationId githubv4.ID `graphql:"clientMutationId"` - } `graphql:"updateTeamReviewAssignment(input:$input)"` - } + var mutation struct { + UpdateTeamReviewAssignment struct { + ClientMutationId githubv4.ID `graphql:"clientMutationId"` + } `graphql:"updateTeamReviewAssignment(input:$input)"` + } - return graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil) - } else { - settings := d.Get("review_request_delegation").([]any)[0].(map[string]any) + if len(reviewRequestDelegation) == 0 { + tflog.Debug(ctx, "No review request delegation settings provided, disabling review request delegation", map[string]any{ + "team_id": d.Id(), + "team_slug": d.Get("team_slug").(string), + "notify": notify, + }) + + input := defaultTeamReviewAssignmentSettings(d.Id()) + input.NotifyTeam = githubv4.NewBoolean(githubv4.Boolean(notify)) - var mutation struct { - UpdateTeamReviewAssignment struct { - ClientMutationId githubv4.ID `graphql:"clientMutationId"` - } `graphql:"updateTeamReviewAssignment(input:$input)"` + err := graphql.Mutate(ctx, &mutation, input, nil) + if err != nil { + return diag.FromErr(err) } + } else { + settings := reviewRequestDelegation[0].(map[string]any) teamReviewAlgorithm := githubv4.TeamReviewAssignmentAlgorithm(settings["algorithm"].(string)) - return graphql.Mutate(ctx, &mutation, githubv4.UpdateTeamReviewAssignmentInput{ + updateTeamReviewAssignmentInput := githubv4.UpdateTeamReviewAssignmentInput{ ID: d.Id(), Enabled: githubv4.Boolean(true), Algorithm: &teamReviewAlgorithm, TeamMemberCount: githubv4.NewInt(githubv4.Int(settings["member_count"].(int))), - NotifyTeam: githubv4.NewBoolean(githubv4.Boolean(settings["notify"].(bool))), - }, nil) + NotifyTeam: githubv4.NewBoolean(githubv4.Boolean(notify)), + } + err := graphql.Mutate(ctx, &mutation, updateTeamReviewAssignmentInput, nil) + if err != nil { + return diag.FromErr(err) + } } } - return resourceGithubTeamSettingsRead(d, meta) + return nil } -func resourceGithubTeamSettingsDelete(d *schema.ResourceData, meta any) error { - ctx := context.WithValue(context.Background(), ctxId, d.Id()) +func resourceGithubTeamSettingsDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { graphql := meta.(*Owner).v4client var mutation struct { @@ -183,11 +281,15 @@ func resourceGithubTeamSettingsDelete(d *schema.ResourceData, meta any) error { } `graphql:"updateTeamReviewAssignment(input:$input)"` } - return graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil) + err := graphql.Mutate(ctx, &mutation, defaultTeamReviewAssignmentSettings(d.Id()), nil) + if err != nil { + return diag.FromErr(err) + } + return nil } -func resourceGithubTeamSettingsImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { - nodeId, slug, err := resolveTeamIDs(d.Id(), meta.(*Owner), context.Background()) +func resourceGithubTeamSettingsImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + nodeId, slug, err := resolveTeamIDs(ctx, meta.(*Owner), d.Id()) if err != nil { return nil, err } @@ -201,37 +303,39 @@ func resourceGithubTeamSettingsImport(d *schema.ResourceData, meta any) ([]*sche if err = d.Set("team_uid", nodeId); err != nil { return nil, err } - return []*schema.ResourceData{d}, resourceGithubTeamSettingsRead(d, meta) + return []*schema.ResourceData{d}, nil } -func resolveTeamIDs(idOrSlug string, meta *Owner, ctx context.Context) (nodeId, slug string, err error) { - client := meta.v3client - orgName := meta.name - orgId := meta.id - - teamId, parseIntErr := strconv.ParseInt(idOrSlug, 10, 64) - if parseIntErr != nil { - // The given id not an integer, assume it is a team slug - team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, idOrSlug) - if slugErr != nil { - return "", "", errors.New(parseIntErr.Error() + slugErr.Error()) - } - return team.GetNodeID(), team.GetSlug(), nil - } else { - // The given id is an integer, assume it is a team id - team, _, teamIdErr := client.Teams.GetTeamByID(ctx, orgId, teamId) - if teamIdErr != nil { - // There isn't a team with the given ID, assume it is a teamslug - team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, idOrSlug) - if slugErr != nil { - return "", "", errors.New(teamIdErr.Error() + slugErr.Error()) - } +func resolveTeamIDs(ctx context.Context, meta *Owner, idOrSlug string) (nodeId, slug string, err error) { + team, err := getTeam(ctx, meta, idOrSlug) + if err != nil { + return "", "", err + } + return team.GetNodeID(), team.GetSlug(), nil +} - return team.GetNodeID(), team.GetSlug(), nil - } +// resolveNotify returns the notify value from the top-level attribute or the +// deprecated nested attribute inside review_request_delegation. The top-level +// attribute takes precedence. Since ConflictsWith prevents both from being set, +// only one source can be active at a time. +func resolveNotify(ctx context.Context, d *schema.ResourceData) bool { + // Check if top-level notify is explicitly configured. + if v, ok := d.GetOk("notify"); ok { + tflog.Debug(ctx, "Top-level notify is explicitly configured", map[string]any{ + "notify": v, + }) + return v.(bool) + } - return team.GetNodeID(), team.GetSlug(), nil + // Fall back to deprecated nested field + if v, ok := d.GetOk("review_request_delegation.0.notify"); ok { + tflog.Debug(ctx, "Deprecated nested notify is explicitly configured", map[string]any{ + "notify": v, + }) + return v.(bool) } + + return false } func defaultTeamReviewAssignmentSettings(id string) githubv4.UpdateTeamReviewAssignmentInput { diff --git a/github/resource_github_team_settings_test.go b/github/resource_github_team_settings_test.go index fb3276b198..268d89e993 100644 --- a/github/resource_github_team_settings_test.go +++ b/github/resource_github_team_settings_test.go @@ -1,13 +1,20 @@ package github import ( + "context" "fmt" "regexp" "strings" "testing" + "github.com/hashicorp/terraform-plugin-testing/compare" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/statecheck" + "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" "github.com/shurcooL/githubv4" ) @@ -26,23 +33,30 @@ func TestAccGithubTeamSettings(t *testing.T) { } `, teamName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("github_team_settings.test", "team_id"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("team_id"), knownvalue.NotNull()), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("team_slug"), knownvalue.NotNull()), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("team_uid"), knownvalue.NotNull()), + }, }, { Config: strings.Replace(config, `github_team.test.id`, `github_team.test.slug`, 1), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("github_team_settings.test", "team_id"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("team_id"), knownvalue.NotNull()), + }, }, }, }) @@ -68,33 +82,42 @@ func TestAccGithubTeamSettings(t *testing.T) { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, teamName, testAlgorithm, 1, true), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_team_settings.test", "review_request_delegation.0.algorithm", string(testAlgorithm)), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact(string(testAlgorithm))), + }, }, { Config: fmt.Sprintf(config, teamName, githubv4.TeamReviewAssignmentAlgorithmLoadBalance, 1, true), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_team_settings.test", "review_request_delegation.0.algorithm", string(githubv4.TeamReviewAssignmentAlgorithmLoadBalance)), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact(string(githubv4.TeamReviewAssignmentAlgorithmLoadBalance))), + }, }, { Config: fmt.Sprintf(config, teamName, testAlgorithm, 3, true), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_team_settings.test", "review_request_delegation.0.member_count", "3"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("member_count"), + knownvalue.Int64Exact(3)), + }, }, { Config: fmt.Sprintf(config, teamName, testAlgorithm, 3, false), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_team_settings.test", "review_request_delegation.0.notify", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("notify"), + knownvalue.Bool(false)), + }, }, }, }) @@ -119,9 +142,10 @@ func TestAccGithubTeamSettings(t *testing.T) { } `, teamName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, Steps: []resource.TestStep{ { Config: config, @@ -130,4 +154,574 @@ func TestAccGithubTeamSettings(t *testing.T) { }, }) }) + + t.Run("manages removing review request delegation", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + configWithDelegation := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + review_request_delegation { + algorithm = "ROUND_ROBIN" + member_count = 1 + notify = true + } + } + `, teamName) + + configWithoutDelegation := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + } + `, teamName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: configWithDelegation, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact("ROUND_ROBIN")), + }, + }, + { + Config: configWithoutDelegation, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation"), + knownvalue.ListExact([]knownvalue.Check{})), + }, + }, + { + Config: configWithDelegation, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact("ROUND_ROBIN")), + }, + }, + }, + }) + }) + + t.Run("creates_with_empty_review_request_delegation_block_without_error", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + config := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + review_request_delegation {} + } + `, teamName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), knownvalue.StringExact(string(githubv4.TeamReviewAssignmentAlgorithmRoundRobin))), + statecheck.ExpectKnownValue("github_team_settings.test", tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("notify"), knownvalue.Bool(false)), + }, + }, + }, + }) + }) + t.Run("validates_member_count_greater_than_0", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + config := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + review_request_delegation { + member_count = 0 + } + } + `, teamName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`expected member_count to be at least \(1\), got 0`), + }, + }, + }) + }) + + t.Run("imports_team_settings_without_delegation_using_team_id", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + config := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + } + `, teamName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.CompareValuePairs("github_team_settings.test", tfjsonpath.New("team_id"), "github_team.test", tfjsonpath.New("id"), compare.ValuesSame()), + }, + }, + { + ResourceName: "github_team_settings.test", + ImportState: true, + ImportStateVerify: true, + ImportStateIdFunc: func(s *terraform.State) (string, error) { + rs, ok := s.RootModule().Resources["github_team.test"] + if !ok { + return "", fmt.Errorf("not found: github_team.test") + } + return rs.Primary.ID, nil + }, + }, + }, + }) + }) + t.Run("imports_team_settings_without_delegation_using_team_slug", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + config := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = github_team.test.slug + } + `, teamName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.CompareValuePairs("github_team_settings.test", tfjsonpath.New("team_id"), "github_team.test", tfjsonpath.New("slug"), compare.ValuesSame()), + }, + }, + { + ResourceName: "github_team_settings.test", + ImportState: true, + ImportStateVerify: true, + ImportStateIdFunc: func(s *terraform.State) (string, error) { + rs, ok := s.RootModule().Resources["github_team.test"] + if !ok { + return "", fmt.Errorf("not found: github_team.test") + } + return rs.Primary.Attributes["slug"], nil + }, + }, + }, + }) + }) + + t.Run("imports team settings with delegation", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + config := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = github_team.test.id + notify = true + review_request_delegation { + algorithm = "ROUND_ROBIN" + member_count = 2 + } + } + `, teamName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact("ROUND_ROBIN")), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("member_count"), + knownvalue.Int64Exact(2)), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("notify"), + knownvalue.Bool(false)), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("notify"), + knownvalue.Bool(true)), + }, + }, + { + ResourceName: "github_team_settings.test", + ImportStateKind: resource.ImportBlockWithID, + ImportState: true, + ImportPlanChecks: resource.ImportPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectKnownValue("github_team_settings.test", tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("notify"), knownvalue.Bool(false)), + plancheck.ExpectKnownValue("github_team_settings.test", tfjsonpath.New("notify"), knownvalue.Bool(true)), + }, + }, + ImportStateIdFunc: func(s *terraform.State) (string, error) { + rs, ok := s.RootModule().Resources["github_team.test"] + if !ok { + return "", fmt.Errorf("not found: github_team.test") + } + return rs.Primary.Attributes["id"], nil + }, + }, + }, + }) + }) + + t.Run("manages adding review request delegation to existing team settings", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + configWithoutDelegation := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + } + `, teamName) + + configWithDelegation := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + review_request_delegation { + algorithm = "LOAD_BALANCE" + member_count = 2 + notify = true + } + } + `, teamName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: configWithoutDelegation, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation"), + knownvalue.ListExact([]knownvalue.Check{})), + }, + }, + { + Config: configWithDelegation, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact("LOAD_BALANCE")), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("member_count"), + knownvalue.Int64Exact(2)), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("notify"), + knownvalue.Bool(true)), + }, + }, + }, + }) + }) + + t.Run("manages top-level notify without delegation", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + config := ` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + notify = %t + } + ` + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, teamName, true), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("notify"), + knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation"), + knownvalue.ListExact([]knownvalue.Check{})), + }, + }, + { + Config: fmt.Sprintf(config, teamName, false), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("notify"), + knownvalue.Bool(false)), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation"), + knownvalue.ListExact([]knownvalue.Check{})), + }, + }, + }, + }) + }) + + t.Run("manages top-level notify with delegation", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + config := ` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + notify = %t + review_request_delegation { + algorithm = "ROUND_ROBIN" + member_count = 2 + } + } + ` + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, teamName, true), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("notify"), + knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact("ROUND_ROBIN")), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("member_count"), + knownvalue.Int64Exact(2)), + }, + }, + { + Config: fmt.Sprintf(config, teamName, false), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("notify"), + knownvalue.Bool(false)), + }, + }, + }, + }) + }) + + t.Run("imports team settings with top-level notify", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + config := fmt.Sprintf(` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = github_team.test.id + notify = true + } + `, teamName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("notify"), + knownvalue.Bool(true)), + }, + }, + { + ResourceName: "github_team_settings.test", + ImportState: true, + ImportStateVerify: true, + ImportStateIdFunc: func(s *terraform.State) (string, error) { + rs, ok := s.RootModule().Resources["github_team.test"] + if !ok { + return "", fmt.Errorf("not found: github_team.test") + } + return rs.Primary.Attributes["id"], nil + }, + }, + }, + }) + }) + + t.Run("manages updating only notify field", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-settings-%s", testResourcePrefix, randomID) + + config := ` + resource "github_team" "test" { + name = "%s" + description = "generated by terraform provider automated testing" + } + + resource "github_team_settings" "test" { + team_id = "${github_team.test.id}" + review_request_delegation { + algorithm = "ROUND_ROBIN" + member_count = 1 + notify = %t + } + } + ` + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + CheckDestroy: testAccCheckGithubTeamSettingsDestroy, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, teamName, true), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("notify"), + knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact("ROUND_ROBIN")), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("member_count"), + knownvalue.Int64Exact(1)), + }, + }, + { + Config: fmt.Sprintf(config, teamName, false), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("notify"), + knownvalue.Bool(false)), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("algorithm"), + knownvalue.StringExact("ROUND_ROBIN")), + statecheck.ExpectKnownValue("github_team_settings.test", + tfjsonpath.New("review_request_delegation").AtSliceIndex(0).AtMapKey("member_count"), + knownvalue.Int64Exact(1)), + }, + }, + }, + }) + }) +} + +func testAccCheckGithubTeamSettingsDestroy(s *terraform.State) error { + meta, err := getTestMeta() + if err != nil { + return err + } + graphql := meta.v4client + orgName := meta.name + + for _, rs := range s.RootModule().Resources { + if rs.Type != "github_team_settings" { + continue + } + + teamSlug := rs.Primary.Attributes["team_slug"] + if teamSlug == "" { + continue + } + + query := queryTeamSettings{} + variables := map[string]any{ + "slug": githubv4.String(teamSlug), + "login": githubv4.String(orgName), + } + + err := graphql.Query(context.Background(), &query, variables) + if err != nil { + // Team itself may have been destroyed, which is fine + continue + } + + if query.Organization.Team.ReviewRequestDelegation { + return fmt.Errorf("team %s still has review request delegation enabled", teamSlug) + } + } + return nil } diff --git a/github/util.go b/github/util.go index 0d9bc0d32c..68f946da93 100644 --- a/github/util.go +++ b/github/util.go @@ -240,34 +240,44 @@ func getTeamSlug(teamIDString string, meta any) (string, error) { return getTeamSlugContext(ctx, teamIDString, meta) } -func getTeamSlugContext(ctx context.Context, teamIDString string, meta any) (string, error) { +func getTeamSlugContext(ctx context.Context, teamIDString string, m any) (string, error) { // Given a string that is either a team id or team slug, return the // team slug it is referring to. - client := meta.(*Owner).v3client - orgName := meta.(*Owner).name - orgId := meta.(*Owner).id + meta := m.(*Owner) - teamId, parseIntErr := strconv.ParseInt(teamIDString, 10, 64) + team, err := getTeam(ctx, meta, teamIDString) + if err != nil { + return "", err + } + return team.GetSlug(), nil +} + +func getTeam(ctx context.Context, meta *Owner, idOrSlug string) (*github.Team, error) { + client := meta.v3client + orgName := meta.name + orgId := meta.id + + teamId, parseIntErr := strconv.ParseInt(idOrSlug, 10, 64) if parseIntErr != nil { // The given id not an integer, assume it is a team slug - team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, teamIDString) + team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, idOrSlug) if slugErr != nil { - return "", errors.New(parseIntErr.Error() + slugErr.Error()) + return nil, errors.New(parseIntErr.Error() + slugErr.Error()) } - return team.GetSlug(), nil + return team, nil } // The given id is an integer, assume it is a team id team, _, teamIdErr := client.Teams.GetTeamByID(ctx, orgId, teamId) if teamIdErr != nil { // There isn't a team with the given ID, assume it is a teamslug - team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, teamIDString) + team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, idOrSlug) if slugErr != nil { - return "", errors.New(teamIdErr.Error() + slugErr.Error()) + return nil, errors.New(teamIdErr.Error() + slugErr.Error()) } - return team.GetSlug(), nil + return team, nil } - return team.GetSlug(), nil + return team, nil } // https://docs.github.com/en/actions/reference/encrypted-secrets#naming-your-secrets diff --git a/website/docs/r/team_settings.html.markdown b/website/docs/r/team_settings.html.markdown index 318062904e..c09d8ad051 100644 --- a/website/docs/r/team_settings.html.markdown +++ b/website/docs/r/team_settings.html.markdown @@ -11,25 +11,40 @@ This resource manages the team settings (in particular the request review delega Creating this resource will alter the team Code Review settings. -The team must both belong to the same organization configured in the provider on GitHub. +The team must both belong to the same organization configured in the provider on GitHub. ~> **Note**: This resource relies on the v4 GraphQl GitHub API. If this API is not available, or the Stone Crop schema preview is not available, then this resource will not work as intended. ## Example Usage +### Notify without delegation + ```hcl -# Add a repository to the team resource "github_team" "some_team" { name = "SomeTeam" description = "Some cool team" } resource "github_team_settings" "code_review_settings" { - team_id = github_team.some_team.id + team_id = github_team.some_team.id + notify = true +} +``` + +### Notify with delegation + +```hcl +resource "github_team" "some_team" { + name = "SomeTeam" + description = "Some cool team" +} + +resource "github_team_settings" "code_review_settings" { + team_id = github_team.some_team.id + notify = true review_request_delegation { - algorithm = "ROUND_ROBIN" - member_count = 1 - notify = true + algorithm = "ROUND_ROBIN" + member_count = 1 } } ``` @@ -38,26 +53,35 @@ resource "github_team_settings" "code_review_settings" { The following arguments are supported: -* `team_id` - (Required) The GitHub team id or the GitHub team slug -* `review_request_delegation` - (Optional) The settings for delegating code reviews to individuals on behalf of the team. If this block is present, even without any fields, then review request delegation will be enabled for the team. See [GitHub Review Request Delegation](#github-review-request-delegation-configuration) below for details. See [GitHub's documentation](https://docs.github.com/en/organizations/organizing-members-into-teams/managing-code-review-settings-for-your-team#configuring-team-notifications) for more configuration details. +- `team_id` - (Required) The GitHub team id or the GitHub team slug +- `notify` - (Optional) Whether to notify the entire team when at least one member is also assigned to the pull request. Can be set independently of `review_request_delegation`. Default value is `false`. +- `review_request_delegation` - (Optional) The settings for delegating code reviews to individuals on behalf of the team. If this block is present, even without any fields, then review request delegation will be enabled for the team. See [GitHub Review Request Delegation](#github-review-request-delegation-configuration) below for details. See [GitHub's documentation](https://docs.github.com/en/organizations/organizing-members-into-teams/managing-code-review-settings-for-your-team#configuring-team-notifications) for more configuration details. ### GitHub Review Request Delegation Configuration The following arguments are supported: -* `algorithm` - (Optional) The algorithm to use when assigning pull requests to team members. Supported values are `ROUND_ROBIN` and `LOAD_BALANCE`. Default value is `ROUND_ROBIN` -* `member_count` - (Optional) The number of team members to assign to a pull request -* `notify` - (Optional) whether to notify the entire team when at least one member is also assigned to the pull request +- `algorithm` - (Optional) The algorithm to use when assigning pull requests to team members. Supported values are `ROUND_ROBIN` and `LOAD_BALANCE`. Default value is `ROUND_ROBIN` +- `member_count` - (Optional) The number of team members to assign to a pull request. Default value is `1`. +- `notify` - (Optional, **Deprecated**: Use the top-level `notify` attribute instead.) Whether to notify the entire team when at least one member is also assigned to the pull request. Conflicts with the top-level `notify` attribute. + +## Attributes Reference +The following additional attributes are exported: + +- `team_slug` - The slug of the Team. +- `team_uid` - The unique node ID of the Team on GitHub. Corresponds to the ID of the `github_team_settings` resource. ## Import GitHub Teams can be imported using the GitHub team ID, or the team slug e.g. +```text +terraform import github_team_settings.code_review_settings 1234567 ``` -$ terraform import github_team.code_review_settings 1234567 -``` + or, + +```text +terraform import github_team_settings.code_review_settings SomeTeam ``` -$ terraform import github_team_settings.code_review_settings SomeTeam -``` \ No newline at end of file