fix (kubernetes-client-api) : Fix NO_PROXY check in HttpClientUtil (#…

…4247) + Modify NO_PROXY check to use `.endsWith` instead of `.contains` to better match host name. + Add a check to throw an Exception if any of NO_PROXY contains `*` Signed-off-by: Rohan Kumar <rohaan@redhat.com>
fabric8io · Aug 5, 2022 · 06d7969 · 06d7969
1 parent fd07e96
commit 06d7969
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 #### Bugs
 * Fix #4256: crd-generator-apt pom.xml includes transitive dependencies
 * Fix #4294: crd-generator respects JsonIgnore annotations on enum properties
+* Fix #4247: NO_PROXY with wildcard throws exception
 
 #### Improvements
 * Fix #4041: adding Quantity.getNumericalAmount with an explanation about bytes and cores.

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
@@ -69,18 +69,8 @@ private HttpClientUtils() {
   public static URL getProxyUrl(Config config) throws MalformedURLException {
     URL master = new URL(config.getMasterUrl());
     String host = master.getHost();
-    if (config.getNoProxy() != null) {
-      for (String noProxy : config.getNoProxy()) {
-        if (isIpAddress(noProxy)) {
-          if (new IpAddressMatcher(noProxy).matches(host)) {
-            return null;
-          }
-        } else {
-          if (host.contains(noProxy)) {
-            return null;
-          }
-        }
-      }
+    if (isValidNoProxy(config.getNoProxy()) && isHostPresentInNoProxy(host, config.getNoProxy())) {
+      return null;
     }
     String proxy = config.getHttpsProxy();
     if (master.getProtocol().equals("http")) {
@@ -96,11 +86,6 @@ public static URL getProxyUrl(Config config) throws MalformedURLException {
     return null;
   }
 
-  private static boolean isIpAddress(String ipAddress) {
-    Matcher ipMatcher = VALID_IPV4_PATTERN.matcher(ipAddress);
-    return ipMatcher.matches();
-  }
-
   public static Map<String, io.fabric8.kubernetes.client.http.Interceptor> createApplicableInterceptors(Config config,
       HttpClient.Factory factory) {
     Map<String, io.fabric8.kubernetes.client.http.Interceptor> interceptors = new LinkedHashMap<>();
@@ -225,4 +210,39 @@ public static void applyCommonConfiguration(Config config, HttpClient.Builder bu
     HttpClientUtils.createApplicableInterceptors(config, factory).forEach(builder::addOrReplaceInterceptor);
   }
 
+  private static boolean isHostPresentInNoProxy(String host, String[] noProxyList) {
+    for (String noProxy : noProxyList) {
+      if (isIpAddressRange(noProxy)) {
+        if (new IpAddressMatcher(noProxy).matches(host)) {
+          return true;
+        }
+      } else {
+        if (host.endsWith(noProxy)) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
+  private static boolean isValidNoProxy(String[] noProxy) throws MalformedURLException {
+    if (noProxy != null) {
+      for (String np : noProxy) {
+        if (np.indexOf('*') > -1) {
+          throw new MalformedURLException("Wildcard not supported in NO_PROXY URL");
+        }
+      }
+      return true;
+    }
+    return false;
+  }
+
+  private static boolean isIpAddress(String ipAddress) {
+    Matcher ipMatcher = VALID_IPV4_PATTERN.matcher(ipAddress);
+    return ipMatcher.matches();
+  }
+
+  private static boolean isIpAddressRange(String ipAddress) {
+    return isIpAddress(ipAddress) && ipAddress.contains("/");
+  }
 }
diff --git a/...etes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/HttpClientUtilsTest.java b/...etes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/HttpClientUtilsTest.java
@@ -26,7 +26,8 @@
 
 import static io.fabric8.kubernetes.client.utils.HttpClientUtils.KUBERNETES_BACKWARDS_COMPATIBILITY_INTERCEPTOR_DISABLE;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 
 class HttpClientUtilsTest {
   @Test
@@ -67,27 +68,18 @@ void testCreateApplicableInterceptorsWithBackwardsCompatibilityDisabled() {
   @Test
   void getProxyUrl_whenHttpsProxyUrlWithNoPort_shouldReturnValidProxyUrl() {
     // Given
-    Config config = new ConfigBuilder()
-        .withMasterUrl("http://localhost")
-        .withHttpProxy("http://192.168.0.1")
-        .build();
+    Config config = createNewConfig("http://localhost", "http://192.168.0.1", null);
 
-    // When
-    final IllegalArgumentException illegalArgumentException = assertThrows(IllegalArgumentException.class,
-        () -> HttpClientUtils.getProxyUrl(config));
-
-    // Then
-    assertThat(illegalArgumentException)
-        .hasMessage("Failure in creating proxy URL. Proxy port is required!");
+    // When + Then
+    assertThatIllegalArgumentException()
+        .isThrownBy(() -> HttpClientUtils.getProxyUrl(config))
+        .withMessage("Failure in creating proxy URL. Proxy port is required!");
   }
 
   @Test
   void getProxyUrl_whenHttpsProxyUrlWithPort_shouldReturnValidProxyUrl() throws MalformedURLException {
     // Given
-    Config config = new ConfigBuilder()
-        .withMasterUrl("http://localhost")
-        .withHttpProxy("http://192.168.0.1:3128")
-        .build();
+    Config config = createNewConfig("http://localhost", "http://192.168.0.1:3128", null);
 
     // When
     URL url = HttpClientUtils.getProxyUrl(config);
@@ -97,4 +89,61 @@ void getProxyUrl_whenHttpsProxyUrlWithPort_shouldReturnValidProxyUrl() throws Ma
         .hasPort(3128)
         .hasHost("192.168.0.1");
   }
+
+  @Test
+  void getProxyUrl_whenNoProxyProvided_shouldReturnNull() throws MalformedURLException {
+    // Given
+    Config config = createNewConfig("http://api.example.x812.my.domain.com:6443", "http://93.2.12.169:3128", ".my.domain.com");
+
+    // When
+    URL url = HttpClientUtils.getProxyUrl(config);
+
+    // Then
+    assertThat(url).isNull();
+  }
+
+  @Test
+  void getProxyUrl_whenHostInNoProxyIPAddressRange_shouldReturnNull() throws MalformedURLException {
+    // Given
+    Config config = createNewConfig("http://192.168.1.100:6443", "http://93.2.12.169:3128", "192.168.1.0/24");
+
+    // When
+    URL url = HttpClientUtils.getProxyUrl(config);
+
+    // Then
+    assertThat(url).isNull();
+  }
+
+  @Test
+  void getProxyUrl_whenHostNotInNoProxyIPAddressRange_shouldReturnProxyUrl() throws MalformedURLException {
+    // Given
+    Config config = createNewConfig("http://192.168.2.100:6443", "http://93.2.12.169:3128", "192.168.1.0/24");
+
+    // When
+    URL url = HttpClientUtils.getProxyUrl(config);
+
+    // Then
+    assertThat(url).isNotNull().hasHost("93.2.12.169");
+  }
+
+  @Test
+  void getProxyUrl_whenNoProxyContainsWildcard_thenThrowException() {
+    // Given
+    Config config = createNewConfig("http://api.example.x812.my.domain.com:6443", "http://93.2.12.169:3128", "*.my.domain.com");
+
+    // When + Then
+    assertThatThrownBy(() -> HttpClientUtils.getProxyUrl(config))
+        .isInstanceOf(MalformedURLException.class)
+        .hasMessage("Wildcard not supported in NO_PROXY URL");
+  }
+
+  private Config createNewConfig(String masterUrl, String httpProxy, String noProxy) {
+    ConfigBuilder configBuilder = new ConfigBuilder()
+        .withMasterUrl(masterUrl)
+        .withHttpProxy(httpProxy);
+    if (noProxy != null) {
+      configBuilder.withNoProxy(noProxy);
+    }
+    return configBuilder.build();
+  }
 }