From 9270f385b5f5b8aee8104106170f5c5c4674c2f2 Mon Sep 17 00:00:00 2001 From: gorenn Date: Sat, 7 Nov 2026 12:16:24 +0000 Subject: [PATCH 1/2] Use OkHttpGitHubConnector so OkHttp interceptors see GitHub API calls The deprecated OkHttpConnector sends each request through github-api's ObsoleteUrlFactory, which clears the client's interceptors and network interceptors on every call. OkHttp instrumentation, such as the OpenTelemetry Java agent, never sees the plugin's GitHub API requests. github-api deprecated OkHttpConnector in favor of OkHttpGitHubConnector (hub4j/github-api#1262) and removes it in 2.0. GitHubLoginFunctionTest checks that client interceptors run, that a cached response is revalidated with its ETag, and that a rate limit response still fails the request. Co-Authored-By: Claude Opus 5.5 --- .../github/internal/GitHubLoginFunction.java | 21 ++- .../internal/GitHubLoginFunctionTest.java | 130 ++++++++++++++++++ 2 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 src/test/java/org/jenkinsci/plugins/github/internal/GitHubLoginFunctionTest.java diff --git a/src/main/java/org/jenkinsci/plugins/github/internal/GitHubLoginFunction.java b/src/main/java/org/jenkinsci/plugins/github/internal/GitHubLoginFunction.java index ecee2d33b..428123a7a 100644 --- a/src/main/java/org/jenkinsci/plugins/github/internal/GitHubLoginFunction.java +++ b/src/main/java/org/jenkinsci/plugins/github/internal/GitHubLoginFunction.java @@ -12,7 +12,7 @@ import org.kohsuke.github.GitHub; import org.kohsuke.github.GitHubBuilder; import org.kohsuke.github.RateLimitHandler; -import org.kohsuke.github.extras.okhttp3.OkHttpConnector; +import org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,6 +48,19 @@ public class GitHubLoginFunction extends NullSafeFunction application = new CopyOnWriteArrayList<>(); + List network = new CopyOnWriteArrayList<>(); + OkHttpClient client = new OkHttpClient.Builder() + .addInterceptor(chain -> { + application.add(chain.request().method() + " " + chain.request().url().encodedPath()); + return chain.proceed(chain.request()); + }) + .addNetworkInterceptor(chain -> { + Response response = chain.proceed(chain.request()); + network.add(chain.request().method() + " " + chain.request().url().encodedPath() + + " " + response.code()); + return response; + }) + .build(); + GitHubServerConfig config = serverConfig(); + config.setClientCacheSize(0); + + GitHub github = new GitHubLoginFunction(client).apply(config); + assertNotNull(github); + assertEquals("repo", github.getRepository("org/repo").getName()); + + assertThat(application, hasItem("GET " + REPO_PATH)); + assertThat(network, hasItem("GET " + REPO_PATH + " 200")); + } + + @Test + void cachedResponseIsRevalidatedWithETag() throws Exception { + // like GitHubClientCacheCleanupTest: the disk cache in JENKINS_HOME stays open on Windows + assumeFalse(Functions.isWindows(), "the open disk cache keeps JENKINS_HOME from being deleted on Windows"); + githubApi.stubFor(get(urlEqualTo(REPO_PATH)) + .withHeader("If-None-Match", absent()) + .willReturn(okJson(REPO_JSON).withHeader("ETag", "\"v1\""))); + githubApi.stubFor(get(urlEqualTo(REPO_PATH)) + .withHeader("If-None-Match", equalTo("\"v1\"")) + .willReturn(aResponse().withStatus(304).withHeader("ETag", "\"v1\""))); + GitHubServerConfig config = serverConfig(); + config.setClientCacheSize(1); + + GitHub github = new GitHubLoginFunction().apply(config); + assertNotNull(github); + assertEquals("repo", github.getRepository("org/repo").getName()); + assertEquals("repo", github.getRepository("org/repo").getName()); + + // Every request asks GitHub (max-age=0); the second one is conditional and is answered from the cache. + githubApi.verify(2, getRequestedFor(urlEqualTo(REPO_PATH)).withHeader("Cache-Control", equalTo("max-age=0"))); + githubApi.verify(1, getRequestedFor(urlEqualTo(REPO_PATH)).withHeader("If-None-Match", equalTo("\"v1\""))); + } + + @Test + void rateLimitExceededFails() throws Exception { + githubApi.stubFor(get(urlEqualTo(REPO_PATH)) + .willReturn(aResponse() + .withStatus(403) + .withHeader("Content-Type", "application/json") + .withHeader("X-RateLimit-Limit", "5000") + .withHeader("X-RateLimit-Remaining", "0") + .withHeader("X-RateLimit-Reset", Long.toString(System.currentTimeMillis() / 1000 + 3600)) + .withBody("{\"message\": \"API rate limit exceeded\"}"))); + GitHubServerConfig config = serverConfig(); + config.setClientCacheSize(0); + + GitHub github = new GitHubLoginFunction().apply(config); + assertNotNull(github); + HttpException e = assertThrows(HttpException.class, () -> github.getRepository("org/repo")); + + assertEquals(403, e.getResponseCode()); + githubApi.verify(1, getRequestedFor(urlEqualTo(REPO_PATH))); + } + + private static GitHubServerConfig serverConfig() { + GitHubServerConfig config = new GitHubServerConfig("creds"); + config.setApiUrl(githubApi.baseUrl()); + return config; + } +} From 7a58f60863a2ca929815370b8d550a2090c42f40 Mon Sep 17 00:00:00 2001 From: gorenn Date: Sat, 7 Nov 2026 13:52:13 +0000 Subject: [PATCH 2/2] Turn the mock GitHub client cache off on Windows JenkinsRule turns URLConnection caches off on Windows. ObsoleteUrlFactory followed that and dropped the OkHttp cache, so on Windows the tests that use GitHubMockExtension.serverConfig() never opened it. OkHttpGitHubConnector uses the cache, and its open journal keeps JenkinsRule from deleting JENKINS_HOME, as in GitHubClientCacheCleanupTest. Co-Authored-By: Claude Opus 5.5 --- .../jenkinsci/plugins/github/test/GitHubMockExtension.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/java/org/jenkinsci/plugins/github/test/GitHubMockExtension.java b/src/test/java/org/jenkinsci/plugins/github/test/GitHubMockExtension.java index fc5687a9f..4e6a01b4f 100644 --- a/src/test/java/org/jenkinsci/plugins/github/test/GitHubMockExtension.java +++ b/src/test/java/org/jenkinsci/plugins/github/test/GitHubMockExtension.java @@ -4,6 +4,7 @@ import com.cloudbees.jenkins.GitHubRepositoryNameContributor; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; +import hudson.Functions; import hudson.model.Item; import org.jenkinsci.plugins.github.config.GitHubServerConfig; @@ -60,6 +61,10 @@ protected void onBeforeEach(WireMockRuntimeInfo wireMockRuntimeInfo) { public GitHubServerConfig serverConfig() { GitHubServerConfig conf = new GitHubServerConfig("creds"); conf.setApiUrl("http://localhost:" + getPort()); + if (Functions.isWindows()) { + // the open disk cache would keep JENKINS_HOME from being deleted, see GitHubClientCacheCleanupTest + conf.setClientCacheSize(0); + } return conf; }