From d225086a45b76f5cbdc595fe4e44bfebcd5c015a Mon Sep 17 00:00:00 2001
From: Chris Laprun <metacosm@gmail.com>
Date: Wed, 1 Mar 2023 18:22:14 +0100
Subject: [PATCH] fix: list found httpclient implementations and output
 selected one

Fixes #4935
---
 CHANGELOG.md                                  |  1 +
 .../client/utils/HttpClientUtils.java         | 60 +++++++++++++++----
 2 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 03f502257be..efeb3e3efa8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@
 #### Improvements
 * Fix #4434: Update CronJobIT to use `batch/v1` CronJob instead
 * Fix #4477 exposing LeaderElector.release to force an elector to give up the lease
+* Fix #4935: improve HTTP client implementation selection messages
 * Fix #4975: exposing scale operations for all Resources
 * Fix #4992: Optimize Quantity parsing to avoid regex overhead
 * Fix #4998: removing the internal usage of the Serialization yaml mapper
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 8f6741b62d3..e1bf99b8031 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
@@ -31,13 +31,14 @@
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;
 import java.util.Base64;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 import java.util.ServiceLoader;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -73,7 +74,6 @@ public void before(BasicBuilder builder, HttpRequest request, RequestTags tags)
   private static final Pattern IPV4_PATTERN = Pattern.compile(
       "(http://|https://)?(?<ipAddressOrSubnet>(([01]?\\d\\d?|2[0-4]\\d|25[0-5])\\.){3}([01]?\\d\\d?|2[0-4]\\d|25[0-5])(\\/([1-2]\\d|3[0-2]|\\d))?)(\\D+|$)");
   private static final Pattern INVALID_HOST_PATTERN = Pattern.compile("[^\\da-zA-Z.\\-/:]+");
-  private static final AtomicBoolean MULTIPLE_HTTP_CLIENT_WARNING_LOGGED = new AtomicBoolean();
 
   private HttpClientUtils() {
   }
@@ -145,23 +145,59 @@ public static HttpClient.Factory getHttpClientFactory() {
             "No httpclient implementations found on the context classloader, please ensure your classpath includes an implementation jar");
       }
     }
+    LOGGER.info("Using httpclient {} factory", factory.getClass().getName());
     return factory;
   }
 
   private static HttpClient.Factory getFactory(ServiceLoader<HttpClient.Factory> loader) {
-    HttpClient.Factory factory = null;
-    for (HttpClient.Factory possible : loader) {
-      if (factory != null && MULTIPLE_HTTP_CLIENT_WARNING_LOGGED.compareAndSet(false, true)) {
-        LOGGER.warn("There are multiple httpclient implementation in the classpath, "
-            + "choosing the first non-default implementation. "
-            + "You should exclude dependencies that aren't needed or use an explicit association of the HttpClient.Factory.");
+    HttpClient.Factory selected = null;
+    Set<String> detectedFactories = new HashSet<>();
+    int currentlySelectedPriority = 0;
+    int selectedPriorityCardinality = 0;
+    Set<String> samePriority = new HashSet<>();
+    for (HttpClient.Factory candidate : loader) {
+      final String candidateClassName = candidate.getClass().getName();
+      detectedFactories.add(candidateClassName);
+      LOGGER.debug("Considering {} httpclient factory", candidateClassName);
+
+      if (selected == null) {
+        selected = candidate;
+        currentlySelectedPriority = selected.priority();
+        LOGGER.debug("Temporarily selected {} as first candidate httpclient factory", candidateClassName);
+        continue;
+      } else if (isNonDefaultWhenSelectedDefault(selected, candidate) || isHigherPriority(selected, candidate)) {
+        currentlySelectedPriority = selected.priority();
+        selected = candidate;
+        selectedPriorityCardinality = 0;
+        samePriority.clear();
+        LOGGER.debug("Temporarily selected {} as httpclient factory, replacing a default factory or one with lower priority",
+            candidateClassName);
+      } else {
+        LOGGER.debug("Ignoring {} httpclient factory as it doesn't supersede currently selected one", candidateClassName);
       }
-      if (factory == null || (factory.isDefault() && !possible.isDefault())
-          || (!factory.isDefault() && factory.priority() < possible.priority())) {
-        factory = possible;
+
+      if (currentlySelectedPriority == candidate.priority()) {
+        selectedPriorityCardinality++;
+        samePriority.add(candidateClassName);
       }
+
     }
-    return factory;
+
+    if (detectedFactories.size() > 1 && selectedPriorityCardinality > 0) {
+      LOGGER.warn("The following httpclient factories were detected on your classpath: {}, "
+          + "{} of which had the same priority ({}) so one was chosen randomly. "
+          + "You should exclude dependencies that aren't needed or use an explicit association of the HttpClient.Factory.",
+          detectedFactories, samePriority.size(), samePriority);
+    }
+    return selected;
+  }
+
+  private static boolean isNonDefaultWhenSelectedDefault(HttpClient.Factory selected, HttpClient.Factory candidate) {
+    return selected.isDefault() && !candidate.isDefault();
+  }
+
+  private static boolean isHigherPriority(HttpClient.Factory selected, HttpClient.Factory candidate) {
+    return !selected.isDefault() && selected.priority() < candidate.priority();
   }
 
   public static void applyCommonConfiguration(Config config, HttpClient.Builder builder, HttpClient.Factory factory) {