Gracefully degrade when fetching annotations fails due to 403 (#9113)
Co-authored-by: William Martin <williammartin@github.com>
This commit is contained in:
parent
04d0ec0e8c
commit
f647131e1d
6 changed files with 125 additions and 12 deletions
|
|
@ -261,6 +261,14 @@ type CheckRun struct {
|
|||
ID int64
|
||||
}
|
||||
|
||||
var ErrMissingAnnotationsPermissions = errors.New("missing annotations permissions error")
|
||||
|
||||
// GetAnnotations fetches annotations from the REST API.
|
||||
//
|
||||
// If the job has no annotations, an empy slice is returned.
|
||||
// If the API returns a 403, a custom ErrMissingAnnotationsPermissions error is returned.
|
||||
//
|
||||
// When fine-grained PATs support checks:read permission, we can remove the need for this at the call sites.
|
||||
func GetAnnotations(client *api.Client, repo ghrepo.Interface, job Job) ([]Annotation, error) {
|
||||
var result []*Annotation
|
||||
|
||||
|
|
@ -269,9 +277,18 @@ func GetAnnotations(client *api.Client, repo ghrepo.Interface, job Job) ([]Annot
|
|||
err := client.REST(repo.RepoHost(), "GET", path, nil, &result)
|
||||
if err != nil {
|
||||
var httpError api.HTTPError
|
||||
if errors.As(err, &httpError) && httpError.StatusCode == 404 {
|
||||
if !errors.As(err, &httpError) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if httpError.StatusCode == http.StatusNotFound {
|
||||
return []Annotation{}, nil
|
||||
}
|
||||
|
||||
if httpError.StatusCode == http.StatusForbidden {
|
||||
return nil, ErrMissingAnnotationsPermissions
|
||||
}
|
||||
|
||||
return nil, err
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -109,6 +109,16 @@ var FailedJob Job = Job{
|
|||
},
|
||||
}
|
||||
|
||||
var SuccessfulJobAnnotations []Annotation = []Annotation{
|
||||
{
|
||||
JobName: "cool job",
|
||||
Message: "the job is happy",
|
||||
Path: "blaze.py",
|
||||
Level: "notice",
|
||||
StartLine: 420,
|
||||
},
|
||||
}
|
||||
|
||||
var FailedJobAnnotations []Annotation = []Annotation{
|
||||
{
|
||||
JobName: "sad job",
|
||||
|
|
|
|||
|
|
@ -338,10 +338,17 @@ func runView(opts *ViewOptions) error {
|
|||
}
|
||||
|
||||
var annotations []shared.Annotation
|
||||
var missingAnnotationsPermissions bool
|
||||
|
||||
for _, job := range jobs {
|
||||
as, err := shared.GetAnnotations(client, repo, job)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to get annotations: %w", err)
|
||||
if err != shared.ErrMissingAnnotationsPermissions {
|
||||
return fmt.Errorf("failed to get annotations: %w", err)
|
||||
}
|
||||
|
||||
missingAnnotationsPermissions = true
|
||||
break
|
||||
}
|
||||
annotations = append(annotations, as...)
|
||||
}
|
||||
|
|
@ -373,7 +380,11 @@ func runView(opts *ViewOptions) error {
|
|||
fmt.Fprintln(out, shared.RenderJobs(cs, jobs, true))
|
||||
}
|
||||
|
||||
if len(annotations) > 0 {
|
||||
if missingAnnotationsPermissions {
|
||||
fmt.Fprintln(out)
|
||||
fmt.Fprintln(out, cs.Bold("ANNOTATIONS"))
|
||||
fmt.Fprintln(out, "requesting annotations returned 403 Forbidden as the token does not have sufficient permissions. Note that it is not currently possible to create a fine-grained PAT with the `checks:read` permission.")
|
||||
} else if len(annotations) > 0 {
|
||||
fmt.Fprintln(out)
|
||||
fmt.Fprintln(out, cs.Bold("ANNOTATIONS"))
|
||||
fmt.Fprintln(out, shared.RenderAnnotations(cs, annotations))
|
||||
|
|
|
|||
|
|
@ -1320,6 +1320,38 @@ func TestViewRun(t *testing.T) {
|
|||
errMsg: "failed to get annotations: HTTP 500 (https://api.github.com/repos/OWNER/REPO/check-runs/20/annotations)",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "annotation endpoint forbidden (fine grained tokens)",
|
||||
tty: true,
|
||||
opts: &ViewOptions{
|
||||
RunID: "1234",
|
||||
},
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"),
|
||||
httpmock.JSONResponse(shared.FailedRun))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"),
|
||||
httpmock.JSONResponse(shared.TestWorkflow))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/artifacts"),
|
||||
httpmock.StringResponse(`{}`))
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query PullRequestForRun`),
|
||||
httpmock.StringResponse(``))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "runs/1234/jobs"),
|
||||
httpmock.JSONResponse(shared.JobsPayload{
|
||||
Jobs: []shared.Job{
|
||||
shared.FailedJob,
|
||||
},
|
||||
}))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/check-runs/20/annotations"),
|
||||
httpmock.StatusStringResponse(403, "Forbidden"))
|
||||
},
|
||||
wantOut: "\nX trunk CI · 1234\nTriggered via push about 59 minutes ago\n\nJOBS\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nrequesting annotations returned 403 Forbidden as the token does not have sufficient permissions. Note that it is not currently possible to create a fine-grained PAT with the `checks:read` permission.\n\nTo see what failed, try: gh run view 1234 --log-failed\nView this run on GitHub: https://github.com/runs/1234\n",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
|
|||
|
|
@ -219,19 +219,24 @@ func renderRun(out io.Writer, opts WatchOptions, client *api.Client, repo ghrepo
|
|||
}
|
||||
|
||||
var annotations []shared.Annotation
|
||||
var missingAnnotationsPermissions bool
|
||||
|
||||
var annotationErr error
|
||||
var as []shared.Annotation
|
||||
for _, job := range jobs {
|
||||
if as, ok := annotationCache[job.ID]; ok {
|
||||
annotations = as
|
||||
continue
|
||||
}
|
||||
|
||||
as, annotationErr = shared.GetAnnotations(client, repo, job)
|
||||
if annotationErr != nil {
|
||||
as, err := shared.GetAnnotations(client, repo, job)
|
||||
if err != nil {
|
||||
if err != shared.ErrMissingAnnotationsPermissions {
|
||||
return nil, fmt.Errorf("failed to get annotations: %w", err)
|
||||
}
|
||||
|
||||
missingAnnotationsPermissions = true
|
||||
break
|
||||
}
|
||||
|
||||
annotations = append(annotations, as...)
|
||||
|
||||
if job.Status != shared.InProgress {
|
||||
|
|
@ -239,10 +244,6 @@ func renderRun(out io.Writer, opts WatchOptions, client *api.Client, repo ghrepo
|
|||
}
|
||||
}
|
||||
|
||||
if annotationErr != nil {
|
||||
return nil, fmt.Errorf("failed to get annotations: %w", annotationErr)
|
||||
}
|
||||
|
||||
fmt.Fprintln(out, shared.RenderRunHeader(cs, *run, text.FuzzyAgo(opts.Now(), run.StartedTime()), prNumber, 0))
|
||||
fmt.Fprintln(out)
|
||||
|
||||
|
|
@ -254,7 +255,11 @@ func renderRun(out io.Writer, opts WatchOptions, client *api.Client, repo ghrepo
|
|||
|
||||
fmt.Fprintln(out, shared.RenderJobs(cs, jobs, true))
|
||||
|
||||
if len(annotations) > 0 {
|
||||
if missingAnnotationsPermissions {
|
||||
fmt.Fprintln(out)
|
||||
fmt.Fprintln(out, cs.Bold("ANNOTATIONS"))
|
||||
fmt.Fprintf(out, "requesting annotations returned 403 Forbidden as the token does not have sufficient permissions. Note that it is not currently possible to create a fine-grained PAT with the `checks:read` permission.")
|
||||
} else if len(annotations) > 0 {
|
||||
fmt.Fprintln(out)
|
||||
fmt.Fprintln(out, cs.Bold("ANNOTATIONS"))
|
||||
fmt.Fprintln(out, shared.RenderAnnotations(cs, annotations))
|
||||
|
|
|
|||
|
|
@ -326,6 +326,44 @@ func TestWatchRun(t *testing.T) {
|
|||
wantErr: true,
|
||||
errMsg: "failed to get run: HTTP 404: run 1234 not found (https://api.github.com/repos/OWNER/REPO/actions/runs/1234?exclude_pull_requests=true)",
|
||||
},
|
||||
{
|
||||
name: "annotation endpoint forbidden (fine grained tokens)",
|
||||
tty: true,
|
||||
opts: &WatchOptions{
|
||||
RunID: "2",
|
||||
},
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
inProgressRun := shared.TestRunWithCommit(2, shared.InProgress, "", "commit2")
|
||||
completedRun := shared.TestRun(2, shared.Completed, shared.Success)
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/2"),
|
||||
httpmock.JSONResponse(inProgressRun))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "runs/2/jobs"),
|
||||
httpmock.JSONResponse(shared.JobsPayload{
|
||||
Jobs: []shared.Job{
|
||||
shared.SuccessfulJob,
|
||||
shared.FailedJob,
|
||||
},
|
||||
}))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/check-runs/10/annotations"),
|
||||
httpmock.JSONResponse(shared.SuccessfulJobAnnotations))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/check-runs/20/annotations"),
|
||||
httpmock.StatusStringResponse(403, "Forbidden"))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/2"),
|
||||
httpmock.JSONResponse(completedRun))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"),
|
||||
httpmock.JSONResponse(shared.TestWorkflow))
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"),
|
||||
httpmock.JSONResponse(shared.TestWorkflow))
|
||||
},
|
||||
wantOut: "\x1b[?1049h\x1b[?1049l✓ trunk CI · 2\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nrequesting annotations returned 403 Forbidden as the token does not have sufficient permissions. Note that it is not currently possible to create a fine-grained PAT with the `checks:read` permission.\n✓ Run CI (2) completed with 'success'\n",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue