Use HttpClient factory to create singleton instances

Signed-off-by: Paolo Di Tommaso <[email protected]>
seqeralabs · Aug 30, 2023 · 464c6b6 · 464c6b6
1 parent cb95c0d
commit 464c6b6
Show file tree

Hide file tree

Showing 10 changed files with 94 additions and 63 deletions.
diff --git a/src/main/groovy/io/seqera/wave/auth/RegistryAuthServiceImpl.groovy b/src/main/groovy/io/seqera/wave/auth/RegistryAuthServiceImpl.groovy
@@ -5,7 +5,6 @@ import java.net.http.HttpRequest
 import java.net.http.HttpResponse
 import java.util.concurrent.ExecutionException
 import java.util.concurrent.TimeUnit
-import javax.annotation.PostConstruct
 
 import com.google.common.cache.CacheBuilder
 import com.google.common.cache.CacheLoader
@@ -20,6 +19,7 @@ import io.seqera.wave.configuration.HttpClientConfig
 import io.seqera.wave.util.Retryable
 import io.seqera.wave.util.StringUtils
 import jakarta.inject.Inject
+import jakarta.inject.Named
 import jakarta.inject.Singleton
 import static io.seqera.wave.WaveDefault.DOCKER_IO
 /**
@@ -58,6 +58,8 @@ class RegistryAuthServiceImpl implements RegistryAuthService {
                     .expireAfterAccess(1, TimeUnit.HOURS)
                     .build(loader)
 
+    @Inject
+    @Named("follow-redirects")
     private HttpClient httpClient
 
     @Inject
@@ -66,15 +68,6 @@ class RegistryAuthServiceImpl implements RegistryAuthService {
     @Inject RegistryCredentialsFactory credentialsFactory
 
 
-    @PostConstruct
-    private void init() {
-        this.httpClient = HttpClient.newBuilder()
-                .version(HttpClient.Version.HTTP_1_1)
-                .followRedirects(HttpClient.Redirect.NORMAL)
-                .connectTimeout(httpConfig.connectTimeout)
-                .build()
-    }
-
     /**
      * Implements container registry login
      *

diff --git a/src/main/groovy/io/seqera/wave/auth/RegistryLookupServiceImpl.groovy b/src/main/groovy/io/seqera/wave/auth/RegistryLookupServiceImpl.groovy
@@ -4,7 +4,6 @@ import java.net.http.HttpClient
 import java.net.http.HttpRequest
 import java.net.http.HttpResponse
 import java.util.concurrent.TimeUnit
-import javax.annotation.PostConstruct
 
 import com.google.common.cache.CacheBuilder
 import com.google.common.cache.CacheLoader
@@ -14,6 +13,7 @@ import groovy.util.logging.Slf4j
 import io.seqera.wave.configuration.HttpClientConfig
 import io.seqera.wave.util.Retryable
 import jakarta.inject.Inject
+import jakarta.inject.Named
 import jakarta.inject.Singleton
 import static io.seqera.wave.WaveDefault.DOCKER_IO
 import static io.seqera.wave.WaveDefault.DOCKER_REGISTRY_1
@@ -32,6 +32,8 @@ class RegistryLookupServiceImpl implements RegistryLookupService {
     @Inject
     private HttpClientConfig httpConfig
 
+    @Inject
+    @Named("follow-redirects")
     private HttpClient httpClient
 
     private CacheLoader<URI, RegistryAuth> loader = new CacheLoader<URI, RegistryAuth>() {
@@ -49,14 +51,6 @@ class RegistryLookupServiceImpl implements RegistryLookupService {
                 .expireAfterAccess(1, TimeUnit.HOURS)
                 .build(loader)
 
-    @PostConstruct
-    private init() {
-        httpClient = HttpClient.newBuilder()
-                .version(HttpClient.Version.HTTP_1_1)
-                .followRedirects(HttpClient.Redirect.NORMAL)
-                .connectTimeout(httpConfig.connectTimeout)
-                .build()
-    }
 
     protected RegistryAuth lookup0(URI endpoint) {
         final request = HttpRequest.newBuilder() .uri(endpoint) .GET() .build()

diff --git a/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy b/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy
@@ -1,5 +1,7 @@
 package io.seqera.wave.core
 
+import java.net.http.HttpClient
+
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
 import io.micronaut.cache.annotation.Cacheable
@@ -11,14 +13,14 @@ import io.seqera.wave.auth.RegistryAuthService
 import io.seqera.wave.auth.RegistryCredentials
 import io.seqera.wave.auth.RegistryCredentialsProvider
 import io.seqera.wave.auth.RegistryLookupService
-import io.seqera.wave.configuration.HttpClientConfig
 import io.seqera.wave.model.ContainerCoordinates
 import io.seqera.wave.proxy.ProxyClient
 import io.seqera.wave.service.CredentialsService
 import io.seqera.wave.service.persistence.PersistenceService
 import io.seqera.wave.storage.DigestStore
 import io.seqera.wave.storage.Storage
 import jakarta.inject.Inject
+import jakarta.inject.Named
 import jakarta.inject.Singleton
 import static io.seqera.wave.proxy.ProxyClient.REDIRECT_CODES
 /**
@@ -54,7 +56,8 @@ class RegistryProxyService {
     private RegistryAuthService loginService
 
     @Inject
-    private HttpClientConfig httpClientConfig
+    @Named("never-redirects")
+    private HttpClient httpClient
 
     @Inject
     private PersistenceService persistenceService
@@ -68,7 +71,7 @@ class RegistryProxyService {
     private ProxyClient client(RoutePath route) {
         final registry = registryLookup.lookup(route.registry)
         final creds = getCredentials(route)
-        new ProxyClient(httpClientConfig)
+        new ProxyClient(httpClient)
                 .withRoute(route)
                 .withImage(route.image)
                 .withRegistry(registry)

diff --git a/src/main/groovy/io/seqera/wave/http/HttpClientFactory.groovy b/src/main/groovy/io/seqera/wave/http/HttpClientFactory.groovy
@@ -0,0 +1,44 @@
+package io.seqera.wave.http
+
+import java.net.http.HttpClient
+
+import groovy.transform.CompileStatic
+import groovy.util.logging.Slf4j
+import io.micronaut.context.annotation.Factory
+import io.seqera.wave.configuration.HttpClientConfig
+import jakarta.inject.Inject
+import jakarta.inject.Named
+import jakarta.inject.Singleton
+/**
+ * Java HttpClient factory
+ *
+ * @author Paolo Di Tommaso <[email protected]>
+ */
+@Factory
+@Slf4j
+@CompileStatic
+class HttpClientFactory {
+
+    @Inject
+    HttpClientConfig httpConfig
+
+    @Singleton
+    @Named("follow-redirects")
+    HttpClient followRedirectsHttpClient() {
+        HttpClient.newBuilder()
+                .version(HttpClient.Version.HTTP_1_1)
+                .followRedirects(HttpClient.Redirect.NORMAL)
+                .connectTimeout(httpConfig.connectTimeout)
+                .build()
+    }
+
+    @Singleton
+    @Named("never-redirects")
+    HttpClient neverRedirectsHttpClient() {
+        HttpClient.newBuilder()
+                .version(HttpClient.Version.HTTP_1_1)
+                .followRedirects(HttpClient.Redirect.NEVER)
+                .connectTimeout(httpConfig.connectTimeout)
+                .build()
+    }
+}
diff --git a/src/main/groovy/io/seqera/wave/proxy/ProxyClient.groovy b/src/main/groovy/io/seqera/wave/proxy/ProxyClient.groovy
@@ -17,7 +17,6 @@ import io.micronaut.http.server.exceptions.InternalServerException
 import io.seqera.wave.auth.RegistryAuthService
 import io.seqera.wave.auth.RegistryCredentials
 import io.seqera.wave.auth.RegistryInfo
