Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/pipelineascode/pipelineascode.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ func (p *PacRun) Run(ctx context.Context) error {
// Defensive skip-CI check: this is a safety net in case events bypass the early check in sinker.
// Primary skip detection happens in sinker.processEvent() for performance, but this ensures
// nothing slips through (e.g., tests that call Run() directly, or edge cases).
// Skip only for non-GitOps events (GitOps commands can override skip-CI).
if p.event.HasSkipCommand && !opscomments.IsAnyOpsEventType(p.event.EventType) {
// Skip only for non-GitOps events (GitOps commands can override skip-CI) and incoming webhook triggers/events.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion to imrpove comment clarity.

Suggested change
// Skip only for non-GitOps events (GitOps commands can override skip-CI) and incoming webhook triggers/events.
// Skip CI unless this is a GitOps event or incoming webhook (both override skip-CI markers).

if p.event.HasSkipCommand && !opscomments.IsAnyOpsEventType(p.event.EventType) && p.event.EventType != triggertype.Incoming.String() {
p.logger.Infof("CI skipped: commit contains skip command in message (secondary check)")
return nil
}
Expand Down
140 changes: 135 additions & 5 deletions pkg/pipelineascode/pipelineascode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func replyString(mux *http.ServeMux, url, body string) {
})
}

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

jj := fmt.Sprintf(`{"sha": "%s", "html_url": "https://git.commit.url/%s", "message": "commit message"}`,
runevent.SHA, runevent.SHA)
commitMsg := "commit message"
if commitMessage != "" {
commitMsg = commitMessage
}
commitJSON, err := json.Marshal(map[string]string{
"sha": runevent.SHA,
"html_url": "https://git.commit.url/" + runevent.SHA,
"message": commitMsg,
})
assert.NilError(t, err)
replyString(mux,
fmt.Sprintf("/repos/%s/%s/git/commits/%s", runevent.Organization, runevent.Repository, runevent.SHA),
jj)
string(commitJSON))

if !noReplyOrgPublicMembers {
mux.HandleFunc("/orgs/"+runevent.Organization+"/members", func(rw http.ResponseWriter, _ *http.Request) {
Expand Down Expand Up @@ -579,7 +587,7 @@ func TestRun(t *testing.T) {
},
}

testSetupCommonGhReplies(t, mux, tt.runevent, tt.finalStatus, tt.finalStatusText, tt.skipReplyingOrgPublicMembers)
testSetupCommonGhReplies(t, mux, tt.runevent, tt.finalStatus, tt.finalStatusText, tt.skipReplyingOrgPublicMembers, "")
if tt.tektondir != "" {
ghtesthelper.SetupGitTree(t, mux, tt.tektondir, &tt.runevent, false)
}
Expand Down Expand Up @@ -791,3 +799,125 @@ type KinterfaceTestWithError struct {
func (k *KinterfaceTestWithError) CreateSecret(_ context.Context, _ string, _ *corev1.Secret) error {
return k.CreateSecretError
}

// TestRunIncomingWebhookIgnoresSkipCI verifies that an incoming webhook always
// creates a PipelineRun even when the head commit message contains a skip-CI
// marker. Incoming webhooks are explicit user-triggered runs; the skip-CI
// convention should not override them.
func TestRunIncomingWebhookIgnoresSkipCI(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
fakeclient, mux, ghTestServerURL, teardown := ghtesthelper.SetupGH()
defer teardown()

runevent := info.Event{
SHA: "resolvedsha123",
Organization: "organizationes",
Repository: "lagaffe",
URL: "https://service/documentation",
Sender: "incoming",
HeadBranch: "refs/heads/main",
BaseBranch: "refs/heads/main",
DefaultBranch: "main",
EventType: "incoming",
TriggerTarget: "push",
}

webhookSecret := "don'tlookatmeplease"
secrets := map[string]string{
info.DefaultPipelinesAscodeSecretName: webhookSecret,
}

testSetupCommonGhReplies(t, mux, runevent, "neutral", "", false, "fix: update docs [skip ci]")
ghtesthelper.SetupGitTree(t, mux, "testdata/push_branch", &runevent, false)

var hubCatalogs sync.Map
hubCatalogs.Store("default", settings.HubCatalog{
Index: "default",
URL: testHubURL,
Name: testCatalogHubName,
})

tdata := testclient.Data{
Namespaces: []*corev1.Namespace{
{ObjectMeta: metav1.ObjectMeta{Name: "namespace"}},
},
Repositories: []*v1alpha1.Repository{
testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{
Name: "test-run",
URL: runevent.URL,
InstallNamespace: "namespace",
}),
},
}
stdata, _ := testclient.SeedTestData(t, ctx, tdata)

logger := zap.NewNop().Sugar()
cs := &params.Run{
Clients: clients.Clients{
PipelineAsCode: stdata.PipelineAsCode,
Log: logger,
Kube: stdata.Kube,
Tekton: stdata.Pipeline,
},
Info: info.Info{
Pac: &info.PacOpts{
Settings: settings.Settings{HubCatalogs: &hubCatalogs},
},
Controller: &info.ControllerInfo{
Secret: info.DefaultPipelinesAscodeSecretName,
GlobalRepository: "global-repo",
},
Kube: &info.KubeOpts{Namespace: "namespace"},
},
}
cs.Clients.SetConsoleUI(consoleui.FallBackConsole{})

mac := hmac.New(sha256.New, []byte(webhookSecret))
payload := []byte(`{"iam": "batman"}`)
mac.Write(payload)
hexs := hex.EncodeToString(mac.Sum(nil))

runevent.Request = &info.Request{
Header: map[string][]string{
github.SHA256SignatureHeader: {"sha256=" + hexs},
},
Payload: payload,
}
runevent.Provider = &info.Provider{URL: ghTestServerURL, Token: "NONE"}
runevent.InstallationID = 12345

ctx = info.StoreCurrentControllerName(ctx, "default")
ctx = info.StoreNS(ctx, "namespace")

pacInfo := &info.PacOpts{
Settings: settings.Settings{
SecretAutoCreation: true,
RemoteTasks: true,
HubCatalogs: &hubCatalogs,
},
}
vcx := &ghprovider.Provider{
Run: cs,
Token: github.Ptr("None"),
Logger: logger,
}
vcx.SetGithubClient(fakeclient)
vcx.SetPacInfo(pacInfo)

kintTest := &kitesthelper.KinterfaceTest{
ConsoleURL: "https://console.url",
GetSecretResult: secrets,
}

p := NewPacs(&runevent, vcx, cs, pacInfo, kintTest, logger, nil)
err := p.Run(ctx)
assert.NilError(t, err)

// Verify skip-CI was actually detected from the commit message β€” if this
// fails, GetCommitInfo or skip parsing regressed before the exemption fires.
assert.Assert(t, runevent.HasSkipCommand, "test precondition: commit message should set HasSkipCommand")

prs, err := cs.Clients.Tekton.TektonV1().PipelineRuns("").List(ctx, metav1.ListOptions{})
assert.NilError(t, err)
assert.Assert(t, len(prs.Items) >= 1, "incoming webhook with [skip ci] commit must still create a PipelineRun")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make this test more robust by adding a verification for the prun sha matching the event sha.

}
Loading