From f7ade72ba0334a4058bec446cfcac6ab84b7f974 Mon Sep 17 00:00:00 2001 From: kirangodishala Date: Wed, 6 Nov 2024 00:24:59 +0530 Subject: [PATCH] refactor(retrofit): replace OkClient with Ok3Client --- .../clouddriver-appengine.gradle | 1 + .../AppengineConfigurationProperties.java | 9 +++---- clouddriver-consul/clouddriver-consul.gradle | 1 + .../clouddriver/consul/api/v1/Consul.groovy | 7 +++-- .../consul/api/v1/ConsulAgent.groovy | 5 ---- .../consul/api/v1/ConsulCatalog.groovy | 6 ----- .../consul/api/v1/ConsulKeyValueStore.groovy | 6 ----- clouddriver-docker/clouddriver-docker.gradle | 2 ++ .../client/DefaultDockerOkClientProvider.java | 18 +++++++------ .../api/v2/client/DockerOkClientProvider.java | 6 ++--- ...erRegistryNamedAccountCredentialsTest.java | 26 +++++++++---------- clouddriver-eureka/clouddriver-eureka.gradle | 1 + .../eureka/api/EurekaApiFactory.groovy | 12 ++++----- 13 files changed, 44 insertions(+), 56 deletions(-) diff --git a/clouddriver-appengine/clouddriver-appengine.gradle b/clouddriver-appengine/clouddriver-appengine.gradle index c8fc60af1d1..a54744fbf34 100644 --- a/clouddriver-appengine/clouddriver-appengine.gradle +++ b/clouddriver-appengine/clouddriver-appengine.gradle @@ -20,6 +20,7 @@ dependencies { implementation "io.spinnaker.kork:kork-cloud-config-server" implementation "io.spinnaker.kork:kork-moniker" implementation "io.spinnaker.kork:kork-retrofit" + implementation "com.jakewharton.retrofit:retrofit1-okhttp3-client" implementation "com.netflix.spectator:spectator-api" implementation "com.squareup.okhttp:okhttp" implementation "com.squareup.retrofit:converter-jackson" diff --git a/clouddriver-appengine/src/main/java/com/netflix/spinnaker/clouddriver/appengine/config/AppengineConfigurationProperties.java b/clouddriver-appengine/src/main/java/com/netflix/spinnaker/clouddriver/appengine/config/AppengineConfigurationProperties.java index 3a7971d28f1..899e97b84dd 100644 --- a/clouddriver-appengine/src/main/java/com/netflix/spinnaker/clouddriver/appengine/config/AppengineConfigurationProperties.java +++ b/clouddriver-appengine/src/main/java/com/netflix/spinnaker/clouddriver/appengine/config/AppengineConfigurationProperties.java @@ -18,18 +18,18 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.jakewharton.retrofit.Ok3Client; import com.netflix.spinnaker.clouddriver.appengine.AppengineJobExecutor; import com.netflix.spinnaker.clouddriver.googlecommon.config.GoogleCommonManagedAccount; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler; -import com.squareup.okhttp.OkHttpClient; import java.io.File; import java.util.ArrayList; import java.util.List; import lombok.Data; import lombok.EqualsAndHashCode; +import okhttp3.OkHttpClient; import org.springframework.util.StringUtils; import retrofit.RestAdapter; -import retrofit.client.OkClient; import retrofit.client.Response; import retrofit.converter.JacksonConverter; import retrofit.http.GET; @@ -99,13 +99,12 @@ public void initialize(AppengineJobExecutor jobExecutor, String gcloudPath) { } static MetadataService createMetadataService() { - OkHttpClient client = new OkHttpClient(); - client.setRetryOnConnectionFailure(true); + OkHttpClient okHttpClient = new OkHttpClient.Builder().retryOnConnectionFailure(true).build(); RestAdapter restAdapter = new RestAdapter.Builder() .setEndpoint(metadataUrl) .setConverter(new JacksonConverter()) - .setClient(new OkClient(client)) + .setClient(new Ok3Client(okHttpClient)) .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance()) .build(); return restAdapter.create(MetadataService.class); diff --git a/clouddriver-consul/clouddriver-consul.gradle b/clouddriver-consul/clouddriver-consul.gradle index 9e1635fb125..1dca4a89a4d 100644 --- a/clouddriver-consul/clouddriver-consul.gradle +++ b/clouddriver-consul/clouddriver-consul.gradle @@ -1,6 +1,7 @@ dependencies { implementation project(":clouddriver-core") + implementation "com.jakewharton.retrofit:retrofit1-okhttp3-client" implementation "com.squareup.okhttp:okhttp" implementation "com.squareup.retrofit:converter-jackson" implementation "com.squareup.retrofit:retrofit" diff --git a/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/Consul.groovy b/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/Consul.groovy index a79ea4361db..83ad486cad4 100644 --- a/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/Consul.groovy +++ b/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/Consul.groovy @@ -16,13 +16,12 @@ package com.netflix.spinnaker.clouddriver.consul.api.v1 - +import com.jakewharton.retrofit.Ok3Client import com.netflix.spinnaker.clouddriver.consul.config.ConsulConfig import com.netflix.spinnaker.clouddriver.consul.config.ConsulProperties import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler -import com.squareup.okhttp.OkHttpClient +import okhttp3.OkHttpClient import retrofit.RestAdapter -import retrofit.client.OkClient import retrofit.converter.JacksonConverter class Consul { @@ -39,7 +38,7 @@ class Consul { this.timeout = timeout this.api = new RestAdapter.Builder() .setEndpoint(this.endpoint) - .setClient(new OkClient(new OkHttpClient())) + .setClient(new Ok3Client(new OkHttpClient())) .setConverter(new JacksonConverter()) .setLogLevel(RestAdapter.LogLevel.NONE) .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance()) diff --git a/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulAgent.groovy b/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulAgent.groovy index a3480fd199a..7e9dafca61d 100644 --- a/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulAgent.groovy +++ b/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulAgent.groovy @@ -19,11 +19,6 @@ package com.netflix.spinnaker.clouddriver.consul.api.v1 import com.netflix.spinnaker.clouddriver.consul.api.v1.services.AgentApi import com.netflix.spinnaker.clouddriver.consul.config.ConsulConfig import com.netflix.spinnaker.clouddriver.consul.config.ConsulProperties -import com.squareup.okhttp.OkHttpClient -import retrofit.RestAdapter -import retrofit.client.OkClient - -import java.util.concurrent.TimeUnit class ConsulAgent extends Consul { ConsulAgent(ConsulConfig config, String agentBaseUrl) { diff --git a/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulCatalog.groovy b/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulCatalog.groovy index d50b97fc032..55154ab5c00 100644 --- a/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulCatalog.groovy +++ b/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulCatalog.groovy @@ -18,12 +18,6 @@ package com.netflix.spinnaker.clouddriver.consul.api.v1 import com.netflix.spinnaker.clouddriver.consul.api.v1.services.CatalogApi import com.netflix.spinnaker.clouddriver.consul.config.ConsulConfig -import com.netflix.spinnaker.clouddriver.consul.config.ConsulProperties -import com.squareup.okhttp.OkHttpClient -import retrofit.RestAdapter -import retrofit.client.OkClient - -import java.util.concurrent.TimeUnit class ConsulCatalog extends Consul { ConsulCatalog(ConsulConfig config) { diff --git a/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulKeyValueStore.groovy b/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulKeyValueStore.groovy index 16e06a230e3..e1606384d7e 100644 --- a/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulKeyValueStore.groovy +++ b/clouddriver-consul/src/main/groovy/com/netflix/spinnaker/clouddriver/consul/api/v1/ConsulKeyValueStore.groovy @@ -18,12 +18,6 @@ package com.netflix.spinnaker.clouddriver.consul.api.v1 import com.netflix.spinnaker.clouddriver.consul.api.v1.services.KeyValueApi import com.netflix.spinnaker.clouddriver.consul.config.ConsulConfig -import com.netflix.spinnaker.clouddriver.consul.config.ConsulProperties -import com.squareup.okhttp.OkHttpClient -import retrofit.RestAdapter -import retrofit.client.OkClient - -import java.util.concurrent.TimeUnit class ConsulKeyValueStore extends Consul { ConsulKeyValueStore(ConsulConfig config) { diff --git a/clouddriver-docker/clouddriver-docker.gradle b/clouddriver-docker/clouddriver-docker.gradle index dd38d1d9aa2..08bae9e92d8 100644 --- a/clouddriver-docker/clouddriver-docker.gradle +++ b/clouddriver-docker/clouddriver-docker.gradle @@ -9,6 +9,7 @@ dependencies { implementation "org.springframework.cloud:spring-cloud-context" implementation "org.apache.groovy:groovy" implementation "com.google.guava:guava" + implementation "com.jakewharton.retrofit:retrofit1-okhttp3-client" implementation "com.netflix.spectator:spectator-api" implementation "com.squareup.okhttp:okhttp" implementation "com.squareup.retrofit:converter-jackson" @@ -27,6 +28,7 @@ dependencies { testImplementation "org.junit.jupiter:junit-jupiter-api" testImplementation "org.junit.jupiter:junit-jupiter-params" testImplementation "org.mockito:mockito-core" + testImplementation 'org.mockito:mockito-inline' testImplementation "org.mockito:mockito-junit-jupiter" testImplementation "org.spockframework:spock-core" testImplementation "org.spockframework:spock-spring" diff --git a/clouddriver-docker/src/main/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DefaultDockerOkClientProvider.java b/clouddriver-docker/src/main/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DefaultDockerOkClientProvider.java index 35e704c747b..b32e7ad3943 100644 --- a/clouddriver-docker/src/main/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DefaultDockerOkClientProvider.java +++ b/clouddriver-docker/src/main/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DefaultDockerOkClientProvider.java @@ -15,35 +15,37 @@ */ package com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client; +import com.jakewharton.retrofit.Ok3Client; import com.netflix.spinnaker.clouddriver.docker.registry.security.TrustAllX509TrustManager; -import com.squareup.okhttp.OkHttpClient; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManager; -import retrofit.client.OkClient; +import javax.net.ssl.X509TrustManager; +import okhttp3.OkHttpClient; public class DefaultDockerOkClientProvider implements DockerOkClientProvider { @Override - public OkClient provide(String address, long timeoutMs, boolean insecure) { - OkHttpClient client = new OkHttpClient(); - client.setReadTimeout(timeoutMs, TimeUnit.MILLISECONDS); + public Ok3Client provide(String address, long timeoutMs, boolean insecure) { + OkHttpClient.Builder clientBuilder = + new OkHttpClient.Builder().readTimeout(timeoutMs, TimeUnit.MILLISECONDS); if (insecure) { SSLContext sslContext; + TrustManager[] trustManagers = {new TrustAllX509TrustManager()}; try { sslContext = SSLContext.getInstance("SSL"); - TrustManager[] trustManagers = {new TrustAllX509TrustManager()}; sslContext.init(null, trustManagers, new SecureRandom()); } catch (NoSuchAlgorithmException | KeyManagementException e) { throw new IllegalStateException("Failed configuring insecure SslSocketFactory", e); } - client.setSslSocketFactory(sslContext.getSocketFactory()); + clientBuilder.sslSocketFactory( + sslContext.getSocketFactory(), (X509TrustManager) trustManagers[0]); } - return new OkClient(client); + return new Ok3Client(clientBuilder.build()); } } diff --git a/clouddriver-docker/src/main/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerOkClientProvider.java b/clouddriver-docker/src/main/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerOkClientProvider.java index cf8b1cd0e29..d111453009b 100644 --- a/clouddriver-docker/src/main/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerOkClientProvider.java +++ b/clouddriver-docker/src/main/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerOkClientProvider.java @@ -15,7 +15,7 @@ */ package com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client; -import retrofit.client.OkClient; +import com.jakewharton.retrofit.Ok3Client; /** Allows custom configuration of the Docker Registry OkHttpClient. */ public interface DockerOkClientProvider { @@ -26,7 +26,7 @@ public interface DockerOkClientProvider { * @param timeoutMs The client timeout in milliseconds * @param insecure Whether or not the registry should be configured to trust all SSL certificates. * If this is true, you may want to fallback to {@code DefaultDockerOkClientProvider} - * @return An OkClient + * @return An Ok3Client */ - OkClient provide(String address, long timeoutMs, boolean insecure); + Ok3Client provide(String address, long timeoutMs, boolean insecure); } diff --git a/clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentialsTest.java b/clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentialsTest.java index 21ba1d79bc6..7fb04a70ff7 100644 --- a/clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentialsTest.java +++ b/clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentialsTest.java @@ -25,6 +25,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.jakewharton.retrofit.Ok3Client; import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client.DockerOkClientProvider; import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client.DockerRegistryTags; import java.io.IOException; @@ -41,7 +42,6 @@ import org.junit.jupiter.api.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import retrofit.client.OkClient; import retrofit.client.Request; import retrofit.client.Response; import retrofit.mime.TypedString; @@ -55,12 +55,12 @@ final class DockerRegistryNamedAccountCredentialsTest { @Test void getTags() throws IOException { ImmutableList tags = ImmutableList.of("latest", "other", "something"); - OkClient okClient = mockDockerOkClient(tags, ImmutableMap.of()); + Ok3Client ok3Client = mockDockerOkClient(tags, ImmutableMap.of()); DockerRegistryNamedAccountCredentials credentials = new DockerRegistryNamedAccountCredentials.Builder() .accountName(ACCOUNT_NAME) .address("https://gcr.io") - .dockerOkClientProvider(new MockDockerOkClientProvider(okClient)) + .dockerOkClientProvider(new MockDockerOkClientProvider(ok3Client)) .build(); assertThat(credentials.getTags(REPO_NAME)).containsExactlyInAnyOrderElementsOf(tags); } @@ -77,26 +77,26 @@ void getTagsInOrder() throws IOException { "oldest", LATEST_DATE.minus(Duration.ofDays(1))); - OkClient okClient = mockDockerOkClient(tags, creationDates); + Ok3Client ok3Client = mockDockerOkClient(tags, creationDates); DockerRegistryNamedAccountCredentials credentials = new DockerRegistryNamedAccountCredentials.Builder() .accountName(ACCOUNT_NAME) .address("https://gcr.io") .sortTagsByDate(true) - .dockerOkClientProvider(new MockDockerOkClientProvider(okClient)) + .dockerOkClientProvider(new MockDockerOkClientProvider(ok3Client)) .build(); assertThat(credentials.getTags(REPO_NAME)) .containsExactly("latest", "older", "oldest", "nodate"); } /** - * Generates a mock OkClient that simulates responses from a docker registry with the supplied + * Generates a mock Ok3Client that simulates responses from a docker registry with the supplied * tags and supplied creation dates for each tag. Tags that are not present in the map of creation * dates will return null as their creation date. */ - private static OkClient mockDockerOkClient( + private static Ok3Client mockDockerOkClient( Iterable tags, Map creationDates) throws IOException { - OkClient okClient = mock(OkClient.class); + Ok3Client ok3Client = mock(Ok3Client.class); doReturn( new Response( "https://gcr.io/v2/myrepo/tags/list", @@ -104,7 +104,7 @@ private static OkClient mockDockerOkClient( "", Collections.emptyList(), new TypedString(objectMapper.writeValueAsString(getTagsResponse(tags))))) - .when(okClient) + .when(ok3Client) .execute(argThat(r -> r.getUrl().equals("https://gcr.io/v2/myrepo/tags/list"))); doAnswer( @@ -134,10 +134,10 @@ private String getTag(String url) { throw new IllegalArgumentException(); } }) - .when(okClient) + .when(ok3Client) .execute(argThat(r -> r.getUrl().matches("https://gcr.io/v2/myrepo/manifests/.*"))); - return okClient; + return ok3Client; } private static DockerRegistryTags getTagsResponse(Iterable tags) { @@ -177,10 +177,10 @@ static HistoryEntry withCreationDate(Instant instant) throws IOException { @RequiredArgsConstructor private static class MockDockerOkClientProvider implements DockerOkClientProvider { - private final OkClient mockClient; + private final Ok3Client mockClient; @Override - public OkClient provide(String address, long timeoutMs, boolean insecure) { + public Ok3Client provide(String address, long timeoutMs, boolean insecure) { return mockClient; } } diff --git a/clouddriver-eureka/clouddriver-eureka.gradle b/clouddriver-eureka/clouddriver-eureka.gradle index 0c1e25c04a8..f76c3572633 100644 --- a/clouddriver-eureka/clouddriver-eureka.gradle +++ b/clouddriver-eureka/clouddriver-eureka.gradle @@ -7,6 +7,7 @@ dependencies { implementation "io.spinnaker.kork:kork-web" implementation "io.spinnaker.kork:kork-retrofit" implementation "com.amazonaws:aws-java-sdk" + implementation "com.jakewharton.retrofit:retrofit1-okhttp3-client" implementation "com.squareup.retrofit:converter-jackson" implementation "com.squareup.retrofit:retrofit" implementation "org.apache.groovy:groovy" diff --git a/clouddriver-eureka/src/main/groovy/com/netflix/spinnaker/clouddriver/eureka/api/EurekaApiFactory.groovy b/clouddriver-eureka/src/main/groovy/com/netflix/spinnaker/clouddriver/eureka/api/EurekaApiFactory.groovy index 5f60ee842a5..e2a9027354c 100644 --- a/clouddriver-eureka/src/main/groovy/com/netflix/spinnaker/clouddriver/eureka/api/EurekaApiFactory.groovy +++ b/clouddriver-eureka/src/main/groovy/com/netflix/spinnaker/clouddriver/eureka/api/EurekaApiFactory.groovy @@ -16,26 +16,26 @@ package com.netflix.spinnaker.clouddriver.eureka.api -import com.netflix.spinnaker.config.OkHttpClientConfiguration +import com.jakewharton.retrofit.Ok3Client +import com.netflix.spinnaker.config.OkHttp3ClientConfiguration import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler import retrofit.RestAdapter -import retrofit.client.OkClient import retrofit.converter.Converter class EurekaApiFactory { private Converter eurekaConverter - private OkHttpClientConfiguration okHttpClientConfiguration + private OkHttp3ClientConfiguration okHttp3ClientConfiguration - EurekaApiFactory(Converter eurekaConverter, OkHttpClientConfiguration okHttpClientConfiguration) { + EurekaApiFactory(Converter eurekaConverter, OkHttp3ClientConfiguration okHttp3ClientConfiguration) { this.eurekaConverter = eurekaConverter - this.okHttpClientConfiguration = okHttpClientConfiguration + this.okHttp3ClientConfiguration = okHttp3ClientConfiguration } public EurekaApi createApi(String endpoint) { new RestAdapter.Builder() .setConverter(eurekaConverter) - .setClient(new OkClient(okHttpClientConfiguration.create())) + .setClient(new Ok3Client(okHttp3ClientConfiguration.create().build())) .setEndpoint(endpoint) .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance()) .build()