From 464c6b673741f0d1dad46ce91fa504ce771150d5 Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Wed, 30 Aug 2023 23:39:56 +0200 Subject: [PATCH] Use HttpClient factory to create singleton instances Signed-off-by: Paolo Di Tommaso --- .../wave/auth/RegistryAuthServiceImpl.groovy | 13 ++---- .../auth/RegistryLookupServiceImpl.groovy | 12 ++--- .../wave/core/RegistryProxyService.groovy | 9 ++-- .../seqera/wave/http/HttpClientFactory.groovy | 44 +++++++++++++++++++ .../io/seqera/wave/proxy/ProxyClient.groovy | 15 ++----- .../ContainerInspectServiceImpl.groovy | 9 ++-- .../connector/HttpTowerConnector.groovy | 12 ++--- .../io/seqera/wave/ProxyClientTest.groovy | 21 +++++---- .../ProxyClientWithLocalRegistryTest.groovy | 10 +++-- .../wave/core/ContainerAugmenterTest.groovy | 12 ++--- 10 files changed, 94 insertions(+), 63 deletions(-) create mode 100644 src/main/groovy/io/seqera/wave/http/HttpClientFactory.groovy diff --git a/src/main/groovy/io/seqera/wave/auth/RegistryAuthServiceImpl.groovy b/src/main/groovy/io/seqera/wave/auth/RegistryAuthServiceImpl.groovy index 8b233a6b4..66eae37ab 100644 --- 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 index c7f61f8f9..10ca24bca 100644 --- 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 loader = new CacheLoader() { @@ -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 index 879e4123e..efe793c90 100644 --- 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,7 +13,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.model.ContainerCoordinates import io.seqera.wave.proxy.ProxyClient import io.seqera.wave.service.CredentialsService @@ -19,6 +20,7 @@ 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 new file mode 100644 index 000000000..5097c88ad --- /dev/null +++ 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 + */ +@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 index 38ea12d6f..cf09a6ee2 100644 --- 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 index a03bf3276..68cfdac62 100644 --- 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 index bc884a9dd..402d1c619 100644 --- 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 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 index e5b25781d..a0ab44c16 100644 --- 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 @@ -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) diff --git a/src/test/groovy/io/seqera/wave/ProxyClientWithLocalRegistryTest.groovy b/src/test/groovy/io/seqera/wave/ProxyClientWithLocalRegistryTest.groovy index c59af3c0a..34483bd97 100644 --- a/src/test/groovy/io/seqera/wave/ProxyClientWithLocalRegistryTest.groovy +++ b/src/test/groovy/io/seqera/wave/ProxyClientWithLocalRegistryTest.groovy @@ -1,18 +1,20 @@ package io.seqera.wave - 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.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 io.seqera.wave.test.DockerRegistryContainer import jakarta.inject.Inject +import jakarta.inject.Named + /** * * @author Paolo Di Tommaso @@ -27,7 +29,7 @@ class ProxyClientWithLocalRegistryTest extends Specification implements DockerRe @Inject RegistryLookupService lookupService @Inject RegistryAuthService loginService @Inject RegistryCredentialsProvider credentialsProvider - @Inject HttpClientConfig config + @Inject @Named("never-redirects") HttpClient httpClient def setupSpec() { initRegistryContainer(applicationContext) @@ -37,7 +39,7 @@ class ProxyClientWithLocalRegistryTest extends Specification implements DockerRe given: def IMAGE = 'library/hello-world' and: - def proxy = new ProxyClient(config) + def proxy = new ProxyClient(httpClient) .withImage(IMAGE) .withRegistry(getLocalTestRegistryInfo()) .withLoginService(loginService) diff --git a/src/test/groovy/io/seqera/wave/core/ContainerAugmenterTest.groovy b/src/test/groovy/io/seqera/wave/core/ContainerAugmenterTest.groovy index eb48b7799..2794e5a19 100644 --- a/src/test/groovy/io/seqera/wave/core/ContainerAugmenterTest.groovy +++ b/src/test/groovy/io/seqera/wave/core/ContainerAugmenterTest.groovy @@ -3,6 +3,7 @@ package io.seqera.wave.core import spock.lang.Shared import spock.lang.Specification +import java.net.http.HttpClient import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths @@ -19,7 +20,6 @@ import io.seqera.wave.auth.RegistryAuthService import io.seqera.wave.auth.RegistryCredentialsProvider import io.seqera.wave.auth.RegistryInfo import io.seqera.wave.auth.RegistryLookupService -import io.seqera.wave.configuration.HttpClientConfig import io.seqera.wave.model.ContentType import io.seqera.wave.proxy.ProxyClient import io.seqera.wave.storage.Storage @@ -27,6 +27,8 @@ import io.seqera.wave.test.ManifestConst import io.seqera.wave.util.ContainerConfigFactory import io.seqera.wave.util.RegHelper import jakarta.inject.Inject +import jakarta.inject.Named + /** * * @author Paolo Di Tommaso @@ -54,7 +56,7 @@ class ContainerAugmenterTest extends Specification { @Inject RegistryAuthService loginService @Inject RegistryLookupService lookupService @Inject RegistryCredentialsProvider credentialsProvider - @Inject HttpClientConfig httpClientConfig + @Inject @Named("never-redirects") HttpClient httpClient def 'should set layer paths' () { given: @@ -744,7 +746,7 @@ class ContainerAugmenterTest extends Specification { def creds = credentialsProvider.getDefaultCredentials(REGISTRY) and: - def client = new ProxyClient(httpClientConfig) + def client = new ProxyClient(httpClient) .withRoute(Mock(RoutePath)) .withImage(IMAGE) .withRegistry(registry) @@ -771,7 +773,7 @@ class ContainerAugmenterTest extends Specification { def creds = credentialsProvider.getDefaultCredentials(REGISTRY) and: - def client = new ProxyClient(httpClientConfig) + def client = new ProxyClient(httpClient) .withRoute(Mock(RoutePath)) .withImage(IMAGE) .withRegistry(registry) @@ -798,7 +800,7 @@ class ContainerAugmenterTest extends Specification { def creds = credentialsProvider.getDefaultCredentials(REGISTRY) and: - def client = new ProxyClient(httpClientConfig) + def client = new ProxyClient(httpClient) .withRoute(Mock(RoutePath)) .withImage(IMAGE) .withRegistry(registry)