Skip to content

Commit 8b19ee7

Browse files
committed
Fix incoming webhook with github apps
When incoming webhook and github apps is set it will try to use the webhook secret from the incoming webhook instead of the current namespace. Remove InfoFromRepo since we don't need and it's GH specific. Closes #744 Signed-off-by: Chmouel Boudjnah <[email protected]>
1 parent a4b9371 commit 8b19ee7

File tree

7 files changed

+44
-13
lines changed

7 files changed

+44
-13
lines changed

pkg/params/info/events.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ type Provider struct {
6363
Token string
6464
URL string
6565
User string
66-
InfoFromRepo bool // whether the provider info come from the repository
6766
WebhookSecret string
6867
WebhookSecretFromRepo bool
6968
}

pkg/pipelineascode/match.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (p *PacRun) matchRepoPR(ctx context.Context) ([]matcher.Match, *v1alpha1.Re
5757
// allow webhook providers users to have a global webhook secret to be used,
5858
// so instead of having to specify their in Repo each time, they use a
5959
// shared one from pac.
60-
if repo.Spec.GitProvider != nil {
60+
if repo.Spec.GitProvider != nil && p.event.InstallationID <= 0 {
6161
err := SecretFromRepository(ctx, p.run, p.k8int, p.vcx.GetConfig(), p.event, repo, p.logger)
6262
if err != nil {
6363
return nil, nil, err

pkg/pipelineascode/pipelinesascode_github_test.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,9 +445,8 @@ func TestRun(t *testing.T) {
445445
Payload: payload,
446446
}
447447
tt.runevent.Provider = &info.Provider{
448-
InfoFromRepo: tt.ProviderInfoFromRepo,
449-
URL: ghTestServerURL,
450-
Token: "NONE",
448+
URL: ghTestServerURL,
449+
Token: "NONE",
451450
}
452451

453452
k8int := &kitesthelper.KinterfaceTest{
@@ -456,6 +455,12 @@ func TestRun(t *testing.T) {
456455
GetSecretResult: secrets,
457456
}
458457

458+
// InstallationID > 0 is used to detect if we are a GitHub APP
459+
tt.runevent.InstallationID = 12345
460+
if tt.ProviderInfoFromRepo {
461+
tt.runevent.InstallationID = 0
462+
}
463+
459464
vcx := &ghprovider.Provider{
460465
Client: fakeclient,
461466
Token: github.String("None"),

pkg/pipelineascode/secret.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ func SecretFromRepository(ctx context.Context, cs *params.Run, k8int kubeinterac
4747
if event.Provider.Token == "" {
4848
return nil
4949
}
50-
event.Provider.InfoFromRepo = true
5150
event.Provider.User = repo.Spec.GitProvider.User
5251

5352
if repo.Spec.GitProvider.WebhookSecret == nil {

pkg/pipelineascode/secret_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ func TestSecretFromRepository(t *testing.T) {
123123
for key, value := range logs {
124124
assert.Assert(t, tt.logmatch[key].MatchString(value.Message), "no match on logs %s => %s", tt.logmatch[key], value.Message)
125125
}
126-
assert.Assert(t, event.Provider.InfoFromRepo)
127126
assert.Equal(t, tt.expectedSecret, event.Provider.Token)
128127
})
129128
}

pkg/provider/github/status.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,11 @@ func (v *Provider) CreateStatus(ctx context.Context, tekton versioned.Interface,
237237
}
238238
statusOpts.Summary = fmt.Sprintf("%s%s %s", pacopts.ApplicationName, onPr, statusOpts.Summary)
239239

240-
if runevent.Provider.InfoFromRepo {
241-
return v.createStatusCommit(ctx, runevent, pacopts, statusOpts)
240+
// If we have an installationID which mean we have a github apps and we can use the checkRun API
241+
if runevent.InstallationID > 0 {
242+
return v.getOrUpdateCheckRunStatus(ctx, tekton, runevent, pacopts, statusOpts)
242243
}
243-
return v.getOrUpdateCheckRunStatus(ctx, tekton, runevent, pacopts, statusOpts)
244+
245+
// Otherwise use the update status commit API
246+
return v.createStatusCommit(ctx, runevent, pacopts, statusOpts)
244247
}

pkg/provider/github/status_test.go

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ func TestGithubProviderCreateStatus(t *testing.T) {
113113
detailsURL string
114114
titleSubstr string
115115
nilCompletedAtDate bool
116+
githubApps bool
116117
}
117118
tests := []struct {
118119
name string
@@ -130,6 +131,21 @@ func TestGithubProviderCreateStatus(t *testing.T) {
130131
text: "Yay",
131132
detailsURL: "https://cireport.com",
132133
titleSubstr: "Success",
134+
githubApps: true,
135+
},
136+
want: &github.CheckRun{ID: &resultid},
137+
wantErr: false,
138+
},
139+
{
140+
name: "success coming from webhook",
141+
args: args{
142+
runevent: runEvent,
143+
status: "completed",
144+
conclusion: "success",
145+
text: "Yay",
146+
detailsURL: "https://cireport.com",
147+
titleSubstr: "Success",
148+
githubApps: false,
133149
},
134150
want: &github.CheckRun{ID: &resultid},
135151
wantErr: false,
@@ -143,6 +159,7 @@ func TestGithubProviderCreateStatus(t *testing.T) {
143159
text: "Yay",
144160
detailsURL: "https://cireport.com",
145161
nilCompletedAtDate: true,
162+
githubApps: true,
146163
},
147164
want: &github.CheckRun{ID: &resultid},
148165
wantErr: false,
@@ -156,6 +173,7 @@ func TestGithubProviderCreateStatus(t *testing.T) {
156173
text: "Nay",
157174
detailsURL: "https://cireport.com",
158175
titleSubstr: "Failed",
176+
githubApps: true,
159177
},
160178
want: &github.CheckRun{ID: &resultid},
161179
wantErr: false,
@@ -169,6 +187,7 @@ func TestGithubProviderCreateStatus(t *testing.T) {
169187
text: "Skipit",
170188
detailsURL: "https://cireport.com",
171189
titleSubstr: "Skipped",
190+
githubApps: true,
172191
},
173192
want: &github.CheckRun{ID: &resultid},
174193
wantErr: false,
@@ -182,6 +201,7 @@ func TestGithubProviderCreateStatus(t *testing.T) {
182201
text: "Je sais pas ce qui se passe wesh",
183202
detailsURL: "https://cireport.com",
184203
titleSubstr: "Unknown",
204+
githubApps: true,
185205
},
186206
want: &github.CheckRun{ID: &resultid},
187207
wantErr: false,
@@ -201,6 +221,7 @@ func TestGithubProviderCreateStatus(t *testing.T) {
201221
gcvs := Provider{
202222
Client: fakeclient,
203223
}
224+
mux.HandleFunc("/repos/check/run/statuses/sha", func(rw http.ResponseWriter, r *http.Request) {})
204225
mux.HandleFunc(fmt.Sprintf("/repos/check/run/check-runs/%d", checkrunid), func(rw http.ResponseWriter, r *http.Request) {
205226
bit, _ := ioutil.ReadAll(r.Body)
206227
checkRun := &github.CheckRun{}
@@ -232,13 +253,18 @@ func TestGithubProviderCreateStatus(t *testing.T) {
232253
pacopts := &info.PacOpts{
233254
LogURL: "https://log",
234255
}
235-
if !tt.notoken {
256+
if tt.notoken {
257+
tt.args.runevent = info.NewEvent()
258+
} else {
236259
tt.args.runevent.Provider = &info.Provider{
237260
Token: "hello",
238261
URL: "moto",
239262
}
240-
} else {
241-
tt.args.runevent = info.NewEvent()
263+
if tt.args.githubApps {
264+
tt.args.runevent.InstallationID = 12345
265+
} else {
266+
tt.args.runevent.SHA = "sha"
267+
}
242268
}
243269
err := gcvs.CreateStatus(ctx, nil, tt.args.runevent, pacopts, status)
244270
if (err != nil) != tt.wantErr {

0 commit comments

Comments
 (0)