fix: Improve GitLab commit status handling

This commit addresses confusion around when GitLab comments are posted
versus when commit status updates are used.

Changes:
* Documentation: Added clear explanation that comments are only posted
  when commit status updates fail, with visual example showing GitLab
  Pipelines tab where status appears when successful
* Code clarity: Refactored commit status logic with better error
  handling and detailed logging when both source/target project status
  updates fail
* Test reliability: Enhanced GitLab MR test to properly verify pipeline
  status success and ensure no comments are posted when status updates work
* Test accuracy: Removed redundant CreateStatus test cases and improved
  SHA tracking for more reliable test execution

The key fix is clarifying that Pipelines-as-Code tries to set commit status
on both source (fork) and target (upstream) projects, and only falls back
to posting comments when both attempts fail due to permission issues.

Fixes the misleading documentation that didn't properly explain the
relationship between GitLab API permissions, commit status updates, and
comment posting behavior.

Jira: https://issues.redhat.com/browse/SRVKP-8908
Signed-off-by: Chmouel Boudjnah <[email protected]>

Author: chmouel

docs/content/docs/guide/repositorycrd.md

@@ -2,6 +2,7 @@
 title: Repository CR
 weight: 1
 ---
+
 # Repository CR
 
 The Repository CR serves the following purposes:
@@ -127,7 +128,29 @@ access to the infrastructure.
 
 ## Disabling all comments for PipelineRuns on GitLab MR
 
-`comment_strategy` allows you to disable the comments on GitLab MR for a Repository
+By default, Pipelines-as-Code attempts to update the commit status through the
+GitLab API. It first tries the source project (fork), then falls back to the
+target project (upstream repository). The source project update succeeds when
+the configured token has access to the source repository and GitLab creates
+pipeline entries for external CI systems like Pipelines-as-Code. The target
+project fallback may fail if there's no CI pipeline running for that commit
+in the target repository, since GitLab only creates pipeline entries for commits
+that actually trigger CI in that specific project. If either status update
+succeeds, no comment is posted on the Merge Request.
+
+When a status update succeeds, you can see the status in the GitLab UI in the `Pipelines` tab, as
+shown in the following example:
+
+![Gitlab Pipelines from Pipelines-as-Code](/images/gitlab-pipelines-tab.png)
+
+Comments are only posted when:
+
+- Both commit status updates fail (typically due to insufficient token permissions)
+- The event type and repository settings allow commenting
+- The `comment_strategy` is not set to `disable_all`
+
+To explicitly disable all comments on GitLab Merge Requests for a Repository,
+use the `comment_strategy` setting:
 
 ```yaml
 spec:
@@ -136,8 +159,6 @@ spec:
       comment_strategy: "disable_all"
 ```
 
-When you set the value of `comment_strategy` to `disable_all` it will not add any comment on the merge request for the start and the end of PipelineRun
-
 ## Disabling all comments for PipelineRuns in GitHub Pull Requests on GitHub Webhook Setup
 
 `comment_strategy` allows you to disable the comments on GitHub PR for a Repository

docs/static/images/gitlab-pipelines-tab.png

@@ -280,17 +280,27 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusOpts
 		Context:     gitlab.Ptr(contextName),
 	}
 
-	// In case we have access, set the status. Typically, on a Merge Request (MR)
-	// from a fork in an upstream repository, the token needs to have write access
-	// to the fork repository in order to create a status. However, the token set on the
-	// Repository CR usually doesn't have such broad access, preventing from creating
-	// a status comment on it.
-	// This would work on a push or an MR from a branch within the same repo.
-	// Ignoring errors because of the write access issues,
-	if _, _, err := v.Client().Commits.SetCommitStatus(event.SourceProjectID, event.SHA, opt); err != nil {
-		v.eventEmitter.EmitMessage(v.repo, zap.ErrorLevel, "FailedToSetCommitStatus",
-			"cannot set status with the GitLab token because of: "+err.Error())
+	// setCommitStatus attempts to set the commit status for a given SHA, handling GitLab's permission model.
+	// First tries the source project (fork), then falls back to the target project (upstream repo).
+	// This fallback is necessary because:
+	// - In fork/MR scenarios, the GitLab token may not have write access to the contributor's fork
+	// - This ensures commit status can be displayed somewhere useful regardless
+	// of permission differences
+	// Returns nil if status is set on either project, logs error if both attempts fail.
+	_, _, err := v.Client().Commits.SetCommitStatus(event.SourceProjectID, event.SHA, opt)
+	if err == nil {
+		v.Logger.Debugf("created commit status on source project ID %d", event.SourceProjectID)
+		return nil
+	}
+	if _, _, err2 := v.Client().Commits.SetCommitStatus(event.TargetProjectID, event.SHA, opt); err2 == nil {
+		v.Logger.Debugf("created commit status on target project ID %d", event.TargetProjectID)
+		return nil
 	}
+	v.Logger.Debugf(
+		"Failed to create commit status: source project ID %d, target project ID %d. "+
+			"If you want Gitlab Pipeline Status update, ensure your GitLab token has access "+
+			"to the source repository. Error: %v",
+		event.SourceProjectID, event.TargetProjectID, err)
 
 	eventType := triggertype.IsPullRequestType(event.EventType)
 	// When a GitOps command is sent on a pushed commit, it mistakenly treats it as a pull_request

pkg/provider/gitlab/gitlab.go

@@ -201,6 +201,66 @@ func TestCreateStatus(t *testing.T) {
 				postStr: "has completed",
 			},
 		},
