Use more information from clone links when determining SCM URLs.

twz123 · twz123 · commit 3f2ca008dc5e · 2018-03-17T13:54:41.000+01:00
Motivation:

Some Bitbucket Server installations use clone URLs that are completely different from the URLs used for web-based access. The clone URLs are correctly advertised via the REST interface, but the the plugin solely uses them to determine alternate port numbers. This leads to SCM checkout failures during builds, since the SCM remote URLs determined by the plugin are wrong.

Modifications:

Pass on the whole clone link to BitbucketApi::getRepositoryUri, instead of just passing on the pre-parsed port number. Let implementors decide what to do with this extra piece of information:

  * Cloud: just ignore
  * Server: Just use if cloning via SSH: Use the scheme and authority (which includes user, host and port number) from the clone link, instead of hard-coding and relying on the URL for web-based access

Result:

Cloning via SSH works, even with Bitbucket Server setups that use completely differrent URLs for cloning and web access.
diff --git a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketGitSCMBuilder.java b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketGitSCMBuilder.java
@@ -170,22 +170,14 @@ public BitbucketGitSCMBuilder withBitbucketRemote() {
                         credentialsId(),
                         StandardCredentials.class
                 );
-        Integer protocolPortOverride = null;
         BitbucketRepositoryProtocol protocol = credentials instanceof SSHUserPrivateKey
                 ? BitbucketRepositoryProtocol.SSH
                 : BitbucketRepositoryProtocol.HTTP;
