Skip to content

AutoConfiguredRestClientSsl overwrites configuration from HttpClientProperties #43618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kzander91 opened this issue Dec 26, 2024 · 3 comments
Closed
Labels
status: superseded An issue that has been superseded by another

Comments

@kzander91
Copy link
Contributor

kzander91 commented Dec 26, 2024

This is similar to #36594 and #27360 but I'm not sure if a solution for those issues would also fix mine, so feel free to close this if you consider this to be a duplicate.

Issue: Configuration made through the spring.http.client properties is ignored if I use AutoConfiguredRestClientSsl. For example, configuration like

spring:
  http:
    client:
      redirects: dont_follow

does not work if I also have this:

@Bean
RestClientCustomizer clientCertConfigurer(RestClientSsl restClientSsl) {
    return builder -> builder.apply(restClientSsl.fromBundle("my-bundle"));
}

The reason for this is that AutoConfiguredRestClientSsl replaces the request factory and thus discards any configuration made via the spring.http.client properties:

return (builder) -> {
ClientHttpRequestFactorySettings settings = ClientHttpRequestFactorySettings.ofSslBundle(bundle);
ClientHttpRequestFactory requestFactory = this.clientHttpRequestFactoryBuilder.build(settings);
builder.requestFactory(requestFactory);
};

For this specific issue I can work around this by removing my RestClientCustomizer and use

spring:
  http:
    client:
      redirects: dont_follow
      ssl:
        bundle: "my-bundle"

instead, but this only works in this static scenario, if I needed some more sophisticated logic to determine the SSL bundle to use at runtime, this wouldn't work.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 26, 2024
@wilkinsona
Copy link
Member

I think this is working as documented:

* </pre> NOTE: Apply SSL configuration will replace any previously
* {@link RestClient.Builder#requestFactory configured} {@link ClientHttpRequestFactory}.
* If you need to configure {@link ClientHttpRequestFactory} with more than just SSL
* consider using a {@link ClientHttpRequestFactorySettings} with
* {@link ClientHttpRequestFactories}.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Jan 10, 2025
@kzander91
Copy link
Contributor Author

This note and also the corresponding sentences in the reference docs don't really tell me that this affects the spring.http.client properties. Without studying the corresponding auto configuration code, how am I supposed to know that these properties work on their own instance of a ClientHttpRequestFactory that may then later be replaced if other configuration beans are defined?
Other features provided by Boot are additive, where I can use a mix of properties and customizer beans, but here for HTTP clients, I can't.

I'm aware that the Boot team is aware of these configuration limitations, and I'm seeing my issue here as another one of those. If you disagree, then I'd say that this is a documentation issue. Maybe you could consider adding warning boxes to the documentation sections where the HTTP properties and SSL configuration are described, warning users that the properties are ignored if certain other beans are defined.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 11, 2025
@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 13, 2025
@philwebb
Copy link
Member

Closing in favor of PR #44979, thanks @nosan!

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Apr 15, 2025
@philwebb philwebb added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided for: team-meeting An issue we'd like to discuss as a team to make progress labels Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

4 participants