fix(gitlab): use TargetProjectID for merge request comments

[infernus01]

📝 Description of the Change

When PaC reports validation errors on GitLab merge requests from repository forks, it was incorrectly using SourceProjectID in API calls. Since merge requests are objects on the TargetProject, this resulted in 404 errors and no comments were added.

Fixed CreateComment method to use event.TargetProjectID instead of v.sourceProjectID for all merge request operations.
JIRA Ref: https://issues.redhat.com/browse/SRVKP-8246

🔗 Linked GitHub Issue

Fixes #

👨🏻‍ Linked Jira

🚀 Type of Change

• 🐛 Bug fix (fix:)
• ✨ New feature (feat:)
• 💥 Breaking change (feat!:, fix!:)
• 📚 Documentation update (docs:)
• ⚙️ Chore (chore:)
• 💅 Refactor (refactor:)
• 🔧 Enhancement (enhance:)

🧪 Testing Strategy

• Unit tests
• Integration tests
• End-to-end tests
• Manual testing
• Not Applicable

✅ Submitter Checklist

• 📝 My commit messages are clear, informative, and follow the project's How to write a git commit message guide. The Gitlint linter ensures in CI it's properly validated
• ✨ I have ensured my commit message prefix (e.g., fix:, feat:) matches the "Type of Change" I selected above.
• ♽ I have run make test and make lint locally to check for and fix any
  issues. For an efficient workflow, I have considered installing
  pre-commit and running pre-commit install to
  automate these checks.
• 📖 I have added or updated documentation for any user-facing changes.
• 🧪 I have added sufficient unit tests for my code changes.
• 🎁 I have added end-to-end tests where feasible. See README for more details.
• 🔎 I have addressed any CI test flakiness or provided a clear reason to bypass it.
• If adding a provider feature, I have filled in the following and updated the provider documentation:
  • GitHub App
  • GitHub Webhook
  • Gitea/Forgejo
  • GitLab
  • Bitbucket Cloud
  • Bitbucket Data Center

[infernus01]

/cc @aThorp96 @chmouel @zakisk

[aThorp96]

Thanks @infernus01! Do you mind adding an end to end test for this?

[zakisk]

@infernus01 can you please confirm that it works with exact same setup where you were debugging this?

[zakisk]

🚀 Mirrored PR Created for E2E Testing

A mirrored PR has been opened for end-to-end testing: View PR

⏳ Follow progress there for E2E results.
If you need to update the PR with new changes, please ask a maintainer to rerun hack/mirror-pr.sh.

[zakisk]

/ok-to-test

[chmouel]

this is a critical fix, thanks, we really need a E2E test (but that may be complicated due of some initial setup to do)a or a demo demonstrating before/after on how it fixes..

[infernus01]

/cc @aThorp96 @chmouel @zakisk

[aThorp96]

/lgtm

Thanks @infernus01! Two nitpick suggestions but functionally this looks good

[aThorp96]

/ok-to-test

[chmouel]

/ok-to-test

@infernus01 i have sent you an invite to join openshift-pipelines org and you should be able to trigger e2e test

[chmouel]

/gemini review

[gemini-code-assist]

Code Review

This pull request correctly fixes an issue where GitLab API calls for merge request comments were using SourceProjectID instead of TargetProjectID, which is crucial for forked repositories. The change is simple and effective. The accompanying unit tests are updated accordingly, and a new end-to-end test has been added to validate the fix in a forked repository scenario.

My review includes one suggestion to improve the robustness of the new E2E test by replacing a fixed time.Sleep with a polling mechanism to check for the fork's readiness, which will help prevent flaky test runs.

[gemini-code-assist]

Using a fixed time.Sleep to wait for the fork to be ready can lead to flaky tests. The GitLab API provides a way to check the status of a fork operation. It's better to poll the project's import_status until it becomes finished. This will make the test more robust.

	// Wait for fork to be ready by polling its import status
	const maxForkRetries = 24
	const forkRetryInterval = 5 * time.Second
	var p *clientGitlab.Project
	for i := 0; i < maxForkRetries; i++ {
		p, _, err = glprovider.Client().Projects.GetProject(forkProject.ID, nil)
		assert.NilError(t, err)
		if p.ImportStatus == "finished" {
			runcnx.Clients.Log.Info("Fork project is ready.")
			break
		}
		if i == maxForkRetries-1 {
			t.Fatalf("Fork of project did not finish in time, status: %s", p.ImportStatus)
		}
		runcnx.Clients.Log.Infof("Waiting for fork to be ready, current status: %s... (%d/%d)", p.ImportStatus, i+1, maxForkRetries)
		time.Sleep(forkRetryInterval)
	}

[infernus01]

@infernus01 i have sent you an invite to join openshift-pipelines org and you should be able to trigger e2e test

Cool, thanks

[infernus01]

failing in CI due to GitLab fork permissions (CI token can't create forks I guess) and I think the test infrastructure has different limitations in different environments.
Any heads up for the next step here?

[chmouel]

the gitlab token for e2e is using my account, we need to change this...

Maybe we should try to create multiple existent repo and not creating/deleting,

Or create a new pac user and have a elevated token in there that can do anything we want...

all of those needs some pre setup and work...

were you able to test your e2e manually?

[infernus01]

On manual e2e testing, called CreateComment() directly(skipping webhook validation) and verified comment appears on target project's MR: