Skip to content

Provide a configuration property for Elasticsearch path prefix #25010

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
TarekSaid opened this issue Jan 27, 2021 · 7 comments
Closed

Provide a configuration property for Elasticsearch path prefix #25010

TarekSaid opened this issue Jan 27, 2021 · 7 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@TarekSaid
Copy link

TarekSaid commented Jan 27, 2021

As of Spring Boot 2.4.2, with spring-data if the ES server is behind a reverse proxy with a prefix, there's no way to use Spring Boot's autoconfiguration for spring.elasticsearch.rest or spring.data.elasticsearch.client.reactive properties. One has to use the AbstractElasticsearchConfiguration in order to use the ClientConfiguration.builder().withPathPrefix(prefix). The horror.

Ideally, we could:

  • directly specify the path-prefix in the spring.elasticsearch.rest.uris property (useful in case there are multiple urls, each with its own path prefix). I don't know how that would work with spring.data.elasticsearch.client.reactive.endpoints, as it only uses a host:port format.
  • specify the path-prefix in a spring.elasticsearch.rest.path-prefix or spring.data.elasticsearch.client.reactive.path-prefix property. In that case, it would behave like the withPathPrefix(String) method, applying the prefix to all the uris/endpoints in the list
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 27, 2021
@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 27, 2021
@wilkinsona
Copy link
Member

If we need a new property, we need to consider #23106.

@philwebb philwebb added type: enhancement A general enhancement status: pending-design-work Needs design work before any code can be developed and removed for: team-meeting An issue we'd like to discuss as a team to make progress status: waiting-for-triage An issue we've not yet triaged labels Feb 3, 2021
@philwebb philwebb added this to the 2.5.x milestone Feb 3, 2021
@philwebb
Copy link
Member

philwebb commented Feb 3, 2021

We'll either add a property when we look at #23106 or we might consider adding a Customizer callback interface.

@philwebb philwebb added the status: blocked An issue that's blocked on an external project change label Mar 8, 2021
@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.6.x Apr 13, 2021
@philwebb philwebb changed the title Elasticsearch Autoconfiguration for ES servers behind reverse proxy Elasticsearch auto-configuration for ES servers behind reverse proxy Jun 14, 2021
@wilkinsona
Copy link
Member

On the imperative side, the path prefix can be configured using a RestClientBuilderCustomizer bean. On the reactive side there's no equivalent customizer. For symmetry, it feels to me like we should add one regardless of whether or not we add a new path-prefix property.

@wilkinsona
Copy link
Member

wilkinsona commented Sep 16, 2021

directly specify the path-prefix in the spring.elasticsearch.rest.uris property (useful in case there are multiple urls, each with its own path prefix)

Elasticsearch's REST client does not appear to support this. The URIs are used to create Node or HttpHost instances, neither of which can convey a path prefix. In other words, the path prefix has to be the same across every node with which the client will communicate. Given this, I think that the only configuration property-based option that makes sense is to provide a new path-prefix property. This will work for both clients.

@TarekSaid
Copy link
Author

I think that the only configuration property-based option that makes sense is to provide a new path-prefix property

Agreed. The only reason why I proposed that first solution was because in our application-dev.yml I was pointing either to a local ES instance (without path prefix) or to a test ES instance (with path prefix), but that's too much of a niche problem (and a non-standard one at that, I've created a separate test profile with its application-test.yml since), a simple path-prefix property will work.

@AlexeiZenin
Copy link

We'll either add a property when we look at #23106 or we might consider adding a Customizer callback interface.

Customizer would be great. Currently just trying to add some request interceptors to do some signing (needed for AWS Elasticsearch Service). Currently hairy to extend the AbstractReactiveElasticsearchConfiguration class and then have to call the public method of ReactiveElasticsearchRestClientAutoConfiguration to get the standard setup to still run without copy-pasting.

@wilkinsona
Copy link
Member

@AlexeiZenin Please open a separate issue for providing a customizer on the reactive side that shows your current approach. I'd like to see it to understand if a callback that is passed the TerminalClientConfigurationBuilder meets your needs.

@wilkinsona wilkinsona self-assigned this Sep 17, 2021
@wilkinsona wilkinsona removed status: blocked An issue that's blocked on an external project change status: pending-design-work Needs design work before any code can be developed labels Sep 17, 2021
@wilkinsona wilkinsona changed the title Elasticsearch auto-configuration for ES servers behind reverse proxy Provide a configuration property for Elasticsearch path prefix Sep 17, 2021
@wilkinsona wilkinsona modified the milestones: 2.6.x, 2.6.0-M3 Sep 20, 2021
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

No branches or pull requests

5 participants