Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor(retrofit): replace OkClient with Ok3Client #6299

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clouddriver-appengine/clouddriver-appengine.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions clouddriver-consul/clouddriver-consul.gradle
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
Expand All @@ -39,7 +38,7 @@ class Consul<T> {
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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<AgentApi> {
ConsulAgent(ConsulConfig config, String agentBaseUrl) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CatalogApi> {
ConsulCatalog(ConsulConfig config) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<KeyValueApi> {
ConsulKeyValueStore(ConsulConfig config) {
Expand Down
2 changes: 2 additions & 0 deletions clouddriver-docker/clouddriver-docker.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -55,12 +55,12 @@ final class DockerRegistryNamedAccountCredentialsTest {
@Test
void getTags() throws IOException {
ImmutableList<String> 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);
}
Expand All @@ -77,34 +77,34 @@ 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<String> tags, Map<String, Instant> creationDates) throws IOException {
OkClient okClient = mock(OkClient.class);
Ok3Client ok3Client = mock(Ok3Client.class);
doReturn(
new Response(
"https://gcr.io/v2/myrepo/tags/list",
200,
"",
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(
Expand Down Expand Up @@ -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<String> tags) {
Expand Down Expand Up @@ -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;
}
}
Expand Down
1 change: 1 addition & 0 deletions clouddriver-eureka/clouddriver-eureka.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading