From 391884ec16ae1032bb42637bcadd1c0bdc3a5970 Mon Sep 17 00:00:00 2001 From: Jane Quintero <42625018+quinqu@users.noreply.github.com> Date: Wed, 10 Nov 2021 14:29:00 -0800 Subject: [PATCH] Use Hardcoded Map to Get Reviewers for Authors (#8928) --- .github/workflows/assign.yaml | 2 +- .github/workflows/check.yaml | 2 +- .github/workflows/ci/cmd/main.go | 21 +++--- .github/workflows/ci/constants.go | 70 ++++++++++++++++++- .../ci/pkg/environment/environment.go | 37 +++------- .../ci/pkg/environment/environment_test.go | 16 +---- 6 files changed, 90 insertions(+), 58 deletions(-) diff --git a/.github/workflows/assign.yaml b/.github/workflows/assign.yaml index d072c255803..318a787b3a2 100644 --- a/.github/workflows/assign.yaml +++ b/.github/workflows/assign.yaml @@ -38,4 +38,4 @@ jobs: uses: actions/setup-go@v2 # Run "assign-reviewers" subcommand on bot. - name: Assigning reviewers - run: cd .github/workflows/ci && go run cmd/main.go --token=${{ secrets.GITHUB_TOKEN }} --reviewers=${{ secrets.reviewers }} assign-reviewers + run: cd .github/workflows/ci && go run cmd/main.go --token=${{ secrets.GITHUB_TOKEN }} assign-reviewers diff --git a/.github/workflows/check.yaml b/.github/workflows/check.yaml index 23dacf70b37..18cf2bff987 100644 --- a/.github/workflows/check.yaml +++ b/.github/workflows/check.yaml @@ -41,4 +41,4 @@ jobs: uses: actions/setup-go@v2 # Run "check-reviewers" subcommand on bot. - name: Checking reviewers - run: cd .github/workflows/ci && go run cmd/main.go --token=${{ secrets.GITHUB_TOKEN }} --reviewers=${{ secrets.reviewers }} check-reviewers + run: cd .github/workflows/ci && go run cmd/main.go --token=${{ secrets.GITHUB_TOKEN }} check-reviewers diff --git a/.github/workflows/ci/cmd/main.go b/.github/workflows/ci/cmd/main.go index a6656a013da..965efad5f1d 100644 --- a/.github/workflows/ci/cmd/main.go +++ b/.github/workflows/ci/cmd/main.go @@ -36,16 +36,11 @@ const ( "\tcheck-reviewers \n\t checks pull request for required reviewers.\n" + "\tdismiss-runs \n\t dismisses stale workflow runs for external contributors.\n" - reviewersHelp = `reviewers is a string representing a json object that maps authors to - required reviewers for that author. example: "{\"author1\": [\"reviewer0\", \"reviewer1\"], \"author2\": - [\"reviewer2\", \"reviewer3\"],\"*\": [\"default-reviewer0\", \"default-reviewer1\"]}"` - workflowRunTimeout = time.Minute ) func main() { var token = flag.String("token", "", "token is the Github authentication token.") - var reviewers = flag.String("reviewers", "", reviewersHelp) flag.Parse() if len(os.Args) < 2 { @@ -53,10 +48,10 @@ func main() { } subcommand := os.Args[len(os.Args)-1] - // Cancel run if it takes longer than `workflowRunTimeout`. - // Note: To re-run a job go to the Actions tab in the Github repo, - // go to the run that failed, and click the `Re-run all jobs` button - // in the top right corner. + // Cancel run if it takes longer than `workflowRunTimeout`. + // Note: To re-run a job go to the Actions tab in the Github repo, + // go to the run that failed, and click the `Re-run all jobs` button + // in the top right corner. ctx, cancel := context.WithTimeout(context.Background(), workflowRunTimeout) defer cancel() @@ -64,7 +59,7 @@ func main() { switch subcommand { case ci.AssignSubcommand: log.Println("Assigning reviewers.") - bot, err := constructBot(ctx, client, *reviewers) + bot, err := constructBot(ctx, client) if err != nil { log.Fatal(err) } @@ -75,7 +70,7 @@ func main() { log.Print("Assign completed.") case ci.CheckSubcommand: log.Println("Checking reviewers.") - bot, err := constructBot(ctx, client, *reviewers) + bot, err := constructBot(ctx, client) if err != nil { log.Fatal(err) } @@ -103,9 +98,9 @@ func main() { } } -func constructBot(ctx context.Context, clt *github.Client, reviewers string) (*bots.Bot, error) { +func constructBot(ctx context.Context, clt *github.Client) (*bots.Bot, error) { env, err := environment.New(environment.Config{Client: clt, - Reviewers: reviewers, + Reviewers: ci.Reviewers, Context: ctx, }) if err != nil { diff --git a/.github/workflows/ci/constants.go b/.github/workflows/ci/constants.go index 63e48d32a43..fffaa5e34d9 100644 --- a/.github/workflows/ci/constants.go +++ b/.github/workflows/ci/constants.go @@ -79,7 +79,73 @@ const ( // Created is an event type that is triggered when a pull request review is created. Created = "created" - // AnyAuthor is the symbol used to get reviewers for external contributors/contrtibutors who - // do not have designated reviewers. + // AnyAuthor is the symbol used to get reviewers for external contributors/contrtibutors who + // do not have designated reviewers. AnyAuthor = "*" ) + +var Reviewers = map[string][]string{ + // Teleport Core + "alex-kovoy": {"russjones", "r0mant"}, + "benarent": {"russjones", "r0mant"}, + "atburke": {"nklaassen", "fspmarshall"}, + "codingllama": {"rosstimothy", "quinqu"}, + "fspmarshall": {"rosstimothy", "codingllama"}, + "gabrielcorado": {"r0mant", "smallinsky"}, + "gzdunek": {"alex-kovoy", "kimlisa"}, + "ibeckermayer": {"zmb3", "alex-kovoy"}, + "joerger": {"zmb3", "atburke"}, + "kimlisa": {"alex-kovoy", "rudream"}, + "klizhentas": {"russjones", "r0mant"}, + "kontsevoy": {"russjones", "r0mant"}, + "nklaassen": {"smallinsky", "tcsc"}, + "quinqu": {"timothyb89", "tcsc"}, + "r0mant": {"smallinsky", "timothyb89"}, + "rosstimothy": {"r0mant", "fspmarshall"}, + "rudream": {"russjones", "r0mant"}, + "russjones": {"zmb3", "r0mant"}, + "smallinsky": {"joerger", "r0mant"}, + "tcsc": {"nklaassen", "codingllama"}, + "timothyb89": {"codingllama", "xacrimon"}, + "twakes": {"russjones", "r0mant"}, + "xacrimon": {"zmb3", "joerger"}, + "zmb3": {"rosstimothy", "xacrimon"}, + + // Teleport + "aelkugia": {"russjones", "r0mant"}, + "aharic": {"russjones", "r0mant"}, + "alexwolfe": {"russjones", "r0mant"}, + "annabambi": {"russjones", "r0mant"}, + "bernardjkim": {"russjones", "r0mant"}, + "c-styr": {"russjones", "r0mant"}, + "dboslee": {"russjones", "r0mant"}, + "deliaconstantino": {"russjones", "r0mant"}, + "justinas": {"russjones", "r0mant"}, + "kapilville": {"russjones", "r0mant"}, + "kbence": {"russjones", "r0mant"}, + "knisbet": {"russjones", "r0mant"}, + "logand22": {"russjones", "r0mant"}, + "michaelmcallister": {"russjones", "r0mant"}, + "mike-battle": {"russjones", "r0mant"}, + "najiobeid": {"russjones", "r0mant"}, + "nataliestaud": {"russjones", "r0mant"}, + "pierrebeaucamp": {"russjones", "r0mant"}, + "programmerq": {"russjones", "r0mant"}, + "pschisa": {"russjones", "r0mant"}, + "recruitingthebest": {"russjones", "r0mant"}, + "rishibarbhaya-design": {"russjones", "r0mant"}, + "sandylcruz": {"russjones", "r0mant"}, + "sshahcodes": {"russjones", "r0mant"}, + "stevengravy": {"russjones", "r0mant"}, + "travelton": {"russjones", "r0mant"}, + "travisgary": {"russjones", "r0mant"}, + "ulysseskan": {"russjones", "r0mant"}, + "valien": {"russjones", "r0mant"}, + "wadells": {"russjones", "r0mant"}, + "webvictim": {"russjones", "r0mant"}, + "williamloy": {"russjones", "r0mant"}, + "yjperez": {"russjones", "r0mant"}, + + // External + AnyAuthor: {"russjones", "r0mant"}, +} diff --git a/.github/workflows/ci/pkg/environment/environment.go b/.github/workflows/ci/pkg/environment/environment.go index b8876af2f5d..4de1f53f2fa 100644 --- a/.github/workflows/ci/pkg/environment/environment.go +++ b/.github/workflows/ci/pkg/environment/environment.go @@ -31,9 +31,9 @@ type Config struct { Context context.Context // Client is the authenticated Github client. Client *github.Client - // Reviewers is a json object encoded as a string with - // authors mapped to their respective required reviewers. - Reviewers string + // Reviewers is a map that maps authors to their respective + // required reviewers. + Reviewers map[string][]string // EventPath is the path of the file with the complete // webhook event payload on the runner. EventPath string @@ -89,9 +89,12 @@ func (c *Config) CheckAndSetDefaults() error { if c.Client == nil { return trace.BadParameter("missing parameter Client") } - if c.Reviewers == "" { + if c.Reviewers == nil { return trace.BadParameter("missing parameter Reviewers") } + if _, ok := c.Reviewers[ci.AnyAuthor]; !ok { + return trace.BadParameter(`default reviewers are not set in reviewers map. set default reviewers with a wildcard (*) as a key`) + } if c.EventPath == "" { c.EventPath = os.Getenv(ci.GithubEventPath) } @@ -104,40 +107,18 @@ func New(c Config) (*PullRequestEnvironment, error) { if err != nil { return nil, trace.Wrap(err) } - revs, err := unmarshalReviewers(c.Context, c.Reviewers) - if err != nil { - return nil, trace.Wrap(err) - } pr, err := GetMetadata(c.EventPath) if err != nil { return nil, trace.Wrap(err) } return &PullRequestEnvironment{ Client: c.Client, - reviewers: revs, - defaultReviewers: revs[ci.AnyAuthor], + reviewers: c.Reviewers, + defaultReviewers: c.Reviewers[ci.AnyAuthor], Metadata: pr, }, nil } -// unmarshalReviewers converts the passed in string representing json object into a map. -func unmarshalReviewers(ctx context.Context, str string) (map[string][]string, error) { - if str == "" { - return nil, trace.NotFound("reviewers not found") - } - m := make(map[string][]string) - - err := json.Unmarshal([]byte(str), &m) - if err != nil { - return nil, trace.Wrap(err) - } - if _, ok := m[ci.AnyAuthor]; !ok { - return nil, trace.BadParameter("default reviewers are not set. set default reviewers with a wildcard (*) as a key") - } - return m, nil - -} - // GetReviewersForAuthor gets the required reviewers for the current user. func (e *PullRequestEnvironment) GetReviewersForAuthor(user string) []string { value, ok := e.reviewers[user] diff --git a/.github/workflows/ci/pkg/environment/environment_test.go b/.github/workflows/ci/pkg/environment/environment_test.go index 1f6cfe0c0f3..10e115d8290 100644 --- a/.github/workflows/ci/pkg/environment/environment_test.go +++ b/.github/workflows/ci/pkg/environment/environment_test.go @@ -52,7 +52,7 @@ func TestNewPullRequestEnvironment(t *testing.T) { { cfg: Config{ Client: github.NewClient(nil), - Reviewers: `{"foo": ["bar", "baz"], "*":["admin"]}`, + Reviewers: map[string][]string{"foo": {"bar", "baz"}, "*": {"admin"}}, }, checkErr: require.NoError, desc: "valid PullRequestEnvironment config", @@ -67,7 +67,7 @@ func TestNewPullRequestEnvironment(t *testing.T) { { cfg: Config{ Client: github.NewClient(nil), - Reviewers: `{"foo": ["bar", "baz"], "*":["admin"]}`, + Reviewers: map[string][]string{"foo": {"bar", "baz"}, "*": {"admin"}}, }, checkErr: require.NoError, desc: "valid PullRequestEnvironment config", @@ -82,23 +82,13 @@ func TestNewPullRequestEnvironment(t *testing.T) { { cfg: Config{ Client: github.NewClient(nil), - Reviewers: `{"foo": ["bar", "baz"]}`, + Reviewers: map[string][]string{"foo": {"bar", "baz"}}, }, checkErr: require.Error, desc: "invalid PullRequestEnvironment config, has no default reviewers key", expected: nil, createFile: true, }, - { - cfg: Config{ - Reviewers: `{"foo": "baz", "*":["admin"]}`, - Client: github.NewClient(nil), - }, - checkErr: require.Error, - desc: "invalid reviewers object format", - expected: nil, - createFile: true, - }, { cfg: Config{}, checkErr: require.Error,