-
Notifications
You must be signed in to change notification settings - Fork 353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support cloning from Bitbucket mirror #796
Support cloning from Bitbucket mirror #796
Conversation
LOGGER.log(Level.INFO, "Received hook from Bitbucket. Processing push event on {0}/{1}", | ||
new Object[] { owner, repository }); | ||
scmSourceReIndex(owner, repository); | ||
new Object[] { owner, repositoryName }); |
Check failure
Code scanning / CodeQL
Log Injection High
user-provided value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This problem is not caused by this pull request. It's already existing. I believe validation of incoming events is another big task and out of scope of this pull request.
f269b46
to
d5750f4
Compare
A mirrored Git repository can be configured for fetching references. The mirror is not used in the following cases: - If the source branch in a pull request resides in a different repository, the source branch is fetched from the primary repository while the target branch is fetched from the mirror. - During initial pull request scanning, the mirror isn't used because PullRequestSCMHead doesn't contain the hash needed for the build and SCMRevision is null. This is the current design limitation and can be refactored later. Cloning from the mirror can only be used with native web-hooks since plugin web-hooks don't provide a mirror identifier. For branches and tags, the mirror sync event is used. Thus, at cloning time, the mirror is already synchronized. However, in the case of a pull request event, there is no such guarantee. The plugin optimistically assumes that the mirror is synced and the required commit hashes exist in the mirrored repository at cloning time. If the plugin can't find the required hashes, it falls back to the primary repository. Fixes jenkinsci#592 Co-authors: - Andrey Fomin https://github.com/andrey-fomin - Andrei Kouznetchik https://github.com/akouznetchik - Eugene Mercuriev https://github.com/zamonier
b1198f6
to
5f7c379
Compare
public R getPull() { | ||
public AbstractGitSCMSource.SCMRevisionImpl getPull() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was using this API in a private plugin and now I get an error:
java.lang.NoSuchMethodError: 'jenkins.scm.api.SCMRevision com.cloudbees.jenkins.plugins.bitbucket.PullRequestSCMRevision.getPull()'
I suppose rebuilding my plugin might fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rolled back to 866.vdea_7dcd3008e for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The private plugin is using this kind of code to get the Git commit ID of the source branch of a pull request:
SCMRevision revision = SCMRevisionAction.getRevision(scmSource, run);
// revision might be one of:
//
// * in <https://github.com/jenkinsci/bitbucket-branch-source-plugin/>:
// - com.cloudbees.jenkins.plugins.bitbucket.BitbucketGitSCMRevision
// - com.cloudbees.jenkins.plugins.bitbucket.PullRequestSCMRevision
// * in <https://github.com/jenkinsci/git-plugin/>:
// - jenkins.plugins.git.GitBranchSCMRevision
//
// Not all of those are derived from
// jenkins.plugins.git.AbstractGitSCMSource.SCMRevisionImpl,
// so we cannot unconditionally cast to that and call getHash().
if (revision instanceof PullRequestSCMRevision) {
final PullRequestSCMRevision pullRevision = (PullRequestSCMRevision)revision;
revision = pullRevision.getPull();
}
AFAIK, this information is not available from scm-api-plugin and must instead be retrieved via the API of bitbucket-branch-source-plugin. This API is not annotated with any restriction like @NoExternalUse
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#817 appears intended to fix this binary incompatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified that Bitbucket Branch Source 877.vb_b_d5243f6794, which includes #817, restores binary compatibility with my private plugin.
In Bitbucket Service "http" is used as http link name, but in Bitbucket Cloud "https" is used. Bug was introduced in jenkinsci#796. Fixes jenkinsci#804.
In Bitbucket Server "http" is used as http link name, but in Bitbucket Cloud "https" is used. Bug was introduced in jenkinsci#796. Fixes jenkinsci#804.
@@ -66,11 +71,12 @@ public class BitbucketSCMSourceBuilder extends SCMSourceBuilder<BitbucketSCMSour | |||
*/ | |||
public BitbucketSCMSourceBuilder(@CheckForNull String id, @NonNull String serverUrl, | |||
@CheckForNull String credentialsId, @NonNull String repoOwner, | |||
@NonNull String repoName) { | |||
@NonNull String repoName, @CheckForNull String mirrorId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an incompatible change and I think it broke Blue Ocean: jenkinsci/bom#2922 (comment)
Either switch the mirrorId
to a (possibly “fluid”) setter, or retain a @Deprecated
constructor with the original signature.
Problem was introduced in jenkinsci#796. Was mentioned in comment jenkinsci#796 (comment)
Problem was introduced in #796. Was mentioned in comment #796 (comment)
Problem was introduced in jenkinsci#796. Fixes jenkinsci#808.
Problem was introduced in jenkinsci#796. Fixes jenkinsci#808.
Makes RefSpecsSCMSourceTrait working again. Fixes jenkinsci#814. See also jenkinsci#812. Problem was introduced in jenkinsci#796.
Method `getPull` is used in some private plugins. jenkinsci#796 changed signature of this method. Now signature is returned back. See also jenkinsci#817.
Method `getPull` is used in some private plugins. jenkinsci#796 changed signature of this method. Now signature is returned back. See also jenkinsci#817.
Previously token was fetched only once and saved in BitbucketSCMSource. Fixes jenkinsci#808. See also jenkinsci#810 and jenkinsci#796.
String scmSourceRepository = scmSource.getRepository(); | ||
String pullRequestRepoOwner = head.getRepoOwner(); | ||
String pullRequestRepository = head.getRepository(); | ||
boolean prFromTargetRepository = pullRequestRepoOwner.equals(scmSourceRepoOwner) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrey-fomin I've run into an issue with this.
When configuring a job with a Bitbucket branch source, if you enter the "Owner" the input is case-insensitive.
The "Repository Name" field below "Owner" will be populated correctly, regardless of case.
But then when you actually build a same-origin PR, the prFromTargetRepository
will return false
, leading to very hard to diagnose issues.
In my case I was severely confused when for 1 specific pipeline my refs were generated as:
"+refs/pull-requests/" + pullId + "/from:refs/remotes/@{remote}/" + headName
instead of the expected:
"+refs/heads/" + branchName + ":refs/remotes/@{remote}/" + branchName
All because I had entered the "Owner" as lower-case (which the Jenkins job config happily accepted), whereas in Bitbucket it is upper-case.
I think this line should be:
boolean prFromTargetRepository = pullRequestRepoOwner.equalsIgnoreCase(scmSourceRepoOwner)
&& pullRequestRepository.equalsIgnoreCase(scmSourceRepository);
A mirrored Git repository can be configured for fetching references.
The mirror is not used in the following cases:
Cloning from the mirror can only be used with native web-hooks since plugin web-hooks don't provide a mirror identifier.
For branches and tags, the mirror sync event is used. Thus, at cloning time, the mirror is already synchronized. However, in the case of a pull request event, there is no such guarantee. The plugin optimistically assumes that the mirror is synced and the required commit hashes exist in the mirrored repository at cloning time. If the plugin can't find the required hashes, it falls back to the primary repository.
Fixes #592
Co-authors:
Your checklist for this pull request