Openshift Client ignores NO_PROXY setting

[brubrz]

Describe the bug

The ConfigBuilder ignores .withNoProxy() Setting. I must set the HttpProxy to null in order to connect to my PaaS.

Fabric8 Kubernetes Client version

6.0.0-RC1

Steps to reproduce

Does not work:

    Config config = new ConfigBuilder()
            .withNoProxy("*.my.domain.com") // does not work: Socket Exception: "read timed out"
            .build();


    try (final KubernetesClient client = new KubernetesClientBuilder().withConfig(config).build()) {

        System.out.println(Arrays.toString(config.getNoProxy()));
        System.out.println(client.getConfiguration().getCurrentContext().getName());
        System.out.println(config.getUsername());
        System.out.println(config.getOauthToken());


        client.pods().inNamespace("default").list().getItems().stream().map(pod -> pod.getMetadata().getName()).forEach(System.out::println);

Weird workaround that works:

    Config config = new ConfigBuilder()
            .withHttpProxy(null)
            .withHttpsProxy(null)
            .build();


    try (final KubernetesClient client = new KubernetesClientBuilder().withConfig(config).build()) {
        //client.authorization().v1().localSubjectAccessReview()

        System.out.println(Arrays.toString(config.getNoProxy()));
        System.out.println(client.getConfiguration().getCurrentContext().getName());
        System.out.println(config.getUsername());
        System.out.println(config.getOauthToken());


        client.pods().inNamespace("default").list().getItems().stream().map(pod -> pod.getMetadata().getName()).forEach(System.out::println);

Expected behavior

    Config config = new ConfigBuilder()
            .withNoProxy("*.my.domain.com")
            .build();

This should work / not be ignored.

Runtime

OpenShift

Kubernetes API Server version

other (please specify in additional context)

Environment

Windows

Fabric8 Kubernetes Client Logs

No response

Additional context

Kubernetes API Server version: v1.21.11+6b3cbdd

[shawkins]

The implementation

kubernetes-client/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java

Line 72 in 10e9f18

if (config.getNoProxy() != null) {

does not expect wildcards for hosts. It does have some match logic for IPs.

[brubrz]

In that case, I would really appreciate an error msg. It appears to me as if the code is passing an IP that could not be parsed along to another component to let it fail there? I think an unparsable proxy ip should result in an error that can be associated with the proxy configuration. If the unparsable proxy string is passed to the level of concern of http connections then ofc I get an error message that is related to that layer: "read timed out", but that makes it hard to find out what's actually wrong.

    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;
          }
         // the proxy string was not parsable                    <--------------------- 
        }
      }
    }

[shawkins]

The biggest issue here is there's no docs defining the expected behavior.

As implemented it would read something like:

noProxy - a list of addresses of hosts for which using the proxy is disabled. If an IP address is specified, it may include subnet masks. If a host name is specified the proxy won't be used if that name appears anywhere in the requested host name - there is no support for wildcard matches.

In that case, I would really appreciate an error msg.

I think you mean checking for non-url characters, so in this case * would cause an exception.

That's a possibility. As would be supporting glob matching - that would be backwards compatible. Full regex would not. In any case I also wonder about using host.contains - searching anywhere in the host name doesn't seem quite right.

@rohanKanojia @manusa what do you vote for expected behavior here?

[manusa]

The NO_PROXY env var support was added in #209 to fix an issue with Fabric8 Maven Plugin (fabric8io/fabric8#5178).

The spec was taken from http://www.gnu.org/software/wget/manual/html_node/Proxies.html

no_proxy
This variable should contain a comma-separated list of domain extensions proxy should not be used for. For instance, if the value of no_proxy is ‘.mit.edu’, proxy will not be used to retrieve documents from MIT.

However, the GNU emacs spec does support wildcards https://www.gnu.org/software/emacs/manual/html_node/url/Proxies.html

The NO_PROXY environment variable specifies URLs that should be excluded from proxying (on servers that should be contacted directly). This should be a comma-separated list of hostnames, domain names, or a mixture of both. Asterisks can be used as wildcards, but other clients may not support that.

The one for OpenShift doesn't accept wildcard characters either.

---

Regarding the current implementation, I think it's fine and I wouldn't overcomplicate it.

However, I think that the line

if (host.contains(noProxy)) {

should be changed to

if (host.endsWith(noProxy)) {

or even something a little bit more involved that actually checks for subdomains if expressed as .example.com or not if expressed as example.com

I would really appreciate an error msg

so in this case * would cause an exception.

Yes, this would improve the usability.