refactor(retrofit): replace OkClient with Ok3Client (#1282)

spinnaker · Nov 6, 2024 · e95c4de · e95c4de
1 parent 8b2d29a
commit e95c4de
Show file tree

Hide file tree

Showing 11 changed files with 34 additions and 100 deletions.
diff --git a/igor-monitor-travis/igor-monitor-travis.gradle b/igor-monitor-travis/igor-monitor-travis.gradle
@@ -28,6 +28,7 @@ dependencies {
   implementation "com.fasterxml.jackson.core:jackson-databind"
   implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-xml"
   implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml"
+  implementation "com.jakewharton.retrofit:retrofit1-okhttp3-client"
   implementation "com.squareup.retrofit:converter-jackson"
   implementation "com.squareup.retrofit:retrofit"
   implementation "io.reactivex:rxjava" // TODO(jervi): get rid of this dependency

diff --git a/igor-monitor-travis/src/main/java/com/netflix/spinnaker/igor/travis/config/TravisConfig.java b/igor-monitor-travis/src/main/java/com/netflix/spinnaker/igor/travis/config/TravisConfig.java
@@ -18,6 +18,7 @@
 package com.netflix.spinnaker.igor.travis.config;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spinnaker.igor.IgorConfigurationProperties;
 import com.netflix.spinnaker.igor.service.ArtifactDecorator;
 import com.netflix.spinnaker.igor.service.BuildServices;
@@ -27,7 +28,6 @@
 import com.netflix.spinnaker.igor.travis.service.TravisService;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
-import com.squareup.okhttp.OkHttpClient;
 import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry;
 import java.util.ArrayList;
 import java.util.Map;
@@ -37,14 +37,14 @@
 import java.util.stream.Collectors;
 import javax.validation.Valid;
 import lombok.extern.slf4j.Slf4j;
+import okhttp3.OkHttpClient;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import retrofit.Endpoints;
 import retrofit.RequestInterceptor;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 import retrofit.converter.JacksonConverter;
 
 /**
@@ -124,13 +124,13 @@ public Map<String, TravisService> travisMasters(
   }
 
   public static TravisClient travisClient(String address, int timeout, ObjectMapper objectMapper) {
-    OkHttpClient client = new OkHttpClient();
-    client.setReadTimeout(timeout, TimeUnit.MILLISECONDS);
+    OkHttpClient client =
+        new OkHttpClient.Builder().readTimeout(timeout, TimeUnit.MILLISECONDS).build();
 
     return new RestAdapter.Builder()
         .setEndpoint(Endpoints.newFixedEndpoint(address))
         .setRequestInterceptor(new TravisHeader())
-        .setClient(new OkClient(client))
+        .setClient(new Ok3Client(client))
         .setConverter(new JacksonConverter(objectMapper))
         .setLog(new Slf4jRetrofitLogger(TravisClient.class))
         .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/BitBucketConfig.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/BitBucketConfig.groovy
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.igor.config
 
+import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spinnaker.igor.scm.bitbucket.client.BitBucketClient
 import com.netflix.spinnaker.igor.scm.bitbucket.client.BitBucketMaster
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
@@ -30,7 +31,6 @@ import org.springframework.context.annotation.Configuration
 import retrofit.Endpoints
 import retrofit.RequestInterceptor
 import retrofit.RestAdapter
-import retrofit.client.OkClient
 import retrofit.converter.JacksonConverter
 import javax.validation.Valid
 
@@ -56,7 +56,7 @@ class BitBucketConfig {
     new RestAdapter.Builder()
       .setEndpoint(Endpoints.newFixedEndpoint(address))
       .setRequestInterceptor(new BasicAuthRequestInterceptor(username, password))
-      .setClient(new OkClient())
+      .setClient(new Ok3Client())
       .setConverter(new JacksonConverter())
       .setLog(new Slf4jRetrofitLogger(BitBucketClient))
       .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/GitHubConfig.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/GitHubConfig.groovy
@@ -17,6 +17,7 @@
 package com.netflix.spinnaker.igor.config
 
 import com.fasterxml.jackson.databind.ObjectMapper
+import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spinnaker.igor.scm.github.client.GitHubClient
 import com.netflix.spinnaker.igor.scm.github.client.GitHubMaster
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
@@ -30,7 +31,6 @@ import org.springframework.context.annotation.Configuration
 import retrofit.Endpoints
 import retrofit.RequestInterceptor
 import retrofit.RestAdapter
-import retrofit.client.OkClient
 import retrofit.converter.JacksonConverter
 
 import javax.validation.Valid
@@ -55,7 +55,7 @@ class GitHubConfig {
         new RestAdapter.Builder()
             .setEndpoint(Endpoints.newFixedEndpoint(address))
             .setRequestInterceptor(new BasicAuthRequestInterceptor(accessToken))
-            .setClient(new OkClient())
+            .setClient(new Ok3Client())
             .setConverter(new JacksonConverter(mapper))
             .setLog(new Slf4jRetrofitLogger(GitHubClient))
             .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/GitLabConfig.java b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/GitLabConfig.java
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.igor.config;
 
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spinnaker.igor.scm.gitlab.client.GitLabClient;
 import com.netflix.spinnaker.igor.scm.gitlab.client.GitLabMaster;
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
@@ -29,7 +30,6 @@
 import retrofit.Endpoints;
 import retrofit.RequestInterceptor;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 import retrofit.converter.JacksonConverter;
 
 @Configuration
@@ -50,7 +50,7 @@ public GitLabClient gitLabClient(String address, String privateToken) {
     return new RestAdapter.Builder()
         .setEndpoint(Endpoints.newFixedEndpoint(address))
         .setRequestInterceptor(new PrivateTokenRequestInterceptor(privateToken))
-        .setClient(new OkClient())
+        .setClient(new Ok3Client())
         .setConverter(new JacksonConverter())
         .setLog(new Slf4jRetrofitLogger(GitLabClient.class))
         .build()

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/GitlabCiConfig.java b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/GitlabCiConfig.java
@@ -17,17 +17,18 @@
 package com.netflix.spinnaker.igor.config;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spinnaker.igor.IgorConfigurationProperties;
 import com.netflix.spinnaker.igor.gitlabci.client.GitlabCiClient;
 import com.netflix.spinnaker.igor.gitlabci.service.GitlabCiService;
 import com.netflix.spinnaker.igor.service.BuildServices;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
-import com.squareup.okhttp.OkHttpClient;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import okhttp3.OkHttpClient;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -38,7 +39,6 @@
 import retrofit.Endpoints;
 import retrofit.RequestInterceptor;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 import retrofit.converter.JacksonConverter;
 
 @Configuration
@@ -86,13 +86,13 @@ private static GitlabCiService gitlabCiService(
 
   public static GitlabCiClient gitlabCiClient(
       String address, String privateToken, int timeout, ObjectMapper objectMapper) {
-    OkHttpClient client = new OkHttpClient();
-    client.setReadTimeout(timeout, TimeUnit.MILLISECONDS);
+    OkHttpClient client =
+        new OkHttpClient.Builder().readTimeout(timeout, TimeUnit.MILLISECONDS).build();
 
     return new RestAdapter.Builder()
         .setEndpoint(Endpoints.newFixedEndpoint(address))
         .setRequestInterceptor(new GitlabCiHeaders(privateToken))
-        .setClient(new OkClient(client))
+        .setClient(new Ok3Client(client))
         .setLog(new Slf4jRetrofitLogger(GitlabCiClient.class))
         .setLogLevel(RestAdapter.LogLevel.FULL)
         .setConverter(new JacksonConverter(objectMapper))

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/StashConfig.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/StashConfig.groovy
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.igor.config
 
+import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spinnaker.igor.scm.stash.client.StashClient
 import com.netflix.spinnaker.igor.scm.stash.client.StashMaster
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger
@@ -29,7 +30,6 @@ import org.springframework.context.annotation.Configuration
 import retrofit.Endpoints
 import retrofit.RequestInterceptor
 import retrofit.RestAdapter
-import retrofit.client.OkClient
 import retrofit.converter.JacksonConverter
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
 
@@ -58,7 +58,7 @@ class StashConfig {
         new RestAdapter.Builder()
             .setEndpoint(Endpoints.newFixedEndpoint(address))
             .setRequestInterceptor(new BasicAuthRequestInterceptor(username, password))
-            .setClient(new OkClient())
+            .setClient(new Ok3Client())
             .setConverter(new JacksonConverter())
             .setLogLevel(retrofitLogLevel)
             .setLog(new Slf4jRetrofitLogger(StashClient))

diff --git a/...eb/src/main/groovy/com/netflix/spinnaker/igor/scm/bitbucket/client/BitBucketMaster.groovy b/...eb/src/main/groovy/com/netflix/spinnaker/igor/scm/bitbucket/client/BitBucketMaster.groovy
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.igor.scm.bitbucket.client
 
+import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spinnaker.igor.config.BitBucketProperties
 import com.netflix.spinnaker.igor.scm.AbstractScmMaster
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger
@@ -25,7 +26,6 @@ import org.springframework.context.annotation.Bean
 import retrofit.Endpoints
 import retrofit.RequestInterceptor
 import retrofit.RestAdapter
-import retrofit.client.OkClient
 import retrofit.converter.JacksonConverter
 import javax.validation.Valid
 
@@ -49,7 +49,7 @@ class BitBucketMaster extends AbstractScmMaster {
     new RestAdapter.Builder()
       .setEndpoint(Endpoints.newFixedEndpoint(address))
       .setRequestInterceptor(new BasicAuthRequestInterceptor(username, password))
-      .setClient(new OkClient())
+      .setClient(new Ok3Client())
       .setConverter(new JacksonConverter())
       .setLog(new Slf4jRetrofitLogger(BitBucketClient))
       .build()

diff --git a/igor-web/src/main/java/com/netflix/spinnaker/igor/concourse/client/ConcourseClient.java b/igor-web/src/main/java/com/netflix/spinnaker/igor/concourse/client/ConcourseClient.java
@@ -20,17 +20,17 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.PropertyNamingStrategy;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spinnaker.igor.concourse.client.model.ClusterInfo;
 import com.netflix.spinnaker.igor.concourse.client.model.Token;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
-import com.squareup.okhttp.OkHttpClient;
 import com.vdurmont.semver4j.Semver;
 import java.time.ZonedDateTime;
 import lombok.Getter;
+import okhttp3.OkHttpClient;
 import retrofit.RequestInterceptor;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 import retrofit.client.Response;
 import retrofit.converter.JacksonConverter;
 
@@ -85,13 +85,13 @@ public ConcourseClient(String host, String user, String password) {
             .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
             .registerModule(new JavaTimeModule());
 
-    this.okHttpClient = OkHttpClientBuilder.retryingClient(this::refreshToken);
+    this.okHttpClient = OkHttpClientBuilder.retryingClient3(this::refreshToken);
     this.jacksonConverter = new JacksonConverter(mapper);
 
     RestAdapter.Builder tokenRestBuilder =
         new RestAdapter.Builder()
             .setEndpoint(host)
-            .setClient(new OkClient(okHttpClient))
+            .setClient(new Ok3Client(okHttpClient))
             .setConverter(jacksonConverter)
             .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
             .setRequestInterceptor(
@@ -102,7 +102,7 @@ public ConcourseClient(String host, String user, String password) {
     this.clusterInfoService =
         new RestAdapter.Builder()
             .setEndpoint(host)
-            .setClient(new OkClient(okHttpClient))
+            .setClient(new Ok3Client(okHttpClient))
             .setConverter(jacksonConverter)
             .setLog(new Slf4jRetrofitLogger(ClusterInfoService.class))
             .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
@@ -195,7 +195,7 @@ public Response userInfo() {
   private <S> S createService(Class<S> serviceClass) {
     return new RestAdapter.Builder()
         .setEndpoint(host)
-        .setClient(new OkClient(okHttpClient))
+        .setClient(new Ok3Client(okHttpClient))
         .setConverter(jacksonConverter)
         .setRequestInterceptor(oauthInterceptor)
         .setLog(new Slf4jRetrofitLogger(serviceClass))

diff --git a/igor-web/src/main/java/com/netflix/spinnaker/igor/concourse/client/OkHttpClientBuilder.java b/igor-web/src/main/java/com/netflix/spinnaker/igor/concourse/client/OkHttpClientBuilder.java
@@ -17,9 +17,6 @@
 package com.netflix.spinnaker.igor.concourse.client;
 
 import com.netflix.spinnaker.igor.concourse.client.model.Token;
-import com.squareup.okhttp.Interceptor;
-import com.squareup.okhttp.OkHttpClient;
-import com.squareup.okhttp.Response;
 import io.github.resilience4j.retry.Retry;
 import io.github.resilience4j.retry.RetryConfig;
 import java.io.IOException;
@@ -30,12 +27,12 @@
 import java.security.SecureRandom;
 import java.security.cert.X509Certificate;
 import java.time.Duration;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Supplier;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.X509TrustManager;
+import okhttp3.OkHttpClient;
 import okio.Buffer;
 import okio.BufferedSource;
 
@@ -56,18 +53,6 @@ public X509Certificate[] getAcceptedIssuers() {
         }
       };
 
-  public static OkHttpClient retryingClient(Supplier<Token> refreshToken) {
-    OkHttpClient okHttpClient = new OkHttpClient();
-    okHttpClient
-        .interceptors()
-        .add(chain -> OkHttpClientBuilder.createRetryInterceptor(chain, refreshToken));
-    okHttpClient.setHostnameVerifier((s, sslSession) -> true);
-    okHttpClient.setSslSocketFactory(getSslContext().getSocketFactory());
-    okHttpClient.setConnectTimeout(15, TimeUnit.SECONDS);
-    okHttpClient.setReadTimeout(15, TimeUnit.SECONDS);
-    return okHttpClient;
-  }
-
   private static SSLContext getSslContext() {
     SSLContext sslContext;
     try {
@@ -79,8 +64,8 @@ private static SSLContext getSslContext() {
     return sslContext;
   }
 
-  public static okhttp3.OkHttpClient retryingClient3(Supplier<Token> refreshToken) {
-    return new okhttp3.OkHttpClient.Builder()
+  public static OkHttpClient retryingClient3(Supplier<Token> refreshToken) {
+    return new OkHttpClient.Builder()
         .addInterceptor(chain -> OkHttpClientBuilder.createRetryInterceptor3(chain, refreshToken))
         .hostnameVerifier((s, sslSession) -> true)
         .sslSocketFactory(getSslContext().getSocketFactory(), (X509TrustManager) trustAllCerts[0])
@@ -89,58 +74,6 @@ public static okhttp3.OkHttpClient retryingClient3(Supplier<Token> refreshToken)
         .build();
   }
 
-  private static Response createRetryInterceptor(
-      Interceptor.Chain chain, Supplier<Token> refreshToken) {
-    Retry retry =
-        Retry.of(
-            "concourse.api.call",
-            RetryConfig.custom().retryExceptions(RetryableApiException.class).build());
-
-    AtomicReference<Response> lastResponse = new AtomicReference<>();
-    try {
-      return retry.executeCallable(
-          () -> {
-            Response response = chain.proceed(chain.request());
-            lastResponse.set(response);
-
-            switch (response.code()) {
-              case 401:
-                String body = null;
-                if (response.body() != null) {
-                  BufferedSource source = response.body().source();
-                  source.request(Long.MAX_VALUE); // request the entire body
-                  Buffer buffer = source.buffer();
-                  body = buffer.clone().readString(Charset.forName("UTF-8"));
-                }
-                if (body == null || !body.contains("Bad credentials")) {
-                  response =
-                      chain.proceed(
-                          chain
-                              .request()
-                              .newBuilder()
-                              .header(
-                                  "Authorization", "bearer " + refreshToken.get().getAccessToken())
-                              .build());
-                  lastResponse.set(response);
-                }
-                break;
-              case 502:
-              case 503:
-              case 504:
-                // after retries fail, the response body for these status codes will get wrapped up
-                // into a ConcourseApiException
-                throw new RetryableApiException();
-            }
-
-            return response;
-          });
-    } catch (IOException e) {
-      throw new UncheckedIOException(e);
-    } catch (Exception e) {
-      return lastResponse.get();
-    }
-  }
-
   private static okhttp3.Response createRetryInterceptor3(
       okhttp3.Interceptor.Chain chain, Supplier<Token> refreshToken) {
     Retry retry =