-import io.seqera.wave.configuration.HttpClientConfig
 import io.seqera.wave.core.ContainerPath
 import io.seqera.wave.util.RegHelper
 /**
@@ -43,8 +42,10 @@ class ProxyClient {
     private RegistryAuthService loginService
     private ContainerPath route
 
-    ProxyClient(HttpClientConfig config) {
-        init(config)
+    ProxyClient(HttpClient httpClient) {
+        if( httpClient.followRedirects()!= HttpClient.Redirect.NEVER )
+            throw new IllegalStateException("HttpClient instance should not follow redirected because they are directly managed by the proxy")
+        this.httpClient = httpClient
     }
 
     ContainerPath getRoute() { route }
@@ -76,14 +77,6 @@ class ProxyClient {
         return this
     }
 
-    private void init(HttpClientConfig config) {
-        this.httpClient = HttpClient.newBuilder()
-                .version(HttpClient.Version.HTTP_1_1)
-                .followRedirects(HttpClient.Redirect.NEVER)
-                .connectTimeout(config.connectTimeout)
-                .build()
-    }
-
     private URI makeUri(String path) {
         assert path.startsWith('/'), "Request past should start with a slash character — offending path: $path"
         URI.create(registry.host.toString() + path)

diff --git a/src/main/groovy/io/seqera/wave/service/inspect/ContainerInspectServiceImpl.groovy b/src/main/groovy/io/seqera/wave/service/inspect/ContainerInspectServiceImpl.groovy
@@ -1,5 +1,7 @@
 package io.seqera.wave.service.inspect
 
+import java.net.http.HttpClient
+
 import groovy.transform.Canonical
 import groovy.transform.CompileStatic
 import groovy.transform.PackageScope
@@ -10,7 +12,6 @@ import io.seqera.wave.auth.RegistryAuthService
 import io.seqera.wave.auth.RegistryCredentials
 import io.seqera.wave.auth.RegistryCredentialsProvider
 import io.seqera.wave.auth.RegistryLookupService
-import io.seqera.wave.configuration.HttpClientConfig
 import io.seqera.wave.core.ContainerAugmenter
 import io.seqera.wave.core.ContainerPath
 import io.seqera.wave.core.RegistryProxyService
@@ -19,6 +20,7 @@ import io.seqera.wave.model.ContainerCoordinates
 import io.seqera.wave.proxy.ProxyClient
 import io.seqera.wave.util.RegHelper
 import jakarta.inject.Inject
+import jakarta.inject.Named
 import jakarta.inject.Singleton
 /**
  * Implements containers inspect service
@@ -54,7 +56,8 @@ class ContainerInspectServiceImpl implements ContainerInspectService {
     private RegistryProxyService proxyService
 
     @Inject
-    private HttpClientConfig httpClientConfig
+    @Named("never-redirects")
+    private HttpClient httpClient
 
     @Inject
     private RegistryAuthService loginService
@@ -176,7 +179,7 @@ class ContainerInspectServiceImpl implements ContainerInspectService {
 
     private ProxyClient client0(ContainerPath route, RegistryCredentials creds) {
         final registry = lookupService.lookup(route.registry)
-        new ProxyClient(httpClientConfig)
+        new ProxyClient(httpClient)
                 .withRoute(route)
                 .withImage(route.image)
                 .withRegistry(registry)

diff --git a/src/main/groovy/io/seqera/wave/tower/client/connector/HttpTowerConnector.groovy b/src/main/groovy/io/seqera/wave/tower/client/connector/HttpTowerConnector.groovy
@@ -4,7 +4,6 @@ import java.net.http.HttpClient
 import java.net.http.HttpRequest
 import java.net.http.HttpResponse
 import java.util.concurrent.CompletableFuture
-import javax.annotation.PostConstruct
 
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
@@ -13,6 +12,7 @@ import io.seqera.wave.configuration.HttpClientConfig
 import io.seqera.wave.service.pairing.socket.msg.ProxyHttpRequest
 import io.seqera.wave.service.pairing.socket.msg.ProxyHttpResponse
 import jakarta.inject.Inject
+import jakarta.inject.Named
 import jakarta.inject.Singleton
 /**
  * Implements a Tower client using a plain HTTP client
@@ -28,16 +28,10 @@ class HttpTowerConnector extends TowerConnector {
     @Inject
     private HttpClientConfig config
 
+    @Inject
+    @Named("never-redirects")
     private HttpClient client
 
-    @PostConstruct
-    void init() {
-        this.client = HttpClient.newBuilder()
-                                .version(HttpClient.Version.HTTP_1_1)
-                                .connectTimeout(config.connectTimeout)
-                                .build()
-    }
-
     @Override
     CompletableFuture<ProxyHttpResponse> sendAsync(String endpoint, ProxyHttpRequest request) {
         client

diff --git a/src/test/groovy/io/seqera/wave/ProxyClientTest.groovy b/src/test/groovy/io/seqera/wave/ProxyClientTest.groovy
@@ -4,15 +4,18 @@ import spock.lang.Requires
 import spock.lang.Shared
 import spock.lang.Specification
 
+import java.net.http.HttpClient
+
 import io.micronaut.context.ApplicationContext
 import io.micronaut.test.extensions.spock.annotation.MicronautTest
 import io.seqera.wave.auth.RegistryAuth
 import io.seqera.wave.auth.RegistryAuthService
 import io.seqera.wave.auth.RegistryCredentialsProvider
 import io.seqera.wave.auth.RegistryLookupService
-import io.seqera.wave.configuration.HttpClientConfig
 import io.seqera.wave.proxy.ProxyClient
 import jakarta.inject.Inject
+import jakarta.inject.Named
+
 /**
  *
  * @author Paolo Di Tommaso <[email protected]>
@@ -27,7 +30,7 @@ class ProxyClientTest extends Specification {
     @Inject RegistryLookupService lookupService
     @Inject RegistryAuthService loginService
     @Inject RegistryCredentialsProvider credentialsProvider
-    @Inject HttpClientConfig config
+    @Inject @Named("never-redirects") HttpClient httpClient
 
     def 'should call target manifests on docker.io' () {
         given:
@@ -36,7 +39,7 @@ class ProxyClientTest extends Specification {
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
         and:
-        def proxy = new ProxyClient(config)
+        def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
                 .withRegistry(registry)
                 .withLoginService(loginService)
@@ -55,7 +58,7 @@ class ProxyClientTest extends Specification {
         def IMAGE = 'library/hello-world'
         def registry = lookupService.lookup(REG)
         and:
-        def proxy = new ProxyClient(config)
+        def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
                 .withRegistry(registry)
                 .withLoginService(loginService)
@@ -74,7 +77,7 @@ class ProxyClientTest extends Specification {
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
         and:
-        def proxy = new ProxyClient(config)
+        def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
                 .withRegistry(registry)
                 .withLoginService(loginService)
@@ -94,7 +97,7 @@ class ProxyClientTest extends Specification {
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
         and:
-        def proxy = new ProxyClient(config)
+        def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
                 .withRegistry(registry)
                 .withLoginService(loginService)
@@ -127,7 +130,7 @@ class ProxyClientTest extends Specification {
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
         and:
-        def proxy = new ProxyClient(config)
+        def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
                 .withRegistry(registry)
                 .withLoginService(loginService)
@@ -147,7 +150,7 @@ class ProxyClientTest extends Specification {
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
         and:
-        def proxy = new ProxyClient(config)
+        def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
                 .withRegistry(registry)
                 .withLoginService(loginService)
@@ -208,7 +211,7 @@ class ProxyClientTest extends Specification {
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
         and:
-        def proxy = new ProxyClient(config)
+        def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
                 .withRegistry(registry)
                 .withLoginService(loginService)