diff --git a/cmd/pullrequest-init/README.md b/cmd/pullrequest-init/README.md index 8f48dc82240..b27a31ebc28 100644 --- a/cmd/pullrequest-init/README.md +++ b/cmd/pullrequest-init/README.md @@ -44,7 +44,16 @@ like: "Text": "my-label" } ], - "Raw": "/tmp/prtest/github/pr.json" + "Statuses": [ + { + "Code": "success", + "Description": "Job succeeded.", + "ID": "pull-tekton-pipeline-go-coverage", + "URL": "https://tekton-releases.appspot.com/build/tekton-prow/pr-logs/pull/tektoncd_pipeline/895/pull-tekton-pipeline-go-coverage/1141483806818045953/" + }, + ], + "Raw": "/tmp/prtest/github/pr.json", + "RawStatus": "/tmp/pr/github/status.json" } ``` @@ -54,6 +63,9 @@ GitHub pull requests will output these additional files: * `$PATH/github/pr.json`: The raw GitHub payload as specified by https://developer.github.com/v3/pulls/#get-a-single-pull-request +* `$PATH/github/status.json`: The raw GitHub combined status payload as + specified by + https://developer.github.com/v3/repos/statuses/#get-the-combined-status-for-a-specific-ref * `$PATH/github/comments/#.json`: Comments associated to the PR as specified by https://developer.github.com/v3/issues/comments/#get-a-single-comment @@ -62,3 +74,18 @@ For now, these files are *read-only*. The binary will look for GitHub credentials in the `${GITHUBTOKEN}` environment variable. This should generally be specified as a secret with the field name `githubToken` in the `PullRequestResource` definition. + +### Status code conversion + +Tekton Status Code | GitHub Status State +------------------ | ------------------- +success | success +neutral | success +queued | pending +in_progress | pending +failure | failure +unknown | error +error | error +timeout | error +canceled | error +action_required | error diff --git a/cmd/pullrequest-init/fake_github.go b/cmd/pullrequest-init/fake_github.go index 68bc78bea83..7752b831f1a 100644 --- a/cmd/pullrequest-init/fake_github.go +++ b/cmd/pullrequest-init/fake_github.go @@ -35,6 +35,7 @@ type FakeGitHub struct { // We need to store references to both to emulate the API properly. prComments map[key][]*github.IssueComment comments map[key]*github.IssueComment + status map[statusKey]map[string]*github.RepoStatus } // NewFakeGitHub returns a new FakeGitHub. @@ -44,6 +45,7 @@ func NewFakeGitHub() *FakeGitHub { pr: make(map[key]*github.PullRequest), prComments: make(map[key][]*github.IssueComment), comments: make(map[key]*github.IssueComment), + status: make(map[statusKey]map[string]*github.RepoStatus), } s.HandleFunc("/repos/{owner}/{repo}/pulls/{number}", s.getPullRequest).Methods(http.MethodGet) s.HandleFunc("/repos/{owner}/{repo}/issues/{number}/comments", s.getComments).Methods(http.MethodGet) @@ -51,6 +53,8 @@ func NewFakeGitHub() *FakeGitHub { s.HandleFunc("/repos/{owner}/{repo}/issues/comments/{number}", s.updateComment).Methods(http.MethodPatch) s.HandleFunc("/repos/{owner}/{repo}/issues/comments/{number}", s.deleteComment).Methods(http.MethodDelete) s.HandleFunc("/repos/{owner}/{repo}/issues/{number}/labels", s.updateLabels).Methods(http.MethodPut) + s.HandleFunc("/repos/{owner}/{repo}/statuses/{revision}", s.createStatus).Methods(http.MethodPost) + s.HandleFunc("/repos/{owner}/{repo}/commits/{revision}/status", s.getStatuses).Methods(http.MethodGet) return s } @@ -230,3 +234,50 @@ func (g *FakeGitHub) updateLabels(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) } + +type statusKey struct { + owner string + repo string + revision string +} + +func getStatusKey(r *http.Request) statusKey { + return statusKey{ + owner: mux.Vars(r)["owner"], + repo: mux.Vars(r)["repo"], + revision: mux.Vars(r)["revision"], + } +} + +func (g *FakeGitHub) createStatus(w http.ResponseWriter, r *http.Request) { + k := getStatusKey(r) + + rs := new(github.RepoStatus) + if err := json.NewDecoder(r.Body).Decode(rs); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + if _, ok := g.status[k]; !ok { + g.status[k] = make(map[string]*github.RepoStatus) + } + g.status[k][rs.GetContext()] = rs +} + +func (g *FakeGitHub) getStatuses(w http.ResponseWriter, r *http.Request) { + k := getStatusKey(r) + + s := make([]github.RepoStatus, 0, len(g.status[k])) + for _, v := range g.status[k] { + s = append(s, *v) + } + + cs := &github.CombinedStatus{ + TotalCount: github.Int(len(s)), + Statuses: s, + } + if err := json.NewEncoder(w).Encode(cs); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } +} diff --git a/cmd/pullrequest-init/fake_github_test.go b/cmd/pullrequest-init/fake_github_test.go index e6dc65ce3e2..4f3ccb42ab3 100644 --- a/cmd/pullrequest-init/fake_github_test.go +++ b/cmd/pullrequest-init/fake_github_test.go @@ -64,3 +64,38 @@ func TestFakeGitHubBadKey(t *testing.T) { t.Errorf("want BadRequest, got %+v, %v", resp, err) } } + +func TestFakeGitHubStatus(t *testing.T) { + ctx := context.Background() + gh := NewFakeGitHub() + client, close := githubClient(t, gh) + defer close() + + sha := "tacocat" + + if got, resp, err := client.Repositories.GetCombinedStatus(ctx, owner, repo, sha, nil); err != nil || resp.StatusCode != http.StatusOK || len(got.Statuses) != 0 { + t.Fatalf("GetCombinedStatus: wanted [], got %+v, %+v, %v", got, resp, err) + } + + rs := &github.RepoStatus{ + Context: github.String("Tekton"), + Description: github.String("Test all the things!"), + State: github.String("success"), + TargetURL: github.String("https://tekton.dev"), + } + if _, _, err := client.Repositories.CreateStatus(ctx, owner, repo, sha, rs); err != nil { + t.Fatalf("CreateStatus: %v", err) + } + + got, resp, err := client.Repositories.GetCombinedStatus(ctx, owner, repo, sha, nil) + if err != nil || resp.StatusCode != http.StatusOK { + t.Fatalf("GetCombinedStatus: wanted OK, got %+v, %v", resp, err) + } + want := &github.CombinedStatus{ + TotalCount: github.Int(1), + Statuses: []github.RepoStatus{*rs}, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("GetCombinedStatus: -want +got: %s", diff) + } +} diff --git a/cmd/pullrequest-init/github.go b/cmd/pullrequest-init/github.go index 04bf4f41825..9793ed3328a 100644 --- a/cmd/pullrequest-init/github.go +++ b/cmd/pullrequest-init/github.go @@ -18,6 +18,29 @@ import ( "go.uber.org/zap" ) +var ( + toGitHub = map[StatusCode]string{ + Unknown: "error", + Success: "success", + Failure: "failure", + Error: "error", + // There's no analog for neutral in GitHub statuses, so default to success + // to make this non-blocking. + Neutral: "success", + Queued: "pending", + InProgress: "pending", + Timeout: "error", + Canceled: "error", + ActionRequired: "error", + } + toTekton = map[string]StatusCode{ + "success": Success, + "failure": Failure, + "error": Error, + "pending": Queued, + } +) + // GitHubHandler handles interactions with the GitHub API. type GitHubHandler struct { *github.Client @@ -97,6 +120,14 @@ func (h *GitHubHandler) Download(ctx context.Context, path string) error { } pr := baseGitHubPullRequest(gpr) + rawStatus := filepath.Join(rawPrefix, "status.json") + statuses, err := h.getStatuses(ctx, pr.Head.SHA, rawStatus) + if err != nil { + return err + } + pr.RawStatus = rawStatus + pr.Statuses = statuses + rawPR := filepath.Join(rawPrefix, "pr.json") if err := writeJSON(rawPR, gpr); err != nil { return err @@ -197,6 +228,11 @@ func (h *GitHubHandler) Upload(ctx context.Context, path string) error { } var merr error + + if err := h.uploadStatuses(ctx, pr.Head.SHA, pr.Statuses); err != nil { + merr = multierror.Append(merr, err) + } + if err := h.uploadLabels(ctx, pr.Labels); err != nil { merr = multierror.Append(merr, err) } @@ -297,3 +333,53 @@ func (h *GitHubHandler) createNewComments(ctx context.Context, comments []*Comme } return merr } + +func (h *GitHubHandler) getStatuses(ctx context.Context, sha string, path string) ([]*Status, error) { + resp, _, err := h.Repositories.GetCombinedStatus(ctx, h.owner, h.repo, sha, nil) + if err != nil { + return nil, err + } + if err := writeJSON(path, resp); err != nil { + return nil, err + } + + statuses := make([]*Status, 0, len(resp.Statuses)) + for _, s := range resp.Statuses { + code, ok := toTekton[s.GetState()] + if !ok { + return nil, fmt.Errorf("unknown GitHub status state: %s", s.GetState()) + } + statuses = append(statuses, &Status{ + ID: s.GetContext(), + Code: code, + Description: s.GetDescription(), + URL: s.GetTargetURL(), + }) + } + return statuses, nil +} + +func (h *GitHubHandler) uploadStatuses(ctx context.Context, sha string, statuses []*Status) error { + var merr error + + for _, s := range statuses { + state, ok := toGitHub[s.Code] + if !ok { + merr = multierror.Append(merr, fmt.Errorf("unknown status code %s", s.Code)) + continue + } + + rs := &github.RepoStatus{ + Context: github.String(s.ID), + State: github.String(state), + Description: github.String(s.Description), + TargetURL: github.String(s.URL), + } + if _, _, err := h.Client.Repositories.CreateStatus(ctx, h.owner, h.repo, sha, rs); err != nil { + merr = multierror.Append(merr, err) + continue + } + } + + return merr +} diff --git a/cmd/pullrequest-init/github_test.go b/cmd/pullrequest-init/github_test.go index ee2bc9e7307..808ffa4eb91 100644 --- a/cmd/pullrequest-init/github_test.go +++ b/cmd/pullrequest-init/github_test.go @@ -17,6 +17,46 @@ import ( "go.uber.org/zap/zaptest" ) +const ( + owner = "foo" + repo = "bar" + prNum = 1 + sha = "tacocat" +) + +var ( + pr = &github.PullRequest{ + ID: github.Int64(int64(prNum)), + HTMLURL: github.String(fmt.Sprintf("https://github.com/%s/%s/pull/%d", owner, repo, prNum)), + Base: &github.PullRequestBranch{ + User: &github.User{ + Login: github.String(owner), + }, + Repo: &github.Repository{ + Name: github.String(repo), + CloneURL: github.String(fmt.Sprintf("https://github.com/%s/%s", owner, repo)), + }, + Ref: github.String("master"), + SHA: github.String("1"), + }, + Head: &github.PullRequestBranch{ + User: &github.User{ + Login: github.String("baz"), + }, + Repo: &github.Repository{ + Name: github.String(repo), + CloneURL: github.String(fmt.Sprintf("https://github.com/baz/%s", repo)), + }, + Ref: github.String("feature"), + SHA: github.String("2"), + }, + } + comment = &github.IssueComment{ + ID: github.Int64(1), + Body: github.String("hello world!"), + } +) + func TestGitHubParseURL(t *testing.T) { wantOwner := "owner" wantRepo := "repo" @@ -63,45 +103,6 @@ func TestGitHubParseURL_errors(t *testing.T) { } } -const ( - owner = "foo" - repo = "bar" - prNum = 1 -) - -var ( - pr = &github.PullRequest{ - ID: github.Int64(int64(prNum)), - HTMLURL: github.String(fmt.Sprintf("https://github.com/%s/%s/pull/%d", owner, repo, prNum)), - Base: &github.PullRequestBranch{ - User: &github.User{ - Login: github.String(owner), - }, - Repo: &github.Repository{ - Name: github.String(repo), - CloneURL: github.String(fmt.Sprintf("https://github.com/%s/%s", owner, repo)), - }, - Ref: github.String("master"), - SHA: github.String("1"), - }, - Head: &github.PullRequestBranch{ - User: &github.User{ - Login: github.String("baz"), - }, - Repo: &github.Repository{ - Name: github.String(repo), - CloneURL: github.String(fmt.Sprintf("https://github.com/baz/%s", repo)), - }, - Ref: github.String("feature"), - SHA: github.String("2"), - }, - } - comment = &github.IssueComment{ - ID: github.Int64(1), - Body: github.String("hello world!"), - } -) - func githubClient(t *testing.T, gh *FakeGitHub) (*github.Client, func()) { t.Helper() @@ -148,6 +149,7 @@ func TestGitHub(t *testing.T) { } prPath := filepath.Join(dir, "pr.json") + rawStatusPath := filepath.Join(dir, "github/status.json") rawPRPath := filepath.Join(dir, "github/pr.json") rawCommentPath := filepath.Join(dir, "github/comments/1.json") @@ -164,14 +166,16 @@ func TestGitHub(t *testing.T) { Branch: pr.GetBase().GetRef(), SHA: pr.GetBase().GetSHA(), }, + Statuses: []*Status{}, Comments: []*Comment{{ ID: comment.GetID(), Author: comment.GetUser().GetLogin(), Text: comment.GetBody(), Raw: rawCommentPath, }}, - Labels: []*Label{}, - Raw: rawPRPath, + Labels: []*Label{}, + Raw: rawPRPath, + RawStatus: rawStatusPath, } gotPR := new(PullRequest) @@ -214,6 +218,12 @@ func TestUpload(t *testing.T) { Labels: []*Label{{ Text: "tacocat", }}, + Statuses: []*Status{{ + ID: "Tekton", + Code: Success, + Description: "Test all the things!", + URL: "https://tekton.dev", + }}, } dir := os.TempDir() prPath := filepath.Join(dir, "pr.json") @@ -253,6 +263,23 @@ func TestUpload(t *testing.T) { t.Errorf("Upload comment -want +got: %s", diff) } + tektonStatus := tektonPR.Statuses[0] + wantStatus := &github.CombinedStatus{ + TotalCount: github.Int(1), + Statuses: []github.RepoStatus{{ + Context: github.String(tektonStatus.ID), + Description: github.String(tektonStatus.Description), + State: github.String("success"), + TargetURL: github.String(tektonStatus.URL), + }}, + } + gotStatus, resp, err := h.Client.Repositories.GetCombinedStatus(ctx, owner, repo, tektonPR.Head.SHA, nil) + if err != nil { + t.Fatalf("GetCombinedStatus: wanted OK, got %+v, %v", resp, err) + } + if diff := cmp.Diff(wantStatus, gotStatus); diff != "" { + t.Errorf("GetCombinedStatus: -want +got: %s", diff) + } } func diffFile(t *testing.T, path string, want interface{}, got interface{}) { @@ -486,3 +513,122 @@ func TestUploadComments(t *testing.T) { t.Errorf("-want +got: %v", diff) } } + +func TestGetStatuses(t *testing.T) { + ctx := context.Background() + h, close := newHandler(ctx, t) + defer close() + + rs := []*github.RepoStatus{ + { + Context: github.String("a"), + State: github.String("success"), + }, + { + Context: github.String("b"), + State: github.String("failure"), + }, + } + for _, s := range rs { + if _, _, err := h.Client.Repositories.CreateStatus(ctx, owner, repo, sha, s); err != nil { + t.Fatalf("CreateStatus: %v", err) + } + } + + testCases := []struct { + sha string + want []*Status + }{ + { + sha: sha, + want: []*Status{ + { + ID: "a", + Code: Success, + }, + { + ID: "b", + Code: Failure, + }, + }, + }, + { + sha: "deadbeef", + want: []*Status{}, + }, + } + + dir := os.TempDir() + for _, tc := range testCases { + t.Run(tc.sha, func(t *testing.T) { + path := filepath.Join(dir, tc.sha) + s, err := h.getStatuses(ctx, tc.sha, path) + if err != nil { + t.Fatalf("getStatuses: %v", err) + } + if diff := cmp.Diff(tc.want, s); diff != "" { + t.Errorf("-want +got: %s", diff) + } + }) + } +} + +func TestGetStatusesError(t *testing.T) { + ctx := context.Background() + h, close := newHandler(ctx, t) + defer close() + + s := &github.RepoStatus{ + Context: github.String("a"), + // Not a valid GitHub status state + State: github.String("unknown"), + } + if _, _, err := h.Client.Repositories.CreateStatus(ctx, owner, repo, sha, s); err != nil { + t.Fatalf("CreateStatus: %v", err) + } + + got, err := h.getStatuses(ctx, sha, "") + if err == nil { + t.Fatalf("getStatuses: want err, got %+v", got) + } +} + +func TestUploadStatuses(t *testing.T) { + ctx := context.Background() + h, close := newHandler(ctx, t) + defer close() + + s := []*Status{ + // Invaid status code. Should fail. + { + Code: StatusCode(""), + }, + { + Code: Failure, + }, + // Should overwrite previous status. + { + Code: "success", + }, + } + + if err := h.uploadStatuses(ctx, sha, s); err == nil { + t.Error("uploadStatus want error, got nil") + } + want := &github.CombinedStatus{ + TotalCount: github.Int(1), + Statuses: []github.RepoStatus{{ + Context: github.String(""), + State: github.String("success"), + Description: github.String(""), + TargetURL: github.String(""), + }}, + } + got, resp, err := h.Client.Repositories.GetCombinedStatus(ctx, owner, repo, sha, nil) + if err != nil { + t.Fatalf("GetCombinedStatus: wanted OK, got %+v, %v", resp, err) + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("GetCombinedStatus: -want +got: %s", diff) + } +} diff --git a/cmd/pullrequest-init/types.go b/cmd/pullrequest-init/types.go index 4b71bc63a7d..825df553639 100644 --- a/cmd/pullrequest-init/types.go +++ b/cmd/pullrequest-init/types.go @@ -3,16 +3,37 @@ package main // These are the generic resource types used for interacting with Pull Requests // and related resources. They should not be tied to any particular SCM system. +type StatusCode string + +// TODO: Figure out how to do case insensitive statuses. +const ( + Unknown StatusCode = "unknown" + Success StatusCode = "success" + Failure StatusCode = "failure" + Error StatusCode = "error" + Neutral StatusCode = "neutral" + Queued StatusCode = "queued" + InProgress StatusCode = "in_progress" + Timeout StatusCode = "timeout" + Canceled StatusCode = "canceled" + ActionRequired StatusCode = "action_required" +) + +// TODO: Add getters to make types nil-safe. + // PullRequest represents a generic pull request resource. type PullRequest struct { Type string ID int64 Head, Base *GitReference + Statuses []*Status Comments []*Comment Labels []*Label // Path to raw pull request payload. Raw string + // Path to raw status payload. + RawStatus string } // GitReference represents a git ref object. See @@ -23,6 +44,17 @@ type GitReference struct { SHA string } +type Status struct { + // ID uniquely distinguish this status from other status types. + ID string + // Code defines the status of the status. + Code StatusCode + // Short summary of the status. + Description string + // Where the status should link to. + URL string +} + // Comment represents a pull request comment. type Comment struct { Text string diff --git a/docs/resources.md b/docs/resources.md index 1c4656d3a19..9ddc072aa18 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -191,6 +191,15 @@ Example payload: ], "Raw": "/tmp/pr/github/pr.json", "Type": "github" + "Statuses": [ + { + "Code": "success", + "Description": "Job succeeded.", + "ID": "pull-tekton-pipeline-go-coverage", + "URL": "https://tekton-releases.appspot.com/build/tekton-prow/pr-logs/pull/tektoncd_pipeline/895/pull-tekton-pipeline-go-coverage/1141483806818045953/" + }, + ], + "RawStatus": "/tmp/pr/github/status.json" } ``` @@ -219,6 +228,29 @@ Params that can be added are the following: 1. `url`: represents the location of the pull request to fetch. +#### Statuses + +The following status codes are available to use for the Pull Request resource: + +Status | +--------------- | +success | +neutral | +queued | +in_progress | +failure | +unknown | +error | +timeout | +canceled | +action_required | + +Note: Status codes are currently **case sensitive**. + +For more information on how Tekton Pull Request status codes convert to SCM +provider statuses, see +[pullrequest-init/README.md](/cmd/pullrequest-init/README.md). + #### GitHub The Pull Request resource will look for GitHub OAuth authentication tokens in