Allow to configure HttpClient used in ConsulClient

[urbanpawel90]

It would be nice to introduce some kind of configurer or configuration properties which will allow to configure Apache HttpClient used in consul-api.

PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(registry);
            connectionManager.setMaxTotal(DEFAULT_MAX_CONNECTIONS);
            connectionManager.setDefaultMaxPerRoute(DEFAULT_MAX_PER_ROUTE_CONNECTIONS);

            RequestConfig requestConfig = RequestConfig.custom().
                    setConnectTimeout(DEFAULT_CONNECTION_TIMEOUT).
                    setConnectionRequestTimeout(DEFAULT_CONNECTION_TIMEOUT).
                    setSocketTimeout(DEFAULT_READ_TIMEOUT).
                    build();

            HttpClientBuilder httpClientBuilder = HttpClientBuilder.create().
                    setConnectionManager(connectionManager).
                    setDefaultRequestConfig(requestConfig);

            this.httpClient = httpClientBuilder.build();

Above you can find a code snippet from DefaultHttpsTransport, the same you can find in DefaultHttpTransport. It would be nice to be able to configure timeouts and connection limits (maxTotal, maxPerRoute).

[spencergibb]

That's a good question. The second you want tls you no longer use HttpClient directly. You can define your own ConsulClient bean.

[urbanpawel90]

That's a good question. The second you want tls you no longer use HttpClient directly. You can define your own ConsulClient bean.

TLS is quite nice to configure in that matter. There's no problem with that. We faced a huge amount of connections between our application using spring-cloud-consul-discovery and Consul and I'm trying to reduce that. Those options will allow developers to control HTTP connection pool used to reach Consul.

Regarding self-created bean. That's right and for single module/project it's perfect, but... In our system we have our custom starter which includes consul-discovery and some auto configuration classes. I've tried to somehow override ConsulClient with @ConditionalOnMIssingBean annotation to allow developers in projects using our starter define their own ConsulClient if needed, but I always get ConsulClient auto-configured from ConsulAutoConfiguration from dependency. I tought that the best would be to resolve the issue at it's root and just add those properties as an configuration parameters to ConsulClient configuration :)

[spencergibb]

HttpClient and TLSConfig are mutually exclusive. If you have auto-configuration you need to use @AutoConfigureBefore(ConsulAutoConfiguration.class).

[urbanpawel90]

HttpClient and TLSConfig are mutually exclusive. If you have auto-configuration you need to use @AutoConfigureBefore(ConsulAutoConfiguration.class).

