Skip to content

Commit 529a725

Browse files
theakshaypantzakisk
authored andcommitted
fix(gitlab): post MR comment on inaccessible fork
When a merge request originates from a fork the bot cannot access, post an informative comment on the MR explaining the issue. Uses CreateComment with an update marker to prevent duplicate comments on reconciler retries. Signed-off-by: Akshay Pant <akpant@redhat.com>
1 parent 4c7b0e0 commit 529a725

2 files changed

Lines changed: 185 additions & 4 deletions

File tree

pkg/provider/gitlab/gitlab.go

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ func (v *Provider) GetConfig() *info.ProviderConfig {
215215
}
216216
}
217217

218-
func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.Event, repo *v1alpha1.Repository, eventsEmitter *events.EventEmitter) error {
218+
func (v *Provider) SetClient(ctx context.Context, run *params.Run, runevent *info.Event, repo *v1alpha1.Repository, eventsEmitter *events.EventEmitter) error {
219219
var err error
220220
if runevent.Provider.Token == "" {
221221
return fmt.Errorf("no git_provider.secret has been set in the repo crd")
@@ -273,11 +273,23 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
273273
_, resp, err := v.Client().Projects.GetProject(runevent.SourceProjectID, &gitlab.GetProjectOptions{})
274274
errmsg := fmt.Sprintf("failed to access GitLab source repository ID %d: please ensure token has 'read_repository' scope on that repository",
275275
runevent.SourceProjectID)
276+
277+
var returnErr error
276278
if resp != nil && resp.StatusCode == http.StatusNotFound {
277-
return fmt.Errorf("%s", errmsg)
279+
returnErr = fmt.Errorf("%s", errmsg)
280+
} else if err != nil {
281+
returnErr = fmt.Errorf("%s: %w", errmsg, err)
278282
}
279-
if err != nil {
280-
return fmt.Errorf("%s: %w", errmsg, err)
283+
284+
if returnErr != nil {
285+
if runevent.PullRequestNumber > 0 {
286+
marker := "<!-- pac-source-repo-inaccessible -->"
287+
comment := fmt.Sprintf("%s\n%s", marker, formatSourceRepoInaccessibleComment(runevent.SourceProjectID))
288+
if commentErr := v.CreateComment(ctx, runevent, comment, marker); commentErr != nil {
289+
run.Clients.Log.Warnf("failed to post source repository access error as MR comment: %v", commentErr)
290+
}
291+
}
292+
return returnErr
281293
}
282294
}
283295

@@ -864,6 +876,13 @@ func (v *Provider) isHeadCommitOfBranch(runevent *info.Event, branchName string)
864876
return fmt.Errorf("provided SHA %s is not the HEAD commit of the branch %s", runevent.SHA, branchName)
865877
}
866878

879+
func formatSourceRepoInaccessibleComment(sourceProjectID int64) string {
880+
return fmt.Sprintf("**Could not access source repository (project ID: %d)**\n\n"+
881+
"Ensure the token has `read_repository` scope on the source project, "+
882+
"or use a branch in the same repository instead of a fork.",
883+
sourceProjectID)
884+
}
885+
867886
func (v *Provider) GetTemplate(commentType provider.CommentType) string {
868887
return provider.GetHTMLTemplate(commentType)
869888
}

pkg/provider/gitlab/gitlab_test.go

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"io"
78
"net/http"
89
"os"
910
"strconv"
@@ -2300,3 +2301,164 @@ func TestGitLabCreateCommentPaging(t *testing.T) {
23002301
assert.NilError(t, err)
23012302
assert.Assert(t, updated == true, "comment update handler has not been called")
23022303
}
2304+
2305+
func TestSetClientSourceRepoAccessPostsComment(t *testing.T) {
2306+
ctx, _ := rtesting.SetupFakeContext(t)
2307+
observer, _ := zapobserver.New(zap.DebugLevel)
2308+
fakelogger := zap.New(observer).Sugar()
2309+
run := &params.Run{
2310+
Clients: clients.Clients{
2311+
Log: fakelogger,
2312+
},
2313+
}
2314+
2315+
tests := []struct {
2316+
name string
2317+
triggerTarget triggertype.Trigger
2318+
sourceProjectID int64
2319+
targetProjectID int64
2320+
pullRequestNumber int
2321+
setupMockResponse func(*http.ServeMux, *bool)
2322+
expectedError string
2323+
expectComment bool
2324+
}{
2325+
{
2326+
name: "404 on source project posts MR comment",
2327+
triggerTarget: triggertype.PullRequest,
2328+
sourceProjectID: 456,
2329+
targetProjectID: 789,
2330+
pullRequestNumber: 42,
2331+
setupMockResponse: func(mux *http.ServeMux, commentPosted *bool) {
2332+
mux.HandleFunc("/projects/456", func(rw http.ResponseWriter, _ *http.Request) {
2333+
rw.WriteHeader(http.StatusNotFound)
2334+
fmt.Fprint(rw, `{"message": "404 Project Not Found"}`)
2335+
})
2336+
mux.HandleFunc("/projects/789/merge_requests/42/notes", func(rw http.ResponseWriter, r *http.Request) {
2337+
if r.Method == http.MethodGet {
2338+
fmt.Fprint(rw, `[]`)
2339+
return
2340+
}
2341+
if r.Method == http.MethodPost {
2342+
body, _ := io.ReadAll(r.Body)
2343+
bodyStr := string(body)
2344+
assert.Assert(t, strings.Contains(bodyStr, "source repository"), "comment should mention source repository, got: %s", bodyStr)
2345+
assert.Assert(t, strings.Contains(bodyStr, "read_repository"), "comment should mention read_repository scope, got: %s", bodyStr)
2346+
*commentPosted = true
2347+
}
2348+
fmt.Fprint(rw, `{}`)
2349+
})
2350+
},
2351+
expectedError: "failed to access GitLab source repository ID 456: please ensure token has 'read_repository' scope on that repository",
2352+
expectComment: true,
2353+
},
2354+
{
2355+
name: "403 on source project posts MR comment",
2356+
triggerTarget: triggertype.PullRequest,
2357+
sourceProjectID: 456,
2358+
targetProjectID: 789,
2359+
pullRequestNumber: 42,
2360+
setupMockResponse: func(mux *http.ServeMux, commentPosted *bool) {
2361+
mux.HandleFunc("/projects/456", func(rw http.ResponseWriter, _ *http.Request) {
2362+
rw.WriteHeader(http.StatusForbidden)
2363+
fmt.Fprint(rw, `{"message": "403 Forbidden"}`)
2364+
})
2365+
mux.HandleFunc("/projects/789/merge_requests/42/notes", func(rw http.ResponseWriter, r *http.Request) {
2366+
if r.Method == http.MethodGet {
2367+
fmt.Fprint(rw, `[]`)
2368+
return
2369+
}
2370+
if r.Method == http.MethodPost {
2371+
*commentPosted = true
2372+
}
2373+
fmt.Fprint(rw, `{}`)
2374+
})
2375+
},
2376+
expectedError: "failed to access GitLab source repository ID 456: please ensure token has 'read_repository' scope on that repository",
2377+
expectComment: true,
2378+
},
2379+
{
2380+
name: "Comment posting failure is non-fatal",
2381+
triggerTarget: triggertype.PullRequest,
2382+
sourceProjectID: 456,
2383+
targetProjectID: 789,
2384+
pullRequestNumber: 42,
2385+
setupMockResponse: func(mux *http.ServeMux, _ *bool) {
2386+
mux.HandleFunc("/projects/456", func(rw http.ResponseWriter, _ *http.Request) {
2387+
rw.WriteHeader(http.StatusNotFound)
2388+
fmt.Fprint(rw, `{"message": "404 Project Not Found"}`)
2389+
})
2390+
mux.HandleFunc("/projects/789/merge_requests/42/notes", func(rw http.ResponseWriter, _ *http.Request) {
2391+
rw.WriteHeader(http.StatusInternalServerError)
2392+
fmt.Fprint(rw, `{"message": "500 Internal Server Error"}`)
2393+
})
2394+
},
2395+
expectedError: "failed to access GitLab source repository ID 456",
2396+
expectComment: false,
2397+
},
2398+
{
2399+
name: "Successful access posts no comment",
2400+
triggerTarget: triggertype.PullRequest,
2401+
sourceProjectID: 456,
2402+
targetProjectID: 789,
2403+
pullRequestNumber: 42,
2404+
setupMockResponse: func(mux *http.ServeMux, _ *bool) {
2405+
mux.HandleFunc("/projects/456", func(rw http.ResponseWriter, _ *http.Request) {
2406+
rw.WriteHeader(http.StatusOK)
2407+
fmt.Fprint(rw, `{"id": 456, "name": "test-repo"}`)
2408+
})
2409+
},
2410+
expectedError: "",
2411+
expectComment: false,
2412+
},
2413+
{
2414+
name: "Push event skips source project check entirely",
2415+
triggerTarget: triggertype.Push,
2416+
sourceProjectID: 456,
2417+
targetProjectID: 789,
2418+
pullRequestNumber: 0,
2419+
setupMockResponse: func(mux *http.ServeMux, _ *bool) {
2420+
mux.HandleFunc("/projects/456", func(rw http.ResponseWriter, _ *http.Request) {
2421+
fmt.Fprint(rw, `{"id": 456, "name": "test-repo"}`)
2422+
})
2423+
},
2424+
expectedError: "",
2425+
expectComment: false,
2426+
},
2427+
}
2428+
2429+
for _, tt := range tests {
2430+
t.Run(tt.name, func(t *testing.T) {
2431+
mockClient, mux, tearDown := thelp.Setup(t)
2432+
defer tearDown()
2433+
2434+
commentPosted := false
2435+
if tt.setupMockResponse != nil {
2436+
tt.setupMockResponse(mux, &commentPosted)
2437+
}
2438+
2439+
v := &Provider{gitlabClient: mockClient}
2440+
event := &info.Event{
2441+
Provider: &info.Provider{
2442+
Token: "test-token",
2443+
},
2444+
Organization: "test-org",
2445+
Repository: "test-repo",
2446+
TriggerTarget: tt.triggerTarget,
2447+
SourceProjectID: tt.sourceProjectID,
2448+
TargetProjectID: tt.targetProjectID,
2449+
PullRequestNumber: tt.pullRequestNumber,
2450+
}
2451+
2452+
err := v.SetClient(ctx, run, event, nil, nil)
2453+
2454+
if tt.expectedError != "" {
2455+
assert.Assert(t, err != nil, "expected error but got none")
2456+
assert.ErrorContains(t, err, tt.expectedError)
2457+
} else {
2458+
assert.NilError(t, err)
2459+
}
2460+
2461+
assert.Equal(t, tt.expectComment, commentPosted, "comment posted mismatch")
2462+
})
2463+
}
2464+
}

0 commit comments

Comments
 (0)