From b75a216c4d2eb94baf97331ae7f4deb813305219 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Wed, 12 Oct 2022 14:28:07 -0400 Subject: [PATCH] fix #4471: kubernetesclientbuilder method for the httpclient.builder --- CHANGELOG.md | 1 + .../client/jdkhttp/JdkHttpClientFactory.java | 10 ----- .../client/jetty/JettyHttpClientFactory.java | 10 ----- .../jetty/JettyHttpClientFactoryTest.java | 2 +- .../client/okhttp/OkHttpClientFactory.java | 8 ++-- .../client/DefaultKubernetesClient.java | 5 --- .../client/KubernetesClientBuilder.java | 27 +++++++++++++- .../kubernetes/client/http/HttpClient.java | 14 ++++++- .../client/utils/HttpClientUtils.java | 2 +- .../client/KubernetesClientBuilderTest.java | 37 +++++++++++++++++++ .../okhttp/OkHttpClientFactoryTest.java | 8 ++-- 11 files changed, 87 insertions(+), 37 deletions(-) create mode 100644 kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/KubernetesClientBuilderTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 758b515cf1a..935c47ce644 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * Fix #4426: [java-generator] Encode an `AnyType` instead of an Object if `x-kubernetes-preserve-unknown-fields` is present and the type is null. #### Improvements +* Fix #4471: Adding KubernetesClientBuilder.withHttpClientBuilderConsumer to further customize the HttpClient for any implementation. * Fix #4348: Introduce specific annotations for the generators * Refactor #4441: refactoring `TokenRefreshInterceptor` * Fix #4365: The Watch retry logic will handle more cases, as well as perform an exceptional close for events that are not properly handled. Informers can directly provide those exceptional outcomes via the SharedIndexInformer.stopped CompletableFuture. diff --git a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientFactory.java b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientFactory.java index d4da7f16d77..685a28f6dba 100644 --- a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientFactory.java +++ b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientFactory.java @@ -18,7 +18,6 @@ import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.http.HttpClient; -import io.fabric8.kubernetes.client.utils.HttpClientUtils; import io.fabric8.kubernetes.client.utils.Utils; import java.util.concurrent.Executor; @@ -46,15 +45,6 @@ public void shutdownNow() { } - @Override - public HttpClient createHttpClient(Config config) { - JdkHttpClientBuilderImpl builderWrapper = newBuilder(); - - HttpClientUtils.applyCommonConfiguration(config, builderWrapper, this); - - return builderWrapper.build(); - } - @Override public JdkHttpClientBuilderImpl newBuilder() { return new JdkHttpClientBuilderImpl(this); diff --git a/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientFactory.java b/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientFactory.java index aabece12900..68c78505f7c 100644 --- a/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientFactory.java +++ b/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientFactory.java @@ -15,20 +15,10 @@ */ package io.fabric8.kubernetes.client.jetty; -import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.http.HttpClient; -import static io.fabric8.kubernetes.client.utils.HttpClientUtils.applyCommonConfiguration; - public class JettyHttpClientFactory implements HttpClient.Factory { - @Override - public HttpClient createHttpClient(Config config) { - final var builder = newBuilder(); - applyCommonConfiguration(config, builder, this); - return builder.build(); - } - @Override public JettyHttpClientBuilder newBuilder() { return new JettyHttpClientBuilder(this); diff --git a/httpclient-jetty/src/test/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientFactoryTest.java b/httpclient-jetty/src/test/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientFactoryTest.java index dcb8403ab03..19f74c1d66f 100644 --- a/httpclient-jetty/src/test/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientFactoryTest.java +++ b/httpclient-jetty/src/test/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientFactoryTest.java @@ -27,7 +27,7 @@ class JettyHttpClientFactoryTest { @DisplayName("createHttpClient instantiates a JettyHttpClient") void createHttpClientInstantiatesJettyHttpClient() { // When - try (var result = new JettyHttpClientFactory().createHttpClient(Config.empty())) { + try (var result = new JettyHttpClientFactory().newBuilder(Config.empty()).build()) { // Then assertThat(result).isNotNull().isInstanceOf(JettyHttpClient.class); } diff --git a/httpclient-okhttp/src/main/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactory.java b/httpclient-okhttp/src/main/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactory.java index 16522e37c5c..330fe5c5e4a 100644 --- a/httpclient-okhttp/src/main/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactory.java +++ b/httpclient-okhttp/src/main/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactory.java @@ -59,13 +59,13 @@ public Builder newBuilder() { } /** - * Creates an HTTP client configured to access the Kubernetes API. + * Creates an HTTP client builder configured to access the Kubernetes API. * * @param config Kubernetes API client config - * @return returns an HTTP client + * @return returns an HTTP client builder */ @Override - public OkHttpClientImpl createHttpClient(Config config) { + public OkHttpClientBuilderImpl newBuilder(Config config) { try { OkHttpClient.Builder httpClientBuilder = newOkHttpClientBuilder(); @@ -101,7 +101,7 @@ public OkHttpClientImpl createHttpClient(Config config) { additionalConfig(httpClientBuilder); - return builderWrapper.build(); + return builderWrapper; } catch (Exception e) { throw KubernetesClientException.launderThrowable(e); } diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/DefaultKubernetesClient.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/DefaultKubernetesClient.java index 3239e467814..0159037ed10 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/DefaultKubernetesClient.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/DefaultKubernetesClient.java @@ -71,11 +71,6 @@ public Builder newBuilder() { // should not be called throw new UnsupportedOperationException(); } - - @Override - public HttpClient createHttpClient(Config config) { - return httpClient; - } }); } this.init(builder.build()); diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/KubernetesClientBuilder.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/KubernetesClientBuilder.java index 9e9b3a54a57..636e410a898 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/KubernetesClientBuilder.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/KubernetesClientBuilder.java @@ -23,6 +23,7 @@ import java.io.InputStream; import java.lang.reflect.InvocationTargetException; import java.util.concurrent.Executor; +import java.util.function.Consumer; import java.util.function.Supplier; /** @@ -44,6 +45,7 @@ default void onClose(Executor executor) { private HttpClient.Factory factory; private Class clazz; private ExecutorSupplier executorSupplier; + private Consumer builderConsumer; public KubernetesClientBuilder() { // basically the same logic as in KubernetesResourceUtil for finding list types @@ -60,6 +62,10 @@ public KubernetesClientBuilder() { } } + KubernetesClientBuilder(Class clazz) { + this.clazz = clazz; + } + public KubernetesClient build() { if (config == null) { config = new ConfigBuilder().build(); @@ -68,7 +74,7 @@ public KubernetesClient build() { if (factory == null) { return clazz.getConstructor(Config.class).newInstance(config); } - HttpClient client = factory.createHttpClient(config); + HttpClient client = getHttpClient(); return clazz.getConstructor(HttpClient.class, Config.class, ExecutorSupplier.class).newInstance(client, config, executorSupplier); } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException @@ -77,6 +83,14 @@ public KubernetesClient build() { } } + HttpClient getHttpClient() { + HttpClient.Builder builder = factory.newBuilder(config); + if (this.builderConsumer != null) { + this.builderConsumer.accept(builder); + } + return builder.build(); + } + public KubernetesClientBuilder withConfig(Config config) { this.config = config; return this; @@ -125,4 +139,15 @@ public KubernetesClientBuilder withTaskExecutorSupplier(ExecutorSupplier executo return this; } + /** + * Provide additional configuration for the {@link HttpClient} that is created for this {@link KubernetesClient}. + * + * @param consumer to modify the {@link HttpClient.Builder} + * @return this builder + */ + public KubernetesClientBuilder withHttpClientBuilderConsumer(Consumer consumer) { + this.builderConsumer = consumer; + return this; + } + } diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/HttpClient.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/HttpClient.java index b8b5eb8657f..ff17fce7084 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/HttpClient.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/HttpClient.java @@ -18,6 +18,7 @@ import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.RequestConfig; +import io.fabric8.kubernetes.client.utils.HttpClientUtils; import java.io.BufferedReader; import java.net.InetSocketAddress; @@ -33,7 +34,18 @@ public interface HttpClient extends AutoCloseable { interface Factory { - HttpClient createHttpClient(Config config); + /** + * Create a builder that is customized by the {@link Config}. By default it + * will apply the common configuration {@link HttpClientUtils#applyCommonConfiguration(Config, Builder, Factory)} + * + * @param config the configuration to apply + * @return the configured {@link Builder} + */ + default HttpClient.Builder newBuilder(Config config) { + Builder builder = newBuilder(); + HttpClientUtils.applyCommonConfiguration(config, builder, this); + return builder; + } HttpClient.Builder newBuilder(); diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java index 1f97dd9119f..ffc20e5d4ce 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java @@ -159,7 +159,7 @@ public static HttpClient createHttpClient(Config config) { throw new KubernetesClientException( "No httpclient implementations found on the context classloader, please ensure your classpath includes an implementation jar"); } - return factory.createHttpClient(config); + return factory.newBuilder(config).build(); } public static void applyCommonConfiguration(Config config, HttpClient.Builder builder, HttpClient.Factory factory) { diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/KubernetesClientBuilderTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/KubernetesClientBuilderTest.java new file mode 100644 index 00000000000..71ccbc3ea5d --- /dev/null +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/KubernetesClientBuilderTest.java @@ -0,0 +1,37 @@ +/** + * Copyright (C) 2015 Red Hat, 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 io.fabric8.kubernetes.client; + +import io.fabric8.kubernetes.client.http.HttpClient; +import io.fabric8.kubernetes.client.http.HttpClient.Factory; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +class KubernetesClientBuilderTest { + + @Test + void testHttpClientConfiguration() { + KubernetesClientBuilder builder = new KubernetesClientBuilder(null); + Factory mockFactory = Mockito.mock(HttpClient.Factory.class); + HttpClient.Builder mockBuilder = Mockito.mock(HttpClient.Builder.class); + Mockito.when(mockFactory.newBuilder(Mockito.any())).thenReturn(mockBuilder); + builder.withHttpClientFactory(mockFactory).withHttpClientBuilderConsumer(b -> b.proxyAuthorization("something")); + builder.getHttpClient(); + Mockito.verify(mockBuilder).proxyAuthorization("something"); + } + +} diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactoryTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactoryTest.java index a231b8eade7..c453bcd66f4 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactoryTest.java +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientFactoryTest.java @@ -37,7 +37,7 @@ class OkHttpClientFactoryTest { @Test void shouldRespectMaxRequests() { - OkHttpClientImpl client = new OkHttpClientFactory().createHttpClient(new ConfigBuilder().build()); + OkHttpClientImpl client = new OkHttpClientFactory().newBuilder(new ConfigBuilder().build()).build(); assertEquals(64, client.getOkHttpClient().dispatcher().getMaxRequests()); @@ -45,13 +45,13 @@ void shouldRespectMaxRequests() { .withMaxConcurrentRequests(120) .build(); - client = new OkHttpClientFactory().createHttpClient(config); + client = new OkHttpClientFactory().newBuilder(config).build(); assertEquals(120, client.getOkHttpClient().dispatcher().getMaxRequests()); } @Test void shouldRespectMaxRequestsPerHost() { - OkHttpClientImpl client = new OkHttpClientFactory().createHttpClient(new ConfigBuilder().build()); + OkHttpClientImpl client = new OkHttpClientFactory().newBuilder(new ConfigBuilder().build()).build(); assertEquals(5, client.getOkHttpClient().dispatcher().getMaxRequestsPerHost()); @@ -59,7 +59,7 @@ void shouldRespectMaxRequestsPerHost() { .withMaxConcurrentRequestsPerHost(20) .build(); - client = new OkHttpClientFactory().createHttpClient(config); + client = new OkHttpClientFactory().newBuilder(config).build(); assertEquals(20, client.getOkHttpClient().dispatcher().getMaxRequestsPerHost()); }