Skip to content

Commit 4be1f06

Browse files
krishhlogantheakshaypant
authored andcommitted
fix(incoming): honor skip-CI exemption for incoming webhooks
Incoming webhooks are explicit user-triggered runs. The secondary skip-CI guard in Run() only exempted GitOps comment events but not incoming webhooks. Because GetCommitInfo sets HasSkipCommand from the head commit message during verifyRepoAndUser, a request whose HEAD commit contained skip-CI marker was silently dropped. Added the incoming-webhook exemption alongside the existing GitOps exemption. Signed-off-by: krishhlogan <kloganat@redhat.com> Assisted-by: Claude Sonnet 4.6 (via Claude Code)
1 parent 3698c72 commit 4be1f06

4 files changed

Lines changed: 65 additions & 8 deletions

File tree

‎pkg/params/info/events.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ type Event struct {
6262

6363
// HasSkipCommand indicates whether the commit message contains a skip CI command
6464
// (e.g., [skip ci], [ci skip], [skip tkn], [tkn skip]). When true, PipelineRun
65-
// execution will be skipped unless overridden by a GitOps command (e.g., /test, /retest).
65+
// execution will be skipped unless overridden by a GitOps command (e.g., /test, /retest)
66+
// or the event came from an incoming webhook, since both are explicit user-triggered runs.
6667
// This allows users to bypass CI for documentation changes or minor fixes while still
6768
// maintaining the ability to manually trigger builds when needed.
6869
HasSkipCommand bool

‎pkg/pipelineascode/pipelineascode.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ func (p *PacRun) Run(ctx context.Context) error {
135135
// Defensive skip-CI check: this is a safety net in case events bypass the early check in sinker.
136136
// Primary skip detection happens in sinker.processEvent() for performance, but this ensures
137137
// nothing slips through (e.g., tests that call Run() directly, or edge cases).
138-
// Skip only for non-GitOps events (GitOps commands can override skip-CI).
139-
if p.event.HasSkipCommand && !opscomments.IsAnyOpsEventType(p.event.EventType) {
138+
// Skip CI unless this is a GitOps event or incoming webhook (both override skip-CI markers).
139+
if p.event.HasSkipCommand && !opscomments.IsAnyOpsEventType(p.event.EventType) && p.event.EventType != triggertype.Incoming.String() {
140140
p.logger.Infof("CI skipped: commit contains skip command in message (secondary check)")
141141
return nil
142142
}

‎pkg/pipelineascode/pipelineascode_test.go‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func replyString(mux *http.ServeMux, url, body string) {
5858
})
5959
}
6060

61-
func testSetupCommonGhReplies(t *testing.T, mux *http.ServeMux, runevent info.Event, finalStatus, finalStatusText string, noReplyOrgPublicMembers bool) {
61+
func testSetupCommonGhReplies(t *testing.T, mux *http.ServeMux, runevent info.Event, finalStatus, finalStatusText string, noReplyOrgPublicMembers bool, commitMessage string) {
6262
t.Helper()
6363
// Take a directory and generate replies as Github for it
6464
replyString(mux,
@@ -72,11 +72,19 @@ func testSetupCommonGhReplies(t *testing.T, mux *http.ServeMux, runevent info.Ev
7272
fmt.Sprintf("/repos/%s/%s/statuses/%s", runevent.Organization, runevent.Repository, runevent.SHA),
7373
"{}")
7474

75-
jj := fmt.Sprintf(`{"sha": "%s", "html_url": "https://git.commit.url/%s", "message": "commit message"}`,
76-
runevent.SHA, runevent.SHA)
75+
commitMsg := "commit message"
76+
if commitMessage != "" {
77+
commitMsg = commitMessage
78+
}
79+
commitJSON, err := json.Marshal(map[string]string{
80+
"sha": runevent.SHA,
81+
"html_url": "https://git.commit.url/" + runevent.SHA,
82+
"message": commitMsg,
83+
})
84+
assert.NilError(t, err)
7785
replyString(mux,
7886
fmt.Sprintf("/repos/%s/%s/git/commits/%s", runevent.Organization, runevent.Repository, runevent.SHA),
79-
jj)
87+
string(commitJSON))
8088

8189
if !noReplyOrgPublicMembers {
8290
mux.HandleFunc("/orgs/"+runevent.Organization+"/members", func(rw http.ResponseWriter, _ *http.Request) {
@@ -137,6 +145,7 @@ func TestRun(t *testing.T) {
137145
PayloadEncodedSecret string
138146
concurrencyLimit int
139147
expectedLogSnippet string
148+
commitMessage string
140149
expectedPostedComment string // TODO: multiple posted comments when we need it
141150
secretCreationError error // Error to inject for secret creation
142151
}{
@@ -357,6 +366,27 @@ func TestRun(t *testing.T) {
357366
tektondir: "testdata/push_branch",
358367
finalStatus: "neutral",
359368
},
369+
{
370+
// Incoming webhooks are explicit user-triggered runs, so a skip-CI
371+
// marker in the head commit message must not suppress them.
372+
name: "Incoming/skip-ci-marker-is-ignored",
373+
runevent: info.Event{
374+
SHA: "resolvedsha123",
375+
Organization: "organizationes",
376+
Repository: "lagaffe",
377+
URL: "https://service/documentation",
378+
Sender: "incoming",
379+
HeadBranch: "refs/heads/main",
380+
BaseBranch: "refs/heads/main",
381+
DefaultBranch: "main",
382+
EventType: triggertype.Incoming.String(),
383+
TriggerTarget: "push",
384+
},
385+
tektondir: "testdata/push_branch",
386+
commitMessage: "fix: update docs [skip ci]",
387+
finalStatus: "neutral",
388+
expectedLogSnippet: "has been created",
389+
},
360390
{
361391
name: "Push/tags",
362392
runevent: info.Event{
@@ -611,7 +641,7 @@ func TestRun(t *testing.T) {
611641
},
612642
}
613643

614-
testSetupCommonGhReplies(t, mux, tt.runevent, tt.finalStatus, tt.finalStatusText, tt.skipReplyingOrgPublicMembers)
644+
testSetupCommonGhReplies(t, mux, tt.runevent, tt.finalStatus, tt.finalStatusText, tt.skipReplyingOrgPublicMembers, tt.commitMessage)
615645
if tt.tektondir != "" {
616646
ghtesthelper.SetupGitTree(t, mux, tt.tektondir, &tt.runevent, false)
617647
}
@@ -738,6 +768,8 @@ func TestRun(t *testing.T) {
738768
logURL, ok := pr.Annotations[path.Join(apipac.GroupName, "log-url")]
739769
assert.Assert(t, ok, "failed to find log-url label on pipelinerun: %s/%s", pr.GetNamespace(), pr.GetGenerateName())
740770
assert.Equal(t, logURL, cs.Clients.ConsoleUI().DetailURL(&pr))
771+
assert.Equal(t, pr.Labels[keys.SHA], tt.runevent.SHA, "sha label mismatch on pipelinerun %s", pr.GetName())
772+
assert.Equal(t, provider.SkipCI(tt.commitMessage), tt.runevent.HasSkipCommand, "HasSkipCommand mismatch for commit message %q", tt.commitMessage)
741773

742774
if pacInfo.SecretAutoCreation {
743775
secretName, ok := pr.GetAnnotations()[keys.GitAuthSecret]

‎test/github_incoming_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ func TestGithubGHEAppIncomingNoMatchExactName(t *testing.T) {
181181
}
182182

183183
func verifyIncomingWebhook(t *testing.T, randomedString, pipelinerunName string, entries map[string]string, targets []string, onWebhook bool, numberOfPR int) {
184+
verifyIncomingWebhookWithCommitTitle(t, randomedString, pipelinerunName, entries, targets, onWebhook, numberOfPR, "")
185+
}
186+
187+
func verifyIncomingWebhookWithCommitTitle(t *testing.T, randomedString, pipelinerunName string, entries map[string]string, targets []string, onWebhook bool, numberOfPR int, commitTitle string) {
184188
ctx := context.Background()
185189
onGHE := true // All incoming webhook tests use GHE
186190
ctx, runcnx, opts, ghprovider, err := tgithub.Setup(ctx, onGHE, onWebhook)
@@ -245,6 +249,9 @@ func verifyIncomingWebhook(t *testing.T, randomedString, pipelinerunName string,
245249
targetRefName := fmt.Sprintf("refs/heads/%s", randomedString)
246250

247251
title := "TestGithubAppIncoming - " + randomedString
252+
if commitTitle != "" {
253+
title = commitTitle
254+
}
248255
sha, vref, err := tgithub.PushFilesToRef(ctx, ghprovider.Client(), title,
249256
repoinfo.GetDefaultBranch(),
250257
targetRefName,
@@ -498,6 +505,23 @@ func TestGithubGHEAppIncomingDefaultBranchProvenance(t *testing.T) {
498505
assert.NilError(t, err, "Error while checking the logs of the pods")
499506
}
500507

508+
// TestGithubGHEAppIncomingSkipCI verifies that an incoming webhook creates a
509+
// PipelineRun even when the HEAD commit message contains a [skip ci] marker.
510+
// Incoming webhooks are explicit user-triggered runs and must not be suppressed
511+
// by the skip-CI convention.
512+
func TestGithubGHEAppIncomingSkipCI(t *testing.T) {
513+
randomedString := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
514+
515+
entries, err := payload.GetEntries(map[string]string{
516+
".tekton/pipelinerun-incoming.yaml": "testdata/pipelinerun-incoming.yaml",
517+
}, randomedString, randomedString, triggertype.Incoming.String(), map[string]string{})
518+
assert.NilError(t, err)
519+
520+
verifyIncomingWebhookWithCommitTitle(t, randomedString, "pipelinerun-incoming", entries,
521+
[]string{randomedString}, false, 1,
522+
"chore: update readme [skip ci]")
523+
}
524+
501525
// Local Variables:
502526
// compile-command: "go test -tags=e2e -v -run TestGithubAppIncoming ."
503527
// End:

0 commit comments

Comments
 (0)