Skip to content

Commit a387d41

Browse files
vdemeestertheakshaypant
authored andcommitted
fix: populate DefaultBranch for incoming webhooks
Incoming webhooks bypass ParsePayload, which means the event's DefaultBranch field is never set. When pipelinerun_provenance is set to 'default_branch' on the Repository CR, GetTektonDir() uses the empty DefaultBranch as the git tree revision, causing a 404 from the GitHub API. Fix this by fetching the repository's default branch from the GitHub API in GetCommitInfo() when the event type is 'incoming' and DefaultBranch is empty. Also fix a pre-existing test gap: the 'error' test case in TestGithubGetCommitInfo was missing wantErr, and the test was not checking for err == nil on the success path. Fixes: #2646
1 parent d78ef4e commit a387d41

File tree

3 files changed

+213
-0
lines changed

3 files changed

+213
-0
lines changed

pkg/provider/github/github.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,21 @@ func (v *Provider) GetCommitInfo(ctx context.Context, runevent *info.Event) erro
463463
}
464464
}
465465

466+
// For incoming webhooks, DefaultBranch is not populated from the event
467+
// payload (since there is no webhook payload to parse). Fetch it from the
468+
// GitHub API so that pipelinerun_provenance: default_branch works correctly.
469+
// For other event types (push, pull_request), DefaultBranch is already set
470+
// by ParsePayload from the webhook payload's repository.default_branch field.
471+
if runevent.DefaultBranch == "" && runevent.EventType == "incoming" {
472+
ghRepo, _, err := wrapAPI(v, "get_repo", func() (*github.Repository, *github.Response, error) {
473+
return v.Client().Repositories.Get(ctx, runevent.Organization, runevent.Repository)
474+
})
475+
if err != nil {
476+
return err
477+
}
478+
runevent.DefaultBranch = ghRepo.GetDefaultBranch()
479+
}
480+
466481
return nil
467482
}
468483