Ah, OK, now I get it. Yes, I've copied configuration code from DefaultHttpsTransport and used it to configure TLS in my auto-configuration. Unfortunately @AutoConfigureBefore didn't help :( Below you can find my auto-configuration class. It's also included in spring.factories file in resources.

import com.ecwid.consul.transport.TLSConfig;
import com.ecwid.consul.transport.TLSConfig.KeyStoreInstanceType;
import com.ecwid.consul.transport.TransportException;
import com.ecwid.consul.v1.ConsulClient;
import com.ecwid.consul.v1.ConsulRawClient;
import org.apache.http.client.HttpClient;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.config.Registry;
import org.apache.http.config.RegistryBuilder;
import org.apache.http.conn.socket.ConnectionSocketFactory;
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
import org.apache.http.conn.ssl.TrustSelfSignedStrategy;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.apache.http.ssl.SSLContexts;
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.cloud.client.ConditionalOnDiscoveryEnabled;
import org.springframework.cloud.consul.ConditionalOnConsulEnabled;
import org.springframework.cloud.consul.ConsulAutoConfiguration;
import org.springframework.cloud.consul.ConsulProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.util.StringUtils;

import java.io.FileInputStream;
import java.io.IOException;
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.SecureRandom;
import javax.net.ssl.*;

@Configuration
@ConditionalOnDiscoveryEnabled
@ConditionalOnConsulEnabled
@AutoConfigureBefore(ConsulAutoConfiguration.class)
public class ConsulClientAutoConfiguration {

  @Bean
  @ConditionalOnMissingBean
  public ConsulHttpPoolingProperties consulHttpPoolingProperties() {
    return new ConsulHttpPoolingProperties();
  }

  /**
   * Modified version of {@link org.springframework.cloud.consul.ConsulAutoConfiguration#consulClient(org.springframework.cloud.consul.ConsulProperties)}
   */
  @Bean
  @ConditionalOnMissingBean
  public ConsulClient consulClient(
      ConsulProperties consulProperties,
      ConsulHttpPoolingProperties poolingProperties) {
    try {
      final int agentPort = consulProperties.getPort();
      final String agentHost = !StringUtils.isEmpty(consulProperties.getScheme())
                               ? consulProperties.getScheme() + "://" + consulProperties.getHost()
                               : consulProperties.getHost();
      if (consulProperties.getTls() != null) {
        ConsulProperties.TLSConfig tls = consulProperties.getTls();
        TLSConfig tlsConfig = new TLSConfig(tls.getKeyStoreInstanceType(),
            tls.getCertificatePath(), tls.getCertificatePassword(),
            tls.getKeyStorePath(), tls.getKeyStorePassword());
        return new ConsulClient(new ConsulRawClient(
            createTlsClient(tlsConfig, poolingProperties), agentHost, agentPort, null)
        );
      }
      return new ConsulClient(new ConsulRawClient(
          createHttpClient(poolingProperties), agentHost, agentPort, null)
      );
    } catch (GeneralSecurityException | IOException e) {
      throw new TransportException(e);
    }
  }

  /**
   * Modified version of {@link com.ecwid.consul.transport.DefaultHttpsTransport#DefaultHttpsTransport(com.ecwid.consul.transport.TLSConfig)}
   */
  private HttpClient createTlsClient(
      TLSConfig tlsConfig, ConsulHttpPoolingProperties poolingProperties) throws
      GeneralSecurityException, IOException {
    KeyStore clientStore = KeyStore.getInstance(tlsConfig.getKeyStoreInstanceType().name());
    clientStore.load(
        new FileInputStream(tlsConfig.getCertificatePath()),
        tlsConfig.getCertificatePassword().toCharArray());

    KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
    kmf.init(clientStore, tlsConfig.getCertificatePassword().toCharArray());
    KeyManager[] kms = kmf.getKeyManagers();

    KeyStore trustStore = KeyStore.getInstance(KeyStoreInstanceType.JKS.name());
    trustStore.load(
        new FileInputStream(tlsConfig.getKeyStorePath()),
        tlsConfig.getKeyStorePassword().toCharArray());

    TrustManagerFactory tmf =
        TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
    tmf.init(trustStore);
    TrustManager[] tms = tmf.getTrustManagers();

    SSLContext
        sslContext = SSLContexts.custom().loadTrustMaterial(new TrustSelfSignedStrategy()).build();
    sslContext.init(kms, tms, new SecureRandom());
    SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(sslContext);

    Registry<ConnectionSocketFactory> registry = RegistryBuilder.<ConnectionSocketFactory>create()
        .register("https", factory).build();

    PoolingHttpClientConnectionManager connectionManager =
        new PoolingHttpClientConnectionManager(registry);
    connectionManager.setMaxTotal(poolingProperties.getMaxConnections());
    connectionManager.setDefaultMaxPerRoute(poolingProperties.getMaxPerRouteConnections());
    // Below was added extra, wasn't present in DefaultHttpsTransport
    connectionManager.setValidateAfterInactivity(
        poolingProperties.getValidateConnectionAfterInactivity());

    RequestConfig requestConfig = RequestConfig.custom().
        setConnectTimeout(poolingProperties.getConnectTimeout()).
        setConnectionRequestTimeout(poolingProperties.getConnectionRequestTimeout()).
        setSocketTimeout(poolingProperties.getSocketTimeout()).
        build();

    HttpClientBuilder httpClientBuilder = HttpClientBuilder.create().
        setConnectionManager(connectionManager).
        setDefaultRequestConfig(requestConfig);

    return httpClientBuilder.build();
  }

  private HttpClient createHttpClient(ConsulHttpPoolingProperties poolingProperties) {
    PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager();
    connectionManager.setMaxTotal(poolingProperties.getMaxConnections());
    connectionManager.setDefaultMaxPerRoute(poolingProperties.getMaxPerRouteConnections());
    // Below was added extra, wasn't present in DefaultHttpsTransport
    connectionManager.setValidateAfterInactivity(
        poolingProperties.getValidateConnectionAfterInactivity());

    RequestConfig requestConfig = RequestConfig.custom().
        setConnectTimeout(poolingProperties.getConnectTimeout()).
        setConnectionRequestTimeout(poolingProperties.getConnectionRequestTimeout()).
        setSocketTimeout(poolingProperties.getSocketTimeout()).
        build();

    HttpClientBuilder httpClientBuilder = HttpClientBuilder.create().
        setConnectionManager(connectionManager).
        setDefaultRequestConfig(requestConfig).
        useSystemProperties();

    return httpClientBuilder.build();
  }
}

[spencergibb]

There's no reason AutoConfigureBefore shouldn't work. Are you using consul config?

[urbanpawel90]

@spencergibb those are my dependencies in starter:

  compileOnly "org.springframework.cloud:spring-cloud-config-server:2.1.3.RELEASE"

  implementation "org.springframework.cloud:spring-cloud-starter-consul:2.1.2.RELEASE"
  implementation "org.springframework.retry:spring-retry:1.2.4.RELEASE"

and in sample project on which I work to test if the starter works I also have spring-cloud-starter-consul-config included. It might be the reason?

[spencergibb]

yes, once you have spring-cloud-starter-consul-config the consul client is created in bootstrap and auto configure before won't work. You'd have to setup a bootstrap configuration

[urbanpawel90]

@spencergibb thank you, I'll try it tomorrow and let you know. Before I'll do it I have one question: making a configuration as a bootstrap one will solve my problem either in projects having consul-config enabled and ones without it? Or should I somehow take care differently for each of them?

[spencergibb]

in the bootstrap configuration check for @ConditionalOnClass(ConsulConfigProperties.class) or something. then it won't run when not needed.

[urbanpawel90]

@spencergibb Introducing bootstrap configuration helped. Thank you a lot! :)