bot: Update isInternal check to include cloud and core maps

bot/internal/bot/backport_test.go

@@ -43,15 +43,15 @@ func TestFindBranches(t *testing.T) {
 
 func TestBackport(t *testing.T) {
 	buildTestBot := func(github Client) (*Bot, context.Context) {
-		r, _ := review.New(&review.Config{
-			CodeReviewers: map[string]review.Reviewer{"dev": review.Reviewer{
-				Team: "core",
-			}},
+		c := &review.Config{
+			CodeReviewers:     map[string]review.Reviewer{"dev": review.Reviewer{}},
 			CodeReviewersOmit: map[string]bool{},
 			DocsReviewers:     map[string]review.Reviewer{},
 			DocsReviewersOmit: map[string]bool{},
 			Admins:            []string{},
-		})
+		}
+		c.CoreReviewers = c.CodeReviewers
+		r, _ := review.New(c)
 
 		return &Bot{
 			c: &Config{

bot/internal/bot/bot_test.go

@@ -258,13 +258,15 @@ func TestIsInternal(t *testing.T) {
 			rc := &review.Config{
 				Admins:            []string{},
 				CodeReviewers:     make(map[string]review.Reviewer),
+				CoreReviewers:     make(map[string]review.Reviewer),
 				CodeReviewersOmit: map[string]bool{},
 				DocsReviewers:     make(map[string]review.Reviewer),
 				DocsReviewersOmit: make(map[string]bool),
 			}
 			for _, cr := range test.codeReviewers {
 				rc.CodeReviewers[cr] = review.Reviewer{}
 			}
+			rc.CoreReviewers = rc.CodeReviewers
 			for _, dr := range test.docsReviewers {
 				rc.DocsReviewers[dr] = review.Reviewer{}
 			}

bot/internal/bot/check_test.go

@@ -119,7 +119,7 @@ func TestDismissUnnecessaryReviewers(t *testing.T) {
 		},
 	} {
 		t.Run(test.desc, func(t *testing.T) {
-			a, err := review.New(&review.Config{
+			c := &review.Config{
 				Admins: []string{"admin1"},
 				CodeReviewers: map[string]review.Reviewer{
 					"user1": {},
@@ -130,7 +130,9 @@ func TestDismissUnnecessaryReviewers(t *testing.T) {
 				CodeReviewersOmit: make(map[string]bool),
 				DocsReviewers:     make(map[string]review.Reviewer),
 				DocsReviewersOmit: make(map[string]bool),
-			})
+			}
+			c.CoreReviewers = c.CodeReviewers
+			a, err := review.New(c)
 			require.NoError(t, err)
 
 			b := &Bot{

bot/internal/review/review.go

@@ -18,7 +18,6 @@ package review
 
 import (
 	"encoding/json"
-	"fmt"
 	"log"
 	"math/rand"
 	"sort"
@@ -86,13 +85,6 @@ func isAllowedRobot(author string) bool {
 
 // Reviewer is a code reviewer.
 type Reviewer struct {
-	// Team the reviewer belongs to. This field is set when loading the configuration file
-	// based on whether the section being loaded is either CoreReviewers or CloudReviewers.
-	// DocsReviewers is automatically set to Core.
-	//
-	// Deprecated: we should remove the need for this field and detect by environment now
-	// that the teams have been split into their own sections in the configuration file.
-	Team string `json:"team"`
 	// Owner is true if the reviewer is a code or docs owner (required for all reviews).
 	Owner bool `json:"owner"`
 	// PreferredReviewerFor contains a list of file paths that this reviewer
@@ -181,17 +173,14 @@ func FromString(e *env.Environment, reviewers string) (*Assignments, error) {
 	if len(c.CodeReviewers) == 0 { // allow this change to deploy before updating the config file
 		switch e.RepoOwnerTeam() {
 		case env.CloudTeam:
-			c.CodeReviewers = setReviewerTeam(env.CloudTeam, c.CloudReviewers)
+			c.CodeReviewers = c.CloudReviewers
 		case env.CoreTeam:
-			c.CodeReviewers = setReviewerTeam(env.CoreTeam, c.CoreReviewers)
+			c.CodeReviewers = c.CoreReviewers
 		default:
 			return nil, trace.BadParameter("unable to detect code reviewers due to invalid team: %s", e.RepoOwnerTeam())
 		}
 	}
 
-	// team for all docs reviewers should be set to Core
-	c.DocsReviewers = setReviewerTeam(env.CoreTeam, c.DocsReviewers)
-
 	r, err := New(&c)
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -218,9 +207,10 @@ func (r *Assignments) IsInternal(author string) bool {
 		return true
 	}
 
-	_, code := r.c.CodeReviewers[author]
+	_, core := r.c.CoreReviewers[author]
+	_, cloud := r.c.CloudReviewers[author]
 	_, docs := r.c.DocsReviewers[author]
-	return code || docs
+	return core || cloud || docs
 }
 
 // Get will return a list of code reviewers for a given author.
@@ -257,12 +247,12 @@ func (r *Assignments) getReleaseReviewers() []string {
 func (r *Assignments) getDocsReviewers(e *env.Environment, files []github.PullRequestFile) []string {
 	// See if any code reviewers are designated preferred reviewers for one of
 	// the changed docs files. If so, add them as docs reviewers.
-	a, b := getReviewerSets(e.Author, "Core", r.c.CodeReviewers, r.c.CodeReviewersOmit)
+	a, b := getReviewerSets(e.Author, r.c.CodeReviewers, r.c.CodeReviewersOmit)
 	prefCodeReviewers := r.getAllPreferredReviewers(append(a, b...), files)
 
 	// Get the docs reviewer pool, which does not depend on the files
 	// changed by a pull request.
-	docsA, docsB := getReviewerSets(e.Author, "Core", r.c.DocsReviewers, r.c.DocsReviewersOmit)
+	docsA, docsB := getReviewerSets(e.Author, r.c.DocsReviewers, r.c.DocsReviewersOmit)
 	reviewers := append(prefCodeReviewers, append(docsA, docsB...)...)
 
 	// If no docs reviewers were assigned, assign admin reviews.
@@ -385,24 +375,12 @@ func (r *Assignments) getAdminReviewers(author string) []string {
 func (r *Assignments) getCodeReviewerSets(e *env.Environment) ([]string, []string) {
 	// Internal non-Core contributors get assigned from the admin reviewer set.
 	// Admins will review, triage, and re-assign.
-	v, ok := r.c.CodeReviewers[e.Author]
-	if !ok || v.Team == env.InternalTeam {
+	if !r.IsInternal(e.Author) {
 		reviewers := r.getAdminReviewers(e.Author)
 		n := len(reviewers) / 2
 		return reviewers[:n], reviewers[n:]
 	}
-
-	team := v.Team
-
-	// Teams do their own internal reviews
-	switch e.Repository {
-	case env.TeleportRepo:
-		team = env.CoreTeam
-	case env.CloudRepo:
-		team = env.CloudTeam
-	}
-
-	return getReviewerSets(e.Author, team, r.c.CodeReviewers, r.c.CodeReviewersOmit)
+	return getReviewerSets(e.Author, r.c.CodeReviewers, r.c.CodeReviewersOmit)
 }
 
 // CheckExternal requires two admins have approved.
@@ -502,18 +480,7 @@ func (r *Assignments) checkInternalDocsReviews(e *env.Environment, reviews []git
 // checkInternalCodeReviews checks whether code review requirements are satisfied
 // for a PR authored by an internal employee
 func (r *Assignments) checkInternalCodeReviews(e *env.Environment, changes env.Changes, reviews []github.Review) error {
-	// Teams do their own internal reviews
-	var team string
-	switch e.Repository {
-	case env.TeleportRepo, env.TeleportERepo:
-		team = env.CoreTeam
-	case env.CloudRepo:
-		team = env.CloudTeam
-	default:
-		return trace.Wrap(fmt.Errorf("unsupported repository: %s", e.Repository))
-	}
-
-	setA, setB := getReviewerSets(e.Author, team, r.c.CodeReviewers, r.c.CodeReviewersOmit)
+	setA, setB := getReviewerSets(e.Author, r.c.CodeReviewers, r.c.CodeReviewersOmit)
 
 	// PRs can be approved if you either have multiple code owners that approve
 	// or code owner and code reviewer. An exception is for PRs that
@@ -544,15 +511,11 @@ func (r *Assignments) GetAdminCheckers(author string) []string {
 	return reviewers
 }
 
-func getReviewerSets(author string, team string, reviewers map[string]Reviewer, reviewersOmit map[string]bool) ([]string, []string) {
+func getReviewerSets(author string, reviewers map[string]Reviewer, reviewersOmit map[string]bool) ([]string, []string) {
 	var setA []string
 	var setB []string
 
 	for k, v := range reviewers {
-		// Only assign within a team.
-		if v.Team != team {
-			continue
-		}
 		// Skip over reviewers that are marked as omit.
 		if _, ok := reviewersOmit[k]; ok {
 			continue
@@ -605,19 +568,6 @@ func reviewsByAuthor(reviews []github.Review) map[string]string {
 	return m
 }
 
-// setReviewerTeam sets the Team field in each Reviewer of the reviewers map.
-//
-// NOTE: This is a hack so the team field doesn't need to be included in the config file.
-// Eventually we should remove the team field and use the environment since the config file
-// now separates cloud and core engineers.
-func setReviewerTeam(team string, reviewers map[string]Reviewer) map[string]Reviewer {
-	for name, r := range reviewers {
-		r.Team = team
-		reviewers[name] = r
-	}
-	return reviewers
-}
-
 const (
 	// Commented is a code review where the reviewer has left comments only.
 	Commented = "COMMENTED"