Skip to content

Conversation

@vpavic
Copy link
Contributor

@vpavic vpavic commented Apr 16, 2020

At present, RestClientBuilderCustomizer allows general customization of RestClientBuilder. This is troublesome for users that want to customize HttpAsyncClientBuilder and RequestConfig.Builder since those are set on the RestClientBuilder. By customizing those two builders users lose out on Spring Boot's support for binding username, password, connection-timeout and read-timeout properties from spring.elasticsearch.rest namespace.

To illustrate the problem, attempting to customize aspects of HttpAsyncClientBuilder like this:

@Bean
public RestClientBuilderCustomizer restClientBuilderCustomizer() throws Exception {
    SSLContext sslContext; // configure custom sslContext
    SchemeIOSessionStrategy sessionStrategy; // configure custom sessionStrategy
    return builder -> builder.setHttpClientConfigCallback(
            httpClientBuilder -> httpClientBuilder.setSSLStrategy(sessionStrategy).setSSLContext(sslContext));
    });
}

Will disable the binding of username and password properties applied here:

CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
Credentials credentials = new UsernamePasswordCredentials(properties.getUsername(),
properties.getPassword());
credentialsProvider.setCredentials(AuthScope.ANY, credentials);
builder.setHttpClientConfigCallback(
(httpClientBuilder) -> httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider));

To work around this, users need to replicate Spring Boot's configuration logic using something like:

@Bean
public RestClientBuilderCustomizer restClientBuilderCustomizer(RestClientProperties properties) throws Exception {
    SSLContext sslContext; // configure custom sslContext
    SchemeIOSessionStrategy sessionStrategy; // configure custom sessionStrategy
    return builder -> builder.setHttpClientConfigCallback(httpClientBuilder -> {
        httpClientBuilder.setSSLStrategy(sessionStrategy).setSSLContext(sslContext);
        PropertyMapper.get().from(properties::getUsername).whenHasText().to(username -> {
            CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
            Credentials credentials = new UsernamePasswordCredentials(properties.getUsername(),
                    properties.getPassword());
            credentialsProvider.setCredentials(AuthScope.ANY, credentials);
            httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
        });
        return httpClientBuilder;
    });
}

This PR enhances the RestClientBuilderCustomizer with support for customizing HttpAsyncClientBuilder and RequestConfig.Builder by providing additional #customize methods that accept the aforementioned builders. Both new methods are optional as they have no-op default implementations.

At present, RestClientBuilderCustomizer allows general customization of RestClientBuilder. This is troublesome for users that want to customize HttpAsyncClientBuilder and RequestConfig.Builder since those are set on the RestClientBuilder. By customizing those two builders user lose out on Spring Boot's support for binding username, password, connection-timeout and read-timeout properties from spring.elasticsearch.rest namespace.

This commit enhances the RestClientBuilderCustomizer with support for customizing HttpAsyncClientBuilder and RequestConfig.Builder by providing additional #customize methods that accept the aforementioned builders. Both new methods are optional as they have no-op default implementations.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 16, 2020
@bclozel bclozel added this to the 2.3.0.RC1 milestone Apr 16, 2020
@bclozel bclozel added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 16, 2020
@bclozel bclozel self-assigned this Apr 16, 2020
@vpavic
Copy link
Contributor Author

vpavic commented Apr 17, 2020

Also, it might be a good idea to expand the javadoc of the original RestClientBuilderCustomizer#customize method (so, the one that takes RestClientBuilder argument) and state that using #setHttpClientConfigCallback and #setRequestConfigCallback on the builder will override the other customizer methods, respectively, as well as the default customizer provided by Spring Boot. WDYT @bclozel?

bclozel pushed a commit that referenced this pull request Apr 21, 2020
At present, RestClientBuilderCustomizer allows general customization of RestClientBuilder.
This is troublesome for users that want to customize `HttpAsyncClientBuilder` and
`RequestConfig.Builder` since those are set on the `RestClientBuilder`. By customizing
those two builders user lose out on Spring Boot's support for binding username, password,
connection-timeout and read-timeout properties from `"spring.elasticsearch.rest"` namespace.

This commit enhances the `RestClientBuilderCustomizer` with support for customizing
`HttpAsyncClientBuilder` and `RequestConfig.Builder` by providing additional `customize`
methods that accept the aforementioned builders. Both new methods are optional as they have
no-op default implementations.

See gh-20994
bclozel added a commit that referenced this pull request Apr 21, 2020
@bclozel bclozel closed this in 11c1980 Apr 21, 2020
@vpavic vpavic deleted the improve-es-config branch April 21, 2020 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants