Merge pull request #454 from bluesliverx/master

JENKINS-66401 Use configured GitHub endpoint for default

src/main/java/org/jenkinsci/plugins/github_branch_source/ApiRateLimitChecker.java

@@ -6,11 +6,13 @@
 import hudson.model.TaskListener;
 import hudson.util.LogTaskListener;
 import java.io.IOException;
+import java.util.List;
 import java.util.Objects;
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.github.config.GitHubServerConfig;
 import org.kohsuke.github.GHRateLimit;
 import org.kohsuke.github.GitHub;
@@ -231,13 +233,22 @@ protected boolean checkRateLimit(GHRateLimit.Record rateLimitRecord, long count)
         throws InterruptedException {
       LocalChecker checker = getLocalChecker();
       if (checker == null) {
-        // If a checker was not configured for this thread, try our best and continue.
-        configureThreadLocalChecker(
-            new LogTaskListener(LOGGER, Level.INFO), GitHubServerConfig.GITHUB_URL);
+        // If a checker was not configured for this thread, try our best by attempting to get the
+        // URL from the first configured GitHub endpoint, else default to the public endpoint.
+        // NOTE: Defaulting to the public GitHub endpoint is insufficient for those using GitHub
+        // enterprise as it forces rate limit checking in those cases.
+        String apiUrl = GitHubServerConfig.GITHUB_URL;
+        List<Endpoint> endpoints = GitHubConfiguration.get().getEndpoints();
+        if (endpoints.size() > 0 && !StringUtils.isBlank(endpoints.get(0).getApiUri())) {
+          apiUrl = endpoints.get(0).getApiUri();
+        }
+        configureThreadLocalChecker(new LogTaskListener(LOGGER, Level.INFO), apiUrl);
         checker = getLocalChecker();
         checker.writeLog(
             "LocalChecker for rate limit was not set for this thread. "
-                + "Configured using system settings.");
+                + "Configured using system settings with API URL '"
+                + apiUrl
+                + "'.");
       }
       return checker.checkRateLimit(rateLimitRecord, count);
     }

src/main/java/org/jenkinsci/plugins/github_branch_source/Connector.java

@@ -544,7 +544,8 @@ public void onError(IOException e, HttpURLConnection uc) throws IOException {
 
   /**
    * Alternative to {@link GitHub#isCredentialValid()} that relies on the cached user object in the
-   * {@link GitHub} instance and hence reduced rate limit consumption.
+   * {@link GitHub} instance and hence reduced rate limit consumption. It also uses a separate
+   * endpoint if rate limit checking is disabled.
    *
    * @param gitHub the instance to check.
    * @return {@code true} if the credentials are valid.
@@ -554,7 +555,15 @@ static boolean isCredentialValid(GitHub gitHub) {
       return true;
     } else {
       try {
-        gitHub.getRateLimit();
+        // If rate limit checking is disabled, use the meta endpoint instead
+        // of the rate limiting endpoint
+        GitHubConfiguration gitHubConfiguration = GitHubConfiguration.get();
+        if (gitHubConfiguration != null
+            && gitHubConfiguration.getApiRateLimitChecker() == ApiRateLimitChecker.NoThrottle) {
+          gitHub.getMeta();
+        } else {
+          gitHub.getRateLimit();
+        }
         return true;
       } catch (IOException e) {
         if (LOGGER.isLoggable(FINE)) {

src/test/java/org/jenkinsci/plugins/github_branch_source/ApiRateLimitCheckerTest.java

@@ -21,6 +21,7 @@
 import java.util.logging.Logger;
 import java.util.stream.Stream;
 import org.jenkinsci.plugins.github.config.GitHubServerConfig;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.kohsuke.github.GHRateLimit;
@@ -92,6 +93,11 @@ public void setUp() throws Exception {
     ApiRateLimitChecker.resetLocalChecker();
   }
 
+  @After
+  public void tearDown() throws Exception {
+    GitHubConfiguration.get().setEndpoints(new ArrayList<>());
+  }
+
   private void setupStubs(List<RateLimit> scenarios) throws Exception {
 
     githubApi.stubFor(
@@ -187,6 +193,7 @@ public void NoCheckerConfigured() throws Exception {
     assertEquals(
         3,
         countOfOutputLinesContaining("LocalChecker for rate limit was not set for this thread."));
+    assertEquals(3, countOfOutputLinesContaining("with API URL 'https://api.github.com'"));
     assertEquals(3, countOfOutputLines(m -> m.matches(".*[sS]leeping.*")));
     // github rate_limit endpoint should be contacted for ThrottleOnOver
     // rateLimit()
@@ -199,6 +206,38 @@ public void NoCheckerConfigured() throws Exception {
     assertEquals(initialRequestCount + 9, getRequestCount(githubApi));
   }
 
+  @Test
+  public void NoCheckerConfiguredWithEndpoint() throws Exception {
+    // set up scenarios
+    List<RateLimit> scenarios = new ArrayList<>();
+    long now = System.currentTimeMillis();
+    int limit = 5000;
+    scenarios.add(new RateLimit(limit, 30, new Date(now - 10000)));
+    scenarios.add(new RateLimit(limit, limit, new Date(now - 8000)));
+    scenarios.add(new RateLimit(limit, 20, new Date(now - 6000)));
+    scenarios.add(new RateLimit(limit, limit, new Date(now - 4000)));
+    scenarios.add(new RateLimit(limit, 10, new Date(now - 2000)));
+    scenarios.add(new RateLimit(limit, limit, new Date(now)));
+    setupStubs(scenarios);
+
+    List<Endpoint> endpoints = new ArrayList<>();
+    endpoints.add(new Endpoint("https://git.company.com/api/v3", "Company GitHub"));
+    endpoints.add(new Endpoint("https://git2.company.com/api/v3", "Company GitHub 2"));
+    GitHubConfiguration.get().setEndpoints(endpoints);
+
+    GitHubConfiguration.get().setApiRateLimitChecker(ApiRateLimitChecker.NoThrottle);
+    github.getMeta();
+
+    assertEquals(
+        1,
+        countOfOutputLinesContaining("LocalChecker for rate limit was not set for this thread."));
+    assertEquals(1, countOfOutputLinesContaining("with API URL 'https://git.company.com/api/v3'"));
+    // ThrottleOnOver should not be used for NoThrottle since it is not the public GitHub endpoint
+    assertEquals(0, countOfOutputLinesContaining("ThrottleOnOver will be used instead"));
+
+    assertEquals(initialRequestCount + 2, getRequestCount(githubApi));
+  }
+
   /**
    * Verify that the throttle does not happen in OnOver throttle when none of the quota has been
    * used