From 92740ed1752a67c4099ccb2f64d46a982246db6b Mon Sep 17 00:00:00 2001 From: Sairam Rekapalli Date: Fri, 11 Sep 2020 17:01:44 -0700 Subject: [PATCH] fix(misc): Remove OkHttp2 usage and use provider impls --- echo-rest/echo-rest.gradle | 4 +- .../spinnaker/echo/config/RestConfig.java | 15 ++++- .../echo/rest/OkHttpClientFactory.java | 23 ------- .../echo/rest/OkHttpClientFactoryImpl.java | 66 ------------------- .../echo/rest/RestClientFactory.java | 44 ------------- .../echo/config/RestConfigSpec.groovy | 6 +- .../events/OkHttpClientFactorySpec.groovy | 24 ------- .../echo/events/RestClientFactorySpec.groovy | 44 ------------- 8 files changed, 17 insertions(+), 209 deletions(-) delete mode 100644 echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/OkHttpClientFactory.java delete mode 100644 echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/OkHttpClientFactoryImpl.java delete mode 100644 echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/RestClientFactory.java delete mode 100644 echo-rest/src/test/groovy/com/netflix/spinnaker/echo/events/OkHttpClientFactorySpec.groovy delete mode 100644 echo-rest/src/test/groovy/com/netflix/spinnaker/echo/events/RestClientFactorySpec.groovy diff --git a/echo-rest/echo-rest.gradle b/echo-rest/echo-rest.gradle index 063f2e38e..54d5c9830 100644 --- a/echo-rest/echo-rest.gradle +++ b/echo-rest/echo-rest.gradle @@ -17,9 +17,7 @@ dependencies { implementation "com.squareup.retrofit:retrofit" implementation "com.squareup.retrofit:converter-jackson" - implementation "com.squareup.okhttp:okhttp" - implementation "com.squareup.okhttp:okhttp-urlconnection" - implementation "com.squareup.okhttp:okhttp-apache" + implementation "com.jakewharton.retrofit:retrofit1-okhttp3-client" implementation project(':echo-model') implementation project(':echo-api') implementation project(':echo-core') diff --git a/echo-rest/src/main/java/com/netflix/spinnaker/echo/config/RestConfig.java b/echo-rest/src/main/java/com/netflix/spinnaker/echo/config/RestConfig.java index 1e7aebd46..6933af95b 100644 --- a/echo-rest/src/main/java/com/netflix/spinnaker/echo/config/RestConfig.java +++ b/echo-rest/src/main/java/com/netflix/spinnaker/echo/config/RestConfig.java @@ -18,7 +18,9 @@ import static retrofit.Endpoints.newFixedEndpoint; -import com.netflix.spinnaker.echo.rest.RestClientFactory; +import com.jakewharton.retrofit.Ok3Client; +import com.netflix.spinnaker.config.DefaultServiceEndpoint; +import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider; import com.netflix.spinnaker.echo.rest.RestService; import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger; import java.io.BufferedReader; @@ -26,6 +28,7 @@ import java.io.FileReader; import java.util.HashMap; import java.util.Map; +import okhttp3.OkHttpClient; import org.apache.commons.codec.binary.Base64; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -89,9 +92,9 @@ HeadersFromFile headersFromFile() { @Bean RestUrls restServices( RestProperties restProperties, - RestClientFactory clientFactory, RestAdapter.LogLevel retrofitLogLevel, RequestInterceptorAttacher requestInterceptorAttacher, + OkHttpClientProvider okHttpClientProvider, HeadersFromFile headersFromFile) { RestUrls restUrls = new RestUrls(); @@ -100,7 +103,13 @@ RestUrls restServices( RestAdapter.Builder restAdapterBuilder = new RestAdapter.Builder() .setEndpoint(newFixedEndpoint(endpoint.getUrl())) - .setClient(clientFactory.getClient(endpoint.insecure)) + .setClient( + endpoint.insecure + ? new Ok3Client( + okHttpClientProvider.getClient( + new DefaultServiceEndpoint( + endpoint.getEventName(), endpoint.getUrl(), false))) + : new Ok3Client(new OkHttpClient())) .setLogLevel(retrofitLogLevel) .setLog(new Slf4jRetrofitLogger(RestService.class)) .setConverter(new JacksonConverter()); diff --git a/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/OkHttpClientFactory.java b/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/OkHttpClientFactory.java deleted file mode 100644 index bfb2a20bc..000000000 --- a/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/OkHttpClientFactory.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2019 Netflix, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.netflix.spinnaker.echo.rest; - -import com.squareup.okhttp.OkHttpClient; - -public interface OkHttpClientFactory { - OkHttpClient getInsecureClient(); -} diff --git a/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/OkHttpClientFactoryImpl.java b/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/OkHttpClientFactoryImpl.java deleted file mode 100644 index 5107c8362..000000000 --- a/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/OkHttpClientFactoryImpl.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright 2019 Netflix, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.netflix.spinnaker.echo.rest; - -import com.squareup.okhttp.OkHttpClient; -import javax.net.ssl.SSLContext; -import javax.net.ssl.TrustManager; -import javax.net.ssl.X509TrustManager; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.stereotype.Component; - -@Component -public class OkHttpClientFactoryImpl implements OkHttpClientFactory { - - private static final Logger log = LoggerFactory.getLogger(OkHttpClientFactoryImpl.class); - - public OkHttpClient getInsecureClient() { - OkHttpClient okHttpClient = new OkHttpClient(); - - try { - // Create a trust manager that does not validate certificate chains - final TrustManager[] trustAllCerts = - new TrustManager[] { - new X509TrustManager() { - @Override - public void checkClientTrusted( - java.security.cert.X509Certificate[] chain, String authType) {} - - @Override - public void checkServerTrusted( - java.security.cert.X509Certificate[] chain, String authType) {} - - @Override - public java.security.cert.X509Certificate[] getAcceptedIssuers() { - return new java.security.cert.X509Certificate[] {}; - } - } - }; - - final SSLContext sslContext = SSLContext.getInstance("SSL"); - sslContext.init(null, trustAllCerts, null); - - okHttpClient.setSslSocketFactory(sslContext.getSocketFactory()); - okHttpClient.setHostnameVerifier((hostname, session) -> true); - } catch (Exception e) { - log.error("Error creating insecure trust manager", e); - } - - return okHttpClient; - } -} diff --git a/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/RestClientFactory.java b/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/RestClientFactory.java deleted file mode 100644 index 5bcc86d15..000000000 --- a/echo-rest/src/main/java/com/netflix/spinnaker/echo/rest/RestClientFactory.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2019 Netflix, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.netflix.spinnaker.echo.rest; - -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.stereotype.Component; -import retrofit.client.Client; -import retrofit.client.OkClient; - -@Component -public class RestClientFactory { - - @Autowired private OkHttpClientFactory httpClientFactory; - - public OkHttpClientFactory getHttpClientFactory() { - return httpClientFactory; - } - - public void setHttpClientFactory(OkHttpClientFactory httpClientFactory) { - this.httpClientFactory = httpClientFactory; - } - - public Client getClient(Boolean insecure) { - if (insecure) { - return new OkClient(httpClientFactory.getInsecureClient()); - } else { - return new OkClient(); - } - } -} diff --git a/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/config/RestConfigSpec.groovy b/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/config/RestConfigSpec.groovy index 064847399..7cb365763 100644 --- a/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/config/RestConfigSpec.groovy +++ b/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/config/RestConfigSpec.groovy @@ -1,6 +1,8 @@ package com.netflix.spinnaker.echo.config -import com.netflix.spinnaker.echo.rest.RestClientFactory +import com.netflix.spinnaker.config.okhttp3.InsecureOkHttpClientBuilderProvider +import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider +import okhttp3.OkHttpClient import retrofit.RequestInterceptor import retrofit.RestAdapter import spock.lang.Specification @@ -23,7 +25,7 @@ class RestConfigSpec extends Specification { void configureRestServices(RestProperties.RestEndpointConfiguration endpoint, RestConfig.HeadersFromFile headersFromFile) { RestProperties restProperties = new RestProperties(endpoints: [endpoint]) - config.restServices(restProperties, new RestClientFactory(), config.retrofitLogLevel("BASIC"), attacher, headersFromFile) + config.restServices(restProperties, config.retrofitLogLevel("BASIC"), attacher, new OkHttpClientProvider([new InsecureOkHttpClientBuilderProvider(new OkHttpClient())]), headersFromFile) } void "Generate basic auth header"() { diff --git a/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/events/OkHttpClientFactorySpec.groovy b/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/events/OkHttpClientFactorySpec.groovy deleted file mode 100644 index 9d7957c47..000000000 --- a/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/events/OkHttpClientFactorySpec.groovy +++ /dev/null @@ -1,24 +0,0 @@ -package com.netflix.spinnaker.echo.events - -import com.netflix.spinnaker.echo.rest.OkHttpClientFactory -import com.netflix.spinnaker.echo.rest.OkHttpClientFactoryImpl -import com.squareup.okhttp.OkHttpClient -import spock.lang.Specification -import spock.lang.Subject - -class OkHttpClientFactorySpec extends Specification { - - @Subject - OkHttpClientFactory clientFactory = new OkHttpClientFactoryImpl() - OkHttpClient insecureClient - - void 'insecure client does not verify hostname'() { - - when: - insecureClient = clientFactory.getInsecureClient() - - then: - insecureClient.getHostnameVerifier().verify("mockdomain", null) - - } -} diff --git a/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/events/RestClientFactorySpec.groovy b/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/events/RestClientFactorySpec.groovy deleted file mode 100644 index f1684ee27..000000000 --- a/echo-rest/src/test/groovy/com/netflix/spinnaker/echo/events/RestClientFactorySpec.groovy +++ /dev/null @@ -1,44 +0,0 @@ -package com.netflix.spinnaker.echo.events - -import com.netflix.spinnaker.echo.rest.OkHttpClientFactory -import com.netflix.spinnaker.echo.rest.RestClientFactory -import com.squareup.okhttp.OkHttpClient -import spock.lang.Specification -import spock.lang.Subject - -class RestClientFactorySpec extends Specification{ - - @Subject - RestClientFactory clientFactory = new RestClientFactory() - - Boolean insecure - - OkHttpClientFactory httpClientFactory - - void setup() { - httpClientFactory = Mock(OkHttpClientFactory) - clientFactory.httpClientFactory = httpClientFactory - } - - void 'returns insecure client'() { - given: - insecure = true - - when: - clientFactory.getClient(insecure) - - then: - 1 * httpClientFactory.getInsecureClient() >> new OkHttpClient() - } - - void 'returns secure client'() { - given: - insecure = false - - when: - clientFactory.getClient(insecure) - - then: - 0 * httpClientFactory.getInsecureClient() >> new OkHttpClient() - } -}