From d85387b594e8543336afa9a6f9107a886c492af9 Mon Sep 17 00:00:00 2001 From: Jim Bishopp Date: Tue, 26 Dec 2023 15:20:41 -0800 Subject: [PATCH] Allow the same reviewer in both core and cloud repos --- bot/internal/env/env.go | 8 ++++++++ bot/internal/review/review.go | 18 +++++++++++++++++- bot/internal/review/review_test.go | 15 +++++++++++++-- bot/main.go | 2 +- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/bot/internal/env/env.go b/bot/internal/env/env.go index 63b0fd72..7c665ad2 100644 --- a/bot/internal/env/env.go +++ b/bot/internal/env/env.go @@ -132,6 +132,14 @@ func (e *Environment) IsCloudDeployBranch() bool { (e.UnsafeBase == CloudProdBranch || e.UnsafeBase == CloudStagingBranch) } +// Team returns CloudTeam when the repository is the cloud repo otherwise CoreTeam. +func (e *Environment) Team() string { + if e.Repository == CloudRepo { + return CloudTeam + } + return CoreTeam +} + func readEvent() (*Event, error) { f, err := os.Open(os.Getenv(githubEventPath)) if err != nil { diff --git a/bot/internal/review/review.go b/bot/internal/review/review.go index 02ff2cf2..1e37dee1 100644 --- a/bot/internal/review/review.go +++ b/bot/internal/review/review.go @@ -109,6 +109,8 @@ type Config struct { // CodeReviewers and CodeReviewersOmit is a map of code reviews and code // reviewers to omit. CodeReviewers map[string]Reviewer `json:"codeReviewers"` + CoreReviewers map[string]Reviewer `json:"coreReviewers"` + CloudReviewers map[string]Reviewer `json:"cloudReviewers"` CodeReviewersOmit map[string]bool `json:"codeReviewersOmit"` // DocsReviewers and DocsReviewersOmit is a map of docs reviews and docs @@ -156,12 +158,26 @@ type Assignments struct { } // FromString parses JSON formatted configuration and returns assignments. -func FromString(reviewers string) (*Assignments, error) { +func FromString(e *env.Environment, reviewers string) (*Assignments, error) { var c Config if err := json.Unmarshal([]byte(reviewers), &c); err != nil { return nil, trace.Wrap(err) } + // hacky way of allowing the same reviewer to be defined in + // both cloud and core repos without having to change the + // bot in a large number of places. + if len(c.CodeReviewers) == 0 { // allow this change to deploy before updating the config file + switch e.Team() { + case env.CloudTeam: + c.CodeReviewers = c.CloudReviewers + case env.CoreTeam: + c.CodeReviewers = c.CoreReviewers + default: + return nil, trace.BadParameter("unable to detect code reviewers due to invalid team: %s", e.Team()) + } + } + r, err := New(&c) if err != nil { return nil, trace.Wrap(err) diff --git a/bot/internal/review/review_test.go b/bot/internal/review/review_test.go index fb08c14e..7b5e967f 100644 --- a/bot/internal/review/review_test.go +++ b/bot/internal/review/review_test.go @@ -1074,7 +1074,8 @@ func TestCheckInternal(t *testing.T) { // TestFromString tests if configuration is correctly read in from a string. func TestFromString(t *testing.T) { - r, err := FromString(reviewers) + e := &env.Environment{Repository: env.TeleportRepo} + r, err := FromString(e, reviewers) require.NoError(t, err) require.EqualValues(t, r.c.CodeReviewers, map[string]Reviewer{ @@ -1194,7 +1195,7 @@ func TestPreferredReviewers(t *testing.T) { const reviewers = ` { - "codeReviewers": { + "coreReviewers": { "1": { "team": "Core", "owner": true @@ -1204,6 +1205,16 @@ const reviewers = ` "owner": false } }, + "cloudReviewers": { + "1": { + "team": "Cloud", + "owner": true + }, + "2": { + "team": "Cloud", + "owner": false + } + }, "codeReviewersOmit": { "3": true }, diff --git a/bot/main.go b/bot/main.go index 0a48cb52..6ae2fc07 100644 --- a/bot/main.go +++ b/bot/main.go @@ -178,7 +178,7 @@ func createBot(ctx context.Context, flags flags) (*bot.Bot, error) { if err != nil { return nil, trace.Wrap(err) } - reviewer, err := review.FromString(flags.reviewers) + reviewer, err := review.FromString(environment, flags.reviewers) if err != nil { return nil, trace.Wrap(err) }