refactor(retrofit): replace OkClient with Ok3Client

spinnaker · Nov 5, 2024 · 8ce8aa0 · 8ce8aa0
1 parent 4307029
commit 8ce8aa0
Show file tree

Hide file tree

Showing 12 changed files with 25 additions and 60 deletions.
diff --git a/orca-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/api/BakeryServiceSpec.groovy b/orca-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/api/BakeryServiceSpec.groovy
@@ -17,11 +17,11 @@
 package com.netflix.spinnaker.orca.bakery.api
 
 import com.github.tomakehurst.wiremock.WireMockServer
+import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.netflix.spinnaker.orca.bakery.config.BakeryConfiguration
 import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper
 import retrofit.RequestInterceptor
-import retrofit.client.OkClient
 import spock.lang.Specification
 import spock.lang.Subject
 import static com.github.tomakehurst.wiremock.client.WireMock.*
@@ -56,7 +56,7 @@ class BakeryServiceSpec extends Specification {
 
   def setup() {
     bakery = new BakeryConfiguration(
-      retrofitClient: new OkClient(),
+      retrofitClient: new Ok3Client(),
       retrofitLogLevel: FULL,
       spinnakerRequestInterceptor: Mock(RequestInterceptor)
     )

diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/JarDiffsTask.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/JarDiffsTask.groovy
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.orca.kato.tasks
 
+import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
 import com.netflix.spinnaker.orca.clouddriver.model.Instance.InstanceInfo
@@ -34,15 +35,14 @@ import com.netflix.spinnaker.orca.libdiffs.Library
 import com.netflix.spinnaker.orca.libdiffs.LibraryDiffTool
 import com.netflix.spinnaker.orca.libdiffs.LibraryDiffs
 import com.netflix.spinnaker.orca.retrofit.RetrofitConfiguration
-import com.squareup.okhttp.OkHttpClient
+import okhttp3.OkHttpClient
 import groovy.util.logging.Slf4j
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.context.annotation.Import
 import org.springframework.stereotype.Component
 import retrofit.RestAdapter
-import retrofit.client.OkClient
 
 @Slf4j
 @Component
@@ -118,17 +118,18 @@ class JarDiffsTask implements DiffTask {
   }
 
   InstanceService createInstanceService(String address) {
-    def okHttpClient = new OkHttpClient(retryOnConnectionFailure: false)
-
+    def okHttpClient = new OkHttpClient.Builder()
     // short circuit as quickly as possible security groups don't allow ingress to <instance>:8077
     // (spinnaker applications don't allow this)
-    okHttpClient.setConnectTimeout(2, TimeUnit.SECONDS)
-    okHttpClient.setReadTimeout(2, TimeUnit.SECONDS)
+    .connectTimeout(2, TimeUnit.SECONDS)
+    .readTimeout(2, TimeUnit.SECONDS)
+    .retryOnConnectionFailure(false)
+    .build()
 
     RestAdapter restAdapter = new RestAdapter.Builder()
       .setEndpoint(address)
       .setConverter(new JacksonConverter())
-      .setClient(new OkClient(okHttpClient))
+      .setClient(new Ok3Client(okHttpClient))
       .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
       .build()
 

diff --git a/...driver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/quip/AbstractQuipTask.groovy b/...driver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/quip/AbstractQuipTask.groovy
@@ -16,12 +16,12 @@
 
 package com.netflix.spinnaker.orca.kato.tasks.quip
 
+import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
 import com.netflix.spinnaker.orca.api.pipeline.Task
 import com.netflix.spinnaker.orca.clouddriver.InstanceService
-import com.squareup.okhttp.OkHttpClient
+import okhttp3.OkHttpClient
 import retrofit.RestAdapter
-import retrofit.client.OkClient
 import retrofit.converter.JacksonConverter
 
 import static retrofit.RestAdapter.LogLevel.BASIC
@@ -32,7 +32,7 @@ abstract class AbstractQuipTask implements Task {
     RestAdapter restAdapter = new RestAdapter.Builder()
       .setEndpoint(address)
       .setConverter(new JacksonConverter())
-      .setClient(new OkClient(new OkHttpClient(retryOnConnectionFailure: false)))
+      .setClient(new Ok3Client(new OkHttpClient(retryOnConnectionFailure: false)))
       .setLogLevel(BASIC)
       .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
       .build()

diff --git a/...ddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/KatoRestServiceSpec.groovy b/...ddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/KatoRestServiceSpec.groovy
@@ -27,7 +27,6 @@ import okhttp3.OkHttpClient
 import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.BeforeAll
 import retrofit.RequestInterceptor
-import retrofit.client.OkClient
 import spock.lang.Specification
 import spock.lang.Subject
 import static com.github.tomakehurst.wiremock.client.WireMock.*

diff --git a/...com/netflix/spinnaker/orca/clouddriver/tasks/cluster/ClusterSizePreconditionTaskTest.java b/...com/netflix/spinnaker/orca/clouddriver/tasks/cluster/ClusterSizePreconditionTaskTest.java
@@ -26,6 +26,7 @@
 import com.github.tomakehurst.wiremock.http.HttpHeaders;
 import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
 import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerConversionException;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
 import com.netflix.spinnaker.okhttp.SpinnakerRequestInterceptor;
@@ -41,7 +42,6 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.springframework.http.HttpStatus;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 import retrofit.converter.JacksonConverter;
 
 public class ClusterSizePreconditionTaskTest {
@@ -59,14 +59,13 @@ public class ClusterSizePreconditionTaskTest {
   @BeforeAll
   public static void setupOnce(WireMockRuntimeInfo wmRuntimeInfo) {
 
-    OkClient okClient = new OkClient();
     RestAdapter.LogLevel retrofitLogLevel = RestAdapter.LogLevel.NONE;
 
     oortService =
         new RestAdapter.Builder()
             .setRequestInterceptor(new SpinnakerRequestInterceptor(true))
             .setEndpoint(wmRuntimeInfo.getHttpBaseUrl())
-            .setClient(okClient)
+            .setClient(new Ok3Client())
             .setLogLevel(retrofitLogLevel)
             .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
             .setConverter(new JacksonConverter(objectMapper))

diff --git a/...ix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/EvaluateDeploymentHealthTaskTest.java b/...ix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/EvaluateDeploymentHealthTaskTest.java
@@ -33,6 +33,7 @@
 import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
 import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
 import com.google.common.collect.Iterables;
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spectator.api.NoopRegistry;
 import com.netflix.spinnaker.config.DeploymentMonitorDefinition;
 import com.netflix.spinnaker.kork.test.log.MemoryAppender;
@@ -63,7 +64,6 @@
 import org.junit.jupiter.params.provider.MethodSource;
 import org.springframework.http.HttpStatus;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 
 public class EvaluateDeploymentHealthTaskTest {
 
@@ -111,12 +111,11 @@ static void setupOnce(WireMockRuntimeInfo wmRuntimeInfo) {
   void setup(TestInfo testInfo) {
     System.out.println("--------------- Test " + testInfo.getDisplayName());
 
-    OkClient okClient = new OkClient();
     RestAdapter.LogLevel retrofitLogLevel = RestAdapter.LogLevel.NONE;
 
     DeploymentMonitorServiceProvider deploymentMonitorServiceProvider =
         new DeploymentMonitorServiceProvider(
-            okClient,
+            new Ok3Client(),
             retrofitLogLevel,
             new SpinnakerRequestInterceptor(true),
             deploymentMonitorDefinitions);

diff --git a/...com/netflix/spinnaker/orca/kato/pipeline/strategy/DetermineSourceServerGroupTaskTest.java b/...com/netflix/spinnaker/orca/kato/pipeline/strategy/DetermineSourceServerGroupTaskTest.java
@@ -27,6 +27,7 @@
 import com.github.tomakehurst.wiremock.http.HttpHeaders;
 import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
 import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spinnaker.kork.core.RetrySupport;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
@@ -51,7 +52,6 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.springframework.http.HttpStatus;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 import retrofit.converter.JacksonConverter;
 
 public class DetermineSourceServerGroupTaskTest {
@@ -82,14 +82,13 @@ private static void simulateGETApiCall(String url, String body, HttpStatus httpS
 
   @BeforeAll
   public static void setupOnce(WireMockRuntimeInfo wmRuntimeInfo) {
-    OkClient okClient = new OkClient();
     RestAdapter.LogLevel retrofitLogLevel = RestAdapter.LogLevel.NONE;
 
     oortService =
         new RestAdapter.Builder()
             .setRequestInterceptor(new SpinnakerRequestInterceptor(true))
             .setEndpoint(wmRuntimeInfo.getHttpBaseUrl())
-            .setClient(okClient)
+            .setClient(new Ok3Client())
             .setLogLevel(retrofitLogLevel)
             .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
             .setConverter(new JacksonConverter(objectMapper))

diff --git a/orca-kayenta/src/test/kotlin/com/netflix/spinnaker/orca/kayenta/KayentaServiceTest.kt b/orca-kayenta/src/test/kotlin/com/netflix/spinnaker/orca/kayenta/KayentaServiceTest.kt
@@ -28,6 +28,7 @@ import com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor
 import com.github.tomakehurst.wiremock.client.WireMock.stubFor
 import com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo
 import com.github.tomakehurst.wiremock.core.WireMockConfiguration.options
+import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spinnaker.orca.kayenta.config.KayentaConfiguration
 import com.netflix.spinnaker.time.fixedClock
 import java.time.Duration
@@ -44,7 +45,6 @@ import retrofit.Endpoint
 import retrofit.Endpoints.newFixedEndpoint
 import retrofit.RequestInterceptor
 import retrofit.RestAdapter.LogLevel
-import retrofit.client.OkClient
 
 object KayentaServiceTest : Spek({
 
@@ -55,7 +55,7 @@ object KayentaServiceTest : Spek({
     configureFor(wireMockServer.port())
     subject = KayentaConfiguration()
       .kayentaService(
-        OkClient(),
+        Ok3Client(),
         wireMockServer.endpoint,
         LogLevel.FULL,
         RequestInterceptor.NONE

diff --git a/...keel/src/test/java/com/netflix/spinnaker/orca/keel/task/ImportDeliveryConfigTaskTest.java b/...keel/src/test/java/com/netflix/spinnaker/orca/keel/task/ImportDeliveryConfigTaskTest.java
@@ -35,6 +35,7 @@
 import com.github.tomakehurst.wiremock.http.HttpHeaders;
 import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
 import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
 import com.netflix.spinnaker.okhttp.SpinnakerRequestInterceptor;
@@ -63,7 +64,6 @@
 import org.junit.jupiter.params.provider.MethodSource;
 import org.springframework.http.HttpStatus;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 import retrofit.converter.JacksonConverter;
 
 /*
@@ -88,14 +88,13 @@ public class ImportDeliveryConfigTaskTest {
 
   @BeforeAll
   static void setupOnce(WireMockRuntimeInfo wmRuntimeInfo) {
-    OkClient okClient = new OkClient();
     RestAdapter.LogLevel retrofitLogLevel = RestAdapter.LogLevel.NONE;
 
     keelService =
         new RestAdapter.Builder()
             .setRequestInterceptor(new SpinnakerRequestInterceptor(true))
             .setEndpoint(wmRuntimeInfo.getHttpBaseUrl())
-            .setClient(okClient)
+            .setClient(new Ok3Client())
             .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
             .setLogLevel(retrofitLogLevel)
             .setConverter(new JacksonConverter(objectMapper))

diff --git a/orca-mine/src/test/java/com/netflix/spinnaker/orca/mine/tasks/RegisterCanaryTaskTest.java b/orca-mine/src/test/java/com/netflix/spinnaker/orca/mine/tasks/RegisterCanaryTaskTest.java
@@ -27,6 +27,7 @@
 import com.github.tomakehurst.wiremock.http.HttpHeaders;
 import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
 import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
 import com.netflix.spinnaker.orca.mine.MineService;
@@ -47,7 +48,6 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.springframework.http.HttpStatus;
 import retrofit.RestAdapter;
-import retrofit.client.OkClient;
 import retrofit.client.Response;
 import retrofit.converter.JacksonConverter;
 import retrofit.mime.TypedString;
@@ -68,13 +68,12 @@ public class RegisterCanaryTaskTest {
 
   @BeforeAll
   static void setupOnce(WireMockRuntimeInfo wmRuntimeInfo) {
-    OkClient okClient = new OkClient();
     RestAdapter.LogLevel retrofitLogLevel = RestAdapter.LogLevel.NONE;
 
     mineService =
         new RestAdapter.Builder()
             .setEndpoint(wmRuntimeInfo.getHttpBaseUrl())
-            .setClient(okClient)
+            .setClient(new Ok3Client())
             .setLogLevel(retrofitLogLevel)
             .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
             .setLog(new RetrofitSlf4jLog(MineService.class))

diff --git a/orca-retrofit/orca-retrofit.gradle b/orca-retrofit/orca-retrofit.gradle
@@ -24,9 +24,6 @@ dependencies {
   api("com.jakewharton.retrofit:retrofit1-okhttp3-client")
 
   implementation(project(":orca-core"))
-  implementation("com.squareup.okhttp:okhttp")
-  implementation("com.squareup.okhttp:okhttp-urlconnection")
-  implementation("com.squareup.okhttp:okhttp-apache")
   implementation("io.reactivex:rxjava")
   implementation("io.spinnaker.kork:kork-retrofit")
   implementation("com.jakewharton.retrofit:retrofit1-okhttp3-client:1.1.0")

diff --git a/...retrofit/src/main/groovy/com/netflix/spinnaker/orca/retrofit/RetrofitConfiguration.groovy b/...retrofit/src/main/groovy/com/netflix/spinnaker/orca/retrofit/RetrofitConfiguration.groovy
@@ -20,13 +20,11 @@ package com.netflix.spinnaker.orca.retrofit
 import com.jakewharton.retrofit.Ok3Client
 import com.netflix.spectator.api.Registry
 import com.netflix.spinnaker.config.OkHttp3ClientConfiguration
-import com.netflix.spinnaker.config.OkHttpClientConfiguration
 import com.netflix.spinnaker.orca.retrofit.exceptions.SpinnakerServerExceptionHandler
 import groovy.transform.CompileStatic
 import okhttp3.Interceptor
 import okhttp3.OkHttpClient
 import okhttp3.Response
-import org.springframework.beans.factory.annotation.Qualifier
 import org.springframework.beans.factory.annotation.Value
 import org.springframework.beans.factory.config.ConfigurableBeanFactory
 import org.springframework.boot.context.properties.EnableConfigurationProperties
@@ -37,7 +35,6 @@ import org.springframework.context.annotation.Scope
 import org.springframework.core.Ordered
 import org.springframework.core.annotation.Order
 import retrofit.RestAdapter.LogLevel
-import retrofit.client.OkClient
 
 @Configuration
 @CompileStatic
@@ -69,30 +66,6 @@ class RetrofitConfiguration {
     new Ok3Client(builder.build())
   }
 
-  @Bean
-  @Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE)
-  OkClient okClient(Registry registry,
-                    @Qualifier("okHttpClientConfiguration") OkHttpClientConfiguration okHttpClientConfig,
-                    Optional<List<RetrofitInterceptorProvider>> retrofitInterceptorProviders) {
-    final String userAgent = "Spinnaker-${System.getProperty('spring.application.name', 'unknown')}/${getClass().getPackage().implementationVersion ?: '1.0'}"
-    def cfg = okHttpClientConfig.create()
-    cfg.networkInterceptors().add(new com.squareup.okhttp.Interceptor() {
-      @Override
-      com.squareup.okhttp.Response intercept(com.squareup.okhttp.Interceptor.Chain chain) throws IOException {
-        def req = chain.request().newBuilder().header('User-Agent', userAgent).build()
-        chain.proceed(req)
-      }
-    })
-
-    (retrofitInterceptorProviders.orElse([])).each { provider ->
-      provider.legacyInterceptors.each { interceptor ->
-        cfg.interceptors().add(interceptor)
-      }
-    }
-
-    new OkClient(cfg)
-  }
-
   @Bean
   LogLevel retrofitLogLevel(@Value('${retrofit.log-level:BASIC}') String retrofitLogLevel) {
     return LogLevel.valueOf(retrofitLogLevel)