fix(misc): Move to new client provider for building retrofit client (#…

…1223)

* fix(misc): Move to new client provider for building retrofit client

* fix(misc): fix format

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

gate-integrations-gremlin/gate-integrations-gremlin.gradle

@@ -1,6 +1,7 @@
 dependencies {
   implementation project(":gate-core")
   implementation "com.netflix.spinnaker.kork:kork-swagger"
+  implementation "com.netflix.spinnaker.kork:kork-web"
   implementation "com.netflix.spectator:spectator-api"
   implementation "com.squareup.okhttp3:okhttp"
   implementation "com.squareup.retrofit:retrofit"

gate-integrations-gremlin/src/main/java/com/netflix/spinnaker/config/GremlinConfig.java

@@ -22,14 +22,14 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.jakewharton.retrofit.Ok3Client;
 import com.netflix.spectator.api.Registry;
+import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider;
 import com.netflix.spinnaker.gate.config.Service;
 import com.netflix.spinnaker.gate.config.ServiceConfiguration;
 import com.netflix.spinnaker.gate.retrofit.Slf4jRetrofitLogger;
 import com.netflix.spinnaker.gate.services.EurekaLookupService;
 import com.netflix.spinnaker.gate.services.gremlin.GremlinService;
 import groovy.transform.CompileStatic;
 import groovy.util.logging.Slf4j;
-import okhttp3.OkHttpClient;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
@@ -46,7 +46,7 @@
 class GremlinConfig {
   @Bean
   GremlinService gremlinService(
-      OkHttpClient okHttpClient,
+      OkHttpClientProvider clientProvider,
       ServiceConfiguration serviceConfiguration,
       Registry registry,
       EurekaLookupService eurekaLookupService,
@@ -55,7 +55,7 @@ GremlinService gremlinService(
     return createClient(
         "gremlin",
         GremlinService.class,
-        okHttpClient,
+        clientProvider,
         serviceConfiguration,
         registry,
         eurekaLookupService,
@@ -66,7 +66,7 @@ GremlinService gremlinService(
   private <T> T createClient(
       String serviceName,
       Class<T> type,
-      OkHttpClient okHttpClient,
+      OkHttpClientProvider clientProvider,
       ServiceConfiguration serviceConfiguration,
       Registry registry,
       EurekaLookupService eurekaLookupService,
@@ -84,11 +84,11 @@ private <T> T createClient(
 
     Endpoint endpoint = newFixedEndpoint(service.getBaseUrl());
     return buildService(
-        okHttpClient, type, endpoint, spinnakerRequestInterceptor, retrofitLogLevel);
+        clientProvider, type, endpoint, spinnakerRequestInterceptor, retrofitLogLevel);
   }
 
   private <T> T buildService(
-      OkHttpClient client,
+      OkHttpClientProvider clientProvider,
       Class<T> type,
       Endpoint endpoint,
       RequestInterceptor spinnakerRequestInterceptor,
@@ -101,7 +101,9 @@ private <T> T buildService(
     return new RestAdapter.Builder()
         .setRequestInterceptor(spinnakerRequestInterceptor)
         .setEndpoint(endpoint)
-        .setClient(new Ok3Client(client))
+        .setClient(
+            new Ok3Client(
+                clientProvider.getClient(new DefaultServiceEndpoint("gremlin", endpoint.getUrl()))))
         .setConverter(new JacksonConverter(objectMapper))
         .setLogLevel(RestAdapter.LogLevel.valueOf(retrofitLogLevel))
         .setLog(new Slf4jRetrofitLogger(type))

gate-plugins-test/src/test/resources/gate-plugins-test.yml

@@ -11,6 +11,8 @@ services:
 
   fiat.enabled: false
 
+  fiat.baseUrl: "http://localhost:8080"
+
   front50.baseUrl: "http://localhost:8080"
 
   igor.enabled: false
@@ -25,6 +27,9 @@ services:
 
   swabbie.enabled: false
 
+slack:
+  baseUrl: "https://slack.com"
+
 spinnaker:
   extensibility:
     plugins-root-path: build/plugins

gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/GateConfig.groovy

@@ -23,8 +23,10 @@ import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule
 import com.jakewharton.retrofit.Ok3Client
 import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext
 import com.netflix.spectator.api.Registry
+import com.netflix.spinnaker.config.DefaultServiceEndpoint
 import com.netflix.spinnaker.config.OkHttp3ClientConfiguration
 import com.netflix.spinnaker.config.PluginsAutoConfiguration
+import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider
 import com.netflix.spinnaker.fiat.shared.FiatClientConfigurationProperties
 import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator
 import com.netflix.spinnaker.fiat.shared.FiatService
@@ -49,7 +51,6 @@ import com.netflix.spinnaker.okhttp.OkHttp3MetricsInterceptor
 import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
-import okhttp3.OkHttpClient
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.beans.factory.annotation.Qualifier
 import org.springframework.beans.factory.annotation.Value
@@ -172,50 +173,50 @@ class GateConfig extends RedisHttpSessionConfiguration {
   }
 
   @Bean
-  OrcaServiceSelector orcaServiceSelector(OkHttpClient okHttpClient, RequestContextProvider contextProvider) {
-    return new OrcaServiceSelector(createClientSelector("orca", OrcaService, okHttpClient), contextProvider)
+  OrcaServiceSelector orcaServiceSelector(OkHttpClientProvider clientProvider, RequestContextProvider contextProvider) {
+    return new OrcaServiceSelector(createClientSelector("orca", OrcaService, clientProvider), contextProvider)
   }
 
   @Bean
   @Primary
-  FiatService fiatService(OkHttpClient okHttpClient) {
+  FiatService fiatService(OkHttpClientProvider clientProvider) {
     // always create the fiat service even if 'services.fiat.enabled' is 'false' (it can be enabled dynamically)
-    createClient "fiat", FiatService, okHttpClient, null, true
+    createClient "fiat", FiatService, clientProvider, null, true
   }
 
   @Bean
-  ExtendedFiatService extendedFiatService(OkHttpClient okHttpClient) {
+  ExtendedFiatService extendedFiatService(OkHttpClientProvider clientProvider) {
     // always create the fiat service even if 'services.fiat.enabled' is 'false' (it can be enabled dynamically)
-    createClient "fiat", ExtendedFiatService, okHttpClient, null, true
+    createClient "fiat", ExtendedFiatService, clientProvider, null, true
   }
 
   @Bean
   @ConditionalOnProperty("services.fiat.config.dynamic-endpoints.login")
-  FiatService fiatLoginService(OkHttpClient okHttpClient) {
+  FiatService fiatLoginService(OkHttpClientProvider clientProvider) {
     // always create the fiat service even if 'services.fiat.enabled' is 'false' (it can be enabled dynamically)
-    createClient "fiat", FiatService, okHttpClient, "login", true
+    createClient "fiat", FiatService, clientProvider, "login", true
   }
 
 
   @Bean
-  Front50Service front50Service(OkHttpClient okHttpClient) {
-    createClient "front50", Front50Service, okHttpClient
+  Front50Service front50Service(OkHttpClientProvider clientProvider) {
+    createClient "front50", Front50Service, clientProvider
   }
 
   @Bean
-  ClouddriverService clouddriverService(OkHttpClient okHttpClient) {
-    createClient "clouddriver", ClouddriverService, okHttpClient
+  ClouddriverService clouddriverService(OkHttpClientProvider clientProvider) {
+    createClient "clouddriver", ClouddriverService, clientProvider
   }
 
   @Bean
   @ConditionalOnProperty("services.keel.enabled")
-  KeelService keelService(OkHttpClient okHttpClient) {
-    createClient "keel", KeelService, okHttpClient
+  KeelService keelService(OkHttpClientProvider clientProvider) {
+    createClient "keel", KeelService, clientProvider
   }
 
   @Bean
   ClouddriverServiceSelector clouddriverServiceSelector(ClouddriverService defaultClouddriverService,
-                                                        OkHttpClient okHttpClient,
+                                                        OkHttpClientProvider clientProvider,
                                                         DynamicConfigService dynamicConfigService,
                                                         DynamicRoutingConfigProperties properties,
                                                         RequestContextProvider contextProvider
@@ -240,56 +241,56 @@ class GateConfig extends RedisHttpSessionConfiguration {
 
       List<ServiceSelector> selectors = []
       endpoints.each { sourceApp, url ->
-        def service = buildService(okHttpClient, ClouddriverService, newFixedEndpoint(url))
+        def service = buildService("clouddriver", clientProvider, ClouddriverService, newFixedEndpoint(url))
         selectors << new ByUserOriginSelector(service, 2, ['origin': (Object) sourceApp])
       }
 
       return new ClouddriverServiceSelector(
         new SelectableService(selectors + defaultSelector), dynamicConfigService, contextProvider)
     }
 
-    SelectableService selectableService = createClientSelector("clouddriver", ClouddriverService, okHttpClient)
+    SelectableService selectableService = createClientSelector("clouddriver", ClouddriverService, clientProvider)
     return new ClouddriverServiceSelector(selectableService, dynamicConfigService, contextProvider)
   }
 
   //---- semi-optional components:
   @Bean
   @ConditionalOnProperty('services.rosco.enabled')
-  RoscoService roscoService(OkHttpClient okHttpClient) {
-    createClient "rosco", RoscoService, okHttpClient
+  RoscoService roscoService(OkHttpClientProvider clientProvider) {
+    createClient "rosco", RoscoService, clientProvider
   }
 
   @Bean
   @ConditionalOnProperty('services.rosco.enabled')
-  RoscoServiceSelector roscoServiceSelector(OkHttpClient okHttpClient, RoscoService defaultService) {
+  RoscoServiceSelector roscoServiceSelector(OkHttpClientProvider clientProvider, RoscoService defaultService) {
     return new RoscoServiceSelector(
-      createClientSelector("rosco", RoscoService, okHttpClient),
+      createClientSelector("rosco", RoscoService, clientProvider),
       defaultService
     )
   }
 
   //---- optional backend components:
   @Bean
   @ConditionalOnProperty('services.echo.enabled')
-  EchoService echoService(OkHttpClient okHttpClient) {
-    createClient "echo", EchoService, okHttpClient
+  EchoService echoService(OkHttpClientProvider clientProvider) {
+    createClient "echo", EchoService, clientProvider
   }
 
   @Bean
   @ConditionalOnProperty('services.igor.enabled')
-  IgorService igorService(OkHttpClient okHttpClient) {
-    createClient "igor", IgorService, okHttpClient
+  IgorService igorService(OkHttpClientProvider clientProvider) {
+    createClient "igor", IgorService, clientProvider
   }
 
   @Bean
   @ConditionalOnProperty('services.mine.enabled')
-  MineService mineService(OkHttpClient okHttpClient) {
-    createClient "mine", MineService, okHttpClient
+  MineService mineService(OkHttpClientProvider clientProvider) {
+    createClient "mine", MineService, clientProvider
   }
 
   @Bean
   @ConditionalOnProperty('services.kayenta.enabled')
-  KayentaService kayentaService(OkHttpClient defaultClient,
+  KayentaService kayentaService(OkHttpClientProvider clientProvider,
                                 OkHttpClientConfigurationProperties props,
                                 OkHttp3MetricsInterceptor interceptor,
                                 @Value('${services.kayenta.externalhttps:false}') boolean kayentaExternalHttps) {
@@ -298,21 +299,21 @@ class GateConfig extends RedisHttpSessionConfiguration {
       noSslCustomizationProps.keyStore = null
       noSslCustomizationProps.trustStore = null
       def okHttpClient = new OkHttp3ClientConfiguration(noSslCustomizationProps, interceptor).create().build()
-      createClient "kayenta", KayentaService, okHttpClient
+      createClient "kayenta", KayentaService, clientProvider
     } else {
-      createClient "kayenta", KayentaService, defaultClient
+      createClient "kayenta", KayentaService, clientProvider
     }
   }
 
   @Bean
   @ConditionalOnProperty('services.swabbie.enabled')
-  SwabbieService swabbieService(OkHttpClient okHttpClient) {
-    createClient("swabbie", SwabbieService, okHttpClient)
+  SwabbieService swabbieService(OkHttpClientProvider clientProvider) {
+    createClient("swabbie", SwabbieService, clientProvider)
   }
 
   private <T> T createClient(String serviceName,
                              Class<T> type,
-                             OkHttpClient okHttpClient,
+                             OkHttpClientProvider clientProvider,
                              String dynamicName = null,
                              boolean forceEnabled = false) {
     Service service = serviceConfiguration.getService(serviceName)
@@ -326,10 +327,10 @@ class GateConfig extends RedisHttpSessionConfiguration {
 
     Endpoint endpoint = serviceConfiguration.getServiceEndpoint(serviceName, dynamicName)
 
-    buildService(okHttpClient, type, endpoint)
+    buildService(serviceName, clientProvider, type, endpoint)
   }
 
-  private <T> T buildService(OkHttpClient client, Class<T> type, Endpoint endpoint) {
+  private <T> T buildService(String serviceName, OkHttpClientProvider clientProvider, Class<T> type, Endpoint endpoint) {
     // New role providers break deserialization if this is not enabled.
     ObjectMapper objectMapper = new ObjectMapper()
       .enable(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL)
@@ -339,15 +340,15 @@ class GateConfig extends RedisHttpSessionConfiguration {
     new RestAdapter.Builder()
       .setRequestInterceptor(spinnakerRequestInterceptor)
       .setEndpoint(endpoint)
-      .setClient(new Ok3Client(client))
+      .setClient(new Ok3Client(clientProvider.getClient(new DefaultServiceEndpoint(serviceName, endpoint.url))))
       .setConverter(new JacksonConverter(objectMapper))
       .setLogLevel(RestAdapter.LogLevel.valueOf(retrofitLogLevel))
       .setLog(new Slf4jRetrofitLogger(type))
       .build()
       .create(type)
   }
 
-  private <T> SelectableService createClientSelector(String serviceName, Class<T> type, OkHttpClient okHttpClient) {
+  private <T> SelectableService createClientSelector(String serviceName, Class<T> type, OkHttpClientProvider clientProvider) {
     Service service = serviceConfiguration.getService(serviceName)
     if (CollectionUtils.isEmpty(service?.getBaseUrls())) {
       throw new IllegalArgumentException("Unknown service ${serviceName} requested of type ${type}")
@@ -357,7 +358,8 @@ class GateConfig extends RedisHttpSessionConfiguration {
       service.getBaseUrls().collect {
         def selector = new DefaultServiceSelector(
           buildService(
-            okHttpClient,
+            serviceName,
+            clientProvider,
             type,
             newFixedEndpoint(it.baseUrl)),
           it.priority,

gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/PagerDutyConfig.groovy

@@ -16,6 +16,9 @@
 
 package com.netflix.spinnaker.gate.config
 
+import com.jakewharton.retrofit.Ok3Client
+import com.netflix.spinnaker.config.DefaultServiceEndpoint
+import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider
 import com.netflix.spinnaker.gate.services.PagerDutyService
 import groovy.transform.CompileStatic
 import org.springframework.beans.factory.annotation.Value
@@ -25,7 +28,6 @@ import org.springframework.context.annotation.Configuration
 import retrofit.Endpoint
 import retrofit.RequestInterceptor
 import retrofit.RestAdapter
-import retrofit.client.OkClient
 import retrofit.converter.JacksonConverter
 
 import static retrofit.Endpoints.newFixedEndpoint
@@ -53,10 +55,10 @@ class PagerDutyConfig {
   }
 
   @Bean
-  PagerDutyService pagerDutyService(Endpoint pagerDutyEndpoint) {
+  PagerDutyService pagerDutyService(Endpoint pagerDutyEndpoint, OkHttpClientProvider clientProvider) {
     new RestAdapter.Builder()
       .setEndpoint(pagerDutyEndpoint)
-      .setClient(new OkClient())
+      .setClient(new Ok3Client(clientProvider.getClient(new DefaultServiceEndpoint("pagerduty", pagerDutyEndpoint.url))))
       .setConverter(new JacksonConverter())
       .setRequestInterceptor(requestInterceptor)
       .build()

gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/SlackConfig.java

@@ -19,6 +19,8 @@
 import static retrofit.Endpoints.newFixedEndpoint;
 
 import com.jakewharton.retrofit.Ok3Client;
+import com.netflix.spinnaker.config.DefaultServiceEndpoint;
+import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider;
 import com.netflix.spinnaker.gate.services.SlackService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
@@ -55,10 +57,13 @@ public void intercept(RequestInterceptor.RequestFacade request) {
       };
 
   @Bean
-  SlackService slackService(Endpoint slackEndpoint) {
+  SlackService slackService(Endpoint slackEndpoint, OkHttpClientProvider clientProvider) {
     return new RestAdapter.Builder()
         .setEndpoint(slackEndpoint)
-        .setClient(new Ok3Client())
+        .setClient(
+            new Ok3Client(
+                clientProvider.getClient(
+                    new DefaultServiceEndpoint("slack", slackEndpoint.getUrl()))))
         .setConverter(new JacksonConverter())
         .setRequestInterceptor(requestInterceptor)
         .build()

gate-web/src/test/groovy/com/netflix/spinnaker/gate/security/ldap/LdapAuthSpec.groovy

@@ -48,7 +48,7 @@ import static org.springframework.security.test.web.servlet.request.SecurityMock
 @Slf4j
 @GateSystemTest
 @SpringBootTest(
-    properties = ['ldap.enabled=true', 'spring.application.name=gate', 'fiat.enabled=false'])
+    properties = ['ldap.enabled=true', 'spring.application.name=gate', 'fiat.enabled=false', 'services.fiat.baseUrl=https://localhost'])
 @ContextConfiguration(
   classes = [LdapSsoConfig, Main, LdapTestConfig, RedisTestConfig],
   initializers = YamlFileApplicationContextInitializer

gate-web/src/test/groovy/com/netflix/spinnaker/gate/swagger/GenerateSwaggerSpec.groovy

@@ -28,7 +28,7 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder
 @TestPropertySource(
   // Enable Controllers we want to document in the spec here.
   properties = [ "services.kayenta.enabled=true","services.kayenta.canary-config-store=true",
-    "services.keel.enabled=true", "spring.application.name=gate" ])
+    "services.keel.enabled=true", "spring.application.name=gate", 'services.fiat.baseUrl=https://localhost', 'services.keel.baseUrl=https://localhost' ])
 class GenerateSwaggerSpec extends Specification {
 
   @Autowired

gate-web/src/test/resources/basic-auth.properties

@@ -2,3 +2,4 @@ security.basicform.enabled=true
 spring.security.user.name=basic-user
 spring.security.user.password=basic-password
 spring.application.name=gate
+services.fiat.baseUrl=https://localhost:8083