fix: bitbucket service pull request build status from different source

[cronik]

The commit 7ac886d switched to the newer repository commit build status api. This introduced a regression in the case the PR source repository is not the same as the target, for example a person fork. In this case the bitbucket api created from the PR head will point to the fork repo and may result in a 401 error if the credentials don't have the necessary permissions.

ERROR: Could not send notifications
com.cloudbees.jenkins.plugins.bitbucket.api.BitbucketRequestException: HTTP request error.
Status: HTTP/1.1 401
Response: {"errors":[{"context":null,"message":"You are not permitted to access this resource","exceptionName":"com.atlassian.bitbucket.AuthorisationException"}]}

This change will force the bitbucket api instance to be the target repo which is the appropriate target for the notification. The old api was commit hash centric, not project centric, so the api client owner/repo didn't matter.

This change also reverts some changes introduced by #954 to the refName parameter. While local testing has shown that by changing the bitbucket client to point to the target repo, setting the refName to null works in all cases, this change has been restricted to only cases when the source repo is the same as the target.

No changes were made to the Bitbucket Cloud code path.

Your checklist for this pull request

• Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or in Jenkins JIRA
• Link to relevant pull requests, esp. upstream and downstream changes
• Did you provide a test-case? That demonstrates feature works or fixes the issue.

[nfalco79]

In this way you reintroduce again the issue for which a branch build and PR build notification are visible in the PR page.
refName is optional but is required to filter status depending on which bucket page you are browsing. Build status also impact the merge checks.
This can not be accepted

[cronik]

In my local testing on Bitbucket Server 8.19.10 the build status shows in all relevant locations:

PR List

PR Overview

Branch Commit History

In all my testing the refName of on PR build status has only worked when the PR source commit and branch exists on the target. This isn't the case when receiving PR's form forks. When refName is not set the build status was updated and displayed no matter the source repo/branch. If you point me to a specific case I'd be happy to test it.

[nfalco79]

The case is what has been fixed and tested in pr #954 with wide description of use cases.
You should impact only the specifc use case of prs from fork and not all pr in bb server that now works as expected.
Refers to the jira issue (already exists) and add test case to cover revision from fork
Build client only one time

[cronik]

#954 doesn't mentioned any specifics around the nature of the source of the PR that triggered the behavior. I was not able to reproduce the issue reported in all the scenarios I tested:

• source and target repo same, source feature branch
• source repo and branch different from target

Is your proposal that refName should only be set when StringUtils.equals(head.getRepoOwner(), source.getRepoOwner()) && StringUtils.equals(head.getRepository(), source.getRepository())?

[cronik]

Could it be that #954 was due to the client api pointing to a forked source repo were the credentials also had permission to update the status? If that's case then that might explain why I'm not able to reproduce and why it would make sense to revert to set refName back to null now that client is always pointing to target repo.

[cronik]

I think your question is if setting the refName=null and keeping the client pointing to head in the case the source repo is different than the target would solve the issue.

// Bitbucket Server branch
bitbucket = source.buildBitbucketClient(head);
refName = null // if source different than target

This will not work because the build status api would still point to the forked repo.

It doesn't make sense to report the build status to the source repo given the build status applies to the target where the merge checks are. Additionally the configured branch source credentials likely don't have permission to the source build status endpoint, especially if coming from a private fork.

bitbucket = source.buildBitbucketClient(head);
// `source` credentials does not have access to this path, results in 401
// POST /rest/api/latest/projects/~FOOBAR/repos/my-forked-repo/commits/c6c1fad1d85fd9352057fd48056d749d347c3fdf/builds

bitbucket = source.buildBitbucketClient();
// `source` is explicitly configured for this repo by definition
// POST /rest/api/latest/projects/TARGETPROJ/repos/target-repo/commits/c6c1fad1d85fd9352057fd48056d749d347c3fdf/builds

With the old build status api either would have worked because the path didn't include the repo owner or repo.

bitbucket = source.buildBitbucketClient(head);
// POST /rest/build-status/1.0/commits/c6c1fad1d85fd9352057fd48056d749d347c3fdf

bitbucket = source.buildBitbucketClient();
// POST /rest/build-status/1.0/commits/c6c1fad1d85fd9352057fd48056d749d347c3fdf

[nfalco79]

no my question is if we do in case of pr from fork

bitbucket = source.buildBitbucketClient();
refName = "refs/heads/" + head.getBranchName();

does it works in the same manner than refName = null?

[cronik]

In my testing refNull=null worked as expected in all scenarios when using bitbucket = source.buildBitbucketClient().

// works in all scenarios, forked or non-forked source
bitbucket = source.buildBitbucketClient();
refName = null;

// works only if source and target are the same
// does not work in forked scenario
bitbucket = source.buildBitbucketClient();
refName = "refs/heads/" + head.getBranchName();

[nfalco79]

I suspect that if you open from the same branch in the forked repository two different PRs to different target, for example master (PR-1) and release/1 (PR-2) branches you will get both build status in the PR pages because the build status is set to the same source commit and bitbucket could not filter which build status is from PR-1 and which from PR-2

[cronik]

You suspicion is correct. Further testing shows that when refName is set all PRs (forked or not) where the source matches the commit and branch name will display the build status. When refName is not set all PRs that match the commit id will show the build status.

I've updated the PR so that the only change is that the client points to the pr target. The refName is always set so the status will be limited to PRs that match the commit and branch name.

[nfalco79]

Thanks @cronik
Merged manually with a unit test and commit refers to the existing JIRA issue