[SERVER-156] No http client support for http_proxy_host, http_proxy_port, and configtimeout Created: 2014/11/11  Updated: 2019/03/25  Resolved: 2019/03/25

Status: Closed
Project: Puppet Server
Component/s: None
Affects Version/s: SERVER 0.4.0
Fix Version/s: SERVER 1.y, SERVER 2.y

Type: Bug Priority: Normal
Reporter: Jeremy Barlow Assignee: Unassigned
Resolution: Won't Do Votes: 1
Labels: http
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to PUP-3666 Replace 'configtimeout' with separate... Closed
relates to SERVER-377 "puppetserver gem" command doesn't wo... Closed
relates to SERVER-449 Expose http-client timeouts from Pupp... Closed
Template:
Epic Link: clj-http-client: the improvementing
Team: Froyo
QA Contact: Erik Dasher

 Description   

On a Ruby master, the "puppet.conf" settings for http_proxy_host, http_proxy_port, and configtimeout were honored for cases where the master made outgoing client requests, e.g., when submitting a report to an external server. See https://github.com/puppetlabs/puppet/blob/master/lib/puppet/network/http/factory.rb.

The Puppet Server master does not honor any of these settings for client connections that it makes. We should consider whether or not it is valuable to restore this capability.



 Comments   
Comment by Chris Price [ 2014/11/11 ]

FWIW the only time this would ever come up is if someone writes a report processor or custom function that chooses to use Puppet's HTTP client API in its implementation. This should generally be considered a worst practice except in the very rare case where you need to use Puppet's certs as part of your HTTPS authentication; in any other case it would make more sense to use some other ruby HTTP client library.

In the case where you do need the certs, going through a proxy is going to have added complexity in that a) you need to set the proxy server itself up to be compatible with Puppet's certs, and b) whatever you're proxying to probably can't be relying too much on the content of the client cert, because the client cert that the proxy uses is likely to be different from the one that the master used.

This is not to say that it's impossible that someone needs this functionality, it just strikes me as probably an extremely rare set of circumstances... but it's entirely possible that I'm wrong!

Comment by Jeremy Barlow [ 2014/11/11 ]

Good point on potentially affected uses. I had confirmed that these settings were functional in the context of the HTTP report processor. It appears that some other areas of Puppet code may be honoring these settings outside of this client API, e.g., the package type, although I didn't look very deep into that. In the latter case, I think Puppet Server may still end up honoring the settings since the HTTP client API back through Puppet Server wouldn't have been used.

Comment by Jeremy Barlow [ 2014/11/12 ]

Josh Cooper pointed out that there is a significant possibility that the configtimeout setting will be broken up into different settings with clearer meanings - e.g., a connection vs. a read timeout. See PUP-3666. If we decide we need to preserve this capability in Puppet Server, we may want to split the timeout settings off into a new Puppet Server ticket that tracks the core Ruby Puppet one.

Comment by Jeremy Barlow [ 2014/11/14 ]

The https://docs.puppetlabs.com/references/latest/configuration.html documents a couple of other proxy settings – http_proxy_user and http_proxy_password. With a 3.7.3-based Ruby Puppet master, I tried hooking up an HTTP report processor, entering values for the http_proxy_user and http_proxy_password settings in the master's puppet.conf file, and having the target host return a 407 proxy authentication challenge. The master failed the connection without providing a Proxy-Authorization header in either the initial request or in response to the challenge from the "proxy". Also, it appears that the code in https://github.com/puppetlabs/puppet/blob/master/lib/puppet/network/http/factory.rb doesn't specifically look at these settings.

I believe, then, that the Ruby Puppet master does not support these settings in the context of an Puppet::Network::HttpPool request. It may still make sense to support these for Puppet Server but it isn't clear that there is a backward compatibility requirement to do so.

Comment by Jeremy Barlow [ 2015/04/14 ]

Chris Price If we end up plumbing through new settings to the http_client TK section and using those to back puppetserver CLI subcommands, e.g., gem per SERVER-377, I think it might make sense to do this in the production code at the same time - meaning we could pull this into the Server 1.1/2.2 epic as well?

Comment by Chris Price [ 2015/04/15 ]

I agree - whenever we tackle anything related to proxy settings for the CLI subcommands we should make sure it works for the server process as well.

Comment by Jeremy Barlow [ 2015/04/16 ]

This ticket originally referred to the configtimeout setting as not being configurable. We ended up addressing that in SERVER-449. So the remaining work on this ticket would be about the proxy settings only.

Comment by Jeremy Barlow [ 2015/07/16 ]

In SERVER-377, we're discussing a possible addition to the http-client section of the Puppet Server Trapperkeeper configuration to allow for user specification of a proxy. That configuration might look like:

http-client: {
  proxy: {
    host: my-proxy-host
    port: 8088
    credentials: {
      user: proxyuser
      password: proxypassword
    }
    no-proxy-hosts: { “*.mydomain.com”, “*.myotherdomain.com” }
  }
}

I spent some time looking at the clj-http-client interaction with the Apache HTTP client library and how we might be able to map such a proxy configuration into the client setup. The code snippets below could all be applied to the JavaClient.createClient() method in that library.

Enabling the selection of a specific proxy is fairly trivial. For example, the following line would allow for a proxy server with hostname of "127.0.0.1" and port of "8088" to be used:

clientBuilder.setProxy(new HttpHost("my-proxy-host", 8088));

With respect to specifying the credentials to use for the proxy, it appears that the best approach is to setup a CredentialsProvider on the client instance and add the credentials for the proxy server. The following code could be used to do that:

BasicCredentialsProvider credsProvider = new BasicCredentialsProvider();
credsProvider.setCredentials(new AuthScope("my-proxy-host", 8088),
  new UsernamePasswordCredentials("proxyuser", "proxypassword"));
