From 06b06cc12275d4f9a990cf37924b210be65a32d4 Mon Sep 17 00:00:00 2001 From: Adrian Orive Date: Mon, 9 Nov 2020 11:58:25 +0100 Subject: [PATCH] Define specific behavior for each PR action including synchronize Signed-off-by: Adrian Orive --- .github/workflows/main.yml | 2 +- verify/check_run_status.go | 37 +++++ verify/cmd/runner.go | 28 +--- verify/common.go | 156 +++++------------- verify/logger.go | 35 ++++ verify/plugin.go | 330 +++++++++++++++++++++++++++++++++++++ 6 files changed, 447 insertions(+), 141 deletions(-) create mode 100644 verify/check_run_status.go create mode 100644 verify/logger.go create mode 100644 verify/plugin.go diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index deef118..ae54e85 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,6 @@ on: pull_request_target: - types: [opened, edited, reopened] + types: [opened, edited, reopened, synchronize] jobs: verify: diff --git a/verify/check_run_status.go b/verify/check_run_status.go new file mode 100644 index 0000000..1a0b25c --- /dev/null +++ b/verify/check_run_status.go @@ -0,0 +1,37 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package verify + +import ( + "github.com/google/go-github/v32/github" +) + +type CheckRunStatus string + +const ( + Queued CheckRunStatus = "queued" + Started CheckRunStatus = "in_progress" + Finished CheckRunStatus = "completed" +) + +func (status CheckRunStatus) StringP() *string { + return github.String(string(status)) +} + +func (status CheckRunStatus) Equal(other string) bool { + return string(status) == other +} diff --git a/verify/cmd/runner.go b/verify/cmd/runner.go index c6d48ad..d24b094 100644 --- a/verify/cmd/runner.go +++ b/verify/cmd/runner.go @@ -18,8 +18,8 @@ package main import ( "fmt" - "strings" "regexp" + "strings" "github.com/google/go-github/v32/github" @@ -31,15 +31,17 @@ import ( type prErrs struct { errs []string } + func (e prErrs) Error() string { return fmt.Sprintf("%d issues found with your PR description", len(e.errs)) } + func (e prErrs) Help() string { res := make([]string, len(e.errs)) for _, err := range e.errs { parts := strings.Split(err, "\n") for i, part := range parts[1:] { - parts[i+1] = " "+part + parts[i+1] = " " + part } res = append(res, "- "+strings.Join(parts, "\n")) } @@ -49,23 +51,15 @@ func (e prErrs) Help() string { func main() { verify.ActionsEntrypoint(verify.RunPlugins( verify.PRPlugin{ - Name: "PR Type", + Name: "PR Type", Title: "PR Type in Title", ProcessPR: func(pr *github.PullRequest) (string, error) { return notesver.VerifyPRTitle(pr.GetTitle()) }, - ForAction: func(action string) bool { - switch action { - case "opened", "edited", "reopened": - return true - default: - return false - } - }, }, verify.PRPlugin{ - Name: "PR Desc", + Name: "PR Desc", Title: "Basic PR Descriptiveness Check", ProcessPR: func(pr *github.PullRequest) (string, error) { var errs []string @@ -83,7 +77,7 @@ func main() { } _, title := notes.PRTypeFromTitle(pr.GetTitle()) - if regexp.MustCompile(`#\d{1,}\b`).MatchString(title) { + if regexp.MustCompile(`#\d+\b`).MatchString(title) { errs = append(errs, "**Your PR has an issue number in the title.**\n\nThe title should just be descriptive.\nIssue numbers belong in the PR body as either `Fixes #XYZ` (if it closes the issue or PR), or something like `Related to #XYZ` (if it's just related).") } @@ -92,14 +86,6 @@ func main() { } return "", prErrs{errs: errs} }, - ForAction: func(action string) bool { - switch action { - case "opened", "edited", "reopened": - return true - default: - return false - } - }, }, )) } diff --git a/verify/common.go b/verify/common.go index daa2d88..336af28 100644 --- a/verify/common.go +++ b/verify/common.go @@ -17,12 +17,10 @@ limitations under the License. package verify import ( + "context" + "encoding/json" "fmt" "os" - "encoding/json" - "errors" - "context" - "time" "strings" "sync" @@ -30,151 +28,71 @@ import ( "golang.org/x/oauth2" ) -type ErrWithHelp interface { - error - Help() string -} - -type PRPlugin struct { - ForAction func(string) bool - ProcessPR func(pr *github.PullRequest) (string, error) - Name string - Title string -} - -func (p *PRPlugin) Entrypoint(env *ActionsEnv) error { - if p.ForAction != nil && !p.ForAction(env.Event.GetAction()) { - return nil - } - - repoParts := strings.Split(env.Event.GetRepo().GetFullName(), "/") - orgName, repoName := repoParts[0], repoParts[1] - - headSHA := env.Event.GetPullRequest().GetHead().GetSHA() - fmt.Printf("::debug::creating check run %q on %s/%s @ %s...\n", p.Name, orgName, repoName, headSHA) - - resRun, runResp, err := env.Client.Checks.CreateCheckRun(context.TODO(), orgName, repoName, github.CreateCheckRunOptions{ - Name: p.Name, - HeadSHA: headSHA, - Status: github.String("in_progress"), - }) - if err != nil { - return fmt.Errorf("unable to submit check result: %w", err) - } - - env.Debugf("create check API response: %+v", runResp) - env.Debugf("created run: %+v", resRun) - - successStatus, procErr := p.ProcessPR(env.Event.PullRequest) - - var summary, fullHelp, conclusion string - if procErr != nil { - summary = procErr.Error() - var helpErr ErrWithHelp - if errors.As(procErr, &helpErr) { - fullHelp = helpErr.Help() - } - conclusion = "failure" - } else { - summary = "Success" - fullHelp = successStatus - conclusion = "success" - } - completedAt := github.Timestamp{Time: time.Now()} - - // log in case we can't submit the result for some reason - env.Debugf("plugin result summary: %q", summary) - env.Debugf("plugin result details: %q", fullHelp) - env.Debugf("plugin conclusion: %q", conclusion) - - resRun, updateResp, err := env.Client.Checks.UpdateCheckRun(context.TODO(), orgName, repoName, resRun.GetID(), github.UpdateCheckRunOptions{ - Name: p.Name, - Status: github.String("completed"), - Conclusion: github.String(conclusion), - CompletedAt: &completedAt, - Output: &github.CheckRunOutput{ - Title: github.String(p.Title), - Summary: github.String(summary), - Text: github.String(fullHelp), - }, - }) - if err != nil { - return fmt.Errorf("unable to update check result: %w", err) - } - - env.Debugf("update check API response: %+v", updateResp) - env.Debugf("updated run: %+v", resRun) - - // return failure here too so that the whole suite fails (since the actions - // suite seems to ignore failing check runs when calculating general failure) - if procErr != nil { - return fmt.Errorf("failed: %v", procErr) - } - return nil -} +var log logger type ActionsEnv struct { - Event *github.PullRequestEvent + Owner string + Repo string + Event *github.PullRequestEvent Client *github.Client } -func (ActionsEnv) Errorf(fmtStr string, args ...interface{}) { - fmt.Printf("::error::"+fmtStr+"\n", args...) -} -func (ActionsEnv) Debugf(fmtStr string, args ...interface{}) { - fmt.Printf("::debug::"+fmtStr+"\n", args...) -} -func (ActionsEnv) Warnf(fmtStr string, args ...interface{}) { - fmt.Printf("::warning::"+fmtStr+"\n", args...) -} -func SetupEnv() (*ActionsEnv, error) { +func setupEnv() (*ActionsEnv, error) { if os.Getenv("GITHUB_ACTIONS") != "true" { return nil, fmt.Errorf("not running in an action, bailing. Set GITHUB_ACTIONS and the other appropriate env vars if you really want to do this.") } - payloadPath := os.Getenv("GITHUB_EVENT_PATH") - if payloadPath == "" { - return nil, fmt.Errorf("no payload path set, something weird is up") + // Get owner and repository + ownerAndRepo := strings.Split(os.Getenv("GITHUB_REPOSITORY"), "/") + + // Get event path + eventPath := os.Getenv("GITHUB_EVENT_PATH") + if eventPath == "" { + return nil, fmt.Errorf("no event path set, something weird is up") } - payload, err := func() (github.PullRequestEvent, error) { - payloadRaw, err := os.Open(payloadPath) + // Parse the event + event, err := func() (github.PullRequestEvent, error) { + eventFile, err := os.Open(eventPath) if err != nil { - return github.PullRequestEvent{}, fmt.Errorf("unable to load payload file: %w", err) + return github.PullRequestEvent{}, fmt.Errorf("unable to load event file: %w", err) } - defer payloadRaw.Close() - - var payload github.PullRequestEvent - if err := json.NewDecoder(payloadRaw).Decode(&payload); err != nil { - return payload, fmt.Errorf("unable to unmarshal payload: %w", err) + defer eventFile.Close() + + var event github.PullRequestEvent + if err := json.NewDecoder(eventFile).Decode(&event); err != nil { + return event, fmt.Errorf("unable to unmarshal event: %w", err) } - return payload, nil + return event, nil }() if err != nil { return nil, err } - ctx := context.Background() - authClient := oauth2.NewClient(ctx, oauth2.StaticTokenSource( + // Create the client + client := github.NewClient(oauth2.NewClient(context.Background(), oauth2.StaticTokenSource( &oauth2.Token{AccessToken: os.Getenv("INPUT_GITHUB_TOKEN")}, - )) + ))) return &ActionsEnv{ - Event: &payload, - Client: github.NewClient(authClient), + Owner: ownerAndRepo[0], + Repo: ownerAndRepo[1], + Event: &event, + Client: client, }, nil } type ActionsCallback func(*ActionsEnv) error + func ActionsEntrypoint(cb ActionsCallback) { - env, err := SetupEnv() + env, err := setupEnv() if err != nil { - env.Errorf("%v", err) + log.errorf("%v", err) os.Exit(1) } if err := cb(env); err != nil { - env.Errorf("%v", err) + log.errorf("%v", err) os.Exit(2) } fmt.Println("Success!") @@ -189,7 +107,7 @@ func RunPlugins(plugins ...PRPlugin) ActionsCallback { done.Add(1) go func(plugin PRPlugin) { defer done.Done() - res <- plugin.Entrypoint(env) + res <- plugin.entrypoint(env) }(plugin) } @@ -204,7 +122,7 @@ func RunPlugins(plugins ...PRPlugin) ActionsCallback { continue } errCount++ - env.Errorf("%v", err) + log.errorf("%v", err) } fmt.Printf("%d plugins ran\n", len(plugins)) diff --git a/verify/logger.go b/verify/logger.go new file mode 100644 index 0000000..0d4a497 --- /dev/null +++ b/verify/logger.go @@ -0,0 +1,35 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package verify + +import ( + "fmt" +) + +type logger struct{} + +func (logger) errorf(format string, args ...interface{}) { + fmt.Printf("::error::"+format+"\n", args...) +} + +func (logger) debugf(format string, args ...interface{}) { + fmt.Printf("::debug::"+format+"\n", args...) +} + +func (logger) warningf(format string, args ...interface{}) { + fmt.Printf("::warning::"+format+"\n", args...) +} diff --git a/verify/plugin.go b/verify/plugin.go new file mode 100644 index 0000000..2d4e3bd --- /dev/null +++ b/verify/plugin.go @@ -0,0 +1,330 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package verify + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/google/go-github/v32/github" +) + +// ErrorWithHelp allows PRPlugin.ProcessPR to provide extended descriptions +type ErrorWithHelp interface { + error + Help() string +} + +// PRPlugin handles pull request events +type PRPlugin struct { + ProcessPR func(pr *github.PullRequest) (string, error) + Name string + Title string + + logger +} + +// processPR executes the provided ProcessPR and parses the result +func (p PRPlugin) processPR(pr *github.PullRequest) (conclusion, summary, text string, err error) { + text, err = p.ProcessPR(pr) + + if err != nil { + conclusion = "failure" + summary = err.Error() + var helpErr ErrorWithHelp + if errors.As(err, &helpErr) { + text = helpErr.Help() + } + } else { + conclusion = "success" + summary = "Success" + } + + // Log in case we can't submit the result for some reason + p.debugf("plugin conclusion: %q", conclusion) + p.debugf("plugin result summary: %q", summary) + p.debugf("plugin result details: %q", text) + + return conclusion, summary, text, err +} + +// processAndSubmit performs the checks and submits the result +func (p PRPlugin) processAndSubmit(env *ActionsEnv, checkRun *github.CheckRun) error { + // Process the PR + conclusion, summary, text, procErr := p.processPR(env.Event.PullRequest) + + // Update the check run + if err := p.finishCheckRun(env.Client, env.Owner, env.Repo, checkRun.GetID(), conclusion, summary, text); err != nil { + return err + } + + // Return failure here too so that the whole suite fails (since the actions + // suite seems to ignore failing check runs when calculating general failure) + if procErr != nil { + return fmt.Errorf("failed: %v", procErr) + } + return nil +} + +//////////////////////////////////////////////////////////////////////////////// +// Check API calls // +//////////////////////////////////////////////////////////////////////////////// + +// createCheckRun creates a new Check-Run. +// It returns an error in case it couldn't be created. +func (p PRPlugin) createCheckRun(client *github.Client, owner, repo, headSHA string) (*github.CheckRun, error) { + p.debugf("creating check run %q on %s/%s @ %s...", p.Name, owner, repo, headSHA) + + checkRun, res, err := client.Checks.CreateCheckRun( + context.TODO(), + owner, + repo, + github.CreateCheckRunOptions{ + Name: p.Name, + HeadSHA: headSHA, + Status: Started.StringP(), + }, + ) + if err != nil { + return nil, fmt.Errorf("unable to submit check result: %w", err) + } + + p.debugf("create check API response: %+v", res) + p.debugf("created run: %+v", checkRun) + + return checkRun, nil +} + +// getCheckRun returns the Check-Run, creating it if it doesn't exist. +// It returns an error in case it didn't exist and couldn't be created, or if there are multiple matches. +func (p PRPlugin) getCheckRun(client *github.Client, owner, repo, headSHA string) (*github.CheckRun, error) { + p.debugf("getting check run %q on %s/%s @ %s...", p.Name, owner, repo, headSHA) + + checkRunList, res, err := client.Checks.ListCheckRunsForRef( + context.TODO(), + owner, + repo, + headSHA, + &github.ListCheckRunsOptions{ + CheckName: github.String(p.Name), + }, + ) + if err != nil { + return nil, err + } + + p.debugf("list check API response: %+v", res) + p.debugf("listed runs: %+v", checkRunList) + + switch n := *checkRunList.Total; { + case n == 0: + return p.createCheckRun(client, owner, repo, headSHA) + case n == 1: + return checkRunList.CheckRuns[0], nil + case n > 1: + return nil, fmt.Errorf("multiple instances of `%s` check run found on %s/%s @ %s", p.Name, owner, repo, headSHA) + default: // Should never happen + return nil, fmt.Errorf("negative number of instances (%d) of `%s` check run found on %s/%s @ %s", n, p.Name, owner, repo, headSHA) + } +} + +// resetCheckRun returns the Check-Run with executing status, creating it if it doesn't exist. +// It returns an error in case it didn't exist and couldn't be created, if there are multiple matches, +// or if it exists but couldn't be updated. +func (p PRPlugin) resetCheckRun(client *github.Client, owner, repo string, headSHA string) (*github.CheckRun, error) { + checkRun, err := p.getCheckRun(client, owner, repo, headSHA) + // If it was created we don't need to update it, check its status + if err != nil || Started.Equal(checkRun.GetStatus()) { + return checkRun, err + } + + p.debugf("updating check run %q on %s/%s...", p.Name, owner, repo) + + checkRun, updateResp, err := client.Checks.UpdateCheckRun( + context.TODO(), + owner, + repo, + checkRun.GetID(), + github.UpdateCheckRunOptions{ + Name: p.Name, + Status: github.String("in-progress"), + }, + ) + if err != nil { + return checkRun, fmt.Errorf("unable to update check result: %w", err) + } + + p.debugf("update check API response: %+v", updateResp) + p.debugf("updated run: %+v", checkRun) + + return checkRun, nil +} + +// rebindCheckRun modifies the bound commit to the Check-Run with id checkRunID to headSHA. +// It returns an error in case it couldn't be rebound. +func (p PRPlugin) rebindCheckRun(client *github.Client, owner, repo string, checkRunID int64, headSHA string) error { + p.debugf("updating check run %q on %s/%s...", p.Name, owner, repo) + + checkRun, updateResp, err := client.Checks.UpdateCheckRun( + context.TODO(), + owner, + repo, + checkRunID, + github.UpdateCheckRunOptions{ + Name: p.Name, + HeadSHA: github.String(headSHA), + }, + ) + if err != nil { + return fmt.Errorf("unable to update check result: %w", err) + } + + p.debugf("update check API response: %+v", updateResp) + p.debugf("updated run: %+v", checkRun) + + return nil +} + +// finishCheckRun updates the Check-Run with id checkRunID setting its output. +// It returns an error in case it couldn't be updated. +func (p PRPlugin) finishCheckRun(client *github.Client, owner, repo string, checkRunID int64, conclusion, summary, text string) error { + p.debugf("updating check run %q on %s/%s...", p.Name, owner, repo) + + checkRun, updateResp, err := client.Checks.UpdateCheckRun(context.TODO(), owner, repo, checkRunID, github.UpdateCheckRunOptions{ + Name: p.Name, + Conclusion: github.String(conclusion), + CompletedAt: &github.Timestamp{Time: time.Now()}, + Output: &github.CheckRunOutput{ + Title: github.String(p.Title), + Summary: github.String(summary), + Text: github.String(text), + }, + }) + if err != nil { + return fmt.Errorf("unable to update check result: %w", err) + } + + p.debugf("update check API response: %+v", updateResp) + p.debugf("updated run: %+v", checkRun) + + return nil +} + +//////////////////////////////////////////////////////////////////////////////// +// Entrypoint // +//////////////////////////////////////////////////////////////////////////////// + +// entrypoint will call the corresponding handler +func (p PRPlugin) entrypoint(env *ActionsEnv) (err error) { + switch env.Event.GetAction() { + case "open": + err = p.onOpen(env) + case "reopen": + err = p.onReopen(env) + case "edit": + err = p.onEdit(env) + case "synchronize": + err = p.onSync(env) + default: + // Do nothing + } + + return +} + +// onOpen handles "open" actions +func (p PRPlugin) onOpen(env *ActionsEnv) error { + headSHA := env.Event.GetPullRequest().GetHead().GetSHA() + + // Create the check run + checkRun, err := p.createCheckRun(env.Client, env.Owner, env.Repo, headSHA) + if err != nil { + return err + } + + // Process the PR and submit the results + return p.processAndSubmit(env, checkRun) +} + +// onReopen handles "reopen" actions +func (p PRPlugin) onReopen(env *ActionsEnv) error { + headSHA := env.Event.GetPullRequest().GetHead().GetSHA() + + // Get the check run + checkRun, err := p.getCheckRun(env.Client, env.Owner, env.Repo, headSHA) + if err != nil { + return err + } + + // Rerun the tests if they weren't finished + if !Finished.Equal(checkRun.GetStatus()) { + // Process the PR and submit the results + return p.processAndSubmit(env, checkRun) + } + + // Return failure here too so that the whole suite fails (since the actions + // suite seems to ignore failing check runs when calculating general failure) + if *checkRun.Conclusion == "failure" { + return fmt.Errorf("failed: %v", *checkRun.Output.Summary) + } + return nil +} + +// onEdit handles "edit" actions +func (p PRPlugin) onEdit(env *ActionsEnv) error { + headSHA := env.Event.GetPullRequest().GetHead().GetSHA() + + // Reset the check run + checkRun, err := p.resetCheckRun(env.Client, env.Owner, env.Repo, headSHA) + if err != nil { + return err + } + + // Process the PR and submit the results + return p.processAndSubmit(env, checkRun) +} + +// onSync handles "synchronize" actions +func (p PRPlugin) onSync(env *ActionsEnv) error { + before, after := env.Event.GetBefore(), env.Event.GetAfter() + + // Get the check run + checkRun, err := p.getCheckRun(env.Client, env.Owner, env.Repo, before) + if err != nil { + return err + } + + // Rebind the check run + if err := p.rebindCheckRun(env.Client, env.Owner, env.Repo, checkRun.GetID(), after); err != nil { + return err + } + + // Rerun the tests if they weren't finished + if !Finished.Equal(checkRun.GetStatus()) { + // Process the PR and submit the results + return p.processAndSubmit(env, checkRun) + } + + // Return failure here too so that the whole suite fails (since the actions + // suite seems to ignore failing check runs when calculating general failure) + if *checkRun.Conclusion == "failure" { + return fmt.Errorf("failed: %v", *checkRun.Output.Summary) + } + return nil +}