pkg/provider/github/github_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,31 @@ func TestGithubGetCommitInfo(t *testing.T) {
932932
message: "chore: deps\n\n[tkn skip]",
933933
wantHasSkipCmd: true,
934934
},
935+
{
936+
name: "incoming webhook populates DefaultBranch",
937+
event: &info.Event{
938+
Organization: "owner",
939+
Repository: "repository",
940+
SHA: "shacommitinfo",
941+
EventType: "incoming",
942+
},
943+
shaurl: "https://git.provider/commit/info",
944+
shatitle: "My beautiful pony",
945+
message: "My beautiful pony",
946+
},
947+
{
948+
name: "DefaultBranch already set is preserved",
949+
event: &info.Event{
950+
Organization: "owner",
951+
Repository: "repository",
952+
SHA: "shacommitinfo",
953+
DefaultBranch: "develop",
954+
EventType: "incoming",
955+
},
956+
shaurl: "https://git.provider/commit/info",
957+
shatitle: "My beautiful pony",
958+
message: "My beautiful pony",
959+
},
935960
{
936961
name: "error",
937962
event: &info.Event{
@@ -940,6 +965,7 @@ func TestGithubGetCommitInfo(t *testing.T) {
940965
SHA: "shacommitinfo",
941966
},
942967
apiReply: "hello moto",
968+
wantErr: "invalid character",
943969
},
944970
{
945971
name: "noclient",
@@ -952,6 +978,12 @@ func TestGithubGetCommitInfo(t *testing.T) {
952978
t.Run(tt.name, func(t *testing.T) {
953979
fakeclient, mux, _, teardown := ghtesthelper.SetupGH()
954980
defer teardown()
981+
// Mock the repo endpoint so GetCommitInfo can resolve DefaultBranch
982+
// when it is not already set on the event (e.g. incoming webhooks).
983+
mux.HandleFunc(fmt.Sprintf("/repos/%s/%s",
984+
tt.event.Organization, tt.event.Repository), func(rw http.ResponseWriter, _ *http.Request) {
985+
fmt.Fprint(rw, `{"default_branch": "main"}`)
986+
})
955987
mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/git/commits/%s",
956988
tt.event.Organization, tt.event.Repository, tt.event.SHA), func(rw http.ResponseWriter, _ *http.Request) {
957989
if tt.apiReply != "" {
@@ -1022,6 +1054,7 @@ func TestGithubGetCommitInfo(t *testing.T) {
10221054
assert.ErrorContains(t, err, tt.wantErr)
10231055
return
10241056
}
1057+
assert.NilError(t, err)
10251058
assert.Equal(t, tt.shatitle, tt.event.SHATitle)
10261059
assert.Equal(t, tt.shaurl, tt.event.SHAURL)
10271060

@@ -1040,6 +1073,16 @@ func TestGithubGetCommitInfo(t *testing.T) {
10401073
assert.DeepEqual(t, expectedCommitterDate, tt.event.SHACommitterDate)
10411074
}
10421075
assert.Equal(t, tt.wantHasSkipCmd, tt.event.HasSkipCommand)
1076+
1077+
// For incoming events, verify DefaultBranch is populated
1078+
if tt.event.EventType == "incoming" {
1079+
if tt.event.DefaultBranch == "develop" {
1080+
// If it was already set, it should be preserved
1081+
assert.Equal(t, "develop", tt.event.DefaultBranch, "DefaultBranch should be preserved")
1082+
} else {
1083+
assert.Equal(t, "main", tt.event.DefaultBranch, "DefaultBranch should be populated from API")
1084+
}
1085+
}
10431086
})
10441087
}
10451088
}

test/github_incoming_test.go

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
2121
tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github"
2222
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload"
23+
repository "github.com/openshift-pipelines/pipelines-as-code/test/pkg/repository"
2324
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/secret"
2425
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
2526
"github.com/tektoncd/pipeline/pkg/names"
@@ -341,6 +342,160 @@ func verifyIncomingWebhook(t *testing.T, randomedString, pipelinerunName string,
341342
}
342343
}
343344

345+
// TestGithubGHEAppIncomingDefaultBranchProvenance tests that incoming webhooks
346+
// work correctly with pipelinerun_provenance set to "default_branch".
347+
// This verifies the fix for https://github.com/tektoncd/pipelines-as-code/issues/2646
348+
// where DefaultBranch was not populated for incoming events.
349+
func TestGithubGHEAppIncomingDefaultBranchProvenance(t *testing.T) {
350+
ctx := context.Background()
351+
onGHE := true
352+
ctx, runcnx, opts, ghprovider, err := tgithub.Setup(ctx, onGHE, false)
353+
assert.NilError(t, err)
354+
355+
randomedString := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
356+
targetBranch := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("incoming-branch")
357+
358+
runcnx.Clients.Log.Infof("Testing incoming webhook with default_branch provenance on %s", randomedString)
359+
360+
repoinfo, resp, err := ghprovider.Client().Repositories.Get(ctx, opts.Organization, opts.Repo)
361+
assert.NilError(t, err)
362+
if resp != nil && resp.StatusCode == http.StatusNotFound {
363+
t.Errorf("Repository %s not found in %s", opts.Organization, opts.Repo)
364+
}
365+
366+
incoming := &[]v1alpha1.Incoming{
367+
{
368+
Type: "webhook-url",
369+
Secret: v1alpha1.Secret{
370+
Name: incomingSecretName,
371+
Key: "incoming",
372+
},
373+
Targets: []string{targetBranch},
374+
Params: []string{
375+
"the_best_superhero_is",
376+
},
377+
},
378+
}
379+
380+
// Create Repository CR with pipelinerun_provenance: default_branch
381+
repo := &v1alpha1.Repository{
382+
ObjectMeta: metav1.ObjectMeta{
383+
Name: randomedString,
384+
},
385+
Spec: v1alpha1.RepositorySpec{
386+
URL: repoinfo.GetHTMLURL(),
387+
Incomings: incoming,
388+
Settings: &v1alpha1.Settings{
389+
PipelineRunProvenance: "default_branch",
390+
},
391+
},
392+
}
393+
394+
err = repository.CreateNS(ctx, randomedString, runcnx)
395+
assert.NilError(t, err)
396+
err = repository.CreateRepo(ctx, randomedString, runcnx, repo)
397+
assert.NilError(t, err)
398+
399+
err = secret.Create(ctx, runcnx, map[string]string{"incoming": incomingSecreteValue}, randomedString, incomingSecretName)
400+
assert.NilError(t, err)
401+
402+
// Push .tekton/ files to the DEFAULT branch (not the incoming target branch)
403+
entries, err := payload.GetEntries(map[string]string{
404+
".tekton/pipelinerun-incoming.yaml": "testdata/pipelinerun-incoming.yaml",
405+
}, randomedString, targetBranch, triggertype.Incoming.String(), map[string]string{})
406+
assert.NilError(t, err)
407+
408+
defaultBranch := repoinfo.GetDefaultBranch()
409+
title := "TestGithubAppIncomingDefaultBranchProvenance - " + randomedString
410+
// Push .tekton/ to default branch by creating a commit and updating the ref
411+
// (PushFilesToRef with empty targetRef creates the commit without creating a new ref)
412+
sha, _, err := tgithub.PushFilesToRef(ctx, ghprovider.Client(), title,
413+
defaultBranch,
414+
"",
415+
opts.Organization,
416+
opts.Repo,
417+
entries)
418+
assert.NilError(t, err)
419+
// Update the existing default branch ref to point to the new commit
420+
_, _, err = ghprovider.Client().Git.UpdateRef(ctx, opts.Organization, opts.Repo,
421+
"refs/heads/"+defaultBranch, github.UpdateRef{SHA: sha})
422+
assert.NilError(t, err)
423+
runcnx.Clients.Log.Infof("Commit %s has been created and pushed to default branch %s", sha, defaultBranch)
424+
425+
// Create the target branch (the incoming webhook targets this branch)
426+
targetSHA, _, err := tgithub.PushFilesToRef(ctx, ghprovider.Client(),
427+
title,
428+
defaultBranch,
429+
fmt.Sprintf("refs/heads/%s", targetBranch),
430+
opts.Organization,
431+
opts.Repo,
432+
map[string]string{"README.md": "# target branch"})
433+
assert.NilError(t, err)
434+
runcnx.Clients.Log.Infof("Target branch %s created with SHA %s", targetBranch, targetSHA)
435+
436+
// Trigger incoming webhook targeting the non-default branch
437+
incomingURL := fmt.Sprintf("%s/incoming", opts.ControllerURL)
438+
jsonBody := map[string]interface{}{
439+
"repository": randomedString,
440+
"branch": targetBranch,
441+
"pipelinerun": "pipelinerun-incoming",
442+
"secret": incomingSecreteValue,
443+
"params": map[string]string{
444+
"the_best_superhero_is": "Superman",
445+
},
446+
}
447+
jsonData, err := json.Marshal(jsonBody)
448+
assert.NilError(t, err)
449+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, incomingURL, strings.NewReader(string(jsonData)))
450+
assert.NilError(t, err)
451+
req.Header.Add("Content-Type", "application/json")
452+
if onGHE {
453+
urlParse, _ := url.Parse(*ghprovider.APIURL)
454+
req.Header.Add("X-GitHub-Enterprise-Host", urlParse.Host)
455+
}
456+
457+
client := &http.Client{}
458+
httpResp, err := client.Do(req)
459+
assert.NilError(t, err)
460+
defer httpResp.Body.Close()
461+
assert.Assert(t, httpResp.StatusCode >= 200 && httpResp.StatusCode < 300,
462+
"HTTP status code mismatch: expected=2xx, actual=%d", httpResp.StatusCode)
463+
464+
runcnx.Clients.Log.Infof("Incoming webhook triggered on URL: %s for branch: %s", incomingURL, targetBranch)
465+
466+
g := tgithub.PRTest{
467+
Cnx: runcnx,
468+
Options: opts,
469+
Provider: ghprovider,
470+
TargetNamespace: randomedString,
471+
TargetRefName: fmt.Sprintf("refs/heads/%s", targetBranch),
472+
PRNumber: -1,
473+
SHA: targetSHA,
474+
Logger: runcnx.Clients.Log,
475+
GHE: onGHE,
476+
}
477+
defer g.TearDown(ctx, t)
478+
479+
sopt := wait.SuccessOpt{
480+
Title: title,
481+
OnEvent: triggertype.Incoming.String(),
482+
TargetNS: randomedString,
483+
NumberofPRMatch: 1,
484+
SHA: "",
485+
}
486+
wait.Succeeded(ctx, t, runcnx, opts, sopt)
487+
488+
// Verify the PipelineRun was created
489+
prs, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(randomedString).List(ctx, metav1.ListOptions{})
490+
assert.NilError(t, err)
491+
assert.Assert(t, len(prs.Items) == 1, "Expected 1 PipelineRun, got %d", len(prs.Items))
492+
493+
err = wait.RegexpMatchingInPodLog(context.Background(), runcnx, randomedString,
494+
"pipelinesascode.tekton.dev/event-type=incoming", "step-task",
495+
*regexp.MustCompile(".*It's a Bird... It's a Plane... It's Superman"), "", 2, nil)
496+
assert.NilError(t, err, "Error while checking the logs of the pods")
497+
}
498+
344499
// Local Variables:
345500
// compile-command: "go test -tags=e2e -v -run TestGithubAppIncoming ."
346501
// End:

0 commit comments

Comments
 (0)