Skip to content

Commit baea213

Browse files
zakiskchmouel
authored andcommitted
fix(github): handle check run status for validation errors
Fix check run creation to properly use status and conclusion from statusOpts instead of hardcoding "in_progress". Also include output fields (Title, Summary, Text) in check runs. Prevent patching PipelineRun when validation fails by checking if PipelineRun has a valid name before patching with checkRunID and logURL. Validation errors provide PipelineRun struct but it's not a valid cluster resource. Also fix infinite loop in test by incrementing counter. https://issues.redhat.com/browse/SRVKP-9206 Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
1 parent f3db57f commit baea213

File tree

4 files changed

+32
-6
lines changed

4 files changed

+32
-6
lines changed

pkg/provider/github/status.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,23 @@ func (v *Provider) canIUseCheckrunID(checkrunid *int64) bool {
112112
func (v *Provider) createCheckRunStatus(ctx context.Context, runevent *info.Event, status provider.StatusOpts) (*int64, error) {
113113
now := github.Timestamp{Time: time.Now()}
114114
checkrunoption := github.CreateCheckRunOptions{
115-
Name: provider.GetCheckName(status, v.pacInfo),
116-
HeadSHA: runevent.SHA,
117-
Status: github.Ptr("in_progress"),
115+
Name: provider.GetCheckName(status, v.pacInfo),
116+
HeadSHA: runevent.SHA,
117+
Status: github.Ptr(status.Status), // take status from statusOpts because it can be in_progress, queued, or failure // same for conclusion as well
118+
Output: &github.CheckRunOutput{
119+
Title: github.Ptr(status.Title),
120+
Summary: github.Ptr(status.Summary),
121+
Text: github.Ptr(status.Text),
122+
},
118123
DetailsURL: github.Ptr(status.DetailsURL),
119124
ExternalID: github.Ptr(status.PipelineRunName),
120125
StartedAt: &now,
121126
}
122127

128+
if status.Status != "in_progress" && status.Status != "queued" {
129+
checkrunoption.Conclusion = github.Ptr(status.Conclusion)
130+
}
131+
123132
checkRun, _, err := wrapAPI(v, "create_check_run", func() (*github.CheckRun, *github.Response, error) {
124133
return v.Client().Checks.CreateCheckRun(ctx, runevent.Organization, runevent.Repository, checkrunoption)
125134
})
@@ -232,7 +241,11 @@ func (v *Provider) getOrUpdateCheckRunStatus(ctx context.Context, runevent *info
232241
return err
233242
}
234243
}
235-
if statusOpts.PipelineRun != nil {
244+
245+
// Patch the pipelineRun with the checkRunID and logURL only when the pipelineRun is not nil and has a name
246+
// because on validation failed PipelineRun will provide PipelineRun struct but it is not a valid resource
247+
// created in cluster so if its only validation error report then ignore patching the pipelineRun.
248+
if statusOpts.PipelineRun != nil && (statusOpts.PipelineRun.GetName() != "" || statusOpts.PipelineRun.GetGenerateName() != "") {
236249
if _, err := action.PatchPipelineRun(ctx, v.Logger, "checkRunID and logURL", v.Run.Clients.Tekton, statusOpts.PipelineRun, metadataPatch(checkRunID, statusOpts.DetailsURL)); err != nil {
237250
return err
238251
}

pkg/provider/github/status_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,20 @@ func TestGithubProviderCreateStatus(t *testing.T) {
336336
want: &github.CheckRun{ID: &resultid},
337337
wantErr: false,
338338
},
339+
{
340+
name: "validation failure",
341+
args: args{
342+
runevent: runEvent,
343+
status: "completed",
344+
conclusion: "failure",
345+
text: "There was an error creating the PipelineRun: ```admission webhook \"validation.webhook.pipeline.tekton.dev\" denied the request: validation failed: invalid value: 0s```",
346+
detailsURL: "https://cireport.com",
347+
titleSubstr: "Failed",
348+
githubApps: true,
349+
},
350+
want: &github.CheckRun{ID: &resultid},
351+
wantErr: false,
352+
},
339353
{
340354
name: "failure from bot",
341355
args: args{

test/github_config_maxkeepruns_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
//go:build e2e
2-
31
package test
42

53
import (

test/github_pullrequest_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ func TestGithubPullRequestInvalidSpecValues(t *testing.T) {
264264
break
265265
}
266266
time.Sleep(5 * time.Second)
267+
counter++
267268
}
268269

269270
assert.Equal(t, len(res.CheckRuns), 1)

0 commit comments

Comments
 (0)