+		{
+			name:       "commit status success on source project",
+			wantClient: true,
+			wantErr:    false,
+			fields: fields{
+				targetProjectID: 100,
+			},
+			args: args{
+				statusOpts: provider.StatusOpts{
+					Conclusion: "success",
+				},
+				event: &info.Event{
+					TriggerTarget:   "pull_request",
+					SourceProjectID: 200,
+					TargetProjectID: 100,
+					SHA:             "abc123",
+				},
+				postStr: "has successfully",
+			},
+		},
+		{
+			name:       "commit status falls back to target project",
+			wantClient: true,
+			wantErr:    false,
+			fields: fields{
+				targetProjectID: 100,
+			},
+			args: args{
+				statusOpts: provider.StatusOpts{
+					Conclusion: "success",
+				},
+				event: &info.Event{
+					TriggerTarget:   "pull_request",
+					SourceProjectID: 404, // Will fail to find this project
+					TargetProjectID: 100,
+					SHA:             "abc123",
+				},
+				postStr: "has successfully",
+			},
+		},
+		{
+			name:       "commit status fails on both projects but continues",
+			wantClient: true,
+			wantErr:    false,
+			fields: fields{
+				targetProjectID: 100,
+			},
+			args: args{
+				statusOpts: provider.StatusOpts{
+					Conclusion: "success",
+				},
+				event: &info.Event{
+					TriggerTarget:   "pull_request",
+					SourceProjectID: 404, // Will fail
+					TargetProjectID: 405, // Will fail
+					SHA:             "abc123",
+				},
+				postStr: "has successfully",
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -216,6 +276,7 @@ func TestCreateStatus(t *testing.T) {
 			v := &Provider{
 				targetProjectID: tt.fields.targetProjectID,
 				run:             params.New(),
+				Logger:          logger,
 				pacInfo: &info.PacOpts{
 					Settings: settings.Settings{
 						ApplicationName: settings.PACApplicationNameDefaultValue,
@@ -232,6 +293,40 @@ func TestCreateStatus(t *testing.T) {
 				client, mux, tearDown := thelp.Setup(t)
 				v.SetGitLabClient(client)
 				defer tearDown()
+
+				// Mock commit status endpoints for both source and target projects
+				if tt.args.event.SourceProjectID != 0 {
+					// Mock source project commit status endpoint
+					sourceStatusPath := fmt.Sprintf("/projects/%d/statuses/%s", tt.args.event.SourceProjectID, tt.args.event.SHA)
+					mux.HandleFunc(sourceStatusPath, func(rw http.ResponseWriter, _ *http.Request) {
+						if tt.args.event.SourceProjectID == 404 {
+							// Simulate failure on source project
+							rw.WriteHeader(http.StatusNotFound)
+							fmt.Fprint(rw, `{"message": "404 Project Not Found"}`)
+							return
+						}
+						// Success on source project
+						rw.WriteHeader(http.StatusCreated)
+						fmt.Fprint(rw, `{}`)
+					})
+				}
+
+				if tt.args.event.TargetProjectID != 0 {
+					// Mock target project commit status endpoint
+					targetStatusPath := fmt.Sprintf("/projects/%d/statuses/%s", tt.args.event.TargetProjectID, tt.args.event.SHA)
+					mux.HandleFunc(targetStatusPath, func(rw http.ResponseWriter, _ *http.Request) {
+						if tt.args.event.TargetProjectID == 404 {
+							// Simulate failure on target project
+							rw.WriteHeader(http.StatusNotFound)
+							fmt.Fprint(rw, `{"message": "404 Project Not Found"}`)
+							return
+						}
+						// Success on target project
+						rw.WriteHeader(http.StatusCreated)
+						fmt.Fprint(rw, `{}`)
+					})
+				}
+
 				thelp.MuxNotePost(t, mux, v.targetProjectID, tt.args.event.PullRequestNumber, tt.args.postStr)
 			}
 
@@ -1044,9 +1139,12 @@ func TestGitLabCreateComment(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			fakeclient, mux, teardown := thelp.Setup(t)
 			defer teardown()
+			observer, _ := zapobserver.New(zap.InfoLevel)
+			logger := zap.New(observer).Sugar()
 
 			if tt.clientNil {
 				p := &Provider{
+					Logger:          logger,
 					sourceProjectID: 666,
 				}
 				err := p.CreateComment(context.Background(), tt.event, tt.commit, tt.updateMarker)