clientBuilder.setDefaultCredentialsProvider(credsProvider);

I was not, however, able to find as straightforward of way to map the no-proxy-hosts into the client. The most tractable way I found was through the registration of a RoutePlanner with its own custom ProxySelector - like what is described here, http://stackoverflow.com/questions/10297837/set-nonproxyhosts-in-apache-httpclient-4-1-3. In SERVER-377, we're also talking about establishing a priority mechanism for selecting proxy settings which would look to an environment variable first, e.g., one named http_proxy, falling back to a configuration setting, e.g. the http-client.proxy.host setting in the configuration file, if no environment variable is found. The built-in ProxySelector which is used by default in Apache HTTP client connections considers only the Java system properties for proxy settings, e.g., one set through -Dhttp.proxyHost via the Java command line, and not any *_proxy environment variables. Given that, I'm thinking that the custom ProxySelector is probably what we would want to try to use for custom proxy selection.

For example, consider the following trivial example:

String host = "127.0.0.1";
int port = 8083;
String noProxyHosts = "puppet-master";
 
try {
  URI proxyUri = new URI(System.getenv("http_proxy"));
  host = proxyUri.getHost();
  port = proxyUri.getPort();
}
catch (Exception e) {}
 
final String proxyHost = host
final int proxyPort = port;
 
SchemePortResolver schemePortResolver = DefaultSchemePortResolver.INSTANCE;
  clientBuilder.setSchemePortResolver(schemePortResolver);
 
clientBuilder.setRoutePlanner(
  new SystemDefaultRoutePlanner(schemePortResolver,
    new ProxySelector() {
      @Override
      public List<Proxy> select(final URI uri) {
        List<Proxy> proxyList = new ArrayList<>();
        if (uri.getHost().equals("puppet-master"))
          proxyList.add(Proxy.NO_PROXY);
        else if (!uri.getScheme().equals("http"))
          proxyList = ProxySelector.getDefault().select(uri);
        else {
          proxyList.add(new Proxy(Proxy.Type.HTTP, new InetSocketAddress(proxyHost, proxyPort)));
        }
        return proxyList;
      }
 
      @Override
      public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {}}));

The ProxySelector.select() method is called for the target uri to which the HTTP client would try to connect. Proxy.NO_PROXY could be returned if that URI matches one of the expressions in a nonProxyHosts list. Further distinctions could be made later based on the uri scheme, e.g., if we wanted to support http vs. https distinctions. For any cases that a proxy should be used, the ProxySelector could make the proxy host/port selection based on whatever source data it deems most appropriate - potentially giving precedence to an environment variable over a supplied setting.

Ideally, it would be nice if the evaluation of a target host against the nonProxyHosts list could reuse the logic which exists in the default ProxySelector which the Apache HTTP client uses. That is done in the sun.net.spi.DefaultProxySelector class, however, and is embedded right in the select method implementation. See http://www.docjar.com/html/api/sun/net/spi/DefaultProxySelector.java.html. The DefaultProxySelector implementation also assumed that the source of the nonProxyHosts is a Java system property. Assuming we would want to have the ability to pull the nonProxyHosts from an alternate source anyway though - e.g., from the no_proxy environment variable (which uses a comma delimiter instead of a pipe delimiter like the Java *_.nonProxyHosts properties do) or from an option list in the HTTP client options configuration - it may not be that bad to have to "reimplement" some of the proxy matching in a custom ProxySelector.

Note that if we went with the RoutePlanner approach in all cases, the selection of a discrete proxy via clientBuilder.setProxy would be irrelevant. A custom RoutePlanner supersedes a custom proxy / HttpHost. See https://github.com/apache/httpasyncclient/blob/4.0.x/httpasyncclient/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java#L723-L733.

Comment by Jeremy Barlow [ 2015/07/16 ]

With respect to clj-http-client one question which has come up is why do the Java proxy properties – e.g., ones you'd set on the Java command line like with -Dhttp.proxyHost=127.0.0.1 -Dhttp.proxyPort=8080 -Dhttp.nonProxyHosts=*.myserver.com – not appear to honored with the implementation in clj-http-client as it exists today. As it turns out, there would be a way to enable the use of these. This could be done by just adding the following line to the Java.createClient() call:

clientBuilder.useSystemProperties();

If we end up going with the custom ProxySelector approach anyway, though, this property would effectively be irrelevant in this case since we'd have to duplicate the functionality to honor the system properties in the custom ProxySelector implementation anyway.

Comment by Jeremy Barlow [ 2015/07/16 ]

Along the lines of SERVER-377, I'm inclined to go with a priority order (highest at the top, lowest at the bottom) of the following around the possible sources in which each proxy setting is derived:

  • Environment variable
  • Trapperkeeper http-client configuration
  • Java system property

Assuming this is the approach we'd want to go with, we'd also need to think about where the priority determination is done. For most general reuse, this could just be done all within the clj-http-client library so it could be generically reused by any clients - substituting "HTTP client option" for "Trapperkeeper http-client configuration". There may be some consternation about that, though, if it is perceived that the "environment variable first" approach, while it may be suitable for Puppet Server, isn't as intuitive from the perspective of HTTP clients in general.

A more flexible (and possibly more intuitive) approach might involve having Puppet Server handle massaging the "Environment variable" vs. "Trapperkeeper http-client configuration" part into an "HTTP client option" that it passes into the clj-http-client library and then have the http-client library prioritize an "HTTP client option" ahead of a "Java system property". That's probably the way I'd lean at this point. Would appreciate feedback from others on that part, though.

Chris Price - WDYT?

Generated at Fri Jan 24 23:25:10 PST 2020 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.