+        String cloneLink = null;
         if (protocol == BitbucketRepositoryProtocol.SSH) {
             for (BitbucketHref link : cloneLinks()) {
                 if ("ssh".equals(link.getName())) {
-                    // extract the port from this link and use that
-                    try {
-                        URI uri = new URI(link.getHref());
-                        if (uri.getPort() != -1) {
-                            protocolPortOverride = uri.getPort();
-                        }
-                    } catch (URISyntaxException e) {
-                        // ignore
-                    }
+                    cloneLink = link.getHref();
                     break;
                 }
             }
@@ -206,7 +198,7 @@ public BitbucketGitSCMBuilder withBitbucketRemote() {
         withRemote(bitbucket.getRepositoryUri(
                 BitbucketRepositoryType.GIT,
                 protocol,
-                protocolPortOverride,
+                cloneLink,
                 repoOwner,
                 repository));
         AbstractBitbucketEndpoint endpoint =
@@ -233,7 +225,7 @@ public BitbucketGitSCMBuilder withBitbucketRemote() {
                         bitbucket.getRepositoryUri(
                                 BitbucketRepositoryType.GIT,
                                 protocol,
-                                protocolPortOverride,
+                                cloneLink,
                                 scmSource().getRepoOwner(),
                                 scmSource().getRepository()),
                         "+refs/heads/" + name + ":refs/remotes/@{remote}/" + localName);
diff --git a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketHgSCMBuilder.java b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketHgSCMBuilder.java
@@ -161,22 +161,14 @@ public BitbucketHgSCMBuilder withBitbucketSource() {
                         credentialsId(),
                         StandardCredentials.class
                 );
-        Integer protocolPortOverride = null;
         BitbucketRepositoryProtocol protocol = credentials instanceof SSHUserPrivateKey
                 ? BitbucketRepositoryProtocol.SSH
                 : BitbucketRepositoryProtocol.HTTP;
+        String cloneLink = null;
         if (protocol == BitbucketRepositoryProtocol.SSH) {
             for (BitbucketHref link : cloneLinks()) {
                 if ("ssh".equals(link.getName())) {
-                    // extract the port from this link and use that
-                    try {
-                        URI uri = new URI(link.getHref());
-                        if (uri.getPort() != -1) {
-                            protocolPortOverride = uri.getPort();
-                        }
-                    } catch (URISyntaxException e) {
-                        // ignore
-                    }
+                    cloneLink = link.getHref();
                     break;
                 }
             }
@@ -197,7 +189,7 @@ public BitbucketHgSCMBuilder withBitbucketSource() {
         withSource(bitbucket.getRepositoryUri(
                 BitbucketRepositoryType.MERCURIAL,
                 protocol,
-                protocolPortOverride,
+                cloneLink,
                 repoOwner,
                 repository));
         AbstractBitbucketEndpoint endpoint =
diff --git a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/api/BitbucketApi.java b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/api/BitbucketApi.java
@@ -57,15 +57,15 @@ public interface BitbucketApi {
      *
      * @param type the type of repository.
      * @param protocol the protocol to access the repository with.
-     * @param protocolPortOverride the port to override or {@code null} to use the default.
+     * @param cloneLink the actual clone link for the repository as sent by the server, or {@code null} if unknown.
      * @param owner the owner
      * @param repository the repository.
      * @return the URI.
      */
     @NonNull
     String getRepositoryUri(@NonNull BitbucketRepositoryType type,
                             @NonNull BitbucketRepositoryProtocol protocol,
-                            @CheckForNull Integer protocolPortOverride,
+                            @CheckForNull String cloneLink,
                             @NonNull String owner,
                             @NonNull String repository);
 
diff --git a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiClient.java b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiClient.java
@@ -195,7 +195,7 @@ public String getRepositoryName() {
     @Override
     public String getRepositoryUri(@NonNull BitbucketRepositoryType type,
                                    @NonNull BitbucketRepositoryProtocol protocol,
-                                   @CheckForNull Integer protocolPortOverride,
+                                   @CheckForNull String cloneLink,
                                    @NonNull String owner,
                                    @NonNull String repository) {
         // ignore port override on Cloud
diff --git a/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java b/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java
@@ -57,16 +57,16 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetSocketAddress;
-import java.net.MalformedURLException;
 import java.net.Proxy;
 import java.net.URI;
-import java.net.URL;
+import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.Callable;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -203,53 +203,47 @@ public String getRepositoryName() {
     @Override
     public String getRepositoryUri(@NonNull BitbucketRepositoryType type,
                                    @NonNull BitbucketRepositoryProtocol protocol,
-                                   @CheckForNull Integer protocolPortOverride,
+                                   @CheckForNull String cloneLink,
                                    @NonNull String owner,
                                    @NonNull String repository) {
         switch (type) {
             case GIT:
-                URL url;
+                URI baseUri;
                 try {
-                    url = new URL(baseURL);
-                } catch (MalformedURLException e) {
-                    throw new IllegalStateException("Server URL is not a valid URL", e);
+                    baseUri = new URI(baseURL);
+                } catch (URISyntaxException e) {
+                    throw new IllegalStateException("Server URL is not a valid URI", e);
                 }
-                StringBuilder result = new StringBuilder();
+
+                UriTemplate template = UriTemplate.fromTemplate("{scheme}://{+authority}{+path}{/owner,repository}.git");
+                template.set("scheme", baseUri.getScheme());
+                template.set("authority", baseUri.getRawAuthority());
+                template.set("owner", owner);
+                template.set("repository", repository);
+
                 switch (protocol) {
                     case HTTP:
-                        result.append(url.getProtocol());
-                        result.append("://");
-                        if (protocolPortOverride != null && protocolPortOverride > 0) {
-                            result.append(url.getHost());
-                            result.append(':');
-                            result.append(protocolPortOverride);
-                        } else {
-                            result.append(url.getAuthority());
-                        }
-                        result.append(url.getPath());
-                        result.append("/scm/");
-                        result.append(owner);
-                        result.append('/');
-                        result.append(repository);
-                        result.append(".git");
+                        template.set("path", Objects.toString(baseUri.getRawPath(), "") + "/scm");
                         break;
                     case SSH:
-                        result.append("ssh://git@");
-                        result.append(url.getHost());
-                        if (protocolPortOverride  != null && protocolPortOverride > 0) {
-                            result.append(':');
-                            result.append(protocolPortOverride);
+                        if (cloneLink != null) {
+                            try {
+                                URI cloneLinkUri = new URI(cloneLink);
+                                if (cloneLinkUri.getScheme() != null) {
+                                    template.set("scheme", cloneLinkUri.getScheme());
+                                }
+                                if (cloneLinkUri.getRawAuthority() != null) {
+                                    template.set("authority", cloneLinkUri.getRawAuthority());
+                                }
+                            } catch (@SuppressWarnings("unused") URISyntaxException ignored) {
+                                // fall through
+                            }
                         }
-                        result.append('/');
-                        result.append(owner);
-                        result.append('/');
-                        result.append(repository);
-                        result.append(".git");
                         break;
                     default:
                         throw new IllegalArgumentException("Unsupported repository protocol: " + protocol);
                 }
-                return result.toString();
+                return template.expand();
                 default:
                     throw new IllegalArgumentException("Unsupported repository type: " + type);
         }
diff --git a/src/test/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketClientMockUtils.java b/src/test/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketClientMockUtils.java
@@ -57,7 +57,7 @@ public class BitbucketClientMockUtils {
     public static BitbucketCloudApiClient getAPIClientMock(BitbucketRepositoryType type, boolean includePullRequests,
             boolean includeWebHooks) throws IOException, InterruptedException {
         BitbucketCloudApiClient bitbucket = mock(BitbucketCloudApiClient.class);
-        when(bitbucket.getRepositoryUri(any(BitbucketRepositoryType.class), any(BitbucketRepositoryProtocol.class), any(Integer.class), anyString(), anyString())).thenCallRealMethod();
+        when(bitbucket.getRepositoryUri(any(BitbucketRepositoryType.class), any(BitbucketRepositoryProtocol.class), anyString(), anyString(), anyString())).thenCallRealMethod();
         // mock branch list
         List<BitbucketCloudBranch> branches = new ArrayList<BitbucketCloudBranch>();
         branches.add(getBranch("branch1", "52fc8e220d77ec400f7fc96a91d2fd0bb1bc553a"));
diff --git a/src/test/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketGitSCMBuilderTest.java b/src/test/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketGitSCMBuilderTest.java
diff --git a/src/test/java/com/cloudbees/jenkins/plugins/bitbucket/UriResolverTest.java b/src/test/java/com/cloudbees/jenkins/plugins/bitbucket/UriResolverTest.java
diff --git a/src/test/java/integration/ScanningFailuresTest.java b/src/test/java/integration/ScanningFailuresTest.java
