Skip to content

Commit

Permalink
Refactor Connector for better GitHubAppCredentials behavior
Browse files Browse the repository at this point in the history
GitHubAppCredentials could not use the existing Connector.connect() and so created its own
GithubBuilder.  However, that means that is missing a number of standard settings provided by
Connector.connect(), including okhttp, Jenkins proxy settings, and rate limit handling.

This change refactors Connector to add an internal createGitHubBuilder() method that returns a
GitHubBuilder with those features configured. For simplicity, The returned GitHubBuilder
is not cached and also does not cache responses.  If there turns out to be a need for it,
that behavior can be added later.
  • Loading branch information
bitwiseman committed Jun 3, 2020
1 parent d8b2c8a commit 342952d
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.plugins.gitclient.GitClient;
import org.jenkinsci.plugins.github.config.GitHubServerConfig;
import org.kohsuke.github.GHAppInstallationToken;
import org.kohsuke.github.GitHub;
import org.kohsuke.github.GitHubBuilder;
import org.kohsuke.github.RateLimitHandler;
Expand Down Expand Up @@ -364,45 +365,10 @@ public static void checkApiUrlValidity(@Nonnull GitHub gitHub, @CheckForNull Sta
usage.put(hub, count == null ? 1 : Math.max(count + 1, 1));
return hub;
}
String host;
try {
host = new URL(apiUrl).getHost();
} catch (MalformedURLException e) {
throw new IOException("Invalid GitHub API URL: " + apiUrl, e);
}

GitHubBuilder gb = new GitHubBuilder();
gb.withEndpoint(apiUrl);
gb.withRateLimitHandler(CUSTOMIZED);

OkHttpClient.Builder clientBuilder = baseClient.newBuilder();
clientBuilder.proxy(getProxy(host));

int cacheSize = GitHubSCMSource.getCacheSize();
if (cacheSize > 0) {
File cacheBase = new File(jenkins.getRootDir(),
GitHubSCMProbe.class.getName() + ".cache");
File cacheDir = null;
try {
MessageDigest sha256 = MessageDigest.getInstance("SHA-256");
sha256.update(apiUrl.getBytes(StandardCharsets.UTF_8));
sha256.update("::".getBytes(StandardCharsets.UTF_8));
if (username != null) {
sha256.update(username.getBytes(StandardCharsets.UTF_8));
}
sha256.update("::".getBytes(StandardCharsets.UTF_8));
sha256.update(authHash.getBytes(StandardCharsets.UTF_8));
cacheDir = new File(cacheBase, Base64.encodeBase64URLSafeString(sha256.digest()));
} catch (NoSuchAlgorithmException e) {
// no cache for you mr non-spec compliant JVM
}
if (cacheDir != null) {
Cache cache = new Cache(cacheDir, cacheSize * 1024L * 1024L);
clientBuilder.cache(cache);
}
}
Cache cache = getCache(jenkins, apiUrl, authHash, username);

gb.withConnector(new OkHttpConnector(clientBuilder.build()));
GitHubBuilder gb = createGitHubBuilder(apiUrl, cache);

if (username != null) {
gb.withPassword(username, password);
Expand All @@ -416,6 +382,73 @@ public static void checkApiUrlValidity(@Nonnull GitHub gitHub, @CheckForNull Sta
}
}

/**
* Creates a {@link GitHubBuilder} that can be used to build a {@link GitHub} instance.
*
* This method creates and configures a new {@link GitHubBuilder}.
* This should be used only when {@link #connect(String, StandardCredentials)} cannot be used,
* such as when using {@link GitHubBuilder#withJwtToken(String)} to getting the {@link GHAppInstallationToken}.
*
* This method intentionally does not support caching requests or {@link GitHub} instances.
*
* @param apiUrl the GitHub API URL to be used for the connection
* @return a configured GitHubBuilder instance
* @throws IOException if I/O error occurs
*/
static GitHubBuilder createGitHubBuilder(@Nonnull String apiUrl) throws IOException {
return createGitHubBuilder(apiUrl, null);
}

@Nonnull
private static GitHubBuilder createGitHubBuilder(@Nonnull String apiUrl, @CheckForNull Cache cache) throws IOException {
String host;
try {
host = new URL(apiUrl).getHost();
} catch (MalformedURLException e) {
throw new IOException("Invalid GitHub API URL: " + apiUrl, e);
}

GitHubBuilder gb = new GitHubBuilder();
gb.withEndpoint(apiUrl);
gb.withRateLimitHandler(CUSTOMIZED);

OkHttpClient.Builder clientBuilder = baseClient.newBuilder();
clientBuilder.proxy(getProxy(host));
if (cache != null) {
clientBuilder.cache(cache);
}
gb.withConnector(new OkHttpConnector(clientBuilder.build()));
return gb;
}

@CheckForNull
private static Cache getCache(@Nonnull Jenkins jenkins, @Nonnull String apiUrl, @Nonnull String authHash, @CheckForNull String username) {
Cache cache = null;
int cacheSize = GitHubSCMSource.getCacheSize();
if (cacheSize > 0) {
File cacheBase = new File(jenkins.getRootDir(),
GitHubSCMProbe.class.getName() + ".cache");
File cacheDir = null;
try {
MessageDigest sha256 = MessageDigest.getInstance("SHA-256");
sha256.update(apiUrl.getBytes(StandardCharsets.UTF_8));
sha256.update("::".getBytes(StandardCharsets.UTF_8));
if (username != null) {
sha256.update(username.getBytes(StandardCharsets.UTF_8));
}
sha256.update("::".getBytes(StandardCharsets.UTF_8));
sha256.update(authHash.getBytes(StandardCharsets.UTF_8));
cacheDir = new File(cacheBase, Base64.encodeBase64URLSafeString(sha256.digest()));
} catch (NoSuchAlgorithmException e) {
// no cache for you mr non-spec compliant JVM
}
if (cacheDir != null) {
cache = new Cache(cacheDir, cacheSize * 1024L * 1024L);
}
}
return cache;
}

public static void release(@CheckForNull GitHub hub) {
if (hub == null) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ public void setOwner(String owner) {
static String generateAppInstallationToken(String appId, String appPrivateKey, String apiUrl, String owner) {
try {
String jwtToken = createJWT(appId, appPrivateKey);
GitHub gitHubApp = new GitHubBuilder().withEndpoint(apiUrl).withJwtToken(jwtToken).build();
GitHub gitHubApp = Connector

This comment has been minimized.

Copy link
@jglick

jglick Jun 10, 2020

Member

JENKINS-62655

.createGitHubBuilder(apiUrl)
.withJwtToken(jwtToken)
.build();

GHApp app = gitHubApp.getApp();

Expand Down

0 comments on commit 342952d

Please sign in to comment.