diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index 7aabde0bf..5d62c6dbf 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -95,7 +95,7 @@ func (v *Provider) CreateComment(_ context.Context, event *info.Event, commit, u // List comments of the merge request if updateMarker != "" { - comments, _, err := v.Client().Notes.ListMergeRequestNotes(v.sourceProjectID, event.PullRequestNumber, &gitlab.ListMergeRequestNotesOptions{ + comments, _, err := v.Client().Notes.ListMergeRequestNotes(event.TargetProjectID, event.PullRequestNumber, &gitlab.ListMergeRequestNotesOptions{ ListOptions: gitlab.ListOptions{ Page: 1, PerPage: 100, @@ -108,7 +108,7 @@ func (v *Provider) CreateComment(_ context.Context, event *info.Event, commit, u re := regexp.MustCompile(updateMarker) for _, comment := range comments { if re.MatchString(comment.Body) { - _, _, err := v.Client().Notes.UpdateMergeRequestNote(v.sourceProjectID, event.PullRequestNumber, comment.ID, &gitlab.UpdateMergeRequestNoteOptions{ + _, _, err := v.Client().Notes.UpdateMergeRequestNote(event.TargetProjectID, event.PullRequestNumber, comment.ID, &gitlab.UpdateMergeRequestNoteOptions{ Body: &commit, }) return err @@ -116,7 +116,7 @@ func (v *Provider) CreateComment(_ context.Context, event *info.Event, commit, u } } - _, _, err := v.Client().Notes.CreateMergeRequestNote(v.sourceProjectID, event.PullRequestNumber, &gitlab.CreateMergeRequestNoteOptions{ + _, _, err := v.Client().Notes.CreateMergeRequestNote(event.TargetProjectID, event.PullRequestNumber, &gitlab.CreateMergeRequestNoteOptions{ Body: &commit, }) diff --git a/pkg/provider/gitlab/gitlab_test.go b/pkg/provider/gitlab/gitlab_test.go index 94e1a1975..aff2b3251 100644 --- a/pkg/provider/gitlab/gitlab_test.go +++ b/pkg/provider/gitlab/gitlab_test.go @@ -991,7 +991,7 @@ func TestGitLabCreateComment(t *testing.T) { }, { name: "create new comment", - event: &info.Event{PullRequestNumber: 123}, + event: &info.Event{PullRequestNumber: 123, TargetProjectID: 666}, commit: "New Comment", updateMarker: "", mockResponses: map[string]func(rw http.ResponseWriter, _ *http.Request){ @@ -1004,7 +1004,7 @@ func TestGitLabCreateComment(t *testing.T) { }, { name: "update existing comment", - event: &info.Event{PullRequestNumber: 123}, + event: &info.Event{PullRequestNumber: 123, TargetProjectID: 666}, commit: "Updated Comment", updateMarker: "MARKER", mockResponses: map[string]func(rw http.ResponseWriter, _ *http.Request){ @@ -1023,7 +1023,7 @@ func TestGitLabCreateComment(t *testing.T) { }, { name: "no matching comment creates new", - event: &info.Event{PullRequestNumber: 123}, + event: &info.Event{PullRequestNumber: 123, TargetProjectID: 666}, commit: "New Comment", updateMarker: "MARKER", mockResponses: map[string]func(rw http.ResponseWriter, _ *http.Request){ diff --git a/test/gitlab_merge_request_test.go b/test/gitlab_merge_request_test.go index 014e394c0..114a7c8df 100644 --- a/test/gitlab_merge_request_test.go +++ b/test/gitlab_merge_request_test.go @@ -690,6 +690,134 @@ func TestGitlabMergeRequestOnUpdateAtAndLabelChange(t *testing.T) { } } +func TestGitlabMergeRequestValidationErrorsFromFork(t *testing.T) { + targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") + ctx := context.Background() + runcnx, opts, glprovider, err := tgitlab.Setup(ctx) + assert.NilError(t, err) + ctx, err = cctx.GetControllerCtxInfo(ctx, runcnx) + assert.NilError(t, err) + runcnx.Clients.Log.Info("Testing GitLab validation error commenting from fork scenario") + + // Get the original project onboarded to PaC + originalProject, resp, err := glprovider.Client().Projects.GetProject(opts.ProjectID, nil) + assert.NilError(t, err) + if resp != nil && resp.StatusCode == http.StatusNotFound { + t.Errorf("Repository %d not found", opts.ProjectID) + } + + err = tgitlab.CreateCRD(ctx, originalProject, runcnx, opts, targetNS, nil) + assert.NilError(t, err) + + // Create a fork of the original project + forkName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-fork-test") + forkProject, _, err := glprovider.Client().Projects.ForkProject(opts.ProjectID, &clientGitlab.ForkProjectOptions{ + Name: &forkName, + Path: &forkName, + }) + assert.NilError(t, err) + runcnx.Clients.Log.Infof("Created fork project: %s (ID: %d) from original: %s (ID: %d)", + forkProject.PathWithNamespace, forkProject.ID, originalProject.PathWithNamespace, originalProject.ID) + + // Cleanup fork when test finishes + defer func() { + runcnx.Clients.Log.Infof("Cleaning up fork project: %d", forkProject.ID) + _, err := glprovider.Client().Projects.DeleteProject(forkProject.ID, nil) + if err != nil { + runcnx.Clients.Log.Warnf("Failed to delete fork project: %v", err) + } + }() + + // Wait sometime for fork to be fully ready + time.Sleep(3 * time.Second) + + // Commit invalid .tekton files to the fork + entries, err := payload.GetEntries(map[string]string{ + ".tekton/bad-yaml.yaml": "testdata/failures/bad-yaml.yaml", + }, targetNS, forkProject.DefaultBranch, + triggertype.PullRequest.String(), map[string]string{}) + assert.NilError(t, err) + + targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-fork-test") + forkCloneURL, err := scm.MakeGitCloneURL(forkProject.WebURL, opts.UserName, opts.Password) + assert.NilError(t, err) + + commitTitle := "Add invalid .tekton file from fork - " + targetRefName + scmOpts := &scm.Opts{ + GitURL: forkCloneURL, + CommitTitle: commitTitle, + Log: runcnx.Clients.Log, + WebURL: forkProject.WebURL, + TargetRefName: targetRefName, + BaseRefName: originalProject.DefaultBranch, + } + _ = scm.PushFilesToRefGit(t, scmOpts, entries) + runcnx.Clients.Log.Infof("Pushed invalid .tekton files to fork branch: %s", targetRefName) + + // Create merge request from fork to original project + mrTitle := "TestValidationErrorsFromFork - " + targetRefName + mrOptions := &clientGitlab.CreateMergeRequestOptions{ + Title: &mrTitle, + SourceBranch: &targetRefName, + TargetBranch: &originalProject.DefaultBranch, + // Create MR on the target project (original), not the source (fork) + TargetProjectID: &originalProject.ID, + } + + mr, _, err := glprovider.Client().MergeRequests.CreateMergeRequest(forkProject.ID, mrOptions) + assert.NilError(t, err) + runcnx.Clients.Log.Infof("Created merge request from fork to original: %s/-/merge_requests/%d", + originalProject.WebURL, mr.IID) + + defer func() { + // Close the MR and clean up branch + runcnx.Clients.Log.Infof("Closing MR %d", mr.IID) + _, _, err := glprovider.Client().MergeRequests.UpdateMergeRequest(originalProject.ID, mr.IID, + &clientGitlab.UpdateMergeRequestOptions{StateEvent: clientGitlab.Ptr("close")}) + if err != nil { + runcnx.Clients.Log.Warnf("Failed to close MR: %v", err) + } + + runcnx.Clients.Log.Infof("Deleting branch %s from fork", targetRefName) + _, err = glprovider.Client().Branches.DeleteBranch(forkProject.ID, targetRefName) + if err != nil { + runcnx.Clients.Log.Warnf("Failed to delete branch: %v", err) + } + }() + + runcnx.Clients.Log.Info("Waiting for webhook validation to process MR and post validation comment...") + + maxLoop := 12 // Wait up to 72 seconds for webhook processing + foundValidationComment := false + + for i := 0; i < maxLoop; i++ { + notes, _, err := glprovider.Client().Notes.ListMergeRequestNotes(originalProject.ID, mr.IID, nil) + assert.NilError(t, err) + + for _, note := range notes { + // Look for the validation error comment that PaC should post via webhook + if regexp.MustCompile(`.*There are some errors in your PipelineRun template.*`).MatchString(note.Body) && + regexp.MustCompile(`.*bad-yaml\.yaml.*yaml validation error.*`).MatchString(note.Body) { + runcnx.Clients.Log.Info("Found validation error comment on original project's MR!") + foundValidationComment = true + + // Verify the comment format matches PaC's validation error format + assert.Assert(t, regexp.MustCompile(`\[!CAUTION\]`).MatchString(note.Body), "Comment should contain caution header") + break + } + } + + if foundValidationComment { + break + } + + runcnx.Clients.Log.Infof("Loop %d/%d: Waiting for webhook validation to post comment (testing TargetProjectID fix)", i+1, maxLoop) + time.Sleep(6 * time.Second) + } + + assert.Assert(t, foundValidationComment, "Validation error comment should appear on original project's MR. ") +} + // Local Variables: // compile-command: "go test -tags=e2e -v -run ^TestGitlabMergeRequest$" // End: